valgrind: Conditional jump or move depends on uninitialised value(s)

fanquake commented at 2:07 pm on March 12, 2024: member

I see this issue with latest master using Clang 17.0.6 on aarch64, and running the functional tests under Valgrind (3.22.0):

 0 node0 stderr ==75935== Thread 25 b-msghand:
 1==75935== Conditional jump or move depends on uninitialised value(s)
 2==75935==    at 0x1955B8: _M_reset (optional:313)
 3==75935==    by 0x1955B8: ~_Optional_payload (optional:437)
 4==75935==    by 0x1955B8: ~_Optional_base (optional:508)
 5==75935==    by 0x1955B8: GetLocalAddress(CNode const&) (???:220)
 6==75935==    by 0x1956A3: GetLocalAddrForPeer(CNode&) (net.cpp:240)
 7==75935==    by 0x1D091F: MaybeSendAddr (net_processing.cpp:5259)
 8==75935==    by 0x1D091F: (anonymous namespace)::PeerManagerImpl::SendMessages(CNode*) (???:5453)
 9==75935==    by 0x1AA183: CConnman::ThreadMessageHandler() (net.cpp:2886)
10==75935==    by 0x750627: operator() (std_function.h:591)
11==75935==    by 0x750627: util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) (???:21)
12==75935==    by 0x1B290F: __invoke_impl<void, void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3231:71)> (invoke.h:61)
13==75935==    by 0x1B290F: __invoke<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3231:71)> (invoke.h:96)
14==75935==    by 0x1B290F: _M_invoke<0UL, 1UL, 2UL> (std_thread.h:292)
15==75935==    by 0x1B290F: operator() (std_thread.h:299)
16==75935==    by 0x1B290F: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), char const*, CConnman::Start(CScheduler&, CConnman::Options const&)::$_5> > >::_M_run() (std_thread.h:244)
17==75935==    by 0x4C501BF: execute_native_thread_routine (thread.cc:104)
18==75935==    by 0x4F85E37: start_thread (pthread_create.c:447)
19==75935==    by 0x4FF0E5B: thread_start (clone.S:79)
20==75935== 
21{
22   <insert_a_suppression_name_here>
23   Memcheck:Cond
24   fun:_M_reset
25   fun:~_Optional_payload
26   fun:~_Optional_base
27   fun:_Z15GetLocalAddressRK5CNode
28   fun:_Z19GetLocalAddrForPeerR5CNode
29   fun:MaybeSendAddr
30   fun:_ZN12_GLOBAL__N_115PeerManagerImpl12SendMessagesEP5CNode
31   fun:_ZN8CConnman20ThreadMessageHandlerEv
32   fun:operator()
33   fun:_ZN4util11TraceThreadESt17basic_string_viewIcSt11char_traitsIcEESt8functionIFvvEE
34   fun:__invoke_impl<void, void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3231:71)>
35   fun:__invoke<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3231:71)>
36   fun:_M_invoke<0UL, 1UL, 2UL>
37   fun:operator()
38   fun:_ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvSt17basic_string_viewIcSt11char_traitsIcEESt8functionIFvvEEEPKcZN8CConnman5StartER10CSchedulerRKNSE_7OptionsEE3$_5EEEEE6_M_runEv
39   fun:execute_native_thread_routine
40   fun:start_thread
41   fun:thread_start
42}
43==75935== 
44==75935== Exit program on first error (--exit-on-first-error=yes)

Bisected to #28078 being the cause.

vasild commented at 2:21 pm on March 12, 2024: contributor

For the records, master is at d14c7286b653ad09a5a196aa351b325f97db546b (in case this vanishes due to some change in master).

I guess it must be 5316ae5dd8 from #28078. That commit can be reversed cleanly on master: git show 5316ae5dd8 |git apply -R. Can you try if the problem disappears if you do that? If it does, I wouldn’t still revert it on master without understanding what the problem is actually.

Which test is causing this?

cc @jonatack, @stickies-v.

fanquake commented at 2:28 pm on March 12, 2024: member

Which test is causing this?

