ScanForWalletTransactions: jump or move depends on uninitialised value #21367

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

Ubuntu 20.04.2 LTS
g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Python 3.8.5
valgrind-3.15.0

make -C depends/ NO_QT=1 NO_UPNP=1 NO_ZMQ=1 NO_NATPMP=1 -j10
./autogen.sh.sh
./configure --prefix=/home/ubuntu/bitcoin/depends/x86_64-pc-linux-gnu
make -j10
test/functional/test_runner.py "wallet_send.py --descriptors" --valgrind

test/functional/test_runner.py "wallet_hd.py --descriptors" --valgrind
Temporary test directory at /tmp/test_runner_₿_🏃_20210305_025257
Running Unit Tests for Test Framework Modules
..........
----------------------------------------------------------------------
Ran 10 tests in 0.601s

OK
Remaining jobs: [wallet_hd.py --descriptors]
1/1 - wallet_hd.py --descriptors failed, Duration: 8 s

stdout:
2021-03-05T02:52:57.805000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20210305_025257/wallet_hd_0
2021-03-05T02:53:05.165000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 108, in _request
    return self._get_response()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 168, in _get_response
    http_response = self.__conn.getresponse()
  File "/usr/lib/python3.8/http/client.py", line 1347, in getresponse
    response.begin()
  File "/usr/lib/python3.8/http/client.py", line 307, in begin
    version, status, reason = self._read_status()
  File "/usr/lib/python3.8/http/client.py", line 276, in _read_status
    raise RemoteDisconnected("Remote end closed connection without"
http.client.RemoteDisconnected: Remote end closed connection without response

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 127, in main
    self.setup()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 278, in setup
    self.setup_network()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 372, in setup_network
    self.setup_nodes()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 399, in setup_nodes
    self.import_deterministic_coinbase_privkeys()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 416, in import_deterministic_coinbase_privkeys
    self.init_wallet(i)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 424, in init_wallet
    n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_node.py", line 696, in importprivkey
    import_res = self.importdescriptors(req)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 144, in __call__
    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 113, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.8/http/client.py", line 1255, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1301, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1250, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1010, in _send_output
    self.send(msg)
  File "/usr/lib/python3.8/http/client.py", line 950, in send
    self.connect()
  File "/usr/lib/python3.8/http/client.py", line 921, in connect
    self.sock = self._create_connection(
  File "/usr/lib/python3.8/socket.py", line 808, in create_connection
    raise err
  File "/usr/lib/python3.8/socket.py", line 796, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused
2021-03-05T02:53:05.216000Z TestFramework (INFO): Stopping nodes
2021-03-05T02:53:05.216000Z TestFramework.node0 (ERROR): Unable to stop node.
Traceback (most recent call last):
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_node.py", line 323, in stop_node
    self.stop(wait=wait)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 144, in __call__
    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
  File "/home/ubuntu/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.8/http/client.py", line 1255, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.8/http/client.py", line 1266, in _send_request
    self.putrequest(method, url, **skips)
  File "/usr/lib/python3.8/http/client.py", line 1092, in putrequest
    raise CannotSendRequest(self.__state)
http.client.CannotSendRequest: Request-sent


stderr:
Traceback (most recent call last):
  File "/home/ubuntu/bitcoin/test/functional/wallet_hd.py", line 279, in <module>
    WalletHDTest().main()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 151, in main
    exit_code = self.shutdown()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 294, in shutdown
    self.stop_nodes()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 541, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_node.py", line 337, in stop_node
    raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
AssertionError: Unexpected stderr ==2717675== Thread 13 b-httpworker.2:
==2717675== Conditional jump or move depends on uninitialised value(s)
==2717675==    at 0x5873EB: CWallet::ScanForWalletTransactions(uint256 const&, int, std::optional<int>, WalletRescanReserver const&, bool) (in /home/ubuntu/bitcoin/src/bitcoind)
==2717675==    by 0x587E2A: CWallet::RescanFromTime(long, WalletRescanReserver const&, bool) (in /home/ubuntu/bitcoin/src/bitcoind)
==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)
==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)
==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)
==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)
==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)
==2717675==    by 0x3BA3C8: CRPCTable::execute(JSONRPCRequest const&) const (in /home/ubuntu/bitcoin/src/bitcoind)
==2717675==    by 0x47EB89: HTTPReq_JSONRPC(util::Ref const&, HTTPRequest*) (in /home/ubuntu/bitcoin/src/bitcoind)
==2717675==    by 0x4832DB: HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) (in /home/ubuntu/bitcoin/src/bitcoind)
==2717675==    by 0x495FD83: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28)
==2717675==    by 0x486F608: start_thread (pthread_create.c:477)
==2717675== 
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZN7CWallet25ScanForWalletTransactionsERK7uint256iSt8optionalIiERK20WalletRescanReserverb
   fun:_ZN7CWallet14RescanFromTimeElRK20WalletRescanReserverb
   fun:_ZZ17importdescriptorsvENKUlRK10RPCHelpManRK14JSONRPCRequestE_clES1_S4_.isra.0
   fun:_ZNSt17_Function_handlerIF8UniValueRK10RPCHelpManRK14JSONRPCRequestEZ17importdescriptorsvEUlS3_S6_E_E9_M_invokeERKSt9_Any_dataS3_S6_
   fun:_ZZN11CRPCCommandC4ENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPF10RPCHelpManvEENKUlRK14JSONRPCRequestR8UniValuebE_clESB_SD_b
   fun:_ZZN6wallet12_GLOBAL__N_116WalletClientImpl12registerRpcsEvENKUlRK14JSONRPCRequestR8UniValuebE_clES4_S6_b
   fun:_ZNSt17_Function_handlerIFbRK14JSONRPCRequestR8UniValuebEZN4node12_GLOBAL__N_114RpcHandlerImplC4ERK11CRPCCommandEUlS2_S4_bE_E9_M_invokeERKSt9_Any_dataS2_S4_Ob
   fun:_ZNK9CRPCTable7executeERK14JSONRPCRequest
   fun:_ZL15HTTPReq_JSONRPCRKN4util3RefEP11HTTPRequest
   fun:_ZL16HTTPWorkQueueRunP9WorkQueueI11HTTPClosureEi
   obj:/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28
   fun:start_thread
}
==2717675== 
==2717675== Exit program on first error (--exit-on-first-error=yes) != 



TEST                       | STATUS    | DURATION

wallet_hd.py --descriptors | ✖ Failed  | 8 s

ALL                        | ✖ Failed  | 8 s (accumulated) 
Runtime: 8 s

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).

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.

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).

I can't reproduce with the valgrind ci config

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:

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

Run the test with:

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

Connect to GDB with:

screen -r

Where you can type in GDB commands like::

disassemble 0x5873EB

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