Partial fix for 9049: False negative: uninitialized variable with nested ifs

lib/analyzer.h

@@ -156,6 +156,10 @@
             Quiet = (1 << 0),
             Absolute = (1 << 1),
             ContainerEmpty = (1 << 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),
         };
     };
 

lib/astutils.cpp

@@ -2230,6 +2230,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())

lib/forwardanalyzer.cpp

@@ -89,6 +89,9 @@ namespace {
             bool isDead() const {
                 return action.isModified() || action.isInconclusive() || isEscape();
             }
+            bool hasGoto() const {
+                return endBlock ? ForwardTraversal::hasGoto(endBlock) : false;
+            }
         };
 
         bool stopUpdates() {
@@ -409,6 +412,36 @@ namespace {
             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;
+                // 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.
+                branch.escape = isEscapeScope(branch.endBlock, branch.escapeUnknown);
+                if (terminate != Analyzer::Terminate::None && terminate != Analyzer::Terminate::Modified) {
+                    branch.action |= analyzeScope(branch.endBlock);
+                }
+            }
+
+            return p;
+        }
+
         bool reentersLoop(Token* endBlock, const Token* condTok, const Token* stepTok) const {
             if (!condTok)
                 return true;
@@ -529,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);
@@ -646,11 +679,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 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;
+                        if (!inLoop && !inElse && hasElse) {
+                            bool unknownEscape = false;
+                            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();
@@ -675,7 +717,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())
@@ -731,71 +773,73 @@ namespace {
                         if (!thenBranch.check && !elseBranch.check && stopOnCondition(condTok) && stopUpdates())
                             return Break(Analyzer::Terminate::Conditional);
                         const 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 (updateScope(endBlock, depth - 1) == Progress::Break)
+                            // 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())
+                                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 (!condTok->hasKnownIntValue() && analyzeScope(thenBranch.endBlock).isModified() &&
+                                !analyzer->lowerToPossible())
+                                return Break(Analyzer::Terminate::Bail);
+                            if (elseBranch.endBlock && updateScope(elseBranch.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;
-                                const Progress result = updateScope(endBlock->linkAt(2), depth - 1);
-                                if (result == 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)
+                            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))') 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);
+                            // Merge the fork's actions so a modification in the then-branch bubbles up
+                            // to the enclosing branch's isModified().
                             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 (!analyzer->lowerToInconclusive())
-                                    return Break(Analyzer::Terminate::Bail);
-                            } else if (thenBranch.check) {
-                                return Break();
-                            } else {
-                                if (stopOnCondition(condTok) && stopUpdates())
-                                    return Break(Analyzer::Terminate::Conditional);
-                                analyzer->assume(condTok, false);
+
+                            // 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
+                            // (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())
+                                    pElse = Break(Analyzer::Terminate::Conditional);
                             }
-                        }
-                        if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) {
-                            if (!analyzer->lowerToInconclusive())
-                                return Break(Analyzer::Terminate::Bail);
-                        } else if (thenBranch.isModified() || elseBranch.isModified()) {
-                            if (!hasElse && analyzer->isConditional() && stopUpdates())
-                                return Break(Analyzer::Terminate::Conditional);
-                            if (!analyzer->lowerToPossible())
+                            if (thenBranch.isModified() || elseBranch.isModified()) {
+                                if (!ft.analyzer->lowerToPossible())
+                                    pThen = Progress::Break;
+                                if (pElse != Progress::Break && !analyzer->lowerToPossible())
+                                    pElse = Break(Analyzer::Terminate::Bail);
+                            }
+                            if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) {
+                                if (!ft.analyzer->lowerToInconclusive())
+                                    pThen = Progress::Break;
+                                if (pElse != Progress::Break && !analyzer->lowerToInconclusive())
+                                    pElse = Break(Analyzer::Terminate::Bail);
+                            }
+                            if (thenBranch.hasGoto() || elseBranch.hasGoto()) {
                                 return Break(Analyzer::Terminate::Bail);
-                            analyzer->assume(condTok, elseBranch.isModified());
+                            }
+                            if (pThen != Progress::Break && !thenBranch.isEscape())
+                                ft.updateRange(thenBranch.endBlock, end, depth - 1);
+                            if (pElse == Progress::Break)
+                                return Break();
                         }
                     }
                 } else if (Token::simpleMatch(tok, "try {")) {