[PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[Florian Weimer]

In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
This realignment does not take into account that the function has
already used part of the red zone at this point, thus clobbering
the initally saved register values located there if the stack
alignment inherited from the caller is unfortunate.

(Note: I do not know to write a good test case for this in the existing
framework.  We saw this as a random LTO plugin crash when building GCC
with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
with this change here.)

---
 sysdeps/x86_64/dl-tlsdesc-dynamic.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sysdeps/x86_64/dl-tlsdesc-dynamic.h b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
index 9f02cfc3eb..8e49e7eece 100644
--- a/sysdeps/x86_64/dl-tlsdesc-dynamic.h
+++ b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
@@ -83,6 +83,8 @@ _dl_tlsdesc_dynamic:
 2:
 #if DL_RUNTIME_RESOLVE_REALIGN_STACK
 	movq	%rbx, -24(%rsp)
+	subq    $24, %rsp
+	cfi_adjust_cfa_offset(24)
 	mov	%RSP_LP, %RBX_LP
 	cfi_def_cfa_register(%rbx)
 	and	$-STATE_SAVE_ALIGNMENT, %RSP_LP
@@ -153,6 +155,8 @@ _dl_tlsdesc_dynamic:
 #if DL_RUNTIME_RESOLVE_REALIGN_STACK
 	mov	%RBX_LP, %RSP_LP
 	cfi_def_cfa_register(%rsp)
+	addq    $24, %rsp
+	cfi_adjust_cfa_offset(-24)
 	movq	-24(%rsp), %rbx
 	cfi_restore(%rbx)
 #else

base-commit: 5ebc24f785dc0dff494a93ca82a369497c3cdc68

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> This realignment does not take into account that the function has
> already used part of the red zone at this point, thus clobbering
> the initally saved register values located there if the stack
> alignment inherited from the caller is unfortunate.
>
> (Note: I do not know to write a good test case for this in the existing
> framework.  We saw this as a random LTO plugin crash when building GCC
> with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> with this change here.)

Will a different STATE_SAVE_OFFSET for TLS descriptor work?

> ---
>  sysdeps/x86_64/dl-tlsdesc-dynamic.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/sysdeps/x86_64/dl-tlsdesc-dynamic.h b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> index 9f02cfc3eb..8e49e7eece 100644
> --- a/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> +++ b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> @@ -83,6 +83,8 @@ _dl_tlsdesc_dynamic:
>  2:
>  #if DL_RUNTIME_RESOLVE_REALIGN_STACK
>         movq    %rbx, -24(%rsp)
> +       subq    $24, %rsp
> +       cfi_adjust_cfa_offset(24)
>         mov     %RSP_LP, %RBX_LP
>         cfi_def_cfa_register(%rbx)
>         and     $-STATE_SAVE_ALIGNMENT, %RSP_LP
> @@ -153,6 +155,8 @@ _dl_tlsdesc_dynamic:
>  #if DL_RUNTIME_RESOLVE_REALIGN_STACK
>         mov     %RBX_LP, %RSP_LP
>         cfi_def_cfa_register(%rsp)
> +       addq    $24, %rsp
> +       cfi_adjust_cfa_offset(-24)
>         movq    -24(%rsp), %rbx
>         cfi_restore(%rbx)
>  #else
>
> base-commit: 5ebc24f785dc0dff494a93ca82a369497c3cdc68
>


-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> > This realignment does not take into account that the function has
> > already used part of the red zone at this point, thus clobbering
> > the initally saved register values located there if the stack
> > alignment inherited from the caller is unfortunate.
> >
> > (Note: I do not know to write a good test case for this in the existing
> > framework.  We saw this as a random LTO plugin crash when building GCC
> > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> > with this change here.)
>
> Will a different STATE_SAVE_OFFSET for TLS descriptor work?

Correction.  REGISTER_SAVE_AREA is for this purpose.   Will a different
value for TLS descriptor work?

> > ---
> >  sysdeps/x86_64/dl-tlsdesc-dynamic.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/sysdeps/x86_64/dl-tlsdesc-dynamic.h b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> > index 9f02cfc3eb..8e49e7eece 100644
> > --- a/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> > +++ b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> > @@ -83,6 +83,8 @@ _dl_tlsdesc_dynamic:
> >  2:
> >  #if DL_RUNTIME_RESOLVE_REALIGN_STACK
> >         movq    %rbx, -24(%rsp)
> > +       subq    $24, %rsp
> > +       cfi_adjust_cfa_offset(24)
> >         mov     %RSP_LP, %RBX_LP
> >         cfi_def_cfa_register(%rbx)
> >         and     $-STATE_SAVE_ALIGNMENT, %RSP_LP
> > @@ -153,6 +155,8 @@ _dl_tlsdesc_dynamic:
> >  #if DL_RUNTIME_RESOLVE_REALIGN_STACK
> >         mov     %RBX_LP, %RSP_LP
> >         cfi_def_cfa_register(%rsp)
> > +       addq    $24, %rsp
> > +       cfi_adjust_cfa_offset(-24)
> >         movq    -24(%rsp), %rbx
> >         cfi_restore(%rbx)
> >  #else
> >
> > base-commit: 5ebc24f785dc0dff494a93ca82a369497c3cdc68
> >
>
>
> --
> H.J.



-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 7:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >
> > > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> > > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> > > This realignment does not take into account that the function has
> > > already used part of the red zone at this point, thus clobbering
> > > the initally saved register values located there if the stack
> > > alignment inherited from the caller is unfortunate.
> > >
> > > (Note: I do not know to write a good test case for this in the existing
> > > framework.  We saw this as a random LTO plugin crash when building GCC
> > > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> > > with this change here.)

We should try to find a testcase.  Can you provide a backtrace when it
happens?  It should be possible to write a testcase with the backtrace.


> > Will a different STATE_SAVE_OFFSET for TLS descriptor work?
>
> Correction.  REGISTER_SAVE_AREA is for this purpose.   Will a different
> value for TLS descriptor work?
>
> > > ---
> > >  sysdeps/x86_64/dl-tlsdesc-dynamic.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/sysdeps/x86_64/dl-tlsdesc-dynamic.h b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> > > index 9f02cfc3eb..8e49e7eece 100644
> > > --- a/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> > > +++ b/sysdeps/x86_64/dl-tlsdesc-dynamic.h
> > > @@ -83,6 +83,8 @@ _dl_tlsdesc_dynamic:
> > >  2:
> > >  #if DL_RUNTIME_RESOLVE_REALIGN_STACK
> > >         movq    %rbx, -24(%rsp)
> > > +       subq    $24, %rsp
> > > +       cfi_adjust_cfa_offset(24)
> > >         mov     %RSP_LP, %RBX_LP
> > >         cfi_def_cfa_register(%rbx)
> > >         and     $-STATE_SAVE_ALIGNMENT, %RSP_LP
> > > @@ -153,6 +155,8 @@ _dl_tlsdesc_dynamic:
> > >  #if DL_RUNTIME_RESOLVE_REALIGN_STACK
> > >         mov     %RBX_LP, %RSP_LP
> > >         cfi_def_cfa_register(%rsp)
> > > +       addq    $24, %rsp
> > > +       cfi_adjust_cfa_offset(-24)
> > >         movq    -24(%rsp), %rbx
> > >         cfi_restore(%rbx)
> > >  #else
> > >
> > > base-commit: 5ebc24f785dc0dff494a93ca82a369497c3cdc68
> > >
> >
> >
> > --
> > H.J.
>
>
>
> --
> H.J.



--
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[Florian Weimer]

* H. J. Lu:

> On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
>> >
>> > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
>> > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
>> > This realignment does not take into account that the function has
>> > already used part of the red zone at this point, thus clobbering
>> > the initally saved register values located there if the stack
>> > alignment inherited from the caller is unfortunate.
>> >
>> > (Note: I do not know to write a good test case for this in the existing
>> > framework.  We saw this as a random LTO plugin crash when building GCC
>> > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
>> > with this change here.)
>>
>> Will a different STATE_SAVE_OFFSET for TLS descriptor work?
>
> Correction.  REGISTER_SAVE_AREA is for this purpose.   Will a different
> value for TLS descriptor work?

I think REGISTER_SAVE_AREA is for the later register saves?

This use of the red zone is specific to to the TLS trampoline.  The lazy
binding trampoline doesn't do that.  REGISTER_SAVE_AREA is used by both.

Thanks,
Florian

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 7:57 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> >
> >> > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> >> > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> >> > This realignment does not take into account that the function has
> >> > already used part of the red zone at this point, thus clobbering
> >> > the initally saved register values located there if the stack
> >> > alignment inherited from the caller is unfortunate.
> >> >
> >> > (Note: I do not know to write a good test case for this in the existing
> >> > framework.  We saw this as a random LTO plugin crash when building GCC
> >> > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> >> > with this change here.)
> >>
> >> Will a different STATE_SAVE_OFFSET for TLS descriptor work?
> >
> > Correction.  REGISTER_SAVE_AREA is for this purpose.   Will a different
> > value for TLS descriptor work?
>
> I think REGISTER_SAVE_AREA is for the later register saves?
>
> This use of the red zone is specific to to the TLS trampoline.  The lazy
> binding trampoline doesn't do that.  REGISTER_SAVE_AREA is used by both.
>

# if DL_RUNTIME_RESOLVE_REALIGN_STACK
        /* STATE_SAVE_OFFSET has space for 8 integer registers.  But we
           need space for RCX, RDX, RSI, RDI, R8, R9, R10 and R11, plus
           RBX above.  */
        sub     $(REGISTER_SAVE_AREA + STATE_SAVE_ALIGNMENT), %RSP_LP
# else
        sub     $REGISTER_SAVE_AREA, %RSP_LP
        cfi_adjust_cfa_offset(REGISTER_SAVE_AREA)
# endif

Let's find a testcase first.

-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[Florian Weimer]

* H. J. Lu:

> On Sat, Mar 16, 2024 at 7:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>> >
>> > On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
>> > >
>> > > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
>> > > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
>> > > This realignment does not take into account that the function has
>> > > already used part of the red zone at this point, thus clobbering
>> > > the initally saved register values located there if the stack
>> > > alignment inherited from the caller is unfortunate.
>> > >
>> > > (Note: I do not know to write a good test case for this in the existing
>> > > framework.  We saw this as a random LTO plugin crash when building GCC
>> > > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
>> > > with this change here.)
>
> We should try to find a testcase.  Can you provide a backtrace when it
> happens?  It should be possible to write a testcase with the backtrace.

In my reproducer, when %rbx is about to be clobbered, I see
(%rsp % 64) == 8 at the start of _dl_tlsdesc_dynamic_xsavec.

The %rbx register does not get clobbered if (%rsp % 64) == 56.

Does this help?

Thanks,
Florian

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 8:04 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Sat, Mar 16, 2024 at 7:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >> >
> >> > On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> > >
> >> > > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> >> > > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> >> > > This realignment does not take into account that the function has
> >> > > already used part of the red zone at this point, thus clobbering
> >> > > the initally saved register values located there if the stack
> >> > > alignment inherited from the caller is unfortunate.
> >> > >
> >> > > (Note: I do not know to write a good test case for this in the existing
> >> > > framework.  We saw this as a random LTO plugin crash when building GCC
> >> > > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> >> > > with this change here.)
> >
> > We should try to find a testcase.  Can you provide a backtrace when it
> > happens?  It should be possible to write a testcase with the backtrace.
>
> In my reproducer, when %rbx is about to be clobbered, I see
> (%rsp % 64) == 8 at the start of _dl_tlsdesc_dynamic_xsavec.
>
> The %rbx register does not get clobbered if (%rsp % 64) == 56.
>
> Does this help?
>

Yes.  I am working on a testcase.


-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]

