Check for a flag to turn linker warnings into errors. When flags are passed to linkers via the compiler driver using a -Wl,-foo flag, linker warnings may be swallowed rather than bubbling up.
This is one of Corys commits that I’ve modified to also add -Wl,-fatal_warnings
for darwin.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
| File | commit b949ac9697a6cfe087f60a16c063ab9c5bf1e81f(master) | commit 996e3f28e5c6b9905342e2317366bf569514511b(master and this pull) |
|---|---|---|
| bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz | a14207517d25fece... |
2d5d8f78231bd88f... |
| bitcoin-0.19.99-aarch64-linux-gnu.tar.gz | 51fd58ca7f14c4bb... |
fd5ccd9659473979... |
| bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz | 0c13785557c412cb... |
24380e11e1fdb5c8... |
| bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz | d717d387941a6f52... |
d71c9392a5801a54... |
| bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz | 485858210efe05c2... |
d138a1306b8843d3... |
| bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz | 69a1cf3b9bf79fbe... |
72a461b011f30050... |
| bitcoin-0.19.99-osx-unsigned.dmg | c876fb9db3d4d189... |
b9e42fecdeea2c8e... |
| bitcoin-0.19.99-osx64.tar.gz | dd8a3ebd540cc66c... |
12d5b37d851bf4ac... |
| bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz | 6642fc1c8d1635d1... |
0bfd90a99ce431ee... |
| bitcoin-0.19.99-riscv64-linux-gnu.tar.gz | 6cccb448a4262e7b... |
0651c0db0caf6b2c... |
| bitcoin-0.19.99-win64-debug.zip | f94ba1033e8cc317... |
8acc4fd954f986a3... |
| bitcoin-0.19.99-win64-setup-unsigned.exe | 8b59c3214bfb6a46... |
0bef0f106215769e... |
| bitcoin-0.19.99-win64.zip | a23d7c88ff460d78... |
8f793e66ae312afb... |
| bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz | 62a5918ebf22fc21... |
596cd0c079f17593... |
| bitcoin-0.19.99-x86_64-linux-gnu.tar.gz | 98cd19fe59bc6eff... |
15937a64b1c2979c... |
| bitcoin-0.19.99.tar.gz | 5a211d09899b034c... |
f885c34c75532d83... |
| bitcoin-core-linux-0.20-res.yml | 7ed024b81ed9b639... |
9abdfb9cc40ea498... |
| bitcoin-core-osx-0.20-res.yml | 5947a980f2b74084... |
8a8d6d4f4dceed25... |
| bitcoin-core-win-0.20-res.yml | 9e64f2d02f143a21... |
95dfb8ad7aabb731... |
| linux-build.log | ef1f26008e10e842... |
e45f60189d89a2ce... |
| osx-build.log | 4ab5188b79e2e50c... |
7ebe457936033f70... |
| win-build.log | 0567536136b41af8... |
4982c2f710176ddc... |
| bitcoin-core-linux-0.20-res.yml.diff | ae47fd004452fa09... |
|
| bitcoin-core-osx-0.20-res.yml.diff | a94472bfb55530fd... |
|
| bitcoin-core-win-0.20-res.yml.diff | ab3897018163b920... |
|
| linux-build.log.diff | e0144ec71849b6cf... |
|
| osx-build.log.diff | 794e9f50b5549bd6... |
|
| win-build.log.diff | 6f48a8ab3566aecb... |
Going to restate my understanding just to make sure I’m clear on the logic here:
The AX_CHECK_LINK_FLAG macro checks whether a linker flag exists by invoking the linker, and defines failures as when the linker “gives an error”, however, warnings are ignored. Since it is possible that when the AX_CHECK_LINK_FLAG macro invokes the linker with a flag it doesn’t understand, the linker only gives a warning, we should make sure this doesn’t happen by turning warnings into errors, forcing the failure case.
@fanquake does that sound right?
@fanquake does that sound right? @dongcarl Thanks for restating. I didn’t like how I’d split the addition of the flag up, so I’ve pushed a new version of this change where they are together.
What this currently does is adding (either variant of) -fatal_warnings to the linker flags for all subsequent AX_CHECK_LINK_FLAG calls. -fatal_warnings does not get added to LDFLAGS and thus is not used at link time.
The practical affect of this change is that linker flags which previously would have been tested, thrown a warning, and still been added to LDFLAGS, are now no-longer being added.
For example, if we were to add an obsoleted macOS linker flag, such as -nofixprebinding, to configure.ac we’d get this behaviour:
0AX_CHECK_LINK_FLAG([[-Wl,-nofixprebinding]]....
1# configure checks and adds to $LDFLAGS
2checking whether the linker accepts -Wl,-nofixprebinding... yes
3# link time
4ld: warning: option -nofixprebinding is obsolete and being ignored
config.log:
0configure:18716: checking whether the linker accepts -Wl,-nofixprebinding
1configure:18735: g++ -std=c++11 -o conftest -g -O2 -Wl,-nofixprebinding conftest.cpp >&5
2ld: warning: option -nofixprebinding is obsolete and being ignored
3configure:18735: $? = 0
4configure:18745: result: yes
0# configure check fails
1checking whether the linker accepts -Wl,-nofixprebinding... no
config.log:
0configure:18752: checking whether the linker accepts -Wl,-nofixprebinding
1configure:18771: g++ -std=c++11 -o conftest -g -O2 -Wl,-fatal_warnings -Wl,-nofixprebinding conftest.cpp >&5
2ld: warning: option -nofixprebinding is obsolete and being ignored
3ld: fatal warning(s) induced error (-fatal_warnings)
4clang: error: linker command failed with exit code 1 (use -v to see invocation)
For master, using an option that would otherwise cause an error, such as a linker flag that doesn’t exist, is already a failure case. i.e:
0AX_CHECK_LINK_FLAG([[-Wl,-doesnt_exist]]...
1checking whether the linker accepts -Wl,-doesnt_exist... no
config.log:
0configure:18716: checking whether the linker accepts -Wl,-doesnt_exist
1configure:18735: g++ -std=c++11 -o conftest -g -O2 -Wl,-doesnt_exist conftest.cpp >&5
2ld: unknown option: -doesnt_exist
3clang: error: linker command failed with exit code 1 (use -v to see invocation)
Is this inline with your understanding?
One other thought I had is, somewhat similar to how we have an option for-Werror maybe we’d like to have a way for users to opt into passing -fatal_warnings at linker time?
296+case $host in
297+ *darwin*)
298+ AX_CHECK_LINK_FLAG([-Wl,-fatal_warnings],[LDFLAG_WERROR="-Wl,-fatal_warnings"],[LDFLAG_WERROR=""])
299+ ;;
300+ *)
301+ AX_CHECK_LINK_FLAG([-Wl,--fatal-warnings],[LDFLAG_WERROR="-Wl,--fatal-warnings"],[LDFLAG_WERROR=""])
- versus -- option prefix :woman_facepalming:
From the AX_CHECK_LINK_FLAG docs:
Warnings, however, are ignored
If
EXTRA-FLAGSis defined, it is added to the linker’s default flags when the check is done. The check is thus made with the flags: “LDFLAGS EXTRA-FLAGS FLAG”. This can for example be used to force the linker to issue an error when a bad flag is given. @dongcarl TheAX_CHECK_LINK_FLAGmacro checks whether a linker flag exists by invoking the linker, and defines failures as when the linker “gives an error”, however, warnings are ignored. Since it is possible that when theAX_CHECK_LINK_FLAGmacro invokes the linker with a flag it doesn’t understand, the linker only gives a warning, we should make sure this doesn’t happen by turning warnings into errors, forcing the failure case.
It seems when the AX_CHECK_LINK_FLAG macro invokes the linker with a flag it doesn’t understand it just runs ACTION-FAILURE.
The only case when a warning could be raised is passing a valid but obsolete options, right?
I’ve tested this pr suggestion on macOS 10.15.3 with an obsolete option -nofixprebinding. It works as expected.
But I could not test on Linux Mint 19.3 because I did not find any mention of obsolete options in the ld docs. Did I miss smth?
@fanquake
One other thought I had is, somewhat similar to how we have an option for
-Werrormaybe we’d like to have a way for users to opt into passing-fatal_warningsat linker time?
Passing -fatal_warnings at linker time could improve build robustness, IMHO.
ACK d333883a0
So, the semantics of this are different than those of compilation warnings (e.g. -Wfoo or -Werror=bar) - with compilation warnings flags during configure time we check whether a given flag is supported and if yes, then we use it during make time. With -Wl,--fatal-warnings, if it is available, we only append it to the linker flags when checking whether other linker flags like -Wl,baz are supported.
So, with this PR, we are not going to use -Wl,baz if it is going to produce a warning. And
-Wl,--fatal-warnings is not supposed to be used during make time by this PR.
I tested this on FreeBSD 12 and confirm it works as expected - it is being used when checking whether other linker flags are supported. No linker flags are being excluded due to it - before and after the PR LDFLAGS is -pthread -Wl,-z,relro -Wl,-z,now -pie -L/usr/local/lib.
This PR omitted the following, I guess on purpose?
0if test x$enable_determinism = xyes; then
1 if test x$TARGET_OS = xwindows; then
2 AX_CHECK_LINK_FLAG([[-Wl,--no-insert-timestamp]], [LDFLAGS="$LDFLAGS -Wl,--no-insert-timestamp"])
3 fi
4fi
Check for a flag to turn linker warnings into errors. When flags are passed to
linkers via the compiler driver using a -Wl,-foo flag, linker warnings may be
swallowed rather than bubbling up.
Co-authored-by: fanquake <fanquake@gmail.com>
Thanks for the review.
This PR omitted the following, I guess on purpose?
Nope. I’ve added that to configure.ac in the interim, and hadn’t updated this PR. Good catch. Have pushed the fix.
re-ACK 03da4c778
My thought was “this is something Windows-specific, I do not want to know about it”…
