PR #20358 made use of the two preprocessor symbols HAVE_STRONG_GETAUXVAL as well as HAVE_WEAK_GETAUXVAL.
These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16.
PR #20358 would have broken the macOS build since getauxval is not supported on macOS (but weak-linking does pass).
This PR defines the two symbols correctly and reduces calls to getauxval to linux.
Code review ACK 836a3dc02c72f917db5be386b9b4787a59d48610 Agree with making the weak link check Linux specific. It will pretty much pass on every platform with weak linking otherwise, redundantly.
utACK 836a3dc02c72f917db5be386b9b4787a59d48610
#20358 is marked for backport, so should this be marked as well or neither?
I don't think this qualifies a backport.
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
<!--a722867cd34abeea1fadc8d60700f111-->
1109 | @@ -1110,18 +1110,20 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ 1110 | ]], [[ 1111 | getauxval(AT_HWCAP); 1112 | ]])], 1113 | - [ AC_MSG_RESULT(yes); HAVE_STRONG_GETAUXVAL=1 ], 1114 | + [ AC_MSG_RESULT(yes); HAVE_STRONG_GETAUXVAL=1; AC_DEFINE(HAVE_STRONG_GETAUXVAL, 1, [Define this symbol to build code that uses getauxval)]) ],
[
AC_MSG_RESULT(yes)
HAVE_STRONG_GETAUXVAL=1
AC_DEFINE(HAVE_STRONG_GETAUXVAL, 1, [Define this symbol to build code that uses getauxval)])
],
1123 | + #endif 1124 | ]], [[ 1125 | getauxval(AT_HWCAP); 1126 | ]])], 1127 | - [ AC_MSG_RESULT(yes); HAVE_WEAK_GETAUXVAL=1 ], 1128 | + [ AC_MSG_RESULT(yes); HAVE_WEAK_GETAUXVAL=1; AC_DEFINE(HAVE_WEAK_GETAUXVAL, 1, [Define this symbol to build code that uses getauxval (weak linking)]) ],
[
AC_MSG_RESULT(yes)
HAVE_WEAK_GETAUXVAL=1
AC_DEFINE(HAVE_WEAK_GETAUXVAL, 1, [Define this symbol to build code that uses getauxval (weak linking)])
],
1117 |
1118 | AC_MSG_CHECKING(for weak getauxval support in the compiler)
1119 | AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
1120 | + #ifdef __linux__
1121 | unsigned long getauxval(unsigned long type) __attribute__((weak));
1122 | #define AT_HWCAP 16
This seems very fragile. On what target is getauxval only weak?
Why would it be (more) fragile? Weak linking is already pretty fragile AFAIK. IMO this weak linking passes always. See masters Win64 build as an example.
AFAIK this check is part of googles crc32c suite.
Limiting it to linux makes absolute sense to me since getauxval (it doesn't exists on Mac/win).
Sorry, I do mean the weak linking entirely. What do we lose, if we only check for strong linking?
That's a valid question.
AFAIK the google crc32c getauxval weak linking check is for android (< API level 20).
Crc32c is an upstream upstream subtree. I guess if we don't want to check for the weak linking, we might want to PR that there.
I assume if we just don't define it, that code block simply won't be used?
This seems very fragile. On what target is getauxval only weak?
a) getauxval only exists on Linux (OSes with the Linux kernel, I mean) b) by nature of weak linking, it could weakly link on every platform, this makes no sense though
I think this fix is fine. Removing the weak check completely is something that could be considered, but might interfere with Android. It doesn't need to be decided in this PR.