[analyzer] Combine all ObjC message CallEvents into ObjCMethodCall.

As pointed out by Anna, we only differentiate between explicit message sends

This also adds support for ObjCSubscriptExprs, which are basically the same
as properties in many ways. We were already checking these, but not emitting
nice messages for them.

This depends on the llvm::PointerIntPair change in r160456.

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

3 files changed, 83 insertions, 62 deletions

diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 69b331c16c..17ed692a6c 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -34,6 +34,7 @@ class CallAndMessageChecker
   mutable OwningPtr<BugType> BT_call_arg;
   mutable OwningPtr<BugType> BT_msg_undef;
   mutable OwningPtr<BugType> BT_objc_prop_undef;
+  mutable OwningPtr<BugType> BT_objc_subscript_undef;
   mutable OwningPtr<BugType> BT_msg_arg;
   mutable OwningPtr<BugType> BT_msg_ret;
 public:
@@ -45,8 +46,8 @@ public:
 private:
   static bool PreVisitProcessArg(CheckerContext &C, SVal V,
                                  SourceRange argRange, const Expr *argEx,
-                                 const bool checkUninitFields,
-                                 const char *BT_desc, OwningPtr<BugType> &BT);
+                                 bool IsFirstArgument, bool checkUninitFields,
+                                 const CallEvent &Call, OwningPtr<BugType> &BT);
 
   static void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE);
   void emitNilReceiverBug(CheckerContext &C, const ObjCMethodCall &msg,
@@ -75,18 +76,46 @@ void CallAndMessageChecker::EmitBadCall(BugType *BT, CheckerContext &C,
   C.EmitReport(R);
 }
 
+StringRef describeUninitializedArgumentInCall(const CallEvent &Call,
+                                              bool IsFirstArgument) {
+  switch (Call.getKind()) {
+  case CE_ObjCMessage: {
+    const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call);
+    switch (Msg.getMessageKind()) {
+    case OCM_Message:
+      return "Argument in message expression is an uninitialized value";
+    case OCM_PropertyAccess:
+      assert(Msg.isSetter() && "Getters have no args");
+      return "Argument for property setter is an uninitialized value";
+    case OCM_Subscript:
+      if (Msg.isSetter() && IsFirstArgument)
+        return "Argument for subscript setter is an uninitialized value";
+      return "Subscript index is an uninitialized value";
+    }
+    llvm_unreachable("Unknown message kind.");
+  }
+  case CE_Block:
+    return "Block call argument is an uninitialized value";
+  default:
+    return "Function call argument is an uninitialized value";
+  }
+}
+
 bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
                                                SVal V, SourceRange argRange,
                                                const Expr *argEx,
