Fixed #6118 (False positive: divide by zero - if condition not evaluated properly)

This commit is contained in:
Daniel Marjamäki 2014-09-04 17:52:14 +02:00
parent 179947aa54
commit 05617d7285
3 changed files with 57 additions and 12 deletions

View File

@ -642,6 +642,23 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, ErrorLogger *errorLog
}
}
static void removeValues(std::list<ValueFlow::Value> &values, const std::list<ValueFlow::Value> &valuesToRemove)
{
for (std::list<ValueFlow::Value>::iterator it = values.begin(); it != values.end();) {
bool found = false;
for (std::list<ValueFlow::Value>::const_iterator it2 = valuesToRemove.begin(); it2 != valuesToRemove.end(); ++it2) {
if (it->intvalue == it2->intvalue) {
found = true;
break;
}
}
if (found)
values.erase(it++);
else
it++;
}
}
static bool valueFlowForward(Token * const startToken,
const Token * const endToken,
const Variable * const var,
@ -710,18 +727,30 @@ static bool valueFlowForward(Token * const startToken,
// conditional block of code that assigns variable..
else if (Token::Match(tok2, "%var% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) {
// Should scope be skipped because variable value is checked?
bool skip = false;
std::list<ValueFlow::Value> truevalues;
for (std::list<ValueFlow::Value>::iterator it = values.begin(); it != values.end(); ++it) {
if (conditionIsFalse(tok2->next()->astOperand2(), getProgramMemory(tok2, varid, *it))) {
skip = true;
break;
}
if (!conditionIsFalse(tok2->next()->astOperand2(), getProgramMemory(tok2, varid, *it)))
truevalues.push_back(*it);
}
if (skip) {
// goto '{'
tok2 = tok2->linkAt(1)->next();
if (truevalues.size() != values.size()) {
// '{'
Token * const startToken1 = tok2->linkAt(1)->next();
valueFlowForward(startToken1->next(),
startToken1->link(),
var,
varid,
truevalues,
constValue,
tokenlist,
errorLogger,
settings);
if (isVariableChanged(startToken1, startToken1->link(), varid))
removeValues(values, truevalues);
// goto '}'
tok2 = tok2->link();
tok2 = startToken1->link();
continue;
}
@ -838,7 +867,7 @@ static bool valueFlowForward(Token * const startToken,
if (tok3->varId() == varid) {
std::list<ValueFlow::Value>::const_iterator it;
for (it = values.begin(); it != values.end(); ++it)
setTokenValue(tok2, *it);
setTokenValue(tok3, *it);
} else if (Token::Match(tok3, "++|--|?|:|;"))
break;
}

View File

@ -128,8 +128,8 @@ private:
"{\n"
" while (tok);\n"
" tok = tok->next();\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", errout.str());
"}", true);
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning, inconclusive) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", errout.str());
// #2681
{

View File

@ -788,6 +788,22 @@ private:
"}";
ASSERT_EQUALS(false, testValueOfX(code, 4U, 0));
code = "void f() {\n" // #6118 - FP
" int x = 0;\n"
" x = x & 0x1;\n"
" if (x == 0) { x = 2; }\n"
" y = 42 / x;\n" // <- x can't be 0
"}";
ASSERT_EQUALS(false, testValueOfX(code, 5U, 0));
code = "void f() {\n" // #6118 - FN
" int x = 0;\n"
" x = x & 0x1;\n"
" if (x == 0) { x += 2; }\n"
" y = 42 / x;\n" // <- x can be 2
"}";
ASSERT_EQUALS(true, testValueOfX(code, 5U, 2));
// multivariables
code = "void f(int a) {\n"
" int x = a;\n"