Do not inline temporary destructors for temporaries that bind to parameters.
http://reviews.llvm.org/D4740
Files:
include/clang/Analysis/CFG.h
lib/Analysis/CFG.cpp
lib/Analysis/LiveVariables.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
lib/StaticAnalyzer/Core/ProgramState.cpp
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/temporaries.cpp
Index: include/clang/Analysis/CFG.h
===================================================================
--- include/clang/Analysis/CFG.h
+++ include/clang/Analysis/CFG.h
@@ -279,14 +279,17 @@
/// CFGTemporaryDtor - Represents C++ object destructor implicitly generated
/// at the end of full expression for temporary object.
class CFGTemporaryDtor : public CFGImplicitDtor {
+ static const bool* T;
public:
- CFGTemporaryDtor(CXXBindTemporaryExpr *expr)
- : CFGImplicitDtor(TemporaryDtor, expr, nullptr) {}
+ CFGTemporaryDtor(CXXBindTemporaryExpr *expr, bool BindsParameter)
+ : CFGImplicitDtor(TemporaryDtor, expr, BindsParameter ? T : nullptr) {}
const CXXBindTemporaryExpr *getBindTemporaryExpr() const {
return static_cast<const CXXBindTemporaryExpr *>(Data1.getPointer());
}
+ bool bindsParameter() const { return Data2.getPointer(); }
+
private:
friend class CFGElement;
CFGTemporaryDtor() {}
@@ -676,8 +679,9 @@
Elements.push_back(CFGMemberDtor(FD), C);
}
- void appendTemporaryDtor(CXXBindTemporaryExpr *E, BumpVectorContext &C) {
- Elements.push_back(CFGTemporaryDtor(E), C);
+ void appendTemporaryDtor(CXXBindTemporaryExpr *E, BumpVectorContext &C,
+ bool BindsParameter) {
+ Elements.push_back(CFGTemporaryDtor(E, BindsParameter), C);
}
void appendAutomaticObjDtor(VarDecl *VD, Stmt *S, BumpVectorContext &C) {
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -481,7 +481,8 @@
CFGBlock *VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E,
TempDtorContext &Context);
CFGBlock *VisitCXXBindTemporaryExprForTemporaryDtors(
- CXXBindTemporaryExpr *E, bool BindToTemporary, TempDtorContext &Context);
+ CXXBindTemporaryExpr *E, bool BindToTemporary, TempDtorContext &Context,
+ bool BindsParameter = false);
CFGBlock *VisitConditionalOperatorForTemporaryDtors(
AbstractConditionalOperator *E, bool BindToTemporary,
TempDtorContext &Context);
@@ -537,8 +538,9 @@
void appendMemberDtor(CFGBlock *B, FieldDecl *FD) {
B->appendMemberDtor(FD, cfg->getBumpVectorContext());
}
- void appendTemporaryDtor(CFGBlock *B, CXXBindTemporaryExpr *E) {
- B->appendTemporaryDtor(E, cfg->getBumpVectorContext());
+ void appendTemporaryDtor(CFGBlock *B, CXXBindTemporaryExpr *E,
+ bool BindsParameter) {
+ B->appendTemporaryDtor(E, cfg->getBumpVectorContext(), BindsParameter);
}
void appendAutomaticObjDtor(CFGBlock *B, VarDecl *VD, Stmt *S) {
B->appendAutomaticObjDtor(VD, S, cfg->getBumpVectorContext());
@@ -3611,6 +3613,22 @@
case Stmt::CXXDefaultInitExprClass:
E = cast<CXXDefaultInitExpr>(E)->getExpr();
goto tryAgain;
+
+ case Stmt::CallExprClass: {
+ CFGBlock *B = Block;
+ for (Expr *Arg : cast<CallExpr>(E)->arguments()) {
+ if (!Arg) continue;
+ if (CXXBindTemporaryExpr *BTE = dyn_cast<CXXBindTemporaryExpr>(Arg)) {
+ if (CFGBlock *R = VisitCXXBindTemporaryExprForTemporaryDtors(
+ BTE, false, Context, /*BindsParameter=*/true)) {
+ B = R;
+ }
+ } else if (CFGBlock *R = VisitForTemporaryDtors(Arg, false, Context)) {
+ B = R;
+ }
+ }
+ return B;
+ }
}
}
@@ -3670,7 +3688,8 @@
}
CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors(
- CXXBindTemporaryExpr *E, bool BindToTemporary, TempDtorContext &Context) {
+ CXXBindTemporaryExpr *E, bool BindToTemporary, TempDtorContext &Context,
+ bool BindsParameter) {
// First add destructors for temporaries in subexpression.
CFGBlock *B = VisitForTemporaryDtors(E->getSubExpr(), false, Context);
if (!BindToTemporary) {
@@ -3697,7 +3716,7 @@
if (Context.needsTempDtorBranch()) {
Context.setDecisionPoint(Succ, E);
}
- appendTemporaryDtor(Block, E);
+ appendTemporaryDtor(Block, E, BindsParameter);
B = Block;
}
@@ -3753,6 +3772,8 @@
} // end anonymous namespace
+const bool *CFGTemporaryDtor::T = new bool(true);
+
/// createBlock - Constructs and adds a new CFGBlock to the CFG. The block has
/// no successors or predecessors. If this is the first block created in the
/// CFG, it is automatically set to be the Entry and Exit of the CFG.
Index: lib/Analysis/LiveVariables.cpp
===================================================================
--- lib/Analysis/LiveVariables.cpp
+++ lib/Analysis/LiveVariables.cpp
@@ -409,6 +409,13 @@
val.liveDecls = DSetFact.add(val.liveDecls, Dtor->getVarDecl());
continue;
}
+ if (Optional<CFGTemporaryDtor> Dtor =
+ elem.getAs<CFGTemporaryDtor>()) {
+ // Temporary objects need to survive until the destructor is called.
+ val.liveStmts = SSetFact.add(val.liveStmts,
+ Dtor->getBindTemporaryExpr()->getSubExpr());
+ continue;
+ }
if (!elem.getAs<CFGStmt>())
continue;
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -959,7 +959,6 @@
CFGElement E = (*B)[CalleeCtx->getIndex()];
assert(E.getAs<CFGImplicitDtor>() &&
"All other CFG elements should have exprs");
- assert(!E.getAs<CFGTemporaryDtor>() && "We don't handle temporaries yet");
SValBuilder &SVB = State->getStateManager().getSValBuilder();
const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CalleeCtx->getDecl());
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -676,13 +676,23 @@
State = State->remove<InitializedTemporariesSet>(
std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame()));
StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State);
-
- QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
assert(CleanDtorState.size() == 1);
ExplodedNode *CleanPred = *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.
- VisitCXXDestructor(varType, nullptr, D.getBindTemporaryExpr(),
+
+ QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
+
+ const LocationContext *LCtx = CleanPred->getLocationContext();
+ SVal Val = CleanPred->getState()->getSVal(
+ D.getBindTemporaryExpr()->getSubExpr(), LCtx->getCurrentStackFrame());
+ const MemRegion *Region = nullptr;
+ // If the class does not have any members, there will not be a region
+ // for it bound in the environment.
+ if (Optional<nonloc::LazyCompoundVal> LCV =
+ Val.getAs<nonloc::LazyCompoundVal>()) {
+ Region = LCV->getRegion();
+ }
+
+ VisitCXXDestructor(varType, Region, D.getBindTemporaryExpr(),
/*IsBase=*/false, CleanPred, Dst);
}
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -637,12 +637,6 @@
if (!Opts.mayInlineCXXMemberFunction(CIMK_Destructors))
return CIP_DisallowedAlways;
- // FIXME: This is a hack. We don't handle temporary destructors
- // right now, so we shouldn't inline their constructors.
- if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
- if (!Target || !isa<DeclRegion>(Target))
- return CIP_DisallowedOnce;
-
break;
}
case CE_CXXDestructor: {
@@ -807,12 +801,15 @@
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;
+ if (Optional<CFGTemporaryDtor> Dtor =
+ (*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>()) {
+ // We must not inline temporary destructors for temporaries that are
+ // bound to parameters, as we currently don't support correctly
+ // invalidating our knowledge about them when they escape.
+ if (Dtor->bindsParameter())
+ return false;
+ }
}
// The auto-synthesized bodies are essential to inline as they are
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -506,16 +506,7 @@
}
bool ScanReachableSymbols::scan(nonloc::LazyCompoundVal val) {
- bool wasVisited = !visited.insert(val.getCVData()).second;
- if (wasVisited)
- return true;
-
- StoreManager &StoreMgr = state->getStateManager().getStoreManager();
- // FIXME: We don't really want to use getBaseRegion() here because pointer
- // arithmetic doesn't apply, but scanReachableSymbols only accepts base
- // regions right now.
- const MemRegion *R = val.getRegion()->getBaseRegion();
- return StoreMgr.scanReachableSymbols(val.getStore(), R, *this);
+ return scan(val.getRegion());
}
bool ScanReachableSymbols::scan(nonloc::CompoundVal val) {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1899,8 +1899,9 @@
QualType Ty = TR->getValueType();
if (Ty->isArrayType())
return bindArray(B, TR, V);
- if (Ty->isStructureOrClassType())
+ if (Ty->isStructureOrClassType()) {
return bindStruct(B, TR, V);
+ }
if (Ty->isVectorType())
return bindVector(B, TR, V);
if (Ty->isUnionType())
@@ -2110,8 +2111,9 @@
// Handle lazy compound values and symbolic values.
if (Optional<nonloc::LazyCompoundVal> LCV =
V.getAs<nonloc::LazyCompoundVal>()) {
- if (Optional<RegionBindingsRef> NewB = tryBindSmallStruct(B, R, RD, *LCV))
+ if (Optional<RegionBindingsRef> NewB = tryBindSmallStruct(B, R, RD, *LCV)) {
return *NewB;
+ }
return bindAggregate(B, R, V);
}
if (V.getAs<nonloc::SymbolVal>())
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -104,9 +104,7 @@
#if __cplusplus >= 201103L
clang_analyzer_eval(((HasCtor){1, 42}).y == 42); // expected-warning{{TRUE}}
- // FIXME: should be TRUE, but we don't inline the constructors of
- // temporaries because we can't model their destructors yet.
- clang_analyzer_eval(((HasCtorDtor){1, 42}).y == 42); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(((HasCtorDtor){1, 42}).y == 42); // expected-warning{{TRUE}}
#endif
}
}
@@ -347,6 +345,49 @@
}
}
+ struct NoWarnDerefDtor {
+ NoWarnDerefDtor(int *p) : p(p) {}
+ ~NoWarnDerefDtor() { *p = 23; } // no warning
+ int *p;
+ };
+ void testDtorInlining() {
+ int x;
+ (NoWarnDerefDtor(&x));
+ clang_analyzer_eval(x == 23); // expected-warning{{TRUE}}
+ }
+ void use(NoWarnDerefDtor);
+ void testArgumentDtorInliningPreventedByValue() {
+ int x;
+ NoWarnDerefDtor p(nullptr);
+ use(p);
+ p.p = &x;
+ }
+
+ struct WarnDerefDtor {
+ WarnDerefDtor(int *p) : p(p) {}
+ ~WarnDerefDtor() {
+ *p = 23; // expected-warning{{Dereference of null pointer}}
+ }
+ int *p;
+ };
+ void useRef(WarnDerefDtor& p) {
+ p.p = nullptr;
+ }
+ void testArgumentDtorInliningWorksByReference() {
+ int x;
+ WarnDerefDtor p(&x);
+ useRef(p);
+ }
+ void useConstRef(const NoWarnDerefDtor& p) {
+ const_cast<NoWarnDerefDtor&>(p).p = nullptr;
+ }
+ void testArgumentDtorInliningWorksByConstReference() {
+ int x;
+ // FIXME: This should warn. We need to implement correct handling of
+ // temporaries bound to parameters first.
+ useConstRef(NoWarnDerefDtor(&x));
+ }
+
void testIfAtEndOfLoop() {
int y = 0;
while (true) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
