Skip to content

Commit 0239dd1

Browse files
author
Your Name
committed
Handle FP
1 parent 293ae8c commit 0239dd1

2 files changed

Lines changed: 31 additions & 21 deletions

File tree

lib/forwardanalyzer.cpp

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -425,15 +425,12 @@ namespace {
425425
if (terminate == Analyzer::Terminate::Escape) {
426426
branch.escape = true;
427427
branch.escapeUnknown = false;
428-
} else if (terminate != Analyzer::Terminate::None) {
429-
if (terminate != Analyzer::Terminate::Escape) {
430-
bool unknown = false;
431-
if (isEscapeScope(branch.endBlock, unknown)) {
432-
branch.escape = true;
433-
branch.escapeUnknown = unknown;
434-
}
435-
}
436-
if (terminate != Analyzer::Terminate::Modified) {
428+
} else {
429+
// Detect an escape that the traversal did not flag (e.g. an unknown noreturn call);
430+
// isEscapeScope also reports a possible (unknown) escape via escapeUnknown. Such a
431+
// branch may not fall through, so the fork must not continue past it.
432+
branch.escape = isEscapeScope(branch.endBlock, branch.escapeUnknown);
433+
if (terminate != Analyzer::Terminate::None && terminate != Analyzer::Terminate::Modified) {
437434
branch.action |= analyzeScope(branch.endBlock);
438435
}
439436
}
@@ -678,11 +675,20 @@ namespace {
678675
const bool inElse = scope->type == ScopeType::eElse;
679676
const bool inDoWhile = scope->type == ScopeType::eDo;
680677
const bool inLoop = contains({ScopeType::eDo, ScopeType::eFor, ScopeType::eWhile}, scope->type);
678+
const bool hasElse = Token::simpleMatch(tok, "} else {");
681679
Token* condTok = getCondTokFromEnd(tok);
682680
if (!condTok)
683681
return Break();
682+
// When leaving the 'then' branch, control only reaches here when the
683+
// condition was true if the 'else' branch escapes (e.g. it returns). In that
684+
// case the value established in 'then' is still definite, so keep it known
685+
// instead of lowering it to possible.
686+
bool elseEscape = false;
687+
bool unknownEscape = false;
688+
if (!inLoop && !inElse && hasElse)
689+
elseEscape = isEscapeScope(tok->linkAt(2), unknownEscape);
684690
if (!condTok->hasKnownIntValue() || inLoop) {
685-
if (!analyzer->lowerToPossible())
691+
if (!elseEscape && !analyzer->lowerToPossible())
686692
return Break(Analyzer::Terminate::Bail);
687693
} else if (condTok->getKnownIntValue() == inElse) {
688694
return Break();
@@ -707,7 +713,7 @@ namespace {
707713
}
708714
analyzer->assume(condTok, !inElse, Analyzer::Assume::Quiet);
709715
assert(!inDoWhile || Token::simpleMatch(tok, "} while ("));
710-
if (Token::simpleMatch(tok, "} else {") || inDoWhile)
716+
if (hasElse || inDoWhile)
711717
tok = tok->linkAt(2);
712718
} else if (contains({ScopeType::eTry, ScopeType::eCatch}, scope->type)) {
713719
if (!analyzer->lowerToPossible())
@@ -789,31 +795,35 @@ namespace {
789795
// matters
790796
if (thenBranch.isDead())
791797
analyzer->assume(condTok, false);
792-
if (hasElse) {
793-
if (updateBranch(elseBranch, depth - 1) == Progress::Break)
794-
return Break();
795-
}
798+
// The else block is traversed on the main path. If it kills the value
799+
// (modified) the main path stops, but the then-fork may still carry the
800+
// value forward, so defer the break until after the fork continues.
801+
Progress pElse = Progress::Continue;
802+
if (hasElse)
803+
pElse = updateBranch(elseBranch, depth - 1);
796804
if (thenBranch.isDead() || elseBranch.isDead()) {
797805
if (conditional && stopUpdates())
798806
return Break(Analyzer::Terminate::Conditional);
799807
}
800808
if (thenBranch.isModified() || elseBranch.isModified()) {
801-
if (!analyzer->lowerToPossible())
802-
return Break(Analyzer::Terminate::Bail);
803809
if (!ft.analyzer->lowerToPossible())
804810
p = Progress::Break;
811+
if (pElse != Progress::Break && !analyzer->lowerToPossible())
812+
return Break(Analyzer::Terminate::Bail);
805813
}
806814
if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) {
807-
if (!analyzer->lowerToInconclusive())
808-
return Break(Analyzer::Terminate::Bail);
809815
if (!ft.analyzer->lowerToInconclusive())
810816
p = Progress::Break;
817+
if (pElse != Progress::Break && !analyzer->lowerToInconclusive())
818+
return Break(Analyzer::Terminate::Bail);
811819
}
812820
if (thenBranch.hasGoto() || elseBranch.hasGoto()) {
813821
return Break(Analyzer::Terminate::Bail);
814822
}
815-
if (p != Progress::Break)
823+
if (p != Progress::Break && !thenBranch.isEscape())
816824
ft.updateRange(thenBranch.endBlock, end, depth - 1);
825+
if (pElse == Progress::Break)
826+
return Break();
817827
}
818828
}
819829
} else if (Token::simpleMatch(tok, "try {")) {

test/testuninitvar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4265,7 +4265,7 @@ class TestUninitVar : public TestFixture {
42654265
" else {}\n"
42664266
" return y;\n"
42674267
"}");
4268-
TODO_ASSERT_EQUALS("", "[test.cpp:5:9] -> [test.cpp:7:12]: (warning) Uninitialized variable: y [uninitvar]\n", errout_str());
4268+
ASSERT_EQUALS("", errout_str()); // #4560: escaping else keeps x known, so x is true and y is initialized
42694269

42704270
valueFlowUninit("int f(int a) {\n" // #6583
42714271
" int x;\n"

0 commit comments

Comments
 (0)