merge #18635: Replace -Wthread-safety-analysis with -Wthread-safety

configure.ac

@@ -362,7 +362,7 @@ if test "x$enable_werror" = "xyes"; then
     AC_MSG_ERROR("enable-werror set but -Werror is not usable")
   fi
   AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]])
-  AX_CHECK_COMPILE_FLAG([-Werror=thread-safety-analysis],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety-analysis"],,[[$CXXFLAG_WERROR]])
+  AX_CHECK_COMPILE_FLAG([-Werror=thread-safety],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety"],,[[$CXXFLAG_WERROR]])
 fi
 
 if test "x$CXXFLAGS_overridden" = "xno"; then
@@ -371,7 +371,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
   AX_CHECK_COMPILE_FLAG([-Wformat],[CXXFLAGS="$CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wvla],[CXXFLAGS="$CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wformat-security],[CXXFLAGS="$CXXFLAGS -Wformat-security"],,[[$CXXFLAG_WERROR]])
-  AX_CHECK_COMPILE_FLAG([-Wthread-safety-analysis],[CXXFLAGS="$CXXFLAGS -Wthread-safety-analysis"],,[[$CXXFLAG_WERROR]])
+  AX_CHECK_COMPILE_FLAG([-Wthread-safety],[CXXFLAGS="$CXXFLAGS -Wthread-safety"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wrange-loop-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"],,[[$CXXFLAG_WERROR]])
 
   ## Some compilers (gcc) ignore unknown -Wno-* options, but warn about all

src/blockencodings.cpp

@@ -106,13 +106,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
     std::vector<bool> have_txn(txn_available.size());
     {
     LOCK(pool->cs);
-    const std::vector<std::pair<uint256, CTxMemPool::txiter> >& vTxHashes = pool->vTxHashes;
-    for (size_t i = 0; i < vTxHashes.size(); i++) {
-        uint64_t shortid = cmpctblock.GetShortID(vTxHashes[i].first);
+    for (size_t i = 0; i < pool->vTxHashes.size(); i++) {
+        uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[i].first);
         std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid);
         if (idit != shorttxids.end()) {
             if (!have_txn[idit->second]) {
-                txn_available[idit->second] = vTxHashes[i].second->GetSharedTx();
+                txn_available[idit->second] = pool->vTxHashes[i].second->GetSharedTx();
                 have_txn[idit->second]  = true;
                 mempool_count++;
             } else {

src/blockencodings.h

@@ -127,7 +127,7 @@ class PartiallyDownloadedBlock {
 protected:
     std::vector<CTransactionRef> txn_available;
     size_t prefilled_count = 0, mempool_count = 0, extra_count = 0;
-    CTxMemPool* pool;
+    const CTxMemPool* pool;
 public:
     CBlockHeader header;
     explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {}

src/logging.cpp

@@ -33,7 +33,7 @@ static int FileWriteStr(const std::string &str, FILE *fp)
 
 bool BCLog::Logger::StartLogging()
 {
-    LockGuard scoped_lock(m_cs);
+    StdLockGuard scoped_lock(m_cs);
 
     assert(m_buffering);
     assert(m_fileout == nullptr);
@@ -263,7 +263,7 @@ std::string BCLog::Logger::LogThreadNameStr(const std::string &str)
 
 void BCLog::Logger::LogPrintStr(const std::string& str)
 {
-    LockGuard scoped_lock(m_cs);
+    StdLockGuard scoped_lock(m_cs);
     std::string strThreadLogged = LogThreadNameStr(str);
     std::string strTimestamped = LogTimestampStr(strThreadLogged);
 

src/logging.h

@@ -83,7 +83,7 @@ namespace BCLog {
     class Logger
     {
     private:
-        mutable std::mutex m_cs;                   // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected
+        mutable StdMutex m_cs; // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected
 
         FILE* m_fileout GUARDED_BY(m_cs) = nullptr;
         std::list<std::string> m_msgs_before_open GUARDED_BY(m_cs);
@@ -119,7 +119,7 @@ namespace BCLog {
         /** Returns whether logs will be written to any output */
         bool Enabled() const
         {
-            LockGuard scoped_lock(m_cs);
+            StdLockGuard scoped_lock(m_cs);
             return m_buffering || m_print_to_console || m_print_to_file;
         }
 

src/stacktraces.cpp

@@ -10,9 +10,9 @@
 #include <fs.h>
 #include <logging.h>
 #include <streams.h>
+#include <threadsafety.h>
 #include <utilstrencodings.h>
 
-#include <mutex>
 #include <map>
 #include <vector>
 #include <memory>
@@ -169,8 +169,8 @@ static __attribute__((noinline)) std::vector<uint64_t> GetStackFrames(size_t ski
     static BOOL symInitialized = SymInitialize(GetCurrentProcess(), nullptr, TRUE);
 
     // dbghelp is not thread safe
-    static std::mutex m;
-    LockGuard l(m);
+    static StdMutex m;
+    StdLockGuard l(m);
 
     HANDLE process = GetCurrentProcess();
     HANDLE thread = GetCurrentThread();
@@ -531,7 +531,7 @@ static void PrintCrashInfo(const crash_info& ci)
 }
 
 #ifdef ENABLE_CRASH_HOOKS
-static std::mutex g_stacktraces_mutex;
+static StdMutex g_stacktraces_mutex;
 static std::map<void*, std::shared_ptr<std::vector<uint64_t>>> g_stacktraces;
 
 #if CRASH_HOOKS_WRAPPED_CXX_ABI
@@ -594,7 +594,7 @@ extern "C" void* __attribute__((noinline)) WRAPPED_NAME(__cxa_allocate_exception
 
     void* p = __real___cxa_allocate_exception(thrown_size);
 
-    LockGuard l(g_stacktraces_mutex);
+    StdLockGuard l(g_stacktraces_mutex);
     g_stacktraces.emplace(p, st);
     return p;
 }
@@ -603,7 +603,7 @@ extern "C" void __attribute__((noinline)) WRAPPED_NAME(__cxa_free_exception)(voi
 {
     __real___cxa_free_exception(thrown_exception);
 
-    LockGuard l(g_stacktraces_mutex);
+    StdLockGuard l(g_stacktraces_mutex);
     g_stacktraces.erase(thrown_exception);
 }
 
@@ -667,7 +667,7 @@ static std::shared_ptr<std::vector<uint64_t>> GetExceptionStacktrace(const std::
 #ifdef ENABLE_CRASH_HOOKS
     void* p = *(void**)&e;
 
-    LockGuard l(g_stacktraces_mutex);
+    StdLockGuard l(g_stacktraces_mutex);
     auto it = g_stacktraces.find(p);
     if (it == g_stacktraces.end()) {
         return nullptr;

src/sync.cpp

@@ -156,7 +156,8 @@ std::string LocksHeld()
     return result;
 }
 
-void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs)
+template <typename MutexType>
+void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs)
 {
     for (const std::pair<void*, CLockLocation>& i : g_lockstack)
         if (i.first == cs)
@@ -164,6 +165,8 @@ void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine,
     fprintf(stderr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str());
     abort();
 }
+template void AssertLockHeldInternal(const char*, const char*, int, Mutex*);
+template void AssertLockHeldInternal(const char*, const char*, int, CCriticalSection*);
 
 void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs)
 {

src/sync.h

@@ -50,7 +50,8 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII
 void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);
 void LeaveCritical();
 std::string LocksHeld();
-void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs);
+template <typename MutexType>
+void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs);
 void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs);
 void DeleteLock(void* cs);
 
@@ -63,7 +64,8 @@ extern bool g_debug_lockorder_abort;
 #else
 void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
 void static inline LeaveCritical() {}
-void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
+template <typename MutexType>
+void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
 void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {}
 void static inline DeleteLock(void* cs) {}
 #endif

src/threadsafety.h

@@ -56,13 +56,19 @@
 #define ASSERT_EXCLUSIVE_LOCK(...)
 #endif // __GNUC__
 
-// LockGuard provides an annotated version of lock_guard for us
-// should only be used when sync.h Mutex/LOCK/etc aren't usable
-class SCOPED_LOCKABLE LockGuard : public std::lock_guard<std::mutex>
+// StdMutex provides an annotated version of std::mutex for us,
+// and should only be used when sync.h Mutex/LOCK/etc are not usable.
+class LOCKABLE StdMutex : public std::mutex
+{
+};
+
+// StdLockGuard provides an annotated version of std::lock_guard for us,
+// and should only be used when sync.h Mutex/LOCK/etc are not usable.
+class SCOPED_LOCKABLE StdLockGuard : public std::lock_guard<StdMutex>
 {
 public:
-    explicit LockGuard(std::mutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<std::mutex>(cs) { }
-    ~LockGuard() UNLOCK_FUNCTION() {};
+    explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<StdMutex>(cs) {}
+    ~StdLockGuard() UNLOCK_FUNCTION() {}
 };
 
 #endif // BITCOIN_THREADSAFETY_H