[danmar]

Partial fix for #15174. These commits make ValueFlow faster without limiting the analysis: nothing is switched off or removed. This is a first step; making normal level give about the same warnings as exhaustive is left for later PRs.

• Commits 1 and 2 fix two cases where lifetime values were created or forwarded where they don't belong: a global variable's lifetime was forwarded through the rest of the file, and arr[i] was treated as a decayed pointer. Both were quadratic.
• Commits 3, 5 and 6 avoid repeated work (filter once, cache, reuse a search result). They give the same results.
• Commits 4 and 7 skip a function in the next ValueFlow iteration when none of its values changed, compared by a hash of the values.

Commits

1. valueFlowForwardLifetime: do not forward lifetime of non-local variable past end of function (colobot restext.cpp: ValueFlow 615 s → 5 s)
2. valueFlowLifetime: subscripted array is not a decayed pointer (spice rop3.c: ValueFlow 352 s → 28 s)
3. isAliasOf: filter the alias values once per reference
4. ValueFlow: skip functions without new values in the next iteration
5. SymbolDatabase: cache getEnumType() during setValueTypeInTokenList()
6. Tokenizer: avoid quadratic backward search for commas in setVarIdParseDeclaration()
7. ValueFlow: compare a hash of the values when skipping unchanged functions

Results on 10,019 Debian packages

This branch vs main, both with --check-level=exhaustive. The packages are random Debian source packages with 3–800 C/C++ files and at most 40 MB. Each package was checked on its own (-j1) with a 20-minute timeout. Times are whole Cppcheck CPU time.

Speed (9,853 packages that both versions finish)

main CPU per package	Packages	main	this PR	Saving
under 1 s	3,779	1,260 s	1,110 s	−12%
1–10 s	3,360	12,860 s	10,382 s	−19%
10–100 s	2,017	68,209 s	51,423 s	−25%
100 s – 20 min	697	230,263 s	158,253 s	−31%
all	9,853	87 h	61 h	−29%

• ValueFlow time alone: −33%.
• Median per-package CPU time is 81% of main (packages ≥ 1 s).
• 33 packages time out with main and finish with this PR.
• At normal level, a smaller run on 1,308 of the packages took about 23% less time.

Warnings

Of 4,962,744 warnings, 11 are lost and 55 are new. Every difference was reproduced and traced to a commit:

Change	Count	Commit	Correct?
lost danglingLifetime, objectIndex	11	1	false positives removed
new knownConditionTrueFalse	16	4	correct
new nullPointerOutOfMemory / nullPointerOutOfResources	9	2	correct
new uninitvar (simh), arrayIndexOutOfBoundsCond (funtools)	2	2	false positives
new, in mailfromd	28	–	noise: output differs between runs also with main

Why commit 4 gives more warnings: in main, running the passes again on an unchanged function can add a possible value to a comparison, and that keeps a known value from being set later. All 16 new warnings have this shape:

for (i = 0; i < 3; i++) {
    if (i == 0) ...
    else if (i == 1) ...
    else if (i == 2) ...   // Condition 'i==2' is always true
}

testrunner: 5,392 tests, 0 failures.

🤖 Generated with Claude Code

[dmcppcheck]

AI review

I found no problems that I could reproduce, and no warning changed compared with base on the inputs I tried. Testing found no false positives, false negatives or crashes from the lifetime changes, the alias filter, the enum-type cache or the comma-search cache. Two remarks from reading the code, both about the new "skip unchanged functions" logic:

• hashValue() leaves out some Value fields, at least indirect and container. If a value changes only in one of those fields, the function's hash stays the same and the function can be skipped by mistake.
• Whether a pass "uses skippedFunctions" is decided by looking for the text skippedFunctions in the pass name, which is the stringified macro argument. This is fragile.

No earlier review comments exist.

Testing

• Built cppcheck and testrunner in pr/, and cppcheck in base/.
• ./testrunner (all tests): 5392 tests, 0 failures. The new tests in TestValueFlow pass.
• Lifetime checks, compared with base. All gave the same results as base:
  • A global pointer or member pointer set to a local inside a nested block and used after the block in the same function. danglingLifetime and invalidLifetime are still reported.
  • The same inside a lambda inside a member function.
  • A global std::vector<int*> given arr[0] = &x through push_back, and returning arr[0] that holds &x. Both are still detected.
  • 2-D arrays int a[3][4]; int *p = a[1]; return p;. Base does not warn either, so this is not a regression.
• Corpus comparison, base vs PR, on test/cfg/.c and samples/ with --check-level=exhaustive --enable=warning,style,performance,portability: the output is identical.
• cppcheck's own sources, base vs PR, on lib/{mathlib,token,checkother,astutils,library,checkstl,programmemory,checkbufferoverrun}.cpp with --debug-warnings --library=cppcheck-lib:
  • Non-bailout output is identical, apart from line shifts in astutils.cpp, which the PR edits.
  • No "ValueFlow maximum iterations exceeded" message appeared.
  • Wall time went from 404 s (base) to 202 s (PR).
• I did not build a test input that gets skipped wrongly because of the hash fields it leaves out. That remark comes from reading the code.

---

Automated review of 4cc4ad2 by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

The hash leaves out indirect, container and a few other fields of ValueFlow::Value. If a pass replaces a value with one that differs only in indirect (for example indirect 0 vs 1 for uninit or null values) or only in container, the function's hash stays the same. The function is then skipped in the next iteration even though its values changed. Consider adding at least indirect and container to the hash.

[dmcppcheck]

AI review

Whether a pass honours skippedFunctions is found by searching for the text "skippedFunctions" in its stringified name. A pass that is renamed, or that gets the set through a helper without that literal text in the VFA(...) argument, is quietly misclassified. Then changedBySkippingPass is not tracked for it, which brings back the iteration problem that commit 4cc4ad2 fixed. An explicit flag in the VFA macro, such as a VFA_SKIP variant, would be more robust.

[danmar]

@pfultz2 I would be interested in your opinion about these tweaks. This is made by AI, however I feel it's pretty well tested and has very promising results. Some tweaks were needed to make CI happy so I am extending the testing, the scripts are measuring performance and warnings on more debian packages with the latest changes..

If your on-going work in valueflow/symboldatabase/etc will be messed up then feel free to say that we should delay this until your changes are in..

[danmar]

I could split this up into several PRs also if wanted.