[cfe-commits] r127572 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp lib/StaticAnalyzer/Checkers/Checkers.td test/Analysis/variadic-method-types.m
Ted Kremenek
kremenek at apple.com
Mon Mar 14 12:55:29 PDT 2011
Looks great to me! I went ahead and did a few tweaks so that only one ExplodedNode gets generated for multiple warnings at the same message expression, and in turn fixed a latent bug in BugReporter. Thanks for doing this!
On Mar 13, 2011, at 1:35 PM, Anders Carlsson wrote:
> Author: andersca
> Date: Sun Mar 13 15:35:21 2011
> New Revision: 127572
>
> URL: http://llvm.org/viewvc/llvm-project?rev=127572&view=rev
> Log:
> 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!
>
> Added:
> cfe/trunk/test/Analysis/variadic-method-types.m
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
> cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp?rev=127572&r1=127571&r2=127572&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp Sun Mar 13 15:35:21 2011
> @@ -477,6 +477,143 @@
> }
>
> //===----------------------------------------------------------------------===//
> +// 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::registerClassReleaseChecker(CheckerManager &mgr) {
> mgr.registerChecker<ClassReleaseChecker>();
> }
> +
> +void ento::registerVariadicMethodTypeChecker(CheckerManager &mgr) {
> + mgr.registerChecker<VariadicMethodTypeChecker>();
> +}
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=127572&r1=127571&r2=127572&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Sun Mar 13 15:35:21 2011
> @@ -60,6 +60,11 @@
> 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">;
>
> Added: cfe/trunk/test/Analysis/variadic-method-types.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/variadic-method-types.m?rev=127572&view=auto
> ==============================================================================
> --- cfe/trunk/test/Analysis/variadic-method-types.m (added)
> +++ cfe/trunk/test/Analysis/variadic-method-types.m Sun Mar 13 15:35:21 2011
> @@ -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;
> + at protocol NSObject
> +- (BOOL)isEqual:(id)object;
> +- (oneway void)release;
> +- (id)retain;
> +- (id)autorelease;
> + at end
> + at protocol NSCopying
> +- (id)copyWithZone:(NSZone *)zone;
> + at end
> + at protocol NSMutableCopying
> +- (id)mutableCopyWithZone:(NSZone *)zone;
> + at end
> + at class NSCoder;
> + at protocol NSCoding
> +- (void)encodeWithCoder:(NSCoder *)aCoder;
> + at end
> + at interface NSObject <NSObject> {}
> +- (id)init;
> ++ (id)alloc;
> + at end
> +typedef struct {} NSFastEnumerationState;
> + at protocol NSFastEnumeration
> +- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len;
> + at end
> + at interface NSArray : NSObject <NSCopying, NSMutableCopying, NSCoding, NSFastEnumeration>
> + at end
> + at interface NSArray (NSArrayCreation)
> ++ (id)arrayWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
> +- (id)initWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
> + at end
> + at interface NSDictionary : NSObject <NSCopying, NSMutableCopying, NSCoding, NSFastEnumeration>
> + at end
> + at interface NSDictionary (NSDictionaryCreation)
> ++ (id)dictionaryWithObjectsAndKeys:(id)firstObject, ... __attribute__((sentinel(0,1)));
> +- (id)initWithObjectsAndKeys:(id)firstObject, ... __attribute__((sentinel(0,1)));
> + at end
> + at interface NSSet : NSObject <NSCopying, NSMutableCopying, NSCoding, NSFastEnumeration>
> + at end
> + at interface NSSet (NSSetCreation)
> ++ (id)setWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
> +- (id)initWithObjects:(id)firstObj, ... __attribute__((sentinel(0,1)));
> + at end
> + at protocol P;
> + at 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);
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list