From: Fariborz Jahanian Date: Tue, 5 Jul 2011 22:38:59 +0000 (+0000) Subject: objc-arc: enforce performSelector rules in rejecting retaining selectors X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9670e179a67d868e171feac44fb8f9e2f108c5e8;p=clang objc-arc: enforce performSelector rules in rejecting retaining selectors passed to it, and unknown selectors causing potential leak. // rdar://9659270 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@134449 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7353f2e813..c5039ff9b9 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -484,6 +484,10 @@ def error_missing_property_ivar_decl : Error< " ivar or must explicitly name an ivar">; def error_synthesize_weak_non_arc_or_gc : Error< "@synthesize of 'weak' property is only allowed in ARC or GC mode">; +def err_arc_perform_selector_retains : Error< + "performSelector names a selector which retains the object">; +def warn_arc_perform_selector_leaks : Warning< + "performSelector may cause a leak because its selector is unknown">; def error_synthesized_ivar_yet_not_supported : Error< "instance variable synthesis not yet supported" diff --git a/include/clang/Basic/IdentifierTable.h b/include/clang/Basic/IdentifierTable.h index e01ab5362d..bebcffddde 100644 --- a/include/clang/Basic/IdentifierTable.h +++ b/include/clang/Basic/IdentifierTable.h @@ -488,7 +488,10 @@ enum ObjCMethodFamily { OMF_release, OMF_retain, OMF_retainCount, - OMF_self + OMF_self, + + // performSelector families + OMF_performSelector }; /// Enough bits to store any enumerator in ObjCMethodFamily or diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp index 67f6531d79..557b681d2f 100644 --- a/lib/AST/DeclObjC.cpp +++ b/lib/AST/DeclObjC.cpp @@ -452,6 +452,34 @@ ObjCMethodFamily ObjCMethodDecl::getMethodFamily() const { if (!isInstanceMethod()) family = OMF_None; break; + + case OMF_performSelector: + if (!isInstanceMethod() || + !getResultType()->isObjCIdType()) + family = OMF_None; + else { + unsigned noParams = param_size(); + if (noParams < 1 || noParams > 3) + family = OMF_None; + else { + ObjCMethodDecl::arg_type_iterator it = arg_type_begin(); + QualType ArgT = (*it); + if (!ArgT->isObjCSelType()) { + family = OMF_None; + break; + } + while (--noParams) { + it++; + ArgT = (*it); + if (!ArgT->isObjCIdType()) { + family = OMF_None; + break; + } + } + } + } + break; + } // Cache the result. diff --git a/lib/Basic/IdentifierTable.cpp b/lib/Basic/IdentifierTable.cpp index 109761c9dc..188e2d46f5 100644 --- a/lib/Basic/IdentifierTable.cpp +++ b/lib/Basic/IdentifierTable.cpp @@ -397,6 +397,8 @@ ObjCMethodFamily Selector::getMethodFamilyImpl(Selector sel) { if (name == "retainCount") return OMF_retainCount; if (name == "self") return OMF_self; } + + if (name == "performSelector") return OMF_performSelector; // The other method families may begin with a prefix of underscores. while (!name.empty() && name.front() == '_') diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index a9f4f95c20..127bb3cdcc 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -286,6 +286,11 @@ static bool CheckARCMethodDecl(Sema &S, ObjCMethodDecl *method) { method->hasAttr()) return false; break; + + case OMF_performSelector: + // we don't annotate performSelector's + return true; + } method->addAttr(new (S.Context) NSReturnsRetainedAttr(SourceLocation(), @@ -366,6 +371,7 @@ void Sema::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, Decl *D) { case OMF_copy: case OMF_new: case OMF_self: + case OMF_performSelector: break; } } @@ -1298,6 +1304,7 @@ static bool checkMethodFamilyMismatch(Sema &S, ObjCMethodDecl *impl, case OMF_dealloc: case OMF_retainCount: case OMF_self: + case OMF_performSelector: // Mismatches for these methods don't change ownership // conventions, so we don't care. return false; @@ -2467,6 +2474,7 @@ Decl *Sema::ActOnMethodDeclaration( case OMF_mutableCopy: case OMF_release: case OMF_retainCount: + case OMF_performSelector: break; case OMF_alloc: diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp index c02b634547..17a7401d65 100644 --- a/lib/Sema/SemaExprObjC.cpp +++ b/lib/Sema/SemaExprObjC.cpp @@ -204,6 +204,7 @@ ExprResult Sema::ParseObjCSelectorExpression(Selector Sel, case OMF_mutableCopy: case OMF_new: case OMF_self: + case OMF_performSelector: break; } } @@ -1417,6 +1418,53 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver, Diag(Loc, diag::err_arc_illegal_explicit_message) << Sel << SelectorLoc; break; + + case OMF_performSelector: + if (Method && NumArgs >= 1) { + if (ObjCSelectorExpr *SelExp = dyn_cast(Args[0])) { + Selector ArgSel = SelExp->getSelector(); + ObjCMethodDecl *SelMethod = + LookupInstanceMethodInGlobalPool(ArgSel, + SelExp->getSourceRange()); + if (!SelMethod) + SelMethod = + LookupFactoryMethodInGlobalPool(ArgSel, + SelExp->getSourceRange()); + if (SelMethod) { + ObjCMethodFamily SelFamily = SelMethod->getMethodFamily(); + switch (SelFamily) { + case OMF_alloc: + case OMF_copy: + case OMF_mutableCopy: + case OMF_new: + case OMF_self: + case OMF_init: + // Issue error, unless ns_returns_not_retained. + if (!SelMethod->hasAttr()) { + // selector names a +1 method + Diag(SelectorLoc, + diag::err_arc_perform_selector_retains); + Diag(SelMethod->getLocation(), diag::note_method_declared_at); + } + break; + default: + // +0 call. OK. unless ns_returns_retained. + if (SelMethod->hasAttr()) { + // selector names a +1 method + Diag(SelectorLoc, + diag::err_arc_perform_selector_retains); + Diag(SelMethod->getLocation(), diag::note_method_declared_at); + } + break; + } + } + } else { + // error (may leak). + Diag(SelectorLoc, diag::warn_arc_perform_selector_leaks); + Diag(Args[0]->getExprLoc(), diag::note_used_here); + } + } + break; } } diff --git a/test/SemaObjC/arc-peformselector.m b/test/SemaObjC/arc-peformselector.m new file mode 100644 index 0000000000..e637f3d76f --- /dev/null +++ b/test/SemaObjC/arc-peformselector.m @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-nonfragile-abi -fsyntax-only -fobjc-arc -verify %s +// rdar://9659270 + +@interface NSObject +- (id)copy; // expected-note {{method declared here}} +- (id) test __attribute__((ns_returns_retained)); // expected-note {{method declared here}} ++ (id) new ; // expected-note {{method declared here}} +- (id) init __attribute__((ns_returns_not_retained)); +- (id)PlusZero; +- (id)PlusOne __attribute__((ns_returns_retained)); // expected-note {{method declared here}} +@end + +@interface I : NSObject +{ + SEL sel1; +} +- (id)performSelector:(SEL)aSelector; +- (id)performSelector:(SEL)aSelector withObject:(id)object; +- (id)performSelector:(SEL)aSelector withObject:(id)object1 withObject:(id)object2; +@end + +@implementation I +- (id) Meth { + return [self performSelector : @selector(copy)]; // expected-error {{performSelector names a selector which retains the object}} + return [self performSelector : @selector(test)]; // expected-error {{performSelector names a selector which retains the object}} + return [self performSelector : @selector(new)]; // expected-error {{performSelector names a selector which retains the object}} + return [self performSelector : @selector(init)]; + return [self performSelector : sel1]; // expected-warning {{performSelector may cause a leak because its selector is unknown}} \ + // expected-note {{used here}} + + return [self performSelector : @selector(PlusZero)]; + return [self performSelector : @selector(PlusOne)]; // expected-error {{performSelector names a selector which retains the object}} +} + +- (id)performSelector:(SEL)aSelector { return 0; } +- (id)performSelector:(SEL)aSelector withObject:(id)object { return 0; } +- (id)performSelector:(SEL)aSelector withObject:(id)object1 withObject:(id)object2 { return 0; } +@end