https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/218442
From c46a5fa899851f1ae083e41a619ebe602b123b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 24 Aug 2026 17:04:45 +0200 Subject: [PATCH 1/5] Remove NodeBuilderContext in handleConstructionContext The value passed to the NodeBuilderContext parameter was always 'currBldrCtx', so we can use 'getNumVisitedCurrent()' to get the current block count. --- .../clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 5 ++--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 4 ++-- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 68d4362aca941..9b3709b2e3c41 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -815,12 +815,11 @@ class ExprEngine { /// and updateObjectsUnderConstruction. std::pair<ProgramStateRef, SVal> handleConstructionContext(const Expr *E, ProgramStateRef State, - const NodeBuilderContext *BldrCtx, const StackFrame *SF, const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx = 0) { - SVal V = computeObjectUnderConstruction(E, State, BldrCtx->blockCount(), SF, - CC, CallOpts, Idx); + SVal V = computeObjectUnderConstruction(E, State, getNumVisitedCurrent(), + SF, CC, CallOpts, Idx); State = updateObjectsUnderConstruction(V, E, State, SF, CC, CallOpts); return std::make_pair(State, V); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 3df2d3d9e3674..9fb167ee2ea4a 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -602,8 +602,8 @@ void ExprEngine::handleConstructor(const Expr *E, ExplodedNode *Pred, } // The target region is found from construction context. - std::tie(State, Target) = handleConstructionContext(CE, State, currBldrCtx, - SF, CC, CallOpts, Idx); + std::tie(State, Target) = + handleConstructionContext(CE, State, SF, CC, CallOpts, Idx); break; } case CXXConstructionKind::VirtualBase: { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index e0139484b2b48..66fdb6d117a96 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -773,7 +773,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call, assert(RTC->getStmt() == Call.getOriginExpr()); EvalCallOptions CallOpts; // FIXME: We won't really need those. std::tie(State, Target) = - handleConstructionContext(Call.getOriginExpr(), State, currBldrCtx, SF, + handleConstructionContext(Call.getOriginExpr(), State, SF, RTC->getConstructionContext(), CallOpts); const MemRegion *TargetR = Target.getAsRegion(); assert(TargetR); From 18743597ac1cc5c8bd922a11fee0900ffac282d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 24 Aug 2026 17:11:41 +0200 Subject: [PATCH 2/5] Remove currBldrCtx --- .../Core/PathSensitive/ExprEngine.h | 54 ++++--------------- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 +- 2 files changed, 10 insertions(+), 46 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 9b3709b2e3c41..a76098aba19b5 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -156,29 +156,8 @@ class ExprEngine { SValBuilder &svalBuilder; unsigned int currStmtIdx = 0; - - /// Pointer to a (so-called, somewhat misnamed) NodeBuilderContext object - /// which has three independent roles: - /// - It holds a pointer to the CFGBlock that is currently under analysis. - /// (This is the primary way to get the current block.) - /// - It holds a pointer to the current StackFrame. (This is rarely - /// used, the stack frame is usually queried from a recent - /// ExplodedNode. Unfortunately it seems that these two sources of truth - /// are not always consistent.) - /// - It can be used for constructing `NodeBuilder`s. Practically all - /// `NodeBuilder` objects are useless complications in the code, so I - /// intend to replace them with direct use of `CoreEngine::makeNode`. - /// TODO: Eventually `currBldrCtx` should be replaced by two separate fields: - /// `const CFGBlock *CurrBlock` & `const StackFrame *CurrStackFrame` - /// that are kept up-to-date and are almost always non-null during the - /// analysis. I will switch to this more natural representation when - /// `NodeBuilder`s are eliminated from the code. - const NodeBuilderContext *currBldrCtx = nullptr; - /// Historically `currBldrCtx` pointed to a local variable in some stack - /// frame. This field is introduced as a temporary measure to allow a gradual - /// transition. Only use this in {re,}setCurrStackFrameAndBlock! - /// TODO: Remove this temporary hack. - std::optional<NodeBuilderContext> OwnedCurrBldrCtx; + const StackFrame *CurrStackFrame = nullptr; + const CFGBlock *CurrBlock = nullptr; /// Helper object to determine if an Objective-C message expression /// implicitly never returns. @@ -235,12 +214,6 @@ class ExprEngine { return &CTU; } - // FIXME: Ideally the body of this method should look like - // CurrStackFrame = SF; - // CurrBlock = B; - // where CurrStackFrame and CurrBlock are new member variables that - // fulfill the roles of `currBldrCtx` in a more natural way. - // This implementation is a temporary measure to allow a gradual transition. void setCurrStackFrameAndBlock(const StackFrame *SF, const CFGBlock *B) { // The current StackFrame and Block is reset at the beginning of // dispatchWorkItem. Ideally, this method should be called only once per @@ -249,20 +222,15 @@ class ExprEngine { // StackFrame and Block needs to change in the middle of a single step // (which currently happens only once, in processCallExit), use an explicit // call to resetCurrStackFrameAndBlock. - assert(!currBldrCtx && !OwnedCurrBldrCtx && + assert(!CurrBlock && !CurrStackFrame && "The current StackFrame and Block is already set"); - OwnedCurrBldrCtx.emplace(Engine, B, SF); - currBldrCtx = &*OwnedCurrBldrCtx; + CurrStackFrame = SF; + CurrBlock = B; } void resetCurrStackFrameAndBlock() { - currBldrCtx = nullptr; - OwnedCurrBldrCtx = std::nullopt; - } - - const NodeBuilderContext &getBuilderContext() const { - assert(currBldrCtx); - return *currBldrCtx; + CurrStackFrame = nullptr; + CurrBlock = nullptr; } const StackFrame *getRootStackFrame() const { @@ -277,15 +245,11 @@ class ExprEngine { /// (e.g. a recent `ExplodedNode`). Traditionally this stack frame is /// only used for block count calculations (`getNumVisited`); it is probably /// wise to follow this tradition until the discrepancies are resolved. - const StackFrame *getCurrStackFrame() const { - return currBldrCtx ? currBldrCtx->getStackFrame() : nullptr; - } + const StackFrame *getCurrStackFrame() const { return CurrStackFrame; } /// Get the 'current' CFGBlock corresponding to the current work item /// (elementary analysis step handled by `dispatchWorkItem`). - const CFGBlock *getCurrBlock() const { - return currBldrCtx ? currBldrCtx->getBlock() : nullptr; - } + const CFGBlock *getCurrBlock() const { return CurrBlock; } ConstCFGElementRef getCFGElementRef() const { return {getCurrBlock(), currStmtIdx}; diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index e6349eb4eba2a..fea5536bf3244 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1153,7 +1153,7 @@ void ExprEngine::ProcessLifetimeEnd(const Stmt *S, const VarDecl *D, ExplodedNodeSet Dst; getCheckerManager().runCheckersForLifetimeEnd(Dst, Src, D, *this); - Engine.enqueueStmtNodes(Dst, currBldrCtx->getBlock(), currStmtIdx); + Engine.enqueueStmtNodes(Dst, CurrBlock, currStmtIdx); } void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit, From 182596181b25a313c61f82ed0ca72c2cbfbbacd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 24 Aug 2026 17:12:51 +0200 Subject: [PATCH 3/5] Remove class NodeBuilderContext --- .../StaticAnalyzer/Core/CheckerManager.h | 1 - .../Core/PathSensitive/CoreEngine.h | 32 ------------------- .../Core/PathSensitive/ExprEngine.h | 1 - 3 files changed, 34 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h index 3311774f069ea..46e648c6b7d90 100644 --- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h +++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -49,7 +49,6 @@ class ExplodedNodeSet; class ExprEngine; struct EvalCallOptions; class MemRegion; -class NodeBuilderContext; class ObjCMethodCall; class RegionAndSymbolInvalidationTraits; class SVal; diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 71443e4434462..d26c0d9257b0f 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 NodeBuilderContext; public: using BlocksExhausted = @@ -210,37 +209,6 @@ class CoreEngine { DataTag::Factory &getDataTags() { return DataTags; } }; -class NodeBuilderContext { - const CoreEngine &Eng; - const CFGBlock *Block; - const StackFrame *SF; - -public: - NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, - const StackFrame *S) - : Eng(E), Block(B), SF(S) { - assert(B); - } - - NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, ExplodedNode *N) - : NodeBuilderContext(E, B, N->getStackFrame()) {} - - /// Return the CoreEngine associated with this builder. - const CoreEngine &getEngine() const { return Eng; } - - /// Return the CFGBlock associated with this builder. - const CFGBlock *getBlock() const { return Block; } - - /// Return the stack frame associated with this builder. - const StackFrame *getStackFrame() const { return SF; } - - /// Returns the number of times the current basic block has been - /// visited on the exploded graph path. - unsigned blockCount() const { - return Eng.WList->getBlockCounter().getNumVisited(SF, Block->getBlockID()); - } -}; - } // namespace ento } // namespace clang diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index a76098aba19b5..478012c3744ca 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -83,7 +83,6 @@ class ConstraintManager; class ExplodedNodeSet; class ExplodedNode; class MemRegion; -class NodeBuilderContext; class ProgramState; class ProgramStateManager; class RegionAndSymbolInvalidationTraits; From faf0ec6435bc5258dce4f9264c4d7bc5e9afd911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 24 Aug 2026 17:17:20 +0200 Subject: [PATCH 4/5] Remove mention of NodeBuilderContext in comment --- clang/test/Analysis/stack-frame-context-revision.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/test/Analysis/stack-frame-context-revision.cpp b/clang/test/Analysis/stack-frame-context-revision.cpp index 3ed85e4cb20b8..5137b60289e38 100644 --- a/clang/test/Analysis/stack-frame-context-revision.cpp +++ b/clang/test/Analysis/stack-frame-context-revision.cpp @@ -1,10 +1,10 @@ // RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=core,cplusplus.NewDelete -verify %s // expected-no-diagnostics: -// From now the profile of the 'StackFrame' also contains the -// 'NodeBuilderContext::blockCount()'. With this addition we can distinguish -// between the 'StackArgumentsSpaceRegion' of the 'P' arguments being different -// on every iteration. +// From now the profile of the 'StackFrame' also contains the 'BlockCount' +// ('ExprEngine::getNumVisitedCurrent()') value at the call site. With this +// addition we can distinguish between the 'StackArgumentsSpaceRegion' of the +// 'P' arguments being different on every iteration. typedef __INTPTR_TYPE__ intptr_t; From 4a2705506b13416424f39ab3b6044544f79e14a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 24 Aug 2026 19:51:14 +0200 Subject: [PATCH 5/5] Follow AI nitpicks --- .../clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 1 + clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 478012c3744ca..64a2ebe5149e4 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -223,6 +223,7 @@ class ExprEngine { // call to resetCurrStackFrameAndBlock. assert(!CurrBlock && !CurrStackFrame && "The current StackFrame and Block is already set"); + assert(SF && B && "The StackFrame and Block must be non-null"); CurrStackFrame = SF; CurrBlock = B; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index fea5536bf3244..a8fa11b078993 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1153,7 +1153,7 @@ void ExprEngine::ProcessLifetimeEnd(const Stmt *S, const VarDecl *D, ExplodedNodeSet Dst; getCheckerManager().runCheckersForLifetimeEnd(Dst, Src, D, *this); - Engine.enqueueStmtNodes(Dst, CurrBlock, currStmtIdx); + Engine.enqueueStmtNodes(Dst, getCurrBlock(), currStmtIdx); } void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit, _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
