ScanForWalletTransactions: jump or move depends on uninitialised value

fanquake commented at 2:57 am on March 5, 2021: member

Reported on IRC this morning. I’ve recreated using the following and master @ da8c7edffe0c9efe99d56ec9e8b96b3b4e3e275f:

0Ubuntu 20.04.2 LTS
1g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
2Python 3.8.5
3valgrind-3.15.0
4
5make -C depends/ NO_QT=1 NO_UPNP=1 NO_ZMQ=1 NO_NATPMP=1 -j10
6./autogen.sh.sh
7./configure --prefix=/home/ubuntu/bitcoin/depends/x86_64-pc-linux-gnu
8make -j10
9test/functional/test_runner.py "wallet_send.py --descriptors" --valgrind

  0test/functional/test_runner.py "wallet_hd.py --descriptors" --valgrind
  1Temporary test directory at /tmp/test_runner_₿_🏃_20210305_025257
  2Running Unit Tests for Test Framework Modules
  3..........
  4----------------------------------------------------------------------
  5Ran 10 tests in 0.601s
  6
  7OK
  8Remaining jobs: [wallet_hd.py --descriptors]
  91/1 - wallet_hd.py --descriptors failed, Duration: 8 s
 10
 11stdout:
 122021-03-05T02:52:57.805000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20210305_025257/wallet_hd_0
 132021-03-05T02:53:05.165000Z TestFramework (ERROR): Unexpected exception caught during testing
 14Traceback (most recent call last):
 15  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 108, in _request
 16    return self._get_response()
 17  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 168, in _get_response
 18    http_response = self.__conn.getresponse()
 19  File "/usr/lib/python3.8/http/client.py", line 1347, in getresponse
 20    response.begin()
 21  File "/usr/lib/python3.8/http/client.py", line 307, in begin
 22    version, status, reason = self._read_status()
 23  File "/usr/lib/python3.8/http/client.py", line 276, in _read_status
 24    raise RemoteDisconnected("Remote end closed connection without"
 25http.client.RemoteDisconnected: Remote end closed connection without response
 26
 27During handling of the above exception, another exception occurred:
 28
 29Traceback (most recent call last):
 30  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 127, in main
 31    self.setup()
 32  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 278, in setup
 33    self.setup_network()
 34  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 372, in setup_network
 35    self.setup_nodes()
 36  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 399, in setup_nodes
 37    self.import_deterministic_coinbase_privkeys()
 38  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 416, in import_deterministic_coinbase_privkeys
 39    self.init_wallet(i)
 40  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 424, in init_wallet
 41    n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')
 42  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_node.py", line 696, in importprivkey
 43    import_res = self.importdescriptors(req)
 44  File "/home/ubuntu/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
 45    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
 46  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 144, in __call__
 47    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
 48  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 113, in _request
 49    self.__conn.request(method, path, postdata, headers)
 50  File "/usr/lib/python3.8/http/client.py", line 1255, in request
 51    self._send_request(method, url, body, headers, encode_chunked)
 52  File "/usr/lib/python3.8/http/client.py", line 1301, in _send_request
 53    self.endheaders(body, encode_chunked=encode_chunked)
 54  File "/usr/lib/python3.8/http/client.py", line 1250, in endheaders
 55    self._send_output(message_body, encode_chunked=encode_chunked)
 56  File "/usr/lib/python3.8/http/client.py", line 1010, in _send_output
 57    self.send(msg)
 58  File "/usr/lib/python3.8/http/client.py", line 950, in send
 59    self.connect()
 60  File "/usr/lib/python3.8/http/client.py", line 921, in connect
 61    self.sock = self._create_connection(
 62  File "/usr/lib/python3.8/socket.py", line 808, in create_connection
 63    raise err
 64  File "/usr/lib/python3.8/socket.py", line 796, in create_connection
 65    sock.connect(sa)
 66ConnectionRefusedError: [Errno 111] Connection refused
 672021-03-05T02:53:05.216000Z TestFramework (INFO): Stopping nodes
 682021-03-05T02:53:05.216000Z TestFramework.node0 (ERROR): Unable to stop node.
 69Traceback (most recent call last):
 70  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_node.py", line 323, in stop_node
 71    self.stop(wait=wait)
 72  File "/home/ubuntu/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
 73    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
 74  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 144, in __call__
 75    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
 76  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
 77    self.__conn.request(method, path, postdata, headers)
 78  File "/usr/lib/python3.8/http/client.py", line 1255, in request
 79    self._send_request(method, url, body, headers, encode_chunked)
 80  File "/usr/lib/python3.8/http/client.py", line 1266, in _send_request
 81    self.putrequest(method, url, **skips)
 82  File "/usr/lib/python3.8/http/client.py", line 1092, in putrequest
 83    raise CannotSendRequest(self.__state)
 84http.client.CannotSendRequest: Request-sent
 85
 86
 87stderr:
 88Traceback (most recent call last):
 89  File "/home/ubuntu/bitcoin/test/functional/wallet_hd.py", line 279, in <module>
 90    WalletHDTest().main()
 91  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 151, in main
 92    exit_code = self.shutdown()
 93  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 294, in shutdown
 94    self.stop_nodes()
 95  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 541, in stop_nodes
 96    node.stop_node(wait=wait, wait_until_stopped=False)
 97  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_node.py", line 337, in stop_node
 98    raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
 99AssertionError: Unexpected stderr ==2717675== Thread 13 b-httpworker.2:
100==2717675== Conditional jump or move depends on uninitialised value(s)
101==2717675==    at 0x5873EB: CWallet::ScanForWalletTransactions(uint256 const&, int, std::optional<int>, WalletRescanReserver const&, bool) (in /home/ubuntu/bitcoin/src/bitcoind)
102==2717675==    by 0x587E2A: CWallet::RescanFromTime(long, WalletRescanReserver const&, bool) (in /home/ubuntu/bitcoin/src/bitcoind)
103==2717675==    by 0x5FEFE7: importdescriptors()::{lambda(RPCHelpMan const&, JSONRPCRequest const&)#1}::operator()(RPCHelpMan const&, JSONRPCRequest const&) const [clone .isra.0] (in /home/ubuntu/bitcoin/src/bitcoind)
104==2717675==    by 0x600024: std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), importdescriptors()::{lambda(RPCHelpMan const&, JSONRPCRequest const&)#1}>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) (in /home/ubuntu/bitcoin/src/bitcoind)
105==2717675==    by 0x32F958: CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const (in /home/ubuntu/bitcoin/src/bitcoind)
106==2717675==    by 0x4C0B5B: wallet::(anonymous namespace)::WalletClientImpl::registerRpcs()::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const (in /home/ubuntu/bitcoin/src/bitcoind)
107==2717675==    by 0x2AF134: std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), node::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) (in /home/ubuntu/bitcoin/src/bitcoind)
108==2717675==    by 0x3BA3C8: CRPCTable::execute(JSONRPCRequest const&) const (in /home/ubuntu/bitcoin/src/bitcoind)
109==2717675==    by 0x47EB89: HTTPReq_JSONRPC(util::Ref const&, HTTPRequest*) (in /home/ubuntu/bitcoin/src/bitcoind)
110==2717675==    by 0x4832DB: HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) (in /home/ubuntu/bitcoin/src/bitcoind)
111==2717675==    by 0x495FD83: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28)
112==2717675==    by 0x486F608: start_thread (pthread_create.c:477)
113==2717675== 
114{
115   <insert_a_suppression_name_here>
116   Memcheck:Cond
117   fun:_ZN7CWallet25ScanForWalletTransactionsERK7uint256iSt8optionalIiERK20WalletRescanReserverb
118   fun:_ZN7CWallet14RescanFromTimeElRK20WalletRescanReserverb
119   fun:_ZZ17importdescriptorsvENKUlRK10RPCHelpManRK14JSONRPCRequestE_clES1_S4_.isra.0
120   fun:_ZNSt17_Function_handlerIF8UniValueRK10RPCHelpManRK14JSONRPCRequestEZ17importdescriptorsvEUlS3_S6_E_E9_M_invokeERKSt9_Any_dataS3_S6_
121   fun:_ZZN11CRPCCommandC4ENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPF10RPCHelpManvEENKUlRK14JSONRPCRequestR8UniValuebE_clESB_SD_b
122   fun:_ZZN6wallet12_GLOBAL__N_116WalletClientImpl12registerRpcsEvENKUlRK14JSONRPCRequestR8UniValuebE_clES4_S6_b
123   fun:_ZNSt17_Function_handlerIFbRK14JSONRPCRequestR8UniValuebEZN4node12_GLOBAL__N_114RpcHandlerImplC4ERK11CRPCCommandEUlS2_S4_bE_E9_M_invokeERKSt9_Any_dataS2_S4_Ob
124   fun:_ZNK9CRPCTable7executeERK14JSONRPCRequest
125   fun:_ZL15HTTPReq_JSONRPCRKN4util3RefEP11HTTPRequest
126   fun:_ZL16HTTPWorkQueueRunP9WorkQueueI11HTTPClosureEi
127   obj:/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28
128   fun:start_thread
129}
130==2717675== 
131==2717675== Exit program on first error (--exit-on-first-error=yes) != 
132
133
134
135TEST                       | STATUS    | DURATION
136
137wallet_hd.py --descriptors | ✖ Failed  | 8 s
138
139ALL                        | ✖ Failed  | 8 s (accumulated) 
140Runtime: 8 s

