Re: gcc 2.8.0 is broken on linux/x86 and more bug

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[H.J. Lu]

The good news is turn off address-off fixes the bug. The bad news
is I can tell from the asm code what is wrong, but it is hard
to generate a small test case and you may have to look at the
asm code directly to verify the bug if you cannot compile glibc 2.1
on Linux/x86.

__register_frame () change in gcc 2.8.0 causes a big problem.
Since __register_frame () called in crtbegin.o is in libgcc2.c and
gcc -shared includes crtbegin.o and -lgcc, any libc.so built by
previous gcc has __register_frame () in libc.so. Now the ABI for
__register_frame () is changed, that is a big mess for Linux since
shared libraries are used a lot. When the ABI of a function in
shared library is changed like this, it doesn't work too well.

Here is the patch. It should apply to both gcc 2.8.0 and egcs.

Thanks.


H.J.
----
Mon Dec  8 15:42:20 1997  H.J. Lu  (hjl@gnu.org)

	* crtstuff.c (__register_frame): Changed to __register_frame_new.
	(__deregister_frame): Changed to __deregister_frame_new.
	* frame.c: Ditto.
	* frame.h: Ditto.
	* collect2.c: Ditto.

	* frame.c (__register_frame_table): Changed to
	__register_frame_table_new.
	* collect2.c: Ditto.

	* frame.c (__register_frame_new, __register_frame_table_new,
	__deregister_frame_new): Added.
	* frame.h: Ditto.

Index: crtstuff.c
===================================================================
RCS file: /home/work/cvs/gnu/egcs/gcc/crtstuff.c,v
retrieving revision 1.1.1.4
diff -u -r1.1.1.4 crtstuff.c
--- crtstuff.c	1997/12/08 17:04:06	1.1.1.4
+++ crtstuff.c	1997/12/08 23:26:29
@@ -142,7 +142,7 @@
     }
 
 #ifdef EH_FRAME_SECTION_ASM_OP
-  __deregister_frame (__EH_FRAME_BEGIN__);
+  __deregister_frame_new (__EH_FRAME_BEGIN__);
 #endif
   completed = 1;
 }
@@ -170,7 +170,7 @@
 frame_dummy ()
 {
   static struct object object;
-  __register_frame (__EH_FRAME_BEGIN__, &object);
+  __register_frame_new (__EH_FRAME_BEGIN__, &object);
 }
 
 static void
@@ -254,7 +254,7 @@
     (*p) ();
 
 #ifdef EH_FRAME_SECTION_ASM_OP
-  __deregister_frame (__EH_FRAME_BEGIN__);
+  __deregister_frame_new (__EH_FRAME_BEGIN__);
 #endif
 }
 #endif
@@ -395,7 +396,7 @@
   func_ptr *p;
 #ifdef EH_FRAME_SECTION_ASM_OP
   static struct oobject object;
-  __register_frame (__EH_FRAME_BEGIN__, &object);
+  __register_frame_new (__EH_FRAME_BEGIN__, &object);
 #endif
   for (p = __CTOR_END__ - 1; *p != (func_ptr) -1; p--)
     (*p) ();
Index: frame.c
===================================================================
RCS file: /home/work/cvs/gnu/egcs/gcc/frame.c,v
retrieving revision 1.1.1.6
diff -u -r1.1.1.6 frame.c
--- frame.c	1997/12/08 17:04:32	1.1.1.6
+++ frame.c	1997/12/08 23:29:53
@@ -501,7 +501,7 @@
 /* Called from crtbegin.o to register the unwind info for an object.  */
 
 void
-__register_frame (void *begin, struct object *ob)
+__register_frame_new (void *begin, struct object *ob)
 {
   ob->fde_begin = begin;
 
@@ -513,12 +513,19 @@
   objects = ob;
 }
 
+void
+__register_frame (void *begin)
+{
+  struct object *ob = (struct object *) malloc (sizeof (struct object));
+  __register_frame_new (begin, ob);
+}
+
 /* Similar, but BEGIN is actually a pointer to a table of unwind entries
    for different translation units.  Called from the file generated by
    collect2.  */
 
 void
