build: more robustly check for fcf-protection support #20720

When using Clang 7, we may end up trying to use the flag when it won't work properly, which can lead to confusing errors. i.e:

/usr/bin/ld: error: ... <corrupt x86 feature size: 0x8>

Use AX_CHECK_LINK_FLAG & --fatal-warnings to ensure we wont use the flag in this case.

We do this as even when the error is emitted, compilation succeeds, and the binaries produced will run. This means we can't just check if the compiler accepts the flag, or if compilation succeeds (without or without -Werror, and/or passing -Wl,--fatal-warnings, which may not be passed through to the linker).

This was reported by someone configuring for fuzzing, on Debian 10, where Clang 7 is the default.

See here for a minimal example of the problematic behaviour: https://gist.github.com/fanquake/9b33555fcfebef8eb8c0795a71732bc6

Concept ACK, the issue is known in other open-source projects: https://github.com/gdnsd/gdnsd/commit/582004ebfee2e0d341e391e96f0ca116f443c413

cr ACK 01ca4bdadd250d24912cc08149a130a538e76a4f: patch looks correct

This makes sense. For me it brings up the question "why aren't we doing this for all hardening flags"?

~ACK 01ca4bdadd250d24912cc08149a130a538e76a4f~ see #20720 (comment)

1. tested on Debian 10.7 with clang 7.0.1

Tested configuration: ./configure --with-incompatible-bdb CC=clang CXX=clang++

• on master (f1dbf92ff0475a01d20170ea422c1d086acbbc57) observing massive messages like these:

/usr/bin/ar: error: libbitcoin_server_a-versionbits.o: <corrupt x86 feature size: 0x8>
...
/usr/bin/ranlib: error: libtest_util.a(libtest_util_a-net.o): <corrupt x86 feature size: 0x8>
...
/usr/bin/ld: error: .libs/libbitcoinconsensus_la-arith_uint256.o: <corrupt x86 feature size: 0x8>

• with this PR no errors or warnings during make

2. tested on Linux Mint 20 with clang 7.0.1

Tested configuration: ./configure --with-incompatible-bdb CC=clang-7 CXX=clang++-7

The same results as for Debian.

3. tested on Linux Mint 20 with clang 8.0.1 -- no errors even on master.

Thanks for testing @hebasto. However the previous patch wasn't working correctly. The flag would actually NEVER be applied (even where it would work correctly), because the source being compiled wasn't valid 🤦 . I've fixed that now.

For me it brings up the question "why aren't we doing this for all hardening flags"?

I'm currently reviewing all of our flags for any other subtle breakages. It very well might make sense to do this for all flags going forward.

... because the source being compiled wasn't valid

Indeed. I confused AC_LANG_SOURCE and AC_LANG_PROGRAM macros :man_facepalming: (btw, the latter seems more concise for this change).

I'm currently reviewing all of our flags for any other subtle breakages. It very well might make sense to do this for all flags going forward.

Thinking about it, it is a bit of a compromise. We wouldn't want to increase the opportunity to silently lose hardening flags. Of course the security checks are the best place to verify this, but we don't have checks for all of the hardening mechanisms (if that is even possible).

I've changed the approach here to check for link errors, as even when the error is emitted, compilation succeeds, and the binaries produced will run. This means we can't just check if the compiler accepts the flag, or if compilation succeeds (without or without -Werror, and/or passing -Wl,--fatal-warnings, which may not be passed through to the linker).

Instead we can do a link check, with --fatal-warnings, and not add -fcf-protection to our HARDENED_CXXFLAGS if we see any errors.

I have only seen this link issue with Clang 7 (when support for -fcf-protection was introduced). Clang 8+ look ok.

Unfortunately still broken on Debian 10.7.

build.log

Unfortunately still broken on Debian 10.7.

Looking at the build.log, the change here is working as expected and isn't adding -fcf-protection to the hardened flags when the linker emits an error:

checking whether the linker accepts -fcf-protection=full... no
....
configure:20769: checking whether the linker accepts -fcf-protection=full
configure:20788: clang++ -std=c++17 -o conftest -g -O2  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS  -Wl,--fatal-warnings -fcf-protection=full conftest.cpp  >&5
/usr/bin/ld: error: /tmp/conftest-afee8c.o: <corrupt x86 feature size: 0x8>
clang: error: linker command failed with exit code 1 (use -v to see invocation)
configure:20788: $? = 1
configure: failed program was:

The build error you're now seeing:

  CXXLD    test/fuzz/fuzz
/usr/bin/ld: test/fuzz/fuzz-addrdb.o: in function `ConsumeNode(FuzzedDataProvider&)':
/home/bitcoin/bitcoin/src/./test/fuzz/util.h:290: multiple definition of `ConsumeNode(FuzzedDataProvider&)'; test/fuzz/fuzz-addition_overflow.o:/home/bitcoin/bitcoin/src/./test/fuzz/util.h:290: first defined here
/usr/bin/ld: test/fuzz/fuzz-addrdb.o: in function `ConsumeSubNet(FuzzedDataProvider&)':
/home/bitcoin/bitcoin/src/./test/fuzz/util.h:275: multiple definition of `ConsumeSubNet(FuzzedDataProvider&)'; test/fuzz/fuzz-addition_overflow.o:/home/bitcoin/bitcoin/src/./test/fuzz/util.h:275: first defined here
/usr/bin/ld: test/fuzz/fuzz-addrdb.o: in function `ConsumeAddress(FuzzedDataProvider&)':
/home/bitcoin/bitcoin/src/./test/fuzz/util.h:285: multiple definition of `ConsumeAddress(FuzzedDataProvider&)'; test/fuzz/fuzz-addition_overflow.o:/home/bitcoin/bitcoin/src/./test/fuzz/util.h:285: first defined here
...

looks more like the one reported here.

I was wrong, you're right, this is another failure described here #20560 (comment)

tested ACK e9189a750b237eba1befc6b16c12c2cee3e0176c

not an ACK e9189a750b237eba1befc6b16c12c2cee3e0176c , I only tested configure on my system (gcc-10, clang-11):

diff --git a/tmp/a_gcc b/tmp/b_gcc
index c0ef19c9a8..d7e3ac31c2 100644
--- a/tmp/a_gcc
+++ b/tmp/b_gcc
@@ -174,7 +174,7 @@ checking whether C++ compiler accepts -fPIC... yes
 checking whether C++ compiler accepts -fstack-reuse=none... yes
 checking whether C++ compiler accepts -Wstack-protector... yes
 checking whether C++ compiler accepts -fstack-protector-all... yes
-checking whether C++ compiler accepts -fcf-protection=full... yes
+checking whether the linker accepts -fcf-protection=full... yes
 checking whether C++ compiler accepts -fstack-clash-protection... yes
 checking whether C++ preprocessor accepts -D_FORTIFY_SOURCE=2... yes
 checking whether C++ preprocessor accepts -U_FORTIFY_SOURCE... yes


diff --git a/tmp/a_clang b/tmp/b_clang
index 68c613d418..f238d83acd 100644
--- a/tmp/a_clang
+++ b/tmp/b_clang
@@ -174,7 +174,7 @@ checking whether C++ compiler accepts -fPIC... yes
 checking whether C++ compiler accepts -fstack-reuse=none... no
 checking whether C++ compiler accepts -Wstack-protector... yes
 checking whether C++ compiler accepts -fstack-protector-all... yes
-checking whether C++ compiler accepts -fcf-protection=full... yes
+checking whether the linker accepts -fcf-protection=full... yes
 checking whether C++ compiler accepts -fstack-clash-protection... yes
 checking whether C++ preprocessor accepts -D_FORTIFY_SOURCE=2... yes
 checking whether C++ preprocessor accepts -U_FORTIFY_SOURCE... yes

ACK e9189a750b237eba1befc6b16c12c2cee3e0176c, tested with clang-7, clang-10 and gcc: the -fcf-protection=full is not applied for clang-7, but applied for others compilers.