[analyzer] fix regression in analyzer of NOT actually aborting on Stmts it doesn...

as aborted, but didn't treat such cases as sinks in the ExplodedGraph.

Along the way, add basic support for CXXCatchStmt, expanding the set of code we actually analyze (hopefully correctly).

Fixes: <rdar://problem/10892489>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@152468 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 5714a7edde08b566a4e2a8e7ff1f256060c18091..e7d6cd6d2c9297ea685c8faaadf32ad6d3e55329 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -27,6 +27,7 @@
 namespace clang {
 
 class AnalysisDeclContextManager;
+class CXXCatchStmt;
 class CXXConstructExpr;
 class CXXDeleteExpr;
 class CXXNewExpr;
@@ -339,6 +340,9 @@ public:
   void VisitIncrementDecrementOperator(const UnaryOperator* U,
                                        ExplodedNode *Pred,
                                        ExplodedNodeSet &Dst);
+  
+  void VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred,
+                         ExplodedNodeSet &Dst);
 
   void VisitCXXThisExpr(const CXXThisExpr *TE, ExplodedNode *Pred, 
                         ExplodedNodeSet & Dst);

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 6d1da6e445d003b2acce31895e56f3b3408181ee..4ad36f9dbb1dfa065d421e3914ddd1bee588e6bc 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -145,16 +145,16 @@ public:
   // Forwarding methods to SymbolManager.
 
   const SymbolConjured* getConjuredSymbol(const Stmt *stmt,
-                                         const LocationContext *LCtx,
-                                         QualType type,
+                                          const LocationContext *LCtx,
+                                          QualType type,
                                           unsigned visitCount,
                                           const void *symbolTag = 0) {
     return SymMgr.getConjuredSymbol(stmt, LCtx, type, visitCount, symbolTag);
   }
 
   const SymbolConjured* getConjuredSymbol(const Expr *expr,
-                                         const LocationContext *LCtx,
-                                         unsigned visitCount,
+                                          const LocationContext *LCtx,
+                                          unsigned visitCount,
                                           const void *symbolTag = 0) {
     return SymMgr.getConjuredSymbol(expr, LCtx, visitCount, symbolTag);
   }
@@ -173,13 +173,18 @@ public:
   /// conjured symbols should be used sparingly.
   DefinedOrUnknownSVal getConjuredSymbolVal(const void *symbolTag,
                                             const Expr *expr,
-                                           const LocationContext *LCtx,
-                                           unsigned count);
+                                            const LocationContext *LCtx,
+                                            unsigned count);
   DefinedOrUnknownSVal getConjuredSymbolVal(const void *symbolTag,
                                             const Expr *expr,
-                                           const LocationContext *LCtx,
-                                           QualType type,
+                                            const LocationContext *LCtx,
+                                            QualType type,
                                             unsigned count);