-__register_frame_table (void *begin, struct object *ob)
+__register_frame_table_new (void *begin, struct object *ob)
 {
   ob->fde_begin = begin;
   ob->fde_array = begin;
@@ -530,10 +537,17 @@
   objects = ob;
 }
 
+void
+__register_frame_table (void *begin)
+{
+  struct object *ob = (struct object *) malloc (sizeof (struct object));
+  __register_frame_table_new (begin, ob);
+}
+
 /* Called from crtend.o to deregister the unwind info for an object.  */
 
-void
-__deregister_frame (void *begin)
+void *
+__deregister_frame_new (void *begin)
 {
   struct object **p = &objects;
 
@@ -548,11 +562,17 @@
 	  if (ob->pc_begin)
 	    free (ob->fde_array);
 
-	  return;
+	  return (void *) ob;
 	}
       p = &((*p)->next);
     }
   abort ();
+}
+
+void
+__deregister_frame (void *begin)
+{
+  free (__deregister_frame_new (begin));
 }
 
 /* Called from __throw to find the registers to restore for a given
Index: frame.h
===================================================================
RCS file: /home/work/cvs/gnu/egcs/gcc/frame.h,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 frame.h
--- frame.h	1997/12/08 17:04:33	1.1.1.2
+++ frame.h	1997/12/08 23:31:02
@@ -37,11 +37,16 @@
 /* Called either from crtbegin.o or a static constructor to register the
    unwind info for an object or translation unit, respectively.  */
 
-extern void __register_frame (void *, struct object *);
+extern void __register_frame_new (void *, struct object *);
+extern void __register_frame (void *);
 
 /* Called from crtend.o to deregister the unwind info for an object.  */
 
+extern void * __deregister_frame_new (void *);
 extern void __deregister_frame (void *);
