From ce44d2fd5ae4ca9af73d42748e2ac075f80a7178 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 12 Jul 2021 12:02:51 -0500 Subject: [PATCH 01/33] Replace with forward expression --- lib/valueflow.cpp | 111 ++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 58 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9889232877a..ddb140070f7 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1677,6 +1677,13 @@ static void valueFlowGlobalStaticVar(TokenList *tokenList, const Settings *setti } } +static Analyzer::Result valueFlowForward(Token* startToken, + const Token* endToken, + const Token* exprTok, + std::list values, + TokenList* const tokenlist, + const Settings* settings); + static Analyzer::Result valueFlowForwardVariable(Token* const startToken, const Token* const endToken, const Variable* const var, @@ -2424,34 +2431,28 @@ static Analyzer::Result valueFlowForwardVariable(Token* const startToken, const Token* const endToken, const Variable* const var, std::list values, - std::vector aliases, TokenList* const tokenlist, const Settings* const settings) { - Analyzer::Action actions; - Analyzer::Terminate terminate = Analyzer::Terminate::None; - for (ValueFlow::Value& v : values) { - VariableAnalyzer a(var, v, aliases, tokenlist); - Analyzer::Result r = valueFlowGenericForward(startToken, endToken, a, settings); - actions |= r.action; - if (terminate == Analyzer::Terminate::None) - terminate = r.terminate; - } - return {actions, terminate}; + const Token * exprTok = Token::findmatch(startToken, "%varid%", var->declarationId()); + if (!exprTok) + exprTok = var->nameToken(); + return valueFlowForward(startToken, endToken, exprTok, std::move(values), tokenlist, settings); } static Analyzer::Result valueFlowForwardVariable(Token* const startToken, const Token* const endToken, const Variable* const var, std::list values, + std::vector, TokenList* const tokenlist, const Settings* const settings) { - auto aliases = getAliasesFromValues(values); return valueFlowForwardVariable( - startToken, endToken, var, std::move(values), std::move(aliases), tokenlist, settings); + startToken, endToken, var, std::move(values), tokenlist, settings); } + // Old deprecated version static bool valueFlowForwardVariable(Token* const startToken, const Token* const endToken, @@ -2478,7 +2479,7 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { ExpressionAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t) : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false) { - setupExprVarIds(); + setupExprVarIds(expr); } virtual const ValueType* getValueType(const Token*) const OVERRIDE { @@ -2486,12 +2487,23 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { } static bool nonLocal(const Variable* var, bool deref) { - return !var || (!var->isLocal() && !var->isArgument()) || (deref && var->isArgument() && var->isPointer()) || var->isStatic() || var->isReference() || var->isExtern(); + return !var || (!var->isLocal() && !var->isArgument()) || (deref && var->isArgument() && var->isPointer()) || + var->isStatic() || var->isReference() || var->isExtern(); } - void setupExprVarIds() { - visitAstNodes(expr, - [&](const Token *tok) { + void setupExprVarIds(const Token* start, int depth = 4) { + if (depth < 0) + return; + visitAstNodes(start, [&](const Token* tok) { + for (const ValueFlow::Value& v : tok->values()) { + if (!v.isLocalLifetimeValue()) + continue; + if (!v.tokvalue) + continue; + if (v.tokvalue == tok) + continue; + setupExprVarIds(v.tokvalue, depth - 1); + } if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") { // unknown variable unknown = true; @@ -2500,10 +2512,13 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { if (tok->varId() > 0) { varids[tok->varId()] = tok->variable(); if (!Token::simpleMatch(tok->previous(), ".")) { - const Variable *var = tok->variable(); - if (var && var->isReference() && var->isLocal() && Token::Match(var->nameToken(), "%var% [=(]") && !isGlobalData(var->nameToken()->next()->astOperand2(), isCPP())) + const Variable* var = tok->variable(); + if (var && var->isReference() && var->isLocal() && Token::Match(var->nameToken(), "%var% [=(]") && + !isGlobalData(var->nameToken()->next()->astOperand2(), isCPP())) return ChildrenToVisit::none; - const bool deref = tok->astParent() && (tok->astParent()->isUnaryOp("*") || (tok->astParent()->str() == "[" && tok == tok->astParent()->astOperand1())); + const bool deref = tok->astParent() && + (tok->astParent()->isUnaryOp("*") || + (tok->astParent()->str() == "[" && tok == tok->astParent()->astOperand1())); local &= !nonLocal(tok->variable(), deref); } } @@ -2628,11 +2643,7 @@ ValuePtr makeAnalyzer(Token* exprTok, const ValueFlow::Value& value, c { std::list values = {value}; const Token* expr = solveExprValues(exprTok, values); - if (expr->variable()) { - return VariableAnalyzer(expr->variable(), value, getAliasesFromValues(values), tokenlist); - } else { - return ExpressionAnalyzer(expr, value, tokenlist); - } + return ExpressionAnalyzer(expr, value, tokenlist); } static Analyzer::Result valueFlowForward(Token* startToken, @@ -2643,13 +2654,24 @@ static Analyzer::Result valueFlowForward(Token* startToken, const Settings* settings) { const Token* expr = solveExprValues(exprTok, values); - if (expr->variable()) { - return valueFlowForwardVariable(startToken, endToken, expr->variable(), values, tokenlist, settings); - } else { - return valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings); + return valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings); +} + + +static void valueFlowReverse(Token* tok, + const Token* const endToken, + const Token* const varToken, + const std::list& values, + TokenList* tokenlist, + const Settings* settings) +{ + for (const ValueFlow::Value& v : values) { + ExpressionAnalyzer a(varToken, v, tokenlist); + valueFlowGenericReverse(tok, endToken, a, settings); } } + static void valueFlowReverse(TokenList* tokenlist, Token* tok, const Token* const varToken, @@ -2661,34 +2683,7 @@ static void valueFlowReverse(TokenList* tokenlist, std::list values = {val}; if (val2.varId != 0) values.push_back(val2); - const Variable* var = varToken->variable(); - auto aliases = getAliasesFromValues(values); - for (ValueFlow::Value& v : values) { - VariableAnalyzer a(var, v, aliases, tokenlist); - valueFlowGenericReverse(tok, a, settings); - } -} - -static void valueFlowReverse(Token* tok, - const Token* const endToken, - const Token* const varToken, - const std::list& values, - TokenList* tokenlist, - const Settings* settings) -{ - const Variable* var = varToken->variable(); - if (var) { - auto aliases = getAliasesFromValues(values); - for (const ValueFlow::Value& v : values) { - VariableAnalyzer a(var, v, aliases, tokenlist); - valueFlowGenericReverse(tok, endToken, a, settings); - } - } else { - for (const ValueFlow::Value& v : values) { - ExpressionAnalyzer a(varToken, v, tokenlist); - valueFlowGenericReverse(tok, endToken, a, settings); - } - } + valueFlowReverse(tok, nullptr, varToken, values, tokenlist, settings); } std::string lifetimeType(const Token *tok, const ValueFlow::Value *val) From 644fed75d40dd6d25455706bacfd85ecc8aec227 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 12 Jul 2021 19:14:57 -0500 Subject: [PATCH 02/33] Check for const expression --- lib/astutils.cpp | 2 ++ lib/valueflow.cpp | 12 +++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 891a836ee1d..dd0a1904cbe 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1396,6 +1396,8 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure, bool { if (!tok) return true; + if (tok->variable() && tok->variable()->isVolatile()) + return false; if (tok->isName() && tok->next()->str() == "(") { if (!tok->function() && !Token::Match(tok->previous(), ".|::") && !library.isFunctionConst(tok->str(), pure)) return false; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ddb140070f7..c659c7b883f 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2537,7 +2537,7 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { } virtual bool match(const Token* tok) const OVERRIDE { - return isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); + return tok->exprId() == expr->exprId() || isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); } virtual bool isGlobal() const OVERRIDE { @@ -4012,9 +4012,6 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { if (!Token::simpleMatch(tok, "if (")) continue; - // Skip known values - if (tok->next()->hasKnownValue()) - continue; Token * parenTok = tok->next(); if (!Token::simpleMatch(parenTok->link(), ") {")) continue; @@ -4022,6 +4019,8 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* const Token* condTok = parenTok->astOperand2(); if (condTok->hasKnownIntValue()) continue; + if (!isConstExpression(condTok, settings->library, true, tokenlist->isCPP())) + continue; const bool is1 = (condTok->isComparisonOp() || condTok->tokType() == Token::eLogicalOp || astIsBool(condTok)); Token* startTok = blockTok; @@ -4387,11 +4386,10 @@ struct ConditionHandler { Condition cond = parse(tok, settings); if (!cond.vartok) continue; - if (cond.vartok->variable() && cond.vartok->variable()->isVolatile()) - continue; if (cond.true_values.empty() || cond.false_values.empty()) continue; - + if (!isConstExpression(cond.vartok, settings->library, true, tokenlist->isCPP())) + continue; if (exprDependsOnThis(cond.vartok)) continue; std::vector vars = getExprVariables(cond.vartok, tokenlist, symboldatabase, settings); From 9d461120e567ad10d023fa7688883edfb327942e Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 13 Jul 2021 19:33:31 -0500 Subject: [PATCH 03/33] Fix issue with uknown in alias --- lib/valueflow.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c659c7b883f..1081d1f4a1d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2491,8 +2491,9 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { var->isStatic() || var->isReference() || var->isExtern(); } - void setupExprVarIds(const Token* start, int depth = 4) { - if (depth < 0) + void setupExprVarIds(const Token* start, int depth = 0) { + const int maxDepth = 4; + if (depth > maxDepth) return; visitAstNodes(start, [&](const Token* tok) { for (const ValueFlow::Value& v : tok->values()) { @@ -2502,9 +2503,9 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { continue; if (v.tokvalue == tok) continue; - setupExprVarIds(v.tokvalue, depth - 1); + setupExprVarIds(v.tokvalue, depth + 1); } - if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") { + if (depth == 0 && tok->varId() == 0 && !tok->function() && tok->isName() && tok->previous()->str() != ".") { // unknown variable unknown = true; return ChildrenToVisit::none; From 2b672725c2b162fab2a6397cc4ea40c7546bb22f Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 13 Jul 2021 19:48:27 -0500 Subject: [PATCH 04/33] Check for expression that depend on this --- lib/valueflow.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1081d1f4a1d..4ee97f83cac 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1980,6 +1980,9 @@ struct ValueFlowAnalyzer : Analyzer { virtual bool isGlobal() const { return false; } + virtual bool dependsOnThis() const { + return false; + } virtual bool invalid() const { return false; @@ -2174,6 +2177,9 @@ struct ValueFlowAnalyzer : Analyzer { return a; } + if (dependsOnThis() && exprDependsOnThis(tok)) + return isAliasModified(tok); + // TODO: Check if function is pure if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { // bailout: global non-const variables @@ -2473,12 +2479,14 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { const Token* expr; bool local; bool unknown; + bool dependOnThis; - ExpressionAnalyzer() : SingleValueFlowAnalyzer(), expr(nullptr), local(true), unknown(false) {} + ExpressionAnalyzer() : SingleValueFlowAnalyzer(), expr(nullptr), local(true), unknown(false), dependOnThis(false) {} ExpressionAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t) - : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false) { + : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false), dependOnThis(false) { + dependOnThis = exprDependsOnThis(expr); setupExprVarIds(expr); } @@ -2541,6 +2549,10 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { return tok->exprId() == expr->exprId() || isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); } + virtual bool dependsOnThis() const OVERRIDE { + return dependOnThis; + } + virtual bool isGlobal() const OVERRIDE { return !local; } From 6261342e5588218f654b16ccd8da75d5e1aa510e Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 13 Jul 2021 19:57:56 -0500 Subject: [PATCH 05/33] Just use expression id --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 4ee97f83cac..06794ec8ebf 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2546,7 +2546,7 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { } virtual bool match(const Token* tok) const OVERRIDE { - return tok->exprId() == expr->exprId() || isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); + return tok->exprId() == expr->exprId(); } virtual bool dependsOnThis() const OVERRIDE { From c2ebc782ece7a5bfe2a259833d86b300caaf56bf Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 13 Jul 2021 20:32:15 -0500 Subject: [PATCH 06/33] Check if this variable is modified --- lib/astutils.cpp | 26 +++++++++++++++----------- lib/astutils.h | 1 + lib/valueflow.cpp | 10 +++++++--- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index dd0a1904cbe..89c1342b9d1 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2094,6 +2094,20 @@ bool isVariablesChanged(const Token* start, return false; } +bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp) +{ + if (Token::Match(tok->previous(), "%name% (")) { + if (tok->previous()->function()) { + return (!tok->previous()->function()->isConst()); + } else if (!tok->previous()->isKeyword()) { + return true; + } + } + if (isVariableChanged(tok, indirect, settings, cpp)) + return true; + return false; +} + bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp) { if (!precedes(start, end)) @@ -2101,17 +2115,7 @@ bool isThisChanged(const Token* start, const Token* end, int indirect, const Set for (const Token* tok = start; tok != end; tok = tok->next()) { if (!exprDependsOnThis(tok)) continue; - if (Token::Match(tok->previous(), "%name% (")) { - if (tok->previous()->function()) { - if (!tok->previous()->function()->isConst()) - return true; - else - continue; - } else if (!tok->previous()->isKeyword()) { - return true; - } - } - if (isVariableChanged(tok, indirect, settings, cpp)) + if (isThisChanged(tok, indirect, settings, cpp)) return true; } return false; diff --git a/lib/astutils.h b/lib/astutils.h index a2598e05cf8..cfc1081c91d 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -226,6 +226,7 @@ bool isVariablesChanged(const Token* start, const Settings* settings, bool cpp); +bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp); bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp); const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 06794ec8ebf..47fce31abda 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2032,6 +2032,12 @@ struct ValueFlowAnalyzer : Analyzer { return Action::None; } + virtual Action isThisModified(const Token* tok) const { + if (isThisChanged(tok, 0, getSettings(), isCPP())) + return Action::Invalid; + return Action::None; + } + virtual Action isWritable(const Token* tok, Direction d) const { const ValueFlow::Value* value = getValue(tok); if (!value) @@ -2178,7 +2184,7 @@ struct ValueFlowAnalyzer : Analyzer { } if (dependsOnThis() && exprDependsOnThis(tok)) - return isAliasModified(tok); + return isThisModified(tok); // TODO: Check if function is pure if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { @@ -4403,8 +4409,6 @@ struct ConditionHandler { continue; if (!isConstExpression(cond.vartok, settings->library, true, tokenlist->isCPP())) continue; - if (exprDependsOnThis(cond.vartok)) - continue; std::vector vars = getExprVariables(cond.vartok, tokenlist, symboldatabase, settings); if (std::any_of(vars.begin(), vars.end(), [](const Variable* var) { return !var; From 5076e916e5a732ad53f1ee84052415884adcfa41 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 14 Jul 2021 11:12:51 -0500 Subject: [PATCH 07/33] Improve checking for side effects --- cfg/std.cfg | 1 + lib/astutils.cpp | 2 +- lib/valueflow.cpp | 14 ++++++++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index c3d13b9a0b8..ecb3febbce6 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -7715,6 +7715,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init + false diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 89c1342b9d1..d493ad54d3b 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1401,7 +1401,7 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure, bool if (tok->isName() && tok->next()->str() == "(") { if (!tok->function() && !Token::Match(tok->previous(), ".|::") && !library.isFunctionConst(tok->str(), pure)) return false; - else if (tok->function() && !tok->function()->isConst()) + else if (tok->function() && !tok->function()->isConst() && !tok->function()->isPure()) return false; } if (tok->tokType() == Token::eIncDecOp) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 47fce31abda..17ca6c48f35 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2186,10 +2186,16 @@ struct ValueFlowAnalyzer : Analyzer { if (dependsOnThis() && exprDependsOnThis(tok)) return isThisModified(tok); - // TODO: Check if function is pure - if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { - // bailout: global non-const variables - if (isGlobal()) { + // bailout: global non-const variables + if (isGlobal() && Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { + // TODO: Check for constexpr functions + if (tok->function()) { + if (!tok->function()->isConst() && !tok->function()->isPure()) + return Action::Invalid; + } else if (const Library::Function* f = getSettings()->library.getFunction(tok)) { + if (!f->ispure && !f->isconst) + return Action::Invalid; + } else { return Action::Invalid; } } From 0abf67a508f69e8c978a7144c506478764b65364 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 14 Jul 2021 11:22:25 -0500 Subject: [PATCH 08/33] Dont set values for literals --- lib/valueflow.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 17ca6c48f35..29b2b489af1 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4411,6 +4411,8 @@ struct ConditionHandler { Condition cond = parse(tok, settings); if (!cond.vartok) continue; + if (cond.vartok->hasKnownIntValue()) + continue; if (cond.true_values.empty() || cond.false_values.empty()) continue; if (!isConstExpression(cond.vartok, settings->library, true, tokenlist->isCPP())) From 4985604869f1be15782c54a0baf7c92992f2287f Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 14 Jul 2021 11:28:12 -0500 Subject: [PATCH 09/33] Assume library function dont modify user-global variables --- lib/valueflow.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 29b2b489af1..63c5a311561 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2192,9 +2192,9 @@ struct ValueFlowAnalyzer : Analyzer { if (tok->function()) { if (!tok->function()->isConst() && !tok->function()->isPure()) return Action::Invalid; - } else if (const Library::Function* f = getSettings()->library.getFunction(tok)) { - if (!f->ispure && !f->isconst) - return Action::Invalid; + } else if (getSettings()->library.getFunction(tok)) { + // Assume library functions doesn't modify user-global variables + return Action::None; } else { return Action::Invalid; } From 8d6b7d99386bf38afa5727be6535765bbd500334 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 14 Jul 2021 18:45:52 -0500 Subject: [PATCH 10/33] Switch to use container expression --- lib/analyzer.h | 6 ++++ lib/valueflow.cpp | 71 +++++++++++++++++++++-------------------------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index 8d0f5be7d47..46e1a24f52c 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -119,6 +119,12 @@ struct Analyzer { {} Action action; Terminate terminate; + + void update(Result rhs) { + if (terminate == Terminate::None) + terminate = rhs.terminate; + action |= rhs.action; + } }; enum class Direction { Forward, Reverse }; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 63c5a311561..5df4ec36cf2 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -99,6 +99,7 @@ #include #include +#include #include #include #include @@ -6081,18 +6082,17 @@ static bool isContainerSizeChangedByFunction(const Token *tok, int depth = 20) return (isChanged || inconclusive); } -struct ContainerVariableAnalyzer : VariableAnalyzer { - ContainerVariableAnalyzer() : VariableAnalyzer() {} +struct ContainerExpressionAnalyzer : ExpressionAnalyzer { + ContainerExpressionAnalyzer() : ExpressionAnalyzer() {} - ContainerVariableAnalyzer(const Variable* v, + ContainerExpressionAnalyzer(const Token* expr, const ValueFlow::Value& val, - std::vector paliases, const TokenList* t) - : VariableAnalyzer(v, val, std::move(paliases), t) + : ExpressionAnalyzer(expr, val, t) {} virtual bool match(const Token* tok) const OVERRIDE { - return tok->varId() == var->declarationId() || (astIsIterator(tok) && isAliasOf(tok, var->declarationId())); + return tok->exprId() == expr->exprId() || (astIsIterator(tok) && isAliasOf(tok, expr->exprId())); } virtual Action isWritable(const Token* tok, Direction d) const OVERRIDE { @@ -6173,26 +6173,26 @@ struct ContainerVariableAnalyzer : VariableAnalyzer { } }; -static Analyzer::Result valueFlowContainerForward(Token* tok, +static Analyzer::Result valueFlowContainerForward(Token* startToken, const Token* endToken, - const Variable* var, - ValueFlow::Value value, + const Token* exprTok, + const ValueFlow::Value& value, TokenList* tokenlist) { - ContainerVariableAnalyzer a(var, value, getAliasesFromValues({value}), tokenlist); - return valueFlowGenericForward(tok, endToken, a, tokenlist->getSettings()); + ContainerExpressionAnalyzer a(exprTok, value, tokenlist); + return valueFlowGenericForward(startToken, endToken, a, tokenlist->getSettings()); } -static Analyzer::Result valueFlowContainerForward(Token* tok, - const Variable* var, - ValueFlow::Value value, + +static Analyzer::Result valueFlowContainerForward(Token* startToken, + const Token* exprTok, + const ValueFlow::Value& value, TokenList* tokenlist) { - const Token * endOfVarScope = nullptr; - if (var->isLocal() || var->isArgument()) - endOfVarScope = var->scope()->bodyEnd; - if (!endOfVarScope) - endOfVarScope = tok->scope()->bodyEnd; - return valueFlowContainerForward(tok, endOfVarScope, var, std::move(value), tokenlist); + const Token* endToken = nullptr; + const Function* f = Scope::nestedInFunction(startToken->scope()); + if (f && f->functionScope) + endToken = f->functionScope->bodyEnd; + return valueFlowContainerForward(startToken, endToken, exprTok, value, tokenlist); } static void valueFlowContainerReverse(Token* tok, @@ -6202,10 +6202,8 @@ static void valueFlowContainerReverse(Token* tok, TokenList* tokenlist, const Settings* settings) { - const Variable* var = varToken->variable(); - auto aliases = getAliasesFromValues(values); for (const ValueFlow::Value& value : values) { - ContainerVariableAnalyzer a(var, value, aliases, tokenlist); + ContainerExpressionAnalyzer a(varToken, value, tokenlist); valueFlowGenericReverse(tok, endToken, a, settings); } } @@ -6508,7 +6506,7 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold values = getInitListSize(initList, var->valueType()->container, known); } for (const ValueFlow::Value& value : values) - valueFlowContainerForward(var->nameToken()->next(), var, value, tokenlist); + valueFlowContainerForward(var->nameToken()->next(), var->nameToken(), value, tokenlist); } // after assignment @@ -6525,14 +6523,14 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold ValueFlow::Value value(Token::getStrLength(containerTok->tokAt(2))); value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; value.setKnown(); - valueFlowContainerForward(containerTok->next(), containerTok->variable(), value, tokenlist); + valueFlowContainerForward(containerTok->next(), containerTok, value, tokenlist); } } else if (Token::Match(tok, "%name%|;|{|}|> %var% = {") && Token::simpleMatch(tok->linkAt(3), "} ;")) { const Token* containerTok = tok->next(); if (astIsContainer(containerTok) && containerTok->valueType()->container->size_templateArgNo < 0) { std::vector values = getInitListSize(tok->tokAt(3), containerTok->valueType()->container); for (const ValueFlow::Value& value : values) - valueFlowContainerForward(containerTok->next(), containerTok->variable(), value, tokenlist); + valueFlowContainerForward(containerTok->next(), containerTok, value, tokenlist); } } else if (Token::Match(tok, "%var% . %name% (") && tok->valueType() && tok->valueType()->container) { Library::Container::Action action = tok->valueType()->container->getAction(tok->strAt(2)); @@ -6540,13 +6538,13 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold ValueFlow::Value value(0); value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; value.setKnown(); - valueFlowContainerForward(tok->next(), tok->variable(), value, tokenlist); + valueFlowContainerForward(tok->next(), tok, value, tokenlist); } else if (action == Library::Container::Action::RESIZE && tok->tokAt(3)->astOperand2() && tok->tokAt(3)->astOperand2()->hasKnownIntValue()) { ValueFlow::Value value(tok->tokAt(3)->astOperand2()->values().front()); value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; value.setKnown(); - valueFlowContainerForward(tok->next(), tok->variable(), value, tokenlist); + valueFlowContainerForward(tok->next(), tok, value, tokenlist); } } } @@ -6604,13 +6602,10 @@ struct ContainerConditionHandler : ConditionHandler { const std::list& values, TokenList* tokenlist, const Settings*) const OVERRIDE { - // TODO: Forward multiple values - if (values.empty()) - return {}; - const Variable* var = exprTok->variable(); - if (!var) - return {}; - return valueFlowContainerForward(start->next(), stop, var, values.front(), tokenlist); + Analyzer::Result result{}; + for(const ValueFlow::Value& value: values) + result.update(valueFlowContainerForward(start->next(), stop, exprTok, value, tokenlist)); + return result; } virtual void reverse(Token* start, @@ -6619,10 +6614,6 @@ struct ContainerConditionHandler : ConditionHandler { const std::list& values, TokenList* tokenlist, const Settings* settings) const OVERRIDE { - if (values.empty()) - return; - if (!exprTok->variable()) - return; return valueFlowContainerReverse(start, endTok, exprTok, values, tokenlist, settings); } @@ -6856,7 +6847,7 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming " + arg.name() + " size is 1000000"); argValues.back().safe = true; for (const ValueFlow::Value &value : argValues) - valueFlowContainerForward(const_cast(functionScope->bodyStart), &arg, value, tokenlist); + valueFlowContainerForward(const_cast(functionScope->bodyStart), arg.nameToken(), value, tokenlist); continue; } From 144cff1884e0c8bfb9d247d402f7064afaa6ed04 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 18:11:52 -0500 Subject: [PATCH 11/33] Fix check for single boolean expression --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 5df4ec36cf2..4c88702355a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4578,7 +4578,7 @@ struct ConditionHandler { elseValues.insert(elseValues.end(), cond.false_values.begin(), cond.false_values.end()); if (isConditionKnown(tok, true)) { insertImpossible(thenValues, cond.true_values); - if (Token::Match(tok, "(|.|%var%") && astIsBool(tok)) + if (tok == cond.vartok && astIsBool(tok)) insertNegateKnown(thenValues, cond.true_values); } } From 425d7c71fe550b0dd70d9740392d69cb9fbfbcb3 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 18:45:55 -0500 Subject: [PATCH 12/33] Update the tests to check for multiple values --- test/testvalueflow.cpp | 44 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index ec8328ca610..5cd9e6ea613 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -152,6 +152,22 @@ class TestValueFlow : public TestFixture { return !val.isLifetimeValue(); } + static bool isNotPossible(const ValueFlow::Value& val) { + return !val.isPossible(); + } + + static bool isNotKnown(const ValueFlow::Value& val) { + return !val.isKnown(); + } + + static bool isNotInconclusive(const ValueFlow::Value& val) { + return !val.isInconclusive(); + } + + static bool isNotImpossible(const ValueFlow::Value& val) { + return !val.isImpossible(); + } + bool testValueOfXKnown(const char code[], unsigned int linenr, int value) { // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -4631,7 +4647,9 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, 0)); } - static std::string isPossibleContainerSizeValue(const std::list &values, MathLib::bigint i) { + static std::string isPossibleContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + if (!unique) + values.remove_if(&isNotPossible); if (values.size() != 1) return "values.size():" + std::to_string(values.size()); if (!values.front().isContainerSizeValue()) @@ -4643,7 +4661,9 @@ class TestValueFlow : public TestFixture { return ""; } - static std::string isImpossibleContainerSizeValue(const std::list& values, MathLib::bigint i) { + static std::string isImpossibleContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + if (!unique) + values.remove_if(&isNotImpossible); if (values.size() != 1) return "values.size():" + std::to_string(values.size()); if (!values.front().isContainerSizeValue()) @@ -4655,7 +4675,9 @@ class TestValueFlow : public TestFixture { return ""; } - static std::string isInconclusiveContainerSizeValue(const std::list& values, MathLib::bigint i) { + static std::string isInconclusiveContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + if (!unique) + values.remove_if(&isNotInconclusive); if (values.size() != 1) return "values.size():" + std::to_string(values.size()); if (!values.front().isContainerSizeValue()) @@ -4667,7 +4689,9 @@ class TestValueFlow : public TestFixture { return ""; } - static std::string isKnownContainerSizeValue(const std::list &values, MathLib::bigint i) { + static std::string isKnownContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + if (!unique) + values.remove_if(&isNotKnown); if (values.size() != 1) return "values.size():" + std::to_string(values.size()); if (!values.front().isContainerSizeValue()) @@ -4790,28 +4814,32 @@ class TestValueFlow : public TestFixture { " ints.front();\n" // <- container size is 3 " }\n" "}"; - ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 3)); + ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 3, false)); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "ints . front"), 4, false)); code = "void f(const std::list &ints) {\n" " if (ints.size() >= 3) {\n" " ints.front();\n" // <- container size is 3 " }\n" "}"; - ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 3)); + ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 3, false)); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "ints . front"), 2, false)); code = "void f(const std::list &ints) {\n" " if (ints.size() < 3) {\n" " ints.front();\n" // <- container size is 2 " }\n" "}"; - ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 2)); + ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 2, false)); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "ints . front"), 3, false)); code = "void f(const std::list &ints) {\n" " if (ints.size() > 3) {\n" " ints.front();\n" // <- container size is 4 " }\n" "}"; - ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 4)); + ASSERT_EQUALS("", isPossibleContainerSizeValue(tokenValues(code, "ints . front"), 4, false)); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "ints . front"), 3, false)); code = "void f(const std::list &ints) {\n" " if (ints.empty() == false) {\n" From d9480154ffce6164647c3fdfd4cc84556ba26f3a Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 19:21:58 -0500 Subject: [PATCH 13/33] Remove variable analyzer --- cmake/compileroptions.cmake | 1 - lib/valueflow.cpp | 206 +++++------------------------------- 2 files changed, 26 insertions(+), 181 deletions(-) diff --git a/cmake/compileroptions.cmake b/cmake/compileroptions.cmake index b3410a7bdc4..a95df1050b2 100644 --- a/cmake/compileroptions.cmake +++ b/cmake/compileroptions.cmake @@ -26,7 +26,6 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" add_compile_options(-Wall) add_compile_options(-Wextra) add_compile_options(-Wcast-qual) # Cast for removing type qualifiers - add_compile_options(-Wno-deprecated-declarations) add_compile_options(-Wfloat-equal) # Floating values used in equality comparisons add_compile_options(-Wmissing-declarations) # If a global function is defined without a previous declaration add_compile_options(-Wmissing-format-attribute) # diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 4c88702355a..6b555c5ac71 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2388,64 +2388,6 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer { } }; -struct VariableAnalyzer : SingleValueFlowAnalyzer { - const Variable* var; - - VariableAnalyzer() : SingleValueFlowAnalyzer(), var(nullptr) {} - - VariableAnalyzer(const Variable* v, - const ValueFlow::Value& val, - std::vector paliases, - const TokenList* t) - : SingleValueFlowAnalyzer(val, t), var(v) { - varids[var->declarationId()] = var; - for (const Variable* av:paliases) { - if (!av) - continue; - aliases[av->declarationId()] = av; - } - } - - virtual const ValueType* getValueType(const Token*) const OVERRIDE { - return var->valueType(); - } - - virtual bool match(const Token* tok) const OVERRIDE { - return tok->varId() == var->declarationId(); - } - - virtual ProgramState getProgramState() const OVERRIDE { - ProgramState ps; - ps[var->declarationId()] = value; - return ps; - } -}; - -static std::vector getAliasesFromValues(std::list values, bool address=false) -{ - std::vector aliases; - for (const ValueFlow::Value& v : values) { - if (!v.tokvalue) - continue; - const Token* lifeTok = nullptr; - for (const ValueFlow::Value& lv:v.tokvalue->values()) { - if (!lv.isLocalLifetimeValue()) - continue; - if (address && lv.lifetimeKind != ValueFlow::Value::LifetimeKind::Address) - continue; - if (lifeTok) { - lifeTok = nullptr; - break; - } - lifeTok = lv.tokvalue; - } - if (lifeTok && lifeTok->variable()) { - aliases.push_back(lifeTok->variable()); - } - } - return aliases; -} - static Analyzer::Result valueFlowForwardVariable(Token* const startToken, const Token* const endToken, const Variable* const var, @@ -2471,23 +2413,6 @@ static Analyzer::Result valueFlowForwardVariable(Token* const startToken, startToken, endToken, var, std::move(values), tokenlist, settings); } - -// Old deprecated version -static bool valueFlowForwardVariable(Token* const startToken, - const Token* const endToken, - const Variable* const var, - const nonneg int, - std::list values, - const bool, - const bool, - TokenList* const tokenlist, - ErrorLogger* const, - const Settings* const settings) -{ - valueFlowForwardVariable(startToken, endToken, var, std::move(values), tokenlist, settings); - return true; -} - struct ExpressionAnalyzer : SingleValueFlowAnalyzer { const Token* expr; bool local; @@ -3177,22 +3102,12 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog } } for (const Variable* var : vars) { - valueFlowForwardVariable(const_cast(nextExpression), - endOfVarScope, - var, - var->declarationId(), - values, - false, - false, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(nextExpression), endOfVarScope, var->nameToken(), values, tokenlist, settings); if (tok->astTop() && Token::simpleMatch(tok->astTop()->previous(), "for (") && Token::simpleMatch(tok->astTop()->link(), ") {")) { Token* start = tok->astTop()->link()->next(); - valueFlowForwardVariable( - start, start->link(), var, var->declarationId(), values, false, false, tokenlist, errorLogger, settings); + valueFlowForward(start, start->link(), var->nameToken(), values, tokenlist, settings); } } // Constructor @@ -3212,16 +3127,7 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog const Token *nextExpression = nextAfterAstRightmostLeaf(parent); // Only forward lifetime values values.remove_if(&isNotLifetimeValue); - valueFlowForwardVariable(const_cast(nextExpression), - endOfVarScope, - var, - var->declarationId(), - values, - false, - false, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(nextExpression), endOfVarScope, tok, values, tokenlist, settings); // Cast } else if (parent->isCast()) { std::list values = tok->values(); @@ -3920,7 +3826,7 @@ static const Token * findEndOfFunctionCallForParameter(const Token * parameterTo return nextAfterAstRightmostLeaf(parent); } -static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatabase, const Settings *settings) { if (!tokenlist->isCPP() || settings->standards.cpp < Standards::CPP11) return; @@ -3949,11 +3855,9 @@ static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatab const Variable *var = varTok->variable(); if (!var || (!var->isLocal() && !var->isArgument())) continue; - const nonneg int varId = varTok->varId(); const Token * const endOfVarScope = var->scope()->bodyEnd; setTokenValue(varTok, value, settings); - valueFlowForwardVariable( - varTok->next(), endOfVarScope, var, varId, values, false, false, tokenlist, errorLogger, settings); + valueFlowForward(varTok->next(), endOfVarScope, varTok, values, tokenlist, settings); continue; } ValueFlow::Value::MoveKind moveKind; @@ -3989,16 +3893,7 @@ static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatab const Token * openParentesisOfMove = findOpenParentesisOfMove(varTok); const Token * endOfFunctionCall = findEndOfFunctionCallForParameter(openParentesisOfMove); if (endOfFunctionCall) - valueFlowForwardVariable(const_cast(endOfFunctionCall), - endOfVarScope, - var, - varId, - values, - false, - false, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(endOfFunctionCall), endOfVarScope, varTok, values, tokenlist, settings); } } } @@ -5170,7 +5065,7 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const nonneg int v } } -static void valueFlowForLoopSimplifyAfter(Token *fortok, nonneg int varid, const MathLib::bigint num, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowForLoopSimplifyAfter(Token *fortok, nonneg int varid, const MathLib::bigint num, TokenList *tokenlist, const Settings *settings) { const Token *vartok = nullptr; for (const Token *tok = fortok; tok; tok = tok->next()) { @@ -5195,8 +5090,7 @@ static void valueFlowForLoopSimplifyAfter(Token *fortok, nonneg int varid, const values.back().errorPath.emplace_back(fortok,"After for loop, " + var->name() + " has value " + values.back().infoString()); if (blockTok != endToken) { - valueFlowForwardVariable( - blockTok->next(), endToken, var, varid, values, false, false, tokenlist, errorLogger, settings); + valueFlowForward(blockTok->next(), endToken, vartok, values, tokenlist, settings); } } @@ -5225,7 +5119,7 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas valueFlowForLoopSimplify(bodyStart, varid, false, lastValue, tokenlist, errorLogger, settings); } const MathLib::bigint afterValue = executeBody ? lastValue + stepValue : initValue; - valueFlowForLoopSimplifyAfter(tok, varid, afterValue, tokenlist, errorLogger, settings); + valueFlowForLoopSimplifyAfter(tok, varid, afterValue, tokenlist, settings); } else { ProgramMemory mem1, mem2, memAfter; if (valueFlowForLoop2(tok, &mem1, &mem2, &memAfter)) { @@ -5243,7 +5137,7 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas for (it = memAfter.values.begin(); it != memAfter.values.end(); ++it) { if (!it->second.isIntValue()) continue; - valueFlowForLoopSimplifyAfter(tok, it->first, it->second.intvalue, tokenlist, errorLogger, settings); + valueFlowForLoopSimplifyAfter(tok, it->first, it->second.intvalue, tokenlist, settings); } } } @@ -5474,7 +5368,7 @@ static void valueFlowInjectParameter(TokenList* tokenlist, SymbolDatabase* symbo } } -static void valueFlowInjectParameter(TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings, const Variable* arg, const Scope* functionScope, const std::list& argvalues) +static void valueFlowInjectParameter(TokenList* tokenlist, const Settings* settings, const Variable* arg, const Scope* functionScope, const std::list& argvalues) { // Is argument passed by value or const reference, and is it a known non-class type? if (arg->isReference() && !arg->isConst() && !arg->isClass()) @@ -5485,16 +5379,7 @@ static void valueFlowInjectParameter(TokenList* tokenlist, ErrorLogger* errorLog if (!varid2) return; - valueFlowForwardVariable(const_cast(functionScope->bodyStart->next()), - functionScope->bodyEnd, - arg, - varid2, - argvalues, - false, - true, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, arg->nameToken(), argvalues, tokenlist, settings); } static void valueFlowSwitchVariable(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) @@ -5853,7 +5738,7 @@ static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldat } } -static void valueFlowFunctionDefaultParameter(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowFunctionDefaultParameter(TokenList *tokenlist, SymbolDatabase* symboldatabase, const Settings *settings) { if (!tokenlist->isCPP()) return; @@ -5875,7 +5760,7 @@ static void valueFlowFunctionDefaultParameter(TokenList *tokenlist, SymbolDataba argvalues.push_back(v); } if (!argvalues.empty()) - valueFlowInjectParameter(tokenlist, errorLogger, settings, var, scope, argvalues); + valueFlowInjectParameter(tokenlist, settings, var, scope, argvalues); } } } @@ -5954,7 +5839,7 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg } } -static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatabase*/, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatabase*/, const Settings *settings) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { if (!Token::Match(tok,"[;{}] %type%")) @@ -5989,19 +5874,7 @@ static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatab std::list values; values.push_back(uninitValue); - const bool constValue = true; - const bool subFunction = false; - - valueFlowForwardVariable(vardecl->next(), - vardecl->scope()->bodyEnd, - var, - vardecl->varId(), - values, - constValue, - subFunction, - tokenlist, - errorLogger, - settings); + valueFlowForward(vardecl->next(), vardecl->scope()->bodyEnd, var->nameToken(), values, tokenlist, settings); } } @@ -6679,7 +6552,7 @@ struct ContainerConditionHandler : ConditionHandler { } }; -static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *symboldatabase, const Settings *settings) { for (const Scope *functionScope : symboldatabase->functionScopes) { for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) { @@ -6737,16 +6610,7 @@ static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *sym value.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE; value.setKnown(); const std::list values{value}; - valueFlowForwardVariable(const_cast(rhs), - functionScope->bodyEnd, - tok->next()->variable(), - tok->next()->varId(), - values, - true, - false, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(rhs), functionScope->bodyEnd, tok->next(), values, tokenlist, settings); } } } @@ -6818,7 +6682,7 @@ static bool getMinMaxValues(const std::string &typestr, const Settings *settings return getMinMaxValues(&vt, *settings, minvalue, maxvalue); } -static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symboldatabase, const Settings *settings) { for (const Scope *functionScope : symboldatabase->functionScopes) { if (!functionScope->bodyStart) @@ -6881,16 +6745,7 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold argValues.back().floatValue = isHigh ? high : 1E25f; argValues.back().errorPath.emplace_back(arg.nameToken(), "Safe checks: Assuming argument has value " + MathLib::toString(argValues.back().floatValue)); argValues.back().safe = true; - valueFlowForwardVariable(const_cast(functionScope->bodyStart->next()), - functionScope->bodyEnd, - &arg, - arg.declarationId(), - argValues, - false, - false, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, arg.nameToken(), argValues, tokenlist, settings); continue; } } @@ -6908,16 +6763,7 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold } if (!argValues.empty()) - valueFlowForwardVariable(const_cast(functionScope->bodyStart->next()), - functionScope->bodyEnd, - &arg, - arg.declarationId(), - argValues, - false, - false, - tokenlist, - errorLogger, - settings); + valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, arg.nameToken(), argValues, tokenlist, settings); } } } @@ -7081,7 +6927,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowImpossibleValues(tokenlist, settings); valueFlowArrayBool(tokenlist); valueFlowRightShift(tokenlist, settings); - valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings); + valueFlowAfterMove(tokenlist, symboldatabase, settings); valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings); valueFlowInferCondition(tokenlist, settings); valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings); @@ -7090,8 +6936,8 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowSubFunction(tokenlist, symboldatabase, errorLogger, settings); valueFlowFunctionReturn(tokenlist, errorLogger); valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings); - valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings); - valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings); + valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, settings); + valueFlowUninit(tokenlist, symboldatabase, settings); valueFlowUninitPointerAliasDeref(tokenlist); if (tokenlist->isCPP()) { valueFlowSmartPointer(tokenlist, errorLogger, settings); @@ -7101,11 +6947,11 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings); valueFlowCondition(ContainerConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings); } - valueFlowSafeFunctions(tokenlist, symboldatabase, errorLogger, settings); + valueFlowSafeFunctions(tokenlist, symboldatabase, settings); n--; } - valueFlowDynamicBufferSize(tokenlist, symboldatabase, errorLogger, settings); + valueFlowDynamicBufferSize(tokenlist, symboldatabase, settings); } From 56521b173c16c1fc20ed1ac2a1ceb7eaa55bbce6 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 19:25:10 -0500 Subject: [PATCH 14/33] Remove unused functions --- lib/valueflow.cpp | 34 +--------------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 6b555c5ac71..19bc20c4344 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -67,7 +67,7 @@ * =============================== * * In forward value flow analysis we know a value and see what happens when we are stepping the program forward. Like - * normal execution. The valueFlowForwardVariable is used in this analysis. + * normal execution. The valueFlowForward is used in this analysis. * * In reverse value flow analysis we know the value of a variable at line X. And try to "execute backwards" to determine * possible values before line X. The valueFlowReverse is used in this analysis. @@ -1685,13 +1685,6 @@ static Analyzer::Result valueFlowForward(Token* startToken, TokenList* const tokenlist, const Settings* settings); -static Analyzer::Result valueFlowForwardVariable(Token* const startToken, - const Token* const endToken, - const Variable* const var, - std::list values, - TokenList* const tokenlist, - const Settings* const settings); - static void valueFlowReverse(TokenList* tokenlist, Token* tok, const Token* const varToken, @@ -2388,31 +2381,6 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer { } }; -static Analyzer::Result valueFlowForwardVariable(Token* const startToken, - const Token* const endToken, - const Variable* const var, - std::list values, - TokenList* const tokenlist, - const Settings* const settings) -{ - const Token * exprTok = Token::findmatch(startToken, "%varid%", var->declarationId()); - if (!exprTok) - exprTok = var->nameToken(); - return valueFlowForward(startToken, endToken, exprTok, std::move(values), tokenlist, settings); -} - -static Analyzer::Result valueFlowForwardVariable(Token* const startToken, - const Token* const endToken, - const Variable* const var, - std::list values, - std::vector, - TokenList* const tokenlist, - const Settings* const settings) -{ - return valueFlowForwardVariable( - startToken, endToken, var, std::move(values), tokenlist, settings); -} - struct ExpressionAnalyzer : SingleValueFlowAnalyzer { const Token* expr; bool local; From 2c521002cd2aecc80d21380b3473f2bb0a097a35 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 19:36:18 -0500 Subject: [PATCH 15/33] Format --- lib/analyzer.h | 3 +- lib/valueflow.cpp | 105 ++++++++++++++++++++++++----------------- test/testvalueflow.cpp | 34 ++++++------- 3 files changed, 82 insertions(+), 60 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index 46e1a24f52c..063649553b7 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -120,7 +120,8 @@ struct Analyzer { Action action; Terminate terminate; - void update(Result rhs) { + void update(Result rhs) + { if (terminate == Terminate::None) terminate = rhs.terminate; action |= rhs.action; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 19bc20c4344..ca302dd01d1 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1679,11 +1679,11 @@ static void valueFlowGlobalStaticVar(TokenList *tokenList, const Settings *setti } static Analyzer::Result valueFlowForward(Token* startToken, - const Token* endToken, - const Token* exprTok, - std::list values, - TokenList* const tokenlist, - const Settings* settings); + const Token* endToken, + const Token* exprTok, + std::list values, + TokenList* const tokenlist, + const Settings* settings); static void valueFlowReverse(TokenList* tokenlist, Token* tok, @@ -1974,9 +1974,7 @@ struct ValueFlowAnalyzer : Analyzer { virtual bool isGlobal() const { return false; } - virtual bool dependsOnThis() const { - return false; - } + virtual bool dependsOnThis() const { return false; } virtual bool invalid() const { return false; @@ -2026,7 +2024,8 @@ struct ValueFlowAnalyzer : Analyzer { return Action::None; } - virtual Action isThisModified(const Token* tok) const { + virtual Action isThisModified(const Token* tok) const + { if (isThisChanged(tok, 0, getSettings(), isCPP())) return Action::Invalid; return Action::None; @@ -2390,7 +2389,8 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { ExpressionAnalyzer() : SingleValueFlowAnalyzer(), expr(nullptr), local(true), unknown(false), dependOnThis(false) {} ExpressionAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t) - : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false), dependOnThis(false) { + : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false), dependOnThis(false) + { dependOnThis = exprDependsOnThis(expr); setupExprVarIds(expr); @@ -2405,7 +2405,8 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { var->isStatic() || var->isReference() || var->isExtern(); } - void setupExprVarIds(const Token* start, int depth = 0) { + void setupExprVarIds(const Token* start, int depth = 0) + { const int maxDepth = 4; if (depth > maxDepth) return; @@ -2451,13 +2452,9 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { return ps; } - virtual bool match(const Token* tok) const OVERRIDE { - return tok->exprId() == expr->exprId(); - } + virtual bool match(const Token* tok) const OVERRIDE { return tok->exprId() == expr->exprId(); } - virtual bool dependsOnThis() const OVERRIDE { - return dependOnThis; - } + virtual bool dependsOnThis() const OVERRIDE { return dependOnThis; } virtual bool isGlobal() const OVERRIDE { return !local; @@ -2576,7 +2573,6 @@ static Analyzer::Result valueFlowForward(Token* startToken, return valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings); } - static void valueFlowReverse(Token* tok, const Token* const endToken, const Token* const varToken, @@ -2590,7 +2586,6 @@ static void valueFlowReverse(Token* tok, } } - static void valueFlowReverse(TokenList* tokenlist, Token* tok, const Token* const varToken, @@ -3070,7 +3065,8 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog } } for (const Variable* var : vars) { - valueFlowForward(const_cast(nextExpression), endOfVarScope, var->nameToken(), values, tokenlist, settings); + valueFlowForward( + const_cast(nextExpression), endOfVarScope, var->nameToken(), values, tokenlist, settings); if (tok->astTop() && Token::simpleMatch(tok->astTop()->previous(), "for (") && Token::simpleMatch(tok->astTop()->link(), ") {")) { @@ -3794,7 +3790,7 @@ static const Token * findEndOfFunctionCallForParameter(const Token * parameterTo return nextAfterAstRightmostLeaf(parent); } -static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatabase, const Settings *settings) +static void valueFlowAfterMove(TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings) { if (!tokenlist->isCPP() || settings->standards.cpp < Standards::CPP11) return; @@ -3861,7 +3857,8 @@ static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatab const Token * openParentesisOfMove = findOpenParentesisOfMove(varTok); const Token * endOfFunctionCall = findEndOfFunctionCallForParameter(openParentesisOfMove); if (endOfFunctionCall) - valueFlowForward(const_cast(endOfFunctionCall), endOfVarScope, varTok, values, tokenlist, settings); + valueFlowForward( + const_cast(endOfFunctionCall), endOfVarScope, varTok, values, tokenlist, settings); } } } @@ -5033,7 +5030,11 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const nonneg int v } } -static void valueFlowForLoopSimplifyAfter(Token *fortok, nonneg int varid, const MathLib::bigint num, TokenList *tokenlist, const Settings *settings) +static void valueFlowForLoopSimplifyAfter(Token* fortok, + nonneg int varid, + const MathLib::bigint num, + TokenList* tokenlist, + const Settings* settings) { const Token *vartok = nullptr; for (const Token *tok = fortok; tok; tok = tok->next()) { @@ -5336,7 +5337,11 @@ static void valueFlowInjectParameter(TokenList* tokenlist, SymbolDatabase* symbo } } -static void valueFlowInjectParameter(TokenList* tokenlist, const Settings* settings, const Variable* arg, const Scope* functionScope, const std::list& argvalues) +static void valueFlowInjectParameter(TokenList* tokenlist, + const Settings* settings, + const Variable* arg, + const Scope* functionScope, + const std::list& argvalues) { // Is argument passed by value or const reference, and is it a known non-class type? if (arg->isReference() && !arg->isConst() && !arg->isClass()) @@ -5347,7 +5352,12 @@ static void valueFlowInjectParameter(TokenList* tokenlist, const Settings* setti if (!varid2) return; - valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, arg->nameToken(), argvalues, tokenlist, settings); + valueFlowForward(const_cast(functionScope->bodyStart->next()), + functionScope->bodyEnd, + arg->nameToken(), + argvalues, + tokenlist, + settings); } static void valueFlowSwitchVariable(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) @@ -5706,7 +5716,7 @@ static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldat } } -static void valueFlowFunctionDefaultParameter(TokenList *tokenlist, SymbolDatabase* symboldatabase, const Settings *settings) +static void valueFlowFunctionDefaultParameter(TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings) { if (!tokenlist->isCPP()) return; @@ -5807,7 +5817,7 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg } } -static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatabase*/, const Settings *settings) +static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDatabase*/, const Settings* settings) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { if (!Token::Match(tok,"[;{}] %type%")) @@ -5926,9 +5936,7 @@ static bool isContainerSizeChangedByFunction(const Token *tok, int depth = 20) struct ContainerExpressionAnalyzer : ExpressionAnalyzer { ContainerExpressionAnalyzer() : ExpressionAnalyzer() {} - ContainerExpressionAnalyzer(const Token* expr, - const ValueFlow::Value& val, - const TokenList* t) + ContainerExpressionAnalyzer(const Token* expr, const ValueFlow::Value& val, const TokenList* t) : ExpressionAnalyzer(expr, val, t) {} @@ -6015,19 +6023,19 @@ struct ContainerExpressionAnalyzer : ExpressionAnalyzer { }; static Analyzer::Result valueFlowContainerForward(Token* startToken, - const Token* endToken, - const Token* exprTok, - const ValueFlow::Value& value, - TokenList* tokenlist) + const Token* endToken, + const Token* exprTok, + const ValueFlow::Value& value, + TokenList* tokenlist) { ContainerExpressionAnalyzer a(exprTok, value, tokenlist); return valueFlowGenericForward(startToken, endToken, a, tokenlist->getSettings()); } static Analyzer::Result valueFlowContainerForward(Token* startToken, - const Token* exprTok, - const ValueFlow::Value& value, - TokenList* tokenlist) + const Token* exprTok, + const ValueFlow::Value& value, + TokenList* tokenlist) { const Token* endToken = nullptr; const Function* f = Scope::nestedInFunction(startToken->scope()); @@ -6444,7 +6452,7 @@ struct ContainerConditionHandler : ConditionHandler { TokenList* tokenlist, const Settings*) const OVERRIDE { Analyzer::Result result{}; - for(const ValueFlow::Value& value: values) + for (const ValueFlow::Value& value : values) result.update(valueFlowContainerForward(start->next(), stop, exprTok, value, tokenlist)); return result; } @@ -6520,7 +6528,7 @@ struct ContainerConditionHandler : ConditionHandler { } }; -static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *symboldatabase, const Settings *settings) +static void valueFlowDynamicBufferSize(TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings) { for (const Scope *functionScope : symboldatabase->functionScopes) { for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) { @@ -6650,7 +6658,7 @@ static bool getMinMaxValues(const std::string &typestr, const Settings *settings return getMinMaxValues(&vt, *settings, minvalue, maxvalue); } -static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symboldatabase, const Settings *settings) +static void valueFlowSafeFunctions(TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings) { for (const Scope *functionScope : symboldatabase->functionScopes) { if (!functionScope->bodyStart) @@ -6679,7 +6687,8 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming " + arg.name() + " size is 1000000"); argValues.back().safe = true; for (const ValueFlow::Value &value : argValues) - valueFlowContainerForward(const_cast(functionScope->bodyStart), arg.nameToken(), value, tokenlist); + valueFlowContainerForward( + const_cast(functionScope->bodyStart), arg.nameToken(), value, tokenlist); continue; } @@ -6713,7 +6722,12 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold argValues.back().floatValue = isHigh ? high : 1E25f; argValues.back().errorPath.emplace_back(arg.nameToken(), "Safe checks: Assuming argument has value " + MathLib::toString(argValues.back().floatValue)); argValues.back().safe = true; - valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, arg.nameToken(), argValues, tokenlist, settings); + valueFlowForward(const_cast(functionScope->bodyStart->next()), + functionScope->bodyEnd, + arg.nameToken(), + argValues, + tokenlist, + settings); continue; } } @@ -6731,7 +6745,12 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold } if (!argValues.empty()) - valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, arg.nameToken(), argValues, tokenlist, settings); + valueFlowForward(const_cast(functionScope->bodyStart->next()), + functionScope->bodyEnd, + arg.nameToken(), + argValues, + tokenlist, + settings); } } } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 5cd9e6ea613..b2fa6ef1c26 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -152,21 +152,13 @@ class TestValueFlow : public TestFixture { return !val.isLifetimeValue(); } - static bool isNotPossible(const ValueFlow::Value& val) { - return !val.isPossible(); - } + static bool isNotPossible(const ValueFlow::Value& val) { return !val.isPossible(); } - static bool isNotKnown(const ValueFlow::Value& val) { - return !val.isKnown(); - } + static bool isNotKnown(const ValueFlow::Value& val) { return !val.isKnown(); } - static bool isNotInconclusive(const ValueFlow::Value& val) { - return !val.isInconclusive(); - } + static bool isNotInconclusive(const ValueFlow::Value& val) { return !val.isInconclusive(); } - static bool isNotImpossible(const ValueFlow::Value& val) { - return !val.isImpossible(); - } + static bool isNotImpossible(const ValueFlow::Value& val) { return !val.isImpossible(); } bool testValueOfXKnown(const char code[], unsigned int linenr, int value) { // Tokenize.. @@ -4647,7 +4639,10 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, 0)); } - static std::string isPossibleContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + static std::string isPossibleContainerSizeValue(std::list values, + MathLib::bigint i, + bool unique = true) + { if (!unique) values.remove_if(&isNotPossible); if (values.size() != 1) @@ -4661,7 +4656,10 @@ class TestValueFlow : public TestFixture { return ""; } - static std::string isImpossibleContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + static std::string isImpossibleContainerSizeValue(std::list values, + MathLib::bigint i, + bool unique = true) + { if (!unique) values.remove_if(&isNotImpossible); if (values.size() != 1) @@ -4675,7 +4673,10 @@ class TestValueFlow : public TestFixture { return ""; } - static std::string isInconclusiveContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + static std::string isInconclusiveContainerSizeValue(std::list values, + MathLib::bigint i, + bool unique = true) + { if (!unique) values.remove_if(&isNotInconclusive); if (values.size() != 1) @@ -4689,7 +4690,8 @@ class TestValueFlow : public TestFixture { return ""; } - static std::string isKnownContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) { + static std::string isKnownContainerSizeValue(std::list values, MathLib::bigint i, bool unique = true) + { if (!unique) values.remove_if(&isNotKnown); if (values.size() != 1) From abb242ffdd26cddeb805b5d3d590555e952b9f4f Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 19:39:23 -0500 Subject: [PATCH 16/33] Use update method --- lib/valueflow.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ca302dd01d1..1f3fd0fca44 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2482,16 +2482,12 @@ static Analyzer::Result valueFlowForwardExpression(Token* startToken, const TokenList* const tokenlist, const Settings* settings) { - Analyzer::Action actions; - Analyzer::Terminate terminate = Analyzer::Terminate::None; + Analyzer::Result result{}; for (const ValueFlow::Value& v : values) { ExpressionAnalyzer a(exprTok, v, tokenlist); - Analyzer::Result r = valueFlowGenericForward(startToken, endToken, a, settings); - actions |= r.action; - if (terminate == Analyzer::Terminate::None) - terminate = r.terminate; + result.update(valueFlowGenericForward(startToken, endToken, a, settings)); } - return {actions, terminate}; + return result; } static const Token* parseBinaryIntOp(const Token* expr, MathLib::bigint& known) From 3e21935416aaef5a61d8accb5b3e84a79b8d6496 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 19:39:52 -0500 Subject: [PATCH 17/33] Add spaces --- lib/valueflow.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1f3fd0fca44..b6cd33b3d3f 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2476,11 +2476,11 @@ struct OppositeExpressionAnalyzer : ExpressionAnalyzer { }; static Analyzer::Result valueFlowForwardExpression(Token* startToken, - const Token* endToken, - const Token* exprTok, - const std::list& values, - const TokenList* const tokenlist, - const Settings* settings) + const Token* endToken, + const Token* exprTok, + const std::list& values, + const TokenList* const tokenlist, + const Settings* settings) { Analyzer::Result result{}; for (const ValueFlow::Value& v : values) { From 9dfdeb001717ff04c5042d3434b4bfcabd12b56a Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 19:40:15 -0500 Subject: [PATCH 18/33] Format --- lib/valueflow.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b6cd33b3d3f..248ba1e242d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2476,11 +2476,11 @@ struct OppositeExpressionAnalyzer : ExpressionAnalyzer { }; static Analyzer::Result valueFlowForwardExpression(Token* startToken, - const Token* endToken, - const Token* exprTok, - const std::list& values, - const TokenList* const tokenlist, - const Settings* settings) + const Token* endToken, + const Token* exprTok, + const std::list& values, + const TokenList* const tokenlist, + const Settings* settings) { Analyzer::Result result{}; for (const ValueFlow::Value& v : values) { From 940c1628f05ace427dc3887082364fb7fd784ea1 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 20:54:05 -0500 Subject: [PATCH 19/33] Fix null pointer deref --- lib/astutils.cpp | 4 +++- test/testvalueflow.cpp | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index d493ad54d3b..0649f2f0406 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -347,8 +347,10 @@ static bool hasToken(const Token * startTok, const Token * stopTok, const Token template )> static T* previousBeforeAstLeftmostLeafGeneric(T* tok) { + if (!tok) + return nullptr; T* leftmostLeaf = tok; - while (leftmostLeaf && leftmostLeaf->astOperand1()) + while (leftmostLeaf->astOperand1()) leftmostLeaf = leftmostLeaf->astOperand1(); return leftmostLeaf->previous(); } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index b2fa6ef1c26..39ac40bd2e8 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2773,6 +2773,17 @@ class TestValueFlow : public TestFixture { " int a = x;\n" "}\n"; ASSERT_EQUALS(true, testValueOfXImpossible(code, 5U, 3)); + + code = "struct a {\n" + " a *b();\n" + " void c();\n" + "};\n" + "void e(a *x) {\n" + " while (x && x->b())\n" + " x = x->b();\n" + " x->c();\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 8U, 0)); } void valueFlowAfterConditionExpr() { From f00ca2c363bad5b46d662a817b1527e4740f846c Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 20:59:12 -0500 Subject: [PATCH 20/33] Fix possible null pointer deref --- lib/checkcondition.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 3f2bf060484..5b27be2d033 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -441,15 +441,15 @@ void CheckCondition::duplicateCondition() if (scope.type != Scope::eIf) continue; - const Token *cond1 = scope.classDef->next()->astOperand2(); + const Token *tok2 = scope.classDef->next(); + if (!tok2) + continue; + const Token *cond1 = tok2->astOperand2(); if (!cond1) continue; if (cond1->hasKnownIntValue()) continue; - const Token *tok2 = scope.classDef->next(); - if (!tok2) - continue; tok2 = tok2->link(); if (!Token::simpleMatch(tok2, ") {")) continue; From 314317cb933f5e0f934b31f5f5b50a58b8378e4e Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 21:02:17 -0500 Subject: [PATCH 21/33] Add const --- test/testvalueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 39ac40bd2e8..15de65a3cad 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2775,7 +2775,7 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(true, testValueOfXImpossible(code, 5U, 3)); code = "struct a {\n" - " a *b();\n" + " a *b() const;\n" " void c();\n" "};\n" "void e(a *x) {\n" From c477e6a0d79496c32bdec446dca98a2891c8712c Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 21:40:31 -0500 Subject: [PATCH 22/33] Add test for non-const --- test/testvalueflow.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 15de65a3cad..633d8265008 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2784,6 +2784,17 @@ class TestValueFlow : public TestFixture { " x->c();\n" "}\n"; ASSERT_EQUALS(true, testValueOfX(code, 8U, 0)); + + code = "struct a {\n" + " a *b();\n" + " void c();\n" + "};\n" + "void e(a *x) {\n" + " while (x && x->b())\n" + " x = x->b();\n" + " x->c();\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 8U, 0)); } void valueFlowAfterConditionExpr() { From 1c0868a26b6fb04ca72ee0484aa319449425f9a8 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 21:42:52 -0500 Subject: [PATCH 23/33] Add test for overloaded const --- test/testvalueflow.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 633d8265008..dd6a90afa3b 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2795,6 +2795,18 @@ class TestValueFlow : public TestFixture { " x->c();\n" "}\n"; ASSERT_EQUALS(false, testValueOfX(code, 8U, 0)); + + code = "struct a {\n" + " a *b();\n" + " a *b() const;\n" + " void c();\n" + "};\n" + "void e(a *x) {\n" + " while (x && x->b())\n" + " x = x->b();\n" + " x->c();\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 9U, 0)); } void valueFlowAfterConditionExpr() { From 661ee2e9cf0882b84f7a55c10a49d39e9bfce38a Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 15 Jul 2021 21:47:46 -0500 Subject: [PATCH 24/33] Fix test with non-const method --- lib/reverseanalyzer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 31873119b77..987e9a8a111 100644 --- a/lib/reverseanalyzer.cpp +++ b/lib/reverseanalyzer.cpp @@ -3,6 +3,7 @@ #include "astutils.h" #include "errortypes.h" #include "forwardanalyzer.h" +#include "settings.h" #include "symboldatabase.h" #include "token.h" #include "valueptr.h" @@ -171,7 +172,7 @@ struct ReverseTraversal { settings); } // Assignment to - } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue()) { + } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue() && isConstExpression(assignTok->astOperand2(), settings->library, true, true)) { const std::string info = "Assignment to '" + assignTok->expressionString() + "'"; ValuePtr a = analyzer->reanalyze(assignTok->astOperand2(), info); if (a) { From faa398238232128704932f7a94a4a40a47626ec0 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 16 Jul 2021 10:17:18 -0500 Subject: [PATCH 25/33] Fix test cases --- lib/astutils.cpp | 66 ++++++++++++++++++++++++++++++++++++++++-- lib/astutils.h | 2 ++ lib/symboldatabase.cpp | 22 ++++++++++++++ lib/symboldatabase.h | 2 ++ lib/valueflow.cpp | 4 +-- test/testvalueflow.cpp | 2 +- 6 files changed, 92 insertions(+), 6 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 0649f2f0406..0a36ff62759 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1394,6 +1394,68 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons return false; } +static bool functionModifiesArguments(const Function* f) +{ + return std::any_of(f->argumentList.begin(), f->argumentList.end(), [](const Variable& var) { + if (var.isReference() || var.isPointer()) + return !var.isConst(); + return true; + }); +} + +bool isConstFunctionCall(const Token *ftok, const Library& library) +{ + if (!Token::Match(ftok, "%name% (")) + return false; + if (const Function* f = ftok->function()) { + if (f->isAttributePure() || f->isAttributeConst()) + return true; + if (Function::returnsVoid(f)) + return false; + // Any modified arguments + if (functionModifiesArguments(f)) + return false; + // Member function call + if (Token::simpleMatch(ftok->previous(), ".")) { + if (f->isConst()) + return true; + // Check for const overloaded function that just return the const version + if (!Function::returnsConst(f)) { + std::vector fs = f->getOverloadedFunctions(); + if (std::any_of(fs.begin(), fs.end(), [&](const Function* g) { + if (f->argumentList.size() != g->argumentList.size()) + return false; + if (functionModifiesArguments(g)) + return false; + if (g->isConst() && Function::returnsConst(g)) + return true; + return false; + })) + return true; + } + return false; + } else if (f->argumentList.empty()) { + // TODO: Check for constexpr + return false; + } + } else if (const Library::Function* f = library.getFunction(ftok)) { + if (f->ispure) + return true; + for(auto&& p:f->argumentChecks) { + const Library::ArgumentChecks& ac = p.second; + if (ac.direction != Library::ArgumentChecks::Direction::DIR_IN) + return false; + } + if (Token::simpleMatch(ftok->previous(), ".")) { + if (!f->isconst) + return false; + } else if (f->argumentChecks.empty()) { + return false; + } + } + return true; +} + bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp) { if (!tok) @@ -1401,9 +1463,7 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure, bool if (tok->variable() && tok->variable()->isVolatile()) return false; if (tok->isName() && tok->next()->str() == "(") { - if (!tok->function() && !Token::Match(tok->previous(), ".|::") && !library.isFunctionConst(tok->str(), pure)) - return false; - else if (tok->function() && !tok->function()->isConst() && !tok->function()->isPure()) + if (!isConstFunctionCall(tok, library)) return false; } if (tok->tokType() == Token::eIncDecOp) diff --git a/lib/astutils.h b/lib/astutils.h index cfc1081c91d..facec41e1a6 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -171,6 +171,8 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); +bool isConstFunctionCall(const Token *ftok, const Library& library); + bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp); bool isWithoutSideEffects(bool cpp, const Token* tok); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 9f771feff50..e863d4ac5ac 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -3934,6 +3934,28 @@ bool Function::isImplicitlyVirtual(bool defaultVal) const return defaultVal; //If we can't see all the bases classes then we can't say conclusively } +std::vector Function::getOverloadedFunctions() const +{ + std::vector result; + const Scope* scope = nestedIn; + + while (scope) { + const bool isMemberFunction = scope->isClassOrStruct() && !isStatic(); + for (std::multimap::const_iterator it = scope->functionMap.find(tokenDef->str()); it != scope->functionMap.end() && it->first == tokenDef->str(); ++it) { + const Function * func = it->second; + if (isMemberFunction == func->isStatic()) + continue; + result.push_back(func); + } + if (isMemberFunction) + break; + scope = scope->nestedIn; + } + + return result; + +} + const Function *Function::getOverriddenFunction(bool *foundAllBaseClasses) const { if (foundAllBaseClasses) diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 6b85a29c49c..18f064533de 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -769,6 +769,8 @@ class CPPCHECKLIB Function { /** @brief check if this function is virtual in the base classes */ bool isImplicitlyVirtual(bool defaultVal = false) const; + std::vector getOverloadedFunctions() const; + /** @brief get function in base class that is overridden */ const Function *getOverriddenFunction(bool *foundAllBaseClasses = nullptr) const; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 248ba1e242d..bafc8a981f4 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2183,10 +2183,10 @@ struct ValueFlowAnalyzer : Analyzer { if (isGlobal() && Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { // TODO: Check for constexpr functions if (tok->function()) { - if (!tok->function()->isConst() && !tok->function()->isPure()) + if (!isConstFunctionCall(tok, getSettings()->library)) return Action::Invalid; } else if (getSettings()->library.getFunction(tok)) { - // Assume library functions doesn't modify user-global variables + // Assume library function doesn't modify user-global variables return Action::None; } else { return Action::Invalid; diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index dd6a90afa3b..4e012fadfcf 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2806,7 +2806,7 @@ class TestValueFlow : public TestFixture { " x = x->b();\n" " x->c();\n" "}\n"; - ASSERT_EQUALS(true, testValueOfX(code, 9U, 0)); + TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 9U, 0)); } void valueFlowAfterConditionExpr() { From 1d673190b17d0ebcb4026da9b2a6c0f9fb28b88c Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 16 Jul 2021 10:33:46 -0500 Subject: [PATCH 26/33] Format --- lib/astutils.cpp | 18 +++++++++--------- lib/astutils.h | 2 +- lib/checkcondition.cpp | 4 ++-- lib/reverseanalyzer.cpp | 3 ++- lib/symboldatabase.cpp | 7 ++++--- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 0a36ff62759..0626ec768c2 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1403,7 +1403,7 @@ static bool functionModifiesArguments(const Function* f) }); } -bool isConstFunctionCall(const Token *ftok, const Library& library) +bool isConstFunctionCall(const Token* ftok, const Library& library) { if (!Token::Match(ftok, "%name% (")) return false; @@ -1423,14 +1423,14 @@ bool isConstFunctionCall(const Token *ftok, const Library& library) if (!Function::returnsConst(f)) { std::vector fs = f->getOverloadedFunctions(); if (std::any_of(fs.begin(), fs.end(), [&](const Function* g) { - if (f->argumentList.size() != g->argumentList.size()) + if (f->argumentList.size() != g->argumentList.size()) + return false; + if (functionModifiesArguments(g)) + return false; + if (g->isConst() && Function::returnsConst(g)) + return true; return false; - if (functionModifiesArguments(g)) - return false; - if (g->isConst() && Function::returnsConst(g)) - return true; - return false; - })) + })) return true; } return false; @@ -1441,7 +1441,7 @@ bool isConstFunctionCall(const Token *ftok, const Library& library) } else if (const Library::Function* f = library.getFunction(ftok)) { if (f->ispure) return true; - for(auto&& p:f->argumentChecks) { + for (auto&& p : f->argumentChecks) { const Library::ArgumentChecks& ac = p.second; if (ac.direction != Library::ArgumentChecks::Direction::DIR_IN) return false; diff --git a/lib/astutils.h b/lib/astutils.h index facec41e1a6..f73017aadc5 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -171,7 +171,7 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); -bool isConstFunctionCall(const Token *ftok, const Library& library); +bool isConstFunctionCall(const Token* ftok, const Library& library); bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp); diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 5b27be2d033..5ff6603a700 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -441,10 +441,10 @@ void CheckCondition::duplicateCondition() if (scope.type != Scope::eIf) continue; - const Token *tok2 = scope.classDef->next(); + const Token* tok2 = scope.classDef->next(); if (!tok2) continue; - const Token *cond1 = tok2->astOperand2(); + const Token* cond1 = tok2->astOperand2(); if (!cond1) continue; if (cond1->hasKnownIntValue()) diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 987e9a8a111..ec2b39f5d2a 100644 --- a/lib/reverseanalyzer.cpp +++ b/lib/reverseanalyzer.cpp @@ -172,7 +172,8 @@ struct ReverseTraversal { settings); } // Assignment to - } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue() && isConstExpression(assignTok->astOperand2(), settings->library, true, true)) { + } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue() && + isConstExpression(assignTok->astOperand2(), settings->library, true, true)) { const std::string info = "Assignment to '" + assignTok->expressionString() + "'"; ValuePtr a = analyzer->reanalyze(assignTok->astOperand2(), info); if (a) { diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index e863d4ac5ac..314e81afceb 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -3941,8 +3941,10 @@ std::vector Function::getOverloadedFunctions() const while (scope) { const bool isMemberFunction = scope->isClassOrStruct() && !isStatic(); - for (std::multimap::const_iterator it = scope->functionMap.find(tokenDef->str()); it != scope->functionMap.end() && it->first == tokenDef->str(); ++it) { - const Function * func = it->second; + for (std::multimap::const_iterator it = scope->functionMap.find(tokenDef->str()); + it != scope->functionMap.end() && it->first == tokenDef->str(); + ++it) { + const Function* func = it->second; if (isMemberFunction == func->isStatic()) continue; result.push_back(func); @@ -3953,7 +3955,6 @@ std::vector Function::getOverloadedFunctions() const } return result; - } const Function *Function::getOverriddenFunction(bool *foundAllBaseClasses) const From 0f2f2d6f0fbb8824a892276c3662a15f0facfaa7 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 16 Jul 2021 13:47:01 -0500 Subject: [PATCH 27/33] Improve const check on unknown functions --- lib/astutils.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 0626ec768c2..cd11a5b4dcc 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1423,6 +1423,8 @@ bool isConstFunctionCall(const Token* ftok, const Library& library) if (!Function::returnsConst(f)) { std::vector fs = f->getOverloadedFunctions(); if (std::any_of(fs.begin(), fs.end(), [&](const Function* g) { + if (f == g) + return false; if (f->argumentList.size() != g->argumentList.size()) return false; if (functionModifiesArguments(g)) @@ -1452,6 +1454,21 @@ bool isConstFunctionCall(const Token* ftok, const Library& library) } else if (f->argumentChecks.empty()) { return false; } + } else { + bool constMember = !Token::Match(ftok->previous(), ". %name% ("); + if (Token::Match(ftok->tokAt(-2), "%var% . %name% (")) { + const Variable* var = ftok->tokAt(-2)->variable(); + if (var) + constMember = var->isConst(); + } + // TODO: Only check const on lvalues + std::vector args = getArguments(ftok); + return constMember && std::all_of(args.begin(), args.end(), [](const Token* tok) { + const Variable* var = tok->variable(); + if (var) + return var->isConst(); + return false; + }); } return true; } From 778ca515f38d8f51d7eda5639bceced6f8edbd39 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 16 Jul 2021 13:50:37 -0500 Subject: [PATCH 28/33] Dont treat unknown nullary functions as no side effects --- lib/astutils.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index cd11a5b4dcc..05730e00ce8 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1455,7 +1455,8 @@ bool isConstFunctionCall(const Token* ftok, const Library& library) return false; } } else { - bool constMember = !Token::Match(ftok->previous(), ". %name% ("); + bool memberFunction = Token::Match(ftok->previous(), ". %name% ("); + bool constMember = !memberFunction; if (Token::Match(ftok->tokAt(-2), "%var% . %name% (")) { const Variable* var = ftok->tokAt(-2)->variable(); if (var) @@ -1463,6 +1464,8 @@ bool isConstFunctionCall(const Token* ftok, const Library& library) } // TODO: Only check const on lvalues std::vector args = getArguments(ftok); + if (memberFunction && args.empty()) + return false; return constMember && std::all_of(args.begin(), args.end(), [](const Token* tok) { const Variable* var = tok->variable(); if (var) From 04b40a241adcca2b4d42f76ae4a8619b10736ef2 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 16 Jul 2021 20:19:18 -0500 Subject: [PATCH 29/33] Assume this pointer cant be modified --- lib/astutils.cpp | 3 +++ test/testvalueflow.cpp | 14 +------------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 05730e00ce8..004feb9625a 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1964,6 +1964,9 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, // Member function call if (tok->variable() && Token::Match(tok2->astParent(), ". %name%") && isFunctionCall(tok2->astParent()->next()) && tok2->astParent()->astOperand1() == tok2) { const Variable * var = tok->variable(); + // Member function cannot change what `this` points to + if (indirect == 0 && var->isPointer()) + return false; bool isConst = var && var->isConst(); if (!isConst) { const ValueType * valueType = var->valueType(); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 4e012fadfcf..a950b20794d 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2794,19 +2794,7 @@ class TestValueFlow : public TestFixture { " x = x->b();\n" " x->c();\n" "}\n"; - ASSERT_EQUALS(false, testValueOfX(code, 8U, 0)); - - code = "struct a {\n" - " a *b();\n" - " a *b() const;\n" - " void c();\n" - "};\n" - "void e(a *x) {\n" - " while (x && x->b())\n" - " x = x->b();\n" - " x->c();\n" - "}\n"; - TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 9U, 0)); + ASSERT_EQUALS(true, testValueOfX(code, 8U, 0)); } void valueFlowAfterConditionExpr() { From 6830f72fc7756eb3d4928f29c8e9e74c6bf4d3d7 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 16 Jul 2021 20:30:49 -0500 Subject: [PATCH 30/33] Fix null pointer deref --- lib/clangimport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 315af99989e..7c986a2aee1 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -1470,7 +1470,7 @@ Token * clangimport::AstNode::createTokensVarDecl(TokenList *tokenList) else if (startToken->str() != "static") startToken = startToken->next(); Token *vartok1 = addtoken(tokenList, name); - Scope *scope = const_cast(tokenList->back()->scope()); + Scope *scope = const_cast(startToken->scope()); scope->varlist.push_back(Variable(vartok1, unquote(type), startToken, vartok1->previous(), 0, scope->defaultAccess(), recordType, scope)); mData->varDecl(addr, vartok1, &scope->varlist.back()); if (mExtTokens.back() == "cinit" && !children.empty()) { From 7f1739fe2f1ad7f80211b1d083e842abb7d0bc1c Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 17 Jul 2021 19:42:32 -0500 Subject: [PATCH 31/33] Fix FPs --- lib/astutils.cpp | 2 +- lib/clangimport.cpp | 2 +- lib/valueflow.cpp | 6 +----- test/testnullpointer.cpp | 16 ++++++++++++++++ 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 004feb9625a..2c970e73386 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1965,7 +1965,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, if (tok->variable() && Token::Match(tok2->astParent(), ". %name%") && isFunctionCall(tok2->astParent()->next()) && tok2->astParent()->astOperand1() == tok2) { const Variable * var = tok->variable(); // Member function cannot change what `this` points to - if (indirect == 0 && var->isPointer()) + if (indirect == 0 && astIsPointer(tok)) return false; bool isConst = var && var->isConst(); if (!isConst) { diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 7c986a2aee1..315af99989e 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -1470,7 +1470,7 @@ Token * clangimport::AstNode::createTokensVarDecl(TokenList *tokenList) else if (startToken->str() != "static") startToken = startToken->next(); Token *vartok1 = addtoken(tokenList, name); - Scope *scope = const_cast(startToken->scope()); + Scope *scope = const_cast(tokenList->back()->scope()); scope->varlist.push_back(Variable(vartok1, unquote(type), startToken, vartok1->previous(), 0, scope->defaultAccess(), recordType, scope)); mData->varDecl(addr, vartok1, &scope->varlist.back()); if (mExtTokens.back() == "cinit" && !children.empty()) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 40774762826..359f145fd6d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2013,12 +2013,8 @@ struct ValueFlowAnalyzer : Analyzer { virtual Action isAliasModified(const Token* tok) const { int indirect = 0; - int baseIndirect = 0; - const ValueType* vt = getValueType(tok); - if (vt) - baseIndirect = vt->pointer; if (tok->valueType()) - indirect = std::max(0, tok->valueType()->pointer - baseIndirect); + indirect = tok->valueType()->pointer; if (isVariableChanged(tok, indirect, getSettings(), isCPP())) return Action::Invalid; return Action::None; diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index dbfb28acdc7..8d93ad35f56 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -115,6 +115,7 @@ class TestNullPointer : public TestFixture { TEST_CASE(nullpointer72); // #10215 TEST_CASE(nullpointer73); // #10321 TEST_CASE(nullpointer74); + TEST_CASE(nullpointer75); TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -2329,6 +2330,21 @@ class TestNullPointer : public TestFixture { ASSERT_EQUALS("", errout.str()); } + void nullpointer75() { + check("struct a {\n" + " a *b() const;\n" + " void c();\n" + " int d() const;\n" + "};\n" + "void e(a *x) {\n" + " while (x->b()->d() == 0)\n" + " x->c();\n" + " x->c();\n" + " if (x->b()) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" From 3a9a5d32d283fe5ab4641544bffa59727087b2c0 Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 17 Jul 2021 19:43:25 -0500 Subject: [PATCH 32/33] Format --- lib/astutils.cpp | 10 +++++----- test/testnullpointer.cpp | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 2c970e73386..4e0d7e844f8 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1467,11 +1467,11 @@ bool isConstFunctionCall(const Token* ftok, const Library& library) if (memberFunction && args.empty()) return false; return constMember && std::all_of(args.begin(), args.end(), [](const Token* tok) { - const Variable* var = tok->variable(); - if (var) - return var->isConst(); - return false; - }); + const Variable* var = tok->variable(); + if (var) + return var->isConst(); + return false; + }); } return true; } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 8d93ad35f56..2369fcb19f0 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2330,7 +2330,8 @@ class TestNullPointer : public TestFixture { ASSERT_EQUALS("", errout.str()); } - void nullpointer75() { + void nullpointer75() + { check("struct a {\n" " a *b() const;\n" " void c();\n" From 67105837dc9a4e9f56a580601e256390bdb91c7b Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 17 Jul 2021 21:06:25 -0500 Subject: [PATCH 33/33] Run dmake --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 431ec54bbcd..329f15d1c29 100644 --- a/Makefile +++ b/Makefile @@ -545,7 +545,7 @@ $(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp. $(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/programmemory.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/programmemory.o $(libcppdir)/programmemory.cpp -$(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/reverseanalyzer.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h +$(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/reverseanalyzer.o $(libcppdir)/reverseanalyzer.cpp $(libcppdir)/settings.o: lib/settings.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/summaries.h lib/suppressions.h lib/timer.h lib/utils.h lib/valueflow.h