On Sat, Mar 16, 2024 at 8:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 8:04 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > > On Sat, Mar 16, 2024 at 7:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >>
> > >> On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >> >
> > >> > On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >> > >
> > >> > > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> > >> > > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> > >> > > This realignment does not take into account that the function has
> > >> > > already used part of the red zone at this point, thus clobbering
> > >> > > the initally saved register values located there if the stack
> > >> > > alignment inherited from the caller is unfortunate.
> > >> > >
> > >> > > (Note: I do not know to write a good test case for this in the existing
> > >> > > framework.  We saw this as a random LTO plugin crash when building GCC
> > >> > > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> > >> > > with this change here.)
> > >
> > > We should try to find a testcase.  Can you provide a backtrace when it
> > > happens?  It should be possible to write a testcase with the backtrace.
> >
> > In my reproducer, when %rbx is about to be clobbered, I see
> > (%rsp % 64) == 8 at the start of _dl_tlsdesc_dynamic_xsavec.
> >
> > The %rbx register does not get clobbered if (%rsp % 64) == 56.
> >
> > Does this help?
> >
>
> Yes.  I am working on a testcase.

Hi Florian,

Please verify if this is the right testcase.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Add-a-test-for-BZ-31501.patch --]
[-- Type: text/x-patch, Size: 8698 bytes --]

