[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20150625231309.GE1173@brightrain.aerifal.cx>
Date: Thu, 25 Jun 2015 19:13:10 -0400
From: Rich Felker <dalias@...c.org>
To: Ted Hess <thess@...schensync.net>,
OpenWrt developers <openwrt-devel@...ts.openwrt.org>,
musl@...ts.openwall.com
Subject: Re: Re: [OpenWrt-Devel] Alsa-lib (libasound) segfaults on TLS
variable (musl on mips)
On Thu, Jun 25, 2015 at 01:06:42PM -0400, Rich Felker wrote:
> On Thu, Jun 25, 2015 at 01:08:48AM +0200, Szabolcs Nagy wrote:
> > * Szabolcs Nagy <nsz@...t70.net> [2015-06-24 22:57:54 +0200]:
> > > the bug is that mips tls access uses a hard coded -32768
> > > offset relative to whatever __tls_get_addr returned.
> > >
> > > and musl did not account for this offset.
> > >
> >
> > only affects mips shared objects with 'static __thread' variables,
> >
> > extern __thread variables worked fine (and only those were
> > tested in the libc-tests).
> >
> > > the attached patch fixes the issue for me,
> > > we will fix it in musl soon.
> >
> > better patch attached that undoes the offset for extern tls
> > vars in relocs handling.. (and with more consistent naming)
>
> > diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
> > index f8e35ae..904a248 100644
> > --- a/arch/mips/pthread_arch.h
> > +++ b/arch/mips/pthread_arch.h
> > @@ -13,4 +13,6 @@ static inline struct pthread *__pthread_self()
> > #define TLS_ABOVE_TP
> > #define TP_ADJ(p) ((char *)(p) + sizeof(struct pthread) + 0x7000)
> >
> > +#define DTP_OFFSET 0x8000
> > +
> > #define CANCEL_REG_IP (3-(union {int __i; char __b;}){1}.__b)
> > diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
> > index b77c6f6..b4ca410 100644
> > --- a/src/ldso/dynlink.c
> > +++ b/src/ldso/dynlink.c
> > @@ -337,7 +337,10 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
> > *reloc_addr = def.dso->tls_id;
> > break;
> > case REL_DTPOFF:
> > - *reloc_addr = tls_val + addend;
> > +#ifndef DTP_OFFSET
> > +#define DTP_OFFSET 0
> > +#endif
> > + *reloc_addr = tls_val + addend - DTP_OFFSET;
> > break;
> > #ifdef TLS_ABOVE_TP
> > case REL_TPOFF:
> > diff --git a/src/thread/__tls_get_addr.c b/src/thread/__tls_get_addr.c
> > index 3633396..94ea03c 100644
> > --- a/src/thread/__tls_get_addr.c
> > +++ b/src/thread/__tls_get_addr.c
> > @@ -1,6 +1,10 @@
> > #include <stddef.h>
> > #include "pthread_impl.h"
> >
> > +#ifndef DTP_OFFSET
> > +#define DTP_OFFSET 0
> > +#endif
> > +
> > void *__tls_get_addr(size_t *v)
> > {
> > pthread_t self = __pthread_self();
> > @@ -8,9 +12,9 @@ void *__tls_get_addr(size_t *v)
> > __attribute__((__visibility__("hidden")))
> > void *__tls_get_new(size_t *);
> > if (v[0]<=(size_t)self->dtv[0])
> > - return (char *)self->dtv[v[0]]+v[1];
> > - return __tls_get_new(v);
> > + return (char *)self->dtv[v[0]]+v[1]+DTP_OFFSET;
> > + return (char *)__tls_get_new(v)+DTP_OFFSET;
> > #else
> > - return (char *)self->dtv[1]+v[1];
> > + return (char *)self->dtv[1]+v[1]+DTP_OFFSET;
> > #endif
> > }
>
> I think having additional instructions in __tls_get_addr is the wrong
> approach. This function is the bottleneck for TLS access performance.
> If the offset is needed, it should be stored in the dtv[i] pointers,
> which then need to be adjusted at the point where they're setup.
While this would be slightly more efficient, it also has more
complexity and it's more invasive. I'll leave this open as an idea for
the future, but for now I've committed something close to nsz's patch
as commit 6ba5517a460c6c438f64d69464fdfc3269a4c91a.
If anything still seems broken, let me know.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
