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

Fix issue 9305: False positive uninitvar - struct initialized via function (#2123)

This commit is contained in:
Paul Fultz II 2019-08-30 11:32:45 -05:00 committed by Daniel Marjamäki
parent 2942be53f7
commit 0b9e823fc8
8 changed files with 94 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/astutils.cpp
View File

@ -910,14 +910,14 @@ bool isReturnScope(const Token * const endToken, const Settings * settings, bool
return false; return false;
} }
bool isVariableChangedByFunctionCall(const Token *tok, nonneg int varid, const Settings *settings, bool *inconclusive) bool isVariableChangedByFunctionCall(const Token *tok, int indirect, nonneg int varid, const Settings *settings, bool *inconclusive)
{ {
if (!tok) if (!tok)
return false; return false;
if (tok->varId() == varid) if (tok->varId() == varid)
return isVariableChangedByFunctionCall(tok, settings, inconclusive); return isVariableChangedByFunctionCall(tok, indirect, settings, inconclusive);
return isVariableChangedByFunctionCall(tok->astOperand1(), varid, settings, inconclusive) || return isVariableChangedByFunctionCall(tok->astOperand1(), indirect, varid, settings, inconclusive) ||
isVariableChangedByFunctionCall(tok->astOperand2(), varid, settings, inconclusive); isVariableChangedByFunctionCall(tok->astOperand2(), indirect, varid, settings, inconclusive);
} }
static bool isScopeBracket(const Token *tok) static bool isScopeBracket(const Token *tok)
@ -933,7 +933,7 @@ static bool isScopeBracket(const Token *tok)
return false; return false;
} }
bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive) bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
{ {
if (!tok) if (!tok)
return false; return false;
@ -1038,7 +1038,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
const Variable *arg = tok->function()->getArgumentVar(argnr); const Variable *arg = tok->function()->getArgumentVar(argnr);
if (addressOf) { if (addressOf || (indirect > 0 && arg && arg->isPointer())) {
if (!(arg && arg->isConst())) if (!(arg && arg->isConst()))
return true; return true;
// If const is applied to the pointer, then the value can still be modified // If const is applied to the pointer, then the value can still be modified
@ -1049,7 +1049,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
return arg && !arg->isConst() && arg->isReference(); return arg && !arg->isConst() && arg->isReference();
} }
bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int depth) bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, bool cpp, int depth)
{ {
if (!tok) if (!tok)
return false; return false;
@ -1104,7 +1104,7 @@ bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int
while (Token::Match(ptok->astParent(), ".|::|[")) while (Token::Match(ptok->astParent(), ".|::|["))
ptok = ptok->astParent(); ptok = ptok->astParent();
bool inconclusive = false; bool inconclusive = false;
bool isChanged = isVariableChangedByFunctionCall(ptok, settings, &inconclusive); bool isChanged = isVariableChangedByFunctionCall(ptok, indirect, settings, &inconclusive);
isChanged |= inconclusive; isChanged |= inconclusive;
if (isChanged) if (isChanged)
return true; return true;
@ -1132,10 +1132,10 @@ 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; return findVariableChanged(start, end, 0, 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) Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth)
{ {
if (!precedes(start, end)) if (!precedes(start, end))
return nullptr; return nullptr;
@ -1148,15 +1148,15 @@ Token* findVariableChanged(Token *start, const Token *end, const nonneg int vari
return tok; return tok;
continue; continue;
} }
if (isVariableChanged(tok, settings, cpp, depth)) if (isVariableChanged(tok, indirect, settings, cpp, depth))
return tok; return tok;
} }
return nullptr; return nullptr;
} }
const Token* findVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth) const Token* findVariableChanged(const Token *start, const Token *end, int indirect, 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); return findVariableChanged(const_cast<Token*>(start), end, indirect, 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)

10
lib/astutils.h
View File

@ -129,7 +129,7 @@ bool isReturnScope(const Token *endToken, const Settings * settings = nullptr, b
* @param settings program settings * @param settings program settings
* @param inconclusive pointer to output variable which indicates that the answer of the question is inconclusive * @param inconclusive pointer to output variable which indicates that the answer of the question is inconclusive
*/ */
bool isVariableChangedByFunctionCall(const Token *tok, nonneg int varid, const Settings *settings, bool *inconclusive); bool isVariableChangedByFunctionCall(const Token *tok, int indirect, nonneg int varid, const Settings *settings, bool *inconclusive);
/** Is variable changed by function call? /** Is variable changed by function call?
* In case the answer of the question is inconclusive, e.g. because the function declaration is not known * In case the answer of the question is inconclusive, e.g. because the function declaration is not known
@ -139,17 +139,17 @@ bool isVariableChangedByFunctionCall(const Token *tok, nonneg int varid, const S
* @param settings program settings * @param settings program settings
* @param inconclusive pointer to output variable which indicates that the answer of the question is inconclusive * @param inconclusive pointer to output variable which indicates that the answer of the question is inconclusive
*/ */
bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive); bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive);
/** Is variable changed in block of code? */ /** Is variable changed in block of code? */
bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20); bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int depth = 20); bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, bool cpp, int depth = 20);
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); const Token* findVariableChanged(const Token *start, const Token *end, int indirect, 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); Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
bool isAliased(const Variable *var); bool isAliased(const Variable *var);

