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

Merge pull request #630 from Dmitry-Me/detectRaceInInterlockedAccess 

Prototype for detecting non-interlocked check after InterlockedDecrement()
This commit is contained in:
amai2012 2015-08-06 15:43:10 +02:00
parent 07a064aab0 43800a9419
commit 53feb88614
3 changed files with 243 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
lib/checkother.cpp
View File

@ -2590,3 +2590,30 @@ void CheckOther::checkLibraryMatchFunctions()
}
}
}
void CheckOther::checkInterlockedDecrement()
{
if (!_settings->isWindowsPlatform()) {
return;
}
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "InterlockedDecrement ( & %name% ) ; if ( %name%|!|0")) {
const Token* interlockedVarTok = tok->tokAt(3);
const Token* checkStartTok = interlockedVarTok->tokAt(5);
if ((Token::Match(checkStartTok, "0 %comp% %name% )") && checkStartTok->strAt(2) == interlockedVarTok->str()) ||
(Token::Match(checkStartTok, "! %name% )") && checkStartTok->strAt(1) == interlockedVarTok->str()) ||
(Token::Match(checkStartTok, "%name% )") && checkStartTok->str() == interlockedVarTok->str()) ||
(Token::Match(checkStartTok, "%name% %comp% 0 )") && checkStartTok->str() == interlockedVarTok->str())) {
raceAfterInterlockedDecrementError(checkStartTok);
}
}
}
}
void CheckOther::raceAfterInterlockedDecrementError(const Token* tok)
{
reportError(tok, Severity::error, "raceAfterInterlockedDecrement",
"Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.");
}

8
lib/checkother.h
View File

@ -72,6 +72,7 @@ public:
// --check-library : functions with nonmatching configuration
checkOther.checkLibraryMatchFunctions();
checkOther.checkInterlockedDecrement();
}
/** @brief Run checks against the simplified token list */
@ -227,6 +228,10 @@ public:
/** @brief --check-library: warn for unconfigured function calls */
void checkLibraryMatchFunctions();
void checkInterlockedDecrement();
void checkNewAfterDelete();
private:
// Error messages..
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result);
@ -282,6 +287,7 @@ private:
void commaSeparatedReturnError(const Token *tok);
void ignoredReturnValueError(const Token* tok, const std::string& function);
void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive);
void raceAfterInterlockedDecrementError(const Token* tok);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckOther c(0, settings, errorLogger);
@ -295,6 +301,7 @@ private:
c.invalidPointerCastError(0, "float", "double", false);
c.negativeBitwiseShiftError(0);
c.checkPipeParameterSizeError(0, "varname", "dimension");
c.raceAfterInterlockedDecrementError(0);
//performance
c.redundantCopyError(0, "varname");
@ -357,6 +364,7 @@ private:
"- provide wrong dimensioned array to pipe() system command (--std=posix)\n"
"- cast the return values of getc(),fgetc() and getchar() to character and compare it to EOF\n"
"- invalid input values for functions\n"
"- race condition with non-interlocked access after InterlockedDecrement() call\n"
// warning
"- either division by zero or useless condition\n"

208
test/testother.cpp
View File

@ -167,6 +167,7 @@ private:
TEST_CASE(redundantPointerOp);
TEST_CASE(test_isSameExpression);
TEST_CASE(raceAfterInterlockedDecrement);
}
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) {
@ -212,6 +213,13 @@ private:
&settings);
}
void checkInterlockedDecrement(const char code[]) {
Settings settings;
settings.platformType = Settings::Win32A;
check(code, nullptr, false, false, true, &settings);
}
void check_preprocess_suppress(const char precode[], const char *filename = nullptr) {
// Clear the error buffer..
errout.str("");
@ -6245,6 +6253,206 @@ private:
"};", "test.cpp", false, false);
TODO_ASSERT_EQUALS("", "[test.cpp:1] -> [test.cpp:1]: (style) Same expression on both sides of '||'.\n", errout.str());
}
void raceAfterInterlockedDecrement() {
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" whatever();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (counter)\n"
" return;\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (!counter)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (counter > 0)\n"
" return;\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (0 < counter)\n"
" return;\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (counter == 0)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (0 == counter)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (0 != counter)\n"
" return;\n"
" destroy()\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (counter != 0)\n"
" return;\n"
" destroy()\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (counter <= 0)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" InterlockedDecrement(&counter);\n"
" if (0 >= counter)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead.\n", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (newCount)\n"
" return;\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (!newCount)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (newCount > 0)\n"
" return;\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (0 < newCount)\n"
" return;\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (newCount == 0)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (0 == newCount)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (0 != newCount)\n"
" return;\n"
" destroy()\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (newCount != 0)\n"
" return;\n"
" destroy()\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (newCount <= 0)\n"
" destroy();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
checkInterlockedDecrement(
"void f() {\n"
" int counter = 0;\n"
" int newCount = InterlockedDecrement(&counter);\n"
" if (0 >= newCount)\n"
" destroy;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestOther)