walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Merge pull request #2708 from pfultz2/multi-mutex-lock 

Handle FPs: mutexes being locked at different scopes
This commit is contained in:
Daniel Marjamäki 2020-07-16 08:35:05 +02:00 committed by GitHub
parent c02b39a3ba 1f74400f13
commit 47ff29f1c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 120 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
lib/checkstl.cpp
View File

@ -2394,12 +2394,25 @@ void CheckStl::useStlAlgorithm()
}
}
static bool isMutex(const Variable* var)
{
const Token* tok = Token::typeDecl(var->nameToken()).first;
return Token::Match(tok, "std :: mutex|recursive_mutex|timed_mutex|recursive_timed_mutex|shared_mutex");
}
static bool isLockGuard(const Variable* var)
{
const Token* tok = Token::typeDecl(var->nameToken()).first;
return Token::Match(tok, "std :: lock_guard|unique_lock|scoped_lock");
}
static bool isLocalMutex(const Variable* var, const Scope* scope)
{
if (!var)
return false;
return !var->isReference() && !var->isRValueReference() && !var->isStatic() && var->scope() == scope;
}
void CheckStl::globalLockGuardError(const Token* tok)
{
reportError(tok, Severity::warning,
@ -2407,19 +2420,42 @@ void CheckStl::globalLockGuardError(const Token* tok)
"Lock guard is defined globally. Lock guards are intended to be local. A global lock guard could lead to a deadlock since it won't unlock until the end of the program.", CWE833, false);
}
void CheckStl::localMutexError(const Token* tok)
{
reportError(tok, Severity::warning,
"localMutex",
"The lock is ineffective because the mutex is locked at the same scope as the mutex itself.", CWE667, false);
}
void CheckStl::checkMutexes()
{
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
std::set<nonneg int> checkedVars;
for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
if (!Token::Match(tok, "%var%"))
continue;
const Variable* var = tok->variable();
if (!var)
continue;
if (Token::Match(tok, "%var% (|{ %var% )|}|,")) {
if (Token::Match(tok, "%var% . lock ( )")) {
if (!isMutex(var))
continue;
if (!checkedVars.insert(var->declarationId()).second)
continue;
if (isLocalMutex(var, tok->scope()))
localMutexError(tok);
} else if (Token::Match(tok, "%var% (|{ %var% )|}|,")) {
if (!isLockGuard(var))
continue;
const Variable* mvar = tok->tokAt(2)->variable();
if (!mvar)
continue;
if (!checkedVars.insert(mvar->declarationId()).second)
continue;
if (var->isStatic() || var->isGlobal())
globalLockGuardError(tok);
else if (isLocalMutex(mvar, tok->scope()))
localMutexError(tok);
}
}
}

2
lib/checkstl.h
View File

@ -233,6 +233,7 @@ private:
void useStlAlgorithmError(const Token *tok, const std::string &algoName);
void globalLockGuardError(const Token *tok);
void localMutexError(const Token *tok);
void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const OVERRIDE {
ErrorPath errorPath;
@ -271,6 +272,7 @@ private:
c.readingEmptyStlContainerError(nullptr);
c.useStlAlgorithmError(nullptr, "");
c.globalLockGuardError(nullptr);
c.localMutexError(nullptr);
}
static std::string myName() {

81
test/teststl.cpp
View File

@ -4445,6 +4445,28 @@ private:
true);
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" std::mutex m;\n"
" std::lock_guard<std::mutex> g(m);\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:3]: (warning) The lock is ineffective because the mutex is locked at the same scope as the mutex itself.\n", errout.str());
check("void f() {\n"
" std::mutex m;\n"
" std::unique_lock<std::mutex> g(m);\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:3]: (warning) The lock is ineffective because the mutex is locked at the same scope as the mutex itself.\n", errout.str());
check("void f() {\n"
" std::mutex m;\n"
" std::unique_lock<std::mutex> g(m, std::defer_lock);\n"
" std::lock(g);\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:3]: (warning) The lock is ineffective because the mutex is locked at the same scope as the mutex itself.\n", errout.str());
check("void g();\n"
"void f() {\n"
" static std::mutex m;\n"
@ -4455,6 +4477,16 @@ private:
true);
ASSERT_EQUALS("", errout.str());
check("void g();\n"
"void f() {\n"
" std::mutex m;\n"
" m.lock();\n"
" g();\n"
" m.unlock();\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:4]: (warning) The lock is ineffective because the mutex is locked at the same scope as the mutex itself.\n", errout.str());
check("class A {\n"
" std::mutex m;\n"
" void f() {\n"
@ -4503,6 +4535,55 @@ private:
"}\n",
true);
ASSERT_EQUALS("", errout.str());
check("std::mutex& h();\n"
"void f() {\n"
" auto m = h();\n"
" std::lock_guard<std::mutex> g(m);\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:4]: (warning) The lock is ineffective because the mutex is locked at the same scope as the mutex itself.\n", errout.str());
check("void g();\n"
"std::mutex& h();\n"
"void f() {\n"
" auto m = h();\n"
" m.lock();\n"
" g();\n"
" m.unlock();\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:5]: (warning) The lock is ineffective because the mutex is locked at the same scope as the mutex itself.\n", errout.str());
check("void foo();\n"
"void bar();\n"
"void f() {\n"
" std::mutex m;\n"
" std::thread t([&m](){\n"
" m.lock();\n"
" foo();\n"
" m.unlock();\n"
" });\n"
" m.lock();\n"
" bar();\n"
" m.unlock();\n"
"}\n",
true);
ASSERT_EQUALS("", errout.str());
check("void foo();\n"
"void bar();\n"
"void f() {\n"
" std::mutex m;\n"
" std::thread t([&m](){\n"
" std::unique_lock<std::mutex> g{m};\n"
" foo();\n"
" });\n"
" std::unique_lock<std::mutex> g{m};\n"
" bar();\n"
"}\n",
true);
ASSERT_EQUALS("", errout.str());
}
};