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

Extend constStatement checker 

This reworks constStatement to find more issues. It catches issue [8827](https://trac.cppcheck.net/ticket/8827):

```cpp
extern void foo(int,const char*,int);
void f(int value)
{
        foo(42,"test",42),(value&42);
}
```

It also catches from issue [8451](https://trac.cppcheck.net/ticket/8451):

```cpp
void f1(int x) {
    1;
    (1);
    (char)1;
    ((char)1);
    !x;
    (!x);
    ~x;
}
```

And also:

```cpp
void f(int x) {
    x;
}
```

The other examples are not caught due to incomplete AST.
This commit is contained in:
Paul Fultz II 2019-02-15 13:31:40 +01:00 committed by Daniel Marjamäki
parent dc4e7cef88
commit cf1ad5087a
4 changed files with 137 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

179
lib/checkother.cpp
View File

@ -734,57 +734,6 @@ void CheckOther::suspiciousCaseInSwitchError(const Token* tok, const std::string
"Using an operator like '" + operatorString + "' in a case label is suspicious. Did you intend to use a bitwise operator, multiple case labels or if/else instead?", CWE398, true);
}
//---------------------------------------------------------------------------
// if (x == 1)
// x == 0; // <- suspicious equality comparison.
//---------------------------------------------------------------------------
void CheckOther::checkSuspiciousEqualityComparison()
{
if (!mSettings->isEnabled(Settings::WARNING) || !mSettings->inconclusive)
return;
const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * scope : symbolDatabase->functionScopes) {
for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
if (Token::simpleMatch(tok, "for (")) {
const Token* const openParen = tok->next();
const Token* const closeParen = tok->linkAt(1);
// Search for any suspicious equality comparison in the initialization
// or increment-decrement parts of the for() loop.
// For example:
// for (i == 2; i < 10; i++)
// or
// for (i = 0; i < 10; i == a)
if (Token::Match(openParen->next(), "%name% =="))
suspiciousEqualityComparisonError(openParen->tokAt(2));
if (closeParen->strAt(-2) == "==")
suspiciousEqualityComparisonError(closeParen->tokAt(-2));
// Skip over for() loop conditions because "for (;running==1;)"
// is a bit strange, but not necessarily incorrect.
tok = closeParen;
} else if (Token::Match(tok, "[;{}] *| %name% == %any% ;")) {
// Exclude compound statements surrounded by parentheses, such as
// printf("%i\n", ({x==0;}));
// because they may appear as an expression in GNU C/C++.
// See http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
const Token* afterStatement = tok->strAt(1) == "*" ? tok->tokAt(6) : tok->tokAt(5);
if (!Token::simpleMatch(afterStatement, "} )"))
suspiciousEqualityComparisonError(tok->next());
}
}
}
}
void CheckOther::suspiciousEqualityComparisonError(const Token* tok)
{
reportError(tok, Severity::warning, "suspiciousEqualityComparison",
"Found suspicious equality comparison. Did you intend to assign a value instead?", CWE482, true);
}
//---------------------------------------------------------------------------
// Find consecutive return, break, continue, goto or throw statements. e.g.:
// break; break;
@ -1389,64 +1338,106 @@ void CheckOther::charBitOpError(const Token *tok)
//---------------------------------------------------------------------------
// Incomplete statement..
//---------------------------------------------------------------------------
static bool isConstStatement(const Token *tok)
{
if (!tok)
return false;
if (tok->isExpandedMacro())
return false;
if (Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL"))
return true;
if (Token::Match(tok, "%var%"))
return true;
if (Token::Match(tok, "*|&|&&") &&
(Token::Match(tok->previous(), "::|.") || Token::Match(tok->astOperand1(), "%type%") ||
Token::Match(tok->astOperand2(), "%type%")))
return false;
if (Token::Match(tok, "<<|>>") && !astIsIntegral(tok, false))
return false;
if (Token::Match(tok, "!|~|%cop%") && (tok->astOperand1() || tok->astOperand2()))
return true;
if (Token::simpleMatch(tok->previous(), "sizeof ("))
return true;
if (isCPPCast(tok))
return isConstStatement(tok->astOperand2());
if (Token::Match(tok, "( %type%"))
return isConstStatement(tok->astOperand1());
if (Token::simpleMatch(tok, ","))
return isConstStatement(tok->astOperand2());
return false;
}
static bool isVoidStmt(const Token *tok)
{
if (Token::simpleMatch(tok, "( void"))
return true;
const Token *tok2 = tok;
while (tok2->astOperand1())
tok2 = tok2->astOperand1();
if (Token::simpleMatch(tok2->previous(), ")") && Token::simpleMatch(tok2->previous()->link(), "( void"))
return true;
if (Token::simpleMatch(tok2, "( void"))
return true;
return Token::Match(tok2->previous(), "delete|throw|return");
}
static bool isConstTop(const Token *tok)
{
if (!tok)
return false;
if (tok == tok->astTop())
return true;
if (Token::simpleMatch(tok->astParent(), ";") && tok->astTop() &&
Token::Match(tok->astTop()->previous(), "for|if (") && Token::simpleMatch(tok->astTop()->astOperand2(), ";")) {
if (Token::simpleMatch(tok->astParent()->astParent(), ";"))
return tok->astParent()->astOperand2() == tok;
else
return tok->astParent()->astOperand1() == tok;
}
return false;
}
void CheckOther::checkIncompleteStatement()
{
if (!mSettings->isEnabled(Settings::WARNING))
return;
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "(|["))
tok = tok->link();
else if (tok->str() == "{" && tok->astParent())
tok = tok->link();
// C++11 struct/array/etc initialization in initializer list
else if (Token::Match(tok->previous(), "%var%|] {"))
tok = tok->link();
if (!Token::Match(tok, "[;{}] %str%|%num%"))
const Scope *scope = tok->scope();
if (scope && !scope->isExecutable())
continue;
// No warning if numeric constant is followed by a "." or ","
if (Token::Match(tok->next(), "%num% [,.]"))
if (!isConstTop(tok))
continue;
// No warning for [;{}] (void *) 0 ;
if (Token::Match(tok, "[;{}] 0 ;") && (tok->next()->isCast() || tok->next()->isExpandedMacro()))
const Token *rtok = nextAfterAstRightmostLeaf(tok);
if (!Token::simpleMatch(tok->astParent(), ";") && !Token::simpleMatch(rtok, ";") &&
!Token::Match(tok->previous(), ";|}|{ %any% ;"))
continue;
// bailout if there is a "? :" in this statement
bool bailout = false;
for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) {
if (tok2->str() == "?") {
bailout = true;
break;
} else if (tok2->str() == ";")
break;
}
if (bailout)
// Skipe statement expressions
if (Token::simpleMatch(rtok, "; } )"))
continue;
// no warning if this is the last statement in a ({})
for (const Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) {
if (tok2->str() == "(")
tok2 = tok2->link();
else if (Token::Match(tok2, "[;{}]")) {
bailout = Token::simpleMatch(tok2, "; } )");
break;
}
}
if (bailout)
if (!isConstStatement(tok))
continue;
constStatementError(tok->next(), tok->next()->isNumber() ? "numeric" : "string");
if (isVoidStmt(tok))
continue;
bool inconclusive = Token::Match(tok, "%cop%");
if (mSettings->inconclusive || !inconclusive)
constStatementError(tok, tok->isNumber() ? "numeric" : "string", inconclusive);
}
}
void CheckOther::constStatementError(const Token *tok, const std::string &type)
void CheckOther::constStatementError(const Token *tok, const std::string &type, bool inconclusive)
{
reportError(tok, Severity::warning, "constStatement", "Redundant code: Found a statement that begins with " + type + " constant.", CWE398, false);
std::string msg;
if (Token::simpleMatch(tok, "=="))
msg = "Found suspicious equality comparison. Did you intend to assign a value instead?";
else if (Token::Match(tok, ",|!|~|%cop%"))
msg = "Found suspicious operator '" + tok->str() + "'";
else if (Token::Match(tok, "%var%"))
msg = "Unused variable value '" + tok->str() + "'";
else
msg = "Redundant code: Found a statement that begins with " + type + " constant.";
reportError(tok, Severity::warning, "constStatement", msg, CWE398, inconclusive);
}
//---------------------------------------------------------------------------

