[Sema][ObjC] Warn about 'performSelector' calls with selectors

that return record or vector types

The performSelector family of methods from Foundation use objc_msgSend to
dispatch the selector invocations to objects. However, method calls to methods
that return record types might have to use the objc_msgSend_stret as the return
value won't find into the register. This is also supported by this sentence from
performSelector documentation: "The method should not have a significant return
value and should take a single argument of type id, or no arguments". This
commit adds a new warning that warns when a selector which corresponds to a
method that returns a record type is passed into performSelector.

rdar://12056271

Differential Revision: https://reviews.llvm.org/D30174


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

include/clang/Basic/DiagnosticSemaKinds.td

@@ -6016,6 +6016,12 @@ def warn_objc_circular_container : Warning<
   "adding '%0' to '%1' might cause circular dependency in container">,
   InGroup<DiagGroup<"objc-circular-container">>;
 def note_objc_circular_container_declared_here : Note<"'%0' declared here">;
+def warn_objc_unsafe_perform_selector : Warning<
+  "%0 is incompatible with selectors that return a "
+  "%select{struct|union|vector}1 type">,
+  InGroup<DiagGroup<"objc-unsafe-perform-selector">>;
+def note_objc_unsafe_perform_selector_method_declared_here :  Note<
+  "method %0 that returns %1 declared here">;
 
 def warn_setter_getter_impl_required : Warning<
   "property %0 requires method %1 to be defined - "

lib/AST/DeclObjC.cpp

