Patchwork libobjc: do not abort upon finding a duplicate class

login
register
mail settings
Submitter Nicola Pero
Date Dec. 22, 2010, 11:06 p.m.
Message ID <1293059164.640917946@192.168.2.228>
Download mbox | patch
Permalink /patch/76455/
State New
Headers show

Comments

Nicola Pero - Dec. 22, 2010, 11:06 p.m.
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

Patch

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  <nicola.pero@meta-innovation.com>
 
+       * 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  <nicola.pero@meta-innovation.com>
+
        * objc-private/objc-list.h: Reindented file.  No code changes.
        * objc-private/sarray.h: Same change.