12
lib/checkother.h
View File

@ -83,6 +83,7 @@ public:
checkOther.checkFuncArgNamesDifferent();
checkOther.checkShadowVariables();
checkOther.checkConstArgument();
checkOther.checkIncompleteStatement();
}
/** @brief Run checks against the simplified token list */
@ -93,7 +94,6 @@ public:
checkOther.clarifyCalculation();
checkOther.clarifyStatement();
checkOther.checkPassByReference();
checkOther.checkIncompleteStatement();
checkOther.checkCastIntToCharAndBack();
checkOther.checkMisusedScopedObject();
@ -101,7 +101,6 @@ public:
checkOther.checkInvalidFree();
checkOther.checkRedundantCopy();
checkOther.checkSuspiciousEqualityComparison();
checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse();
checkOther.checkAccessOfMovedVariable();
}
@ -149,9 +148,6 @@ public:
/** @brief %Check for code like 'case A||B:'*/
void checkSuspiciousCaseInSwitch();
/** @brief %Check for code like 'case A||B:'*/
void checkSuspiciousEqualityComparison();
/** @brief %Check for objects that are destroyed immediately */
void checkMisusedScopedObject();
@ -228,7 +224,7 @@ private:
void cstyleCastError(const Token *tok);
void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive);
void passedByValueError(const Token *tok, const std::string &parname, bool inconclusive);
void constStatementError(const Token *tok, const std::string &type);
void constStatementError(const Token *tok, const std::string &type, bool inconclusive);
void signedCharArrayIndexError(const Token *tok);
void unknownSignCharArrayIndexError(const Token *tok);
void charBitOpError(const Token *tok);
@ -241,7 +237,6 @@ private:
void redundantCopyInSwitchError(const Token *tok1, const Token* tok2, const std::string &var);
void redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname);
void suspiciousCaseInSwitchError(const Token* tok, const std::string& operatorString);
void suspiciousEqualityComparisonError(const Token* tok);
void selfAssignmentError(const Token *tok, const std::string &varname);
void misusedScopeObjectError(const Token *tok, const std::string &varname);
void duplicateBranchError(const Token *tok1, const Token *tok2, ErrorPath errors);
@ -298,7 +293,7 @@ private:
c.checkCastIntToCharAndBackError(nullptr, "func_name");
c.cstyleCastError(nullptr);
c.passedByValueError(nullptr, "parametername", false);
c.constStatementError(nullptr, "type");
c.constStatementError(nullptr, "type", false);
c.signedCharArrayIndexError(nullptr);
c.unknownSignCharArrayIndexError(nullptr);
c.charBitOpError(nullptr);
@ -306,7 +301,6 @@ private:
c.redundantAssignmentInSwitchError(nullptr, nullptr, "var");
c.redundantCopyInSwitchError(nullptr, nullptr, "var");
c.suspiciousCaseInSwitchError(nullptr, "||");
c.suspiciousEqualityComparisonError(nullptr);
c.selfAssignmentError(nullptr, "varname");
c.clarifyCalculationError(nullptr, "+");
c.clarifyStatementError(nullptr);