It will likely show up with almost any test i.e you can run example_test.py:

 0./autogen.sh && ./configure CC=clang CXX=clang++ && make -C src bitcoind && test/functional/test_runner.py example_test.py --combinedlogslen=19999 --timeout-factor=12 --valgrind
 1.....
 2 test  2024-03-12T14:26:46.321000Z TestFramework.node0 (DEBUG): Stopping node 
 3
 4 node0 stderr ==205278== Thread 26 b-msghand:
 5==205278== Conditional jump or move depends on uninitialised value(s)
 6==205278==    at 0x1934C4: _M_reset (optional:313)
 7==205278==    by 0x1934C4: ~_Optional_payload (optional:437)
 8==205278==    by 0x1934C4: ~_Optional_base (optional:508)
 9==205278==    by 0x1934C4: GetLocalAddress(CNode const&) (???:220)
10==205278==    by 0x1935B3: GetLocalAddrForPeer(CNode&) (net.cpp:240)
11==205278==    by 0x1D0F87: MaybeSendAddr (net_processing.cpp:5401)
12==205278==    by 0x1D0F87: (anonymous namespace)::PeerManagerImpl::SendMessages(CNode*) (???:5585)
13==205278==    by 0x1A92C7: CConnman::ThreadMessageHandler() (net.cpp:2918)
14==205278==    by 0x79017F: operator() (std_function.h:591)
15==205278==    by 0x79017F: util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) (???:21)
16==205278==    by 0x1B2653: __invoke_impl<void, void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3267:71)> (invoke.h:61)
17==205278==    by 0x1B2653: __invoke<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3267:71)> (invoke.h:96)
18==205278==    by 0x1B2653: _M_invoke<0UL, 1UL, 2UL> (std_thread.h:292)
19==205278==    by 0x1B2653: operator() (std_thread.h:299)
20==205278==    by 0x1B2653: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), char const*, CConnman::Start(CScheduler&, CConnman::Options const&)::$_6> > >::_M_run() (std_thread.h:244)
21==205278==    by 0x4C501BF: execute_native_thread_routine (thread.cc:104)
22==205278==    by 0x4F85E37: start_thread (pthread_create.c:447)
23==205278==    by 0x4FF0E5B: thread_start (clone.S:79)
24==205278== 
25{
26   <insert_a_suppression_name_here>
27   Memcheck:Cond
28   fun:_M_reset
29   fun:~_Optional_payload
30   fun:~_Optional_base
31   fun:_Z15GetLocalAddressRK5CNode
32   fun:_Z19GetLocalAddrForPeerR5CNode
33   fun:MaybeSendAddr
34   fun:_ZN12_GLOBAL__N_115PeerManagerImpl12SendMessagesEP5CNode
35   fun:_ZN8CConnman20ThreadMessageHandlerEv
36   fun:operator()
37   fun:_ZN4util11TraceThreadESt17basic_string_viewIcSt11char_traitsIcEESt8functionIFvvEE
38   fun:__invoke_impl<void, void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3267:71)>
39   fun:__invoke<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3267:71)>
40   fun:_M_invoke<0UL, 1UL, 2UL>
41   fun:operator()
42   fun:_ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvSt17basic_string_viewIcSt11char_traitsIcEESt8functionIFvvEEEPKcZN8CConnman5StartER10CSchedulerRKNSE_7OptionsEE3$_6EEEEE6_M_runEv
43   fun:execute_native_thread_routine
44   fun:start_thread
45   fun:thread_start
46}
47==205278== 
48==205278== Exit program on first error (--exit-on-first-error=yes) 
49
50TEST            | STATUS    | DURATION
51
52example_test.py | ✖ Failed  | 15 s
53
54ALL             | ✖ Failed  | 15 s (accumulated) 
55Runtime: 15 s

vasild commented at 4:02 pm on March 12, 2024: contributor

@fanquake, what’s in those lines in your optional header?

cat -n /usr/include/c++/13.2.1/optional |grep -B5 -A5 -E '^[[:space:]]*(313|437|508)'

(that 13.2.1 will probably be different in your environment).

vasild commented at 7:07 am on March 13, 2024: contributor

A blind attempt, would this fix it?

 0--- i/src/net.cpp
 1+++ w/src/net.cpp
 2@@ -214,13 +214,17 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn)
 3 // Determine the "best" local address for a particular peer.
 4 // If none, return the unroutable 0.0.0.0 but filled in with
 5 // the normal parameters, since the IP may be changed to a useful
 6 // one by discovery.
 7 CService GetLocalAddress(const CNode& peer)
 8 {
 9-    return GetLocal(peer).value_or(CService{CNetAddr(), GetListenPort()});
10+    auto a = GetLocal(peer);
11+    if (a.has_value()) {
12+        return a.value();
13+    }
14+    return CService{CNetAddr(), GetListenPort()};
15 }
16 
17 static int GetnScore(const CService& addr)

maflcko commented at 7:26 am on March 13, 2024: member

@vasild This is a false positive. I am not sure about changing the source code to accommodate broken (test-only?) tools.

maflcko added the label Upstream on Mar 13, 2024

maflcko added the label Tests on Mar 13, 2024

maflcko commented at 7:27 am on March 13, 2024: member

The gcc duplicate is #27741

vasild commented at 8:24 am on March 13, 2024: contributor

I am not saying to commit the above to master, but just to check if it fixes the problem. If it does, then this is a proof that it is a problem outside of our source code. You seem to be convinced already.

maflcko commented at 8:31 am on March 13, 2024: member

You seem to be convinced already.

Yes, it seems highly unlikely that no one ran into this UB on any platform, except for a single specific compiler, with a specific compiler version, on a specific arch, with a specific version of valgrind. If this isn’t a valgrind upstream bug, it would mean that the current Bitcoin Core testing efforts are highly insufficient.

maflcko commented at 11:33 am on March 13, 2024: member

I couldn’t reproduce on a fresh Ubuntu 24.04 Noble:

0export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git  --depth=1 ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3     libevent-dev libboost-dev clang valgrind   -y   &&  ./autogen.sh && ./configure   CC=clang CXX=clang++ && make -j $(nproc) && test/functional/test_runner.py example_test.py --valgrind
1
2root@659a40f0d803:/bitcoin-core# clang --version 
3Ubuntu clang version 17.0.6 (5build1)
4Target: aarch64-unknown-linux-gnu
5Thread model: posix
6InstalledDir: /usr/bin
7root@659a40f0d803:/bitcoin-core# valgrind --version 
8valgrind-3.22.0

fanquake commented at 11:45 am on March 13, 2024: member

Still produces for me on Fedora:

 0./autogen.sh && ./configure CC=clang CXX=clang++ && make -C src bitcoind -j17
 1test/functional/test_runner.py example_test.py --combinedlogslen=19999 --timeout-factor=12 --valgrind
 2...
 3 node0 stderr ==12061== Thread 25 b-msghand:
 4==12061== Conditional jump or move depends on uninitialised value(s)
 5==12061==    at 0x193CAC: _M_reset (optional:313)
 6==12061==    by 0x193CAC: ~_Optional_payload (optional:437)
 7==12061==    by 0x193CAC: ~_Optional_base (optional:508)
 8==12061==    by 0x193CAC: GetLocalAddress(CNode const&) (???:220)
 9==12061==    by 0x193D9B: GetLocalAddrForPeer(CNode&) (net.cpp:240)
10==12061==    by 0x1D1963: MaybeSendAddr (net_processing.cpp:5407)
11==12061==    by 0x1D1963: (anonymous namespace)::PeerManagerImpl::SendMessages(CNode*) (???:5591)
12==12061==    by 0x1A9F43: CConnman::ThreadMessageHandler() (net.cpp:2906)
13==12061==    by 0x7925F3: operator() (std_function.h:591)
14==12061==    by 0x7925F3: util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) (???:21)
15==12061==    by 0x1B3383: __invoke_impl<void, void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3255:71)> (invoke.h:61)
16==12061==    by 0x1B3383: __invoke<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3255:71)> (invoke.h:96)
17==12061==    by 0x1B3383: _M_invoke<0UL, 1UL, 2UL> (std_thread.h:292)
18==12061==    by 0x1B3383: operator() (std_thread.h:299)
19==12061==    by 0x1B3383: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), char const*, CConnman::Start(CScheduler&, CConnman::Options const&)::$_6> > >::_M_run() (std_thread.h:244)
20==12061==    by 0x4C501BF: execute_native_thread_routine (thread.cc:104)
21==12061==    by 0x4F85E37: start_thread (pthread_create.c:447)
22==12061==    by 0x4FF0E5B: thread_start (clone.S:79)
23==12061== 
24{
25   <insert_a_suppression_name_here>
26   Memcheck:Cond
27   fun:_M_reset
28   fun:~_Optional_payload
29   fun:~_Optional_base
30   fun:_Z15GetLocalAddressRK5CNode
31   fun:_Z19GetLocalAddrForPeerR5CNode
32   fun:MaybeSendAddr
33   fun:_ZN12_GLOBAL__N_115PeerManagerImpl12SendMessagesEP5CNode
34   fun:_ZN8CConnman20ThreadMessageHandlerEv
35   fun:operator()
36   fun:_ZN4util11TraceThreadESt17basic_string_viewIcSt11char_traitsIcEESt8functionIFvvEE
37   fun:__invoke_impl<void, void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3255:71)>
38   fun:__invoke<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>), const char *, (lambda at net.cpp:3255:71)>
39   fun:_M_invoke<0UL, 1UL, 2UL>
40   fun:operator()
41   fun:_ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvSt17basic_string_viewIcSt11char_traitsIcEESt8functionIFvvEEEPKcZN8CConnman5StartER10CSchedulerRKNSE_7OptionsEE3$_6EEEEE6_M_runEv
42   fun:execute_native_thread_routine
43   fun:start_thread
44   fun:thread_start
45}
46==12061== 
47==12061== Exit program on first error (--exit-on-first-error=yes) 
48
49TEST            | STATUS    | DURATION
50
51example_test.py | ✖ Failed  | 15 s
52
53ALL             | ✖ Failed  | 15 s (accumulated) 
54Runtime: 15 s