fanquake added the label Bug on Mar 5, 2021

fanquake added the label Wallet on Mar 5, 2021

sipa commented at 3:03 am on March 5, 2021: member

I can reproduce this all the way back to commit fab7d954b261d74b369fe2a3c1785540c4f056b4 (at which point a BDB valgrind error takes over which I’ll need to silence).

ryanofsky commented at 3:18 am on March 5, 2021: contributor

Might not be relevant here but sometimes valgrind “Conditional jump or move depends on uninitialised value(s)” errors are spurious. This was the case previously in #18785.

sipa commented at 3:24 am on March 5, 2021: member

I would not be surprised if this was spurious (the fact that it goes away when trying to add logging to inspect is a sign, but not a guarantee - if it’s the result of miscompilation, it may actually be invalid machine code, and also actually go away with changed C++ code).

MarcoFalke commented at 7:03 am on March 5, 2021: member

I can’t reproduce with the valgrind ci config

ryanofsky commented at 2:38 pm on March 5, 2021: contributor

I couldn’t reproduce this either, but I’m using an older version of gcc.

It should be pretty straightforward to look at the assembly code at the address printed by valgrind (0x5873EB in the example above) and see what uninitialized value it’s referring to, and whether it’s a really a bug for the value to be uninitialized. Like if the line number mentioned in IRC is referring to:

https://github.com/bitcoin/bitcoin/blob/ed25cb58f605ba583c735f330482df0bf9348f3a/src/wallet/wallet.cpp#L1822-L1824

It would be perfectly fine for optimized && code to not short circuit and for the block_height >= *max_height comparison to execute with some garbage uninitialized *max_height value when the other branch of the && expression is false anyway.

Getting GDB to run with functional tests is a minor PITA. I use little wrapper scripts that fork off gdb in screen processes like:

0#!/bin/bash
1exec screen -S "bitcoind_${PPID}_$$" -D -m gdb -ex run --args /path/to/src/bitcoind "$@"

Run the test with:

0BITCOIND=/path/to/little_script test/functional/wallet_hd.py --descriptors --valgrind

Connect to GDB with:

0screen -r

Where you can type in GDB commands like::

0disassemble 0x5873EB

MarcoFalke commented at 11:35 am on August 15, 2022: member

Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

MarcoFalke closed this on Aug 15, 2022

bitcoin locked this on Aug 15, 2023

ScanForWalletTransactions: jump or move depends on uninitialised value #21367