[PATCHi,v2] aarch64: Add split-stack TCB field

Changes from previous version [1]

  - Instead of allocate the split-stack field on tcbhead_t, allocate
    is before thread pointer and adjust struct pthread get methods.

--

This patch adds split-stack support pointer guard on glibc for aarch64.
Different from other architectures (powerpc, s390) where the memory is
placed on TCB, aarch64 one is placed bofore thread pointer initial
position.  It has an advantage over extending TCB because for aarch64
TLS variable placement take in consideration tcbhead_t size and by
changing its value would require to also update the static linker
(and it would also add incompatibility with glibc and older linkers).

For aarch64 tcb direct access is fastest for thread local variable on
all mode and related TLS access.  It requires just a direct load with
displacement of -8 (since thread pointer points to tcbhead_t).

It also adds a loader symbol (__tcb_private_ss) to signal de existence
of the split stack guard area.

Checked on aarch64-linux-gnu.

	* sysdeps/aarch64/Makefile [$(subdir) = elf] (sysdeps-dl-routines):
	Add tcb-version.
	* sysdeps/aarch64/Versions [ld] (GLIBC_2.26): Add __tcb_private_ss.
	* sysdeps/aarch64/nptl/tls.h (tcbprehead_t): New struct.
	(TLS_PRE_TCB_SIZE): Take tcbprehead_t in consideration.
	(TLS_DEFINE_INIT_TP): Likewise.
	(THREAD_SELF): Likewise.
	(DB_THREAD_SELF): Likewise.
	* sysdeps/aarch64/tcb-version.c: New file.
	* sysdeps/unix/sysv/linux/aarch64/ld.abilist (GLIBC_2.26): Add.
	(__tcb_private_ss): Likewise.
	* sysdeps/aarch64/nptl/tcb-offsets.sym (pthread_sizeof): rename to
	pthread_pre_tcb_size.
	* sysdeps/unix/sysv/linux/aarch64/sysdep-cancel.h
	[!is_in (libprthread) && !is_in (libc) (single_thread_p): use
	pthread_pre_tcb_size instead of pthread_sizeof.

[1] https://sourceware.org/ml/libc-alpha/2016-07/msg00647.html
---
 ChangeLog                                       | 19 +++++++++++++++++++
 sysdeps/aarch64/Makefile                        |  2 +-
 sysdeps/aarch64/Versions                        |  8 ++++++++
 sysdeps/aarch64/nptl/tcb-offsets.sym            |  2 +-
 sysdeps/aarch64/nptl/tls.h                      | 25 ++++++++++++++++++++-----
 sysdeps/aarch64/tcb-version.c                   | 24 ++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/aarch64/ld.abilist      |  2 ++
 sysdeps/unix/sysv/linux/aarch64/sysdep-cancel.h |  2 +-
 8 files changed, 76 insertions(+), 8 deletions(-)
 create mode 100644 sysdeps/aarch64/tcb-version.c

On 02/13/2017 08:45 PM, Adhemerval Zanella wrote:
> +/* This is the size we need before TCB.  Check if there is room for
> +   tcbprehead_t in struct pthread's final padding and if not add it on
> +   required pre-tcb size.  */
> +# define TLS_PRE_TCB_SIZE \
> +  (sizeof (struct pthread)						\
> +   + (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)		\
> +      ? ALIGN_UP (sizeof (tcbprehead_t), sizeof (struct pthread))	\
> +      : 0))

How does this preserve the alignment of struct pthread?

It's also not clear to me how the “version control” aspect of 
__tcb_private_ss is supposed to work.  If the intent is to prevent 
loading of split-stack binaries with an older glibc, then a data symbol 
would be a safer choice.

Thanks,
Florian

On 14/02/2017 08:03, Florian Weimer wrote:
> On 02/13/2017 08:45 PM, Adhemerval Zanella wrote:
>> +/* This is the size we need before TCB.  Check if there is room for
>> +   tcbprehead_t in struct pthread's final padding and if not add it on
>> +   required pre-tcb size.  */
>> +# define TLS_PRE_TCB_SIZE \
>> +  (sizeof (struct pthread)                        \
>> +   + (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)        \
>> +      ? ALIGN_UP (sizeof (tcbprehead_t), sizeof (struct pthread))    \
>> +      : 0))
> 
> How does this preserve the alignment of struct pthread?

