https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/186186
From 22f9ce383d8468bf28c8e29579123db534c6e00a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 10 Mar 2026 11:39:16 +0100 Subject: [PATCH 1/3] [NFC][analyzer] Simplify computeObjectUnderConstruction part 1 Previously the method `ExprEngine::computeObjectUnderConstruction` took a `NodeBuilderContext` parameter which was only used to call its `blockCount()` method; this commit replaces this with directly taking `NumVisitedCaller` (= number of times the caller was visited, the `blockCount`) as an `unsigned` value. This moves toward the elimination of the type `NodeBuilderContext`. Note that I'm using the name `NumVisitedCaller` instead of e.g. `CallerBlockCount` because I think this is more self-documenting. This commit leaves behind some non-idiomatic constructs which will be cleaned up in an immediate follow-up. --- .../StaticAnalyzer/Core/PathSensitive/ExprEngine.h | 6 +++--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 2 +- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 11 ++++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 2023a7a5b1ac8..360a260d4d803 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -796,7 +796,7 @@ class ExprEngine { /// A multi-dimensional array is also a continuous memory location in a /// row major order, so for arr[0][0] Idx is 0 and for arr[3][3] Idx is 8. SVal computeObjectUnderConstruction(const Expr *E, ProgramStateRef State, - const NodeBuilderContext *BldrCtx, + unsigned NumVisitedCaller, const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts, @@ -818,8 +818,8 @@ class ExprEngine { const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx = 0) { - SVal V = computeObjectUnderConstruction(E, State, BldrCtx, LCtx, CC, - CallOpts, Idx); + SVal V = computeObjectUnderConstruction(E, State, BldrCtx->blockCount(), + LCtx, CC, CallOpts, Idx); State = updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts); return std::make_pair(State, V); diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index f6c3a8e3266da..cff3116caf6e6 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -563,7 +563,7 @@ std::optional<SVal> CallEvent::getReturnValueUnderConstruction() const { EvalCallOptions CallOpts; ExprEngine &Engine = getState()->getStateManager().getOwningEngine(); SVal RetVal = Engine.computeObjectUnderConstruction( - getOriginExpr(), getState(), &Engine.getBuilderContext(), + getOriginExpr(), getState(), Engine.getBuilderContext().blockCount(), getLocationContext(), CC, CallOpts); return RetVal; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 0866dda766667..c91b5ddd0d46e 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -129,7 +129,7 @@ SVal ExprEngine::makeElementRegion(ProgramStateRef State, SVal LValue, // it's materialization happens in context of the caller. // We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context. SVal ExprEngine::computeObjectUnderConstruction( - const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx, + const Expr *E, ProgramStateRef State, unsigned NumVisitedCaller, const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx) { @@ -232,8 +232,9 @@ SVal ExprEngine::computeObjectUnderConstruction( NodeBuilderContext CallerBldrCtx(getCoreEngine(), SFC->getCallSiteBlock(), CallerLCtx); + unsigned NumVisitedCaller = CallerBldrCtx.blockCount(); return computeObjectUnderConstruction( - cast<Expr>(SFC->getCallSite()), State, &CallerBldrCtx, CallerLCtx, + cast<Expr>(SFC->getCallSite()), State, NumVisitedCaller, CallerLCtx, RTC->getConstructionContext(), CallOpts); } else { // We are on the top frame of the analysis. We do not know where is the @@ -273,7 +274,7 @@ SVal ExprEngine::computeObjectUnderConstruction( EvalCallOptions PreElideCallOpts = CallOpts; SVal V = computeObjectUnderConstruction( - TCC->getConstructorAfterElision(), State, BldrCtx, LCtx, + TCC->getConstructorAfterElision(), State, NumVisitedCaller, LCtx, TCC->getConstructionContextAfterElision(), CallOpts); // FIXME: This definition of "copy elision has not failed" is unreliable. @@ -346,7 +347,7 @@ SVal ExprEngine::computeObjectUnderConstruction( CallEventManager &CEMgr = getStateManager().getCallEventManager(); auto getArgLoc = [&](CallEventRef<> Caller) -> std::optional<SVal> { const LocationContext *FutureSFC = - Caller->getCalleeStackFrame(BldrCtx->blockCount()); + Caller->getCalleeStackFrame(NumVisitedCaller); // Return early if we are unable to reliably foresee // the future stack frame. if (!FutureSFC) @@ -365,7 +366,7 @@ SVal ExprEngine::computeObjectUnderConstruction( // because this-argument is implemented as a normal argument in // operator call expressions but not in operator declarations. const TypedValueRegion *TVR = Caller->getParameterLocation( - *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount()); + *Caller->getAdjustedParameterIndex(Idx), NumVisitedCaller); if (!TVR) return std::nullopt; From a80473647ab61fb3d4a1a60c91441652c72df4d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 10 Mar 2026 14:22:43 +0100 Subject: [PATCH 2/3] [NFC][analyzer] Simplify computeObjectUnderConstruction part 2 Clean-up non-idiomatic code left behind by the previous change. Also leave a FIXME comment on some logic that seems to be fishy (I _suspect_ that sometimes it uses the wrong LocationContext and Block, but I'm not certain.) --- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 6 +++++- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 7 ++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index cff3116caf6e6..86ffd92cdf6f5 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -562,8 +562,12 @@ std::optional<SVal> CallEvent::getReturnValueUnderConstruction() const { EvalCallOptions CallOpts; ExprEngine &Engine = getState()->getStateManager().getOwningEngine(); + // FIXME: This code assumes that the _current_ location context and block is + // the location and block where this `CallExpr` is called. For a more stable + // solution `Engine.getNumVisitedCurrent()` should be replaced with a call to + // `Engine.getNumVisited(<CallerLCtx>, <CallerBlock>)`. SVal RetVal = Engine.computeObjectUnderConstruction( - getOriginExpr(), getState(), Engine.getBuilderContext().blockCount(), + getOriginExpr(), getState(), Engine.getNumVisitedCurrent(), getLocationContext(), CC, CallOpts); return RetVal; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index c91b5ddd0d46e..cf22b62225f2f 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -127,7 +127,6 @@ SVal ExprEngine::makeElementRegion(ProgramStateRef State, SVal LValue, // In case when the prvalue is returned from the function (kind is one of // SimpleReturnedValueKind, CXX17ElidedCopyReturnedValueKind), then // it's materialization happens in context of the caller. -// We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context. SVal ExprEngine::computeObjectUnderConstruction( const Expr *E, ProgramStateRef State, unsigned NumVisitedCaller, const LocationContext *LCtx, const ConstructionContext *CC, @@ -230,11 +229,9 @@ SVal ExprEngine::computeObjectUnderConstruction( assert(!isa<BlockInvocationContext>(CallerLCtx)); } - NodeBuilderContext CallerBldrCtx(getCoreEngine(), - SFC->getCallSiteBlock(), CallerLCtx); - unsigned NumVisitedCaller = CallerBldrCtx.blockCount(); + unsigned NVCaller = getNumVisited(CallerLCtx, SFC->getCallSiteBlock()); return computeObjectUnderConstruction( - cast<Expr>(SFC->getCallSite()), State, NumVisitedCaller, CallerLCtx, + cast<Expr>(SFC->getCallSite()), State, NVCaller, CallerLCtx, RTC->getConstructionContext(), CallOpts); } else { // We are on the top frame of the analysis. We do not know where is the From a17505ee08717c354fb327731b15ec43466237f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Mon, 16 Mar 2026 17:41:59 +0100 Subject: [PATCH 3/3] [NFCI][analyzer] Fix logic in CallEvent::getReturnValueUnderConstruction The `CallEvent` has data members that store the `LocationContext` and the `CFGElementRef` (i.e. `CFGBlock` + index of statement within that block); but the method `getReturnValueUnderConstruction` ignored these and used the currently analyzed `LocationContext` and `CFGBlock` instead of them. This was logically incorrect and would have caused problems if the `CallEvent` was used later when the "currently analyzed" things are different. However, the lit tests do pass even if I assert that the currently analyzed `LocationContext` and `CFGBlock` is the same as the ones saved in the `CallEvent`, so I'm pretty sure that there was no actual problem caused by this bad logic and this commit won't cause functional changes. --- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 86ffd92cdf6f5..cd52083a278ae 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -562,13 +562,11 @@ std::optional<SVal> CallEvent::getReturnValueUnderConstruction() const { EvalCallOptions CallOpts; ExprEngine &Engine = getState()->getStateManager().getOwningEngine(); - // FIXME: This code assumes that the _current_ location context and block is - // the location and block where this `CallExpr` is called. For a more stable - // solution `Engine.getNumVisitedCurrent()` should be replaced with a call to - // `Engine.getNumVisited(<CallerLCtx>, <CallerBlock>)`. + unsigned NumVisitedCall = Engine.getNumVisited( + getLocationContext(), getCFGElementRef().getParent()); SVal RetVal = Engine.computeObjectUnderConstruction( - getOriginExpr(), getState(), Engine.getNumVisitedCurrent(), - getLocationContext(), CC, CallOpts); + getOriginExpr(), getState(), NumVisitedCall, getLocationContext(), CC, + CallOpts); return RetVal; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
