[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.

llvm-svn: 160461

6 files changed, 150 insertions, 98 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 69b331c16cc..17ed692a6c1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/clang/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/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index 842cbc5f595..0e9efaa5ade 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/clang/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/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index c44fc67d265..aca5f798d46 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/clang/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;
           }
         }
diff --git a/clang/lib/StaticAnalyzer/Core/Calls.cpp b/clang/lib/StaticAnalyzer/Core/Calls.cpp
index ba56364d513..5c161ba27b9 100644
--- a/clang/lib/StaticAnalyzer/Core/Calls.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Calls.cpp
@@ -518,7 +518,7 @@ SVal ObjCMethodCall::getReceiverSVal() const {
   if (!isInstanceMessage())
     return UnknownVal();
     
-  if (const Expr *Base = getInstanceReceiverExpr())
+  if (const Expr *Base = getOriginExpr()->getInstanceReceiver())
     return getSVal(Base);
 
   // An instance message with no expression means we are sending to super.
@@ -529,10 +529,67 @@ SVal ObjCMethodCall::getReceiverSVal() const {
   return getState()->getSVal(getState()->getRegion(SelfDecl, LCtx));
 }
 
-SourceRange ObjCPropertyAccess::getSourceRange() const {
-  const ParentMap &PM = getLocationContext()->getParentMap();
-  const ObjCMessageExpr *ME = getOriginExpr();
-  const PseudoObjectExpr *PO = cast<PseudoObjectExpr>(PM.getParent(ME));
-  return PO->getSourceRange();
+SourceRange ObjCMethodCall::getSourceRange() const {
+  switch (getMessageKind()) {
+  case OCM_Message:
+    return getOriginExpr()->getSourceRange();
+  case OCM_PropertyAccess:
+  case OCM_Subscript:
+    return getContainingPseudoObjectExpr()->getSourceRange();
+  }
+}
+
+typedef llvm::PointerIntPair<const PseudoObjectExpr *, 2> ObjCMessageDataTy;
+
+const PseudoObjectExpr *ObjCMethodCall::getContainingPseudoObjectExpr() const {
+  assert(Data != 0 && "Lazy lookup not yet performed.");
+  assert(getMessageKind() != OCM_Message && "Explicit message send.");
+  return ObjCMessageDataTy::getFromOpaqueValue(Data).getPointer();
+}
+
+ObjCMessageKind ObjCMethodCall::getMessageKind() const {
+  if (Data == 0) {
+    ParentMap &PM = getLocationContext()->getParentMap();
+    const Stmt *S = PM.getParent(getOriginExpr());
+    if (const PseudoObjectExpr *POE = dyn_cast_or_null<PseudoObjectExpr>(S)) {
+      const Expr *Syntactic = POE->getSyntacticForm();
+
+      // This handles the funny case of assigning to the result of a getter.
+      // This can happen if the getter returns a non-const reference.
+      if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Syntactic))
+        Syntactic = BO->getLHS();
+
+      ObjCMessageKind K;
+      switch (Syntactic->getStmtClass()) {
+      case Stmt::ObjCPropertyRefExprClass:
+        K = OCM_PropertyAccess;
+        break;
+      case Stmt::ObjCSubscriptRefExprClass:
+        K = OCM_Subscript;
+        break;
+      default:
+        // FIXME: Can this ever happen?
+        K = OCM_Message;
+        break;
+      }
+
+      if (K != OCM_Message) {
+        const_cast<ObjCMethodCall *>(this)->Data
+          = ObjCMessageDataTy(POE, K).getOpaqueValue();
+        assert(getMessageKind() == K);
+        return K;
+      }
+    }
+    
+    const_cast<ObjCMethodCall *>(this)->Data
+      = ObjCMessageDataTy(0, 1).getOpaqueValue();
+    assert(getMessageKind() == OCM_Message);
+    return OCM_Message;
+  }
+
+  ObjCMessageDataTy Info = ObjCMessageDataTy::getFromOpaqueValue(Data);
+  if (!Info.getPointer())
+    return OCM_Message;
+  return static_cast<ObjCMessageKind>(Info.getInt());
 }
 
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index fb99d916040..895e20eca17 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -890,35 +890,10 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
 
     case Stmt::ObjCMessageExprClass: {
       Bldr.takeNodes(Pred);
-      // Is this a property access?
-
-      const LocationContext *LCtx = Pred->getLocationContext();
-      const ParentMap &PM = LCtx->getParentMap();
-      const ObjCMessageExpr *ME = cast<ObjCMessageExpr>(S);
-      bool evaluated = false;
-      
-      if (const PseudoObjectExpr *PO =
-            dyn_cast_or_null<PseudoObjectExpr>(PM.getParent(S))) {
-        const Expr *syntactic = PO->getSyntacticForm();
-
-        // This handles the funny case of assigning to the result of a getter.
-        // This can happen if the getter returns a non-const reference.
-        if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(syntactic))
-          syntactic = BO->getLHS();
-
-        if (const ObjCPropertyRefExpr *PR =
-              dyn_cast<ObjCPropertyRefExpr>(syntactic)) {
-          VisitObjCMessage(ObjCPropertyAccess(PR, PO->getSourceRange(), ME,
-                                              Pred->getState(), LCtx),
-                           Pred, Dst);
-          evaluated = true;
-        }
-      }
-      
-      if (!evaluated)
-        VisitObjCMessage(ObjCMessageSend(ME, Pred->getState(), LCtx),
-                         Pred, Dst);
-
+      VisitObjCMessage(ObjCMethodCall(cast<ObjCMessageExpr>(S),
+                                      Pred->getState(),
+                                      Pred->getLocationContext()),
+                       Pred, Dst);
       Bldr.addNodes(Dst);
       break;
     }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 493ca91b48c..7cedb7ac5e8 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -306,7 +306,6 @@ bool ExprEngine::inlineCall(ExplodedNodeSet &Dst,
     break;
   }
   case CE_ObjCMessage:
-  case CE_ObjCPropertyAccess:
     // These always use dynamic dispatch; enabling inlining means assuming
     // that a particular method will be called at runtime.
     llvm_unreachable("Dynamic dispatch should be handled above.");