[analyzer] Add very limited support for temporary destructors

This is an improved version of r186498. It enables ExprEngine to reason about
temporary object destructors.  However, these destructor calls are never
inlined, since this feature is still broken. Still, this is sufficient to
properly handle noreturn temporary destructors.

Now, the analyzer correctly handles expressions like "a || A()", and executes the
destructor of "A" only on the paths where "a" evaluted to false.

Temporary destructor processing is still off by default and one has to
explicitly request it by setting cfg-temporary-dtors=true.

Reviewers: jordan_rose

CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1259

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@189746 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 52706b012630263edd152370a2e084952a5f58fe..ad0acb7abe5c9f33aafc1390d24d9e7d3a7b7671 100644 (file)
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -3765,8 +3765,9 @@ static void print_elem(raw_ostream &OS, StmtPrinterHelper* Helper,
 
   } else if (Optional<CFGTemporaryDtor> TE = E.getAs<CFGTemporaryDtor>()) {
     const CXXBindTemporaryExpr *BT = TE->getBindTemporaryExpr();
-    OS << "~" << BT->getType()->getAsCXXRecordDecl()->getName() << "()";
-    OS << " (Temporary object destructor)\n";
+    OS << "~";
+    BT->getType().print(OS, PrintingPolicy(Helper->getLangOpts()));
+    OS << "() (Temporary object destructor)\n";
   }
 }
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 6aa9174ad6ebcd50ddd869ce1f7bd174e214e161..79139df786eeab93a0e986a3dfea2020c83961e9 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -601,7 +601,15 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
 
 void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
                                       ExplodedNode *Pred,
-                                      ExplodedNodeSet &Dst) {}
+                                      ExplodedNodeSet &Dst) {
+
+  QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
+
+  // 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, NULL, D.getBindTemporaryExpr(),
+                     /*IsBase=*/ false, Pred, Dst);
+}
 
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
                        ExplodedNodeSet &DstTop) {
@@ -1332,7 +1340,8 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
                                ExplodedNodeSet &Dst,
                                const CFGBlock *DstT,
                                const CFGBlock *DstF) {
-  PrettyStackTraceLocationContext StackCrashInfo(Pred->getLocationContext());
+  const LocationContext *LCtx = Pred->getLocationContext();
+  PrettyStackTraceLocationContext StackCrashInfo(LCtx);
   currBldrCtx = &BldCtx;
 
   // Check for NULL conditions; e.g. "for(;;)"
@@ -1347,10 +1356,14 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
   SVal TrueVal = SVB.makeTruthVal(true);
   SVal FalseVal = SVB.makeTruthVal(false);
 
-  // Resolve the condition in the precense of nested '||' and '&&'.
   if (const Expr *Ex = dyn_cast<Expr>(Condition))
     Condition = Ex->IgnoreParens();
-  Condition = ResolveCondition(Condition, BldCtx.getBlock());
+
+  // If the value is already available, we don't need to do anything.
+  if (Pred->getState()->getSVal(Condition, LCtx).isUnknownOrUndef()) {
+    // Resolve the condition in the presence of nested '||' and '&&'.
+    Condition = ResolveCondition(Condition, BldCtx.getBlock());
+  }
 
   // Cast truth values to the correct type.
   if (const Expr *Ex = dyn_cast<Expr>(Condition)) {
@@ -1360,7 +1373,6 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
                             getContext().getLogicalOperationType());
   }
 
-
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Condition->getLocStart(),
                                 "Error evaluating branch");

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 27963ade934c6409e25087e357aedc3825963104..b128bee1429c2efa4cda0387f47cd77d3f673842 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -91,6 +91,12 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
 /// If the type is not an array type at all, the original value is returned.
 static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
                                   QualType &Ty) {
+  // FIXME: This check is just a temporary workaround, because
+  // ProcessTemporaryDtor sends us NULL regions. It will not be necessary once
+  // we can properly process temporary destructors.
+  if (!LValue.getAsRegion())
+    return LValue;
+
   SValBuilder &SVB = State->getStateManager().getSValBuilder();
   ASTContext &Ctx = SVB.getContext();
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 4fb6523944ff95aa69684feeaef80f88048eb934..08d239edab7b41fec8419a482130cb2735f00a9f 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -807,6 +807,14 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   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<CXXDestructorCall>(Call)) {
+    if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())
+      return false;
+  }
+
   // The auto-synthesized bodies are essential to inline as they are
   // usually small and commonly used. Note: we should do this check early on to
   // ensure we always inline these calls.

diff --git a/test/Analysis/temp-obj-dtors-cfg-output.cpp b/test/Analysis/temp-obj-dtors-cfg-output.cpp
index 1ddccb704b132d8b1719169d4dff37b4a8191409..ff68a876e91049bb80bb78736003a2c1bdf29492 100644 (file)
--- a/test/Analysis/temp-obj-dtors-cfg-output.cpp
+++ b/test/Analysis/temp-obj-dtors-cfg-output.cpp
@@ -108,6 +108,24 @@ TestCtorInits::TestCtorInits()
   : a(int(A()) + int(B()))
   , b() {}
 
