Add check for incorrect usage of mutexes and lock guards

This commit is contained in:
Paul 2020-06-15 19:40:54 -05:00
parent 6faad9cd90
commit 172537807b
3 changed files with 137 additions and 0 deletions

View File

@ -46,11 +46,13 @@ static const struct CWE CWE398(398U); // Indicator of Poor Code Quality
static const struct CWE CWE597(597U); // Use of Wrong Operator in String Comparison
static const struct CWE CWE628(628U); // Function Call with Incorrectly Specified Arguments
static const struct CWE CWE664(664U); // Improper Control of a Resource Through its Lifetime
static const struct CWE CWE667(667U); // Improper Locking
static const struct CWE CWE704(704U); // Incorrect Type Conversion or Cast
static const struct CWE CWE762(762U); // Mismatched Memory Management Routines
static const struct CWE CWE786(786U); // Access of Memory Location Before Start of Buffer
static const struct CWE CWE788(788U); // Access of Memory Location After End of Buffer
static const struct CWE CWE825(825U); // Expired Pointer Dereference
static const struct CWE CWE833(833U); // Deadlock
static const struct CWE CWE834(834U); // Excessive Iteration
void CheckStl::outOfBounds()
@ -2391,3 +2393,55 @@ void CheckStl::useStlAlgorithm()
}
}
}
static bool isMutex(const Variable* var)
{
const Token* tok = var->typeStartToken();
return Token::Match(tok, "std :: mutex|recursive_mutex|timed_mutex|recursive_timed_mutex|shared_mutex");
}
static bool isLockGuard(const Variable* var)
{
const Token* tok = var->typeStartToken();
return Token::Match(tok, "std :: lock_guard|unique_lock|scoped_lock");
}
void CheckStl::globalLockGuardError(const Token* tok)
{
reportError(tok, Severity::error,
"globalLockGuard",
"The lock guard won't unlock until the end of the program which could lead to a deadlock.", CWE833, false);
}
void CheckStl::localMutexError(const Token* tok)
{
reportError(tok, Severity::error,
"localMutex",
"The mutex is locked at the same scope as the mutex itself.", CWE667, false);
}
void CheckStl::checkMutexes()
{
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
const Variable* var = tok->variable();
if (!var)
continue;
if (Token::Match(tok, "%var% . lock ( )")) {
if (!isMutex(var))
continue;
if (!var->isStatic() && var->scope() == tok->scope())
localMutexError(tok);
} else if (Token::Match(tok, "%var% (|{ %var% )|}")) {
if (!isLockGuard(var))
continue;
const Variable* mvar = tok->tokAt(2)->variable();
if (var->isStatic() || var->isGlobal())
globalLockGuardError(tok);
else if (mvar && !mvar->isStatic() && mvar->scope() == tok->scope())
localMutexError(tok);
}
}
}
}

View File

@ -84,6 +84,7 @@ public:
checkStl.stlBoundaries();
checkStl.checkDereferenceInvalidIterator();
checkStl.checkMutexes();
// Style check
checkStl.size();
@ -185,6 +186,8 @@ public:
/** @brief Look for loops that can replaced with std algorithms */
void useStlAlgorithm();
void checkMutexes();
private:
bool isContainerSize(const Token *containerToken, const Token *expr) const;
@ -229,6 +232,9 @@ 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;
CheckStl c(nullptr, settings, errorLogger);
@ -265,6 +271,8 @@ private:
c.dereferenceInvalidIteratorError(nullptr, "i");
c.readingEmptyStlContainerError(nullptr);
c.useStlAlgorithmError(nullptr, "");
c.globalLockGuardError(nullptr);
c.localMutexError(nullptr);
}
static std::string myName() {

View File

@ -165,6 +165,8 @@ private:
TEST_CASE(invalidContainer);
TEST_CASE(invalidContainerLoop);
TEST_CASE(findInsert);
TEST_CASE(checkMutexes);
}
void check(const char code[], const bool inconclusive=false, const Standards::cppstd_t cppstandard=Standards::CPPLatest) {
@ -4411,6 +4413,79 @@ private:
true);
ASSERT_EQUALS("", errout.str());
}
void checkMutexes() {
check("void f() {\n"
" static std::mutex m;\n"
" static std::lock_guard<std::mutex> g(m);\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:3]: (error) The lock guard won't unlock until the end of the program which could lead to a deadlock.\n", errout.str());
check("void f() {\n"
" static std::mutex m;\n"
" std::lock_guard<std::mutex> g(m);\n"
"}\n",
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]: (error) 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"
" m.lock();\n"
" g();\n"
" m.unlock();\n"
"}\n",
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]: (error) 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"
" std::lock_guard<std::mutex> g(m);\n"
" }\n"
"};\n",
true);
ASSERT_EQUALS("", errout.str());
check("class A {\n"
" std::mutex m;\n"
" void g();\n"
" void f() {\n"
" m.lock();\n"
" g();\n"
" m.unlock();\n"
" }\n"
"};\n",
true);
ASSERT_EQUALS("", errout.str());
check("class A {\n"
" std::mutex m;\n"
" void f() {\n"
" static std::lock_guard<std::mutex> g(m);\n"
" }\n"
"};\n",
true);
ASSERT_EQUALS("[test.cpp:4]: (error) The lock guard won't unlock until the end of the program which could lead to a deadlock.\n", errout.str());
}
};
REGISTER_TEST(TestStl)