Hurd: Avoid init-first.c miscompilation.

2 files changed, 38 insertions, 14 deletions

diff --git a/ChangeLog b/ChangeLog
index 9e747cd..e399542 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2012-05-10  Thomas Schwinge  <thomas@schwinge.name>
 
+	* sysdeps/mach/hurd/i386/init-first.c (init): Use
+	__builtin_frame_address instead of making assumptions about the
+	location of the return address relative to DATA.  Force early load of
+	the return address.
+	(_dl_init_first, doinit1 in doinit in _hurd_stack_setup): Don't use
+	__builtin_frame_address.
+
 	dup3 for GNU Hurd.
 	* include/unistd.h: Declare __dup3 and use libc_hidden_proto on it.
 	* sysdeps/mach/hurd/dup3.c: New file, copy from dup2.c.  Evolve it to
diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
index 4785e8d..f4bf624 100644
--- a/sysdeps/mach/hurd/i386/init-first.c
+++ b/sysdeps/mach/hurd/i386/init-first.c
@@ -122,7 +122,7 @@ init1 (int argc, char *arg0, ...)
     {
 #ifndef SHARED
       /* We may need to see our own phdrs, e.g. for TLS setup.
-         Try the usual kludge to find the headers without help from
+	 Try the usual kludge to find the headers without help from
 	 the exec server.  */
       extern const void _start;
       const ElfW(Ehdr) *const ehdr = &_start;
@@ -225,7 +225,7 @@ init (int *data)
 
 #ifdef SHARED
       /* And readjust the dynamic linker's idea of where the argument
-         vector lives.  */
+	 vector lives.  */
       assert (_dl_argv == argv);
       _dl_argv = (void *) (newsp + 1);
 #endif
@@ -244,9 +244,16 @@ init (int *data)
       /* Push the user code address on the top of the new stack.  It will
 	 be the return address for `init1'; we will jump there with NEWSP
 	 as the stack pointer.  */
-      *--newsp = data[-1];
-      ((void **) data)[-1] = switch_stacks;
-      /* Force NEWSP into %ecx and &init1 into %eax, which are not restored
+      /* The following expression would typically be written as
+	 ``__builtin_return_address (0)''.  But, for example, GCC 4.4.6 doesn't
+	 recognize that this read operation may alias the following write
+	 operation, and thus is free to reorder the two, clobbering the
+	 original return address.  */
+      *--newsp = *((int *) __builtin_frame_address (0) + 1);
+      /* GCC 4.4.6 also wants us to force loading *NEWSP already here.  */
+      asm volatile ("# %0" : : "X" (*newsp));
+      *((void **) __builtin_frame_address (0) + 1) = &switch_stacks;
+      /* Force NEWSP into %eax and &init1 into %ecx, which are not restored
 	 by function return.  */
       asm volatile ("# a %0 c %1" : : "a" (newsp), "c" (&init1));
     }
@@ -273,8 +280,15 @@ init (int *data)
 
       /* The argument data is just above the stack frame we will unwind by
 	 returning.  Mutate our own return address to run the code below.  */
-      usercode = data[-1];
-      data[-1] = (int) &call_init1;
+      /* The following expression would typically be written as
+	 ``__builtin_return_address (0)''.  But, for example, GCC 4.4.6 doesn't
+	 recognize that this read operation may alias the following write
+	 operation, and thus is free to reorder the two, clobbering the
+	 original return address.  */
+      usercode = *((int *) __builtin_frame_address (0) + 1);
+      /* GCC 4.4.6 also wants us to force loading USERCODE already here.  */
+      asm volatile ("# %0" : : "X" (usercode));
+      *((void **) __builtin_frame_address (0) + 1) = &call_init1;
       /* Force USERCODE into %eax and &init1 into %ecx, which are not
 	 restored by function return.  */
       asm volatile ("# a %0 c %1" : : "a" (usercode), "c" (&init1));
@@ -322,11 +336,12 @@ first_init (void)
    stack set up just as the user will see it, so it can switch stacks.  */
 
 void
-_dl_init_first (void)
+_dl_init_first (int argc, ...)
 {
   first_init ();
 
-  init ((int *) __builtin_frame_address (0) + 2);
+  /* If we use ``__builtin_frame_address (0) + 2'' here, GCC gets confused.  */
+  init (&argc);
 }
 #endif
 
@@ -360,15 +375,17 @@ _hurd_stack_setup (void)
   void doinit (intptr_t *data)
     {
       /* This function gets called with the argument data at TOS.  */
-      void doinit1 (void)
+      void doinit1 (int argc, ...)
 	{
-	  init ((int *) __builtin_frame_address (0) + 2);
+	  /* If we use ``__builtin_frame_address (0) + 2'' here, GCC gets
+	     confused.  */
+	  init ((int *) &argc);
 	}
 
       /* Push the user return address after the argument data, and then
-         jump to `doinit1' (above), so it is as if __libc_init_first's
-         caller had called `doinit1' with the argument data already on the
-         stack.  */
+	 jump to `doinit1' (above), so it is as if __libc_init_first's
+	 caller had called `doinit1' with the argument data already on the
+	 stack.  */
       *--data = caller;
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
 		    "movl $0, %%ebp\n" /* Clear outermost frame pointer.  */