Fix issue 8715: regression uninitvar not detected (#2092)

This commit is contained in:
Paul Fultz II 2019-08-16 00:48:54 -05:00 committed by Daniel Marjamäki
parent ef714225bb
commit 3aef0c9bd3
7 changed files with 49 additions and 10 deletions

View File

@ -1093,22 +1093,32 @@ bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int
} }
bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth) bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth)
{
return findVariableChanged(start, end, varid, globalvar, settings, cpp, depth) != nullptr;
}
Token* findVariableChanged(Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth)
{ {
if (!precedes(start, end)) if (!precedes(start, end))
return false; return nullptr;
if (depth < 0) if (depth < 0)
return true; return start;
for (const Token *tok = start; tok != end; tok = tok->next()) { for (Token *tok = start; tok != end; tok = tok->next()) {
if (tok->varId() != varid) { if (tok->varId() != varid) {
if (globalvar && Token::Match(tok, "%name% (")) if (globalvar && Token::Match(tok, "%name% ("))
// TODO: Is global variable really changed by function call? // TODO: Is global variable really changed by function call?
return true; return tok;
continue; continue;
} }
if (isVariableChanged(tok, settings, cpp, depth)) if (isVariableChanged(tok, settings, cpp, depth))
return true; return tok;
} }
return false; return nullptr;
}
const Token* findVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth)
{
return findVariableChanged(const_cast<Token*>(start), end, varid, globalvar, settings, cpp, depth);
} }
bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth) bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth)

View File

@ -142,6 +142,9 @@ bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int
bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth = 20); bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth = 20);
const Token* findVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
Token* findVariableChanged(Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
bool isAliased(const Variable *var); bool isAliased(const Variable *var);
/** Determines the number of arguments - if token is a function call or macro /** Determines the number of arguments - if token is a function call or macro

View File

@ -1299,7 +1299,7 @@ void CheckUninitVar::valueFlowUninit()
} }
if (!tok->variable()) if (!tok->variable())
continue; continue;
if (Token::simpleMatch(tok->astParent(), ".") && tok->astParent()->astOperand1() == tok) if (Token::Match(tok->astParent(), ". %var%") && tok->astParent()->astOperand1() == tok)
continue; continue;
auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue));
if (v == tok->values().end()) if (v == tok->values().end())
@ -1308,7 +1308,7 @@ void CheckUninitVar::valueFlowUninit()
continue; continue;
if (!isVariableUsage(tok, tok->variable()->isPointer(), tok->variable()->isArray() ? ARRAY : NO_ALLOC)) if (!isVariableUsage(tok, tok->variable()->isPointer(), tok->variable()->isArray() ? ARRAY : NO_ALLOC))
continue; continue;
if (isVariableChanged(tok, mSettings, mTokenizer->isCPP())) if (!Token::Match(tok->astParent(), ". %name% (") && isVariableChanged(tok, mSettings, mTokenizer->isCPP()))
continue; continue;
uninitvarError(tok, tok->str(), v->errorPath); uninitvarError(tok, tok->str(), v->errorPath);
} }

View File

@ -2145,7 +2145,16 @@ static bool valueFlowForward(Token * const startToken,
// conditional block of code that assigns variable.. // conditional block of code that assigns variable..
else if (!tok2->varId() && Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) { else if (!tok2->varId() && Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) {
// is variable changed in condition? // is variable changed in condition?
if (isVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) { Token* tokChanged = findVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings, tokenlist->isCPP());
if (tokChanged != nullptr) {
// Set the value before bailing
if (tokChanged->varId() == varid) {
for (const ValueFlow::Value &v : values) {
if (!v.isNonValue())
continue;
setTokenValue(tokChanged, v, settings);
}
}
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition"); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition");
return false; return false;

View File

@ -139,6 +139,10 @@ namespace ValueFlow {
return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Argument; return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Argument;
} }
bool isNonValue() const {
return isMovedValue() || isUninitValue() || isLifetimeValue();
}
/** int value */ /** int value */
long long intvalue; long long intvalue;

View File

@ -82,6 +82,7 @@ private:
TEST_CASE(trac_5970); TEST_CASE(trac_5970);
TEST_CASE(valueFlowUninit); TEST_CASE(valueFlowUninit);
TEST_CASE(uninitvar_ipa); TEST_CASE(uninitvar_ipa);
TEST_CASE(uninitvar_memberfunction);
TEST_CASE(isVariableUsageDeref); // *p TEST_CASE(isVariableUsageDeref); // *p
@ -4030,6 +4031,18 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void uninitvar_memberfunction() {
// # 8715
valueFlowUninit("struct C {\n"
" int x();\n"
"};\n"
"void f() {\n"
" C *c;\n"
" if (c->x() == 4) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: c\n", errout.str());
}
void isVariableUsageDeref() { void isVariableUsageDeref() {
// *p // *p
checkUninitVar("void f() {\n" checkUninitVar("void f() {\n"

View File

@ -3474,7 +3474,7 @@ private:
" if (c->x() == 4) {}\n" " if (c->x() == 4) {}\n"
"}"; "}";
values = tokenValues(code, "c ."); values = tokenValues(code, "c .");
TODO_ASSERT_EQUALS(true, false, values.size()==1U && values.front().isUninitValue()); ASSERT_EQUALS(true, values.size()==1U && values.front().isUninitValue());
code = "void f() {\n" code = "void f() {\n"
" int **x;\n" " int **x;\n"