[analyzer] Add sink after construction of temporary with no-return destructor.

The analyzer's CFG currently doesn't have nodes for calls to temporary
destructors. This causes the analyzer to explore infeasible paths in which
a no-return destructor would have stopped exploration and so results in false
positives when no-return destructors are used to implement assertions.

To mitigate these false positives, this patch stops generates a sink after
evaluating a constructor on a temporary object that has a no-return destructor.
This results in a loss of coverage because the time at which the destructor is
called may be after the time of construction (especially for lifetime-extended
temporaries).

This addresses PR15599.

rdar://problem/29131566

llvm-svn: 290140

2 files changed, 50 insertions, 0 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 011f776d0b0..7e9b2033ca3 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -346,6 +346,30 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
       defaultEvalCall(Bldr, *I, *Call);
   }
 
+  // If the CFG was contructed without elements for temporary destructors
+  // and the just-called constructor created a temporary object then
+  // stop exploration if the temporary object has a noreturn constructor.
+  // This can lose coverage because the destructor, if it were present
+  // in the CFG, would be called at the end of the full expression or
+  // later (for life-time extended temporaries) -- but avoids infeasible
+  // paths when no-return temporary destructors are used for assertions.
+  const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
+  if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
+      const MemRegion *Target = Call->getCXXThisVal().getAsRegion();
+      if (Target && isa<CXXTempObjectRegion>(Target) &&
+          Call->getDecl()->getParent()->isAnyDestructorNoReturn()) {
+
+      for (ExplodedNode *N : DstEvaluated) {
+        Bldr.generateSink(CE, N, N->getState());
+      }
+
+      // There is no need to run the PostCall and PostStmtchecker
+      // callbacks because we just generated sinks on all nodes in th
+      // frontier.
+      return;
+    }
+ }
+
   ExplodedNodeSet DstPostCall;
   getCheckerManager().runCheckersForPostCall(DstPostCall, DstEvaluated,
                                              *Call, *this);
diff --git a/clang/test/Analysis/temporaries.cpp b/clang/test/Analysis/temporaries.cpp
index e96e9b0e28d..49cf070177f 100644
--- a/clang/test/Analysis/temporaries.cpp
+++ b/clang/test/Analysis/temporaries.cpp
@@ -413,6 +413,32 @@ namespace destructors {
       value ? DefaultParam(42) : DefaultParam(42);
     }
   }
+#else // !TEMPORARY_DTORS
+
+// Test for fallback logic that conservatively stops exploration after
+// executing a temporary constructor for a class with a no-return destructor
+// when temporary destructors are not enabled in the CFG.
+
+  struct CtorWithNoReturnDtor {
+    CtorWithNoReturnDtor() = default;
+
+    ~CtorWithNoReturnDtor() __attribute__((noreturn));
+  };
+
+  void testDefaultContructorWithNoReturnDtor() {
+    CtorWithNoReturnDtor();
+    clang_analyzer_warnIfReached();  // no-warning
+  }
+
+  void testLifeExtensionWithNoReturnDtor() {
+    const CtorWithNoReturnDtor &c = CtorWithNoReturnDtor();
+
+    // This represents an (expected) loss of coverage, since the destructor
+    // of the lifetime-exended temporary is executed at at the end of
+    // scope.
+    clang_analyzer_warnIfReached();  // no-warning
+  }
+
 #endif // TEMPORARY_DTORS
 }