[PATCH] ldso: fix x86_64 R_X86_64_TPOFF64 and R_X86_64_DTPOFF64 relocations

[Joakim Tjernlund]

Roman I Khimov <khimov at altell.ru> wrote on 2010/05/04 15:34:21:
>
> В сообщении от Вторник 04 мая 2010 17:23:15 автор Joakim Tjernlund написал:
> > Roman I Khimov <khimov at altell.ru> wrote on 2010/05/04 14:41:24:
> > > В сообщении от Вторник 04 мая 2010 16:37:35 автор Joakim Tjernlund
> написал:
> > > > > R_X86_64_TPOFF64 revealed by trivial testcase:
> > > > > ===================================================================
> > > > >  #include <stdio.h>
> > > > >  #include <errno.h>
> > > > >
> > > > > int main() {
> > > > >         void *a = &errno;
> > > > >
> > > > >         printf("errno addr: %llx\n", a);
> > > > >         __asm__("movq    errno at gottpoff(%%rip), %0;\n"
> > > > >                 "add    %%fs:0x0,%0;" : "=r"(a) );
> > > > >         printf("got errno addr: %llx\n", a);
> > > > >
> > > > >         return 0;
> > > > > }
> > > > > ===================================================================
> > > > >
> > > > > The addresses application got with R_X86_64_TPOFF64 was different
> > > > > than the once libc internal __errno_location returned.
> > > > >
> > > > > R_X86_64_DTPOFF64 testcase is even simpler than that:
> > > > > ===================================================================
> > > > >  #include <stdio.h>
> > > > >  #include <errno.h>
> > > > >  #include <netdb.h>
> > > > >  #undef h_errno
> > > > >
> > > > > extern __thread int h_errno;
> > > > >
> > > > > int main() {
> > > > >         printf("h_errno addr: %llx\n", &h_errno);
> > > > >         printf("__h_errno_location addr: %llx\n",
> > > > > __h_errno_location()); return 0;
> > > > > }
> > > > > ===================================================================
> > > > >
> > > > > but needs to be linked with "-lpthread". This way we've got h_errno
> > > > > relocation via R_X86_64_TPOFF64 in application and h_errno relocation
> > > > > via R_X86_64_DTPOFF64 in libpthread which has its own
> > > > > __h_errno_location() (probably we can kill it later?). And addresses
> > > > > were different again.
> > > > >
> > > > > The problem is that both relocations resolve symbols in external
> > > > > modules and thus should use symbol_addr instead of sym->st_value.
> > > >
> > > > Last I looked this does not match glibc, are you sure this is the
> > > > correct fix?
> > >
> > > Well, that's why there are testcases included. This fixes both.
> > >
> > > Started with R_X86_64_TPOFF64 as it was used in libpthread to reach errno
> > > from assembler routines (asm snippet above, .c sources used
> > > __errno_location()) and as the address got via relocation was different
> > > from the one returned by __errno_location(), we've had two errno-s and
> > > random breakage in applications or libc code that tried to use that
> > > (R_X86_64_TPOFF64 fixed 7 nptl tests for me).
> > >
> > > Then I've also checked R_X86_64_DTPOFF64 just out of curiosity (h_errno
> > > in libpthread resolved with it) and discovered that it also has a
> > > problem.
> >
> > Perhaps it is our errno impl. that is at fault? Doesn't feel right to
> > deviate from glibc. Is errno protected? The i suspect it is our impl.
> > of protected that is fault. The one that is in repo noe doesn't handle
> > TLS I think
>
> errno is not protected. And I believe it should work fine with TLS, since it's
>
> #ifdef __UCLIBC_HAS_TLS__
> ...snip...
> __thread int errno;
> ...snip...
>
> which I've also checked with a bit different testcase, starting a thread that
> does the same errno address check, the address was different.
>
> OK, if you don't like the testcases above I've just made another one, like
> this:
>
> ==============================================================================
> #include <stdio.h>
> #include <errno.h>
> #include <netdb.h>
> #undef h_errno
>
> extern __thread int h_errno;
>
> int main() {
>         void *a = &errno;
>         printf("errno addr: %llx\n", a);
>         __asm__("movq    errno at gottpoff(%%rip), %0;\n"
>                 "add    %%fs:0x0,%0;" : "=r"(a) );
>         printf("got errno addr: %llx\n", a);
>         printf("h_errno addr: %llx\n", &h_errno);
>         printf("__h_errno_location addr: %llx\n", __h_errno_location());
>         return 0;
> }
> ===============================================================================
>
> Link with "-lpthread". Now let's take a look at libc:
>
> $ objdump -x lib/libuClibc-0.9.32-git.so | grep tbss | grep errno
> 000000000000000c l       .tbss  0000000000000004 .hidden __libc_h_errno
> 0000000000000008 l       .tbss  0000000000000004 .hidden __libc_errno
> 0000000000000008 g       .tbss  0000000000000004 errno
> 000000000000000c g       .tbss  0000000000000004 h_errno
>
> And run the test without patch:
>
> $ ./errno-killer
> errno addr: 7f6802dd7688
> got errno addr: 7f6802dd7680
> h_errno addr: 7f6802dd7680
> __h_errno_location addr: 7f6802dd7680
>
> Whoops! h_errno and errno are the same. And the difference between two errno-s
> is kinda familiar. Now apply R_X86_64_TPOFF64 fix:
>
> $ ./errno-killer
> errno addr: 7f795117d688
> got errno addr: 7f795117d688
> h_errno addr: 7f795117d68c
> __h_errno_location addr: 7f795117d680
>
> Suddenly errno works and h_errno resolved with R_X86_64_TPOFF64 works too. But
> the one resolved via R_X86_64_DTPOFF64 doesn't, although the difference
> between two h_errno-s looks familiar. Now apply R_X86_64_DTPOFF64 fix:
>
> $ ./errno-killer
> errno addr: 7f8b65b1b688
> got errno addr: 7f8b65b1b688
> h_errno addr: 7f8b65b1b68c
> __h_errno_location addr: 7f8b65b1b68c
>
> Voila!
>
> i386 code currently uses symbol_addr for similar relocations too.
>
> Given all the above, yes, I think this is a correct patch.

Yes, you probably are. We differ a little from glibc how we lookup
symbols so if this works for you go for it.

    Jocke