[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