Author: Donát Nagy Date: 2026-06-26T14:32:39+02:00 New Revision: 236f63474d40850a33fd4bcc6d8d63a31cebb8f1
URL: https://github.com/llvm/llvm-project/commit/236f63474d40850a33fd4bcc6d8d63a31cebb8f1 DIFF: https://github.com/llvm/llvm-project/commit/236f63474d40850a33fd4bcc6d8d63a31cebb8f1.diff LOG: [NFC][analyzer] Remove NodeBuilders around defaultEvalCall (#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`). Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h clang/lib/StaticAnalyzer/Core/CheckerManager.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index bb2de45cec92a..ce9b20185444e 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 = {}); @@ -908,9 +908,9 @@ 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, + 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. @@ -918,20 +918,20 @@ 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. - void BifurcateCall(const MemRegion *BifurReg, - const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, - ExplodedNode *Pred); + void dynDispatchBifurcate(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 80c03899d1e39..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(Pred, 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 291533d0c3289..e048fc210e608 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -42,9 +42,7 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME, Bldr.generateNode(ME, Pred, state); } -// 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; @@ -67,8 +65,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, const StackFrame *SF = Pred->getStackFrame(); const Expr *CallExpr = Call.getOriginExpr(); - ExplodedNodeSet Dst; - Bldr.takeNodes(Pred); + ExplodedNodeSet DstEval; assert(ThisRD); @@ -87,23 +84,22 @@ 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); else State = bindReturnValue(Call, SF, State); - Bldr.generateNode(PS, State, N); + Dst.insert(Engine.makePostStmtNode(CallExpr, State, N)); } } @@ -729,10 +725,9 @@ void ExprEngine::handleConstructor(const Expr *E, if (CE && CE->getConstructor()->isTrivial() && CE->getConstructor()->isCopyOrMoveConstructor() && !CallOpts.IsArrayCtorOrDtor) { - NodeBuilder Bldr(DstPreCall, 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) @@ -862,9 +857,8 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, *Call, *this); ExplodedNodeSet DstInvalidated; - NodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx); for (ExplodedNode *N : DstPreCall) - defaultEvalCall(Bldr, N, *Call, CallOpts); + defaultEvalCall(DstInvalidated, N, *Call, CallOpts); getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, *Call, *this); @@ -887,7 +881,6 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, *Call, *this); ExplodedNodeSet DstPostCall; - NodeBuilder CallBldr(DstPreCall, 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. @@ -897,7 +890,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. @@ -1095,13 +1088,12 @@ void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE, ExplodedNodeSet DstPostCall; if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) { - NodeBuilder Bldr(DstPreCall, 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 97c655c103b0a..b66b5c5e358e4 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -493,36 +493,33 @@ 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()) { const auto IK = AMgr.options.getCTUPhase1Inlining(); const bool DoInline = IK == CTUPhase1InliningKind::All || (IK == CTUPhase1InliningKind::Small && isSmall(AMgr.getAnalysisDeclContext(D))); if (DoInline) { - inlineCall(Engine.getWorkList(), Call, D, Bldr, Pred, State); + 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); + inlineCall(Engine.getCTUWorkList(), Call, D, Pred, State); // Conservatively evaluate in the first phase. - ConservativeEvalState = State->set<CTUDispatchBifurcation>(true); - conservativeEvalCall(Call, Bldr, Pred, ConservativeEvalState); - } else { - conservativeEvalCall(Call, Bldr, Pred, State); + State = State->set<CTUDispatchBifurcation>(true); } + Dst.insert(conservativeEvalCall(Call, Pred, State)); return; } - inlineCall(Engine.getWorkList(), Call, D, Bldr, Pred, State); + 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 +553,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); @@ -837,14 +830,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 @@ -1219,7 +1213,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. @@ -1227,7 +1221,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; } @@ -1247,17 +1241,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); + dynDispatchBifurcate(RD.getDispatchRegion(), Call, D, Dst, Pred); return; } // Don't inline if we're not in any dynamic dispatch mode. if (Options.getIPAMode() != IPAK_DynamicDispatch) { - conservativeEvalCall(Call, Bldr, Pred, State); + Dst.insert(conservativeEvalCall(Call, Pred, State)); return; } } - ctuBifurcate(Call, D, Bldr, Pred, State); + ctuBifurcate(Call, D, Dst, Pred, State); return; } } @@ -1268,12 +1262,13 @@ 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); + Dst.insert(conservativeEvalCall(Call, Pred, State)); } -void ExprEngine::BifurcateCall(const MemRegion *BifurReg, - const CallEvent &Call, const Decl *D, - NodeBuilder &Bldr, ExplodedNode *Pred) { +void ExprEngine::dynDispatchBifurcate(const MemRegion *BifurReg, + const CallEvent &Call, const Decl *D, + ExplodedNodeSet &Dst, + ExplodedNode *Pred) { assert(BifurReg); BifurReg = BifurReg->StripCasts(); @@ -1285,11 +1280,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. - conservativeEvalCall(Call, Bldr, Pred, State); + Dst.insert(conservativeEvalCall(Call, Pred, State)); return; } @@ -1298,12 +1293,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); - conservativeEvalCall(Call, Bldr, 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 15f0275691e92..26b8ca3e1f2bc 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -256,36 +256,21 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, // Proceed with evaluate the message expression. 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); - 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. - Bldr.generateSink(ME, Pred, State); - 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. - Bldr.generateSink(ME, Pred, Pred->getState()); - 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(Bldr, Pred, *UpdatedMsg); + defaultEvalCall(dstEval, Pred, *UpdatedMsg); } // If there were constructors called for object-type arguments, clean them up. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
