From 40cdf41d2f79e89eaa60589494a1ca83cf751957 Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 10 Jul 2022 16:47:00 -0500 Subject: [PATCH 01/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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);