Re: [PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
- On Jun 24, 2020, at 3:24 PM, Florian Weimer fwei...@redhat.com wrote: > * Mathieu Desnoyers: > >>> I think we should keep things simple on the glibc side for now and do >>> this changes to the kernel headers first. >> >> Just to be sure I understand what you mean by "keep things simple", do you >> recommend removing the following lines completely for now from sys/rseq.h ? >> >> /* Ensure the compiler supports rseq_align. */ >> __rseq_static_assert (__rseq_alignof (struct rseq_cs) >= 32, "alignment"); >> __rseq_static_assert (__rseq_alignof (struct rseq) >= 32, "alignment"); > > Yes, that's what I meant. Agreed, I queued this change for my next round. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
* Mathieu Desnoyers: >> I think we should keep things simple on the glibc side for now and do >> this changes to the kernel headers first. > > Just to be sure I understand what you mean by "keep things simple", do you > recommend removing the following lines completely for now from sys/rseq.h ? > > /* Ensure the compiler supports rseq_align. */ > __rseq_static_assert (__rseq_alignof (struct rseq_cs) >= 32, "alignment"); > __rseq_static_assert (__rseq_alignof (struct rseq) >= 32, "alignment"); Yes, that's what I meant. Thanks, Florian
Re: [PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
- On Jun 24, 2020, at 3:11 PM, Florian Weimer fwei...@redhat.com wrote: > * Mathieu Desnoyers: > >>> I'm still worried that __rseq_static_assert and __rseq_alignof will show >>> up in the UAPI with textually different definitions. (This does not >>> apply to __rseq_tls_model_ie.) >> >> What makes this worry not apply to __rseq_tls_model_ie ? > > It's not needed by the kernel header because it doesn't contain a > __rseq_abi declaration. > >>> >>> Is my worry unfounded? >> >> So AFAIU you worry that eventually sys/rseq.h and linux/rseq.h carry >> different >> definitions of __rseq_static_assert and __rseq_alignof. >> >> Indeed, I did not surround those #define with #ifndef/#endif. Maybe we >> should ? >> >> Just in case the definitions end up being different (worse case scenario), we >> should expect their behavior to be pretty much equivalent. So going for the >> following should address your concern I think: > > I think we should keep things simple on the glibc side for now and do > this changes to the kernel headers first. Just to be sure I understand what you mean by "keep things simple", do you recommend removing the following lines completely for now from sys/rseq.h ? /* Ensure the compiler supports rseq_align. */ __rseq_static_assert (__rseq_alignof (struct rseq_cs) >= 32, "alignment"); __rseq_static_assert (__rseq_alignof (struct rseq) >= 32, "alignment"); Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
* Mathieu Desnoyers: >> I'm still worried that __rseq_static_assert and __rseq_alignof will show >> up in the UAPI with textually different definitions. (This does not >> apply to __rseq_tls_model_ie.) > > What makes this worry not apply to __rseq_tls_model_ie ? It's not needed by the kernel header because it doesn't contain a __rseq_abi declaration. >> >> Is my worry unfounded? > > So AFAIU you worry that eventually sys/rseq.h and linux/rseq.h carry different > definitions of __rseq_static_assert and __rseq_alignof. > > Indeed, I did not surround those #define with #ifndef/#endif. Maybe we should > ? > > Just in case the definitions end up being different (worse case scenario), we > should expect their behavior to be pretty much equivalent. So going for the > following should address your concern I think: I think we should keep things simple on the glibc side for now and do this changes to the kernel headers first. Thanks, Florian
Re: [PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
- On Jun 24, 2020, at 10:20 AM, Florian Weimer fwei...@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> diff --git a/manual/threads.texi b/manual/threads.texi
>> index bb7a42c655..d5069d5581 100644
>> --- a/manual/threads.texi
>> +++ b/manual/threads.texi
>
>> +@deftypevar {struct rseq} __rseq_abi
>> +@standards{Linux, sys/rseq.h}
>> +@Theglibc{} implements a @code{__rseq_abi} TLS symbol to interact with
>> +the Restartable Sequences system call. The layout of this structure is
>> +defined by the @file{sys/rseq.h} header. Registration of each thread's
>> +@code{__rseq_abi} is performed by @theglibc{} at library initialization
>> +and thread creation. The manual for the rseq system call can be found
>> +at
>> @uref{https://git.kernel.org/pub/scm/libs/librseq/librseq.git/tree/doc/man/rseq.2}.
>
> Should be “creation. The” (two spaces after a sentence-ending period).
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h
>> b/sysdeps/unix/sysv/linux/sys/rseq.h
>> new file mode 100644
>> index 00..5e118c1781
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
>
>> +#ifdef __cplusplus
>> +# if __cplusplus >= 201103L
>> +# define __rseq_static_assert(expr, diagnostic) static_assert (expr,
>> diagnostic)
>> +# define __rseq_alignof(type) alignof (type)
>> +# define __rseq_tls_storage_class thread_local
>> +# endif
>> +#elif (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) >= 201112L
>> +# define __rseq_static_assert(expr, diagnostic) _Static_assert (expr,
>> diagnostic)
>> +# define __rseq_alignof(type)_Alignof (type)
>> +# define __rseq_tls_storage_class_Thread_local
>> +#endif
>> +
>> +#ifndef __rseq_static_assert
>> +/* Try to use _Static_assert macro from sys/cdefs.h. */
>> +# ifdef _Static_assert
>> +# define __rseq_static_assert(expr, diagnostic) _Static_assert (expr,
>> diagnostic)
>> +# else
>> +# define __rseq_static_assert(expr, diagnostic) /* Nothing. */
>> +# endif
>> +#endif
>> +
>> +/* Rely on GNU extensions for older standards and tls model. */
>> +#ifdef __GNUC__
>> +# ifndef __rseq_alignof
>> +# define __rseq_alignof(x) __alignof__ (x)
>> +# endif
>> +# define __rseq_tls_model_ie __attribute__ ((__tls_model__
>> ("initial-exec")))
>> +#else
>> +/* Specifying the TLS model on the declaration is optional. */
>> +# define __rseq_tls_model_ie /* Nothing. */
>> +#endif
>
> I'm still worried that __rseq_static_assert and __rseq_alignof will show
> up in the UAPI with textually different definitions. (This does not
> apply to __rseq_tls_model_ie.)
What makes this worry not apply to __rseq_tls_model_ie ?
>
> Is my worry unfounded?
So AFAIU you worry that eventually sys/rseq.h and linux/rseq.h carry different
definitions of __rseq_static_assert and __rseq_alignof.
Indeed, I did not surround those #define with #ifndef/#endif. Maybe we should ?
Just in case the definitions end up being different (worse case scenario), we
should expect their behavior to be pretty much equivalent. So going for the
following should address your concern I think:
#ifdef __cplusplus
# if __cplusplus >= 201103L
# ifndef __rseq_static_assert
# define __rseq_static_assert(expr, diagnostic) static_assert (expr,
diagnostic)
# endif
# ifndef __rseq_alignof
# define __rseq_alignof(type) alignof (type)
# endif
# ifndef __rseq_tls_storage_class
# define __rseq_tls_storage_class thread_local
# endif
# endif
#elif (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) >= 201112L
# ifndef __rseq_static_assert
# define __rseq_static_assert(expr, diagnostic) _Static_assert (expr,
diagnostic)
# endif
# ifndef __rseq_alignof
# define __rseq_alignof(type)_Alignof (type)
# endif
# ifndef __rseq_tls_storage_class
# define __rseq_tls_storage_class_Thread_local
# endif
#endif
#ifndef __rseq_static_assert
/* Try to use _Static_assert macro from sys/cdefs.h. */
# ifdef _Static_assert
# define __rseq_static_assert(expr, diagnostic) _Static_assert (expr,
diagnostic)
# else
# define __rseq_static_assert(expr, diagnostic) /* Nothing. */
# endif
#endif
/* Rely on GNU extensions for older standards and tls model. */
#ifdef __GNUC__
# ifndef __rseq_alignof
# define __rseq_alignof(x) __alignof__ (x)
# endif
# ifndef __rseq_tls_model_ie
# define __rseq_tls_model_ie __attribute__ ((__tls_model__ ("initial-exec")))
# endif
#else
/* Specifying the TLS model on the declaration is optional. */
# ifndef __rseq_tls_model_ie
# define __rseq_tls_model_ie /* Nothing. */
# endif
#endif
/* Fall back to __thread for TLS storage class. */
#ifndef __rseq_tls_storage_class
# define __rseq_tls_storage_class __thread
#endif
Thoughts ?
Thanks,
Mathieu
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Re: [PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
* Mathieu Desnoyers:
> diff --git a/manual/threads.texi b/manual/threads.texi
> index bb7a42c655..d5069d5581 100644
> --- a/manual/threads.texi
> +++ b/manual/threads.texi
> +@deftypevar {struct rseq} __rseq_abi
> +@standards{Linux, sys/rseq.h}
> +@Theglibc{} implements a @code{__rseq_abi} TLS symbol to interact with
> +the Restartable Sequences system call. The layout of this structure is
> +defined by the @file{sys/rseq.h} header. Registration of each thread's
> +@code{__rseq_abi} is performed by @theglibc{} at library initialization
> +and thread creation. The manual for the rseq system call can be found
> +at
> @uref{https://git.kernel.org/pub/scm/libs/librseq/librseq.git/tree/doc/man/rseq.2}.
Should be “creation. The” (two spaces after a sentence-ending period).
> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h
> b/sysdeps/unix/sysv/linux/sys/rseq.h
> new file mode 100644
> index 00..5e118c1781
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
> +#ifdef __cplusplus
> +# if __cplusplus >= 201103L
> +# define __rseq_static_assert(expr, diagnostic) static_assert (expr,
> diagnostic)
> +# define __rseq_alignof(type) alignof (type)
> +# define __rseq_tls_storage_class thread_local
> +# endif
> +#elif (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) >= 201112L
> +# define __rseq_static_assert(expr, diagnostic) _Static_assert (expr,
> diagnostic)
> +# define __rseq_alignof(type)_Alignof (type)
> +# define __rseq_tls_storage_class_Thread_local
> +#endif
> +
> +#ifndef __rseq_static_assert
> +/* Try to use _Static_assert macro from sys/cdefs.h. */
> +# ifdef _Static_assert
> +# define __rseq_static_assert(expr, diagnostic) _Static_assert (expr,
> diagnostic)
> +# else
> +# define __rseq_static_assert(expr, diagnostic) /* Nothing. */
> +# endif
> +#endif
> +
> +/* Rely on GNU extensions for older standards and tls model. */
> +#ifdef __GNUC__
> +# ifndef __rseq_alignof
> +# define __rseq_alignof(x) __alignof__ (x)
> +# endif
> +# define __rseq_tls_model_ie __attribute__ ((__tls_model__ ("initial-exec")))
> +#else
> +/* Specifying the TLS model on the declaration is optional. */
> +# define __rseq_tls_model_ie /* Nothing. */
> +#endif
I'm still worried that __rseq_static_assert and __rseq_alignof will show
up in the UAPI with textually different definitions. (This does not
apply to __rseq_tls_model_ie.)
Is my worry unfounded?
Thanks,
Florian
[PATCH 1/3] glibc: Perform rseq registration at C startup and thread creation (v21)
Register rseq TLS for each thread (including main), and unregister for each thread (excluding main). "rseq" stands for Restartable Sequences. See the rseq(2) man page proposed here: https://lkml.org/lkml/2018/9/19/647 Those are based on glibc master branch commit c013d5d3aa. The rseq system call was merged into Linux 4.18. The TLS_STATIC_SURPLUS define is increased to leave additional room for dlopen'd initial-exec TLS, which keeps elf/tst-auditmany working. The increase (76 bytes) is larger than 32 bytes because it has not been increased in quite a while. The cost in terms of additional TLS storage is quite significant, but it will also obscure some initial-exec-related dlopen failures. CC: Carlos O'Donell CC: Florian Weimer CC: Joseph Myers CC: Szabolcs Nagy CC: Thomas Gleixner CC: Ben Maurer CC: Peter Zijlstra CC: "Paul E. McKenney" CC: Boqun Feng CC: Will Deacon CC: Dave Watson CC: Paul Turner CC: Rich Felker CC: libc-al...@sourceware.org CC: linux-kernel@vger.kernel.org CC: linux-...@vger.kernel.org --- Changes since v1: - Move __rseq_refcount to an extra field at the end of __rseq_abi to eliminate one symbol. All libraries/programs which try to register rseq (glibc, early-adopter applications, early-adopter libraries) should use the rseq refcount. It becomes part of the ABI within a user-space process, but it's not part of the ABI shared with the kernel per se. - Restructure how this code is organized so glibc keeps building on non-Linux targets. - Use non-weak symbol for __rseq_abi. - Move rseq registration/unregistration implementation into its own nptl/rseq.c compile unit. - Move __rseq_abi symbol under GLIBC_2.29. Changes since v2: - Move __rseq_refcount to its own symbol, which is less ugly than trying to play tricks with the rseq uapi. - Move __rseq_abi from nptl to csu (C start up), so it can be used across glibc, including memory allocator and sched_getcpu(). The __rseq_refcount symbol is kept in nptl, because there is no reason to use it elsewhere in glibc. Changes since v3: - Set __rseq_refcount TLS to 1 on register/set to 0 on unregister because glibc is the first/last user. - Unconditionally register/unregister rseq at thread start/exit, because glibc is the first/last user. - Add missing abilist items. - Rebase on glibc master commit a502c5294. - Add NEWS entry. Changes since v4: - Do not use "weak" symbols for __rseq_abi and __rseq_refcount. Based on "System V Application Binary Interface", weak only affects the link editor, not the dynamic linker. - Install a new sys/rseq.h system header on Linux, which contains the RSEQ_SIG definition, __rseq_abi declaration and __rseq_refcount declaration. Move those definition/declarations from rseq-internal.h to the installed sys/rseq.h header. - Considering that rseq is only available on Linux, move csu/rseq.c to sysdeps/unix/sysv/linux/rseq-sym.c. - Move __rseq_refcount from nptl/rseq.c to sysdeps/unix/sysv/linux/rseq-sym.c, so it is only defined on Linux. - Move both ABI definitions for __rseq_abi and __rseq_refcount to sysdeps/unix/sysv/linux/Versions, so they only appear on Linux. - Document __rseq_abi and __rseq_refcount volatile. - Document the RSEQ_SIG signature define. - Move registration functions from rseq.c to rseq-internal.h static inline functions. Introduce empty stubs in misc/rseq-internal.h, which can be overridden by architecture code in sysdeps/unix/sysv/linux/rseq-internal.h. - Rename __rseq_register_current_thread and __rseq_unregister_current_thread to rseq_register_current_thread and rseq_unregister_current_thread, now that those are only visible as internal static inline functions. - Invoke rseq_register_current_thread() from libc-start.c LIBC_START_MAIN rather than nptl init, so applications not linked against libpthread.so have rseq registered for their main() thread. Note that it is invoked separately for SHARED and !SHARED builds. Changes since v5: - Replace __rseq_refcount by __rseq_lib_abi, which contains two uint32_t: register_state and refcount. The "register_state" field allows inhibiting rseq registration from signal handlers nested on top of glibc registration and occuring after rseq unregistration by glibc. - Introduce enum rseq_register_state, which contains the states allowed for the struct rseq_lib_abi register_state field. Changes since v6: - Introduce bits/rseq.h to define RSEQ_SIG for each architecture. The generic bits/rseq.h does not define RSEQ_SIG, meaning that each architecture implementing rseq needs to implement bits/rseq.h. - Rename enum item RSEQ_REGISTER_NESTED to RSEQ_REGISTER_ONGOING. - Port to glibc-2.29. Changes since v7: - Remove __rseq_lib_abi symbol, including refcount and register_state fields. - Remove reference counting and nested signals handling from registration/unregistration functions. - Introduce new __rseq_handled exported symbol, which is set to 1 by glibc on C