Note that this is now with Clang 18.1.0:

0clang --version
1clang version 18.1.0 (Fedora 18.1.0~rc4-2.fc41)
2Target: aarch64-redhat-linux-gnu
3Thread model: posix
4InstalledDir: /usr/bin
5Configuration file: /etc/clang/clang.cfg

vasild commented at 12:16 pm on March 13, 2024: contributor

The bug could be inside /usr/include/c++/.../optional.

maflcko commented at 12:58 pm on March 13, 2024: member

The bug could be inside /usr/include/c++/.../optional.

That’d be even more unlikely, that the bug is in a std lib header file, and it triggers for no one, except for a single specific line, in a specific project, on a specific arch, on a specific compiler, on a specific compiler version.

maflcko commented at 1:25 pm on March 13, 2024: member

The only alternative I could see is that the bug is in the optimizer (compiler), but assuming that the compiler receives more testing than valgrind, it is still more likely that the bug is in valgrind.

maflcko commented at 3:20 pm on March 13, 2024: member

Note that this is now with Clang 18.1.0:

Ok, so that seems like a typo in the initial report? Same here:

0root@659a40f0d803:/bitcoin-core# clang++-18 --version 
1Ubuntu clang version 18.1.0 (rc2-4)
2Target: aarch64-unknown-linux-gnu
3Thread model: posix
4InstalledDir: /usr/bin

fanquake commented at 3:21 pm on March 13, 2024: member

Ok, so that seems like a typo in the initial report?

No, it was Clang 17 yesterday.

fanquake commented at 3:25 pm on March 13, 2024: member

It seems like the easiest path forward here is to suppress the two assumed Valgrind false positives, linking to the upstream report.

maflcko commented at 8:03 am on March 14, 2024: member

I tried again with clang 17 (on fedora), but it still wouldn’t fail:

0clang version 17.0.6 (Fedora 17.0.6-2.fc39)
1Target: aarch64-redhat-linux-gnu
2Thread model: posix
3InstalledDir: /usr/bin

maflcko commented at 6:39 pm on March 14, 2024: member

Also fails in libc++-18:

0export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git  --depth=1 ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3     libevent-dev libboost-dev clang-18 libclang-rt-18-dev libc++abi-18-dev libc++-18-dev valgrind   -y   &&  ./autogen.sh && ./configure   CC=clang-18 CXX='clang++-18 -stdlib=libc++' && make -j $(nproc) src/bitcoind && ./test/functional/p2p_addr_relay.py --valgrind

 0 node0 stderr ==20019== Thread 14 b-msghand:
 1==20019== Conditional jump or move depends on uninitialised value(s)
 2==20019==    at 0x18AF84: ~__optional_destruct_base (optional:298)
 3==20019==    by 0x18AF84: GetLocalAddress(CNode const&) (???:220)
 4==20019==    by 0x18B06B: GetLocalAddrForPeer(CNode&) (net.cpp:240)
 5==20019==    by 0x1C68A3: MaybeSendAddr (net_processing.cpp:5407)
 6==20019==    by 0x1C68A3: (anonymous namespace)::PeerManagerImpl::SendMessages(CNode*) (???:5591)
 7==20019==    by 0x19FD93: CConnman::ThreadMessageHandler() (net.cpp:2906)
 8==20019==    by 0x58FBBF: operator() (function.h:428)
 9==20019==    by 0x58FBBF: operator() (function.h:981)
