From patchwork Wed Dec 22 23:06:04 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Pero X-Patchwork-Id: 76455 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 910B8B70DA for ; Thu, 23 Dec 2010 10:06:19 +1100 (EST) Received: (qmail 13438 invoked by alias); 22 Dec 2010 23:06:17 -0000 Received: (qmail 13422 invoked by uid 22791); 22 Dec 2010 23:06:15 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL, BAYES_00, SARE_MONEYTERMS, 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; Wed, 22 Dec 2010 23:06:09 +0000 Received: from eggs.gnu.org ([140.186.70.92]:40003) by fencepost.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1PVXky-0000eP-MH for gcc-patches@gnu.org; Wed, 22 Dec 2010 18:06:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PVXkz-0005En-JF for gcc-patches@gnu.org; Wed, 22 Dec 2010 18:06:07 -0500 Received: from smtp131.iad.emailsrvr.com ([207.97.245.131]:55222) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PVXkz-0005Ei-Eq for gcc-patches@gnu.org; Wed, 22 Dec 2010 18:06:05 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp43.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id DB55E2D0561 for ; Wed, 22 Dec 2010 18:06:04 -0500 (EST) Received: from dynamic1.wm-web.iad.mlsrvr.com (dynamic1.wm-web.iad1a.rsapps.net [192.168.2.150]) by smtp43.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id C88582D0519 for ; Wed, 22 Dec 2010 18:06:04 -0500 (EST) Received: from meta-innovation.com (localhost [127.0.0.1]) by dynamic1.wm-web.iad.mlsrvr.com (Postfix) with ESMTP id 9D976C98070 for ; Wed, 22 Dec 2010 18:06:04 -0500 (EST) Received: by www2.webmail.us (Authenticated sender: nicola.pero@meta-innovation.com, from: nicola.pero@meta-innovation.com) with HTTP; Thu, 23 Dec 2010 00:06:04 +0100 (CET) Date: Thu, 23 Dec 2010 00:06:04 +0100 (CET) Subject: libobjc: do not abort upon finding a duplicate class From: "Nicola Pero" To: "gcc-patches@gnu.org" MIME-Version: 1.0 X-Type: plain Message-ID: <1293059164.640917946@192.168.2.228> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) 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 is a follow-up to the one that fixed libobjc/18764 ("segfault in libobjc when sending a message to a conflicting class"). The previous patch fixed the segmentation fault by aborting the program when a duplicate class was found. But I just realized that GNUstep did rely on ignoring duplicate classes in the past on some platforms for NXConstantString (where, eg, libobjc and gnustep-base would provide multiple implementations with one overriding the other), and I don't want to break that behaviour; these NXConstantString hacks are really obsolete, but being able to replace a class in a shared library with another one by linking a new one in the right order may turn up to be useful again at some point in the future to someone, so I don't want to break it (at least not on purpose). So, this patch makes all the necessary adjustments so that when a duplicate class is found, the duplicate class is completely ignored. There are no segfaults as the code now keeps track of duplicate classes and doesn't execute the +load, or the load callback, on the duplicate class. It now looks quite good. :-) Committed to trunk. Thanks Index: init.c =================================================================== --- init.c (revision 168182) +++ init.c (working copy) @@ -56,6 +56,16 @@ static struct objc_list *unclaimed_proto_list = 0; /* List of unresolved static instances. */ static struct objc_list *uninitialized_statics = 0; /* !T:MUTEX */ +/* List of duplicated classes found while loading modules. If we find + a class twice, we ignore it the second time. On some platforms, + where the order in which modules are loaded is well defined, this + allows you to replace a class in a shared library by linking in a + new implementation which is loaded in in the right order, and which + overrides the existing one. + + Protected by __objc_runtime_mutex. */ +static cache_ptr duplicate_classes = NULL; + /* Global runtime "write" mutex. Having a single mutex prevents deadlocks, but reduces concurrency. To improve concurrency, some groups of functions in the runtime have their own separate mutex @@ -87,7 +97,7 @@ static void __objc_class_add_protocols (Class, str /* Load callback hook. */ void (*_objc_load_callback) (Class class, struct objc_category *category) = 0; /* !T:SAFE */ -/* Are all categories/classes resolved? */ +/* Are all categories/classes resolved ? */ BOOL __objc_dangling_categories = NO; /* !T:UNUSED */ /* Sends +load to all classes and categories in certain @@ -110,9 +120,11 @@ static void __objc_call_load_callback (struct objc installed in the runtime. */ static BOOL class_is_subclass_of_class (Class class, Class superclass); -typedef struct objc_class_tree { +typedef struct objc_class_tree +{ Class class; - struct objc_list *subclasses; /* `head' is pointer to an objc_class_tree */ + struct objc_list *subclasses; /* `head' is a pointer to an + objc_class_tree. */ } objc_class_tree; /* This is a linked list of objc_class_tree trees. The head of these @@ -583,6 +595,9 @@ __objc_exec_class (struct objc_module *module) __objc_init_selector_tables (); __objc_init_class_tables (); __objc_init_dispatch_tables (); + duplicate_classes = objc_hash_new (8, + (hash_func_type)objc_hash_ptr, + objc_compare_ptrs); __objc_class_tree_list = list_cons (NULL, __objc_class_tree_list); __objc_load_methods = objc_hash_new (128, (hash_func_type)objc_hash_ptr, @@ -619,14 +634,15 @@ __objc_exec_class (struct objc_module *module) isn't and this crashes the program. */ class->subclass_list = NULL; - __objc_init_class (class); + if (__objc_init_class (class)) + { + /* Check to see if the superclass is known in this point. If + it's not add the class to the unresolved_classes list. */ + if (superclass && ! objc_getClass (superclass)) + unresolved_classes = list_cons (class, unresolved_classes); + } + } - /* Check to see if the superclass is known in this point. If - it's not add the class to the unresolved_classes list. */ - if (superclass && ! objc_getClass (superclass)) - unresolved_classes = list_cons (class, unresolved_classes); - } - /* Process category information from the module. */ for (i = 0; i < symtab->cat_def_cnt; ++i) { @@ -637,7 +653,6 @@ __objc_exec_class (struct objc_module *module) methods. */ if (class) { - DEBUG_PRINTF ("processing categories from (module,object): %s, %s\n", module->name, class->name); @@ -808,7 +823,8 @@ __objc_create_classes_tree (struct objc_module *mo { Class class = (Class) symtab->defs[i]; - objc_tree_insert_class (class); + if (!objc_hash_is_key_in_hash (duplicate_classes, class)) + objc_tree_insert_class (class); } /* Now iterate over "claimed" categories too (ie, categories that @@ -845,9 +861,12 @@ __objc_call_load_callback (struct objc_module *mod for (i = 0; i < symtab->cls_def_cnt; i++) { Class class = (Class) symtab->defs[i]; - - /* Call the _objc_load_callback for this class. */ - _objc_load_callback (class, 0); + + if (!objc_hash_is_key_in_hash (duplicate_classes, class)) + { + /* Call the _objc_load_callback for this class. */ + _objc_load_callback (class, 0); + } } /* Call the _objc_load_callback for categories. Don't register @@ -874,8 +893,11 @@ init_check_module_version (struct objc_module *mod } } -/* __objc_init_class must be called with __objc_runtime_mutex already locked. */ -void +/* __objc_init_class must be called with __objc_runtime_mutex already + locked. Return YES if the class could be setup; return NO if the + class could not be setup because a class with the same name already + exists. */ +BOOL __objc_init_class (Class class) { /* Store the class in the class table and assign class numbers. */ @@ -895,10 +917,16 @@ __objc_init_class (Class class) if (class->protocols) __objc_init_protocols (class->protocols); + + return YES; } else - _objc_abort ("Module contains duplicate class '%s'\n", - class->name); + { + /* The module contains a duplicate class. Remember it so that + we will ignore it later. */ + objc_hash_add (&duplicate_classes, class, class); + return NO; + } } /* __objc_init_protocol must be called with __objc_runtime_mutex Index: objc-private/runtime.h =================================================================== --- objc-private/runtime.h (revision 168182) +++ objc-private/runtime.h (working copy) @@ -57,7 +57,7 @@ extern void __objc_update_dispatch_table_for_class extern int __objc_init_thread_system (void); /* thread.c */ extern int __objc_fini_thread_system (void); /* thread.c */ -extern void __objc_init_class (Class class); /* init.c */ +extern BOOL __objc_init_class (Class class); /* init.c */ extern void class_add_method_list (Class, struct objc_method_list *); /* Registering instance methods as class methods for root classes */ Index: ChangeLog =================================================================== --- ChangeLog (revision 168182) +++ ChangeLog (working copy) @@ -1,5 +1,17 @@ 2010-12-22 Nicola Pero + * init.c (duplicate_classes): New. + (__objc_exec_class): Initialize duplicate_classes. + (__objc_create_classes_tree): Ignore classes in the + duplicate_classes table. + (__objc_call_load_callback): Same. + (__objc_init_class): If a duplicate class is found, add it to + duplicate_classes instead of aborting. Return YES if the class is + not a duplicate, and NO if it is. + * objc-private/runtime.h (__objc_init_class): Updated prototype. + +2010-12-22 Nicola Pero + * objc-private/objc-list.h: Reindented file. No code changes. * objc-private/sarray.h: Same change.