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

["H.J. Lu" <hjl.tools@gmail.com>]

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.