build: improve macro for testing -latomic requirement #21920

1. 
   MarcoFalke commented at 6:10 pm on May 11, 2021: member

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

   Steps to reproduce on vanilla Ubuntu Focal:

   0export 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:

    0/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':
    1net.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
    2/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)':
    3net.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
    4/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<ServiceFlags>::load(std::memory_order) const':
    5net.cpp:(.text._ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order[_ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
    6/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<ServiceFlags>::store(ServiceFlags, std::memory_order)':
    7net_processing.cpp:(.text._ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order[_ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order]+0x6c): undefined reference to `__atomic_store'
    8/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':
    9net_processing.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
   10/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)':
   11net_processing.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
   12clang: error: linker command failed with exit code 1 (use -v to see invocation)
   

   After:

   Clean

2. build: improve macro for testing -latomic requirement fa25ce45e9
3. 
   hebasto commented at 6:22 pm on May 11, 2021: member

   Steps to reproduce on vanilla Ubuntu Focal:

   0export 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 :)

4. in build-aux/m4/l_atomic.m4:24 in fa25ce45e9

   19   int main() {
   20     std::atomic<bool> lock{true};
   21     std::atomic_exchange(&lock, false);
   22 
   23+    std::atomic<std::chrono::seconds> t{0s};
   24+    t.store(2s);
   

   ---

   ---

   

   hebasto commented at 6:25 pm on May 11, 2021:

   Isn’t just declaration of std::atomic<std::chrono::seconds> variable enough?

   ---

   

   MarcoFalke commented at 6:26 pm on May 11, 2021:

   I doubt it. The linker error complain about a missing store or load

   ---

   

   fanquake commented at 12:30 pm on May 18, 2021:

   Isn’t just declaration of std::atomicstd::chrono::seconds variable enough?

   Yep that isn’t enough.

5. 
   MarcoFalke commented at 6:29 pm on May 11, 2021: member

   Somehow this just fixed itself, it seems :confused:
6. MarcoFalke closed this on May 11, 2021

   ---

7. MarcoFalke deleted the branch on May 11, 2021
8. MarcoFalke restored the branch on May 12, 2021
9. 
   MarcoFalke commented at 1:19 pm on May 12, 2021: member

   Was testing the wrong thing. Issue still exists with the “steps to reproduce” in OP.
10. MarcoFalke reopened this on May 12, 2021

    ---

11. DrahtBot added the label Build system on May 12, 2021
12. 
    jarolrod commented at 5:18 pm on May 13, 2021: member

    ACK fa25ce45e9d54a2f0d5f2c0de9254d102a855a76

    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.

13. 
    hebasto commented at 7:40 am on May 17, 2021: member

    Reproducing steps from the OP:

     0...
     1  CXXLD    test/fuzz/fuzz
     2/usr/bin/ld: test/fuzz/fuzz-multiplication_overflow.o: in function `void (anonymous namespace)::TestMultiplicationOverflow<long long>(FuzzedDataProvider&)':
     3multiplication_overflow.cpp:(.text._ZN12_GLOBAL__N_126TestMultiplicationOverflowIxEEvR18FuzzedDataProvider[_ZN12_GLOBAL__N_126TestMultiplicationOverflowIxEEvR18FuzzedDataProvider$f74c2a0666fdfec672dc24de475b04f3]+0x8d): undefined reference to `__mulodi4'
     4/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':
     5net.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
     6/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)':
     7net.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1000000EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
     8/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net.o): in function `std::atomic<ServiceFlags>::load(std::memory_order) const':
     9net.cpp:(.text._ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order[_ZNKSt6atomicI12ServiceFlagsE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
    10/usr/bin/ld: libbitcoin_server.a(libbitcoin_server_a-net_processing.o): in function `std::atomic<ServiceFlags>::store(ServiceFlags, std::memory_order)':
    11net_processing.cpp:(.text._ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order[_ZNSt6atomicI12ServiceFlagsE5storeES0_St12memory_order]+0x6c): undefined reference to `__atomic_store'
    12/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':
    13net_processing.cpp:(.text._ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order[_ZNKSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE4loadESt12memory_order]+0x51): undefined reference to `__atomic_load'
    14/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)':
    15net_processing.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE5storeES4_St12memory_order]+0x5f): undefined reference to `__atomic_store'
    16clang: error: linker command failed with exit code 1 (use -v to see invocation)
    17make[2]: *** [Makefile:6140: test/fuzz/fuzz] Error 1
    18make[2]: Leaving directory '/home/hebasto/bitcoin/src'
    19make[1]: *** [Makefile:15995: all-recursive] Error 1
    20make[1]: Leaving directory '/home/hebasto/bitcoin/src'
    21make: *** [Makefile:821: all-recursive] Error 1
    

14. hebasto approved
15. 
    hebasto commented at 9:28 am on May 17, 2021: member

    ACK fa25ce45e9d54a2f0d5f2c0de9254d102a855a76, tested on Ubuntu 20.04.2 LTS.

    It seems this, more concise, diff also works:

     0--- a/build-aux/m4/l_atomic.m4
     1+++ b/build-aux/m4/l_atomic.m4
     2@@ -14,14 +14,9 @@ m4_define([_CHECK_ATOMIC_testbody], [[
     3   #include <cstdint>
     4 
     5   int main() {
     6-    std::atomic<bool> lock{true};
     7-    std::atomic_exchange(&lock, false);
     8-
     9-    std::atomic<int64_t> a{};
    10-
    11-    int64_t v = 5;
    12-    int64_t r = a.fetch_add(v);
    13-    return static_cast<int>(r);
    14+    enum e : uint64_t { e0, e1 };
    15+    std::atomic<e> a{e0};
    16+    a.store(e1);
    17   }
    18 ]])
    19 
    

16. 
    MarcoFalke commented at 10:58 am on May 17, 2021: member

    It seems this, more concise, diff also works:

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

17. MarcoFalke requested review from fanquake on May 17, 2021
18. 
    hebasto commented at 11:53 am on May 17, 2021: member

    Does this also work for riscv64 from commit 54ce4fa ?

    After reverting 54ce4fac80689621dcbcc76169b2b00b179ee743 on master, I still able to successfully cross compile for RISC-V (including gitian build) :man_shrugging:

    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 :)

19. 
    MarcoFalke commented at 5:32 am on May 18, 2021: member

    @fanquake Mind taking a look?
20. fanquake approved
21. 
    fanquake commented at 12:32 pm on May 18, 2021: member

    ACK fa25ce45e9d54a2f0d5f2c0de9254d102a855a76
22. fanquake merged this on May 18, 2021
23. fanquake closed this on May 18, 2021

    ---

24. sidhujag referenced this in commit 7bbac0c554 on May 18, 2021
25. MarcoFalke deleted the branch on May 18, 2021
26. luke-jr referenced this in commit 9331713f10 on Jun 27, 2021
27. furszy referenced this in commit f2b45fded0 on Jan 9, 2022
28. gwillen referenced this in commit 25d1b780eb on Jun 1, 2022
29. DrahtBot locked this on Aug 18, 2022

Contributors
MarcoFalke hebasto fanquake jarolrod

Labels
Build system