NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
`getLocationForConstructedObject()` looks at the construction context and
figures out what region should represent the object.
`markStatementsCorrespondingToConstructedObject()` looks at the construction
context and figures out what statements will need to retrieve that region
directly later.
These functions are coupled and code is duplicated between them. They should be
merged. The resulting function is large, so it'd probably later need to be
split in a different manner (i.e. by construction context kinds). It'll also
soon become recursive as we add better support for copy elision at return
sites. I really hope we don't end up coding any sort of
ConstructionContextVisitor.
No functional change intended here; this is a refactoring pass.
Repository:
rC Clang
https://reviews.llvm.org/D47304
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -587,18 +587,20 @@
unsigned Count = currBldrCtx->blockCount();
if (auto RTC = getCurrentCFGElement().getAs<CFGCXXRecordTypedCall>()) {
// Conjure a temporary if the function returns an object by value.
- MemRegionManager &MRMgr = svalBuilder.getRegionManager();
- const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx);
- State = markStatementsCorrespondingToConstructedObject(
- State, RTC->getConstructionContext(), LCtx, loc::MemRegionVal(TR));
-
+ SVal Target;
+ assert(RTC->getStmt() == Call.getOriginExpr());
+ EvalCallOptions CallOpts; // FIXME: We won't really need those.
+ std::tie(State, Target) =
+ prepareForObjectConstruction(Call.getOriginExpr(), State, LCtx,
+ RTC->getConstructionContext(), CallOpts);
+ assert(Target.getAsRegion());
// Invalidate the region so that it didn't look uninitialized. Don't notify
// the checkers.
- State = State->invalidateRegions(TR, E, Count, LCtx,
+ State = State->invalidateRegions(Target.getAsRegion(), E, Count, LCtx,
/* CausedByPointerEscape=*/false, nullptr,
&Call, nullptr);
- R = State->getSVal(TR, E->getType());
+ R = State->getSVal(Target.castAs<Loc>(), E->getType());
} else {
// Conjure a symbol if the return value is unknown.
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -110,13 +110,9 @@
return LValue;
}
-
-SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
- ExplodedNode *Pred,
- const ConstructionContext *CC,
- EvalCallOptions &CallOpts) {
- const LocationContext *LCtx = Pred->getLocationContext();
- ProgramStateRef State = Pred->getState();
+std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
+ const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+ const ConstructionContext *CC, EvalCallOptions &CallOpts) {
MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
// See if we're constructing an existing region by looking at the
@@ -129,8 +125,9 @@
const auto *Var = cast<VarDecl>(DS->getSingleDecl());
SVal LValue = State->getLValue(Var, LCtx);
QualType Ty = Var->getType();
- return makeZeroElementRegion(State, LValue, Ty,
- CallOpts.IsArrayCtorOrDtor);
+ return std::make_pair(
+ State,
+ makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor));
}
case ConstructionContext::SimpleConstructorInitializerKind: {
const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
@@ -154,7 +151,7 @@
QualType Ty = Field->getType();
FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
CallOpts.IsArrayCtorOrDtor);
- return FieldVal;
+ return std::make_pair(State, FieldVal);
}
case ConstructionContext::NewAllocatedObjectKind: {
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
@@ -167,65 +164,97 @@
// TODO: In fact, we need to call the constructor for every
// allocated element, not just the first one!
CallOpts.IsArrayCtorOrDtor = true;
- return loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
- MR, NE->getType()->getPointeeType()));
+ return std::make_pair(
+ State, loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
+ MR, NE->getType()->getPointeeType())));
}
- return V;
+ return std::make_pair(State, V);
}
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
}
break;
}
- case ConstructionContext::TemporaryObjectKind: {
- const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC);
- if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) {
- if (const ValueDecl *VD = MTE->getExtendingDecl()) {
- assert(MTE->getStorageDuration() != SD_FullExpression);
- if (!VD->getType()->isReferenceType()) {
- // We're lifetime-extended by a surrounding aggregate.
- // Automatic destructors aren't quite working in this case
- // on the CFG side. We should warn the caller about that.
- // FIXME: Is there a better way to retrieve this information from
- // the MaterializeTemporaryExpr?
- CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
- }
- }
- }
- // TODO: Support temporaries lifetime-extended via static references.
- // They'd need a getCXXStaticTempObjectRegion().
- CallOpts.IsTemporaryCtorOrDtor = true;
- return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
- }
case ConstructionContext::SimpleReturnedValueKind: {
// The temporary is to be managed by the parent stack frame.
// So build it in the parent stack frame if we're not in the
// top frame of the analysis.
- // TODO: What exactly happens when we are? Does the temporary object live
- // long enough in the region store in this case? Would checkers think
- // that this object immediately goes out of scope?
- const LocationContext *TempLCtx = LCtx;
const StackFrameContext *SFC = LCtx->getCurrentStackFrame();
if (const LocationContext *CallerLCtx = SFC->getParent()) {
auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
.getAs<CFGCXXRecordTypedCall>();
if (!RTC) {
// We were unable to find the correct construction context for the
// call in the parent stack frame. This is equivalent to not being
// able to find construction context at all.
- CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+ break;
} else if (!isa<TemporaryObjectConstructionContext>(
RTC->getConstructionContext())) {
// FIXME: The return value is constructed directly into a
// non-temporary due to C++17 mandatory copy elision. This is not
// implemented yet.
assert(getContext().getLangOpts().CPlusPlus17);
- CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+ break;
+ }
+ CC = RTC->getConstructionContext();
+ LCtx = CallerLCtx;
+ } else {
+ // We are on the top frame of the analysis.
+ // TODO: What exactly happens when we are? Does the temporary object
+ // live long enough in the region store in this case? Would checkers
+ // think that this object immediately goes out of scope?
+ CallOpts.IsTemporaryCtorOrDtor = true;
+ SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
+ return std::make_pair(State, V);
+ }
+
+ // Continue as if we have a temporary with a different location context.
+ // FALLTHROUGH.
+ }
+ case ConstructionContext::TemporaryObjectKind: {
+ const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
+ const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
+ const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
+
+ if (!BTE) {
+ // FIXME: Lifetime extension for temporaries without destructors
+ // is not implemented yet.
+ MTE = nullptr;
+ }
+
+ if (MTE) {
+ if (const ValueDecl *VD = MTE->getExtendingDecl()) {
+ assert(MTE->getStorageDuration() != SD_FullExpression);
+ if (!VD->getType()->isReferenceType()) {
+ // We're lifetime-extended by a surrounding aggregate.
+ // Automatic destructors aren't quite working in this case
+ // on the CFG side. We should warn the caller about that.
+ // FIXME: Is there a better way to retrieve this information from
+ // the MaterializeTemporaryExpr?
+ CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
+ }
}
- TempLCtx = CallerLCtx;
}
+
+ if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
+ // If the temporary is lifetime-extended, don't save the BTE,
+ // because we don't need a temporary destructor, but an automatic
+ // destructor.
+ BTE = nullptr;
+ }
+
+ // FIXME: Support temporaries lifetime-extended via static references.
+ // They'd need a getCXXStaticTempObjectRegion().
+ SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
+
+ if (BTE)
+ State = addObjectUnderConstruction(State, BTE, LCtx, V);
+
+ if (MTE)
+ State = addObjectUnderConstruction(State, MTE, LCtx, V);
+
CallOpts.IsTemporaryCtorOrDtor = true;
- return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, TempLCtx));
+ return std::make_pair(State, V);
}
case ConstructionContext::CXX17ElidedCopyVariableKind:
case ConstructionContext::CXX17ElidedCopyReturnedValueKind:
@@ -237,7 +266,8 @@
// If we couldn't find an existing region to construct into, assume we're
// constructing a temporary. Notify the caller of our failure.
CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
- return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
+ return std::make_pair(
+ State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
}
const CXXConstructExpr *
@@ -288,7 +318,8 @@
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
- Target = getLocationForConstructedObject(CE, Pred, CC, CallOpts);
+ std::tie(State, Target) =
+ prepareForObjectConstruction(CE, State, LCtx, CC, CallOpts);
break;
}
case CXXConstructExpr::CK_VirtualBase:
@@ -349,6 +380,18 @@
}
}
+ if (State != Pred->getState()) {
+ static SimpleProgramPointTag T("ExprEngine",
+ "Prepare for object construction");
+ ExplodedNodeSet DstPrepare;
+ StmtNodeBuilder BldrPrepare(Pred, DstPrepare, *currBldrCtx);
+ BldrPrepare.generateNode(CE, Pred, State, &T, ProgramPoint::PreStmtKind);
+ assert(DstPrepare.size() <= 1);
+ if (DstPrepare.size() == 0)
+ return;
+ Pred = *BldrPrepare.begin();
+ }
+
CallEventManager &CEMgr = getStateManager().getCallEventManager();
CallEventRef<CXXConstructorCall> Call =
CEMgr.getCXXConstructorCall(CE, Target.getAsRegion(), State, LCtx);
@@ -380,9 +423,6 @@
State = State->bindDefaultZero(Target, LCtx);
}
- State = markStatementsCorrespondingToConstructedObject(State, CC, LCtx,
- Target);
-
Bldr.generateNode(CE, *I, State, /*tag=*/nullptr,
ProgramPoint::PreStmtKind);
}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -411,81 +411,6 @@
return true;
}
-ProgramStateRef ExprEngine::markStatementsCorrespondingToConstructedObject(
- ProgramStateRef State, const ConstructionContext *CC,
- const LocationContext *LC, SVal V) {
- if (CC) {
- // If the temporary is being returned from the function, it will be
- // destroyed or lifetime-extended in the caller stack frame.
- if (isa<ReturnedValueConstructionContext>(CC)) {
- const StackFrameContext *SFC = LC->getCurrentStackFrame();
- assert(SFC);
- LC = SFC->getParent();
- if (!LC) {
- // We are on the top frame. We won't ever need any info
- // for this temporary, so don't set anything.
- return State;
- }
- const CFGElement &CallElem =
- (*SFC->getCallSiteBlock())[SFC->getIndex()];
- auto RTCElem = CallElem.getAs<CFGCXXRecordTypedCall>();
- if (!RTCElem) {
- // We have a parent stack frame, but no construction context for the
- // return value. Give up until we provide the construction context
- // at the call site.
- return State;
- }
- // We use the ReturnedValueConstructionContext as an indication that we
- // need to look for the actual construction context on the parent stack
- // frame. This purpose has been fulfilled, so now we replace CC with the
- // actual construction context.
- CC = RTCElem->getConstructionContext();
- if (!isa<TemporaryObjectConstructionContext>(CC)) {
- // TODO: We are not returning an object into a temporary. There must
- // be copy elision happening at the call site. We still need to
- // explicitly support the situation when the return value is put
- // into another return statement, i.e.
- // ReturnedValueConstructionContexts are chained through multiple
- // stack frames before finally settling in a temporary.
- // We don't seem to need to explicitly support construction into
- // a variable after a return.
- return State;
- }
- // Proceed to deal with the temporary we've found on the parent
- // stack frame.
- }
- // In case of temporary object construction, extract data necessary for
- // destruction and lifetime extension.
- if (const auto *TCC = dyn_cast<TemporaryObjectConstructionContext>(CC)) {
- if (AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) {
- const CXXBindTemporaryExpr *BTE =
- TCC->getCXXBindTemporaryExpr();
- const MaterializeTemporaryExpr *MTE =
- TCC->getMaterializedTemporaryExpr();
- if (!BTE) {
- // FIXME: Lifetime extension for temporaries without destructors
- // is not implemented yet.
- MTE = nullptr;
- }
- if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
- // If the temporary is lifetime-extended, don't save the BTE,
- // because we don't need a temporary destructor, but an automatic
- // destructor.
- BTE = nullptr;
- }
-
- if (BTE)
- State = addObjectUnderConstruction(State, BTE, LC, V);
-
- if (MTE)
- State = addObjectUnderConstruction(State, MTE, LC, V);
- }
- }
- }
-
- return State;
-}
-
//===----------------------------------------------------------------------===//
// Top-level transfer function logic (Dispatcher).
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -744,14 +744,15 @@
/// constructing into an existing region.
const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
- /// For a given constructor, look forward in the current CFG block to
- /// determine the region into which an object will be constructed by \p CE.
- /// When the lookahead fails, a temporary region is returned, and the
+ /// Update the program state with all the path-sensitive information
+ /// that's necessary to perform construction of an object with a given
+ /// syntactic construction context. If the construction context is unavailable
+ /// or unusable for any reason, a dummy temporary region is returned, and the
/// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts.
- SVal getLocationForConstructedObject(const CXXConstructExpr *CE,
- ExplodedNode *Pred,
- const ConstructionContext *CC,
- EvalCallOptions &CallOpts);
+ /// Returns the updated program state and the new object's this-region.
+ std::pair<ProgramStateRef, SVal> prepareForObjectConstruction(
+ const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+ const ConstructionContext *CC, EvalCallOptions &CallOpts);
/// Store the location of a C++ object corresponding to a statement
/// until the statement is actually encountered. For example, if a DeclStmt
@@ -782,14 +783,6 @@
static bool areAllObjectsFullyConstructed(ProgramStateRef State,
const LocationContext *FromLC,
const LocationContext *ToLC);
-
- /// Adds an initialized temporary and/or a materialization, whichever is
- /// necessary, by looking at the whole construction context. Handles
- /// function return values, which need the construction context of the parent
- /// stack frame, automagically.
- ProgramStateRef markStatementsCorrespondingToConstructedObject(
- ProgramStateRef State, const ConstructionContext *CC,
- const LocationContext *LC, SVal V);
};
/// Traits for storing the call processing policy inside GDM.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
