This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325209: [analyzer] Decide on inlining destructors via
EvalCallOptions. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42991?vs=133299&id=134364#toc
Repository:
rL LLVM
https://reviews.llvm.org/D42991
Files:
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
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -84,16 +84,8 @@
}
-/// 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 @@
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 @@
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 @@
// 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 @@
QualType Ty = Field->getType();
FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
- CallOpts.IsArrayConstructorOrDestructor);
+ CallOpts.IsArrayCtorOrDtor);
return FieldVal.getAsRegion();
}
@@ -187,7 +179,7 @@
}
// 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 @@
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 @@
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 @@
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();
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -653,7 +653,7 @@
// 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 @@
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 @@
(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 @@
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.
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -802,8 +802,15 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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,
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -55,6 +55,20 @@
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 @@
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 @@
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 @@
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
Index: cfe/trunk/test/Analysis/new.cpp
===================================================================
--- cfe/trunk/test/Analysis/new.cpp
+++ cfe/trunk/test/Analysis/new.cpp
@@ -311,7 +311,8 @@
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
