Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -448,10 +448,6 @@ ExplodedNode *Pred, ExplodedNodeSet &Dst); - void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE, - ExplodedNodeSet &PreVisit, - ExplodedNodeSet &Dst); - void VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred, ExplodedNodeSet &Dst); @@ -696,8 +692,22 @@ /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts. const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE, ExplodedNode *Pred, + const ConstructionContext *CC, EvalCallOptions &CallOpts); + /// Store the region of a C++ temporary object corresponding to a + /// CXXBindTemporaryExpr for later destruction. + static ProgramStateRef addInitializedTemporary( + ProgramStateRef State, const CXXBindTemporaryExpr *BTE, + const LocationContext *LC, const CXXTempObjectRegion *R); + + /// Check if all initialized temporary regions are clear for the given + /// context range (including FromLC, not including ToLC). + /// This is useful for assertions. + static bool areInitializedTemporariesClear(ProgramStateRef State, + const LocationContext *FromLC, + const LocationContext *ToLC); + /// Store the region returned by operator new() so that the constructor /// that follows it knew what location to initialize. The value should be /// cleared once the respective CXXNewExpr CFGStmt element is processed. Index: lib/StaticAnalyzer/Core/CallEvent.cpp =================================================================== --- lib/StaticAnalyzer/Core/CallEvent.cpp +++ lib/StaticAnalyzer/Core/CallEvent.cpp @@ -1197,9 +1197,8 @@ // destructors, though this could change in the future. const CFGBlock *B = CalleeCtx->getCallSiteBlock(); CFGElement E = (*B)[CalleeCtx->getIndex()]; - assert(E.getAs() && + assert(E.getAs() || 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 @@ -54,18 +54,21 @@ STATISTIC(NumTimesRetriedWithoutInlining, "The # of times we re-evaluated a call without inlining"); -typedef std::pair - CXXBindTemporaryContext; +typedef llvm::ImmutableMap, + const CXXTempObjectRegion *> + InitializedTemporariesMap; // Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated. // The StackFrameContext assures that nested calls due to inlined recursive // functions do not interfere. -REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporariesSet, - llvm::ImmutableSet) +REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries, + InitializedTemporariesMap) typedef llvm::ImmutableMap, SVal> - CXXNewAllocatorValuesMap; + const LocationContext *>, + SVal> + CXXNewAllocatorValuesMap; // Keeps track of return values of various operator new() calls between // evaluation of the inlined operator new(), through the constructor call, @@ -323,6 +326,36 @@ return State; } +ProgramStateRef ExprEngine::addInitializedTemporary( + ProgramStateRef State, const CXXBindTemporaryExpr *BTE, + const LocationContext *LC, const CXXTempObjectRegion *R) { + const auto &Key = std::make_pair(BTE, LC->getCurrentStackFrame()); + if (!State->contains(Key)) { + return State->set(Key, R); + } + + // FIXME: Currently the state might already contain the marker due to + // incorrect handling of temporaries bound to default parameters; for + // those, we currently skip the CXXBindTemporaryExpr but rely on adding + // temporary destructor nodes. Otherwise, this branch should be unreachable. + return State; +} + +bool ExprEngine::areInitializedTemporariesClear(ProgramStateRef State, + const LocationContext *FromLC, + const LocationContext *ToLC) { + const LocationContext *LC = FromLC; + do { + for (auto I : State->get()) + if (I.first.second == LC) + return false; + + LC = LC->getParent(); + assert(LC && "ToLC must be a parent of FromLC!"); + } while (LC != ToLC); + return true; +} + ProgramStateRef ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE, @@ -390,11 +423,16 @@ const LocationContext *LC) { PrintingPolicy PP = LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy(); - for (auto I : State->get()) { - if (I.second != LC) + for (auto I : State->get()) { + std::pair Key = + I.first; + const MemRegion *Value = I.second; + if (Key.second != LC) continue; - Out << '(' << I.second << ',' << I.first << ") "; - I.first->printPretty(Out, nullptr, PP); + Out << '(' << Key.second << ',' << Key.first << ") "; + Key.first->printPretty(Out, nullptr, PP); + if (Value) + Out << " : " << Value; Out << NL; } } @@ -422,7 +460,7 @@ const char *NL, const char *Sep, const LocationContext *LCtx) { if (LCtx) { - if (!State->get().isEmpty()) { + if (!State->get().isEmpty()) { Out << Sep << "Initialized temporaries:" << NL; LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) { @@ -536,6 +574,10 @@ const StackFrameContext *SFC = LC ? LC->getCurrentStackFrame() : nullptr; SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager()); + for (auto I : CleanedState->get()) + if (I.second) + SymReaper.markLive(I.second); + for (auto I : CleanedState->get()) { if (SymbolRef Sym = I.second.getAsSymbol()) SymReaper.markLive(Sym); @@ -900,12 +942,18 @@ ExplodedNodeSet CleanDtorState; StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx); ProgramStateRef State = Pred->getState(); - if (State->contains( - std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame()))) { + const MemRegion *MR = nullptr; + if (const CXXTempObjectRegion *const *MRPtr = + State->get(std::make_pair( + D.getBindTemporaryExpr(), Pred->getStackFrame()))) { // FIXME: Currently we insert temporary destructors for default parameters, - // but we don't insert the constructors. - State = State->remove( + // but we don't insert the constructors, so the entry in + // InitializedTemporaries may be missing. + State = State->remove( std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame())); + // *MRPtr may still be null when the construction context for the temporary + // was not implemented. + MR = *MRPtr; } StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State); @@ -916,12 +964,11 @@ 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(), + if (!MR) + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; + VisitCXXDestructor(varType, MR, D.getBindTemporaryExpr(), /*IsBase=*/false, CleanPred, Dst, CallOpts); } @@ -932,7 +979,7 @@ const CFGBlock *DstT, const CFGBlock *DstF) { BranchNodeBuilder TempDtorBuilder(Pred, Dst, BldCtx, DstT, DstF); - if (Pred->getState()->contains( + if (Pred->getState()->contains( std::make_pair(BTE, Pred->getStackFrame()))) { TempDtorBuilder.markInfeasible(false); TempDtorBuilder.generateNode(Pred->getState(), true, Pred); @@ -942,32 +989,6 @@ } } -void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE, - ExplodedNodeSet &PreVisit, - ExplodedNodeSet &Dst) { - if (!getAnalysisManager().options.includeTemporaryDtorsInCFG()) { - // In case we don't have temporary destructors in the CFG, do not mark - // the initialization - we would otherwise never clean it up. - Dst = PreVisit; - return; - } - StmtNodeBuilder StmtBldr(PreVisit, Dst, *currBldrCtx); - for (ExplodedNode *Node : PreVisit) { - ProgramStateRef State = Node->getState(); - - if (!State->contains( - std::make_pair(BTE, Node->getStackFrame()))) { - // FIXME: Currently the state might already contain the marker due to - // incorrect handling of temporaries bound to default parameters; for - // those, we currently skip the CXXBindTemporaryExpr but rely on adding - // temporary destructor nodes. - State = State->add( - std::make_pair(BTE, Node->getStackFrame())); - } - StmtBldr.generateNode(BTE, Node, State); - } -} - namespace { class CollectReachableSymbolsCallback final : public SymbolVisitor { InvalidatedSymbols Symbols; @@ -1130,9 +1151,7 @@ Bldr.takeNodes(Pred); ExplodedNodeSet PreVisit; getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this); - ExplodedNodeSet Next; - VisitCXXBindTemporaryExpr(cast(S), PreVisit, Next); - getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this); + getCheckerManager().runCheckersForPostStmt(Dst, PreVisit, S, *this); Bldr.addNodes(Dst); break; } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -102,6 +102,7 @@ const MemRegion * ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE, ExplodedNode *Pred, + const ConstructionContext *CC, EvalCallOptions &CallOpts) { const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); @@ -109,7 +110,7 @@ // See if we're constructing an existing region by looking at the // current construction context. - if (auto CC = getCurrentCFGElement().getAs()) { + if (CC) { if (const Stmt *TriggerStmt = CC->getTriggerStmt()) { if (const CXXNewExpr *CNE = dyn_cast(TriggerStmt)) { if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) { @@ -217,10 +218,20 @@ // the entire array). EvalCallOptions CallOpts; + auto C = getCurrentCFGElement().getAs(); + const ConstructionContext *CC = C ? C->getConstructionContext() : nullptr; + + const CXXBindTemporaryExpr *BTE = nullptr; switch (CE->getConstructionKind()) { case CXXConstructExpr::CK_Complete: { - Target = getRegionForConstructedObject(CE, Pred, CallOpts); + Target = getRegionForConstructedObject(CE, Pred, CC, CallOpts); + if (CC && AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG() && + !CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion && + CallOpts.IsTemporaryCtorOrDtor) { + // May as well be a ReturnStmt. + BTE = dyn_cast(CC->getTriggerStmt()); + } break; } case CXXConstructExpr::CK_VirtualBase: @@ -288,17 +299,18 @@ ExplodedNodeSet DstPreVisit; getCheckerManager().runCheckersForPreStmt(DstPreVisit, Pred, CE, *this); + // FIXME: Is it possible and/or useful to do this before PreStmt? ExplodedNodeSet PreInitialized; { StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx); - if (CE->requiresZeroInitialization()) { - // Type of the zero doesn't matter. - SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy); - - for (ExplodedNodeSet::iterator I = DstPreVisit.begin(), - E = DstPreVisit.end(); - I != E; ++I) { - ProgramStateRef State = (*I)->getState(); + for (ExplodedNodeSet::iterator I = DstPreVisit.begin(), + E = DstPreVisit.end(); + I != E; ++I) { + ProgramStateRef State = (*I)->getState(); + if (CE->requiresZeroInitialization()) { + // Type of the zero doesn't matter. + SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy); + // FIXME: Once we properly handle constructors in new-expressions, we'll // need to invalidate the region before setting a default value, to make // sure there aren't any lingering bindings around. This probably needs @@ -312,9 +324,15 @@ // since it's then possible to be initializing one part of a multi- // dimensional array. State = State->bindDefault(loc::MemRegionVal(Target), ZeroVal, LCtx); - Bldr.generateNode(CE, *I, State, /*tag=*/nullptr, - ProgramPoint::PreStmtKind); } + + if (BTE) { + State = addInitializedTemporary(State, BTE, LCtx, + cast(Target)); + } + + Bldr.generateNode(CE, *I, State, /*tag=*/nullptr, + ProgramPoint::PreStmtKind); } } Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -332,6 +332,7 @@ // See if we have any stale C++ allocator values. assert(areCXXNewAllocatorValuesClear(CEEState, calleeCtx, callerCtx)); + assert(areInitializedTemporariesClear(CEEState, calleeCtx, callerCtx)); ExplodedNode *CEENode = G.getNode(Loc, CEEState, false, &isNew); CEENode->addPredecessor(*I, G); Index: test/Analysis/temporaries.cpp =================================================================== --- test/Analysis/temporaries.cpp +++ test/Analysis/temporaries.cpp @@ -779,3 +779,81 @@ } } // namespace test_temporary_object_expr + +namespace test_match_constructors_and_destructors { +class C { +public: + int &x, &y; + C(int &_x, int &_y) : x(_x), y(_y) { ++x; } + C(const C &c): x(c.x), y(c.y) { ++x; } + ~C() { ++y; } +}; + +void test_simple_temporary() { + int x = 0, y = 0; + { + const C &c = C(x, y); + } + // One constructor and one destructor. + clang_analyzer_eval(x == 1); + clang_analyzer_eval(y == 1); +#ifdef TEMPORARY_DTORS + // expected-warning@-3{{TRUE}} + // expected-warning@-3{{TRUE}} +#else + // expected-warning@-6{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} +#endif +} + +void test_simple_temporary_with_copy() { + int x = 0, y = 0; + { + C c = C(x, y); + } + // Two constructors (temporary object expr and copy) and two destructors. + clang_analyzer_eval(x == 2); + clang_analyzer_eval(y == 2); +#ifdef TEMPORARY_DTORS + // expected-warning@-3{{TRUE}} + // expected-warning@-3{{TRUE}} +#else + // expected-warning@-6{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} +#endif +} + +void test_ternary_temporary(int coin) { + int x = 0, y = 0, z = 0, w = 0; + { + C c = coin ? C(x, y) : C(z, w); + } + // This time each branch contains an additional elidable copy constructor. + if (coin) { + clang_analyzer_eval(x == 3); + clang_analyzer_eval(y == 3); +#ifdef TEMPORARY_DTORS + // expected-warning@-3{{TRUE}} + // expected-warning@-3{{TRUE}} +#else + // expected-warning@-6{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} +#endif + clang_analyzer_eval(z == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(w == 0); // expected-warning{{TRUE}} + + } else { + clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(z == 3); + clang_analyzer_eval(w == 3); +#ifdef TEMPORARY_DTORS + // expected-warning@-3{{TRUE}} + // expected-warning@-3{{TRUE}} +#else + // expected-warning@-6{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} +#endif + } +} +} // namespace test_match_constructors_and_destructors