Re: [bug #17647] glibc: GCC 4.1

[Thomas Schwinge]

Hello!

On Mon, Nov 20, 2006 at 03:40:03PM +0100, Samuel Thibault wrote:
> Thomas Schwinge, le Sun 19 Nov 2006 22:59:29 +0000, a ?crit :
> > | #v+
> > | [...]
> > | void
> > | -_dl_init_first (int argc, ...)
> > | +_dl_init_first (void)
> > | {
> > | first_init ();
> > | 
> > | - init (&argc);
> > | + init ((int *) __builtin_frame_address (0) + 2);
> > | }
> > | [...]
> > | #v-
> > 
> 
> The old version looks much more safe to me.

And in fact, applying the following patch which reverts those changes
from above and then adds another one, makes the GCC 4.1 built glibc
mostly work (see below for what doesn't work) and rebuilding the current
Debian hurd package against that glibc it also works -- especially
ext2fs.static was interesting to test.  But it also works only with the
limitation of what is described below.


The patch for glibc follows:

Revert Roland patch.  (Part of `-r 1.44' to '-r 1.43').

Index: sysdeps/mach/hurd/i386/init-first.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/mach/hurd/i386/init-first.c,v
retrieving revision 1.43.4.1
diff -u -p -r1.43.4.1 init-first.c
--- sysdeps/mach/hurd/i386/init-first.c 23 Oct 2005 01:00:09 -0000      1.43.4.1
+++ sysdeps/mach/hurd/i386/init-first.c 20 Nov 2006 11:05:47 -0000
@@ -320,11 +320,11 @@ 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);
+  init (&argc);
 }
 #endif
 
@@ -351,23 +351,21 @@ strong_alias (posixland_init, __libc_ini
    This poorly-named function is called by static-start.S,
    which should not exist at all.  */
 void
-_hurd_stack_setup (void)
+_hurd_stack_setup (volatile int argc, ...)
 {
-  intptr_t caller = (intptr_t) __builtin_return_address (0);
-
   void doinit (intptr_t *data)
     {
       /* This function gets called with the argument data at TOS.  */
-      void doinit1 (void)
+      void doinit1 (volatile int argc, ...)
        {
-         init ((int *) __builtin_frame_address (0) + 2);
+         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.  */
-      *--data = caller;
+      *--data = (&argc)[-1];
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
                    "movl $0, %%ebp\n" /* Clear outermost frame pointer.  */
                    "jmp *%1" : : "r" (data), "r" (&doinit1) : "sp");
@@ -376,7 +374,7 @@ _hurd_stack_setup (void)
 
   first_init ();
 
-  _hurd_startup ((void **) __builtin_frame_address (0) + 2, &doinit);
+  _hurd_startup ((void **) &argc, &doinit);
 }
 #endif


Make the code work with GCC 4.1.  This is essentially what Roland had
done -- moving the resolution of the return address out of the nested
subfunction -- but this time without replacing it with
`__builtin_return_address'.

Index: sysdeps/mach/hurd/i386/init-first.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/mach/hurd/i386/init-first.c,v
retrieving revision 1.43.4.1
diff -u -p -r1.43.4.1 init-first.c
--- sysdeps/mach/hurd/i386/init-first.c 23 Oct 2005 01:00:09 -0000      1.43.4.1
+++ sysdeps/mach/hurd/i386/init-first.c 20 Nov 2006 11:05:47 -0000
@@ -353,6 +355,8 @@
 void
 _hurd_stack_setup (volatile int argc, ...)
 {
+  intptr_t caller = (&argc)[-1];
+
   void doinit (intptr_t *data)
     {
       /* This function gets called with the argument data at TOS.  */
@@ -365,7 +369,7 @@
          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 = (&argc)[-1];
+      *--data = caller;
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
                    "movl $0, %%ebp\n" /* Clear outermost frame pointer.  */
                    "jmp *%1" : : "r" (data), "r" (&doinit1) : "sp");


Cosmetical and documentation fixes.

Index: sysdeps/mach/hurd/i386/init-first.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/mach/hurd/i386/init-first.c,v
retrieving revision 1.43.4.1
diff -u -p -r1.43.4.1 init-first.c
--- sysdeps/mach/hurd/i386/init-first.c 23 Oct 2005 01:00:09 -0000      1.43.4.1
+++ sysdeps/mach/hurd/i386/init-first.c 20 Nov 2006 11:05:47 -0000
@@ -1,6 +1,8 @@
 /* Initialization code run first thing by the ELF startup code.  For i386/Hurd.
-   Copyright (C) 1995,96,97,98,99,2000,01,02,03,04,05
-       Free Software Foundation, Inc.
+
+   Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
+   2005, 2006 Free Software Foundation, Inc.
+
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -243,8 +243,8 @@ init (int *data)
         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
+      data[-1] = (int) &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));
     }


Now, what doesn't work: I was working on integrating Ben's rpctrace
patch, when I noticed the following:

#v+
$ rpctrace --help
Usage: rpctrace [OPTION...] COMMAND [ARG...]
Trace Mach Remote Procedure Calls.

Segmentation fault
#v-

And in fact also glibc-proper code is affected:

#v+
thomas@leibniz:/var/tmp/glibc-GCC4.1-without_Roland_patches/glibc-2.3.6.ds1/build-tree/hurd-i386-libc
 $ LD_LIBRARY_PATH=. gdb --args locale/localedef --help
GNU gdb 6.5-debian
[...]
This GDB was configured as "i486-gnu"...
(gdb) r
Starting program: 
/var/tmp/glibc-GCC4.1-without_Roland_patches/glibc-2.3.6.ds1/build-tree/hurd-i386-libc/locale/localedef
 --help
Usage: localedef [OPTION...] NAME
  or:  localedef [OPTION...] [--add-to-archive|--delete-from-archive] FILE...
  or:  localedef [OPTION...] --list-archive [FILE]
Compile locale specification

 Input Files:
  -f, --charmap=FILE         Symbolic character names defined in FILE
  -i, --inputfile=FILE       Source definitions are found in FILE

Program received signal SIGSEGV, Segmentation fault.
0x011389a8 in __argp_fmtstream_update (fs=0x8098158) at argp-fmtstream.c:232
232               while (p >= buf && !isblank (*p))
(gdb) print p
$1 = 0x80981e9 "values\n\n Input Files:\n  -f, e] FILE...\n", '\223' <repeats 
88 times>, "???"
(gdb) print *p
$2 = 118 'v'
(gdb) bt
#0  0x011389a8 in __argp_fmtstream_update (fs=0x8098158) at argp-fmtstream.c:232
#1  0x0113cc74 in _help (argp=0x1019ab0, state=0x1019ba4, stream=0x1188be0, 
flags=<value optimized out>, name=0x101a05e "localedef")
    at argp-fmtstream.h:255
#2  0x0113d31e in __argp_state_help (state=0x1019ba4, stream=0x80981e9, 
flags=634) at argp-help.c:1730
#3  0x0113d7a8 in argp_default_parser (key=63, arg=0x0, state=0x1019ba4) at 
argp-parse.c:119
#4  0x0113d595 in group_parse (group=0x8097efc, state=0x1019ba4, key=63, 
arg=0x0) at argp-parse.c:257
#5  0x0113df93 in __argp_parse (argp=0x1019ab0, argc=2, argv=0x1019db4, 
flags=0, end_index=0x1019d58, input=0x0) at argp-parse.c:755
#6  0x0804a854 in main (argc=2, argv=0x1019db4) at programs/localedef.c:206
#v-

Any offhand guesses what's going on there?  I would expect nothing in the
argp code to be Hurd specific and I'm not aware of any Linux system
having such a problem.


Regards,
 Thomas

signature.asc
Description: Digital signature