From 59a1c1a9d8a42db4aff485608923fd7c8ed07d33 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 18 Jul 2021 00:46:31 -0500 Subject: [PATCH] Refactor: Remove variable analyzer (#3339) --- Makefile | 2 +- cfg/std.cfg | 1 + cmake/compileroptions.cmake | 1 - lib/analyzer.h | 7 + lib/astutils.cpp | 121 +++++++-- lib/astutils.h | 3 + lib/checkcondition.cpp | 8 +- lib/reverseanalyzer.cpp | 4 +- lib/symboldatabase.cpp | 23 ++ lib/symboldatabase.h | 2 + lib/valueflow.cpp | 518 ++++++++++++------------------------ test/testnullpointer.cpp | 17 ++ test/testvalueflow.cpp | 68 ++++- 13 files changed, 403 insertions(+), 372 deletions(-) diff --git a/Makefile b/Makefile index 431ec54bb..329f15d1c 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 diff --git a/cfg/std.cfg b/cfg/std.cfg index 8d6769cf7..54860b0de 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -7717,6 +7717,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init + false diff --git a/cmake/compileroptions.cmake b/cmake/compileroptions.cmake index 94a6d75b0..30037ba21 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/analyzer.h b/lib/analyzer.h index 592c1ee5f..515570266 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -119,6 +119,13 @@ 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/astutils.cpp b/lib/astutils.cpp index 891a836ee..4e0d7e844 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(); } @@ -1392,14 +1394,96 @@ 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 == g) + return false; + 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; + } + } else { + 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) + constMember = var->isConst(); + } + // 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) + return var->isConst(); + return false; + }); + } + return true; +} + bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp) { 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; - else if (tok->function() && !tok->function()->isConst()) + if (!isConstFunctionCall(tok, library)) return false; } if (tok->tokType() == Token::eIncDecOp) @@ -1880,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 && astIsPointer(tok)) + return false; bool isConst = var && var->isConst(); if (!isConst) { const ValueType * valueType = var->valueType(); @@ -2092,6 +2179,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)) @@ -2099,17 +2200,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 a2598e05c..f73017aad 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); @@ -226,6 +228,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/checkcondition.cpp b/lib/checkcondition.cpp index d62c7984e..5e677c3c6 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; diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 31873119b..ec2b39f5d 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,8 @@ 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) { diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 22c494c8e..6d15983d2 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -3943,6 +3943,29 @@ 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 6b85a29c4..18f064533 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -769,6 +769,8 @@ public: /** @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 114c0b267..359f145fd 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. @@ -99,6 +99,7 @@ #include #include +#include #include #include #include @@ -1677,12 +1678,12 @@ static void valueFlowGlobalStaticVar(TokenList *tokenList, const Settings *setti } } -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 Analyzer::Result valueFlowForward(Token* startToken, + const Token* endToken, + const Token* exprTok, + std::list values, + TokenList* const tokenlist, + const Settings* settings); static void valueFlowReverse(TokenList* tokenlist, Token* tok, @@ -1973,6 +1974,7 @@ struct ValueFlowAnalyzer : Analyzer { virtual bool isGlobal() const { return false; } + virtual bool dependsOnThis() const { return false; } virtual bool invalid() const { return false; @@ -2011,17 +2013,20 @@ 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; } + 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) @@ -2167,10 +2172,19 @@ struct ValueFlowAnalyzer : Analyzer { return a; } - // TODO: Check if function is pure - if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { - // bailout: global non-const variables - if (isGlobal()) { + if (dependsOnThis() && exprDependsOnThis(tok)) + return isThisModified(tok); + + // 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 (!isConstFunctionCall(tok, getSettings()->library)) + return Action::Invalid; + } else if (getSettings()->library.getFunction(tok)) { + // Assume library function doesn't modify user-global variables + return Action::None; + } else { return Action::Invalid; } } @@ -2382,123 +2396,20 @@ 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, - 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}; -} - -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) -{ - auto aliases = getAliasesFromValues(values); - return valueFlowForwardVariable( - startToken, endToken, var, std::move(values), std::move(aliases), 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; 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) + { - setupExprVarIds(); + dependOnThis = exprDependsOnThis(expr); + setupExprVarIds(expr); } virtual const ValueType* getValueType(const Token*) const OVERRIDE { @@ -2506,13 +2417,26 @@ 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) { - if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") { + 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()) { + if (!v.isLocalLifetimeValue()) + continue; + if (!v.tokvalue) + continue; + if (v.tokvalue == tok) + continue; + setupExprVarIds(v.tokvalue, depth + 1); + } + if (depth == 0 && tok->varId() == 0 && !tok->function() && tok->isName() && tok->previous()->str() != ".") { // unknown variable unknown = true; return ChildrenToVisit::none; @@ -2520,10 +2444,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); } } @@ -2541,9 +2468,9 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { return ps; } - virtual bool match(const Token* tok) const OVERRIDE { - return isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); - } + virtual bool match(const Token* tok) const OVERRIDE { return tok->exprId() == expr->exprId(); } + + virtual bool dependsOnThis() const OVERRIDE { return dependOnThis; } virtual bool isGlobal() const OVERRIDE { return !local; @@ -2565,22 +2492,18 @@ 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::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) @@ -2648,11 +2571,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, @@ -2663,10 +2582,19 @@ 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); } } @@ -2681,34 +2609,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) @@ -3176,22 +3077,13 @@ 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 @@ -3211,16 +3103,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(); @@ -3919,7 +3802,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; @@ -3948,11 +3831,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; @@ -3988,16 +3869,8 @@ 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); } } } @@ -4037,9 +3910,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; @@ -4047,6 +3917,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; @@ -4412,12 +4284,11 @@ struct ConditionHandler { Condition cond = parse(tok, settings); if (!cond.vartok) continue; - if (cond.vartok->variable() && cond.vartok->variable()->isVolatile()) + if (cond.vartok->hasKnownIntValue()) continue; if (cond.true_values.empty() || cond.false_values.empty()) continue; - - if (exprDependsOnThis(cond.vartok)) + if (!isConstExpression(cond.vartok, settings->library, true, tokenlist->isCPP())) continue; std::vector vars = getExprVariables(cond.vartok, tokenlist, symboldatabase, settings); if (std::any_of(vars.begin(), vars.end(), [](const Variable* var) { @@ -4579,7 +4450,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); } } @@ -5171,7 +5042,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, 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()) { @@ -5196,8 +5071,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); } } @@ -5226,7 +5100,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)) { @@ -5244,7 +5118,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); } } } @@ -5475,7 +5349,11 @@ 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()) @@ -5486,16 +5364,12 @@ 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) @@ -5854,7 +5728,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; @@ -5876,7 +5750,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); } } } @@ -5955,7 +5829,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%")) @@ -5990,19 +5864,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); } } @@ -6083,18 +5945,15 @@ 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, - const ValueFlow::Value& val, - std::vector paliases, - const TokenList* t) - : VariableAnalyzer(v, val, std::move(paliases), t) + ContainerExpressionAnalyzer(const Token* expr, const ValueFlow::Value& val, const TokenList* 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 { @@ -6175,26 +6034,26 @@ struct ContainerVariableAnalyzer : VariableAnalyzer { } }; -static Analyzer::Result valueFlowContainerForward(Token* tok, - const Token* endToken, - const Variable* var, - ValueFlow::Value value, - TokenList* tokenlist) +static Analyzer::Result valueFlowContainerForward(Token* startToken, + const Token* endToken, + 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, - TokenList* tokenlist) + +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, @@ -6204,10 +6063,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); } } @@ -6510,7 +6367,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 @@ -6527,14 +6384,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)); @@ -6542,13 +6399,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); } } } @@ -6606,13 +6463,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, @@ -6621,10 +6475,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); } @@ -6690,7 +6540,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()) { @@ -6748,16 +6598,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); } } } @@ -6829,7 +6670,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) @@ -6858,7 +6699,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, value, tokenlist); + valueFlowContainerForward( + const_cast(functionScope->bodyStart), arg.nameToken(), value, tokenlist); continue; } @@ -6892,16 +6734,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; - 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; } } @@ -6919,16 +6757,12 @@ 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); } } } @@ -7092,7 +6926,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); @@ -7101,8 +6935,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); @@ -7112,11 +6946,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); } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index dbfb28acd..2369fcb19 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -115,6 +115,7 @@ private: 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,22 @@ private: 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" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index ec8328ca6..a950b2079 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -152,6 +152,14 @@ private: 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); @@ -2765,6 +2773,28 @@ private: " int a = x;\n" "}\n"; ASSERT_EQUALS(true, testValueOfXImpossible(code, 5U, 3)); + + code = "struct a {\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, 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(true, testValueOfX(code, 8U, 0)); } void valueFlowAfterConditionExpr() { @@ -4631,7 +4661,12 @@ private: 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 +4678,12 @@ private: 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 +4695,12 @@ private: 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 +4712,10 @@ private: 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 +4838,32 @@ private: " 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"