Add an Objective-C checker that checks that arguments passed to some variadic Objective-C methods are of Objective-C pointer types.

Ted or Argiris, I'd appreciate a review!


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

lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

@@ -476,6 +476,143 @@ void ClassReleaseChecker::checkPreObjCMessage(ObjCMessage msg,
   }
 }
 
+//===----------------------------------------------------------------------===//
+// Check for passing non-Objective-C types to variadic methods that expect
+// only Objective-C types.
+//===----------------------------------------------------------------------===//
+
+namespace {
+class VariadicMethodTypeChecker : public Checker<check::PreObjCMessage> {
+  mutable Selector arrayWithObjectsS;
+  mutable Selector dictionaryWithObjectsAndKeysS;
+  mutable Selector setWithObjectsS;
+  mutable Selector initWithObjectsS;
+  mutable Selector initWithObjectsAndKeysS;
+  mutable llvm::OwningPtr<BugType> BT;
+
+  bool isVariadicMessage(const ObjCMessage &msg) const;
+
+public:
+  void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const;
+};
+}
+
+/// isVariadicMessage - Returns whether the given message is a variadic message,
+/// where all arguments must be Objective-C types.
+bool
+VariadicMethodTypeChecker::isVariadicMessage(const ObjCMessage &msg) const {
+  const ObjCMethodDecl *MD = msg.getMethodDecl();
+  if (!MD)
+    return false;
+  
+  if (!MD->isVariadic())
+    return false;
+  
+  Selector S = msg.getSelector();
+  
+  if (msg.isInstanceMessage()) {
+    // FIXME: Ideally we'd look at the receiver interface here, but that's not
+    // useful for init, because alloc returns 'id'. In theory, this could lead
+    // to false positives, for example if there existed a class that had an
+    // initWithObjects: implementation that does accept non-Objective-C pointer
+    // types, but the chance of that happening is pretty small compared to the
+    // gains that this analysis gives.
+    const ObjCInterfaceDecl *Class = MD->getClassInterface();
+
+    // -[NSArray initWithObjects:]
+    if (isReceiverClassOrSuperclass(Class, "NSArray") &&
+        S == initWithObjectsS)
+      return true;
+
+    // -[NSDictionary initWithObjectsAndKeys:]
+    if (isReceiverClassOrSuperclass(Class, "NSDictionary") &&
+        S == initWithObjectsAndKeysS)
+      return true;
+
+    // -[NSSet initWithObjects:]
+    if (isReceiverClassOrSuperclass(Class, "NSSet") &&
+        S == initWithObjectsS)
+      return true;
+  } else {
+    const ObjCInterfaceDecl *Class = msg.getReceiverInterface();
+
+    // -[NSArray arrayWithObjects:]
+    if (isReceiverClassOrSuperclass(Class, "NSArray") &&
+        S == arrayWithObjectsS)
+      return true;
+
+    // -[NSDictionary dictionaryWithObjectsAndKeys:]
+    if (isReceiverClassOrSuperclass(Class, "NSDictionary") &&
+        S == dictionaryWithObjectsAndKeysS)
+      return true;
+
+    // -[NSSet setWithObjects:]
+    if (isReceiverClassOrSuperclass(Class, "NSSet") &&
+        S == setWithObjectsS)
+      return true;
+  }
+
+  return false;
+}
+
+void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg,
+                                                    CheckerContext &C) const {
+  if (!BT) {
+    BT.reset(new APIMisuse("Arguments passed to variadic method aren't all "
+                           "Objective-C pointer types"));
+
+    ASTContext &Ctx = C.getASTContext();
+    arrayWithObjectsS = GetUnarySelector("arrayWithObjects", Ctx);
+    dictionaryWithObjectsAndKeysS = 
+      GetUnarySelector("dictionaryWithObjectsAndKeys", Ctx);
+    setWithObjectsS = GetUnarySelector("setWithObjects", Ctx);
+
+    initWithObjectsS = GetUnarySelector("initWithObjects", Ctx);
+    initWithObjectsAndKeysS = GetUnarySelector("initWithObjectsAndKeys", Ctx);
+  }
+
+  if (!isVariadicMessage(msg))
+      return;
+
+  // We are not interested in the selector arguments since they have
+  // well-defined types, so the compiler will issue a warning for them.
+  unsigned variadicArgsBegin = msg.getSelector().getNumArgs();
+
+  // We're not interested in the last argument since it has to be nil or the
+  // compiler would have issued a warning for it elsewhere.
+  unsigned variadicArgsEnd = msg.getNumArgs() - 1;
+
+  if (variadicArgsEnd <= variadicArgsBegin)
+    return;
+
+  // Verify that all arguments have Objective-C types.
+  for (unsigned I = variadicArgsBegin; I != variadicArgsEnd; ++I) {
+    QualType ArgTy = msg.getArgType(I);
+    if (ArgTy->isObjCObjectPointerType())
+      continue;
+
+    ExplodedNode *N = C.generateNode();
+    if (!N)
+      continue;
+
+    llvm::SmallString<128> sbuf;
+    llvm::raw_svector_ostream os(sbuf);
+
+    if (const char *TypeName = GetReceiverNameType(msg))
+      os << "Argument to '" << TypeName << "' method '";
+    else
+      os << "Argument to method '";
+
+    os << msg.getSelector().getAsString() 
+      << "' should be an Objective-C pointer type, not '" 
+      << ArgTy.getAsString() << "'";
+
+    RangedBugReport *R = new RangedBugReport(*BT, os.str(), N);
+    R->addRange(msg.getArgSourceRange(I));
+    C.EmitReport(R);
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // Check registration.
 //===----------------------------------------------------------------------===//
@@ -495,3 +632,7 @@ void ento::registerCFRetainReleaseChecker(CheckerManager &mgr) {
 void ento::registerClassReleaseChecker(CheckerManager &mgr) {
   mgr.registerChecker<ClassReleaseChecker>();
 }
+
+void ento::registerVariadicMethodTypeChecker(CheckerManager &mgr) {
+  mgr.registerChecker<VariadicMethodTypeChecker>();
+}

lib/StaticAnalyzer/Checkers/Checkers.td

@@ -60,6 +60,11 @@ def ClassReleaseChecker : Checker<"ClassRelease">,
   HelpText<"Check for sending 'retain', 'release', or 'autorelease' directly to a Class">,
   DescFile<"BasicObjCFoundationChecks.cpp">;
 
+def VariadicMethodTypeChecker : Checker<"VariadicMethodTypes">,
+  HelpText<"Check for passing non-Objective-C types to variadic methods that expect"
+           "only Objective-C types">,
+  DescFile<"BasicObjCFoundationChecks.cpp">;
+
 def NSAutoreleasePoolChecker : Checker<"NSAutoreleasePool">,
   HelpText<"Warn for subpar uses of NSAutoreleasePool">,
   DescFile<"NSAutoreleasePoolChecker.cpp">;

test/Analysis/variadic-method-types.m

@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,cocoa.VariadicMethodTypes -analyzer-store=basic -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,cocoa.VariadicMethodTypes -analyzer-store=region -fblocks -verify %s
+
+//===----------------------------------------------------------------------===//
+// The following code is reduced using delta-debugging from
+// Foundation.h (Mac OS X).
+//
+// It includes the basic definitions for the test cases below.
+// Not directly including Foundation.h directly makes this test case 
+// both svelte and portable to non-Mac platforms.
+//===----------------------------------------------------------------------===//
+
+#define nil (void*)0
+
+typedef signed char BOOL;
+typedef struct _NSZone NSZone;
+typedef unsigned int NSUInteger;
+@protocol NSObject
+- (BOOL)isEqual:(id)object;
+- (oneway void)release;
+- (id)retain;
+- (id)autorelease;
+@end
+@protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+@end
+@protocol NSMutableCopying
+- (id)mutableCopyWithZone:(NSZone *)zone;
+@end
+@class NSCoder;
+@protocol NSCoding
+- (void)encodeWithCoder:(NSCoder *)aCoder;
+@end
+@interface NSObject <NSObject> {}
+- (id)init;
++ (id)alloc;
+@end
+typedef struct {} NSFastEnumerationState;
+@protocol NSFastEnumeration
+- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len;
+@end
+@interface NSArray : NSObject <NSCopying, NSMutableCopying, NSCoding, NSFastEnumeration>
+@end
+@interface NSArray (NSArrayCreation)
++ (id)arrayWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
+- (id)initWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
+@end
+@interface NSDictionary : NSObject <NSCopying, NSMutableCopying, NSCoding, NSFastEnumeration>
+@end
+@interface NSDictionary (NSDictionaryCreation)
++ (id)dictionaryWithObjectsAndKeys:(id)firstObject, ... __attribute__((sentinel(0,1)));
+- (id)initWithObjectsAndKeys:(id)firstObject, ... __attribute__((sentinel(0,1)));
+@end
+@interface NSSet : NSObject <NSCopying, NSMutableCopying, NSCoding, NSFastEnumeration>
+@end
+@interface NSSet (NSSetCreation)
++ (id)setWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
+- (id)initWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
+@end
+@protocol P;
+@class C;
+
+void f(id a, id<P> b, C* c, C<P> *d) {
+  [NSArray arrayWithObjects:@"Hello", a, b, c, d, nil];
+
+  [NSArray arrayWithObjects:@"Foo", "Bar", nil]; // expected-warning {{Argument to 'NSArray' method 'arrayWithObjects:' should be an Objective-C pointer type, not 'char *'}}
+  [NSDictionary dictionaryWithObjectsAndKeys:@"Foo", "Bar", nil]; // expected-warning {{Argument to 'NSDictionary' method 'dictionaryWithObjectsAndKeys:' should be an Objective-C pointer type, not 'char *'}}
+  [NSSet setWithObjects:@"Foo", "Bar", nil]; // expected-warning {{Argument to 'NSSet' method 'setWithObjects:' should be an Objective-C pointer type, not 'char *'}}
+
+  [[[NSArray alloc] initWithObjects:@"Foo", "Bar", nil] autorelease]; // expected-warning {{Argument to method 'initWithObjects:' should be an Objective-C pointer type, not 'char *'}}
+  [[[NSDictionary alloc] initWithObjectsAndKeys:@"Foo", "Bar", nil] autorelease]; // expected-warning {{Argument to method 'initWithObjectsAndKeys:' should be an Objective-C pointer type, not 'char *'}}
+  [[[NSSet alloc] initWithObjects:@"Foo", "Bar", nil] autorelease]; // expected-warning {{Argument to method 'initWithObjects:' should be an Objective-C pointer type, not 'char *'}}
+}
+
+int main() {
+  f(nil, nil, nil, nil);
+}