Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251012030214.GL1827@brightrain.aerifal.cx>
Date: Sat, 11 Oct 2025 23:02:14 -0400
From: Rich Felker <dalias@...c.org>
To: Alex Rønne Petersen <alex@...xrp.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] s390x: manually inline __tls_get_addr in
 __tls_get_offset

On Sun, Oct 12, 2025 at 04:56:02AM +0200, Alex Rønne Petersen wrote:
> On Sun, Oct 12, 2025, at 04:46, Rich Felker wrote:
> > On Fri, Jan 24, 2025 at 06:12:13AM +0100, Alex Rønne Petersen wrote:
> >> Calling __tls_get_addr with brasl is not valid since it's a global symbol; doing
> >> so results in an R_390_PC32DBL relocation error from lld. We could fix this by
> >> marking __tls_get_addr hidden since it is not part of the s390x ABI, or by using
> >> a different instruction. However, given its simplicity, it makes more sense to
> >> just manually inline it into __tls_get_offset for performance.
> >> 
> >> The patch has been tested by applying to Zig's bundled musl copy and running the
> >> full Zig test suite under qemu-s390x.
> >> ---
> >>  src/thread/s390x/__tls_get_offset.s | 20 ++++++++++----------
> >>  1 file changed, 10 insertions(+), 10 deletions(-)
> >> 
> >> diff --git a/src/thread/s390x/__tls_get_offset.s b/src/thread/s390x/__tls_get_offset.s
> >> index 8ee92de8..405f118b 100644
> >> --- a/src/thread/s390x/__tls_get_offset.s
> >> +++ b/src/thread/s390x/__tls_get_offset.s
> >> @@ -1,17 +1,17 @@
> >>  	.global __tls_get_offset
> >>  	.type __tls_get_offset,%function
> >>  __tls_get_offset:
> >> -	stmg  %r14, %r15, 112(%r15)
> >> -	aghi  %r15, -160
> >> +	ear   %r0, %a0
> >> +	sllg  %r0, %r0, 32
> >> +	ear   %r0, %a1
> >>  
> >> -	la    %r2, 0(%r2, %r12)
> >> -	brasl %r14, __tls_get_addr
> >> +	la    %r1, 0(%r2, %r12)
> >>  
> >> -	ear   %r1, %a0
> >> -	sllg  %r1, %r1, 32
> >> -	ear   %r1, %a1
> >> +	lg    %r3, 0(%r1)
> >> +	sllg  %r4, %r3, 3
> >> +	lg    %r5, 8(%r0)
> >> +	lg    %r2, 0(%r4, %r5)
> >> +	ag    %r2, 8(%r1)
> >> +	sgr   %r2, %r0
> >>  
> >> -	sgr   %r2, %r1
> >> -
> >> -	lmg   %r14, %r15, 272(%r15)
> >>  	br    %r14
> >> -- 
> >> 2.43.0
> >
> > This must not have been covered by tests because it doesn't work. The
> > problem is that there is apparently no such thing as 8(%r0); it looks
> > like r0 is special in addressing and means constant zero. So it should
> > be fixable just shuffling around which registers are used for what.
> >
> > I can try to prepare a fix if you don't have time for it.
> 
> Oh, yikes, yes; I actually got bit by this exact s390x quirk
> recently in an unrelated context while working on Zig:
> https://github.com/ziglang/zig/commit/7b92d5f4052be651e9bc5cd4ad78a69ccbee865d
> 
> And indeed, another thing I've since discovered is that our test
> suite has rather poor coverage of TLS usage. Something we'll
> definitely need work on.
> 
> Sorry for the trouble; I'll send a v2 of this patch shortly.

Thanks! The commit is already in git history, so please just send fix
as a patch against the old rather than a v2 of the above..

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.