40
test/testincompletestatement.cpp
View File

@ -33,10 +33,11 @@ public:
private:
Settings settings;
void check(const char code[]) {
void check(const char code[], bool inconclusive = false) {
// Clear the error buffer..
errout.str("");
settings.inconclusive = inconclusive;
// Raw tokens..
std::vector<std::string> files(1, "test.cpp");
@ -52,7 +53,6 @@ private:
Tokenizer tokenizer(&settings, this);
tokenizer.createTokens(&tokens2);
tokenizer.simplifyTokens1("");
tokenizer.simplifyTokenList2();
// Check for incomplete statements..
CheckOther checkOther(&tokenizer, &settings, this);
@ -82,6 +82,8 @@ private:
TEST_CASE(cpp11init2); // #8449
TEST_CASE(block); // ({ do_something(); 0; })
TEST_CASE(mapindex);
TEST_CASE(commaoperator);
TEST_CASE(redundantstmts);
}
void test1() {
@ -299,6 +301,40 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
}
// #8827
void commaoperator() {
check("void foo(int,const char*,int);\n"
"void f(int value) {\n"
" foo(42,\"test\",42),(value&42);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (warning) Found suspicious operator ','\n", errout.str());
}
// #8451
void redundantstmts() {
check("void f1(int x) {\n"
" 1;\n"
" (1);\n"
" (char)1;\n"
" ((char)1);\n"
" !x;\n"
" (!x);\n"
" (unsigned int)!x;\n"
" ~x;\n"
"}\n", true);
ASSERT_EQUALS("[test.cpp:2]: (warning) Redundant code: Found a statement that begins with numeric constant.\n"
"[test.cpp:3]: (warning) Redundant code: Found a statement that begins with numeric constant.\n"
"[test.cpp:4]: (warning) Redundant code: Found a statement that begins with string constant.\n"
"[test.cpp:5]: (warning) Redundant code: Found a statement that begins with string constant.\n"
"[test.cpp:6]: (warning, inconclusive) Found suspicious operator '!'\n"
"[test.cpp:7]: (warning, inconclusive) Found suspicious operator '!'\n"
"[test.cpp:9]: (warning, inconclusive) Found suspicious operator '~'\n", errout.str());
check("void f1(int x) { x; }", true);
ASSERT_EQUALS("[test.cpp:1]: (warning) Unused variable value 'x'\n", errout.str());
}
};
REGISTER_TEST(TestIncompleteStatement)

