build: improve macro for testing -latomic requirement #21920

maflcko · 2021-05-11T18:10:36Z

This fixes the issue where -latomic is incorrectly omitted from the linker flags.

Steps to reproduce on vanilla Ubuntu Focal:

export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl clang llvm g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y  && ( cd depends && make DEBUG=1 HOST=i686-pc-linux-gnu NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/i686-pc-linux-gnu/share/config.site" ./configure CC='clang -m32' CXX='clang++ -m32' --enable-fuzz --with-sanitizers=fuzzer && make  -j $(nproc)

Before:

/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::load(std::memory_order) const':
net.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1000000ll> >, std::memory_order)':
net.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<ServiceFlags>::load(std::memory_order) const':
net.cpp:(.text._ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order[_ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<ServiceFlags>::store(ServiceFlags, std::memory_order)':
net_processing.cpp:(.text._ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order[_ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order]+0x6c): undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::load(std::memory_order) const':
net_processing.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1ll> >, std::memory_order)':
net_processing.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

After:

Clean

hebasto · 2021-05-11T18:22:28Z

Steps to reproduce on vanilla Ubuntu Focal:

export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl clang llvm g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y  && ( cd depends && make DEBUG=1 HOST=i686-pc-linux-gnu NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/i686-pc-linux-gnu/share/config.site" ./configure CC='clang -m32' CXX='clang++ -m32' --enable-fuzz --with-sanitizers=fuzzer LDFLAGS='-latomic' && make  -j $(nproc)

I've scrolled it to the end :)

build-aux/m4/l_atomic.m4

maflcko · 2021-05-11T18:29:30Z

Somehow this just fixed itself, it seems 😕

maflcko · 2021-05-12T13:19:17Z

Was testing the wrong thing. Issue still exists with the "steps to reproduce" in OP.

jarolrod

ACK fa25ce4

Confirming the issue on master with the provided instructions. This PR fixes the mentioned issue.

With this PR, i run into an unrelated issue with undefined reference to _mulodi4. Cherry-picking #21882 fixes this.

hebasto · 2021-05-17T07:40:59Z

Reproducing steps from the OP:

...
  CXXLD    test/fuzz/fuzz
/usr/bin/ld: test/fuzz/fuzz-multiplication_overflow.o: in function `void (anonymous namespace)::TestMultiplicationOverflow<long long>(FuzzedDataProvider&)':
multiplication_overflow.cpp:(.text._ZN12_GLOBAL__N_126TestMultiplicationOverflowIxEEvR18FuzzedDataProvider[_ZN12_GLOBAL__N_126TestMultiplicationOverflowIxEEvR18FuzzedDataProvider$f74c2a0666fdfec672dc24de475b04f3]+0x8d): undefined reference to `__mulodi4'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::load(std::memory_order) const':
net.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1000000ll> >, std::memory_order)':
net.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<ServiceFlags>::load(std::memory_order) const':
net.cpp:(.text._ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order[_ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<ServiceFlags>::store(ServiceFlags, std::memory_order)':
net_processing.cpp:(.text._ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order[_ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order]+0x6c): undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::load(std::memory_order) const':
net_processing.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1ll> >, std::memory_order)':
net_processing.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [Makefile:6140: test/fuzz/fuzz] Error 1
make[2]: Leaving directory '/home/hebasto/bitcoin/src'
make[1]: *** [Makefile:15995: all-recursive] Error 1
make[1]: Leaving directory '/home/hebasto/bitcoin/src'
make: *** [Makefile:821: all-recursive] Error 1

hebasto

ACK fa25ce4, tested on Ubuntu 20.04.2 LTS.

It seems this, more concise, diff also works:

--- a/build-aux/m4/l_atomic.m4
+++ b/build-aux/m4/l_atomic.m4
@@ -14,14 +14,9 @@ m4_define([_CHECK_ATOMIC_testbody], [[
   #include <cstdint>
 
   int main() {
-    std::atomic<bool> lock{true};
-    std::atomic_exchange(&lock, false);
-
-    std::atomic<int64_t> a{};
-
-    int64_t v = 5;
-    int64_t r = a.fetch_add(v);
-    return static_cast<int>(r);
+    enum e : uint64_t { e0, e1 };
+    std::atomic<e> a{e0};
+    a.store(e1);
   }
 ]])

maflcko · 2021-05-17T10:58:21Z

It seems this, more concise, diff also works:

Does this also work for riscv64 from commit 54ce4fa ? In either case the macro is already ~50 LOC, so I think +-3 lines shouldn't matter too much.

hebasto · 2021-05-17T11:53:33Z

Does this also work for riscv64 from commit 54ce4fa ?

After reverting 54ce4fa on master, I still able to successfully cross compile for RISC-V (including gitian build) 🤷‍♂️

In either case the macro is already ~50 LOC, so I think +-3 lines shouldn't matter too much.

I assumed, that a test program should be a kind of minimal working example :)

maflcko · 2021-05-18T05:32:57Z

@fanquake Mind taking a look?

fanquake

ACK fa25ce4

…ement fa25ce4 build: improve macro for testing -latomic requirement (MarcoFalke) Pull request description: This fixes the issue where `-latomic` is incorrectly omitted from the linker flags. Steps to reproduce on vanilla Ubuntu Focal: ``` export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl clang llvm g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y && ( cd depends && make DEBUG=1 HOST=i686-pc-linux-gnu NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j $(nproc) ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/i686-pc-linux-gnu/share/config.site" ./configure CC='clang -m32' CXX='clang++ -m32' --enable-fuzz --with-sanitizers=fuzzer && make -j $(nproc) ``` Before: ``` /usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::load(std::memory_order) const': net.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1000000ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1000000ll> >, std::memory_order)': net.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<ServiceFlags>::load(std::memory_order) const': net.cpp:(.text._ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order[_ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<ServiceFlags>::store(ServiceFlags, std::memory_order)': net_processing.cpp:(.text._ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order[_ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order]+0x6c): undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::load(std::memory_order) const': net_processing.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::store(std::chrono::duration<long long, std::ratio<1ll, 1ll> >, std::memory_order)': net_processing.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store' clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` After: Clean ACKs for top commit: jarolrod: ACK fa25ce4 hebasto: ACK fa25ce4, tested on Ubuntu 20.04.2 LTS. fanquake: ACK fa25ce4 Tree-SHA512: 7b0e76876c28a4ef8d8cde1ea26c18aeec51da3d54e4da75829276b9b2a259f9d0feab215be0d39e651c6bd89071859bc29aa48131e627e745a828d9f8d4a98e

Github-Pull: bitcoin#21920 Rebased-From: fa25ce4

c76e7b6 [Core] Fix data races in bls_worker and use ctpl_stl queue (Fuzzbawls) 55babf4 [Build] Improve std::atomic test macro (Fuzzbawls) Pull request description: Coming from dashpay#4240, bitcoin#20938, and bitcoin#21920, this resolves numerous compile-time inconsistencies/failures on certain combos of gcc and cpu arch that were observed on launchpad.net over the 5.4.0 release cycle. ACKs for top commit: furszy: ACK c76e7b6 random-zebra: ACK c76e7b6 Tree-SHA512: d931142f978d0d22668b9ff2e16d48c148b7d163b35aeef149ab08a13174536c035f2581a8ec5bc9782226fcb27ed33b08aec475d1522d8e8532d00a12c5df72

build: improve macro for testing -latomic requirement

fa25ce4

hebasto reviewed May 11, 2021

View reviewed changes

build-aux/m4/l_atomic.m4 Show resolved Hide resolved

maflcko closed this May 11, 2021

maflcko deleted the 2105-buildAtomicLink branch May 11, 2021 18:30

maflcko restored the 2105-buildAtomicLink branch May 12, 2021 13:00

maflcko reopened this May 12, 2021

DrahtBot added the Build system label May 12, 2021

jarolrod reviewed May 13, 2021

View reviewed changes

maflcko mentioned this pull request May 13, 2021

build: Fix undefined reference to __mulodi4 #21882

Merged

hebasto approved these changes May 17, 2021

View reviewed changes

maflcko requested a review from fanquake May 17, 2021 10:58

fanquake approved these changes May 18, 2021

View reviewed changes

fanquake merged commit 741749a into bitcoin:master May 18, 2021

maflcko deleted the 2105-buildAtomicLink branch May 18, 2021 15:00

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Jun 27, 2021

build: improve macro for testing -latomic requirement

9331713

Github-Pull: bitcoin#21920 Rebased-From: fa25ce4

Fuzzbawls mentioned this pull request Dec 28, 2021

[Core][Build] Fix atomic data races in BLS operations PIVX-Project/PIVX#2702

Merged

gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022

Merge 741749a into merged_master (Bitcoin PR bitcoin/bitcoin#21920)

25d1b78

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: improve macro for testing -latomic requirement #21920

build: improve macro for testing -latomic requirement #21920

maflcko commented May 11, 2021 •

edited

Loading

hebasto commented May 11, 2021

maflcko commented May 11, 2021

maflcko commented May 12, 2021

jarolrod left a comment

hebasto commented May 17, 2021

hebasto left a comment

maflcko commented May 17, 2021

hebasto commented May 17, 2021 •

edited

Loading

maflcko commented May 18, 2021

fanquake left a comment

build: improve macro for testing -latomic requirement #21920

build: improve macro for testing -latomic requirement #21920

Conversation

maflcko commented May 11, 2021 • edited Loading

hebasto commented May 11, 2021

maflcko commented May 11, 2021

maflcko commented May 12, 2021

jarolrod left a comment

Choose a reason for hiding this comment

hebasto commented May 17, 2021

hebasto left a comment

Choose a reason for hiding this comment

maflcko commented May 17, 2021

hebasto commented May 17, 2021 • edited Loading

maflcko commented May 18, 2021

fanquake left a comment

Choose a reason for hiding this comment

maflcko commented May 11, 2021 •

edited

Loading

hebasto commented May 17, 2021 •

edited

Loading