10==20019==    by 0x58FBBF: util::TraceThread(std::__1::basic_string_view<char, std::__1::char_traits<char> >, std::__1::function<void ()>) (???:21)
11==20019==    by 0x1A8F27: __invoke<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char> >, std::__1::function<void ()>), const char *, (lambda at net.cpp:3255:71)> (invoke.h:344)
12==20019==    by 0x1A8F27: __thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char> >, std::__1::function<void ()>), const char *, (lambda at net.cpp:3255:71), 2UL, 3UL> (thread.h:193)
13==20019==    by 0x1A8F27: void* std::__1::__thread_proxy[abi:ne180100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char> >, std::__1::function<void ()>), char const*, CConnman::Start(CScheduler&, CConnman::Options const&)::$_4> >(void*) (thread.h:202)
14==20019==    by 0x4C9597B: start_thread (pthread_create.c:447)
15==20019==    by 0x4CFB7DB: thread_start (clone.S:79)
16==20019== 
17{
18   <insert_a_suppression_name_here>
19   Memcheck:Cond
20   fun:~__optional_destruct_base
21   fun:_Z15GetLocalAddressRK5CNode
22   fun:_Z19GetLocalAddrForPeerR5CNode
23   fun:MaybeSendAddr
24   fun:_ZN12_GLOBAL__N_115PeerManagerImpl12SendMessagesEP5CNode
25   fun:_ZN8CConnman20ThreadMessageHandlerEv
26   fun:operator()
27   fun:operator()
28   fun:_ZN4util11TraceThreadENSt3__117basic_string_viewIcNS0_11char_traitsIcEEEENS0_8functionIFvvEEE
29   fun:__invoke<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char> >, std::__1::function<void ()>), const char *, (lambda at net.cpp:3255:71)>
30   fun:__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char> >, std::__1::function<void ()>), const char *, (lambda at net.cpp:3255:71), 2UL, 3UL>
31   fun:_ZNSt3__114__thread_proxyB8ne180100INS_5tupleIJNS_10unique_ptrINS_15__thread_structENS_14default_deleteIS3_EEEEPFvNS_17basic_string_viewIcNS_11char_traitsIcEEEENS_8functionIFvvEEEEPKcZN8CConnman5StartER10CSchedulerRKNSI_7OptionsEE3$_4EEEEEPvSQ_
32   fun:start_thread
33   fun:thread_start
34}
35==20019== 
36==20019== Exit program on first error (--exit-on-first-error=yes)

maflcko commented at 7:55 pm on March 14, 2024: member

Minimized so far:

 0# batcat main.cpp dummy.* && clang++-18 -std=c++17  -g -O2   -I.   main.cpp dummy.cpp -o /tmp/exe && valgrind /tmp/exe 
 1───────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 2       │ File: main.cpp
 3───────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 4   1   │ #include <dummy.h>
 5   2   │ 
 6   3   │ #include <optional>
 7   4   │ #include <set>
 8   5   │ 
 9   6   │ static std::set<int> empty_set;
