From 4597b7b28e3a71f3c4f0ee3a3bd6a34423e6f885 Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Sun, 13 Mar 2011 20:35:21 +0000 Subject: [PATCH] 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 --- .../Checkers/BasicObjCFoundationChecks.cpp | 141 ++++++++++++++++++ lib/StaticAnalyzer/Checkers/Checkers.td | 5 + test/Analysis/variadic-method-types.m | 77 ++++++++++ 3 files changed, 223 insertions(+) create mode 100644 test/Analysis/variadic-method-types.m diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index f7a1ebe386..a6f980ad49 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/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 { + mutable Selector arrayWithObjectsS; + mutable Selector dictionaryWithObjectsAndKeysS; + mutable Selector setWithObjectsS; + mutable Selector initWithObjectsS; + mutable Selector initWithObjectsAndKeysS; + mutable llvm::OwningPtr 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(); } + +void ento::registerVariadicMethodTypeChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 33611dae83..c139476960 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/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">; diff --git a/test/Analysis/variadic-method-types.m b/test/Analysis/variadic-method-types.m new file mode 100644 index 0000000000..5b187edced --- /dev/null +++ b/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 {} +- (id)init; ++ (id)alloc; +@end +typedef struct {} NSFastEnumerationState; +@protocol NSFastEnumeration +- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len; +@end +@interface NSArray : NSObject +@end +@interface NSArray (NSArrayCreation) ++ (id)arrayWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1))); +- (id)initWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1))); +@end +@interface NSDictionary : NSObject +@end +@interface NSDictionary (NSDictionaryCreation) ++ (id)dictionaryWithObjectsAndKeys:(id)firstObject, ... __attribute__((sentinel(0,1))); +- (id)initWithObjectsAndKeys:(id)firstObject, ... __attribute__((sentinel(0,1))); +@end +@interface NSSet : NSObject +@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

b, C* c, C

*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); +} -- 2.40.0