r325209 - [analyzer] Decide on inlining destructors via EvalCallOptions.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 14 18:51:58 PST 2018
Author: dergachev
Date: Wed Feb 14 18:51:58 2018
New Revision: 325209
URL: http://llvm.org/viewvc/llvm-project?rev=325209&view=rev
Log:
[analyzer] Decide on inlining destructors via EvalCallOptions.
EvalCallOptions were introduced in r324018 for allowing various parts of
ExprEngine to notify the inlining mechanism, while preparing for evaluating a
function call, of possible difficulties with evaluating the call that they
foresee. Then mayInlineCall() would still be a single place for making the
decision.
Use that mechanism for destructors as well - pass the necessary flags from the
CFG-element-specific destructor handlers.
Part of this patch accidentally leaked into r324018, which led into a change in
tests; this change is reverted now, because even though the change looked
correct, the underlying behavior wasn't. Both of these commits were not intended
to introduce any function changes otherwise.
Differential Revision: https://reviews.llvm.org/D42991
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/test/Analysis/new.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=325209&r1=325208&r2=325209&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Wed Feb 14 18:51:58 2018
@@ -55,6 +55,20 @@ public:
Inline_Minimal = 0x1
};
+ /// Hints for figuring out of a call should be inlined during evalCall().
+ struct EvalCallOptions {
+ /// This call is a constructor or a destructor for which we do not currently
+ /// compute the this-region correctly.
+ bool IsCtorOrDtorWithImproperlyModeledTargetRegion = false;
+ /// This call is a constructor or a destructor for a single element within
+ /// an array, a part of array construction or destruction.
+ bool IsArrayCtorOrDtor = false;
+ /// This call is a constructor or a destructor of a temporary value.
+ bool IsTemporaryCtorOrDtor = false;
+
+ EvalCallOptions() {}
+ };
+
private:
AnalysisManager &AMgr;
@@ -449,7 +463,8 @@ public:
void VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest,
const Stmt *S, bool IsBaseDtor,
- ExplodedNode *Pred, ExplodedNodeSet &Dst);
+ ExplodedNode *Pred, ExplodedNodeSet &Dst,
+ const EvalCallOptions &Options);
void VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
ExplodedNode *Pred,
@@ -568,14 +583,6 @@ public:
const LocationContext *LCtx,
ProgramStateRef State);
- struct EvalCallOptions {
- bool IsConstructorWithImproperlyModeledTargetRegion = false;
- bool IsArrayConstructorOrDestructor = false;
- bool IsConstructorIntoTemporary = false;
-
- EvalCallOptions() {}
- };
-
/// Evaluate a call, running pre- and post-call checks and allowing checkers
/// to be responsible for handling the evaluation of the call itself.
void evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
@@ -659,6 +666,17 @@ private:
const Expr *InitWithAdjustments,
const Expr *Result = nullptr);
+ /// Returns a region representing the first element of a (possibly
+ /// multi-dimensional) array, for the purposes of element construction or
+ /// destruction.
+ ///
+ /// On return, \p Ty will be set to the base type of the array.
+ ///
+ /// If the type is not an array type at all, the original value is returned.
+ /// Otherwise the "IsArray" flag is set.
+ static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
+ QualType &Ty, bool &IsArray);
+
/// For a DeclStmt or CXXInitCtorInitializer, walk backward in the current CFG
/// block to find the constructor expression that directly constructed into
/// the storage for this statement. Returns null if the constructor for this
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=325209&r1=325208&r2=325209&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Feb 14 18:51:58 2018
@@ -802,8 +802,15 @@ void ExprEngine::ProcessAutomaticObjDtor
varType = cast<TypedValueRegion>(Region)->getValueType();
}
+ // FIXME: We need to run the same destructor on every element of the array.
+ // This workaround will just run the first destructor (which will still
+ // invalidate the entire array).
+ EvalCallOptions CallOpts;
+ Region = makeZeroElementRegion(state, loc::MemRegionVal(Region), varType,
+ CallOpts.IsArrayCtorOrDtor).getAsRegion();
+
VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false,
- Pred, Dst);
+ Pred, Dst, CallOpts);
}
void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
@@ -813,12 +820,12 @@ void ExprEngine::ProcessDeleteDtor(const
const LocationContext *LCtx = Pred->getLocationContext();
const CXXDeleteExpr *DE = Dtor.getDeleteExpr();
const Stmt *Arg = DE->getArgument();
+ QualType DTy = DE->getDestroyedType();
SVal ArgVal = State->getSVal(Arg, LCtx);
// If the argument to delete is known to be a null value,
// don't run destructor.
if (State->isNull(ArgVal).isConstrainedTrue()) {
- QualType DTy = DE->getDestroyedType();
QualType BTy = getContext().getBaseElementType(DTy);
const CXXRecordDecl *RD = BTy->getAsCXXRecordDecl();
const CXXDestructorDecl *Dtor = RD->getDestructor();
@@ -829,10 +836,19 @@ void ExprEngine::ProcessDeleteDtor(const
return;
}
- VisitCXXDestructor(DE->getDestroyedType(),
- ArgVal.getAsRegion(),
- DE, /*IsBase=*/ false,
- Pred, Dst);
+ EvalCallOptions CallOpts;
+ const MemRegion *ArgR = ArgVal.getAsRegion();
+ if (DE->isArrayForm()) {
+ // FIXME: We need to run the same destructor on every element of the array.
+ // This workaround will just run the first destructor (which will still
+ // invalidate the entire array).
+ CallOpts.IsArrayCtorOrDtor = true;
+ if (ArgR)
+ ArgR = getStoreManager().GetElementZeroRegion(cast<SubRegion>(ArgR), DTy);
+ }
+
+ VisitCXXDestructor(DE->getDestroyedType(), ArgR, DE, /*IsBase=*/false,
+ Pred, Dst, CallOpts);
}
void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
@@ -851,12 +867,13 @@ void ExprEngine::ProcessBaseDtor(const C
Base->isVirtual());
VisitCXXDestructor(BaseTy, BaseVal.castAs<loc::MemRegionVal>().getRegion(),
- CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst);
+ CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst, {});
}
void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
ExplodedNode *Pred, ExplodedNodeSet &Dst) {
const FieldDecl *Member = D.getFieldDecl();
+ QualType T = Member->getType();
ProgramStateRef State = Pred->getState();
const LocationContext *LCtx = Pred->getLocationContext();
@@ -866,9 +883,15 @@ void ExprEngine::ProcessMemberDtor(const
SVal FieldVal =
State->getLValue(Member, State->getSVal(ThisVal).castAs<Loc>());
- VisitCXXDestructor(Member->getType(),
- FieldVal.castAs<loc::MemRegionVal>().getRegion(),
- CurDtor->getBody(), /*IsBase=*/false, Pred, Dst);
+ // FIXME: We need to run the same destructor on every element of the array.
+ // This workaround will just run the first destructor (which will still
+ // invalidate the entire array).
+ EvalCallOptions CallOpts;
+ FieldVal = makeZeroElementRegion(State, FieldVal, T,
+ CallOpts.IsArrayCtorOrDtor);
+
+ VisitCXXDestructor(T, FieldVal.castAs<loc::MemRegionVal>().getRegion(),
+ CurDtor->getBody(), /*IsBase=*/false, Pred, Dst, CallOpts);
}
void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
@@ -892,10 +915,14 @@ void ExprEngine::ProcessTemporaryDtor(co
assert(CleanDtorState.size() <= 1);
ExplodedNode *CleanPred =
CleanDtorState.empty() ? Pred : *CleanDtorState.begin();
+
// FIXME: Inlining of temporary destructors is not supported yet anyway, so
// we just put a NULL region for now. This will need to be changed later.
+ EvalCallOptions CallOpts;
+ CallOpts.IsTemporaryCtorOrDtor = true;
+ CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
VisitCXXDestructor(varType, nullptr, D.getBindTemporaryExpr(),
- /*IsBase=*/false, CleanPred, Dst);
+ /*IsBase=*/false, CleanPred, Dst, CallOpts);
}
void ExprEngine::processCleanupTemporaryBranch(const CXXBindTemporaryExpr *BTE,
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=325209&r1=325208&r2=325209&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Feb 14 18:51:58 2018
@@ -84,16 +84,8 @@ void ExprEngine::performTrivialCopy(Node
}
-/// Returns a region representing the first element of a (possibly
-/// multi-dimensional) array, for the purposes of element construction or
-/// destruction.
-///
-/// On return, \p Ty will be set to the base type of the array.
-///
-/// If the type is not an array type at all, the original value is returned.
-/// Otherwise the "IsArray" flag is set.
-static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
- QualType &Ty, bool &IsArray) {
+SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue,
+ QualType &Ty, bool &IsArray) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
ASTContext &Ctx = SVB.getContext();
@@ -131,7 +123,7 @@ ExprEngine::getRegionForConstructedObjec
if (CNE->isArray()) {
// TODO: In fact, we need to call the constructor for every
// allocated element, not just the first one!
- CallOpts.IsArrayConstructorOrDestructor = true;
+ CallOpts.IsArrayCtorOrDtor = true;
return getStoreManager().GetElementZeroRegion(
MR, CNE->getType()->getPointeeType());
}
@@ -143,7 +135,7 @@ ExprEngine::getRegionForConstructedObjec
SVal LValue = State->getLValue(Var, LCtx);
QualType Ty = Var->getType();
LValue = makeZeroElementRegion(State, LValue, Ty,
- CallOpts.IsArrayConstructorOrDestructor);
+ CallOpts.IsArrayCtorOrDtor);
return LValue.getAsRegion();
} else if (auto *RS = dyn_cast<ReturnStmt>(TriggerStmt)) {
// TODO: We should construct into a CXXBindTemporaryExpr or a
@@ -154,7 +146,7 @@ ExprEngine::getRegionForConstructedObjec
// function, we should be able to take the call-site CFG element,
// and it should contain (but right now it wouldn't) some sort of
// construction context that'd give us the right temporary expression.
- CallOpts.IsConstructorIntoTemporary = true;
+ CallOpts.IsTemporaryCtorOrDtor = true;
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
// TODO: Consider other directly initialized elements.
@@ -177,7 +169,7 @@ ExprEngine::getRegionForConstructedObjec
QualType Ty = Field->getType();
FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
- CallOpts.IsArrayConstructorOrDestructor);
+ CallOpts.IsArrayCtorOrDtor);
return FieldVal.getAsRegion();
}
@@ -187,7 +179,7 @@ ExprEngine::getRegionForConstructedObjec
}
// If we couldn't find an existing region to construct into, assume we're
// constructing a temporary. Notify the caller of our failure.
- CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true;
+ CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
@@ -273,7 +265,7 @@ void ExprEngine::VisitCXXConstructExpr(c
if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) {
MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
Target = MRMgr.getCXXTempObjectRegion(CE, LCtx);
- CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true;
+ CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
break;
}
// FALLTHROUGH
@@ -343,7 +335,7 @@ void ExprEngine::VisitCXXConstructExpr(c
if (CE->getConstructor()->isTrivial() &&
CE->getConstructor()->isCopyOrMoveConstructor() &&
- !CallOpts.IsArrayConstructorOrDestructor) {
+ !CallOpts.IsArrayCtorOrDtor) {
// FIXME: Handle other kinds of trivial constructors as well.
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
@@ -400,23 +392,11 @@ void ExprEngine::VisitCXXDestructor(Qual
const Stmt *S,
bool IsBaseDtor,
ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {
+ ExplodedNodeSet &Dst,
+ const EvalCallOptions &CallOpts) {
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
- // FIXME: We need to run the same destructor on every element of the array.
- // This workaround will just run the first destructor (which will still
- // invalidate the entire array).
- SVal DestVal = UnknownVal();
- if (Dest)
- DestVal = loc::MemRegionVal(Dest);
-
- EvalCallOptions CallOpts;
- DestVal = makeZeroElementRegion(State, DestVal, ObjectType,
- CallOpts.IsArrayConstructorOrDestructor);
-
- Dest = DestVal.getAsRegion();
-
const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
assert(RecordDecl && "Only CXXRecordDecls should have destructors");
const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor();
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=325209&r1=325208&r2=325209&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Feb 14 18:51:58 2018
@@ -653,7 +653,7 @@ ExprEngine::mayInlineCallKind(const Call
// initializers for array fields in default move/copy constructors.
// We still allow construction into ElementRegion targets when they don't
// represent array elements.
- if (CallOpts.IsArrayConstructorOrDestructor)
+ if (CallOpts.IsArrayCtorOrDtor)
return CIP_DisallowedOnce;
// Inlining constructors requires including initializers in the CFG.
@@ -673,14 +673,14 @@ ExprEngine::mayInlineCallKind(const Call
if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) {
// If we don't handle temporary destructors, we shouldn't inline
// their constructors.
- if (CallOpts.IsConstructorIntoTemporary &&
+ if (CallOpts.IsTemporaryCtorOrDtor &&
!Opts.includeTemporaryDtorsInCFG())
return CIP_DisallowedOnce;
- // If we did not construct the correct this-region, it would be pointless
+ // If we did not find the correct this-region, it would be pointless
// to inline the constructor. Instead we will simply invalidate
// the fake temporary target.
- if (CallOpts.IsConstructorWithImproperlyModeledTargetRegion)
+ if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion)
return CIP_DisallowedOnce;
}
@@ -696,9 +696,14 @@ ExprEngine::mayInlineCallKind(const Call
(void)ADC;
// FIXME: We don't handle constructors or destructors for arrays properly.
- if (CallOpts.IsArrayConstructorOrDestructor)
+ if (CallOpts.IsArrayCtorOrDtor)
return CIP_DisallowedOnce;
+ // If we did not find the correct this-region, it would be pointless
+ // to inline the destructor. Instead we will simply invalidate
+ // the fake temporary target.
+ if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion)
+ return CIP_DisallowedOnce;
break;
}
case CE_CXXAllocator:
@@ -847,14 +852,6 @@ bool ExprEngine::shouldInlineCall(const
AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager();
AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);
- // Temporary object destructor processing is currently broken, so we never
- // inline them.
- // FIXME: Remove this once temp destructors are working.
- if (isa<CXXDestructorCall>(Call)) {
- if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())
- return false;
- }
-
// The auto-synthesized bodies are essential to inline as they are
// usually small and commonly used. Note: we should do this check early on to
// ensure we always inline these calls.
Modified: cfe/trunk/test/Analysis/new.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new.cpp?rev=325209&r1=325208&r2=325209&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/new.cpp (original)
+++ cfe/trunk/test/Analysis/new.cpp Wed Feb 14 18:51:58 2018
@@ -311,7 +311,8 @@ void testArrayNull() {
void testArrayDestr() {
NoReturnDtor *p = new NoReturnDtor[2];
delete[] p; // Calls the base destructor which aborts, checked below
- clang_analyzer_eval(true); // no-warning
+ //TODO: clang_analyzer_eval should not be called
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
}
// Invalidate Region even in case of default destructor
More information about the cfe-commits
mailing list