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

Merge pull request #2790 from pfultz2/forward-analyze-accumulate-actions 

Fix issue 9833: False positive: Division by zero when using pointer to struct
This commit is contained in:
Daniel Marjamäki 2020-09-11 07:17:10 +02:00 committed by GitHub
parent 2b12ef653d 6394ad2643
commit ac66f67bad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 121 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/forwardanalyzer.cpp
View File

@ -10,8 +10,18 @@
struct ForwardTraversal { struct ForwardTraversal {
enum class Progress { Continue, Break, Skip }; enum class Progress { Continue, Break, Skip };
ForwardTraversal(const ValuePtr<ForwardAnalyzer>& analyzer, const Settings* settings)
: analyzer(analyzer), settings(settings), actions(ForwardAnalyzer::Action::None), analyzeOnly(false)
{}
ValuePtr<ForwardAnalyzer> analyzer; ValuePtr<ForwardAnalyzer> analyzer;
const Settings* settings; const Settings* settings;
ForwardAnalyzer::Action actions;
bool analyzeOnly;
bool stopUpdates() {
analyzeOnly = true;
return actions.isModified();
}
std::pair<bool, bool> evalCond(const Token* tok) { std::pair<bool, bool> evalCond(const Token* tok) {
std::vector<int> result = analyzer->evaluate(tok); std::vector<int> result = analyzer->evaluate(tok);
@ -91,7 +101,7 @@ struct ForwardTraversal {
std::tie(checkThen, checkElse) = evalCond(condTok); std::tie(checkThen, checkElse) = evalCond(condTok);
if (!checkThen && !checkElse) { if (!checkThen && !checkElse) {
// Stop if the value is conditional // Stop if the value is conditional
if (!traverseUnknown && analyzer->isConditional()) if (!traverseUnknown && analyzer->isConditional() && stopUpdates())
return Progress::Break; return Progress::Break;
checkThen = true; checkThen = true;
checkElse = true; checkElse = true;
@ -115,7 +125,8 @@ struct ForwardTraversal {
Progress update(Token* tok) { Progress update(Token* tok) {
ForwardAnalyzer::Action action = analyzer->analyze(tok); ForwardAnalyzer::Action action = analyzer->analyze(tok);
if (!action.isNone()) actions |= action;
if (!action.isNone() && !analyzeOnly)
analyzer->update(tok, action); analyzer->update(tok, action);
if (action.isInconclusive() && !analyzer->lowerToInconclusive()) if (action.isInconclusive() && !analyzer->lowerToInconclusive())
return Progress::Break; return Progress::Break;
@ -225,6 +236,7 @@ struct ForwardTraversal {
allAnalysis |= analyzeRecursive(initTok); allAnalysis |= analyzeRecursive(initTok);
if (stepTok) if (stepTok)
allAnalysis |= analyzeRecursive(stepTok); allAnalysis |= analyzeRecursive(stepTok);
actions |= allAnalysis;
if (allAnalysis.isInconclusive()) { if (allAnalysis.isInconclusive()) {
if (!analyzer->lowerToInconclusive()) if (!analyzer->lowerToInconclusive())
return Progress::Break; return Progress::Break;
@ -399,6 +411,7 @@ struct ForwardTraversal {
} else { } else {
tok = endBlock; tok = endBlock;
} }
actions |= (thenAction | elseAction);
if (bail) if (bail)
return Progress::Break; return Progress::Break;
if (returnThen && returnElse) if (returnThen && returnElse)
@ -412,7 +425,7 @@ struct ForwardTraversal {
if (checkThen) { if (checkThen) {
return Progress::Break; return Progress::Break;
} else { } else {
if (analyzer->isConditional()) if (analyzer->isConditional() && stopUpdates())
return Progress::Break; return Progress::Break;
analyzer->assume(condTok, false); analyzer->assume(condTok, false);
} }
@ -421,8 +434,8 @@ struct ForwardTraversal {
if (!analyzer->lowerToInconclusive()) if (!analyzer->lowerToInconclusive())
return Progress::Break; return Progress::Break;
} else if (thenAction.isModified() || elseAction.isModified()) { } else if (thenAction.isModified() || elseAction.isModified()) {
if (!hasElse && analyzer->isConditional()) if (!hasElse && analyzer->isConditional() && stopUpdates())
return Progress::Break; return Progress::Break;
if (!analyzer->lowerToPossible()) if (!analyzer->lowerToPossible())
return Progress::Break; return Progress::Break;
analyzer->assume(condTok, elseAction.isModified()); analyzer->assume(condTok, elseAction.isModified());
@ -552,8 +565,12 @@ struct ForwardTraversal {
}; };
void valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<ForwardAnalyzer>& fa, const Settings* settings) ForwardAnalyzer::Action valueFlowGenericForward(Token* start,
const Token* end,
const ValuePtr<ForwardAnalyzer>& fa,
const Settings* settings)
{ {
ForwardTraversal ft{fa, settings}; ForwardTraversal ft{fa, settings};
ft.updateRange(start, end); ft.updateRange(start, end);
return ft.actions;
} }

5
lib/forwardanalyzer.h
View File

@ -113,6 +113,9 @@ struct ForwardAnalyzer {
virtual ~ForwardAnalyzer() {} virtual ~ForwardAnalyzer() {}
}; };
void valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<ForwardAnalyzer>& fa, const Settings* settings); ForwardAnalyzer::Action valueFlowGenericForward(Token* start,
const Token* end,
const ValuePtr<ForwardAnalyzer>& fa,
const Settings* settings);
#endif #endif

126
lib/valueflow.cpp
View File

@ -1710,12 +1710,12 @@ static void valueFlowGlobalStaticVar(TokenList *tokenList, const Settings *setti
} }
} }
static void valueFlowForwardVariable(Token* const startToken, static ForwardAnalyzer::Action valueFlowForwardVariable(Token* const startToken,
const Token* const endToken, const Token* const endToken,
const Variable* const var, const Variable* const var,
std::list<ValueFlow::Value> values, std::list<ValueFlow::Value> values,
TokenList* const tokenlist, TokenList* const tokenlist,
const Settings* const settings); const Settings* const settings);
// Old deprecated version // Old deprecated version
static void valueFlowForward(Token* startToken, static void valueFlowForward(Token* startToken,
@ -2697,28 +2697,31 @@ static std::vector<const Variable*> getAliasesFromValues(std::list<ValueFlow::Va
return aliases; return aliases;
} }
static void valueFlowForwardVariable(Token* const startToken, static ForwardAnalyzer::Action valueFlowForwardVariable(Token* const startToken,
const Token* const endToken, const Token* const endToken,
const Variable* const var, const Variable* const var,
std::list<ValueFlow::Value> values, std::list<ValueFlow::Value> values,
std::vector<const Variable*> aliases, std::vector<const Variable*> aliases,
TokenList* const tokenlist, TokenList* const tokenlist,
const Settings* const settings) const Settings* const settings)
{ {
ForwardAnalyzer::Action actions;
for (ValueFlow::Value& v : values) { for (ValueFlow::Value& v : values) {
VariableForwardAnalyzer a(var, v, aliases, tokenlist); VariableForwardAnalyzer a(var, v, aliases, tokenlist);
valueFlowGenericForward(startToken, endToken, a, settings); actions |= valueFlowGenericForward(startToken, endToken, a, settings);
} }
return actions;
} }
static void valueFlowForwardVariable(Token* const startToken, static ForwardAnalyzer::Action valueFlowForwardVariable(Token* const startToken,
const Token* const endToken, const Token* const endToken,
const Variable* const var, const Variable* const var,
std::list<ValueFlow::Value> values, std::list<ValueFlow::Value> values,
TokenList* const tokenlist, TokenList* const tokenlist,
const Settings* const settings) const Settings* const settings)
{ {
valueFlowForwardVariable(startToken, endToken, var, std::move(values), getAliasesFromValues(values), tokenlist, settings); return valueFlowForwardVariable(
startToken, endToken, var, std::move(values), getAliasesFromValues(values), tokenlist, settings);
} }
// Old deprecated version // Old deprecated version
@ -2805,17 +2808,19 @@ struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer {
} }
}; };
static void valueFlowForwardExpression(Token* startToken, static ForwardAnalyzer::Action valueFlowForwardExpression(Token* startToken,
const Token* endToken, const Token* endToken,
const Token* exprTok, const Token* exprTok,
const std::list<ValueFlow::Value>& values, const std::list<ValueFlow::Value>& values,
const TokenList* const tokenlist, const TokenList* const tokenlist,
const Settings* settings) const Settings* settings)
{ {
ForwardAnalyzer::Action actions;
for (const ValueFlow::Value& v : values) { for (const ValueFlow::Value& v : values) {
ExpressionForwardAnalyzer a(exprTok, v, tokenlist); ExpressionForwardAnalyzer a(exprTok, v, tokenlist);
valueFlowGenericForward(startToken, endToken, a, settings); actions |= valueFlowGenericForward(startToken, endToken, a, settings);
} }
return actions;
} }
static const Token* parseBinaryIntOp(const Token* expr, MathLib::bigint& known) static const Token* parseBinaryIntOp(const Token* expr, MathLib::bigint& known)
@ -2879,23 +2884,18 @@ static const Token* solveExprValues(const Token* expr, std::list<ValueFlow::Valu
return expr; return expr;
} }
static void valueFlowForward(Token* startToken, static ForwardAnalyzer::Action valueFlowForward(Token* startToken,
const Token* endToken, const Token* endToken,
const Token* exprTok, const Token* exprTok,
std::list<ValueFlow::Value> values, std::list<ValueFlow::Value> values,
TokenList* const tokenlist, TokenList* const tokenlist,
const Settings* settings) const Settings* settings)
{ {
const Token* expr = solveExprValues(exprTok, values); const Token* expr = solveExprValues(exprTok, values);
if (expr->variable()) { if (expr->variable()) {
valueFlowForwardVariable(startToken, return valueFlowForwardVariable(startToken, endToken, expr->variable(), values, tokenlist, settings);
endToken,
expr->variable(),
values,
tokenlist,
settings);
} else { } else {
valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings); return valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings);
} }
} }
@ -4549,15 +4549,10 @@ static void valueFlowAfterCondition(TokenList *tokenlist,
const Settings *settings) const Settings *settings)
{ {
ValueFlowConditionHandler handler; ValueFlowConditionHandler handler;
handler.forward = [&](Token* start, handler.forward =
const Token* stop, [&](Token* start, const Token* stop, const Token* vartok, const std::list<ValueFlow::Value>& values, bool) {
const Token* vartok, return valueFlowForward(start->next(), stop, vartok, values, tokenlist, settings).isModified();
const std::list<ValueFlow::Value>& values, };
bool) {
valueFlowForward(start->next(), stop, vartok, values, tokenlist, settings);
std::vector<const Variable*> vars = getExprVariables(vartok, tokenlist, symboldatabase, settings);
return isVariablesChanged(start, stop, 0, vars, settings, tokenlist->isCPP());
};
handler.parse = [&](const Token *tok) { handler.parse = [&](const Token *tok) {
ValueFlowConditionHandler::Condition cond; ValueFlowConditionHandler::Condition cond;
ValueFlow::Value true_value; ValueFlow::Value true_value;
@ -5887,19 +5882,26 @@ struct ContainerVariableForwardAnalyzer : VariableForwardAnalyzer {
} }
}; };
static void valueFlowContainerForward(Token *tok, const Token* endToken, const Variable* var, ValueFlow::Value value, TokenList *tokenlist) static ForwardAnalyzer::Action valueFlowContainerForward(Token* tok,
const Token* endToken,
const Variable* var,
ValueFlow::Value value,
TokenList* tokenlist)
{ {
ContainerVariableForwardAnalyzer a(var, value, getAliasesFromValues({value}), tokenlist); ContainerVariableForwardAnalyzer a(var, value, getAliasesFromValues({value}), tokenlist);
valueFlowGenericForward(tok, endToken, a, tokenlist->getSettings()); return valueFlowGenericForward(tok, endToken, a, tokenlist->getSettings());
} }
static void valueFlowContainerForward(Token *tok, const Variable* var, ValueFlow::Value value, TokenList *tokenlist) static ForwardAnalyzer::Action valueFlowContainerForward(Token* tok,
const Variable* var,
ValueFlow::Value value,
TokenList* tokenlist)
{ {
const Token * endOfVarScope = nullptr; const Token * endOfVarScope = nullptr;
if (var->isLocal() || var->isArgument()) if (var->isLocal() || var->isArgument())
endOfVarScope = var->scope()->bodyEnd; endOfVarScope = var->scope()->bodyEnd;
if (!endOfVarScope) if (!endOfVarScope)
endOfVarScope = tok->scope()->bodyEnd; endOfVarScope = tok->scope()->bodyEnd;
valueFlowContainerForward(tok, endOfVarScope, var, std::move(value), tokenlist); return valueFlowContainerForward(tok, endOfVarScope, var, std::move(value), tokenlist);
} }
static bool isContainerSizeChanged(const Token *tok, int depth) static bool isContainerSizeChanged(const Token *tok, int depth)
@ -6048,15 +6050,10 @@ static void valueFlowIteratorAfterCondition(TokenList *tokenlist,
const Settings *settings) const Settings *settings)
{ {
ValueFlowConditionHandler handler; ValueFlowConditionHandler handler;
handler.forward = [&](Token* start, handler.forward =
const Token* stop, [&](Token* start, const Token* stop, const Token* vartok, const std::list<ValueFlow::Value>& values, bool) {
const Token* vartok, return valueFlowForward(start->next(), stop, vartok, values, tokenlist, settings).isModified();
const std::list<ValueFlow::Value>& values, };
bool) {
valueFlowForward(start->next(), stop, vartok, values, tokenlist, settings);
std::vector<const Variable*> vars = getExprVariables(vartok, tokenlist, symboldatabase, settings);
return isVariablesChanged(start, stop, 0, vars, settings, tokenlist->isCPP());
};
handler.parse = [&](const Token *tok) { handler.parse = [&](const Token *tok) {
ValueFlowConditionHandler::Condition cond; ValueFlowConditionHandler::Condition cond;
@ -6230,8 +6227,7 @@ static void valueFlowContainerAfterCondition(TokenList *tokenlist,
const Variable* var = vartok->variable(); const Variable* var = vartok->variable();
if (!var) if (!var)
return false; return false;
valueFlowContainerForward(start->next(), stop, var, values.front(), tokenlist); return valueFlowContainerForward(start->next(), stop, var, values.front(), tokenlist).isModified();
return isContainerSizeChanged(var->declarationId(), start, stop);
}; };
handler.parse = [&](const Token *tok) { handler.parse = [&](const Token *tok) {
ValueFlowConditionHandler::Condition cond; ValueFlowConditionHandler::Condition cond;

10
test/testother.cpp
View File

@ -697,6 +697,16 @@ private:
" a = b / -x;\n" " a = b / -x;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("struct A {\n"
" int x;\n"
"};\n"
"int f(A* a) {\n"
" if (a->x == 0) \n"
" a->x = 1;\n"
" return 1/a->x;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
} }
void nanInArithmeticExpression() { void nanInArithmeticExpression() {

23
test/testvalueflow.cpp
View File

@ -2424,6 +2424,29 @@ private:
" }" " }"
"}"; "}";
ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, 2)); ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, 2));
code = "int f(int i, int j) {\n"
" if (i == 0) {\n"
" if (j < 0)\n"
" return 0;\n"
" i = j+1;\n"
" }\n"
" int x = i;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(false, testValueOfX(code, 8U, 0));
code = "int f(int i, int j) {\n"
" if (i == 0) {\n"
" if (j < 0)\n"
" return 0;\n"
" if (j < 0)\n"
" i = j+1;\n"
" }\n"
" int x = i;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 9U, 0));
} }
void valueFlowAfterConditionExpr() { void valueFlowAfterConditionExpr() {