From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12849 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: TLS issue on aarch64 Date: Mon, 28 May 2018 18:15:21 -0400 Message-ID: <20180528221521.GI1392@brightrain.aerifal.cx> References: <20180525145059.GG4418@port70.net> <20180526005415.GI4418@port70.net> <20180527003430.GG1392@brightrain.aerifal.cx> <20180528204730.GJ4418@port70.net> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1527545609 5265 195.159.176.226 (28 May 2018 22:13:29 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 28 May 2018 22:13:29 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12865-gllmg-musl=m.gmane.org@lists.openwall.com Tue May 29 00:13:25 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1fNQOL-0001IV-Cm for gllmg-musl@m.gmane.org; Tue, 29 May 2018 00:13:25 +0200 Original-Received: (qmail 26595 invoked by uid 550); 28 May 2018 22:15:34 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 26577 invoked from network); 28 May 2018 22:15:33 -0000 Content-Disposition: inline In-Reply-To: <20180528204730.GJ4418@port70.net> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12849 Archived-At: On Mon, May 28, 2018 at 10:47:31PM +0200, Szabolcs Nagy wrote: > * Rich Felker [2018-05-26 20:34:30 -0400]: > > On Sat, May 26, 2018 at 02:54:16AM +0200, Szabolcs Nagy wrote: > > > (on mips/ppc i expect it not to change anything: tp is > > > at a page aligned offset from the end of struct pthread, > > > so one alignment is enough there, but on aarch64/arm/sh4 > > > this makes a difference, and seems to pass my simple tests) > > > > > > diff --git a/src/env/__init_tls.c b/src/env/__init_tls.c > > > index 1c5d98a0..8e70024d 100644 > > > --- a/src/env/__init_tls.c > > > +++ b/src/env/__init_tls.c > > > @@ -41,9 +41,12 @@ void *__copy_tls(unsigned char *mem) > > > #ifdef TLS_ABOVE_TP > > > dtv = (void **)(mem + libc.tls_size) - (libc.tls_cnt + 1); > > > > > > - mem += -((uintptr_t)mem + sizeof(struct pthread)) & (libc.tls_align-1); > > > + /* Ensure TP is aligned. */ > > > + mem += -(uintptr_t)TP_ADJ(mem) & (libc.tls_align-1); > > > td = (pthread_t)mem; > > > mem += sizeof(struct pthread); > > > + /* Ensure TLS is aligned after struct pthread. */ > > > + mem += -(uintptr_t)mem & (libc.tls_align-1); > > > > > > for (i=1, p=libc.tls_head; p; i++, p=p->next) { > > > dtv[i] = mem + p->offset; > > > > As written this (or anything using libc.tls_align to adjust offset of > > the TLS from the TP) is not valid. The value of libc.tls_align is > > runtime-variable and will increase upon dlopen, and even without > > dlopen, will be non-deterministic dependent on shared libraries from > > DT_NEEDED in dynamic-linked programs. The offset between TP and TLS is > > a property of the linker's handling of local-exec TLS in the main > > program only, and thus probably should be using libc.tls_head.align. > > > > ok, makes sense. > > > However, care needs to be taken that libc.tls_head may initially be > > null if the main program has no TLS, but could later become non-null > > due to dlopen. If the offset between TP and TLS changed due to this, > > any initial-exec-model TLS access would be wrong. Fortunately such a > > program cannot have initial-exec-model accesses (initial-exec is only > > valid for TLS that existed at program start), so we can probably just > > ignore the issue and always use libc.tls_head?libc.tls_head.align:1; > > this will cause gratuitous padding for threads created after dlopen of > > a library with larger alignment, but should otherwise not hurt > > anything. > > > > yes i think we only need to consider the tls alignment requirements > of the main executable, if libc.tls_head can only be changed by > loading libs with initial-exec tls that should be fine. The only transition possible for libc.tls_head is from null to non-null, and it's only initially null if there is initially no TLS, in which case the initial-exec (and local-exec) models are invalid. > another issue with the patch is that if tp is aligned then pthread_t > may not get aligned: > > tp == self + sizeof(pthread_t) - reserved This expression does not look correct; it would have tp point somewhere inside of the pthread structure rather than just past the end of it. Maybe your code is doing this to avoid wasted padding, but if so I think that's a bad idea. It violates the invariant that the pthread structure is at a constant offset from tp, which is needed for efficient pthread_self and for access to fixed slots at the end of it (like canary or dtv copy). > so sizeof(pthread_t) - reserved must be divisible with > gcd(alignment of tp, alignof(pthread_t)) to be able to make both > self and tp aligned. > > this is not an issue on current targets with current pthread_t, > but we may want to decouple internal struct pthread alignment > details and the abi reserved tls size, i.e. tp_adj could be like > > tp == alignup(self + sizeof(pthread_t) - reserved, alignof(pthread_t)) > > or we add a static assert that reserved and alignof(pthread_t) > are not conflicting. Maybe I'm misunderstanding what "reserved" is, since you're talking about a static assert...? Rich