build: Fix false positive `CHECK_ATOMIC` test #29859

hebasto · 2024-04-12T08:13:57Z

On the master branch @ 0de63b8, a building bitcoind with clang-15 for i686-pc-linux-gnu fails to link:

  CXXLD    bitcoind
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load'
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o):/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: more undefined references to `__atomic_store' follow
clang: error: linker command failed with exit code 1 (use -v to see invocation)

due to false positive CHECK_ATOMIC test in the configure script.

This PR fixes this test.

DrahtBot · 2024-04-12T08:14:46Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko · 2024-04-12T09:19:30Z

The same error happens with clang-18 locally, but why does CI pass?

fad23a0

hebasto · 2024-04-14T09:01:14Z

The same error happens with clang-18 locally, but why does CI pass?

fad23a0

Depends on standard C++ library version?

maflcko · 2024-04-14T10:29:19Z

The same error happens with clang-18 locally, but why does CI pass?
fad23a0

Depends on standard C++ library version?

I can re-try, but my findings didn't indicate that IIRC.

In any case, the title needs to be adjusted to remove the clang-15, no?

hebasto · 2024-04-14T10:30:47Z

In any case, the title needs to be adjusted to remove the clang-15, no?

Updated.

maflcko · 2024-04-15T09:24:19Z

For reference, on a fresh Ubuntu 24.04 Noble (which should be identical to the current CI config), it fails:

apt update && apt install clang-18 g++-multilib -y
clang++-18  -std=c++20 test.cpp -m32

/usr/bin/ld: /tmp/test-9da16c.o: in function `std::__atomic_float<double>::store(double, std::memory_order)':
test.cpp:(.text._ZNSt14__atomic_floatIdE5storeEdSt12memory_order[_ZNSt14__atomic_floatIdE5storeEdSt12memory_order]+0x5a): undefined reference to `__atomic_store'
/usr/bin/ld: /tmp/test-9da16c.o: in function `std::__atomic_float<double>::load(std::memory_order) const':
test.cpp:(.text._ZNKSt14__atomic_floatIdE4loadESt12memory_order[_ZNKSt14__atomic_floatIdE4loadESt12memory_order]+0x47): undefined reference to `__atomic_load'
/usr/bin/ld: /tmp/test-9da16c.o: in function `std::atomic<std::chrono::duration<long long, std::ratio<1ll, 1ll> > >::compare_exchange_strong(std::chrono::duration<long long, std::ratio<1ll, 1ll> >&, std::chrono::duration<long long, std::ratio<1ll, 1ll> >, std::memory_order, std::memory_order)':
test.cpp:(.text._ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE23compare_exchange_strongERS4_S4_St12memory_orderS7_[_ZNSt6atomicINSt6chrono8durationIxSt5ratioILx1ELx1EEEEE23compare_exchange_strongERS4_S4_St12memory_orderS7_]+0x6e): undefined reference to `__atomic_compare_exchange'
clang++-18: error: linker command failed with exit code 1 (use -v to see invocation)

maflcko · 2024-04-15T13:45:29Z

On the master branch @ 0de63b8, a building bitcoind with clang-15 for i686-pc-linux-gnu fails to link:

Can you add exact steps to reproduce, ideally starting from a fresh install of the operating system?

maflcko · 2024-04-15T20:46:09Z

Ok, I could reproduce by setting this CI to lunar or bookworm (I tried clang-15 and clang-16): FILE_ENV="./ci/test/00_setup_env_i686_multiprocess.sh" ./ci/test_run_all.sh. However, on jammy, mantic, and noble it passes.

maflcko · 2024-04-16T09:16:11Z

review ACK dd3e0fa

fanquake

ACK dd3e0fa

Github-Pull: bitcoin#29859 Rebased-From: dd3e0fa

fanquake · 2024-04-17T13:06:26Z

Backported to 27.x in #29888.

Backport changes from bitcoin#29859.

5caf5a8 fixup! cmake: Check system symbols (Hennadii Stepanov) Pull request description: Backport changes from bitcoin#29859. ACKs for top commit: theuni: utACK 5caf5a8 Tree-SHA512: 89ecbff6d4df60a83fb2c6e0e9df078ba3a9a4b03a369efa9b9d4ef09f3f0b429f171ba82b97f1b6a85b185e466d5479d469fc819bd6e7044ddb0d4be178429b

Github-Pull: bitcoin#29859 Rebased-From: dd3e0fa

bd5860b [WIP] doc: release notes for 27.x (fanquake) 475aac4 doc: add LLVM instruction for macOS < 13 (Sjors Provoost) a995902 depends: Fix build of Qt for 32-bit platforms (laanwj) 0fcceef Fix #29767, set m_synced = true after Commit() (nanlour) ae9a2ed sign: don't assume we are parsing a sane Miniscript (Antoine Poinsot) a6a59cf rpc: Reword SighashFromStr error message (MarcoFalke) 364bf01 build: Fix false positive `CHECK_ATOMIC` test for clang-15 (Hennadii Stepanov) 9277793 test: Fix failing univalue float test (MarcoFalke) 5c09791 doc: archive 27.0 release notes (fanquake) 897e5af [rpc, bugfix] Enforce maximum value for setmocktime (dergoegge) 602cfd5 ci: Bump s390x to ubuntu:24.04 (MarcoFalke) 20e6e8d Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us (Luke Dashjr) a6862c5 depends: fix mingw-w64 Qt DEBUG=1 build (fanquake) Pull request description: Backports: * #29691 * #29747 * #29776 * #29853 * #29856 * #29859 * #29869 * #29870 * #29886 * #29892 * #29934 * #29985 ACKs for top commit: willcl-ark: reACK bd5860b stickies-v: re-ACK bd5860b TheCharlatan: ACK bd5860b Tree-SHA512: a1a40de70cf52b5fc01d9dcc772421751a18c6a48a726c4c05c0371c585a53a27902e17daed9e0d721ab7763c94bb32de05c146bd6bc73fd558edd08b31e8547

dd3e0fa build: Fix false positive `CHECK_ATOMIC` test for clang-15 (Hennadii Stepanov) Pull request description: On the master branch @ 0de63b8, a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link: ``` CXXLD bitcoind /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o):/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: more undefined references to `__atomic_store' follow clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` due to false positive `CHECK_ATOMIC` test in the `configure` script. This PR fixes this test. ACKs for top commit: maflcko: review ACK dd3e0fa fanquake: ACK dd3e0fa Tree-SHA512: b60acf8d83fc84cc3280d95028395d341ed9ed2fcf38ae0a101d50aa19cc35540e9763aa36668c4dc1e1bc7e1f33dbda0e662df39c9e414a284ef91d7fc55fba