-                                               const bool checkUninitFields,
-                                               const char *BT_desc,
+                                               bool IsFirstArgument,
+                                               bool checkUninitFields,
+                                               const CallEvent &Call,
                                                OwningPtr<BugType> &BT) {
   if (V.isUndef()) {
     if (ExplodedNode *N = C.generateSink()) {
       LazyInit_BT("Uninitialized argument value", BT);
 
       // Generate a report for this bug.
-      BugReport *R = new BugReport(*BT, BT_desc, N);
+      StringRef Desc = describeUninitializedArgumentInCall(Call,
+                                                           IsFirstArgument);
+      BugReport *R = new BugReport(*BT, Desc, N);
       R->addRange(argRange);
       if (argEx)
         R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, argEx,
@@ -148,7 +177,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
 
     if (F.Find(D->getRegion())) {
       if (ExplodedNode *N = C.generateSink()) {
-        LazyInit_BT(BT_desc, BT);
+        LazyInit_BT("Uninitialized argument value", BT);
         SmallString<512> Str;
         llvm::raw_svector_ostream os(Str);
         os << "Passed-by-value struct argument contains uninitialized data";
@@ -220,32 +249,15 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
       (D && D->getBody()));
 
   OwningPtr<BugType> *BT;
-  const char *Desc;
-
-  switch (Call.getKind()) {
-  case CE_ObjCPropertyAccess:
-    BT = &BT_msg_arg;
-    // Getters do not have arguments, so we don't need to worry about this.
-    Desc = "Argument for property setter is an uninitialized value";
-    break;
-  case CE_ObjCMessage:
+  if (isa<ObjCMethodCall>(Call))
     BT = &BT_msg_arg;
-    Desc = "Argument in message expression is an uninitialized value";
-    break;
-  case CE_Block:
+  else
     BT = &BT_call_arg;
-    Desc = "Block call argument is an uninitialized value";
-    break;
-  default:
-    BT = &BT_call_arg;
-    Desc = "Function call argument is an uninitialized value";
-    break;
-  }
 
   for (unsigned i = 0, e = Call.getNumArgs(); i != e; ++i)
-    if (PreVisitProcessArg(C, Call.getArgSVal(i),
-                           Call.getArgSourceRange(i), Call.getArgExpr(i),
-                           checkUninitFields, Desc, *BT))
+    if (PreVisitProcessArg(C, Call.getArgSVal(i), Call.getArgSourceRange(i),
+                           Call.getArgExpr(i), /*IsFirstArgument=*/i == 0,
+                           checkUninitFields, Call, *BT))
       return;
 }
 
@@ -255,22 +267,36 @@ void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
   if (recVal.isUndef()) {
     if (ExplodedNode *N = C.generateSink()) {
       BugType *BT = 0;
-      if (isa<ObjCPropertyAccess>(msg)) {
-        if (!BT_objc_prop_undef)
-          BT_objc_prop_undef.reset(new BuiltinBug("Property access on an "
-                                                  "uninitialized object pointer"));
-        BT = BT_objc_prop_undef.get();
-      } else {
+      switch (msg.getMessageKind()) {
+      case OCM_Message:
         if (!BT_msg_undef)
           BT_msg_undef.reset(new BuiltinBug("Receiver in message expression "
                                             "is an uninitialized value"));
         BT = BT_msg_undef.get();
+        break;
+      case OCM_PropertyAccess:
+        if (!BT_objc_prop_undef)
+          BT_objc_prop_undef.reset(new BuiltinBug("Property access on an "
+                                                  "uninitialized object "
+                                                  "pointer"));
+        BT = BT_objc_prop_undef.get();
+        break;
+      case OCM_Subscript:
+        if (!BT_objc_subscript_undef)
+          BT_objc_subscript_undef.reset(new BuiltinBug("Subscript access on an "
+                                                       "uninitialized object "
+                                                       "pointer"));
+        BT = BT_objc_subscript_undef.get();
+        break;
       }
+      assert(BT && "Unknown message kind.");
+
       BugReport *R = new BugReport(*BT, BT->getName(), N);
-      R->addRange(msg.getReceiverSourceRange());
+      const ObjCMessageExpr *ME = msg.getOriginExpr();
+      R->addRange(ME->getReceiverRange());
 
       // FIXME: getTrackNullOrUndefValueVisitor can't handle "super" yet.
-      if (const Expr *ReceiverE = msg.getInstanceReceiverExpr())
+      if (const Expr *ReceiverE = ME->getInstanceReceiver())
         R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
                                                                    ReceiverE,
                                                                    R));
@@ -302,17 +328,19 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C,
       new BuiltinBug("Receiver in message expression is "
                      "'nil' and returns a garbage value"));
 
+  const ObjCMessageExpr *ME = msg.getOriginExpr();
+
   SmallString<200> buf;
   llvm::raw_svector_ostream os(buf);
-  os << "The receiver of message '" << msg.getSelector().getAsString()
+  os << "The receiver of message '" << ME->getSelector().getAsString()
      << "' is nil and returns a value of type '";
   msg.getResultType().print(os, C.getLangOpts());
   os << "' that will be garbage";
 
   BugReport *report = new BugReport(*BT_msg_ret, os.str(), N);
-  report->addRange(msg.getReceiverSourceRange());
+  report->addRange(ME->getReceiverRange());
   // FIXME: This won't track "self" in messages to super.
-  if (const Expr *receiver = msg.getInstanceReceiverExpr()) {
+  if (const Expr *receiver = ME->getInstanceReceiver()) {
     report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
                                                                     receiver,
                                                                     report));
diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index 842cbc5f59..0e9efaa5ad 100644
--- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
@@ -81,8 +81,7 @@ public:
   ///
   /// This will be called before the analyzer core processes the method call.
   /// This is called for any action which produces an Objective-C message send,
-  /// including explicit message syntax and property access. See the subclasses
-  /// of ObjCMethodCall for more details.
+  /// including explicit message syntax and property access.
   ///
   /// check::PreObjCMessage
   void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const {}
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index c44fc67d26..aca5f798d4 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -952,8 +952,7 @@ RetainSummaryManager::getSummary(const CallEvent &Call,
   case CE_CXXAllocator:
     // FIXME: These calls are currently unsupported.
     return getPersistentStopSummary();
-  case CE_ObjCMessage:
-  case CE_ObjCPropertyAccess: {
+  case CE_ObjCMessage: {
     const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call);
     if (Msg.isInstanceMessage())
       Summ = getInstanceMethodSummary(Msg, State);
@@ -1911,20 +1910,6 @@ static bool isNumericLiteralExpression(const Expr *E) {
          isa<CXXBoolLiteralExpr>(E);
 }
 
-static bool isPropertyAccess(const Stmt *S, ParentMap &PM) {
-  unsigned maxDepth = 4;
-  while (S && maxDepth) {
-    if (const PseudoObjectExpr *PO = dyn_cast<PseudoObjectExpr>(S)) {
-      if (!isa<ObjCMessageExpr>(PO->getSyntacticForm()))
-        return true;
-      return false;
-    }
-    S = PM.getParent(S);
-    --maxDepth;
-  }
-  return false;
-}
-
 PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N,
                                                    const ExplodedNode *PrevN,
                                                    BugReporterContext &BRC,
@@ -1989,10 +1974,19 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N,
           os << "function call";
       }
       else {
-        assert(isa<ObjCMessageExpr>(S));      
-        // The message expression may have between written directly or as
-        // a property access.  Lazily determine which case we are looking at.
-        os << (isPropertyAccess(S, N->getParentMap()) ? "Property" : "Method");
+        assert(isa<ObjCMessageExpr>(S));
+        ObjCMethodCall Call(cast<ObjCMessageExpr>(S), CurrSt, LCtx);
+        switch (Call.getMessageKind()) {
+        case OCM_Message:
+          os << "Method";
+          break;
+        case OCM_PropertyAccess:
+          os << "Property";
+          break;
+        case OCM_Subscript:
+          os << "Subscript";
+          break;
+        }
       }
 
       if (CurrV.getObjKind() == RetEffect::CF) {
@@ -2824,7 +2818,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
           state = updateSymbol(state, Sym, *T, Summ.getReceiverEffect(),
                                hasErr, C);
           if (hasErr) {
-            ErrorRange = MsgInvocation->getReceiverSourceRange();
+            ErrorRange = MsgInvocation->getOriginExpr()->getReceiverRange();
             ErrorSym = Sym;
           }
         }