From 8c167bd4d9faddf9580f55182542e8a05a25fd61 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 16 Mar 2024 08:42:27 -0700
Subject: [PATCH] Add a test for BZ #31501

---
 sysdeps/x86_64/Makefile                    | 16 +++++
 sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S | 57 +++++++++++++++++
 sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S | 74 ++++++++++++++++++++++
 sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod2.S | 57 +++++++++++++++++
 sysdeps/x86_64/tst-gnu2-tls2-x86-64.c      | 21 ++++++
 5 files changed, 225 insertions(+)
 create mode 100644 sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
 create mode 100644 sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
 create mode 100644 sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod2.S
 create mode 100644 sysdeps/x86_64/tst-gnu2-tls2-x86-64.c

diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
index 66b21954f3..f020773e56 100644
--- a/sysdeps/x86_64/Makefile
+++ b/sysdeps/x86_64/Makefile
@@ -217,6 +217,22 @@ valgrind-suppressions-tst-valgrind-smoke = \
   --suppressions=$(..)sysdeps/x86_64/tst-valgrind-smoke.supp
 endif
 
+tests += \
+  tst-gnu2-tls2-x86-64 \
+# tests
+
+modules-names += \
+  tst-gnu2-tls2-x86-64-mod0 \
+  tst-gnu2-tls2-x86-64-mod1 \
+  tst-gnu2-tls2-x86-64-mod2 \
+# modules-names
+
+$(objpfx)tst-gnu2-tls2-x86-64: $(shared-thread-library)
+$(objpfx)tst-gnu2-tls2-x86-64.out: \
+  $(objpfx)tst-gnu2-tls2-x86-64-mod0.so \
+  $(objpfx)tst-gnu2-tls2-x86-64-mod1.so \
+  $(objpfx)tst-gnu2-tls2-x86-64-mod2.so
+
 endif # $(subdir) == elf
 
 ifeq ($(subdir),csu)
diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
new file mode 100644
index 0000000000..8129b28061
--- /dev/null
+++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
@@ -0,0 +1,57 @@
+/* Test TLSDESC relocation with x86-64.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+	.text
+	.p2align 4
+	.globl	apply_tls
+	.type	apply_tls, @function
+apply_tls:
+	.cfi_startproc
+	subq	$24, %rsp
+	.cfi_def_cfa_offset 32
+	movdqu	(%rdi), %xmm0
+	movq	%fs:40, %rax
+	movq	%rax, 8(%rsp)
+	xorl	%eax, %eax
+	leaq	tls_var0@TLSDESC(%rip), %rax
+	call	*tls_var0@TLSCALL(%rax)
+	addq	%fs:0, %rax
+	movups	%xmm0, (%rax)
+	movdqu	16(%rdi), %xmm1
+	movups	%xmm1, 16(%rax)
+	movq	8(%rsp), %rdx
+	subq	%fs:40, %rdx
+	jne	.L5
+	addq	$24, %rsp
+	.cfi_remember_state
+	.cfi_def_cfa_offset 8
+	ret
+.L5:
+	.cfi_restore_state
+	call	__stack_chk_fail@PLT
+	.cfi_endproc
+	.size	apply_tls, .-apply_tls
+	.hidden	tls_var0
+	.globl	tls_var0
+	.section	.tbss,"awT",@nobits
+	.align 8
+	.type	tls_var0, @object
+	.size	tls_var0, 32
+tls_var0:
+	.zero	32
+	.section	.note.GNU-stack,"",@progbits
diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
new file mode 100644
index 0000000000..af4b7ca761
--- /dev/null
+++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
@@ -0,0 +1,74 @@
+/* Test TLSDESC relocation with x86-64.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Select an offset which will cause _dl_tlsdesc_dynamic_xsavec to
+   clobber %rbx.  */
+#define OFFSET (56 + 16 + 16 + 16)
+
+	.text
+	.p2align 4
+	.globl	apply_tls
+	.type	apply_tls, @function
+apply_tls:
+	.cfi_startproc
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	/* Align stack to 64 bytes.  */
+	andq	$-64, %rsp
+	pushq	%rbx
+	subq	$OFFSET, %rsp
+	movdqu	(%rdi), %xmm0
+	movq	%fs:40, %rax
+	movq	%rax, 56(%rsp)
+	xorl	%eax, %eax
+	leaq	tls_var1@TLSDESC(%rip), %rax
+	call	*tls_var1@TLSCALL(%rax)
+	addq	%fs:0, %rax
+	movups	%xmm0, 32(%rax)
+	movdqu	16(%rdi), %xmm1
+	addq	$32, %rax
+	movups	%xmm1, 16(%rax)
+	movq	56(%rsp), %rdx
+	subq	%fs:40, %rdx
+	jne	.L5
+	/* All versions of _dl_tlsdesc_dynamic should preserve %rbx.  */
+	cmpq	OFFSET(%rsp), %rbx
+	jne	.Lhlt
+	leave
+	.cfi_remember_state
+	.cfi_def_cfa 7, 8
+	ret
+.Lhlt:
+	hlt
+.L5:
+	.cfi_restore_state
+	call	__stack_chk_fail@PLT
+	.cfi_endproc
+	.size	apply_tls, .-apply_tls
+	.hidden	tls_var1
+	.globl	tls_var1
+	.section	.tbss,"awT",@nobits
+	.align 16
+	.type	tls_var1, @object
+	.size	tls_var1, 3200
+tls_var1:
+	.zero	3200
+	.section	.note.GNU-stack,"",@progbits
diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod2.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod2.S
new file mode 100644
index 0000000000..d82f5f029b
--- /dev/null
+++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod2.S
@@ -0,0 +1,57 @@
+/* Test TLSDESC relocation with x86-64.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+	.text
+	.p2align 4
+	.globl	apply_tls
+	.type	apply_tls, @function
+apply_tls:
+	.cfi_startproc
+	subq	$24, %rsp
+	.cfi_def_cfa_offset 32
+	movdqu	(%rdi), %xmm0
+	movq	%fs:40, %rax
+	movq	%rax, 8(%rsp)
+	xorl	%eax, %eax
+	leaq	tls_var2@TLSDESC(%rip), %rax
+	call	*tls_var2@TLSCALL(%rax)
+	addq	%fs:0, %rax
+	movups	%xmm0, (%rax)
+	movdqu	16(%rdi), %xmm1
+	movups	%xmm1, 16(%rax)
+	movq	8(%rsp), %rdx
+	subq	%fs:40, %rdx
+	jne	.L5
+	addq	$24, %rsp
+	.cfi_remember_state
+	.cfi_def_cfa_offset 8
+	ret
+.L5:
+	.cfi_restore_state
+	call	__stack_chk_fail@PLT
+	.cfi_endproc
+	.size	apply_tls, .-apply_tls
+	.hidden	tls_var2
+	.globl	tls_var2
+	.section	.tbss,"awT",@nobits
+	.align 8
+	.type	tls_var2, @object
+	.size	tls_var2, 32
+tls_var2:
+	.zero	32
+	.section	.note.GNU-stack,"",@progbits
diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64.c b/sysdeps/x86_64/tst-gnu2-tls2-x86-64.c
new file mode 100644
index 0000000000..787181545b
--- /dev/null
+++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64.c
@@ -0,0 +1,21 @@
+/* Test TLSDESC relocation with x86-64.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define MOD(i) "tst-gnu2-tls2-x86-64-mod" #i ".so"
+
+#include <elf/tst-gnu2-tls2.c>
-- 
2.44.0

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 9:32 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 8:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sat, Mar 16, 2024 at 8:04 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >
> > > * H. J. Lu:
> > >
> > > > On Sat, Mar 16, 2024 at 7:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >>
> > > >> On Sat, Mar 16, 2024 at 7:37 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >> >
> > > >> > On Sat, Mar 16, 2024 at 7:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> > > >> > >
> > > >> > > In sysdeps/x86_64/dl-tlsdesc-dynamic.h, the stack pointer is
> > > >> > > realigned for some variants (notably _dl_tlsdesc_dynamic_xsavec).
> > > >> > > This realignment does not take into account that the function has
> > > >> > > already used part of the red zone at this point, thus clobbering
> > > >> > > the initally saved register values located there if the stack
> > > >> > > alignment inherited from the caller is unfortunate.
> > > >> > >
> > > >> > > (Note: I do not know to write a good test case for this in the existing
> > > >> > > framework.  We saw this as a random LTO plugin crash when building GCC
> > > >> > > with -mtls-dialect=gnu2.  The existing tests on pass on x86_64-linux-gnu
> > > >> > > with this change here.)
> > > >
> > > > We should try to find a testcase.  Can you provide a backtrace when it
> > > > happens?  It should be possible to write a testcase with the backtrace.
> > >
> > > In my reproducer, when %rbx is about to be clobbered, I see
> > > (%rsp % 64) == 8 at the start of _dl_tlsdesc_dynamic_xsavec.
> > >
> > > The %rbx register does not get clobbered if (%rsp % 64) == 56.
> > >
> > > Does this help?
> > >
> >
> > Yes.  I am working on a testcase.
>
> Hi Florian,
>
> Please verify if this is the right testcase.

This test fails only on AVX512 machines.


-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[Florian Weimer]

* H. J. Lu:

> Please verify if this is the right testcase.

Test case works (fails without my fix, succeeds with my fix).  Some
comments below.

> diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> new file mode 100644
> index 0000000000..8129b28061
> --- /dev/null
> +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> @@ -0,0 +1,57 @@

> +	.text
> +	.p2align 4
> +	.globl	apply_tls
> +	.type	apply_tls, @function
> +apply_tls:
> +	.cfi_startproc

Missing CET marker.

> +	subq	$24, %rsp
> +	.cfi_def_cfa_offset 32
> +	movdqu	(%rdi), %xmm0
> +	movq	%fs:40, %rax
> +	movq	%rax, 8(%rsp)
> +	xorl	%eax, %eax
> +	leaq	tls_var0@TLSDESC(%rip), %rax
> +	call	*tls_var0@TLSCALL(%rax)
> +	addq	%fs:0, %rax
> +	movups	%xmm0, (%rax)
> +	movdqu	16(%rdi), %xmm1
> +	movups	%xmm1, 16(%rax)
> +	movq	8(%rsp), %rdx
> +	subq	%fs:40, %rdx
> +	jne	.L5
> +	addq	$24, %rsp
> +	.cfi_remember_state
> +	.cfi_def_cfa_offset 8
> +	ret
> +.L5:
> +	.cfi_restore_state
> +	call	__stack_chk_fail@PLT

Not sure if we need this?

Maybe add some comment what exactly this subtest is exercising?

These are present in the other TLS modules as well.

> diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> new file mode 100644
> index 0000000000..af4b7ca761
> --- /dev/null
> +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S

> +/* Select an offset which will cause _dl_tlsdesc_dynamic_xsavec to
> +   clobber %rbx.  */
> +#define OFFSET (56 + 16 + 16 + 16)
> +
> +	.text
> +	.p2align 4
> +	.globl	apply_tls
> +	.type	apply_tls, @function
> +apply_tls:
> +	.cfi_startproc
> +	pushq	%rbp
> +	.cfi_def_cfa_offset 16
> +	.cfi_offset 6, -16
> +	movq	%rsp, %rbp
> +	.cfi_def_cfa_register 6
> +	/* Align stack to 64 bytes.  */
> +	andq	$-64, %rsp
> +	pushq	%rbx
> +	subq	$OFFSET, %rsp

