Merge #21202: [validation] Two small clang lock annotation improvements

25c57d640992255ed67964a44b17afbfd4bed0cf [doc] Add a note about where lock annotations should go. (Amiti Uttarwar)
ad5f01b96045f304b6cf9100879592b835c49c40 [validation] Move the lock annotation from function definition to declaration (Amiti Uttarwar)

Pull request description:

  Based on reviewing #21188

  the first commit switches the lock annotations on `CheckInputScripts` to be on the function declaration instead of on the function definition. this ensures that all call sites are checked, not just ones that come after the definition.

  the second commit adds a note to the developer-notes section to clarify where the annotations should be applied.

ACKs for top commit:
  MarcoFalke:
    ACK 25c57d640992255ed67964a44b17afbfd4bed0cf 🥘
  promag:
    Code review ACK 25c57d640992255ed67964a44b17afbfd4bed0cf.

Tree-SHA512: 61b6ef856bf6c6016d535fbdd19daf57b9e59fe54a1f30d47282a071b9b9d60b2466b044ee57929e0320cb1bdef52e7a1687cacaa27031bbc43d058ffffe22ba
This commit is contained in:
MarcoFalke 2021-02-22 09:47:11 +01:00
commit 34d7030063
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
3 changed files with 18 additions and 3 deletions

View File

@ -785,6 +785,11 @@ Threads and synchronization
get compile-time warnings about potential race conditions in code. Combine annotations in function declarations with
run-time asserts in function definitions:
- In functions that are declared separately from where they are defined, the
thread safety annotations should be added exclusively to the function
declaration. Annotations on the definition could lead to false positives
(lack of compile failure) at call sites between the two.
```C++
// txmempool.h
class CTxMemPool

View File

@ -13,7 +13,10 @@
#include <boost/test/unit_test.hpp>
bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks);
bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
std::vector<CScriptCheck>* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
BOOST_AUTO_TEST_SUITE(txvalidationcache_tests)

View File

@ -199,7 +199,11 @@ CBlockIndex* BlockManager::FindForkInGlobalIndex(const CChain& chain, const CBlo
std::unique_ptr<CBlockTreeDB> pblocktree;
bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks = nullptr);
bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
std::vector<CScriptCheck>* pvChecks = nullptr)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false);
static FlatFileSeq BlockFileSeq();
static FlatFileSeq UndoFileSeq();
@ -1481,7 +1485,10 @@ void InitScriptExecutionCache() {
*
* Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp
*/
bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
std::vector<CScriptCheck>* pvChecks)
{
if (tx.IsCoinBase()) return true;