Patchwork Fix for PR libobjc/45953

login
register
mail settings
Submitter Nicola Pero
Date Dec. 21, 2010, 1:44 p.m.
Message ID <1292939063.40476389@192.168.2.228>
Download mbox | patch
Permalink /patch/76292/
State New
Headers show

Comments

Nicola Pero - Dec. 21, 2010, 1:44 p.m.
This patch fixes libobjc/45953.  A testcase (fixed by the patch) is included.

Committed to trunk.

Thanks

Patch

Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog     (revision 168113)
+++ gcc/testsuite/ChangeLog     (working copy)
@@ -1,3 +1,8 @@ 
+2010-12-21  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR libobjc/45953
+       * objc.dg/libobjc-selector-1.m: New test.
+
 2010-12-21  Jakub Jelinek  <jakub@redhat.com>
 
        PR middle-end/45852
Index: gcc/testsuite/objc.dg/libobjc-selector-1.m
===================================================================
--- gcc/testsuite/objc.dg/libobjc-selector-1.m  (revision 0)
+++ gcc/testsuite/objc.dg/libobjc-selector-1.m  (revision 0)
@@ -0,0 +1,39 @@ 
+/* Test a little inefficiency that was fixed in libobjc when dealing
+   with selectors (PR libobjc/45953).  */
+
+/* { dg-do run } */
+/* { dg-skip-if "" { *-*-* } { "-fnext-runtime" } { "" } } */
+
+/* To get the modern GNU Objective-C Runtime API, you include
+   objc/runtime.h.  */
+#include <objc/runtime.h>
+#include <stdlib.h>
+
+/* Test that registering a new selector, with the same name but a
+   different type than the previous one, does not change the original
+   name string.  It is actually fine to change it (there is no
+   guarantee that it won't change), except for runtime performance /
+   memory consumption, since changing it means that the runtime is
+   doing an unneeded objc_malloc()/strcpy(), which is inefficient.  */
+
+int main (void)
+{
+  SEL selector_1;
+  SEL selector_2;
+  const char *name_1;
+  const char *name_2;
+
+  /* These method type strings may well be invalid.  Please don't use
+     them as examples.  They are irrelevant for this test; any string
+     will do.  */
+  selector_1 = sel_registerTypedName ("method", "v@:");
+  name_1 = sel_getName (selector_1);
+
+  selector_2 = sel_registerTypedName ("method", "i@:");
+  name_2 = sel_getName (selector_1);
+
+  if (name_1 != name_2)
+    abort ();
+
+  return 0;
+}
Index: libobjc/selector.c
===================================================================
--- libobjc/selector.c  (revision 168113)
+++ libobjc/selector.c  (working copy)
@@ -451,17 +451,19 @@  __sel_register_typed_name (const char *name, const
   i = (sidx) objc_hash_value_for_key (__objc_selector_hash, name);
   if (soffset_decode (i) != 0)
     {
-      for (l = (struct objc_list *) sarray_get_safe (__objc_selector_array, i);
+      /* There are already selectors with that name.  Examine them to
+        see if the one we're registering already exists.  */
+      for (l = (struct objc_list *)sarray_get_safe (__objc_selector_array, i);
           l; l = l->tail)
        {
-         SEL s = (SEL) l->head;
+         SEL s = (SEL)l->head;
          if (types == 0 || s->sel_types == 0)
            {
              if (s->sel_types == types)
                {
                  if (orig)
                    {
-                     orig->sel_id = (void *) i;
+                     orig->sel_id = (void *)i;
                      return orig;
                    }
                  else
@@ -472,79 +474,93 @@  __sel_register_typed_name (const char *name, const
            {
              if (orig)
                {
-                 orig->sel_id = (void *) i;
+                 orig->sel_id = (void *)i;
                  return orig;
                }
              else
                return s;
            }
        }
+      /* A selector with this specific name/type combination does not
+        exist yet.  We need to register it.  */
       if (orig)
        j = orig;
       else
        j = pool_alloc_selector ();
       
-      j->sel_id = (void *) i;
-      /* Can we use the pointer or must copy types?  Don't copy if
+      j->sel_id = (void *)i;
+      /* Can we use the pointer or must we copy types ?  Don't copy if
         NULL.  */
       if ((is_const) || (types == 0))
-       j->sel_types = (const char *) types;
+       j->sel_types = types;
       else
        {
-         j->sel_types = (char *) objc_malloc (strlen (types) + 1);
-         strcpy ((char *) j->sel_types, types);
+         j->sel_types = (char *)objc_malloc (strlen (types) + 1);
+         strcpy ((char *)j->sel_types, types);
        }
-      l = (struct objc_list *) sarray_get_safe (__objc_selector_array, i);
+      l = (struct objc_list *)sarray_get_safe (__objc_selector_array, i);
     }
   else
     {
+      /* There are no other selectors with this name registered in the
+        runtime tables.  */
+      const char *new_name;
+
+      /* Determine i.  */
       __objc_selector_max_index += 1;
       i = soffset_encode (__objc_selector_max_index);
+
+      /* Prepare the selector.  */
       if (orig)
        j = orig;
       else
        j = pool_alloc_selector ();
       
-      j->sel_id = (void *) i;
-      /* Can we use the pointer or must copy types?  Don't copy if
+      j->sel_id = (void *)i;
+      /* Can we use the pointer or must we copy types ?  Don't copy if
         NULL.  */
-      if ((is_const) || (types == 0))
-       j->sel_types = (const char *) types;
+      if (is_const || (types == 0))
+       j->sel_types = types;
       else
        {
-         j->sel_types = (char *) objc_malloc (strlen (types) + 1);
-         strcpy ((char *) j->sel_types, types);
+         j->sel_types = (char *)objc_malloc (strlen (types) + 1);
+         strcpy ((char *)j->sel_types, types);
        }
+
+      /* Since this is the first selector with this name, we need to
+        register the correspondence between 'i' (the sel_id) and
+        'name' (the actual string) in __objc_selector_names and
+        __objc_selector_hash.  */
+      
+      /* Can we use the pointer or must we copy name ?  Don't copy if
+        NULL.  (FIXME: Can the name really be NULL here ?)  */
+      if (is_const || (name == 0))
+       new_name = name;
+      else
+       {
+         new_name = (char *)objc_malloc (strlen (name) + 1);
+         strcpy ((char *)new_name, name);
+       }
+      
+      /* This maps the sel_id to the name.  */
+      sarray_at_put_safe (__objc_selector_names, i, (void *)new_name);
+
+      /* This maps the name to the sel_id.  */
+      objc_hash_add (&__objc_selector_hash, (void *)new_name, (void *)i);
+
       l = 0;
     }
 
   DEBUG_PRINTF ("Record selector %s[%s] as: %ld\n", name, types, 
-               (long) soffset_decode (i));
-  
-  {
-    int is_new = (l == 0);
-    const char *new_name;
+               (long)soffset_decode (i));
 
-    /* Can we use the pointer or must copy name?  Don't copy if
-       NULL.  */
-    if ((is_const) || (name == 0))
-      new_name = name;
-    else
-      {
-       new_name = (char *) objc_malloc (strlen (name) + 1);
-       strcpy ((char *) new_name, name);
-      }
-    
-    l = list_cons ((void *) j, l);
-    sarray_at_put_safe (__objc_selector_names, i, (void *) new_name);
-    sarray_at_put_safe (__objc_selector_array, i, (void *) l);
-    if (is_new)
-      objc_hash_add (&__objc_selector_hash, (void *) new_name, (void *) i);
-  }
-  
+  /* Now add the selector to the list of selectors with that id.  */
+  l = list_cons ((void *)j, l);
+  sarray_at_put_safe (__objc_selector_array, i, (void *)l);
+
   sarray_realloc (__objc_uninstalled_dtable, __objc_selector_max_index + 1);
   
-  return (SEL) j;
+  return (SEL)j;
 }
 
 SEL
Index: libobjc/ChangeLog
===================================================================
--- libobjc/ChangeLog   (revision 168113)
+++ libobjc/ChangeLog   (working copy)
@@ -1,5 +1,13 @@ 
 2010-12-21  Nicola Pero  <nicola.pero@meta-innovation.com>
 
+       PR libobjc/45953
+       * selector.c (__sel_register_typed_name): When registering a new
+       selector with the same name as an existing one, reuse the existing
+       name string.  Also updated types, casts and comments in the whole
+       function.
+
+2010-12-21  Nicola Pero  <nicola.pero@meta-innovation.com>
+
        * objc-private/module-abi-8.h (struct objc_symtab): Declare 'refs'
        to be 'struct objc_selector *' and not 'SEL'.
        * init.c (__objc_exec_class): Call