The offset could be loaded from a global variable or something like
that.  We should exercise a wide range of stack alignments—the
individual tests are cheap.  And maybe check extra registers.

Thanks,
Florian

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 10:42 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > Please verify if this is the right testcase.
>
> Test case works (fails without my fix, succeeds with my fix).  Some
> comments below.
>
> > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > new file mode 100644
> > index 0000000000..8129b28061
> > --- /dev/null
> > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > @@ -0,0 +1,57 @@
>
> > +     .text
> > +     .p2align 4
> > +     .globl  apply_tls
> > +     .type   apply_tls, @function
> > +apply_tls:
> > +     .cfi_startproc
>
> Missing CET marker.
>
> > +     subq    $24, %rsp
> > +     .cfi_def_cfa_offset 32
> > +     movdqu  (%rdi), %xmm0
> > +     movq    %fs:40, %rax
> > +     movq    %rax, 8(%rsp)
> > +     xorl    %eax, %eax
> > +     leaq    tls_var0@TLSDESC(%rip), %rax
> > +     call    *tls_var0@TLSCALL(%rax)
> > +     addq    %fs:0, %rax
> > +     movups  %xmm0, (%rax)
> > +     movdqu  16(%rdi), %xmm1
> > +     movups  %xmm1, 16(%rax)
> > +     movq    8(%rsp), %rdx
> > +     subq    %fs:40, %rdx
> > +     jne     .L5
> > +     addq    $24, %rsp
> > +     .cfi_remember_state
> > +     .cfi_def_cfa_offset 8
> > +     ret
> > +.L5:
> > +     .cfi_restore_state
> > +     call    __stack_chk_fail@PLT
>
> Not sure if we need this?
>
> Maybe add some comment what exactly this subtest is exercising?
>
> These are present in the other TLS modules as well.
>
> > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> > new file mode 100644
> > index 0000000000..af4b7ca761
> > --- /dev/null
> > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
>
> > +/* Select an offset which will cause _dl_tlsdesc_dynamic_xsavec to
> > +   clobber %rbx.  */
> > +#define OFFSET (56 + 16 + 16 + 16)
> > +
> > +     .text
> > +     .p2align 4
> > +     .globl  apply_tls
> > +     .type   apply_tls, @function
> > +apply_tls:
> > +     .cfi_startproc
> > +     pushq   %rbp
> > +     .cfi_def_cfa_offset 16
> > +     .cfi_offset 6, -16
> > +     movq    %rsp, %rbp
> > +     .cfi_def_cfa_register 6
> > +     /* Align stack to 64 bytes.  */
> > +     andq    $-64, %rsp
> > +     pushq   %rbx
> > +     subq    $OFFSET, %rsp
>
> The offset could be loaded from a global variable or something like
> that.  We should exercise a wide range of stack alignments—the
> individual tests are cheap.  And maybe check extra registers.

