Author: Donát Nagy Date: 2026-02-19T13:17:03+01:00 New Revision: 6a78d37e83ba51a1cf320aee77ec9d2efb53d65d
URL: https://github.com/llvm/llvm-project/commit/6a78d37e83ba51a1cf320aee77ec9d2efb53d65d DIFF: https://github.com/llvm/llvm-project/commit/6a78d37e83ba51a1cf320aee77ec9d2efb53d65d.diff LOG: [analyzer] Add BranchCondition callback to 'switch' (#182058) Previously the condition of a 'switch' statement did not trigger a `BranchCondition` callback. This commit resolves this old FIXME and e.g. lets the checker `core.uninitialzed.Branch` report code where the condition of a `switch` statement is undefined. This commit also contains a very small unrelated change that removes a short fragment of dead code from `processBranch`. Added: clang/test/Analysis/uninitialized-branch.c Modified: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 8c9265ae553119..1aa35f2e2078ee 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2872,9 +2872,6 @@ void ExprEngine::processBranch( BranchNodeBuilder Builder(Dst, BldCtx, DstT, DstF); for (ExplodedNode *PredN : CheckersOutSet) { - if (PredN->isSink()) - continue; - ProgramStateRef PrevState = PredN->getState(); ProgramStateRef StTrue = PrevState, StFalse = PrevState; @@ -3106,70 +3103,83 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC, /// nodes by processing the 'effects' of a switch statement. void ExprEngine::processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch, ExplodedNode *Pred, ExplodedNodeSet &Dst) { + currBldrCtx = &BC; const Expr *Condition = Switch->getCond(); SwitchNodeBuilder Builder(Dst, BC); + ExplodedNodeSet CheckersOutSet; - ProgramStateRef State = Pred->getState(); - SVal CondV = State->getSVal(Condition, Pred->getLocationContext()); - - if (CondV.isUndef()) { - // FIXME: Emit warnings when the switch condition is undefined. - return; - } + getCheckerManager().runCheckersForBranchCondition( + Condition->IgnoreParens(), CheckersOutSet, Pred, *this); - std::optional<NonLoc> CondNL = CondV.getAs<NonLoc>(); + for (ExplodedNode *Node : CheckersOutSet) { + ProgramStateRef State = Node->getState(); - for (const CFGBlock *Block : Builder) { - // Successor may be pruned out during CFG construction. - if (!Block) + SVal CondV = State->getSVal(Condition, Node->getLocationContext()); + if (CondV.isUndef()) { + // This can only happen if core.uninitialized.Branch is disabled. continue; + } - const CaseStmt *Case = cast<CaseStmt>(Block->getLabel()); + std::optional<NonLoc> CondNL = CondV.getAs<NonLoc>(); - // Evaluate the LHS of the case value. - llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(getContext()); - assert(V1.getBitWidth() == getContext().getIntWidth(Condition->getType())); + for (const CFGBlock *Block : Builder) { + // Successor may be pruned out during CFG construction. + if (!Block) + continue; - // Get the RHS of the case, if it exists. - llvm::APSInt V2; - if (const Expr *E = Case->getRHS()) - V2 = E->EvaluateKnownConstInt(getContext()); - else - V2 = V1; + const CaseStmt *Case = cast<CaseStmt>(Block->getLabel()); - ProgramStateRef StateMatching; - if (CondNL) { - // Split the state: this "case:" matches / does not match. - std::tie(StateMatching, State) = - State->assumeInclusiveRange(*CondNL, V1, V2); - } else { - // The switch condition is UnknownVal, so we enter each "case:" without - // any state update. - StateMatching = State; - } + // Evaluate the LHS of the case value. + llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(getContext()); + assert(V1.getBitWidth() == + getContext().getIntWidth(Condition->getType())); - if (StateMatching) - Builder.generateCaseStmtNode(Block, StateMatching, Pred); + // Get the RHS of the case, if it exists. + llvm::APSInt V2; + if (const Expr *E = Case->getRHS()) + V2 = E->EvaluateKnownConstInt(getContext()); + else + V2 = V1; - // If _not_ entering the current case is infeasible, we are done with - // processing this branch. + ProgramStateRef StateMatching; + if (CondNL) { + // Split the state: this "case:" matches / does not match. + std::tie(StateMatching, State) = + State->assumeInclusiveRange(*CondNL, V1, V2); + } else { + // The switch condition is UnknownVal, so we enter each "case:" without + // any state update. + StateMatching = State; + } + + if (StateMatching) + Builder.generateCaseStmtNode(Block, StateMatching, Node); + + // If _not_ entering the current case is infeasible, then we are done + // with processing the paths through the current Node. + if (!State) + break; + } if (!State) - return; - } - // If we have switch(enum value), the default branch is not - // feasible if all of the enum constants not covered by 'case:' statements - // are not feasible values for the switch condition. - // - // Note that this isn't as accurate as it could be. Even if there isn't - // a case for a particular enum value as long as that enum value isn't - // feasible then it shouldn't be considered for making 'default:' reachable. - if (Condition->IgnoreParenImpCasts()->getType()->isEnumeralType()) { - if (Switch->isAllEnumCasesCovered()) - return; + continue; + + // If we have switch(enum value), the default branch is not + // feasible if all of the enum constants not covered by 'case:' statements + // are not feasible values for the switch condition. + // + // Note that this isn't as accurate as it could be. Even if there isn't + // a case for a particular enum value as long as that enum value isn't + // feasible then it shouldn't be considered for making 'default:' reachable. + if (Condition->IgnoreParenImpCasts()->getType()->isEnumeralType()) { + if (Switch->isAllEnumCasesCovered()) + continue; + } + + Builder.generateDefaultCaseNode(State, Node); } - Builder.generateDefaultCaseNode(State, Pred); + currBldrCtx = nullptr; } //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/uninitialized-branch.c b/clang/test/Analysis/uninitialized-branch.c new file mode 100644 index 00000000000000..1829f10f2a17eb --- /dev/null +++ b/clang/test/Analysis/uninitialized-branch.c @@ -0,0 +1,61 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -Wno-uninitialized -verify %s + +int if_cond(void) { + int foo; + if (foo) //expected-warning {{Branch condition evaluates to a garbage value}} + return 1; + return 2; +} + +int logical_op_and_if_cond(void) { + int foo, bar; + if (foo && bar) //expected-warning {{Branch condition evaluates to a garbage value}} + return 1; + return 2; +} + +int logical_op_cond(int arg) { + int foo; + if (foo && arg) //expected-warning {{Branch condition evaluates to a garbage value}} + return 1; + return 2; +} + +int if_cond_after_logical_op(int arg) { + int foo; + if (arg && foo) //expected-warning {{Branch condition evaluates to a garbage value}} + return 1; + return 2; +} + +int ternary_cond(void) { + int foo; + return foo ? 1 : 2; //expected-warning {{Branch condition evaluates to a garbage value}} +} + +int while_cond(void) { + int foo; + while (foo) //expected-warning {{Branch condition evaluates to a garbage value}} + return 1; + return 2; +} + +int do_while_cond(void) { + int foo, bar; + do { + foo = 43; + } while (bar); //expected-warning {{Branch condition evaluates to a garbage value}} + return foo; +} + +int switch_cond(void) { + int foo; + switch (foo) { //expected-warning {{Branch condition evaluates to a garbage value}} + case 1: + return 3; + case 2: + return 440; + default: + return 6772; + } +} diff --git a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp index d15bec02879f29..b3ba76e2d31727 100644 --- a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp +++ b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp @@ -362,6 +362,7 @@ TEST(BlockEntranceTester, BlockEntranceVSBranchCondition) { "Within 'top' B5 -> B3", "Within 'top' B6 -> B4", "Within 'top': branch condition 'x == 6'", + "Within 'top': branch condition 'x'", }), Diags); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
