https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/180960
From f309d2aef493fe4443fbb768d828bdc1da87a93c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 11 Feb 2026 15:45:30 +0100 Subject: [PATCH 1/7] [NFCI][analyzer] Regularize NodeBuilder classes Previously `IndirectGotoNodeBuilder` and `SwitchNodeBuilder` were not subclasses of the base class `NodeBuilder` and duplicated some `generateNode()`-like functions. This commit reimplements these two classes as subclasses of `NodeBuilder`. Updating `SwitchNodeBuilder` is a prerequisite for activating the `BranchCondition` checkers on the condition of the `switch` statement -- because `CheckerContext` requires the presence of a `NodeBuilder`. Updating `IndirectGotoNodeBuilder` doesn't have any analogous goals -- I'm just doing it for the sake of consistency. I also added some very basic tests because this area wasn't properly covered by the old tests. --- .../Core/PathSensitive/CoreEngine.h | 51 ++++---- .../Core/PathSensitive/ExprEngine.h | 7 +- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 81 ++++++------- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 46 +++----- clang/test/Analysis/indirect-goto-basics.c | 83 +++++++++++++ clang/test/Analysis/switch-basics.c | 110 ++++++++++++++++++ 6 files changed, 276 insertions(+), 102 deletions(-) create mode 100644 clang/test/Analysis/indirect-goto-basics.c create mode 100644 clang/test/Analysis/switch-basics.c diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 96ddd12b286a6..6d7dca2622a89 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -399,55 +399,56 @@ class BranchNodeBuilder: public NodeBuilder { ExplodedNode *Pred); }; -class IndirectGotoNodeBuilder { - const CoreEngine &Eng; - const CFGBlock *Src; +class IndirectGotoNodeBuilder : public NodeBuilder { const CFGBlock &DispatchBlock; - const Expr *E; - ExplodedNode *Pred; + const Expr *Target; public: - IndirectGotoNodeBuilder(ExplodedNode *Pred, const CFGBlock *Src, - const Expr *E, const CFGBlock *Dispatch, - const CoreEngine *Eng) - : Eng(*Eng), Src(Src), DispatchBlock(*Dispatch), E(E), Pred(Pred) {} + IndirectGotoNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, + NodeBuilderContext &Ctx, const Expr *Tgt, + const CFGBlock *Dispatch) + : NodeBuilder(SrcNode, DstSet, Ctx), DispatchBlock(*Dispatch), + Target(Tgt) { + // The indirect goto node builder does not generate autotransitions. + takeNodes(SrcNode); + } using iterator = CFGBlock::const_succ_iterator; iterator begin() { return DispatchBlock.succ_begin(); } iterator end() { return DispatchBlock.succ_end(); } - ExplodedNode *generateNode(const CFGBlock *Block, ProgramStateRef State, - bool IsSink = false); + using NodeBuilder::generateNode; - const Expr *getTarget() const { return E; } + ExplodedNode *generateNode(const CFGBlock *Block, ProgramStateRef State, + ExplodedNode *Pred); - ProgramStateRef getState() const { return Pred->State; } + const Expr *getTarget() const { return Target; } const LocationContext *getLocationContext() const { - return Pred->getLocationContext(); + return C.getLocationContext(); } }; -class SwitchNodeBuilder { - const CoreEngine &Eng; - const CFGBlock *Src; - ExplodedNode *Pred; - +class SwitchNodeBuilder : public NodeBuilder { public: - SwitchNodeBuilder(ExplodedNode *P, const CFGBlock *S, CoreEngine &E) - : Eng(E), Src(S), Pred(P) {} + SwitchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, + const NodeBuilderContext &Ctx) + : NodeBuilder(SrcNode, DstSet, Ctx) { + // The switch node builder does not generate autotransitions. + takeNodes(SrcNode); + } using iterator = CFGBlock::const_succ_reverse_iterator; - iterator begin() { return Src->succ_rbegin() + 1; } - iterator end() { return Src->succ_rend(); } + iterator begin() { return C.getBlock()->succ_rbegin() + 1; } + iterator end() { return C.getBlock()->succ_rend(); } ExplodedNode *generateCaseStmtNode(const CFGBlock *Block, - ProgramStateRef State); + ProgramStateRef State, ExplodedNode *Pred); ExplodedNode *generateDefaultCaseNode(ProgramStateRef State, - bool IsSink = false); + ExplodedNode *Pred); }; } // namespace ento diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 6705716932bbc..20f62f93f9095 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -355,12 +355,13 @@ class ExprEngine { /// processIndirectGoto - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a computed goto jump. - void processIndirectGoto(IndirectGotoNodeBuilder& builder); + void processIndirectGoto(IndirectGotoNodeBuilder &Builder, + ExplodedNode *Pred); /// ProcessSwitch - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a switch statement. - void processSwitch(const SwitchStmt *Switch, CoreEngine &CoreEng, - const CFGBlock *B, ExplodedNode *Pred); + void processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch, + ExplodedNode *Pred, ExplodedNodeSet &Dst); /// Called by CoreEngine. Used to notify checkers that processing a /// function has begun. Called for both inlined and top-level functions. diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index bfaa874f1632a..8f087cc53aaa9 100644 --- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -425,12 +425,23 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { case Stmt::IndirectGotoStmtClass: { // Only 1 successor: the indirect goto dispatch block. assert(B->succ_size() == 1); - - IndirectGotoNodeBuilder - builder(Pred, B, cast<IndirectGotoStmt>(Term)->getTarget(), - *(B->succ_begin()), this); - - ExprEng.processIndirectGoto(builder); + NodeBuilderContext Ctx(*this, B, Pred); + ExplodedNodeSet Dst; + IndirectGotoNodeBuilder Builder( + Pred, Dst, Ctx, cast<IndirectGotoStmt>(Term)->getTarget(), + *(B->succ_begin())); + + ExprEng.processIndirectGoto(Builder, Pred); + // Enqueue the new frontier onto the worklist. + llvm::errs() << "Pred location is "; + Pred->getLocation().dump(); + llvm::errs() << "\n"; + for (auto *N : Dst) { + llvm::errs() << "Enqueueing node at "; + N->getLocation().dump(); + llvm::errs() << "\n"; + WList->enqueue(N); + } return; } @@ -449,7 +460,12 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { return; case Stmt::SwitchStmtClass: { - ExprEng.processSwitch(cast<SwitchStmt>(Term), *this, B, Pred); + NodeBuilderContext Ctx(*this, B, Pred); + ExplodedNodeSet Dst; + ExprEng.processSwitch(Ctx, cast<SwitchStmt>(Term), Pred, Dst); + // Enqueue the new frontier onto the worklist. + for (auto *N : Dst) + WList->enqueue(N); return; } @@ -726,38 +742,22 @@ ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State, ExplodedNode *IndirectGotoNodeBuilder::generateNode(const CFGBlock *Block, ProgramStateRef St, - bool IsSink) { - bool IsNew; - ExplodedNode *Succ = Eng.G.getNode( - BlockEdge(Src, Block, Pred->getLocationContext()), St, IsSink, &IsNew); - Succ->addPredecessor(Pred, Eng.G); - - if (!IsNew) - return nullptr; - - if (!IsSink) - Eng.WList->enqueue(Succ); - - return Succ; + ExplodedNode *Pred) { + BlockEdge BE(C.getBlock(), Block, Pred->getLocationContext()); + return generateNode(BE, St, Pred); } ExplodedNode *SwitchNodeBuilder::generateCaseStmtNode(const CFGBlock *Block, - ProgramStateRef St) { - bool IsNew; - ExplodedNode *Succ = Eng.G.getNode( - BlockEdge(Src, Block, Pred->getLocationContext()), St, false, &IsNew); - Succ->addPredecessor(Pred, Eng.G); - if (!IsNew) - return nullptr; - - Eng.WList->enqueue(Succ); - return Succ; + ProgramStateRef St, + ExplodedNode *Pred) { + BlockEdge BE(C.getBlock(), Block, Pred->getLocationContext()); + return generateNode(BE, St, Pred); } -ExplodedNode* -SwitchNodeBuilder::generateDefaultCaseNode(ProgramStateRef St, - bool IsSink) { +ExplodedNode *SwitchNodeBuilder::generateDefaultCaseNode(ProgramStateRef St, + ExplodedNode *Pred) { // Get the block for the default case. + const CFGBlock *Src = C.getBlock(); assert(Src->succ_rbegin() != Src->succ_rend()); CFGBlock *DefaultBlock = *Src->succ_rbegin(); @@ -766,17 +766,6 @@ SwitchNodeBuilder::generateDefaultCaseNode(ProgramStateRef St, if (!DefaultBlock) return nullptr; - bool IsNew; - ExplodedNode *Succ = - Eng.G.getNode(BlockEdge(Src, DefaultBlock, Pred->getLocationContext()), - St, IsSink, &IsNew); - Succ->addPredecessor(Pred, Eng.G); - - if (!IsNew) - return nullptr; - - if (!IsSink) - Eng.WList->enqueue(Succ); - - return Succ; + BlockEdge BE(Src, DefaultBlock, Pred->getLocationContext()); + return generateNode(BE, St, Pred); } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 883c7b5d66c32..47312a53f61ff 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2992,23 +2992,18 @@ void ExprEngine::processStaticInitializer( /// processIndirectGoto - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a computed goto jump. -void ExprEngine::processIndirectGoto(IndirectGotoNodeBuilder &builder) { - ProgramStateRef state = builder.getState(); - SVal V = state->getSVal(builder.getTarget(), builder.getLocationContext()); - - // Three possibilities: - // - // (1) We know the computed label. - // (2) The label is NULL (or some other constant), or Undefined. - // (3) We have no clue about the label. Dispatch to all targets. - // +void ExprEngine::processIndirectGoto(IndirectGotoNodeBuilder &Builder, + ExplodedNode *Pred) { + ProgramStateRef State = Pred->getState(); + SVal V = State->getSVal(Builder.getTarget(), Builder.getLocationContext()); + // Case 1: We know the computed label. if (std::optional<loc::GotoLabel> LV = V.getAs<loc::GotoLabel>()) { const LabelDecl *L = LV->getLabel(); - for (const CFGBlock *Succ : builder) { + for (const CFGBlock *Succ : Builder) { if (cast<LabelStmt>(Succ->getLabel())->getDecl() == L) { - builder.generateNode(Succ, state); + Builder.generateNode(Succ, State, Pred); return; } } @@ -3016,19 +3011,16 @@ void ExprEngine::processIndirectGoto(IndirectGotoNodeBuilder &builder) { llvm_unreachable("No block with label."); } + // Case 2: The label is NULL (or some other constant), or Undefined. if (isa<UndefinedVal, loc::ConcreteInt>(V)) { - // Dispatch to the first target and mark it as a sink. - //ExplodedNode* N = builder.generateNode(builder.begin(), state, true); - // FIXME: add checker visit. - // UndefBranches.insert(N); + // FIXME: Emit warnings when the jump target is undefined or numerical. return; } - // This is really a catch-all. We don't support symbolics yet. + // Case 3: We have no clue about the label. Dispatch to all targets. // FIXME: Implement dispatch for symbolic pointers. - - for (const CFGBlock *Succ : builder) - builder.generateNode(Succ, state); + for (const CFGBlock *Succ : Builder) + Builder.generateNode(Succ, State, Pred); } void ExprEngine::processBeginOfFunction(NodeBuilderContext &BC, @@ -3112,19 +3104,17 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC, /// ProcessSwitch - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a switch statement. -void ExprEngine::processSwitch(const SwitchStmt *Switch, CoreEngine &CoreEng, - const CFGBlock *B, ExplodedNode *Pred) { +void ExprEngine::processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch, + ExplodedNode *Pred, ExplodedNodeSet &Dst) { const Expr *Condition = Switch->getCond(); - SwitchNodeBuilder Builder(Pred, B, CoreEng); + SwitchNodeBuilder Builder(Pred, Dst, BC); ProgramStateRef State = Pred->getState(); SVal CondV = State->getSVal(Condition, Pred->getLocationContext()); if (CondV.isUndef()) { - // ExplodedNode* N = builder.generateDefaultCaseNode(state, true); - // FIXME: add checker - // UndefBranches.insert(N); + // FIXME: Emit warnings when the switch condition is undefined. return; } @@ -3160,7 +3150,7 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, CoreEngine &CoreEng, } if (StateMatching) - Builder.generateCaseStmtNode(Block, StateMatching); + Builder.generateCaseStmtNode(Block, StateMatching, Pred); // If _not_ entering the current case is infeasible, we are done with // processing this branch. @@ -3179,7 +3169,7 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, CoreEngine &CoreEng, return; } - Builder.generateDefaultCaseNode(State); + Builder.generateDefaultCaseNode(State, Pred); } //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/indirect-goto-basics.c b/clang/test/Analysis/indirect-goto-basics.c new file mode 100644 index 0000000000000..93ee5b39ce913 --- /dev/null +++ b/clang/test/Analysis/indirect-goto-basics.c @@ -0,0 +1,83 @@ +// RUN: %clang_analyze_cc1 -verify %s -analyzer-checker=core + +// This file tests ExprEngine::processIndirectGoto and the class +// IndirectGotoNodeBuilder. + +int goto_known_label_1(int x) { + // In this example the ternary operator splits the state, the analyzer can + // exactly follow the computed goto on both execution paths and validate that + // we reach "33 / x" with a state where x is constrained to zero. + void *target = x ? &&nonzero : &&zero; + goto *target; +zero: + return 33 / x; // expected-warning {{Division by zero}} +nonzero: + return 66 / (x + 1); +} + +int goto_known_label_2(int x) { + // In this example the ternary operator splits the state, the analyzer can + // exactly follow the computed goto on both execution paths and validate that + // we do not reach "66 / x" with the state where x is constrained to zero. + void *target = x ? &&nonzero : &&zero; + goto *target; +zero: + return 33 / (x + 1); +nonzero: + return 66 / x; +} + +void *select(int, void *, void *, void *); +int goto_symbolic(int x, int y) { + // In this example the target of the indirect goto is a symbolic value so the + // analyzer dispatches to all possible labels and we get the zero division + // errors at all of them. + void *target = select(x, &&first, &&second, &&third); + if (y) + return 41; + goto *target; +first: + return 33 / y; // expected-warning {{Division by zero}} +second: + return 66 / y; // expected-warning {{Division by zero}} +third: + return 123 / y; // expected-warning {{Division by zero}} +} + +int goto_nullpointer(int x, int y) { + // In this example the target of the indirect goto is a loc::ConcreteInt (a + // null pointer), so the analyzer doesn't dispatch anywhere. + // FIXME: We should emit a warning in this situation. + void *target = (void *)0; + (void)&&first; + (void)&&second; + (void)&&third; + if (y) + return 41; + goto *target; +first: + return 33 / y; +second: + return 66 / y; +third: + return 123 / y; +} + +int goto_undefined(int x, int y) { + // In this example the target of the indirect goto is an uninitialized + // pointer, so the analyzer doesn't dispatch anywhere. + // FIXME: We should emit a warning in this situation. + void *target; + (void)&&first; + (void)&&second; + (void)&&third; + if (y) + return 41; + goto *target; +first: + return 33 / y; +second: + return 66 / y; +third: + return 123 / y; +} diff --git a/clang/test/Analysis/switch-basics.c b/clang/test/Analysis/switch-basics.c new file mode 100644 index 0000000000000..2a0f9c2a3cf1b --- /dev/null +++ b/clang/test/Analysis/switch-basics.c @@ -0,0 +1,110 @@ +// RUN: %clang_analyze_cc1 -verify %s -analyzer-checker=core + +// This file tests ExprEngine::processSwitch and the class SwitchNodeBuilder. + +int switch_simple(int x) { + // Validate that switch behaves as expected in a very simple situation. + switch (x) { + case 1: + return 13 / x; + case 0: + return 14 / x; // expected-warning {{Division by zero}} + case 2: + return 15 / x; + default: + return 67 / (x - x); // expected-warning {{Division by zero}} + } +} + +int switch_default(int x) { + // Validate that the default case is evaluated after excluding the other + // cases -- even if it appears first. + if (x > 2 || x < 0) + return 0; + switch (x) { + default: + return 16 / x; // expected-warning {{Division by zero}} + case 1: + return 13 / x; + case 2: + return 15 / x; + } +} + +int switch_unreachable_default(int x) { + // Validate that the default case is not evaluated if it is infeasible. + int zero = 0; + if (x > 2 || x < 0) + return 0; + switch (x) { + default: + return 16 / zero; // no-warning + case 0: + return 456; + case 1: + return 13 / x; + case 2: + return 15 / x; + } +} + +enum Color {Red, Green, Blue}; + +int switch_all_enum_cases_covered(enum Color x) { + // Validate that the default case is not evaluated if the switch is over an + // enum value and all enumerators appear as 'case's. + int zero = 0; + switch (x) { + default: + return 16 / zero; // no-warning + case Red: + case Green: + return 2; + case Blue: + return 3; + } +} + +int switch_all_feasible_enum_cases_covered(enum Color x) { + // Highlight a shortcoming of enum/switch handling: here the 'case's cover + // all the enumerators that could appear in the symbolic value 'x', but the + // default is still executed. + // FIXME: The default branch shouldn't be executed here. + int zero = 0; + + if (x == Red) + return 1; + switch (x) { + default: + return 16 / zero; // expected-warning {{Division by zero}} + case Green: + return 2; + case Blue: + return 3; + } +} + +int switch_no_compound_stmt(int x) { + // Validate that the engine can follow the switch statement even if there is + // no compound statement around the cases. (Yes, this is valid, although + // not very practical.) + switch (x) case 1: case 0: return 16 / x; // expected-warning {{Division by zero}} + + return 0; +} + +int switch_with_case_range(int x) { + // Validate that the GNU case range extension is properly handled. + switch (x) { + case 5: + return 55 / x; + case 2 ... 4: + return 3; + case 0 ... 1: + return 44 / x; // no-warning: there is no state split between 0 and 1 + default: + if (x) + return 8; + return 45 / x; // no-warning: x cannot be 0 on the default branch + } +} From 9b8e1236cd8ea1147eb158530c60735ab64e8ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Feb 2026 11:56:34 +0100 Subject: [PATCH 2/7] Remove debug printouts, simplify enqueueing --- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index 8f087cc53aaa9..619e2dc58ca78 100644 --- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -433,15 +433,7 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { ExprEng.processIndirectGoto(Builder, Pred); // Enqueue the new frontier onto the worklist. - llvm::errs() << "Pred location is "; - Pred->getLocation().dump(); - llvm::errs() << "\n"; - for (auto *N : Dst) { - llvm::errs() << "Enqueueing node at "; - N->getLocation().dump(); - llvm::errs() << "\n"; - WList->enqueue(N); - } + enqueue(Dst); return; } @@ -464,8 +456,7 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { ExplodedNodeSet Dst; ExprEng.processSwitch(Ctx, cast<SwitchStmt>(Term), Pred, Dst); // Enqueue the new frontier onto the worklist. - for (auto *N : Dst) - WList->enqueue(N); + enqueue(Dst); return; } From 33969d7d024a82bfd010de61ae698f4b042eaa71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Feb 2026 17:34:01 +0100 Subject: [PATCH 3/7] Declare NodeBuilder subclasses as final --- .../clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 6d7dca2622a89..32f8e24918d36 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -321,7 +321,7 @@ class NodeBuilder { /// This builder class is useful for generating nodes that resulted from /// visiting a statement. The main difference from its parent NodeBuilder is /// that it creates a statement specific ProgramPoint. -class StmtNodeBuilder: public NodeBuilder { +class StmtNodeBuilder final : public NodeBuilder { NodeBuilder *EnclosingBldr; public: @@ -372,7 +372,7 @@ class StmtNodeBuilder: public NodeBuilder { /// BranchNodeBuilder is responsible for constructing the nodes /// corresponding to the two branches of the if statement - true and false. -class BranchNodeBuilder: public NodeBuilder { +class BranchNodeBuilder final : public NodeBuilder { const CFGBlock *DstT; const CFGBlock *DstF; @@ -399,7 +399,7 @@ class BranchNodeBuilder: public NodeBuilder { ExplodedNode *Pred); }; -class IndirectGotoNodeBuilder : public NodeBuilder { +class IndirectGotoNodeBuilder final : public NodeBuilder { const CFGBlock &DispatchBlock; const Expr *Target; @@ -430,7 +430,7 @@ class IndirectGotoNodeBuilder : public NodeBuilder { } }; -class SwitchNodeBuilder : public NodeBuilder { +class SwitchNodeBuilder final : public NodeBuilder { public: SwitchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) From 892909689c585fa2ef690b166ddec40ac61ac03e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Feb 2026 17:39:53 +0100 Subject: [PATCH 4/7] Add anchor methods --- .../clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h | 4 ++++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 32f8e24918d36..d35b5ebeeb6c4 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -403,6 +403,8 @@ class IndirectGotoNodeBuilder final : public NodeBuilder { const CFGBlock &DispatchBlock; const Expr *Target; + void anchor() override; + public: IndirectGotoNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, NodeBuilderContext &Ctx, const Expr *Tgt, @@ -431,6 +433,8 @@ class IndirectGotoNodeBuilder final : public NodeBuilder { }; class SwitchNodeBuilder final : public NodeBuilder { + void anchor() override; + public: SwitchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index 619e2dc58ca78..acde7662c2f52 100644 --- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -731,6 +731,8 @@ ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State, return Succ; } +void IndirectGotoNodeBuilder::anchor() {} + ExplodedNode *IndirectGotoNodeBuilder::generateNode(const CFGBlock *Block, ProgramStateRef St, ExplodedNode *Pred) { @@ -738,6 +740,8 @@ ExplodedNode *IndirectGotoNodeBuilder::generateNode(const CFGBlock *Block, return generateNode(BE, St, Pred); } +void SwitchNodeBuilder::anchor() {} + ExplodedNode *SwitchNodeBuilder::generateCaseStmtNode(const CFGBlock *Block, ProgramStateRef St, ExplodedNode *Pred) { From 57eac6d19e36a893ae677f5f556daf0f20e9ea82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Feb 2026 18:32:38 +0100 Subject: [PATCH 5/7] Devirtualize the NodeBuilder hierarchy --- .../Core/PathSensitive/CoreEngine.h | 40 +++++-------------- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 14 ------- 2 files changed, 9 insertions(+), 45 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index d35b5ebeeb6c4..736a9ad9896e9 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -236,8 +236,6 @@ class NodeBuilderContext { /// added to the builder (either as the input node set or as the newly /// constructed nodes) but did not have any outgoing transitions added. class NodeBuilder { - virtual void anchor(); - protected: const NodeBuilderContext &C; @@ -273,8 +271,6 @@ class NodeBuilder { assert(hasNoSinksInFrontier()); } - virtual ~NodeBuilder() = default; - /// Generates a node in the ExplodedGraph. ExplodedNode *generateNode(const ProgramPoint &PP, ProgramStateRef State, @@ -321,31 +317,19 @@ class NodeBuilder { /// This builder class is useful for generating nodes that resulted from /// visiting a statement. The main difference from its parent NodeBuilder is /// that it creates a statement specific ProgramPoint. -class StmtNodeBuilder final : public NodeBuilder { - NodeBuilder *EnclosingBldr; - +/// FIXME: This class is not meaningfully different from plain NodeBuilder. +class StmtNodeBuilder : public NodeBuilder { public: - /// Constructs a StmtNodeBuilder. If the builder is going to process - /// nodes currently owned by another builder(with larger scope), use - /// Enclosing builder to transfer ownership. StmtNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, - const NodeBuilderContext &Ctx, - NodeBuilder *Enclosing = nullptr) - : NodeBuilder(SrcNode, DstSet, Ctx), EnclosingBldr(Enclosing) { - if (EnclosingBldr) - EnclosingBldr->takeNodes(SrcNode); + const NodeBuilderContext &Ctx) + : NodeBuilder(SrcNode, DstSet, Ctx) { } StmtNodeBuilder(ExplodedNodeSet &SrcSet, ExplodedNodeSet &DstSet, - const NodeBuilderContext &Ctx, - NodeBuilder *Enclosing = nullptr) - : NodeBuilder(SrcSet, DstSet, Ctx), EnclosingBldr(Enclosing) { - if (EnclosingBldr) - EnclosingBldr->takeNodes(SrcSet); + const NodeBuilderContext &Ctx) + : NodeBuilder(SrcSet, DstSet, Ctx) { } - ~StmtNodeBuilder() override; - using NodeBuilder::generateNode; using NodeBuilder::generateSink; @@ -372,12 +356,10 @@ class StmtNodeBuilder final : public NodeBuilder { /// BranchNodeBuilder is responsible for constructing the nodes /// corresponding to the two branches of the if statement - true and false. -class BranchNodeBuilder final : public NodeBuilder { +class BranchNodeBuilder : public NodeBuilder { const CFGBlock *DstT; const CFGBlock *DstF; - void anchor() override; - public: BranchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, const NodeBuilderContext &C, const CFGBlock *DT, @@ -399,12 +381,10 @@ class BranchNodeBuilder final : public NodeBuilder { ExplodedNode *Pred); }; -class IndirectGotoNodeBuilder final : public NodeBuilder { +class IndirectGotoNodeBuilder : public NodeBuilder { const CFGBlock &DispatchBlock; const Expr *Target; - void anchor() override; - public: IndirectGotoNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, NodeBuilderContext &Ctx, const Expr *Tgt, @@ -432,9 +412,7 @@ class IndirectGotoNodeBuilder final : public NodeBuilder { } }; -class SwitchNodeBuilder final : public NodeBuilder { - void anchor() override; - +class SwitchNodeBuilder : public NodeBuilder { public: SwitchNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index acde7662c2f52..1e5a04b8efbe4 100644 --- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -688,8 +688,6 @@ void CoreEngine::enqueueEndOfFunction(ExplodedNodeSet &Set, const ReturnStmt *RS } } -void NodeBuilder::anchor() {} - ExplodedNode* NodeBuilder::generateNodeImpl(const ProgramPoint &Loc, ProgramStateRef State, ExplodedNode *FromN, @@ -709,14 +707,6 @@ ExplodedNode* NodeBuilder::generateNodeImpl(const ProgramPoint &Loc, return N; } -StmtNodeBuilder::~StmtNodeBuilder() { - if (EnclosingBldr) - for (const auto I : Frontier) - EnclosingBldr->addNodes(I); -} - -void BranchNodeBuilder::anchor() {} - ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State, bool Branch, ExplodedNode *NodePred) { @@ -731,8 +721,6 @@ ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State, return Succ; } -void IndirectGotoNodeBuilder::anchor() {} - ExplodedNode *IndirectGotoNodeBuilder::generateNode(const CFGBlock *Block, ProgramStateRef St, ExplodedNode *Pred) { @@ -740,8 +728,6 @@ ExplodedNode *IndirectGotoNodeBuilder::generateNode(const CFGBlock *Block, return generateNode(BE, St, Pred); } -void SwitchNodeBuilder::anchor() {} - ExplodedNode *SwitchNodeBuilder::generateCaseStmtNode(const CFGBlock *Block, ProgramStateRef St, ExplodedNode *Pred) { From f24a9db332e222d0938066e055ddc1fc8e60cd70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Feb 2026 18:37:10 +0100 Subject: [PATCH 6/7] Satisfy git-clang-format --- .../clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 736a9ad9896e9..217aaba4a00c5 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -322,13 +322,11 @@ class StmtNodeBuilder : public NodeBuilder { public: StmtNodeBuilder(ExplodedNode *SrcNode, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) - : NodeBuilder(SrcNode, DstSet, Ctx) { - } + : NodeBuilder(SrcNode, DstSet, Ctx) {} StmtNodeBuilder(ExplodedNodeSet &SrcSet, ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) - : NodeBuilder(SrcSet, DstSet, Ctx) { - } + : NodeBuilder(SrcSet, DstSet, Ctx) {} using NodeBuilder::generateNode; using NodeBuilder::generateSink; From eb973044a4b9659d122ff22ed5b9819ca8dfc9cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Feb 2026 18:44:17 +0100 Subject: [PATCH 7/7] Clarify comments in the test file --- clang/test/Analysis/indirect-goto-basics.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/test/Analysis/indirect-goto-basics.c b/clang/test/Analysis/indirect-goto-basics.c index 93ee5b39ce913..2de378cb70595 100644 --- a/clang/test/Analysis/indirect-goto-basics.c +++ b/clang/test/Analysis/indirect-goto-basics.c @@ -47,7 +47,8 @@ int goto_symbolic(int x, int y) { int goto_nullpointer(int x, int y) { // In this example the target of the indirect goto is a loc::ConcreteInt (a // null pointer), so the analyzer doesn't dispatch anywhere. - // FIXME: We should emit a warning in this situation. + // FIXME: The analyzer should report that nullptr (or some other concrete + // value) was passed to an indirect goto. void *target = (void *)0; (void)&&first; (void)&&second; @@ -66,7 +67,8 @@ int goto_nullpointer(int x, int y) { int goto_undefined(int x, int y) { // In this example the target of the indirect goto is an uninitialized // pointer, so the analyzer doesn't dispatch anywhere. - // FIXME: We should emit a warning in this situation. + // FIXME: The analyzer should report that an uninitialized value was passed + // to an indirect goto. void *target; (void)&&first; (void)&&second; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
