Skip to content

Commit a9d518e

Browse files
author
Your Name
committed
Fix remaining tests
1 parent 9b86cb7 commit a9d518e

5 files changed

Lines changed: 38 additions & 9 deletions

File tree

lib/forwardanalyzer.cpp

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,23 @@ namespace {
412412
return bail;
413413
}
414414

415-
Progress updateBranch(Branch& branch, int depth)
415+
Progress updateBranch(Branch& branch, int depth, bool flow)
416416
{
417+
// Determine the branch's effect on the value without reporting (read-only)
418+
branch.action |= analyzeScope(branch.endBlock);
419+
// Mirror checkBranch()/tryForkUpdateScope(): only flow the value into the branch when
420+
// the analyzer allows it. A conditional value (e.g. a possible null carried under an
421+
// unrelated condition) must not be propagated into the branch, otherwise it produces
422+
// false positives. 'flow' is evaluated on the unassumed value by the caller.
423+
if (!flow) {
424+
bool unknown = false;
425+
if (isEscapeScope(branch.endBlock, unknown)) {
426+
branch.escape = true;
427+
branch.escapeUnknown = unknown;
428+
}
429+
return Progress::Continue;
430+
}
431+
417432
// Save and reset actions
418433
Analyzer::Action prevActions = actions;
419434
actions = Analyzer::Action::None;
@@ -765,20 +780,34 @@ namespace {
765780
const bool hasElse = Token::simpleMatch(endBlock, "} else {");
766781
tok = hasElse ? endBlock->linkAt(2) : endBlock;
767782
if (thenBranch.check) {
783+
// The condition is only "known" because of an earlier assumption, so the
784+
// skipped else block could still modify the value -> lower to possible
785+
if (analyzer->isConditional() && hasElse && analyzeScope(elseBranch.endBlock).isModified() &&
786+
!analyzer->lowerToPossible())
787+
return Break(Analyzer::Terminate::Bail);
768788
if (updateScope(thenBranch.endBlock, depth - 1) == Progress::Break)
769789
return Break();
770790
} else if (elseBranch.check) {
791+
// Likewise the skipped then block could still modify the value
792+
if (analyzer->isConditional() && analyzeScope(thenBranch.endBlock).isModified() &&
793+
!analyzer->lowerToPossible())
794+
return Break(Analyzer::Terminate::Bail);
771795
if (elseBranch.endBlock && updateScope(elseBranch.endBlock, depth - 1) == Progress::Break)
772796
return Break();
773797
} else {
774-
const bool conditional = analyzer->isConditional();
798+
const bool conditional = stopOnCondition(condTok);
799+
// Decide whether the value may flow into each branch using the unassumed
800+
// value (as checkBranch() does), before assume() makes the forks conditional
801+
const bool flowThen = analyzer->updateScope(thenBranch.endBlock, analyzeScope(thenBranch.endBlock).isModified());
802+
const bool flowElse = hasElse && elseBranch.endBlock &&
803+
analyzer->updateScope(elseBranch.endBlock, analyzeScope(elseBranch.endBlock).isModified());
775804
ForwardTraversal ft = fork();
776805
ft.analyzer->assume(condTok, true);
777-
Progress p = ft.updateBranch(thenBranch, depth - 1);
806+
Progress p = ft.updateBranch(thenBranch, depth - 1, flowThen);
778807

779808
analyzer->assume(condTok, false);
780809
if (hasElse) {
781-
if (updateBranch(elseBranch, depth - 1) == Progress::Break)
810+
if (updateBranch(elseBranch, depth - 1, flowElse) == Progress::Break)
782811
return Break();
783812
}
784813
if (thenBranch.isDead() || elseBranch.isDead()) {

test/testautovariables.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4848,7 +4848,7 @@ class TestAutoVariables : public TestFixture {
48484848
" return *iPtr;\n"
48494849
" return 0;\n"
48504850
"}");
4851-
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());
4851+
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());
48524852

48534853
// #11753
48544854
check("int main(int argc, const char *argv[]) {\n"

test/testother.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11491,7 +11491,7 @@ class TestOther : public TestFixture {
1149111491
" x = a + b;\n"
1149211492
" return x;\n"
1149311493
"}\n");
11494-
ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:4:9]: (style) Variable 'x' is assigned an expression that holds the same value. [redundantAssignment]\n",
11494+
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",
1149511495
errout_str());
1149611496
}
1149711497

test/teststl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ class TestStl : public TestFixture {
368368
" if(b) ++x;\n"
369369
" return s[x];\n"
370370
"}");
371-
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());
371+
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());
372372

373373
checkNormal("void f() {\n"
374374
" static const int N = 4;\n"
@@ -2736,7 +2736,7 @@ class TestStl : public TestFixture {
27362736
"}\n", s);
27372737
ASSERT_EQUALS("[test.cpp:5:9]: warning: Array index -1 is out of bounds. [negativeContainerIndex]\n"
27382738
"[test.cpp:8:8]: note: Calling function 'f', 1st argument '-1' value is -1\n"
2739-
"[test.cpp:3:9]: note: Assuming condition is false\n"
2739+
"[test.cpp:3:9]: note: Assuming condition is true\n"
27402740
"[test.cpp:5:9]: note: Negative array index\n",
27412741
errout_str());
27422742
}

test/testuninitvar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4284,7 +4284,7 @@ class TestUninitVar : public TestFixture {
42844284
" else y = 123;\n" // <- y is always initialized
42854285
" return y;\n"
42864286
"}");
4287-
TODO_ASSERT_EQUALS("", "[test.cpp:5:9] -> [test.cpp:7:12]: (warning) Uninitialized variable: y [uninitvar]\n", errout_str());
4287+
ASSERT_EQUALS("", errout_str()); // #4560: fork-based condition analysis tracks x==0 -> else branch -> y initialized
42884288

42894289
valueFlowUninit("void f(int x) {\n" // #3948
42904290
" int value;\n"

0 commit comments

Comments
 (0)