diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e2e19b6bd91..f7e40ce1dbe 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1088,29 +1088,32 @@ bool isAliasOf(const Token* tok, const Token* expr, nonneg int* indirect) if (!tok) return false; const Token* r = nullptr; + std::vector aliasValues; for (const ReferenceToken& ref : tok->refs()) { const bool pointer = astIsPointer(ref.token); + const bool deref = expr && ref.token != tok && ref.token->isUnaryOp("*") && expr->exprId() == ref.token->astOperand1()->exprId(); + aliasValues.clear(); + for (const ValueFlow::Value& val : ref.token->values()) { + if (val.isImpossible()) + continue; + if (val.isLocalLifetimeValue() || (pointer && val.isSymbolicValue() && val.intvalue == 0)) + aliasValues.push_back(val.tokvalue); + } + if (!deref && aliasValues.empty()) + continue; r = findAstNode(expr, [&](const Token* childTok) { if (childTok->exprId() == 0) return false; - if (ref.token != tok && expr->exprId() == childTok->exprId() && ref.token->isUnaryOp("*") && expr->exprId() == ref.token->astOperand1()->exprId()) { + if (deref && expr->exprId() == childTok->exprId()) { if (indirect) *indirect = 0; return true; } - for (const ValueFlow::Value& val : ref.token->values()) { - if (val.isImpossible()) - continue; - if (val.isLocalLifetimeValue() || (pointer && val.isSymbolicValue() && val.intvalue == 0)) { - if (findAstNode(val.tokvalue, - [&](const Token* aliasTok) { - return aliasTok != childTok && aliasTok->exprId() == childTok->exprId(); - })) { - return true; - } - } - } - return false; + return std::any_of(aliasValues.cbegin(), aliasValues.cend(), [&](const Token* aliasValue) { + return findAstNode(aliasValue, [&](const Token* aliasTok) { + return aliasTok != childTok && aliasTok->exprId() == childTok->exprId(); + }) != nullptr; + }); }); if (r) break; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 26801640e99..8c21f59f447 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -6952,7 +6952,7 @@ void SymbolDatabase::setValueType(Token* tok, const Variable& var, const SourceL } } -static ValueType::Type getEnumType(const Scope* scope, const Platform& platform); +static ValueType::Type getEnumTypeCached(const Scope* scope, const Platform& platform); void SymbolDatabase::setValueType(Token* tok, const Enumerator& enumerator, const SourceLocation &loc) { @@ -6982,7 +6982,7 @@ void SymbolDatabase::setValueType(Token* tok, const Enumerator& enumerator, cons setValueType(tok, valuetype); } else { valuetype.sign = ValueType::SIGNED; - valuetype.type = getEnumType(enumerator.scope, mSettings.platform); + valuetype.type = getEnumTypeCached(enumerator.scope, mSettings.platform); setValueType(tok, valuetype); } } @@ -7547,7 +7547,7 @@ void SymbolDatabase::setValueType(Token* tok, const ValueType& valuetype, const } } -static ValueType::Type getEnumType(const Scope* scope, const Platform& platform) // TODO: also determine sign? +static ValueType::Type getEnumType(const Scope* scope, const Platform& platform, bool* complete = nullptr) // TODO: also determine sign? { ValueType::Type type = ValueType::Type::INT; for (const Token* tok = scope->bodyStart; tok && tok != scope->bodyEnd; tok = tok->next()) { @@ -7564,6 +7564,9 @@ static ValueType::Type getEnumType(const Scope* scope, const Platform& platform) if (const ValueType* vt = vTok->valueType()) { if ((vt->type > type && (vt->type == ValueType::Type::LONG || vt->type == ValueType::Type::LONGLONG))) type = vt->type; + } else if (complete) { + // the literal does not have a valuetype yet, the result might change later + *complete = false; } continue; } @@ -7577,6 +7580,23 @@ static ValueType::Type getEnumType(const Scope* scope, const Platform& platform) return type; } +// Cache for getEnumType(), only active during SymbolDatabase::setValueTypeInTokenList() +static thread_local std::unordered_map* enumTypeCache = nullptr; + +static ValueType::Type getEnumTypeCached(const Scope* scope, const Platform& platform) +{ + if (!enumTypeCache) + return getEnumType(scope, platform); + const auto it = enumTypeCache->find(scope); + if (it != enumTypeCache->end()) + return it->second; + bool complete = true; + const ValueType::Type type = getEnumType(scope, platform, &complete); + if (complete) + enumTypeCache->emplace(scope, type); + return type; +} + static const Token* parsedecl(const Token* type, ValueType* const valuetype, ValueType::Sign defaultSignedness, @@ -7609,7 +7629,7 @@ static const Token* parsedecl(const Token* type, else if (enum_type->isStandardType()) valuetype->fromLibraryType(enum_type->str(), settings); } else - valuetype->type = getEnumType(valuetype->typeScope, settings.platform); + valuetype->type = getEnumTypeCached(valuetype->typeScope, settings.platform); } else valuetype->type = ValueType::Type::RECORD; const bool cpp = type->isCpp(); @@ -7776,7 +7796,7 @@ static const Token* parsedecl(const Token* type, else if (!valuetype->typeScope && type->type() && type->type()->classScope && valuetype->type != ValueType::Type::SMART_POINTER) { if (type->type()->classScope->type == ScopeType::eEnum) { valuetype->sign = ValueType::Sign::SIGNED; - valuetype->type = getEnumType(type->type()->classScope, settings.platform); + valuetype->type = getEnumTypeCached(type->type()->classScope, settings.platform); } else { valuetype->type = ValueType::Type::RECORD; } @@ -7966,6 +7986,17 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to if (!tokens) tokens = mTokenizer.list.front(); + std::unordered_map enumTypes; + struct EnumTypeCacheGuard { + explicit EnumTypeCacheGuard(std::unordered_map* cache) { + enumTypeCache = cache; + } + ~EnumTypeCacheGuard() { + enumTypeCache = prev; + } + std::unordered_map* prev{enumTypeCache}; + } enumTypeCacheGuard(&enumTypes); + for (Token *tok = tokens; tok; tok = tok->next()) tok->setValueType(nullptr); diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index bbcd1bd6b89..54066f3a613 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -4393,7 +4393,15 @@ void VariableMap::addVariable(const std::string& varname, bool globalNamespace) /** * @throws Token* thrown when closing brackets are missing */ -static bool setVarIdParseDeclaration(Token*& tok, const VariableMap& variableMap, bool executableScope, Standards::cstd_t cStandard) +namespace { + // Result of the last backward search from a comma in setVarIdParseDeclaration() + struct CommaSearchCache { + const Token* comma = nullptr; + const Token* result = nullptr; + }; +} + +static bool setVarIdParseDeclaration(Token*& tok, const VariableMap& variableMap, bool executableScope, Standards::cstd_t cStandard, CommaSearchCache& commaCache) { const Token* const tok1 = tok; Token* tok2 = tok; @@ -4484,11 +4492,19 @@ static bool setVarIdParseDeclaration(Token*& tok, const VariableMap& variableMap { const Token *tok3 = tok->previous(); if (tok3 && tok3->str() == ",") { + // In a long comma separated list the search from the previous comma gives the same result + const Token* const comma = tok3; while (tok3 && !Token::Match(tok3,";|(|[|{")) { + if (tok3 != comma && tok3 == commaCache.comma) { + tok3 = commaCache.result; + break; + } if (Token::Match(tok3, ")|]")) tok3 = tok3->link(); tok3 = tok3->previous(); } + commaCache.comma = comma; + commaCache.result = tok3; if (tok3 && executableScope && Token::Match(tok3->previous(), "%name% (")) { const Token *fdecl = tok3->previous(); @@ -4791,6 +4807,7 @@ void Tokenizer::setVarIdPass1() VariableMap variableMap; std::map> structMembers; + CommaSearchCache commaCache; std::stack scopeStack; @@ -4991,7 +5008,7 @@ void Tokenizer::setVarIdPass1() } try { /* Ticket #8151 */ - decl = setVarIdParseDeclaration(tok2, variableMap, scopeStack.top().isExecutable, mSettings.standards.c); + decl = setVarIdParseDeclaration(tok2, variableMap, scopeStack.top().isExecutable, mSettings.standards.c, commaCache); } catch (const Token * errTok) { syntaxError(errTok); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ed5e25c791d..95cfca4d14b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -122,6 +122,7 @@ #include #include #include +#include #include #include #include @@ -2007,6 +2008,12 @@ static void valueFlowForwardLifetime(Token * tok, const TokenList &tokenlist, Er } else if (tok->variable() && tok->variable()->scope()) { const Variable *var = tok->variable(); const Token *endOfVarScope = var->scope()->bodyEnd; + // Non-local variable: do not forward past the end of the current function + if (!var->isLocal() && !var->isArgument()) { + const Token* endOfFunction = ValueFlow::getEndOfExprScope(tok); + if (endOfFunction && (!endOfVarScope || precedes(endOfFunction, endOfVarScope))) + endOfVarScope = endOfFunction; + } std::list values = tok->values(); Token *nextExpression = nextAfterAstRightmostLeaf(parent); @@ -2829,6 +2836,9 @@ static bool isDecayedPointer(const Token *tok) return false; if (!tok->astParent()) return false; + // Subscript 'arr[i]': the element is accessed, the array itself is not used as a pointer + if (Token::simpleMatch(tok->astParent(), "[") && astIsLHS(tok)) + return false; if (Token::simpleMatch(tok->astParent(), "=") && astIsRHS(tok)) return true; if (astIsPointer(tok->astParent()) && !Token::simpleMatch(tok->astParent(), "return")) @@ -7268,6 +7278,8 @@ struct ValueFlowPass { virtual void run(const ValueFlowState& state) const = 0; // Returns true if pass needs C++ virtual bool cpp() const = 0; + // Returns true if pass does not analyse the skipped functions + virtual bool usesSkippedFunctions() const = 0; virtual ~ValueFlowPass() noexcept = default; }; @@ -7288,12 +7300,31 @@ struct ValueFlowPassRunner { }); } - bool run(std::initializer_list> passes) const + bool run(std::initializer_list> passes) { std::size_t values = 0; std::size_t n = state.settings().vfOptions.maxIterations; const bool doProgress = state.settings().reportProgress >= 0; + // A function is skipped in the next iteration when no value changed in the previous iteration + // and the passes that can skip it did not change it either. Other passes can remove a value that + // a skipping pass adds again; such a function must not be skipped. + const std::set skippedFunctions = state.skippedFunctions; + const std::vector& functionScopes = state.symboldatabase().functionScopes; + std::vector functionValues = getFunctionValues(); + std::vector changedBySkippingPass(functionScopes.size(), false); + bool first = true; while (n > 0 && values != getTotalValues()) { + if (!first) { + std::vector newFunctionValues = getFunctionValues(); + state.skippedFunctions = skippedFunctions; + for (std::size_t i = 0; i < functionScopes.size(); ++i) { + if (newFunctionValues[i] == functionValues[i] && !changedBySkippingPass[i]) + state.skippedFunctions.insert(functionScopes[i]); + } + functionValues = std::move(newFunctionValues); + } + first = false; + changedBySkippingPass.assign(functionScopes.size(), false); values = getTotalValues(); const std::size_t passnum = state.settings().vfOptions.maxIterations - n + 1; const std::string passnum_s = std::to_string(passnum); @@ -7301,11 +7332,21 @@ struct ValueFlowPassRunner { // the string concatination is a hot spot in TestIO::testScanfArgument and TestIO::testPrintfArgumentVariables std::string stage = doProgress ? std::string("ValueFlow::") + pass->name() + (' ' + passnum_s) : ""; ProgressReporter progressReporter(state.errorLogger, state.settings().reportProgress, state.tokenlist().getSourceFilePath(), std::move(stage)); - return run(pass, passnum); + if (!pass->usesSkippedFunctions()) + return run(pass, passnum); + const std::vector before = getFunctionValues(true); + const bool ret = run(pass, passnum); + const std::vector after = getFunctionValues(true); + for (std::size_t i = 0; i < functionScopes.size(); ++i) { + if (before[i] != after[i]) + changedBySkippingPass[i] = true; + } + return ret; })) return true; --n; } + state.skippedFunctions = skippedFunctions; if (state.settings().debugwarnings) { if (n == 0 && values != getTotalValues()) { ErrorMessage::FileLocation loc(state.tokenlist().getFiles()[0], 0, 0); @@ -7344,6 +7385,52 @@ struct ValueFlowPassRunner { return false; } + static std::size_t hashValue(const ValueFlow::Value& v) + { + std::size_t h = std::hash{}(v.intvalue); + const auto combine = [&h](std::size_t x) { + h ^= x + 0x9e3779b97f4a7c15ULL + (h << 6) + (h >> 2); + }; + combine(static_cast(v.valueType)); + combine(static_cast(v.valueKind)); + combine(static_cast(v.bound)); + combine(std::hash{}(v.tokvalue)); + combine(std::hash{}(v.condition)); + combine(std::hash{}(v.floatValue)); + combine(std::hash{}(v.varvalue)); + combine(std::hash{}(v.path)); + combine(v.varId); + combine(static_cast(v.conditional)); + combine(static_cast(v.moveKind)); + combine(static_cast(v.lifetimeKind)); + combine(static_cast(v.lifetimeScope)); + return h; + } + + // Hash of the values in each function scope (same order as symboldatabase.functionScopes) + // onlyAnalysed: the skipped functions get hash 0 + std::vector getFunctionValues(bool onlyAnalysed = false) const + { + const std::vector& functionScopes = state.symboldatabase().functionScopes; + std::vector result(functionScopes.size()); + for (std::size_t i = 0; i < functionScopes.size(); ++i) { + const Scope* scope = functionScopes[i]; + if (!scope->bodyStart) + continue; + if (onlyAnalysed && state.skippedFunctions.count(scope) != 0) + continue; + std::size_t h = 0; + for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { + const std::size_t k = (2 * reinterpret_cast(tok)) + 1; + h = std::accumulate(tok->values().cbegin(), tok->values().cend(), h, [&](std::size_t acc, const ValueFlow::Value& v) { + return acc + (hashValue(v) * k); + }); + } + result[i] = h; + } + return result; + } + std::size_t getTotalValues() const { std::size_t n = 1; @@ -7404,8 +7491,10 @@ template struct ValueFlowPassAdaptor : ValueFlowPass { const char* mName = nullptr; bool mCPP = false; + bool mUsesSkippedFunctions = false; F mRun; - ValueFlowPassAdaptor(const char* pname, bool pcpp, F prun) : ValueFlowPass(), mName(pname), mCPP(pcpp), mRun(prun) {} + ValueFlowPassAdaptor(const char* pname, bool pcpp, F prun) + : ValueFlowPass(), mName(pname), mCPP(pcpp), mUsesSkippedFunctions(std::strstr(pname, "skippedFunctions") != nullptr), mRun(prun) {} const char* name() const override { return mName; } @@ -7416,6 +7505,9 @@ struct ValueFlowPassAdaptor : ValueFlowPass { bool cpp() const override { return mCPP; } + bool usesSkippedFunctions() const override { + return mUsesSkippedFunctions; + } }; template diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index b8496c863d5..c31c12110d2 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -826,6 +826,23 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(true, testLifetimeOfX(code, 5, "a ;")); } + { + // lifetime of global array must not be forwarded into the next function + const char code[] = "int x[10];\n" + "void f() { int* p = x + 1; }\n" + "void g() { x[0] = 0; }\n"; + ASSERT_EQUALS(false, testLifetimeOfX(code, 3, "x [")); + } + + { + // array of pointers is not decayed when it is subscripted + const char code[] = "const char* x[10];\n" + "void f() {\n" + " x[0] = \"a\";\n" + "}\n"; + ASSERT_EQUALS(false, testLifetimeOfX(code, 3, "x [")); + } + { const char code[] = "void f() {\n" " std::vector v;\n" @@ -8171,6 +8188,20 @@ class TestValueFlow : public TestFixture { void valueFlowCrash() { const char* code; + code = "struct N {\n" // isAliasOf() was called with expr=nullptr + " N* r;\n" + " N root() const;\n" + " N f(const char* path, char d) const;\n" + "};\n" + "N N::f(const char* path, char d) const {\n" + " N c = path[0] == d ? root() : *this;\n" + " if (!c.r) return N();\n" + " const char* p = path;\n" + " while (*p == d) ++p;\n" + " return c;\n" + "}\n"; + (void)valueOfTok(code, "p"); + code = "void f(int x) {\n" " if (0 * (x > 2)) {}\n" "}\n";