ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED

vasild commented at 2:34 pm on January 6, 2025: contributor

For the task MSan, depends (Cirrus CI) we build a custom libc++ for which we already use -DLIBCXX_HARDENING_MODE=debug. Compile it also with _LIBCPP_ABI_BOUNDED_* to enable further checks.

Docs at: https://libcxx.llvm.org/Hardening.html#abi-options

DrahtBot commented at 2:34 pm on January 6, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31612.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko

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

DrahtBot added the label Tests on Jan 6, 2025

vasild renamed this:
ci: build msan's libc with _LIBCPP_ABI_BOUNDED_*
ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*
on Jan 6, 2025

vasild force-pushed on Jan 6, 2025

fanquake commented at 4:08 pm on January 6, 2025: member

https://cirrus-ci.com/task/5516452708483072:

0[16:01:15.125] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src && /usr/bin/ccache /usr/bin/clang++ -DABORT_ON_FAILED_ASSUME -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DDEBUG -DDEBUG_LOCKCONTENTION -DDEBUG_LOCKORDER -DRPC_DOC_CHECK -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -I/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src -I/ci_container_base/src -isystem /ci_container_base/depends/x86_64-pc-linux-gnu/include -fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls -nostdinc++ -nostdlib++ -isystem /msan/cxx_build/include/c++/v1 -L/msan/cxx_build/lib -Wl,-rpath,/msan/cxx_build/lib -lc++ -lc++abi -lpthread -Wno-unused-command-line-argument -std=c++20 -fPIE -fdebug-prefix-map=/ci_container_base/src=. -fmacro-prefix-map=/ci_container_base/src=. -Werror -fsanitize=memory -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -MD -MT src/CMakeFiles/bitcoin-wallet.dir/wallet/wallettool.cpp.o -MF CMakeFiles/bitcoin-wallet.dir/wallet/wallettool.cpp.o.d -o CMakeFiles/bitcoin-wallet.dir/wallet/wallettool.cpp.o -c /ci_container_base/src/wallet/wallettool.cpp -U_FORTIFY_SOURCE 
1[16:01:28.166] [ 23%] Linking CXX executable bitcoin-wallet
2[16:01:28.167] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src && /usr/bin/cmake -E cmake_link_script CMakeFiles/bitcoin-wallet.dir/link.txt --verbose=1
3[16:01:28.179] /usr/bin/clang++ -fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls -nostdinc++ -nostdlib++ -isystem /msan/cxx_build/include/c++/v1 -L/msan/cxx_build/lib -Wl,-rpath,/msan/cxx_build/lib -lc++ -lc++abi -lpthread -Wno-unused-command-line-argument -fsanitize=memory -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -fPIE -pie "CMakeFiles/bitcoin-wallet.dir/bitcoin-wallet.cpp.o" "CMakeFiles/bitcoin-wallet.dir/init/bitcoin-wallet.cpp.o" "CMakeFiles/bitcoin-wallet.dir/wallet/wallettool.cpp.o" -o bitcoin-wallet  wallet/libbitcoin_wallet.a libbitcoin_common.a util/libbitcoin_util.a libbitcoin_consensus.a crypto/libbitcoin_crypto.a crypto/libbitcoin_crypto_sse41.a crypto/libbitcoin_crypto_avx2.a crypto/libbitcoin_crypto_x86_shani.a secp256k1/lib/libsecp256k1.a univalue/libunivalue.a /ci_container_base/depends/x86_64-pc-linux-gnu/lib/libsqlite3.a  
4[16:01:29.288] /usr/bin/ld: util/libbitcoin_util.a(moneystr.cpp.o): in function `ParseMoney(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)':
5[16:01:29.291] /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/util/./util/moneystr.cpp:75:(.text+0xdaa): undefined reference to `std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert(std::__1::__wrap_iter<char const*>, char)'
6[16:01:29.457] clang++: error: linker command failed with exit code 1 (use -v to see invocation)
7[16:01:29.464] gmake[2]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu'

maflcko commented at 4:20 pm on January 6, 2025: member