+class NoReturn {
+public:
+  ~NoReturn() __attribute__((noreturn));
+  void f();
+};
+
+void test_noreturn1() {
+  int a;
+  NoReturn().f();
+  int b;
+}
+
+void test_noreturn2() {
+  int a;
+  NoReturn(), 47;
+  int b;
+}
+
 // CHECK:   [B1 (ENTRY)]
 // CHECK:     Succs (1): B0
 // CHECK:   [B0 (EXIT)]
@@ -846,3 +864,36 @@ TestCtorInits::TestCtorInits()
 // CHECK:   [B0 (EXIT)]
 // CHECK:     Preds (1): B1
 
+// CHECK:   [B3 (ENTRY)]
+// CHECK:     Succs (1): B2
+// CHECK:   [B1]
+// CHECK:     1: int b;
+// CHECK:     Succs (1): B0
+// CHECK:   [B2]
+// CHECK:     1: int a;
+// CHECK:     2: NoReturn() (CXXConstructExpr, class NoReturn)
+// CHECK:     3: [B2.2] (BindTemporary)
+// CHECK:     4: [B2.3].f
+// CHECK:     5: [B2.4]()
+// CHECK:     6: ~NoReturn() (Temporary object destructor)
+// CHECK:     Preds (1): B3
+// CHECK:     Succs (1): B0
+// CHECK:   [B0 (EXIT)]
+// CHECK:     Preds (2): B1 B2
+
+// CHECK:   [B3 (ENTRY)]
+// CHECK:     Succs (1): B2
+// CHECK:   [B1]
+// CHECK:     1: int b;
+// CHECK:     Succs (1): B0
+// CHECK:   [B2]
+// CHECK:     1: int a;
+// CHECK:     2: NoReturn() (CXXConstructExpr, class NoReturn)
+// CHECK:     3: [B2.2] (BindTemporary)
+// CHECK:     4: 47
+// CHECK:     5: ... , [B2.4]
+// CHECK:     6: ~NoReturn() (Temporary object destructor)
+// CHECK:     Preds (1): B3
+// CHECK:     Succs (1): B0
+// CHECK:   [B0 (EXIT)]
+// CHECK:     Preds (2): B1 B2

diff --git a/test/Analysis/temporaries.cpp b/test/Analysis/temporaries.cpp
index 5e1771cc4411159f3cce730965809b9e016e7c92..d5237e597cf706c5a2b24713c92063922ca0f148 100644 (file)
--- a/test/Analysis/temporaries.cpp
+++ b/test/Analysis/temporaries.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++03 %s
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++11 %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -analyzer-config cfg-temporary-dtors=true %s -DTEMPORARY_DTORS
 
 extern bool clang_analyzer_eval(bool);
 
@@ -123,23 +124,76 @@ namespace destructors {
     }
   }
 
-  void testConsistency(int i) {
-    struct NoReturnDtor {
-      ~NoReturnDtor() __attribute__((noreturn));
-    };
-    extern bool check(const NoReturnDtor &);
-    
+#ifdef TEMPORARY_DTORS
+  struct NoReturnDtor {
+    ~NoReturnDtor() __attribute__((noreturn));
+  };
+
+  void noReturnTemp(int *x) {
+    if (! x) NoReturnDtor();
+    *x = 47; // no warning
+  }
+
+  void noReturnInline(int **x) {
+    NoReturnDtor();
+  }
+
+  void callNoReturn() {
+    int *x;
+    noReturnInline(&x);
+    *x = 47; // no warning
+  }
+
+  extern bool check(const NoReturnDtor &);
+
+  void testConsistencyIf(int i) {
     if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
       clang_analyzer_eval(true); // expected-warning{{TRUE}}
 
     if (i != 5)
       return;
     if (i == 5 && (i == 4 || check(NoReturnDtor()) || i == 5)) {
-      // FIXME: Should be no-warning, because the noreturn destructor should
-      // fire on all paths.
+      clang_analyzer_eval(true); // no warning, unreachable code
+    }
+  }
+
+  void testConsistencyTernary(int i) {
+    (i == 5 && (i == 4 || check(NoReturnDtor()) || i == 5)) ? 1 : 0;
+
+    clang_analyzer_eval(true);  // expected-warning{{TRUE}}
+
+    if (i != 5)
+      return;
+
+    (i == 5 && (i == 4 || check(NoReturnDtor()) || i == 5)) ? 1 : 0;
+
+    clang_analyzer_eval(true); // no warning, unreachable code
+  }
+
+  void testConsistencyNested(int i) {
+    extern bool compute(bool);
+
+    if (i == 5 && (i == 4 || i == 5 || check(NoReturnDtor())))
+      clang_analyzer_eval(true);  // expected-warning{{TRUE}}
+
+    if (i != 5)
+      return;
+
+    if (compute(i == 5 &&
+                (i == 4 || compute(true) ||
+                 compute(i == 5 && (i == 4 || check(NoReturnDtor()))))) ||
+        i != 4) {
       clang_analyzer_eval(true); // expected-warning{{TRUE}}
     }
+
+    if (compute(i == 5 &&
+                (i == 4 || i == 4 ||
+                 compute(i == 5 && (i == 4 || check(NoReturnDtor()))))) ||
+        i != 4) {
+      clang_analyzer_eval(true); // no warning, unreachable code
+    }
   }
+#endif // TEMPORARY_DTORS
 }
 
 void testStaticMaterializeTemporaryExpr() {