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 @@ -452,6 +452,10 @@ ExplodedNode *Pred, ExplodedNodeSet &Dst); + void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE, + ExplodedNodeSet &PreVisit, + ExplodedNodeSet &Dst); + void VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred, ExplodedNodeSet &Dst); Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1089,6 +1089,34 @@ } } +void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE, + ExplodedNodeSet &PreVisit, + ExplodedNodeSet &Dst) { + // This is a fallback solution in case we didn't have a construction + // context when we were constructing the temporary. Otherwise the map should + // have been populated there. + 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(); + const auto &Key = std::make_pair(BTE, Node->getStackFrame()); + + if (!State->contains(Key)) { + // FIXME: Currently the state might also 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->set(Key, nullptr); + } + StmtBldr.generateNode(BTE, Node, State); + } +} + namespace { class CollectReachableSymbolsCallback final : public SymbolVisitor { InvalidatedSymbols Symbols; @@ -1251,7 +1279,9 @@ Bldr.takeNodes(Pred); ExplodedNodeSet PreVisit; getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this); - getCheckerManager().runCheckersForPostStmt(Dst, PreVisit, S, *this); + ExplodedNodeSet Next; + VisitCXXBindTemporaryExpr(cast(S), PreVisit, Next); + getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this); Bldr.addNodes(Dst); break; } @@ -2194,13 +2224,13 @@ Pred->getLocationContext(), Pred->getStackFrame()->getParent())); - // FIXME: We currently assert that temporaries are clear, as lifetime extended - // temporaries are not always modelled correctly. In some cases when we - // materialize the temporary, we do createTemporaryRegionIfNeeded(), and - // the region changes, and also the respective destructor becomes automatic - // from temporary. So for now clean up the state manually before asserting. - // Ideally, the code above the assertion should go away, but the assertion - // should remain. + // FIXME: We currently cannot assert that temporaries are clear, because + // lifetime extended temporaries are not always modelled correctly. In some + // cases when we materialize the temporary, we do + // createTemporaryRegionIfNeeded(), and the region changes, and also the + // respective destructor becomes automatic from temporary. So for now clean up + // the state manually before asserting. Ideally, the code above the assertion + // should go away, but the assertion should remain. { ExplodedNodeSet CleanUpTemporaries; NodeBuilder Bldr(Pred, CleanUpTemporaries, BC); @@ -2217,9 +2247,12 @@ LC = LC->getParent(); } if (State != Pred->getState()) { - Bldr.generateNode(Pred->getLocation(), State, Pred); - assert(CleanUpTemporaries.size() <= 1); - Pred = CleanUpTemporaries.empty() ? Pred : *CleanUpTemporaries.begin(); + Pred = Bldr.generateNode(Pred->getLocation(), State, Pred); + if (!Pred) { + // The node with clean temporaries already exists. We might have reached + // it on a path on which we initialize different temporaries. + return; + } } } assert(areInitializedTemporariesClear(Pred->getState(), Index: cfe/trunk/test/Analysis/temporaries.cpp =================================================================== --- cfe/trunk/test/Analysis/temporaries.cpp +++ cfe/trunk/test/Analysis/temporaries.cpp @@ -900,7 +900,13 @@ class C { public: - ~C() { glob = 1; } + ~C() { + glob = 1; + clang_analyzer_checkInlined(true); +#ifdef TEMPORARY_DTORS + // expected-warning@-2{{TRUE}} +#endif + } }; C get(); @@ -913,12 +919,11 @@ // temporaries: the return value of get() and the elidable copy constructor // of that return value into is(). According to the CFG, we need to cleanup // both of them depending on whether the temporary corresponding to the - // return value of get() was initialized. However, for now we don't track - // temporaries returned from functions, so we take the wrong branch. + // return value of get() was initialized. However, we didn't track + // temporaries returned from functions, so we took the wrong branch. coin && is(get()); // no-crash - // FIXME: We should have called the destructor, i.e. should be TRUE, - // at least when we inline temporary destructors. - clang_analyzer_eval(glob == 1); // expected-warning{{UNKNOWN}} + // FIXME: Should be true once we inline all destructors. + clang_analyzer_eval(glob); // expected-warning{{UNKNOWN}} } } // namespace dont_forget_destructor_around_logical_op