Does that mean it needs to be passed again at build time? This seems a bit confusing, given that they are supposed to be vendor settings.

DrahtBot added the label CI failed on Jan 6, 2025

DrahtBot commented at 7:36 pm on January 6, 2025: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/35202397286

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

vasild commented at 5:32 am on January 7, 2025: contributor

@maflcko, good question. I guess the answer is “yes”. @ldionne, @var-const is it indeed the case that if libc++ itself is compiled with _LIBCPP_ABI_BOUNDED_*, then the user programs that link against such a libc++ must be compiled with _LIBCPP_ABI_BOUNDED_* as well?

In this PR we have a libc++ with and a user program without _LIBCPP_ABI_BOUNDED_* and it results in:

0undefined reference to `std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert(std::__1::__wrap_iter<char const*>, char)'

In #31424 we have libc++ without and user program with:

0undefined reference to `std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert(std::__1::__bounded_iter<std::__1::__wrap_iter<char const*>, void>, char)'

ldionne commented at 2:42 pm on January 7, 2025: none

Our documentation is misleading. We shouldn’t mention _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING and friends the way we do it – you’re never intended to set these macros yourself. Instead, you must pass them at CMake configuration time as -DLIBCXX_ABI_DEFINES="_LIBCPP_ABI_BOUNDED_ITERATORS;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STD_ARRAY;etc".

The difference is that passing them at CMake configuration time (when building libc++) will result in these macros being baked into the library headers (look at the __config_site header), and that means that you get the right definition for these macros both when building libc++ and when using it. I’ll update the documentation.

ldionne referenced this in commit 180dd2a27f on Jan 7, 2025

ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*

For the task `MSan, depends (Cirrus CI)` we build a custom libc++ for
which we already use `-DLIBCXX_HARDENING_MODE=debug`. Compile it also
with `_LIBCPP_ABI_BOUNDED_*` to enable further checks.

Docs at: https://libcxx.llvm.org/Hardening.html#abi-options

fb37acd932

vasild force-pushed on Jan 7, 2025

vasild commented at 3:01 pm on January 7, 2025: contributor

Alright, changed that to

-DLIBCXX_ABI_DEFINES="_LIBCPP_ABI_BOUNDED_ITERATORS;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STD_ARRAY;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING;_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR;_LIBCPP_ABI_BOUNDED_UNIQUE_PTR"

when compiling libc++.

vasild commented at 3:08 pm on January 7, 2025: contributor

Hmm, once libc++ itself is compiled with _LIBCPP_ABI_BOUNDED_* does the user program have to be compiled with those as well, e.g. by passing -D_LIBCPP_ABI_BOUNDED_ITERATORS to the compiler? Or will those extra checks be enabled regardless of how the user program is compiled?

maflcko commented at 3:40 pm on January 7, 2025: member

review ACK fb37acd932b06c2386d07efe88a65ee3ac9aaa24

ldionne commented at 4:35 pm on January 7, 2025: none

Hmm, once libc++ itself is compiled with _LIBCPP_ABI_BOUNDED_* does the user program have to be compiled with those as well, e.g. by passing -D_LIBCPP_ABI_BOUNDED_ITERATORS to the compiler? Or will those extra checks be enabled regardless of how the user program is compiled?

Users don’t have anything to do, and in fact they shouldn’t try to do anything. The idea is that ABI macros are not knobs that can be changed by “end users” (as in people using libc++), they are things that must be baked in when the library is configured and never changed afterwards, since they affect the library’s ABI.

TLDR, configure the library at CMake configuration time the way you did and you’re done!

DrahtBot removed the label CI failed on Jan 7, 2025

vasild commented at 5:05 pm on January 7, 2025: contributor

Excellent, that will simplify #31424

Thank you!

ldionne referenced this in commit 1855333e3a on Jan 7, 2025

fanquake merged this on Jan 10, 2025

fanquake closed this on Jan 10, 2025

ci: build msan’s libc++ with LIBCPP_ABI_BOUNDED* #31612

Code Coverage & Benchmarks

Reviews

ci: build msan’s libc++ with *LIBCPP_ABI_BOUNDED** #31612