Re: [PATCH v4] x86-64: Allocate state buffer space for RDI, RSI and RBX

[Florian Weimer <fweimer@redhat.com>]

* H. J. Lu:

> _dl_tlsdesc_dynamic preserves RDI, RSI and RBX before realigning stack.
> After realigning stack, it saves RCX, RDX, R8, R9, R10 and R11.  Define
> TLSDESC_CALL_REGISTER_SAVE_AREA to allocate space for RDI, RSI and RBX
> to avoid clobbering saved RDI, RSI and RBX values on stack by xsave to
> STATE_SAVE_OFFSET(%rsp).
>
>    +==================+<- stack frame start aligned at 8 or 16 bytes

It's 8-byte aligned only?  If the caller uses the psABI convention, we
have %rsp ≡ 8 (mod 16).

>    |                  |<- RDI
>    |                  |<- RSI
>    |                  |<- RBX

I would add something like “originally in the red zone” here.

>    |                  |<- paddings from stack realignment of 64 bytes
>    |------------------|<- xsave buffer end aligned at 64 bytes


>    |                  |<-
>    |                  |<-
>    |                  |<-
>    |------------------|<- xsave buffer start at STATE_SAVE_OFFSET(%rsp)
>    |                  |<- 8-byte padding

Maybe add “to achieve 64-bit alignment”.

>    |                  |<- 8-byte padding
>    |                  |<- R11
>    |                  |<- R10
>    |                  |<- R9
>    |                  |<- R8
>    |                  |<- RDX
>    |                  |<- RCX
>    +==================+<- State buffer start aligned at 64 bytes

Isn't this “%rsp  aligned at 64 bytes”?

Likewise for the comment in the sources below.

> diff --git a/sysdeps/x86/sysdep.h b/sysdeps/x86/sysdep.h
> index db8e576e91..46fcd27345 100644
> --- a/sysdeps/x86/sysdep.h
> +++ b/sysdeps/x86/sysdep.h
> @@ -46,6 +46,34 @@
>     red-zone into account.  */
>  # define STATE_SAVE_OFFSET (8 * 7 + 8)

The comment on STATE_SAVE_OFFSET needs updating, too.

I would like to see comments from Noah or Sunil.  You'll have to
maintain this, too. 8-)

I find the macro consts rather confusing, but maybe that's just me.

Thanks,
Florian