[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190516002051.GX23599@brightrain.aerifal.cx>
Date: Wed, 15 May 2019 20:20:51 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix tls offsets when p_vaddr%p_align != 0 for
TLS_ABOVE_TP
On Tue, May 14, 2019 at 04:01:31AM +0200, Szabolcs Nagy wrote:
> this came up because lld changed tls alignment on aarch64 as a
> workaround for a bionic abi issue https://reviews.llvm.org/D53906
> but lld does not handle p_vaddr%p_align!=0 right so it broke on glibc
> https://reviews.llvm.org/D61824
>
> the patch is untested (bfd linker cannot seem to create problematic
> elf objects), but at least there are no regressions with libc-test.
> >From 8c94fcbc9faeb8b07132506757c3d3973652420e Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <nsz@...t70.net>
> Date: Mon, 13 May 2019 18:47:11 +0000
> Subject: [PATCH] fix tls offsets when p_vaddr%p_align != 0 for TLS_ABOVE_TP
>
> currently the bfd linker does not seem to create tls segments where
> p_vaddr%p_align != 0, but this is valid in ELF and then the runtime
> computed tls offset must satisfy
>
> offset%p_align == (base+p_vaddr)%p_align
>
> and in case of local exec tls (main executable) the smallest such
> offset must be used (otherwise it is incompatible with the offset
> computed by the static linker). the !TLS_ABOVE_TP case handled this
> correctly (the offset is negative then in the formula).
>
> the ldso code for TLS_ABOVE_TP is changed so the static tls offset
> of each module satisfies the formula and tls_offset always points
> to the end of the allocated static tls area (and not aligned up to
> tls_align or MIN_TLS_ALIGN).
I guess this saves some wasted memory?
> the tls_offset computation was wrong
> when multiple modules were loaded with static tls and in some the
> tls segment p_memsz%p_align != 0.
I don't understand this part. Are you saying we're currently
misaligning TLS for some libraries now?
> ---
> ldso/dynlink.c | 13 ++++++-------
> src/env/__init_tls.c | 3 ++-
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index 42a5470d..6dc39483 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -1126,9 +1126,9 @@ static struct dso *load_library(const char *name, struct dso *needed_by)
> p->tls_id = ++tls_cnt;
> tls_align = MAXP2(tls_align, p->tls.align);
> #ifdef TLS_ABOVE_TP
> - p->tls.offset = tls_offset + ( (tls_align-1) &
> - -(tls_offset + (uintptr_t)p->tls.image) );
> - tls_offset += p->tls.size;
> + p->tls.offset = tls_offset + ( (p->tls.align-1) &
> + (-tls_offset + (uintptr_t)p->tls.image) );
> + tls_offset = p->tls.offset + p->tls.size;
Is there a motivation for the seemingly independent change from use of
tls_align to use of p->tls.align here?
> #else
> tls_offset += p->tls.size + p->tls.align - 1;
> tls_offset -= (tls_offset + (uintptr_t)p->tls.image)
> @@ -1797,10 +1797,9 @@ _Noreturn void __dls3(size_t *sp)
> app.tls_id = tls_cnt = 1;
> #ifdef TLS_ABOVE_TP
> app.tls.offset = GAP_ABOVE_TP;
> - app.tls.offset += -GAP_ABOVE_TP & (app.tls.align-1);
> - tls_offset = app.tls.offset + app.tls.size
> - + ( -((uintptr_t)app.tls.image + app.tls.size)
> - & (app.tls.align-1) );
> + app.tls.offset += (-GAP_ABOVE_TP + (uintptr_t)app.tls.image)
> + & (app.tls.align-1);
> + tls_offset = app.tls.offset + app.tls.size;
> #else
> tls_offset = app.tls.offset = app.tls.size
> + ( -((uintptr_t)app.tls.image + app.tls.size)
> diff --git a/src/env/__init_tls.c b/src/env/__init_tls.c
> index 5f12500c..772baba3 100644
> --- a/src/env/__init_tls.c
> +++ b/src/env/__init_tls.c
> @@ -115,7 +115,8 @@ static void static_init_tls(size_t *aux)
> & (main_tls.align-1);
> #ifdef TLS_ABOVE_TP
> main_tls.offset = GAP_ABOVE_TP;
> - main_tls.offset += -GAP_ABOVE_TP & (main_tls.align-1);
> + main_tls.offset += (-GAP_ABOVE_TP + (uintptr_t)main_tls.image)
> + & (main_tls.align-1);
> #else
> main_tls.offset = main_tls.size;
> #endif
> --
> 2.21.0
>
I think you're probably right about all these things and I want to
apply this, but I also want to understand it a bit better first.
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.
