https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/203923
This change removes `NodeBuilder`s from the functions connected to `defaultEvalCall` that were previously passing around `NodeBuilder` arguments instead of the more usual `ExplodedNodeSet &Dst` out-parameters. Although these `NodeBuilder`s "travelled through" many functions, their usage pattern was relatively simple and their back-and-forth set manipulation didn't provide any advantage over a plain exploded node set. In addition to the removal of the `NodeBuilder`s, this commit performs minor simplifications in the affected code and renames the old method `BifurcateCall` to the more specific `dynDispatchBifurcate` (because the old name was too vague now that we also have `ctuBifurcate`). ------ Note for reviewers: **I strongly suggest reviewing commit by commit** because the individual commits clearly show the "sweep" through the call graph that gradually extracts and then removes the `takeNodes` operations. Before creating this PR, I made the following call graph of the affected functions; this may be useful during the review: <img width="1881" height="971" alt="call_eval_call_graph" src="https://github.com/user-attachments/assets/e32f94b6-8322-4d91-8914-7a18a258ca2e"; /> From 77fc48d9972d8ce9ab820ccef1078a29506b4e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 11 Jun 2026 14:31:09 +0200 Subject: [PATCH 01/13] Remove NodeBuilder from ExprEngine::inlineCall This method took a NodeBuilder as an argument, unconditionally called `Bldr.takeNodes(Pred)` on it and did nothing else with it. This commit removes the `NodeBuilder` argument and temporarily places the `Bldr.takeNodes(Pred)` calls just before the call sites of `inlineCall`. Follow-up commits will consolidate (in fact, eliminate) these repeated calls. --- .../Core/PathSensitive/ExprEngine.h | 2 +- .../Core/ExprEngineCallAndReturn.cpp | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index bb2de45cec92a..5667375d37bb3 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -908,7 +908,7 @@ class ExprEngine { const StackFrame *SF); void inlineCall(WorkList *WList, const CallEvent &Call, const Decl *D, - NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State); + ExplodedNode *Pred, ProgramStateRef State); void ctuBifurcate(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 4cbcaa2721639..50e82c8d4b220 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -502,13 +502,15 @@ void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, (IK == CTUPhase1InliningKind::Small && isSmall(AMgr.getAnalysisDeclContext(D))); if (DoInline) { - inlineCall(Engine.getWorkList(), Call, D, Bldr, Pred, State); + Bldr.takeNodes(Pred); + inlineCall(Engine.getWorkList(), Call, D, Pred, State); return; } const bool BState = State->get<CTUDispatchBifurcation>(); if (!BState) { // This is the first time we see this foreign function. // Enqueue it to be analyzed in the second (ctu) phase. - inlineCall(Engine.getCTUWorkList(), Call, D, Bldr, Pred, State); + Bldr.takeNodes(Pred); + inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State); // Conservatively evaluate in the first phase. ConservativeEvalState = State->set<CTUDispatchBifurcation>(true); conservativeEvalCall(Call, Bldr, Pred, ConservativeEvalState); @@ -517,12 +519,13 @@ void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, } return; } - inlineCall(Engine.getWorkList(), Call, D, Bldr, Pred, State); + Bldr.takeNodes(Pred); + inlineCall(Engine.getWorkList(), Call, D, Pred, State); } void ExprEngine::inlineCall(WorkList *WList, const CallEvent &Call, - const Decl *D, NodeBuilder &Bldr, - ExplodedNode *Pred, ProgramStateRef State) { + const Decl *D, ExplodedNode *Pred, + ProgramStateRef State) { assert(D); const StackFrame *CallerSF = Pred->getStackFrame(); @@ -556,10 +559,6 @@ void ExprEngine::inlineCall(WorkList *WList, const CallEvent &Call, WList->enqueue(N); } - // If we decided to inline the call, the successor has been manually - // added onto the work list so remove it from the node builder. - Bldr.takeNodes(Pred); - NumInlinedCalls++; Engine.FunctionSummaries->bumpNumTimesInlined(D); From 930c1bfb2e3ec0097409b398fe27f5b04f615fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Thu, 11 Jun 2026 15:56:17 +0200 Subject: [PATCH 02/13] Remove NodeBuilder from conservativeEvalCall Previously `ExprEngine::conservativeEvalCall` used `generateNode` to make a single node and place it in the `Frontier` (destination set) of the `NodeBuilder`. The method `NodeBuilder::generateNode` removes the predecessor (`Pred`) from the `Frontier` of the `NodeBuilder` instance, makes a new node and then adds it to the `Frontier`, which is equivalent to a chain of `takeNodes(Pred)` then `makeNode` then `addNodes`. This commit decomposes the `generateNode` call into these steps and moves the `NodeBuilder` manipulation out of `conservativeEvalCall`, which will now just make a node (after creating the right `State`) and return it. Follow-up commits will consolidate and eliminate the `takeNodes` calls which now appear separately on practically every each execution path. (And the `addNodes` call will be replaced with `ExplodedNodeSet::insert` when the node builder is eliminated.) --- .../Core/PathSensitive/ExprEngine.h | 4 ++-- .../Core/ExprEngineCallAndReturn.cpp | 24 ++++++++++++------- 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 5667375d37bb3..ae29f35a950b3 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -918,8 +918,8 @@ class ExprEngine { /// Conservatively evaluate call by invalidating regions and binding /// a conjured return value. - void conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr, - ExplodedNode *Pred, ProgramStateRef State); + ExplodedNode *conservativeEvalCall(const CallEvent &Call, ExplodedNode *Pred, + ProgramStateRef State); /// Either inline or process the call conservatively (or both), based /// on DynamicDispatchBifurcation data. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 50e82c8d4b220..2d5caa3b189f6 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -513,9 +513,10 @@ void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State); // Conservatively evaluate in the first phase. ConservativeEvalState = State->set<CTUDispatchBifurcation>(true); - conservativeEvalCall(Call, Bldr, Pred, ConservativeEvalState); + Bldr.addNodes(conservativeEvalCall(Call, Pred, ConservativeEvalState)); } else { - conservativeEvalCall(Call, Bldr, Pred, State); + Bldr.takeNodes(Pred); + Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); } return; } @@ -836,14 +837,15 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call, // Conservatively evaluate call by invalidating regions and binding // a conjured return value. -void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr, - ExplodedNode *Pred, ProgramStateRef State) { +ExplodedNode *ExprEngine::conservativeEvalCall(const CallEvent &Call, + ExplodedNode *Pred, + ProgramStateRef State) { State = Call.invalidateRegions(getNumVisitedCurrent(), State); State = bindReturnValue(Call, Pred->getStackFrame(), State); // And make the result node. static SimpleProgramPointTag PT("ExprEngine", "Conservative eval call"); - Bldr.generateNode(Call.getProgramPoint(false, &PT), State, Pred); + return Engine.makeNode(Call.getProgramPoint(false, &PT), State, Pred); } ExprEngine::CallInlinePolicy @@ -1252,7 +1254,8 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Don't inline if we're not in any dynamic dispatch mode. if (Options.getIPAMode() != IPAK_DynamicDispatch) { - conservativeEvalCall(Call, Bldr, Pred, State); + Bldr.takeNodes(Pred); + Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); return; } } @@ -1267,7 +1270,8 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, State, dyn_cast_or_null<CXXConstructExpr>(E), Call.getStackFrame()); // Also handle the return value and invalidate the regions. - conservativeEvalCall(Call, Bldr, Pred, State); + Bldr.takeNodes(Pred); + Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); } void ExprEngine::BifurcateCall(const MemRegion *BifurReg, @@ -1288,7 +1292,8 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, // If inline failed, or we are on the path where we assume we // don't have enough info about the receiver to inline, conjure the // return value and invalidate the regions. - conservativeEvalCall(Call, Bldr, Pred, State); + Bldr.takeNodes(Pred); + Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); return; } @@ -1302,7 +1307,8 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, ProgramStateRef NoIState = State->set<DynamicDispatchBifurcationMap>(BifurReg, DynamicDispatchModeConservative); - conservativeEvalCall(Call, Bldr, Pred, NoIState); + Bldr.takeNodes(Pred); + Bldr.addNodes(conservativeEvalCall(Call, Pred, NoIState)); NumOfDynamicDispatchPathSplits++; } From 6263ca71fbfbe616ab770843ca489a75e25502b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Fri, 12 Jun 2026 16:15:06 +0200 Subject: [PATCH 03/13] Rename variable Dst to DstEval in performTrivialCopy ...because I want to replace the `NodeBuilder` (which currently collects the new nodes) with a customary out-parameter `ExplodedNodeSet &Dst` in a follow-up change, and this local occupies that name. --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 291533d0c3289..3f259a3b5b769 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -67,7 +67,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, const StackFrame *SF = Pred->getStackFrame(); const Expr *CallExpr = Call.getOriginExpr(); - ExplodedNodeSet Dst; + ExplodedNodeSet DstEval; Bldr.takeNodes(Pred); assert(ThisRD); @@ -87,17 +87,17 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V, /*isLoad=*/true); for (ExplodedNode *N : Tmp) - evalBind(Dst, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue); + evalBind(DstEval, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue); } else { // We can't copy empty classes because of empty base class optimization. // In that case, copying the empty base class subobject would overwrite the // object that it overlaps with - so let's not do that. // See issue-157467.cpp for an example. - Dst.insert(Pred); + DstEval.insert(Pred); } PostStmt PS(CallExpr, SF); - for (ExplodedNode *N : Dst) { + for (ExplodedNode *N : DstEval) { ProgramStateRef State = N->getState(); if (AlwaysReturnsLValue) State = State->BindExpr(CallExpr, SF, ThisVal); From 9ca2b560f6bba60bd59ff6360c8f29cab63e5541 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Fri, 12 Jun 2026 16:21:02 +0200 Subject: [PATCH 04/13] Extract Bldr.takeNodes(Pred) from performTrivialCopy `performTrivialCopy` unconditionally calls `Bldr.takeNodes(Pred)` so this commit moves it out of the function. In `handleConstructor` this "cancels out" the side effect of the constructor of the node builder `Bldr`, which inserted all elements of `DstPreCall` into `DstEvaluated`. (Removing `DstPreCall` from the argument list of the constructor of `Bldr` means skipping this initial insertion -- which is equivalent to inserting them and taking out each node in the subsequent for loop.) In `defaultEvalCall` this commit just places `Bldr.takeNodes(Pred)` before the call to `performTrivialCopy`. As this also appears on many alternative execution paths, follow-up commits will be able to consolidate its uses. --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 3 +-- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 3f259a3b5b769..e1d5869d95e21 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -68,7 +68,6 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, const Expr *CallExpr = Call.getOriginExpr(); ExplodedNodeSet DstEval; - Bldr.takeNodes(Pred); assert(ThisRD); @@ -729,7 +728,7 @@ void ExprEngine::handleConstructor(const Expr *E, if (CE && CE->getConstructor()->isTrivial() && CE->getConstructor()->isCopyOrMoveConstructor() && !CallOpts.IsArrayCtorOrDtor) { - NodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx); + NodeBuilder Bldr(DstEvaluated, *currBldrCtx); // FIXME: Handle other kinds of trivial constructors as well. for (ExplodedNode *N : DstPreCall) performTrivialCopy(Bldr, N, *Call); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 2d5caa3b189f6..a111db8653f07 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -1228,6 +1228,7 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Special-case trivial assignment operators. if (isTrivialObjectAssignment(Call)) { + Bldr.takeNodes(Pred); performTrivialCopy(Bldr, Pred, Call); return; } From 1b5c08d558b2f14676f8ce1bf10c86d27f3540f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Fri, 12 Jun 2026 17:04:19 +0200 Subject: [PATCH 05/13] Extract Bldr.takeNodes(Pred) from ctuBifurcate It was called on all paths within `ctuBifurcate`, place it before each call to `ctuBifurcate` instead. These will be moved further by the follow-up commits. --- .../StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index a111db8653f07..159e642f91c83 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -502,25 +502,21 @@ void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, (IK == CTUPhase1InliningKind::Small && isSmall(AMgr.getAnalysisDeclContext(D))); if (DoInline) { - Bldr.takeNodes(Pred); inlineCall(Engine.getWorkList(), Call, D, Pred, State); return; } const bool BState = State->get<CTUDispatchBifurcation>(); if (!BState) { // This is the first time we see this foreign function. // Enqueue it to be analyzed in the second (ctu) phase. - Bldr.takeNodes(Pred); inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State); // Conservatively evaluate in the first phase. ConservativeEvalState = State->set<CTUDispatchBifurcation>(true); Bldr.addNodes(conservativeEvalCall(Call, Pred, ConservativeEvalState)); } else { - Bldr.takeNodes(Pred); Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); } return; } - Bldr.takeNodes(Pred); inlineCall(Engine.getWorkList(), Call, D, Pred, State); } @@ -1260,6 +1256,7 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, return; } } + Bldr.takeNodes(Pred); ctuBifurcate(Call, D, Bldr, Pred, State); return; } @@ -1288,8 +1285,10 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, State->get<DynamicDispatchBifurcationMap>(BifurReg); if (BState) { // If we are on "inline path", keep inlining if possible. - if (*BState == DynamicDispatchModeInlined) + if (*BState == DynamicDispatchModeInlined) { + Bldr.takeNodes(Pred); ctuBifurcate(Call, D, Bldr, Pred, State); + } // If inline failed, or we are on the path where we assume we // don't have enough info about the receiver to inline, conjure the // return value and invalidate the regions. @@ -1303,6 +1302,7 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, ProgramStateRef IState = State->set<DynamicDispatchBifurcationMap>(BifurReg, DynamicDispatchModeInlined); + Bldr.takeNodes(Pred); ctuBifurcate(Call, D, Bldr, Pred, IState); ProgramStateRef NoIState = From fbbfeb16f6878a591fc5650d21eeb81605bbb0a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Fri, 12 Jun 2026 17:07:48 +0200 Subject: [PATCH 06/13] Extract Bldr.takeNodes(Pred) from BifurcateCall It was called on all paths within `BifurcateCall`, place it before each call to `BifurcateCall` instead. These will be moved further by the follow-up commits. --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 159e642f91c83..640b7ce52c6ef 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -1245,6 +1245,7 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Explore with and without inlining the call. if (Options.getIPAMode() == IPAK_DynamicDispatchBifurcate) { + Bldr.takeNodes(Pred); BifurcateCall(RD.getDispatchRegion(), Call, D, Bldr, Pred); return; } @@ -1285,14 +1286,11 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, State->get<DynamicDispatchBifurcationMap>(BifurReg); if (BState) { // If we are on "inline path", keep inlining if possible. - if (*BState == DynamicDispatchModeInlined) { - Bldr.takeNodes(Pred); + if (*BState == DynamicDispatchModeInlined) ctuBifurcate(Call, D, Bldr, Pred, State); - } // If inline failed, or we are on the path where we assume we // don't have enough info about the receiver to inline, conjure the // return value and invalidate the regions. - Bldr.takeNodes(Pred); Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); return; } @@ -1302,13 +1300,11 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, ProgramStateRef IState = State->set<DynamicDispatchBifurcationMap>(BifurReg, DynamicDispatchModeInlined); - Bldr.takeNodes(Pred); ctuBifurcate(Call, D, Bldr, Pred, IState); ProgramStateRef NoIState = State->set<DynamicDispatchBifurcationMap>(BifurReg, DynamicDispatchModeConservative); - Bldr.takeNodes(Pred); Bldr.addNodes(conservativeEvalCall(Call, Pred, NoIState)); NumOfDynamicDispatchPathSplits++; From f91e2a5a074f97cefe6e67f3cf08a0311d923052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Fri, 12 Jun 2026 17:12:35 +0200 Subject: [PATCH 07/13] Extract Bldr.takeNodes(Pred) from performTrivialCopy `defaultEvalCall` called `Bldr.takeNodes(Pred)` on every execution path, so this commit moves it out of the function. In most call sites the node builder was constructed with the 3-argument constructor that inserts all nodes from the first argument into the second argument (which is the `Frontier`, the out-parameter of the node builer) -- which was followed by a for loop that took out each of these nodes from the `Frontier` with the `takeNodes` calls in `defaultEvalCall`. This commit "cancels out" these back-and-forth set manipulations. In `VisitObjCMessage` the analogous loop is more complex, and will be refactored in a separate commit; this change just temporarily places `Bldr.takeNodes(Pred)` in front of the `defaultEvalCall` invocation. --- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp | 2 +- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 6 +++--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 5 ----- clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp | 1 + 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp index 80c03899d1e39..1a57a40c02441 100644 --- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -809,7 +809,7 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst, // If none of the checkers evaluated the call, ask ExprEngine to handle it. if (!evaluatorChecker) { - NodeBuilder B(Pred, Dst, Eng.getBuilderContext()); + NodeBuilder B(Dst, Eng.getBuilderContext()); Eng.defaultEvalCall(B, Pred, *UpdatedCall, CallOpts); } } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index e1d5869d95e21..a202d92b8b74c 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -861,7 +861,7 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, *Call, *this); ExplodedNodeSet DstInvalidated; - NodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx); + NodeBuilder Bldr(DstInvalidated, *currBldrCtx); for (ExplodedNode *N : DstPreCall) defaultEvalCall(Bldr, N, *Call, CallOpts); @@ -886,7 +886,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, *Call, *this); ExplodedNodeSet DstPostCall; - NodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx); + NodeBuilder CallBldr(DstPostCall, *currBldrCtx); for (ExplodedNode *I : DstPreCall) { // Operator new calls (CXXNewExpr) are intentionally not eval-called, // because it does not make sense to eval-call user-provided functions. @@ -1094,7 +1094,7 @@ void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE, ExplodedNodeSet DstPostCall; if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) { - NodeBuilder Bldr(DstPreCall, DstPostCall, *currBldrCtx); + NodeBuilder Bldr(DstPostCall, *currBldrCtx); for (ExplodedNode *I : DstPreCall) { // Intentionally either inline or conservative eval-call the operator // delete, but avoid triggering an eval-call event for checkers. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 640b7ce52c6ef..47f67f17b17ba 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -1224,7 +1224,6 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Special-case trivial assignment operators. if (isTrivialObjectAssignment(Call)) { - Bldr.takeNodes(Pred); performTrivialCopy(Bldr, Pred, Call); return; } @@ -1245,19 +1244,16 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Explore with and without inlining the call. if (Options.getIPAMode() == IPAK_DynamicDispatchBifurcate) { - Bldr.takeNodes(Pred); BifurcateCall(RD.getDispatchRegion(), Call, D, Bldr, Pred); return; } // Don't inline if we're not in any dynamic dispatch mode. if (Options.getIPAMode() != IPAK_DynamicDispatch) { - Bldr.takeNodes(Pred); Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); return; } } - Bldr.takeNodes(Pred); ctuBifurcate(Call, D, Bldr, Pred, State); return; } @@ -1269,7 +1265,6 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, State, dyn_cast_or_null<CXXConstructExpr>(E), Call.getStackFrame()); // Also handle the return value and invalidate the regions. - Bldr.takeNodes(Pred); Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index 15f0275691e92..15b970fc673d5 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -285,6 +285,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, } } + Bldr.takeNodes(Pred); defaultEvalCall(Bldr, Pred, *UpdatedMsg); } From 3ef458c83ec290bea6e73a62ebf94206ae032136 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 10 Jun 2026 14:51:02 +0200 Subject: [PATCH 08/13] Simplify an old-style loop --- clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index 15b970fc673d5..e20e9e3ec5c92 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -258,9 +258,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, ExplodedNodeSet dstEval; NodeBuilder Bldr(dstGenericPrevisit, dstEval, *currBldrCtx); - for (ExplodedNodeSet::iterator DI = dstGenericPrevisit.begin(), - DE = dstGenericPrevisit.end(); DI != DE; ++DI) { - ExplodedNode *Pred = *DI; + for (ExplodedNode *Pred : dstGenericPrevisit) { ProgramStateRef State = Pred->getState(); CallEventRef<ObjCMethodCall> UpdatedMsg = Msg.cloneWithState(State); From 1d2ccbaf5c018dbeac52842baf74948936a5bc77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 15 Jun 2026 13:52:07 +0200 Subject: [PATCH 09/13] Simplify last builder in VisitObjCMessage The constructor of this node builder inserted the exploded node set `dstGenericPrevisit` into `dstEval` but then each of these nodes was removed from the set by either one of the two `generateSink` calls or by the call `Bldr.takeNodes(Pred)`. As this temporary presence of the nodes in the set doesn't affect anything, this commit: - removes this first argument of the `NodeBuilder` constructor; - removes `Bldr.takeNodes(Pred)`; - replaces the `generateSink` calls with `makePostStmtNode` calls where `MarkAsSink` is true. Note that a `generatNode` call has three effects: removing the old node from the `Frontier`, making the new node and inserting the new node into the `Frontier` -- but the third one is irrelevant for sinks because `ExplodedNodeSet`s ignore the insertion of sink nodes. Therefore "cancelling out" the removal of the old node leaves just the node-making call (`makePostStmtNode` in this case). --- clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index e20e9e3ec5c92..d5c38890800db 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -256,7 +256,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, // Proceed with evaluate the message expression. ExplodedNodeSet dstEval; - NodeBuilder Bldr(dstGenericPrevisit, dstEval, *currBldrCtx); + NodeBuilder Bldr(dstEval, *currBldrCtx); for (ExplodedNode *Pred : dstGenericPrevisit) { ProgramStateRef State = Pred->getState(); @@ -268,7 +268,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, if (ObjCNoRet.isImplicitNoReturn(ME)) { // If we raise an exception, for now treat it as a sink. // Eventually we will want to handle exceptions properly. - Bldr.generateSink(ME, Pred, State); + Engine.makePostStmtNode(ME, State, Pred, /*MarkAsSink=*/true); continue; } } @@ -278,12 +278,11 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, if (ObjCNoRet.isImplicitNoReturn(ME)) { // If we raise an exception, for now treat it as a sink. // Eventually we will want to handle exceptions properly. - Bldr.generateSink(ME, Pred, Pred->getState()); + Engine.makePostStmtNode(ME, State, Pred, /*MarkAsSink=*/true); continue; } } - Bldr.takeNodes(Pred); defaultEvalCall(Bldr, Pred, *UpdatedMsg); } From d3cfb6d1ab41490ba330fb044a7ad873e3eabfde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 15 Jun 2026 16:34:57 +0200 Subject: [PATCH 10/13] Remove NodeBuilders from the defaultEvalCall function family Instead of passing around the NodeBuilder, use its destination (a/k/a `Frontier`) set directly. Previous changes reduced the usage of these node builders to a few `addNodes` calls, which directly correspond to `ExplodedNodeSet::insert`. --- .../Core/PathSensitive/ExprEngine.h | 11 +++---- .../StaticAnalyzer/Core/CheckerManager.cpp | 6 ++-- .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 17 ++++------ .../Core/ExprEngineCallAndReturn.cpp | 32 +++++++++---------- .../StaticAnalyzer/Core/ExprEngineObjC.cpp | 3 +- 5 files changed, 30 insertions(+), 39 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index ae29f35a950b3..5f19784158ed2 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -772,7 +772,7 @@ class ExprEngine { const CallEvent &Call); /// Default implementation of call evaluation. - void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred, + void defaultEvalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, const CallEvent &Call, const EvalCallOptions &CallOpts = {}); @@ -910,7 +910,7 @@ class ExprEngine { void inlineCall(WorkList *WList, const CallEvent &Call, const Decl *D, ExplodedNode *Pred, ProgramStateRef State); - void ctuBifurcate(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, + void ctuBifurcate(const CallEvent &Call, const Decl *D, ExplodedNodeSet &Dst, ExplodedNode *Pred, ProgramStateRef State); /// Returns true if the CTU analysis is running its second phase. @@ -923,15 +923,14 @@ class ExprEngine { /// Either inline or process the call conservatively (or both), based /// on DynamicDispatchBifurcation data. - void BifurcateCall(const MemRegion *BifurReg, - const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, - ExplodedNode *Pred); + void BifurcateCall(const MemRegion *BifurReg, const CallEvent &Call, + const Decl *D, ExplodedNodeSet &Dst, ExplodedNode *Pred); bool replayWithoutInlining(ExplodedNode *P, const StackFrame *CalleeSF); /// Models a trivial copy or move constructor or trivial assignment operator /// call with a simple bind. - void performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, + void performTrivialCopy(ExplodedNodeSet &Dst, ExplodedNode *Pred, const CallEvent &Call); /// If the value of the given expression \p InitWithAdjustments is a NonLoc, diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp index 1a57a40c02441..4db6b6ecaa9f7 100644 --- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -808,10 +808,8 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst, } // If none of the checkers evaluated the call, ask ExprEngine to handle it. - if (!evaluatorChecker) { - NodeBuilder B(Dst, Eng.getBuilderContext()); - Eng.defaultEvalCall(B, Pred, *UpdatedCall, CallOpts); - } + if (!evaluatorChecker) + Eng.defaultEvalCall(Dst, Pred, *UpdatedCall, CallOpts); } } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index a202d92b8b74c..b4873a2280ff0 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -44,7 +44,7 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME, // FIXME: This is the sort of code that should eventually live in a Core // checker rather than as a special case in ExprEngine. -void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, +void ExprEngine::performTrivialCopy(ExplodedNodeSet &Dst, ExplodedNode *Pred, const CallEvent &Call) { SVal ThisVal; bool AlwaysReturnsLValue; @@ -95,14 +95,13 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, DstEval.insert(Pred); } - PostStmt PS(CallExpr, SF); for (ExplodedNode *N : DstEval) { ProgramStateRef State = N->getState(); if (AlwaysReturnsLValue) State = State->BindExpr(CallExpr, SF, ThisVal); else State = bindReturnValue(Call, SF, State); - Bldr.generateNode(PS, State, N); + Dst.insert(Engine.makePostStmtNode(CallExpr, State, Pred)); } } @@ -728,10 +727,9 @@ void ExprEngine::handleConstructor(const Expr *E, if (CE && CE->getConstructor()->isTrivial() && CE->getConstructor()->isCopyOrMoveConstructor() && !CallOpts.IsArrayCtorOrDtor) { - NodeBuilder Bldr(DstEvaluated, *currBldrCtx); // FIXME: Handle other kinds of trivial constructors as well. for (ExplodedNode *N : DstPreCall) - performTrivialCopy(Bldr, N, *Call); + performTrivialCopy(DstEvaluated, N, *Call); } else { for (ExplodedNode *N : DstPreCall) @@ -861,9 +859,8 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, *Call, *this); ExplodedNodeSet DstInvalidated; - NodeBuilder Bldr(DstInvalidated, *currBldrCtx); for (ExplodedNode *N : DstPreCall) - defaultEvalCall(Bldr, N, *Call, CallOpts); + defaultEvalCall(DstInvalidated, N, *Call, CallOpts); getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, *Call, *this); @@ -886,7 +883,6 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, *Call, *this); ExplodedNodeSet DstPostCall; - NodeBuilder CallBldr(DstPostCall, *currBldrCtx); for (ExplodedNode *I : DstPreCall) { // Operator new calls (CXXNewExpr) are intentionally not eval-called, // because it does not make sense to eval-call user-provided functions. @@ -896,7 +892,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, // is what we want anyway. // So the best is to not allow eval-calling CXXNewExprs from checkers. // Checkers can provide their pre/post-call callbacks if needed. - defaultEvalCall(CallBldr, I, *Call); + defaultEvalCall(DstPostCall, I, *Call); } // If the call is inlined, DstPostCall will be empty and we bail out now. @@ -1094,13 +1090,12 @@ void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE, ExplodedNodeSet DstPostCall; if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) { - NodeBuilder Bldr(DstPostCall, *currBldrCtx); for (ExplodedNode *I : DstPreCall) { // Intentionally either inline or conservative eval-call the operator // delete, but avoid triggering an eval-call event for checkers. // As detailed at handling CXXNewExprs, in short, because it does not // really make sense to eval-call user-provided functions. - defaultEvalCall(Bldr, I, *Call); + defaultEvalCall(DstPostCall, I, *Call); } } else { DstPostCall = std::move(DstPreCall); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 47f67f17b17ba..099db8d081dfe 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -493,7 +493,7 @@ REGISTER_MAP_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap, REGISTER_TRAIT_WITH_PROGRAMSTATE(CTUDispatchBifurcation, bool) void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, - NodeBuilder &Bldr, ExplodedNode *Pred, + ExplodedNodeSet &Dst, ExplodedNode *Pred, ProgramStateRef State) { ProgramStateRef ConservativeEvalState = nullptr; if (Call.isForeign() && !isSecondPhaseCTU()) { @@ -511,9 +511,9 @@ void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State); // Conservatively evaluate in the first phase. ConservativeEvalState = State->set<CTUDispatchBifurcation>(true); - Bldr.addNodes(conservativeEvalCall(Call, Pred, ConservativeEvalState)); + Dst.insert(conservativeEvalCall(Call, Pred, ConservativeEvalState)); } else { - Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); + Dst.insert(conservativeEvalCall(Call, Pred, State)); } return; } @@ -1216,7 +1216,7 @@ static bool isTrivialObjectAssignment(const CallEvent &Call) { return MD->isTrivial(); } -void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, +void ExprEngine::defaultEvalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, const CallEvent &Call, const EvalCallOptions &CallOpts) { // Make sure we have the most recent state attached to the call. @@ -1224,7 +1224,7 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Special-case trivial assignment operators. if (isTrivialObjectAssignment(Call)) { - performTrivialCopy(Bldr, Pred, Call); + performTrivialCopy(Dst, Pred, Call); return; } @@ -1244,17 +1244,17 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, // Explore with and without inlining the call. if (Options.getIPAMode() == IPAK_DynamicDispatchBifurcate) { - BifurcateCall(RD.getDispatchRegion(), Call, D, Bldr, Pred); + BifurcateCall(RD.getDispatchRegion(), Call, D, Dst, Pred); return; } // Don't inline if we're not in any dynamic dispatch mode. if (Options.getIPAMode() != IPAK_DynamicDispatch) { - Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); + Dst.insert(conservativeEvalCall(Call, Pred, State)); return; } } - ctuBifurcate(Call, D, Bldr, Pred, State); + ctuBifurcate(Call, D, Dst, Pred, State); return; } } @@ -1265,12 +1265,12 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, State, dyn_cast_or_null<CXXConstructExpr>(E), Call.getStackFrame()); // Also handle the return value and invalidate the regions. - Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); + Dst.insert(conservativeEvalCall(Call, Pred, State)); } -void ExprEngine::BifurcateCall(const MemRegion *BifurReg, - const CallEvent &Call, const Decl *D, - NodeBuilder &Bldr, ExplodedNode *Pred) { +void ExprEngine::BifurcateCall(const MemRegion *BifurReg, const CallEvent &Call, + const Decl *D, ExplodedNodeSet &Dst, + ExplodedNode *Pred) { assert(BifurReg); BifurReg = BifurReg->StripCasts(); @@ -1282,11 +1282,11 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, if (BState) { // If we are on "inline path", keep inlining if possible. if (*BState == DynamicDispatchModeInlined) - ctuBifurcate(Call, D, Bldr, Pred, State); + ctuBifurcate(Call, D, Dst, Pred, State); // If inline failed, or we are on the path where we assume we // don't have enough info about the receiver to inline, conjure the // return value and invalidate the regions. - Bldr.addNodes(conservativeEvalCall(Call, Pred, State)); + Dst.insert(conservativeEvalCall(Call, Pred, State)); return; } @@ -1295,12 +1295,12 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, ProgramStateRef IState = State->set<DynamicDispatchBifurcationMap>(BifurReg, DynamicDispatchModeInlined); - ctuBifurcate(Call, D, Bldr, Pred, IState); + ctuBifurcate(Call, D, Dst, Pred, IState); ProgramStateRef NoIState = State->set<DynamicDispatchBifurcationMap>(BifurReg, DynamicDispatchModeConservative); - Bldr.addNodes(conservativeEvalCall(Call, Pred, NoIState)); + Dst.insert(conservativeEvalCall(Call, Pred, NoIState)); NumOfDynamicDispatchPathSplits++; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index d5c38890800db..d4be050427b8b 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -256,7 +256,6 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, // Proceed with evaluate the message expression. ExplodedNodeSet dstEval; - NodeBuilder Bldr(dstEval, *currBldrCtx); for (ExplodedNode *Pred : dstGenericPrevisit) { ProgramStateRef State = Pred->getState(); @@ -283,7 +282,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, } } - defaultEvalCall(Bldr, Pred, *UpdatedMsg); + defaultEvalCall(dstEval, Pred, *UpdatedMsg); } // If there were constructors called for object-type arguments, clean them up. From d3ee7461bd1671bddb99586215ea333aa9aa47f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 15 Jun 2026 16:46:16 +0200 Subject: [PATCH 11/13] [side] Simplify isImplicitNoReturn logic in VisitObjCMessage Unify two `if` blocks that had identical bodies and related conditions. --- .../StaticAnalyzer/Core/ExprEngineObjC.cpp | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index d4be050427b8b..26b8ca3e1f2bc 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -261,25 +261,13 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, ProgramStateRef State = Pred->getState(); CallEventRef<ObjCMethodCall> UpdatedMsg = Msg.cloneWithState(State); - if (UpdatedMsg->isInstanceMessage()) { - SVal recVal = UpdatedMsg->getReceiverSVal(); - if (!recVal.isUndef()) { - if (ObjCNoRet.isImplicitNoReturn(ME)) { - // If we raise an exception, for now treat it as a sink. - // Eventually we will want to handle exceptions properly. - Engine.makePostStmtNode(ME, State, Pred, /*MarkAsSink=*/true); - continue; - } - } - } else { - // Check for special class methods that are known to not return - // and that we should treat as a sink. - if (ObjCNoRet.isImplicitNoReturn(ME)) { - // If we raise an exception, for now treat it as a sink. - // Eventually we will want to handle exceptions properly. - Engine.makePostStmtNode(ME, State, Pred, /*MarkAsSink=*/true); - continue; - } + if (ObjCNoRet.isImplicitNoReturn(ME) && + !(UpdatedMsg->isInstanceMessage() && + UpdatedMsg->getReceiverSVal().isUndef())) { + // If we raise an exception, for now treat it as a sink. + // Eventually we will want to handle exceptions properly. + Engine.makePostStmtNode(ME, State, Pred, /*MarkAsSink=*/true); + continue; } defaultEvalCall(dstEval, Pred, *UpdatedMsg); From ad27ff870f3978ffa98915130bf9ce5c3f0e8bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 15 Jun 2026 16:52:51 +0200 Subject: [PATCH 12/13] [side] Simplify ctuBifurcate Avoid duplicating the call of `conservativeEvalCall`. --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 099db8d081dfe..33eba2e69e58e 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -495,7 +495,6 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(CTUDispatchBifurcation, bool) void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, ExplodedNodeSet &Dst, ExplodedNode *Pred, ProgramStateRef State) { - ProgramStateRef ConservativeEvalState = nullptr; if (Call.isForeign() && !isSecondPhaseCTU()) { const auto IK = AMgr.options.getCTUPhase1Inlining(); const bool DoInline = IK == CTUPhase1InliningKind::All || @@ -510,11 +509,9 @@ void ExprEngine::ctuBifurcate(const CallEvent &Call, const Decl *D, // Enqueue it to be analyzed in the second (ctu) phase. inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State); // Conservatively evaluate in the first phase. - ConservativeEvalState = State->set<CTUDispatchBifurcation>(true); - Dst.insert(conservativeEvalCall(Call, Pred, ConservativeEvalState)); - } else { - Dst.insert(conservativeEvalCall(Call, Pred, State)); + State = State->set<CTUDispatchBifurcation>(true); } + Dst.insert(conservativeEvalCall(Call, Pred, State)); return; } inlineCall(Engine.getWorkList(), Call, D, Pred, State); From 2fbe1642d585fd876419782c13a855e6cc34f631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 15 Jun 2026 16:57:39 +0200 Subject: [PATCH 13/13] [side] Rename BifurcateCall to dynDispatchBifurcate As I'm already changing the parametrization of this method, I take this opportunity to update its name to clarify its role and conform to the naming conventions. This is an old method and when it was originally introduced, 'BifuracetCall' was a reasonable name for it, but since then we also have 'ctuBifurcate', so it is better to use a more specific name for this method. --- .../clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 5 +++-- .../lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 5f19784158ed2..ce9b20185444e 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -923,8 +923,9 @@ class ExprEngine { /// Either inline or process the call conservatively (or both), based /// on DynamicDispatchBifurcation data. - void BifurcateCall(const MemRegion *BifurReg, const CallEvent &Call, - const Decl *D, ExplodedNodeSet &Dst, ExplodedNode *Pred); + void dynDispatchBifurcate(const MemRegion *BifurReg, const CallEvent &Call, + const Decl *D, ExplodedNodeSet &Dst, + ExplodedNode *Pred); bool replayWithoutInlining(ExplodedNode *P, const StackFrame *CalleeSF); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 33eba2e69e58e..d5733d40ff60d 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -1241,7 +1241,7 @@ void ExprEngine::defaultEvalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, // Explore with and without inlining the call. if (Options.getIPAMode() == IPAK_DynamicDispatchBifurcate) { - BifurcateCall(RD.getDispatchRegion(), Call, D, Dst, Pred); + dynDispatchBifurcate(RD.getDispatchRegion(), Call, D, Dst, Pred); return; } @@ -1265,9 +1265,10 @@ void ExprEngine::defaultEvalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, Dst.insert(conservativeEvalCall(Call, Pred, State)); } -void ExprEngine::BifurcateCall(const MemRegion *BifurReg, const CallEvent &Call, - const Decl *D, ExplodedNodeSet &Dst, - ExplodedNode *Pred) { +void ExprEngine::dynDispatchBifurcate(const MemRegion *BifurReg, + const CallEvent &Call, const Decl *D, + ExplodedNodeSet &Dst, + ExplodedNode *Pred) { assert(BifurReg); BifurReg = BifurReg->StripCasts(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
