arphaman created this revision.
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 [1]. 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" [2]. This patch adds a new warning that warns when a selector which
corresponds to a method that returns a record type is passed into
`performSelector`.
[1]
http://www.tomdalling.com/blog/cocoa/why-performselector-is-more-dangerous-than-i-thought/
[2]
https://developer.apple.com/reference/objectivec/nsobject/1416176-performselector?language=objc
Repository:
rL LLVM
https://reviews.llvm.org/D30174
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/DeclObjC.cpp
lib/Basic/IdentifierTable.cpp
lib/Sema/SemaExprObjC.cpp
test/SemaObjC/unsafe-perform-selector.m
Index: test/SemaObjC/unsafe-perform-selector.m
===================================================================
--- /dev/null
+++ test/SemaObjC/unsafe-perform-selector.m
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -verify %s
+// rdar://12056271
+
+__attribute__((objc_root_class))
+@interface NSObject
+
+- (id)performSelector:(SEL)sel;
+- (void)performSelectorInBackground:(SEL)sel withObject:(id)arg;
+- (void)performSelectorOnMainThread:(SEL)sel;
+
+@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 {{method 'returnsUnion' that returns 'Union' declared here}}
+
+@end
+
+@interface I : Base<IP>
+
+- (Rectangle)returnsStruct; // expected-note 2 {{method 'returnsStruct' that returns 'Rectangle' declared here}}
+- (id)returnsId; // shadows base 'returnsId'
+- (int)returnsInt;
+- (I *)returnPtr;
+
++ (Rectangle)returnsStructClass; // expected-note {{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
+}
Index: lib/Sema/SemaExprObjC.cpp
===================================================================
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2260,6 +2260,41 @@
edit::rewriteObjCRedundantCallWithLiteral);
}
+static void checkFoundationAPI(Sema &S, SourceLocation Loc,
+ const ObjCMethodDecl *Method,
+ ArrayRef<Expr *> Args, QualType ReceiverType,
+ bool IsInstanceMethod) {
+ // Check if this is a performSelector method that uses a selector that returns
+ // a record type.
+ if (Method->getMethodFamily() != OMF_performSelector || Args.empty())
+ return;
+ const auto *SE = dyn_cast<ObjCSelectorExpr>(Args[0]->IgnoreParens());
+ if (!SE)
+ return;
+ ObjCMethodDecl *ImpliedMethod;
+ if (IsInstanceMethod) {
+ const auto *OPT = ReceiverType->getAs<ObjCObjectPointerType>();
+ if (!OPT)
+ return;
+ ImpliedMethod =
+ OPT->getInterfaceDecl()->lookupInstanceMethod(SE->getSelector());
+ } else {
+ const auto *IT = ReceiverType->getAs<ObjCInterfaceType>();
+ if (!IT)
+ return;
+ ImpliedMethod = IT->getDecl()->lookupClassMethod(SE->getSelector());
+ }
+ if (ImpliedMethod && ImpliedMethod->getReturnType()->isRecordType()) {
+ QualType Ret = ImpliedMethod->getReturnType();
+ S.Diag(Loc, diag::warn_objc_unsafe_perform_selector)
+ << Method->getSelector()
+ << (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
@@ -2461,6 +2496,9 @@
RBracLoc, isImplicit);
if (!isImplicit)
checkCocoaAPI(*this, Result);
+ if (Method)
+ checkFoundationAPI(*this, SelLoc, Method, makeArrayRef(Args, NumArgs),
+ ReceiverType, /*IsInstanceMethod=*/false);
}
return MaybeBindToTemporary(Result);
}
@@ -2987,6 +3025,9 @@
isImplicit);
if (!isImplicit)
checkCocoaAPI(*this, Result);
+ if (Method)
+ checkFoundationAPI(*this, SelLoc, Method, makeArrayRef(Args, NumArgs),
+ ReceiverType, /*IsInstanceMethod=*/true);
}
if (getLangOpts().ObjCAutoRefCount) {
Index: lib/Basic/IdentifierTable.cpp
===================================================================
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -487,8 +487,10 @@
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() == '_')
Index: lib/AST/DeclObjC.cpp
===================================================================
--- lib/AST/DeclObjC.cpp
+++ lib/AST/DeclObjC.cpp
@@ -979,7 +979,8 @@
break;
case OMF_performSelector:
- if (!isInstanceMethod() || !getReturnType()->isObjCIdType())
+ if (!isInstanceMethod() ||
+ !(getReturnType()->isObjCIdType() || getReturnType()->isVoidType()))
family = OMF_None;
else {
unsigned noParams = param_size();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6001,6 +6001,11 @@
"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}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 - "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
