[analyzer] When checking Foundation method calls, match the selectors exactly.

This also includes some infrastructure to make it easier to build multi-argument
selectors, rather than trying to use string matching on each piece. There's a bit
more setup code, but less cost at runtime.

PR18908

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

lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

@@ -14,6 +14,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
+#include "SelectorExtras.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
@@ -97,6 +98,18 @@ namespace {
                                        check::PostStmt<ObjCArrayLiteral> > {
     mutable std::unique_ptr<APIMisuse> BT;
 
+    mutable llvm::SmallDenseMap<Selector, unsigned, 16> StringSelectors;
+    mutable Selector ArrayWithObjectSel;
+    mutable Selector AddObjectSel;
+    mutable Selector InsertObjectAtIndexSel;
+    mutable Selector ReplaceObjectAtIndexWithObjectSel;
+    mutable Selector SetObjectAtIndexedSubscriptSel;
+    mutable Selector ArrayByAddingObjectSel;
+    mutable Selector DictionaryWithObjectForKeySel;
+    mutable Selector SetObjectForKeySel;
+    mutable Selector SetObjectForKeyedSubscriptSel;
+    mutable Selector RemoveObjectForKeySel;
+
     void warnIfNilExpr(const Expr *E,
                        const char *Msg,
                        CheckerContext &C) const;
@@ -214,50 +227,62 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
   
   if (Class == FC_NSString) {
     Selector S = msg.getSelector();
-    
+
     if (S.isUnarySelector())
       return;
-    
-    // FIXME: This is going to be really slow doing these checks with
-    //  lexical comparisons.
-    
-    std::string NameStr = S.getAsString();
-    StringRef Name(NameStr);
-    assert(!Name.empty());
-    
-    // FIXME: Checking for initWithFormat: will not work in most cases
-    //  yet because [NSString alloc] returns id, not NSString*.  We will
-    //  need support for tracking expected-type information in the analyzer
-    //  to find these errors.
-    if (Name == "caseInsensitiveCompare:" ||
-        Name == "compare:" ||
-        Name == "compare:options:" ||
-        Name == "compare:options:range:" ||
-        Name == "compare:options:range:locale:" ||
-        Name == "componentsSeparatedByCharactersInSet:" ||
-        Name == "initWithFormat:") {
-      Arg = 0;
+
+    if (StringSelectors.empty()) {
+      ASTContext &Ctx = C.getASTContext();
+      Selector Sels[] = {
+        getKeywordSelector(Ctx, "caseInsensitiveCompare", nullptr),
+        getKeywordSelector(Ctx, "compare", nullptr),
+        getKeywordSelector(Ctx, "compare", "options", nullptr),
+        getKeywordSelector(Ctx, "compare", "options", "range", nullptr),
+        getKeywordSelector(Ctx, "compare", "options", "range", "locale",
+                           nullptr),
+        getKeywordSelector(Ctx, "componentsSeparatedByCharactersInSet",
+                           nullptr),
+        getKeywordSelector(Ctx, "initWithFormat",
+                           nullptr),
+        getKeywordSelector(Ctx, "localizedCaseInsensitiveCompare", nullptr),
+        getKeywordSelector(Ctx, "localizedCompare", nullptr),
+        getKeywordSelector(Ctx, "localizedStandardCompare", nullptr),
+      };
+      for (Selector KnownSel : Sels)
+        StringSelectors[KnownSel] = 0;
     }
+    auto I = StringSelectors.find(S);
+    if (I == StringSelectors.end())
+      return;
+    Arg = I->second;
   } else if (Class == FC_NSArray) {
     Selector S = msg.getSelector();
 
     if (S.isUnarySelector())
       return;
 
-    if (S.getNameForSlot(0).equals("addObject")) {
-      Arg = 0;
-    } else if (S.getNameForSlot(0).equals("insertObject") &&
-               S.getNameForSlot(1).equals("atIndex")) {
+    if (ArrayWithObjectSel.isNull()) {
+      ASTContext &Ctx = C.getASTContext();
+      ArrayWithObjectSel = getKeywordSelector(Ctx, "arrayWithObject", nullptr);
+      AddObjectSel = getKeywordSelector(Ctx, "addObject", nullptr);
+      InsertObjectAtIndexSel =
+        getKeywordSelector(Ctx, "insertObject", "atIndex", nullptr);
+      ReplaceObjectAtIndexWithObjectSel =
+        getKeywordSelector(Ctx, "replaceObjectAtIndex", "withObject", nullptr);
+      SetObjectAtIndexedSubscriptSel =
+        getKeywordSelector(Ctx, "setObject", "atIndexedSubscript", nullptr);
+      ArrayByAddingObjectSel =
+        getKeywordSelector(Ctx, "arrayByAddingObject", nullptr);
+    }
+
+    if (S == ArrayWithObjectSel || S == AddObjectSel ||
+        S == InsertObjectAtIndexSel || S == ArrayByAddingObjectSel) {
       Arg = 0;
-    } else if (S.getNameForSlot(0).equals("replaceObjectAtIndex") &&
-               S.getNameForSlot(1).equals("withObject")) {
-      Arg = 1;
-    } else if (S.getNameForSlot(0).equals("setObject") &&
-               S.getNameForSlot(1).equals("atIndexedSubscript")) {
+    } else if (S == SetObjectAtIndexedSubscriptSel) {
       Arg = 0;
       CanBeSubscript = true;
-    } else if (S.getNameForSlot(0).equals("arrayByAddingObject")) {
-      Arg = 0;
+    } else if (S == ReplaceObjectAtIndexWithObjectSel) {
+      Arg = 1;
     }
   } else if (Class == FC_NSDictionary) {
     Selector S = msg.getSelector();
@@ -265,20 +290,26 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
     if (S.isUnarySelector())
       return;
 
-    if (S.getNameForSlot(0).equals("dictionaryWithObject") &&
-        S.getNameForSlot(1).equals("forKey")) {
-      Arg = 0;
-      warnIfNilArg(C, msg, /* Arg */1, Class);
-    } else if (S.getNameForSlot(0).equals("setObject") &&
-               S.getNameForSlot(1).equals("forKey")) {
+    if (DictionaryWithObjectForKeySel.isNull()) {
+      ASTContext &Ctx = C.getASTContext();
+      DictionaryWithObjectForKeySel =
+        getKeywordSelector(Ctx, "dictionaryWithObject", "forKey", nullptr);
+      SetObjectForKeySel =
+        getKeywordSelector(Ctx, "setObject", "forKey", nullptr);
+      SetObjectForKeyedSubscriptSel =
+        getKeywordSelector(Ctx, "setObject", "forKeyedSubscript", nullptr);
+      RemoveObjectForKeySel =
+        getKeywordSelector(Ctx, "removeObjectForKey", nullptr);
+    }
+
+    if (S == DictionaryWithObjectForKeySel || S == SetObjectForKeySel) {
       Arg = 0;
       warnIfNilArg(C, msg, /* Arg */1, Class);
-    } else if (S.getNameForSlot(0).equals("setObject") &&
-               S.getNameForSlot(1).equals("forKeyedSubscript")) {
+    } else if (S == SetObjectForKeyedSubscriptSel) {
       CanBeSubscript = true;
       Arg = 0;
       warnIfNilArg(C, msg, /* Arg */1, Class, CanBeSubscript);
-    } else if (S.getNameForSlot(0).equals("removeObjectForKey")) {
+    } else if (S == RemoveObjectForKeySel) {
       Arg = 0;
     }
   }
@@ -286,7 +317,6 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
   // If argument is '0', report a warning.
   if ((Arg != InvalidArgIndex))
     warnIfNilArg(C, msg, Arg, Class, CanBeSubscript);
-
 }
 
 void NilArgChecker::checkPostStmt(const ObjCArrayLiteral *AL,

lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
+#include "SelectorExtras.h"
 #include "clang/AST/Attr.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -28,6 +29,8 @@ namespace {
 
 class NoReturnFunctionChecker : public Checker< check::PostCall,
                                                 check::PostObjCMessage > {
+  mutable Selector HandleFailureInFunctionSel;
+  mutable Selector HandleFailureInMethodSel;
 public:
   void checkPostCall(const CallEvent &CE, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
@@ -81,24 +84,6 @@ void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE,
     C.generateSink();
 }
 
-static bool END_WITH_NULL isMultiArgSelector(const Selector *Sel, ...) {
-  va_list argp;
-  va_start(argp, Sel);
-
-  unsigned Slot = 0;
-  const char *Arg;
-  while ((Arg = va_arg(argp, const char *))) {
-    if (!Sel->getNameForSlot(Slot).equals(Arg))
-      break; // still need to va_end!
-    ++Slot;
-  }
-
-  va_end(argp);
-
-  // We only succeeded if we made it to the end of the argument list.
-  return (Arg == NULL);
-}
-
 void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg,
                                                    CheckerContext &C) const {
   // Check if the method is annotated with analyzer_noreturn.
@@ -135,13 +120,17 @@ void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg,
   default:
     return;
   case 4:
-    if (!isMultiArgSelector(&Sel, "handleFailureInFunction", "file",
-                            "lineNumber", "description", NULL))
+    lazyInitKeywordSelector(HandleFailureInFunctionSel, C.getASTContext(),
+                            "handleFailureInFunction", "file", "lineNumber",
+                            "description", nullptr);
+    if (Sel != HandleFailureInFunctionSel)
       return;
     break;
   case 5:
-    if (!isMultiArgSelector(&Sel, "handleFailureInMethod", "object", "file",
-                            "lineNumber", "description", NULL))
+    lazyInitKeywordSelector(HandleFailureInMethodSel, C.getASTContext(),
+                            "handleFailureInMethod", "object", "file",
+                            "lineNumber", "description", nullptr);
+    if (Sel != HandleFailureInMethodSel)
       return;
     break;
   }

lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

@@ -14,6 +14,7 @@
 
 #include "ClangSACheckers.h"
 #include "AllocationDiagnostics.h"
+#include "SelectorExtras.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
@@ -675,18 +676,9 @@ private:
     ObjCMethodSummaries[ObjCSummaryKey(ClsII, S)]  = Summ;
   }
 
-  Selector generateSelector(va_list argp) {
-    SmallVector<IdentifierInfo*, 10> II;
-
-    while (const char* s = va_arg(argp, const char*))
-      II.push_back(&Ctx.Idents.get(s));
-
-    return Ctx.Selectors.getSelector(II.size(), &II[0]);
-  }
-
-  void addMethodSummary(IdentifierInfo *ClsII, ObjCMethodSummariesTy& Summaries,
-                        const RetainSummary * Summ, va_list argp) {
-    Selector S = generateSelector(argp);
+  void addMethodSummary(IdentifierInfo *ClsII, ObjCMethodSummariesTy &Summaries,
+                        const RetainSummary *Summ, va_list argp) {
+    Selector S = getKeywordSelector(Ctx, argp);
     Summaries[ObjCSummaryKey(ClsII, S)] = Summ;
   }
 

lib/StaticAnalyzer/Checkers/SelectorExtras.h

@@ -0,0 +1,67 @@
+//=== SelectorExtras.h - Helpers for checkers using selectors -----*- C++ -*-=//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SA_CHECKERS_SELECTOREXTRAS
+#define LLVM_CLANG_SA_CHECKERS_SELECTOREXTRAS
+
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace ento {
+
+static inline Selector getKeywordSelectorImpl(ASTContext &Ctx,
+                                              const char *First,
+                                              va_list argp) {
+  SmallVector<IdentifierInfo*, 10> II;
+  II.push_back(&Ctx.Idents.get(First));
+
+  while (const char *s = va_arg(argp, const char *))
+    II.push_back(&Ctx.Idents.get(s));
+
+  return Ctx.Selectors.getSelector(II.size(), &II[0]);
+}
+
+static inline Selector getKeywordSelector(ASTContext &Ctx, va_list argp) {
+  const char *First = va_arg(argp, const char *);
+  assert(First && "keyword selectors must have at least one argument");
+  return getKeywordSelectorImpl(Ctx, First, argp);
+}
+
+END_WITH_NULL
+static inline Selector getKeywordSelector(ASTContext &Ctx,
+                                          const char *First, ...) {
+  va_list argp;
+  va_start(argp, First);
+  Selector result = getKeywordSelectorImpl(Ctx, First, argp);
+  va_end(argp);
+  return result;
+}
+
+END_WITH_NULL
+static inline void lazyInitKeywordSelector(Selector &Sel, ASTContext &Ctx,
+                                           const char *First, ...) {
+  if (!Sel.isNull())
+    return;
+  va_list argp;
+  va_start(argp, First);
+  Sel = getKeywordSelectorImpl(Ctx, First, argp);
+  va_end(argp);
+}
+
+static inline void lazyInitNullarySelector(Selector &Sel, ASTContext &Ctx,
+                                           const char *Name) {
+  if (!Sel.isNull())
+    return;
+  Sel = GetNullarySelector(Name, Ctx);
+}
+
+} // end namespace ento
+} // end namespace clang
+
+#endif

test/Analysis/NSContainers.m

@@ -284,3 +284,11 @@ void testLiteralsNonNil() {
   clang_analyzer_eval(!!@{}); // expected-warning{{TRUE}}
 }
 
+@interface NSMutableArray (MySafeAdd)
+- (void)addObject:(id)obj safe:(BOOL)safe;
+@end
+
+void testArrayCategory(NSMutableArray *arr) {
+  [arr addObject:0 safe:1]; // no-warning
+}
+