[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.
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=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 , bool ) {
+SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue,
+ QualType , bool ) {
SValBuilder = State->getStateManager().getSValBuilder();
ASTContext = 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(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(LCtx->getParentMap().getParent(CE))) {
MemRegionManager = 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 ) {
+ExplodedNodeSet ,
+const EvalCallOptions ) {
const LocationContext *LCtx =
[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.
NoQ updated this revision to Diff 133299.
NoQ added a comment.
Remove the check in `shouldInlineCall()` as well. It's quite covered by the
`IsConstructorWithImproperlyModeledTargetRegion` check.
https://reviews.llvm.org/D42991
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new.cpp
Index: test/Analysis/new.cpp
===
--- test/Analysis/new.cpp
+++ 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
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -650,7 +650,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.
@@ -670,14 +670,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;
}
@@ -693,9 +693,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:
@@ -844,14 +849,6 @@
AnalysisDeclContextManager = 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(Call)) {
-if ((*currBldrCtx->getBlock())[currStmtIdx].getAs())
- 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: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ 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 , bool ) {
+SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue,
+ QualType , bool ) {
SValBuilder = State->getStateManager().getSValBuilder();
ASTContext = SVB.getContext();
@@ -128,7 +120,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,
[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.
NoQ updated this revision to Diff 133106.
NoQ added a comment.
Remove a couple of accidental blank lines.
https://reviews.llvm.org/D42991
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new.cpp
Index: test/Analysis/new.cpp
===
--- test/Analysis/new.cpp
+++ 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
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -650,7 +650,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.
@@ -670,14 +670,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;
}
@@ -693,9 +693,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:
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ 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 , bool ) {
+SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue,
+ QualType , bool ) {
SValBuilder = State->getStateManager().getSValBuilder();
ASTContext = SVB.getContext();
@@ -128,7 +120,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());
}
@@ -140,10 +132,10 @@
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(TriggerStmt)) {
-CallOpts.IsConstructorIntoTemporary = true;
+CallOpts.IsTemporaryCtorOrDtor = true;
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
// TODO: Consider other directly initialized elements.
@@ -166,7 +158,7 @@
