From 51fababb9c7934749beb33c8ee61de01e824f1f5 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 9 Jun 2021 17:32:42 +0530 Subject: [PATCH] merge #18635: Replace -Wthread-safety-analysis with -Wthread-safety --- configure.ac | 4 ++-- src/blockencodings.cpp | 7 +++---- src/blockencodings.h | 2 +- src/logging.cpp | 4 ++-- src/logging.h | 4 ++-- src/stacktraces.cpp | 14 +++++++------- src/sync.cpp | 5 ++++- src/sync.h | 6 ++++-- src/threadsafety.h | 16 +++++++++++----- 9 files changed, 36 insertions(+), 26 deletions(-) diff --git a/configure.ac b/configure.ac index f8368abfab..4de43ecdf0 100644 --- a/configure.ac +++ b/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 diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index b4574ed407..daa3aa0b55 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -106,13 +106,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c std::vector have_txn(txn_available.size()); { LOCK(pool->cs); - const std::vector >& 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::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 { diff --git a/src/blockencodings.h b/src/blockencodings.h index 954055488a..2f3a829796 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -127,7 +127,7 @@ class PartiallyDownloadedBlock { protected: std::vector 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) {} diff --git a/src/logging.cpp b/src/logging.cpp index 65a2c3c8c9..ba693b18f4 100644 --- a/src/logging.cpp +++ b/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); diff --git a/src/logging.h b/src/logging.h index df49833f4e..f04dde9dab 100644 --- a/src/logging.h +++ b/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 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; } diff --git a/src/stacktraces.cpp b/src/stacktraces.cpp index 83cf125a23..ab1abde382 100644 --- a/src/stacktraces.cpp +++ b/src/stacktraces.cpp @@ -10,9 +10,9 @@ #include #include #include +#include #include -#include #include #include #include @@ -169,8 +169,8 @@ static __attribute__((noinline)) std::vector 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>> 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> 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; diff --git a/src/sync.cpp b/src/sync.cpp index 422a25daa3..dd82b523f7 100644 --- a/src/sync.cpp +++ b/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 +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { for (const std::pair& 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) { diff --git a/src/sync.h b/src/sync.h index 94152b9080..916633c4f0 100644 --- a/src/sync.h +++ b/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 +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 +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 diff --git a/src/threadsafety.h b/src/threadsafety.h index 1fe1e50f78..300ea44298 100644 --- a/src/threadsafety.h +++ b/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 +// 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 { public: - explicit LockGuard(std::mutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard(cs) { } - ~LockGuard() UNLOCK_FUNCTION() {}; + explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard(cs) {} + ~StdLockGuard() UNLOCK_FUNCTION() {} }; #endif // BITCOIN_THREADSAFETY_H