Date: Thu, 19 Jan 2023 15:50:06 -0800 From: John Baldwin <jhb@FreeBSD.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 43703bc489ec - main - stdlib.h: Fix qsort_r compatibility with GCC 12. Message-ID: <343ea0b5-28fa-efa6-b3d3-4126570e40f2@FreeBSD.org> In-Reply-To: <0F50986C-BC29-4A7A-89F0-DAB3863BF51B@freebsd.org> References: <202301192249.30JMnCXf040410@gitrepo.freebsd.org> <F42A1D72-387E-4F3F-8C27-C95C5A244E1D@freebsd.org> <8C902ED3-D9F8-4F88-8D43-F8E4809C9D44@freebsd.org> <0F50986C-BC29-4A7A-89F0-DAB3863BF51B@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/19/23 3:36 PM, Jessica Clarke wrote: > On 19 Jan 2023, at 23:31, Jessica Clarke <jrtc27@FreeBSD.org> wrote: >> >> On 19 Jan 2023, at 23:11, Jessica Clarke <jrtc27@FreeBSD.org> wrote: >>> >>> On 19 Jan 2023, at 22:49, John Baldwin <jhb@FreeBSD.org> wrote: >>>> >>>> The branch main has been updated by jhb: >>>> >>>> URL: https://cgit.FreeBSD.org/src/commit/?id=43703bc489ec504b947b869045c492ed38c1a69c >>>> >>>> commit 43703bc489ec504b947b869045c492ed38c1a69c >>>> Author: John Baldwin <jhb@FreeBSD.org> >>>> AuthorDate: 2023-01-19 22:48:52 +0000 >>>> Commit: John Baldwin <jhb@FreeBSD.org> >>>> CommitDate: 2023-01-19 22:48:52 +0000 >>>> >>>> stdlib.h: Fix qsort_r compatibility with GCC 12. >>>> >>>> GCC 12 (unlike GCC 9) does not match a function argument passed to the >>>> old qsort_r() API (as is used in the qsort_r_compat test) to a >>>> function pointer type via __generic. It treats the function type as a >>>> distinct type from a function pointer. As a workaround, add a second >>>> definition of qsort_r for GCC 12 which uses the bare function type. >>> >>> As far as I can tell both versions of GCC behave the same. The >>> difference is whether __generic is using _Generic or >>> __builtin_choose_expr with __builtin_types_compatible_p, since function >>> and function pointer types are not compatible. Clang will take the >>> __has_extension path, but GCC will take the builtins path, and so Clang >>> works but GCC doesn’t. >>> >>> As a result of this change you’ve likely broken code that does >>> qsort_r(..., &f) as that will have the opposite problem. The right fix >>> is to force arg5 to decay, such as by (ab)using the comma operator with >>> __generic((0, arg5), ...). I guess that probably belongs in the >>> fallback implementation of __generic though, not here, which would give >>> the following real fix: >>> >>> diff --git a/sys/sys/cdefs.h b/sys/sys/cdefs.h >>> index 83ba7584e5b9..f7eff4768151 100644 >>> --- a/sys/sys/cdefs.h >>> +++ b/sys/sys/cdefs.h >>> @@ -312,6 +312,9 @@ >>> * __generic(). Unlike _Generic(), this macro can only distinguish >>> * between a single type, so it requires nested invocations to >>> * distinguish multiple cases. >>> + * >>> + * Note that the comma operator is used to force expr to decay in order to >>> + * match _Generic. >>> */ >>> >>> #if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) || \ >>> @@ -321,7 +324,7 @@ >>> #elif __GNUC_PREREQ__(3, 1) && !defined(__cplusplus) >>> #define __generic(expr, t, yes, no) \ >>> __builtin_choose_expr( \ >>> - __builtin_types_compatible_p(__typeof(expr), t), yes, no) >>> + __builtin_types_compatible_p(__typeof((0, expr)), t), yes, no) >> >> With (expr) instead of expr, of course... > > And as for why GCC 9 works: > > It doesn’t. The tests just aren’t built because MK_CXX=no disables > MK_TESTS. GCC 12 only hits it because it’s new enough to be able to > build libc++ and not force MK_CXX=no. > > It would be nice to unpick that... Hmm, ok I thought I had managed to build this test with GCC 9 previously, but perhaps not. I will give the cdefs.h change a go locally though and see what happens. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?343ea0b5-28fa-efa6-b3d3-4126570e40f2>