* [PATCH glibc 0/3] Restartable Sequences enablement
@ 2020-05-01 2:14 Mathieu Desnoyers via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19) Mathieu Desnoyers via Libc-alpha
` (3 more replies)
0 siblings, 4 replies; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-01 2:14 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers, Mathieu Desnoyers
Hi,
Please find the rseq enablement patchset for inclusion into
glibc.
Florian, if you find the patches OK and possibly with a bit of
tweaking of the commit messages, can you please commit them
to glibc ?
Thanks for the feedback!
Mathieu
Mathieu Desnoyers (3):
glibc: Perform rseq registration at C startup and thread creation
(v19)
glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7)
rseq registration tests (v10)
NEWS | 10 +
elf/libc_early_init.c | 4 +
manual/threads.texi | 66 +++-
nptl/pthread_create.c | 13 +
sysdeps/generic/rseq-internal.h | 26 ++
sysdeps/unix/sysv/linux/Makefile | 11 +-
sysdeps/unix/sysv/linux/Versions | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/rseq.h | 43 +++
sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/be/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/bits/rseq.h | 83 +++++
sysdeps/unix/sysv/linux/arm/le/libc.abilist | 1 +
sysdeps/unix/sysv/linux/bits/rseq.h | 29 ++
sysdeps/unix/sysv/linux/csky/libc.abilist | 1 +
sysdeps/unix/sysv/linux/hppa/libc.abilist | 1 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 1 +
sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 +
.../sysv/linux/m68k/coldfire/libc.abilist | 1 +
.../unix/sysv/linux/m68k/m680x0/libc.abilist | 1 +
.../sysv/linux/microblaze/be/libc.abilist | 1 +
.../sysv/linux/microblaze/le/libc.abilist | 1 +
sysdeps/unix/sysv/linux/mips/bits/rseq.h | 62 ++++
.../sysv/linux/mips/mips32/fpu/libc.abilist | 1 +
.../sysv/linux/mips/mips32/nofpu/libc.abilist | 1 +
.../sysv/linux/mips/mips64/n32/libc.abilist | 1 +
.../sysv/linux/mips/mips64/n64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/nios2/libc.abilist | 1 +
sysdeps/unix/sysv/linux/powerpc/bits/rseq.h | 37 ++
.../linux/powerpc/powerpc32/fpu/libc.abilist | 1 +
.../powerpc/powerpc32/nofpu/libc.abilist | 1 +
.../linux/powerpc/powerpc64/be/libc.abilist | 1 +
.../linux/powerpc/powerpc64/le/libc.abilist | 1 +
.../unix/sysv/linux/riscv/rv64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/rseq-internal.h | 73 ++++
sysdeps/unix/sysv/linux/rseq-sym.c | 26 ++
sysdeps/unix/sysv/linux/s390/bits/rseq.h | 37 ++
.../unix/sysv/linux/s390/s390-32/libc.abilist | 1 +
.../unix/sysv/linux/s390/s390-64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sched_getcpu.c | 27 +-
sysdeps/unix/sysv/linux/sh/be/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sh/le/libc.abilist | 1 +
.../sysv/linux/sparc/sparc32/libc.abilist | 1 +
.../sysv/linux/sparc/sparc64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sys/rseq.h | 207 +++++++++++
sysdeps/unix/sysv/linux/tst-rseq-nptl.c | 338 ++++++++++++++++++
sysdeps/unix/sysv/linux/tst-rseq.c | 108 ++++++
sysdeps/unix/sysv/linux/x86/bits/rseq.h | 30 ++
.../unix/sysv/linux/x86_64/64/libc.abilist | 1 +
.../unix/sysv/linux/x86_64/x32/libc.abilist | 1 +
50 files changed, 1253 insertions(+), 8 deletions(-)
create mode 100644 sysdeps/generic/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/arm/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/mips/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/rseq-sym.c
create mode 100644 sysdeps/unix/sysv/linux/s390/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/sys/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-nptl.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq.c
create mode 100644 sysdeps/unix/sysv/linux/x86/bits/rseq.h
--
2.17.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-01 2:14 [PATCH glibc 0/3] Restartable Sequences enablement Mathieu Desnoyers via Libc-alpha
@ 2020-05-01 2:14 ` Mathieu Desnoyers via Libc-alpha
2020-05-20 11:40 ` Florian Weimer via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 2/3] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7) Mathieu Desnoyers via Libc-alpha
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-01 2:14 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, libc-alpha, linux-api, Boqun Feng, Will Deacon,
linux-kernel, Peter Zijlstra, Ben Maurer, Mathieu Desnoyers,
Dave Watson, Thomas Gleixner, Paul E. McKenney, Paul Turner,
Joseph Myers
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 6f0baacf0f89.
The rseq system call was merged into Linux 4.18.
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Paul Turner <pjt@google.com>
CC: Rich Felker <dalias@libc.org>
CC: libc-alpha@sourceware.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@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 startup when it handles restartable sequences.
This allows glibc to coexist with early adopter libraries and
applications wishing to register restartable sequences when it
is not handled by glibc.
- Introduce rseq_init (), which sets __rseq_handled to 1 from
C startup.
- Update NEWS entry.
- Update comments at the beginning of new files.
- Registration depends on both __NR_rseq and RSEQ_SIG.
- Remove ARM, powerpc, MIPS RSEQ_SIG until we agree with maintainers
on the signature choice.
- Update x86, s390 RSEQ_SIG based on discussion with arch maintainers.
- Remove rseq-internal.h from headers list of misc/Makefile, so it
it not installed by make install.
Changes since v8:
- Introduce RSEQ_SIG_CODE and RSEQ_SIG_DATA on aarch64 to handle
compiling with -mbig-endian.
Changes since v9:
- Update Changelog.
- Remove unneeded new file comment header newlines.
Changes since v10:
- Remove volatile from __rseq_abi declaration.
- Document that __rseq_handled is about library managing rseq
registration, independently of whether rseq is available or not.
- Move __rseq_handled symbol to ld.so, initialize this symbol within
the dynamic linker initialization for both shared (rtld.c) and static
(dl-support.c) builds.
- Only register the rseq TLS on initialization once in multiple-libc
scenarios. Use rtld_active () for this purpose.
- In the static libc case, register the rseq TLS after LD_PRELOAD
constructors are run, so it matches the order of this initialization
vs LD_PRELOAD contructors execution for the shared libc.
- Agreed on signature choice with powerpc and MIPS maintainers,
re-adding those signatures,
- The main architecture still left out signature-wise is ARM32.
Changes since v11:
- Rebase on glibc 2.30.
- Re-introduce ARM RSEQ_SIG following feedback from Will Deacon.
Changes since v12:
- Remove __rseq_handled,
- Rely on OS implicit rseq unregistration on thread teardown,
- Register main thread in __libc_early_init ().
- Add Restartable Sequences entry to threads manual.
Changes since v13:
- Update following be/le abilist split for arm, microblaze, and sh.
- Update manual to add the __rseq_abi variable and RSEQ_SIG macro to
generate manual index entries, and add missing "Restartable Sequences"
menu entry to the threads chapter.
Changes since v14:
- Update copyright range to include 2020.
- Introduce __ASSUME_RSEQ defined for --enable-kernel=4.18.0 and higher.
- Use ifdef __ASSUME_RSEQ rather than ifdef __NR_rseq to discover rseq
availability. This is necessary now that the system call numbers are
integrated within glibc.
Changes since v15:
- Remove __ASSUME_RSEQ from kernel features.
- rseq internal: remove assume rseq
- remove assume rseq and struct rseq def from sysdeps/unix/sysv/linux/rseq-sym.c
- sys/rseq.h: detect rseq header, implement fallback
- sysdeps/unix/sysv/linux/sys/rseq.h include cdefs.h, add _Static_assert
to validate struct rseq and struct rseq_cs alignment.
- sys/rseq.h: document that posix_memalign should be used rather than
malloc if allocating struct rseq or struct rseq_cs on the heap. This
is required to guarantee 32-byte alignement.
Changes since v16:
- Move rseq NEWS entry under 2.32.
- Move new __rseq_abi symbol to GLIBC_2.32.
Changes since v17:
- Change copyright year to 2020.
- Refer to GNU C Library manual rather than rseq manpage in NEWS.
- Use "initial" parameter from __libc_early_init ().
- Manual: rseq is Linux rather than GNU std.
- Remove rseq_unregister_current_thread () (unused).
- rseq_register_current_thread () returns void.
- Coding style fixes.
- sys/rseq.h: use "32" for alignment.
- Change http:// for https:// in comments.
- Add const struct rseq_cs * field to rseq_cs union.
Changes since v18:
- NEWS update.
- Manual update.
- Move misc/rseq-internal.h to sysdeps/generic/rseq-internal.h.
- Fix coding style in sysdeps/unix/sysv/linux/sys/rseq.h.
- Abort libc if __rseq_abi is already initialized in
rseq_register_current_thread (), which is unexpected.
- Abort libc in rseq_register_current_thread () on all
errno except EPERM (seccomp), EACCES (seccomp), and ENOSYS
(not implemented).
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Paul Turner <pjt@google.com>
CC: Rich Felker <dalias@libc.org>
CC: libc-alpha@sourceware.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
NEWS | 10 +
elf/libc_early_init.c | 4 +
manual/threads.texi | 66 +++++-
nptl/pthread_create.c | 13 ++
sysdeps/generic/rseq-internal.h | 26 +++
sysdeps/unix/sysv/linux/Makefile | 5 +-
sysdeps/unix/sysv/linux/Versions | 1 +
sysdeps/unix/sysv/linux/aarch64/bits/rseq.h | 43 ++++
sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/be/libc.abilist | 1 +
sysdeps/unix/sysv/linux/arm/bits/rseq.h | 83 +++++++
sysdeps/unix/sysv/linux/arm/le/libc.abilist | 1 +
sysdeps/unix/sysv/linux/bits/rseq.h | 29 +++
sysdeps/unix/sysv/linux/csky/libc.abilist | 1 +
sysdeps/unix/sysv/linux/hppa/libc.abilist | 1 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 1 +
sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 +
.../sysv/linux/m68k/coldfire/libc.abilist | 1 +
.../unix/sysv/linux/m68k/m680x0/libc.abilist | 1 +
.../sysv/linux/microblaze/be/libc.abilist | 1 +
.../sysv/linux/microblaze/le/libc.abilist | 1 +
sysdeps/unix/sysv/linux/mips/bits/rseq.h | 62 ++++++
.../sysv/linux/mips/mips32/fpu/libc.abilist | 1 +
.../sysv/linux/mips/mips32/nofpu/libc.abilist | 1 +
.../sysv/linux/mips/mips64/n32/libc.abilist | 1 +
.../sysv/linux/mips/mips64/n64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/nios2/libc.abilist | 1 +
sysdeps/unix/sysv/linux/powerpc/bits/rseq.h | 37 ++++
.../linux/powerpc/powerpc32/fpu/libc.abilist | 1 +
.../powerpc/powerpc32/nofpu/libc.abilist | 1 +
.../linux/powerpc/powerpc64/be/libc.abilist | 1 +
.../linux/powerpc/powerpc64/le/libc.abilist | 1 +
.../unix/sysv/linux/riscv/rv64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/rseq-internal.h | 73 ++++++
sysdeps/unix/sysv/linux/rseq-sym.c | 26 +++
sysdeps/unix/sysv/linux/s390/bits/rseq.h | 37 ++++
.../unix/sysv/linux/s390/s390-32/libc.abilist | 1 +
.../unix/sysv/linux/s390/s390-64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sh/be/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sh/le/libc.abilist | 1 +
.../sysv/linux/sparc/sparc32/libc.abilist | 1 +
.../sysv/linux/sparc/sparc64/libc.abilist | 1 +
sysdeps/unix/sysv/linux/sys/rseq.h | 207 ++++++++++++++++++
sysdeps/unix/sysv/linux/x86/bits/rseq.h | 30 +++
.../unix/sysv/linux/x86_64/64/libc.abilist | 1 +
.../unix/sysv/linux/x86_64/x32/libc.abilist | 1 +
47 files changed, 778 insertions(+), 4 deletions(-)
create mode 100644 sysdeps/generic/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/arm/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/mips/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/rseq-sym.c
create mode 100644 sysdeps/unix/sysv/linux/s390/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/sys/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/x86/bits/rseq.h
diff --git a/NEWS b/NEWS
index 141078c319..c4e0370fc4 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,16 @@ Major new features:
toolchains. It is recommended to use GCC 8 or newer when testing
this option.
+* Support for automatically registering threads with the Linux rseq
+ system call has been added. This system call is implemented starting
+ from Linux 4.18. The Restartable Sequences ABI accelerates user-space
+ operations on per-cpu data. It allows user-space to perform updates
+ on per-cpu data without requiring heavy-weight atomic operations.
+ Automatically registering threads allows all libraries, including libc,
+ to make immediate use of the rseq(2) support by using the documented ABI.
+ The GNU C Library manual has details on integration of Restartable
+ Sequences.
+
Deprecated and removed features, and other changes affecting compatibility:
* The deprecated <sys/sysctl.h> header and the sysctl function have been
diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c
index e6c64fb526..f0fcf6448e 100644
--- a/elf/libc_early_init.c
+++ b/elf/libc_early_init.c
@@ -18,10 +18,14 @@
#include <ctype.h>
#include <libc-early-init.h>
+#include <rseq-internal.h>
void
__libc_early_init (_Bool initial)
{
/* Initialize ctype data. */
__ctype_init ();
+ /* Register rseq ABI to the kernel for the main program's libc. */
+ if (initial)
+ rseq_register_current_thread ();
}
diff --git a/manual/threads.texi b/manual/threads.texi
index 0858ef8f92..a565095c43 100644
--- a/manual/threads.texi
+++ b/manual/threads.texi
@@ -9,8 +9,10 @@ This chapter describes functions used for managing threads.
POSIX threads.
@menu
-* ISO C Threads:: Threads based on the ISO C specification.
-* POSIX Threads:: Threads based on the POSIX specification.
+* ISO C Threads:: Threads based on the ISO C specification.
+* POSIX Threads:: Threads based on the POSIX specification.
+* Restartable Sequences:: Linux-specific Restartable Sequences
+ integration.
@end menu
@@ -881,3 +883,63 @@ Behaves like @code{pthread_timedjoin_np} except that the absolute time in
@c pthread_spin_unlock
@c pthread_testcancel
@c pthread_yield
+
+@node Restartable Sequences
+@section Restartable Sequences
+@cindex Restartable Sequences
+
+This section describes Restartable Sequences integration for
+@theglibc{}.
+
+@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 (Linux-specific). 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 main executable and shared libraries may either have an undefined
+@code{__rseq_abi} TLS symbol, or define their own, with the same
+declaration as the one present in @file{sys/rseq.h}. The dynamic linker
+will ensure that only one of those available symbols will be used at
+runtime across the process.
+
+If the main executable or shared libraries observe an uninitialized
+@code{__rseq_abi.cpu_id} field (value @code{RSEQ_CPU_ID_UNINITIALIZED}), they
+may perform rseq registration to the kernel: this means either glibc was
+prevented from doing the registration, or an older glibc version, which does
+not include rseq support, is in use. When the main executable or a library
+thus takes ownership of the registration, the memory used to hold the
+@code{__rseq_abi} TLS variable must stay allocated, and is not re-used, until
+the very end of the thread lifetime or until an explicit rseq unregistration
+for that thread is performed. It is not recommended to @code{dlclose} libraries
+owning the @code{__rseq_abi} TLS variable.
+
+Users of the @code{__rseq_abi} TLS symbol can store the address of a
+@code{struct rseq_cs} to the @code{__rseq_abi.rseq_cs.uptr.ptr} TLS variable,
+thus informing the kernel that it enters a Restartable Sequence critical
+section. This pointer and the code areas it itself points to must not be left
+pointing to memory areas which are freed or re-used. Several approaches can
+guarantee this. If the application or library can guarantee that the memory
+used to hold the @code{struct rseq_cs} and the code areas it refers to are
+never freed or re-used, no special action must be taken. Else, before that
+memory is re-used of freed, the application is responsible for setting the
+@code{__rseq_abi.rseq_cs.uptr.ptr} TLS variable to @code{NULL} in each thread's
+TLS to guarantee that it does not leak dangling references. Because the
+application does not typically have knowledge of libraries' use of Restartable
+Sequences, it is recommended that libraries using Restartable Sequences which
+may end up freeing or re-using their memory set the
+@code{__rseq_abi.rseq_cs.uptr.ptr} TLS variable to @code{NULL} before returning
+from library functions which use Restartable Sequences.
+
+@end deftypevar
+
+@deftypevr Macro int RSEQ_SIG
+@standards{Linux, sys/rseq.h}
+Each supported architecture provides a @code{RSEQ_SIG} macro in
+@file{sys/rseq.h} which contains a signature. That signature is expected to be
+present in the code before each Restartable Sequences abort handler. Failure
+to provide the expected signature may terminate the process with a segmentation
+fault.
+@end deftypevr
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index afd379e89a..6dacb0e284 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -33,6 +33,7 @@
#include <default-sched.h>
#include <futex-internal.h>
#include <tls-setup.h>
+#include <rseq-internal.h>
#include "libioP.h"
#include <shlib-compat.h>
@@ -384,6 +385,9 @@ START_THREAD_DEFN
/* Initialize pointers to locale data. */
__ctype_init ();
+ /* Register rseq TLS to the kernel. */
+ rseq_register_current_thread ();
+
#ifndef __ASSUME_SET_ROBUST_LIST
if (__set_robust_list_avail >= 0)
#endif
@@ -578,6 +582,15 @@ START_THREAD_DEFN
process is really dead since 'clone' got passed the CLONE_CHILD_CLEARTID
flag. The 'tid' field in the TCB will be set to zero.
+ rseq TLS is still registered at this point. Rely on implicit
+ unregistration performed by the kernel on thread teardown. This is not a
+ problem because the rseq TLS lives on the stack, and the stack outlives
+ the thread. If TCB allocation is ever changed, additional steps may be
+ required, such as performing explicit rseq unregistration before
+ reclaiming the rseq TLS area memory. It is NOT sufficient to block
+ signals because the kernel may write to the rseq area even without
+ signals.
+
The exit code is zero since in case all threads exit by calling
'pthread_exit' the exit status must be 0 (zero). */
__exit_thread ();
diff --git a/sysdeps/generic/rseq-internal.h b/sysdeps/generic/rseq-internal.h
new file mode 100644
index 0000000000..16f197397f
--- /dev/null
+++ b/sysdeps/generic/rseq-internal.h
@@ -0,0 +1,26 @@
+/* Restartable Sequences internal API. Stub version.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef RSEQ_INTERNAL_H
+#define RSEQ_INTERNAL_H
+
+static inline void
+rseq_register_current_thread (void)
+{
+}
+
+#endif /* rseq-internal.h */
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 0326f92c40..e3ed374d9e 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -41,7 +41,7 @@ update-syscall-lists: arch-syscall.h
endif
ifeq ($(subdir),csu)
-sysdep_routines += errno-loc
+sysdep_routines += errno-loc rseq-sym
endif
ifeq ($(subdir),assert)
@@ -92,7 +92,8 @@ sysdep_headers += sys/mount.h sys/acct.h \
bits/termios-c_lflag.h bits/termios-tcflow.h \
bits/termios-misc.h \
bits/types/struct_semid_ds.h \
- bits/ipc-perm.h
+ bits/ipc-perm.h \
+ sys/rseq.h bits/rseq.h
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
index 9a58dda9f2..52ca223ab2 100644
--- a/sysdeps/unix/sysv/linux/Versions
+++ b/sysdeps/unix/sysv/linux/Versions
@@ -178,6 +178,7 @@ libc {
getdents64; gettid; tgkill;
}
GLIBC_2.32 {
+ __rseq_abi;
}
GLIBC_PRIVATE {
# functions used in other libraries
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
new file mode 100644
index 0000000000..37d83fcb4a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
@@ -0,0 +1,43 @@
+/* Restartable Sequences Linux aarch64 architecture header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* RSEQ_SIG is a signature required before each abort handler code.
+
+ It is a 32-bit value that maps to actual architecture code compiled
+ into applications and libraries. It needs to be defined for each
+ architecture. When choosing this value, it needs to be taken into
+ account that generating invalid instructions may have ill effects on
+ tools like objdump, and may also have impact on the CPU speculative
+ execution efficiency in some cases.
+
+ aarch64 -mbig-endian generates mixed endianness code vs data:
+ little-endian code and big-endian data. Ensure the RSEQ_SIG signature
+ matches code endianness. */
+
+#define RSEQ_SIG_CODE 0xd428bc00 /* BRK #0x45E0. */
+
+#ifdef __AARCH64EB__
+#define RSEQ_SIG_DATA 0x00bc28d4 /* BRK #0x45E0. */
+#else
+#define RSEQ_SIG_DATA RSEQ_SIG_CODE
+#endif
+
+#define RSEQ_SIG RSEQ_SIG_DATA
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
index 41bb214bb9..0c9cefbada 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
@@ -2146,4 +2146,5 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
index 6430af207f..53c18cf47d 100644
--- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
+++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
@@ -2226,6 +2226,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/arm/be/libc.abilist b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
index f4ea1756d5..29b02fc165 100644
--- a/sysdeps/unix/sysv/linux/arm/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
@@ -133,6 +133,7 @@ GLIBC_2.30 twalk_r F
GLIBC_2.31 msgctl F
GLIBC_2.31 semctl F
GLIBC_2.31 shmctl F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
new file mode 100644
index 0000000000..c132f0327c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
@@ -0,0 +1,83 @@
+/* Restartable Sequences Linux arm architecture header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/*
+ RSEQ_SIG is a signature required before each abort handler code.
+
+ It is a 32-bit value that maps to actual architecture code compiled
+ into applications and libraries. It needs to be defined for each
+ architecture. When choosing this value, it needs to be taken into
+ account that generating invalid instructions may have ill effects on
+ tools like objdump, and may also have impact on the CPU speculative
+ execution efficiency in some cases.
+
+ - ARM little endian
+
+ RSEQ_SIG uses the udf A32 instruction with an uncommon immediate operand
+ value 0x5de3. This traps if user-space reaches this instruction by mistake,
+ and the uncommon operand ensures the kernel does not move the instruction
+ pointer to attacker-controlled code on rseq abort.
+
+ The instruction pattern in the A32 instruction set is:
+
+ e7f5def3 udf #24035 ; 0x5de3
+
+ This translates to the following instruction pattern in the T16 instruction
+ set:
+
+ little endian:
+ def3 udf #243 ; 0xf3
+ e7f5 b.n <7f5>
+
+ - ARMv6+ big endian (BE8):
+
+ ARMv6+ -mbig-endian generates mixed endianness code vs data: little-endian
+ code and big-endian data. The data value of the signature needs to have its
+ byte order reversed to generate the trap instruction:
+
+ Data: 0xf3def5e7
+
+ Translates to this A32 instruction pattern:
+
+ e7f5def3 udf #24035 ; 0x5de3
+
+ Translates to this T16 instruction pattern:
+
+ def3 udf #243 ; 0xf3
+ e7f5 b.n <7f5>
+
+ - Prior to ARMv6 big endian (BE32):
+
+ Prior to ARMv6, -mbig-endian generates big-endian code and data
+ (which match), so the endianness of the data representation of the
+ signature should not be reversed. However, the choice between BE32
+ and BE8 is done by the linker, so we cannot know whether code and
+ data endianness will be mixed before the linker is invoked. So rather
+ than try to play tricks with the linker, the rseq signature is simply
+ data (not a trap instruction) prior to ARMv6 on big endian. This is
+ why the signature is expressed as data (.word) rather than as
+ instruction (.inst) in assembler. */
+
+#ifdef __ARMEB__
+#define RSEQ_SIG 0xf3def5e7 /* udf #24035 ; 0x5de3 (ARMv6+) */
+#else
+#define RSEQ_SIG 0xe7f5def3 /* udf #24035 ; 0x5de3 */
+#endif
diff --git a/sysdeps/unix/sysv/linux/arm/le/libc.abilist b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
index f1456b26b2..7fa5f13745 100644
--- a/sysdeps/unix/sysv/linux/arm/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
@@ -130,6 +130,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h b/sysdeps/unix/sysv/linux/bits/rseq.h
new file mode 100644
index 0000000000..014c08fe0f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/bits/rseq.h
@@ -0,0 +1,29 @@
+/* Restartable Sequences architecture header. Stub version.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* RSEQ_SIG is a signature required before each abort handler code.
+
+ It is a 32-bit value that maps to actual architecture code compiled
+ into applications and libraries. It needs to be defined for each
+ architecture. When choosing this value, it needs to be taken into
+ account that generating invalid instructions may have ill effects on
+ tools like objdump, and may also have impact on the CPU speculative
+ execution efficiency in some cases. */
diff --git a/sysdeps/unix/sysv/linux/csky/libc.abilist b/sysdeps/unix/sysv/linux/csky/libc.abilist
index c54aed2f8e..2784c9c2b4 100644
--- a/sysdeps/unix/sysv/linux/csky/libc.abilist
+++ b/sysdeps/unix/sysv/linux/csky/libc.abilist
@@ -2090,4 +2090,5 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
index 87373f755b..fa57e7b9a1 100644
--- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
+++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
@@ -2047,6 +2047,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index 1bd2e02f79..ddb068d77f 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2213,6 +2213,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
index 07e51d46bf..20cbe0a67e 100644
--- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
@@ -2079,6 +2079,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
index 42ea4c24bf..6d7aff8738 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
@@ -134,6 +134,7 @@ GLIBC_2.30 twalk_r F
GLIBC_2.31 msgctl F
GLIBC_2.31 semctl F
GLIBC_2.31 shmctl F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0x98
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
index e9358fb092..d935a1220f 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
@@ -2159,6 +2159,7 @@ GLIBC_2.30 twalk_r F
GLIBC_2.31 msgctl F
GLIBC_2.31 semctl F
GLIBC_2.31 shmctl F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
index 2cefe739c0..8d8ae846d0 100644
--- a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
@@ -2141,4 +2141,5 @@ GLIBC_2.30 twalk_r F
GLIBC_2.31 msgctl F
GLIBC_2.31 semctl F
GLIBC_2.31 shmctl F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
diff --git a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
index 3474ef1490..6b25430fe6 100644
--- a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
@@ -2138,4 +2138,5 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
diff --git a/sysdeps/unix/sysv/linux/mips/bits/rseq.h b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
new file mode 100644
index 0000000000..cbad4290cc
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
@@ -0,0 +1,62 @@
+/* Restartable Sequences Linux mips architecture header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* RSEQ_SIG is a signature required before each abort handler code.
+
+ It is a 32-bit value that maps to actual architecture code compiled
+ into applications and libraries. It needs to be defined for each
+ architecture. When choosing this value, it needs to be taken into
+ account that generating invalid instructions may have ill effects on
+ tools like objdump, and may also have impact on the CPU speculative
+ execution efficiency in some cases.
+
+ RSEQ_SIG uses the break instruction. The instruction pattern is:
+
+ On MIPS:
+ 0350000d break 0x350
+
+ On nanoMIPS:
+ 00100350 break 0x350
+
+ On microMIPS:
+ 0000d407 break 0x350
+
+ For nanoMIPS32 and microMIPS, the instruction stream is encoded as
+ 16-bit halfwords, so the signature halfwords need to be swapped
+ accordingly for little-endian. */
+
+#if defined(__nanomips__)
+# ifdef __MIPSEL__
+# define RSEQ_SIG 0x03500010
+# else
+# define RSEQ_SIG 0x00100350
+# endif
+#elif defined(__mips_micromips)
+# ifdef __MIPSEL__
+# define RSEQ_SIG 0xd4070000
+# else
+# define RSEQ_SIG 0x0000d407
+# endif
+#elif defined(__mips__)
+# define RSEQ_SIG 0x0350000d
+#else
+/* Unknown MIPS architecture. */
+#endif
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
index a6f99a7369..d78dd9c881 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
@@ -2130,6 +2130,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
index 48222af11c..3b576e05f0 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
@@ -2128,6 +2128,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
index 99965cfb0f..ee06d83cbe 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
@@ -2136,6 +2136,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
index 2c8bafc669..2e8e658422 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
@@ -2130,6 +2130,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
index 52cf72052c..6b07bd9ef7 100644
--- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
+++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
@@ -2179,4 +2179,5 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
new file mode 100644
index 0000000000..0313b9cba9
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
@@ -0,0 +1,37 @@
+/* Restartable Sequences Linux powerpc architecture header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* RSEQ_SIG is a signature required before each abort handler code.
+
+ It is a 32-bit value that maps to actual architecture code compiled
+ into applications and libraries. It needs to be defined for each
+ architecture. When choosing this value, it needs to be taken into
+ account that generating invalid instructions may have ill effects on
+ tools like objdump, and may also have impact on the CPU speculative
+ execution efficiency in some cases.
+
+ RSEQ_SIG uses the following trap instruction:
+
+ powerpc-be: 0f e5 00 0b twui r5,11
+ powerpc64-le: 0b 00 e5 0f twui r5,11
+ powerpc64-be: 0f e5 00 0b twui r5,11 */
+
+#define RSEQ_SIG 0x0fe5000b
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
index 2ca5bbccf3..7cdfda4a4e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
@@ -2186,6 +2186,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
index e6c4d002d5..aa85a07020 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
@@ -2219,6 +2219,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
index 82d77b7e48..7f63a82038 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
@@ -2049,6 +2049,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
index 0c2513a4b3..18a9b1dc1c 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
@@ -2288,6 +2288,7 @@ GLIBC_2.32 __qecvtieee128_r F
GLIBC_2.32 __qfcvtieee128 F
GLIBC_2.32 __qfcvtieee128_r F
GLIBC_2.32 __qgcvtieee128 F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 __scanfieee128 F
GLIBC_2.32 __snprintf_chkieee128 F
GLIBC_2.32 __snprintfieee128 F
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
index 234d34929a..c0d9de3fe2 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
@@ -2108,4 +2108,5 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
new file mode 100644
index 0000000000..6583691285
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/rseq-internal.h
@@ -0,0 +1,73 @@
+/* Restartable Sequences internal API. Linux implementation.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef RSEQ_INTERNAL_H
+#define RSEQ_INTERNAL_H
+
+#include <sysdep.h>
+#include <errno.h>
+#include <kernel-features.h>
+#include <stdio.h>
+#include <sys/rseq.h>
+
+#ifdef RSEQ_SIG
+static inline void
+rseq_register_current_thread (void)
+{
+ int ret;
+
+ if (__rseq_abi.cpu_id != RSEQ_CPU_ID_UNINITIALIZED)
+ __libc_fatal ("glibc fatal error: "
+ "rseq already initialized for this thread\n");
+ ret = INTERNAL_SYSCALL_CALL (rseq, &__rseq_abi, sizeof (struct rseq),
+ 0, RSEQ_SIG);
+ if (INTERNAL_SYSCALL_ERROR_P (ret))
+ {
+ const char *msg = NULL;
+
+ switch (INTERNAL_SYSCALL_ERRNO (ret))
+ {
+ case ENOSYS: /* rseq system call not implemented. */
+ case EPERM: /* rseq system call filtered by seccomp. */
+ case EACCES: /* rseq system call filtered by seccomp. */
+ __rseq_abi.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
+ break;
+ case EBUSY:
+ msg = "glibc fatal error: rseq already registered for this thread\n";
+ break;
+ case EFAULT:
+ msg = "glibc fatal error: rseq parameter is an invalid address\n";
+ break;
+ case EINVAL:
+ msg = "glibc fatal error: rseq parameters are invalid\n";
+ break;
+ default:
+ msg = "glibc fatal error: unexpected rseq errno\n";
+ break;
+ }
+ if (msg)
+ __libc_fatal (msg);
+ }
+}
+#else
+static inline void
+rseq_register_current_thread (void)
+{
+}
+#endif
+
+#endif /* rseq-internal.h */
diff --git a/sysdeps/unix/sysv/linux/rseq-sym.c b/sysdeps/unix/sysv/linux/rseq-sym.c
new file mode 100644
index 0000000000..090093408f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/rseq-sym.c
@@ -0,0 +1,26 @@
+/* Restartable Sequences exported symbols. Linux Implementation.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#include <sys/syscall.h>
+#include <stdint.h>
+#include <kernel-features.h>
+#include <sys/rseq.h>
+
+__thread struct rseq __rseq_abi =
+ {
+ .cpu_id = RSEQ_CPU_ID_UNINITIALIZED,
+ };
diff --git a/sysdeps/unix/sysv/linux/s390/bits/rseq.h b/sysdeps/unix/sysv/linux/s390/bits/rseq.h
new file mode 100644
index 0000000000..ef0cedaac3
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/bits/rseq.h
@@ -0,0 +1,37 @@
+/* Restartable Sequences Linux s390 architecture header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* RSEQ_SIG is a signature required before each abort handler code.
+
+ It is a 32-bit value that maps to actual architecture code compiled
+ into applications and libraries. It needs to be defined for each
+ architecture. When choosing this value, it needs to be taken into
+ account that generating invalid instructions may have ill effects on
+ tools like objdump, and may also have impact on the CPU speculative
+ execution efficiency in some cases.
+
+ RSEQ_SIG uses the trap4 instruction. As Linux does not make use of the
+ access-register mode nor the linkage stack this instruction will always
+ cause a special-operation exception (the trap-enabled bit in the DUCT
+ is and will stay 0). The instruction pattern is
+ b2 ff 0f ff trap4 4095(%r0) */
+
+#define RSEQ_SIG 0xB2FF0FFF
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
index 1f06cce028..1699bcbf6d 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
@@ -2184,6 +2184,7 @@ GLIBC_2.30 twalk_r F
GLIBC_2.31 msgctl F
GLIBC_2.31 semctl F
GLIBC_2.31 shmctl F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
index 26c2ce32e5..cc7c04c1d0 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
@@ -2085,6 +2085,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/sh/be/libc.abilist b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
index 7ad2e920c3..0641e0fca0 100644
--- a/sysdeps/unix/sysv/linux/sh/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
@@ -2054,6 +2054,7 @@ GLIBC_2.30 twalk_r F
GLIBC_2.31 msgctl F
GLIBC_2.31 semctl F
GLIBC_2.31 shmctl F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/sh/le/libc.abilist b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
index d2611bf0a5..f06d7066b8 100644
--- a/sysdeps/unix/sysv/linux/sh/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
@@ -2051,6 +2051,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
index 18a528f0e9..ef5a0b688e 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
@@ -2175,6 +2175,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 _IO_fprintf F
GLIBC_2.4 _IO_printf F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
index a1d48b0f3c..096bb62978 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
@@ -2102,6 +2102,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h b/sysdeps/unix/sysv/linux/sys/rseq.h
new file mode 100644
index 0000000000..ea51194bf8
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/sys/rseq.h
@@ -0,0 +1,207 @@
+/* Restartable Sequences exported symbols. Linux header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+#define _SYS_RSEQ_H 1
+
+/* Architecture-specific rseq signature. */
+#include <bits/rseq.h>
+
+#include <stdint.h>
+#include <sys/cdefs.h>
+
+#ifdef __has_include
+# if __has_include ("linux/rseq.h")
+# define __GLIBC_HAVE_KERNEL_RSEQ
+# endif
+#else
+# include <linux/version.h>
+# if LINUX_VERSION_CODE >= KERNEL_VERSION (4, 18, 0)
+# define __GLIBC_HAVE_KERNEL_RSEQ
+# endif
+#endif
+
+#ifdef __GLIBC_HAVE_KERNEL_RSEQ
+/* We use the structures declarations from the kernel headers. */
+# include <linux/rseq.h>
+#else
+/* We use a copy of the include/uapi/linux/rseq.h kernel header. */
+
+# include <asm/byteorder.h>
+
+enum rseq_cpu_id_state
+ {
+ RSEQ_CPU_ID_UNINITIALIZED = -1,
+ RSEQ_CPU_ID_REGISTRATION_FAILED = -2,
+ };
+
+enum rseq_flags
+ {
+ RSEQ_FLAG_UNREGISTER = (1 << 0),
+ };
+
+enum rseq_cs_flags_bit
+ {
+ RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT_BIT = 0,
+ RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL_BIT = 1,
+ RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE_BIT = 2,
+ };
+
+enum rseq_cs_flags
+ {
+ RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT =
+ (1U << RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT_BIT),
+ RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL =
+ (1U << RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL_BIT),
+ RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE =
+ (1U << RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE_BIT),
+ };
+
+/* struct rseq_cs is aligned on 32 bytes to ensure it is always
+ contained within a single cache-line. It is usually declared as
+ link-time constant data. */
+struct rseq_cs
+ {
+ /* Version of this structure. */
+ uint32_t version;
+ /* enum rseq_cs_flags. */
+ uint32_t flags;
+ uint64_t start_ip;
+ /* Offset from start_ip. */
+ uint64_t post_commit_offset;
+ uint64_t abort_ip;
+ } __attribute__ ((aligned (32)));
+
+/* struct rseq is aligned on 32 bytes to ensure it is always
+ contained within a single cache-line.
+
+ A single struct rseq per thread is allowed. */
+struct rseq
+ {
+ /* Restartable sequences cpu_id_start field. Updated by the
+ kernel. Read by user-space with single-copy atomicity
+ semantics. This field should only be read by the thread which
+ registered this data structure. Aligned on 32-bit. Always
+ contains a value in the range of possible CPUs, although the
+ value may not be the actual current CPU (e.g. if rseq is not
+ initialized). This CPU number value should always be compared
+ against the value of the cpu_id field before performing a rseq
+ commit or returning a value read from a data structure indexed
+ using the cpu_id_start value. */
+ uint32_t cpu_id_start;
+ /* Restartable sequences cpu_id field. Updated by the kernel.
+ Read by user-space with single-copy atomicity semantics. This
+ field should only be read by the thread which registered this
+ data structure. Aligned on 32-bit. Values
+ RSEQ_CPU_ID_UNINITIALIZED and RSEQ_CPU_ID_REGISTRATION_FAILED
+ have a special semantic: the former means "rseq uninitialized",
+ and latter means "rseq initialization failed". This value is
+ meant to be read within rseq critical sections and compared
+ with the cpu_id_start value previously read, before performing
+ the commit instruction, or read and compared with the
+ cpu_id_start value before returning a value loaded from a data
+ structure indexed using the cpu_id_start value. */
+ uint32_t cpu_id;
+ /* Restartable sequences rseq_cs field.
+
+ Contains NULL when no critical section is active for the current
+ thread, or holds a pointer to the currently active struct rseq_cs.
+
+ Updated by user-space, which sets the address of the currently
+ active rseq_cs at the beginning of assembly instruction sequence
+ block, and set to NULL by the kernel when it restarts an assembly
+ instruction sequence block, as well as when the kernel detects that
+ it is preempting or delivering a signal outside of the range
+ targeted by the rseq_cs. Also needs to be set to NULL by user-space
+ before reclaiming memory that contains the targeted struct rseq_cs.
+
+ Read and set by the kernel. Set by user-space with single-copy
+ atomicity semantics. This field should only be updated by the
+ thread which registered this data structure. Aligned on 64-bit.
+
+ User-space may perform the update through the rseq_cs.uptr.ptr
+ field. The padding needs to be initialized to zero on 32-bit. */
+ union
+ {
+ uint64_t ptr64;
+# ifdef __LP64__
+ uint64_t ptr;
+# else
+ struct
+ {
+# if (defined (__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || defined (__BIG_ENDIAN)
+ uint32_t padding; /* Initialized to zero. */
+ uint32_t ptr32;
+# else /* LITTLE */
+ uint32_t ptr32;
+ uint32_t padding; /* Initialized to zero. */
+# endif /* ENDIAN */
+ } ptr;
+# endif
+
+# ifndef __KERNEL__
+ struct
+ {
+# ifdef __LP64__
+ const struct rseq_cs *ptr;
+# else
+# if (defined (__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || defined (__BIG_ENDIAN)
+ uint32_t padding; /* Initialized to zero. */
+ const struct rseq_cs *ptr;
+# else /* LITTLE */
+ const struct rseq_cs *ptr;
+ uint32_t padding; /* Initialized to zero. */
+# endif /* ENDIAN */
+# endif
+ } uptr;
+# endif
+ } rseq_cs;
+
+ /* Restartable sequences flags field.
+
+ This field should only be updated by the thread which
+ registered this data structure. Read by the kernel.
+ Mainly used for single-stepping through rseq critical sections
+ with debuggers.
+
+ - RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT
+ Inhibit instruction sequence block restart on preemption
+ for this thread.
+ - RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL
+ Inhibit instruction sequence block restart on signal
+ delivery for this thread.
+ - RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE
+ Inhibit instruction sequence block restart on migration for
+ this thread. */
+ uint32_t flags;
+ } __attribute__ ((aligned (32)));
+
+#endif
+
+/* Ensure the compiler supports __attribute__ ((aligned)). */
+_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
+_Static_assert (__alignof__ (struct rseq) >= 32, "alignment");
+
+/* Allocations of struct rseq and struct rseq_cs on the heap need to
+ be aligned on 32 bytes. Therefore, use of malloc is discouraged
+ because it does not guarantee alignment. posix_memalign should be
+ used instead. */
+
+extern __thread struct rseq __rseq_abi
+ __attribute__ ((tls_model ("initial-exec")));
+
+#endif /* sys/rseq.h */
diff --git a/sysdeps/unix/sysv/linux/x86/bits/rseq.h b/sysdeps/unix/sysv/linux/x86/bits/rseq.h
new file mode 100644
index 0000000000..33a6aa7f47
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/bits/rseq.h
@@ -0,0 +1,30 @@
+/* Restartable Sequences Linux x86 architecture header.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* RSEQ_SIG is a signature required before each abort handler code.
+
+ RSEQ_SIG is used with the following reserved undefined instructions, which
+ trap in user-space:
+
+ x86-32: 0f b9 3d 53 30 05 53 ud1 0x53053053,%edi
+ x86-64: 0f b9 3d 53 30 05 53 ud1 0x53053053(%rip),%edi */
+
+#define RSEQ_SIG 0x53053053
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index 6418ace78a..f78e9f894e 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -2060,6 +2060,7 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
GLIBC_2.4 __confstr_chk F
GLIBC_2.4 __fgets_chk F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index edb9f2f004..2867b0adb2 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2159,4 +2159,5 @@ GLIBC_2.30 getdents64 F
GLIBC_2.30 gettid F
GLIBC_2.30 tgkill F
GLIBC_2.30 twalk_r F
+GLIBC_2.32 __rseq_abi T 0x20
GLIBC_2.32 pthread_sigmask F
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH glibc 2/3] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7)
2020-05-01 2:14 [PATCH glibc 0/3] Restartable Sequences enablement Mathieu Desnoyers via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19) Mathieu Desnoyers via Libc-alpha
@ 2020-05-01 2:14 ` Mathieu Desnoyers via Libc-alpha
2020-05-20 10:14 ` Florian Weimer via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 3/3] rseq registration tests (v10) Mathieu Desnoyers via Libc-alpha
2020-05-20 11:44 ` [PATCH glibc 0/3] Restartable Sequences enablement Florian Weimer via Libc-alpha
3 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-01 2:14 UTC (permalink / raw)
To: Florian Weimer
Cc: libc-alpha, linux-api, Boqun Feng, Will Deacon, linux-kernel,
Peter Zijlstra, Ben Maurer, Mathieu Desnoyers, Thomas Gleixner,
Paul E. McKenney, Paul Turner, Joseph Myers
When available, use the cpu_id field from __rseq_abi on Linux to
implement sched_getcpu(). Fall-back on the vgetcpu vDSO if unavailable.
Benchmarks:
x86-64: Intel E5-2630 v3@2.40GHz, 16-core, hyperthreading
glibc sched_getcpu(): 13.7 ns (baseline)
glibc sched_getcpu() using rseq: 2.5 ns (speedup: 5.5x)
inline load cpuid from __rseq_abi TLS: 0.8 ns (speedup: 17.1x)
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Paul Turner <pjt@google.com>
CC: libc-alpha@sourceware.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
Changes since v1:
- rseq is only used if both __NR_rseq and RSEQ_SIG are defined.
Changes since v2:
- remove duplicated __rseq_abi extern declaration.
Changes since v3:
- update ChangeLog.
Changes since v4:
- Use atomic_load_relaxed to load the __rseq_abi.cpu_id field, a
consequence of the fact that __rseq_abi is not volatile anymore.
- Include atomic.h which provides atomic_load_relaxed.
Changes since v5:
- Use __ASSUME_RSEQ to detect rseq availability.
Changes since v6:
- Remove use of __ASSUME_RSEQ.
---
sysdeps/unix/sysv/linux/sched_getcpu.c | 27 ++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
index c019cfb3cf..2269c4f2bd 100644
--- a/sysdeps/unix/sysv/linux/sched_getcpu.c
+++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
@@ -18,10 +18,15 @@
#include <errno.h>
#include <sched.h>
#include <sysdep.h>
+#include <atomic.h>
#include <sysdep-vdso.h>
-int
-sched_getcpu (void)
+#ifdef HAVE_GETCPU_VSYSCALL
+# define HAVE_VSYSCALL
+#endif
+
+static int
+vsyscall_sched_getcpu (void)
{
unsigned int cpu;
int r = -1;
@@ -32,3 +37,21 @@ sched_getcpu (void)
#endif
return r == -1 ? r : cpu;
}
+
+#include <sys/rseq.h>
+
+#ifdef RSEQ_SIG
+int
+sched_getcpu (void)
+{
+ int cpu_id = atomic_load_relaxed (&__rseq_abi.cpu_id);
+
+ return cpu_id >= 0 ? cpu_id : vsyscall_sched_getcpu ();
+}
+#else
+int
+sched_getcpu (void)
+{
+ return vsyscall_sched_getcpu ();
+}
+#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-01 2:14 [PATCH glibc 0/3] Restartable Sequences enablement Mathieu Desnoyers via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19) Mathieu Desnoyers via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 2/3] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7) Mathieu Desnoyers via Libc-alpha
@ 2020-05-01 2:14 ` Mathieu Desnoyers via Libc-alpha
2020-05-20 10:52 ` Florian Weimer via Libc-alpha
2020-05-20 11:44 ` [PATCH glibc 0/3] Restartable Sequences enablement Florian Weimer via Libc-alpha
3 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-01 2:14 UTC (permalink / raw)
To: Florian Weimer
Cc: libc-alpha, Boqun Feng, Will Deacon, Peter Zijlstra, Ben Maurer,
Mathieu Desnoyers, Thomas Gleixner, Paul E. McKenney, Paul Turner,
Joseph Myers
These tests validate that rseq is registered from various execution
contexts (main thread, constructor, destructor, other threads, other
threads created from constructor and destructor, forked process
(without exec), pthread_atfork handlers, pthread setspecific
destructors, C++ thread and process destructors, signal handlers,
atexit handlers).
tst-rseq.c only links against libc.so, testing registration of rseq in
a non-multithreaded environment.
tst-rseq-nptl.c also links against libpthread.so, testing registration
of rseq in a multithreaded environment.
See the Linux kernel selftests for extensive rseq stress-tests.
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Paul Turner <pjt@google.com>
CC: libc-alpha@sourceware.org
---
Changes since v1:
- Rename tst-rseq.c to tst-rseq-nptl.c.
- Introduce tst-rseq.c testing rseq registration in a non-multithreaded
environment.
Chances since v2:
- Update file headers.
- use xpthread key create/delete.
- remove set stacksize.
- Tests depend on both __NR_rseq and RSEQ_SIG being defined.
Changes since v3:
- Update ChangeLog.
Changes since v4:
- Remove volatile from sys_rseq() rseq_abi parameter.
- Use atomic_load_relaxed to load __rseq_abi.cpu_id, consequence of the
fact that __rseq_abi is not volatile anymore.
- Include atomic.h from tst-rseq.c for use of atomic_load_relaxed.
Move tst-rseq.c to internal tests within Makefile due to its use of
atomic.h.
- Test __rseq_handled initialization by glibc.
Changes since v5:
- Rebase on glibc 2.30.
Changes since v6:
- Remove __rseq_handled.
Changes since v7:
- Update copyright range to include 2020.
- Use __ASSUME_RSEQ to detect rseq availability.
Changes since v8:
- Remove use of __ASSUME_RSEQ.
Changes since v9:
- Adapt to new prototype for xpthread_key_create.
- Update copyright year to 2020.
- Remove constructor test (moved to later patch due to test harness
modification dependency).
- Change http:// for https://.
---
sysdeps/unix/sysv/linux/Makefile | 6 +-
sysdeps/unix/sysv/linux/tst-rseq-nptl.c | 338 ++++++++++++++++++++++++
sysdeps/unix/sysv/linux/tst-rseq.c | 108 ++++++++
3 files changed, 450 insertions(+), 2 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-nptl.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq.c
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index e3ed374d9e..598c0ecfdd 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -100,7 +100,9 @@ tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
test-errno-linux tst-memfd_create tst-mlock2 tst-pkey \
tst-rlimit-infinity tst-ofdlocks tst-gettid tst-gettid-kill \
tst-tgkill
-tests-internal += tst-ofdlocks-compat tst-sigcontext-get_pc
+
+tests-internal += tst-ofdlocks-compat tst-sigcontext-get_pc \
+ tst-ofdlocks-compat tst-rseq
CFLAGS-tst-sigcontext-get_pc.c = -fasynchronous-unwind-tables
@@ -303,5 +305,5 @@ ifeq ($(subdir),nptl)
tests += tst-align-clone tst-getpid1 \
tst-thread-affinity-pthread tst-thread-affinity-pthread2 \
tst-thread-affinity-sched
-tests-internal += tst-setgetname
+tests-internal += tst-setgetname tst-rseq-nptl
endif
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-nptl.c b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
new file mode 100644
index 0000000000..3e3996d686
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
@@ -0,0 +1,338 @@
+/* Restartable Sequences NPTL test.
+
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+/* These tests validate that rseq is registered from various execution
+ contexts (main thread, destructor, other threads, other threads created
+ from destructor, forked process (without exec), pthread_atfork handlers,
+ pthread setspecific destructors, C++ thread and process destructors,
+ signal handlers, atexit handlers).
+
+ See the Linux kernel selftests for extensive rseq stress-tests. */
+
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/xthread.h>
+#include <sys/rseq.h>
+
+#ifdef RSEQ_SIG
+#include <pthread.h>
+#include <syscall.h>
+#include <stdlib.h>
+#include <error.h>
+#include <errno.h>
+#include <string.h>
+#include <stdint.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <atomic.h>
+
+static pthread_key_t rseq_test_key;
+
+static int
+rseq_thread_registered (void)
+{
+ return (int32_t) atomic_load_relaxed (&__rseq_abi.cpu_id) >= 0;
+}
+
+static void
+atfork_prepare (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in pthread atfork prepare\n");
+ support_record_failure ();
+ }
+}
+
+static void
+atfork_parent (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in pthread atfork parent\n");
+ support_record_failure ();
+ }
+}
+
+static void
+atfork_child (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in pthread atfork child\n");
+ support_record_failure ();
+ }
+}
+
+static void
+rseq_key_destructor (void *arg)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (!rseq_thread_registered ())
+ FAIL_EXIT1 ("rseq not registered in pthread key destructor");
+}
+
+static void
+atexit_handler (void)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (!rseq_thread_registered ())
+ FAIL_EXIT1 ("rseq not registered in atexit handler");
+}
+
+static int
+do_rseq_main_test (void)
+{
+ if (atexit (atexit_handler))
+ FAIL_EXIT1 ("error calling atexit");
+ rseq_test_key = xpthread_key_create (rseq_key_destructor);
+ if (pthread_atfork (atfork_prepare, atfork_parent, atfork_child))
+ FAIL_EXIT1 ("error calling pthread_atfork");
+ if (raise (SIGUSR1))
+ FAIL_EXIT1 ("error raising signal");
+ if (pthread_setspecific (rseq_test_key, (void *) 1l))
+ FAIL_EXIT1 ("error in pthread_setspecific");
+ if (!rseq_thread_registered ())
+ {
+ FAIL_RET ("rseq not registered in main thread");
+ }
+ return 0;
+}
+
+static void
+cancel_routine (void *arg)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in cancel routine\n");
+ support_record_failure ();
+ }
+}
+
+static int cancel_thread_ready;
+
+static void
+test_cancel_thread (void)
+{
+ pthread_cleanup_push (cancel_routine, NULL);
+ atomic_store_release (&cancel_thread_ready, 1);
+ for (;;)
+ usleep (100);
+ pthread_cleanup_pop (0);
+}
+
+static void *
+thread_function (void * arg)
+{
+ int i = (int) (intptr_t) arg;
+
+ if (raise (SIGUSR1))
+ FAIL_EXIT1 ("error raising signal");
+ if (i == 0)
+ test_cancel_thread ();
+ if (pthread_setspecific (rseq_test_key, (void *) 1l))
+ FAIL_EXIT1 ("error in pthread_setspecific");
+ return rseq_thread_registered () ? NULL : (void *) 1l;
+}
+
+static void
+sighandler (int sig)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in signal handler\n");
+ support_record_failure ();
+ }
+}
+
+static void
+setup_signals (void)
+{
+ struct sigaction sa;
+
+ sigemptyset (&sa.sa_mask);
+ sigaddset (&sa.sa_mask, SIGUSR1);
+ sa.sa_flags = 0;
+ sa.sa_handler = sighandler;
+ if (sigaction (SIGUSR1, &sa, NULL) != 0)
+ {
+ FAIL_EXIT1 ("sigaction failure: %s", strerror (errno));
+ }
+}
+
+#define N 7
+static const int t[N] = { 1, 2, 6, 5, 4, 3, 50 };
+
+static int
+do_rseq_threads_test (int nr_threads)
+{
+ pthread_t th[nr_threads];
+ int i;
+ int result = 0;
+
+ cancel_thread_ready = 0;
+ for (i = 0; i < nr_threads; ++i)
+ if (pthread_create (&th[i], NULL, thread_function,
+ (void *) (intptr_t) i) != 0)
+ {
+ FAIL_EXIT1 ("creation of thread %d failed", i);
+ }
+
+ while (!atomic_load_acquire (&cancel_thread_ready))
+ usleep (100);
+
+ if (pthread_cancel (th[0]))
+ FAIL_EXIT1 ("error in pthread_cancel");
+
+ for (i = 0; i < nr_threads; ++i)
+ {
+ void *v;
+ if (pthread_join (th[i], &v) != 0)
+ {
+ printf ("join of thread %d failed\n", i);
+ result = 1;
+ }
+ else if (i != 0 && v != NULL)
+ {
+ printf ("join %d successful, but child failed\n", i);
+ result = 1;
+ }
+ else if (i == 0 && v == NULL)
+ {
+ printf ("join %d successful, child did not fail as expected\n", i);
+ result = 1;
+ }
+ }
+ return result;
+}
+
+static int
+sys_rseq (struct rseq *rseq_abi, uint32_t rseq_len, int flags, uint32_t sig)
+{
+ return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
+}
+
+static int
+rseq_available (void)
+{
+ int rc;
+
+ rc = sys_rseq (NULL, 0, 0, 0);
+ if (rc != -1)
+ FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
+ switch (errno)
+ {
+ case ENOSYS:
+ return 0;
+ case EINVAL:
+ return 1;
+ default:
+ FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
+ }
+}
+
+static int
+do_rseq_fork_test (void)
+{
+ int status;
+ pid_t pid, retpid;
+
+ pid = fork ();
+ switch (pid)
+ {
+ case 0:
+ exit (do_rseq_main_test ());
+ case -1:
+ FAIL_EXIT1 ("Unexpected fork error %s", strerror (errno));
+ }
+ retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
+ if (retpid != pid)
+ {
+ FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
+ (long int) retpid, (long int) pid);
+ }
+ if (WEXITSTATUS (status))
+ {
+ printf ("rseq not registered in child\n");
+ return 1;
+ }
+ return 0;
+}
+
+static int
+do_rseq_test (void)
+{
+ int i, result = 0;
+
+ if (!rseq_available ())
+ {
+ FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
+ }
+ setup_signals ();
+ if (raise (SIGUSR1))
+ FAIL_EXIT1 ("error raising signal");
+ if (do_rseq_main_test ())
+ result = 1;
+ for (i = 0; i < N; i++)
+ {
+ if (do_rseq_threads_test (t[i]))
+ result = 1;
+ }
+ if (do_rseq_fork_test ())
+ result = 1;
+ return result;
+}
+
+static void __attribute__ ((destructor))
+do_rseq_destructor_test (void)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (do_rseq_test ())
+ FAIL_EXIT1 ("rseq not registered within destructor");
+ xpthread_key_delete (rseq_test_key);
+}
+
+/* Test C++ destructor called at thread and process exit. */
+void
+__call_tls_dtors (void)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (!rseq_thread_registered ())
+ FAIL_EXIT1 ("rseq not registered in C++ thread/process exit destructor");
+}
+#else
+static int
+do_rseq_test (void)
+{
+#ifndef RSEQ_SIG
+ FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test");
+#endif
+ return 0;
+}
+#endif
+
+static int
+do_test (void)
+{
+ return do_rseq_test ();
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
new file mode 100644
index 0000000000..48a61f9998
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq.c
@@ -0,0 +1,108 @@
+/* Restartable Sequences single-threaded tests.
+
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ 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
+ <https://www.gnu.org/licenses/>. */
+
+/* These tests validate that rseq is registered from main in an executable
+ not linked against libpthread. */
+
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <sys/rseq.h>
+
+#ifdef RSEQ_SIG
+#include <syscall.h>
+#include <stdlib.h>
+#include <error.h>
+#include <errno.h>
+#include <stdint.h>
+#include <string.h>
+#include <atomic.h>
+
+static int
+rseq_thread_registered (void)
+{
+ return (int32_t) atomic_load_relaxed (&__rseq_abi.cpu_id) >= 0;
+}
+
+static int
+do_rseq_main_test (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ FAIL_RET ("rseq not registered in main thread");
+ }
+ return 0;
+}
+
+static int
+sys_rseq (struct rseq *rseq_abi, uint32_t rseq_len, int flags, uint32_t sig)
+{
+ return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
+}
+
+static int
+rseq_available (void)
+{
+ int rc;
+
+ rc = sys_rseq (NULL, 0, 0, 0);
+ if (rc != -1)
+ FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
+ switch (errno)
+ {
+ case ENOSYS:
+ return 0;
+ case EINVAL:
+ return 1;
+ default:
+ FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
+ }
+}
+
+static int
+do_rseq_test (void)
+{
+ int result = 0;
+
+ if (!rseq_available ())
+ {
+ FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
+ }
+ if (do_rseq_main_test ())
+ result = 1;
+ return result;
+}
+#else
+static int
+do_rseq_test (void)
+{
+#ifndef RSEQ_SIG
+ FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test");
+#endif
+ return 0;
+}
+#endif
+
+static int
+do_test (void)
+{
+ return do_rseq_test ();
+}
+
+#include <support/test-driver.c>
--
2.17.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 2/3] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7)
2020-05-01 2:14 ` [PATCH glibc 2/3] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7) Mathieu Desnoyers via Libc-alpha
@ 2020-05-20 10:14 ` Florian Weimer via Libc-alpha
0 siblings, 0 replies; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-20 10:14 UTC (permalink / raw)
To: Mathieu Desnoyers via Libc-alpha
Cc: Peter Zijlstra, linux-api, Boqun Feng, Will Deacon, linux-kernel,
Ben Maurer, Mathieu Desnoyers, Thomas Gleixner, Paul E. McKenney,
Paul Turner, Joseph Myers
* Mathieu Desnoyers via Libc-alpha:
> diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
> index c019cfb3cf..2269c4f2bd 100644
> --- a/sysdeps/unix/sysv/linux/sched_getcpu.c
> +++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
> @@ -18,10 +18,15 @@
> #include <errno.h>
> #include <sched.h>
> #include <sysdep.h>
> +#include <atomic.h>
> #include <sysdep-vdso.h>
>
> -int
> -sched_getcpu (void)
> +#ifdef HAVE_GETCPU_VSYSCALL
> +# define HAVE_VSYSCALL
> +#endif
I think the #ifdef is a result of an incorrect merge of commit
d0def09ff6bbf1537beec305fdfe96a21174fb31 ("linux: Fix vDSO macros build
with time64 interfaces") and it should be removed.
The commit subject should probably say “Linux: Use rseq in sched_getcpu
if available”.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-01 2:14 ` [PATCH glibc 3/3] rseq registration tests (v10) Mathieu Desnoyers via Libc-alpha
@ 2020-05-20 10:52 ` Florian Weimer via Libc-alpha
2020-05-25 17:07 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-20 10:52 UTC (permalink / raw)
To: Mathieu Desnoyers via Libc-alpha
Cc: Peter Zijlstra, Boqun Feng, Will Deacon, Ben Maurer,
Mathieu Desnoyers, Thomas Gleixner, Paul E. McKenney, Paul Turner,
Joseph Myers
* Mathieu Desnoyers via Libc-alpha:
> - Include atomic.h from tst-rseq.c for use of atomic_load_relaxed.
> Move tst-rseq.c to internal tests within Makefile due to its use of
> atomic.h.
You could use __atomic builtins to avoid that, or <stdatomic.h>. Both
are fine in tests. I suggest to do that, so that the test can be built
more easily outside of glibc.
However, this needs to remain an internal test because the test needs a
defintiion of __NR_rseq from the internal system call list. Regular
tests use the installed kernel headers, which may not define __NR_rseq.
Maybe mention this in a comment in nptl/Makefile, along with the
tests-internal change?
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-nptl.c b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
> new file mode 100644
> index 0000000000..3e3996d686
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
> @@ -0,0 +1,338 @@
> +/* Restartable Sequences NPTL test.
> +
> + Copyright (C) 2020 Free Software Foundation, Inc.
Maybe rmoeve this empty line.
> +#ifdef RSEQ_SIG
> +#include <pthread.h>
> +#include <syscall.h>
> +#include <stdlib.h>
> +#include <error.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <signal.h>
> +#include <atomic.h>
This should be:
#ifdef RSEQ_SIG
# include <pthread.h>
# include <stdlib.h>
And so on.
Nested preprocessor conditionals need to be indented per our style
rules.
Please also remove the redundant <syscall.h> inclusion.
> +static pthread_key_t rseq_test_key;
> +
> +static int
> +rseq_thread_registered (void)
> +{
> + return (int32_t) atomic_load_relaxed (&__rseq_abi.cpu_id) >= 0;
> +}
The return type should be bool.
> +static void
> +rseq_key_destructor (void *arg)
> +{
> + /* Cannot use deferred failure reporting after main () returns. */
No () after function name.
> + if (!rseq_thread_registered ())
> + FAIL_EXIT1 ("rseq not registered in pthread key destructor");
> +}
> +
> +static void
> +atexit_handler (void)
> +{
> + /* Cannot use deferred failure reporting after main () returns. */
> + if (!rseq_thread_registered ())
> + FAIL_EXIT1 ("rseq not registered in atexit handler");
> +}
> +
> +static int
> +do_rseq_main_test (void)
> +{
> + if (atexit (atexit_handler))
> + FAIL_EXIT1 ("error calling atexit");
> + rseq_test_key = xpthread_key_create (rseq_key_destructor);
> + if (pthread_atfork (atfork_prepare, atfork_parent, atfork_child))
> + FAIL_EXIT1 ("error calling pthread_atfork");
> + if (raise (SIGUSR1))
> + FAIL_EXIT1 ("error raising signal");
> + if (pthread_setspecific (rseq_test_key, (void *) 1l))
> + FAIL_EXIT1 ("error in pthread_setspecific");
> + if (!rseq_thread_registered ())
> + {
> + FAIL_RET ("rseq not registered in main thread");
> + }
You could use
TEST_COMPARE (atexit (atexit_handler), 0);
…
TEST_VERIFY (rseq_thread_registered ());
from <support/check.h>. The automatically generated error messages will
provide sufficient context, I think.
> + return 0;
> +}
> +
> +static void
> +cancel_routine (void *arg)
> +{
> + if (!rseq_thread_registered ())
> + {
> + printf ("rseq not registered in cancel routine\n");
> + support_record_failure ();
> + }
> +}
Please add "error: " to the error message, to make it clearer that it is
an error.
> +
> +static int cancel_thread_ready;
> +
> +static void
> +test_cancel_thread (void)
> +{
> + pthread_cleanup_push (cancel_routine, NULL);
> + atomic_store_release (&cancel_thread_ready, 1);
> + for (;;)
> + usleep (100);
> + pthread_cleanup_pop (0);
> +}
This can use a real barrier (pthread_barrier_t), I think. No need for
polling then.
> +static void *
> +thread_function (void * arg)
> +{
> + int i = (int) (intptr_t) arg;
> +
> + if (raise (SIGUSR1))
> + FAIL_EXIT1 ("error raising signal");
This can use xraise.
> + if (i == 0)
> + test_cancel_thread ();
> + if (pthread_setspecific (rseq_test_key, (void *) 1l))
> + FAIL_EXIT1 ("error in pthread_setspecific");
Could use TEST_COMPARE.
> + return rseq_thread_registered () ? NULL : (void *) 1l;
> +}
> +
> +static void
> +sighandler (int sig)
> +{
> + if (!rseq_thread_registered ())
> + {
> + printf ("rseq not registered in signal handler\n");
> + support_record_failure ();
> + }
> +}
Please add "error: ".
> +static void
> +setup_signals (void)
> +{
> + struct sigaction sa;
> +
> + sigemptyset (&sa.sa_mask);
> + sigaddset (&sa.sa_mask, SIGUSR1);
> + sa.sa_flags = 0;
> + sa.sa_handler = sighandler;
> + if (sigaction (SIGUSR1, &sa, NULL) != 0)
> + {
> + FAIL_EXIT1 ("sigaction failure: %s", strerror (errno));
> + }
> +}
This could use xsigaction.
> +#define N 7
> +static const int t[N] = { 1, 2, 6, 5, 4, 3, 50 };
Should these definitions be local to do_rseq_test below? You can avoid
defining N by using array_length from <array_length.h>.
> +static int
> +do_rseq_threads_test (int nr_threads)
> +{
> + pthread_t th[nr_threads];
> + int i;
> + int result = 0;
> +
> + cancel_thread_ready = 0;
> + for (i = 0; i < nr_threads; ++i)
> + if (pthread_create (&th[i], NULL, thread_function,
> + (void *) (intptr_t) i) != 0)
> + {
> + FAIL_EXIT1 ("creation of thread %d failed", i);
> + }
Could use xpthread_create.
> + while (!atomic_load_acquire (&cancel_thread_ready))
> + usleep (100);
See above, could use xpthread_barrier_wait.
The present code does not wait until all threads have entered their
cancellation region, so I'm not sure if the test object is actually met
here.
> + if (pthread_cancel (th[0]))
> + FAIL_EXIT1 ("error in pthread_cancel");
Could use xpthread_cancel.
> +
> + for (i = 0; i < nr_threads; ++i)
> + {
> + void *v;
> + if (pthread_join (th[i], &v) != 0)
> + {
> + printf ("join of thread %d failed\n", i);
> + result = 1;
> + }
> + else if (i != 0 && v != NULL)
> + {
> + printf ("join %d successful, but child failed\n", i);
> + result = 1;
> + }
> + else if (i == 0 && v == NULL)
> + {
> + printf ("join %d successful, child did not fail as expected\n", i);
> + result = 1;
> + }
Could use xpthread_join.
> +static int
> +sys_rseq (struct rseq *rseq_abi, uint32_t rseq_len, int flags, uint32_t sig)
> +{
> + return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
> +}
(This is the only remaining place where internal headers are potentially
required, I think.)
> +static int
> +rseq_available (void)
> +{
> + int rc;
> +
> + rc = sys_rseq (NULL, 0, 0, 0);
> + if (rc != -1)
> + FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
> + switch (errno)
> + {
> + case ENOSYS:
> + return 0;
> + case EINVAL:
> + return 1;
> + default:
> + FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
> + }
> +}
Maybe add a comment to explain what EINVAL means in this context?
> +static int
> +do_rseq_fork_test (void)
> +{
> + int status;
> + pid_t pid, retpid;
> +
> + pid = fork ();
> + switch (pid)
> + {
> + case 0:
> + exit (do_rseq_main_test ());
> + case -1:
> + FAIL_EXIT1 ("Unexpected fork error %s", strerror (errno));
> + }
Could use xfork.
> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
> + if (retpid != pid)
> + {
> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
> + (long int) retpid, (long int) pid);
> + }
Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
have this.
> + if (WEXITSTATUS (status))
> + {
> + printf ("rseq not registered in child\n");
> + return 1;
> + }
This whole thing looks a lot lie support_isolate_in_subprocess from
<support/namespace.h>.
> +static int
> +do_rseq_test (void)
> +{
> + int i, result = 0;
> +
> + if (!rseq_available ())
> + {
> + FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
> + }
Braces are not needed here.
> + setup_signals ();
> + if (raise (SIGUSR1))
> + FAIL_EXIT1 ("error raising signal");
Could use xraise.
> + if (do_rseq_main_test ())
> + result = 1;
> + for (i = 0; i < N; i++)
> + {
> + if (do_rseq_threads_test (t[i]))
> + result = 1;
> + }
Braces are unnecessary.
> +/* Test C++ destructor called at thread and process exit. */
> +void
> +__call_tls_dtors (void)
> +{
> + /* Cannot use deferred failure reporting after main () returns. */
> + if (!rseq_thread_registered ())
> + FAIL_EXIT1 ("rseq not registered in C++ thread/process exit destructor");
> +}
Uhm, what is this supposed to accomplish, under the __call_tls_dtors
name in particular? I don't think this gets ever called.
It may make sense to have a separate, smaller C++ test to cover this
(perhaps as a separate patch).
> +#else
This needs a comment on the same line. I think it corresponds to the
earlier “#ifdef RSEQ_SIG”.
> +static int
> +do_rseq_test (void)
> +{
> +#ifndef RSEQ_SIG
> + FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test");
> +#endif
And with the comment, it should be obvious that the #ifndef is not
needed here. 8-)
> + return 0;
> +}
> +#endif
Likewise: Add comment.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
> new file mode 100644
> index 0000000000..48a61f9998
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-rseq.c
> @@ -0,0 +1,108 @@
> +/* Restartable Sequences single-threaded tests.
> +
> + Copyright (C) 2020 Free Software Foundation, Inc.
Perhaps remove the empty line?
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <sys/rseq.h>
> +
> +#ifdef RSEQ_SIG
> +#include <syscall.h>
Duplicate <syscall.h>.
> +#include <stdlib.h>
> +#include <error.h>
> +#include <errno.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <atomic.h>
> +
> +static int
> +rseq_thread_registered (void)
> +{
> + return (int32_t) atomic_load_relaxed (&__rseq_abi.cpu_id) >= 0;
> +}
(See above.)
> +static int
> +do_rseq_main_test (void)
> +{
> + if (!rseq_thread_registered ())
> + {
> + FAIL_RET ("rseq not registered in main thread");
> + }
> + return 0;
> +}
Additional braces.
> +static int
> +sys_rseq (struct rseq *rseq_abi, uint32_t rseq_len, int flags, uint32_t sig)
> +{
> + return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
> +}
> +
> +static int
> +rseq_available (void)
> +{
> + int rc;
> +
> + rc = sys_rseq (NULL, 0, 0, 0);
> + if (rc != -1)
> + FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
> + switch (errno)
> + {
> + case ENOSYS:
> + return 0;
> + case EINVAL:
> + return 1;
> + default:
> + FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
> + }
> +}
It looks these three functions could be moved in to a tst-rseq.h header
file (just create the file, no Makefile updates needed).
> +static int
> +do_rseq_test (void)
> +{
> + int result = 0;
> +
> + if (!rseq_available ())
> + {
> + FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
> + }
> + if (do_rseq_main_test ())
> + result = 1;
> + return result;
> +}
> +#else
> +static int
> +do_rseq_test (void)
> +{
> +#ifndef RSEQ_SIG
> + FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test");
> +#endif
> + return 0;
> +}
> +#endif
Comments on #else and #endif, see above.
C++ test could exercise the thread exit path via thread_local, without
linking against libpthread.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-01 2:14 ` [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19) Mathieu Desnoyers via Libc-alpha
@ 2020-05-20 11:40 ` Florian Weimer via Libc-alpha
2020-05-25 14:51 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-20 11:40 UTC (permalink / raw)
To: Mathieu Desnoyers via Libc-alpha
Cc: Rich Felker, Peter Zijlstra, linux-api, Boqun Feng, Will Deacon,
linux-kernel, Ben Maurer, Mathieu Desnoyers, Dave Watson,
Thomas Gleixner, Paul E. McKenney, Paul Turner, Joseph Myers
* Mathieu Desnoyers via Libc-alpha:
> diff --git a/NEWS b/NEWS
> index 141078c319..c4e0370fc4 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -23,6 +23,16 @@ Major new features:
> toolchains. It is recommended to use GCC 8 or newer when testing
> this option.
>
> +* Support for automatically registering threads with the Linux rseq
> + system call has been added. This system call is implemented starting
> + from Linux 4.18. The Restartable Sequences ABI accelerates user-space
> + operations on per-cpu data. It allows user-space to perform updates
> + on per-cpu data without requiring heavy-weight atomic operations.
> + Automatically registering threads allows all libraries, including libc,
> + to make immediate use of the rseq(2) support by using the documented ABI.
> + The GNU C Library manual has details on integration of Restartable
> + Sequences.
“rseq” instead “rseq(2)”.
> diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c
> index e6c64fb526..f0fcf6448e 100644
> --- a/elf/libc_early_init.c
> +++ b/elf/libc_early_init.c
> @@ -18,10 +18,14 @@
>
> #include <ctype.h>
> #include <libc-early-init.h>
> +#include <rseq-internal.h>
>
> void
> __libc_early_init (_Bool initial)
> {
> /* Initialize ctype data. */
> __ctype_init ();
> + /* Register rseq ABI to the kernel for the main program's libc. */
> + if (initial)
> + rseq_register_current_thread ();
> }
Okay.
> diff --git a/manual/threads.texi b/manual/threads.texi
> index 0858ef8f92..a565095c43 100644
> --- a/manual/threads.texi
> +++ b/manual/threads.texi
> @@ -9,8 +9,10 @@ This chapter describes functions used for managing threads.
> POSIX threads.
>
> @menu
> -* ISO C Threads:: Threads based on the ISO C specification.
> -* POSIX Threads:: Threads based on the POSIX specification.
> +* ISO C Threads:: Threads based on the ISO C specification.
> +* POSIX Threads:: Threads based on the POSIX specification.
> +* Restartable Sequences:: Linux-specific Restartable Sequences
> + integration.
> @end menu
This should go into the extensions menu (@node Non-POSIX Extensions).
General comment: Please wrap the lines around 72 or so characters.
Thanks.
> @@ -881,3 +883,63 @@ Behaves like @code{pthread_timedjoin_np} except that the absolute time in
> @c pthread_spin_unlock
> @c pthread_testcancel
> @c pthread_yield
> +
> +@node Restartable Sequences
> +@section Restartable Sequences
> +@cindex Restartable Sequences
> +
> +This section describes Restartable Sequences integration for
> +@theglibc{}.
“This functionality is only available on Linux.” (The @standards parts
are not visible to readers.)
> +
> +@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 (Linux-specific). 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.
Can drop “(Linux-specific)” here.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> new file mode 100644
> index 0000000000..37d83fcb4a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> +#ifdef __AARCH64EB__
> +#define RSEQ_SIG_DATA 0x00bc28d4 /* BRK #0x45E0. */
> +#else
> +#define RSEQ_SIG_DATA RSEQ_SIG_CODE
> +#endif
Missing indentation on the #defines (sorry!).
> diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> new file mode 100644
> index 0000000000..c132f0327c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> +#ifdef __ARMEB__
> +#define RSEQ_SIG 0xf3def5e7 /* udf #24035 ; 0x5de3 (ARMv6+) */
> +#else
> +#define RSEQ_SIG 0xe7f5def3 /* udf #24035 ; 0x5de3 */
> +#endif
Likewise, missing indentation.
> diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h b/sysdeps/unix/sysv/linux/bits/rseq.h
> new file mode 100644
> index 0000000000..014c08fe0f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/rseq.h
> @@ -0,0 +1,29 @@
> +/* RSEQ_SIG is a signature required before each abort handler code.
> +
> + It is a 32-bit value that maps to actual architecture code compiled
> + into applications and libraries. It needs to be defined for each
> + architecture. When choosing this value, it needs to be taken into
> + account that generating invalid instructions may have ill effects on
> + tools like objdump, and may also have impact on the CPU speculative
> + execution efficiency in some cases. */
I wonder if we should say something somewhere that the correct way to
check for compile-time rseq support in glibc is something like this?
#if __has_include (<sys/rseq.h>)
# include <sys/rseq.h>
#endif
#ifdef RSEQ_SIG
…
#endif
Or perhaps we should suppress installation of the headers if we only
have support for the stub.
(I think this fine tuning can be deferred to later patch.)
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/rseq.h b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
> new file mode 100644
> index 0000000000..cbad4290cc
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
> @@ -0,0 +1,62 @@
> +#if defined(__nanomips__)
> +# ifdef __MIPSEL__
> +# define RSEQ_SIG 0x03500010
> +# else
> +# define RSEQ_SIG 0x00100350
> +# endif
> +#elif defined(__mips_micromips)
> +# ifdef __MIPSEL__
> +# define RSEQ_SIG 0xd4070000
> +# else
> +# define RSEQ_SIG 0x0000d407
> +# endif
> +#elif defined(__mips__)
> +# define RSEQ_SIG 0x0350000d
> +#else
> +/* Unknown MIPS architecture. */
> +#endif
Please use “defined (”, with a space.
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> new file mode 100644
> index 0000000000..6583691285
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
> +#ifdef RSEQ_SIG
> +static inline void
> +rseq_register_current_thread (void)
> + if (msg)
> + __libc_fatal (msg);
> + }
“if (msg != NULL)”, please.
> +#else
> +static inline void
> +rseq_register_current_thread (void)
> +{
> +}
> +#endif
Maybe add /* RSEQ_SIG */ comments to #else/#endif here as well.
> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h b/sysdeps/unix/sysv/linux/sys/rseq.h
> new file mode 100644
> index 0000000000..ea51194bf8
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
> +#ifdef __has_include
> +# if __has_include ("linux/rseq.h")
> +# define __GLIBC_HAVE_KERNEL_RSEQ
> +# endif
> +#else
> +# include <linux/version.h>
> +# if LINUX_VERSION_CODE >= KERNEL_VERSION (4, 18, 0)
> +# define __GLIBC_HAVE_KERNEL_RSEQ
> +# endif
> +#endif
Too much indentation on the define line, I think.
Still missing: #ifdef __ASSEMBLER__. .S files should be able to include
<sys/rseq.h> to get the definition of RSEQ_SIG. But I think this can be
deferred to a follow-up.
> +#ifdef __GLIBC_HAVE_KERNEL_RSEQ
> +/* We use the structures declarations from the kernel headers. */
> +# include <linux/rseq.h>
> +#else
> +/* We use a copy of the include/uapi/linux/rseq.h kernel header. */
This comment is not true, the kernel headers do not have uptr support.
If we revert the uptr change, we also need to update the manual, I
think.
> +/* Ensure the compiler supports __attribute__ ((aligned)). */
> +_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
> +_Static_assert (__alignof__ (struct rseq) >= 32, "alignment");
This needs #ifndef __cplusplus or something like that. I'm surprised
that this passes the installed header tests.
> +/* Allocations of struct rseq and struct rseq_cs on the heap need to
> + be aligned on 32 bytes. Therefore, use of malloc is discouraged
> + because it does not guarantee alignment. posix_memalign should be
> + used instead. */
> +
> +extern __thread struct rseq __rseq_abi
> + __attribute__ ((tls_model ("initial-exec")));
Should be __tls_model__.
We're getting really close now. 8-)
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 0/3] Restartable Sequences enablement
2020-05-01 2:14 [PATCH glibc 0/3] Restartable Sequences enablement Mathieu Desnoyers via Libc-alpha
` (2 preceding siblings ...)
2020-05-01 2:14 ` [PATCH glibc 3/3] rseq registration tests (v10) Mathieu Desnoyers via Libc-alpha
@ 2020-05-20 11:44 ` Florian Weimer via Libc-alpha
2020-05-25 13:52 ` Mathieu Desnoyers via Libc-alpha
3 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-20 11:44 UTC (permalink / raw)
To: Mathieu Desnoyers via Libc-alpha; +Cc: Mathieu Desnoyers, Joseph Myers
* Mathieu Desnoyers via Libc-alpha:
> Florian, if you find the patches OK and possibly with a bit of
> tweaking of the commit messages, can you please commit them
> to glibc ?
I had some final review comments, and I found a real bug: the uptr
changes in <sys/rseq.h> are problematic.
What do others think about the level of testing and documentation in
these patches?
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 0/3] Restartable Sequences enablement
2020-05-20 11:44 ` [PATCH glibc 0/3] Restartable Sequences enablement Florian Weimer via Libc-alpha
@ 2020-05-25 13:52 ` Mathieu Desnoyers via Libc-alpha
2020-05-25 14:28 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-25 13:52 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers
----- On May 20, 2020, at 7:44 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers via Libc-alpha:
>
>> Florian, if you find the patches OK and possibly with a bit of
>> tweaking of the commit messages, can you please commit them
>> to glibc ?
>
> I had some final review comments, and I found a real bug: the uptr
> changes in <sys/rseq.h> are problematic.
Hi Florian,
Can you share what issue you have found with the uptr changes ?
Thanks,
Mathieu
>
> What do others think about the level of testing and documentation in
> these patches?
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 0/3] Restartable Sequences enablement
2020-05-25 13:52 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-25 14:28 ` Florian Weimer via Libc-alpha
0 siblings, 0 replies; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-25 14:28 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: libc-alpha, Joseph Myers
* Mathieu Desnoyers:
> ----- On May 20, 2020, at 7:44 AM, Florian Weimer fweimer@redhat.com wrote:
>
>> * Mathieu Desnoyers via Libc-alpha:
>>
>>> Florian, if you find the patches OK and possibly with a bit of
>>> tweaking of the commit messages, can you please commit them
>>> to glibc ?
>>
>> I had some final review comments, and I found a real bug: the uptr
>> changes in <sys/rseq.h> are problematic.
>
> Hi Florian,
>
> Can you share what issue you have found with the uptr changes ?
I thought I had written about something here:
<https://sourceware.org/pipermail/libc-alpha/2020-May/114142.html>
But it's not very explicit. I must have deleted it before sending it.
The problem is that you made substantial changes to the fallback
definitions compared to the kernel version. But those will never be
used if the user has the appropriate kernel headers installed. The
fallback part really has to be an almost-verbatim copy of the kernel
definitions.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-20 11:40 ` Florian Weimer via Libc-alpha
@ 2020-05-25 14:51 ` Mathieu Desnoyers via Libc-alpha
2020-05-25 15:20 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-25 14:51 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
----- On May 20, 2020, at 7:40 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers via Libc-alpha:
>
>> diff --git a/NEWS b/NEWS
>> index 141078c319..c4e0370fc4 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -23,6 +23,16 @@ Major new features:
>> toolchains. It is recommended to use GCC 8 or newer when testing
>> this option.
>>
>> +* Support for automatically registering threads with the Linux rseq
>> + system call has been added. This system call is implemented starting
>> + from Linux 4.18. The Restartable Sequences ABI accelerates user-space
>> + operations on per-cpu data. It allows user-space to perform updates
>> + on per-cpu data without requiring heavy-weight atomic operations.
>> + Automatically registering threads allows all libraries, including libc,
>> + to make immediate use of the rseq(2) support by using the documented ABI.
>> + The GNU C Library manual has details on integration of Restartable
>> + Sequences.
>
> “rseq” instead “rseq(2)”.
OK
[...]
>
>> diff --git a/manual/threads.texi b/manual/threads.texi
>> index 0858ef8f92..a565095c43 100644
>> --- a/manual/threads.texi
>> +++ b/manual/threads.texi
>> @@ -9,8 +9,10 @@ This chapter describes functions used for managing threads.
>> POSIX threads.
>>
>> @menu
>> -* ISO C Threads:: Threads based on the ISO C specification.
>> -* POSIX Threads:: Threads based on the POSIX specification.
>> +* ISO C Threads:: Threads based on the ISO C specification.
>> +* POSIX Threads:: Threads based on the POSIX specification.
>> +* Restartable Sequences:: Linux-specific Restartable Sequences
>> + integration.
>> @end menu
>
> This should go into the extensions menu (@node Non-POSIX Extensions).
OK
>
> General comment: Please wrap the lines around 72 or so characters.
> Thanks.
OK
>
>> @@ -881,3 +883,63 @@ Behaves like @code{pthread_timedjoin_np} except that the
>> absolute time in
>> @c pthread_spin_unlock
>> @c pthread_testcancel
>> @c pthread_yield
>> +
>> +@node Restartable Sequences
>> +@section Restartable Sequences
>> +@cindex Restartable Sequences
>> +
>> +This section describes Restartable Sequences integration for
>> +@theglibc{}.
>
> “This functionality is only available on Linux.” (The @standards parts
> are not visible to readers.)
OK
>
>> +
>> +@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 (Linux-specific). 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.
>
> Can drop “(Linux-specific)” here.
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>> new file mode 100644
>> index 0000000000..37d83fcb4a
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>
>> +#ifdef __AARCH64EB__
>> +#define RSEQ_SIG_DATA 0x00bc28d4 /* BRK #0x45E0. */
>> +#else
>> +#define RSEQ_SIG_DATA RSEQ_SIG_CODE
>> +#endif
>
> Missing indentation on the #defines (sorry!).
Sorry! My bad!
>
>> diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
>> new file mode 100644
>> index 0000000000..c132f0327c
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
>
>> +#ifdef __ARMEB__
>> +#define RSEQ_SIG 0xf3def5e7 /* udf #24035 ; 0x5de3 (ARMv6+) */
>> +#else
>> +#define RSEQ_SIG 0xe7f5def3 /* udf #24035 ; 0x5de3 */
>> +#endif
>
> Likewise, missing indentation.
At least I was consistently wrong. ;-)
>
>> diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/bits/rseq.h
>> new file mode 100644
>> index 0000000000..014c08fe0f
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/bits/rseq.h
>> @@ -0,0 +1,29 @@
>
>> +/* RSEQ_SIG is a signature required before each abort handler code.
>> +
>> + It is a 32-bit value that maps to actual architecture code compiled
>> + into applications and libraries. It needs to be defined for each
>> + architecture. When choosing this value, it needs to be taken into
>> + account that generating invalid instructions may have ill effects on
>> + tools like objdump, and may also have impact on the CPU speculative
>> + execution efficiency in some cases. */
>
> I wonder if we should say something somewhere that the correct way to
> check for compile-time rseq support in glibc is something like this?
>
> #if __has_include (<sys/rseq.h>)
> # include <sys/rseq.h>
> #endif
> #ifdef RSEQ_SIG
> …
> #endif
>
> Or perhaps we should suppress installation of the headers if we only
> have support for the stub.
>
> (I think this fine tuning can be deferred to later patch.)
I would prefer we keep this for a later patch.
That being said, I notice that gcc prior to 4.9 has trouble compiling
__has_include. Therefore, I would tend to recommend the following as a
backward-compatible way of using the rseq header from external projects:
- At configure time, check for availability of sys/rseq.h to ensure the
glibc version is recent enough. In the autotools world, this is usually
done with a small test program which will fail to build if the header is
not there.
- At preprocessing time, include <sys/rseq.h> and check whether RSEQ_SIG
is defined.
If a project only aims at building with gcc 4.9 or newer, then it can use
__has_include.
>
>> diff --git a/sysdeps/unix/sysv/linux/mips/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
>> new file mode 100644
>> index 0000000000..cbad4290cc
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
>> @@ -0,0 +1,62 @@
>
>> +#if defined(__nanomips__)
>> +# ifdef __MIPSEL__
>> +# define RSEQ_SIG 0x03500010
>> +# else
>> +# define RSEQ_SIG 0x00100350
>> +# endif
>> +#elif defined(__mips_micromips)
>> +# ifdef __MIPSEL__
>> +# define RSEQ_SIG 0xd4070000
>> +# else
>> +# define RSEQ_SIG 0x0000d407
>> +# endif
>> +#elif defined(__mips__)
>> +# define RSEQ_SIG 0x0350000d
>> +#else
>> +/* Unknown MIPS architecture. */
>> +#endif
>
> Please use “defined (”, with a space.
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h
>> b/sysdeps/unix/sysv/linux/rseq-internal.h
>> new file mode 100644
>> index 0000000000..6583691285
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
>
>> +#ifdef RSEQ_SIG
>> +static inline void
>> +rseq_register_current_thread (void)
>
>> + if (msg)
>> + __libc_fatal (msg);
>> + }
>
> “if (msg != NULL)”, please.
OK
>
>> +#else
>> +static inline void
>> +rseq_register_current_thread (void)
>> +{
>> +}
>> +#endif
>
> Maybe add /* RSEQ_SIG */ comments to #else/#endif here as well.
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h
>> b/sysdeps/unix/sysv/linux/sys/rseq.h
>> new file mode 100644
>> index 0000000000..ea51194bf8
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
>
>> +#ifdef __has_include
>> +# if __has_include ("linux/rseq.h")
>> +# define __GLIBC_HAVE_KERNEL_RSEQ
>> +# endif
>> +#else
>> +# include <linux/version.h>
>> +# if LINUX_VERSION_CODE >= KERNEL_VERSION (4, 18, 0)
>> +# define __GLIBC_HAVE_KERNEL_RSEQ
>> +# endif
>> +#endif
>
> Too much indentation on the define line, I think.
OK
>
> Still missing: #ifdef __ASSEMBLER__. .S files should be able to include
> <sys/rseq.h> to get the definition of RSEQ_SIG. But I think this can be
> deferred to a follow-up.
Would the plan be to only do the #include <bits/rseq.h> bits in #ifdef
__ASSEMBLER__ and skip all the rest ?
>
>> +#ifdef __GLIBC_HAVE_KERNEL_RSEQ
>> +/* We use the structures declarations from the kernel headers. */
>> +# include <linux/rseq.h>
>> +#else
>> +/* We use a copy of the include/uapi/linux/rseq.h kernel header. */
>
> This comment is not true, the kernel headers do not have uptr support.
>
> If we revert the uptr change, we also need to update the manual, I
> think.
Ah, I get to your uptr concern now! Good :)
The larger question here is: considering that we re-implement the entire
uapi header within glibc (which includes the uptr addition), do we still
care about using the header provided by the Linux kernel ?
Having different definitions depending on whether a kernel header is
installed or not when including a glibc header seems rather unexpected.
*If* we want to use the uapi header, I think something is semantically
missing. Here is the scheme I envision. We could rely on the kernel header
version.h to figure out which of glibc or kernel uapi header is more
recent. Any new concept we try to integrate into glibc (e.g. uptr)
should go into the upstream Linux uapi header first.
For the coming glibc e.g. 2.32, we use the kernel uapi header if
kernel version is >= 4.18.0. Within glibc, the fallback implements
exactly the API exposed by the kernel rseq.h header.
As we eventually introduce the uptr change into the Linux kernel, and
say it gets merged for Linux 5.9.0, we mirror this change into glibc
(e.g. release 2.33), and bump the Linux kernel version cutoff to 5.9.0.
So starting from that version, we use the Linux kernel header only if
version >= 5.9.0, else we fallback on glibc's own implementation.
This is clearly something we need to get right.
>
>> +/* Ensure the compiler supports __attribute__ ((aligned)). */
>> +_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
>> +_Static_assert (__alignof__ (struct rseq) >= 32, "alignment");
>
> This needs #ifndef __cplusplus or something like that. I'm surprised
> that this passes the installed header tests.
Would the following be ok ?
#ifdef __cplusplus
#define rseq_static_assert static_assert
#else
#define rseq_static_assert _Static_assert
#endif
/* Ensure the compiler supports __attribute__ ((aligned)). */
rseq_static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
rseq_static_assert (__alignof__ (struct rseq) >= 32, "alignment");
>
>> +/* Allocations of struct rseq and struct rseq_cs on the heap need to
>> + be aligned on 32 bytes. Therefore, use of malloc is discouraged
>> + because it does not guarantee alignment. posix_memalign should be
>> + used instead. */
>> +
>> +extern __thread struct rseq __rseq_abi
>> + __attribute__ ((tls_model ("initial-exec")));
>
> Should be __tls_model__.
OK
>
> We're getting really close now. 8-)
Great!
Thanks for the feedback!
Mathieu
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-25 14:51 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-25 15:20 ` Florian Weimer via Libc-alpha
2020-05-25 17:36 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-25 15:20 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
* Mathieu Desnoyers:
> The larger question here is: considering that we re-implement the entire
> uapi header within glibc (which includes the uptr addition), do we still
> care about using the header provided by the Linux kernel ?
We don't care, but our users do. Eventually, they want to include
<sys/rseq.h> and <linux/rseq.h> to get new constants that are not yet
known to glibc.
> Having different definitions depending on whether a kernel header is
> installed or not when including a glibc header seems rather unexpected.
Indeed.
> *If* we want to use the uapi header, I think something is semantically
> missing. Here is the scheme I envision. We could rely on the kernel header
> version.h to figure out which of glibc or kernel uapi header is more
> recent. Any new concept we try to integrate into glibc (e.g. uptr)
> should go into the upstream Linux uapi header first.
I think we should always prefer the uapi header. The Linux version
check does not tell you anything about backports.
> For the coming glibc e.g. 2.32, we use the kernel uapi header if
> kernel version is >= 4.18.0. Within glibc, the fallback implements
> exactly the API exposed by the kernel rseq.h header.
Agreed.
> As we eventually introduce the uptr change into the Linux kernel, and
> say it gets merged for Linux 5.9.0, we mirror this change into glibc
> (e.g. release 2.33), and bump the Linux kernel version cutoff to 5.9.0.
> So starting from that version, we use the Linux kernel header only if
> version >= 5.9.0, else we fallback on glibc's own implementation.
Fortunately, we don't need to settle this today. 8-)
Let's stick to the 4.18 definitions for the fallback for now, and
discuss the incorporation of future changes later.
>>> +/* Ensure the compiler supports __attribute__ ((aligned)). */
>>> +_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
>>> +_Static_assert (__alignof__ (struct rseq) >= 32, "alignment");
>>
>> This needs #ifndef __cplusplus or something like that. I'm surprised
>> that this passes the installed header tests.
>
> Would the following be ok ?
>
> #ifdef __cplusplus
> #define rseq_static_assert static_assert
> #else
> #define rseq_static_assert _Static_assert
> #endif
>
> /* Ensure the compiler supports __attribute__ ((aligned)). */
> rseq_static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
> rseq_static_assert (__alignof__ (struct rseq) >= 32, "alignment");
Seems reasonable, yes. __alignof__ is still a GCC extension. C++11 has
alignof, C11 has _Alignof. So you could use something like this
(perhaps without indentation for the kernel header version):
#ifdef __cplusplus
# if __cplusplus >= 201103L
# define rseq_static_assert(x) static_assert x;
# define rseq_alignof alignof
# endif
#elif __STDC_VERSION__ >= 201112L
# define rseq_static_assert(x) _Static_assert x;
# define rseq_alignof _Alignof
#endif
#ifndef rseq_static_assert
# define rseq_static_assert /* nothing */
#endif
rseq_static_assert ((rseq_alignof__ (struct rseq_cs) >= 32, "alignment"))
rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment"))
And something similar for _Alignas/attribute aligned, with an error for
older standards and !__GNUC__ compilers (because neither the type nor
__thread can be represented there).
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-20 10:52 ` Florian Weimer via Libc-alpha
@ 2020-05-25 17:07 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 12:47 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-25 17:07 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers
(trimming CC list)
----- On May 20, 2020, at 6:52 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers via Libc-alpha:
>
>> - Include atomic.h from tst-rseq.c for use of atomic_load_relaxed.
>> Move tst-rseq.c to internal tests within Makefile due to its use of
>> atomic.h.
>
> You could use __atomic builtins to avoid that, or <stdatomic.h>. Both
> are fine in tests. I suggest to do that, so that the test can be built
> more easily outside of glibc.
OK will use __atomic_load (&__rseq_abi.cpu_id, &v, __ATOMIC_RELAXED);
>
> However, this needs to remain an internal test because the test needs a
> defintiion of __NR_rseq from the internal system call list. Regular
> tests use the installed kernel headers, which may not define __NR_rseq.
> Maybe mention this in a comment in nptl/Makefile, along with the
> tests-internal change?
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
>> b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
>> new file mode 100644
>> index 0000000000..3e3996d686
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
>> @@ -0,0 +1,338 @@
>> +/* Restartable Sequences NPTL test.
>> +
>> + Copyright (C) 2020 Free Software Foundation, Inc.
>
> Maybe rmoeve this empty line.
OK
>
>> +#ifdef RSEQ_SIG
>> +#include <pthread.h>
>> +#include <syscall.h>
>> +#include <stdlib.h>
>> +#include <error.h>
>> +#include <errno.h>
>> +#include <string.h>
>> +#include <stdint.h>
>> +#include <sys/types.h>
>> +#include <sys/wait.h>
>> +#include <signal.h>
>> +#include <atomic.h>
>
> This should be:
>
> #ifdef RSEQ_SIG
> # include <pthread.h>
> # include <stdlib.h>
>
> And so on.
>
> Nested preprocessor conditionals need to be indented per our style
> rules.
OK
>
> Please also remove the redundant <syscall.h> inclusion.
OK. I will remove the first <sys/syscall.h> include and keep the <syscall.h> include.
>
>> +static pthread_key_t rseq_test_key;
>> +
>> +static int
>> +rseq_thread_registered (void)
>> +{
>> + return (int32_t) atomic_load_relaxed (&__rseq_abi.cpu_id) >= 0;
>> +}
>
> The return type should be bool.
OK, and will include <stdbool.h> as well.
>
>> +static void
>> +rseq_key_destructor (void *arg)
>> +{
>> + /* Cannot use deferred failure reporting after main () returns. */
>
> No () after function name.
OK
>
>> + if (!rseq_thread_registered ())
>> + FAIL_EXIT1 ("rseq not registered in pthread key destructor");
>> +}
>> +
>> +static void
>> +atexit_handler (void)
>> +{
>> + /* Cannot use deferred failure reporting after main () returns. */
>> + if (!rseq_thread_registered ())
>> + FAIL_EXIT1 ("rseq not registered in atexit handler");
>> +}
>> +
>> +static int
>> +do_rseq_main_test (void)
>> +{
>> + if (atexit (atexit_handler))
>> + FAIL_EXIT1 ("error calling atexit");
>> + rseq_test_key = xpthread_key_create (rseq_key_destructor);
>> + if (pthread_atfork (atfork_prepare, atfork_parent, atfork_child))
>> + FAIL_EXIT1 ("error calling pthread_atfork");
>> + if (raise (SIGUSR1))
>> + FAIL_EXIT1 ("error raising signal");
>> + if (pthread_setspecific (rseq_test_key, (void *) 1l))
>> + FAIL_EXIT1 ("error in pthread_setspecific");
>> + if (!rseq_thread_registered ())
>> + {
>> + FAIL_RET ("rseq not registered in main thread");
>> + }
>
> You could use
>
> TEST_COMPARE (atexit (atexit_handler), 0);
OK
> …
> TEST_VERIFY (rseq_thread_registered ());
That would change the behavior from "return 1 on error" to "continue
executing on error", which is not what we want here. I think we could
use TEST_VERIFY_EXIT to achieve a similar goal here. I'll change do_rseq_main_test
to return void rather than int.
>
> from <support/check.h>. The automatically generated error messages will
> provide sufficient context, I think.
OK
>
>> + return 0;
>> +}
>> +
>> +static void
>> +cancel_routine (void *arg)
>> +{
>> + if (!rseq_thread_registered ())
>> + {
>> + printf ("rseq not registered in cancel routine\n");
>> + support_record_failure ();
>> + }
>> +}
>
> Please add "error: " to the error message, to make it clearer that it is
> an error.
OK
>
>> +
>> +static int cancel_thread_ready;
>> +
>> +static void
>> +test_cancel_thread (void)
>> +{
>> + pthread_cleanup_push (cancel_routine, NULL);
>> + atomic_store_release (&cancel_thread_ready, 1);
>> + for (;;)
>> + usleep (100);
>> + pthread_cleanup_pop (0);
>> +}
>
> This can use a real barrier (pthread_barrier_t), I think. No need for
> polling then.
OK.
>
>> +static void *
>> +thread_function (void * arg)
>> +{
>> + int i = (int) (intptr_t) arg;
>> +
>> + if (raise (SIGUSR1))
>> + FAIL_EXIT1 ("error raising signal");
>
> This can use xraise.
OK
>
>> + if (i == 0)
>> + test_cancel_thread ();
>> + if (pthread_setspecific (rseq_test_key, (void *) 1l))
>> + FAIL_EXIT1 ("error in pthread_setspecific");
>
> Could use TEST_COMPARE.
OK
>
>> + return rseq_thread_registered () ? NULL : (void *) 1l;
>> +}
>> +
>> +static void
>> +sighandler (int sig)
>> +{
>> + if (!rseq_thread_registered ())
>> + {
>> + printf ("rseq not registered in signal handler\n");
>> + support_record_failure ();
>> + }
>> +}
>
> Please add "error: ".
OK
>
>> +static void
>> +setup_signals (void)
>> +{
>> + struct sigaction sa;
>> +
>> + sigemptyset (&sa.sa_mask);
>> + sigaddset (&sa.sa_mask, SIGUSR1);
>> + sa.sa_flags = 0;
>> + sa.sa_handler = sighandler;
>> + if (sigaction (SIGUSR1, &sa, NULL) != 0)
>> + {
>> + FAIL_EXIT1 ("sigaction failure: %s", strerror (errno));
>> + }
>> +}
>
> This could use xsigaction.
OK
>
>> +#define N 7
>> +static const int t[N] = { 1, 2, 6, 5, 4, 3, 50 };
>
> Should these definitions be local to do_rseq_test below? You can avoid
> defining N by using array_length from <array_length.h>.
OK
>
>> +static int
>> +do_rseq_threads_test (int nr_threads)
>> +{
>> + pthread_t th[nr_threads];
>> + int i;
>> + int result = 0;
>> +
>> + cancel_thread_ready = 0;
>> + for (i = 0; i < nr_threads; ++i)
>> + if (pthread_create (&th[i], NULL, thread_function,
>> + (void *) (intptr_t) i) != 0)
>> + {
>> + FAIL_EXIT1 ("creation of thread %d failed", i);
>> + }
>
> Could use xpthread_create.
OK
>
>> + while (!atomic_load_acquire (&cancel_thread_ready))
>> + usleep (100);
>
> See above, could use xpthread_barrier_wait.
OK
>
> The present code does not wait until all threads have entered their
> cancellation region, so I'm not sure if the test object is actually met
> here.
We're only cancelling the first thread in the test, which is the intent.
In terms of barrier, it's a barrier involving only 2 threads.
>
>> + if (pthread_cancel (th[0]))
>> + FAIL_EXIT1 ("error in pthread_cancel");
>
> Could use xpthread_cancel.
OK
>
>> +
>> + for (i = 0; i < nr_threads; ++i)
>> + {
>> + void *v;
>> + if (pthread_join (th[i], &v) != 0)
>> + {
>> + printf ("join of thread %d failed\n", i);
>> + result = 1;
>> + }
>> + else if (i != 0 && v != NULL)
>> + {
>> + printf ("join %d successful, but child failed\n", i);
>> + result = 1;
>> + }
>> + else if (i == 0 && v == NULL)
>> + {
>> + printf ("join %d successful, child did not fail as expected\n", i);
>> + result = 1;
>> + }
>
> Could use xpthread_join.
OK
>
>> +static int
>> +sys_rseq (struct rseq *rseq_abi, uint32_t rseq_len, int flags, uint32_t sig)
>> +{
>> + return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
>> +}
>
> (This is the only remaining place where internal headers are potentially
> required, I think.)
OK
>
>> +static int
>> +rseq_available (void)
>> +{
>> + int rc;
>> +
>> + rc = sys_rseq (NULL, 0, 0, 0);
>> + if (rc != -1)
>> + FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
>> + switch (errno)
>> + {
>> + case ENOSYS:
>> + return 0;
>> + case EINVAL:
>> + return 1;
>> + default:
>> + FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
>> + }
>> +}
>
> Maybe add a comment to explain what EINVAL means in this context?
For instance:
/* rseq is implemented, but detected an invalid parameter. */
>
>> +static int
>> +do_rseq_fork_test (void)
>> +{
>> + int status;
>> + pid_t pid, retpid;
>> +
>> + pid = fork ();
>> + switch (pid)
>> + {
>> + case 0:
>> + exit (do_rseq_main_test ());
>> + case -1:
>> + FAIL_EXIT1 ("Unexpected fork error %s", strerror (errno));
>> + }
>
> Could use xfork.
OK
>
>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>> + if (retpid != pid)
>> + {
>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>> + (long int) retpid, (long int) pid);
>> + }
>
> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
> have this.
Then how does it deal with a signal interrupting the system call performing
the waitpid (EINTR) ? I do not see WNOHANG being used.
>
>> + if (WEXITSTATUS (status))
>> + {
>> + printf ("rseq not registered in child\n");
>> + return 1;
>> + }
>
> This whole thing looks a lot lie support_isolate_in_subprocess from
> <support/namespace.h>.
I'll wait until we reach an agreement on TEMP_FAILURE_RETRY before chaning
the waitpid-related code. But indeed, it looks very similar.
>
>> +static int
>> +do_rseq_test (void)
>> +{
>> + int i, result = 0;
>> +
>> + if (!rseq_available ())
>> + {
>> + FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
>> + }
>
> Braces are not needed here.
OK
>
>> + setup_signals ();
>> + if (raise (SIGUSR1))
>> + FAIL_EXIT1 ("error raising signal");
>
> Could use xraise.
OK
>
>> + if (do_rseq_main_test ())
>> + result = 1;
>> + for (i = 0; i < N; i++)
>> + {
>> + if (do_rseq_threads_test (t[i]))
>> + result = 1;
>> + }
>
> Braces are unnecessary.
OK
>
>> +/* Test C++ destructor called at thread and process exit. */
>> +void
>> +__call_tls_dtors (void)
>> +{
>> + /* Cannot use deferred failure reporting after main () returns. */
>> + if (!rseq_thread_registered ())
>> + FAIL_EXIT1 ("rseq not registered in C++ thread/process exit destructor");
>> +}
>
> Uhm, what is this supposed to accomplish, under the __call_tls_dtors
> name in particular? I don't think this gets ever called.
>
> It may make sense to have a separate, smaller C++ test to cover this
> (perhaps as a separate patch).
Hrm, the intent was to implement __call_tls_dtors locally so it would
be invoked by libc on thread/process exit, but looking deeper into
stdlib/cxa_thread_atexit_impl.c I suspect the hidden _call_tls_dtors
defined there will be used.
Indeed, a separate C++ test for this would be better. Could be done in a
follow up patch later perhaps ?
>
>> +#else
>
> This needs a comment on the same line. I think it corresponds to the
> earlier “#ifdef RSEQ_SIG”.
OK
>
>> +static int
>> +do_rseq_test (void)
>> +{
>> +#ifndef RSEQ_SIG
>> + FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test");
>> +#endif
>
> And with the comment, it should be obvious that the #ifndef is not
> needed here. 8-)
Indeed! :)
>
>> + return 0;
>> +}
>> +#endif
>
> Likewise: Add comment.
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c
>> b/sysdeps/unix/sysv/linux/tst-rseq.c
>> new file mode 100644
>> index 0000000000..48a61f9998
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/tst-rseq.c
>> @@ -0,0 +1,108 @@
>> +/* Restartable Sequences single-threaded tests.
>> +
>> + Copyright (C) 2020 Free Software Foundation, Inc.
>
> Perhaps remove the empty line?
OK
>
>> +#include <sys/syscall.h>
>> +#include <unistd.h>
>> +#include <stdio.h>
>> +#include <support/check.h>
>> +#include <sys/rseq.h>
>> +
>> +#ifdef RSEQ_SIG
>> +#include <syscall.h>
>
> Duplicate <syscall.h>.
OK, and fixing indentation.
>
>> +#include <stdlib.h>
>> +#include <error.h>
>> +#include <errno.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +#include <atomic.h>
>> +
>> +static int
>> +rseq_thread_registered (void)
>> +{
>> + return (int32_t) atomic_load_relaxed (&__rseq_abi.cpu_id) >= 0;
>> +}
>
> (See above.)
OK
>
>> +static int
>> +do_rseq_main_test (void)
>> +{
>> + if (!rseq_thread_registered ())
>> + {
>> + FAIL_RET ("rseq not registered in main thread");
>> + }
>> + return 0;
>> +}
>
> Additional braces.
Will use TEST_VERIFY_EXIT (rseq_thread_registered ()); instead.
>
>> +static int
>> +sys_rseq (struct rseq *rseq_abi, uint32_t rseq_len, int flags, uint32_t sig)
>> +{
>> + return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
>> +}
>> +
>> +static int
>> +rseq_available (void)
>> +{
>> + int rc;
>> +
>> + rc = sys_rseq (NULL, 0, 0, 0);
>> + if (rc != -1)
>> + FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
>> + switch (errno)
>> + {
>> + case ENOSYS:
>> + return 0;
>> + case EINVAL:
>> + return 1;
>> + default:
>> + FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
>> + }
>> +}
>
> It looks these three functions could be moved in to a tst-rseq.h header
> file (just create the file, no Makefile updates needed).
OK
>
>> +static int
>> +do_rseq_test (void)
>> +{
>> + int result = 0;
>> +
>> + if (!rseq_available ())
>> + {
>> + FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
>> + }
>> + if (do_rseq_main_test ())
>> + result = 1;
>> + return result;
>> +}
>> +#else
>> +static int
>> +do_rseq_test (void)
>> +{
>> +#ifndef RSEQ_SIG
>> + FAIL_UNSUPPORTED ("glibc does not define RSEQ_SIG, skipping test");
>> +#endif
>> + return 0;
>> +}
>> +#endif
>
> Comments on #else and #endif, see above.
OK
>
> C++ test could exercise the thread exit path via thread_local, without
> linking against libpthread.
Should we keep this for a future patch ?
Thanks,
Mathieu
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-25 15:20 ` Florian Weimer via Libc-alpha
@ 2020-05-25 17:36 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 12:41 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-25 17:36 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
----- On May 25, 2020, at 11:20 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> The larger question here is: considering that we re-implement the entire
>> uapi header within glibc (which includes the uptr addition), do we still
>> care about using the header provided by the Linux kernel ?
>
> We don't care, but our users do. Eventually, they want to include
> <sys/rseq.h> and <linux/rseq.h> to get new constants that are not yet
> known to glibc.
Good point!
>
>> Having different definitions depending on whether a kernel header is
>> installed or not when including a glibc header seems rather unexpected.
>
> Indeed.
>
>> *If* we want to use the uapi header, I think something is semantically
>> missing. Here is the scheme I envision. We could rely on the kernel header
>> version.h to figure out which of glibc or kernel uapi header is more
>> recent. Any new concept we try to integrate into glibc (e.g. uptr)
>> should go into the upstream Linux uapi header first.
>
> I think we should always prefer the uapi header. The Linux version
> check does not tell you anything about backports.
Fair enough.
>
>> For the coming glibc e.g. 2.32, we use the kernel uapi header if
>> kernel version is >= 4.18.0. Within glibc, the fallback implements
>> exactly the API exposed by the kernel rseq.h header.
>
> Agreed.
>
>> As we eventually introduce the uptr change into the Linux kernel, and
>> say it gets merged for Linux 5.9.0, we mirror this change into glibc
>> (e.g. release 2.33), and bump the Linux kernel version cutoff to 5.9.0.
>> So starting from that version, we use the Linux kernel header only if
>> version >= 5.9.0, else we fallback on glibc's own implementation.
>
> Fortunately, we don't need to settle this today. 8-)
>
> Let's stick to the 4.18 definitions for the fallback for now, and
> discuss the incorporation of future changes later.
OK
>
>>>> +/* Ensure the compiler supports __attribute__ ((aligned)). */
>>>> +_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
>>>> +_Static_assert (__alignof__ (struct rseq) >= 32, "alignment");
>>>
>>> This needs #ifndef __cplusplus or something like that. I'm surprised
>>> that this passes the installed header tests.
>>
>> Would the following be ok ?
>>
>> #ifdef __cplusplus
>> #define rseq_static_assert static_assert
>> #else
>> #define rseq_static_assert _Static_assert
>> #endif
>>
>> /* Ensure the compiler supports __attribute__ ((aligned)). */
>> rseq_static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment");
>> rseq_static_assert (__alignof__ (struct rseq) >= 32, "alignment");
>
> Seems reasonable, yes. __alignof__ is still a GCC extension. C++11 has
> alignof, C11 has _Alignof. So you could use something like this
> (perhaps without indentation for the kernel header version):
>
> #ifdef __cplusplus
> # if __cplusplus >= 201103L
> # define rseq_static_assert(x) static_assert x;
> # define rseq_alignof alignof
> # endif
> #elif __STDC_VERSION__ >= 201112L
> # define rseq_static_assert(x) _Static_assert x;
> # define rseq_alignof _Alignof
> #endif
> #ifndef rseq_static_assert
> # define rseq_static_assert /* nothing */
> #endif
> rseq_static_assert ((rseq_alignof__ (struct rseq_cs) >= 32, "alignment"))
> rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment"))
Something like this ?
#ifdef __cplusplus
# if __cplusplus >= 201103L
# define rseq_static_assert (expr, diagnostic) static_assert (expr, diagnostic)
# define rseq_alignof alignof
# endif
#elif __STDC_VERSION__ >= 201112L
# define rseq_static_assert (expr, diagnostic) _Static_assert (expr, diagnostic)
# define rseq_alignof _Alignof
#endif
#ifndef rseq_static_assert
# define rseq_static_assert (expr, diagnostic) /* nothing */
#endif
/* Ensure the compiler supports __attribute__ ((aligned)). */
rseq_static_assert ((rseq_alignof (struct rseq_cs) >= 32, "alignment"));
rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment"));
> And something similar for _Alignas/attribute aligned,
I don't see where _Alignas is needed here ?
For attribute aligned, what would be the oldest supported C and C++
standards ?
> with an error for
> older standards and !__GNUC__ compilers (because neither the type nor
> __thread can be represented there).
By "type" you mean "struct rseq" here ? What does it contain that requires
a __GNUC__ compiler ?
About __thread, I recall other compilers have other means to declare it.
In liburcu, I end up with the following:
#if defined (__cplusplus) && (__cplusplus >= 201103L)
# define URCU_TLS_STORAGE_CLASS thread_local
#elif defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)
# define URCU_TLS_STORAGE_CLASS _Thread_local
#elif defined (_MSC_VER)
# define URCU_TLS_STORAGE_CLASS __declspec(thread)
#else
# define URCU_TLS_STORAGE_CLASS __thread
#endif
Would something along those lines be OK for libc ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-25 17:36 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-26 12:41 ` Florian Weimer via Libc-alpha
2020-05-26 14:32 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-26 12:41 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
* Mathieu Desnoyers:
> Something like this ?
>
> #ifdef __cplusplus
> # if __cplusplus >= 201103L
> # define rseq_static_assert (expr, diagnostic) static_assert (expr, diagnostic)
> # define rseq_alignof alignof
> # endif
> #elif __STDC_VERSION__ >= 201112L
> # define rseq_static_assert (expr, diagnostic) _Static_assert (expr, diagnostic)
> # define rseq_alignof _Alignof
> #endif
>
> #ifndef rseq_static_assert
> # define rseq_static_assert (expr, diagnostic) /* nothing */
> #endif
You can't have a space in #defines like that, no matter what GNU style
says. 8-)
> /* Ensure the compiler supports __attribute__ ((aligned)). */
> rseq_static_assert ((rseq_alignof (struct rseq_cs) >= 32, "alignment"));
> rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment"));
You need to move the ; into rseq_static_assert. And if you use explicit
arguments, you can't use double parentheses.
>> And something similar for _Alignas/attribute aligned,
>
> I don't see where _Alignas is needed here ?
>
> For attribute aligned, what would be the oldest supported C and C++
> standards ?
There are no standardized attributes for C, there is only _Alignas.
C++11 has an alignas specifier; it's not an attribute either. I think
these are syntactically similar.
>> with an error for
>> older standards and !__GNUC__ compilers (because neither the type nor
>> __thread can be represented there).
>
> By "type" you mean "struct rseq" here ? What does it contain that requires
> a __GNUC__ compiler ?
__attribute__ and __thread support.
> About __thread, I recall other compilers have other means to declare it.
> In liburcu, I end up with the following:
>
> #if defined (__cplusplus) && (__cplusplus >= 201103L)
> # define URCU_TLS_STORAGE_CLASS thread_local
> #elif defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)
> # define URCU_TLS_STORAGE_CLASS _Thread_local
> #elif defined (_MSC_VER)
> # define URCU_TLS_STORAGE_CLASS __declspec(thread)
> #else
> # define URCU_TLS_STORAGE_CLASS __thread
> #endif
>
> Would something along those lines be OK for libc ?
Yes, it would be okay (minus the Visual C++ part). This part does not
have to go into UAPI headers first. A fallback definition of __thread
should be okay. Outside glibc, the TLS model declaration is optional, I
think. The glibc *definition* ensures that the variable is
initial-exec.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-25 17:07 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-26 12:47 ` Florian Weimer via Libc-alpha
2020-05-26 14:43 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-26 12:47 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: libc-alpha, Joseph Myers
* Mathieu Desnoyers:
>> The present code does not wait until all threads have entered their
>> cancellation region, so I'm not sure if the test object is actually met
>> here.
>
> We're only cancelling the first thread in the test, which is the intent.
> In terms of barrier, it's a barrier involving only 2 threads.
Huh. I need to look at the version with the real barrier.
>>> +static int
>>> +rseq_available (void)
>>> +{
>>> + int rc;
>>> +
>>> + rc = sys_rseq (NULL, 0, 0, 0);
>>> + if (rc != -1)
>>> + FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
>>> + switch (errno)
>>> + {
>>> + case ENOSYS:
>>> + return 0;
>>> + case EINVAL:
>>> + return 1;
>>> + default:
>>> + FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
>>> + }
>>> +}
>>
>> Maybe add a comment to explain what EINVAL means in this context?
>
> For instance:
>
> /* rseq is implemented, but detected an invalid parameter. */
Ah, so 0 is an invalid operation?
>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>> + if (retpid != pid)
>>> + {
>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>> + (long int) retpid, (long int) pid);
>>> + }
>>
>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>> have this.
>
> Then how does it deal with a signal interrupting the system call performing
> the waitpid (EINTR) ? I do not see WNOHANG being used.
It obscures spurious signals. In most test cases, if an unexpected
signal is delivered, something is quite wrong indeed. This is why we
don't generally hide EINTR errors.
>>> +/* Test C++ destructor called at thread and process exit. */
>>> +void
>>> +__call_tls_dtors (void)
>>> +{
>>> + /* Cannot use deferred failure reporting after main () returns. */
>>> + if (!rseq_thread_registered ())
>>> + FAIL_EXIT1 ("rseq not registered in C++ thread/process exit destructor");
>>> +}
>>
>> Uhm, what is this supposed to accomplish, under the __call_tls_dtors
>> name in particular? I don't think this gets ever called.
>>
>> It may make sense to have a separate, smaller C++ test to cover this
>> (perhaps as a separate patch).
>
> Hrm, the intent was to implement __call_tls_dtors locally so it would
> be invoked by libc on thread/process exit, but looking deeper into
> stdlib/cxa_thread_atexit_impl.c I suspect the hidden _call_tls_dtors
> defined there will be used.
Right, it's not an interposable symbol.
> Indeed, a separate C++ test for this would be better. Could be done in a
> follow up patch later perhaps ?
Yes, let's remove this and add a real C++ test later.
>> C++ test could exercise the thread exit path via thread_local, without
>> linking against libpthread.
>
> Should we keep this for a future patch ?
Yes, please.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-26 12:41 ` Florian Weimer via Libc-alpha
@ 2020-05-26 14:32 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 14:38 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-26 14:32 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
----- On May 26, 2020, at 8:41 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> Something like this ?
>>
>> #ifdef __cplusplus
>> # if __cplusplus >= 201103L
>> # define rseq_static_assert (expr, diagnostic) static_assert (expr,
>> diagnostic)
>> # define rseq_alignof alignof
>> # endif
>> #elif __STDC_VERSION__ >= 201112L
>> # define rseq_static_assert (expr, diagnostic) _Static_assert (expr,
>> diagnostic)
>> # define rseq_alignof _Alignof
>> #endif
>>
>> #ifndef rseq_static_assert
>> # define rseq_static_assert (expr, diagnostic) /* nothing */
>> #endif
>
> You can't have a space in #defines like that, no matter what GNU style
> says. 8-)
Yes, I noticed when failing to build this ;)
>
>> /* Ensure the compiler supports __attribute__ ((aligned)). */
>> rseq_static_assert ((rseq_alignof (struct rseq_cs) >= 32, "alignment"));
>> rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment"));
>
> You need to move the ; into rseq_static_assert. And if you use explicit
> arguments, you can't use double parentheses.
Why move the ";" into the macro ?
AFAIU, the only gain here would be to make sure we don't emit useless
";" in the "/* nothing */" case. But does it matter ?
Examples I can find of "static_assert" explicitly have the ";" at the
end, so I find it weird to integrate it into the rseq_static_assert
macro, which makes it different from static_assert.
Agreed on the need to remove the double-parentheses.
>
>>> And something similar for _Alignas/attribute aligned,
>>
>> I don't see where _Alignas is needed here ?
>>
>> For attribute aligned, what would be the oldest supported C and C++
>> standards ?
>
> There are no standardized attributes for C, there is only _Alignas.
> C++11 has an alignas specifier; it's not an attribute either. I think
> these are syntactically similar.
There appears to be an interesting difference between attribute aligned
and alignas. It seems like alignas cannot be used on a structure declaration,
only on fields, e.g.:
struct blah {
int a;
} _Alignas (16);
o.c:3:1: warning: useless ‘_Alignas’ in empty declaration
} _Alignas (16);
But
struct blah {
int _Alignas (16) a;
};
is OK. So if I change e.g. struct rseq_cs to align
the first field:
struct rseq_cs
{
/* Version of this structure. */
uint32_t rseq_align (32) version;
/* enum rseq_cs_flags. */
uint32_t flags;
uint64_t start_ip;
/* Offset from start_ip. */
uint64_t post_commit_offset;
uint64_t abort_ip;
};
It should work.
>
>>> with an error for
>>> older standards and !__GNUC__ compilers (because neither the type nor
>>> __thread can be represented there).
>>
>> By "type" you mean "struct rseq" here ? What does it contain that requires
>> a __GNUC__ compiler ?
>
> __attribute__ and __thread support.
OK
>
>> About __thread, I recall other compilers have other means to declare it.
>> In liburcu, I end up with the following:
>>
>> #if defined (__cplusplus) && (__cplusplus >= 201103L)
>> # define URCU_TLS_STORAGE_CLASS thread_local
>> #elif defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)
>> # define URCU_TLS_STORAGE_CLASS _Thread_local
>> #elif defined (_MSC_VER)
>> # define URCU_TLS_STORAGE_CLASS __declspec(thread)
>> #else
>> # define URCU_TLS_STORAGE_CLASS __thread
>> #endif
>>
>> Would something along those lines be OK for libc ?
>
> Yes, it would be okay (minus the Visual C++ part). This part does not
> have to go into UAPI headers first. A fallback definition of __thread
> should be okay. Outside glibc, the TLS model declaration is optional, I
> think. The glibc *definition* ensures that the variable is
> initial-exec.
AFAIU you are technically correct when stating that the tls model
on the declaration is optional, but I think it's a good thing to have
it there rather than only at the definition. It makes it clear to all
users of this variable that its model is IE. Especially in scenarios where
early-adopter libraries and applications can define their own __rseq_abi
symbol, I think it's good to explicitly keep the IE tls model attribute in
the header.
I end up with the following:
#ifdef __cplusplus
# if __cplusplus >= 201103L
# define rseq_static_assert(expr, diagnostic) static_assert (expr, diagnostic)
# define rseq_alignof(type) alignof (type)
# define rseq_alignas(x) alignas (x)
# 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_alignas(x) _Alignas (x)
# 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
# ifndef rseq_alignas
# define rseq_alignas(x) __attribute__ ((aligned (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
/* Fall back to __thread for TLS storage class. */
#ifndef rseq_tls_storage_class
# define rseq_tls_storage_class __thread
#endif
[...]
/* 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");
/* Allocations of struct rseq and struct rseq_cs on the heap need to
be aligned on 32 bytes. Therefore, use of malloc is discouraged
because it does not guarantee alignment. posix_memalign should be
used instead. */
extern rseq_tls_storage_class struct rseq __rseq_abi rseq_tls_model_ie;
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-26 14:32 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-26 14:38 ` Florian Weimer via Libc-alpha
2020-05-26 14:53 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-26 14:38 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
* Mathieu Desnoyers:
> AFAIU, the only gain here would be to make sure we don't emit useless
> ";" in the "/* nothing */" case. But does it matter ?
I don't think C allows empty constructs like this at the top level.
>>>> And something similar for _Alignas/attribute aligned,
>>>
>>> I don't see where _Alignas is needed here ?
>>>
>>> For attribute aligned, what would be the oldest supported C and C++
>>> standards ?
>>
>> There are no standardized attributes for C, there is only _Alignas.
>> C++11 has an alignas specifier; it's not an attribute either. I think
>> these are syntactically similar.
>
> There appears to be an interesting difference between attribute aligned
> and alignas. It seems like alignas cannot be used on a structure declaration,
> only on fields, e.g.:
>
> struct blah {
> int a;
> } _Alignas (16);
>
> o.c:3:1: warning: useless ‘_Alignas’ in empty declaration
> } _Alignas (16);
>
> But
>
> struct blah {
> int _Alignas (16) a;
> };
Like the attribute, it needs to come right after the struct keyword, I
think. (Trailing attributes can be ambiguous, but not in this case.)
> is OK. So if I change e.g. struct rseq_cs to align
> the first field:
>
> struct rseq_cs
> {
> /* Version of this structure. */
> uint32_t rseq_align (32) version;
> /* enum rseq_cs_flags. */
> uint32_t flags;
> uint64_t start_ip;
> /* Offset from start_ip. */
> uint64_t post_commit_offset;
> uint64_t abort_ip;
> };
>
> It should work.
Indeed.
> /* Rely on GNU extensions for older standards and tls model. */
> #ifdef __GNUC__
> # ifndef rseq_alignof
> # define rseq_alignof(x) __alignof__ (x)
> # endif
> # ifndef rseq_alignas
> # define rseq_alignas(x) __attribute__ ((aligned (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
>
> /* Fall back to __thread for TLS storage class. */
> #ifndef rseq_tls_storage_class
> # define rseq_tls_storage_class __thread
> #endif
If they are only used in the glibc headers, they should have __rseq
prefixes, so that application code doesn't start using them (in case we
have to change/fix them, or move the into <sys/cdefs.h> later).
Rest looks fine.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-26 12:47 ` Florian Weimer via Libc-alpha
@ 2020-05-26 14:43 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:05 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:12 ` Florian Weimer via Libc-alpha
0 siblings, 2 replies; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-26 14:43 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers
----- On May 26, 2020, at 8:47 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>>> The present code does not wait until all threads have entered their
>>> cancellation region, so I'm not sure if the test object is actually met
>>> here.
>>
>> We're only cancelling the first thread in the test, which is the intent.
>> In terms of barrier, it's a barrier involving only 2 threads.
>
> Huh. I need to look at the version with the real barrier.
Useful bits:
static void
cancel_routine (void *arg)
{
if (!rseq_thread_registered ())
{
printf ("error: rseq not registered in cancel routine\n");
support_record_failure ();
}
}
static pthread_barrier_t cancel_thread_barrier;
static pthread_cond_t cancel_thread_cond = PTHREAD_COND_INITIALIZER;
static pthread_mutex_t cancel_thread_mutex = PTHREAD_MUTEX_INITIALIZER;
static void
test_cancel_thread (void)
{
pthread_cleanup_push (cancel_routine, NULL);
(void) xpthread_barrier_wait (&cancel_thread_barrier);
/* Wait forever until cancellation. */
xpthread_cond_wait (&cancel_thread_cond, &cancel_thread_mutex);
pthread_cleanup_pop (0);
}
static void *
thread_function (void * arg)
{
int i = (int) (intptr_t) arg;
xraise (SIGUSR1);
if (i == 0)
test_cancel_thread ();
TEST_COMPARE (pthread_setspecific (rseq_test_key, (void *) 1l), 0);
return rseq_thread_registered () ? NULL : (void *) 1l;
}
static int
do_rseq_threads_test (int nr_threads)
{
pthread_t th[nr_threads];
int i;
int result = 0;
xpthread_barrier_init (&cancel_thread_barrier, NULL, 2);
for (i = 0; i < nr_threads; ++i)
th[i] = xpthread_create (NULL, thread_function,
(void *) (intptr_t) i);
(void) xpthread_barrier_wait (&cancel_thread_barrier);
xpthread_cancel (th[0]);
for (i = 0; i < nr_threads; ++i)
{
void *v;
v = xpthread_join (th[i]);
if (i != 0 && v != NULL)
{
printf ("error: join %d successful, but child failed\n", i);
result = 1;
}
else if (i == 0 && v == NULL)
{
printf ("error: join %d successful, child did not fail as expected\n", i);
result = 1;
}
}
xpthread_barrier_destroy (&cancel_thread_barrier);
return result;
}
>
>>>> +static int
>>>> +rseq_available (void)
>>>> +{
>>>> + int rc;
>>>> +
>>>> + rc = sys_rseq (NULL, 0, 0, 0);
>>>> + if (rc != -1)
>>>> + FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
>>>> + switch (errno)
>>>> + {
>>>> + case ENOSYS:
>>>> + return 0;
>>>> + case EINVAL:
>>>> + return 1;
>>>> + default:
>>>> + FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
>>>> + }
>>>> +}
>>>
>>> Maybe add a comment to explain what EINVAL means in this context?
>>
>> For instance:
>>
>> /* rseq is implemented, but detected an invalid parameter. */
>
> Ah, so 0 is an invalid operation?
So the @flags parameter is 0, which is fine.
It's the @rseq_len parameter being 0 (which differs from sizeof(struct rseq))
which returns -EINVAL. I will clarify this in the comment:
/* rseq is implemented, but detected an invalid rseq_len parameter. */
>
>>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>>> + if (retpid != pid)
>>>> + {
>>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>>> + (long int) retpid, (long int) pid);
>>>> + }
>>>
>>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>>> have this.
>>
>> Then how does it deal with a signal interrupting the system call performing
>> the waitpid (EINTR) ? I do not see WNOHANG being used.
>
> It obscures spurious signals. In most test cases, if an unexpected
> signal is delivered, something is quite wrong indeed. This is why we
> don't generally hide EINTR errors.
So it means you may have trouble using tools like strace and gdb on those
tests ? AFAIU those are heavy users of SIGSTOP and SIGCONT. Similarly for
profilers, those usually rely on a timer-driven signal.
>
>>>> +/* Test C++ destructor called at thread and process exit. */
>>>> +void
>>>> +__call_tls_dtors (void)
>>>> +{
>>>> + /* Cannot use deferred failure reporting after main () returns. */
>>>> + if (!rseq_thread_registered ())
>>>> + FAIL_EXIT1 ("rseq not registered in C++ thread/process exit destructor");
>>>> +}
>>>
>>> Uhm, what is this supposed to accomplish, under the __call_tls_dtors
>>> name in particular? I don't think this gets ever called.
>>>
>>> It may make sense to have a separate, smaller C++ test to cover this
>>> (perhaps as a separate patch).
>>
>> Hrm, the intent was to implement __call_tls_dtors locally so it would
>> be invoked by libc on thread/process exit, but looking deeper into
>> stdlib/cxa_thread_atexit_impl.c I suspect the hidden _call_tls_dtors
>> defined there will be used.
>
> Right, it's not an interposable symbol.
>
>> Indeed, a separate C++ test for this would be better. Could be done in a
>> follow up patch later perhaps ?
>
> Yes, let's remove this and add a real C++ test later.
OK
>
>>> C++ test could exercise the thread exit path via thread_local, without
>>> linking against libpthread.
>>
>> Should we keep this for a future patch ?
>
> Yes, please.
OK, thanks!
Mathieu
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-26 14:38 ` Florian Weimer via Libc-alpha
@ 2020-05-26 14:53 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 14:57 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-26 14:53 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
----- On May 26, 2020, at 10:38 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> AFAIU, the only gain here would be to make sure we don't emit useless
>> ";" in the "/* nothing */" case. But does it matter ?
>
> I don't think C allows empty constructs like this at the top level.
>
>>>>> And something similar for _Alignas/attribute aligned,
>>>>
>>>> I don't see where _Alignas is needed here ?
>>>>
>>>> For attribute aligned, what would be the oldest supported C and C++
>>>> standards ?
>>>
>>> There are no standardized attributes for C, there is only _Alignas.
>>> C++11 has an alignas specifier; it's not an attribute either. I think
>>> these are syntactically similar.
>>
>> There appears to be an interesting difference between attribute aligned
>> and alignas. It seems like alignas cannot be used on a structure declaration,
>> only on fields, e.g.:
>>
>> struct blah {
>> int a;
>> } _Alignas (16);
>>
>> o.c:3:1: warning: useless ‘_Alignas’ in empty declaration
>> } _Alignas (16);
>>
>> But
>>
>> struct blah {
>> int _Alignas (16) a;
>> };
>
> Like the attribute, it needs to come right after the struct keyword, I
> think. (Trailing attributes can be ambiguous, but not in this case.)
Nope. _Alignas really _is_ special :-(
struct _Alignas (16) blah {
int a;
};
p.c:1:8: error: expected ‘{’ before ‘_Alignas’
struct _Alignas (16) blah {
Also:
struct blah _Alignas (16) {
int a;
};
p.c:1:27: error: expected identifier or ‘(’ before ‘{’ token
struct blah _Alignas (16) {
>
>> is OK. So if I change e.g. struct rseq_cs to align
>> the first field:
>>
>> struct rseq_cs
>> {
>> /* Version of this structure. */
>> uint32_t rseq_align (32) version;
>> /* enum rseq_cs_flags. */
>> uint32_t flags;
>> uint64_t start_ip;
>> /* Offset from start_ip. */
>> uint64_t post_commit_offset;
>> uint64_t abort_ip;
>> };
>>
>> It should work.
>
> Indeed.
OK, so let's go for that approach.
>
>> /* Rely on GNU extensions for older standards and tls model. */
>> #ifdef __GNUC__
>> # ifndef rseq_alignof
>> # define rseq_alignof(x) __alignof__ (x)
>> # endif
>> # ifndef rseq_alignas
>> # define rseq_alignas(x) __attribute__ ((aligned (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
>>
>> /* Fall back to __thread for TLS storage class. */
>> #ifndef rseq_tls_storage_class
>> # define rseq_tls_storage_class __thread
>> #endif
>
> If they are only used in the glibc headers, they should have __rseq
> prefixes, so that application code doesn't start using them (in case we
> have to change/fix them, or move the into <sys/cdefs.h> later).
OK will do.
>
> Rest looks fine.
One last thing I'm planning to add in sys/rseq.h to cover acessing the
rseq_cs pointers with both the UAPI headers and the glibc struct rseq
declarations:
/* The rseq_cs_ptr macro can be used to access the pointer to the current
rseq critical section descriptor. */
#ifdef __LP64__
# define rseq_cs_ptr(rseq) \
((const struct rseq_cs *) (rseq)->rseq_cs.ptr)
#else /* __LP64__ */
# define rseq_cs_ptr(rseq) \
((const struct rseq_cs *) (rseq)->rseq_cs.ptr.ptr32)
#endif /* __LP64__ */
Does it make sense ?
Thanks,
Mathieu
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-26 14:53 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-26 14:57 ` Florian Weimer via Libc-alpha
2020-05-26 15:22 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-26 14:57 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
* Mathieu Desnoyers:
>> Like the attribute, it needs to come right after the struct keyword, I
>> think. (Trailing attributes can be ambiguous, but not in this case.)
>
> Nope. _Alignas really _is_ special :-(
>
> struct _Alignas (16) blah {
> int a;
> };
>
> p.c:1:8: error: expected ‘{’ before ‘_Alignas’
> struct _Alignas (16) blah {
Meh, yet another unnecessary C++ incompatibility. C does not support
empty structs, so I assume they didn't see the field requirement as a
burden.
> One last thing I'm planning to add in sys/rseq.h to cover acessing the
> rseq_cs pointers with both the UAPI headers and the glibc struct rseq
> declarations:
>
> /* The rseq_cs_ptr macro can be used to access the pointer to the current
> rseq critical section descriptor. */
> #ifdef __LP64__
> # define rseq_cs_ptr(rseq) \
> ((const struct rseq_cs *) (rseq)->rseq_cs.ptr)
> #else /* __LP64__ */
> # define rseq_cs_ptr(rseq) \
> ((const struct rseq_cs *) (rseq)->rseq_cs.ptr.ptr32)
> #endif /* __LP64__ */
>
> Does it make sense ?
Written this way, it's an aliasing violation. I don't think it's very
useful.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
2020-05-26 14:57 ` Florian Weimer via Libc-alpha
@ 2020-05-26 15:22 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 0 replies; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-26 15:22 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, libc-alpha, Peter Zijlstra, linux-api, Boqun Feng,
Will Deacon, linux-kernel, Ben Maurer, Dave Watson,
Thomas Gleixner, Paul, Paul Turner, Joseph Myers
----- On May 26, 2020, at 10:57 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>>> Like the attribute, it needs to come right after the struct keyword, I
>>> think. (Trailing attributes can be ambiguous, but not in this case.)
>>
>> Nope. _Alignas really _is_ special :-(
>>
>> struct _Alignas (16) blah {
>> int a;
>> };
>>
>> p.c:1:8: error: expected ‘{’ before ‘_Alignas’
>> struct _Alignas (16) blah {
>
> Meh, yet another unnecessary C++ incompatibility. C does not support
> empty structs, so I assume they didn't see the field requirement as a
> burden.
Indeed, it's weird.
>
>> One last thing I'm planning to add in sys/rseq.h to cover acessing the
>> rseq_cs pointers with both the UAPI headers and the glibc struct rseq
>> declarations:
>>
>> /* The rseq_cs_ptr macro can be used to access the pointer to the current
>> rseq critical section descriptor. */
>> #ifdef __LP64__
>> # define rseq_cs_ptr(rseq) \
>> ((const struct rseq_cs *) (rseq)->rseq_cs.ptr)
>> #else /* __LP64__ */
>> # define rseq_cs_ptr(rseq) \
>> ((const struct rseq_cs *) (rseq)->rseq_cs.ptr.ptr32)
>> #endif /* __LP64__ */
>>
>> Does it make sense ?
>
> Written this way, it's an aliasing violation. I don't think it's very
> useful.
OK, I'll just remove it.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-26 14:43 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-27 15:05 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:12 ` Florian Weimer via Libc-alpha
1 sibling, 0 replies; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-27 15:05 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers
----- On May 26, 2020, at 10:43 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
> ----- On May 26, 2020, at 8:47 AM, Florian Weimer fweimer@redhat.com wrote:
>
>> * Mathieu Desnoyers:
[...]
The one question below is the last thing we need to complete discussing
before I can re-spin another version of the patches:
>>
>>>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>>>> + if (retpid != pid)
>>>>> + {
>>>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>>>> + (long int) retpid, (long int) pid);
>>>>> + }
>>>>
>>>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>>>> have this.
>>>
>>> Then how does it deal with a signal interrupting the system call performing
>>> the waitpid (EINTR) ? I do not see WNOHANG being used.
>>
>> It obscures spurious signals. In most test cases, if an unexpected
>> signal is delivered, something is quite wrong indeed. This is why we
>> don't generally hide EINTR errors.
>
> So it means you may have trouble using tools like strace and gdb on those
> tests ? AFAIU those are heavy users of SIGSTOP and SIGCONT. Similarly for
> profilers, those usually rely on a timer-driven signal.
>
Should I use xwaitpid instead, and should xwaitpid be fixed to use
TEMP_FAILURE_RETRY ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-26 14:43 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:05 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-27 15:12 ` Florian Weimer via Libc-alpha
2020-05-27 15:17 ` Mathieu Desnoyers via Libc-alpha
1 sibling, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-27 15:12 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: libc-alpha, Joseph Myers
* Mathieu Desnoyers:
>>>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>>>> + if (retpid != pid)
>>>>> + {
>>>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>>>> + (long int) retpid, (long int) pid);
>>>>> + }
>>>>
>>>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>>>> have this.
>>>
>>> Then how does it deal with a signal interrupting the system call performing
>>> the waitpid (EINTR) ? I do not see WNOHANG being used.
>>
>> It obscures spurious signals. In most test cases, if an unexpected
>> signal is delivered, something is quite wrong indeed. This is why we
>> don't generally hide EINTR errors.
>
> So it means you may have trouble using tools like strace and gdb on those
> tests ? AFAIU those are heavy users of SIGSTOP and SIGCONT. Similarly for
> profilers, those usually rely on a timer-driven signal.
I have never seen any problems with strace due to this. ptrace has
become a bit more transparent to the tracee since the early days, I
think.
I haven't seen problems under GDB, either, but then tests that fork can
be rather annoying to debug anyway.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-27 15:12 ` Florian Weimer via Libc-alpha
@ 2020-05-27 15:17 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:21 ` Florian Weimer via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-27 15:17 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers
----- On May 27, 2020, at 11:12 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>>>>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>>>>> + if (retpid != pid)
>>>>>> + {
>>>>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>>>>> + (long int) retpid, (long int) pid);
>>>>>> + }
>>>>>
>>>>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>>>>> have this.
>>>>
>>>> Then how does it deal with a signal interrupting the system call performing
>>>> the waitpid (EINTR) ? I do not see WNOHANG being used.
>>>
>>> It obscures spurious signals. In most test cases, if an unexpected
>>> signal is delivered, something is quite wrong indeed. This is why we
>>> don't generally hide EINTR errors.
>>
>> So it means you may have trouble using tools like strace and gdb on those
>> tests ? AFAIU those are heavy users of SIGSTOP and SIGCONT. Similarly for
>> profilers, those usually rely on a timer-driven signal.
>
> I have never seen any problems with strace due to this. ptrace has
> become a bit more transparent to the tracee since the early days, I
> think.
>
> I haven't seen problems under GDB, either, but then tests that fork can
> be rather annoying to debug anyway.
OK so I'll use the xwaitpid wrapper and let this be someone else's problem.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-27 15:17 ` Mathieu Desnoyers via Libc-alpha
@ 2020-05-27 15:21 ` Florian Weimer via Libc-alpha
2020-05-27 15:30 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-05-27 15:21 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: libc-alpha, Joseph Myers
* Mathieu Desnoyers:
> ----- On May 27, 2020, at 11:12 AM, Florian Weimer fweimer@redhat.com wrote:
>
>> * Mathieu Desnoyers:
>>
>>>>>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>>>>>> + if (retpid != pid)
>>>>>>> + {
>>>>>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>>>>>> + (long int) retpid, (long int) pid);
>>>>>>> + }
>>>>>>
>>>>>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>>>>>> have this.
>>>>>
>>>>> Then how does it deal with a signal interrupting the system call performing
>>>>> the waitpid (EINTR) ? I do not see WNOHANG being used.
>>>>
>>>> It obscures spurious signals. In most test cases, if an unexpected
>>>> signal is delivered, something is quite wrong indeed. This is why we
>>>> don't generally hide EINTR errors.
>>>
>>> So it means you may have trouble using tools like strace and gdb on those
>>> tests ? AFAIU those are heavy users of SIGSTOP and SIGCONT. Similarly for
>>> profilers, those usually rely on a timer-driven signal.
>>
>> I have never seen any problems with strace due to this. ptrace has
>> become a bit more transparent to the tracee since the early days, I
>> think.
>>
>> I haven't seen problems under GDB, either, but then tests that fork can
>> be rather annoying to debug anyway.
>
> OK so I'll use the xwaitpid wrapper and let this be someone else's problem.
Yes please. Or support_isolate_in_subprocess.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH glibc 3/3] rseq registration tests (v10)
2020-05-27 15:21 ` Florian Weimer via Libc-alpha
@ 2020-05-27 15:30 ` Mathieu Desnoyers via Libc-alpha
0 siblings, 0 replies; 27+ messages in thread
From: Mathieu Desnoyers via Libc-alpha @ 2020-05-27 15:30 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, Joseph Myers
----- On May 27, 2020, at 11:21 AM, Florian Weimer fweimer@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> ----- On May 27, 2020, at 11:12 AM, Florian Weimer fweimer@redhat.com wrote:
>>
>>> * Mathieu Desnoyers:
>>>
>>>>>>>> + retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
>>>>>>>> + if (retpid != pid)
>>>>>>>> + {
>>>>>>>> + FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
>>>>>>>> + (long int) retpid, (long int) pid);
>>>>>>>> + }
>>>>>>>
>>>>>>> Hmm. Is the TEMP_FAILURE_RETRY really needed? Our xwaitpid does not
>>>>>>> have this.
>>>>>>
>>>>>> Then how does it deal with a signal interrupting the system call performing
>>>>>> the waitpid (EINTR) ? I do not see WNOHANG being used.
>>>>>
>>>>> It obscures spurious signals. In most test cases, if an unexpected
>>>>> signal is delivered, something is quite wrong indeed. This is why we
>>>>> don't generally hide EINTR errors.
>>>>
>>>> So it means you may have trouble using tools like strace and gdb on those
>>>> tests ? AFAIU those are heavy users of SIGSTOP and SIGCONT. Similarly for
>>>> profilers, those usually rely on a timer-driven signal.
>>>
>>> I have never seen any problems with strace due to this. ptrace has
>>> become a bit more transparent to the tracee since the early days, I
>>> think.
>>>
>>> I haven't seen problems under GDB, either, but then tests that fork can
>>> be rather annoying to debug anyway.
>>
>> OK so I'll use the xwaitpid wrapper and let this be someone else's problem.
>
> Yes please. Or support_isolate_in_subprocess.
Right, even better, thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-05-27 15:30 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-01 2:14 [PATCH glibc 0/3] Restartable Sequences enablement Mathieu Desnoyers via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19) Mathieu Desnoyers via Libc-alpha
2020-05-20 11:40 ` Florian Weimer via Libc-alpha
2020-05-25 14:51 ` Mathieu Desnoyers via Libc-alpha
2020-05-25 15:20 ` Florian Weimer via Libc-alpha
2020-05-25 17:36 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 12:41 ` Florian Weimer via Libc-alpha
2020-05-26 14:32 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 14:38 ` Florian Weimer via Libc-alpha
2020-05-26 14:53 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 14:57 ` Florian Weimer via Libc-alpha
2020-05-26 15:22 ` Mathieu Desnoyers via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 2/3] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux (v7) Mathieu Desnoyers via Libc-alpha
2020-05-20 10:14 ` Florian Weimer via Libc-alpha
2020-05-01 2:14 ` [PATCH glibc 3/3] rseq registration tests (v10) Mathieu Desnoyers via Libc-alpha
2020-05-20 10:52 ` Florian Weimer via Libc-alpha
2020-05-25 17:07 ` Mathieu Desnoyers via Libc-alpha
2020-05-26 12:47 ` Florian Weimer via Libc-alpha
2020-05-26 14:43 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:05 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:12 ` Florian Weimer via Libc-alpha
2020-05-27 15:17 ` Mathieu Desnoyers via Libc-alpha
2020-05-27 15:21 ` Florian Weimer via Libc-alpha
2020-05-27 15:30 ` Mathieu Desnoyers via Libc-alpha
2020-05-20 11:44 ` [PATCH glibc 0/3] Restartable Sequences enablement Florian Weimer via Libc-alpha
2020-05-25 13:52 ` Mathieu Desnoyers via Libc-alpha
2020-05-25 14:28 ` Florian Weimer via Libc-alpha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).