Re: __thread errno (was: [SCM] glibc maintenance branch, tschwinge/Roger_Whittaker, updated. glibc-2.12-3577-g5b1f2ae)

[Thomas Schwinge]

Hi!

On Thu, 10 May 2012 03:22:20 +0200, Samuel Thibault <samuel.thibault@gnu.org> 
wrote:
> Thomas Schwinge, le Thu 10 May 2012 09:17:33 +0800, a écrit :
> > On Thu, 10 May 2012 00:10:52 +0000, Samuel Thibault 
> > <samuel.thibault@ens-lyon.org> wrote:
> > > commit b7f913cb1bdf9ca718661af315b9f344dc77ebe8
> > > Author: Samuel Thibault <samuel.thibault@ens-lyon.org>
> > > Date:   Fri Apr 27 05:42:42 2012 +0200
> > > 
> > >     Move errno to TLS
> > >     
> > >     * sysdeps/mach/hurd/errno-loc.c: Remove file.
> > >     * sysdeps/mach/hurd/errno.c: Remove file.
> > >     * sysdeps/mach/hurd/dl-sysdep.c (errno): Remove variable.

How did you test this?  It doesn't build for me.  Mimicking what Linux is
doing:

diff --git a/hurd/Makefile b/hurd/Makefile
index 4f74ccc..15577e3 100644
--- a/hurd/Makefile
+++ b/hurd/Makefile
@@ -58,7 +58,6 @@ routines = hurdstartup hurdinit \
           fopenport \
           vpprintf \
           ports-get ports-set hurdports hurdmsg \
-          errno-loc \
           sysvshm \
           $(sig) $(dtable) $(inlines) port-cleanup report-wait xattr
 sig    = hurdsig hurdfault siginfo hurd-raise preempt-sig \
diff --git a/sysdeps/mach/hurd/Makefile b/sysdeps/mach/hurd/Makefile
index 9ad475d..5460f50 100644
--- a/sysdeps/mach/hurd/Makefile
+++ b/sysdeps/mach/hurd/Makefile
@@ -183,6 +183,10 @@ $(objpfx)crt0.o: $(objpfx)static-start.o 
$(objpfx)abi-note.o $(objpfx)init.o
 
 endif

+ifeq ($(subdir),csu)
+sysdep_routines += errno-loc
+endif
+
 ifeq (hurd, $(subdir))
 sysdep_routines += cthreads
 endif

... it still fails when invoking ld.so:

    $ gdb -q --args ./ld.so 
    Reading symbols from /home/tschwinge/tmp/ld.so...done.
    (gdb) r
    Starting program: /home/tschwinge/tmp/ld.so 
    
    Program received signal EXC_BAD_ACCESS, Could not access memory.
    0x00015797 in __strerror_r (errnum=0, buf=0x0, buflen=2) at dl-minimal.c:173
    173     dl-minimal.c: No such file or directory.
            in dl-minimal.c
    (gdb) bt
    #0  0x00015797 in __strerror_r (errnum=0, buf=0x0, buflen=2) at 
dl-minimal.c:173
    #1  0x00000000 in ?? ()
    (gdb) info registers
    eax            0x0      0
    ecx            0xa      10
    edx            0x2      2
    ebx            0x26ff4  159732
    esp            0x1028c60        0x1028c60
    ebp            0x1028cb8        0x1028cb8
    esi            0xa      10
    edi            0x21b4c  138060
    eip            0x15797  0x15797 <__strerror_r+167>
    eflags         0x10202  [ IF RF ]
    cs             0x17     23
    ss             0x1f     31
    ds             0x1f     31
    es             0x1f     31
    fs             0x1f     31
    gs             0x1f     31

0x15797 is bogus: it's not even an instruction boundary.

Apparently I forgot how to debug ld.so from the very beginning...

It seems that gs is not set up, but even if that were an invalid TLS gs:X
access, that doesn't explain to me how the PC would be badly affected by
that?


> > This breaks ABI compatibility, doesn't it?
> 
> AIUI, no.
> 
> > On a current system:
> > 
> >     $ echo -e '#include <errno.h>\nint e(){return errno;}' | gcc -S -o - -O 
> > -x c -
> >             .file   ""
> >             .text
> >             .globl  e
> >             .type   e, @function
> >     e:
> >     .LFB0:
> >             .cfi_startproc
> >             subl    $12, %esp
> >             .cfi_def_cfa_offset 16
> >             call    __errno_location
> >             movl    (%eax), %eax
> >             addl    $12, %esp
> >             .cfi_def_cfa_offset 4
> >             ret
> >             .cfi_endproc
> >     .LFE0:
> >             .size   e, .-e
> >             .ident  "GCC: (Debian 4.6.3-1+hurd.3) 4.6.3"
> >             .section        .note.GNU-stack,"",@progbits
> > 
> > But I think solving this is easy, too: just return the __thread &errno
> > From __errno_location (and un-export that symbol for later glibc
> > versions).  But this is for a later time, I'd say.
> 
> Isn't that already done by ./csu/errno-loc.c, uncovered by the remove?

You're right.  (And I got it right, too, mostly.)  ;-) User code doesn't
directly refer to the TLS variable, but always goes though
__erno_location.


In sysdeps/unix/sysv/linux/bits/errno.h, the »#define errno
(*__errno_location ())« is conditioned by »#if !defined _LIBC || defined
_LIBC_REENTRANT« -- should we be doing that, too?

What about the __hurd_errno_location symbol from
sysdeps/mach/hurd/errno-loc.c (and still referenced in hurd/Versions)?
According to ChangeLog.13 that has been removed from
hurd/hurd/threadvar.h in 2002, so there shouldn't be any binaries left
using this and we should silently remove it from hurd/Versions, too?


Grüße,
 Thomas

pgpzxOnJyaDwb.pgp
Description: PGP signature