2
lib/checkother.cpp
View File

@ -2719,7 +2719,7 @@ void CheckOther::checkAccessOfMovedVariable()
else else
inconclusive = true; inconclusive = true;
} else { } else {
const bool isVariableChanged = isVariableChangedByFunctionCall(tok, mSettings, &inconclusive); const bool isVariableChanged = isVariableChangedByFunctionCall(tok, 0, mSettings, &inconclusive);
accessOfMoved = !isVariableChanged && checkUninitVar.isVariableUsage(tok, false, CheckUninitVar::NO_ALLOC); accessOfMoved = !isVariableChanged && checkUninitVar.isVariableUsage(tok, false, CheckUninitVar::NO_ALLOC);
if (inconclusive) { if (inconclusive) {
accessOfMoved = !isMovedParameterAllowedForInconclusiveFunction(tok); accessOfMoved = !isMovedParameterAllowedForInconclusiveFunction(tok);

2
lib/checkuninitvar.cpp
View File

@ -1357,7 +1357,7 @@ void CheckUninitVar::valueFlowUninit()
const bool isleaf = isLeafDot(tok) || uninitderef; const bool isleaf = isLeafDot(tok) || uninitderef;
if (Token::Match(tok->astParent(), ". %var%") && !isleaf) if (Token::Match(tok->astParent(), ". %var%") && !isleaf)
continue; continue;
if (!Token::Match(tok->astParent(), ". %name% (") && !uninitderef && isVariableChanged(tok, mSettings, mTokenizer->isCPP())) if (!Token::Match(tok->astParent(), ". %name% (") && !uninitderef && isVariableChanged(tok, v->indirect, mSettings, mTokenizer->isCPP()))
continue; continue;
uninitvarError(tok, tok->str(), v->errorPath); uninitvarError(tok, tok->str(), v->errorPath);
const Token * nextTok = tok; const Token * nextTok = tok;

78
lib/valueflow.cpp
View File

@ -169,11 +169,13 @@ static void bailoutInternal(TokenList *tokenlist, ErrorLogger *errorLogger, cons
#define bailout(tokenlist, errorLogger, tok, what) bailoutInternal(tokenlist, errorLogger, tok, what, __FILE__, __LINE__, "(valueFlow)") #define bailout(tokenlist, errorLogger, tok, what) bailoutInternal(tokenlist, errorLogger, tok, what, __FILE__, __LINE__, "(valueFlow)")
#endif #endif
static void changeKnownToPossible(std::list<ValueFlow::Value> &values) static void changeKnownToPossible(std::list<ValueFlow::Value> &values, int indirect=-1)
{ {
std::list<ValueFlow::Value>::iterator it; for (ValueFlow::Value& v: values) {
for (it = values.begin(); it != values.end(); ++it) if (indirect >= 0 && v.indirect != indirect)
it->changeKnownToPossible(); continue;
v.changeKnownToPossible();
}
} }
/** /**
@ -1690,7 +1692,7 @@ static void valueFlowReverse(TokenList *tokenlist,
// assigned by subfunction? // assigned by subfunction?
bool inconclusive = false; bool inconclusive = false;
if (isVariableChangedByFunctionCall(tok2, settings, &inconclusive)) { if (isVariableChangedByFunctionCall(tok2, std::max(val.indirect, val2.indirect), settings, &inconclusive)) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction"); bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction");
break; break;
@ -2071,6 +2073,15 @@ static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid,
return false; return false;
} }
static std::set<int> getIndirections(const std::list<ValueFlow::Value>& values)
{
std::set<int> result;
std::transform(values.begin(), values.end(), std::inserter(result, result.end()), [](const ValueFlow::Value& v) {
return std::max(0, v.indirect);
});
return result;
}
static bool valueFlowForward(Token * const startToken, static bool valueFlowForward(Token * const startToken,
const Token * const endToken, const Token * const endToken,
const Variable * const var, const Variable * const var,
@ -2204,16 +2215,21 @@ 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?
Token* tokChanged = findVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings, tokenlist->isCPP()); for(int i:getIndirections(values)) {
if (tokChanged != nullptr) { Token* tokChanged = findVariableChanged(tok2->next(), tok2->next()->link(), i, varid, var->isGlobal(), settings, tokenlist->isCPP());
// Set the value before bailing if (tokChanged != nullptr) {
if (tokChanged->varId() == varid) { // Set the value before bailing
for (const ValueFlow::Value &v : values) { if (tokChanged->varId() == varid) {
if (!v.isNonValue()) for (const ValueFlow::Value &v : values) {
continue; if (!v.isNonValue())
setTokenValue(tokChanged, v, settings); continue;
setTokenValue(tokChanged, v, settings);
}
} }
values.remove_if([&](const ValueFlow::Value& v) { return v.indirect == i; });
} }
}
if (values.empty()) {
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;
@ -2534,8 +2550,10 @@ static bool valueFlowForward(Token * const startToken,
Token *expr = (condValue.intvalue != 0) ? op2->astOperand1() : op2->astOperand2(); Token *expr = (condValue.intvalue != 0) ? op2->astOperand1() : op2->astOperand2();
for (const ValueFlow::Value &v : values) for (const ValueFlow::Value &v : values)
valueFlowAST(expr, varid, v, settings); valueFlowAST(expr, varid, v, settings);
if (isVariableChangedByFunctionCall(expr, varid, settings, nullptr)) if (isVariableChangedByFunctionCall(expr, 0, varid, settings, nullptr))
changeKnownToPossible(values); changeKnownToPossible(values, 0);
if (isVariableChangedByFunctionCall(expr, 1, varid, settings, nullptr))
changeKnownToPossible(values, 1);
} else { } else {
for (const ValueFlow::Value &v : values) { for (const ValueFlow::Value &v : values) {
const ProgramMemory programMemory(getProgramMemory(tok2, varid, v)); const ProgramMemory programMemory(getProgramMemory(tok2, varid, v));
@ -2730,16 +2748,24 @@ static bool valueFlowForward(Token * const startToken,
} }
// assigned by subfunction? // assigned by subfunction?
bool inconclusive = false; for(int i:getIndirections(values)) {
if (isVariableChangedByFunctionCall(tok2, settings, &inconclusive)) { bool inconclusive = false;
if (isVariableChangedByFunctionCall(tok2, i, settings, &inconclusive)) {
values.remove_if([&](const ValueFlow::Value& v) { return v.indirect <= i; });
}
if (inconclusive) {
for (ValueFlow::Value &v : values) {
if (v.indirect != i)
continue;
v.setInconclusive();
}
}
}
if (values.empty()) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction"); bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction");
return false; return false;
} }
if (inconclusive) {
for (ValueFlow::Value &v : values)
v.setInconclusive();
}
if (tok2->strAt(1) == "." && tok2->next()->originalName() != "->") { if (tok2->strAt(1) == "." && tok2->next()->originalName() != "->") {
if (settings->inconclusive) { if (settings->inconclusive) {
for (ValueFlow::Value &v : values) for (ValueFlow::Value &v : values)
@ -2751,10 +2777,12 @@ static bool valueFlowForward(Token * const startToken,
} }
} }
// Variable changed // Variable changed
if (isVariableChanged(tok2, settings, tokenlist->isCPP())) { for(int i:getIndirections(values)) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isUninitValue)); // Remove unintialized values if modified
} if (isVariableChanged(tok2, i, settings, tokenlist->isCPP()))
} else if (isAliasOf(var, tok2, varid, values) && isVariableChanged(tok2, settings, tokenlist->isCPP())) { values.remove_if([&](const ValueFlow::Value& v) { return v.isUninitValue() && v.indirect <= i; });
}
} else if (isAliasOf(var, tok2, varid, values) && isVariableChanged(tok2, 0, settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "Alias variable was modified."); bailout(tokenlist, errorLogger, tok2, "Alias variable was modified.");
// Bail at the end of the statement if its in an assignment // Bail at the end of the statement if its in an assignment

2
test/testastutils.cpp
View File

@ -159,7 +159,7 @@ private:
std::istringstream istr(code); std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp"); tokenizer.tokenize(istr, "test.cpp");
const Token * const argtok = Token::findmatch(tokenizer.tokens(), pattern); const Token * const argtok = Token::findmatch(tokenizer.tokens(), pattern);
return ::isVariableChangedByFunctionCall(argtok, &settings, inconclusive); return ::isVariableChangedByFunctionCall(argtok, 0, &settings, inconclusive);
} }
void isVariableChangedByFunctionCall() { void isVariableChangedByFunctionCall() {

5
test/testsimplifytypedef.cpp
View File

@ -1150,7 +1150,10 @@ private:
"}"; "}";
ASSERT_EQUALS(expected, tok(code, false)); ASSERT_EQUALS(expected, tok(code, false));
ASSERT_EQUALS_WITHOUT_LINENUMBERS("[test.cpp:28]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n", errout.str()); ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:28]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n"
"[test.cpp:26]: (debug) valueflow.cpp::valueFlowForward bailout: possible assignment of s by subfunction\n"
, errout.str());
} }
void simplifyTypedef36() { void simplifyTypedef36() {

16
test/testuninitvar.cpp
View File

@ -4411,6 +4411,22 @@ private:
" return;\n" " return;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// # 9305
valueFlowUninit("struct kf {\n"
" double x;\n"
"};\n"
"void set(kf* k) {\n"
" k->x = 0;\n"
"}\n"
"void cal() {\n"
" KF b;\n"
" KF* pb = &b;\n"
" set( pb);\n"
" if (pb->x)\n"
" return;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
} }
void uninitvar_memberfunction() { void uninitvar_memberfunction() {