What is better than fixing bugs? Fixing entire bug classes of course! :)
Add option --valgrind to run the functional tests under Valgrind.
Regular functional testing under Valgrind would have caught many of the uninitialized reads we’ve seen historically.
Let’s kill this bug class once and for all: let’s never use an uninitialized value ever again. Or at least not one that would be triggered by running the functional tests! :)
My hope is that this addition will make it super-easy to run the functional tests under Valgrind and thus increase the probability of people making use of it :)
Hopefully test/functional/test_runner.py --valgrind will become a natural part of the pre-release QA process.
Usage:
0$ test/functional/test_runner.py --help
1…
2 --valgrind run nodes under the valgrind memory error detector:
3 expect at least a ~10x slowdown, valgrind 3.14 or
4 later required
Live demo:
First, let’s re-introduce a memory bug by reverting the recent P2P uninitialized read bug fix from PR #17624 (“net: Fix an uninitialized read in ProcessMessage(…, “tx”, …) when receiving a transaction we already have”).
0$ git diff
1diff --git a/src/consensus/validation.h b/src/consensus/validation.h
2index 3401eb64c..940adea33 100644
3--- a/src/consensus/validation.h
4+++ b/src/consensus/validation.h
5@@ -114,7 +114,7 @@ inline ValidationState::~ValidationState() {};
6
7 class TxValidationState : public ValidationState {
8 private:
9- TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
10+ TxValidationResult m_result;
11 public:
12 bool Invalid(TxValidationResult result,
13 const std::string &reject_reason="",
Second, let’s test as normal without Valgrind:
0$ test/functional/p2p_segwit.py -l INFO
12019-11-28T09:30:42.810000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__fc8q3qo
2…
32019-11-28T09:31:57.187000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
4…
52019-11-28T09:32:08.265000Z TestFramework (INFO): Tests successful
Third, let’s test with --valgrind and see if the test fail (as we expect) when the unitialized value is used:
0$ test/functional/p2p_segwit.py -l INFO --valgrind
12019-11-28T09:32:33.018000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_gtjecx2l
2…
32019-11-28T09:40:36.702000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
42019-11-28T09:40:37.813000Z TestFramework (ERROR): Assertion failed
5ConnectionRefusedError: [Errno 111] Connection refused