Indeed, it should be ALIGN_UP (sizeof (tcbprehead_t), __alignof__ (struct pthread)

> 
> It's also not clear to me how the “version control” aspect of __tcb_private_ss is supposed to work.  If the intent is to prevent loading of split-stack binaries with an older glibc, then a data symbol would be a safer choice.
> 

My idea is to mimic 67385a01d22 (powerpc: Add hwcap/hwcap2/platform data to TCB) and
is indeed to prevent new binaries to run on older glibc where the pre-tcb header
is not allocated.  I will check if a data symbol works better.

On 02/14/2017 05:03 AM, Florian Weimer wrote:
> On 02/13/2017 08:45 PM, Adhemerval Zanella wrote:
>> +/* This is the size we need before TCB.  Check if there is room
>> for +   tcbprehead_t in struct pthread's final padding and if not
>> add it on +   required pre-tcb size.  */ +# define TLS_PRE_TCB_SIZE
>> \ +  (sizeof (struct pthread)                        \ +   +
>> (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)        \ +
>> ? ALIGN_UP (sizeof (tcbprehead_t), sizeof (struct pthread))    \ +
>> : 0))
> 
> How does this preserve the alignment of struct pthread?
> 
> It's also not clear to me how the “version control” aspect of
> __tcb_private_ss is supposed to work.  If the intent is to prevent
> loading of split-stack binaries with an older glibc, then a data
> symbol would be a safer choice.

Why is a data symbol a safer choice?

I'm curious, because I recommended the design pattern that Adhemerval
is copying from the POWER example.

On 03/14/2017 02:17 AM, Carlos O'Donell wrote:
> On 02/14/2017 05:03 AM, Florian Weimer wrote:
>> On 02/13/2017 08:45 PM, Adhemerval Zanella wrote:
>>> +/* This is the size we need before TCB.  Check if there is room
>>> for +   tcbprehead_t in struct pthread's final padding and if not
>>> add it on +   required pre-tcb size.  */ +# define TLS_PRE_TCB_SIZE
>>> \ +  (sizeof (struct pthread)                        \ +   +
>>> (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)        \ +
>>> ? ALIGN_UP (sizeof (tcbprehead_t), sizeof (struct pthread))    \ +
>>> : 0))
>>
>> How does this preserve the alignment of struct pthread?
>>
>> It's also not clear to me how the “version control” aspect of
>> __tcb_private_ss is supposed to work.  If the intent is to prevent
>> loading of split-stack binaries with an older glibc, then a data
>> symbol would be a safer choice.
>
> Why is a data symbol a safer choice?

No lazy binding and hence a more well-defined failure mode.

Thanks,
Florian

On 14/03/2017 03:53, Florian Weimer wrote:
> On 03/14/2017 02:17 AM, Carlos O'Donell wrote:
>> On 02/14/2017 05:03 AM, Florian Weimer wrote:
>>> On 02/13/2017 08:45 PM, Adhemerval Zanella wrote:
>>>> +/* This is the size we need before TCB.  Check if there is room
>>>> for +   tcbprehead_t in struct pthread's final padding and if not
>>>> add it on +   required pre-tcb size.  */ +# define TLS_PRE_TCB_SIZE
>>>> \ +  (sizeof (struct pthread)                        \ +   +
>>>> (PTHREAD_STRUCT_END_PADDING < sizeof (tcbprehead_t)        \ +
>>>> ? ALIGN_UP (sizeof (tcbprehead_t), sizeof (struct pthread))    \ +
>>>> : 0))
>>>
>>> How does this preserve the alignment of struct pthread?
>>>
>>> It's also not clear to me how the “version control” aspect of
>>> __tcb_private_ss is supposed to work.  If the intent is to prevent
>>> loading of split-stack binaries with an older glibc, then a data
>>> symbol would be a safer choice.
>>
>> Why is a data symbol a safer choice?
> 
> No lazy binding and hence a more well-defined failure mode.
> 
> Thanks,
> Florian
> 

Carlos, I also see Florian suggestion a better approach.  Using a weak
function symbol, it will fail at runtime after symbol resolution with
an undefined jump to a null symbol (for aarch64 it will be a segfault
anyway), while with data symbol loader will explicit dump the missing
symbol.