18
test/testother.cpp
View File

@ -1988,7 +1988,7 @@ private:
" switch (a)\n"
" {\n"
" case 2:\n"
" y;\n"
" (void)y;\n"
" case 3:\n"
" ++y;\n"
" }\n"
@ -2042,7 +2042,7 @@ private:
" switch (a)\n"
" {\n"
" case 2:\n"
" y;\n"
" (void)y;\n"
" case 3:\n"
" --y;\n"
" }\n"
@ -2741,8 +2741,12 @@ private:
// #4711 lambda functions
check("int f() {\n"
" return g([](int x){x+1; return x;});\n"
"}", nullptr, false, false, false);
" return g([](int x){(void)x+1; return x;});\n"
"}",
nullptr,
false,
false,
false);
ASSERT_EQUALS("", errout.str());
// #4756
@ -2754,7 +2758,7 @@ private:
" __v = ((unsigned short int) ((((__x) >> 8) & 0xff) | (((__x) & 0xff) << 8)));\n"
" else\n"
" __asm__ (\"rorw $8, %w0\" : \"=r\" (__v) : \"0\" (__x) : \"cc\");\n"
" __v;\n"
" (void)__v;\n"
" }));\n"
"}", nullptr, false, false, false);
ASSERT_EQUALS("", errout.str());
@ -5468,13 +5472,13 @@ private:
check("void foo()\n"
"{\n"
" int a; a = 123;\n"
" a << -1;\n"
" (void)(a << -1);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value is undefined behaviour\n", errout.str());
check("void foo()\n"
"{\n"
" int a; a = 123;\n"
" a >> -1;\n"
" (void)(a >> -1);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value is undefined behaviour\n", errout.str());
check("void foo()\n"