From patchwork Mon Feb 28 14:48:46 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Pero X-Patchwork-Id: 84843 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2AE24B7119 for ; Tue, 1 Mar 2011 01:49:10 +1100 (EST) Received: (qmail 10325 invoked by alias); 28 Feb 2011 14:49:08 -0000 Received: (qmail 10315 invoked by uid 22791); 28 Feb 2011 14:49:05 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL, BAYES_00, SARE_SUB_ENC_UTF8, TW_BJ, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 28 Feb 2011 14:48:52 +0000 Received: from eggs.gnu.org ([140.186.70.92]:53142) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1Pu4P4-0000Xv-B3 for gcc-patches@gnu.org; Mon, 28 Feb 2011 09:48:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pu4P2-0002pB-1R for gcc-patches@gnu.org; Mon, 28 Feb 2011 09:48:49 -0500 Received: from smtp141.iad.emailsrvr.com ([207.97.245.141]:50394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pu4P1-0002p4-T7 for gcc-patches@gnu.org; Mon, 28 Feb 2011 09:48:48 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp54.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id 354172F835B for ; Mon, 28 Feb 2011 09:48:47 -0500 (EST) Received: from dynamic5.wm-web.iad.mlsrvr.com (dynamic5.wm-web.iad1a.rsapps.net [192.168.2.146]) by smtp54.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id 1E90C2F834A for ; Mon, 28 Feb 2011 09:48:47 -0500 (EST) Received: from meta-innovation.com (localhost [127.0.0.1]) by dynamic5.wm-web.iad.mlsrvr.com (Postfix) with ESMTP id F20818D0A85 for ; Mon, 28 Feb 2011 09:48:46 -0500 (EST) Received: by www2.webmail.us (Authenticated sender: nicola.pero@meta-innovation.com, from: nicola.pero@meta-innovation.com) with HTTP; Mon, 28 Feb 2011 15:48:46 +0100 (CET) Date: Mon, 28 Feb 2011 15:48:46 +0100 (CET) Subject: =?UTF-8?Q?Fix=20libobjc's=20sel=5FgetTypedSelector?= From: "Nicola Pero" To: "gcc-patches@gnu.org" MIME-Version: 1.0 X-Type: plain Message-ID: <1298904526.98517107@www2.webmail.us> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 207.97.245.141 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This patch fixes the new function sel_getTypedSelector(). The function would return a random selector when multiple selectors with the same name but different types were in the runtime, which can crash your program. I changed it to return NULL if multiple selectors with the same type are registered in the runtime; in that case, there simply isn't any sensible way to return a typed selector that is guaranteed not to crash your program. The caller should use sel_copyTypedSelectorList() and inspect the selectors if it has some logic to pick one over the other. Testcase included. Committed to trunk. Thanks PS: This fix is very important - and also very safe because the change is limited to exactly sel_getTypedSelector() which is not used anywhere in the compiler or runtime. At worst we break that function even more. So, I approve it for going in at this stage. Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 170559) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2011-02-28 Nicola Pero + + * objc.dg/gnu-api-2-sel.m: Test that sel_getTypedSelector return + NULL in case of a selector with conflicting types. + * obj-c++.dg/gnu-api-2-sel.mm: Same change. + 2011-02-28 Kazu Hirata * gcc.target/arm/vfp-ldmdbd.c, gcc.target/arm/vfp-ldmdbs.c, Index: gcc/testsuite/objc.dg/gnu-api-2-sel.m =================================================================== --- gcc/testsuite/objc.dg/gnu-api-2-sel.m (revision 170559) +++ gcc/testsuite/objc.dg/gnu-api-2-sel.m (working copy) @@ -46,7 +46,22 @@ - (void) method { return; } @end +@interface ClassA : MyRootClass +- (id) conflictingSelectorMethod; +@end +@implementation ClassA +- (id) conflictingSelectorMethod { return nil; } +@end + +@interface ClassB : MyRootClass +- (void) conflictingSelectorMethod; +@end + +@implementation ClassB +- (void) conflictingSelectorMethod { return; } +@end + int main(int argc, void **args) { /* Functions are tested in alphabetical order. */ @@ -134,6 +149,13 @@ int main(int argc, void **args) if (selector != NULL) abort (); + + /* Now try a selector with multiple, conflicting types. NULL + should be returned. */ + selector = sel_getTypedSelector ("conflictingSelectorMethod"); + + if (selector != NULL) + abort (); } #endif Index: gcc/testsuite/obj-c++.dg/gnu-api-2-sel.mm =================================================================== --- gcc/testsuite/obj-c++.dg/gnu-api-2-sel.mm (revision 170559) +++ gcc/testsuite/obj-c++.dg/gnu-api-2-sel.mm (working copy) @@ -46,7 +46,22 @@ - (void) method { return; } @end +@interface ClassA : MyRootClass +- (id) conflictingSelectorMethod; +@end +@implementation ClassA +- (id) conflictingSelectorMethod { return nil; } +@end + +@interface ClassB : MyRootClass +- (void) conflictingSelectorMethod; +@end + +@implementation ClassB +- (void) conflictingSelectorMethod { return; } +@end + int main () { /* Functions are tested in alphabetical order. */ @@ -134,6 +149,13 @@ int main () if (selector != NULL) abort (); + + /* Now try a selector with multiple, conflicting types. NULL + should be returned. */ + selector = sel_getTypedSelector ("conflictingSelectorMethod"); + + if (selector != NULL) + abort (); } #endif Index: libobjc/selector.c =================================================================== --- libobjc/selector.c (revision 170559) +++ libobjc/selector.c (working copy) @@ -369,6 +369,7 @@ sel_getTypedSelector (const char *name) if (i != 0) { struct objc_list *l; + SEL returnValue = NULL; for (l = (struct objc_list *) sarray_get_safe (__objc_selector_array, i); l; l = l->tail) @@ -376,10 +377,40 @@ sel_getTypedSelector (const char *name) SEL s = (SEL) l->head; if (s->sel_types) { - objc_mutex_unlock (__objc_runtime_mutex); - return s; + if (returnValue == NULL) + { + /* First typed selector that we find. Keep it in + returnValue, but keep checking as we want to + detect conflicts. */ + returnValue = s; + } + else + { + /* We had already found a typed selectors, so we + have multiple ones. Double-check that they have + different types, just in case for some reason we + got duplicates with the same types. If so, it's + OK, we'll ignore the duplicate. */ + if (returnValue->sel_types == s->sel_types) + continue; + else if (sel_types_match (returnValue->sel_types, s->sel_types)) + continue; + else + { + /* The types of the two selectors are different; + it's a conflict. Too bad. Return NULL. */ + objc_mutex_unlock (__objc_runtime_mutex); + return NULL; + } + } } } + + if (returnValue != NULL) + { + objc_mutex_unlock (__objc_runtime_mutex); + return returnValue; + } } /* No typed selector found. Return NULL. */ Index: libobjc/ChangeLog =================================================================== --- libobjc/ChangeLog (revision 170561) +++ libobjc/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2011-02-28 Nicola Pero + + * selector.c (sel_getTypedSelector): Return NULL if there are + multiple selectors with conflicting types. + * objc/runtime.h (sel_getTypedSelector): Updated documentation. + 2011-02-28 Richard Frith-Macdonald PR libobjc/47922 Index: libobjc/objc/runtime.h =================================================================== --- libobjc/objc/runtime.h (revision 170559) +++ libobjc/objc/runtime.h (working copy) @@ -226,14 +226,16 @@ objc_EXPORT SEL * sel_copyTypedSelectorList (const unsigned int *numberOfReturnedSelectors); /* Return a selector with name 'name' and a non-zero type encoding, if - any such selector is registered with the runtime. If there is no - such selector, NULL is returned. Return NULL if 'name' is NULL. + there is a single selector with a type, and with that name, + registered with the runtime. If there is no such selector, or if + there are multiple selectors with the same name but conflicting + types, NULL is returned. Return NULL if 'name' is NULL. This is useful if you have the name of the selector, and would really like to get a selector for it that includes the type encoding. Unfortunately, if the program contains multiple selector - with the same name but different types, sel_getTypedSelector - returns a random one of them, which may not be the right one. + with the same name but different types, sel_getTypedSelector can + not possibly know which one you need, and so will return NULL. Compatibility Note: the Apple/NeXT runtime has untyped selectors, so it does not have this function, which is specific to the GNU