+  
+  DefinedOrUnknownSVal getConjuredSymbolVal(const Stmt *stmt,
+                                            const LocationContext *LCtx,
+                                            QualType type,
+                                            unsigned visitCount);
 
   DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(
       SymbolRef parentSymbol, const TypedValueRegion *region);

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index f50cc31e9d8b6ea8b9756416bb5ad28f92b89ed4..e19381dcd81d060d75ff68ea83acd1c35eeea005 100644 (file)
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -2620,9 +2620,18 @@ CFGBlock *CFGBuilder::VisitCXXCatchStmt(CXXCatchStmt *CS) {
   CFGBlock *CatchBlock = Block;
   if (!CatchBlock)
     CatchBlock = createBlock();
-
+  
+  // CXXCatchStmt is more than just a label.  They have semantic meaning
+  // as well, as they implicitly "initialize" the catch variable.  Add
+  // it to the CFG as a CFGElement so that the control-flow of these
+  // semantics gets captured.
+  appendStmt(CatchBlock, CS);
+
+  // Also add the CXXCatchStmt as a label, to mirror handling of regular
+  // labels.
   CatchBlock->setLabel(CS);
 
+  // Bail out if the CFG is bad.
   if (badCFG)
     return 0;
 

diff --git a/lib/StaticAnalyzer/Core/CoreEngine.cpp b/lib/StaticAnalyzer/Core/CoreEngine.cpp
index a350757a0f1854e57d77dcdc9892875fd5130f90..326ecbfbfbcb78252a6f4b4106ae281c6f7daa6e 100644 (file)
--- a/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -335,6 +335,19 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) {
         HandleBranch(cast<ChooseExpr>(Term)->getCond(), Term, B, Pred);
         return;
 
+      case Stmt::CXXTryStmtClass: {
+        // Generate a node for each of the successors.
+        // Our logic for EH analysis can certainly be improved.
+        for (CFGBlock::const_succ_iterator it = B->succ_begin(),
+             et = B->succ_end(); it != et; ++it) {
+          if (const CFGBlock *succ = *it) {
+            generateNode(BlockEdge(B, succ, Pred->getLocationContext()),
+                         Pred->State, Pred);
+          }
+        }
+        return;
+      }
+        
       case Stmt::DoStmtClass:
         HandleBranch(cast<DoStmt>(Term)->getCond(), Term, B, Pred);
         return;

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 63027175cad400b7b51370747f9475378e1b7a19..d9f983944413931033278b4072363ebf51947b43 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -23,6 +23,7 @@
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtObjC.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/SourceManager.h"
@@ -481,10 +482,8 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
   switch (S->getStmtClass()) {
     // C++ and ARC stuff we don't support yet.
     case Expr::ObjCIndirectCopyRestoreExprClass:
-    case Stmt::CXXCatchStmtClass:
     case Stmt::CXXDependentScopeMemberExprClass:
     case Stmt::CXXPseudoDestructorExprClass:
-    case Stmt::CXXThrowExprClass:
     case Stmt::CXXTryStmtClass:
     case Stmt::CXXTypeidExprClass:
     case Stmt::CXXUuidofExprClass:
@@ -505,7 +504,8 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::SEHExceptStmtClass:
     case Stmt::LambdaExprClass:
     case Stmt::SEHFinallyStmtClass: {
-      const ExplodedNode *node = Bldr.generateNode(S, Pred, Pred->getState());
+      const ExplodedNode *node = Bldr.generateNode(S, Pred, Pred->getState(),
+                                                   /* sink */ true);
       Engine.addAbortedBlock(node, currentBuilderContext->getBlock());
       break;
     }
@@ -600,8 +600,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::OpaqueValueExprClass:
     case Stmt::AsTypeExprClass:
     case Stmt::AtomicExprClass:
-        // Fall through.
+      // 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:
@@ -720,6 +725,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       Bldr.addNodes(Dst);
       break;
     }
+    
+    case Stmt::CXXCatchStmtClass: {
+      Bldr.takeNodes(Pred);
+      VisitCXXCatchStmt(cast<CXXCatchStmt>(S), Pred, Dst);
+      Bldr.addNodes(Dst);
+      break;
+    }
 
     case Stmt::CXXTemporaryObjectExprClass:
     case Stmt::CXXConstructExprClass: {

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index fa0245145cbbb9ed58bed29c9ce6d990b29b5147..72ab48ec00838b99da0e089c41902955da41c7d0 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -16,6 +16,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/StmtCXX.h"
 
 using namespace clang;
 using namespace ento;
@@ -340,6 +341,20 @@ void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE,
   Bldr.generateNode(CDE, Pred, state);
 }
 
+void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS,
+                                   ExplodedNode *Pred,
+                                   ExplodedNodeSet &Dst) {
+  const VarDecl *VD = CS->getExceptionDecl();
+  const LocationContext *LCtx = Pred->getLocationContext();
+  SVal V = svalBuilder.getConjuredSymbolVal(CS, LCtx, VD->getType(),
+                                 currentBuilderContext->getCurrentBlockCount());
+  ProgramStateRef state = Pred->getState();
+  state = state->bindLoc(state->getLValue(VD, LCtx), V);
+
+  StmtNodeBuilder Bldr(Pred, Dst, *currentBuilderContext);
+  Bldr.generateNode(CS, Pred, state);
+}
+
 void ExprEngine::VisitCXXThisExpr(const CXXThisExpr *TE, ExplodedNode *Pred,
                                     ExplodedNodeSet &Dst) {
   StmtNodeBuilder Bldr(Pred, Dst, *currentBuilderContext);

diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 6f5eb375f480f8f95d1b9b5916d536c9299933b2..9e97f5e7d12d4dc952d3666feacf9c684dc660f7 100644 (file)
--- a/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -106,19 +106,21 @@ SValBuilder::getRegionValueSymbolVal(const TypedValueRegion* region) {
   return nonloc::SymbolVal(sym);
 }
 
-DefinedOrUnknownSVal SValBuilder::getConjuredSymbolVal(const void *symbolTag,
-                                                       const Expr *expr,
-                                                      const LocationContext *LCtx,
-                                                       unsigned count) {
+DefinedOrUnknownSVal
+SValBuilder::getConjuredSymbolVal(const void *symbolTag,
+                                  const Expr *expr,
+                                  const LocationContext *LCtx,
+                                  unsigned count) {
   QualType T = expr->getType();
   return getConjuredSymbolVal(symbolTag, expr, LCtx, T, count);
 }
 
-DefinedOrUnknownSVal SValBuilder::getConjuredSymbolVal(const void *symbolTag,
-                                                       const Expr *expr,
-                                                      const LocationContext *LCtx,
-                                                       QualType type,
-                                                       unsigned count) {
+DefinedOrUnknownSVal
+SValBuilder::getConjuredSymbolVal(const void *symbolTag,
+                                  const Expr *expr,
+                                  const LocationContext *LCtx,
+                                  QualType type,
+                                  unsigned count) {
   if (!SymbolManager::canSymbolicate(type))
     return UnknownVal();
 
@@ -130,6 +132,23 @@ DefinedOrUnknownSVal SValBuilder::getConjuredSymbolVal(const void *symbolTag,
   return nonloc::SymbolVal(sym);
 }
 
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredSymbolVal(const Stmt *stmt,
+                                  const LocationContext *LCtx,
+                                  QualType type,
+                                  unsigned visitCount) {
+  if (!SymbolManager::canSymbolicate(type))
+    return UnknownVal();
+
+  SymbolRef sym = SymMgr.getConjuredSymbol(stmt, LCtx, type, visitCount);
+  
+  if (Loc::isLocType(type))
+    return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
+  
+  return nonloc::SymbolVal(sym);
+}
+
 DefinedSVal SValBuilder::getMetadataSymbolVal(const void *symbolTag,
                                               const MemRegion *region,
                                               const Expr *expr, QualType type,

diff --git a/test/Analysis/auto-obj-dtors-cfg-output.cpp b/test/Analysis/auto-obj-dtors-cfg-output.cpp
index 2402e0072e54f506e41577fed746bc113ae6aa7e..20804dbfaf808d9930b669d90948940e5120099b 100644 (file)
--- a/test/Analysis/auto-obj-dtors-cfg-output.cpp
+++ b/test/Analysis/auto-obj-dtors-cfg-output.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -cfg-add-implicit-dtors %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -cfg-add-implicit-dtors %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
 // XPASS: *
 
 class A {
@@ -323,6 +324,15 @@ void test_catch_copy() {
 // CHECK:    Succs (2): B3 B2
 // CHECK:  [B0 (EXIT)]
 // CHECK:    Preds (1): B1
+// CHECK:  [B2 (ENTRY)]
+// CHECK:    Succs (1): B1
+// CHECK:  [B1]
+// CHECK:    1: 1
+// CHECK:    2: return [B1.1];
+// CHECK:    Preds (1): B2
+// CHECK:    Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK:    Preds (1): B1
 // CHECK:  [B9 (ENTRY)]
 // CHECK:    Succs (1): B8
 // CHECK:  [B1]
@@ -824,6 +834,8 @@ void test_catch_copy() {
 // CHECK:    Succs (2): B2 B0
 // CHECK:  [B2]
 // CHECK:   catch (const A &e):
+// CHECK:    1: catch (const A &e) {
+// CHECK: }
 // CHECK:    Preds (1): B1
 // CHECK:    Succs (1): B0
 // CHECK:  [B0 (EXIT)]
@@ -835,10 +847,10 @@ void test_catch_copy() {
 // CHECK:    Succs (2): B2 B0
 // CHECK:  [B2]
 // CHECK:   catch (A e):
-// CHECK:    1: .~A() (Implicit destructor)
+// CHECK:    1: catch (A e) {
+// CHECK: }
+// CHECK:    2: [B2.1].~A() (Implicit destructor)
 // CHECK:    Preds (1): B1
 // CHECK:    Succs (1): B0
 // CHECK:  [B0 (EXIT)]
 // CHECK:    Preds (3): B2 B1 B3
-
-

diff --git a/test/Analysis/misc-ps-region-store.cpp b/test/Analysis/misc-ps-region-store.cpp
index b00a8f2d6eeecbdabdc6189a23e0ac27a0929f7a..9fa0b860f2a52d9102dd0656eae1dd0a736b1408 100644 (file)
--- a/test/Analysis/misc-ps-region-store.cpp
+++ b/test/Analysis/misc-ps-region-store.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,experimental.core -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,experimental.core -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,experimental.core -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks %s -fexceptions -fcxx-exceptions
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,experimental.core -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks %s -fexceptions -fcxx-exceptions
 
 // Test basic handling of references.
 char &test1_aux();
@@ -506,3 +506,26 @@ void TestNullThis::test() {
   field = 2; // no-warning
 }
 
+// Test handling of 'catch' exception variables, and not warning
+// about uninitialized values.
+enum MyEnum { MyEnumValue };
+MyEnum rdar10892489() {
+  try {
+      throw MyEnumValue;
+  } catch (MyEnum e) {
+      return e; // no-warning
+  }
+  return MyEnumValue;
+}
+
+MyEnum rdar10892489_positive() {
+  try {
+    throw MyEnumValue;
+  } catch (MyEnum e) {
+    int *p = 0;
+    *p = 0xDEADBEEF; // expected-warning {{null}}
+    return e;
+  }
+  return MyEnumValue;
+}
+