This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Patch: fix objc/formal-protocol-6.m


Hi,

since we are in bug fixing stage, I spent some time to fix the
objc/formal-protocol-6.m testcase, which has always been failing with GCC
and the GNU runtime I think.  It is also present on GNATS as PR/1727.

The bug was that whenever a @protocol() expression was found, and no
class implementing the referenced protocol was being compiled in that
compilation unit, the compiled program would crash at runtime.  As
also noted by the author of PR/1727, this is due to the fact that the
statically allocated Protocol object instance which is automatically
generated by the compiler is not properly setup at runtime: its 'isa'
pointer is not fixed up properly to point to the Protocol class.

Protocol instances compiled because there are classes implementing
them are properly fixed up, since the class contains a reference to
the Protocol object, and the GNU runtime loops on all Protocols in a
class and fixes them up when loading a class (the same of course for
categories).

To have protocol instances compiled because there is a @protocol()
expression referring to them fixed up (even if there is no class
containing a reference to them), I modified the compiler to add them
to the list of statically allocated instances.  The list of statically
allocated instances is a list of objects to be fixed up at runtime by
the GNU runtime library; in the GNU runtime is used for statically
allocated NSConstantString objects at the moment, but it's naturally
written in such a way as to work with objects of other classes as well
- in particular Protocol objects.

So, I simply modified build_protocol_expr() to create a list of
statically allocated Protocol instance, and to add a reference to the
specific instance in this list.  Everything was already setup to
support it.

I rebuilt the compiler with this change, run all the ObjC testcases, and
they all passed - including the formal-protocol-6.m one (Yuppie!)  :-)

The change is backward compatible with older runtime libraries too.

Ok to commit ?

We can then remove the testsuite/objc/formal-protocol-6.x file, and
close PR/1727.

I did also add some verbose comments in the objc-act.c file. It's very
difficult for everyone to pick up on the ObjC compiler code, I think
mainly because it's very poorly commented, so I'm trying to push verbose
comments in 

Btw, I'm also still waiting for approval for the bugfix of PR/5956,
http://gcc.gnu.org/ml/gcc-patches/2002-07/msg00143.html

Sat Aug 31 20:56:36 2002  Nicola Pero  <n.pero@mi.flashnet.it>

        Fixed PR/1727 and long-standing failing testcase
        objc/formal-protocol-6.m.
        * objc-act.c (build_protocol_expr): If compiling for the GNU
        runtime, create a list of Protocol statically allocated instances
        if it doesn't exist, then add the Protocol object to this same
        list.
        (get_objc_string_decl): Fixed typo/bug - TREE_VALUE had been used
        instead of TREE_CHAIN.
        

Index: objc-act.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/objc/objc-act.c,v
retrieving revision 1.154
diff -u -r1.154 objc-act.c
--- objc-act.c  27 Aug 2002 21:57:47 -0000      1.154
+++ objc-act.c  31 Aug 2002 19:23:55 -0000
@@ -1679,7 +1679,7 @@
   else
     abort ();
 
-  for (; chain != 0; chain = TREE_VALUE (chain))
+  for (; chain != 0; chain = TREE_CHAIN (chain))
     if (TREE_VALUE (chain) == ident)
       return (TREE_PURPOSE (chain));
 
@@ -1721,6 +1721,7 @@
 
       /* Output {class_name, ...}.  */
       class = TREE_VALUE (cl_chain);
+
       class_name = get_objc_string_decl (TYPE_NAME (class), class_names);
       initlist = build_tree_list (NULL_TREE,
                                  build_unary_op (ADDR_EXPR, class_name,
1));
@@ -2867,6 +2868,42 @@
     }
 }
 