I will clean it up with a different fix.

-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 10:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 10:42 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > > Please verify if this is the right testcase.
> >
> > Test case works (fails without my fix, succeeds with my fix).  Some
> > comments below.
> >
> > > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > > new file mode 100644
> > > index 0000000000..8129b28061
> > > --- /dev/null
> > > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > > @@ -0,0 +1,57 @@
> >
> > > +     .text
> > > +     .p2align 4
> > > +     .globl  apply_tls
> > > +     .type   apply_tls, @function
> > > +apply_tls:
> > > +     .cfi_startproc
> >
> > Missing CET marker.
> >
> > > +     subq    $24, %rsp
> > > +     .cfi_def_cfa_offset 32
> > > +     movdqu  (%rdi), %xmm0
> > > +     movq    %fs:40, %rax
> > > +     movq    %rax, 8(%rsp)
> > > +     xorl    %eax, %eax
> > > +     leaq    tls_var0@TLSDESC(%rip), %rax
> > > +     call    *tls_var0@TLSCALL(%rax)
> > > +     addq    %fs:0, %rax
> > > +     movups  %xmm0, (%rax)
> > > +     movdqu  16(%rdi), %xmm1
> > > +     movups  %xmm1, 16(%rax)
> > > +     movq    8(%rsp), %rdx
> > > +     subq    %fs:40, %rdx
> > > +     jne     .L5
> > > +     addq    $24, %rsp
> > > +     .cfi_remember_state
> > > +     .cfi_def_cfa_offset 8
> > > +     ret
> > > +.L5:
> > > +     .cfi_restore_state
> > > +     call    __stack_chk_fail@PLT
> >
> > Not sure if we need this?
> >
> > Maybe add some comment what exactly this subtest is exercising?
> >
> > These are present in the other TLS modules as well.
> >
> > > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> > > new file mode 100644
> > > index 0000000000..af4b7ca761
> > > --- /dev/null
> > > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> >
> > > +/* Select an offset which will cause _dl_tlsdesc_dynamic_xsavec to
> > > +   clobber %rbx.  */
> > > +#define OFFSET (56 + 16 + 16 + 16)
> > > +
> > > +     .text
> > > +     .p2align 4
> > > +     .globl  apply_tls
> > > +     .type   apply_tls, @function
> > > +apply_tls:
> > > +     .cfi_startproc
> > > +     pushq   %rbp
> > > +     .cfi_def_cfa_offset 16
> > > +     .cfi_offset 6, -16
> > > +     movq    %rsp, %rbp
> > > +     .cfi_def_cfa_register 6
> > > +     /* Align stack to 64 bytes.  */
> > > +     andq    $-64, %rsp
> > > +     pushq   %rbx
> > > +     subq    $OFFSET, %rsp
> >
> > The offset could be loaded from a global variable or something like
> > that.  We should exercise a wide range of stack alignments—the
> > individual tests are cheap.  And maybe check extra registers.
>
> I will clean it up with a different fix.
>

