r189746 - [analyzer] Add very limited support for temporary destructors

Pavel Labath labath at google.com
Mon Sep 2 02:09:16 PDT 2013


Author: labath
Date: Mon Sep  2 04:09:15 2013
New Revision: 189746

URL: http://llvm.org/viewvc/llvm-project?rev=189746&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/Analysis/CFG.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
    cfe/trunk/test/Analysis/temporaries.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=189746&r1=189745&r2=189746&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Mon Sep  2 04:09:15 2013
@@ -3765,8 +3765,9 @@ static void print_elem(raw_ostream &OS,
 
   } 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";
   }
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=189746&r1=189745&r2=189746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Sep  2 04:09:15 2013
@@ -601,7 +601,15 @@ void ExprEngine::ProcessMemberDtor(const
 
 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 Stm
                                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 Stm
   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 Stm
                             getContext().getLogicalOperationType());
   }
 
-
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Condition->getLocStart(),
                                 "Error evaluating branch");

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=189746&r1=189745&r2=189746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Mon Sep  2 04:09:15 2013
@@ -91,6 +91,12 @@ void ExprEngine::performTrivialCopy(Node
 /// 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();
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=189746&r1=189745&r2=189746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Sep  2 04:09:15 2013
@@ -807,6 +807,14 @@ bool ExprEngine::shouldInlineCall(const
   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.

Modified: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=189746&r1=189745&r2=189746&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp (original)
+++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Mon Sep  2 04:09:15 2013
@@ -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

Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=189746&r1=189745&r2=189746&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Mon Sep  2 04:09:15 2013
@@ -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() {





More information about the cfe-commits mailing list