[clang] [analyzer] Add BranchCondition callback to 'switch' (PR #182058)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 18 12:24:20 PST 2026
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?= <donat.nagy at ericsson.com>
Date: Wed, 4 Feb 2026 15:07:37 +0100
Subject: [PATCH 1/4] [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?= <donat.nagy at ericsson.com>
Date: Wed, 18 Feb 2026 20:53:59 +0100
Subject: [PATCH 2/4] 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?= <donat.nagy at ericsson.com>
Date: Wed, 18 Feb 2026 21:05:03 +0100
Subject: [PATCH 3/4] 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);
}
>From 683a4e647c394765bcb7fdc9dd4009e155dda416 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 18 Feb 2026 21:18:58 +0100
Subject: [PATCH 4/4] Remove paranoid check instead of adding a FIXME
As I was extending `processSwitch` following the example of
`processBranch` I noticed that there was an apparently redundant `if` in
`processBranch` and added a FIXME note to mark it.
Now I answer this FIXME by removing the `if` statement, because I
validated that `runCheckersForBranchCondition` never places sink nodes
in its destination set, because the destination set is populated by
`NodeBuilder::generateNodeImpl` calls which only add a node to the
`Frontier`/destination if it is not a sink.
---
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 5 -----
1 file changed, 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 9e8b7e9281717..0c7a080d42928 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2867,16 +2867,11 @@ 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;
BranchNodeBuilder Builder(Dst, BldCtx, DstT, DstF);
for (ExplodedNode *PredN : CheckersOutSet) {
- if (PredN->isSink())
- continue;
-
ProgramStateRef PrevState = PredN->getState();
ProgramStateRef StTrue = PrevState, StFalse = PrevState;
More information about the cfe-commits
mailing list