I submitted a patch with a testase:

https://patchwork.sourceware.org/project/glibc/list/?series=31963

My patch allocates 64 more bytes to avoid clobbering saved RDI,
RSI and RBX values on stack by xsave.  It avoids 2 stack
adjustments.  Either my fix or Florian's fix should fix the issue.
I don't have a strong preference as long as my testcase is
included.

Thanks.

-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 3:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 10:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sat, Mar 16, 2024 at 10:42 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >
> > > * H. J. Lu:
> > >
> > > > Please verify if this is the right testcase.
> > >
> > > Test case works (fails without my fix, succeeds with my fix).  Some
> > > comments below.
> > >
> > > > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > > > new file mode 100644
> > > > index 0000000000..8129b28061
> > > > --- /dev/null
> > > > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > > > @@ -0,0 +1,57 @@
> > >
> > > > +     .text
> > > > +     .p2align 4
> > > > +     .globl  apply_tls
> > > > +     .type   apply_tls, @function
> > > > +apply_tls:
> > > > +     .cfi_startproc
> > >
> > > Missing CET marker.
> > >
> > > > +     subq    $24, %rsp
> > > > +     .cfi_def_cfa_offset 32
> > > > +     movdqu  (%rdi), %xmm0
> > > > +     movq    %fs:40, %rax
> > > > +     movq    %rax, 8(%rsp)
> > > > +     xorl    %eax, %eax
> > > > +     leaq    tls_var0@TLSDESC(%rip), %rax
> > > > +     call    *tls_var0@TLSCALL(%rax)
> > > > +     addq    %fs:0, %rax
> > > > +     movups  %xmm0, (%rax)
> > > > +     movdqu  16(%rdi), %xmm1
> > > > +     movups  %xmm1, 16(%rax)
> > > > +     movq    8(%rsp), %rdx
> > > > +     subq    %fs:40, %rdx
> > > > +     jne     .L5
> > > > +     addq    $24, %rsp
> > > > +     .cfi_remember_state
> > > > +     .cfi_def_cfa_offset 8
> > > > +     ret
> > > > +.L5:
> > > > +     .cfi_restore_state
> > > > +     call    __stack_chk_fail@PLT
> > >
> > > Not sure if we need this?
> > >
> > > Maybe add some comment what exactly this subtest is exercising?
> > >
> > > These are present in the other TLS modules as well.
> > >
> > > > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> > > > new file mode 100644
> > > > index 0000000000..af4b7ca761
> > > > --- /dev/null
> > > > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> > >
> > > > +/* Select an offset which will cause _dl_tlsdesc_dynamic_xsavec to
> > > > +   clobber %rbx.  */
> > > > +#define OFFSET (56 + 16 + 16 + 16)
> > > > +
> > > > +     .text
> > > > +     .p2align 4
> > > > +     .globl  apply_tls
> > > > +     .type   apply_tls, @function
> > > > +apply_tls:
> > > > +     .cfi_startproc
> > > > +     pushq   %rbp
> > > > +     .cfi_def_cfa_offset 16
> > > > +     .cfi_offset 6, -16
> > > > +     movq    %rsp, %rbp
> > > > +     .cfi_def_cfa_register 6
> > > > +     /* Align stack to 64 bytes.  */
> > > > +     andq    $-64, %rsp
> > > > +     pushq   %rbx
> > > > +     subq    $OFFSET, %rsp
> > >
> > > The offset could be loaded from a global variable or something like
> > > that.  We should exercise a wide range of stack alignments—the
> > > individual tests are cheap.  And maybe check extra registers.
> >
> > I will clean it up with a different fix.
> >
>
> I submitted a patch with a testase:
>
> https://patchwork.sourceware.org/project/glibc/list/?series=31963
>
> My patch allocates 64 more bytes to avoid clobbering saved RDI,
> RSI and RBX values on stack by xsave.  It avoids 2 stack
> adjustments.  Either my fix or Florian's fix should fix the issue.
> I don't have a strong preference as long as my testcase is
> included.
>
>
I think my testcase may fail on AMD AVX CPUs without the
fix.  On Intel AVX CPUs, the state size is 960 bytes.  But the
last 128 bytes may be unused.

