[analyzer] Treat C++ 'throw' as a sink.

Our current handling of 'throw' is all CFG-based: it jumps to a 'catch' block
if there is one and the function exit block if not. But this doesn't really
get the right behavior when a function is inlined: execution will continue on
the caller's side, which is always the wrong thing to do.

Even within a single function, 'throw' completely skips any destructors that
are to be run. This is essentially the same problem as @finally -- a CFGBlock
that can have multiple entry points, whose exit points depend on whether it
was entered normally or exceptionally.

Representing 'throw' as a sink matches our current (non-)handling of @throw.
It's not a perfect solution, but it's better than continuing analysis in an
inconsistent or even impossible state.

<rdar://problem/12113713>

llvm-svn: 162157

3 files changed, 21 insertions, 9 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index e7b009a176e..c64a35eafbb 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -607,11 +607,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::AtomicExprClass:
       // Fall through.
 
-    // Currently all handling of 'throw' just falls to the CFG.  We
-    // can consider doing more if necessary.
-    case Stmt::CXXThrowExprClass:
-      // Fall through.
-      
     // Cases we intentionally don't evaluate, since they don't need
     // to be explicitly evaluated.
     case Stmt::AddrLabelExprClass:
@@ -886,12 +881,12 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       Bldr.addNodes(Dst);
       break;
 
-    case Stmt::ObjCAtThrowStmtClass: {
+    case Stmt::ObjCAtThrowStmtClass:
+    case Stmt::CXXThrowExprClass:
       // FIXME: This is not complete.  We basically treat @throw as
       // an abort.
       Bldr.generateNode(S, Pred, Pred->getState(), /*IsSink=*/true);
       break;
-    }
 
     case Stmt::ReturnStmtClass:
       Bldr.takeNodes(Pred);
diff --git a/clang/test/Analysis/exceptions.mm b/clang/test/Analysis/exceptions.mm
index 7306038ba1d..ab2a6a68519 100644
--- a/clang/test/Analysis/exceptions.mm
+++ b/clang/test/Analysis/exceptions.mm
@@ -21,3 +21,18 @@ int testObjC() {
   return a; // no-warning
 }
 
+
+void inlinedCXX() {
+  clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+  throw -1;
+}
+
+int testCXX() {
+  int a; // uninitialized
+  // FIXME: this should be reported as a leak, because C++ exceptions are
+  // often not fatal.
+  void *mem = malloc(4);
+  inlinedCXX();
+  free(mem);
+  return a; // no-warning
+}
diff --git a/clang/test/Analysis/misc-ps-region-store.cpp b/clang/test/Analysis/misc-ps-region-store.cpp
index e30cedb9118..164bffc70f7 100644
--- a/clang/test/Analysis/misc-ps-region-store.cpp
+++ b/clang/test/Analysis/misc-ps-region-store.cpp
@@ -537,7 +537,8 @@ MyEnum rdar10892489_positive() {
     throw MyEnumValue;
   } catch (MyEnum e) {
     int *p = 0;
-    *p = 0xDEADBEEF; // expected-warning {{null}}
+    // FALSE NEGATIVE
+    *p = 0xDEADBEEF; // {{null}}
     return e;
   }
   return MyEnumValue;
@@ -562,7 +563,8 @@ void PR11545_positive() {
   catch (...)
   {
     int *p = 0;
-    *p = 0xDEADBEEF; // expected-warning {{null}}
+    // FALSE NEGATIVE
+    *p = 0xDEADBEEF; // {{null}}
   }
 }