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

Use followVar in checking duplicateBranch (#1423) 

* Use isSameExpression for duplicate branches

* Add errorPath

* Add another test
This commit is contained in:
Paul Fultz II 2018-10-18 04:56:23 -05:00 committed by Daniel Marjamäki
parent 511bca1e62
commit 16c62281d0
6 changed files with 75 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
lib/astutils.cpp
View File

@ -133,6 +133,20 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
return ret;
}
const Token * nextAfterAstRightmostLeaf(const Token * tok)
{
const Token * rightmostLeaf = tok;
if (!rightmostLeaf || !rightmostLeaf->astOperand1())
return nullptr;
do {
if (rightmostLeaf->astOperand2())
rightmostLeaf = rightmostLeaf->astOperand2();
else
rightmostLeaf = rightmostLeaf->astOperand1();
} while (rightmostLeaf->astOperand1());
return rightmostLeaf->next();
}
static const Token * getVariableInitExpression(const Variable * var)
{
if (!var || !var->declEndToken())

2
lib/astutils.h
View File

@ -56,6 +56,8 @@ std::string astCanonicalType(const Token *expr);
/** Is given syntax tree a variable comparison against value */
const Token * astIsVariableComparison(const Token *tok, const std::string &comp, const std::string &rhs, const Token **vartok=nullptr);
const Token * nextAfterAstRightmostLeaf(const Token * tok);
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr);
bool isEqualKnownValue(const Token * const tok1, const Token * const tok2);

38
lib/checkother.cpp
View File

@ -1769,6 +1769,19 @@ void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& va
"Instance of '$symbol' object is destroyed immediately.", CWE563, false);
}
static const Token * getSingleExpressionInBlock(const Token * tok)
{
if(!tok)
return nullptr;
const Token * top = tok->astTop();
if(!top)
return nullptr;
const Token * nextExpression = nextAfterAstRightmostLeaf(top);
if (!Token::simpleMatch(nextExpression, "; }"))
return nullptr;
return top;
}
//-----------------------------------------------------------------------------
// check for duplicate code in if and else branches
// if (a) { b = true; } else { b = true; }
@ -1815,18 +1828,33 @@ void CheckOther::checkDuplicateBranch()
// save else branch code
const std::string branch2 = scope.bodyEnd->tokAt(3)->stringifyList(scope.bodyEnd->linkAt(2));
ErrorPath errorPath;
// check for duplicates
if (branch1 == branch2)
duplicateBranchError(scope.classDef, scope.bodyEnd->next());
if (branch1 == branch2) {
duplicateBranchError(scope.classDef, scope.bodyEnd->next(), errorPath);
continue;
}
// check for duplicates using isSameExpression
const Token * branchTop1 = getSingleExpressionInBlock(scope.bodyStart->next());
const Token * branchTop2 = getSingleExpressionInBlock(scope.bodyEnd->tokAt(3));
if (!branchTop1 || !branchTop2)
continue;
if(branchTop1->str() != branchTop2->str())
continue;
if(isSameExpression(mTokenizer->isCPP(), false, branchTop1->astOperand1(), branchTop2->astOperand1(), mSettings->library, true, true, &errorPath) &&
isSameExpression(mTokenizer->isCPP(), false, branchTop1->astOperand2(), branchTop2->astOperand2(), mSettings->library, true, true, &errorPath))
duplicateBranchError(scope.classDef, scope.bodyEnd->next(), errorPath);
}
}
}
void CheckOther::duplicateBranchError(const Token *tok1, const Token *tok2)
void CheckOther::duplicateBranchError(const Token *tok1, const Token *tok2, ErrorPath errors)
{
const std::list<const Token *> toks = { tok2, tok1 };
errors.emplace_back(tok2, "");
errors.emplace_back(tok1, "");
reportError(toks, Severity::style, "duplicateBranch", "Found duplicate branches for 'if' and 'else'.\n"
reportError(errors, Severity::style, "duplicateBranch", "Found duplicate branches for 'if' and 'else'.\n"
"Finding the same code in an 'if' and related 'else' branch is suspicious and "
"might indicate a cut and paste or logic error. Please examine this code "
"carefully to determine if it is correct.", CWE398, true);

4
lib/checkother.h
View File

@ -241,7 +241,7 @@ private:
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);
void duplicateBranchError(const Token *tok1, const Token *tok2, ErrorPath errors);
void duplicateAssignExpressionError(const Token *tok1, const Token *tok2, bool inconclusive);
void oppositeExpressionError(const Token *opTok, ErrorPath errors);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors);
@ -306,7 +306,7 @@ private:
c.selfAssignmentError(nullptr, "varname");
c.clarifyCalculationError(nullptr, "+");
c.clarifyStatementError(nullptr);
c.duplicateBranchError(nullptr, nullptr);
c.duplicateBranchError(nullptr, nullptr, errorPath);
c.duplicateAssignExpressionError(nullptr, nullptr, true);
c.oppositeExpressionError(nullptr, errorPath);
c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath);

14
lib/valueflow.cpp
View File

@ -2245,20 +2245,6 @@ static bool isOpenParenthesisMemberFunctionCallOfVarId(const Token * openParenth
varTok->next()->originalName() == emptyString;
}
static const Token * nextAfterAstRightmostLeaf(Token const * tok)
{
const Token * rightmostLeaf = tok;
if (!rightmostLeaf || !rightmostLeaf->astOperand1())
return nullptr;
do {
if (rightmostLeaf->astOperand2())
rightmostLeaf = rightmostLeaf->astOperand2();
else
rightmostLeaf = rightmostLeaf->astOperand1();
} while (rightmostLeaf->astOperand1());
return rightmostLeaf->next();
}
static const Token * findOpenParentesisOfMove(const Token * moveVarTok)
{
const Token * tok = moveVarTok;

24
test/testother.cpp
View File

@ -124,6 +124,7 @@ private:
TEST_CASE(duplicateBranch);
TEST_CASE(duplicateBranch1); // tests extracted by http://www.viva64.com/en/b/0149/ ( Comparison between PVS-Studio and cppcheck ): Errors detected in Quake 3: Arena by PVS-Studio: Fragment 2
TEST_CASE(duplicateBranch2); // empty macro
TEST_CASE(duplicateBranch3);
TEST_CASE(duplicateExpression1);
TEST_CASE(duplicateExpression2); // ticket #2730
TEST_CASE(duplicateExpression3); // ticket #3317
@ -3491,6 +3492,29 @@ private:
ASSERT_EQUALS("", errout.str());
}
void duplicateBranch3() {
check("void f(bool b, int i) {\n"
" int j = i;\n"
" if (b) {\n"
" x = i;\n"
" } else {\n"
" x = j;\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5] -> [test.cpp:3]: (style, inconclusive) Found duplicate branches for 'if' and 'else'.\n", errout.str());
check("void f(bool b, int i) {\n"
" int j = i;\n"
" i++;\n"
" if (b) {\n"
" x = i;\n"
" } else {\n"
" x = j;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void duplicateExpression1() {
check("void foo(int a) {\n"
" if (a == a) { }\n"