700b8c5 Merge bitcoin#29658: Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one (merge-script) a0cd305 Merge bitcoin#29948: test: add missing comparison of node1's mempool in MempoolPackagesTest (Ava Chow) f1907ea Merge bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in Discover (merge-script) daa6eee Merge bitcoin#29960: depends: pass verbose through to cmake based makefiles (merge-script) 61a5832 Merge bitcoin#29907: test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (merge-script) 23f25a9 Merge bitcoin#29872: test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (merge-script) a7daee7 Merge bitcoin#29689: lint: scripted-diff verification also requires GNU grep (Ava Chow) 3df1ca1 Merge bitcoin#29953: doc: Bash is needed in gen_id and is not installed on FreeBSD by default (merge-script) b53b854 Merge bitcoin#29850: net: Decrease nMaxIPs when learning from DNS seeds (Ava Chow) c4a147c Merge bitcoin#28340: security: restrict abis in bitcoind.service (Ryan Ofsky) acfdf9e Merge bitcoin#28373: doc: Add example of mixing private and public keys in descriptors (Ava Chow) 51bc8bd Merge bitcoin#29859: build: Fix false positive `CHECK_ATOMIC` test (merge-script) bb4102c Merge bitcoin#29893: test: fix intermittent failure in p2p_compactblocks_hb.py (glozow) 4ecb761 Merge bitcoin#29786: Drop Windows Socket dependency for `randomenv.cpp` (fanquake) 1a8e805 Merge bitcoin#29498: test: Update --tmpdir doc string to say directory must not exist (fanquake) 81ca71c Merge bitcoin#29781: depends: add new LLVM debug macro (fanquake) 5ce92ca Merge bitcoin#29527: depends: add -g to DEBUG=1 flags (fanquake) Pull request description: ## Issue being fixed or feature implemented Trivial backports ## What was done? ## How Has This Been Tested? built locally ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 700b8c5 but pls confirm Guix is happy now before merging kwvg: utACK 700b8c5 Tree-SHA512: 9722979c4f0589cb02bfeaf39373713372a4aa1c8c5a55aa5b4d33388f73ef19231de3963b80ffaad25fbe0db90a133de5080baac691daed41b86a762b867b2d

