Author: Donát Nagy Date: 2026-03-19T18:00:29+01:00 New Revision: 65f6a346a96acf1e0914b5288af8253801973b6d
URL: https://github.com/llvm/llvm-project/commit/65f6a346a96acf1e0914b5288af8253801973b6d DIFF: https://github.com/llvm/llvm-project/commit/65f6a346a96acf1e0914b5288af8253801973b6d.diff LOG: [NFC][analyzer] Eliminate IndirectGotoNodeBuilder (#187343) This commit removes the class `IndirectGotoNodeBuilder` because it was an abstraction that just complicated and obscured the underlying logic without any practical advantage. Incidentally, this PR also prepares the ground for dispatch based on symbolic pointers in indirect goto statements. (If somebody would need it, I could implement it reasonably quickly.) Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h clang/lib/StaticAnalyzer/Core/CoreEngine.cpp clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/test/Analysis/indirect-goto-basics.c Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index c2135ac6f7225..8d5dcc1ca8b13 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -49,7 +49,6 @@ class ExprEngine; /// It traverses the CFG and generates the ExplodedGraph. class CoreEngine { friend class ExprEngine; - friend class IndirectGotoNodeBuilder; friend class NodeBuilder; friend class NodeBuilderContext; friend class SwitchNodeBuilder; @@ -333,33 +332,6 @@ class BranchNodeBuilder : public NodeBuilder { ExplodedNode *Pred); }; -class IndirectGotoNodeBuilder : public NodeBuilder { - const CFGBlock &DispatchBlock; - const Expr *Target; - -public: - IndirectGotoNodeBuilder(ExplodedNodeSet &DstSet, - const NodeBuilderContext &Ctx, const Expr *Tgt, - const CFGBlock *Dispatch) - : NodeBuilder(DstSet, Ctx), DispatchBlock(*Dispatch), Target(Tgt) {} - - using iterator = CFGBlock::const_succ_iterator; - - iterator begin() { return DispatchBlock.succ_begin(); } - iterator end() { return DispatchBlock.succ_end(); } - - using NodeBuilder::generateNode; - - ExplodedNode *generateNode(const CFGBlock *Block, ProgramStateRef State, - ExplodedNode *Pred); - - const Expr *getTarget() const { return Target; } - - const LocationContext *getLocationContext() const { - return C.getLocationContext(); - } -}; - class SwitchNodeBuilder : public NodeBuilder { public: SwitchNodeBuilder(ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h index 29c252472b93d..a536f5ee046e1 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h @@ -67,7 +67,6 @@ class ExplodedNode : public llvm::FoldingSetNode { friend class BranchNodeBuilder; friend class CoreEngine; friend class ExplodedGraph; - friend class IndirectGotoNodeBuilder; friend class NodeBuilder; friend class SwitchNodeBuilder; diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 40ce9084e7f78..c346c20a7af87 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -83,7 +83,6 @@ class CheckerManager; class ConstraintManager; class ExplodedNodeSet; class ExplodedNode; -class IndirectGotoNodeBuilder; class MemRegion; class NodeBuilderContext; class ProgramState; @@ -422,8 +421,8 @@ class ExprEngine { /// processIndirectGoto - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a computed goto jump. - void processIndirectGoto(IndirectGotoNodeBuilder &Builder, - ExplodedNode *Pred); + void processIndirectGoto(ExplodedNodeSet &Dst, const Expr *Tgt, + const CFGBlock *Dispatch, ExplodedNode *Pred); /// ProcessSwitch - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a switch statement. diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index 3dc9c53bd30d7..26672ff75996c 100644 --- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -436,12 +436,9 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { // Only 1 successor: the indirect goto dispatch block. assert(B->succ_size() == 1); ExplodedNodeSet Dst; - IndirectGotoNodeBuilder Builder( - Dst, ExprEng.getBuilderContext(), - cast<IndirectGotoStmt>(Term)->getTarget(), *(B->succ_begin())); - - ExprEng.processIndirectGoto(Builder, Pred); - // Enqueue the new frontier onto the worklist. + ExprEng.processIndirectGoto(Dst, + cast<IndirectGotoStmt>(Term)->getTarget(), + *(B->succ_begin()), Pred); enqueue(Dst); return; } @@ -711,13 +708,6 @@ ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State, return Succ; } -ExplodedNode *IndirectGotoNodeBuilder::generateNode(const CFGBlock *Block, - ProgramStateRef St, - ExplodedNode *Pred) { - BlockEdge BE(C.getBlock(), Block, Pred->getLocationContext()); - return generateNode(BE, St, Pred); -} - ExplodedNode *SwitchNodeBuilder::generateCaseStmtNode(const CFGBlock *Block, ProgramStateRef St, ExplodedNode *Pred) { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 30aee25d35dea..0fb8f1d00b618 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2973,35 +2973,35 @@ void ExprEngine::processStaticInitializer(const DeclStmt *DS, /// processIndirectGoto - Called by CoreEngine. Used to generate successor /// nodes by processing the 'effects' of a computed goto jump. -void ExprEngine::processIndirectGoto(IndirectGotoNodeBuilder &Builder, +void ExprEngine::processIndirectGoto(ExplodedNodeSet &Dst, const Expr *Tgt, + const CFGBlock *Dispatch, ExplodedNode *Pred) { ProgramStateRef State = Pred->getState(); - SVal V = State->getSVal(Builder.getTarget(), Builder.getLocationContext()); + SVal V = State->getSVal(Tgt, getCurrLocationContext()); - // Case 1: We know the computed label. - if (std::optional<loc::GotoLabel> LV = V.getAs<loc::GotoLabel>()) { - const LabelDecl *L = LV->getLabel(); + // We cannot dispatch anywhere if the label is undefined, NULL or some other + // concrete number. + // FIXME: Emit a warning in this situation. + if (isa<UndefinedVal, loc::ConcreteInt>(V)) + return; - for (const CFGBlock *Succ : Builder) { - if (cast<LabelStmt>(Succ->getLabel())->getDecl() == L) { - Builder.generateNode(Succ, State, Pred); - return; - } + // If 'V' is the address of a concrete goto label (on this execution path), + // then only transition along the edge to that label. + // FIXME: Implement dispatch for symbolic pointers, utilizing information + // that they are equal or not equal to pointers to a certain goto label. + const LabelDecl *L = nullptr; + if (auto LV = V.getAs<loc::GotoLabel>()) + L = LV->getLabel(); + + // Dispatch to the label 'L' or to all labels if 'L' is null. + for (const CFGBlock *Succ : Dispatch->succs()) { + if (!L || cast<LabelStmt>(Succ->getLabel())->getDecl() == L) { + // FIXME: If 'V' was a symbolic value, then record that on this execution + // path it is equal to the address of the label leading to 'Succ'. + BlockEdge BE(getCurrBlock(), Succ, Pred->getLocationContext()); + Dst.Add(Engine.makeNode(BE, State, Pred)); } - - llvm_unreachable("No block with label."); } - - // Case 2: The label is NULL (or some other constant), or Undefined. - if (isa<UndefinedVal, loc::ConcreteInt>(V)) { - // FIXME: Emit warnings when the jump target is undefined or numerical. - return; - } - - // 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, Pred); } void ExprEngine::processBeginOfFunction(ExplodedNode *Pred, diff --git a/clang/test/Analysis/indirect-goto-basics.c b/clang/test/Analysis/indirect-goto-basics.c index 2de378cb70595..75d6ad6c6cdac 100644 --- a/clang/test/Analysis/indirect-goto-basics.c +++ b/clang/test/Analysis/indirect-goto-basics.c @@ -1,7 +1,6 @@ // RUN: %clang_analyze_cc1 -verify %s -analyzer-checker=core -// This file tests ExprEngine::processIndirectGoto and the class -// IndirectGotoNodeBuilder. +// This file tests ExprEngine::processIndirectGoto. int goto_known_label_1(int x) { // In this example the ternary operator splits the state, the analyzer can _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