+
+extern void __register_frame_table_new (void *, struct object *);
+extern void __register_frame_table (void *);
 
 /* Called from __throw to find the registers to restore for a given
    PC_TARGET.  The caller should allocate a local variable of `struct
Index: collect2.c
===================================================================
RCS file: /home/work/cvs/gnu/egcs/gcc/collect2.c,v
retrieving revision 1.1.1.4
diff -u -r1.1.1.4 collect2.c
--- collect2.c	1997/12/08 17:04:00	1.1.1.4
+++ collect2.c	1997/12/12 02:41:38
@@ -1790,16 +1790,16 @@
       fprintf (stream, "  struct object *next;\n");
       fprintf (stream, "};\n");
 
-      fprintf (stream, "extern void __register_frame_table (void *, struct object *);\n");
-      fprintf (stream, "extern void __deregister_frame (void *);\n");
+      fprintf (stream, "extern void __register_frame_table_new (void *, struct object *);\n");
+      fprintf (stream, "extern void *__deregister_frame_new (void *);\n");
 
       fprintf (stream, "static void reg_frame () {\n");
       fprintf (stream, "\tstatic struct object ob;\n");
-      fprintf (stream, "\t__register_frame_table (frame_table, &ob);\n");
+      fprintf (stream, "\t__register_frame_table_new (frame_table, &ob);\n");
       fprintf (stream, "\t}\n");
 
       fprintf (stream, "static void dereg_frame () {\n");
-      fprintf (stream, "\t__deregister_frame (frame_table);\n");
+      fprintf (stream, "\t__deregister_frame_new (frame_table);\n");
       fprintf (stream, "\t}\n");
     }
 
@@ -1866,15 +1866,16 @@
       write_list (stream, "\t\t&", frame_tables.first);
       fprintf (stream, "\t0\n};\n");
 
-      fprintf (stream, "extern void __register_frame_table (void *);\n");
-      fprintf (stream, "extern void __deregister_frame (void *);\n");
+      fprintf (stream, "extern void __register_frame_table_new (void *, struct object *);\n");
+      fprintf (stream, "extern void *__deregister_frame_new (void *);\n");
 
       fprintf (stream, "static void reg_frame () {\n");
-      fprintf (stream, "\t__register_frame_table (frame_table);\n");
+      fprintf (stream, "\tstatic struct object ob;\n");
+      fprintf (stream, "\t__register_frame_table_new (frame_table, &ob);\n");
       fprintf (stream, "\t}\n");
 
       fprintf (stream, "static void dereg_frame () {\n");
-      fprintf (stream, "\t__deregister_frame (frame_table);\n");
+      fprintf (stream, "\t__deregister_frame_new (frame_table);\n");
       fprintf (stream, "\t}\n");
     }
 

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[Jeffrey A Law]

  In message < m0xgM3M-0004ecC@ocean.lucon.org >you write:
  > __register_frame () change in gcc 2.8.0 causes a big problem.
  > Since __register_frame () called in crtbegin.o is in libgcc2.c and
  > gcc -shared includes crtbegin.o and -lgcc, any libc.so built by
  > previous gcc has __register_frame () in libc.so. Now the ABI for
  > __register_frame () is changed, that is a big mess for Linux since
  > shared libraries are used a lot. When the ABI of a function in
  > shared library is changed like this, it doesn't work too well.
It seems to me that we need two interfaces one with the old way one
with the new way.

Furthermore, we must avoid calling "free" on any of the objects which
come from static storage instead of malloc.  Thus, we have to keep track
of which interface was used for each object so that we know whether or
not its memory should be free'd.

The compiler should always emit calls to the new interfaces; the old
interfaces would be for existing copies of ld.so which use the old
interfaces.

jeff

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[Jeffrey A Law]

  In message < m0xgQ0h-0004ecC@ocean.lucon.org >you write:
  > > Furthermore, we must avoid calling "free" on any of the objects which
  > > come from static storage instead of malloc.  Thus, we have to keep track
  > > of which interface was used for each object so that we know whether or
  > > not its memory should be free'd.
  > > 
  > > The compiler should always emit calls to the new interfaces; the old
  > > interfaces would be for existing copies of ld.so which use the old
  > > interfaces.
  > > 
  > 
  > I think that is what my patch does. It keeps the old name and
  > appends _new to new interface. It seems to work for me.
I don't believe it handles the "free" issue correctly, then again, I'm
not real familiar with that code.

Please explain to me how it works if both register_frame and
register_frame_new are are called by one program.  

jeff

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[H.J. Lu]

>   > I think that is what my patch does. It keeps the old name and
>   > appends _new to new interface. It seems to work for me.
> I don't believe it handles the "free" issue correctly, then again, I'm
> not real familiar with that code.
> 
> Please explain to me how it works if both register_frame and
> register_frame_new are are called by one program.  
> 

How does register_frame/register_frame_new get called? They are
called by crtbegin.o/collect2. User doesn't use them directly.
Since they are under our control, we always call the right one.

My glibc.so calls register_frame. But the binaries generated by
new egcs and gcc 2.8.0 call register_frame_new. In my environment
register_frame/register_frame_new are mixed when libc.so is used.


-- 
H.J. Lu (hjl@gnu.org)

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[H.J. Lu]

>   > How does register_frame/register_frame_new get called? They are
>   > called by crtbegin.o/collect2. User doesn't use them directly.
>   > Since they are under our control, we always call the right one.
> But we don't have control over any programs or libraries that were
> compiled with the old interfaces right?  Thus, it's possible to get
> code which uses both.  Thus we have to be able to handle both right?

That is ok since we keep both. My patch keeps the old interface and
gives the new interface a new name.

> 
> Or are you saying that a library called with the old version would be
> entirely self contained and thus we don't have to worry about mixing?
> 

Yes. One shared library will only call one of the old/new interfaces
which is determined at the library build time by gcc.

> 
> Please be explicit so that we can all understand the problem and work
> towards a correct solution and avoid having to go through this again.
 
My current glibc shared library, which, BTW, uses the old interface
in its private crtbegin.o. But my new binaries generated by gcc 2.8.0
use the new one. At the startup time, the .init section is called first.

1. My glibc shared library calls __register_frame (__EH_FRAME_BEGIN__);
2. My binary calls __register_frame_new (__EH_FRAME_BEGIN__, &object);

Upon exit, the .fini section is called.

1. My glibc shared library calls __deregister_frame (__EH_FRAME_BEGIN__);
2. My binary calls __deregister_frame_new (__EH_FRAME_BEGIN__);

Please keep in mind, __EH_FRAME_BEGIN__ is a static variable local
to each binary. Since we keep both interfaces, we are ok.



-- 
H.J. Lu (hjl@gnu.org)

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[Jeffrey A Law]

  In message < m0xgQVs-0004ecC@ocean.lucon.org >you write:
  > > Or are you saying that a library called with the old version would be
  > > entirely self contained and thus we don't have to worry about mixing?
  > > 
  > 
  > Yes. One shared library will only call one of the old/new interfaces
  > which is determined at the library build time by gcc.
OK.  It sounds like this is the key point to remember.  Any particular
library or main program will only call one of the two interfaces and
thus we don't have a mixing problem.

[ ... ]
  > Please keep in mind, __EH_FRAME_BEGIN__ is a static variable local
  > to each binary. Since we keep both interfaces, we are ok.
This was one of the key points I was missing.  With your explaination
this aspect of the code makes much more sense.

I've got your patch intmy tree and I'll set up a build overnight.

I think we also need to install Paul Eggert's most recent change to
write_c_file_glob, then tweak it to use the new names.  



jeff

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[H.J. Lu]

> 
> 
>   In message < m0xgM3M-0004ecC@ocean.lucon.org >you write:
>   > __register_frame () change in gcc 2.8.0 causes a big problem.
>   > Since __register_frame () called in crtbegin.o is in libgcc2.c and
>   > gcc -shared includes crtbegin.o and -lgcc, any libc.so built by
>   > previous gcc has __register_frame () in libc.so. Now the ABI for
>   > __register_frame () is changed, that is a big mess for Linux since
>   > shared libraries are used a lot. When the ABI of a function in
>   > shared library is changed like this, it doesn't work too well.
> It seems to me that we need two interfaces one with the old way one
> with the new way.
> 
> Furthermore, we must avoid calling "free" on any of the objects which
> come from static storage instead of malloc.  Thus, we have to keep track
> of which interface was used for each object so that we know whether or
> not its memory should be free'd.
> 
> The compiler should always emit calls to the new interfaces; the old
> interfaces would be for existing copies of ld.so which use the old
> interfaces.
> 

I think that is what my patch does. It keeps the old name and
appends _new to new interface. It seems to work for me.


-- 
H.J. Lu (hjl@gnu.org)

Re: gcc 2.8.0 is broken on linux/x86 and more bug

[Jeffrey A Law]

  In message < m0xgQDb-0004ecC@ocean.lucon.org >you write:
  > >   > I think that is what my patch does. It keeps the old name and
  > >   > appends _new to new interface. It seems to work for me.
  > > I don't believe it handles the "free" issue correctly, then again, I'm
  > > not real familiar with that code.
  > > 
  > > Please explain to me how it works if both register_frame and
  > > register_frame_new are are called by one program.  
  > > 
  > 
  > How does register_frame/register_frame_new get called? They are
  > called by crtbegin.o/collect2. User doesn't use them directly.
  > Since they are under our control, we always call the right one.
But we don't have control over any programs or libraries that were
compiled with the old interfaces right?  Thus, it's possible to get
code which uses both.  Thus we have to be able to handle both right?

Or are you saying that a library called with the old version would be
entirely self contained and thus we don't have to worry about mixing?


Please be explicit so that we can all understand the problem and work
towards a correct solution and avoid having to go through this again.


jeff