https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/182058
From 755f04d2988288e34b7c711766482ced1d4af33a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 4 Feb 2026 15:07:37 +0100 Subject: [PATCH 1/3] [NFCI][analyzer] Add BranchCondition callback to 'switch' Previously the condition of a 'switch' statement did not trigger a `BranchCondition` callback. This commit resolves this old FIXME. --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 110 +++++++++++-------- clang/test/Analysis/uninitialized-branch.c | 61 ++++++++++ 2 files changed, 123 insertions(+), 48 deletions(-) create mode 100644 clang/test/Analysis/uninitialized-branch.c diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 8c9265ae55311..be7d366514e8e 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2867,6 +2867,8 @@ void ExprEngine::processBranch( getCheckerManager().runCheckersForBranchCondition(Condition, CheckersOutSet, Pred, *this); // We generated only sinks. + // FIXME: Do sinks appear in CheckersOutSet? If yes, update this comment, if + // not, remove the PredN->isSink() test at the start of the for loop. if (CheckersOutSet.empty()) return; @@ -3106,70 +3108,82 @@ 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())); + + // 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; + + 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, Pred); + if (StateMatching) + Builder.generateCaseStmtNode(Block, StateMatching, Node); - // If _not_ entering the current case is infeasible, we are done with - // processing this branch. + // If _not_ entering the current case is infeasible, 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 0000000000000..1829f10f2a17e --- /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; + } +} From 026dd859c438d77ccb0830261d134539e1d15a01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 18 Feb 2026 20:53:59 +0100 Subject: [PATCH 2/3] Satisfy git-clang-format --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index be7d366514e8e..9e8b7e9281717 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3114,8 +3114,8 @@ void ExprEngine::processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch, SwitchNodeBuilder Builder(Dst, BC); ExplodedNodeSet CheckersOutSet; - getCheckerManager().runCheckersForBranchCondition(Condition->IgnoreParens(), CheckersOutSet, - Pred, *this); + getCheckerManager().runCheckersForBranchCondition( + Condition->IgnoreParens(), CheckersOutSet, Pred, *this); for (ExplodedNode *Node : CheckersOutSet) { ProgramStateRef State = Node->getState(); @@ -3137,7 +3137,8 @@ void ExprEngine::processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch, // Evaluate the LHS of the case value. llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(getContext()); - assert(V1.getBitWidth() == getContext().getIntWidth(Condition->getType())); + assert(V1.getBitWidth() == + getContext().getIntWidth(Condition->getType())); // Get the RHS of the case, if it exists. llvm::APSInt V2; From cf82da9127b3b1fd7ec61f6cfcd5df937b0ed3ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 18 Feb 2026 21:05:03 +0100 Subject: [PATCH 3/3] Update unit test to expect the condition of the switch --- clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp b/clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp index d15bec02879f2..b3ba76e2d3172 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
