[validation] Two small clang lock annotation improvements#21202
[validation] Two small clang lock annotation improvements#21202maflcko merged 2 commits intobitcoin:masterfrom
Conversation
|
to test this out, reviewers can apply this diff locally - bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck>* pvChecks)
+ RecursiveMutex cs_new;
+ 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_new)and see that the compiler warns about the call site from validation.cpp:2136:35: warning: calling function 'CheckInputScripts' requires holding mutex 'cs_new' exclusively [-Wthread-safety-analysis]
if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], g_parallel_script_checks ? &vChecks : nullptr)) {but there is no warning from the call site from then you can apply the annotation to the declaration instead of the definition, and see that all the call sites generate warnings. |
|
If this pr a case to apply #20986? |
…laration When the annotation is on the definition, it does not check call sites between the declaration and the definition.
amitiuttarwar
force-pushed
the
2021-02-locks
branch
from
9d0975c to
25c57d6
Compare
|
ACK 25c57d6 🥘 Show signature and timestampSignature: Timestamp of file with hash |
|
@promag where are other examples of forward declaring functions from the tests? I see examples for other files eg. |
5 participants
Based on reviewing #21188
the first commit switches the lock annotations on
CheckInputScriptsto 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.