From a1c9327e0be389ee0ed9e4171767f00e98139969 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 28 May 2018 16:06:53 +0200 Subject: [PATCH] Merge #13300: qa: Initialize lockstack to prevent null pointer deref fa9da85b7cc759d06bc24854be2bad0ea87b6006 qa: Initialize lockstack to prevent null pointer deref (MarcoFalke) Pull request description: It is currently impossible to call debug methods such as `AssertLock(Not)Held` on a thread without running into undefined behavior, unless a lock was pushed on the stack in this thread. Initializing the global `lockstack` seems to fix both issues. Tree-SHA512: 8cb76b22cb31887ddf15742fdc790f01e8f04ed837367d0fd4996535748d124342e8bfde68952b903847b96ad33406c64907a53ebab9646f78d97fa4365c3061 --- src/sync.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/sync.cpp b/src/sync.cpp index 96df74a778..0409ed2911 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -72,7 +72,7 @@ struct LockData { std::mutex dd_mutex; } static lockdata; -static thread_local std::unique_ptr lockstack; +static thread_local LockStack g_lockstack; static void potential_deadlock_detected(const std::pair& mismatch, const LockStack& s1, const LockStack& s2) { @@ -107,21 +107,18 @@ static void potential_deadlock_detected(const std::pair& mismatch, static void push_lock(void* c, const CLockLocation& locklocation) { - if (!lockstack) - lockstack.reset(new LockStack); - std::lock_guard lock(lockdata.dd_mutex); - lockstack->push_back(std::make_pair(c, locklocation)); + g_lockstack.push_back(std::make_pair(c, locklocation)); - for (const std::pair & i : (*lockstack)) { + for (const std::pair& i : g_lockstack) { if (i.first == c) break; std::pair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) continue; - lockdata.lockorders[p1] = (*lockstack); + lockdata.lockorders[p1] = g_lockstack; std::pair p2 = std::make_pair(c, i.first); lockdata.invlockorders.insert(p2); @@ -132,7 +129,7 @@ static void push_lock(void* c, const CLockLocation& locklocation) static void pop_lock() { - (*lockstack).pop_back(); + g_lockstack.pop_back(); } void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) @@ -148,14 +145,14 @@ void LeaveCritical() std::string LocksHeld() { std::string result; - for (const std::pair & i : *lockstack) + for (const std::pair& i : g_lockstack) result += i.second.ToString() + std::string("\n"); return result; } void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) { - for (const std::pair & i : *lockstack) + for (const std::pair& i : g_lockstack) if (i.first == cs) return; fprintf(stderr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str()); @@ -164,7 +161,7 @@ void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) { - for (const std::pair& i : *lockstack) { + for (const std::pair& i : g_lockstack) { if (i.first == cs) { fprintf(stderr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str()); abort();