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

Refactor valueFlowTerminatingCondition to handle inner conditions and complex conditions (#3060)

This commit is contained in:
Paul Fultz II 2021-01-21 13:18:53 -06:00 committed by GitHub
parent d05acf3c41
commit f1cc3ada86
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 139 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/checkcondition.cpp
View File

@ -507,14 +507,12 @@ void CheckCondition::multiCondition()
break;
tok2 = tok2->tokAt(4);
if (tok2->astOperand2() &&
!cond1->hasKnownIntValue() &&
!tok2->astOperand2()->hasKnownIntValue()) {
if (tok2->astOperand2()) {
ErrorPath errorPath;
if (isOverlappingCond(cond1, tok2->astOperand2(), true))
overlappingElseIfConditionError(tok2, cond1->linenr());
overlappingElseIfConditionError(tok2->astOperand2(), cond1->linenr());
else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath))
oppositeElseIfConditionError(cond1, tok2, errorPath);
oppositeElseIfConditionError(cond1, tok2->astOperand2(), errorPath);
}
}
}

114
lib/valueflow.cpp
View File

@ -1427,37 +1427,6 @@ static void valueFlowRightShift(TokenList *tokenList, const Settings* settings)
}
}
static void valueFlowOppositeCondition(SymbolDatabase *symboldatabase, const Settings *settings)
{
for (const Scope &scope : symboldatabase->scopeList) {
if (scope.type != Scope::eIf)
continue;
Token *tok = const_cast<Token *>(scope.classDef);
if (!Token::simpleMatch(tok, "if ("))
continue;
const Token *cond1 = tok->next()->astOperand2();
if (!cond1 || !cond1->isComparisonOp())
continue;
const bool cpp = symboldatabase->isCPP();
Token *tok2 = tok->linkAt(1);
while (Token::simpleMatch(tok2, ") {")) {
tok2 = tok2->linkAt(1);
if (!Token::simpleMatch(tok2, "} else { if ("))
break;
Token *ifOpenBraceTok = tok2->tokAt(4);
Token *cond2 = ifOpenBraceTok->astOperand2();
if (!cond2 || !cond2->isComparisonOp())
continue;
if (isOppositeCond(true, cpp, cond1, cond2, settings->library, true, true)) {
ValueFlow::Value value(1);
value.setKnown();
setTokenValue(cond2, value, settings);
}
tok2 = ifOpenBraceTok->link();
}
}
}
static void valueFlowEnumValue(SymbolDatabase * symboldatabase, const Settings * settings)
{
@ -3756,7 +3725,19 @@ static const Token* findIncompleteVar(const Token* start, const Token* end)
return nullptr;
}
static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
ValueFlow::Value makeConditionValue(long long val, const Token* condTok, bool assume)
{
ValueFlow::Value v(val);
v.setKnown();
v.condition = condTok;
if (assume)
v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true");
else
v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
return v;
}
//
static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
{
for (const Scope * scope : symboldatabase->functionScopes) {
if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
@ -3773,28 +3754,58 @@ static void valueFlowTerminatingCondition(TokenList *tokenlist, SymbolDatabase*
// Skip known values
if (tok->next()->hasKnownValue())
continue;
const Token * parenTok = tok->next();
Token * parenTok = tok->next();
if (!Token::simpleMatch(parenTok->link(), ") {"))
continue;
const Token * blockTok = parenTok->link()->tokAt(1);
// Check if the block terminates early
if (!isEscapeScope(blockTok, tokenlist))
continue;
Token * blockTok = parenTok->link()->tokAt(1);
const Token* condTok = parenTok->astOperand2();
ValueFlow::Value v1(0);
v1.setKnown();
v1.condition = condTok;
v1.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true");
ExpressionAnalyzer a1(condTok, v1, tokenlist);
valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a1, settings);
ValueFlow::Value v2(1);
v2.setKnown();
v2.condition = condTok;
v2.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
OppositeExpressionAnalyzer a2(true, condTok, v2, tokenlist);
valueFlowGenericForward(blockTok->link()->next(), scope->bodyEnd, a2, settings);
Token* startTok = blockTok;
// Inner condition
{
std::vector<const Token*> conds = {condTok};
if (Token::simpleMatch(condTok, "&&")) {
std::vector<const Token*> args = astFlatten(condTok, "&&");
conds.insert(conds.end(), args.begin(), args.end());
}
for(const Token* condTok2:conds) {
ExpressionAnalyzer a1(condTok2, makeConditionValue(1, condTok2, true), tokenlist);
valueFlowGenericForward(startTok, startTok->link(), a1, settings);
OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(0, condTok2, true), tokenlist);
valueFlowGenericForward(startTok, startTok->link(), a2, settings);
}
}
std::vector<const Token*> conds = {condTok};
if (Token::simpleMatch(condTok, "||")) {
std::vector<const Token*> args = astFlatten(condTok, "||");
conds.insert(conds.end(), args.begin(), args.end());
}
// Check else block
if (Token::simpleMatch(startTok->link(), "} else {")) {
startTok = startTok->link()->tokAt(2);
for(const Token* condTok2:conds) {
ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist);
valueFlowGenericForward(startTok, startTok->link(), a1, settings);
OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist);
valueFlowGenericForward(startTok, startTok->link(), a2, settings);
}
}
// Check if the block terminates early
if (isEscapeScope(blockTok, tokenlist)) {
for(const Token* condTok2:conds) {
ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist);
valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a1, settings);
OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist);
valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a2, settings);
}
}
}
}
}
@ -6624,6 +6635,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings);
valueFlowBitAnd(tokenlist);
valueFlowSameExpressions(tokenlist);
valueFlowConditionExpressions(tokenlist, symboldatabase, errorLogger, settings);
valueFlowFwdAnalysis(tokenlist, settings);
std::size_t values = 0;
@ -6633,8 +6645,6 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowPointerAliasDeref(tokenlist);
valueFlowArrayBool(tokenlist);
valueFlowRightShift(tokenlist, settings);
valueFlowOppositeCondition(symboldatabase, settings);
valueFlowTerminatingCondition(tokenlist, symboldatabase, errorLogger, settings);
valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings);
valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
valueFlowInferCondition(tokenlist, settings);

