Attachment 242064 Details for Bug 271234 – devel/libgit2: fix build with clang 16

[patch] devel/libgit2: fix build with clang 16

devel__libgit2-fix-clang16-build-1.diff (text/plain), 10.09 KB, created by Dimitry Andric on 2023-05-08 19:04:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Dimitry Andric

Created: 2023-05-08 19:04:57 UTC

Size: 10.09 KB

patch

obsolete

>commit e44803f8d2be354caae852ee91233561e9f47735
>Author: Dimitry Andric <dim@FreeBSD.org>
>Date:   2023-05-08 20:56:14 +0200
>
>    devel/libgit2: fix build with clang 16
>    
>    Clang 16 has a new error about incompatible function types, which shows
>    up when building devel/libgit2:
>    
>      /wrkdirs/usr/ports/devel/libgit2/work/libgit2-1.5.2/src/util/util.c:731:28: error: incompatible function pointer types passing 'int (void *, const void *, const void *)' to parameter of type 'int (*)(const void *, const void *, void *)' [-Wincompatible-function-pointer-types]
>              qsort_s(els, nel, elsize, git__qsort_r_glue_cmp, &glue);
>                                        ^~~~~~~~~~~~~~~~~~~~~
>      /usr/include/stdlib.h:397:11: note: passing argument to parameter here
>          int (*)(const void *, const void *, void *), void *);
>                ^
>    
>    Clang is indeed right, as the version of qsort_s(3) in FreeBSD 13 and
>    later has the 'void *payload' parameter last:
>    
>      errno_t  qsort_s(void *, rsize_t, rsize_t,
>          int (*)(const void *, const void *, void *), void *);
>    
>    This could be fixed by putting the arguments in the right place for
>    qsort_s(3), but it turns out the rabbit hole goes a bit deeper: on
>    14-CURRENT, libgit2's CMake configuration is not able to detect
>    qsort_r(3), which is actually why it chooses qsort_s():
>    
>      -- Checking prototype qsort_r for GIT_QSORT_R_BSD
>      -- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
>      -- Checking prototype qsort_r for GIT_QSORT_R_GNU
>      -- Checking prototype qsort_r for GIT_QSORT_R_GNU - False
>      -- Looking for qsort_s
>      -- Looking for qsort_s - found
>    
>    The problem with the GIT_QSORT_R_BSD detection is due to the check in
>    libgit2's src/CMakeLists.txt, where it does:
>    
>      check_prototype_definition(qsort_r
>              "void qsort_r(void *base, size_t nmemb, size_t size, void
>              *thunk, int (*compar)(void *, const void *, const void *))"
>              "" "stdlib.h" GIT_QSORT_R_BSD)
>    
>    and CMake attempts to define a function with a similar prototype in its
>    test program, which then fails to compile, at least on 14-CURRENT:
>    
>      /wrkdirs/share/dim/ports/devel/libgit2/work/.build/CMakeFiles/CMakeScratch/TryCompile-tILE28/CheckPrototypeDefinition.c:14:6:
>      error: expected identifier or '('
>      void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int
>      (*compar)(void *, const void *, const void *)) {
>           ^
>      /usr/include/stdlib.h:357:5: note: expanded from macro 'qsort_r'
>          __generic(arg5, int (*)(void *, const void *, const void *),
>           \\
>          ^
>      /usr/include/sys/cdefs.h:323:2: note: expanded from macro '__generic'
>              _Generic(expr, t: yes, default: no)
>              ^
>    
>    This is because in https://cgit.freebsd.org/src/commit/?id=af3c78886fd8d
>    Ed Schouten changed the prototype of qsort_r(3) to match POSIX, using a
>    C11 _Generic macro. When CMake tries to compile its own custom
>    definition of qsort_r, that fails with the above compile error, because
>    the macro gets expanded in place of the function declaration.
>    
>    So the summarized situation is:
>    
>    * On 12.x and 13.x, qsort_r(3) is a plain function, and uses the old
>      comparison function type: 'int (*compar)(void *thunk, const void
>      *elem1, const void *elem2)'.
>      Therefore, CMake detects GIT_QSORT_R_BSD, and libgit2's
>      src/util/util.c uses the correct comparison function type.
>    * On 14.x, qsort_r(3) is a macro, and uses the POSIX comparison function
>      type: 'int (*compar)(const void *elem1, const void *elem2, void *thunk)'.
>      Therefore, CMake fails to detect GIT_QSORT_R_BSD, and detects
>      GIT_QSORT_S instead, and libgit2's src/util/util.c uses an incorrect
>      comparison function type.
>    
>    I submitted https://github.com/libgit2/libgit2/pull/6555 upstream to
>    remedy the situation and correctly detect all qsort variants.
>    
>    This is an adjust version that applies on top of libgit-1.5.2.
>    
>    PR:             271234
>
>diff --git a/devel/libgit2/files/patch-github-pr6555 b/devel/libgit2/files/patch-github-pr6555
>new file mode 100644
>index 000000000000..9b456a27d55f
>--- /dev/null
>+++ b/devel/libgit2/files/patch-github-pr6555
>@@ -0,0 +1,148 @@
>+commit d873966fdeae7f0ac974318ce21772af555f1660
>+Author: Dimitry Andric <dimitry@andric.com>
>+Date:   2023-05-03 22:47:12 +0200
>+
>+    util: detect all possible qsort_r and qsort_s variants
>+    
>+    As reported in https://bugs.freebsd.org/271234, recent versions of
>+    FreeBSD have adjusted the prototype for qsort_r() to match the POSIX
>+    interface. This causes libgit2's CMake configuration check to fail to
>+    detect qsort_r(), making it fall back to qsort_s(), which in libgit2
>+    also has an incompatible interface. With recent versions of clang this
>+    results in a "incompatible function pointer types" compile error.
>+    
>+    Summarizing, there are four variations of 'qsort-with-context':
>+    * old style BSD qsort_r(), used in FreeBSD 13 and earlier, where the
>+      comparison function has the context parameter first
>+    * GNU or POSIX qsort_r(), also used in FreeBSD 14 and later, where the
>+      comparison function has the context parameter last
>+    * C11 qsort_s(), where the comparison function has the context parameter
>+      last
>+    * Microsoft qsort_s(), where the comparison function has the context
>+      parameter first
>+    
>+    Add explicit detections for all these variants, so they get detected as
>+    (in the same order as above):
>+    
>+    * `GIT_QSORT_R_BSD`
>+    * `GIT_QSORT_R_GNU`
>+    * `GIT_QSORT_S_C11`
>+    * `GIT_QSORT_S_MSC`
>+    
>+    An additional complication is that on FreeBSD 14 and later, <stdlib.h>
>+    uses the C11 _Generic() macro mechanism to automatically select the
>+    correct qsort_r() prototype, depending on the caller's comparison
>+    function argument. This breaks CMake's check_prototype_definition()
>+    functionality, since it tries to redefine the function, and _Generic
>+    macro is expanded inline causing a compile error.
>+    
>+    Work around that problem by putting the function names in parentheses,
>+    to prevent the preprocessor from using a macro to replace the function
>+    name.
>+    
>+    Also, in `git__qsort_r()`, change the `#if` order so the variants that
>+    do not have to use glue are preferred.
>+
>+diff --git src/CMakeLists.txt src/CMakeLists.txt
>+index cc0a0d4dc..de591e4e4 100644
>+--- src/CMakeLists.txt
>++++ src/CMakeLists.txt
>+@@ -58,15 +58,29 @@ add_feature_info(futimens GIT_USE_FUTIMENS "futimens support")
>+ 
>+ # qsort
>+ 
>++# old-style FreeBSD qsort_r() has the 'context' parameter as the first argument
>++# of the comparison function:
>+ check_prototype_definition(qsort_r
>+-	"void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int (*compar)(void *, const void *, const void *))"
>++	"void (qsort_r)(void *base, size_t nmemb, size_t size, void *context, int (*compar)(void *, const void *, const void *))"
>+ 	"" "stdlib.h" GIT_QSORT_R_BSD)
>+ 
>++# GNU or POSIX qsort_r() has the 'context' parameter as the last argument of the
>++# comparison function:
>+ check_prototype_definition(qsort_r
>+-	"void qsort_r(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *arg)"
>++	"void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context)"
>+ 	"" "stdlib.h" GIT_QSORT_R_GNU)
>+ 
>+-check_function_exists(qsort_s GIT_QSORT_S)
>++# C11 qsort_s() has the 'context' parameter as the last argument of the
>++# comparison function, and returns an error status:
>++check_prototype_definition(qsort_s
>++	"errno_t (qsort_s)(void *base, rsize_t nmemb, rsize_t size, int (*compar)(const void *, const void *, void *), void *context)"
>++	"0" "stdlib.h" GIT_QSORT_S_C11)
>++
>++# MSC qsort_s() has the 'context' parameter as the first argument of the
>++# comparison function, and as the last argument of qsort_s():
>++check_prototype_definition(qsort_s
>++	"void (qsort_s)(void *base, size_t num, size_t width, int (*compare )(void *, const void *, const void *), void *context)"
>++	"" "stdlib.h" GIT_QSORT_S_MSC)
>+ 
>+ # random / entropy data
>+ 
>+diff --git src/features.h.in src/features.h.in
>+index 1575be641..5f606df84 100644
>+--- src/features.h.in
>++++ src/features.h.in
>+@@ -26,7 +26,8 @@
>+ 
>+ #cmakedefine GIT_QSORT_R_BSD
>+ #cmakedefine GIT_QSORT_R_GNU
>+-#cmakedefine GIT_QSORT_S
>++#cmakedefine GIT_QSORT_S_C11
>++#cmakedefine GIT_QSORT_S_MSC
>+ 
>+ #cmakedefine GIT_SSH 1
>+ #cmakedefine GIT_SSH_MEMORY_CREDENTIALS 1
>+diff --git src/util/util.c src/util/util.c
>+index 9c9f2c040..f4eaf5835 100644
>+--- src/util/util.c
>++++ src/util/util.c
>+@@ -18,7 +18,7 @@
>+ # endif
>+ # include <windows.h>
>+ 
>+-# ifdef GIT_QSORT_S
>++# ifdef GIT_QSORT_S_MSC
>+ #  include <search.h>
>+ # endif
>+ #endif
>+@@ -673,7 +673,7 @@ size_t git__unescape(char *str)
>+ 	return (pos - str);
>+ }
>+ 
>+-#if defined(GIT_QSORT_S) || defined(GIT_QSORT_R_BSD)
>++#if defined(GIT_QSORT_S_MSC) || defined(GIT_QSORT_R_BSD)
>+ typedef struct {
>+ 	git__sort_r_cmp cmp;
>+ 	void *payload;
>+@@ -690,7 +690,8 @@ static int GIT_LIBGIT2_CALL git__qsort_r_glue_cmp(
>+ 
>+ #if !defined(GIT_QSORT_R_BSD) && \
>+ 	!defined(GIT_QSORT_R_GNU) && \
>+-	!defined(GIT_QSORT_S)
>++	!defined(GIT_QSORT_S_C11) && \
>++	!defined(GIT_QSORT_S_MSC)
>+ static void swap(uint8_t *a, uint8_t *b, size_t elsize)
>+ {
>+ 	char tmp[256];
>+@@ -721,12 +722,14 @@ static void insertsort(
>+ void git__qsort_r(
>+ 	void *els, size_t nel, size_t elsize, git__sort_r_cmp cmp, void *payload)
>+ {
>+-#if defined(GIT_QSORT_R_BSD)
>+-	git__qsort_r_glue glue = { cmp, payload };
>+-	qsort_r(els, nel, elsize, &glue, git__qsort_r_glue_cmp);
>+-#elif defined(GIT_QSORT_R_GNU)
>++#if defined(GIT_QSORT_R_GNU)
>+ 	qsort_r(els, nel, elsize, cmp, payload);
>+-#elif defined(GIT_QSORT_S)
>++#elif defined(GIT_QSORT_S_C11)
>++	qsort_s(els, nel, elsize, cmp, payload);
>++#elif defined(GIT_QSORT_R_BSD)
>++	git__qsort_r_glue glue = { cmp, payload };
>++	qsort_r(els, nel, elsize, &glue, git__qsort_r_glue_cmp);
>++#elif defined(GIT_QSORT_S_MSC)
>+ 	git__qsort_r_glue glue = { cmp, payload };
>+ 	qsort_s(els, nel, elsize, git__qsort_r_glue_cmp, &glue);
>+ #else

Actions: View | Diff

Attachments on bug 271234: 242064 | 242162