10   7   │ 
11   8   │ std::optional<dummy> static none()
12   9   │ {
13  10   │     std::optional<dummy> maybe;
14  11   │     {
15  12   │         for (const auto& a : empty_set) {
16  13   │             if (empty_set.size() > 10) {
17  14   │                 maybe.emplace(dummy{});
18  15   │             }
19  16   │         }
20  17   │     }
21  18   │     return maybe;
22  19   │ }
23  20   │ 
24  21   │ dummy dummy_or_default()
25  22   │ {
26  23   │     return none().value_or(dummy{});
27  24   │ }
28  25   │ 
29  26   │ int main()
30  27   │ {
31  28   │     dummy_or_default();
32  29   │ }
33───────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
34───────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
35       │ File: dummy.cpp
36───────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
37   1   │ #include <dummy.h>
38   2   │ 
39   3   │ dummy::dummy() = default;
40───────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
41───────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
42       │ File: dummy.h
43───────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
44   1   │ #include <cstdlib>
45   2   │ 
46   3   │ struct dummy {
47   4   │     char* ptr{};
48   5   │ 
49   6   │     unsigned zero = 0;
50   7   │     bool hp() const { return zero > 0; }
51   8   │ 
52   9   │     dummy();
53  10   │ 
54  11   │     ~dummy()
55  12   │     {
56  13   │         if (hp()) std::free(ptr);
57  14   │     }
58  15   │ };
59───────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
60==73841== Memcheck, a memory error detector
61==73841== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
62==73841== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
63==73841== Command: /tmp/exe
64==73841== 
65==73841== Conditional jump or move depends on uninitialised value(s)
66==73841==    at 0x108BB8: _M_reset (optional:317)
67==73841==    by 0x108BB8: ~_Optional_payload (optional:441)
68==73841==    by 0x108BB8: ~_Optional_base (optional:512)
69==73841==    by 0x108BB8: dummy_or_default() (???:23)
70==73841==    by 0x108C27: main (main.cpp:28)
71==73841== 
72==73841== 
73==73841== HEAP SUMMARY:
74==73841==     in use at exit: 0 bytes in 0 blocks
75==73841==   total heap usage: 1 allocs, 1 frees, 73,728 bytes allocated
76==73841== 
77==73841== All heap blocks were freed -- no leaks are possible
78==73841== 
79==73841== Use --track-origins=yes to see where uninitialised values come from
80==73841== For lists of detected and suppressed errors, rerun with: -s
81==73841== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

maflcko commented at 1:52 pm on April 9, 2024: member

https://bugs.kde.org/show_bug.cgi?id=485276

maflcko commented at 3:28 pm on April 10, 2024: member

Some options:

Add a valgrind suppression
Avoid clang17+ for now, use up to clang16 in CI
Require clang++ -O1 for now
Something else?

fanquake commented at 5:50 pm on November 12, 2024: member

I think we could move to Ocular (gives us Valgrind 3.23.0, and we remember to switch to 25 before June), Clang 18 (31273) + some suppressions. Was running some tests of #31273 + this diff:

 0diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
 1index d3c95af99e..f0e8e92889 100755
 2--- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
 3+++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh
 4@@ -6,7 +6,7 @@
 5 
 6 export LC_ALL=C.UTF-8
 7 
 8-export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"
 9+export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.10"
10 export CONTAINER_NAME=ci_native_fuzz_valgrind
11 export PACKAGES="clang-18 llvm-18 libclang-rt-18-dev libevent-dev libboost-dev libsqlite3-dev valgrind"
12 export NO_DEPENDS=1
13diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh
14index b467a93920..c44528111d 100755
15--- a/ci/test/00_setup_env_native_valgrind.sh
16+++ b/ci/test/00_setup_env_native_valgrind.sh
17@@ -6,7 +6,7 @@
18 
19 export LC_ALL=C.UTF-8
20 
21-export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"
22+export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.10"
23 export CONTAINER_NAME=ci_native_valgrind
24 export PACKAGES="valgrind clang-18 llvm-18 libclang-rt-18-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libzmq3-dev libsqlite3-dev"
25 export USE_VALGRIND=1
26diff --git a/contrib/valgrind.supp b/contrib/valgrind.supp
27index 1ec5b8d20d..16088c7f7b 100644
28--- a/contrib/valgrind.supp
29+++ b/contrib/valgrind.supp
30@@ -15,6 +15,24 @@
31 # Tested on:
32 # * aarch64 (Ubuntu Noble system libs, clang, without gui)
33 # * x86_64  (Ubuntu Noble system libs, clang, without gui)
34+{
35+   <insert_a_suppression_name_here>
36+   Memcheck:Cond
37+   fun:_M_reset
38+   fun:~_Optional_payload
39+   fun:~_Optional_base
40+   fun:_Z15GetLocalAddressRK5CNode
41+   fun:_Z19GetLocalAddrForPeerR5CNode
42+}
43+{
44+   <insert_a_suppression_name_here>
45+   Memcheck:Cond
46+   fun:_ZN6wallet12MakeDatabaseERKN2fs4pathERKNS_15DatabaseOptionsERNS_14DatabaseStatusER13bilingual_str
47+   fun:_ZN6wallet18MakeWalletDatabaseERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_15DatabaseOptionsERNS_14DatabaseStatusER13bilingual_str
48+   fun:_ZN6wallet14TestLoadWalletERNS_13WalletContextE
49+   fun:_ZN6wallet12wallet_tests12CreateWallet11test_methodEv
50+   fun:_ZN6wallet12wallet_testsL20CreateWallet_invokerEv
51+}
52 {
53    Suppress libdb warning - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=662917
54    Memcheck:Cond

which seems to work ok. I did see a timeout in validation_block_tests, but I can bump the timeout factor.

maflcko commented at 5:57 pm on November 12, 2024: member

I think we could move to Ocular (gives us Valgrind 3.23.0

Isn’t valgrind 3.22 enough, so we could just stay with the LTS Ubuntu? Also, the suppression won’t work for libc++, likely. Not sure if this is worth it to support.

fanquake commented at 5:58 pm on November 12, 2024: member

Also, the suppression won’t work for libc++

We aren’t using libc++ though?

maflcko commented at 6:43 am on November 15, 2024: member

The current CI with clang-16 seems to fail on arm64 anyway?

0/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/test/test_bitcoin -t wallet_tests

 0==7187== Conditional jump or move depends on uninitialised value(s)
 1==7187==    at 0xD363B4: wallet::MakeDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) (./wallet/walletdb.cpp:1447)
 2==7187==    by 0xCF6DCB: wallet::MakeWalletDatabase(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) (./wallet/wallet.cpp:2980)
 3==7187==    by 0x8C2167: wallet::TestLoadWallet(wallet::WalletContext&) (./wallet/test/util.cpp:69)
 4==7187==    by 0x875E3B: wallet::wallet_tests::CreateWallet::test_method() (./wallet/test/wallet_tests.cpp:906)
 5==7187==    by 0x8749D7: wallet::wallet_tests::CreateWallet_invoker() (./wallet/test/wallet_tests.cpp:820)
 6==7187==    by 0x22943B: operator() (function_template.hpp:771)
 7==7187==    by 0x22943B: operator() (execution_monitor.ipp:1395)
 8==7187==    by 0x22943B: boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) (function_template.hpp:137)
 9==7187==    by 0x1D8C43: operator() (function_template.hpp:771)