+/* For each protocol which was referenced either from a @protocol()
+   expression, or because a class/category implements it (then a
+   pointer to the protocol is stored in the struct describing the
+   class/category), we create a statically allocated instance of the
+   Protocol class.  The code is written in such a way as to generate
+   as few Protocol objects as possible; we generate a unique Protocol
+   instance for each protocol, and we don't generate a Protocol
+   instance if the protocol is never referenced (either from a
+   @protocol() or from a class/category implementation).  These
+   statically allocated objects can be referred to via the static
+   (that is, private to this module) symbols _OBJC_PROTOCOL_n.
+   
+   The statically allocated Protocol objects that we generate here
+   need to be fixed up at runtime in order to be used: the 'isa'
+   pointer of the objects need to be set up to point to the 'Protocol'
+   class, as known at runtime.
+
+   The NeXT runtime, I believe, uses some low-level trick to look up
+   all those symbols, then loops on them and fixes them up.
+
+   The GNU runtime requires pointers to those symbols to be put in the
+   objc_symtab (which is then passed as argument to the function
+   __objc_exec_class() which the compiler sets up to be executed
+   automatically when the module is loaded); setup of those Protocol
+   objects happen in two ways in the GNU runtime: all Protocol objects
+   referred to by a class or category implementation are fixed up when
+   the class/category is loaded; all Protocol objects referred to by a
+   @protocol() expression are added by the compiler to the list of
+   statically allocated instances to fixup (the same list holding the
+   statically allocated constant string objects).  Because, as
+   explained above, the compiler generates as few Protocol objects as
+   possible, some Protocol object might end up being referenced
+   multiple times when compiled with the GNU runtime, and end up being
+   fixed up multiple times at runtime inizialization.  But that
+   doesn't hurt, it's just a little inefficient.
+*/
 static void
 generate_protocols ()
 {
@@ -5081,6 +5118,9 @@
   PROTOCOL_FORWARD_DECL (p) = decl;
 }
 
+/* This function is called by the parser when (and only when) a
+   @protocol() expression is found, in order to compile it.
+ */
 tree
 build_protocol_expr (protoname)
      tree protoname;
@@ -5102,6 +5142,50 @@
 
   TREE_TYPE (expr) = protocol_type;
 
+  /* The @protocol() expression is being compiled into a pointer
+     to a statically allocated instance of the Protocol class.
+     To become usable at runtime, the 'isa' pointer of the instance need
+     to be fixed up at runtime by the runtime library, to point to
+     the actual 'Protocol' class.  */
+
+  /* For the GNU runtime, put the static Protocol instance in the list
+     of statically allocated instances, so that we make sure that its
+     'isa' pointer is fixed up at runtime by the GNU runtime library
+     to point to the Protocol class (at runtime, when loading the module,
+     the GNU runtime library loops on the statically allocated instances
+     (as found in the defs field in objc_symtab) and fixups all the 'isa'
+     pointers of those objects).  */
+  if (! flag_next_runtime)
+    {
+      /* This type is a struct containing the fields of a Protocol
+        object.  (Cfr. protocol_type instead is the type of a pointer
+        to such a struct).  */
+      tree protocol_struct_type = xref_tag 
+       (RECORD_TYPE, get_identifier (PROTOCOL_OBJECT_CLASS_NAME));
+      tree *chain;
+      
+      /* Look for the list of Protocol statically allocated instances
+        to fixup at runtime.  Create a new list to hold Protocol
+        statically allocated instances, if the list is not found.  At
+        present there is only another list, holding NSConstantString
+        static instances to be fixed up at runtime.  */
+      for (chain = &objc_static_instances;
+          *chain && TREE_VALUE (*chain) != protocol_struct_type;
+          chain = &TREE_CHAIN (*chain));
+      if (!*chain)
+       {
+         *chain = tree_cons (NULL_TREE, protocol_struct_type, NULL_TREE);
+         add_objc_string (TYPE_NAME (protocol_struct_type),
+                          class_names);
+       }
+      
+      /* Add this statically allocated instance to the Protocol
+        list.  */
+      TREE_PURPOSE (*chain) = tree_cons (NULL_TREE, 
+                                        PROTOCOL_FORWARD_DECL (p),
+                                        TREE_PURPOSE (*chain));
+    }
+  
   return expr;
 }



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]