https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/186182
This commit converts `ExprEngine::processCallExit` to the new paradigm introduced in 1c424bfb03d6dd4b994a0d549e1f3e23852f1e16 where the current `LocationContext` and `Block` is populated near the beginning of the `dispatchWorkItem` call (= elementary analysis step) and remains available during the whole step. Unfortunately the first half of the `CallExit` procedure (`removeDead`) happens within the callee context, while the second half (`PostCall` and similar callbacks) happen in the caller context -- so I need to change the current `LocationContext` and `Block` at the middle of this big method. This means that I need to discard my invariant that `setCurrLocationContextAndBlock` is only called once per each `dispatchWorkItem`; but I think this exceptional case (first half in callee, second half in caller) is still clear enough. In addition to this main goal, I perform many small changes to clarify and modernize the code of this old method. From 63e25a149eb21b46a4de3d731e0801c780aa4b4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 11 Mar 2026 17:45:14 +0100 Subject: [PATCH 01/11] [NFC][analyzer] Spelling fix: s/Binded/Bound/ --- .../StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index a4a22ce10952c..ed85bd5884813 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -270,7 +270,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { CallEventManager &CEMgr = getStateManager().getCallEventManager(); CallEventRef<> Call = CEMgr.getCaller(calleeCtx, state); - // Step 2: generate node with bound return value: CEBNode -> BindedRetNode. + // Step 2: generate node with bound return value: CEBNode -> BoundRetNode. // If this variable is set to 'true' the analyzer will evaluate the call // statement we are about to exit again, instead of continuing the execution @@ -342,7 +342,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { state, dyn_cast_or_null<CXXConstructExpr>(CE), callerCtx); } - // Step 3: BindedRetNode -> CleanedNodes + // Step 3: BoundRetNode -> CleanedNodes // If we can find a statement and a block in the inlined function, run remove // dead bindings before returning from the call. This is important to ensure // that we report the issues such as leaks in the stack contexts in which @@ -357,18 +357,18 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { const CFGBlock *PrePurgeBlock = isa<ReturnStmt>(LastSt) ? Blk : &CEBNode->getCFG().getExit(); bool isNew; - ExplodedNode *BindedRetNode = G.getNode(Loc, state, false, &isNew); - BindedRetNode->addPredecessor(CEBNode, G); + ExplodedNode *BoundRetNode = G.getNode(Loc, state, false, &isNew); + BoundRetNode->addPredecessor(CEBNode, G); if (!isNew) return; - NodeBuilderContext Ctx(getCoreEngine(), PrePurgeBlock, BindedRetNode); + NodeBuilderContext Ctx(getCoreEngine(), PrePurgeBlock, BoundRetNode); currBldrCtx = &Ctx; // Here, we call the Symbol Reaper with 0 statement and callee location // context, telling it to clean up everything in the callee's context // (and its children). We use the callee's function body as a diagnostic // statement, with which the program point will be associated. - removeDead(BindedRetNode, CleanedNodes, nullptr, calleeCtx, + removeDead(BoundRetNode, CleanedNodes, nullptr, calleeCtx, calleeCtx->getAnalysisDeclContext()->getBody(), ProgramPoint::PostStmtPurgeDeadSymbolsKind); currBldrCtx = nullptr; From 14430776c5d4a6488dd462447d5bc3df836a533e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 14:20:28 +0100 Subject: [PATCH 02/11] [NFC][analyzer] Shorten a comment Replace a verbose comment with inline marks that name the arguments of the following method call. By the way, the "0 statement" in the original comment refers to an integer literal zero that was originally written instead of 'nullptr'. --- .../lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index ed85bd5884813..227cfbc488901 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -364,12 +364,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { NodeBuilderContext Ctx(getCoreEngine(), PrePurgeBlock, BoundRetNode); currBldrCtx = &Ctx; - // Here, we call the Symbol Reaper with 0 statement and callee location - // context, telling it to clean up everything in the callee's context - // (and its children). We use the callee's function body as a diagnostic - // statement, with which the program point will be associated. - removeDead(BoundRetNode, CleanedNodes, nullptr, calleeCtx, - calleeCtx->getAnalysisDeclContext()->getBody(), + // We call removeDead in the context of the callee. + removeDead(BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, calleeCtx, + /*DiagnosticStmt=*/calleeCtx->getAnalysisDeclContext()->getBody(), ProgramPoint::PostStmtPurgeDeadSymbolsKind); currBldrCtx = nullptr; } else { From 73a2a0de8d90a11688695ed7c0e0363f0a5ca139 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 15:05:49 +0100 Subject: [PATCH 03/11] [NFC][analyzer] Use setCurrLocationContextAndBlock in processCallExit To replace the old code that directly accessed `currBldrCtx`. In my recent commit 1c424bfb03d6dd4b994a0d549e1f3e23852f1e16 I introduced an invariant that the current `LocationContext` and `Block` is set only once per `dispatchWorkItem`; unfortunately now I need to relax this because here the first half of the `CallExit` procedure (`removeDead`) happens within the callee context, while the second half (`PostCall` and similar callbacks) happen in the caller context. --- .../Core/PathSensitive/ExprEngine.h | 11 ++++++++--- .../Core/ExprEngineCallAndReturn.cpp | 17 +++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 2023a7a5b1ac8..b3ef09d464685 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -246,10 +246,15 @@ class ExprEngine { // This implementation is a temporary measure to allow a gradual transition. void setCurrLocationContextAndBlock(const LocationContext *LC, const CFGBlock *B) { - // Note that there is a call to resetCurrLocationContextAndBlock at the - // beginning of dispatchWorkItem. + // The current LocationContext and Block is reset at the beginning of + // dispacthWorkItem. Ideally, this method should be called only once per + // dipatchWorkItem call (= elementary analysis step); so the following + // assertion is there to catch accidental repeated calls. If the current + // LocationContext and Block needs to change in the middle of a single step + // (which currently happens only once, in processCallExit), use an explicit + // call to resetCurrLocationContextAndBlock. assert(!currBldrCtx && !OwnedCurrBldrCtx && - "This should be called at most once per call to dispatchWorkItem"); + "The current LocationContext and Block is already set"); OwnedCurrBldrCtx.emplace(Engine, B, LC); currBldrCtx = &*OwnedCurrBldrCtx; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 227cfbc488901..32a0ea56a6b7d 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -362,13 +362,14 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { if (!isNew) return; - NodeBuilderContext Ctx(getCoreEngine(), PrePurgeBlock, BoundRetNode); - currBldrCtx = &Ctx; // We call removeDead in the context of the callee. - removeDead(BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, calleeCtx, - /*DiagnosticStmt=*/calleeCtx->getAnalysisDeclContext()->getBody(), - ProgramPoint::PostStmtPurgeDeadSymbolsKind); - currBldrCtx = nullptr; + setCurrLocationContextAndBlock(BoundRetNode->getLocationContext(), + PrePurgeBlock); + removeDead( + BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, calleeCtx, + /*DiagnosticStmt=*/calleeCtx->getAnalysisDeclContext()->getBody(), + ProgramPoint::PostStmtPurgeDeadSymbolsKind); + resetCurrLocationContextAndBlock(); } else { CleanedNodes.Add(CEBNode); } @@ -388,8 +389,8 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 5: Perform the post-condition check of the CallExpr and enqueue the // result onto the work list. // CEENode -> Dst -> WorkList - NodeBuilderContext Ctx(Engine, calleeCtx->getCallSiteBlock(), CEENode); - SaveAndRestore<const NodeBuilderContext *> NBCSave(currBldrCtx, &Ctx); + setCurrLocationContextAndBlock(CEENode->getLocationContext(), + calleeCtx->getCallSiteBlock()); SaveAndRestore CBISave(currStmtIdx, calleeCtx->getIndex()); CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState); From b26f718b86ba12f8e7f6a48109c4b0c51d0af2f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 16:43:15 +0100 Subject: [PATCH 04/11] [NFC][analyzer] Capitalize variables in processCallExit --- .../Core/ExprEngineCallAndReturn.cpp | 71 +++++++++---------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 32a0ea56a6b7d..c31c7d3c6e5ba 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -251,24 +251,23 @@ ProgramStateRef ExprEngine::removeStateTraitsUsedForArrayEvaluation( /// 5. PostStmt<CallExpr> void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 1 CEBNode was generated before the call. - const StackFrameContext *calleeCtx = CEBNode->getStackFrame(); + const StackFrameContext *CalleeCtx = CEBNode->getStackFrame(); // The parent context might not be a stack frame, so make sure we // look up the first enclosing stack frame. - const StackFrameContext *callerCtx = - calleeCtx->getParent()->getStackFrame(); + const StackFrameContext *CallerCtx = CalleeCtx->getParent()->getStackFrame(); - const Stmt *CE = calleeCtx->getCallSite(); - ProgramStateRef state = CEBNode->getState(); + const Stmt *CE = CalleeCtx->getCallSite(); + ProgramStateRef State = CEBNode->getState(); // Find the last statement in the function and the corresponding basic block. const Stmt *LastSt = nullptr; const CFGBlock *Blk = nullptr; std::tie(LastSt, Blk) = getLastStmt(CEBNode); - // Generate a CallEvent /before/ cleaning the state, so that we can get the + // Generate a CallEvent /before/ cleaning the State, so that we can get the // correct value for 'this' (if necessary). CallEventManager &CEMgr = getStateManager().getCallEventManager(); - CallEventRef<> Call = CEMgr.getCaller(calleeCtx, state); + CallEventRef<> Call = CEMgr.getCaller(CalleeCtx, State); // Step 2: generate node with bound return value: CEBNode -> BoundRetNode. @@ -281,11 +280,11 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { if (const auto *DtorDecl = dyn_cast_or_null<CXXDestructorDecl>(Call->getDecl())) { - if (auto Idx = getPendingArrayDestruction(state, callerCtx)) { + if (auto Idx = getPendingArrayDestruction(State, CallerCtx)) { ShouldRepeatCall = *Idx > 0; - auto ThisVal = svalBuilder.getCXXThis(DtorDecl->getParent(), calleeCtx); - state = state->killBinding(ThisVal); + auto ThisVal = svalBuilder.getCXXThis(DtorDecl->getParent(), CalleeCtx); + State = State->killBinding(ThisVal); } } @@ -293,12 +292,12 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { if (CE) { if (const ReturnStmt *RS = dyn_cast_or_null<ReturnStmt>(LastSt)) { const LocationContext *LCtx = CEBNode->getLocationContext(); - SVal V = state->getSVal(RS, LCtx); + SVal V = State->getSVal(RS, LCtx); // Ensure that the return type matches the type of the returned Expr. - if (wasDifferentDeclUsedForInlining(Call, calleeCtx)) { + if (wasDifferentDeclUsedForInlining(Call, CalleeCtx)) { QualType ReturnedTy = - CallEvent::getDeclaredResultType(calleeCtx->getDecl()); + CallEvent::getDeclaredResultType(CalleeCtx->getDecl()); if (!ReturnedTy.isNull()) { if (const Expr *Ex = dyn_cast<Expr>(CE)) { V = adjustReturnValue(V, Ex->getType(), ReturnedTy, @@ -307,18 +306,18 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { } } - state = state->BindExpr(CE, callerCtx, V); + State = State->BindExpr(CE, CallerCtx, V); } // Bind the constructed object value to CXXConstructExpr. if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(CE)) { loc::MemRegionVal This = - svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx); - SVal ThisV = state->getSVal(This); - ThisV = state->getSVal(ThisV.castAs<Loc>()); - state = state->BindExpr(CCE, callerCtx, ThisV); + svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), CalleeCtx); + SVal ThisV = State->getSVal(This); + ThisV = State->getSVal(ThisV.castAs<Loc>()); + State = State->BindExpr(CCE, CallerCtx, ThisV); - ShouldRepeatCall = shouldRepeatCtorCall(state, CCE, callerCtx); + ShouldRepeatCall = shouldRepeatCtorCall(State, CCE, CallerCtx); } if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) { @@ -327,19 +326,19 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // region for later use. // Additionally cast the return value of the inlined operator new // (which is of type 'void *') to the correct object type. - SVal AllocV = state->getSVal(CNE, callerCtx); + SVal AllocV = State->getSVal(CNE, CallerCtx); AllocV = svalBuilder.evalCast( AllocV, CNE->getType(), getContext().getPointerType(getContext().VoidTy)); - state = addObjectUnderConstruction(state, CNE, calleeCtx->getParent(), + State = addObjectUnderConstruction(State, CNE, CalleeCtx->getParent(), AllocV); } } if (!ShouldRepeatCall) { - state = removeStateTraitsUsedForArrayEvaluation( - state, dyn_cast_or_null<CXXConstructExpr>(CE), callerCtx); + State = removeStateTraitsUsedForArrayEvaluation( + State, dyn_cast_or_null<CXXConstructExpr>(CE), CallerCtx); } // Step 3: BoundRetNode -> CleanedNodes @@ -349,15 +348,15 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // they occurred. ExplodedNodeSet CleanedNodes; if (LastSt && Blk && AMgr.options.AnalysisPurgeOpt != PurgeNone) { - static SimpleProgramPointTag retValBind("ExprEngine", "Bind Return Value"); + static SimpleProgramPointTag RetValBind("ExprEngine", "Bind Return Value"); auto Loc = isa<ReturnStmt>(LastSt) - ? ProgramPoint{PostStmt(LastSt, calleeCtx, &retValBind)} - : ProgramPoint{EpsilonPoint(calleeCtx, /*Data1=*/nullptr, - /*Data2=*/nullptr, &retValBind)}; + ? ProgramPoint{PostStmt(LastSt, CalleeCtx, &RetValBind)} + : ProgramPoint{EpsilonPoint(CalleeCtx, /*Data1=*/nullptr, + /*Data2=*/nullptr, &RetValBind)}; const CFGBlock *PrePurgeBlock = isa<ReturnStmt>(LastSt) ? Blk : &CEBNode->getCFG().getExit(); bool isNew; - ExplodedNode *BoundRetNode = G.getNode(Loc, state, false, &isNew); + ExplodedNode *BoundRetNode = G.getNode(Loc, State, false, &isNew); BoundRetNode->addPredecessor(CEBNode, G); if (!isNew) return; @@ -366,8 +365,8 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { setCurrLocationContextAndBlock(BoundRetNode->getLocationContext(), PrePurgeBlock); removeDead( - BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, calleeCtx, - /*DiagnosticStmt=*/calleeCtx->getAnalysisDeclContext()->getBody(), + BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, CalleeCtx, + /*DiagnosticStmt=*/CalleeCtx->getAnalysisDeclContext()->getBody(), ProgramPoint::PostStmtPurgeDeadSymbolsKind); resetCurrLocationContextAndBlock(); } else { @@ -377,9 +376,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { for (ExplodedNode *N : CleanedNodes) { // Step 4: Generate the CallExit and leave the callee's context. // CleanedNodes -> CEENode - CallExitEnd Loc(calleeCtx, callerCtx); + CallExitEnd Loc(CalleeCtx, CallerCtx); bool isNew; - ProgramStateRef CEEState = (N == CEBNode) ? state : N->getState(); + ProgramStateRef CEEState = (N == CEBNode) ? State : N->getState(); ExplodedNode *CEENode = G.getNode(Loc, CEEState, false, &isNew); CEENode->addPredecessor(N, G); @@ -390,8 +389,8 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // result onto the work list. // CEENode -> Dst -> WorkList setCurrLocationContextAndBlock(CEENode->getLocationContext(), - calleeCtx->getCallSiteBlock()); - SaveAndRestore CBISave(currStmtIdx, calleeCtx->getIndex()); + CalleeCtx->getCallSiteBlock()); + SaveAndRestore CBISave(currStmtIdx, CalleeCtx->getIndex()); CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState); @@ -428,9 +427,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Enqueue the next element in the block. for (ExplodedNodeSet::iterator PSI = Dst.begin(), PSE = Dst.end(); PSI != PSE; ++PSI) { - unsigned Idx = calleeCtx->getIndex() + (ShouldRepeatCall ? 0 : 1); + unsigned Idx = CalleeCtx->getIndex() + (ShouldRepeatCall ? 0 : 1); - Engine.getWorkList()->enqueue(*PSI, calleeCtx->getCallSiteBlock(), Idx); + Engine.getWorkList()->enqueue(*PSI, CalleeCtx->getCallSiteBlock(), Idx); } } } From 5d62a79769d5cd7126e5b66882a34d20b2e41861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 16:59:42 +0100 Subject: [PATCH 05/11] [NFC][analyzer] Use structured binding in processCallExit --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index c31c7d3c6e5ba..2597c3f7b46d3 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -260,9 +260,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { const Stmt *CE = CalleeCtx->getCallSite(); ProgramStateRef State = CEBNode->getState(); // Find the last statement in the function and the corresponding basic block. - const Stmt *LastSt = nullptr; - const CFGBlock *Blk = nullptr; - std::tie(LastSt, Blk) = getLastStmt(CEBNode); + auto [LastSt, Blk] = getLastStmt(CEBNode); // Generate a CallEvent /before/ cleaning the State, so that we can get the // correct value for 'this' (if necessary). From 39e65cbea60793e364e27348ee9ea46d5d1f154a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 17:04:23 +0100 Subject: [PATCH 06/11] [NFC][analyzer] Use makeNode instead of bare graph operations ...in processCallExit. --- .../StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 2597c3f7b46d3..916a69ab9f2e8 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -353,10 +353,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { /*Data2=*/nullptr, &RetValBind)}; const CFGBlock *PrePurgeBlock = isa<ReturnStmt>(LastSt) ? Blk : &CEBNode->getCFG().getExit(); - bool isNew; - ExplodedNode *BoundRetNode = G.getNode(Loc, State, false, &isNew); - BoundRetNode->addPredecessor(CEBNode, G); - if (!isNew) + + ExplodedNode *BoundRetNode = Engine.makeNode(Loc, State, CEBNode); + if (!BoundRetNode) return; // We call removeDead in the context of the callee. @@ -375,12 +374,10 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 4: Generate the CallExit and leave the callee's context. // CleanedNodes -> CEENode CallExitEnd Loc(CalleeCtx, CallerCtx); - bool isNew; ProgramStateRef CEEState = (N == CEBNode) ? State : N->getState(); - ExplodedNode *CEENode = G.getNode(Loc, CEEState, false, &isNew); - CEENode->addPredecessor(N, G); - if (!isNew) + ExplodedNode *CEENode = Engine.makeNode(Loc, CEEState, N); + if (!CEENode) return; // Step 5: Perform the post-condition check of the CallExpr and enqueue the From f96112f785943c72a2164450c2194257db0876ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 17:08:24 +0100 Subject: [PATCH 07/11] [NFC][analyzer] De-duplicate a repeated statement --- .../StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 916a69ab9f2e8..b632efb1549d7 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -389,22 +389,21 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState); + ExplodedNodeSet DstPostPostCallCallback; + getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback, CEENode, + *UpdatedCall, *this, + /*wasInlined=*/true); ExplodedNodeSet DstPostCall; if (llvm::isa_and_nonnull<CXXNewExpr>(CE)) { - ExplodedNodeSet DstPostPostCallCallback; - getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback, - CEENode, *UpdatedCall, *this, - /*wasInlined=*/true); for (ExplodedNode *I : DstPostPostCallCallback) { getCheckerManager().runCheckersForNewAllocator( cast<CXXAllocatorCall>(*UpdatedCall), DstPostCall, I, *this, /*wasInlined=*/true); } } else { - getCheckerManager().runCheckersForPostCall(DstPostCall, CEENode, - *UpdatedCall, *this, - /*wasInlined=*/true); + DstPostCall.insert(DstPostPostCallCallback); } + ExplodedNodeSet Dst; if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { getCheckerManager().runCheckersForPostObjCMessage(Dst, DstPostCall, *Msg, From 2faa6c8c019fd0270dae832f3ca0a2a40208e78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 17:13:24 +0100 Subject: [PATCH 08/11] [NFC][analyzer] Specify current LocationContext directly The `LocationContext` of an `ExplodedNode` is the `LocationContext` of its location (a `ProgramPoint`), so we can increase the clarity of the code by specifying it directly. --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index b632efb1549d7..48892fd21d21f 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -359,8 +359,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { return; // We call removeDead in the context of the callee. - setCurrLocationContextAndBlock(BoundRetNode->getLocationContext(), - PrePurgeBlock); + setCurrLocationContextAndBlock(CalleeCtx, PrePurgeBlock); removeDead( BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, CalleeCtx, /*DiagnosticStmt=*/CalleeCtx->getAnalysisDeclContext()->getBody(), @@ -383,8 +382,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 5: Perform the post-condition check of the CallExpr and enqueue the // result onto the work list. // CEENode -> Dst -> WorkList - setCurrLocationContextAndBlock(CEENode->getLocationContext(), - CalleeCtx->getCallSiteBlock()); + setCurrLocationContextAndBlock(CallerCtx, CalleeCtx->getCallSiteBlock()); SaveAndRestore CBISave(currStmtIdx, CalleeCtx->getIndex()); CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState); From 5fb40d14a7bab502a44ecea1814903ae753e6d13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 17:22:55 +0100 Subject: [PATCH 09/11] [NFC][analyzer] Move setCurrLocationContextAndBlock outwards The old model was that the current location context and block was filled on a just-when-needed basis (and it was undefined otherwise). However, this caused lots of redundancy when multiple alternative cases or consecutive steps needed the same current location context and block (and duplicated the code specifying it). To fix this, I recently switched to a model where this information is set near the beginning of each `dispatcWorkItem` step and remains valid through the whole step. To convert `processCallExit` to this new paradigm, I'm moving the `setCurrLocationContextAndBlock` calls to the top level of the function. --- .../Core/ExprEngineCallAndReturn.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 48892fd21d21f..896eb0cda16a5 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -262,6 +262,10 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Find the last statement in the function and the corresponding basic block. auto [LastSt, Blk] = getLastStmt(CEBNode); + const CFGBlock *PrePurgeBlock = + isa_and_nonnull<ReturnStmt>(LastSt) ? Blk : &CEBNode->getCFG().getExit(); + setCurrLocationContextAndBlock(CalleeCtx, PrePurgeBlock); + // Generate a CallEvent /before/ cleaning the State, so that we can get the // correct value for 'this' (if necessary). CallEventManager &CEMgr = getStateManager().getCallEventManager(); @@ -351,24 +355,24 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { ? ProgramPoint{PostStmt(LastSt, CalleeCtx, &RetValBind)} : ProgramPoint{EpsilonPoint(CalleeCtx, /*Data1=*/nullptr, /*Data2=*/nullptr, &RetValBind)}; - const CFGBlock *PrePurgeBlock = - isa<ReturnStmt>(LastSt) ? Blk : &CEBNode->getCFG().getExit(); ExplodedNode *BoundRetNode = Engine.makeNode(Loc, State, CEBNode); if (!BoundRetNode) return; // We call removeDead in the context of the callee. - setCurrLocationContextAndBlock(CalleeCtx, PrePurgeBlock); removeDead( BoundRetNode, CleanedNodes, /*ReferenceStmt=*/nullptr, CalleeCtx, /*DiagnosticStmt=*/CalleeCtx->getAnalysisDeclContext()->getBody(), ProgramPoint::PostStmtPurgeDeadSymbolsKind); - resetCurrLocationContextAndBlock(); } else { CleanedNodes.Add(CEBNode); } + resetCurrLocationContextAndBlock(); + setCurrLocationContextAndBlock(CallerCtx, CalleeCtx->getCallSiteBlock()); + SaveAndRestore CBISave(currStmtIdx, CalleeCtx->getIndex()); + for (ExplodedNode *N : CleanedNodes) { // Step 4: Generate the CallExit and leave the callee's context. // CleanedNodes -> CEENode @@ -382,8 +386,6 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 5: Perform the post-condition check of the CallExpr and enqueue the // result onto the work list. // CEENode -> Dst -> WorkList - setCurrLocationContextAndBlock(CallerCtx, CalleeCtx->getCallSiteBlock()); - SaveAndRestore CBISave(currStmtIdx, CalleeCtx->getIndex()); CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState); From 6e8873cf515c6dc43a5092db37ab28eec337b701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 17:39:53 +0100 Subject: [PATCH 10/11] [NFC][analyzer] Improve comments in processCallExit --- .../StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 896eb0cda16a5..0df1a272bf594 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -247,8 +247,10 @@ ProgramStateRef ExprEngine::removeStateTraitsUsedForArrayEvaluation( /// 1. CallExitBegin (triggers the start of call exit sequence) /// 2. Bind the return value /// 3. Run Remove dead bindings to clean up the dead symbols from the callee. -/// 4. CallExitEnd (switch to the caller context) +/// 4. CallExitEnd /// 5. PostStmt<CallExpr> +/// Steps 1-3. happen in the callee context; but there is a context switch and +/// steps 4-5. happen in the caller context. void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 1 CEBNode was generated before the call. const StackFrameContext *CalleeCtx = CEBNode->getStackFrame(); @@ -264,6 +266,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { const CFGBlock *PrePurgeBlock = isa_and_nonnull<ReturnStmt>(LastSt) ? Blk : &CEBNode->getCFG().getExit(); + // The first half of this process happens in the callee context: setCurrLocationContextAndBlock(CalleeCtx, PrePurgeBlock); // Generate a CallEvent /before/ cleaning the State, so that we can get the @@ -369,12 +372,15 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { CleanedNodes.Add(CEBNode); } + // The second half of this process happens in the caller context. This is an + // exception to the general rule that the current LocationContext and Block + // stay the same within a single call to dispatchWorkItem. resetCurrLocationContextAndBlock(); setCurrLocationContextAndBlock(CallerCtx, CalleeCtx->getCallSiteBlock()); SaveAndRestore CBISave(currStmtIdx, CalleeCtx->getIndex()); for (ExplodedNode *N : CleanedNodes) { - // Step 4: Generate the CallExit and leave the callee's context. + // Step 4: Generate the CallExitEnd node. // CleanedNodes -> CEENode CallExitEnd Loc(CalleeCtx, CallerCtx); ProgramStateRef CEEState = (N == CEBNode) ? State : N->getState(); From 67dfa687e4ca553338dd6ce6f9616b0d3d0d15d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 12 Mar 2026 17:44:51 +0100 Subject: [PATCH 11/11] [NFC][analyzer] Modernize a for loop in processCallExit --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 0df1a272bf594..f6ba3699312ec 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -425,11 +425,11 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { } // Enqueue the next element in the block. - for (ExplodedNodeSet::iterator PSI = Dst.begin(), PSE = Dst.end(); - PSI != PSE; ++PSI) { + for (ExplodedNode *DstNode : Dst) { unsigned Idx = CalleeCtx->getIndex() + (ShouldRepeatCall ? 0 : 1); - Engine.getWorkList()->enqueue(*PSI, CalleeCtx->getCallSiteBlock(), Idx); + Engine.getWorkList()->enqueue(DstNode, CalleeCtx->getCallSiteBlock(), + Idx); } } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
