miner: Avoid stack-use-after-return in validationinterface

MarcoFalke · fanquake · commit cc7d34465bbb · 2020-05-15T07:42:08.000+08:00
This is achieved by switching to a shared_ptr. Also, switch the validationinterfaces in the tests to use shared_ptrs for the same reason. Github-Pull: #18742 Rebased-From: 7777f2a
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
@@ -724,7 +724,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
     return result;
 }
 
-class submitblock_StateCatcher : public CValidationInterface
+class submitblock_StateCatcher final : public CValidationInterface
 {
 public:
     uint256 hash;
@@ -792,17 +792,17 @@ static UniValue submitblock(const JSONRPCRequest& request)
     }
 
     bool new_block;
-    submitblock_StateCatcher sc(block.GetHash());
-    RegisterValidationInterface(&sc);
+    auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash());
+    RegisterSharedValidationInterface(sc);
     bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block);
-    UnregisterValidationInterface(&sc);
+    UnregisterSharedValidationInterface(sc);
     if (!new_block && accepted) {
         return "duplicate";
     }
-    if (!sc.found) {
+    if (!sc->found) {
         return "inconclusive";
     }
-    return BIP22ValidationResult(sc.state);
+    return BIP22ValidationResult(sc->state);
 }
 
 static UniValue submitheader(const JSONRPCRequest& request)
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
@@ -32,7 +32,7 @@ struct MinerTestingSetup : public RegTestingSetup {
 
 BOOST_FIXTURE_TEST_SUITE(validation_block_tests, MinerTestingSetup)
 
-struct TestSubscriber : public CValidationInterface {
+struct TestSubscriber final : public CValidationInterface {
     uint256 m_expected_tip;
 
     explicit TestSubscriber(uint256 tip) : m_expected_tip(tip) {}
@@ -175,8 +175,8 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
         LOCK(cs_main);
         initial_tip = ::ChainActive().Tip();
     }
-    TestSubscriber sub(initial_tip->GetBlockHash());
-    RegisterValidationInterface(&sub);
+    auto sub = std::make_shared<TestSubscriber>(initial_tip->GetBlockHash());
+    RegisterSharedValidationInterface(sub);
 
     // create a bunch of threads that repeatedly process a block generated above at random
     // this will create parallelism and randomness inside validation - the ValidationInterface
@@ -208,10 +208,10 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
         UninterruptibleSleep(std::chrono::milliseconds{100});
     }
 
-    UnregisterValidationInterface(&sub);
+    UnregisterSharedValidationInterface(sub);
 
     LOCK(cs_main);
-    BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash());
+    BOOST_CHECK_EQUAL(sub->m_expected_tip, ::ChainActive().Tip()->GetBlockHash());
 }
 
 /**
diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp
@@ -12,7 +12,6 @@
 
 BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup)
 
-/**
 struct TestSubscriberNoop final : public CValidationInterface {
     void BlockChecked(const CBlock&, const BlockValidationState&) override {}
 };
@@ -34,9 +33,9 @@ BOOST_AUTO_TEST_CASE(unregister_validation_interface_race)
     std::thread sub{[&] {
         // keep going for about 1 sec, which is 250k iterations
         for (int i = 0; i < 250000; i++) {
-            TestSubscriberNoop sub{};
-            RegisterValidationInterface(&sub);
-            UnregisterValidationInterface(&sub);
+            auto sub = std::make_shared<TestSubscriberNoop>();
+            RegisterSharedValidationInterface(sub);
+            UnregisterSharedValidationInterface(sub);
         }
         // tell the other thread we are done
         generate = false;
@@ -46,7 +45,6 @@ BOOST_AUTO_TEST_CASE(unregister_validation_interface_race)
     sub.join();
     BOOST_CHECK(!generate);
 }
-*/
 
 class TestInterface : public CValidationInterface
 {

Original file line number	Diff line number	Diff line change
`@@ -724,7 +724,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)`
`724`	`724`	`return result;`
`725`	`725`	`}`
`726`	`726`
`727`		`-class submitblock_StateCatcher : public CValidationInterface`
	`727`	`+class submitblock_StateCatcher final : public CValidationInterface`
`728`	`728`	`{`
`729`	`729`	`public:`
`730`	`730`	`uint256 hash;`
`@@ -792,17 +792,17 @@ static UniValue submitblock(const JSONRPCRequest& request)`
`792`	`792`	`}`
`793`	`793`
`794`	`794`	`bool new_block;`
`795`		`- submitblock_StateCatcher sc(block.GetHash());`
`796`		`- RegisterValidationInterface(&sc);`
	`795`	`+ auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash());`
	`796`	`+ RegisterSharedValidationInterface(sc);`
`797`	`797`	`bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing / true, / fNewBlock */ &new_block);`
`798`		`- UnregisterValidationInterface(&sc);`
	`798`	`+ UnregisterSharedValidationInterface(sc);`
`799`	`799`	`if (!new_block && accepted) {`
`800`	`800`	`return "duplicate";`
`801`	`801`	`}`
`802`		`- if (!sc.found) {`
	`802`	`+ if (!sc->found) {`
`803`	`803`	`return "inconclusive";`
`804`	`804`	`}`
`805`		`- return BIP22ValidationResult(sc.state);`
	`805`	`+ return BIP22ValidationResult(sc->state);`
`806`	`806`	`}`
`807`	`807`
`808`	`808`	`static UniValue submitheader(const JSONRPCRequest& request)`