valgrind currently does not work on GCC compiled executables, due to an upstream bug. https://bugs.kde.org/show_bug.cgi?id=472329
So temporarily switch to clang, so that a long term solution can be figured out in the meantime.
valgrind currently does not work on GCC compiled executables, due to an upstream bug. https://bugs.kde.org/show_bug.cgi?id=472329
So temporarily switch to clang, so that a long term solution can be figured out in the meantime.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--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.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
16 | @@ -17,5 +17,6 @@ export FUZZ_TESTS_CONFIG="--valgrind" 17 | export GOAL="all" 18 | export BITCOIN_CONFIG="\ 19 | -DBUILD_FOR_FUZZING=ON \ 20 | - -DCMAKE_CXX_FLAGS='-Wno-error=array-bounds' \ 21 | + -DCMAKE_C_COMPILER=clang \ 22 | + -DCMAKE_CXX_COMPILER=clang++ \
is the removed no-error=array-bounds related to the mentioned bug?
No, this is just a gcc false-positive, which can be removed, because clang is used
thx, added to the commit msg
5 | @@ -6,9 +6,9 @@ 6 | 7 | export LC_ALL=C.UTF-8 8 | 9 | -export CI_IMAGE_NAME_TAG="mirror.gcr.io/debian:trixie" 10 | +export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:26.04"
container image version
it's not strictly just the version, is this part of a unification attempts?
It will have to be bumped anyway at some point, so might as well do it now. It shouldn't matter much
yeah, but is there an ubuntu 26.04 version? In other cases we've used the24.04 LTS.
https://releases.ubuntu.com/26.04-snapshot1 seems empty.
The build is passing (though I don't see the valgrind tasks referenced by the current GH Actions matrix anymore), not sure what I'm missing :/
Added to the commit msg:
I still don't understand the 26.04 tag - is that a custom snapshot that we want to use temporarily? We don't usually depend on unreleased OS, I'm confused why it's necessary here.
I don't see a downside here to using it, and it has the benefit of using the latest clang compiler version and the latest valgrind version, so it is nice to know that they are working. I can keep it, but then someone will likely bump it later.
(reverted for now, can be done in a follow-up i guess)
11 | @@ -12,13 +12,28 @@ 12 | # --error-limit=no build/bin/test_bitcoin 13 | # 14 | # Note that suppressions may depend on OS and/or library versions. 15 | -# Tested on aarch64 and x86_64 with Ubuntu Noble system libs, using clang-16 16 | -# and GCC, without gui. 17 | +# Tested on x86_64 with Ubuntu 26.04 system libs, using clang, without gui. 18 | +{
the title claims switching compiler, but I don't know how to review these lines
Reverting them will make the CI fail.
I think you can also test locally by compiling with clang-21 and then running:
./bld-cmake/test/functional/interface_bitcoin_cli.py --valgrind
I tried this on an RPI5 with Clang version 22.0.0 (aarch64) for fa173ec59a9dcc2e7956366344d68999d1d1249e.
It seems to trigger similar valgrind false positives on aarch64 - the updated suppression doesn't seem to cover it, see:
2026-02-14T19:42:42.017831Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/mnt/my_storage/bitcoin/test/functional/test_framework/test_framework.py", line 138, in main
self.setup()
~~~~~~~~~~^^
File "/mnt/my_storage/bitcoin/test/functional/test_framework/test_framework.py", line 272, in setup
self.setup_network()
~~~~~~~~~~~~~~~~~~^^
File "/mnt/my_storage/bitcoin/test/functional/test_framework/test_framework.py", line 363, in setup_network
self.setup_nodes()
~~~~~~~~~~~~~~~~^^
File "/mnt/my_storage/bitcoin/test/functional/test_framework/test_framework.py", line 385, in setup_nodes
self.start_nodes()
~~~~~~~~~~~~~~~~^^
File "/mnt/my_storage/bitcoin/test/functional/test_framework/test_framework.py", line 531, in start_nodes
node.wait_for_rpc_connection()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/mnt/my_storage/bitcoin/test/functional/test_framework/test_node.py", line 316, in wait_for_rpc_connection
raise FailedToStartError(self._node_msg(
f'bitcoind exited with status {self.process.returncode} during initialization. {str_error}'))
test_framework.test_node.FailedToStartError: [node 0] bitcoind exited with status 1 during initialization. ==68399== Use of uninitialised value of size 8
==68399== at 0x22AD68: _M_is_engaged (optional:563)
==68399== by 0x22AD68: operator=<node::BlockfileCursor> (optional:1036)
==68399== by 0x22AD68: node::BlockManager::LoadBlockIndexDB(std::optional<uint256> const&) (???:565)
==68399== by 0x46C717: ChainstateManager::LoadBlockIndex() (../src/validation.cpp:4939)
==68399== by 0x241A8F: node::CompleteChainstateInitialization(ChainstateManager&, node::ChainstateLoadOptions const&) (../src/node/chainstate.cpp:43)
==68399== by 0x24142B: node::LoadChainstate(ChainstateManager&, kernel::CacheSizes const&, node::ChainstateLoadOptions const&) (../src/node/chainstate.cpp:184)
==68399== by 0x1950C7: operator() (../src/init.cpp:1392)
==68399== by 0x1950C7: operator()<(lambda at ../src/init.cpp:1392:45)> (???:1386)
==68399== by 0x1950C7: InitAndLoadChainstate(node::NodeContext&, bool, bool, kernel::CacheSizes const&, ArgsManager const&) (???:1392)
==68399== by 0x18FE6B: AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) (../src/init.cpp:1809)
==68399== by 0x17BA97: AppInit (../src/bitcoind.cpp:242)
==68399== by 0x17BA97: main (???:283)
==68399==
{
<insert_a_suppression_name_here>
Memcheck:Value8
fun:_M_is_engaged
fun:operator=<node::BlockfileCursor>
fun:_ZN4node12BlockManager16LoadBlockIndexDBERKSt8optionalI7uint256E
fun:_ZN17ChainstateManager14LoadBlockIndexEv
fun:_ZN4nodeL32CompleteChainstateInitializationER17ChainstateManagerRKNS_21ChainstateLoadOptionsE
fun:_ZN4node14LoadChainstateER17ChainstateManagerRKN6kernel10CacheSizesERKNS_21ChainstateLoadOptionsE
fun:operator()
fun:operator()<(lambda at ../src/init.cpp:1392:45)>
fun:_ZL21InitAndLoadChainstateRN4node11NodeContextEbbRKN6kernel10CacheSizesERK11ArgsManager
fun:_Z11AppInitMainRN4node11NodeContextEPN10interfaces21BlockAndHeaderTipInfoE
fun:AppInit
fun:main
}
==68399==
==68399== Exit program on first error (--exit-on-first-error=yes)
************************
concept ACK
Could you please add a link to the bug for more context and split the change to commits that explain all the different changes. And it's "temporary", can we add a todo to the code to make it official :)?
And it's "temporary", can we add a todo to the code to make it official :)?
It is not clear what to do, so a tracking issue seems more appropriate than a vague todo.
Could you please add a link to the bug for more context and split the change to commits that explain all the different changes.
The upstream bugs are https://bugs.kde.org/show_bug.cgi?id=485276 and https://bugs.kde.org/show_bug.cgi?id=472329
rebased and ready for nack/ack again
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
This allows to run the test under valgrind:
./bld-cmake/test/functional/feature_dbcrash.py --timeout-factor=10 --valgrind
For testing, the same test can be run multiple times in parallel:
./bld-cmake/test/functional/test_runner.py -j 10 $( printf 'feature_dbcrash.py %.0s' {1..10} ) --timeout-factor=10 --valgrind
(Running the test under valgrind may take several hours!)
I found that before this commit, 9 out of the 10 runs failed via:
```
...
TestFramework (INFO): Iteration 36, generating 2500 transactions [11, 5, 6]
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/b-c/test/functional/test_framework/test_framework.py", line 142, in main
self.run_test()
~~~~~~~~~~~~~^^
File "/b-c/bld-cmake/test/functional/feature_dbcrash.py", line 262, in run_test
self.sync_node3blocks(block_hashes)
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
File "/b-c/bld-cmake/test/functional/feature_dbcrash.py", line 151, in sync_node3blocks
nodei_utxo_hash = self.restart_node(i, block_hash)
File "/b-c/bld-cmake/test/functional/feature_dbcrash.py", line 102, in restart_node
raise AssertionError(f"Unable to successfully restart node {node_index} in allotted time")
AssertionError: Unable to successfully restart node 0 in allotted time
```
With this commit, all 10 runs passed.11 | @@ -12,8 +12,7 @@ 12 | # --error-limit=no build/bin/test_bitcoin 13 | # 14 | # Note that suppressions may depend on OS and/or library versions. 15 | -# Tested on aarch64 and x86_64 with Ubuntu Noble system libs, using clang-16 16 | -# and GCC, without gui. 17 | +# Tested on x86_64 with Debian Trixie system libs, using clang, without gui.
No, I have not tested the latest push at all. I've only tested this on Ubuntu 26.04. It should be easy to re-test by manually spinning up a x86_64 box and running the two CI valgrind configs, which use Debian Trixie.
Code review ACK fa6ba11768cfa27d60b43ec43e84e88980837e74
16 | @@ -17,7 +17,9 @@ export TEST_RUNNER_EXTRA="--exclude rpc_bind --exclude feature_bind_extra"
17 | export GOAL="install"
18 | # TODO enable GUI
I wonder why this isn't enabled. Let me try to enable it ...
Edit: Running with system libs gives:
$ valgrind --gen-suppressions=all --exit-on-first-error=yes --error-exitcode=1 --quiet ./bld-cmake/bin/test_bitcoin-qt
...
==906896== Conditional jump or move depends on uninitialised value(s)
==906896== at 0x243F0E26: ???
==906896== by 0x1F08261F: ???
==906896==
{
<insert_a_suppression_name_here>
Memcheck:Cond
obj:*
obj:*
}
==906896==
==906896== Exit program on first error (--exit-on-first-error=yes)
Only seems to pass with DEBUG=1 depends.
For reference, the following command did pass for me:
( cd depends && make DEBUG=1 -j $(nproc) ) && cmake --fresh -B ./bld-cmake --toolchain depends/*/toolchain.cmake -DAPPEND_CXXFLAGS='-O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=mold -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' --preset=dev-mode -DCMAKE_EXPORT_COMPILE_COMMANDS=ON && cmake --build ./bld-cmake --parallel $(nproc) && valgrind --exit-on-first-error=yes --error-exitcode=1 --quiet ./bld-cmake/bin/test_bitcoin-qt
Pushed a new commit to remove the TODO for now.
I've also tried:
diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh
index 4a17fb4..0e1697b 100755
--- a/ci/test/00_setup_env_native_msan.sh
+++ b/ci/test/00_setup_env_native_msan.sh
@@ -16,14 +16,13 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}"
export CONTAINER_NAME="ci_native_msan"
export PACKAGES="clang-${APT_LLVM_V} llvm-${APT_LLVM_V} llvm-${APT_LLVM_V}-dev libclang-${APT_LLVM_V}-dev libclang-rt-${APT_LLVM_V}-dev python3-pip"
export PIP_PACKAGES="--break-system-packages pycapnp"
-export DEP_OPTS="DEBUG=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
+export DEP_OPTS="DEBUG=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
export GOAL="install"
export CI_LIMIT_STACK_SIZE=1
# Setting CMAKE_{C,CXX}_FLAGS_DEBUG flags to an empty string ensures that the flags set in MSAN_FLAGS remain unaltered.
# _FORTIFY_SOURCE is not compatible with MSAN.
export BITCOIN_CONFIG="\
--preset=dev-mode \
- -DBUILD_GUI=OFF \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_C_FLAGS_DEBUG='' \
-DCMAKE_CXX_FLAGS_DEBUG='' \
Which prints:
All tests passed.
~InitExecutor : Stopping thread
~InitExecutor : Stopped thread
==87986==WARNING: MemorySanitizer: use-of-uninitialized-value
[#0](/bitcoin-bitcoin/0/) 0x56145624af4f in QLibraryStore::cleanup() /usr/qtbase/src/corelib/plugin/qlibrary.cpp:364:26
[#1](/bitcoin-bitcoin/1/) 0x56145623d05c in qlibraryCleanup() /usr/qtbase/src/corelib/plugin/qlibrary.cpp:378:5
[#2](/bitcoin-bitcoin/2/) 0x56145623d05c in (anonymous namespace)::qlibraryCleanup_dtor_class_::~qlibraryCleanup_dtor_class_() /usr/qtbase/src/corelib/plugin/qlibrary.cpp:380:23
[#3](/bitcoin-bitcoin/3/) 0x561450f4884f in MSanCxaAtExitWrapper(void*) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/test_bitcoin-qt+0x2df84f) (BuildId: 0a3f6add7bb2dd2a7bfc097a6edf88a12aebb62f)
[#4](/bitcoin-bitcoin/4/) 0x7fb04940da75 (/lib/x86_64-linux-gnu/libc.so.6+0x47a75) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#5](/bitcoin-bitcoin/5/) 0x7fb04940dbbd in exit (/lib/x86_64-linux-gnu/libc.so.6+0x47bbd) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#6](/bitcoin-bitcoin/6/) 0x7fb0493f01d0 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1d0) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#7](/bitcoin-bitcoin/7/) 0x7fb0493f028a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#8](/bitcoin-bitcoin/8/) 0x561450f16fb4 in _start (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/test_bitcoin-qt+0x2adfb4) (BuildId: 0a3f6add7bb2dd2a7bfc097a6edf88a12aebb62f)
Member fields were destroyed
[#0](/bitcoin-bitcoin/0/) 0x561450f47de1 in __sanitizer_dtor_callback_fields (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/test_bitcoin-qt+0x2dede1) (BuildId: 0a3f6add7bb2dd2a7bfc097a6edf88a12aebb62f)
[#1](/bitcoin-bitcoin/1/) 0x561455b213ae in QLoggingCategory::~QLoggingCategory() /usr/qtbase/src/corelib/io/qloggingcategory.h:41:32
[#2](/bitcoin-bitcoin/2/) 0x561455b213ae in QLoggingCategory::~QLoggingCategory() /usr/qtbase/src/corelib/io/qloggingcategory.cpp:194:1
[#3](/bitcoin-bitcoin/3/) 0x561450f4884f in MSanCxaAtExitWrapper(void*) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/test_bitcoin-qt+0x2df84f) (BuildId: 0a3f6add7bb2dd2a7bfc097a6edf88a12aebb62f)
[#4](/bitcoin-bitcoin/4/) 0x7fb04940dbbd in exit (/lib/x86_64-linux-gnu/libc.so.6+0x47bbd) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#5](/bitcoin-bitcoin/5/) 0x7fb0493f01d0 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1d0) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#6](/bitcoin-bitcoin/6/) 0x7fb0493f028a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
[#7](/bitcoin-bitcoin/7/) 0x561450f16fb4 in _start (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/test_bitcoin-qt+0x2adfb4) (BuildId: 0a3f6add7bb2dd2a7bfc097a6edf88a12aebb62f)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /usr/qtbase/src/corelib/plugin/qlibrary.cpp:364:26 in QLibraryStore::cleanup()
Exiting
99% tests passed, 1 tests failed out of 157
Total Test time (real) = 608.27 sec
The following tests FAILED:
7 - test_bitcoin-qt (Failed)
Errors while running CTest
Command '['/ci_container_base/ci/test/03_test_script.sh']' returned non-zero exit status 8.
Code review ACK fac9ae4c931f1b0ad176d2558fa767de04d89521
A build with system libs (or with a normal depends build) will fail
with:
```sh
$ valgrind --exit-on-first-error=yes --error-exitcode=1 --quiet ./bld-cmake/bin/test_bitcoin-qt
Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, and has switched to "C.UTF-8" instead.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.
********* Start testing of AppTests *********
Config: Using QtTest library 6.10.2, Qt 6.10.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 15.2.0), ubuntu 26.04
PASS : AppTests::initTestCase()
QINFO : AppTests::appTests() Backing up GUI settings to "/tmp/test_common bitcoin/60d474ffae390f81657d/regtest/guisettings.ini.bak"
==18007== Conditional jump or move depends on uninitialised value(s)
==18007== at 0x12655E26: ???
==18007== by 0xCB28E7F: ???
==18007==
==18007==
==18007== Exit program on first error (--exit-on-first-error=yes)
```
A DEBUG=1 depends build would work, but that seems tedious for
questionable benefit.
valgrind currently does not work on GCC -O2 compiled executables, which
contain std::optional use, due to an upstream bug. See
https://bugs.kde.org/show_bug.cgi?id=472329
One workaround could be to use -O1. However, that seems brittle, as
variantions of the bug were seen with -O1 as well.
So temporarily use clang in the valgrind CI tasks, because this also
allows to drop a false-positive suppression for:
-DCMAKE_CXX_FLAGS='-Wno-error=array-bounds'
Also, update the comment in contrib/valgrind.supp to mention the
background:
* GCC -O2 wasn't tested with the suppressions file, due to the mentioned
bug.
* Clang-17 (or later) on aarch64 wasn't tested due to bug
https://github.com/bitcoin/bitcoin/issues/29635 and the minimum
supported clang version is clang-17 right now.
* GUI isn't tested, because it requires a debug build, see the prior
commit.
This means the only tested config right now is the one mentioned in the
suppression file.
Reordered the last two commits and pushed a doc-only update.
ACK fa70b9ebaa46b5fedcc02200626499d25762d44a
ACK fa70b9ebaa46b5fedcc02200626499d25762d44a - also checked that it doesn't currently work under aarch64.