Page MenuHomeFreeBSD
Differential D52933

cross-build: Define __GNUC_PREREQ__ in cdefs.h
ClosedPublic
Actions

Authored by markj on Mon, Oct 6, 2:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 7, 4:15 PM
Unknown Object (File)
Tue, Oct 7, 2:52 AM
Unknown Object (File)
Mon, Oct 6, 6:50 PM
Unknown Object (File)
Mon, Oct 6, 5:57 PM
Unknown Object (File)
Mon, Oct 6, 5:03 PM
Unknown Object (File)
Mon, Oct 6, 5:02 PM
Unknown Object (File)
Mon, Oct 6, 4:45 PM
Subscribers
brooks
imp
jlduran

Details

Reviewers
emaste
jrtc27
arichardson
imp
Commits
rG53ab85a55408: cross-build: Define __GNUC_PREREQ__ in cdefs.h
Summary

This is required when including stdckdint.h and doesn't seem to be
provided by older clang.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 67611
Build 64494: arc lint + arc unit

Event Timeline

markj created this revision.Mon, Oct 6, 2:09 PM
Herald added a subscriber: imp. · View Herald TranscriptMon, Oct 6, 2:09 PM
markj requested review of this revision.Mon, Oct 6, 2:09 PM
Harbormaster completed remote builds in B67587: Diff 163630.Mon, Oct 6, 2:09 PM
markj added a parent revision: D52932: tools/build: Bring in stdckdint.h if needed.Mon, Oct 6, 2:09 PM
jlduran added a subscriber: jlduran.Mon, Oct 6, 3:29 PM

Thank you, these two commits fix GitHub's cross-builds on Linux.

emaste added a comment.Mon, Oct 6, 3:47 PM

Seems OK, although I wonder if __has_builtin(__builtin_add_overflow) will be true on GCC versions that are still relevant (and we could remove the __GNUC_PREREQ__ from stdckdint.h

markj added a comment.Tue, Oct 7, 1:59 PM
In D52933#1209484, @emaste wrote:

Seems OK, although I wonder if __has_builtin(__builtin_add_overflow) will be true on GCC versions that are still relevant (and we could remove the __GNUC_PREREQ__ from stdckdint.h

Apparently __has_builtin() itself only came in gcc 10, which is not that old really. Maybe not very relevant to us, but the header is likely used outside of FreeBSD. So I'd slightly prefer to fix this on the cross-build side.

imp requested changes to this revision.EditedTue, Oct 7, 2:22 PM

I know it's just for cross compiling, but maybe we can use the version we have in cdefs.h to avoid other foot-cannons in the future? The only reason this works is the || <something that works> in the instant use case.

/*
 * Macro to test if we're using a specific version of gcc or later.
 */
#if defined(__GNUC__)
#define __GNUC_PREREQ__(ma, mi) \
        (__GNUC__ > (ma) || __GNUC__ == (ma) && __GNUC_MINOR__ >= (mi))
#else
#define __GNUC_PREREQ__(ma, mi) 0
#endif

clang defines GNUC these days too...

This revision now requires changes to proceed.Tue, Oct 7, 2:22 PM
markj updated this revision to Diff 163696.Tue, Oct 7, 2:55 PM

Apply Warner's suggestion.

Harbormaster completed remote builds in B67611: Diff 163696.Tue, Oct 7, 2:55 PM
emaste accepted this revision.Tue, Oct 7, 3:02 PM
This revision was not accepted when it landed; it landed in state Needs Review.Wed, Oct 8, 1:02 PM
Closed by commit rG53ab85a55408: cross-build: Define __GNUC_PREREQ__ in cdefs.h (authored by markj). · Explain Why
This revision was automatically updated to reflect the committed changes.
markj added a commit: rG53ab85a55408: cross-build: Define __GNUC_PREREQ__ in cdefs.h.
brooks added a subscriber: brooks.Wed, Oct 8, 1:34 PM

Could we not just swap the order of the checks in stdckdint.h so clang never evaluates __GNUC_PREREQ__? We're already requiring at least an always-false __has_builtin in the next section in cdefs.h.

That being said I don't think we care about except in the most abstract sense about gcc 9 support for a C23 header.

markj added a comment.Wed, Oct 8, 2:24 PM
In D52933#1210459, @brooks wrote:

Could we not just swap the order of the checks in stdckdint.h so clang never evaluates __GNUC_PREREQ__? We're already requiring at least an always-false __has_builtin in the next section in cdefs.h.

That being said I don't think we care about except in the most abstract sense about gcc 9 support for a C23 header.

That would probably work too, assuming that all the versions of clang we care about have __has_builtin as well. At a glance that seems to be the case.

What's wrong with simply redefining __GNUC_PREREQ__ here though?

brooks added a comment.Wed, Oct 8, 3:30 PM
In D52933#1210494, @markj wrote:

That would probably work too, assuming that all the versions of clang we care about have __has_builtin as well. At a glance that seems to be the case.

Certainly. I don't think we care about things before 15, probably closer to 17. I wasn't able to determine when __has_builtin was added, but there was an issue with non-function-like builtins fixed in 10 and these are function-like so were fine even then.

What's wrong with simply redefining __GNUC_PREREQ__ here though?

It's a GNUism and pretty meaningless on with clang (which pretends to be GCC 4.2.1).

I'm also a little confused as to where __GNUC_PREREQ__ came from when all the references I can find online are to __GNUC_PREREQ (no trailing __).

markj added a comment.Wed, Oct 8, 3:36 PM
In D52933#1210529, @brooks wrote:
In D52933#1210494, @markj wrote:

That would probably work too, assuming that all the versions of clang we care about have __has_builtin as well. At a glance that seems to be the case.

Certainly. I don't think we care about things before 15, probably closer to 17. I wasn't able to determine when __has_builtin was added, but there was an issue with non-function-like builtins fixed in 10 and these are function-like so were fine even then.

What's wrong with simply redefining __GNUC_PREREQ__ here though?

It's a GNUism and pretty meaningless on with clang (which pretends to be GCC 4.2.1).

I'm also a little confused as to where __GNUC_PREREQ__ came from when all the references I can find online are to __GNUC_PREREQ (no trailing __).

Are you sure that's not __GNU_PREREQ?

brooks added a comment.Wed, Oct 8, 3:53 PM
In D52933#1210535, @markj wrote:
In D52933#1210529, @brooks wrote:

I'm also a little confused as to where __GNUC_PREREQ__ came from when all the references I can find online are to __GNUC_PREREQ (no trailing __).

Are you sure that's not __GNU_PREREQ?

I'm seeing https://github.com/lattera/glibc/blob/master/include/features.h#L158 (also true in upstream glibc, but the sourceware server seems to be getting killed by bots). I'm not objecting to the macro when used for GCC, but I am failing to find documentation so am confused.

Revision Contents
Changeset List

PathSize
tools/
build/
cross-build/
include/
common/
sys/
10 lines

Diff 163696

View Options

tools/build/cross-build/include/common/sys/cdefs.h

Loading...