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
This commit is contained in:
Wladimir J. van der Laan 2018-05-28 16:06:53 +02:00 committed by pasta
parent cd1dc46115
commit a1c9327e0b

View File

@ -72,7 +72,7 @@ struct LockData {
std::mutex dd_mutex;
} static lockdata;
static thread_local std::unique_ptr<LockStack> lockstack;
static thread_local LockStack g_lockstack;
static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch, const LockStack& s1, const LockStack& s2)
{
@ -107,21 +107,18 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch,
static void push_lock(void* c, const CLockLocation& locklocation)
{
if (!lockstack)
lockstack.reset(new LockStack);
std::lock_guard<std::mutex> 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<void*, CLockLocation> & i : (*lockstack)) {
for (const std::pair<void*, CLockLocation>& i : g_lockstack) {
if (i.first == c)
break;
std::pair<void*, void*> p1 = std::make_pair(i.first, c);
if (lockdata.lockorders.count(p1))
continue;
lockdata.lockorders[p1] = (*lockstack);
lockdata.lockorders[p1] = g_lockstack;
std::pair<void*, void*> 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<void*, CLockLocation> & i : *lockstack)
for (const std::pair<void*, CLockLocation>& 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<void*, CLockLocation> & i : *lockstack)
for (const std::pair<void*, CLockLocation>& 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<void*, CLockLocation>& i : *lockstack) {
for (const std::pair<void*, CLockLocation>& 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();