sync: use proper TSA attributes

Use the proper attribute from Clang's thread safety analysis for
`AssertLockHeld()`:

* if `DEBUG_LOCKORDER` is defined then `AssertLockHeld()` will check if
  the caller owns the given mutex and will `abort()` if not. Clang has
  an attribute exactly for that - `ASSERT_EXCLUSIVE_LOCK()`, documented
  as: "These are attributes on a function or method that does a run-time
  test to see whether the calling thread holds the given capability. The
  function is assumed to fail (no return) if the capability is not
  held." [1]

* if `DEBUG_LOCKORDER` is not defined, then `AssertLockHeld()` does
  nothing, thus don't tag it with any attributes (don't fool the
  compiler that we do something which we don't).

Replace `LockAssertion` with `AssertLockHeld` and remove the former.

On the places where Clang cannot deduce that a mutex is held, use
`NO_THREAD_SAFETY_ANALYSIS`, intended to be used when a code is
"too complicated for the analysis to understand" [2].

[1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#requires-requires-shared
[2] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-thread-safety-analysis

Author: vasild

src/net_processing.cpp

@@ -662,8 +662,8 @@ static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman& connma
                 return;
             }
         }
-        connman.ForNode(nodeid, [&connman](CNode* pfrom){
-            LockAssertion lock(::cs_main);
+        connman.ForNode(nodeid, [&connman](CNode* pfrom) NO_THREAD_SAFETY_ANALYSIS {
+            AssertLockHeld(::cs_main);
             uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1;
             if (lNodesAnnouncingHeaderAndIDs.size() >= 3) {
                 // As per BIP152, we only get 3 of our peers to announce
@@ -1354,8 +1354,9 @@ void PeerManager::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_
         fWitnessesPresentInMostRecentCompactBlock = fWitnessEnabled;
     }
 
-    m_connman.ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) {
-        LockAssertion lock(::cs_main);
+    m_connman.ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled,
+                           &hashBlock](CNode* pnode) NO_THREAD_SAFETY_ANALYSIS {
+        AssertLockHeld(::cs_main);
 
         // TODO: Avoid the repeated-serialization here
         if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect)
@@ -1488,9 +1489,8 @@ bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED
 
 void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman)
 {
-    connman.ForEachNode([&txid, &wtxid](CNode* pnode)
-    {
-        LockAssertion lock(::cs_main);
+    connman.ForEachNode([&txid, &wtxid](CNode* pnode) NO_THREAD_SAFETY_ANALYSIS {
+        AssertLockHeld(::cs_main);
 
         CNodeState &state = *State(pnode->GetId());
         if (state.m_wtxid_relay) {
@@ -3979,8 +3979,8 @@ void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds)
         NodeId worst_peer = -1;
         int64_t oldest_block_announcement = std::numeric_limits<int64_t>::max();
 
-        m_connman.ForEachNode([&](CNode* pnode) {
-            LockAssertion lock(::cs_main);
+        m_connman.ForEachNode([&](CNode* pnode) NO_THREAD_SAFETY_ANALYSIS {
+            AssertLockHeld(::cs_main);
 
             // Ignore non-outbound peers, or nodes marked for disconnect already
             if (!pnode->IsOutboundOrBlockRelayConn() || pnode->fDisconnect) return;
@@ -3996,8 +3996,8 @@ void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds)
             }
         });
         if (worst_peer != -1) {
-            bool disconnected = m_connman.ForNode(worst_peer, [&](CNode *pnode) {
-                LockAssertion lock(::cs_main);
+            bool disconnected = m_connman.ForNode(worst_peer, [&](CNode *pnode) NO_THREAD_SAFETY_ANALYSIS {
+                AssertLockHeld(::cs_main);
 
                 // Only disconnect a peer that has been connected to us for
                 // some reasonable fraction of our check-frequency, to give

src/sync.h

@@ -53,9 +53,9 @@ void LeaveCritical();
 void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line);
 std::string LocksHeld();
 template <typename MutexType>
-void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs);
+void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs);
 template <typename MutexType>
-void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs);
+void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(!cs);
 void DeleteLock(void* cs);
 bool LockStackEmpty();
 
@@ -70,9 +70,9 @@ inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, v
 inline void LeaveCritical() {}
 inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {}
 template <typename MutexType>
-inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs) {}
+inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) {}
 template <typename MutexType>
-void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) {}
+void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) {}
 inline void DeleteLock(void* cs) {}
 inline bool LockStackEmpty() { return true; }
 #endif
@@ -352,18 +352,4 @@ class CSemaphoreGrant
     }
 };
 
-// Utility class for indicating to compiler thread analysis that a mutex is
-// locked (when it couldn't be determined otherwise).
-struct SCOPED_LOCKABLE LockAssertion
-{
-    template <typename Mutex>
-    explicit LockAssertion(Mutex& mutex) EXCLUSIVE_LOCK_FUNCTION(mutex)
-    {
-#ifdef DEBUG_LOCKORDER
-        AssertLockHeld(mutex);
-#endif
-    }
-    ~LockAssertion() UNLOCK_FUNCTION() {}
-};
-
 #endif // BITCOIN_SYNC_H