Bumps LLVM to 18.1.3:
Drops --with-asm=no (only being passed to secp256k1) from the MSAN CI. New MSAN annotations were pulled in as part of #29803.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For detailed information about the code coverage, see the test coverage report.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | hebasto, maflcko |
| Concept ACK | theuni, real-or-random |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Nice. Obvious concept ACK. Can discuss the secp subtree bump separately.
Why is the llvm 18.1.2 bump bundled here, though?
Why is the llvm 18.1.2 bump bundled here, though?
Mostly just because I was going to include another MSAN patch, to fix the instrumentation on libuwind (https://github.com/llvm/llvm-project/issues/84348) but haven't quite got that working yet.
Note that depending on timings, if https://github.com/llvm/llvm-project/pull/86201 goes into the next 18.x release, I'll want to bump/pull that in too, because it'll remove the need for a workaround when running the CI locally on newer kernels (same issue as this one https://github.com/bitcoin-core/secp256k1/issues/1506). Happy to split up the changes in any case, just had a MSAN bundle to test here.
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.
Possibly this is 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.
Leave a comment here, if you need help tracking down a confusing failure.
<sub>Debug: https://github.com/bitcoin/bitcoin/runs/23120272058</sub>
Working ok locally.
Seems to be failing in CI. I guess due to build-architecture differences, or is there a bug in the CI system?
Seems to be failing in CI. I guess due to build-architecture differences, or is there a bug in the CI system?
I think so: aarch64 native_msan: pass native_fuzz_with_msan: pass
x86_64 native_msan: fail: (reported here: https://github.com/bitcoin-core/secp256k1/issues/1511). pass: with the secp256k1 tests disabled. native_fuzz_with_msan: pass
Ready for bump now that https://github.com/bitcoin-core/secp256k1/pull/1512 is merged.
Ready for bump now that https://github.com/bitcoin-core/secp256k1/pull/1512 is merged.
Converted to a proper subtree bump. Rewrote the PR description.
Concept ACK on removing --with-asm=no
16 | @@ -17,7 +17,7 @@ export PACKAGES="ninja-build" 17 | # BDB generates false-positives and will be removed in future 18 | export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" 19 | export GOAL="install" 20 | -export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" 21 | +export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CFLAGS='${MSAN_FLAGS}' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'
nit: While touching, can remove the duplicate CFLAGS and CXXFLAGS. Same below.
Opened https://github.com/google/oss-fuzz/pull/11785 for oss-fuzz.
Also cc @jonasnick.
ACK 61641e2466768e128fef995e9fcb24cad90e527d.
Have run this successfully on all 4 MSAN jobs locally.
lgtm ACK 61641e2466768e128fef995e9fcb24cad90e527d