NoQ updated this revision to Diff 134341.
NoQ added a comment.

All right, so the assertion that we actually destroy all temporaries in our 
`InitializedTemporaries` map is still violated quite often -  every time we do 
any sort of lifetime extension, we're actually calling the destructor on a 
different object, because `createTemporaryRegionIfNeeded()` has moved the 
object to a different place. I made a large brain dump on this matter in 
http://lists.llvm.org/pipermail/cfe-dev/2018-February/056898.html . For now it 
means that the assertion is still not going in. I added it in a commented-out 
form. In fact, @klimek has tried that long before me, and failed in a similar 
manner. If only i added the assertion in the right place (not in 
`processCallExit`, but in `processEndOfFunction`, which is also called for the 
top frame), i would have seen it earlier :) So i've reinvented quite a bit of a 
wheel here.

- Move the assertion to the right place and disable it, explaining that 
lifetime extension is broken.
- Move the similar operator-new assertion to the right place as well, while 
we're at it.
- Add a flag to disable inlining of temporary destructors, which is different 
from having them at all. It's on by default but does nothing until 
`cfg-temporary-dtors` are also enabled.
- Add a test for that flag. This flag cannot be tested in `analyzer-config.cpp` 
because it is never read unless `cfg-temporary-dtors` takes a non-default 
value, so `ConfigDumper` doesn't see it.


https://reviews.llvm.org/D43104

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/analyzer-config.cpp
  test/Analysis/temp-obj-dtors-option.cpp
  test/Analysis/temporaries.cpp

Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -779,3 +779,116 @@
 }
 
 } // 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;
+  {
+    const 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 == 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
+    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 == 2);
+    clang_analyzer_eval(w == 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_with_copy(int coin) {
+  int x = 0, y = 0, z = 0, w = 0;
+  {
+    C c = coin ? C(x, y) : C(z, w);
+  }
+  // Temporary expression, elidable copy within branch,
+  // constructor for variable - 3 total.
+  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
Index: test/Analysis/temp-obj-dtors-option.cpp
===================================================================
--- /dev/null
+++ test/Analysis/temp-obj-dtors-option.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=false -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DINLINE -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct S {
+  int &x;
+
+  S(int &x) : x(x) { ++x; }
+  ~S() { --x; }
+};
+
+void foo() {
+  int x = 0;
+  S(x).x += 1;
+  clang_analyzer_eval(x == 1);
+#ifdef INLINE
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+}
Index: test/Analysis/analyzer-config.cpp
===================================================================
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -12,7 +12,9 @@
 
 class Foo {
 public:
-	void bar() {}
+  ~Foo() {}
+  void baz();
+	void bar() { const Foo &f = Foo(); }
 	void foo() { bar(); }
 };
 
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -330,9 +330,6 @@
     bool isNew;
     ProgramStateRef CEEState = (*I == CEBNode) ? state : (*I)->getState();
 
-    // See if we have any stale C++ allocator values.
-    assert(areCXXNewAllocatorValuesClear(CEEState, calleeCtx, callerCtx));
-
     ExplodedNode *CEENode = G.getNode(Loc, CEEState, false, &isNew);
     CEENode->addPredecessor(*I, G);
     if (!isNew)
@@ -696,6 +693,10 @@
     if (CallOpts.IsArrayCtorOrDtor)
       return CIP_DisallowedOnce;
 
+    // Allow disabling temporary destructor inlining with a separate option.
+    if (CallOpts.IsTemporaryCtorOrDtor && !Opts.mayInlineCXXTemporaryDtors())
+      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.
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -102,14 +102,15 @@
 const MemRegion *
 ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
                                           ExplodedNode *Pred,
+                                          const ConstructionContext *CC,
                                           EvalCallOptions &CallOpts) {
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
   MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
 
   // See if we're constructing an existing region by looking at the
   // current construction context.
-  if (auto CC = getCurrentCFGElement().getAs<CFGConstructor>()) {
+  if (CC) {
     if (const Stmt *TriggerStmt = CC->getTriggerStmt()) {
       if (const CXXNewExpr *CNE = dyn_cast<CXXNewExpr>(TriggerStmt)) {
         if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
@@ -221,10 +222,20 @@
   // the entire array).
 
   EvalCallOptions CallOpts;
+  auto C = getCurrentCFGElement().getAs<CFGConstructor>();
+  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<CXXBindTemporaryExpr>(CC->getTriggerStmt());
+    }
     break;
   }
   case CXXConstructExpr::CK_VirtualBase:
@@ -292,17 +303,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
@@ -316,9 +328,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<CXXTempObjectRegion>(Target));
+      }
+
+      Bldr.generateNode(CE, *I, State, /*tag=*/nullptr,
+                        ProgramPoint::PreStmtKind);
     }
   }
 
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<const CXXBindTemporaryExpr *, const StackFrameContext *>
-    CXXBindTemporaryContext;
+typedef llvm::ImmutableMap<std::pair<const CXXBindTemporaryExpr *,
+                                     const StackFrameContext *>,
+                           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<CXXBindTemporaryContext>)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries,
+                                 InitializedTemporariesMap)
 
 typedef llvm::ImmutableMap<std::pair<const CXXNewExpr *,
-                           const LocationContext *>, 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<InitializedTemporaries>(Key)) {
+    return State->set<InitializedTemporaries>(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;
+  while (LC != ToLC) {
+    assert(LC && "ToLC must be a parent of FromLC!");
+    for (auto I : State->get<InitializedTemporaries>())
+      if (I.first.second == LC)
+        return false;
+
+    LC = LC->getParent();
+  }
+  return true;
+}
+
 ProgramStateRef
 ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State,
                                     const CXXNewExpr *CNE,
@@ -349,14 +382,14 @@
                                                const LocationContext *FromLC,
                                                const LocationContext *ToLC) {
   const LocationContext *LC = FromLC;
-  do {
+  while (LC != ToLC) {
+    assert(LC && "ToLC must be a parent of FromLC!");
     for (auto I : State->get<CXXNewAllocatorValues>())
       if (I.first.second == LC)
         return false;
 
     LC = LC->getParent();
-    assert(LC && "ToLC must be a parent of FromLC!");
-  } while (LC != ToLC);
+  }
   return true;
 }
 