@@ -979,11 +979,12 @@ ObjCMethodFamily ObjCMethodDecl::getMethodFamily() const {
     break;
       
   case OMF_performSelector:
-    if (!isInstanceMethod() || !getReturnType()->isObjCIdType())
+    if (!isInstanceMethod() ||
+        !(getReturnType()->isObjCIdType() || getReturnType()->isVoidType()))
       family = OMF_None;
     else {
       unsigned noParams = param_size();
-      if (noParams < 1 || noParams > 3)
+      if (noParams < 1 || noParams > 5)
         family = OMF_None;
       else {
         ObjCMethodDecl::param_type_iterator it = param_type_begin();
@@ -992,10 +993,11 @@ ObjCMethodFamily ObjCMethodDecl::getMethodFamily() const {
           family = OMF_None;
           break;
         }
-        while (--noParams) {
-          it++;
-          ArgT = (*it);
-          if (!ArgT->isObjCIdType()) {
+        // The first type should generally always be 'id' or 'Thread *', the
+        // other types can vary.
+        if (noParams > 1) {
+          ArgT = *(it + 1);
+          if (!ArgT->isObjCObjectPointerType()) {
             family = OMF_None;
             break;
           }

lib/Basic/IdentifierTable.cpp

@@ -487,8 +487,10 @@ ObjCMethodFamily Selector::getMethodFamilyImpl(Selector sel) {
     if (name == "self") return OMF_self;
     if (name == "initialize") return OMF_initialize;
   }
- 
-  if (name == "performSelector") return OMF_performSelector;
+
+  if (name == "performSelector" || name == "performSelectorInBackground" ||
+      name == "performSelectorOnMainThread")
+    return OMF_performSelector;
 
   // The other method families may begin with a prefix of underscores.
   while (!name.empty() && name.front() == '_')

lib/Sema/SemaExprObjC.cpp

@@ -2266,6 +2266,52 @@ static void checkCocoaAPI(Sema &S, const ObjCMessageExpr *Msg) {
                      edit::rewriteObjCRedundantCallWithLiteral);
 }
 
+static void checkFoundationAPI(Sema &S, SourceLocation Loc,
+                               const ObjCMethodDecl *Method,
+                               ArrayRef<Expr *> Args, QualType ReceiverType,
+                               bool IsClassObjectCall) {
+  // Check if this is a performSelector method that uses a selector that returns
+  // a record or a vector type.
+  if (Method->getMethodFamily() != OMF_performSelector || Args.empty())
+    return;
+  const auto *SE = dyn_cast<ObjCSelectorExpr>(Args[0]->IgnoreParens());
+  if (!SE)
+    return;
+  ObjCMethodDecl *ImpliedMethod;
+  if (!IsClassObjectCall) {
+    const auto *OPT = ReceiverType->getAs<ObjCObjectPointerType>();
+    if (!OPT || !OPT->getInterfaceDecl())
+      return;
+    ImpliedMethod =
+        OPT->getInterfaceDecl()->lookupInstanceMethod(SE->getSelector());
+    if (!ImpliedMethod)
+      ImpliedMethod =
+          OPT->getInterfaceDecl()->lookupPrivateMethod(SE->getSelector());
+  } else {
+    const auto *IT = ReceiverType->getAs<ObjCInterfaceType>();
+    if (!IT)
+      return;
+    ImpliedMethod = IT->getDecl()->lookupClassMethod(SE->getSelector());
+    if (!ImpliedMethod)
+      ImpliedMethod =
+          IT->getDecl()->lookupPrivateClassMethod(SE->getSelector());
+  }
+  if (!ImpliedMethod)
+    return;
+  QualType Ret = ImpliedMethod->getReturnType();
+  if (Ret->isRecordType() || Ret->isVectorType() || Ret->isExtVectorType()) {
+    QualType Ret = ImpliedMethod->getReturnType();
+    S.Diag(Loc, diag::warn_objc_unsafe_perform_selector)
+        << Method->getSelector()
+        << (!Ret->isRecordType()
+                ? /*Vector*/ 2
+                : Ret->isUnionType() ? /*Union*/ 1 : /*Struct*/ 0);
+    S.Diag(ImpliedMethod->getLocStart(),
+           diag::note_objc_unsafe_perform_selector_method_declared_here)
+        << ImpliedMethod->getSelector() << Ret;
+  }
+}
+
 /// \brief Diagnose use of %s directive in an NSString which is being passed
 /// as formatting string to formatting method.
 static void
@@ -2468,6 +2514,9 @@ ExprResult Sema::BuildClassMessage(TypeSourceInfo *ReceiverTypeInfo,
     if (!isImplicit)
       checkCocoaAPI(*this, Result);
   }
+  if (Method)
+    checkFoundationAPI(*this, SelLoc, Method, makeArrayRef(Args, NumArgs),
+                       ReceiverType, /*IsClassObjectCall=*/true);
   return MaybeBindToTemporary(Result);
 }
 
@@ -2994,6 +3043,26 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
     if (!isImplicit)
       checkCocoaAPI(*this, Result);
   }
+  if (Method) {
+    bool IsClassObjectCall = ClassMessage;
+    // 'self' message receivers in class methods should be treated as message
+    // sends to the class object in order for the semantic checks to be
+    // performed correctly. Messages to 'super' already count as class messages,
+    // so they don't need to be handled here.
+    if (Receiver && isSelfExpr(Receiver)) {
+      if (const auto *OPT = ReceiverType->getAs<ObjCObjectPointerType>()) {
+        if (OPT->getObjectType()->isObjCClass()) {
+          if (const auto *CurMeth = getCurMethodDecl()) {
+            IsClassObjectCall = true;
+            ReceiverType =
+                Context.getObjCInterfaceType(CurMeth->getClassInterface());
+          }
+        }
+      }
+    }
+    checkFoundationAPI(*this, SelLoc, Method, makeArrayRef(Args, NumArgs),
+                       ReceiverType, IsClassObjectCall);
+  }
 
   if (getLangOpts().ObjCAutoRefCount) {
     // In ARC, annotate delegate init calls.

test/SemaObjC/unsafe-perform-selector.m

@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -verify %s
+// rdar://12056271
+
+@class Thread;
+
+__attribute__((objc_root_class))
+@interface NSObject
+
+- (id)performSelector:(SEL)sel;
+- (void)performSelectorInBackground:(SEL)sel withObject:(id)arg;
+- (void)performSelectorOnMainThread:(SEL)sel;
+
+- (void)performSelectorOnMainThread:(SEL)aSelector
+                           onThread:(Thread *)thread
+                         withObject:(id)arg
+                      waitUntilDone:(int)wait
+                              modes:(id *)array;
+
+@end
+
+typedef struct { int x; int y; int width; int height; } Rectangle;
+
+struct Struct { Rectangle r; };
+
+typedef union { int x; float f; } Union;
+
+@interface Base : NSObject
+
+- (struct Struct)returnsStruct2; // expected-note {{method 'returnsStruct2' that returns 'struct Struct' declared here}}
+- (Union)returnsId;
+
+@end
+
+@protocol IP
+
+- (Union)returnsUnion; // expected-note 2 {{method 'returnsUnion' that returns 'Union' declared here}}
+
+@end
+
+typedef __attribute__((__ext_vector_type__(3))) float float3;
+typedef int int4 __attribute__ ((vector_size (16)));
+
+@interface I : Base<IP>
+
+- (Rectangle)returnsStruct; // expected-note 4 {{method 'returnsStruct' that returns 'Rectangle' declared here}}
+- (id)returnsId; // shadows base 'returnsId'
+- (int)returnsInt;
+- (I *)returnPtr;
+- (float3)returnsExtVector; // expected-note {{method 'returnsExtVector' that returns 'float3' (vector of 3 'float' values) declared here}}
+- (int4)returnsVector; // expected-note {{method 'returnsVector' that returns 'int4' (vector of 4 'int' values) declared here}}
+
++ (Rectangle)returnsStructClass; // expected-note 2 {{method 'returnsStructClass' that returns 'Rectangle' declared here}}
++ (void)returnsUnion; // Not really
+
+@end
+
+void foo(I *i) {
+  [i performSelector: @selector(returnsStruct)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+  [i performSelectorInBackground: @selector(returnsStruct) withObject:0]; // expected-warning {{'performSelectorInBackground:withObject:' is incompatible with selectors that return a struct type}}
+  [i performSelector: ((@selector(returnsUnion)))]; // expected-warning {{'performSelector:' is incompatible with selectors that return a union type}}
+  [i performSelectorOnMainThread: @selector(returnsStruct2)]; // expected-warning {{'performSelectorOnMainThread:' is incompatible with selectors that return a struct type}}
+  [I performSelector: (@selector(returnsStructClass))]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+
+  [i performSelector: @selector(returnsId)];
+  [i performSelector: @selector(returnsInt)];
+  [i performSelector: @selector(returnsPtr)];
+  [I performSelector: @selector(returnsUnion)]; // No warning expected
+
+  id obj = i;
+  [obj performSelector: @selector(returnsId)];
+  [obj performSelector: @selector(returnsStruct)];
+}
+
+@interface SubClass: I
+
+@end
+
+@interface SubClass ()
+- (struct Struct)returnsSubStructExt; // expected-note {{method 'returnsSubStructExt' that returns 'struct Struct' declared here}} expected-note {{method 'returnsSubStructExt' declared here}}
+@end
+
+@implementation SubClass // expected-warning {{method definition for 'returnsSubStructExt' not found}}
+
+- (struct Struct)returnsSubStructImpl { // expected-note {{method 'returnsSubStructImpl' that returns 'struct Struct' declared here}}
+  struct Struct Result;
+  return Result;
+}
+
+- (void)checkPrivateCalls {
+  [self performSelector: @selector(returnsSubStructExt)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+  [self performSelector: @selector(returnsSubStructImpl)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+}
+
+- (void)checkSuperCalls {
+  [super performSelector: @selector(returnsStruct)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+  [super performSelectorInBackground: @selector(returnsUnion) withObject: self]; // expected-warning {{'performSelectorInBackground:withObject:' is incompatible with selectors that return a union type}}
+  [super performSelector: @selector(returnsId)];
+}
+
++ (struct Struct)returnsSubStructClassImpl { // expected-note {{method 'returnsSubStructClassImpl' that returns 'struct Struct' declared here}}
+  struct Struct Result;
+  return Result;
+}
+
++ (void)checkClassPrivateCalls {
+  [self performSelector: @selector(returnsSubStructClassImpl)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+}
+
++ (void)checkClassSuperCalls {
+  [super performSelector: @selector(returnsStructClass)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a struct type}}
+  [super performSelector: @selector(returnsUnion)]; // No warning expected
+}
+
+@end
+
+@implementation I (LongPerformSelectors)
+
+- (void)checkLongCallsFromCategory {
+  [self performSelectorOnMainThread: @selector(returnsStruct) onThread:0 withObject:self waitUntilDone:1 modes:0]; // expected-warning {{'performSelectorOnMainThread:onThread:withObject:waitUntilDone:modes:' is incompatible with selectors that return a struct type}}
+}
+
+- (void)checkVectorReturn {
+  [self performSelector: @selector(returnsExtVector)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a vector type}}
+  [self performSelector: @selector(returnsVector)]; // expected-warning {{'performSelector:' is incompatible with selectors that return a vector type}}
+}
+
+@end