From 0e23c61c87551d8d6e796d54f99b654a73c67cff Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Mon, 6 Mar 2017 15:58:34 +0000 Subject: [PATCH] [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 llvm-svn: 297019 --- .../clang/Basic/DiagnosticSemaKinds.td | 6 + clang/lib/AST/DeclObjC.cpp | 14 +- clang/lib/Basic/IdentifierTable.cpp | 6 +- clang/lib/Sema/SemaExprObjC.cpp | 69 ++++++++++ clang/test/SemaObjC/unsafe-perform-selector.m | 127 ++++++++++++++++++ 5 files changed, 214 insertions(+), 8 deletions(-) create mode 100644 clang/test/SemaObjC/unsafe-perform-selector.m diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 86a445ba4ab2..54aa1f14814e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/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>; 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>; +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 - " diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp index 9218eb537a60..a1ef58a45453 100644 --- a/clang/lib/AST/DeclObjC.cpp +++ b/clang/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; } diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp index af424cd92390..5caa8a6fb211 100644 --- a/clang/lib/Basic/IdentifierTable.cpp +++ b/clang/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() == '_') diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp index 3039e12585af..67876c261d0b 100644 --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/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 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(Args[0]->IgnoreParens()); + if (!SE) + return; + ObjCMethodDecl *ImpliedMethod; + if (!IsClassObjectCall) { + const auto *OPT = ReceiverType->getAs(); + 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(); + 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()) { + 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. diff --git a/clang/test/SemaObjC/unsafe-perform-selector.m b/clang/test/SemaObjC/unsafe-perform-selector.m new file mode 100644 index 000000000000..661ff363603f --- /dev/null +++ b/clang/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 + +- (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 -- GitLab