@@ -390,11 +423,16 @@
                                                   const LocationContext *LC) {
   PrintingPolicy PP =
       LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy();
-  for (auto I : State->get<InitializedTemporariesSet>()) {
-    if (I.second != LC)
+  for (auto I : State->get<InitializedTemporaries>()) {
+    std::pair<const CXXBindTemporaryExpr *, const LocationContext *> 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<InitializedTemporariesSet>().isEmpty()) {
+    if (!State->get<InitializedTemporaries>().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<InitializedTemporaries>())
+    if (I.second)
+      SymReaper.markLive(I.second);
+
   for (auto I : CleanedState->get<CXXNewAllocatorValues>()) {
     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<InitializedTemporariesSet>(
-      std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame()))) {
+  const MemRegion *MR = nullptr;
+  if (const CXXTempObjectRegion *const *MRPtr =
+          State->get<InitializedTemporaries>(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<InitializedTemporariesSet>(
+    // but we don't insert the constructors, so the entry in
+    // InitializedTemporaries may be missing.
+    State = State->remove<InitializedTemporaries>(
         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<InitializedTemporariesSet>(
+  if (Pred->getState()->contains<InitializedTemporaries>(
           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<InitializedTemporariesSet>(
-            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<InitializedTemporariesSet>(
-          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<CXXBindTemporaryExpr>(S), PreVisit, Next);
-      getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this);
+      getCheckerManager().runCheckersForPostStmt(Dst, PreVisit, S, *this);
       Bldr.addNodes(Dst);
       break;
     }
@@ -2050,22 +2069,6 @@
     builder.generateNode(I, state);
 }
 
-#if 0
-static bool stackFrameDoesNotContainInitializedTemporaries(ExplodedNode &Pred) {
-  const StackFrameContext* Frame = Pred.getStackFrame();
-  const llvm::ImmutableSet<CXXBindTemporaryContext> &Set =
-      Pred.getState()->get<InitializedTemporariesSet>();
-  return std::find_if(Set.begin(), Set.end(),
-                      [&](const CXXBindTemporaryContext &Ctx) {
-                        if (Ctx.second == Frame) {
-                          Ctx.first->dump();
-                          llvm::errs() << "\n";
-                        }
-           return Ctx.second == Frame;
-         }) == Set.end();
-}
-#endif
-
 void ExprEngine::processBeginOfFunction(NodeBuilderContext &BC,
                                         ExplodedNode *Pred,
                                         ExplodedNodeSet &Dst,
@@ -2079,9 +2082,18 @@
 void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
                                       ExplodedNode *Pred,
                                       const ReturnStmt *RS) {
-  // FIXME: Assert that stackFrameDoesNotContainInitializedTemporaries(*Pred)).
-  // We currently cannot enable this assert, as lifetime extended temporaries
-  // are not modelled correctly.
+  // See if we have any stale C++ allocator values.
+  assert(areCXXNewAllocatorValuesClear(Pred->getState(),
+                                       Pred->getLocationContext(),
+                                       Pred->getStackFrame()->getParent()));
+  // FIXME: We currently cannot enable this assert, as lifetime extended
+  // temporaries are not modelled correctly. When we materialize the temporary,
+  // we do createTemporaryRegionIfNeeded(), and the region changes, and also
+  // the respective destructor becomes automatic from temporary.
+  // assert(areInitializedTemporariesClear(Pred->getState(),
+  //                                       Pred->getLocationContext(),
+  //                                       Pred->getStackFrame()->getParent()));
+
   PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
   StateMgr.EndPath(Pred->getState());
 
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<CFGImplicitDtor>() &&
+  assert(E.getAs<CFGImplicitDtor>() || E.getAs<CFGTemporaryDtor>() &&
          "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/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -224,6 +224,11 @@
                           /*Default=*/false);
 }
 
+bool AnalyzerOptions::mayInlineCXXTemporaryDtors() {
+  return getBooleanOption(InlineCXXTemporaryDtors,
+                          "c++-temp-dtor-inlining",
+                          /*Default=*/true);
+}
 
 bool AnalyzerOptions::mayInlineObjCMethod() {
   return getBooleanOption(ObjCInliningMode,
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: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -239,6 +239,9 @@
   /// \sa mayInlineCXXSharedPtrDtor
   Optional<bool> InlineCXXSharedPtrDtor;
 
+  /// \sa mayInlineCXXTemporaryDtors
+  Optional<bool> InlineCXXTemporaryDtors;
+
   /// \sa mayInlineObjCMethod
   Optional<bool> ObjCInliningMode;
 
@@ -478,6 +481,17 @@
   /// accepts the values "true" and "false".
   bool mayInlineCXXSharedPtrDtor();
 
+  /// Returns true if C++ temporary destructors should be inlined during
+  /// analysis.
+  ///
+  /// If temporary destructors are disabled in the CFG via the
+  /// 'cfg-temporary-dtors' option, temporary destructors would not be
+  /// inlined anyway.
+  ///
+  /// This is controlled by the 'c++-temp-dtor-inlining' config option, which
+  /// accepts the values "true" and "false".
+  bool mayInlineCXXTemporaryDtors();
+
   /// Returns whether or not paths that go through null returns should be
   /// suppressed.
   ///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to