From 40cdf41d2f79e89eaa60589494a1ca83cf751957 Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 10 Jul 2022 16:47:00 -0500 Subject: [PATCH 01/29] Fork condition branches --- lib/forwardanalyzer.cpp | 120 +++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 57 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 8a3bc6105fc..7467909d700 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -90,6 +90,9 @@ struct ForwardTraversal { bool isDead() const { return action.isModified() || action.isInconclusive() || isEscape(); } + bool hasGoto() const { + return endBlock ? ForwardTraversal::hasGoto(endBlock) : false; + } }; bool stopUpdates() { @@ -389,6 +392,35 @@ struct ForwardTraversal { return bail; } + Progress updateBranch(Branch& branch, int depth) + { + // Save and reset actions + Analyzer::Action prevActions = actions; + actions = Analyzer::Action::None; + Progress p = updateRange(branch.endBlock->link(), branch.endBlock, depth); + branch.action |= actions; + // Restore actions + actions |= prevActions; + + if (terminate == Analyzer::Terminate::Escape) { + branch.escape = true; + branch.escapeUnknown = false; + } else if (terminate != Analyzer::Terminate::None) { + if (terminate != Analyzer::Terminate::Escape) { + bool unknown = false; + if (isEscapeScope(branch.endBlock, unknown)) { + branch.escape = true; + branch.escapeUnknown = unknown; + } + } + if (terminate != Analyzer::Terminate::Modified) { + branch.action |= analyzeScope(branch.endBlock); + } + } + + return p; + } + bool reentersLoop(Token* endBlock, const Token* condTok, const Token* stepTok) { if (!condTok) return true; @@ -697,71 +729,45 @@ struct ForwardTraversal { if (!thenBranch.check && !elseBranch.check && analyzer->stopOnCondition(condTok) && stopUpdates()) return Break(Analyzer::Terminate::Conditional); bool hasElse = Token::simpleMatch(endBlock, "} else {"); - bool bail = false; - - // Traverse then block - thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown); + tok = hasElse ? endBlock->linkAt(2) : endBlock; if (thenBranch.check) { - thenBranch.active = true; - if (updateRange(endCond->next(), endBlock, depth - 1) == Progress::Break) + if (updateRange(thenBranch.endBlock->link(), thenBranch.endBlock, depth - 1) == Progress::Break) return Break(); - } else if (!elseBranch.check) { - thenBranch.active = true; - if (checkBranch(thenBranch)) - bail = true; - } - // Traverse else block - if (hasElse) { - elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown); - if (elseBranch.check) { - elseBranch.active = true; - Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2), depth - 1); - if (result == Progress::Break) + } else if (elseBranch.check) { + if (elseBranch.endBlock && updateRange(elseBranch.endBlock->link(), elseBranch.endBlock, depth - 1) == Progress::Break) + return Break(); + } else { + const bool conditional = analyzer->isConditional(); + ForwardTraversal ft = fork(); + ft.analyzer->assume(condTok, true); + Progress p = ft.updateBranch(thenBranch, depth - 1); + + analyzer->assume(condTok, false); + if (hasElse) { + if (updateBranch(elseBranch, depth - 1) == Progress::Break) return Break(); - } else if (!thenBranch.check) { - elseBranch.active = true; - if (checkBranch(elseBranch)) - bail = true; } - tok = endBlock->linkAt(2); - } else { - tok = endBlock; - } - if (thenBranch.active) - actions |= thenBranch.action; - if (elseBranch.active) - actions |= elseBranch.action; - if (bail) - return Break(Analyzer::Terminate::Bail); - if (thenBranch.isDead() && elseBranch.isDead()) { - if (thenBranch.isModified() && elseBranch.isModified()) - return Break(Analyzer::Terminate::Modified); - if (thenBranch.isConclusiveEscape() && elseBranch.isConclusiveEscape()) - return Break(Analyzer::Terminate::Escape); - return Break(Analyzer::Terminate::Bail); - } - // Conditional return - if (thenBranch.active && thenBranch.isEscape() && !hasElse) { - if (!thenBranch.isConclusiveEscape()) { + if (thenBranch.isDead() || elseBranch.isDead()) { + if (conditional && stopUpdates()) + return Break(Analyzer::Terminate::Conditional); + } + if (thenBranch.isModified() || elseBranch.isModified()) { + if (!analyzer->lowerToPossible()) + return Break(Analyzer::Terminate::Bail); + if (!ft.analyzer->lowerToPossible()) + p = Progress::Break; + } + if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) { if (!analyzer->lowerToInconclusive()) return Break(Analyzer::Terminate::Bail); - } else if (thenBranch.check) { - return Break(); - } else { - if (analyzer->isConditional() && stopUpdates()) - return Break(Analyzer::Terminate::Conditional); - analyzer->assume(condTok, false); + if (!ft.analyzer->lowerToInconclusive()) + p = Progress::Break; } - } - if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) { - if (!analyzer->lowerToInconclusive()) + if (thenBranch.hasGoto() || elseBranch.hasGoto()) { return Break(Analyzer::Terminate::Bail); - } else if (thenBranch.isModified() || elseBranch.isModified()) { - if (!hasElse && analyzer->isConditional() && stopUpdates()) - return Break(Analyzer::Terminate::Conditional); - if (!analyzer->lowerToPossible()) - return Break(Analyzer::Terminate::Bail); - analyzer->assume(condTok, elseBranch.isModified()); + } + if (p != Progress::Break) + ft.updateRange(thenBranch.endBlock, end, depth - 1); } } } else if (Token::simpleMatch(tok, "try {")) { From 1b80983774695200488ba108f3c9a72f3fd8821f Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 22 Aug 2022 18:00:37 -0500 Subject: [PATCH 02/29] Check for exit|abort --- lib/astutils.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index dc1f6ac7a46..f92b9691513 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1906,6 +1906,8 @@ bool isEscapeFunction(const Token* ftok, const Library* library) { if (!Token::Match(ftok, "%name% (")) return false; + if (Token::Match(ftok, "exit|abort")) + return true; const Function* function = ftok->function(); if (function) { if (function->isEscapeFunction()) From a9d518e3ab62c9417598aec9e6cdf89733a5f0bd Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 25 Jun 2026 13:07:54 -0500 Subject: [PATCH 03/29] Fix remaining tests --- lib/forwardanalyzer.cpp | 37 +++++++++++++++++++++++++++++++++---- test/testautovariables.cpp | 2 +- test/testother.cpp | 2 +- test/teststl.cpp | 4 ++-- test/testuninitvar.cpp | 2 +- 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index baafbe11da2..a4e71fc63a0 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -412,8 +412,23 @@ namespace { return bail; } - Progress updateBranch(Branch& branch, int depth) + Progress updateBranch(Branch& branch, int depth, bool flow) { + // Determine the branch's effect on the value without reporting (read-only) + branch.action |= analyzeScope(branch.endBlock); + // Mirror checkBranch()/tryForkUpdateScope(): only flow the value into the branch when + // the analyzer allows it. A conditional value (e.g. a possible null carried under an + // unrelated condition) must not be propagated into the branch, otherwise it produces + // false positives. 'flow' is evaluated on the unassumed value by the caller. + if (!flow) { + bool unknown = false; + if (isEscapeScope(branch.endBlock, unknown)) { + branch.escape = true; + branch.escapeUnknown = unknown; + } + return Progress::Continue; + } + // Save and reset actions Analyzer::Action prevActions = actions; actions = Analyzer::Action::None; @@ -765,20 +780,34 @@ namespace { const bool hasElse = Token::simpleMatch(endBlock, "} else {"); tok = hasElse ? endBlock->linkAt(2) : endBlock; if (thenBranch.check) { + // The condition is only "known" because of an earlier assumption, so the + // skipped else block could still modify the value -> lower to possible + if (analyzer->isConditional() && hasElse && analyzeScope(elseBranch.endBlock).isModified() && + !analyzer->lowerToPossible()) + return Break(Analyzer::Terminate::Bail); if (updateScope(thenBranch.endBlock, depth - 1) == Progress::Break) return Break(); } else if (elseBranch.check) { + // Likewise the skipped then block could still modify the value + if (analyzer->isConditional() && analyzeScope(thenBranch.endBlock).isModified() && + !analyzer->lowerToPossible()) + return Break(Analyzer::Terminate::Bail); if (elseBranch.endBlock && updateScope(elseBranch.endBlock, depth - 1) == Progress::Break) return Break(); } else { - const bool conditional = analyzer->isConditional(); + const bool conditional = stopOnCondition(condTok); + // Decide whether the value may flow into each branch using the unassumed + // value (as checkBranch() does), before assume() makes the forks conditional + const bool flowThen = analyzer->updateScope(thenBranch.endBlock, analyzeScope(thenBranch.endBlock).isModified()); + const bool flowElse = hasElse && elseBranch.endBlock && + analyzer->updateScope(elseBranch.endBlock, analyzeScope(elseBranch.endBlock).isModified()); ForwardTraversal ft = fork(); ft.analyzer->assume(condTok, true); - Progress p = ft.updateBranch(thenBranch, depth - 1); + Progress p = ft.updateBranch(thenBranch, depth - 1, flowThen); analyzer->assume(condTok, false); if (hasElse) { - if (updateBranch(elseBranch, depth - 1) == Progress::Break) + if (updateBranch(elseBranch, depth - 1, flowElse) == Progress::Break) return Break(); } if (thenBranch.isDead() || elseBranch.isDead()) { diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 9fe3e126660..920cb315646 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -4848,7 +4848,7 @@ class TestAutoVariables : public TestFixture { " return *iPtr;\n" " return 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:5:16] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:5:16] -> [test.cpp:7:10] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", errout_str()); // #11753 check("int main(int argc, const char *argv[]) {\n" diff --git a/test/testother.cpp b/test/testother.cpp index b8134623a37..ded218e1d7a 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -11491,7 +11491,7 @@ class TestOther : public TestFixture { " x = a + b;\n" " return x;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:4:9]: (style) Variable 'x' is assigned an expression that holds the same value. [redundantAssignment]\n", + ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:3:9] -> [test.cpp:4:9]: (style) Variable 'x' is assigned an expression that holds the same value. [redundantAssignment]\n", errout_str()); } diff --git a/test/teststl.cpp b/test/teststl.cpp index 35941f6422c..3f0b36e2e21 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -368,7 +368,7 @@ class TestStl : public TestFixture { " if(b) ++x;\n" " return s[x];\n" "}"); - ASSERT_EQUALS("[test.cpp:5:13]: error: Out of bounds access in 's[x]', if 's' size is 6 and 'x' is 6 [containerOutOfBounds]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:5:13]: error: Out of bounds access in 's[x]', if 's' size is 6 and 'x' is 7 [containerOutOfBounds]\n", errout_str()); checkNormal("void f() {\n" " static const int N = 4;\n" @@ -2736,7 +2736,7 @@ class TestStl : public TestFixture { "}\n", s); ASSERT_EQUALS("[test.cpp:5:9]: warning: Array index -1 is out of bounds. [negativeContainerIndex]\n" "[test.cpp:8:8]: note: Calling function 'f', 1st argument '-1' value is -1\n" - "[test.cpp:3:9]: note: Assuming condition is false\n" + "[test.cpp:3:9]: note: Assuming condition is true\n" "[test.cpp:5:9]: note: Negative array index\n", errout_str()); } diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index e0c5512f1fd..c35a6c7be93 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4284,7 +4284,7 @@ class TestUninitVar : public TestFixture { " else y = 123;\n" // <- y is always initialized " return y;\n" "}"); - TODO_ASSERT_EQUALS("", "[test.cpp:5:9] -> [test.cpp:7:12]: (warning) Uninitialized variable: y [uninitvar]\n", errout_str()); + ASSERT_EQUALS("", errout_str()); // #4560: fork-based condition analysis tracks x==0 -> else branch -> y initialized valueFlowUninit("void f(int x) {\n" // #3948 " int value;\n" From 312193e43b263d94dc9d8006ce3d49237c26aa7f Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 25 Jun 2026 14:23:21 -0500 Subject: [PATCH 04/29] Remove flow check --- lib/forwardanalyzer.cpp | 26 +++----------------------- test/teststl.cpp | 2 +- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index a4e71fc63a0..f37336c95e6 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -412,23 +412,8 @@ namespace { return bail; } - Progress updateBranch(Branch& branch, int depth, bool flow) + Progress updateBranch(Branch& branch, int depth) { - // Determine the branch's effect on the value without reporting (read-only) - branch.action |= analyzeScope(branch.endBlock); - // Mirror checkBranch()/tryForkUpdateScope(): only flow the value into the branch when - // the analyzer allows it. A conditional value (e.g. a possible null carried under an - // unrelated condition) must not be propagated into the branch, otherwise it produces - // false positives. 'flow' is evaluated on the unassumed value by the caller. - if (!flow) { - bool unknown = false; - if (isEscapeScope(branch.endBlock, unknown)) { - branch.escape = true; - branch.escapeUnknown = unknown; - } - return Progress::Continue; - } - // Save and reset actions Analyzer::Action prevActions = actions; actions = Analyzer::Action::None; @@ -796,18 +781,13 @@ namespace { return Break(); } else { const bool conditional = stopOnCondition(condTok); - // Decide whether the value may flow into each branch using the unassumed - // value (as checkBranch() does), before assume() makes the forks conditional - const bool flowThen = analyzer->updateScope(thenBranch.endBlock, analyzeScope(thenBranch.endBlock).isModified()); - const bool flowElse = hasElse && elseBranch.endBlock && - analyzer->updateScope(elseBranch.endBlock, analyzeScope(elseBranch.endBlock).isModified()); ForwardTraversal ft = fork(); ft.analyzer->assume(condTok, true); - Progress p = ft.updateBranch(thenBranch, depth - 1, flowThen); + Progress p = ft.updateBranch(thenBranch, depth - 1); analyzer->assume(condTok, false); if (hasElse) { - if (updateBranch(elseBranch, depth - 1, flowElse) == Progress::Break) + if (updateBranch(elseBranch, depth - 1) == Progress::Break) return Break(); } if (thenBranch.isDead() || elseBranch.isDead()) { diff --git a/test/teststl.cpp b/test/teststl.cpp index 3f0b36e2e21..107ef9e444b 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2736,7 +2736,7 @@ class TestStl : public TestFixture { "}\n", s); ASSERT_EQUALS("[test.cpp:5:9]: warning: Array index -1 is out of bounds. [negativeContainerIndex]\n" "[test.cpp:8:8]: note: Calling function 'f', 1st argument '-1' value is -1\n" - "[test.cpp:3:9]: note: Assuming condition is true\n" + "[test.cpp:3:9]: note: Assuming condition is false\n" "[test.cpp:5:9]: note: Negative array index\n", errout_str()); } From 293ae8c8f820b88f49925557dfad111bdf339688 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 25 Jun 2026 19:48:43 -0500 Subject: [PATCH 05/29] Fix valueflow issue --- lib/forwardanalyzer.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index f37336c95e6..569d46d2304 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -785,7 +785,10 @@ namespace { ft.analyzer->assume(condTok, true); Progress p = ft.updateBranch(thenBranch, depth - 1); - analyzer->assume(condTok, false); + // Only commit the condition as false on the main path when it actually + // matters + if (thenBranch.isDead()) + analyzer->assume(condTok, false); if (hasElse) { if (updateBranch(elseBranch, depth - 1) == Progress::Break) return Break(); From 0239dd14a78f32e4c0590cc484d14c753d17a848 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 26 Jun 2026 09:18:20 -0500 Subject: [PATCH 06/29] Handle FP --- lib/forwardanalyzer.cpp | 50 ++++++++++++++++++++++++----------------- test/testuninitvar.cpp | 2 +- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 569d46d2304..438715c9c26 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -425,15 +425,12 @@ namespace { if (terminate == Analyzer::Terminate::Escape) { branch.escape = true; branch.escapeUnknown = false; - } else if (terminate != Analyzer::Terminate::None) { - if (terminate != Analyzer::Terminate::Escape) { - bool unknown = false; - if (isEscapeScope(branch.endBlock, unknown)) { - branch.escape = true; - branch.escapeUnknown = unknown; - } - } - if (terminate != Analyzer::Terminate::Modified) { + } else { + // Detect an escape that the traversal did not flag (e.g. an unknown noreturn call); + // isEscapeScope also reports a possible (unknown) escape via escapeUnknown. Such a + // branch may not fall through, so the fork must not continue past it. + branch.escape = isEscapeScope(branch.endBlock, branch.escapeUnknown); + if (terminate != Analyzer::Terminate::None && terminate != Analyzer::Terminate::Modified) { branch.action |= analyzeScope(branch.endBlock); } } @@ -678,11 +675,20 @@ namespace { const bool inElse = scope->type == ScopeType::eElse; const bool inDoWhile = scope->type == ScopeType::eDo; const bool inLoop = contains({ScopeType::eDo, ScopeType::eFor, ScopeType::eWhile}, scope->type); + const bool hasElse = Token::simpleMatch(tok, "} else {"); Token* condTok = getCondTokFromEnd(tok); if (!condTok) return Break(); + // When leaving the 'then' branch, control only reaches here when the + // condition was true if the 'else' branch escapes (e.g. it returns). In that + // case the value established in 'then' is still definite, so keep it known + // instead of lowering it to possible. + bool elseEscape = false; + bool unknownEscape = false; + if (!inLoop && !inElse && hasElse) + elseEscape = isEscapeScope(tok->linkAt(2), unknownEscape); if (!condTok->hasKnownIntValue() || inLoop) { - if (!analyzer->lowerToPossible()) + if (!elseEscape && !analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); } else if (condTok->getKnownIntValue() == inElse) { return Break(); @@ -707,7 +713,7 @@ namespace { } analyzer->assume(condTok, !inElse, Analyzer::Assume::Quiet); assert(!inDoWhile || Token::simpleMatch(tok, "} while (")); - if (Token::simpleMatch(tok, "} else {") || inDoWhile) + if (hasElse || inDoWhile) tok = tok->linkAt(2); } else if (contains({ScopeType::eTry, ScopeType::eCatch}, scope->type)) { if (!analyzer->lowerToPossible()) @@ -789,31 +795,35 @@ namespace { // matters if (thenBranch.isDead()) analyzer->assume(condTok, false); - if (hasElse) { - if (updateBranch(elseBranch, depth - 1) == Progress::Break) - return Break(); - } + // The else block is traversed on the main path. If it kills the value + // (modified) the main path stops, but the then-fork may still carry the + // value forward, so defer the break until after the fork continues. + Progress pElse = Progress::Continue; + if (hasElse) + pElse = updateBranch(elseBranch, depth - 1); if (thenBranch.isDead() || elseBranch.isDead()) { if (conditional && stopUpdates()) return Break(Analyzer::Terminate::Conditional); } if (thenBranch.isModified() || elseBranch.isModified()) { - if (!analyzer->lowerToPossible()) - return Break(Analyzer::Terminate::Bail); if (!ft.analyzer->lowerToPossible()) p = Progress::Break; + if (pElse != Progress::Break && !analyzer->lowerToPossible()) + return Break(Analyzer::Terminate::Bail); } if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) { - if (!analyzer->lowerToInconclusive()) - return Break(Analyzer::Terminate::Bail); if (!ft.analyzer->lowerToInconclusive()) p = Progress::Break; + if (pElse != Progress::Break && !analyzer->lowerToInconclusive()) + return Break(Analyzer::Terminate::Bail); } if (thenBranch.hasGoto() || elseBranch.hasGoto()) { return Break(Analyzer::Terminate::Bail); } - if (p != Progress::Break) + if (p != Progress::Break && !thenBranch.isEscape()) ft.updateRange(thenBranch.endBlock, end, depth - 1); + if (pElse == Progress::Break) + return Break(); } } } else if (Token::simpleMatch(tok, "try {")) { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index c35a6c7be93..30ff49b1808 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4265,7 +4265,7 @@ class TestUninitVar : public TestFixture { " else {}\n" " return y;\n" "}"); - TODO_ASSERT_EQUALS("", "[test.cpp:5:9] -> [test.cpp:7:12]: (warning) Uninitialized variable: y [uninitvar]\n", errout_str()); + ASSERT_EQUALS("", errout_str()); // #4560: escaping else keeps x known, so x is true and y is initialized valueFlowUninit("int f(int a) {\n" // #6583 " int x;\n" From 145858a305452b3def23c330dca44ce8adc90f0c Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 26 Jun 2026 09:38:04 -0500 Subject: [PATCH 07/29] Rename variable --- lib/forwardanalyzer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 438715c9c26..3df32ed7a07 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -789,7 +789,7 @@ namespace { const bool conditional = stopOnCondition(condTok); ForwardTraversal ft = fork(); ft.analyzer->assume(condTok, true); - Progress p = ft.updateBranch(thenBranch, depth - 1); + Progress pThen = ft.updateBranch(thenBranch, depth - 1); // Only commit the condition as false on the main path when it actually // matters @@ -807,20 +807,20 @@ namespace { } if (thenBranch.isModified() || elseBranch.isModified()) { if (!ft.analyzer->lowerToPossible()) - p = Progress::Break; + pThen = Progress::Break; if (pElse != Progress::Break && !analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); } if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) { if (!ft.analyzer->lowerToInconclusive()) - p = Progress::Break; + pThen = Progress::Break; if (pElse != Progress::Break && !analyzer->lowerToInconclusive()) return Break(Analyzer::Terminate::Bail); } if (thenBranch.hasGoto() || elseBranch.hasGoto()) { return Break(Analyzer::Terminate::Bail); } - if (p != Progress::Break && !thenBranch.isEscape()) + if (pThen != Progress::Break && !thenBranch.isEscape()) ft.updateRange(thenBranch.endBlock, end, depth - 1); if (pElse == Progress::Break) return Break(); From 63b81eae2ab931dbfa3352056279e1e8cba094c1 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 26 Jun 2026 09:39:22 -0500 Subject: [PATCH 08/29] Dont stop when else breaks --- lib/forwardanalyzer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 3df32ed7a07..a57aca13d0f 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -803,19 +803,19 @@ namespace { pElse = updateBranch(elseBranch, depth - 1); if (thenBranch.isDead() || elseBranch.isDead()) { if (conditional && stopUpdates()) - return Break(Analyzer::Terminate::Conditional); + pElse = Break(Analyzer::Terminate::Conditional); } if (thenBranch.isModified() || elseBranch.isModified()) { if (!ft.analyzer->lowerToPossible()) pThen = Progress::Break; if (pElse != Progress::Break && !analyzer->lowerToPossible()) - return Break(Analyzer::Terminate::Bail); + pElse = Break(Analyzer::Terminate::Bail); } if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) { if (!ft.analyzer->lowerToInconclusive()) pThen = Progress::Break; if (pElse != Progress::Break && !analyzer->lowerToInconclusive()) - return Break(Analyzer::Terminate::Bail); + pElse = Break(Analyzer::Terminate::Bail); } if (thenBranch.hasGoto() || elseBranch.hasGoto()) { return Break(Analyzer::Terminate::Bail); From 2a47ac28f7725ccced6edeff4fe281b76dbf00a8 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 26 Jun 2026 09:42:51 -0500 Subject: [PATCH 09/29] Check known condition --- lib/forwardanalyzer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index a57aca13d0f..f9f8602f7f8 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -773,14 +773,14 @@ namespace { if (thenBranch.check) { // The condition is only "known" because of an earlier assumption, so the // skipped else block could still modify the value -> lower to possible - if (analyzer->isConditional() && hasElse && analyzeScope(elseBranch.endBlock).isModified() && + if (!condTok->hasKnownIntValue() && hasElse && analyzeScope(elseBranch.endBlock).isModified() && !analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); if (updateScope(thenBranch.endBlock, depth - 1) == Progress::Break) return Break(); } else if (elseBranch.check) { // Likewise the skipped then block could still modify the value - if (analyzer->isConditional() && analyzeScope(thenBranch.endBlock).isModified() && + if (!condTok->hasKnownIntValue() && analyzeScope(thenBranch.endBlock).isModified() && !analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); if (elseBranch.endBlock && updateScope(elseBranch.endBlock, depth - 1) == Progress::Break) From e71dc890af460293bd1a164c3289ed390a3920a4 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 26 Jun 2026 10:23:28 -0500 Subject: [PATCH 10/29] Check update scope --- lib/forwardanalyzer.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index f9f8602f7f8..d29c6aeb547 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -787,7 +787,11 @@ namespace { return Break(); } else { const bool conditional = stopOnCondition(condTok); - ForwardTraversal ft = fork(); + // The value only flows into the then-branch when the condition can split + // it. For an opaque or correlated condition (e.g. 'if (f(x))' or + // 'if (do_write)') it does not really reach there, so fork in analyze-only + // mode: the branch's effect is still tracked but nothing is reported in it. + ForwardTraversal ft = fork(!analyzer->updateScope(thenBranch.endBlock, false)); ft.analyzer->assume(condTok, true); Progress pThen = ft.updateBranch(thenBranch, depth - 1); From c111d2e5aa5d49b930e5c216ca28c8301a8b2844 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 26 Jun 2026 19:33:51 -0500 Subject: [PATCH 11/29] Update assume logic --- lib/analyzer.h | 3 +++ lib/forwardanalyzer.cpp | 9 ++++++--- lib/vf_analyzers.cpp | 6 +++++- test/testvalueflow.cpp | 6 +++--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index a4546eb7dc4..ab4eb8589bc 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -156,6 +156,9 @@ struct Analyzer { Quiet = (1 << 0), Absolute = (1 << 1), ContainerEmpty = (1 << 2), + // Do not record the program state at the branch boundaries. Used when assuming a + // condition before the branch is traversed, where those states would be premature. + NoState = (1 << 3), }; }; diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index d29c6aeb547..9353673b821 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -792,13 +792,16 @@ namespace { // 'if (do_write)') it does not really reach there, so fork in analyze-only // mode: the branch's effect is still tracked but nothing is reported in it. ForwardTraversal ft = fork(!analyzer->updateScope(thenBranch.endBlock, false)); - ft.analyzer->assume(condTok, true); + // The branch is traversed below, so don't record its boundary state here. + ft.analyzer->assume(condTok, true, Analyzer::Assume::NoState); Progress pThen = ft.updateBranch(thenBranch, depth - 1); // Only commit the condition as false on the main path when it actually - // matters + // matters. With an else the else block is traversed below (so suppress the + // boundary state); without one the false path continues past the closing + // brace and must record the assumed state there. if (thenBranch.isDead()) - analyzer->assume(condTok, false); + analyzer->assume(condTok, false, hasElse ? Analyzer::Assume::NoState : Analyzer::Assume::None); // The else block is traversed on the main path. If it kills the value // (modified) the main path stops, but the then-fork may still carry the // value forward, so defer the break until after the fork continues. diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 10efc6d6f39..ff9d3153ff2 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -735,7 +735,7 @@ struct ValueFlowAnalyzer : Analyzer { isCondBlock = Token::Match(parent->previous(), "if|while ("); } - if (isCondBlock) { + if (isCondBlock && !(flags & Assume::NoState)) { const Token* startBlock = parent->link()->next(); if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while (")) startBlock = parent->linkAt(-2); @@ -746,6 +746,10 @@ struct ValueFlowAnalyzer : Analyzer { } else { if (Token::simpleMatch(endBlock, "} else {")) pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); + else + // No else: the false path continues past the closing brace, so record the + // assumed state there so it is available to the rest of the enclosing scope. + pms.addState(endBlock, getProgramState()); } } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 8305a63bc56..dd179a51c80 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6097,9 +6097,9 @@ class TestValueFlow : public TestFixture { " c++;\n" "}\n"; values = tokenValues(code, "c ++ ; }"); - TODO_ASSERT_EQUALS(true, false, values.size() == 2); - // ASSERT_EQUALS(true, values.front().isUninitValue() || values.back().isUninitValue()); - // ASSERT_EQUALS(true, values.front().isPossible() || values.back().isPossible()); + ASSERT_EQUALS(true, values.size() == 2); + ASSERT_EQUALS(true, values.front().isUninitValue() || values.back().isUninitValue()); + ASSERT_EQUALS(true, values.front().isPossible() || values.back().isPossible()); // ASSERT_EQUALS(true, values.front().intvalue == 0 || values.back().intvalue == 0); code = "void b(bool d, bool e) {\n" From a216f27020832ec19c609d1f2e83e2790944867c Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 28 Jun 2026 16:04:53 -0500 Subject: [PATCH 12/29] Update to handle the vars in execute --- lib/analyzer.h | 4 +++ lib/programmemory.cpp | 68 +++++++++++++++++++++++++++++----------- lib/programmemory.h | 11 ++++--- lib/vf_analyzers.cpp | 40 ++++++++++++++++------- test/testnullpointer.cpp | 5 +++ test/testuninitvar.cpp | 39 +++++++++++++++++++++++ 6 files changed, 133 insertions(+), 34 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index ab4eb8589bc..fec9059d947 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -158,6 +158,10 @@ struct Analyzer { ContainerEmpty = (1 << 2), // Do not record the program state at the branch boundaries. Used when assuming a // condition before the branch is traversed, where those states would be premature. + // When this is not set the branch has already been traversed and control is continuing + // past it, so the assumed state is anchored at the block's end (see the analyzer's + // assume()): this keeps assumptions on variables modified inside the block from being + // discarded as "modified" once control leaves it. NoState = (1 << 3), }; }; diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index b86ca2d8812..970fe6aa649 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -262,26 +262,26 @@ ProgramMemory::Map::iterator ProgramMemory::find(nonneg int exprid) return mValues->find(ExprIdToken::create(exprid)); } -static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings); +static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars = {}); -static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, const Settings& settings) +static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars = {}) { if (!condition) return false; MathLib::bigint result = 0; bool error = false; - execute(condition, pm, &result, &error, settings); + execute(condition, pm, &result, &error, settings, vars); return !error && result == r; } -bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings) +bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars) { - return evaluateCondition(0, condition, pm, settings); + return evaluateCondition(0, condition, pm, settings, vars); } -bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings) +bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars) { - return evaluateCondition(1, condition, pm, settings); + return evaluateCondition(1, condition, pm, settings, vars); } static bool frontIs(const std::vector& v, bool i) @@ -443,7 +443,7 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok if (!pm.hasValue(vartok->exprId())) { const Token* valuetok = tok2->astOperand2(); ProgramMemory local = state; - pm.setValue(vartok, execute(valuetok, local, settings)); + pm.setValue(vartok, execute(valuetok, local, settings, vars)); } } } else if (Token::simpleMatch(tok2, ")") && tok2->link() && @@ -547,19 +547,21 @@ void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& va replace(std::move(local), tok); } -void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty) +void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty, const Token* origin) { ProgramMemory pm = state; if (isEmpty) pm.setContainerSizeValue(tok, 0, b); else programMemoryParseCondition(pm, tok, nullptr, settings, b); - const Token* origin = tok; - const Token* top = tok->astTop(); - if (Token::Match(top->previous(), "for|while|if (") && !Token::simpleMatch(tok->astParent(), "?")) { - origin = top->link()->next(); - if (!b && origin->link()) { - origin = origin->link(); + if (!origin) { + origin = tok; + const Token* top = tok->astTop(); + if (Token::Match(top->previous(), "for|while|if (") && !Token::simpleMatch(tok->astParent(), "?")) { + origin = top->link()->next(); + if (!b && origin->link()) { + origin = origin->link(); + } } } replace(std::move(pm), origin); @@ -1316,6 +1318,10 @@ namespace { struct Executor { ProgramMemory* pm; const Settings& settings; + // The values that are being tracked by the forward/reverse analysis. These take precedence + // over what is stored in the program memory: a tracked value is the authoritative current + // value of its expression, so any cached value that depends on it must be re-evaluated. + const ProgramMemory::Map* vars = nullptr; int fdepth = 4; int depth = 10; @@ -1324,6 +1330,25 @@ namespace { assert(pm != nullptr); } + // Is the tracked value for this expression available? + const ValueFlow::Value* getTrackedValue(const Token* expr) const { + if (!vars || expr->exprId() == 0) + return nullptr; + const auto it = vars->find(ExprIdToken::create(expr->exprId())); + return it == vars->end() ? nullptr : &it->second; + } + + // Does the expression read a tracked value? If so, any value cached in the program memory + // for it may be stale (the tracked value may have changed since it was cached), so it must + // be re-evaluated rather than served from the cache. + bool dependsOnTrackedValue(const Token* expr) const { + if (!vars || vars->empty()) + return false; + return findAstNode(expr, [&](const Token* tok) { + return getTrackedValue(tok) != nullptr; + }) != nullptr; + } + static ValueFlow::Value unknown() { return ValueFlow::Value::unknown(); } @@ -1622,7 +1647,10 @@ namespace { } return execute(expr->astOperand1()); } - if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) { + // A tracked value is the authoritative current value of its expression. + if (const ValueFlow::Value* tracked = getTrackedValue(expr)) + return *tracked; + if (expr->exprId() > 0 && pm->hasValue(expr->exprId()) && !dependsOnTrackedValue(expr)) { ValueFlow::Value result = utils::as_const(*pm).at(expr->exprId()); if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) { result.intvalue = !result.intvalue; @@ -1815,9 +1843,10 @@ namespace { }; } // namespace -static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings) +static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars) { Executor ex{&pm, settings}; + ex.vars = &vars; return ex.execute(expr); } @@ -1907,9 +1936,10 @@ void execute(const Token* expr, ProgramMemory& programMemory, MathLib::bigint* result, bool* error, - const Settings& settings) + const Settings& settings, + const ProgramMemory::Map& vars) { - ValueFlow::Value v = execute(expr, programMemory, settings); + ValueFlow::Value v = execute(expr, programMemory, settings, vars); if (!v.isIntValue() || v.isImpossible()) { if (error) *error = true; diff --git a/lib/programmemory.h b/lib/programmemory.h index ec24c0df59f..5896cda4aaa 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -171,7 +171,7 @@ struct ProgramMemoryState { void addState(const Token* tok, const ProgramMemory::Map& vars); - void assume(const Token* tok, bool b, bool isEmpty = false); + void assume(const Token* tok, bool b, bool isEmpty = false, const Token* origin = nullptr); void removeModifiedVars(const Token* tok); @@ -184,21 +184,24 @@ void execute(const Token* expr, ProgramMemory& programMemory, MathLib::bigint* result, bool* error, - const Settings& settings); + const Settings& settings, + const ProgramMemory::Map& vars = {}); /** * Is condition always false when variable has given value? * \param condition top ast token in condition * \param pm program memory + * \param vars optional tracked values that take precedence over the program memory */ -bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings); +bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars = {}); /** * Is condition always true when variable has given value? * \param condition top ast token in condition * \param pm program memory + * \param vars optional tracked values that take precedence over the program memory */ -bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings); +bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars = {}); /** * Get program memory by looking backwards from given token. diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index ff9d3153ff2..114f33dcc35 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -679,15 +679,19 @@ struct ValueFlowAnalyzer : Analyzer { return {v->intvalue}; std::vector result; ProgramMemory pm = getProgramMemoryFunc(); + // The tracked values are authoritative: pass them so a value cached in the program memory + // that depends on a tracked value (e.g. 'h(p)' after 'p' was reassigned) is re-evaluated + // rather than served stale. + const ProgramState vars = getProgramState(); if (Token::Match(tok, "&&|%oror%")) { - if (conditionIsTrue(tok, pm, getSettings())) + if (conditionIsTrue(tok, pm, getSettings(), vars)) result.push_back(1); - if (conditionIsFalse(tok, std::move(pm), getSettings())) + if (conditionIsFalse(tok, std::move(pm), getSettings(), vars)) result.push_back(0); } else { MathLib::bigint out = 0; bool error = false; - execute(tok, pm, &out, &error, getSettings()); + execute(tok, pm, &out, &error, getSettings(), vars); if (!error) result.push_back(out); } @@ -724,22 +728,36 @@ struct ValueFlowAnalyzer : Analyzer { } void assume(const Token* tok, bool state, unsigned int flags) override { - // Update program state - pms.removeModifiedVars(tok); - pms.addState(tok, getProgramState()); - pms.assume(tok, state, flags & Assume::ContainerEmpty); - bool isCondBlock = false; const Token* parent = tok->astParent(); if (parent) { isCondBlock = Token::Match(parent->previous(), "if|while ("); } - if (isCondBlock && !(flags & Assume::NoState)) { - const Token* startBlock = parent->link()->next(); + const Token* startBlock = nullptr; + const Token* endBlock = nullptr; + if (isCondBlock) { + startBlock = parent->link()->next(); if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while (")) startBlock = parent->linkAt(-2); - const Token* endBlock = startBlock->link(); + endBlock = startBlock->link(); + } + + // NoState is set only for the pre-traversal assume; without it the 'then' block has already + // been traversed and control is leaving it, so anchor the assumed state at the end of the + // block rather than at the condition. Assumptions about variables modified inside the block + // (e.g. an 'if' that narrows a value computed in the block) then survive past it, instead of + // being discarded because the variable was "modified" since the condition was evaluated. + const bool scopeEnd = !(flags & Assume::NoState) && state && endBlock; + const Token* anchor = scopeEnd ? endBlock : tok; + const Token* origin = scopeEnd ? endBlock : nullptr; + + // Update program state + pms.removeModifiedVars(anchor); + pms.addState(anchor, getProgramState()); + pms.assume(tok, state, flags & Assume::ContainerEmpty, origin); + + if (isCondBlock && !(flags & Assume::NoState) && !scopeEnd) { if (state) { pms.removeModifiedVars(endBlock); pms.addState(endBlock->previous(), getProgramState()); diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 369c04a2101..2483a967947 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2450,6 +2450,9 @@ class TestNullPointer : public TestFixture { void nullpointer77() { + // No warning: 'i' is passed to the unknown function 'h' in the same condition that guards the + // dereference. 'h' may validate the pointer (e.g. return false for null), so '*i' can be safe + // - this is the common "if (check(p) && p->...)" pattern, so we must not assume 'i' is null. check("bool h(int*);\n" "void f(int* i) {\n" " int* i = nullptr;\n" @@ -2465,6 +2468,8 @@ class TestNullPointer : public TestFixture { "}\n"); ASSERT_EQUALS("", errout_str()); + // Likewise here, even though 'i' is null when the first 'h(i)' was true: the second 'h(i)' is an + // independent call that may validate 'i', so '*i' is not necessarily a null dereference. check("bool h(int*);\n" "void f(int* x) {\n" " int* i = x;\n" diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 30ff49b1808..8931fb9bdc4 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -5614,6 +5614,45 @@ class TestUninitVar : public TestFixture { "}"); ASSERT_EQUALS("[test.cpp:18:9] -> [test.cpp:12:13] -> [test.cpp:8:15]: (warning) Uninitialized variable: s->flag [uninitvar]\n", errout_str()); + // A value narrowed by a nested condition (dimensions < 1 here) must survive the enclosing + // 'if' so a later correlated condition can be resolved: dimensions == 1 is then false, the + // function does not return, and the uninitialized read is reached. + valueFlowUninit("unsigned int g();\n" + "void f(bool a) {\n" + " unsigned int dimensions = 0;\n" + " bool mightBeLarger;\n" + " if (a) {\n" + " dimensions = g();\n" + " if (dimensions >= 1)\n" + " mightBeLarger = false;\n" + " } else {\n" + " mightBeLarger = false;\n" + " }\n" + " if (dimensions == 1)\n" + " return;\n" + " if (!mightBeLarger) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:5:9] -> [test.cpp:7:24] -> [test.cpp:14:10]: (warning) Uninitialized variable: mightBeLarger [uninitvar]\n", errout_str()); + + // Same shape, but the later condition (dimensions == 0) is implied by the narrowing, so the + // early return fires on the uninitialized path and there must be no warning. + valueFlowUninit("unsigned int g();\n" + "void f(bool a) {\n" + " unsigned int dimensions = 0;\n" + " bool mightBeLarger;\n" + " if (a) {\n" + " dimensions = g();\n" + " if (dimensions >= 1)\n" + " mightBeLarger = false;\n" + " } else {\n" + " mightBeLarger = false;\n" + " }\n" + " if (dimensions == 0)\n" + " return;\n" + " if (!mightBeLarger) {}\n" + "}"); + ASSERT_EQUALS("", errout_str()); + // Ticket #2207 - False negative valueFlowUninit("void foo() {\n" " int a;\n" From dd2d159ced49f55bbbfa021063eef58c06d4998c Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 28 Jun 2026 16:38:45 -0500 Subject: [PATCH 13/29] Use execute --- lib/programmemory.cpp | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 970fe6aa649..87019d941be 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -429,22 +429,12 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok for (const Token *tok2 = tok; tok2; tok2 = tok2->previous()) { if ((Token::simpleMatch(tok2, "=") || Token::Match(tok2->previous(), "%var% (|{")) && tok2->astOperand1() && tok2->astOperand2()) { - bool setvar = false; const Token* vartok = tok2->astOperand1(); - for (const auto& p:vars) { - if (p.first.getExpressionId() != vartok->exprId()) - continue; - if (vartok == tok) - continue; - pm.setValue(vartok, p.second); - setvar = true; - } - if (!setvar) { - if (!pm.hasValue(vartok->exprId())) { - const Token* valuetok = tok2->astOperand2(); - ProgramMemory local = state; - pm.setValue(vartok, execute(valuetok, local, settings, vars)); - } + if (!pm.hasValue(vartok->exprId())) { + const Token* valuetok = tok2->astOperand2(); + ProgramMemory local = state; + // Tracked values are substituted by execute() when the expression is evaluated. + pm.setValue(vartok, execute(valuetok, local, settings, vars)); } } else if (Token::simpleMatch(tok2, ")") && tok2->link() && Token::Match(tok2->link()->previous(), "assert|ASSERT ( !!)")) { @@ -1647,9 +1637,14 @@ namespace { } return execute(expr->astOperand1()); } - // A tracked value is the authoritative current value of its expression. - if (const ValueFlow::Value* tracked = getTrackedValue(expr)) + // A tracked value is the authoritative current value of its expression. Write it back + // into the program memory when it differs from what is stored, so that later reads see + // the same value (matching the substitution done by fillProgramMemoryFromAssignments). + if (const ValueFlow::Value* tracked = getTrackedValue(expr)) { + if (!pm->hasValue(expr->exprId()) || utils::as_const(*pm).at(expr->exprId()) != *tracked) + pm->setValue(expr, *tracked); return *tracked; + } if (expr->exprId() > 0 && pm->hasValue(expr->exprId()) && !dependsOnTrackedValue(expr)) { ValueFlow::Value result = utils::as_const(*pm).at(expr->exprId()); if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) { From 91fd4f92ebe32d50ee1d7d5170ba06bf67f9c582 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 28 Jun 2026 16:57:51 -0500 Subject: [PATCH 14/29] Simplify --- lib/programmemory.cpp | 3 ++- lib/vf_analyzers.cpp | 36 ++++++++++++++++-------------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 87019d941be..5df1661da4b 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -1641,7 +1641,8 @@ namespace { // into the program memory when it differs from what is stored, so that later reads see // the same value (matching the substitution done by fillProgramMemoryFromAssignments). if (const ValueFlow::Value* tracked = getTrackedValue(expr)) { - if (!pm->hasValue(expr->exprId()) || utils::as_const(*pm).at(expr->exprId()) != *tracked) + const ValueFlow::Value* stored = pm->getValue(expr->exprId(), /*impossible*/ true); + if (!stored || *stored != *tracked) pm->setValue(expr, *tracked); return *tracked; } diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 114f33dcc35..eae9993a5ff 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -678,11 +678,12 @@ struct ValueFlowAnalyzer : Analyzer { if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT)) return {v->intvalue}; std::vector result; - ProgramMemory pm = getProgramMemoryFunc(); // The tracked values are authoritative: pass them so a value cached in the program memory // that depends on a tracked value (e.g. 'h(p)' after 'p' was reassigned) is re-evaluated - // rather than served stale. + // rather than served stale. The program memory is built from the same state, so compute it + // once here and hand it to the builder. const ProgramState vars = getProgramState(); + ProgramMemory pm = getProgramMemoryFunc(vars); if (Token::Match(tok, "&&|%oror%")) { if (conditionIsTrue(tok, pm, getSettings(), vars)) result.push_back(1); @@ -700,16 +701,16 @@ struct ValueFlowAnalyzer : Analyzer { std::vector evaluateInt(const Token* tok) const { - return evaluateInt(tok, [&] { - return ProgramMemory{getProgramState()}; + return evaluateInt(tok, [](const ProgramState& vars) { + return ProgramMemory{vars}; }); } std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const override { if (e == Evaluate::Integral) { - return evaluateInt(tok, [&] { - return pms.get(tok, ctx, getProgramState()); + return evaluateInt(tok, [&](const ProgramState& vars) { + return pms.get(tok, ctx, vars); }); } if (e == Evaluate::ContainerEmpty) { @@ -734,10 +735,9 @@ struct ValueFlowAnalyzer : Analyzer { isCondBlock = Token::Match(parent->previous(), "if|while ("); } - const Token* startBlock = nullptr; const Token* endBlock = nullptr; if (isCondBlock) { - startBlock = parent->link()->next(); + const Token* startBlock = parent->link()->next(); if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while (")) startBlock = parent->linkAt(-2); endBlock = startBlock->link(); @@ -757,18 +757,14 @@ struct ValueFlowAnalyzer : Analyzer { pms.addState(anchor, getProgramState()); pms.assume(tok, state, flags & Assume::ContainerEmpty, origin); - if (isCondBlock && !(flags & Assume::NoState) && !scopeEnd) { - if (state) { - pms.removeModifiedVars(endBlock); - pms.addState(endBlock->previous(), getProgramState()); - } else { - if (Token::simpleMatch(endBlock, "} else {")) - pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); - else - // No else: the false path continues past the closing brace, so record the - // assumed state there so it is available to the rest of the enclosing scope. - pms.addState(endBlock, getProgramState()); - } + // On the false path the block was already traversed (the true path is handled by scopeEnd + // above), so record the assumed state where control continues: past the else block, or past + // the closing brace when there is no else, so it is available to the enclosing scope. + if (isCondBlock && !(flags & Assume::NoState) && !state) { + if (Token::simpleMatch(endBlock, "} else {")) + pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); + else + pms.addState(endBlock, getProgramState()); } if (!(flags & Assume::Quiet)) { From 9fdfa557bb21a994711a69f5cca571b05277b0e2 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 28 Jun 2026 17:07:16 -0500 Subject: [PATCH 15/29] Rename to pending --- lib/analyzer.h | 15 ++++++++------- lib/forwardanalyzer.cpp | 4 ++-- lib/vf_analyzers.cpp | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index fec9059d947..9395a5bc183 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -156,13 +156,14 @@ struct Analyzer { Quiet = (1 << 0), Absolute = (1 << 1), ContainerEmpty = (1 << 2), - // Do not record the program state at the branch boundaries. Used when assuming a - // condition before the branch is traversed, where those states would be premature. - // When this is not set the branch has already been traversed and control is continuing - // past it, so the assumed state is anchored at the block's end (see the analyzer's - // assume()): this keeps assumptions on variables modified inside the block from being - // discarded as "modified" once control leaves it. - NoState = (1 << 3), + // The branch this condition guards is still pending traversal (it is walked by a + // separate path), so this assume must not record the program state at the branch + // boundaries - those states would be premature. When this is not set the branch has + // already been traversed and control is continuing past it, so the assumed state is + // anchored at the block's end (see the analyzer's assume()): this keeps assumptions on + // variables modified inside the block from being discarded as "modified" once control + // leaves it. + Pending = (1 << 3), }; }; diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 9353673b821..2fb5240ab4e 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -793,7 +793,7 @@ namespace { // mode: the branch's effect is still tracked but nothing is reported in it. ForwardTraversal ft = fork(!analyzer->updateScope(thenBranch.endBlock, false)); // The branch is traversed below, so don't record its boundary state here. - ft.analyzer->assume(condTok, true, Analyzer::Assume::NoState); + ft.analyzer->assume(condTok, true, Analyzer::Assume::Pending); Progress pThen = ft.updateBranch(thenBranch, depth - 1); // Only commit the condition as false on the main path when it actually @@ -801,7 +801,7 @@ namespace { // boundary state); without one the false path continues past the closing // brace and must record the assumed state there. if (thenBranch.isDead()) - analyzer->assume(condTok, false, hasElse ? Analyzer::Assume::NoState : Analyzer::Assume::None); + analyzer->assume(condTok, false, hasElse ? Analyzer::Assume::Pending : Analyzer::Assume::None); // The else block is traversed on the main path. If it kills the value // (modified) the main path stops, but the then-fork may still carry the // value forward, so defer the break until after the fork continues. diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index eae9993a5ff..29d0d379a18 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -743,12 +743,12 @@ struct ValueFlowAnalyzer : Analyzer { endBlock = startBlock->link(); } - // NoState is set only for the pre-traversal assume; without it the 'then' block has already + // Pending is set only for the pre-traversal assume; without it the 'then' block has already // been traversed and control is leaving it, so anchor the assumed state at the end of the // block rather than at the condition. Assumptions about variables modified inside the block // (e.g. an 'if' that narrows a value computed in the block) then survive past it, instead of // being discarded because the variable was "modified" since the condition was evaluated. - const bool scopeEnd = !(flags & Assume::NoState) && state && endBlock; + const bool scopeEnd = !(flags & Assume::Pending) && state && endBlock; const Token* anchor = scopeEnd ? endBlock : tok; const Token* origin = scopeEnd ? endBlock : nullptr; @@ -760,7 +760,7 @@ struct ValueFlowAnalyzer : Analyzer { // On the false path the block was already traversed (the true path is handled by scopeEnd // above), so record the assumed state where control continues: past the else block, or past // the closing brace when there is no else, so it is available to the enclosing scope. - if (isCondBlock && !(flags & Assume::NoState) && !state) { + if (isCondBlock && !(flags & Assume::Pending) && !state) { if (Token::simpleMatch(endBlock, "} else {")) pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); else From d3f03655e6e8ba8585d9675c429587d97adcbf56 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 28 Jun 2026 17:20:43 -0500 Subject: [PATCH 16/29] Update comments --- lib/analyzer.h | 10 +++------- lib/forwardanalyzer.cpp | 26 ++++++++++++-------------- lib/programmemory.cpp | 15 ++++++--------- lib/vf_analyzers.cpp | 22 ++++++++++------------ 4 files changed, 31 insertions(+), 42 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index 9395a5bc183..deb8e9df3f0 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -156,13 +156,9 @@ struct Analyzer { Quiet = (1 << 0), Absolute = (1 << 1), ContainerEmpty = (1 << 2), - // The branch this condition guards is still pending traversal (it is walked by a - // separate path), so this assume must not record the program state at the branch - // boundaries - those states would be premature. When this is not set the branch has - // already been traversed and control is continuing past it, so the assumed state is - // anchored at the block's end (see the analyzer's assume()): this keeps assumptions on - // variables modified inside the block from being discarded as "modified" once control - // leaves it. + // The branch this condition guards is not traversed yet (a separate path walks it), so + // the assume must not record the program state at the branch boundaries - they would be + // premature. When unset, the branch has been traversed and control is leaving it. Pending = (1 << 3), }; }; diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 2fb5240ab4e..af5bf30ba8c 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -426,9 +426,8 @@ namespace { branch.escape = true; branch.escapeUnknown = false; } else { - // Detect an escape that the traversal did not flag (e.g. an unknown noreturn call); - // isEscapeScope also reports a possible (unknown) escape via escapeUnknown. Such a - // branch may not fall through, so the fork must not continue past it. + // Detect an escape the traversal did not flag (e.g. an unknown noreturn call); + // escapeUnknown reports a possible (unknown) escape. branch.escape = isEscapeScope(branch.endBlock, branch.escapeUnknown); if (terminate != Analyzer::Terminate::None && terminate != Analyzer::Terminate::Modified) { branch.action |= analyzeScope(branch.endBlock); @@ -679,10 +678,9 @@ namespace { Token* condTok = getCondTokFromEnd(tok); if (!condTok) return Break(); - // When leaving the 'then' branch, control only reaches here when the - // condition was true if the 'else' branch escapes (e.g. it returns). In that - // case the value established in 'then' is still definite, so keep it known - // instead of lowering it to possible. + // When the 'else' branch escapes (e.g. returns), control can only continue + // here via the 'then' branch, so the value established there is still + // definite - keep it known instead of lowering to possible. bool elseEscape = false; bool unknownEscape = false; if (!inLoop && !inElse && hasElse) @@ -788,18 +786,18 @@ namespace { } else { const bool conditional = stopOnCondition(condTok); // The value only flows into the then-branch when the condition can split - // it. For an opaque or correlated condition (e.g. 'if (f(x))' or - // 'if (do_write)') it does not really reach there, so fork in analyze-only - // mode: the branch's effect is still tracked but nothing is reported in it. + // it; for an opaque or correlated condition (e.g. 'if (f(x))') it does + // not, so fork in analyze-only mode: the branch's effect is still tracked + // but nothing is reported in it. ForwardTraversal ft = fork(!analyzer->updateScope(thenBranch.endBlock, false)); // The branch is traversed below, so don't record its boundary state here. ft.analyzer->assume(condTok, true, Analyzer::Assume::Pending); Progress pThen = ft.updateBranch(thenBranch, depth - 1); - // Only commit the condition as false on the main path when it actually - // matters. With an else the else block is traversed below (so suppress the - // boundary state); without one the false path continues past the closing - // brace and must record the assumed state there. + // Commit the condition as false on the main path only when the then-branch + // is dead. The else block, if any, is traversed separately (Pending); with + // no else the false path continues past the closing brace, so record the + // assumed state there (None). if (thenBranch.isDead()) analyzer->assume(condTok, false, hasElse ? Analyzer::Assume::Pending : Analyzer::Assume::None); // The else block is traversed on the main path. If it kills the value diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 5df1661da4b..fec7eccc77f 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -1308,9 +1308,8 @@ namespace { struct Executor { ProgramMemory* pm; const Settings& settings; - // The values that are being tracked by the forward/reverse analysis. These take precedence - // over what is stored in the program memory: a tracked value is the authoritative current - // value of its expression, so any cached value that depends on it must be re-evaluated. + // Values tracked by the forward/reverse analysis. A tracked value is the authoritative + // current value of its expression and takes precedence over the program memory. const ProgramMemory::Map* vars = nullptr; int fdepth = 4; int depth = 10; @@ -1328,9 +1327,8 @@ namespace { return it == vars->end() ? nullptr : &it->second; } - // Does the expression read a tracked value? If so, any value cached in the program memory - // for it may be stale (the tracked value may have changed since it was cached), so it must - // be re-evaluated rather than served from the cache. + // Does the expression read a tracked value? If so, any value cached for it may be stale + // (the tracked value may have changed since), so it must be re-evaluated, not served cached. bool dependsOnTrackedValue(const Token* expr) const { if (!vars || vars->empty()) return false; @@ -1637,9 +1635,8 @@ namespace { } return execute(expr->astOperand1()); } - // A tracked value is the authoritative current value of its expression. Write it back - // into the program memory when it differs from what is stored, so that later reads see - // the same value (matching the substitution done by fillProgramMemoryFromAssignments). + // Return the tracked value and write it back when it differs, so later reads see the + // same value (as fillProgramMemoryFromAssignments used to do). if (const ValueFlow::Value* tracked = getTrackedValue(expr)) { const ValueFlow::Value* stored = pm->getValue(expr->exprId(), /*impossible*/ true); if (!stored || *stored != *tracked) diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 29d0d379a18..38cc85e2f2f 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -678,10 +678,9 @@ struct ValueFlowAnalyzer : Analyzer { if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT)) return {v->intvalue}; std::vector result; - // The tracked values are authoritative: pass them so a value cached in the program memory - // that depends on a tracked value (e.g. 'h(p)' after 'p' was reassigned) is re-evaluated - // rather than served stale. The program memory is built from the same state, so compute it - // once here and hand it to the builder. + // Pass the tracked values so a cached program-memory value that depends on one (e.g. 'h(p)' + // after 'p' was reassigned) is re-evaluated rather than served stale. The memory is built + // from the same state, so compute it once and hand it to the builder. const ProgramState vars = getProgramState(); ProgramMemory pm = getProgramMemoryFunc(vars); if (Token::Match(tok, "&&|%oror%")) { @@ -743,11 +742,10 @@ struct ValueFlowAnalyzer : Analyzer { endBlock = startBlock->link(); } - // Pending is set only for the pre-traversal assume; without it the 'then' block has already - // been traversed and control is leaving it, so anchor the assumed state at the end of the - // block rather than at the condition. Assumptions about variables modified inside the block - // (e.g. an 'if' that narrows a value computed in the block) then survive past it, instead of - // being discarded because the variable was "modified" since the condition was evaluated. + // Without Pending the 'then' block has been traversed and control is leaving it, so anchor + // the assumed state at the block end instead of the condition. That keeps assumptions on + // variables modified inside the block (e.g. an 'if' narrowing a value computed there) from + // being discarded as "modified" once control leaves the block. const bool scopeEnd = !(flags & Assume::Pending) && state && endBlock; const Token* anchor = scopeEnd ? endBlock : tok; const Token* origin = scopeEnd ? endBlock : nullptr; @@ -757,9 +755,9 @@ struct ValueFlowAnalyzer : Analyzer { pms.addState(anchor, getProgramState()); pms.assume(tok, state, flags & Assume::ContainerEmpty, origin); - // On the false path the block was already traversed (the true path is handled by scopeEnd - // above), so record the assumed state where control continues: past the else block, or past - // the closing brace when there is no else, so it is available to the enclosing scope. + // The false path (the true path uses scopeEnd above): record the assumed state where control + // continues - the end of the else block, or the closing brace when there is no else - so it + // reaches the enclosing scope. if (isCondBlock && !(flags & Assume::Pending) && !state) { if (Token::simpleMatch(endBlock, "} else {")) pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); From 1d6c8869b24fc3fe0d65315d0f5fcb38a33df690 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 09:42:52 -0500 Subject: [PATCH 17/29] Merge from fork --- lib/forwardanalyzer.cpp | 8 ++++++-- test/testcondition.cpp | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index af5bf30ba8c..119722cba8b 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -682,9 +682,10 @@ namespace { // here via the 'then' branch, so the value established there is still // definite - keep it known instead of lowering to possible. bool elseEscape = false; - bool unknownEscape = false; - if (!inLoop && !inElse && hasElse) + if (!inLoop && !inElse && hasElse) { + bool unknownEscape = false; elseEscape = isEscapeScope(tok->linkAt(2), unknownEscape); + } if (!condTok->hasKnownIntValue() || inLoop) { if (!elseEscape && !analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); @@ -793,6 +794,9 @@ namespace { // The branch is traversed below, so don't record its boundary state here. ft.analyzer->assume(condTok, true, Analyzer::Assume::Pending); Progress pThen = ft.updateBranch(thenBranch, depth - 1); + // Merge the fork's actions so a modification in the then-branch bubbles up + // to the enclosing branch's isModified(). + actions |= thenBranch.action; // Commit the condition as false on the main path only when the then-branch // is dead. The else block, if any, is traversed separately (Pending); with diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 3143b6eb201..e38626fbc58 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4911,6 +4911,17 @@ class TestCondition : public TestFixture { ASSERT_EQUALS("[test.cpp:3:10]: (style) Condition 'b()' is always false [knownConditionTrueFalse]\n" "[test.cpp:4:9]: (style) Condition '!b()' is always true [knownConditionTrueFalse]\n", errout_str()); + + check("int g();\n" // a value modified inside a nested branch must be lowered to possible + "void f(int outer, int inner) {\n" + " int bits = 0;\n" + " if (outer) {\n" + " if (inner == 1)\n" + " bits = g();\n" + " }\n" + " if (bits > 0) {}\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); } void alwaysTrueSymbolic() From fd54aaccf583f77bf69e61c76a20cd2ace9e565d Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 13:55:14 -0500 Subject: [PATCH 18/29] Fix another FP --- lib/forwardanalyzer.cpp | 9 +++++++-- test/testcondition.cpp | 13 +++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 119722cba8b..1730ca9bf70 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -424,7 +424,12 @@ namespace { if (terminate == Analyzer::Terminate::Escape) { branch.escape = true; - branch.escapeUnknown = false; + // The traversal followed an escaping path, but if the scope does not structurally + // always escape then another path (e.g. a modified fork) falls through, so the escape + // is only conditional - keep isModified() meaningful by not treating it as conclusive. + bool structuralUnknown = false; + const bool structuralEscape = isEscapeScope(branch.endBlock, structuralUnknown); + branch.escapeUnknown = !structuralEscape || structuralUnknown; } else { // Detect an escape the traversal did not flag (e.g. an unknown noreturn call); // escapeUnknown reports a possible (unknown) escape. @@ -557,7 +562,7 @@ namespace { forkContinue = false; } - if (allAnalysis.isModified() || !forkContinue) { + if (!forkContinue) { // TODO: Don't bail on missing condition if (!condTok) return Break(Analyzer::Terminate::Bail); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index e38626fbc58..837cd4e78ba 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4922,6 +4922,19 @@ class TestCondition : public TestFixture { " if (bits > 0) {}\n" "}\n"); ASSERT_EQUALS("", errout_str()); + + check("int g();\n" // the modifying branch has an escaping sibling - still must be lowered + "void f(int t, int u) {\n" + " int v = 0;\n" + " if (t) {\n" + " if (u == 2)\n" + " v = g();\n" + " else\n" + " return;\n" + " }\n" + " if (v > 0) {}\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); } void alwaysTrueSymbolic() From 3693a290ce02bc366ebc7abbd58a1b0a47d66695 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 13:57:08 -0500 Subject: [PATCH 19/29] Format --- lib/forwardanalyzer.cpp | 8 +++++--- lib/programmemory.cpp | 24 ++++++++++++++++++------ lib/programmemory.h | 10 ++++++++-- lib/vf_analyzers.cpp | 3 ++- test/testautovariables.cpp | 4 +++- test/testother.cpp | 5 +++-- test/teststl.cpp | 4 +++- test/testuninitvar.cpp | 7 +++++-- 8 files changed, 47 insertions(+), 18 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 1730ca9bf70..55fafc0f53d 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -777,8 +777,8 @@ namespace { if (thenBranch.check) { // The condition is only "known" because of an earlier assumption, so the // skipped else block could still modify the value -> lower to possible - if (!condTok->hasKnownIntValue() && hasElse && analyzeScope(elseBranch.endBlock).isModified() && - !analyzer->lowerToPossible()) + if (!condTok->hasKnownIntValue() && hasElse && + analyzeScope(elseBranch.endBlock).isModified() && !analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); if (updateScope(thenBranch.endBlock, depth - 1) == Progress::Break) return Break(); @@ -808,7 +808,9 @@ namespace { // no else the false path continues past the closing brace, so record the // assumed state there (None). if (thenBranch.isDead()) - analyzer->assume(condTok, false, hasElse ? Analyzer::Assume::Pending : Analyzer::Assume::None); + analyzer->assume(condTok, + false, + hasElse ? Analyzer::Assume::Pending : Analyzer::Assume::None); // The else block is traversed on the main path. If it kills the value // (modified) the main path stops, but the then-fork may still carry the // value forward, so defer the break until after the fork continues. diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index fec7eccc77f..c062b5f9ade 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -262,9 +262,16 @@ ProgramMemory::Map::iterator ProgramMemory::find(nonneg int exprid) return mValues->find(ExprIdToken::create(exprid)); } -static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars = {}); - -static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars = {}) +static ValueFlow::Value execute(const Token* expr, + ProgramMemory& pm, + const Settings& settings, + const ProgramMemory::Map& vars = {}); + +static bool evaluateCondition(MathLib::bigint r, + const Token* condition, + ProgramMemory& pm, + const Settings& settings, + const ProgramMemory::Map& vars = {}) { if (!condition) return false; @@ -1320,7 +1327,8 @@ namespace { } // Is the tracked value for this expression available? - const ValueFlow::Value* getTrackedValue(const Token* expr) const { + const ValueFlow::Value* getTrackedValue(const Token* expr) const + { if (!vars || expr->exprId() == 0) return nullptr; const auto it = vars->find(ExprIdToken::create(expr->exprId())); @@ -1329,7 +1337,8 @@ namespace { // Does the expression read a tracked value? If so, any value cached for it may be stale // (the tracked value may have changed since), so it must be re-evaluated, not served cached. - bool dependsOnTrackedValue(const Token* expr) const { + bool dependsOnTrackedValue(const Token* expr) const + { if (!vars || vars->empty()) return false; return findAstNode(expr, [&](const Token* tok) { @@ -1836,7 +1845,10 @@ namespace { }; } // namespace -static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars) +static ValueFlow::Value execute(const Token* expr, + ProgramMemory& pm, + const Settings& settings, + const ProgramMemory::Map& vars) { Executor ex{&pm, settings}; ex.vars = &vars; diff --git a/lib/programmemory.h b/lib/programmemory.h index 5896cda4aaa..e4d48c74f11 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -193,7 +193,10 @@ void execute(const Token* expr, * \param pm program memory * \param vars optional tracked values that take precedence over the program memory */ -bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars = {}); +bool conditionIsFalse(const Token* condition, + ProgramMemory pm, + const Settings& settings, + const ProgramMemory::Map& vars = {}); /** * Is condition always true when variable has given value? @@ -201,7 +204,10 @@ bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& * \param pm program memory * \param vars optional tracked values that take precedence over the program memory */ -bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars = {}); +bool conditionIsTrue(const Token* condition, + ProgramMemory pm, + const Settings& settings, + const ProgramMemory::Map& vars = {}); /** * Get program memory by looking backwards from given token. diff --git a/lib/vf_analyzers.cpp b/lib/vf_analyzers.cpp index 38cc85e2f2f..645f508d1ce 100644 --- a/lib/vf_analyzers.cpp +++ b/lib/vf_analyzers.cpp @@ -727,7 +727,8 @@ struct ValueFlowAnalyzer : Analyzer { return {}; } - void assume(const Token* tok, bool state, unsigned int flags) override { + void assume(const Token* tok, bool state, unsigned int flags) override + { bool isCondBlock = false; const Token* parent = tok->astParent(); if (parent) { diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 920cb315646..9eba6ca7aaf 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -4848,7 +4848,9 @@ class TestAutoVariables : public TestFixture { " return *iPtr;\n" " return 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:5:16] -> [test.cpp:7:10] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:5:16] -> [test.cpp:7:10] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", + errout_str()); // #11753 check("int main(int argc, const char *argv[]) {\n" diff --git a/test/testother.cpp b/test/testother.cpp index ded218e1d7a..09ac2c69f21 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -11491,8 +11491,9 @@ class TestOther : public TestFixture { " x = a + b;\n" " return x;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:3:9] -> [test.cpp:4:9]: (style) Variable 'x' is assigned an expression that holds the same value. [redundantAssignment]\n", - errout_str()); + ASSERT_EQUALS( + "[test.cpp:2:11] -> [test.cpp:3:9] -> [test.cpp:4:9]: (style) Variable 'x' is assigned an expression that holds the same value. [redundantAssignment]\n", + errout_str()); } void varFuncNullUB() { // #4482 diff --git a/test/teststl.cpp b/test/teststl.cpp index 107ef9e444b..93b53b43671 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -368,7 +368,9 @@ class TestStl : public TestFixture { " if(b) ++x;\n" " return s[x];\n" "}"); - ASSERT_EQUALS("[test.cpp:5:13]: error: Out of bounds access in 's[x]', if 's' size is 6 and 'x' is 7 [containerOutOfBounds]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:5:13]: error: Out of bounds access in 's[x]', if 's' size is 6 and 'x' is 7 [containerOutOfBounds]\n", + errout_str()); checkNormal("void f() {\n" " static const int N = 4;\n" diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 8931fb9bdc4..6a43493cd0d 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4284,7 +4284,8 @@ class TestUninitVar : public TestFixture { " else y = 123;\n" // <- y is always initialized " return y;\n" "}"); - ASSERT_EQUALS("", errout_str()); // #4560: fork-based condition analysis tracks x==0 -> else branch -> y initialized + ASSERT_EQUALS("", + errout_str()); // #4560: fork-based condition analysis tracks x==0 -> else branch -> y initialized valueFlowUninit("void f(int x) {\n" // #3948 " int value;\n" @@ -5632,7 +5633,9 @@ class TestUninitVar : public TestFixture { " return;\n" " if (!mightBeLarger) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:5:9] -> [test.cpp:7:24] -> [test.cpp:14:10]: (warning) Uninitialized variable: mightBeLarger [uninitvar]\n", errout_str()); + ASSERT_EQUALS( + "[test.cpp:5:9] -> [test.cpp:7:24] -> [test.cpp:14:10]: (warning) Uninitialized variable: mightBeLarger [uninitvar]\n", + errout_str()); // Same shape, but the later condition (dimensions == 0) is implied by the narrowing, so the // early return fires on the uninitialized path and there must be no warning. From 625d747cfcb284a22f28a9a525b3146dd40eb6bf Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 17:22:50 -0500 Subject: [PATCH 20/29] Fix hang --- lib/forwardanalyzer.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 55fafc0f53d..070892ff70f 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -562,14 +562,12 @@ namespace { forkContinue = false; } - if (!forkContinue) { - // TODO: Don't bail on missing condition - if (!condTok) - return Break(Analyzer::Terminate::Bail); - if (analyzer->isConditional() && stopUpdates()) - return Break(Analyzer::Terminate::Conditional); - analyzer->assume(condTok, false); - } + // TODO: Don't bail on missing condition + if (!condTok) + return Break(Analyzer::Terminate::Bail); + if (analyzer->isConditional() && stopUpdates()) + return Break(Analyzer::Terminate::Conditional); + analyzer->assume(condTok, false); if (forkContinue) { for (ForwardTraversal& ft : ftv) { if (!ft.actions.isIncremental()) From 5d3e4d1a6cdb3168a6dc7dc087a7e77cda20c447 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 17:41:12 -0500 Subject: [PATCH 21/29] Remove unused function --- lib/forwardanalyzer.cpp | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 070892ff70f..48224b48f06 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -386,32 +386,6 @@ namespace { return a; } - bool checkBranch(Branch& branch) const { - Analyzer::Action a = analyzeScope(branch.endBlock); - branch.action = a; - std::vector ft1 = tryForkUpdateScope(branch.endBlock, a.isModified()); - const bool bail = hasGoto(branch.endBlock); - if (!a.isModified() && !bail) { - if (ft1.empty()) { - // Traverse into the branch to see if there is a conditional escape - if (!branch.escape && hasInnerReturnScope(branch.endBlock->previous(), branch.endBlock->link())) { - ForwardTraversal ft2 = fork(true); - ft2.updateScope(branch.endBlock); - if (ft2.terminate == Analyzer::Terminate::Escape) { - branch.escape = true; - branch.escapeUnknown = false; - } - } - } else { - if (ft1.front().terminate == Analyzer::Terminate::Escape) { - branch.escape = true; - branch.escapeUnknown = false; - } - } - } - return bail; - } - Progress updateBranch(Branch& branch, int depth) { // Save and reset actions From 8c7ee9258f4b2e3c1bec14050f18d7fcc9b26ca3 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 17:43:38 -0500 Subject: [PATCH 22/29] Make left operand unsigned --- lib/analyzer.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index deb8e9df3f0..9b87310530f 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -153,13 +153,13 @@ struct Analyzer { struct Assume { enum Flags : std::uint8_t { None = 0, - Quiet = (1 << 0), - Absolute = (1 << 1), - ContainerEmpty = (1 << 2), + Quiet = (1u << 0), + Absolute = (1u << 1), + ContainerEmpty = (1u << 2), // The branch this condition guards is not traversed yet (a separate path walks it), so // the assume must not record the program state at the branch boundaries - they would be // premature. When unset, the branch has been traversed and control is leaving it. - Pending = (1 << 3), + Pending = (1u << 3), }; }; From 3fea32a8184c0ad2331b384d44acbf560e8bee5e Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 29 Jun 2026 17:48:42 -0500 Subject: [PATCH 23/29] Remove unused function --- lib/forwardanalyzer.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 48224b48f06..4609e044b97 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -351,18 +351,6 @@ namespace { return Token::findmatch(endBlock->link(), "goto|break", endBlock); } - bool hasInnerReturnScope(const Token* start, const Token* end) const { - for (const Token* tok=start; tok != end; tok = tok->previous()) { - if (Token::simpleMatch(tok, "}")) { - const Token* ftok = nullptr; - const bool r = isReturnScope(tok, settings.library, &ftok); - if (r) - return true; - } - } - return false; - } - bool isEscapeScope(const Token* endBlock, bool& unknown) const { const Token* ftok = nullptr; const bool r = isReturnScope(endBlock, settings.library, &ftok); From d86eea6a5380c4e7d7e91256647c795bf35225d5 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 30 Jun 2026 15:32:02 -0500 Subject: [PATCH 24/29] Cache the findExpressionChanged --- lib/programmemory.cpp | 28 +++++++++++++++++++--------- lib/programmemory.h | 8 ++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index c062b5f9ade..1986941466b 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -513,7 +513,7 @@ static ProgramMemory getInitialProgramState(const Token* tok, return pm; } -ProgramMemoryState::ProgramMemoryState(const Settings& s) : settings(s) +ProgramMemoryState::ProgramMemoryState(const Settings& s) : settings(s), changedCache(std::make_shared()) {} void ProgramMemoryState::replace(ProgramMemory pm, const Token* origin) @@ -564,26 +564,36 @@ void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty, const To replace(std::move(pm), origin); } -void ProgramMemoryState::removeModifiedVars(const Token* tok) +const Token* ProgramMemoryState::findExpressionChanged(const Token* expr, const Token* start, const Token* tok) const { - const ProgramMemory& pm = state; + // Memoized structural pre-filter (a superset) gates the expensive dead-code-aware walk. + const auto key = std::make_tuple(expr, start, tok); + auto it = changedCache->find(key); + if (it == changedCache->end()) + it = changedCache->emplace(key, ::findExpressionChanged(expr, start, tok, settings)).first; + if (!it->second) + return nullptr; auto eval = [&](const Token* cond) -> std::vector { - ProgramMemory pm2 = pm; - auto result = execute(cond, pm2, settings); + ProgramMemory pm2 = state; + const auto result = execute(cond, pm2, settings); if (isTrue(result)) return {1}; if (isFalse(result)) return {0}; return {}; }; + return ::findExpressionChangedSkipDeadCode(expr, start, tok, settings, eval); +} + +void ProgramMemoryState::removeModifiedVars(const Token* tok) +{ state.erase_if([&](const ExprIdToken& e) { const Token* start = origins[e.getExpressionId()]; const Token* expr = e.tok; - if (!expr || findExpressionChangedSkipDeadCode(expr, start, tok, settings, eval)) { + const bool changed = !expr || findExpressionChanged(expr, start, tok); + if (changed) origins.erase(e.getExpressionId()); - return true; - } - return false; + return changed; }); } diff --git a/lib/programmemory.h b/lib/programmemory.h index e4d48c74f11..0c7f4d4ee35 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -161,9 +162,13 @@ struct CPPCHECKLIB ProgramMemory { }; struct ProgramMemoryState { + using ChangedCache = std::map, const Token*>; + ProgramMemory state; std::map origins; const Settings& settings; + // Memoized findExpressionChanged() pre-filter; structural, so never invalidated. + std::shared_ptr changedCache; explicit ProgramMemoryState(const Settings& s); @@ -175,6 +180,9 @@ struct ProgramMemoryState { void removeModifiedVars(const Token* tok); + // Token modifying expr between start and tok, or nullptr. + const Token* findExpressionChanged(const Token* expr, const Token* start, const Token* tok) const; + ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const; }; From 4d6070c2a9fbde4710a48c69a868e3e73fad4d18 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 30 Jun 2026 19:32:28 -0500 Subject: [PATCH 25/29] Use the cache in fillProgramMemoryFromConditions --- lib/programmemory.cpp | 51 +++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 1986941466b..c1f83eb57be 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -344,7 +344,20 @@ static bool isBasicForLoop(const Token* tok) return true; } -static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings& settings, bool then) +// findExpressionChanged() is a pure structural query, so memoize it via the program-state cache, +// keyed by (expr, start, end). This dominates the cost of building program memory from conditions. +static const Token* cachedFindExpressionChanged(ProgramMemoryState::ChangedCache* cache, const Token* expr, const Token* start, const Token* end, const Settings& settings) +{ + if (!cache) + return findExpressionChanged(expr, start, end, settings); + const auto key = std::make_tuple(expr, start, end); + const auto it = cache->find(key); + if (it != cache->end()) + return it->second; + return cache->emplace(key, findExpressionChanged(expr, start, end, settings)).first->second; +} + +static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings& settings, bool then, ProgramMemoryState::ChangedCache* cache = nullptr) { auto eval = [&](const Token* t) -> std::vector { if (!t) @@ -368,7 +381,7 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con return; if (!truevalue.isIntValue()) return; - if (endTok && findExpressionChanged(vartok, tok->next(), endTok, settings)) + if (endTok && cachedFindExpressionChanged(cache, vartok, tok->next(), endTok, settings)) return; const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); const ValueFlow::Value& v = then ? truevalue : falsevalue; @@ -377,26 +390,26 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con if (containerTok) pm.setContainerSizeValue(containerTok, v.intvalue, !impossible); } else if (Token::simpleMatch(tok, "!")) { - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then, cache); } else if (then && Token::simpleMatch(tok, "&&")) { - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then); - programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, cache); + programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, cache); } else if (!then && Token::simpleMatch(tok, "||")) { - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then); - programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, cache); + programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, cache); } else if (Token::Match(tok, "&&|%oror%")) { std::vector lhs = eval(tok->astOperand1()); std::vector rhs = eval(tok->astOperand2()); if (lhs.empty() || rhs.empty()) { if (frontIs(lhs, !then)) - programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then); + programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, cache); else if (frontIs(rhs, !then)) - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, cache); else pm.setIntValue(tok, 0, then); } } else if (tok && tok->exprId() > 0) { - if (endTok && findExpressionChanged(tok, tok->next(), endTok, settings)) + if (endTok && cachedFindExpressionChanged(cache, tok, tok->next(), endTok, settings)) return; pm.setIntValue(tok, 0, then); const Token* containerTok = settings.library.getContainerFromYield(tok, Library::Container::Yield::EMPTY); @@ -405,14 +418,14 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con } } -static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scope, const Token* endTok, const Settings& settings) +static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scope, const Token* endTok, const Settings& settings, ProgramMemoryState::ChangedCache* cache) { if (!scope) return; if (!scope->isLocal()) return; assert(scope != scope->nestedIn); - fillProgramMemoryFromConditions(pm, scope->nestedIn, endTok, settings); + fillProgramMemoryFromConditions(pm, scope->nestedIn, endTok, settings, cache); if (scope->type == ScopeType::eIf || scope->type == ScopeType::eWhile || scope->type == ScopeType::eElse || scope->type == ScopeType::eFor) { const Token* condTok = getCondTokFromEnd(scope->bodyEnd); if (!condTok) @@ -421,13 +434,13 @@ static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scop bool error = false; execute(condTok, pm, &result, &error, settings); if (error) - programMemoryParseCondition(pm, condTok, endTok, settings, scope->type != ScopeType::eElse); + programMemoryParseCondition(pm, condTok, endTok, settings, scope->type != ScopeType::eElse, cache); } } -static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Token* tok, const Settings& settings) +static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Token* tok, const Settings& settings, ProgramMemoryState::ChangedCache* cache = nullptr) { - fillProgramMemoryFromConditions(pm, tok->scope(), tok, settings); + fillProgramMemoryFromConditions(pm, tok->scope(), tok, settings, cache); } static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok, const Settings& settings, const ProgramMemory& state, const ProgramMemory::Map& vars) @@ -536,7 +549,7 @@ void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& va { ProgramMemory local = state; addVars(local, vars); - fillProgramMemoryFromConditions(local, tok, settings); + fillProgramMemoryFromConditions(local, tok, settings, changedCache.get()); ProgramMemory pm; fillProgramMemoryFromAssignments(pm, tok, settings, local, vars); local.replace(std::move(pm)); @@ -567,11 +580,7 @@ void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty, const To const Token* ProgramMemoryState::findExpressionChanged(const Token* expr, const Token* start, const Token* tok) const { // Memoized structural pre-filter (a superset) gates the expensive dead-code-aware walk. - const auto key = std::make_tuple(expr, start, tok); - auto it = changedCache->find(key); - if (it == changedCache->end()) - it = changedCache->emplace(key, ::findExpressionChanged(expr, start, tok, settings)).first; - if (!it->second) + if (!cachedFindExpressionChanged(changedCache.get(), expr, start, tok, settings)) return nullptr; auto eval = [&](const Token* cond) -> std::vector { ProgramMemory pm2 = state; From 39a8d0213026b2ae873d5cc7f931f59a88e0cf76 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 30 Jun 2026 21:58:59 -0500 Subject: [PATCH 26/29] Use a function to cache --- lib/programmemory.cpp | 87 ++++++++++++++++++++++--------------------- lib/programmemory.h | 6 ++- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index c1f83eb57be..1e0bb1bb45b 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -344,20 +344,8 @@ static bool isBasicForLoop(const Token* tok) return true; } -// findExpressionChanged() is a pure structural query, so memoize it via the program-state cache, -// keyed by (expr, start, end). This dominates the cost of building program memory from conditions. -static const Token* cachedFindExpressionChanged(ProgramMemoryState::ChangedCache* cache, const Token* expr, const Token* start, const Token* end, const Settings& settings) -{ - if (!cache) - return findExpressionChanged(expr, start, end, settings); - const auto key = std::make_tuple(expr, start, end); - const auto it = cache->find(key); - if (it != cache->end()) - return it->second; - return cache->emplace(key, findExpressionChanged(expr, start, end, settings)).first->second; -} - -static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings& settings, bool then, ProgramMemoryState::ChangedCache* cache = nullptr) +// findChanged: optional cached findExpressionChanged (see ProgramMemoryState::FindChangedFn). +static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Token* endTok, const Settings& settings, bool then, const ProgramMemoryState::FindChangedFn& findChanged = {}) { auto eval = [&](const Token* t) -> std::vector { if (!t) @@ -371,6 +359,10 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con return {result}; return std::vector{}; }; + // Use the cached closure if given, else compute directly. + auto changed = [&](const Token* e, const Token* s, const Token* en) -> const Token* { + return findChanged ? findChanged(e, s, en) : findExpressionChanged(e, s, en, settings); + }; if (Token::Match(tok, "==|>=|<=|<|>|!=")) { ValueFlow::Value truevalue; ValueFlow::Value falsevalue; @@ -381,7 +373,7 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con return; if (!truevalue.isIntValue()) return; - if (endTok && cachedFindExpressionChanged(cache, vartok, tok->next(), endTok, settings)) + if (endTok && changed(vartok, tok->next(), endTok)) return; const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then); const ValueFlow::Value& v = then ? truevalue : falsevalue; @@ -390,26 +382,26 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con if (containerTok) pm.setContainerSizeValue(containerTok, v.intvalue, !impossible); } else if (Token::simpleMatch(tok, "!")) { - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then, cache); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then, findChanged); } else if (then && Token::simpleMatch(tok, "&&")) { - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, cache); - programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, cache); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, findChanged); + programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, findChanged); } else if (!then && Token::simpleMatch(tok, "||")) { - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, cache); - programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, cache); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, findChanged); + programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, findChanged); } else if (Token::Match(tok, "&&|%oror%")) { std::vector lhs = eval(tok->astOperand1()); std::vector rhs = eval(tok->astOperand2()); if (lhs.empty() || rhs.empty()) { if (frontIs(lhs, !then)) - programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, cache); + programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then, findChanged); else if (frontIs(rhs, !then)) - programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, cache); + programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then, findChanged); else pm.setIntValue(tok, 0, then); } } else if (tok && tok->exprId() > 0) { - if (endTok && cachedFindExpressionChanged(cache, tok, tok->next(), endTok, settings)) + if (endTok && changed(tok, tok->next(), endTok)) return; pm.setIntValue(tok, 0, then); const Token* containerTok = settings.library.getContainerFromYield(tok, Library::Container::Yield::EMPTY); @@ -418,14 +410,14 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con } } -static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scope, const Token* endTok, const Settings& settings, ProgramMemoryState::ChangedCache* cache) +static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scope, const Token* endTok, const Settings& settings, const ProgramMemoryState::FindChangedFn& findChanged) { if (!scope) return; if (!scope->isLocal()) return; assert(scope != scope->nestedIn); - fillProgramMemoryFromConditions(pm, scope->nestedIn, endTok, settings, cache); + fillProgramMemoryFromConditions(pm, scope->nestedIn, endTok, settings, findChanged); if (scope->type == ScopeType::eIf || scope->type == ScopeType::eWhile || scope->type == ScopeType::eElse || scope->type == ScopeType::eFor) { const Token* condTok = getCondTokFromEnd(scope->bodyEnd); if (!condTok) @@ -434,13 +426,13 @@ static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Scope* scop bool error = false; execute(condTok, pm, &result, &error, settings); if (error) - programMemoryParseCondition(pm, condTok, endTok, settings, scope->type != ScopeType::eElse, cache); + programMemoryParseCondition(pm, condTok, endTok, settings, scope->type != ScopeType::eElse, findChanged); } } -static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Token* tok, const Settings& settings, ProgramMemoryState::ChangedCache* cache = nullptr) +static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Token* tok, const Settings& settings, const ProgramMemoryState::FindChangedFn& findChanged = {}) { - fillProgramMemoryFromConditions(pm, tok->scope(), tok, settings, cache); + fillProgramMemoryFromConditions(pm, tok->scope(), tok, settings, findChanged); } static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok, const Settings& settings, const ProgramMemory& state, const ProgramMemory::Map& vars) @@ -549,7 +541,7 @@ void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& va { ProgramMemory local = state; addVars(local, vars); - fillProgramMemoryFromConditions(local, tok, settings, changedCache.get()); + fillProgramMemoryFromConditions(local, tok, settings, getCachedFindExpressionChanged(/*skipDeadCode*/ false)); ProgramMemory pm; fillProgramMemoryFromAssignments(pm, tok, settings, local, vars); local.replace(std::move(pm)); @@ -577,29 +569,38 @@ void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty, const To replace(std::move(pm), origin); } -const Token* ProgramMemoryState::findExpressionChanged(const Token* expr, const Token* start, const Token* tok) const +ProgramMemoryState::FindChangedFn ProgramMemoryState::getCachedFindExpressionChanged(bool skipDeadCode) const { - // Memoized structural pre-filter (a superset) gates the expensive dead-code-aware walk. - if (!cachedFindExpressionChanged(changedCache.get(), expr, start, tok, settings)) - return nullptr; - auto eval = [&](const Token* cond) -> std::vector { - ProgramMemory pm2 = state; - const auto result = execute(cond, pm2, settings); - if (isTrue(result)) - return {1}; - if (isFalse(result)) - return {0}; - return {}; + // The structural findExpressionChanged() is pure, so memoize it in changedCache (never + // invalidated). skipDeadCode gates it as a pre-filter for the dead-code walk, which needs state. + return [cache = changedCache, sp = &settings, statePtr = &state, skipDeadCode](const Token* expr, const Token* start, const Token* end) -> const Token* { + const auto key = std::make_tuple(expr, start, end); + const auto it = cache->find(key); + const Token* modified = (it != cache->end()) + ? it->second + : cache->emplace(key, findExpressionChanged(expr, start, end, *sp)).first->second; + if (!skipDeadCode || !modified) + return modified; + auto eval = [&](const Token* cond) -> std::vector { + ProgramMemory pm2 = *statePtr; + const auto result = execute(cond, pm2, *sp); + if (isTrue(result)) + return {1}; + if (isFalse(result)) + return {0}; + return {}; + }; + return findExpressionChangedSkipDeadCode(expr, start, end, *sp, eval); }; - return ::findExpressionChangedSkipDeadCode(expr, start, tok, settings, eval); } void ProgramMemoryState::removeModifiedVars(const Token* tok) { + const auto findChanged = getCachedFindExpressionChanged(/*skipDeadCode*/ true); state.erase_if([&](const ExprIdToken& e) { const Token* start = origins[e.getExpressionId()]; const Token* expr = e.tok; - const bool changed = !expr || findExpressionChanged(expr, start, tok); + const bool changed = !expr || findChanged(expr, start, tok); if (changed) origins.erase(e.getExpressionId()); return changed; diff --git a/lib/programmemory.h b/lib/programmemory.h index 0c7f4d4ee35..26f39ffb6d2 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -163,6 +163,8 @@ struct CPPCHECKLIB ProgramMemory { struct ProgramMemoryState { using ChangedCache = std::map, const Token*>; + // The token modifying expr between start and end, or nullptr. + using FindChangedFn = std::function; ProgramMemory state; std::map origins; @@ -180,8 +182,8 @@ struct ProgramMemoryState { void removeModifiedVars(const Token* tok); - // Token modifying expr between start and tok, or nullptr. - const Token* findExpressionChanged(const Token* expr, const Token* start, const Token* tok) const; + // A findExpressionChanged() closure memoized in changedCache + FindChangedFn getCachedFindExpressionChanged(bool skipDeadCode) const; ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const; }; From 3c8db85d59828eb84158525b9a1ca7568e3152f2 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 30 Jun 2026 22:33:12 -0500 Subject: [PATCH 27/29] Add evalcache --- lib/programmemory.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 1e0bb1bb45b..021439764e8 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -571,9 +571,11 @@ void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty, const To ProgramMemoryState::FindChangedFn ProgramMemoryState::getCachedFindExpressionChanged(bool skipDeadCode) const { - // The structural findExpressionChanged() is pure, so memoize it in changedCache (never - // invalidated). skipDeadCode gates it as a pre-filter for the dead-code walk, which needs state. - return [cache = changedCache, sp = &settings, statePtr = &state, skipDeadCode](const Token* expr, const Token* start, const Token* end) -> const Token* { + // Structural findExpressionChanged() is pure, so memoize it in changedCache (never invalidated). + // skipDeadCode adds the dead-code walk; it evaluates guards against a fixed state snapshot (so every + // variable follows the same path) and memoizes those evals in evalCache for the closure's lifetime. + using EvalCache = std::map>; + return [cache = changedCache, sp = &settings, snapshot = (skipDeadCode ? state : ProgramMemory{}), skipDeadCode, evalCache = std::make_shared()](const Token* expr, const Token* start, const Token* end) -> const Token* { const auto key = std::make_tuple(expr, start, end); const auto it = cache->find(key); const Token* modified = (it != cache->end()) @@ -582,13 +584,17 @@ ProgramMemoryState::FindChangedFn ProgramMemoryState::getCachedFindExpressionCha if (!skipDeadCode || !modified) return modified; auto eval = [&](const Token* cond) -> std::vector { - ProgramMemory pm2 = *statePtr; + const auto cit = evalCache->find(cond); + if (cit != evalCache->end()) + return cit->second; + ProgramMemory pm2 = snapshot; const auto result = execute(cond, pm2, *sp); + std::vector r; if (isTrue(result)) - return {1}; - if (isFalse(result)) - return {0}; - return {}; + r = {1}; + else if (isFalse(result)) + r = {0}; + return evalCache->emplace(cond, std::move(r)).first->second; }; return findExpressionChangedSkipDeadCode(expr, start, end, *sp, eval); }; From 39a0bb523495f8e63eb8e3416b3ea24fe0b5a967 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Jul 2026 08:28:15 -0500 Subject: [PATCH 28/29] Dont use lambda initializer --- lib/programmemory.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 021439764e8..deb16ecd1b2 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -575,7 +575,11 @@ ProgramMemoryState::FindChangedFn ProgramMemoryState::getCachedFindExpressionCha // skipDeadCode adds the dead-code walk; it evaluates guards against a fixed state snapshot (so every // variable follows the same path) and memoizes those evals in evalCache for the closure's lifetime. using EvalCache = std::map>; - return [cache = changedCache, sp = &settings, snapshot = (skipDeadCode ? state : ProgramMemory{}), skipDeadCode, evalCache = std::make_shared()](const Token* expr, const Token* start, const Token* end) -> const Token* { + const std::shared_ptr cache = changedCache; + const Settings* const sp = &settings; + ProgramMemory snapshot = skipDeadCode ? state : ProgramMemory{}; + const std::shared_ptr evalCache = std::make_shared(); + return [cache, sp, snapshot, skipDeadCode, evalCache](const Token* expr, const Token* start, const Token* end) -> const Token* { const auto key = std::make_tuple(expr, start, end); const auto it = cache->find(key); const Token* modified = (it != cache->end()) From 72fdef33f63eee5cab418f8f37864d1bab4d2777 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 1 Jul 2026 08:39:10 -0500 Subject: [PATCH 29/29] Use unordered_map instead --- lib/programmemory.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/programmemory.h b/lib/programmemory.h index 26f39ffb6d2..7327e30c0a8 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -162,7 +162,16 @@ struct CPPCHECKLIB ProgramMemory { }; struct ProgramMemoryState { - using ChangedCache = std::map, const Token*>; + struct ChangedKeyHash { + std::size_t operator()(const std::tuple& t) const { + const std::hash h; + std::size_t seed = h(std::get<0>(t)); + seed ^= h(std::get<1>(t)) + 0x9e3779b9 + (seed << 6) + (seed >> 2); + seed ^= h(std::get<2>(t)) + 0x9e3779b9 + (seed << 6) + (seed >> 2); + return seed; + } + }; + using ChangedCache = std::unordered_map, const Token*, ChangedKeyHash>; // The token modifying expr between start and end, or nullptr. using FindChangedFn = std::function;