Refactor NilReceiverStructRet and NilReceiverLargerThanVoidPtrRet into

CallAndMessageChecker.


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

6 files changed, 138 insertions, 219 deletions

diff --git a/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h
index e94096ba4f..f6a5074fd8 100644
--- a/include/clang/Analysis/PathSensitive/Checker.h
+++ b/include/clang/Analysis/PathSensitive/Checker.h
@@ -81,6 +81,10 @@ public:
     return getBugReporter().getSourceManager();
   }
 
+  ValueManager &getValueManager() {
+    return Eng.getValueManager();
+  }
+
   ExplodedNode *GenerateNode(bool autoTransition = true) {
     assert(statement && "Only transitions with statements currently supported");
     ExplodedNode *N = GenerateNodeImpl(statement, getState(), false);
diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index cca548ea47..1512b9099f 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -90,38 +90,6 @@ class GRExprEngine : public GRSubEngine {
 public:
   typedef llvm::SmallPtrSet<ExplodedNode*,2> ErrorNodes;
 
-  /// NilReceiverStructRetExplicit - Nodes in the ExplodedGraph that resulted
-  ///  from [x ...] with 'x' definitely being nil and the result was a 'struct'
-  //  (an undefined value).
-  ErrorNodes NilReceiverStructRetExplicit;
-
-  /// NilReceiverStructRetImplicit - Nodes in the ExplodedGraph that resulted
-  ///  from [x ...] with 'x' possibly being nil and the result was a 'struct'
-  //  (an undefined value).
-  ErrorNodes NilReceiverStructRetImplicit;
-
-  /// NilReceiverLargerThanVoidPtrRetExplicit - Nodes in the ExplodedGraph that
-  /// resulted from [x ...] with 'x' definitely being nil and the result's size
-  // was larger than sizeof(void *) (an undefined value).
-  ErrorNodes NilReceiverLargerThanVoidPtrRetExplicit;
-
-  /// NilReceiverLargerThanVoidPtrRetImplicit - Nodes in the ExplodedGraph that
-  /// resulted from [x ...] with 'x' possibly being nil and the result's size
-  // was larger than sizeof(void *) (an undefined value).
-  ErrorNodes NilReceiverLargerThanVoidPtrRetImplicit;
-
-  /// UndefBranches - Nodes in the ExplodedGraph that result from
-  ///  taking a branch based on an undefined value.
-  ErrorNodes UndefBranches;
-
-  /// UndefStores - Sinks in the ExplodedGraph that result from
-  ///  making a store to an undefined lvalue.
-  ErrorNodes UndefStores;
-
-  /// NoReturnCalls - Sinks in the ExplodedGraph that result from
-  //  calling a function with the attribute "noreturn".
-  ErrorNodes NoReturnCalls;
-
   /// UndefResults - Nodes in the ExplodedGraph where the operands are defined
   ///  by the result is not.  Excludes divide-by-zero errors.
   ErrorNodes UndefResults;
@@ -185,36 +153,6 @@ public:
      return static_cast<CHECKER*>(lookupChecker(CHECKER::getTag()));
   }
 
-  bool isNoReturnCall(const ExplodedNode* N) const {
-    return N->isSink() && NoReturnCalls.count(const_cast<ExplodedNode*>(N)) != 0;
-  }
-
-  typedef ErrorNodes::iterator undef_branch_iterator;
-  undef_branch_iterator undef_branches_begin() { return UndefBranches.begin(); }
-  undef_branch_iterator undef_branches_end() { return UndefBranches.end(); }
-
-  typedef ErrorNodes::iterator nil_receiver_struct_ret_iterator;
-
-  nil_receiver_struct_ret_iterator nil_receiver_struct_ret_begin() {
-    return NilReceiverStructRetExplicit.begin();
-  }
-
-  nil_receiver_struct_ret_iterator nil_receiver_struct_ret_end() {
-    return NilReceiverStructRetExplicit.end();
-  }
-
-  typedef ErrorNodes::iterator nil_receiver_larger_than_voidptr_ret_iterator;
-
-  nil_receiver_larger_than_voidptr_ret_iterator
-  nil_receiver_larger_than_voidptr_ret_begin() {
-    return NilReceiverLargerThanVoidPtrRetExplicit.begin();
-  }
-
-  nil_receiver_larger_than_voidptr_ret_iterator
-  nil_receiver_larger_than_voidptr_ret_end() {
-    return NilReceiverLargerThanVoidPtrRetExplicit.end();
-  }
-
   typedef ErrorNodes::iterator undef_result_iterator;
   undef_result_iterator undef_results_begin() { return UndefResults.begin(); }
   undef_result_iterator undef_results_end() { return UndefResults.end(); }
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 55e5f174cb..433e350acc 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -3066,6 +3066,16 @@ void CFRefCount::EvalObjCMessageExpr(ExplodedNodeSet& Dst,
                                      GRStmtNodeBuilder& Builder,
                                      ObjCMessageExpr* ME,
                                      ExplodedNode* Pred) {
+  // FIXME: Since we moved the nil check into a checker, we could get nil
+  // receiver here. Need a better way to check such case. 
+  if (Expr* Receiver = ME->getReceiver()) {
+    const GRState *state = Pred->getState();
+    DefinedOrUnknownSVal L=cast<DefinedOrUnknownSVal>(state->getSVal(Receiver));
+    if (!state->Assume(L, true)) {
+      Dst.Add(Pred);
+      return;
+    }
+  }
   
   RetainSummary *Summ =
     ME->getReceiver()
diff --git a/lib/Analysis/CallAndMessageChecker.cpp b/lib/Analysis/CallAndMessageChecker.cpp
index 8386d03e23..ebc3a4fd56 100644
--- a/lib/Analysis/CallAndMessageChecker.cpp
+++ b/lib/Analysis/CallAndMessageChecker.cpp
@@ -14,6 +14,7 @@
 
 #include "clang/Analysis/PathSensitive/CheckerVisitor.h"
 #include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/AST/ParentMap.h"
 #include "GRExprEngineInternalChecks.h"
 
 using namespace clang;
@@ -26,10 +27,13 @@ class VISIBILITY_HIDDEN CallAndMessageChecker
   BugType *BT_call_arg;
   BugType *BT_msg_undef;
   BugType *BT_msg_arg;
+  BugType *BT_struct_ret;
+  BugType *BT_void_ptr;
 public:
   CallAndMessageChecker() :
     BT_call_null(0), BT_call_undef(0), BT_call_arg(0),
-    BT_msg_undef(0), BT_msg_arg(0) {}
+    BT_msg_undef(0), BT_msg_arg(0), BT_struct_ret(0), BT_void_ptr(0) {}
+
   static void *getTag() {
     static int x = 0;
     return &x;
@@ -119,8 +123,8 @@ void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C,
     }
 
   // Check for any arguments that are uninitialized/undefined.
-  for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(), E = ME->arg_end();
-       I != E; ++I) {
+  for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(),
+         E = ME->arg_end(); I != E; ++I) {
     if (state->getSVal(*I).isUndef()) {
       if (ExplodedNode *N = C.GenerateSink()) {
         if (!BT_msg_arg)
@@ -137,4 +141,112 @@ void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C,
       }
     }
   }
+
+  // Check if the receiver was nil and then return value a struct.
+  if (const Expr *Receiver = ME->getReceiver()) {
+    SVal L_untested = state->getSVal(Receiver);
+    // Assume that the receiver is not NULL.
+    DefinedOrUnknownSVal L = cast<DefinedOrUnknownSVal>(L_untested);
+    const GRState *StNotNull = state->Assume(L, true);
+
+    // Assume that the receiver is NULL.
+    const GRState *StNull = state->Assume(L, false);
+
+    if (StNull) {
+      QualType RetTy = ME->getType();
+      if (RetTy->isRecordType()) {
+        if (C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
+          // The [0 ...] expressions will return garbage.  Flag either an
+          // explicit or implicit error.  Because of the structure of this
+          // function we currently do not bifurfacte the state graph at
+          // this point.
+          // FIXME: We should bifurcate and fill the returned struct with
+          //  garbage.
+          if (ExplodedNode* N = C.GenerateSink(StNull)) {
+            if (!StNotNull) {
+              if (!BT_struct_ret) {
+                std::string sbuf;
+                llvm::raw_string_ostream os(sbuf);
+                os << "The receiver in the message expression is 'nil' and "
+                  "results in the returned value (of type '"
+                   << ME->getType().getAsString()
+                   << "') to be garbage or otherwise undefined";
+                BT_struct_ret = new BuiltinBug(os.str().c_str());
+              }
+              
+              EnhancedBugReport *R = new EnhancedBugReport(*BT_struct_ret, 
+                                                   BT_struct_ret->getName(), N);
+              R->addRange(Receiver->getSourceRange());
+              R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, 
+                                   Receiver);
+              C.EmitReport(R);
+              return;
+            }
+            else
+              // Do not report implicit bug.
+              return;
+          }
+        }
+      } else {
+        ASTContext &Ctx = C.getASTContext();
+        if (RetTy != Ctx.VoidTy) {
+          if (C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
+            // sizeof(void *)
+            const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
+            // sizeof(return type)
+            const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
+            
+            if (voidPtrSize < returnTypeSize) {
+              if (ExplodedNode* N = C.GenerateSink(StNull)) {
+                if (!StNotNull) {
+                  if (!BT_struct_ret) {
+                    std::string sbuf;
+                    llvm::raw_string_ostream os(sbuf);
+                    os << "The receiver in the message expression is 'nil' and "
+                      "results in the returned value (of type '"
+                       << ME->getType().getAsString()
+                       << "' and of size "
+                       << returnTypeSize / 8
+                       << " bytes) to be garbage or otherwise undefined";
+                    BT_void_ptr = new BuiltinBug(os.str().c_str());
+                  }
+              
+                  EnhancedBugReport *R = new EnhancedBugReport(*BT_void_ptr, 
+                                                     BT_void_ptr->getName(), N);
+                  R->addRange(Receiver->getSourceRange());
+                  R->addVisitorCreator(
+                          bugreporter::registerTrackNullOrUndefValue, Receiver);
+                  C.EmitReport(R);
+                  return;
+                } else
+                  // Do not report implicit bug.
+                  return;
+              }
+            }
+            else if (!StNotNull) {
+              // Handle the safe cases where the return value is 0 if the
+              // receiver is nil.
+              //
+              // FIXME: For now take the conservative approach that we only
+              // return null values if we *know* that the receiver is nil.
+              // This is because we can have surprises like:
+              //
+              //   ... = [[NSScreens screens] objectAtIndex:0];
+              //
+              // What can happen is that [... screens] could return nil, but
+              // it most likely isn't nil.  We should assume the semantics
+              // of this case unless we have *a lot* more knowledge.
+              //
+              SVal V = C.getValueManager().makeZeroVal(ME->getType());
+              C.GenerateNode(StNull->BindExpr(ME, V));
+              return;
+            }
+          }
+        }
+      }
+    }
+    // Do not propagate null state.
+    if (StNotNull)
+      C.GenerateNode(StNotNull);
+  }
 }
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index f657aebdf6..2f7ea944e7 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -860,8 +860,9 @@ void GRExprEngine::ProcessIndirectGoto(GRIndirectGotoNodeBuilder& builder) {
 
   if (isa<loc::ConcreteInt>(V) || isa<UndefinedVal>(V)) {
     // Dispatch to the first target and mark it as a sink.
-    ExplodedNode* N = builder.generateNode(builder.begin(), state, true);
-    UndefBranches.insert(N);
+    //ExplodedNode* N = builder.generateNode(builder.begin(), state, true);
+    // FIXME: add checker visit.
+    //    UndefBranches.insert(N);
     return;
   }
 
@@ -912,8 +913,10 @@ void GRExprEngine::ProcessSwitch(GRSwitchNodeBuilder& builder) {
   SVal  CondV_untested = state->getSVal(CondE);
 
   if (CondV_untested.isUndef()) {
-    ExplodedNode* N = builder.generateDefaultCaseNode(state, true);
-    UndefBranches.insert(N);
+    //ExplodedNode* N = builder.generateDefaultCaseNode(state, true);
+    // FIXME: add checker 
+    //UndefBranches.insert(N);
+
     return;
   }
   DefinedOrUnknownSVal CondV = cast<DefinedOrUnknownSVal>(CondV_untested);
@@ -1858,88 +1861,9 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME,
   for (ExplodedNodeSet::iterator DI = DstTmp.begin(), DE = DstTmp.end();
        DI!=DE; ++DI) {    
     Pred = *DI;
-    // FIXME: More logic for the processing the method call.
-    const GRState* state = GetState(Pred);
     bool RaisesException = false;
 
-    if (Expr* Receiver = ME->getReceiver()) {
-      SVal L_untested = state->getSVal(Receiver);
-
-      // "Assume" that the receiver is not NULL.
-      DefinedOrUnknownSVal L = cast<DefinedOrUnknownSVal>(L_untested);
-      const GRState *StNotNull = state->Assume(L, true);
-
-      // "Assume" that the receiver is NULL.
-      const GRState *StNull = state->Assume(L, false);
-
-      if (StNull) {
-        QualType RetTy = ME->getType();
-
-        // Check if the receiver was nil and the return value a struct.
-        if (RetTy->isRecordType()) {
-          if (Pred->getParentMap().isConsumedExpr(ME)) {
-            // The [0 ...] expressions will return garbage.  Flag either an
-            // explicit or implicit error.  Because of the structure of this
-            // function we currently do not bifurfacte the state graph at
-            // this point.
-            // FIXME: We should bifurcate and fill the returned struct with
-            //  garbage.
-            if (ExplodedNode* N = Builder->generateNode(ME, StNull, Pred)) {
-              N->markAsSink();
-              if (StNotNull)
-                NilReceiverStructRetImplicit.insert(N);
-              else
-                NilReceiverStructRetExplicit.insert(N);
-            }
-          }
-        }
-        else {
-          ASTContext& Ctx = getContext();
-          if (RetTy != Ctx.VoidTy) {
-            if (Pred->getParentMap().isConsumedExpr(ME)) {
-              // sizeof(void *)
-              const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
-              // sizeof(return type)
-              const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
-
-              if (voidPtrSize < returnTypeSize) {
-                if (ExplodedNode* N = Builder->generateNode(ME, StNull, Pred)) {
-                  N->markAsSink();
-                  if (StNotNull)
-                    NilReceiverLargerThanVoidPtrRetImplicit.insert(N);
-                  else
-                    NilReceiverLargerThanVoidPtrRetExplicit.insert(N);
-                }
-              }
-              else if (!StNotNull) {
-                // Handle the safe cases where the return value is 0 if the
-                // receiver is nil.
-                //
-                // FIXME: For now take the conservative approach that we only
-                // return null values if we *know* that the receiver is nil.
-                // This is because we can have surprises like:
-                //
-                //   ... = [[NSScreens screens] objectAtIndex:0];
-                //
-                // What can happen is that [... screens] could return nil, but
-                // it most likely isn't nil.  We should assume the semantics
-                // of this case unless we have *a lot* more knowledge.
-                //
-                SVal V = ValMgr.makeZeroVal(ME->getType());
-                MakeNode(Dst, ME, Pred, StNull->BindExpr(ME, V));
-                return;
-              }
-            }
-          }
-        }
-        // We have handled the cases where the receiver is nil.  The remainder
-        // of this method should assume that the receiver is not nil.
-        if (!StNotNull)
-          return;
-
-        state = StNotNull;
-      }
-
+    if (ME->getReceiver()) {
       // Check if the "raise" message was sent.
       if (ME->getSelector() == RaiseSel)
         RaisesException = true;
@@ -2840,11 +2764,10 @@ struct VISIBILITY_HIDDEN DOTGraphTraits<ExplodedNode*> :
         GraphPrintCheckerState->isBadCall(N) ||
         GraphPrintCheckerState->isUndefArg(N))
       return "color=\"red\",style=\"filled\"";
-#endif
 
     if (GraphPrintCheckerState->isNoReturnCall(N))
       return "color=\"blue\",style=\"filled\"";
-
+#endif
     return "";
   }
 
diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp
index 2e698d7d3f..93ade9d208 100644
--- a/lib/Analysis/GRExprEngineInternalChecks.cpp
+++ b/lib/Analysis/GRExprEngineInternalChecks.cpp
@@ -62,72 +62,6 @@ void BuiltinBug::Emit(BugReporter& BR, ITER I, ITER E) {
                                                          GetNode(I)));
 }
 
-class VISIBILITY_HIDDEN NilReceiverStructRet : public BuiltinBug {
-public:
-  NilReceiverStructRet(GRExprEngine* eng) :
-    BuiltinBug(eng, "'nil' receiver with struct return type") {}
-
-  void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
-    for (GRExprEngine::nil_receiver_struct_ret_iterator
-          I=Eng.nil_receiver_struct_ret_begin(),
-          E=Eng.nil_receiver_struct_ret_end(); I!=E; ++I) {
-
-      std::string sbuf;
-      llvm::raw_string_ostream os(sbuf);
-      PostStmt P = cast<PostStmt>((*I)->getLocation());
-      const ObjCMessageExpr *ME = cast<ObjCMessageExpr>(P.getStmt());
-      os << "The receiver in the message expression is 'nil' and results in the"
-            " returned value (of type '"
-         << ME->getType().getAsString()
-         << "') to be garbage or otherwise undefined";
-
-      BuiltinBugReport *R = new BuiltinBugReport(*this, os.str().c_str(), *I);
-      R->addRange(ME->getReceiver()->getSourceRange());
-      BR.EmitReport(R);
-    }
-  }
-
-  void registerInitialVisitors(BugReporterContext& BRC,
-                               const ExplodedNode* N,
-                               BuiltinBugReport *R) {
-    registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N);
-  }
-};
-
-class VISIBILITY_HIDDEN NilReceiverLargerThanVoidPtrRet : public BuiltinBug {
-public:
-  NilReceiverLargerThanVoidPtrRet(GRExprEngine* eng) :
-    BuiltinBug(eng,
-               "'nil' receiver with return type larger than sizeof(void *)") {}
-
-  void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
-    for (GRExprEngine::nil_receiver_larger_than_voidptr_ret_iterator
-         I=Eng.nil_receiver_larger_than_voidptr_ret_begin(),
-         E=Eng.nil_receiver_larger_than_voidptr_ret_end(); I!=E; ++I) {
-
-      std::string sbuf;
-      llvm::raw_string_ostream os(sbuf);
-      PostStmt P = cast<PostStmt>((*I)->getLocation());
-      const ObjCMessageExpr *ME = cast<ObjCMessageExpr>(P.getStmt());
-      os << "The receiver in the message expression is 'nil' and results in the"
-      " returned value (of type '"
-      << ME->getType().getAsString()
-      << "' and of size "
-      << Eng.getContext().getTypeSize(ME->getType()) / 8
-      << " bytes) to be garbage or otherwise undefined";
-
-      BuiltinBugReport *R = new BuiltinBugReport(*this, os.str().c_str(), *I);
-      R->addRange(ME->getReceiver()->getSourceRange());
-      BR.EmitReport(R);
-    }
-  }
-  void registerInitialVisitors(BugReporterContext& BRC,
-                               const ExplodedNode* N,
-                               BuiltinBugReport *R) {
-    registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N);
-  }
-};
-
 class VISIBILITY_HIDDEN UndefResult : public BuiltinBug {
 public:
   UndefResult(GRExprEngine* eng)
@@ -217,8 +151,6 @@ void GRExprEngine::RegisterInternalChecks() {
   // analyzing a function.  Generation of BugReport objects is done via a call
   // to 'FlushReports' from BugReporter.
   BR.Register(new UndefResult(this));
-  BR.Register(new NilReceiverStructRet(this));
-  BR.Register(new NilReceiverLargerThanVoidPtrRet(this));
 
   // The following checks do not need to have their associated BugTypes
   // explicitly registered with the BugReporter.  If they issue any BugReports,