14
test/testcondition.cpp
View File

@ -533,27 +533,27 @@ private:
" if (a) { b = 1; }\n"
" else { if (a) { b = 2; } }\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'a' is always false\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a) { b = 1; }\n"
" else { if (a) { b = 2; } }\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'a' is always false\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a == 1) { b = 1; }\n"
" else { if (a == 2) { b = 2; }\n"
" else { if (a == 1) { b = 3; } } }\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'a==1' is always false\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a == 1) { b = 1; }\n"
" else { if (a == 2) { b = 2; }\n"
" else { if (a == 2) { b = 3; } } }\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'a==2' is always false\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a++) { b = 1; }\n"
@ -721,9 +721,9 @@ private:
" if (x) {}\n"
" else if (!x) {}\n"
"}");
ASSERT_EQUALS("test.cpp:3:style:Condition '!x' is always true\n"
"test.cpp:2:note:condition 'x'\n"
"test.cpp:3:note:Condition '!x' is always true\n", errout.str());
ASSERT_EQUALS("test.cpp:3:style:Expression is always true because 'else if' condition is opposite to previous condition at line 2.\n"
"test.cpp:2:note:first condition\n"
"test.cpp:3:note:else if condition is opposite to first condition\n", errout.str());
check("void f(int x) {\n"
" int y = x;\n"

4
test/testsimplifytypedef.cpp
View File

@ -1670,7 +1670,7 @@ private:
"( ( int ( * * ( * ) ( char * , char * , int , int ) ) ( ) ) global [ 6 ] ) ( \"assoc\" , \"eggdrop\" , 106 , 0 ) ; "
"}";
ASSERT_EQUALS(expected, tok(code));
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1319:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable global\n", errout.str());
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:3]: (debug) valueflow.cpp:1319:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable global\n", errout.str());
}
void simplifyTypedef68() { // ticket #2355
@ -1877,7 +1877,7 @@ private:
" B * b = new B;\n"
" b->f = new A::F * [ 10 ];\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:12]: (debug) valueflow.cpp:1319:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable idx\n", errout.str());
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:12]: (debug) valueflow.cpp:1319:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable idx\n", errout.str());
}
void simplifyTypedef83() { // ticket #2620

2
test/testsymboldatabase.cpp
View File

@ -2743,7 +2743,7 @@ private:
check("::y(){x}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:1]: (debug) Executable scope 'y' with unknown function.\n"
"[test.cpp:1]: (debug) valueflow.cpp:1321:valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable x\n", errout.str());
"[test.cpp:1]: (debug) valueflow.cpp:1321:valueFlowConditionExpressions bailout: Skipping function due to incomplete variable x\n", errout.str());
}
void symboldatabase20() {

74
test/testvalueflow.cpp
View File

@ -122,7 +122,7 @@ private:
TEST_CASE(valueFlowUninit);
TEST_CASE(valueFlowTerminatingCond);
TEST_CASE(valueFlowConditionExpressions);
TEST_CASE(valueFlowContainerSize);
@ -1266,7 +1266,7 @@ private:
" if (x == 123) {}\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n",
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable y\n",
errout.str());
}
@ -1383,7 +1383,7 @@ private:
" y = ((x<0) ? x : ((x==2)?3:4));\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable y\n",
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable y\n",
errout.str());
bailout("int f(int x) {\n"
@ -1448,7 +1448,7 @@ private:
" if (x == 123) {}\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable b\n",
"[test.cpp:2]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable b\n",
errout.str());
code = "void f(int x, bool abc) {\n"
@ -1497,7 +1497,7 @@ private:
" };\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n",
errout.str());
bailout("void f(int x, int y) {\n"
@ -1507,7 +1507,7 @@ private:
" };\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n",
errout.str());
}
@ -1519,7 +1519,7 @@ private:
" M;\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n"
"[test.cpp:4]: (debug) valueflow.cpp:1260:(valueFlow) bailout: variable 'x', condition is defined in macro\n",
errout.str());
@ -1529,7 +1529,7 @@ private:
" FREE(x);\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n"
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n"
"[test.cpp:4]: (debug) valueflow.cpp:1260:(valueFlow) bailout: variable 'x', condition is defined in macro\n",
errout.str());
}
@ -1543,7 +1543,7 @@ private:
" if (x==123){}\n"
"}");
ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowTerminatingCondition bailout: Skipping function due to incomplete variable a\n",
"[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n",
errout.str());
// #5721 - FP
@ -4202,7 +4202,7 @@ private:
ASSERT_EQUALS(0, values.size());
}
void valueFlowTerminatingCond() {
void valueFlowConditionExpressions() {
const char* code;
// opposite condition
@ -4272,6 +4272,60 @@ private:
"}\n";
ASSERT_EQUALS(false, valueOfTok(code, "!=").intvalue == 1);
code = "void f(bool b, int i, int j) {\n"
" if (b || i == j) return;\n"
" if(i != j) {}\n"
"}\n";
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
code = "void f(bool b, int i, int j) {\n"
" if (b && i == j) return;\n"
" if(i != j) {}\n"
"}\n";
ASSERT_EQUALS(true, tokenValues(code, "!=").empty());
code = "void f(int i, int j) {\n"
" if (i == j) {\n"
" if (i != j) {}\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 0);
code = "void f(int i, int j) {\n"
" if (i == j) {} else {\n"
" if (i != j) {}\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
code = "void f(bool b, int i, int j) {\n"
" if (b && i == j) {\n"
" if (i != j) {}\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 0);
code = "void f(bool b, int i, int j) {\n"
" if (b || i == j) {\n"
" if (i != j) {}\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, tokenValues(code, "!=").empty());
code = "void f(bool b, int i, int j) {\n"
" if (b || i == j) {} else {\n"
" if (i != j) {}\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, valueOfTok(code, "!=").intvalue == 1);
code = "void f(bool b, int i, int j) {\n"
" if (b && i == j) {} else {\n"
" if (i != j) {}\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, tokenValues(code, "!=").empty());
code = "void foo()\n" // #8924
"{\n"
" if ( this->FileIndex >= 0 )\n"