FIx issue 6140: ValueFlow: valueFlowForward, loop (#2770)

This commit is contained in:
Paul Fultz II 2020-09-03 00:17:36 -05:00 committed by GitHub
parent 638dcd0aca
commit 090eba7e7f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 42 additions and 9 deletions

View File

@ -65,12 +65,19 @@ struct ForwardTraversal {
return Progress::Continue; return Progress::Continue;
if (recursion > 10000) if (recursion > 10000)
return Progress::Skip; return Progress::Skip;
if (tok->astOperand1() && traverseRecursive(tok->astOperand1(), f, traverseUnknown, recursion+1) == Progress::Break) T* firstOp = tok->astOperand1();
T* secondOp = tok->astOperand2();
// Evaluate RHS of assignment before LHS
if (tok->isAssignmentOp())
std::swap(firstOp, secondOp);
if (firstOp && traverseRecursive(firstOp, f, traverseUnknown, recursion+1) == Progress::Break)
return Progress::Break; return Progress::Break;
Progress p = traverseTok(tok, f, traverseUnknown); Progress p = tok->isAssignmentOp() ? Progress::Continue : traverseTok(tok, f, traverseUnknown);
if (p == Progress::Break) if (p == Progress::Break)
return Progress::Break; return Progress::Break;
if (p == Progress::Continue && tok->astOperand2() && traverseRecursive(tok->astOperand2(), f, traverseUnknown, recursion+1) == Progress::Break) if (p == Progress::Continue && secondOp && traverseRecursive(secondOp, f, traverseUnknown, recursion+1) == Progress::Break)
return Progress::Break;
if (tok->isAssignmentOp() && traverseTok(tok, f, traverseUnknown) == Progress::Break)
return Progress::Break; return Progress::Break;
return Progress::Continue; return Progress::Continue;
} }
@ -271,11 +278,7 @@ struct ForwardTraversal {
// Evaluate RHS of assignment before LHS // Evaluate RHS of assignment before LHS
if (Token* assignTok = assignExpr(tok)) { if (Token* assignTok = assignExpr(tok)) {
if (updateRecursive(assignTok->astOperand2()) == Progress::Break) if (updateRecursive(assignTok) == Progress::Break)
return Progress::Break;
if (updateRecursive(assignTok->astOperand1()) == Progress::Break)
return Progress::Break;
if (update(assignTok) == Progress::Break)
return Progress::Break; return Progress::Break;
tok = nextAfterAstRightmostLeaf(assignTok); tok = nextAfterAstRightmostLeaf(assignTok);
if (!tok) if (!tok)
@ -299,7 +302,7 @@ struct ForwardTraversal {
return Progress::Break; return Progress::Break;
if (Token::Match(tok->link()->previous(), ")|else {")) { if (Token::Match(tok->link()->previous(), ")|else {")) {
const bool inElse = Token::simpleMatch(tok->link()->previous(), "else {"); const bool inElse = Token::simpleMatch(tok->link()->previous(), "else {");
const Token* condTok = getCondTokFromEnd(tok); Token* condTok = getCondTokFromEnd(tok);
if (!condTok) if (!condTok)
return Progress::Break; return Progress::Break;
if (!condTok->hasKnownIntValue()) { if (!condTok->hasKnownIntValue()) {
@ -308,6 +311,16 @@ struct ForwardTraversal {
} else if (condTok->values().front().intvalue == inElse) { } else if (condTok->values().front().intvalue == inElse) {
return Progress::Break; return Progress::Break;
} }
// Handle for loop
Token* stepTok = getStepTokFromEnd(tok);
bool checkThen, checkElse;
std::tie(checkThen, checkElse) = evalCond(condTok);
if (stepTok && !checkElse) {
if (updateRecursive(stepTok) == Progress::Break)
return Progress::Break;
if (updateRecursive(condTok) == Progress::Break)
return Progress::Break;
}
analyzer->assume(condTok, !inElse, tok); analyzer->assume(condTok, !inElse, tok);
if (Token::simpleMatch(tok, "} else {")) if (Token::simpleMatch(tok, "} else {"))
tok = tok->linkAt(2); tok = tok->linkAt(2);
@ -528,6 +541,15 @@ struct ForwardTraversal {
return tok->astOperand2()->astOperand2()->astOperand2(); return tok->astOperand2()->astOperand2()->astOperand2();
} }
static Token* getStepTokFromEnd(Token* tok) {
if (!Token::simpleMatch(tok, "}"))
return nullptr;
Token* end = tok->link()->previous();
if (!Token::simpleMatch(end, ")"))
return nullptr;
return getStepTok(end->link());
}
}; };
void valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<ForwardAnalyzer>& fa, const Settings* settings) void valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<ForwardAnalyzer>& fa, const Settings* settings)

View File

@ -289,6 +289,17 @@ private:
" if (!d) throw;\n" " if (!d) throw;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("struct b {\n"
" b * c;\n"
" int i;\n"
"};\n"
"void f(b* e1, b* e2) {\n"
" for (const b* d = e1; d != e2; d = d->c) {\n"
" if (d && d->i != 0) {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (warning) Either the condition 'd' is redundant or there is possible null pointer dereference: d.\n", errout.str());
} }
void nullpointer1() { void nullpointer1() {