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

Fixed #8037 (ValueFlow: global variable might be modified by function call)

This commit is contained in:
Daniel Marjamäki 2017-07-09 12:50:17 +02:00
parent 0a91ced941
commit 32fe0aba41
5 changed files with 38 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/astutils.cpp
View File

@ -418,10 +418,13 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
return arg && !arg->isConst() && arg->isReference(); return arg && !arg->isConst() && arg->isReference();
} }
bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, const Settings *settings) bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, bool globalvar, const Settings *settings)
{ {
for (const Token *tok = start; tok != end; tok = tok->next()) { for (const Token *tok = start; tok != end; tok = tok->next()) {
if (tok->varId() != varid) { if (tok->varId() != varid) {
if (globalvar && Token::Match(tok, "%name% ("))
// TODO: Is global variable really changed by function call?
return true;
continue; continue;
} }

2
lib/astutils.h
View File

@ -85,7 +85,7 @@ bool isReturnScope(const Token *endToken);
bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive); bool isVariableChangedByFunctionCall(const Token *tok, 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 unsigned int varid, const Settings *settings); bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, bool globalvar, const Settings *settings);
/** 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
* @param start token which is supposed to be the function/macro name. * @param start token which is supposed to be the function/macro name.

2
lib/checkcondition.cpp
View File

@ -162,7 +162,7 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok,
// is variable changed in loop? // is variable changed in loop?
const Token *bodyStart = tok2->linkAt(1)->next(); const Token *bodyStart = tok2->linkAt(1)->next();
const Token *bodyEnd = bodyStart ? bodyStart->link() : nullptr; const Token *bodyEnd = bodyStart ? bodyStart->link() : nullptr;
if (!bodyEnd || bodyEnd->str() != "}" || isVariableChanged(bodyStart, bodyEnd, varid, _settings)) if (!bodyEnd || bodyEnd->str() != "}" || isVariableChanged(bodyStart, bodyEnd, varid, !islocal, _settings))
continue; continue;
} }

43
lib/valueflow.cpp
View File

@ -1063,7 +1063,7 @@ static void valueFlowReverse(TokenList *tokenlist,
const Token *start = tok2; const Token *start = tok2;
const Token *end = start->link(); const Token *end = start->link();
if (isVariableChanged(start,end,varid, settings)) { if (isVariableChanged(start,end,varid,var->isGlobal(),settings)) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in loop. so valueflow analysis bailout when start of loop is reached."); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in loop. so valueflow analysis bailout when start of loop is reached.");
break; break;
@ -1142,7 +1142,7 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo
// Variable changed in 3rd for-expression // Variable changed in 3rd for-expression
if (Token::simpleMatch(tok2->previous(), "for (")) { if (Token::simpleMatch(tok2->previous(), "for (")) {
if (tok2->astOperand2() && tok2->astOperand2()->astOperand2() && isVariableChanged(tok2->astOperand2()->astOperand2(), tok2->link(), varid, settings)) { if (tok2->astOperand2() && tok2->astOperand2()->astOperand2() && isVariableChanged(tok2->astOperand2()->astOperand2(), tok2->link(), varid, var->isGlobal(), settings)) {
varid = 0U; varid = 0U;
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop"); bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
@ -1154,7 +1154,7 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo
const Token * const start = tok2->link()->next(); const Token * const start = tok2->link()->next();
const Token * const end = start->link(); const Token * const end = start->link();
if (isVariableChanged(start,end,varid, settings)) { if (isVariableChanged(start,end,varid,var->isGlobal(),settings)) {
varid = 0U; varid = 0U;
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop"); bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
@ -1248,13 +1248,14 @@ static void handleKnownValuesInLoop(const Token *startToken,
const Token *endToken, const Token *endToken,
std::list<ValueFlow::Value> *values, std::list<ValueFlow::Value> *values,
unsigned int varid, unsigned int varid,
bool globalvar,
const Settings *settings) const Settings *settings)
{ {
bool isChanged = false; bool isChanged = false;
for (std::list<ValueFlow::Value>::iterator it = values->begin(); it != values->end(); ++it) { for (std::list<ValueFlow::Value>::iterator it = values->begin(); it != values->end(); ++it) {
if (it->isKnown()) { if (it->isKnown()) {
if (!isChanged) { if (!isChanged) {
if (!isVariableChanged(startToken, endToken, varid, settings)) if (!isVariableChanged(startToken, endToken, varid, globalvar, settings))
break; break;
isChanged = true; isChanged = true;
} }
@ -1320,7 +1321,7 @@ static bool valueFlowForward(Token * const startToken,
} else if (indentlevel <= 0 && } else if (indentlevel <= 0 &&
Token::simpleMatch(tok2->link()->previous(), "else {") && Token::simpleMatch(tok2->link()->previous(), "else {") &&
!isReturnScope(tok2->link()->tokAt(-2)) && !isReturnScope(tok2->link()->tokAt(-2)) &&
isVariableChanged(tok2->link(), tok2, varid, settings)) { isVariableChanged(tok2->link(), tok2, varid, var->isGlobal(), settings)) {
std::list<ValueFlow::Value>::iterator it; std::list<ValueFlow::Value>::iterator it;
for (it = values.begin(); it != values.end(); ++it) for (it = values.begin(); it != values.end(); ++it)
it->changeKnownToPossible(); it->changeKnownToPossible();
@ -1366,19 +1367,19 @@ static bool valueFlowForward(Token * const startToken,
if (Token::simpleMatch(end, "} while (")) if (Token::simpleMatch(end, "} while ("))
end = end->linkAt(2); end = end->linkAt(2);
if (isVariableChanged(start, end, varid, settings)) { if (isVariableChanged(start, end, varid, var->isGlobal(), settings)) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in do-while"); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in do-while");
return false; return false;
} }
handleKnownValuesInLoop(start, end, &values, varid, settings); handleKnownValuesInLoop(start, end, &values, varid, var->isGlobal(), settings);
} }
// 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, settings)) { if (isVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), 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;
@ -1386,7 +1387,7 @@ static bool valueFlowForward(Token * const startToken,
// if known variable is changed in loop body, change it to a possible value.. // if known variable is changed in loop body, change it to a possible value..
if (Token::Match(tok2, "for|while")) if (Token::Match(tok2, "for|while"))
handleKnownValuesInLoop(tok2, tok2->linkAt(1)->linkAt(1), &values, varid, settings); handleKnownValuesInLoop(tok2, tok2->linkAt(1)->linkAt(1), &values, varid, var->isGlobal(), settings);
// Set values in condition // Set values in condition
for (Token* tok3 = tok2->tokAt(2); tok3 != tok2->next()->link(); tok3 = tok3->next()) { for (Token* tok3 = tok2->tokAt(2); tok3 != tok2->next()->link(); tok3 = tok3->next()) {
@ -1426,7 +1427,7 @@ static bool valueFlowForward(Token * const startToken,
errorLogger, errorLogger,
settings); settings);
if (isVariableChanged(startToken1, startToken1->link(), varid, settings)) { if (isVariableChanged(startToken1, startToken1->link(), varid, var->isGlobal(), settings)) {
removeValues(values, truevalues); removeValues(values, truevalues);
std::list<ValueFlow::Value>::iterator it; std::list<ValueFlow::Value>::iterator it;
@ -1449,7 +1450,7 @@ static bool valueFlowForward(Token * const startToken,
Token * const start = tok2->linkAt(1)->next(); Token * const start = tok2->linkAt(1)->next();
Token * const end = start->link(); Token * const end = start->link();
bool varusage = (indentlevel >= 0 && constValue && number_of_if == 0U) ? bool varusage = (indentlevel >= 0 && constValue && number_of_if == 0U) ?
isVariableChanged(start,end,varid, settings) : isVariableChanged(start,end,varid,var->isGlobal(),settings) :
(nullptr != Token::findmatch(start, "%varid%", end, varid)); (nullptr != Token::findmatch(start, "%varid%", end, varid));
if (!read) { if (!read) {
read = bool(nullptr != Token::findmatch(tok2, "%varid% !!=", end, varid)); read = bool(nullptr != Token::findmatch(tok2, "%varid% !!=", end, varid));
@ -1518,7 +1519,7 @@ static bool valueFlowForward(Token * const startToken,
return false; return false;
} }
if (isVariableChanged(start, end, varid, settings)) { if (isVariableChanged(start, end, varid, var->isGlobal(), settings)) {
if ((!read || number_of_if == 0) && if ((!read || number_of_if == 0) &&
Token::simpleMatch(tok2, "if (") && Token::simpleMatch(tok2, "if (") &&
!(Token::simpleMatch(end, "} else {") && !(Token::simpleMatch(end, "} else {") &&
@ -1804,7 +1805,7 @@ static bool valueFlowForward(Token * const startToken,
Token::simpleMatch(tok2->linkAt(1), "] (") && Token::simpleMatch(tok2->linkAt(1), "] (") &&
Token::simpleMatch(tok2->linkAt(1)->linkAt(1), ") {")) { Token::simpleMatch(tok2->linkAt(1)->linkAt(1), ") {")) {
const Token *bodyStart = tok2->linkAt(1)->linkAt(1)->next(); const Token *bodyStart = tok2->linkAt(1)->linkAt(1)->next();
if (isVariableChanged(bodyStart, bodyStart->link(), varid, settings)) { if (isVariableChanged(bodyStart, bodyStart->link(), varid, var->isGlobal(), settings)) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "valueFlowForward, " + var->name() + " is changed in lambda function"); bailout(tokenlist, errorLogger, tok2, "valueFlowForward, " + var->name() + " is changed in lambda function");
return false; return false;
@ -2107,7 +2108,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol
// does condition reassign variable? // does condition reassign variable?
if (tok != top->astOperand2() && if (tok != top->astOperand2() &&
Token::Match(top->astOperand2(), "%oror%|&&") && Token::Match(top->astOperand2(), "%oror%|&&") &&
isVariableChanged(top, top->link(), varid, settings)) { isVariableChanged(top, top->link(), varid, var->isGlobal(), settings)) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok, "assignment in condition"); bailout(tokenlist, errorLogger, tok, "assignment in condition");
continue; continue;
@ -2150,7 +2151,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol
if (!valueFlowForward(startToken->next(), startToken->link(), var, varid, values, true, false, tokenlist, errorLogger, settings)) if (!valueFlowForward(startToken->next(), startToken->link(), var, varid, values, true, false, tokenlist, errorLogger, settings))
continue; continue;
values.front().setPossible(); values.front().setPossible();
if (isVariableChanged(startToken, startToken->link(), varid, settings)) { if (isVariableChanged(startToken, startToken->link(), varid, var->isGlobal(), settings)) {
// TODO: The endToken should not be startToken->link() in the valueFlowForward call // TODO: The endToken should not be startToken->link() in the valueFlowForward call
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, startToken->link(), "valueFlowAfterCondition: " + var->name() + " is changed in conditional block"); bailout(tokenlist, errorLogger, startToken->link(), "valueFlowAfterCondition: " + var->name() + " is changed in conditional block");
@ -2449,12 +2450,12 @@ static bool valueFlowForLoop2(const Token *tok,
return true; return true;
} }
static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int varid, const MathLib::bigint value, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int varid, bool globalvar, const MathLib::bigint value, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
{ {
const Token * const bodyEnd = bodyStart->link(); const Token * const bodyEnd = bodyStart->link();
// Is variable modified inside for loop // Is variable modified inside for loop
if (isVariableChanged(bodyStart, bodyEnd, varid, settings)) if (isVariableChanged(bodyStart, bodyEnd, varid, globalvar, settings))
return; return;
for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) {
@ -2587,8 +2588,8 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas
if (valueFlowForLoop1(tok, &varid, &num1, &num2, &numAfter)) { if (valueFlowForLoop1(tok, &varid, &num1, &num2, &numAfter)) {
if (num1 <= num2) { if (num1 <= num2) {
valueFlowForLoopSimplify(bodyStart, varid, num1, tokenlist, errorLogger, settings); valueFlowForLoopSimplify(bodyStart, varid, false, num1, tokenlist, errorLogger, settings);
valueFlowForLoopSimplify(bodyStart, varid, num2, tokenlist, errorLogger, settings); valueFlowForLoopSimplify(bodyStart, varid, false, num2, tokenlist, errorLogger, settings);
valueFlowForLoopSimplifyAfter(tok, varid, numAfter, tokenlist, errorLogger, settings); valueFlowForLoopSimplifyAfter(tok, varid, numAfter, tokenlist, errorLogger, settings);
} else } else
valueFlowForLoopSimplifyAfter(tok, varid, num1, tokenlist, errorLogger, settings); valueFlowForLoopSimplifyAfter(tok, varid, num1, tokenlist, errorLogger, settings);
@ -2599,12 +2600,12 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas
for (it = mem1.values.begin(); it != mem1.values.end(); ++it) { for (it = mem1.values.begin(); it != mem1.values.end(); ++it) {
if (!it->second.isIntValue()) if (!it->second.isIntValue())
continue; continue;
valueFlowForLoopSimplify(bodyStart, it->first, it->second.intvalue, tokenlist, errorLogger, settings); valueFlowForLoopSimplify(bodyStart, it->first, false, it->second.intvalue, tokenlist, errorLogger, settings);
} }
for (it = mem2.values.begin(); it != mem2.values.end(); ++it) { for (it = mem2.values.begin(); it != mem2.values.end(); ++it) {
if (!it->second.isIntValue()) if (!it->second.isIntValue())
continue; continue;
valueFlowForLoopSimplify(bodyStart, it->first, it->second.intvalue, tokenlist, errorLogger, settings); valueFlowForLoopSimplify(bodyStart, it->first, false, it->second.intvalue, tokenlist, errorLogger, settings);
} }
for (it = memAfter.values.begin(); it != memAfter.values.end(); ++it) { for (it = memAfter.values.begin(); it != memAfter.values.end(); ++it) {
if (!it->second.isIntValue()) if (!it->second.isIntValue())

10
test/testvalueflow.cpp
View File

@ -2296,6 +2296,16 @@ private:
"}\n"; "}\n";
ASSERT(isNotKnownValues(code, "<")); ASSERT(isNotKnownValues(code, "<"));
code = "int x;\n"
"void f() {\n"
" x = 4;\n"
" while (1) {\n"
" a = x+2;\n"
" dostuff();\n"
" }\n"
"}";
ASSERT(isNotKnownValues(code, "+"));
code = "void f() {\n" code = "void f() {\n"
" int x = 0;\n" " int x = 0;\n"
" if (y) { dostuff(x); }\n" " if (y) { dostuff(x); }\n"