build: Fix false positive CHECK_ATOMIC test for clang-15

dd3e0fa

DrahtBot added the Build system label Apr 12, 2024

hebasto changed the title ~~build: Fix false positive CHECK_ATOMIC test for clang-15~~ build: Fix false positive CHECK_ATOMIC test Apr 14, 2024

maflcko requested a review from fanquake April 16, 2024 09:26

fanquake approved these changes Apr 17, 2024

View reviewed changes

fanquake merged commit dd9007d into bitcoin:master Apr 17, 2024
16 checks passed

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Apr 17, 2024

build: Fix false positive CHECK_ATOMIC test for clang-15

364bf01

Github-Pull: bitcoin#29859 Rebased-From: dd3e0fa

fanquake mentioned this pull request Apr 17, 2024

[27.x] Backports #29888

Merged

hebasto deleted the 240412-atomic branch April 18, 2024 08:11

hebasto added a commit to hebasto/bitcoin that referenced this pull request Apr 18, 2024

fixup! cmake: Check system symbols

5caf5a8

Backport changes from bitcoin#29859.

hebasto mentioned this pull request Apr 18, 2024

cmake: Backport changes from PR29859 hebasto/bitcoin#151

Merged

hebasto mentioned this pull request Apr 21, 2024

cmake, ci: Bump clang version to 15 hebasto/bitcoin#159

Merged

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Apr 24, 2024

build: Fix false positive CHECK_ATOMIC test for clang-15

e683f05

Github-Pull: bitcoin#29859 Rebased-From: dd3e0fa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Fix false positive `CHECK_ATOMIC` test #29859

build: Fix false positive `CHECK_ATOMIC` test #29859

hebasto commented Apr 12, 2024

DrahtBot commented Apr 12, 2024 •

edited

Loading

maflcko commented Apr 12, 2024 •

edited

Loading

hebasto commented Apr 14, 2024

maflcko commented Apr 14, 2024

hebasto commented Apr 14, 2024

maflcko commented Apr 15, 2024

maflcko commented Apr 15, 2024

maflcko commented Apr 15, 2024 •

edited

Loading

maflcko commented Apr 16, 2024

fanquake left a comment

fanquake commented Apr 17, 2024

build: Fix false positive CHECK_ATOMIC test #29859

build: Fix false positive CHECK_ATOMIC test #29859

Conversation

hebasto commented Apr 12, 2024

DrahtBot commented Apr 12, 2024 • edited Loading

Code Coverage

Reviews

maflcko commented Apr 12, 2024 • edited Loading

hebasto commented Apr 14, 2024

maflcko commented Apr 14, 2024

hebasto commented Apr 14, 2024

maflcko commented Apr 15, 2024

maflcko commented Apr 15, 2024

maflcko commented Apr 15, 2024 • edited Loading

maflcko commented Apr 16, 2024

fanquake left a comment

Choose a reason for hiding this comment

fanquake commented Apr 17, 2024

build: Fix false positive `CHECK_ATOMIC` test #29859

build: Fix false positive `CHECK_ATOMIC` test #29859

DrahtBot commented Apr 12, 2024 •

edited

Loading

maflcko commented Apr 12, 2024 •

edited

Loading

maflcko commented Apr 15, 2024 •

edited

Loading