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

Find duplicate expressions assigned to the same variable (#1129) 

* Check for duplicate assignments

* Improve checking of expression

* Add more tests

* Use simple match

* Improve robustness of check

* check for null

* Reduce side effects by checking for side effects

* Improve verbose message

* Reword the error message
This commit is contained in:
Paul Fultz II 2018-04-08 07:43:19 -05:00 committed by Daniel Marjamäki
parent ee5c60e8f6
commit 95fc84a26b
4 changed files with 164 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/astutils.cpp
View File

@ -165,7 +165,7 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
if (tok1->isName() && tok1->next()->str() == "(" && tok1->str() != "sizeof") {
if (!tok1->function() && !Token::Match(tok1->previous(), ".|::") && pure && !library.isFunctionConst(tok1->str(), true) && !tok1->isAttributeConst() && !tok1->isAttributePure())
return false;
else if (tok1->function() && !tok1->function()->isConst() && !tok1->function()->isAttributeConst() && !tok1->function()->isAttributePure())
else if (tok1->function() && !tok1->function()->isConst() && !tok1->function()->isAttributeConst() && !tok1->function()->isAttributePure() && pure)
return false;
}
// templates/casts

40
lib/checkother.cpp
View File

@ -1928,6 +1928,35 @@ void CheckOther::checkDuplicateExpression()
continue;
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
if(tok->str() == "=" && Token::Match(tok->astOperand1(), "%var%")) {
const Token * endStatement = Token::findsimplematch(tok, ";");
if(Token::Match(endStatement, "; %type% %var% ;")) {
endStatement = endStatement->tokAt(4);
}
if(Token::Match(endStatement, "%var% %assign%")) {
const Token * nextAssign = endStatement->tokAt(1);
const Token * var1 = tok->astOperand1();
const Token * var2 = nextAssign->astOperand1();
if(var1 && var2 &&
Token::Match(var1->previous(), ";|{|} %var%") &&
Token::Match(var2->previous(), ";|{|} %var%") &&
var2->valueType() && var1->valueType() &&
var2->valueType()->originalTypeName == var1->valueType()->originalTypeName &&
var2->valueType()->pointer == var1->valueType()->pointer &&
var2->valueType()->constness == var1->valueType()->constness &&
var2->varId() != var1->varId() && (
tok->astOperand2()->isArithmeticalOp() ||
tok->astOperand2()->str() == "." ||
Token::Match(tok->astOperand2()->previous(), "%name% (")
) &&
tok->next()->tokType() != Token::eType &&
tok->next()->tokType() != Token::eName &&
isSameExpression(_tokenizer->isCPP(), true, tok->next(), nextAssign->next(), _settings->library, true) &&
isSameExpression(_tokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), _settings->library, false)) {
duplicateAssignExpressionError(var1, var2);
}
}
}
if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) {
if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true))
continue;
@ -1987,6 +2016,17 @@ void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2,
"determine if it is correct.", CWE398, false);
}
void CheckOther::duplicateAssignExpressionError(const Token *tok1, const Token *tok2)
{
const std::list<const Token *> toks = make_container< std::list<const Token *> >() << tok2 << tok1;
reportError(toks, Severity::style, "duplicateAssignExpression",
"Same expression used in consecutive assignments of '" + tok1->str() + "' and '" + tok2->str() + "'.\n"
"Finding variables '" + tok1->str() + "' and '" + tok2->str() + "' that are assigned the same expression "
"is suspicious and might indicate a cut and paste or logic error. Please examine this code carefully to "
"determine if it is correct.", CWE398, false);
}
void CheckOther::duplicateExpressionTernaryError(const Token *tok)
{
reportError(tok, Severity::style, "duplicateExpressionTernary", "Same expression in both branches of ternary operator.\n"

1
lib/checkother.h
View File

@ -238,6 +238,7 @@ private:
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);
void duplicateAssignExpressionError(const Token *tok1, const Token *tok2);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void duplicateValueTernaryError(const Token *tok);
void duplicateExpressionTernaryError(const Token *tok);

122
test/testother.cpp
View File

@ -131,6 +131,7 @@ private:
TEST_CASE(duplicateValueTernary);
TEST_CASE(duplicateExpressionTernary); // #6391
TEST_CASE(duplicateExpressionTemplate); // #6930
TEST_CASE(duplicateVarExpression);
TEST_CASE(checkSignOfUnsignedVariable);
TEST_CASE(checkSignOfPointer);
@ -3918,6 +3919,127 @@ private:
ASSERT_EQUALS("", errout.str());
}
void duplicateVarExpression() {
check("int f() __attribute__((pure));\n"
"void test() {\n"
" int i = f();\n"
" int j = f();\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("struct Foo { int f() const; };\n"
"void test() {\n"
" Foo f = Foo{};\n"
" int i = f.f();\n"
" int j = f.f();\n"
"}");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("int f() __attribute__((pure));\n"
"void test() {\n"
" int i = 1 + f();\n"
" int j = 1 + f();\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("int f() __attribute__((pure));\n"
"void test() {\n"
" int i = f() + f();\n"
" int j = f() + f();\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("int f(int) __attribute__((pure));\n"
"void test() {\n"
" int i = f(0);\n"
" int j = f(0);\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("void test(int * p) {\n"
" int i = *p;\n"
" int j = *p;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("struct A { int x; };"
"void test(A a) {\n"
" int i = a.x;\n"
" int j = a.x;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("void test() {\n"
" int i = 0;\n"
" int j = 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void test() {\n"
" int i = -1;\n"
" int j = -1;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f(int);\n"
"void test() {\n"
" int i = f(0);\n"
" int j = f(1);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f();\n"
"void test() {\n"
" int i = f() || f();\n"
" int j = f() && f();\n"
"}");
ASSERT_EQUALS("", errout.str());
check("struct Foo {};\n"
"void test() {\n"
" Foo i = Foo();\n"
" Foo j = Foo();\n"
"}");
ASSERT_EQUALS("", errout.str());
check("struct Foo {};\n"
"void test() {\n"
" Foo i = Foo{};\n"
" Foo j = Foo{};\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void test() {\n"
" int i = f();\n"
" int j = f();\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void test(int x) {\n"
" int i = ++x;\n"
" int j = ++x;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void test(int x) {\n"
" int i = x++;\n"
" int j = x++;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void test(int x) {\n"
" int i = --x;\n"
" int j = --x;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void test(int x) {\n"
" int i = x--;\n"
" int j = x--;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void checkSignOfUnsignedVariable() {
check(
"void foo() {\n"