diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h index 220aee759a935c..a595d517cd2768 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h @@ -214,8 +214,14 @@ struct NodeBuilderContext { const CFGBlock *Block; const LocationContext *LC; + NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, + const LocationContext *L) + : Eng(E), Block(B), LC(L) { + assert(B); + } + NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, ExplodedNode *N) - : Eng(E), Block(B), LC(N->getLocationContext()) { assert(B); } + : NodeBuilderContext(E, B, N->getLocationContext()) {} /// Return the CFGBlock associated with this builder. const CFGBlock *getBlock() const { return Block; } diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index a905f9097750d0..848e43d15fbff4 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -732,6 +732,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[2][2] Idx is 8. SVal computeObjectUnderConstruction(const Expr *E, ProgramStateRef State, + const NodeBuilderContext *BldrCtx, const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts, @@ -748,13 +749,13 @@ class ExprEngine { /// A convenient wrapper around computeObjectUnderConstruction /// and updateObjectsUnderConstruction. - std::pair - handleConstructionContext(const Expr *E, ProgramStateRef State, - const LocationContext *LCtx, - const ConstructionContext *CC, - EvalCallOptions &CallOpts, unsigned Idx = 0) { + std::pair handleConstructionContext( + const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx, + const LocationContext *LCtx, const ConstructionContext *CC, + EvalCallOptions &CallOpts, unsigned Idx = 0) { - SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx); + SVal V = computeObjectUnderConstruction(E, State, BldrCtx, 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 57591960a14017..a8b49adfb4c9a6 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -485,9 +485,9 @@ CallEvent::getReturnValueUnderConstruction() const { EvalCallOptions CallOpts; ExprEngine &Engine = getState()->getStateManager().getOwningEngine(); - SVal RetVal = - Engine.computeObjectUnderConstruction(getOriginExpr(), getState(), - getLocationContext(), CC, CallOpts); + SVal RetVal = Engine.computeObjectUnderConstruction( + getOriginExpr(), getState(), &Engine.getBuilderContext(), + getLocationContext(), CC, CallOpts); return RetVal; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index ae878ecbcc34a7..476afc598ac6cd 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -111,9 +111,15 @@ SVal ExprEngine::makeElementRegion(ProgramStateRef State, SVal LValue, return 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, const LocationContext *LCtx, - const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx) { + const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx, + const LocationContext *LCtx, const ConstructionContext *CC, + EvalCallOptions &CallOpts, unsigned Idx) { + SValBuilder &SVB = getSValBuilder(); MemRegionManager &MRMgr = SVB.getRegionManager(); ASTContext &ACtx = SVB.getContext(); @@ -210,8 +216,11 @@ SVal ExprEngine::computeObjectUnderConstruction( CallerLCtx = CallerLCtx->getParent(); assert(!isa(CallerLCtx)); } + + NodeBuilderContext CallerBldrCtx(getCoreEngine(), + SFC->getCallSiteBlock(), CallerLCtx); return computeObjectUnderConstruction( - cast(SFC->getCallSite()), State, CallerLCtx, + cast(SFC->getCallSite()), State, &CallerBldrCtx, CallerLCtx, RTC->getConstructionContext(), CallOpts); } else { // We are on the top frame of the analysis. We do not know where is the @@ -251,7 +260,7 @@ SVal ExprEngine::computeObjectUnderConstruction( EvalCallOptions PreElideCallOpts = CallOpts; SVal V = computeObjectUnderConstruction( - TCC->getConstructorAfterElision(), State, LCtx, + TCC->getConstructorAfterElision(), State, BldrCtx, LCtx, TCC->getConstructionContextAfterElision(), CallOpts); // FIXME: This definition of "copy elision has not failed" is unreliable. @@ -319,7 +328,7 @@ SVal ExprEngine::computeObjectUnderConstruction( CallEventManager &CEMgr = getStateManager().getCallEventManager(); auto getArgLoc = [&](CallEventRef<> Caller) -> Optional { const LocationContext *FutureSFC = - Caller->getCalleeStackFrame(currBldrCtx->blockCount()); + Caller->getCalleeStackFrame(BldrCtx->blockCount()); // Return early if we are unable to reliably foresee // the future stack frame. if (!FutureSFC) @@ -338,7 +347,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), currBldrCtx->blockCount()); + *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount()); if (!TVR) return None; @@ -643,8 +652,8 @@ void ExprEngine::handleConstructor(const Expr *E, } // The target region is found from construction context. - std::tie(State, Target) = - handleConstructionContext(CE, State, LCtx, CC, CallOpts, Idx); + std::tie(State, Target) = handleConstructionContext( + CE, State, currBldrCtx, LCtx, CC, CallOpts, Idx); break; } case CXXConstructExpr::CK_VirtualBase: { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index dbfded29c1ae4f..48b5db1eb4a521 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -774,9 +774,9 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call, SVal Target; assert(RTC->getStmt() == Call.getOriginExpr()); EvalCallOptions CallOpts; // FIXME: We won't really need those. - std::tie(State, Target) = - handleConstructionContext(Call.getOriginExpr(), State, LCtx, - RTC->getConstructionContext(), CallOpts); + std::tie(State, Target) = handleConstructionContext( + Call.getOriginExpr(), State, currBldrCtx, LCtx, + RTC->getConstructionContext(), CallOpts); const MemRegion *TargetR = Target.getAsRegion(); assert(TargetR); // Invalidate the region so that it didn't look uninitialized. If this is diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp index dee1a5fc86e7fa..991f325c05853d 100644 --- a/clang/test/Analysis/copy-elision.cpp +++ b/clang/test/Analysis/copy-elision.cpp @@ -20,6 +20,7 @@ #endif void clang_analyzer_eval(bool); +void clang_analyzer_dump(int); namespace variable_functional_cast_crash { @@ -418,3 +419,31 @@ void test_copy_elision() { } } // namespace address_vector_tests + +namespace arg_directly_from_return_in_loop { + +struct Result { + int value; +}; + +Result create() { + return Result{10}; +} + +int accessValue(Result r) { + return r.value; +} + +void test() { + for (int i = 0; i < 3; ++i) { + int v = accessValue(create()); + if (i == 0) { + clang_analyzer_dump(v); // expected-warning {{10 S32b}} + } else { + clang_analyzer_dump(v); // expected-warning {{10 S32b}} + // was {{reg_${{[0-9]+}} }} for C++11 + } + } +} + +} // namespace arg_directly_from_return_in_loop