-- 
H.J.

Re: [PATCH] x86-64: Stack alignment in _dl_tlsdesc_dynamic and red zone usage (bug 31501)

[H.J. Lu]

On Sat, Mar 16, 2024 at 6:19 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Mar 16, 2024 at 3:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sat, Mar 16, 2024 at 10:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Sat, Mar 16, 2024 at 10:42 AM Florian Weimer <fweimer@redhat.com> wrote:
> > > >
> > > > * H. J. Lu:
> > > >
> > > > > Please verify if this is the right testcase.
> > > >
> > > > Test case works (fails without my fix, succeeds with my fix).  Some
> > > > comments below.
> > > >
> > > > > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > > > > new file mode 100644
> > > > > index 0000000000..8129b28061
> > > > > --- /dev/null
> > > > > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod0.S
> > > > > @@ -0,0 +1,57 @@
> > > >
> > > > > +     .text
> > > > > +     .p2align 4
> > > > > +     .globl  apply_tls
> > > > > +     .type   apply_tls, @function
> > > > > +apply_tls:
> > > > > +     .cfi_startproc
> > > >
> > > > Missing CET marker.
> > > >
> > > > > +     subq    $24, %rsp
> > > > > +     .cfi_def_cfa_offset 32
> > > > > +     movdqu  (%rdi), %xmm0
> > > > > +     movq    %fs:40, %rax
> > > > > +     movq    %rax, 8(%rsp)
> > > > > +     xorl    %eax, %eax
> > > > > +     leaq    tls_var0@TLSDESC(%rip), %rax
> > > > > +     call    *tls_var0@TLSCALL(%rax)
> > > > > +     addq    %fs:0, %rax
> > > > > +     movups  %xmm0, (%rax)
> > > > > +     movdqu  16(%rdi), %xmm1
> > > > > +     movups  %xmm1, 16(%rax)
> > > > > +     movq    8(%rsp), %rdx
> > > > > +     subq    %fs:40, %rdx
> > > > > +     jne     .L5
> > > > > +     addq    $24, %rsp
> > > > > +     .cfi_remember_state
> > > > > +     .cfi_def_cfa_offset 8
> > > > > +     ret
> > > > > +.L5:
> > > > > +     .cfi_restore_state
> > > > > +     call    __stack_chk_fail@PLT
> > > >
> > > > Not sure if we need this?
> > > >
> > > > Maybe add some comment what exactly this subtest is exercising?
> > > >
> > > > These are present in the other TLS modules as well.
> > > >
> > > > > diff --git a/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> > > > > new file mode 100644
> > > > > index 0000000000..af4b7ca761
> > > > > --- /dev/null
> > > > > +++ b/sysdeps/x86_64/tst-gnu2-tls2-x86-64-mod1.S
> > > >
> > > > > +/* Select an offset which will cause _dl_tlsdesc_dynamic_xsavec to
> > > > > +   clobber %rbx.  */
> > > > > +#define OFFSET (56 + 16 + 16 + 16)
> > > > > +
> > > > > +     .text
> > > > > +     .p2align 4
> > > > > +     .globl  apply_tls
> > > > > +     .type   apply_tls, @function
> > > > > +apply_tls:
> > > > > +     .cfi_startproc
> > > > > +     pushq   %rbp
> > > > > +     .cfi_def_cfa_offset 16
> > > > > +     .cfi_offset 6, -16
> > > > > +     movq    %rsp, %rbp
> > > > > +     .cfi_def_cfa_register 6
> > > > > +     /* Align stack to 64 bytes.  */
> > > > > +     andq    $-64, %rsp
> > > > > +     pushq   %rbx
> > > > > +     subq    $OFFSET, %rsp
> > > >
> > > > The offset could be loaded from a global variable or something like
> > > > that.  We should exercise a wide range of stack alignments—the
> > > > individual tests are cheap.  And maybe check extra registers.
> > >
> > > I will clean it up with a different fix.
> > >
> >
> > I submitted a patch with a testase:
> >
> > https://patchwork.sourceware.org/project/glibc/list/?series=31963
> >
> > My patch allocates 64 more bytes to avoid clobbering saved RDI,
> > RSI and RBX values on stack by xsave.  It avoids 2 stack
> > adjustments.  Either my fix or Florian's fix should fix the issue.
> > I don't have a strong preference as long as my testcase is
> > included.
> >
> >
> I think my testcase may fail on AMD AVX CPUs without the
> fix.  On Intel AVX CPUs, the state size is 960 bytes.  But the
> last 128 bytes may be unused.
>

I sent out the v2 patch:

https://patchwork.sourceware.org/project/glibc/list/?series=31966

to simplify the testcase.

-- 
H.J.