10==7187==    by 0x1D8C43: do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> > (execution_monitor.ipp:308)
11==7187==    by 0x1D8C43: boost::execution_monitor::catch_signals(boost::function<int ()> const&) (execution_monitor.ipp:910)
12==7187==    by 0x1D8E93: boost::execution_monitor::execute(boost::function<int ()> const&) (execution_monitor.ipp:1308)
13==7187==    by 0x1D4CDF: vexecute (execution_monitor.ipp:1404)
14==7187==    by 0x1D4CDF: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) (unit_test_monitor.ipp:49)
15==7187==    by 0x1F735F: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) (framework.ipp:815)
16==7187==    by 0x1F7153: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) (framework.ipp:784)
17==7187==    by 0x1F7153: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) (framework.ipp:784)
18==7187== 
19{
20   <insert_a_suppression_name_here>
21   Memcheck:Cond
22   fun:_ZN6wallet12MakeDatabaseERKN2fs4pathERKNS_15DatabaseOptionsERNS_14DatabaseStatusER13bilingual_str
23   fun:_ZN6wallet18MakeWalletDatabaseERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKNS_15DatabaseOptionsERNS_14DatabaseStatusER13bilingual_str
24   fun:_ZN6wallet14TestLoadWalletERNS_13WalletContextE
25   fun:_ZN6wallet12wallet_tests12CreateWallet11test_methodEv
26   fun:_ZN6wallet12wallet_testsL20CreateWallet_invokerEv
27   fun:operator()
28   fun:operator()
29   fun:_ZN5boost6detail8function21function_obj_invoker0INS0_7forwardEiE6invokeERNS1_15function_bufferE
30   fun:operator()
31   fun:do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >
32   fun:_ZN5boost17execution_monitor13catch_signalsERKNS_8functionIFivEEE
33   fun:_ZN5boost17execution_monitor7executeERKNS_8functionIFivEEE
34   fun:vexecute
35   fun:_ZN5boost9unit_test19unit_test_monitor_t21execute_and_translateERKNS_8functionIFvvEEEm
36   fun:_ZN5boost9unit_test9framework5state17execute_test_treeEmmPKNS2_23random_generator_helperE
37   fun:_ZN5boost9unit_test9framework5state17execute_test_treeEmmPKNS2_23random_generator_helperE
38   fun:_ZN5boost9unit_test9framework5state17execute_test_treeEmmPKNS2_23random_generator_helperE
39}
40
41*** No errors detected

valgrind: Conditional jump or move depends on uninitialised value(s) #29635