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(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(E)->getExpr(); goto tryAgain; + + case Stmt::CallExprClass: { + CFGBlock *B = Block; + for (Expr *Arg : cast(E)->arguments()) { + if (!Arg) continue; + if (CXXBindTemporaryExpr *BTE = dyn_cast(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 Dtor = + elem.getAs()) { + // Temporary objects need to survive until the destructor is called. + val.liveStmts = SSetFact.add(val.liveStmts, + Dtor->getBindTemporaryExpr()->getSubExpr()); + continue; + } if (!elem.getAs()) 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() && "All other CFG elements should have exprs"); - assert(!E.getAs() && "We don't handle temporaries yet"); SValBuilder &SVB = State->getStateManager().getSValBuilder(); const CXXDestructorDecl *Dtor = cast(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( 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 LCV = + Val.getAs()) { + 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(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(Call)) { - if ((*currBldrCtx->getBlock())[currStmtIdx].getAs()) - return false; + if (Optional Dtor = + (*currBldrCtx->getBlock())[currStmtIdx].getAs()) { + // 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 LCV = V.getAs()) { - if (Optional NewB = tryBindSmallStruct(B, R, RD, *LCV)) + if (Optional NewB = tryBindSmallStruct(B, R, RD, *LCV)) { return *NewB; + } return bindAggregate(B, R, V); } if (V.getAs()) 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(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) {