From b60f30482d54be4755b218a57bc93c32c4adc4b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 4 Nov 2026 17:26:07 +0100 Subject: [PATCH 1/9] valueFlowForwardLifetime: do not forward lifetime of non-local variable past end of function For a global variable the scope body end is null so the lifetime was forwarded through the rest of the file, into unrelated functions. colobot restext.cpp: ValueFlow 615s => 5s. In 10000 Debian packages (--check-level=exhaustive) this removes 11 false positives (danglingLifetime, objectIndex) and adds no warnings. Co-Authored-By: Claude Opus 5.5 --- lib/valueflow.cpp | 6 ++++++ test/testvalueflow.cpp | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ed5e25c791d..6f13d632aac 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2007,6 +2007,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); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index b8496c863d5..cc820e49f8a 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -826,6 +826,14 @@ 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 [")); + } + { const char code[] = "void f() {\n" " std::vector v;\n" From 7e562f0424e463d1a66a2a391f74861edd3f5397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 4 Nov 2026 17:28:04 +0100 Subject: [PATCH 2/9] valueFlowLifetime: subscripted array is not a decayed pointer For 'arr[i]' where arr is an array of pointers, isDecayedPointer() returned true because the type of 'arr[i]' is a pointer. A lifetime value was then created and forwarded for each subscript; with many 'arr[N] = ...' statements in one function this is quadratic. spice rop3.c: ValueFlow 352s => 28s. In 10000 Debian packages (--check-level=exhaustive) this adds 11 warnings: 9 correct nullPointerOutOfMemory/nullPointerOutOfResources and 2 false positives (uninitvar, arrayIndexOutOfBoundsCond). Co-Authored-By: Claude Opus 5.5 --- lib/valueflow.cpp | 3 +++ test/testvalueflow.cpp | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 6f13d632aac..da06f21c587 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2835,6 +2835,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")) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index cc820e49f8a..8dd92344ca8 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -834,6 +834,15 @@ class TestValueFlow : public TestFixture { 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" From 9c1d989e7b26a8d5a4f65a375ac10606f3fad5b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 5 Nov 2026 09:03:24 +0100 Subject: [PATCH 3/9] isAliasOf: filter the alias values once per reference The values of the reference token were filtered again for every node in the expression. Filter them once and skip the AST search when there is nothing that can match. No changed warnings in a 50 package debian corpus, ValueFlow ~10% faster. Co-Authored-By: Claude Opus 5.5 --- lib/astutils.cpp | 31 +++++++++++++++++-------------- test/testvalueflow.cpp | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 14 deletions(-) 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/test/testvalueflow.cpp b/test/testvalueflow.cpp index 8dd92344ca8..c31c12110d2 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -8188,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"; From 128d874637e4be552de8be2f4934cd66d1547e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 5 Nov 2026 09:18:30 +0100 Subject: [PATCH 4/9] ValueFlow: skip functions without new values in the next iteration The iterated ValueFlow passes are run again for all functions until no new values are found. A function where no value changed in the previous iteration will not get new values from the intra-function passes, so skip it. Running the passes again on an unchanged function can add a possible value to a comparison, which then keeps a known value from being set later. In 10000 Debian packages (--check-level=exhaustive) this adds 16 correct knownConditionTrueFalse warnings and loses none. Co-Authored-By: Claude Opus 5.5 --- lib/valueflow.cpp | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index da06f21c587..4e754dc4124 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7297,12 +7297,27 @@ 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 where no value changed in the previous iteration is skipped in the next iteration + const std::set skippedFunctions = state.skippedFunctions; + std::vector functionValues = getFunctionValues(); + bool first = true; while (n > 0 && values != getTotalValues()) { + if (!first) { + std::vector newFunctionValues = getFunctionValues(); + state.skippedFunctions = skippedFunctions; + const std::vector& functionScopes = state.symboldatabase().functionScopes; + for (std::size_t i = 0; i < functionScopes.size(); ++i) { + if (newFunctionValues[i] == functionValues[i]) + state.skippedFunctions.insert(functionScopes[i]); + } + functionValues = std::move(newFunctionValues); + } + first = false; values = getTotalValues(); const std::size_t passnum = state.settings().vfOptions.maxIterations - n + 1; const std::string passnum_s = std::to_string(passnum); @@ -7315,6 +7330,7 @@ struct ValueFlowPassRunner { return true; --n; } + state.skippedFunctions = skippedFunctions; if (state.settings().debugwarnings) { if (n == 0 && values != getTotalValues()) { ErrorMessage::FileLocation loc(state.tokenlist().getFiles()[0], 0, 0); @@ -7353,6 +7369,21 @@ struct ValueFlowPassRunner { return false; } + // Number of values in each function scope (same order as symboldatabase.functionScopes) + std::vector getFunctionValues() 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; + for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) + result[i] += tok->values().size(); + } + return result; + } + std::size_t getTotalValues() const { std::size_t n = 1; From abe904f4fc4ddfb6f28c26f63aa5161ae7ffe4d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 5 Nov 2026 09:44:55 +0100 Subject: [PATCH 5/9] SymbolDatabase: cache getEnumType() during setValueTypeInTokenList() getEnumType() scans the whole enum body each time a token with enum type gets a valuetype. With big enums that are used a lot this is quadratic. Cache the result per enum scope while the valuetypes are set; the result is only cached when every literal it looked at already had a valuetype. crispy-doom: total time 124s => 78s. No changed warnings. Co-Authored-By: Claude Opus 5.5 --- lib/symboldatabase.cpp | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 26801640e99..ccfaab7411a 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,13 @@ 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) : prev(enumTypeCache) { enumTypeCache = cache; } + ~EnumTypeCacheGuard() { enumTypeCache = prev; } + std::unordered_map* prev; + } enumTypeCacheGuard(&enumTypes); + for (Token *tok = tokens; tok; tok = tok->next()) tok->setValueType(nullptr); From e2c76894727e5bda69d32a37cb058561dd23e53a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 5 Nov 2026 09:58:21 +0100 Subject: [PATCH 6/9] Tokenizer: avoid quadratic backward search for commas in setVarIdParseDeclaration() For a name after a comma the tokens before it were searched back to ';', '(', '[' or '{'. In a long comma separated list of names this search was repeated from every element. Reuse the result of the search from the previous comma. crispy-doom: total time 78s => 64s. No changed warnings. Co-Authored-By: Claude Opus 5.5 --- lib/tokenize.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) 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); } From 1b1100980522e89dba556e32ff0d4cfbb5d33b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 5 Nov 2026 20:11:23 +0100 Subject: [PATCH 7/9] ValueFlow: compare a hash of the values when skipping unchanged functions Skipping a function in the next iteration when its number of values did not change missed changes where a value is replaced or changes kind (possible -> known) without changing the count. For example a call whose return value got known in valueFlowFunctionReturn. Compare a hash of all values in the function instead. Co-Authored-By: Claude Opus 5.5 --- lib/valueflow.cpp | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 4e754dc4124..ca9a7951c10 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7369,7 +7369,29 @@ struct ValueFlowPassRunner { return false; } - // Number of values in each function scope (same order as symboldatabase.functionScopes) + 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) std::vector getFunctionValues() const { const std::vector& functionScopes = state.symboldatabase().functionScopes; @@ -7378,8 +7400,12 @@ struct ValueFlowPassRunner { const Scope* scope = functionScopes[i]; if (!scope->bodyStart) continue; - for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) - result[i] += tok->values().size(); + std::size_t h = 0; + for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { + for (const ValueFlow::Value& v : tok->values()) + h += hashValue(v) * (2 * reinterpret_cast(tok) + 1); + } + result[i] = h; } return result; } From c60290f5f7ddb2a5be69659478b3fc608dacb99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 8 Nov 2026 13:25:05 +0100 Subject: [PATCH 8/9] Fix clang-tidy, selfcheck and uncrustify findings Co-Authored-By: Claude Opus 5.5 --- lib/symboldatabase.cpp | 10 +++++++--- lib/valueflow.cpp | 7 +++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index ccfaab7411a..8c21f59f447 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7988,9 +7988,13 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to std::unordered_map enumTypes; struct EnumTypeCacheGuard { - explicit EnumTypeCacheGuard(std::unordered_map* cache) : prev(enumTypeCache) { enumTypeCache = cache; } - ~EnumTypeCacheGuard() { enumTypeCache = prev; } - std::unordered_map* prev; + 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()) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ca9a7951c10..9f60914a141 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -122,6 +122,7 @@ #include #include #include +#include #include #include #include @@ -7402,8 +7403,10 @@ struct ValueFlowPassRunner { continue; std::size_t h = 0; for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { - for (const ValueFlow::Value& v : tok->values()) - h += hashValue(v) * (2 * reinterpret_cast(tok) + 1); + 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; } From 4cc4ad24e226aaae0fb4140b4ecb26c8fe367fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 8 Nov 2026 13:31:49 +0100 Subject: [PATCH 9/9] ValueFlow: do not skip a function that a skipping pass changed Only some passes skip the unchanged functions. Another pass can remove a value that a skipping pass then adds again, so the function's values look unchanged after the iteration. When such a function was skipped only the removal happened, and the values kept changing in the next iterations. In selfcheck lib/mathlib.cpp then needed 6 iterations instead of 4 and gave 'ValueFlow maximum iterations exceeded'. Now a function is skipped only if its values are unchanged and the skipping passes did not change it in the previous iteration. Co-Authored-By: Claude Opus 5.5 --- lib/valueflow.cpp | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9f60914a141..95cfca4d14b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7278,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; }; @@ -7303,22 +7305,26 @@ struct ValueFlowPassRunner { std::size_t values = 0; std::size_t n = state.settings().vfOptions.maxIterations; const bool doProgress = state.settings().reportProgress >= 0; - // A function where no value changed in the previous iteration is skipped in the next iteration + // 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; - const std::vector& functionScopes = state.symboldatabase().functionScopes; for (std::size_t i = 0; i < functionScopes.size(); ++i) { - if (newFunctionValues[i] == functionValues[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); @@ -7326,7 +7332,16 @@ 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; @@ -7393,7 +7408,8 @@ struct ValueFlowPassRunner { } // Hash of the values in each function scope (same order as symboldatabase.functionScopes) - std::vector getFunctionValues() const + // 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()); @@ -7401,6 +7417,8 @@ struct ValueFlowPassRunner { 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; @@ -7473,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; } @@ -7485,6 +7505,9 @@ struct ValueFlowPassAdaptor : ValueFlowPass { bool cpp() const override { return mCPP; } + bool usesSkippedFunctions() const override { + return mUsesSkippedFunctions; + } }; template