* [PATCH v8 0/8] Extend rseq support
@ 2024-02-06 16:27 Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols Michael Jeanson
` (7 more replies)
0 siblings, 8 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson
This series rebases the standalone "Add rseq extensible ABI" patch on
current master and adds an accelerated getcpu() implementation using the
rseq extensible ABI with initial support for aarch64 and x86_64.
On an aarch64 system (Snapdragon 8cx Gen 3) which lacks a vDSO for
getcpu() we measured an improvement from 130 ns to 1 ns while on x86_64
(i7-8550U) which has a vDSO we measured a more modest improvement from
10 ns to 2 ns.
Tested on i386, aarch64 and x86_64.
---
Changes since v7:
* Fix sorting of symbols in abilist files
Mathieu Desnoyers (2):
x86-64: Add rseq_load32_load32_relaxed
aarch64: Add rseq_load32_load32_relaxed
Michael Jeanson (6):
nptl: fix potential merge of __rseq_* relro symbols
Add rseq extensible ABI support
nptl: Add public __rseq_feature_size symbol
nptl: Add features to internal 'struct rseq_area'
nptl: Add rseq internal utils
Linux: Use rseq to accelerate getcpu
csu/Makefile | 2 +-
csu/libc-tls.c | 66 ++++++-
csu/rseq-sizes.sym | 11 ++
elf/Makefile | 1 +
elf/dl-rseq-symbols.S | 72 ++++++++
elf/dl-tls.c | 62 +++++++
elf/rtld_static_init.c | 12 ++
manual/threads.texi | 8 +
nptl/descr.h | 20 +-
nptl/pthread_create.c | 2 +-
sysdeps/generic/dl-rseq.h | 26 +++
sysdeps/generic/ldsodefs.h | 12 ++
sysdeps/i386/nptl/tcb-access.h | 56 ++++++
sysdeps/nptl/dl-tls_init_tp.c | 16 +-
sysdeps/nptl/tcb-access.h | 5 +
sysdeps/unix/sysv/linux/Makefile | 10 +
sysdeps/unix/sysv/linux/Versions | 3 +
sysdeps/unix/sysv/linux/aarch64/ld.abilist | 1 +
.../unix/sysv/linux/aarch64/rseq-internal.h | 173 ++++++++++++++++++
sysdeps/unix/sysv/linux/alpha/ld.abilist | 1 +
sysdeps/unix/sysv/linux/arc/ld.abilist | 1 +
sysdeps/unix/sysv/linux/arm/be/ld.abilist | 1 +
sysdeps/unix/sysv/linux/arm/le/ld.abilist | 1 +
sysdeps/unix/sysv/linux/csky/ld.abilist | 1 +
sysdeps/unix/sysv/linux/dl-parse_auxv.h | 6 +
sysdeps/unix/sysv/linux/getcpu.c | 32 +++-
sysdeps/unix/sysv/linux/hppa/ld.abilist | 1 +
sysdeps/unix/sysv/linux/i386/ld.abilist | 1 +
.../unix/sysv/linux/loongarch/lp64/ld.abilist | 1 +
.../unix/sysv/linux/m68k/coldfire/ld.abilist | 1 +
.../unix/sysv/linux/m68k/m680x0/ld.abilist | 1 +
sysdeps/unix/sysv/linux/microblaze/ld.abilist | 1 +
.../unix/sysv/linux/mips/mips32/ld.abilist | 1 +
.../sysv/linux/mips/mips64/n32/ld.abilist | 1 +
.../sysv/linux/mips/mips64/n64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/nios2/ld.abilist | 1 +
sysdeps/unix/sysv/linux/or1k/ld.abilist | 1 +
.../sysv/linux/powerpc/powerpc32/ld.abilist | 1 +
.../linux/powerpc/powerpc64/be/ld.abilist | 1 +
.../linux/powerpc/powerpc64/le/ld.abilist | 1 +
sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist | 1 +
sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/rseq-internal.h | 89 ++++++++-
.../unix/sysv/linux/s390/s390-32/ld.abilist | 1 +
.../unix/sysv/linux/s390/s390-64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/sched_getcpu.c | 3 +-
sysdeps/unix/sysv/linux/sh/be/ld.abilist | 1 +
sysdeps/unix/sysv/linux/sh/le/ld.abilist | 1 +
.../unix/sysv/linux/sparc/sparc32/ld.abilist | 1 +
.../unix/sysv/linux/sparc/sparc64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/sys/rseq.h | 4 +
.../unix/sysv/linux/tst-rseq-disable-static.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq-disable.c | 20 +-
.../unix/sysv/linux/tst-rseq-nptl-static.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq-static.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq.c | 24 ++-
sysdeps/unix/sysv/linux/tst-rseq.h | 9 +-
sysdeps/unix/sysv/linux/x86_64/64/ld.abilist | 1 +
.../unix/sysv/linux/x86_64/rseq-internal.h | 109 +++++++++++
sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist | 1 +
sysdeps/x86_64/nptl/tcb-access.h | 56 ++++++
61 files changed, 886 insertions(+), 56 deletions(-)
create mode 100644 csu/rseq-sizes.sym
create mode 100644 elf/dl-rseq-symbols.S
create mode 100644 sysdeps/generic/dl-rseq.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-disable-static.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-static.c
create mode 100644 sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
@ 2024-02-06 16:27 ` Michael Jeanson
2024-02-06 16:41 ` Mathieu Desnoyers
2024-02-16 2:02 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 2/8] Add rseq extensible ABI support Michael Jeanson
` (6 subsequent siblings)
7 siblings, 2 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson, Mathieu Desnoyers
While working on a patch to add support for the extensible rseq ABI, we
came across an issue where a new 'const' variable would be merged with
the existing '__rseq_size' variable. We tracked this to the use of
'-fmerge-all-constants' which allows the compiler to merge identical
constant variables. This means that all 'const' variables in a compile
unit that are of the same size and are initialized to the same value can
be merged.
In this specific case, on 32 bit systems 'unsigned int' and 'ptrdiff_t'
are both 4 bytes and initialized to 0 which should trigger the merge.
However for reasons we haven't delved into when the attribute 'section
(".data.rel.ro")' is added to the mix, only variables of the same exact
types are merged. As far as we know this behavior is not specified
anywhere and could change with a new compiler version, hence this patch.
Move the definitions of these variables into an assembler file and add
hidden writable aliases for internal use. This has the added bonus of
removing the asm workaround to set the values on rseq registration.
Tested on Debian 12 with GCC 12.2.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
csu/Makefile | 2 +-
csu/rseq-sizes.sym | 8 +++++
elf/Makefile | 1 +
elf/dl-rseq-symbols.S | 55 +++++++++++++++++++++++++++++++++++
sysdeps/nptl/dl-tls_init_tp.c | 14 ++++-----
5 files changed, 71 insertions(+), 9 deletions(-)
create mode 100644 csu/rseq-sizes.sym
create mode 100644 elf/dl-rseq-symbols.S
diff --git a/csu/Makefile b/csu/Makefile
index ac05ab24d5..0bf51a0e48 100644
--- a/csu/Makefile
+++ b/csu/Makefile
@@ -99,7 +99,7 @@ before-compile += $(objpfx)abi-tag.h
generated += abi-tag.h
# Put it here to generate it earlier.
-gen-as-const-headers += rtld-sizes.sym
+gen-as-const-headers += rtld-sizes.sym rseq-sizes.sym
# These are the special initializer/finalizer files. They are always the
# first and last file in the link. crti.o ... crtn.o define the global
diff --git a/csu/rseq-sizes.sym b/csu/rseq-sizes.sym
new file mode 100644
index 0000000000..c959758ff0
--- /dev/null
+++ b/csu/rseq-sizes.sym
@@ -0,0 +1,8 @@
+#include <stddef.h>
+
+--
+RSEQ_SIZE_SIZE sizeof (unsigned int)
+RSEQ_SIZE_ALIGN __alignof (unsigned int)
+
+RSEQ_OFFSET_SIZE sizeof (ptrdiff_t)
+RSEQ_OFFSET_ALIGN __alignof (ptrdiff_t)
diff --git a/elf/Makefile b/elf/Makefile
index 5d78b659ce..7d711aedf0 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -73,6 +73,7 @@ dl-routines = \
dl-origin \
dl-printf \
dl-reloc \
+ dl-rseq-symbols \
dl-runtime \
dl-scope \
dl-setup_hash \
diff --git a/elf/dl-rseq-symbols.S b/elf/dl-rseq-symbols.S
new file mode 100644
index 0000000000..2d8e88367f
--- /dev/null
+++ b/elf/dl-rseq-symbols.S
@@ -0,0 +1,55 @@
+/* Define symbols used by rseq.
+ Copyright (C) 2024 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ 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 <rseq-sizes.h>
+#include <sysdep.h>
+
+/* Some targets define a macro to denote the zero register. */
+#undef zero
+
+/* Define 2 symbols, __rseq_size is public const and _rseq_size, which is an
+ alias of __rseq_size, but hidden and writable for internal use. */
+
+ .globl __rseq_size
+ .type __rseq_size, %object
+ .size __rseq_size, RSEQ_SIZE_SIZE
+ .hidden _rseq_size
+ .globl _rseq_size
+ .type _rseq_size, %object
+ .size _rseq_size, RSEQ_SIZE_SIZE
+ .section .data.rel.ro
+ .balign RSEQ_SIZE_ALIGN
+__rseq_size:
+_rseq_size:
+ .zero RSEQ_SIZE_SIZE
+
+/* Define 2 symbols, __rseq_offset is public const and _rseq_offset, which is an
+ alias of __rseq_offset, but hidden and writable for internal use. */
+
+ .globl __rseq_offset
+ .type __rseq_offset, %object
+ .size __rseq_offset, RSEQ_OFFSET_SIZE
+ .hidden _rseq_offset
+ .globl _rseq_offset
+ .type _rseq_offset, %object
+ .size _rseq_offset, RSEQ_OFFSET_SIZE
+ .section .data.rel.ro
+ .balign RSEQ_OFFSET_ALIGN
+__rseq_offset:
+_rseq_offset:
+ .zero RSEQ_OFFSET_SIZE
diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
index 092c274f36..80eb0107b5 100644
--- a/sysdeps/nptl/dl-tls_init_tp.c
+++ b/sysdeps/nptl/dl-tls_init_tp.c
@@ -45,8 +45,10 @@ rtld_mutex_dummy (pthread_mutex_t *lock)
#endif
const unsigned int __rseq_flags;
-const unsigned int __rseq_size attribute_relro;
-const ptrdiff_t __rseq_offset attribute_relro;
+
+/* The variables are in .data.relro but are not yet write-protected. */
+extern unsigned int _rseq_size attribute_relro attribute_hidden;
+extern ptrdiff_t _rseq_offset attribute_relro attribute_hidden;
void
__tls_pre_init_tp (void)
@@ -105,10 +107,7 @@ __tls_init_tp (void)
do_rseq = TUNABLE_GET (rseq, int, NULL);
if (rseq_register_current_thread (pd, do_rseq))
{
- /* We need a writable view of the variables. They are in
- .data.relro and are not yet write-protected. */
- extern unsigned int size __asm__ ("__rseq_size");
- size = sizeof (pd->rseq_area);
+ _rseq_size = sizeof (pd->rseq_area);
}
#ifdef RSEQ_SIG
@@ -117,8 +116,7 @@ __tls_init_tp (void)
all targets support __thread_pointer, so set __rseq_offset only
if the rseq registration may have happened because RSEQ_SIG is
defined. */
- extern ptrdiff_t offset __asm__ ("__rseq_offset");
- offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
+ _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
#endif
}
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols Michael Jeanson
@ 2024-02-06 16:27 ` Michael Jeanson
2024-02-16 4:42 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol Michael Jeanson
` (5 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson, Mathieu Desnoyers
Introduced in Linux v6.3 the rseq extensible ABI [1] will allow adding
rseq features past the initial 32 bytes of the original ABI.
While the rseq features in the latest kernel still fit within the
original ABI size, there are currently only 4 bytes left. It would thus
be a good time to add support for the extensible ABI so that when new
features are added, they are immediately available to GNU libc users.
We use the ELF auxiliary vectors to query the kernel for the size and
alignment of the rseq area, if this fails we default to the original
fixed size and alignment of '32' which the kernel will accept as a
compatibility mode with the original ABI.
This makes the size of the rseq area variable and thus requires to
relocate it out of 'struct pthread'. We chose to move it after (in block
allocation order) the last TLS block since it required a fairly small
modification to the TLS block allocator and did not interfere with the
main executable TLS block which must always be first.
[1] https://lore.kernel.org/all/20221122203932.231377-4-mathieu.desnoyers@efficios.com/
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Co-Authored-By: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-By: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
Changes since RFC v1:
- Insert the rseq area after the last TLS block
- Add proper support for TLS_TCB_AT_TP variant
Changes since RFC v2:
- Set __rseq_size even when the registration fails
- Adjust rseq tests to the new ABI
- Added support for statically linked executables
Changes since RFC v3:
- Fix RSEQ_SETMEM for rseq disabled
- Replace sys/auxv.h usage with dl-parse_auxv.h
- Fix offset for TLS_TCB_AT_TP with statically linked executables
- Zero the rseq area before registration
Changes since RFC v4:
- Move dynamic linker defines to a header file
- Fix alignment when tls block align is smaller than rseq align with
statically linked executables
- Add statically linked rseq tests
- Revert: Set __rseq_size even when the registration fails
- Use minimum size when rseq is disabled by tunable
Changes since v5:
- Fix TLS_DTV_AT_TP rseq offset with statically linked executables
Changes since v6:
- Fix tst-rseq for feature size over 32 bytes
- Rebased on 'nptl: fix potential merge of __rseq_* relro symbols'
---
csu/libc-tls.c | 66 ++++++++++++++++---
elf/dl-tls.c | 62 +++++++++++++++++
elf/rtld_static_init.c | 12 ++++
nptl/descr.h | 20 +-----
nptl/pthread_create.c | 2 +-
sysdeps/generic/dl-rseq.h | 26 ++++++++
sysdeps/generic/ldsodefs.h | 12 ++++
sysdeps/i386/nptl/tcb-access.h | 56 ++++++++++++++++
sysdeps/nptl/dl-tls_init_tp.c | 4 +-
sysdeps/nptl/tcb-access.h | 5 ++
sysdeps/unix/sysv/linux/Makefile | 10 +++
sysdeps/unix/sysv/linux/dl-parse_auxv.h | 6 ++
sysdeps/unix/sysv/linux/rseq-internal.h | 29 ++++++--
sysdeps/unix/sysv/linux/sched_getcpu.c | 3 +-
.../unix/sysv/linux/tst-rseq-disable-static.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq-disable.c | 19 +++---
.../unix/sysv/linux/tst-rseq-nptl-static.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq-static.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq.c | 22 +++++--
sysdeps/unix/sysv/linux/tst-rseq.h | 9 ++-
sysdeps/x86_64/nptl/tcb-access.h | 56 ++++++++++++++++
21 files changed, 375 insertions(+), 47 deletions(-)
create mode 100644 sysdeps/generic/dl-rseq.h
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-disable-static.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-static.c
diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index b7682bdf43..9c70f53284 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -26,6 +26,8 @@
#include <array_length.h>
#include <pthreadP.h>
#include <dl-call_tls_init_tp.h>
+#include <dl-rseq.h>
+#include <elf/dl-tunables.h>
#ifdef SHARED
#error makefile bug, this file is for static only
@@ -62,6 +64,18 @@ size_t _dl_tls_static_surplus;
dynamic TLS access (e.g. with TLSDESC). */
size_t _dl_tls_static_optional;
+/* Size of the features present in the rseq area. */
+size_t _dl_tls_rseq_feature_size;
+
+/* Alignment requirement of the rseq area. */
+size_t _dl_tls_rseq_align;
+
+/* Size of the rseq area in the static TLS block. */
+size_t _dl_tls_rseq_size;
+
+/* Offset of the rseq area from the thread pointer. */
+ptrdiff_t _dl_tls_rseq_offset;
+
/* Generation counter for the dtv. */
size_t _dl_tls_generation;
@@ -110,6 +124,7 @@ __libc_setup_tls (void)
size_t filesz = 0;
void *initimage = NULL;
size_t align = 0;
+ size_t tls_blocks_size = 0;
size_t max_align = TCB_ALIGNMENT;
size_t tcb_offset;
const ElfW(Phdr) *phdr;
@@ -135,22 +150,61 @@ __libc_setup_tls (void)
/* Calculate the size of the static TLS surplus, with 0 auditors. */
_dl_tls_static_surplus_init (0);
+ /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
+ bytes when rseq is disabled by tunables. */
+ size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
+ size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
+ bool do_rseq = true;
+ do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
+ if (do_rseq)
+ {
+ rseq_align = GLRO(dl_tls_rseq_align);
+ /* Make sure the rseq area size is at least the minimum ABI size and a
+ multiple of the requested aligment. */
+ rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
+ TLS_DL_RSEQ_MIN_SIZE), rseq_align);
+ }
+
+ /* Increase the max_align if necessary. */
+ max_align = MAX (max_align, rseq_align);
+
+ /* Record the rseq_area block size. */
+ GLRO (dl_tls_rseq_size) = rseq_size;
+
/* We have to set up the TCB block which also (possibly) contains
'errno'. Therefore we avoid 'malloc' which might touch 'errno'.
Instead we use 'sbrk' which would only uses 'errno' if it fails.
In this case we are right away out of memory and the user gets
what she/he deserves. */
#if TLS_TCB_AT_TP
+ /* Before the the thread pointer, add the aligned tls block size and then
+ align the rseq area block on top. */
+ tls_blocks_size = roundup (roundup (memsz, align ?: 1) + rseq_size, rseq_align);
+
+ /* Record the rseq_area offset. The offset is negative with TLS_TCB_AT_TP
+ because the TLS blocks are located before the thread pointer. */
+ GLRO (dl_tls_rseq_offset) = -tls_blocks_size;
+
/* Align the TCB offset to the maximum alignment, as
_dl_allocate_tls_storage (in elf/dl-tls.c) does using __libc_memalign
and dl_tls_static_align. */
- tcb_offset = roundup (memsz + GLRO(dl_tls_static_surplus), max_align);
+ tcb_offset = roundup (tls_blocks_size + GLRO(dl_tls_static_surplus), max_align);
tlsblock = _dl_early_allocate (tcb_offset + TLS_INIT_TCB_SIZE + max_align);
if (tlsblock == NULL)
_startup_fatal_tls_error ();
#elif TLS_DTV_AT_TP
+ /* Align memsz on top of the initial tcb. */
tcb_offset = roundup (TLS_INIT_TCB_SIZE, align ?: 1);
- tlsblock = _dl_early_allocate (tcb_offset + memsz + max_align
+
+ /* After the thread pointer, add the initial tcb plus the tls block size and
+ then align the rseq area block on top. */
+ tls_blocks_size = roundup (tcb_offset + memsz + rseq_size, rseq_align);
+
+ /* Record the rseq_area offset. The offset is positive with TLS_DTV_AT_TP
+ because the TLS blocks are located after the thread pointer. */
+ GLRO (dl_tls_rseq_offset) = tls_blocks_size - rseq_size;
+
+ tlsblock = _dl_early_allocate (tls_blocks_size + max_align
+ TLS_PRE_TCB_SIZE
+ GLRO(dl_tls_static_surplus));
if (tlsblock == NULL)
@@ -209,11 +263,5 @@ __libc_setup_tls (void)
/* static_slotinfo.slotinfo[1].gen = 0; -- Already zero. */
static_slotinfo.slotinfo[1].map = main_map;
- memsz = roundup (memsz, align ?: 1);
-
-#if TLS_DTV_AT_TP
- memsz += tcb_offset;
-#endif
-
- init_static_tls (memsz, MAX (TCB_ALIGNMENT, max_align));
+ init_static_tls (tls_blocks_size, MAX (TCB_ALIGNMENT, max_align));
}
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 7b3dd9ab60..70fbe7095f 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -27,6 +27,7 @@
#include <tls.h>
#include <dl-tls.h>
+#include <dl-rseq.h>
#include <ldsodefs.h>
#if PTHREAD_IN_LIBC
@@ -298,6 +299,35 @@ _dl_determine_tlsoffset (void)
slotinfo[cnt].map->l_tls_offset = off;
}
+ /* Insert the rseq area block after the last TLS block. */
+
+ /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
+ bytes when rseq is disabled by tunables. */
+ size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
+ size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
+ bool do_rseq = true;
+ do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
+ if (do_rseq)
+ {
+ rseq_align = GLRO(dl_tls_rseq_align);
+ /* Make sure the rseq area size is at least the minimum ABI size and a
+ multiple of the requested aligment. */
+ rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
+ TLS_DL_RSEQ_MIN_SIZE), rseq_align);
+ }
+
+ /* Add the rseq area block to the global offset. */
+ offset = roundup (offset, rseq_align) + rseq_size;
+
+ /* Increase the max_align if necessary. */
+ max_align = MAX (max_align, rseq_align);
+
+ /* Record the rseq_area block size and offset. The offset is negative
+ with TLS_TCB_AT_TP because the TLS blocks are located before the
+ thread pointer. */
+ GLRO (dl_tls_rseq_offset) = -offset;
+ GLRO (dl_tls_rseq_size) = rseq_size;
+
GL(dl_tls_static_used) = offset;
GLRO (dl_tls_static_size) = (roundup (offset + GLRO(dl_tls_static_surplus),
max_align)
@@ -343,6 +373,38 @@ _dl_determine_tlsoffset (void)
offset = off + slotinfo[cnt].map->l_tls_blocksize - firstbyte;
}
+ /* Insert the rseq area block after the last TLS block. */
+
+ /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
+ bytes when rseq is disabled by tunables. */
+ size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
+ size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
+ bool do_rseq = true;
+ do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
+ if (do_rseq)
+ {
+ rseq_align = GLRO(dl_tls_rseq_align);
+ /* Make sure the rseq area size is at least the minimum ABI size and a
+ multiple of the requested aligment. */
+ rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
+ TLS_DL_RSEQ_MIN_SIZE), rseq_align);
+ }
+
+ /* Align the global offset to the beginning of the rseq area. */
+ offset = roundup (offset, rseq_align);
+
+ /* Record the rseq_area block size and offset. The offset is positive
+ with TLS_DTV_AT_TP because the TLS blocks are located after the
+ thread pointer. */
+ GLRO (dl_tls_rseq_size) = rseq_size;
+ GLRO (dl_tls_rseq_offset) = offset;
+
+ /* Add the rseq area block to the global offset. */
+ offset += rseq_size;
+
+ /* Increase the max_align if necessary. */
+ max_align = MAX (max_align, rseq_align);
+
GL(dl_tls_static_used) = offset;
GLRO (dl_tls_static_size) = roundup (offset + GLRO(dl_tls_static_surplus),
TCB_ALIGNMENT);
diff --git a/elf/rtld_static_init.c b/elf/rtld_static_init.c
index e918e4ebdf..0606317b8c 100644
--- a/elf/rtld_static_init.c
+++ b/elf/rtld_static_init.c
@@ -78,6 +78,18 @@ __rtld_static_init (struct link_map *map)
extern __typeof (dl->_dl_tls_static_size) _dl_tls_static_size
attribute_hidden;
dl->_dl_tls_static_size = _dl_tls_static_size;
+ extern __typeof (dl->_dl_tls_rseq_feature_size) _dl_tls_rseq_feature_size
+ attribute_hidden;
+ dl->_dl_tls_rseq_feature_size = _dl_tls_rseq_feature_size;
+ extern __typeof (dl->_dl_tls_rseq_align) _dl_tls_rseq_align
+ attribute_hidden;
+ dl->_dl_tls_rseq_align = _dl_tls_rseq_align;
+ extern __typeof (dl->_dl_tls_rseq_size) _dl_tls_rseq_size
+ attribute_hidden;
+ dl->_dl_tls_rseq_size = _dl_tls_rseq_size;
+ extern __typeof (dl->_dl_tls_rseq_offset) _dl_tls_rseq_offset
+ attribute_hidden;
+ dl->_dl_tls_rseq_offset = _dl_tls_rseq_offset;
dl->_dl_find_object = _dl_find_object;
__rtld_static_init_arch (map, dl);
diff --git a/nptl/descr.h b/nptl/descr.h
index 8cef95810c..cdc3c82d9a 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -404,25 +404,11 @@ struct pthread
/* Used on strsignal. */
struct tls_internal_t tls_state;
- /* rseq area registered with the kernel. Use a custom definition
- here to isolate from kernel struct rseq changes. The
- implementation of sched_getcpu needs acccess to the cpu_id field;
- the other fields are unused and not included here. */
- union
- {
- struct
- {
- uint32_t cpu_id_start;
- uint32_t cpu_id;
- };
- char pad[32]; /* Original rseq area size. */
- } rseq_area __attribute__ ((aligned (32)));
-
/* Amount of end padding, if any, in this structure.
- This definition relies on rseq_area being last. */
+ This definition relies on tls_state being last. */
#define PTHREAD_STRUCT_END_PADDING \
- (sizeof (struct pthread) - offsetof (struct pthread, rseq_area) \
- + sizeof ((struct pthread) {}.rseq_area))
+ (sizeof (struct pthread) - offsetof (struct pthread, tls_state) \
+ + sizeof ((struct pthread) {}.tls_state))
} __attribute ((aligned (TCB_ALIGNMENT)));
static inline bool
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 1d3665d5ed..9b49ee7121 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -691,7 +691,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
/* Inherit rseq registration state. Without seccomp filters, rseq
registration will either always fail or always succeed. */
- if ((int) THREAD_GETMEM_VOLATILE (self, rseq_area.cpu_id) >= 0)
+ if ((int) RSEQ_GETMEM_VOLATILE (rseq_get_area(), cpu_id) >= 0)
pd->flags |= ATTR_FLAG_DO_RSEQ;
/* Initialize the field for the ID of the thread which is waiting
diff --git a/sysdeps/generic/dl-rseq.h b/sysdeps/generic/dl-rseq.h
new file mode 100644
index 0000000000..0855981c89
--- /dev/null
+++ b/sysdeps/generic/dl-rseq.h
@@ -0,0 +1,26 @@
+/* RSEQ defines for the dynamic linker. Generic version.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ 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/>. */
+
+/* Minimum size of the rseq area. */
+#define TLS_DL_RSEQ_MIN_SIZE 32
+
+/* Minimum feature size of the rseq area. */
+#define TLS_DL_RSEQ_MIN_FEATURE_SIZE 20
+
+/* Minimum size of the rseq area alignment. */
+#define TLS_DL_RSEQ_MIN_ALIGN 32
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 117c901ccc..53a0a208d8 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -610,6 +610,18 @@ struct rtld_global_ro
See comments in elf/dl-tls.c where it is initialized. */
EXTERN size_t _dl_tls_static_surplus;
+ /* Size of the features present in the rseq area. */
+ EXTERN size_t _dl_tls_rseq_feature_size;
+
+ /* Alignment requirement of the rseq area. */
+ EXTERN size_t _dl_tls_rseq_align;
+
+ /* Size of the rseq area in the static TLS block. */
+ EXTERN size_t _dl_tls_rseq_size;
+
+ /* Offset of the rseq area from the thread pointer. */
+ EXTERN ptrdiff_t _dl_tls_rseq_offset;
+
/* Name of the shared object to be profiled (if any). */
EXTERN const char *_dl_profile;
/* Filename of the output file. */
diff --git a/sysdeps/i386/nptl/tcb-access.h b/sysdeps/i386/nptl/tcb-access.h
index 4b6221e103..e6988186d0 100644
--- a/sysdeps/i386/nptl/tcb-access.h
+++ b/sysdeps/i386/nptl/tcb-access.h
@@ -123,3 +123,59 @@
"i" (offsetof (struct pthread, member)), \
"r" (idx)); \
}})
+
+
+/* Read member of the RSEQ area directly. */
+#define RSEQ_GETMEM_VOLATILE(descr, member) \
+ ({ __typeof (descr->member) __value; \
+ ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
+ _Static_assert (sizeof (__value) == 1 \
+ || sizeof (__value) == 4 \
+ || sizeof (__value) == 8, \
+ "size of per-thread data"); \
+ if (sizeof (__value) == 1) \
+ asm volatile ("movb %%gs:%P2(%3),%b0" \
+ : "=q" (__value) \
+ : "0" (0), "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else if (sizeof (__value) == 4) \
+ asm volatile ("movl %%gs:%P1(%2),%0" \
+ : "=r" (__value) \
+ : "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else /* 8 */ \
+ { \
+ asm volatile ("movl %%gs:%P1(%2),%%eax\n\t" \
+ "movl %%gs:4+%P1(%2),%%edx" \
+ : "=&A" (__value) \
+ : "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ } \
+ __value; })
+
+/* Set member of the RSEQ area directly. */
+#define RSEQ_SETMEM(descr, member, value) \
+ ({ \
+ ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
+ _Static_assert (sizeof (descr->member) == 1 \
+ || sizeof (descr->member) == 4 \
+ || sizeof (descr->member) == 8, \
+ "size of per-thread data"); \
+ if (sizeof (descr->member) == 1) \
+ asm volatile ("movb %b0,%%gs:%P1(%2)" : \
+ : "iq" (value), \
+ "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else if (sizeof (descr->member) == 4) \
+ asm volatile ("movl %0,%%gs:%P1(%2)" : \
+ : "ir" (value), \
+ "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else /* 8 */ \
+ { \
+ asm volatile ("movl %%eax,%%gs:%P1(%2)\n\t" \
+ "movl %%edx,%%gs:4+%P1(%2)" : \
+ : "A" ((uint64_t) cast_to_integer (value)), \
+ "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ }})
diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
index 80eb0107b5..7aa15558e6 100644
--- a/sysdeps/nptl/dl-tls_init_tp.c
+++ b/sysdeps/nptl/dl-tls_init_tp.c
@@ -107,7 +107,7 @@ __tls_init_tp (void)
do_rseq = TUNABLE_GET (rseq, int, NULL);
if (rseq_register_current_thread (pd, do_rseq))
{
- _rseq_size = sizeof (pd->rseq_area);
+ _rseq_size = GLRO (dl_tls_rseq_size);
}
#ifdef RSEQ_SIG
@@ -116,7 +116,7 @@ __tls_init_tp (void)
all targets support __thread_pointer, so set __rseq_offset only
if the rseq registration may have happened because RSEQ_SIG is
defined. */
- _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
+ _rseq_offset = GLRO (dl_tls_rseq_offset);
#endif
}
diff --git a/sysdeps/nptl/tcb-access.h b/sysdeps/nptl/tcb-access.h
index 600433766f..9532f30022 100644
--- a/sysdeps/nptl/tcb-access.h
+++ b/sysdeps/nptl/tcb-access.h
@@ -30,3 +30,8 @@
descr->member = (value)
#define THREAD_SETMEM_NC(descr, member, idx, value) \
descr->member[idx] = (value)
+
+#define RSEQ_GETMEM_VOLATILE(descr, member) \
+ THREAD_GETMEM_VOLATILE(descr, member)
+#define RSEQ_SETMEM(descr, member, value) \
+ THREAD_SETMEM(descr, member, value)
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 415aa1f14d..6bcf81461b 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -261,6 +261,11 @@ tests-internal += \
tst-rseq-disable \
# tests-internal
+tests-static += \
+ tst-rseq-disable-static \
+ tst-rseq-static \
+ # tests-static
+
tests-time64 += \
tst-adjtimex-time64 \
tst-clock_adjtime-time64 \
@@ -394,6 +399,7 @@ $(objpfx)tst-mount-compile.out: ../sysdeps/unix/sysv/linux/tst-mount-compile.py
$(objpfx)tst-mount-compile.out: $(sysdeps-linux-python-deps)
tst-rseq-disable-ENV = GLIBC_TUNABLES=glibc.pthread.rseq=0
+tst-rseq-disable-static-ENV = GLIBC_TUNABLES=glibc.pthread.rseq=0
endif # $(subdir) == misc
@@ -655,4 +661,8 @@ tests += \
tests-internal += \
tst-rseq-nptl \
# tests-internal
+
+tests-static += \
+ tst-rseq-nptl-static \
+ # tests-static
endif
diff --git a/sysdeps/unix/sysv/linux/dl-parse_auxv.h b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
index e3d758b163..fbc90fc786 100644
--- a/sysdeps/unix/sysv/linux/dl-parse_auxv.h
+++ b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
@@ -21,6 +21,7 @@
#include <fpu_control.h>
#include <ldsodefs.h>
#include <link.h>
+#include <dl-rseq.h>
typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];
@@ -57,5 +58,10 @@ void _dl_parse_auxv (ElfW(auxv_t) *av, dl_parse_auxv_t auxv_values)
GLRO(dl_sysinfo) = auxv_values[AT_SYSINFO];
#endif
+ GLRO(dl_tls_rseq_feature_size) = MAX (auxv_values[AT_RSEQ_FEATURE_SIZE],
+ TLS_DL_RSEQ_MIN_FEATURE_SIZE);
+ GLRO(dl_tls_rseq_align) = MAX (auxv_values[AT_RSEQ_ALIGN],
+ TLS_DL_RSEQ_MIN_ALIGN);
+
DL_PLATFORM_AUXV
}
diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
index 48eebc1e16..b6c9deea6b 100644
--- a/sysdeps/unix/sysv/linux/rseq-internal.h
+++ b/sysdeps/unix/sysv/linux/rseq-internal.h
@@ -24,6 +24,24 @@
#include <stdbool.h>
#include <stdio.h>
#include <sys/rseq.h>
+#include <thread_pointer.h>
+#include <ldsodefs.h>
+
+/* rseq area registered with the kernel. Use a custom definition
+ here to isolate from kernel struct rseq changes. The
+ implementation of sched_getcpu needs acccess to the cpu_id field;
+ the other fields are unused and not included here. */
+struct rseq_area
+{
+ uint32_t cpu_id_start;
+ uint32_t cpu_id;
+};
+
+static inline struct rseq_area *
+rseq_get_area(void)
+{
+ return (struct rseq_area *) ((char *) __thread_pointer() + GLRO (dl_tls_rseq_offset));
+}
#ifdef RSEQ_SIG
static inline bool
@@ -31,20 +49,23 @@ rseq_register_current_thread (struct pthread *self, bool do_rseq)
{
if (do_rseq)
{
- int ret = INTERNAL_SYSCALL_CALL (rseq, &self->rseq_area,
- sizeof (self->rseq_area),
+ /* The kernel expects 'rseq_area->rseq_cs == NULL' on registration, zero
+ the whole rseq area. */
+ memset(rseq_get_area(), 0, GLRO (dl_tls_rseq_size));
+ int ret = INTERNAL_SYSCALL_CALL (rseq, rseq_get_area(),
+ GLRO (dl_tls_rseq_size),
0, RSEQ_SIG);
if (!INTERNAL_SYSCALL_ERROR_P (ret))
return true;
}
- THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
+ RSEQ_SETMEM (rseq_get_area(), cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
return false;
}
#else /* RSEQ_SIG */
static inline bool
rseq_register_current_thread (struct pthread *self, bool do_rseq)
{
- THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
+ RSEQ_SETMEM (rseq_get_area(), cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
return false;
}
#endif /* RSEQ_SIG */
diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
index dfb884568d..4f7cd499d3 100644
--- a/sysdeps/unix/sysv/linux/sched_getcpu.c
+++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
@@ -19,6 +19,7 @@
#include <sched.h>
#include <sysdep.h>
#include <sysdep-vdso.h>
+#include <rseq-internal.h>
static int
vsyscall_sched_getcpu (void)
@@ -37,7 +38,7 @@ vsyscall_sched_getcpu (void)
int
sched_getcpu (void)
{
- int cpu_id = THREAD_GETMEM_VOLATILE (THREAD_SELF, rseq_area.cpu_id);
+ int cpu_id = RSEQ_GETMEM_VOLATILE (rseq_get_area(), cpu_id);
return __glibc_likely (cpu_id >= 0) ? cpu_id : vsyscall_sched_getcpu ();
}
#else /* RSEQ_SIG */
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable-static.c b/sysdeps/unix/sysv/linux/tst-rseq-disable-static.c
new file mode 100644
index 0000000000..2687d13d3d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq-disable-static.c
@@ -0,0 +1 @@
+#include "tst-rseq-disable.c"
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable.c b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
index bbc655bec4..ae4143916b 100644
--- a/sysdeps/unix/sysv/linux/tst-rseq-disable.c
+++ b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
@@ -26,27 +26,30 @@
#include <unistd.h>
#ifdef RSEQ_SIG
+# include <sys/auxv.h>
+# include "tst-rseq.h"
+
+static __thread struct rseq local_rseq;
/* Check that rseq can be registered and has not been taken by glibc. */
static void
check_rseq_disabled (void)
{
- struct pthread *pd = THREAD_SELF;
+ struct rseq *rseq_area = (struct rseq *) ((char *) __thread_pointer () + __rseq_offset);
TEST_COMPARE (__rseq_flags, 0);
- TEST_VERIFY ((char *) __thread_pointer () + __rseq_offset
- == (char *) &pd->rseq_area);
TEST_COMPARE (__rseq_size, 0);
- TEST_COMPARE ((int) pd->rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
+ TEST_COMPARE ((int) rseq_area->cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
+
+ TEST_COMPARE (sizeof (local_rseq), RSEQ_TEST_MIN_SIZE);
- int ret = syscall (__NR_rseq, &pd->rseq_area, sizeof (pd->rseq_area),
- 0, RSEQ_SIG);
+ int ret = syscall (__NR_rseq, &local_rseq, RSEQ_TEST_MIN_SIZE, 0, RSEQ_SIG);
if (ret == 0)
{
- ret = syscall (__NR_rseq, &pd->rseq_area, sizeof (pd->rseq_area),
+ ret = syscall (__NR_rseq, &local_rseq, RSEQ_TEST_MIN_SIZE,
RSEQ_FLAG_UNREGISTER, RSEQ_SIG);
TEST_COMPARE (ret, 0);
- pd->rseq_area.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
+ rseq_area->cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
}
else
{
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c b/sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c
new file mode 100644
index 0000000000..6e2c923bb9
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c
@@ -0,0 +1 @@
+#include "tst-rseq-nptl.c"
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-static.c b/sysdeps/unix/sysv/linux/tst-rseq-static.c
new file mode 100644
index 0000000000..1d97f3bd3d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq-static.c
@@ -0,0 +1 @@
+#include "tst-rseq.c"
diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
index 2c90409ba0..c8c0518a5d 100644
--- a/sysdeps/unix/sysv/linux/tst-rseq.c
+++ b/sysdeps/unix/sysv/linux/tst-rseq.c
@@ -31,18 +31,32 @@
# include <syscall.h>
# include <thread_pointer.h>
# include <tls.h>
+# include <sys/auxv.h>
# include "tst-rseq.h"
static void
do_rseq_main_test (void)
{
- struct pthread *pd = THREAD_SELF;
+ size_t rseq_align = MAX (getauxval (AT_RSEQ_ALIGN), RSEQ_TEST_MIN_ALIGN);
+ size_t rseq_size = roundup (MAX (getauxval (AT_RSEQ_FEATURE_SIZE), RSEQ_TEST_MIN_SIZE), rseq_align);
+ struct rseq *rseq = __thread_pointer () + __rseq_offset;
TEST_VERIFY_EXIT (rseq_thread_registered ());
TEST_COMPARE (__rseq_flags, 0);
- TEST_VERIFY ((char *) __thread_pointer () + __rseq_offset
- == (char *) &pd->rseq_area);
- TEST_COMPARE (__rseq_size, sizeof (pd->rseq_area));
+ TEST_COMPARE (__rseq_size, rseq_size);
+ /* The size of the rseq area must be a multiple of the alignment. */
+ TEST_VERIFY ((__rseq_size % rseq_align) == 0);
+ /* The rseq area address must be aligned. */
+ TEST_VERIFY (((unsigned long) rseq % rseq_align) == 0);
+#if TLS_TCB_AT_TP
+ /* The rseq area block should come before the thread pointer and be at least 32 bytes. */
+ TEST_VERIFY (__rseq_offset <= RSEQ_TEST_MIN_SIZE);
+#elif TLS_DTV_AT_TP
+ /* The rseq area block should come after the thread pointer. */
+ TEST_VERIFY (__rseq_offset >= 0);
+#else
+# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
+#endif
}
static void
diff --git a/sysdeps/unix/sysv/linux/tst-rseq.h b/sysdeps/unix/sysv/linux/tst-rseq.h
index dc603327d3..4931aa3d14 100644
--- a/sysdeps/unix/sysv/linux/tst-rseq.h
+++ b/sysdeps/unix/sysv/linux/tst-rseq.h
@@ -23,11 +23,18 @@
#include <syscall.h>
#include <sys/rseq.h>
#include <tls.h>
+#include <rseq-internal.h>
+
+#define RSEQ_TEST_MIN_SIZE 32
+#define RSEQ_TEST_MIN_FEATURE_SIZE 20
+#define RSEQ_TEST_MIN_ALIGN 32
static inline bool
rseq_thread_registered (void)
{
- return THREAD_GETMEM_VOLATILE (THREAD_SELF, rseq_area.cpu_id) >= 0;
+ struct rseq_area *rseq = (struct rseq_area *) ((char *) __thread_pointer () + __rseq_offset);
+
+ return __atomic_load_n (&rseq->cpu_id, __ATOMIC_RELAXED) >= 0;
}
static inline int
diff --git a/sysdeps/x86_64/nptl/tcb-access.h b/sysdeps/x86_64/nptl/tcb-access.h
index d35948f111..75ba4b3ce9 100644
--- a/sysdeps/x86_64/nptl/tcb-access.h
+++ b/sysdeps/x86_64/nptl/tcb-access.h
@@ -130,3 +130,59 @@
"i" (offsetof (struct pthread, member[0])), \
"r" (idx)); \
}})
+
+/* Read member of the RSEQ area directly. */
+# define RSEQ_GETMEM_VOLATILE(descr, member) \
+ ({ __typeof (descr->member) __value; \
+ ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
+ _Static_assert (sizeof (__value) == 1 \
+ || sizeof (__value) == 4 \
+ || sizeof (__value) == 8, \
+ "size of per-thread data"); \
+ if (sizeof (__value) == 1) \
+ asm volatile ("movb %%fs:%P2(%q3),%b0" \
+ : "=q" (__value) \
+ : "0" (0), "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else if (sizeof (__value) == 4) \
+ asm volatile ("movl %%fs:%P1(%q2),%0" \
+ : "=r" (__value) \
+ : "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else /* 8 */ \
+ { \
+ asm volatile ("movq %%fs:%P1(%q2),%q0" \
+ : "=r" (__value) \
+ : "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ } \
+ __value; })
+
+/* Set member of the RSEQ area directly. */
+# define RSEQ_SETMEM(descr, member, value) \
+ ({ \
+ ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
+ _Static_assert (sizeof (descr->member) == 1 \
+ || sizeof (descr->member) == 4 \
+ || sizeof (descr->member) == 8, \
+ "size of per-thread data"); \
+ if (sizeof (descr->member) == 1) \
+ asm volatile ("movb %b0,%%fs:%P1(%q2)" : \
+ : "iq" (value), \
+ "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else if (sizeof (descr->member) == 4) \
+ asm volatile ("movl %0,%%fs:%P1(%q2)" : \
+ : IMM_MODE (value), \
+ "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ else /* 8 */ \
+ { \
+ /* Since movq takes a signed 32-bit immediate or a register source \
+ operand, use "er" constraint for 32-bit signed integer constant \
+ or register. */ \
+ asm volatile ("movq %q0,%%fs:%P1(%q2)" : \
+ : "er" ((uint64_t) cast_to_integer (value)), \
+ "i" (offsetof (struct rseq_area, member)), \
+ "r" (_rseq_offset)); \
+ }})
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 2/8] Add rseq extensible ABI support Michael Jeanson
@ 2024-02-06 16:27 ` Michael Jeanson
2024-02-16 22:07 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 4/8] nptl: Add features to internal 'struct rseq_area' Michael Jeanson
` (4 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson
Exposing this symbol allows applications wishing to use rseq features
which are part of the extensible rseq ABI like 'node_id' and 'mm_cid' to
test the two following conditions in a single load / conditional branch:
- rseq is registered
- the specific rseq feature is available
This is useful as rseq is expected to be used in hot paths.
This variable is either zero (if restartable sequence registration
failed or has been disabled) or the size of the available restartable
sequence features.
Changes since v7:
* Fix sorting of symbols in abilist files
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
csu/rseq-sizes.sym | 3 +++
elf/dl-rseq-symbols.S | 17 +++++++++++++++++
manual/threads.texi | 8 ++++++++
sysdeps/nptl/dl-tls_init_tp.c | 2 ++
sysdeps/unix/sysv/linux/Versions | 3 +++
sysdeps/unix/sysv/linux/aarch64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/alpha/ld.abilist | 1 +
sysdeps/unix/sysv/linux/arc/ld.abilist | 1 +
sysdeps/unix/sysv/linux/arm/be/ld.abilist | 1 +
sysdeps/unix/sysv/linux/arm/le/ld.abilist | 1 +
sysdeps/unix/sysv/linux/csky/ld.abilist | 1 +
sysdeps/unix/sysv/linux/hppa/ld.abilist | 1 +
sysdeps/unix/sysv/linux/i386/ld.abilist | 1 +
.../unix/sysv/linux/loongarch/lp64/ld.abilist | 1 +
.../unix/sysv/linux/m68k/coldfire/ld.abilist | 1 +
sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist | 1 +
sysdeps/unix/sysv/linux/microblaze/ld.abilist | 1 +
sysdeps/unix/sysv/linux/mips/mips32/ld.abilist | 1 +
.../unix/sysv/linux/mips/mips64/n32/ld.abilist | 1 +
.../unix/sysv/linux/mips/mips64/n64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/nios2/ld.abilist | 1 +
sysdeps/unix/sysv/linux/or1k/ld.abilist | 1 +
.../sysv/linux/powerpc/powerpc32/ld.abilist | 1 +
.../sysv/linux/powerpc/powerpc64/be/ld.abilist | 1 +
.../sysv/linux/powerpc/powerpc64/le/ld.abilist | 1 +
sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist | 1 +
sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist | 1 +
sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/sh/be/ld.abilist | 1 +
sysdeps/unix/sysv/linux/sh/le/ld.abilist | 1 +
.../unix/sysv/linux/sparc/sparc32/ld.abilist | 1 +
.../unix/sysv/linux/sparc/sparc64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/sys/rseq.h | 4 ++++
sysdeps/unix/sysv/linux/tst-rseq-disable.c | 1 +
sysdeps/unix/sysv/linux/tst-rseq.c | 4 +++-
sysdeps/unix/sysv/linux/x86_64/64/ld.abilist | 1 +
sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist | 1 +
38 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/csu/rseq-sizes.sym b/csu/rseq-sizes.sym
index c959758ff0..dde53bfa60 100644
--- a/csu/rseq-sizes.sym
+++ b/csu/rseq-sizes.sym
@@ -4,5 +4,8 @@
RSEQ_SIZE_SIZE sizeof (unsigned int)
RSEQ_SIZE_ALIGN __alignof (unsigned int)
+RSEQ_FEATURE_SIZE_SIZE sizeof (unsigned int)
+RSEQ_FEATURE_SIZE_ALIGN __alignof (unsigned int)
+
RSEQ_OFFSET_SIZE sizeof (ptrdiff_t)
RSEQ_OFFSET_ALIGN __alignof (ptrdiff_t)
diff --git a/elf/dl-rseq-symbols.S b/elf/dl-rseq-symbols.S
index 2d8e88367f..709188ae22 100644
--- a/elf/dl-rseq-symbols.S
+++ b/elf/dl-rseq-symbols.S
@@ -38,6 +38,23 @@ __rseq_size:
_rseq_size:
.zero RSEQ_SIZE_SIZE
+/* Define 2 symbols, __rseq_feature_size is public const and
+ _rseq_feature_size, which is an alias of __rseq_feature_size, but hidden and
+ writable for internal use. */
+
+ .globl __rseq_feature_size
+ .type __rseq_feature_size, %object
+ .size __rseq_feature_size, RSEQ_FEATURE_SIZE_SIZE
+ .hidden _rseq_feature_size
+ .globl _rseq_feature_size
+ .type _rseq_feature_size, %object
+ .size _rseq_feature_size, RSEQ_FEATURE_SIZE_SIZE
+ .section .data.rel.ro
+ .balign RSEQ_FEATURE_SIZE_ALIGN
+__rseq_feature_size:
+_rseq_feature_size:
+ .zero RSEQ_FEATURE_SIZE_SIZE
+
/* Define 2 symbols, __rseq_offset is public const and _rseq_offset, which is an
alias of __rseq_offset, but hidden and writable for internal use. */
diff --git a/manual/threads.texi b/manual/threads.texi
index e5544ff3da..2c9aaa9fb5 100644
--- a/manual/threads.texi
+++ b/manual/threads.texi
@@ -1011,6 +1011,14 @@ registration is successful, @code{__rseq_size} is at least 32 (the
initial size of @code{struct rseq}).
@end deftypevar
+@deftypevar {unsigned int} __rseq_feature_size
+@standards{Linux, sys/rseq.h}
+This variable is either zero (if restartable sequence registration
+failed or has been disabled) or the size of the restartable sequence
+features. If registration is successful, @code{__rseq_feature_size}
+is at least 20 (the initial feature size of @code{struct rseq}).
+@end deftypevar
+
@deftypevar {unsigned int} __rseq_flags
@standards{Linux, sys/rseq.h}
The flags used during restartable sequence registration with the kernel.
diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
index 7aa15558e6..8f19c22a74 100644
--- a/sysdeps/nptl/dl-tls_init_tp.c
+++ b/sysdeps/nptl/dl-tls_init_tp.c
@@ -48,6 +48,7 @@ const unsigned int __rseq_flags;
/* The variables are in .data.relro but are not yet write-protected. */
extern unsigned int _rseq_size attribute_relro attribute_hidden;
+extern unsigned int _rseq_feature_size attribute_relro attribute_hidden;
extern ptrdiff_t _rseq_offset attribute_relro attribute_hidden;
void
@@ -108,6 +109,7 @@ __tls_init_tp (void)
if (rseq_register_current_thread (pd, do_rseq))
{
_rseq_size = GLRO (dl_tls_rseq_size);
+ _rseq_feature_size = GLRO (dl_tls_rseq_feature_size);
}
#ifdef RSEQ_SIG
diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
index 268ba1b6ac..055be26dde 100644
--- a/sysdeps/unix/sysv/linux/Versions
+++ b/sysdeps/unix/sysv/linux/Versions
@@ -356,6 +356,9 @@ ld {
__rseq_offset;
__rseq_size;
}
+ GLIBC_2.40 {
+ __rseq_feature_size;
+ }
GLIBC_PRIVATE {
__nptl_change_stack_perm;
}
diff --git a/sysdeps/unix/sysv/linux/aarch64/ld.abilist b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
index 5151c0781d..93039b756d 100644
--- a/sysdeps/unix/sysv/linux/aarch64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/alpha/ld.abilist b/sysdeps/unix/sysv/linux/alpha/ld.abilist
index 3e296c5473..fc67e31293 100644
--- a/sysdeps/unix/sysv/linux/alpha/ld.abilist
+++ b/sysdeps/unix/sysv/linux/alpha/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x8
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/arc/ld.abilist b/sysdeps/unix/sysv/linux/arc/ld.abilist
index 55f0c2ab9c..31156b6ee1 100644
--- a/sysdeps/unix/sysv/linux/arc/ld.abilist
+++ b/sysdeps/unix/sysv/linux/arc/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/arm/be/ld.abilist b/sysdeps/unix/sysv/linux/arm/be/ld.abilist
index f1da2c636d..3d16fa60db 100644
--- a/sysdeps/unix/sysv/linux/arm/be/ld.abilist
+++ b/sysdeps/unix/sysv/linux/arm/be/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.4 __stack_chk_guard D 0x4
GLIBC_2.4 __tls_get_addr F
GLIBC_2.4 _dl_mcount F
GLIBC_2.4 _r_debug D 0x14
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/arm/le/ld.abilist b/sysdeps/unix/sysv/linux/arm/le/ld.abilist
index f1da2c636d..3d16fa60db 100644
--- a/sysdeps/unix/sysv/linux/arm/le/ld.abilist
+++ b/sysdeps/unix/sysv/linux/arm/le/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.4 __stack_chk_guard D 0x4
GLIBC_2.4 __tls_get_addr F
GLIBC_2.4 _dl_mcount F
GLIBC_2.4 _r_debug D 0x14
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/csky/ld.abilist b/sysdeps/unix/sysv/linux/csky/ld.abilist
index 7f482276ed..088f000c4e 100644
--- a/sysdeps/unix/sysv/linux/csky/ld.abilist
+++ b/sysdeps/unix/sysv/linux/csky/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/hppa/ld.abilist b/sysdeps/unix/sysv/linux/hppa/ld.abilist
index 7f5527fb30..8ae26c46c1 100644
--- a/sysdeps/unix/sysv/linux/hppa/ld.abilist
+++ b/sysdeps/unix/sysv/linux/hppa/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/i386/ld.abilist b/sysdeps/unix/sysv/linux/i386/ld.abilist
index 9c4a45d8dc..d9761c34e3 100644
--- a/sysdeps/unix/sysv/linux/i386/ld.abilist
+++ b/sysdeps/unix/sysv/linux/i386/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist b/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist
index 93fcd64eee..f5dbb17ec9 100644
--- a/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.36 __stack_chk_guard D 0x8
GLIBC_2.36 __tls_get_addr F
GLIBC_2.36 _dl_mcount F
GLIBC_2.36 _r_debug D 0x28
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist
index f1da2c636d..3d16fa60db 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.4 __stack_chk_guard D 0x4
GLIBC_2.4 __tls_get_addr F
GLIBC_2.4 _dl_mcount F
GLIBC_2.4 _r_debug D 0x14
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist
index dadbf852d0..3888e39812 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/microblaze/ld.abilist b/sysdeps/unix/sysv/linux/microblaze/ld.abilist
index 89a0b7e4fd..c83b62dce7 100644
--- a/sysdeps/unix/sysv/linux/microblaze/ld.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist b/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist
index e304d1bb46..9710fdb941 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist
index e304d1bb46..9710fdb941 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist
index f26e594a13..ec1bdfd965 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x8
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/nios2/ld.abilist b/sysdeps/unix/sysv/linux/nios2/ld.abilist
index 811ae9da2f..d2b742ec25 100644
--- a/sysdeps/unix/sysv/linux/nios2/ld.abilist
+++ b/sysdeps/unix/sysv/linux/nios2/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/or1k/ld.abilist b/sysdeps/unix/sysv/linux/or1k/ld.abilist
index cff2ffd23b..eb225ca4c5 100644
--- a/sysdeps/unix/sysv/linux/or1k/ld.abilist
+++ b/sysdeps/unix/sysv/linux/or1k/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.35 __stack_chk_guard D 0x4
GLIBC_2.35 __tls_get_addr F
GLIBC_2.35 _dl_mcount F
GLIBC_2.35 _r_debug D 0x14
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
index b1073f0942..7a94751723 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
@@ -9,3 +9,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.39 __parse_hwcap_3_4_and_convert_at_platform F
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist
index 40942a2cc6..f6bdd89083 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist
@@ -9,3 +9,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.39 __parse_hwcap_3_4_and_convert_at_platform F
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist
index 01f2694a4d..672bade9ba 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist
@@ -9,3 +9,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.39 __parse_hwcap_3_4_and_convert_at_platform F
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist
index 068368878e..d3be9236c2 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist
index a7758a0e52..46b22238ce 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist
index c15288394a..f438808c08 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist
index 78d071600b..f5fbc6de43 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/sh/be/ld.abilist b/sysdeps/unix/sysv/linux/sh/be/ld.abilist
index 7f5527fb30..8ae26c46c1 100644
--- a/sysdeps/unix/sysv/linux/sh/be/ld.abilist
+++ b/sysdeps/unix/sysv/linux/sh/be/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/sh/le/ld.abilist b/sysdeps/unix/sysv/linux/sh/le/ld.abilist
index 7f5527fb30..8ae26c46c1 100644
--- a/sysdeps/unix/sysv/linux/sh/le/ld.abilist
+++ b/sysdeps/unix/sysv/linux/sh/le/ld.abilist
@@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
GLIBC_2.4 __stack_chk_guard D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist
index 3aac73f3df..df3f7fefb3 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist
index 5471b24d59..3bf61e1210 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h b/sysdeps/unix/sysv/linux/sys/rseq.h
index b8afab8945..a048ba8958 100644
--- a/sysdeps/unix/sysv/linux/sys/rseq.h
+++ b/sysdeps/unix/sysv/linux/sys/rseq.h
@@ -163,6 +163,10 @@ extern const ptrdiff_t __rseq_offset;
unsuccessful. */
extern const unsigned int __rseq_size;
+/* Size of the registered rseq features. 0 if the registration was
+ unsuccessful. */
+extern const unsigned int __rseq_feature_size;
+
/* Flags used during rseq registration. */
extern const unsigned int __rseq_flags;
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable.c b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
index ae4143916b..eb4b6278dc 100644
--- a/sysdeps/unix/sysv/linux/tst-rseq-disable.c
+++ b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
@@ -39,6 +39,7 @@ check_rseq_disabled (void)
TEST_COMPARE (__rseq_flags, 0);
TEST_COMPARE (__rseq_size, 0);
+ TEST_COMPARE (__rseq_feature_size, 0);
TEST_COMPARE ((int) rseq_area->cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
TEST_COMPARE (sizeof (local_rseq), RSEQ_TEST_MIN_SIZE);
diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
index c8c0518a5d..c5d9afbb0a 100644
--- a/sysdeps/unix/sysv/linux/tst-rseq.c
+++ b/sysdeps/unix/sysv/linux/tst-rseq.c
@@ -38,12 +38,14 @@ static void
do_rseq_main_test (void)
{
size_t rseq_align = MAX (getauxval (AT_RSEQ_ALIGN), RSEQ_TEST_MIN_ALIGN);
- size_t rseq_size = roundup (MAX (getauxval (AT_RSEQ_FEATURE_SIZE), RSEQ_TEST_MIN_SIZE), rseq_align);
+ size_t rseq_feature_size = MAX (getauxval (AT_RSEQ_FEATURE_SIZE), RSEQ_TEST_MIN_FEATURE_SIZE);
+ size_t rseq_size = roundup (MAX (rseq_feature_size, RSEQ_TEST_MIN_SIZE), rseq_align);
struct rseq *rseq = __thread_pointer () + __rseq_offset;
TEST_VERIFY_EXIT (rseq_thread_registered ());
TEST_COMPARE (__rseq_flags, 0);
TEST_COMPARE (__rseq_size, rseq_size);
+ TEST_COMPARE (__rseq_feature_size, rseq_feature_size);
/* The size of the rseq area must be a multiple of the alignment. */
TEST_VERIFY ((__rseq_size % rseq_align) == 0);
/* The rseq area address must be aligned. */
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
index 5a8bd322cd..25c2153c21 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x8
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
index e17496d124..05327004e2 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
@@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
GLIBC_2.35 __rseq_flags D 0x4
GLIBC_2.35 __rseq_offset D 0x4
GLIBC_2.35 __rseq_size D 0x4
+GLIBC_2.40 __rseq_feature_size D 0x4
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 4/8] nptl: Add features to internal 'struct rseq_area'
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
` (2 preceding siblings ...)
2024-02-06 16:27 ` [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol Michael Jeanson
@ 2024-02-06 16:27 ` Michael Jeanson
2024-02-17 2:04 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 5/8] nptl: Add rseq internal utils Michael Jeanson
` (3 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson
The 'struct rseq_area' is used to define the offset of the various
features available in the rseq ABI. Add fields that follow cpu_id
(rseq_cs, flags, node_id, mm_cid) in preparation for their use.
Access to features following the original rseq ABI 20 bytes (after
'flags') starting with 'node_id' must be gated by an rseq feature size
test.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
sysdeps/unix/sysv/linux/rseq-internal.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
index b6c9deea6b..a63e4afbdb 100644
--- a/sysdeps/unix/sysv/linux/rseq-internal.h
+++ b/sysdeps/unix/sysv/linux/rseq-internal.h
@@ -28,13 +28,17 @@
#include <ldsodefs.h>
/* rseq area registered with the kernel. Use a custom definition
- here to isolate from kernel struct rseq changes. The
- implementation of sched_getcpu needs acccess to the cpu_id field;
- the other fields are unused and not included here. */
+ here to isolate from kernel struct rseq changes. Access to fields
+ beyond the 20 bytes of the original ABI (after 'flags') must be gated
+ by a check of the feature size. */
struct rseq_area
{
uint32_t cpu_id_start;
uint32_t cpu_id;
+ uint64_t rseq_cs;
+ uint32_t flags;
+ uint32_t node_id;
+ uint32_t mm_cid;
};
static inline struct rseq_area *
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 5/8] nptl: Add rseq internal utils
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
` (3 preceding siblings ...)
2024-02-06 16:27 ` [PATCH v8 4/8] nptl: Add features to internal 'struct rseq_area' Michael Jeanson
@ 2024-02-06 16:27 ` Michael Jeanson
2024-02-17 2:33 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed Michael Jeanson
` (2 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson, Mathieu Desnoyers
Implement internal rseq utils in preparation for their use in
rseq_load32_load32_relaxed().
This implementation is imported from the librseq project.
Co-authored-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
sysdeps/unix/sysv/linux/rseq-internal.h | 56 +++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
index a63e4afbdb..b6c29c14cc 100644
--- a/sysdeps/unix/sysv/linux/rseq-internal.h
+++ b/sysdeps/unix/sysv/linux/rseq-internal.h
@@ -41,12 +41,68 @@ struct rseq_area
uint32_t mm_cid;
};
+/*
+ * gcc prior to 4.8.2 miscompiles asm goto.
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
+ *
+ * gcc prior to 8.1.0 miscompiles asm goto at O1.
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908
+ *
+ * clang prior to version 13.0.1 miscompiles asm goto at O2.
+ * https://github.com/llvm/llvm-project/issues/52735
+ *
+ * Work around these issues by adding a volatile inline asm with
+ * memory clobber in the fallthrough after the asm goto and at each
+ * label target. Emit this for all compilers in case other similar
+ * issues are found in the future.
+ */
+#define rseq_after_asm_goto() __asm__ __volatile__ ("" : : : "memory")
+
+#define __rseq_str_1(x) #x
+#define __rseq_str(x) __rseq_str_1(x)
+
+/* Offset of rseq_cs field in struct rseq. */
+#define RSEQ_CS_OFFSET 8
+
+#define rseq_sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
+#define rseq_offsetofend(TYPE, MEMBER) \
+ (offsetof(TYPE, MEMBER) + rseq_sizeof_field(TYPE, MEMBER))
+
+/* Returns a pointer to the current thread rseq area. */
static inline struct rseq_area *
rseq_get_area(void)
{
return (struct rseq_area *) ((char *) __thread_pointer() + GLRO (dl_tls_rseq_offset));
}
+/* Returns the int value of 'rseq_area->cpu_id'. */
+static inline int
+rseq_get_cpu_id(void)
+{
+ return (int) RSEQ_GETMEM_VOLATILE (rseq_get_area(), cpu_id);
+}
+
+/* Returns true if the rseq registration is active. */
+static inline bool
+rseq_is_registered(void)
+{
+ return rseq_get_cpu_id() >= 0;
+}
+
+/* Returns true if the current rseq registration has the 'node_id' field. */
+static inline bool
+rseq_node_id_available(void)
+{
+ return __rseq_feature_size >= rseq_offsetofend(struct rseq_area, node_id);
+}
+
+/* Returns true if the current rseq registration has the 'mm_cid' field. */
+static inline bool
+rseq_mm_cid_available(void)
+{
+ return __rseq_feature_size >= rseq_offsetofend(struct rseq_area, mm_cid);
+}
+
#ifdef RSEQ_SIG
static inline bool
rseq_register_current_thread (struct pthread *self, bool do_rseq)
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
` (4 preceding siblings ...)
2024-02-06 16:27 ` [PATCH v8 5/8] nptl: Add rseq internal utils Michael Jeanson
@ 2024-02-06 16:27 ` Michael Jeanson
2024-02-17 3:08 ` DJ Delorie
2024-02-06 16:28 ` [PATCH v8 7/8] aarch64: " Michael Jeanson
2024-02-06 16:28 ` [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu Michael Jeanson
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Mathieu Desnoyers, Michael Jeanson
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Implement rseq_load32_load32_relaxed() for the x86-64 architecture. This
static inline function implements a rseq critical section to load two
32-bit integer values atomically with respect to preemption and signal
delivery.
This implementation is imported from the librseq project.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
.../unix/sysv/linux/x86_64/rseq-internal.h | 109 ++++++++++++++++++
1 file changed, 109 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
diff --git a/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h b/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
new file mode 100644
index 0000000000..fdca1b6439
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
@@ -0,0 +1,109 @@
+/* Restartable Sequences internal API. x86_64 macros.
+ Copyright (C) 2023 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 <sysdeps/unix/sysv/linux/rseq-internal.h>
+
+#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
+ start_ip, post_commit_offset, abort_ip) \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
+ ".balign 32\n\t" \
+ __rseq_str(label) ":\n\t" \
+ ".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
+ ".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(label) "b\n\t" \
+ ".popsection\n\t"
+
+#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
+ __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
+ (post_commit_ip - start_ip), abort_ip)
+
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n\t" \
+ ".popsection\n\t"
+
+#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
+ "leaq " __rseq_str(cs_label) "(%%rip), %%rax\n\t" \
+ "movq %%rax, " __rseq_str(rseq_cs) "\n\t" \
+ __rseq_str(label) ":\n\t"
+
+#define RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, label) \
+ "cmpl %[" __rseq_str(cpu_id) "], " __rseq_str(current_cpu_id) "\n\t" \
+ "jnz " __rseq_str(label) "\n\t"
+
+#define RSEQ_ASM_DEFINE_ABORT(label, teardown, abort_label) \
+ ".pushsection __rseq_failure, \"ax\"\n\t" \
+ /* Disassembler-friendly signature: ud1 <sig>(%rip),%edi. */ \
+ ".byte 0x0f, 0xb9, 0x3d\n\t" \
+ ".long " __rseq_str(RSEQ_SIG) "\n\t" \
+ __rseq_str(label) ":\n\t" \
+ teardown \
+ "jmp %l[" __rseq_str(abort_label) "]\n\t" \
+ ".popsection\n\t"
+
+#define RSEQ_ASM_DEFINE_CMPFAIL(label, teardown, cmpfail_label) \
+ ".pushsection __rseq_failure, \"ax\"\n\t" \
+ __rseq_str(label) ":\n\t" \
+ teardown \
+ "jmp %l[" __rseq_str(cmpfail_label) "]\n\t" \
+ ".popsection\n\t"
+
+/*
+ * Load @src1 (32-bit) into @dst1 and load @src2 (32-bit) into @dst2.
+ */
+#define RSEQ_HAS_LOAD32_LOAD32_RELAXED 1
+static __always_inline int
+rseq_load32_load32_relaxed(uint32_t *dst1, uint32_t *src1,
+ uint32_t *dst2, uint32_t *src2)
+{
+ __asm__ __volatile__ goto (
+ RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ /* Start rseq by storing table entry pointer into rseq_cs. */
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3b, %%fs:RSEQ_CS_OFFSET(%[rseq_offset]))
+ "movl %[src1], %%ebx\n\t"
+ "movl %[src2], %%ecx\n\t"
+ "movl %%ebx, %[dst1]\n\t"
+ /* final store */
+ "movl %%ecx, %[dst2]\n\t"
+ "2:\n\t"
+ RSEQ_ASM_DEFINE_ABORT(4, "", abort)
+ : /* gcc asm goto does not allow outputs */
+ : [rseq_offset] "r" (__rseq_offset),
+ /* final store input */
+ [dst1] "m" (*dst1),
+ [dst2] "m" (*dst2),
+ [src1] "m" (*src1),
+ [src2] "m" (*src2)
+ : "memory", "cc", "ebx", "ecx", "rax"
+ : abort
+ );
+ rseq_after_asm_goto();
+ return 0;
+abort:
+ rseq_after_asm_goto();
+ return -1;
+}
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 7/8] aarch64: Add rseq_load32_load32_relaxed
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
` (5 preceding siblings ...)
2024-02-06 16:27 ` [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed Michael Jeanson
@ 2024-02-06 16:28 ` Michael Jeanson
2024-02-17 3:53 ` DJ Delorie
2024-02-06 16:28 ` [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu Michael Jeanson
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:28 UTC (permalink / raw)
To: libc-alpha; +Cc: Mathieu Desnoyers, Michael Jeanson
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Implement rseq_load32_load32_relaxed() for the aarch64 architecture.
This static inline function implements a rseq critical section to load
two 32-bit integer values atomically with respect to preemption and
signal delivery.
This implementation is imported from the librseq project.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
.../unix/sysv/linux/aarch64/rseq-internal.h | 173 ++++++++++++++++++
1 file changed, 173 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/aarch64/rseq-internal.h
diff --git a/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h b/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h
new file mode 100644
index 0000000000..3c03f67dbe
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h
@@ -0,0 +1,173 @@
+/* Restartable Sequences internal API. aarch64 macros.
+ Copyright (C) 2023 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 <sysdeps/unix/sysv/linux/rseq-internal.h>
+
+#define RSEQ_ASM_TMP_REG32 "w15"
+#define RSEQ_ASM_TMP_REG "x15"
+#define RSEQ_ASM_TMP_REG_2 "x14"
+
+#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, start_ip, \
+ post_commit_offset, abort_ip) \
+ " .pushsection __rseq_cs, \"aw\"\n" \
+ " .balign 32\n" \
+ __rseq_str(label) ":\n" \
+ " .long " __rseq_str(version) ", " __rseq_str(flags) "\n" \
+ " .quad " __rseq_str(start_ip) ", " \
+ __rseq_str(post_commit_offset) ", " \
+ __rseq_str(abort_ip) "\n" \
+ " .popsection\n\t" \
+ " .pushsection __rseq_cs_ptr_array, \"aw\"\n" \
+ " .quad " __rseq_str(label) "b\n" \
+ " .popsection\n"
+
+#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
+ __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
+ (post_commit_ip - start_ip), abort_ip)
+
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ " .pushsection __rseq_exit_point_array, \"aw\"\n" \
+ " .quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n" \
+ " .popsection\n"
+
+#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
+ " adrp " RSEQ_ASM_TMP_REG ", " __rseq_str(cs_label) "\n" \
+ " add " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
+ ", :lo12:" __rseq_str(cs_label) "\n" \
+ " str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(rseq_cs) "]\n" \
+ __rseq_str(label) ":\n"
+
+#define RSEQ_ASM_DEFINE_ABORT(label, abort_label) \
+ " b 222f\n" \
+ " .inst " __rseq_str(RSEQ_SIG_CODE) "\n" \
+ __rseq_str(label) ":\n" \
+ " b %l[" __rseq_str(abort_label) "]\n" \
+ "222:\n"
+
+#define RSEQ_ASM_OP_STORE(value, var) \
+ " str %[" __rseq_str(value) "], %[" __rseq_str(var) "]\n"
+
+#define RSEQ_ASM_OP_STORE_RELEASE(value, var) \
+ " stlr %[" __rseq_str(value) "], %[" __rseq_str(var) "]\n"
+
+#define RSEQ_ASM_OP_FINAL_STORE(value, var, post_commit_label) \
+ RSEQ_ASM_OP_STORE(value, var) \
+ __rseq_str(post_commit_label) ":\n"
+
+#define RSEQ_ASM_OP_FINAL_STORE_RELEASE(value, var, post_commit_label) \
+ RSEQ_ASM_OP_STORE_RELEASE(value, var) \
+ __rseq_str(post_commit_label) ":\n"
+
+#define RSEQ_ASM_OP_CMPEQ(var, expect, label) \
+ " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
+ " sub " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
+ ", %[" __rseq_str(expect) "]\n" \
+ " cbnz " RSEQ_ASM_TMP_REG ", " __rseq_str(label) "\n"
+
+#define RSEQ_ASM_OP_CMPEQ32(var, expect, label) \
+ " ldr " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n" \
+ " sub " RSEQ_ASM_TMP_REG32 ", " RSEQ_ASM_TMP_REG32 \
+ ", %w[" __rseq_str(expect) "]\n" \
+ " cbnz " RSEQ_ASM_TMP_REG32 ", " __rseq_str(label) "\n"
+
+#define RSEQ_ASM_OP_CMPNE(var, expect, label) \
+ " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
+ " sub " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
+ ", %[" __rseq_str(expect) "]\n" \
+ " cbz " RSEQ_ASM_TMP_REG ", " __rseq_str(label) "\n"
+
+#define RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, label) \
+ RSEQ_ASM_OP_CMPEQ32(current_cpu_id, cpu_id, label)
+
+#define RSEQ_ASM_OP_R_LOAD(var) \
+ " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n"
+
+#define RSEQ_ASM_OP_R_STORE(var) \
+ " str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n"
+
+#define RSEQ_ASM_OP_R_LOAD32(var) \
+ " ldr " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n"
+
+#define RSEQ_ASM_OP_R_STORE32(var) \
+ " str " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n"
+
+#define RSEQ_ASM_OP_R_LOAD_OFF(offset) \
+ " ldr " RSEQ_ASM_TMP_REG ", [" RSEQ_ASM_TMP_REG \
+ ", %[" __rseq_str(offset) "]]\n"
+
+#define RSEQ_ASM_OP_R_ADD(count) \
+ " add " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
+ ", %[" __rseq_str(count) "]\n"
+
+#define RSEQ_ASM_OP_R_FINAL_STORE(var, post_commit_label) \
+ " str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
+ __rseq_str(post_commit_label) ":\n"
+
+#define RSEQ_ASM_OP_R_FINAL_STORE32(var, post_commit_label) \
+ " str " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n" \
+ __rseq_str(post_commit_label) ":\n"
+
+#define RSEQ_ASM_OP_R_BAD_MEMCPY(dst, src, len) \
+ " cbz %[" __rseq_str(len) "], 333f\n" \
+ " mov " RSEQ_ASM_TMP_REG_2 ", %[" __rseq_str(len) "]\n" \
+ "222: sub " RSEQ_ASM_TMP_REG_2 ", " RSEQ_ASM_TMP_REG_2 ", #1\n" \
+ " ldrb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(src) "]" \
+ ", " RSEQ_ASM_TMP_REG_2 "]\n" \
+ " strb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(dst) "]" \
+ ", " RSEQ_ASM_TMP_REG_2 "]\n" \
+ " cbnz " RSEQ_ASM_TMP_REG_2 ", 222b\n" \
+ "333:\n"
+
+/*
+ * Load @src1 (32-bit) into @dst1 and load @src2 (32-bit) into @dst2.
+ */
+#define RSEQ_HAS_LOAD32_LOAD32_RELAXED 1
+static __always_inline int
+rseq_load32_load32_relaxed(uint32_t *dst1, uint32_t *src1,
+ uint32_t *dst2, uint32_t *src2)
+{
+ __asm__ __volatile__ goto (
+ RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_OP_R_LOAD32(src1)
+ RSEQ_ASM_OP_R_STORE32(dst1)
+ RSEQ_ASM_OP_R_LOAD32(src2)
+ RSEQ_ASM_OP_R_FINAL_STORE32(dst2, 3)
+ RSEQ_ASM_DEFINE_ABORT(4, abort)
+ : /* gcc asm goto does not allow outputs */
+ : [rseq_cs] "m" (rseq_get_area()->rseq_cs),
+ [dst1] "Qo" (*dst1),
+ [dst2] "Qo" (*dst2),
+ [src1] "Qo" (*src1),
+ [src2] "Qo" (*src2)
+ : "memory", RSEQ_ASM_TMP_REG
+ : abort
+ );
+ rseq_after_asm_goto();
+ return 0;
+abort:
+ rseq_after_asm_goto();
+ return -1;
+}
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
` (6 preceding siblings ...)
2024-02-06 16:28 ` [PATCH v8 7/8] aarch64: " Michael Jeanson
@ 2024-02-06 16:28 ` Michael Jeanson
2024-02-17 3:57 ` DJ Delorie
7 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-06 16:28 UTC (permalink / raw)
To: libc-alpha; +Cc: Michael Jeanson, Mathieu Desnoyers
On architectures that implement rseq_load32_load32_relaxed() (and thus
define RSEQ_HAS_LOAD32_LOAD32_RELAXED), when the node_id feature is
available, use rseq to fetch the cpu_id and node_id atomically with
respect to preemption and signal delivery to speed up getcpu() compared
to a vsyscall or system call implementation.
Loading both cpu_id and node_id atomically with respect to preemption
is required to ensure consistency of the topology mapping between cpu_id
and node_id due to migration between both loads.
On an aarch64 system (Snapdragon 8cx Gen 3) which lacks a vDSO for
getcpu() we measured an improvement from 130 ns to 1 ns while on x86_64
(i7-8550U) which has a vDSO we measured a more modest improvement from
10 ns to 2 ns.
Co-authored-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
sysdeps/unix/sysv/linux/getcpu.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/getcpu.c b/sysdeps/unix/sysv/linux/getcpu.c
index 0e7c3238c9..7e34d6d1eb 100644
--- a/sysdeps/unix/sysv/linux/getcpu.c
+++ b/sysdeps/unix/sysv/linux/getcpu.c
@@ -19,9 +19,10 @@
#include <sched.h>
#include <sysdep.h>
#include <sysdep-vdso.h>
+#include <rseq-internal.h>
-int
-__getcpu (unsigned int *cpu, unsigned int *node)
+static int
+vsyscall_getcpu (unsigned int *cpu, unsigned int *node)
{
#ifdef HAVE_GETCPU_VSYSCALL
return INLINE_VSYSCALL (getcpu, 3, cpu, node, NULL);
@@ -29,5 +30,32 @@ __getcpu (unsigned int *cpu, unsigned int *node)
return INLINE_SYSCALL_CALL (getcpu, cpu, node, NULL);
#endif
}
+
+#if defined (RSEQ_SIG) && defined (RSEQ_HAS_LOAD32_LOAD32_RELAXED)
+int
+__getcpu (unsigned int *cpu, unsigned int *node)
+{
+ /* Check if rseq is registered and the node_id feature is available. */
+ if (__glibc_likely (rseq_node_id_available()))
+ {
+ struct rseq_area *rseq_area = rseq_get_area();
+
+ if (rseq_load32_load32_relaxed(cpu, &rseq_area->cpu_id,
+ node, &rseq_area->node_id) == 0)
+ {
+ /* The critical section was not aborted, return 0. */
+ return 0;
+ }
+ }
+
+ return vsyscall_getcpu (cpu, node);
+}
+#else
+int
+__getcpu (unsigned int *cpu, unsigned int *node)
+{
+ return vsyscall_getcpu (cpu, node);
+}
+#endif
weak_alias (__getcpu, getcpu)
libc_hidden_def (__getcpu)
--
2.34.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols
2024-02-06 16:27 ` [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols Michael Jeanson
@ 2024-02-06 16:41 ` Mathieu Desnoyers
2024-02-16 2:02 ` DJ Delorie
1 sibling, 0 replies; 39+ messages in thread
From: Mathieu Desnoyers @ 2024-02-06 16:41 UTC (permalink / raw)
To: Florian Weimer, carlos; +Cc: Michael Jeanson, libc-alpha
On 2024-02-06 11:27, Michael Jeanson wrote:
> While working on a patch to add support for the extensible rseq ABI, we
> came across an issue where a new 'const' variable would be merged with
> the existing '__rseq_size' variable. We tracked this to the use of
> '-fmerge-all-constants' which allows the compiler to merge identical
> constant variables. This means that all 'const' variables in a compile
> unit that are of the same size and are initialized to the same value can
> be merged.
>
> In this specific case, on 32 bit systems 'unsigned int' and 'ptrdiff_t'
> are both 4 bytes and initialized to 0 which should trigger the merge.
> However for reasons we haven't delved into when the attribute 'section
> (".data.rel.ro")' is added to the mix, only variables of the same exact
> types are merged. As far as we know this behavior is not specified
> anywhere and could change with a new compiler version, hence this patch.
>
> Move the definitions of these variables into an assembler file and add
> hidden writable aliases for internal use. This has the added bonus of
> removing the asm workaround to set the values on rseq registration.
>
> Tested on Debian 12 with GCC 12.2.
Hi Florian,
Just to make sure you don't duplicate this effort: we ended up
implementing this fix, even though we originally said we did not
intend to.
Feedback is welcome!
Thanks,
Mathieu
>
> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
> Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
> csu/Makefile | 2 +-
> csu/rseq-sizes.sym | 8 +++++
> elf/Makefile | 1 +
> elf/dl-rseq-symbols.S | 55 +++++++++++++++++++++++++++++++++++
> sysdeps/nptl/dl-tls_init_tp.c | 14 ++++-----
> 5 files changed, 71 insertions(+), 9 deletions(-)
> create mode 100644 csu/rseq-sizes.sym
> create mode 100644 elf/dl-rseq-symbols.S
>
> diff --git a/csu/Makefile b/csu/Makefile
> index ac05ab24d5..0bf51a0e48 100644
> --- a/csu/Makefile
> +++ b/csu/Makefile
> @@ -99,7 +99,7 @@ before-compile += $(objpfx)abi-tag.h
> generated += abi-tag.h
>
> # Put it here to generate it earlier.
> -gen-as-const-headers += rtld-sizes.sym
> +gen-as-const-headers += rtld-sizes.sym rseq-sizes.sym
>
> # These are the special initializer/finalizer files. They are always the
> # first and last file in the link. crti.o ... crtn.o define the global
> diff --git a/csu/rseq-sizes.sym b/csu/rseq-sizes.sym
> new file mode 100644
> index 0000000000..c959758ff0
> --- /dev/null
> +++ b/csu/rseq-sizes.sym
> @@ -0,0 +1,8 @@
> +#include <stddef.h>
> +
> +--
> +RSEQ_SIZE_SIZE sizeof (unsigned int)
> +RSEQ_SIZE_ALIGN __alignof (unsigned int)
> +
> +RSEQ_OFFSET_SIZE sizeof (ptrdiff_t)
> +RSEQ_OFFSET_ALIGN __alignof (ptrdiff_t)
> diff --git a/elf/Makefile b/elf/Makefile
> index 5d78b659ce..7d711aedf0 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -73,6 +73,7 @@ dl-routines = \
> dl-origin \
> dl-printf \
> dl-reloc \
> + dl-rseq-symbols \
> dl-runtime \
> dl-scope \
> dl-setup_hash \
> diff --git a/elf/dl-rseq-symbols.S b/elf/dl-rseq-symbols.S
> new file mode 100644
> index 0000000000..2d8e88367f
> --- /dev/null
> +++ b/elf/dl-rseq-symbols.S
> @@ -0,0 +1,55 @@
> +/* Define symbols used by rseq.
> + Copyright (C) 2024 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + 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 <rseq-sizes.h>
> +#include <sysdep.h>
> +
> +/* Some targets define a macro to denote the zero register. */
> +#undef zero
> +
> +/* Define 2 symbols, __rseq_size is public const and _rseq_size, which is an
> + alias of __rseq_size, but hidden and writable for internal use. */
> +
> + .globl __rseq_size
> + .type __rseq_size, %object
> + .size __rseq_size, RSEQ_SIZE_SIZE
> + .hidden _rseq_size
> + .globl _rseq_size
> + .type _rseq_size, %object
> + .size _rseq_size, RSEQ_SIZE_SIZE
> + .section .data.rel.ro
> + .balign RSEQ_SIZE_ALIGN
> +__rseq_size:
> +_rseq_size:
> + .zero RSEQ_SIZE_SIZE
> +
> +/* Define 2 symbols, __rseq_offset is public const and _rseq_offset, which is an
> + alias of __rseq_offset, but hidden and writable for internal use. */
> +
> + .globl __rseq_offset
> + .type __rseq_offset, %object
> + .size __rseq_offset, RSEQ_OFFSET_SIZE
> + .hidden _rseq_offset
> + .globl _rseq_offset
> + .type _rseq_offset, %object
> + .size _rseq_offset, RSEQ_OFFSET_SIZE
> + .section .data.rel.ro
> + .balign RSEQ_OFFSET_ALIGN
> +__rseq_offset:
> +_rseq_offset:
> + .zero RSEQ_OFFSET_SIZE
> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
> index 092c274f36..80eb0107b5 100644
> --- a/sysdeps/nptl/dl-tls_init_tp.c
> +++ b/sysdeps/nptl/dl-tls_init_tp.c
> @@ -45,8 +45,10 @@ rtld_mutex_dummy (pthread_mutex_t *lock)
> #endif
>
> const unsigned int __rseq_flags;
> -const unsigned int __rseq_size attribute_relro;
> -const ptrdiff_t __rseq_offset attribute_relro;
> +
> +/* The variables are in .data.relro but are not yet write-protected. */
> +extern unsigned int _rseq_size attribute_relro attribute_hidden;
> +extern ptrdiff_t _rseq_offset attribute_relro attribute_hidden;
>
> void
> __tls_pre_init_tp (void)
> @@ -105,10 +107,7 @@ __tls_init_tp (void)
> do_rseq = TUNABLE_GET (rseq, int, NULL);
> if (rseq_register_current_thread (pd, do_rseq))
> {
> - /* We need a writable view of the variables. They are in
> - .data.relro and are not yet write-protected. */
> - extern unsigned int size __asm__ ("__rseq_size");
> - size = sizeof (pd->rseq_area);
> + _rseq_size = sizeof (pd->rseq_area);
> }
>
> #ifdef RSEQ_SIG
> @@ -117,8 +116,7 @@ __tls_init_tp (void)
> all targets support __thread_pointer, so set __rseq_offset only
> if the rseq registration may have happened because RSEQ_SIG is
> defined. */
> - extern ptrdiff_t offset __asm__ ("__rseq_offset");
> - offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
> + _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
> #endif
> }
>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols
2024-02-06 16:27 ` [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols Michael Jeanson
2024-02-06 16:41 ` Mathieu Desnoyers
@ 2024-02-16 2:02 ` DJ Delorie
2024-02-16 20:18 ` Michael Jeanson
1 sibling, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-16 2:02 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
> We tracked this to the use of '-fmerge-all-constants' which allows the
> compiler to merge identical constant variables.
I assume the linker's constant (string?) merging won't affect this
because the section is marked as writable to the linker?
> diff --git a/csu/Makefile b/csu/Makefile
> # Put it here to generate it earlier.
> -gen-as-const-headers += rtld-sizes.sym
> +gen-as-const-headers += rtld-sizes.sym rseq-sizes.sym
Ok.
> diff --git a/csu/rseq-sizes.sym b/csu/rseq-sizes.sym
> new file mode 100644
> index 0000000000..c959758ff0
> --- /dev/null
> +++ b/csu/rseq-sizes.sym
> @@ -0,0 +1,8 @@
> +#include <stddef.h>
> +
> +--
> +RSEQ_SIZE_SIZE sizeof (unsigned int)
> +RSEQ_SIZE_ALIGN __alignof (unsigned int)
> +
> +RSEQ_OFFSET_SIZE sizeof (ptrdiff_t)
> +RSEQ_OFFSET_ALIGN __alignof (ptrdiff_t)
Looks like rtld-sizes.sym, so OK.
> diff --git a/elf/Makefile b/elf/Makefile
> + dl-rseq-symbols \
Ok.
> diff --git a/elf/dl-rseq-symbols.S b/elf/dl-rseq-symbols.S
> +#include <rseq-sizes.h>
> +#include <sysdep.h>
Ok
> +/* Some targets define a macro to denote the zero register. */
> +#undef zero
Ok.
> +/* Define 2 symbols, __rseq_size is public const and _rseq_size, which is an
> + alias of __rseq_size, but hidden and writable for internal use. */
> +
> + .globl __rseq_size
> + .type __rseq_size, %object
> + .size __rseq_size, RSEQ_SIZE_SIZE
> + .hidden _rseq_size
> + .globl _rseq_size
> + .type _rseq_size, %object
> + .size _rseq_size, RSEQ_SIZE_SIZE
> + .section .data.rel.ro
> + .balign RSEQ_SIZE_ALIGN
> +__rseq_size:
> +_rseq_size:
> + .zero RSEQ_SIZE_SIZE
Ok.
> +/* Define 2 symbols, __rseq_offset is public const and _rseq_offset, which is an
> + alias of __rseq_offset, but hidden and writable for internal use. */
> +
> + .globl __rseq_offset
> + .type __rseq_offset, %object
> + .size __rseq_offset, RSEQ_OFFSET_SIZE
> + .hidden _rseq_offset
> + .globl _rseq_offset
> + .type _rseq_offset, %object
> + .size _rseq_offset, RSEQ_OFFSET_SIZE
> + .section .data.rel.ro
> + .balign RSEQ_OFFSET_ALIGN
> +__rseq_offset:
> +_rseq_offset:
> + .zero RSEQ_OFFSET_SIZE
Ok.
> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
> index 092c274f36..80eb0107b5 100644
> --- a/sysdeps/nptl/dl-tls_init_tp.c
> +++ b/sysdeps/nptl/dl-tls_init_tp.c
> @@ -45,8 +45,10 @@ rtld_mutex_dummy (pthread_mutex_t *lock)
> #endif
>
> const unsigned int __rseq_flags;
> -const unsigned int __rseq_size attribute_relro;
> -const ptrdiff_t __rseq_offset attribute_relro;
> +
> +/* The variables are in .data.relro but are not yet write-protected. */
> +extern unsigned int _rseq_size attribute_relro attribute_hidden;
> +extern ptrdiff_t _rseq_offset attribute_relro attribute_hidden;
er, extern *and* hidden? And relro?
In theory the relro one is harmless but meaningless, as externs don't
have sections in the local compilation unit.
The hidden one will mark the symbol hidden, but does that really matter
as long as it's marked hidden where it's defined?
> void
> __tls_pre_init_tp (void)
> @@ -105,10 +107,7 @@ __tls_init_tp (void)
> do_rseq = TUNABLE_GET (rseq, int, NULL);
> if (rseq_register_current_thread (pd, do_rseq))
> {
> - /* We need a writable view of the variables. They are in
> - .data.relro and are not yet write-protected. */
> - extern unsigned int size __asm__ ("__rseq_size");
> - size = sizeof (pd->rseq_area);
> + _rseq_size = sizeof (pd->rseq_area);
> }
Ok.
> #ifdef RSEQ_SIG
> @@ -117,8 +116,7 @@ __tls_init_tp (void)
> all targets support __thread_pointer, so set __rseq_offset only
> if the rseq registration may have happened because RSEQ_SIG is
> defined. */
> - extern ptrdiff_t offset __asm__ ("__rseq_offset");
> - offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
> + _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
> #endif
> }
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-06 16:27 ` [PATCH v8 2/8] Add rseq extensible ABI support Michael Jeanson
@ 2024-02-16 4:42 ` DJ Delorie
2024-02-16 20:19 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-16 4:42 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
One incorrect year
Could use more comments around the tcb math
Questions in sysdeps/unix/sysv/linux/tst-rseq-disable.c
Michael Jeanson <mjeanson@efficios.com> writes:
> This makes the size of the rseq area variable and thus requires to
> relocate it out of 'struct pthread'. We chose to move it after (in block
> allocation order) the last TLS block since it required a fairly small
> modification to the TLS block allocator and did not interfere with the
> main executable TLS block which must always be first.
The TLS doesn't get resized, does it? Or am I thinking of the DTV?
> diff --git a/csu/libc-tls.c b/csu/libc-tls.c
> index b7682bdf43..9c70f53284 100644
> --- a/csu/libc-tls.c
> +++ b/csu/libc-tls.c
> @@ -26,6 +26,8 @@
> #include <array_length.h>
> #include <pthreadP.h>
> #include <dl-call_tls_init_tp.h>
> +#include <dl-rseq.h>
> +#include <elf/dl-tunables.h>
Ok.
> #ifdef SHARED
> #error makefile bug, this file is for static only
> @@ -62,6 +64,18 @@ size_t _dl_tls_static_surplus;
> dynamic TLS access (e.g. with TLSDESC). */
> size_t _dl_tls_static_optional;
>
> +/* Size of the features present in the rseq area. */
> +size_t _dl_tls_rseq_feature_size;
> +
> +/* Alignment requirement of the rseq area. */
> +size_t _dl_tls_rseq_align;
> +
> +/* Size of the rseq area in the static TLS block. */
> +size_t _dl_tls_rseq_size;
> +
> +/* Offset of the rseq area from the thread pointer. */
> +ptrdiff_t _dl_tls_rseq_offset;
> +
Ok.
> /* Generation counter for the dtv. */
> size_t _dl_tls_generation;
>
> @@ -110,6 +124,7 @@ __libc_setup_tls (void)
> size_t filesz = 0;
> void *initimage = NULL;
> size_t align = 0;
> + size_t tls_blocks_size = 0;
This may be overriden later, ok.
> @@ -135,22 +150,61 @@ __libc_setup_tls (void)
> /* Calculate the size of the static TLS surplus, with 0 auditors. */
> _dl_tls_static_surplus_init (0);
>
> + /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
> + bytes when rseq is disabled by tunables. */
> + size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
> + size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
> + bool do_rseq = true;
> + do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
> + if (do_rseq)
> + {
> + rseq_align = GLRO(dl_tls_rseq_align);
> + /* Make sure the rseq area size is at least the minimum ABI size and a
> + multiple of the requested aligment. */
> + rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
> + TLS_DL_RSEQ_MIN_SIZE), rseq_align);
> + }
> +
> + /* Increase the max_align if necessary. */
> + max_align = MAX (max_align, rseq_align);
> +
> + /* Record the rseq_area block size. */
> + GLRO (dl_tls_rseq_size) = rseq_size;
Ok.
> #if TLS_TCB_AT_TP
> + /* Before the the thread pointer, add the aligned tls block size and then
> + align the rseq area block on top. */
> + tls_blocks_size = roundup (roundup (memsz, align ?: 1) + rseq_size, rseq_align);
> +
> + /* Record the rseq_area offset. The offset is negative with TLS_TCB_AT_TP
> + because the TLS blocks are located before the thread pointer. */
> + GLRO (dl_tls_rseq_offset) = -tls_blocks_size;
> +
So the allocated area will be at negative offsets to TP anyway; we add
at the beginning, so *our* location is at the beginning, and thus the
most negative offset.
What if the rseq area needs alignment greater than what
_dl_early_allocate() guarantees? This code takes the time to round the
tls size up to rseq_align, but never aligns the actual address of the
rseq area after allocation. Ah, the adjustments to max_align cause this
to happen later, as long as the offsets are suitably aligned also.
Perhaps a quick note in the comment that "max_align enforces the
alignment of the resulting pointer" or something like that?
Ok.
> /* Align the TCB offset to the maximum alignment, as
> _dl_allocate_tls_storage (in elf/dl-tls.c) does using __libc_memalign
> and dl_tls_static_align. */
> - tcb_offset = roundup (memsz + GLRO(dl_tls_static_surplus), max_align);
> + tcb_offset = roundup (tls_blocks_size + GLRO(dl_tls_static_surplus), max_align);
Ok.
> #elif TLS_DTV_AT_TP
> + /* Align memsz on top of the initial tcb. */
> tcb_offset = roundup (TLS_INIT_TCB_SIZE, align ?: 1);
> - tlsblock = _dl_early_allocate (tcb_offset + memsz + max_align
> +
> + /* After the thread pointer, add the initial tcb plus the tls block size and
> + then align the rseq area block on top. */
> + tls_blocks_size = roundup (tcb_offset + memsz + rseq_size, rseq_align);
> +
> + /* Record the rseq_area offset. The offset is positive with TLS_DTV_AT_TP
> + because the TLS blocks are located after the thread pointer. */
> + GLRO (dl_tls_rseq_offset) = tls_blocks_size - rseq_size;
> +
> + tlsblock = _dl_early_allocate (tls_blocks_size + max_align
> + TLS_PRE_TCB_SIZE
> + GLRO(dl_tls_static_surplus));
So here all the offsets are positive wrt TP, so we extend the block and
calculate the old "end" as our new offset. Note that rseq_size was
previously rounded up to rseq_align so the resulting pointer should be
properly aligned. Ok.
> @@ -209,11 +263,5 @@ __libc_setup_tls (void)
> /* static_slotinfo.slotinfo[1].gen = 0; -- Already zero. */
> static_slotinfo.slotinfo[1].map = main_map;
>
> - memsz = roundup (memsz, align ?: 1);
> -
> -#if TLS_DTV_AT_TP
> - memsz += tcb_offset;
> -#endif
> -
> - init_static_tls (memsz, MAX (TCB_ALIGNMENT, max_align));
> + init_static_tls (tls_blocks_size, MAX (TCB_ALIGNMENT, max_align));
> }
Ok.
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 7b3dd9ab60..70fbe7095f 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -27,6 +27,7 @@
>
> #include <tls.h>
> #include <dl-tls.h>
> +#include <dl-rseq.h>
> #include <ldsodefs.h>
Ok.
> @@ -298,6 +299,35 @@ _dl_determine_tlsoffset (void)
> slotinfo[cnt].map->l_tls_offset = off;
> }
>
> + /* Insert the rseq area block after the last TLS block. */
> +
> + /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
> + bytes when rseq is disabled by tunables. */
> + size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
> + size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
> + bool do_rseq = true;
> + do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
> + if (do_rseq)
> + {
> + rseq_align = GLRO(dl_tls_rseq_align);
> + /* Make sure the rseq area size is at least the minimum ABI size and a
> + multiple of the requested aligment. */
> + rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
> + TLS_DL_RSEQ_MIN_SIZE), rseq_align);
> + }
> +
> + /* Add the rseq area block to the global offset. */
> + offset = roundup (offset, rseq_align) + rseq_size;
> +
> + /* Increase the max_align if necessary. */
> + max_align = MAX (max_align, rseq_align);
> +
> + /* Record the rseq_area block size and offset. The offset is negative
> + with TLS_TCB_AT_TP because the TLS blocks are located before the
> + thread pointer. */
> + GLRO (dl_tls_rseq_offset) = -offset;
> + GLRO (dl_tls_rseq_size) = rseq_size;
> +
Ok. Same as above, this time for shared objects.
> @@ -343,6 +373,38 @@ _dl_determine_tlsoffset (void)
> offset = off + slotinfo[cnt].map->l_tls_blocksize - firstbyte;
> }
>
> + /* Insert the rseq area block after the last TLS block. */
> +
> + /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
> + bytes when rseq is disabled by tunables. */
> + size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
> + size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
> + bool do_rseq = true;
> + do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
> + if (do_rseq)
> + {
> + rseq_align = GLRO(dl_tls_rseq_align);
> + /* Make sure the rseq area size is at least the minimum ABI size and a
> + multiple of the requested aligment. */
> + rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
> + TLS_DL_RSEQ_MIN_SIZE), rseq_align);
> + }
> +
> + /* Align the global offset to the beginning of the rseq area. */
> + offset = roundup (offset, rseq_align);
> +
> + /* Record the rseq_area block size and offset. The offset is positive
> + with TLS_DTV_AT_TP because the TLS blocks are located after the
> + thread pointer. */
> + GLRO (dl_tls_rseq_size) = rseq_size;
> + GLRO (dl_tls_rseq_offset) = offset;
> +
> + /* Add the rseq area block to the global offset. */
> + offset += rseq_size;
> +
> + /* Increase the max_align if necessary. */
> + max_align = MAX (max_align, rseq_align);
> +
Ok.
> diff --git a/elf/rtld_static_init.c b/elf/rtld_static_init.c
> index e918e4ebdf..0606317b8c 100644
> --- a/elf/rtld_static_init.c
> +++ b/elf/rtld_static_init.c
> @@ -78,6 +78,18 @@ __rtld_static_init (struct link_map *map)
> extern __typeof (dl->_dl_tls_static_size) _dl_tls_static_size
> attribute_hidden;
> dl->_dl_tls_static_size = _dl_tls_static_size;
> + extern __typeof (dl->_dl_tls_rseq_feature_size) _dl_tls_rseq_feature_size
> + attribute_hidden;
> + dl->_dl_tls_rseq_feature_size = _dl_tls_rseq_feature_size;
> + extern __typeof (dl->_dl_tls_rseq_align) _dl_tls_rseq_align
> + attribute_hidden;
> + dl->_dl_tls_rseq_align = _dl_tls_rseq_align;
> + extern __typeof (dl->_dl_tls_rseq_size) _dl_tls_rseq_size
> + attribute_hidden;
> + dl->_dl_tls_rseq_size = _dl_tls_rseq_size;
> + extern __typeof (dl->_dl_tls_rseq_offset) _dl_tls_rseq_offset
> + attribute_hidden;
> + dl->_dl_tls_rseq_offset = _dl_tls_rseq_offset;
Ok.
> diff --git a/nptl/descr.h b/nptl/descr.h
> index 8cef95810c..cdc3c82d9a 100644
> --- a/nptl/descr.h
> +++ b/nptl/descr.h
> @@ -404,25 +404,11 @@ struct pthread
> /* Used on strsignal. */
> struct tls_internal_t tls_state;
>
> - /* rseq area registered with the kernel. Use a custom definition
> - here to isolate from kernel struct rseq changes. The
> - implementation of sched_getcpu needs acccess to the cpu_id field;
> - the other fields are unused and not included here. */
> - union
> - {
> - struct
> - {
> - uint32_t cpu_id_start;
> - uint32_t cpu_id;
> - };
> - char pad[32]; /* Original rseq area size. */
> - } rseq_area __attribute__ ((aligned (32)));
> -
> /* Amount of end padding, if any, in this structure.
> - This definition relies on rseq_area being last. */
> + This definition relies on tls_state being last. */
> #define PTHREAD_STRUCT_END_PADDING \
> - (sizeof (struct pthread) - offsetof (struct pthread, rseq_area) \
> - + sizeof ((struct pthread) {}.rseq_area))
> + (sizeof (struct pthread) - offsetof (struct pthread, tls_state) \
> + + sizeof ((struct pthread) {}.tls_state))
This memory is no longer statically sized; ok.
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 1d3665d5ed..9b49ee7121 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -691,7 +691,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>
> /* Inherit rseq registration state. Without seccomp filters, rseq
> registration will either always fail or always succeed. */
> - if ((int) THREAD_GETMEM_VOLATILE (self, rseq_area.cpu_id) >= 0)
> + if ((int) RSEQ_GETMEM_VOLATILE (rseq_get_area(), cpu_id) >= 0)
Ok.
> diff --git a/sysdeps/generic/dl-rseq.h b/sysdeps/generic/dl-rseq.h
> new file mode 100644
> index 0000000000..0855981c89
> --- /dev/null
> +++ b/sysdeps/generic/dl-rseq.h
> @@ -0,0 +1,26 @@
> +/* RSEQ defines for the dynamic linker. Generic version.
> + Copyright (C) 2023 Free Software Foundation, Inc.
2023 ?
> +/* Minimum size of the rseq area. */
> +#define TLS_DL_RSEQ_MIN_SIZE 32
> +
> +/* Minimum feature size of the rseq area. */
> +#define TLS_DL_RSEQ_MIN_FEATURE_SIZE 20
> +
> +/* Minimum size of the rseq area alignment. */
> +#define TLS_DL_RSEQ_MIN_ALIGN 32
Ok.
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 117c901ccc..53a0a208d8 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -610,6 +610,18 @@ struct rtld_global_ro
> See comments in elf/dl-tls.c where it is initialized. */
> EXTERN size_t _dl_tls_static_surplus;
>
> + /* Size of the features present in the rseq area. */
> + EXTERN size_t _dl_tls_rseq_feature_size;
> +
> + /* Alignment requirement of the rseq area. */
> + EXTERN size_t _dl_tls_rseq_align;
> +
> + /* Size of the rseq area in the static TLS block. */
> + EXTERN size_t _dl_tls_rseq_size;
> +
> + /* Offset of the rseq area from the thread pointer. */
> + EXTERN ptrdiff_t _dl_tls_rseq_offset;
> +
Ok.
> diff --git a/sysdeps/i386/nptl/tcb-access.h b/sysdeps/i386/nptl/tcb-access.h
> index 4b6221e103..e6988186d0 100644
> --- a/sysdeps/i386/nptl/tcb-access.h
> +++ b/sysdeps/i386/nptl/tcb-access.h
> @@ -123,3 +123,59 @@
> "i" (offsetof (struct pthread, member)), \
> "r" (idx)); \
> }})
> +
> +
> +/* Read member of the RSEQ area directly. */
> +#define RSEQ_GETMEM_VOLATILE(descr, member) \
> + ({ __typeof (descr->member) __value; \
> + ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
> + _Static_assert (sizeof (__value) == 1 \
> + || sizeof (__value) == 4 \
> + || sizeof (__value) == 8, \
> + "size of per-thread data"); \
> + if (sizeof (__value) == 1) \
> + asm volatile ("movb %%gs:%P2(%3),%b0" \
> + : "=q" (__value) \
> + : "0" (0), "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else if (sizeof (__value) == 4) \
> + asm volatile ("movl %%gs:%P1(%2),%0" \
> + : "=r" (__value) \
> + : "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else /* 8 */ \
> + { \
> + asm volatile ("movl %%gs:%P1(%2),%%eax\n\t" \
> + "movl %%gs:4+%P1(%2),%%edx" \
> + : "=&A" (__value) \
> + : "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + } \
> + __value; })
Ok.
> +/* Set member of the RSEQ area directly. */
> +#define RSEQ_SETMEM(descr, member, value) \
> + ({ \
> + ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
> + _Static_assert (sizeof (descr->member) == 1 \
> + || sizeof (descr->member) == 4 \
> + || sizeof (descr->member) == 8, \
> + "size of per-thread data"); \
> + if (sizeof (descr->member) == 1) \
> + asm volatile ("movb %b0,%%gs:%P1(%2)" : \
> + : "iq" (value), \
> + "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else if (sizeof (descr->member) == 4) \
> + asm volatile ("movl %0,%%gs:%P1(%2)" : \
> + : "ir" (value), \
> + "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else /* 8 */ \
> + { \
> + asm volatile ("movl %%eax,%%gs:%P1(%2)\n\t" \
> + "movl %%edx,%%gs:4+%P1(%2)" : \
> + : "A" ((uint64_t) cast_to_integer (value)), \
> + "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + }})
Ok.
> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
> index 80eb0107b5..7aa15558e6 100644
> --- a/sysdeps/nptl/dl-tls_init_tp.c
> +++ b/sysdeps/nptl/dl-tls_init_tp.c
> @@ -107,7 +107,7 @@ __tls_init_tp (void)
> do_rseq = TUNABLE_GET (rseq, int, NULL);
> if (rseq_register_current_thread (pd, do_rseq))
> {
> - _rseq_size = sizeof (pd->rseq_area);
> + _rseq_size = GLRO (dl_tls_rseq_size);
Ok.
> @@ -116,7 +116,7 @@ __tls_init_tp (void)
> all targets support __thread_pointer, so set __rseq_offset only
> if the rseq registration may have happened because RSEQ_SIG is
> defined. */
> - _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
> + _rseq_offset = GLRO (dl_tls_rseq_offset);
Ok.
> diff --git a/sysdeps/nptl/tcb-access.h b/sysdeps/nptl/tcb-access.h
> index 600433766f..9532f30022 100644
> --- a/sysdeps/nptl/tcb-access.h
> +++ b/sysdeps/nptl/tcb-access.h
> @@ -30,3 +30,8 @@
> descr->member = (value)
> #define THREAD_SETMEM_NC(descr, member, idx, value) \
> descr->member[idx] = (value)
> +
> +#define RSEQ_GETMEM_VOLATILE(descr, member) \
> + THREAD_GETMEM_VOLATILE(descr, member)
> +#define RSEQ_SETMEM(descr, member, value) \
> + THREAD_SETMEM(descr, member, value)
Ok.
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> +tests-static += \
> + tst-rseq-disable-static \
> + tst-rseq-static \
> + # tests-static
> +
Ok.
> @@ -394,6 +399,7 @@ $(objpfx)tst-mount-compile.out: ../sysdeps/unix/sysv/linux/tst-mount-compile.py
> $(objpfx)tst-mount-compile.out: $(sysdeps-linux-python-deps)
>
> tst-rseq-disable-ENV = GLIBC_TUNABLES=glibc.pthread.rseq=0
> +tst-rseq-disable-static-ENV = GLIBC_TUNABLES=glibc.pthread.rseq=0
Ok.
> @@ -655,4 +661,8 @@ tests += \
> tests-internal += \
> tst-rseq-nptl \
> # tests-internal
> +
> +tests-static += \
> + tst-rseq-nptl-static \
> + # tests-static
Ok.
> diff --git a/sysdeps/unix/sysv/linux/dl-parse_auxv.h b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
> index e3d758b163..fbc90fc786 100644
> --- a/sysdeps/unix/sysv/linux/dl-parse_auxv.h
> +++ b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
> @@ -21,6 +21,7 @@
> #include <fpu_control.h>
> #include <ldsodefs.h>
> #include <link.h>
> +#include <dl-rseq.h>
>
> typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];
>
> @@ -57,5 +58,10 @@ void _dl_parse_auxv (ElfW(auxv_t) *av, dl_parse_auxv_t auxv_values)
> GLRO(dl_sysinfo) = auxv_values[AT_SYSINFO];
> #endif
>
> + GLRO(dl_tls_rseq_feature_size) = MAX (auxv_values[AT_RSEQ_FEATURE_SIZE],
> + TLS_DL_RSEQ_MIN_FEATURE_SIZE);
> + GLRO(dl_tls_rseq_align) = MAX (auxv_values[AT_RSEQ_ALIGN],
> + TLS_DL_RSEQ_MIN_ALIGN);
> +
Ok.
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> index 48eebc1e16..b6c9deea6b 100644
> --- a/sysdeps/unix/sysv/linux/rseq-internal.h
> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
> @@ -24,6 +24,24 @@
> #include <stdbool.h>
> #include <stdio.h>
> #include <sys/rseq.h>
> +#include <thread_pointer.h>
> +#include <ldsodefs.h>
> +
> +/* rseq area registered with the kernel. Use a custom definition
> + here to isolate from kernel struct rseq changes. The
> + implementation of sched_getcpu needs acccess to the cpu_id field;
> + the other fields are unused and not included here. */
> +struct rseq_area
> +{
> + uint32_t cpu_id_start;
> + uint32_t cpu_id;
> +};
> +
> +static inline struct rseq_area *
> +rseq_get_area(void)
> +{
> + return (struct rseq_area *) ((char *) __thread_pointer() + GLRO (dl_tls_rseq_offset));
> +}
Ok.
> @@ -31,20 +49,23 @@ rseq_register_current_thread (struct pthread *self, bool do_rseq)
> {
> if (do_rseq)
> {
> - int ret = INTERNAL_SYSCALL_CALL (rseq, &self->rseq_area,
> - sizeof (self->rseq_area),
> + /* The kernel expects 'rseq_area->rseq_cs == NULL' on registration, zero
> + the whole rseq area. */
> + memset(rseq_get_area(), 0, GLRO (dl_tls_rseq_size));
> + int ret = INTERNAL_SYSCALL_CALL (rseq, rseq_get_area(),
> + GLRO (dl_tls_rseq_size),
> 0, RSEQ_SIG);
Ok.
> - THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
> + RSEQ_SETMEM (rseq_get_area(), cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
Ok.
> - THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
> + RSEQ_SETMEM (rseq_get_area(), cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
> - int cpu_id = THREAD_GETMEM_VOLATILE (THREAD_SELF, rseq_area.cpu_id);
> + int cpu_id = RSEQ_GETMEM_VOLATILE (rseq_get_area(), cpu_id);
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable-static.c b/sysdeps/unix/sysv/linux/tst-rseq-disable-static.c
> new file mode 100644
> index 0000000000..2687d13d3d
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-rseq-disable-static.c
> @@ -0,0 +1 @@
> +#include "tst-rseq-disable.c"
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable.c b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
> index bbc655bec4..ae4143916b 100644
> --- a/sysdeps/unix/sysv/linux/tst-rseq-disable.c
> +++ b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
> @@ -26,27 +26,30 @@
> #include <unistd.h>
>
> #ifdef RSEQ_SIG
> +# include <sys/auxv.h>
> +# include "tst-rseq.h"
> +
> +static __thread struct rseq local_rseq;
>
> /* Check that rseq can be registered and has not been taken by glibc. */
> static void
> check_rseq_disabled (void)
> {
> - struct pthread *pd = THREAD_SELF;
> + struct rseq *rseq_area = (struct rseq *) ((char *) __thread_pointer () + __rseq_offset);
>
> TEST_COMPARE (__rseq_flags, 0);
> - TEST_VERIFY ((char *) __thread_pointer () + __rseq_offset
> - == (char *) &pd->rseq_area);
> TEST_COMPARE (__rseq_size, 0);
Isn't __rseq_size always at least 32 ?
> - TEST_COMPARE ((int) pd->rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
> + TEST_COMPARE ((int) rseq_area->cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
Ok.
> + TEST_COMPARE (sizeof (local_rseq), RSEQ_TEST_MIN_SIZE);
This happens to be true at this point, but the comparison should really
be >= not ==, esp as the features patch changes the size of this
structure to be larger than MIN_SIZE.
> - int ret = syscall (__NR_rseq, &pd->rseq_area, sizeof (pd->rseq_area),
> - 0, RSEQ_SIG);
> + int ret = syscall (__NR_rseq, &local_rseq, RSEQ_TEST_MIN_SIZE, 0, RSEQ_SIG);
> if (ret == 0)
> {
> - ret = syscall (__NR_rseq, &pd->rseq_area, sizeof (pd->rseq_area),
> + ret = syscall (__NR_rseq, &local_rseq, RSEQ_TEST_MIN_SIZE,
> RSEQ_FLAG_UNREGISTER, RSEQ_SIG);
> TEST_COMPARE (ret, 0);
> - pd->rseq_area.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
> + rseq_area->cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
> }
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c b/sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c
> new file mode 100644
> index 0000000000..6e2c923bb9
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-rseq-nptl-static.c
> @@ -0,0 +1 @@
> +#include "tst-rseq-nptl.c"
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-static.c b/sysdeps/unix/sysv/linux/tst-rseq-static.c
> new file mode 100644
> index 0000000000..1d97f3bd3d
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-rseq-static.c
> @@ -0,0 +1 @@
> +#include "tst-rseq.c"
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
> index 2c90409ba0..c8c0518a5d 100644
> --- a/sysdeps/unix/sysv/linux/tst-rseq.c
> +++ b/sysdeps/unix/sysv/linux/tst-rseq.c
> @@ -31,18 +31,32 @@
> # include <syscall.h>
> # include <thread_pointer.h>
> # include <tls.h>
> +# include <sys/auxv.h>
> # include "tst-rseq.h"
Ok.
> static void
> do_rseq_main_test (void)
> {
> - struct pthread *pd = THREAD_SELF;
> + size_t rseq_align = MAX (getauxval (AT_RSEQ_ALIGN), RSEQ_TEST_MIN_ALIGN);
> + size_t rseq_size = roundup (MAX (getauxval (AT_RSEQ_FEATURE_SIZE), RSEQ_TEST_MIN_SIZE), rseq_align);
> + struct rseq *rseq = __thread_pointer () + __rseq_offset;
Ok.
> TEST_VERIFY_EXIT (rseq_thread_registered ());
> TEST_COMPARE (__rseq_flags, 0);
> - TEST_VERIFY ((char *) __thread_pointer () + __rseq_offset
> - == (char *) &pd->rseq_area);
> - TEST_COMPARE (__rseq_size, sizeof (pd->rseq_area));
> + TEST_COMPARE (__rseq_size, rseq_size);
Ok.
> + /* The size of the rseq area must be a multiple of the alignment. */
> + TEST_VERIFY ((__rseq_size % rseq_align) == 0);
Ok.
> + /* The rseq area address must be aligned. */
> + TEST_VERIFY (((unsigned long) rseq % rseq_align) == 0);
Ok.
> +#if TLS_TCB_AT_TP
> + /* The rseq area block should come before the thread pointer and be at least 32 bytes. */
> + TEST_VERIFY (__rseq_offset <= RSEQ_TEST_MIN_SIZE);
> +#elif TLS_DTV_AT_TP
> + /* The rseq area block should come after the thread pointer. */
> + TEST_VERIFY (__rseq_offset >= 0);
> +#else
> +# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
> +#endif
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq.h b/sysdeps/unix/sysv/linux/tst-rseq.h
> index dc603327d3..4931aa3d14 100644
> --- a/sysdeps/unix/sysv/linux/tst-rseq.h
> +++ b/sysdeps/unix/sysv/linux/tst-rseq.h
> @@ -23,11 +23,18 @@
> #include <syscall.h>
> #include <sys/rseq.h>
> #include <tls.h>
> +#include <rseq-internal.h>
> +
> +#define RSEQ_TEST_MIN_SIZE 32
> +#define RSEQ_TEST_MIN_FEATURE_SIZE 20
> +#define RSEQ_TEST_MIN_ALIGN 32
>
> static inline bool
> rseq_thread_registered (void)
> {
> - return THREAD_GETMEM_VOLATILE (THREAD_SELF, rseq_area.cpu_id) >= 0;
> + struct rseq_area *rseq = (struct rseq_area *) ((char *) __thread_pointer () + __rseq_offset);
> +
> + return __atomic_load_n (&rseq->cpu_id, __ATOMIC_RELAXED) >= 0;
> }
Ok.
> diff --git a/sysdeps/x86_64/nptl/tcb-access.h b/sysdeps/x86_64/nptl/tcb-access.h
> index d35948f111..75ba4b3ce9 100644
> --- a/sysdeps/x86_64/nptl/tcb-access.h
> +++ b/sysdeps/x86_64/nptl/tcb-access.h
> @@ -130,3 +130,59 @@
> "i" (offsetof (struct pthread, member[0])), \
> "r" (idx)); \
> }})
> +
> +/* Read member of the RSEQ area directly. */
> +# define RSEQ_GETMEM_VOLATILE(descr, member) \
> + ({ __typeof (descr->member) __value; \
> + ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
> + _Static_assert (sizeof (__value) == 1 \
> + || sizeof (__value) == 4 \
> + || sizeof (__value) == 8, \
> + "size of per-thread data"); \
> + if (sizeof (__value) == 1) \
> + asm volatile ("movb %%fs:%P2(%q3),%b0" \
> + : "=q" (__value) \
> + : "0" (0), "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else if (sizeof (__value) == 4) \
> + asm volatile ("movl %%fs:%P1(%q2),%0" \
> + : "=r" (__value) \
> + : "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else /* 8 */ \
> + { \
> + asm volatile ("movq %%fs:%P1(%q2),%q0" \
> + : "=r" (__value) \
> + : "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + } \
> + __value; })
Ok.
> +/* Set member of the RSEQ area directly. */
> +# define RSEQ_SETMEM(descr, member, value) \
> + ({ \
> + ptrdiff_t _rseq_offset = GLRO (dl_tls_rseq_offset); \
> + _Static_assert (sizeof (descr->member) == 1 \
> + || sizeof (descr->member) == 4 \
> + || sizeof (descr->member) == 8, \
> + "size of per-thread data"); \
> + if (sizeof (descr->member) == 1) \
> + asm volatile ("movb %b0,%%fs:%P1(%q2)" : \
> + : "iq" (value), \
> + "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else if (sizeof (descr->member) == 4) \
> + asm volatile ("movl %0,%%fs:%P1(%q2)" : \
> + : IMM_MODE (value), \
> + "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + else /* 8 */ \
> + { \
> + /* Since movq takes a signed 32-bit immediate or a register source \
> + operand, use "er" constraint for 32-bit signed integer constant \
> + or register. */ \
> + asm volatile ("movq %q0,%%fs:%P1(%q2)" : \
> + : "er" ((uint64_t) cast_to_integer (value)), \
> + "i" (offsetof (struct rseq_area, member)), \
> + "r" (_rseq_offset)); \
> + }})
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols
2024-02-16 2:02 ` DJ Delorie
@ 2024-02-16 20:18 ` Michael Jeanson
2024-02-16 20:29 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-16 20:18 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-15 21 h 02, DJ Delorie wrote:
>
> Michael Jeanson <mjeanson@efficios.com> writes:
>> We tracked this to the use of '-fmerge-all-constants' which allows the
>> compiler to merge identical constant variables.
>
> I assume the linker's constant (string?) merging won't affect this
> because the section is marked as writable to the linker?
I checked and the .data.rel.ro section does indeed have the SHF_WRITE
flag set. My understanding is that the section would need to be
read-only and the SHF_MERGE flag would need to be set for the linker to
try merging things.
>> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
>> index 092c274f36..80eb0107b5 100644
>> --- a/sysdeps/nptl/dl-tls_init_tp.c
>> +++ b/sysdeps/nptl/dl-tls_init_tp.c
>> @@ -45,8 +45,10 @@ rtld_mutex_dummy (pthread_mutex_t *lock)
>> #endif
>>
>> const unsigned int __rseq_flags;
>> -const unsigned int __rseq_size attribute_relro;
>> -const ptrdiff_t __rseq_offset attribute_relro;
>> +
>> +/* The variables are in .data.relro but are not yet write-protected. */
>> +extern unsigned int _rseq_size attribute_relro attribute_hidden;
>> +extern ptrdiff_t _rseq_offset attribute_relro attribute_hidden;
>
> er, extern *and* hidden? And relro?
>
> In theory the relro one is harmless but meaningless, as externs don't
> have sections in the local compilation unit.
>
> The hidden one will mark the symbol hidden, but does that really matter
> as long as it's marked hidden where it's defined?
Both attributes can be removed, the definition in the assembly file is
what matters. I'll clean this in the next patch series.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-16 4:42 ` DJ Delorie
@ 2024-02-16 20:19 ` Michael Jeanson
2024-02-16 20:37 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-16 20:19 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-15 23 h 42, DJ Delorie wrote:
>
> One incorrect year
> Could use more comments around the tcb math
Ack, I'll address this in the next patch series.
> Questions in sysdeps/unix/sysv/linux/tst-rseq-disable. > Michael Jeanson <mjeanson@efficios.com> writes:
>> This makes the size of the rseq area variable and thus requires to
>> relocate it out of 'struct pthread'. We chose to move it after (in block
>> allocation order) the last TLS block since it required a fairly small
>> modification to the TLS block allocator and did not interfere with the
>> main executable TLS block which must always be first.
>
> The TLS doesn't get resized, does it? Or am I thinking of the DTV?
The terminology is a bit confusing and could be clarified, my
understanding is that the 'static TLS block' is the area where the 'TLS
blocks' from shared objects and the main executable are allocated and
the DTV is the array of descriptors that point to each of these 'TLS
blocks' inside the 'static TLS block'.
What we are doing here is adding a 'fake' 'TLS block' at the end of the
'static TLS block' (in allocation order which differs depending of the
TLS model). We move the offset so our fake block is included in
dl_tls_static_used but we don't register it in the DTV since it's not a
'real' TLS block with the required metadata.
I think I should rewrite the commit message to make the distinction
between the 'static TLS block' and 'TLS blocks'.
>> #if TLS_TCB_AT_TP
>> + /* Before the the thread pointer, add the aligned tls block size and then
>> + align the rseq area block on top. */
>> + tls_blocks_size = roundup (roundup (memsz, align ?: 1) + rseq_size, rseq_align);
>> +
>> + /* Record the rseq_area offset. The offset is negative with TLS_TCB_AT_TP
>> + because the TLS blocks are located before the thread pointer. */
>> + GLRO (dl_tls_rseq_offset) = -tls_blocks_size;
>> +
>
> So the allocated area will be at negative offsets to TP anyway; we add
> at the beginning, so *our* location is at the beginning, and thus the
> most negative offset.
>
> What if the rseq area needs alignment greater than what
> _dl_early_allocate() guarantees? This code takes the time to round the
> tls size up to rseq_align, but never aligns the actual address of the
> rseq area after allocation. Ah, the adjustments to max_align cause this
> to happen later, as long as the offsets are suitably aligned also.
>
> Perhaps a quick note in the comment that "max_align enforces the
> alignment of the resulting pointer" or something like that?
Ack, I'll add a comment.
>> diff --git a/sysdeps/generic/dl-rseq.h b/sysdeps/generic/dl-rseq.h
>> new file mode 100644
>> index 0000000000..0855981c89
>> --- /dev/null
>> +++ b/sysdeps/generic/dl-rseq.h
>> @@ -0,0 +1,26 @@
>> +/* RSEQ defines for the dynamic linker. Generic version.
>> + Copyright (C) 2023 Free Software Foundation, Inc.
>
> 2023 ?
Ack.
>> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable.c b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
>> index bbc655bec4..ae4143916b 100644
>> --- a/sysdeps/unix/sysv/linux/tst-rseq-disable.c
>> +++ b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
>> @@ -26,27 +26,30 @@
>> #include <unistd.h>
>>
>> #ifdef RSEQ_SIG
>> +# include <sys/auxv.h>
>> +# include "tst-rseq.h"
>> +
>> +static __thread struct rseq local_rseq;
>>
>> /* Check that rseq can be registered and has not been taken by glibc. */
>> static void
>> check_rseq_disabled (void)
>> {
>> - struct pthread *pd = THREAD_SELF;
>> + struct rseq *rseq_area = (struct rseq *) ((char *) __thread_pointer () + __rseq_offset);
>>
>> TEST_COMPARE (__rseq_flags, 0);
>> - TEST_VERIFY ((char *) __thread_pointer () + __rseq_offset
>> - == (char *) &pd->rseq_area);
>> TEST_COMPARE (__rseq_size, 0);
>
> Isn't __rseq_size always at least 32 ?
It's at least 32 when the registration succeeded, however if the
registration failed or was disabled by tunable then it's set to 0.
>> + TEST_COMPARE (sizeof (local_rseq), RSEQ_TEST_MIN_SIZE);
>
> This happens to be true at this point, but the comparison should really
> be >= not ==, esp as the features patch changes the size of this
> structure to be larger than MIN_SIZE.
This test doesn't really make sense anymore, it's a leftover from a
previous iteration, I'll remove it.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols
2024-02-16 20:18 ` Michael Jeanson
@ 2024-02-16 20:29 ` DJ Delorie
0 siblings, 0 replies; 39+ messages in thread
From: DJ Delorie @ 2024-02-16 20:29 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
>> I assume the linker's constant (string?) merging won't affect this
>> because the section is marked as writable to the linker?
>
> I checked and the .data.rel.ro section does indeed have the SHF_WRITE
> flag set. My understanding is that the section would need to be
> read-only and the SHF_MERGE flag would need to be set for the linker to
> try merging things.
That's what I figured too, but good to confirm.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-16 20:19 ` Michael Jeanson
@ 2024-02-16 20:37 ` DJ Delorie
2024-02-16 21:10 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-16 20:37 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
>> The TLS doesn't get resized, does it? Or am I thinking of the DTV?
>
> The terminology is a bit confusing and could be clarified, my
> understanding is that the 'static TLS block' is the area where the 'TLS
> blocks' from shared objects and the main executable are allocated and
> the DTV is the array of descriptors that point to each of these 'TLS
> blocks' inside the 'static TLS block'.
I might be thinking of where TLS data for dlopen()'d objects go, after
the static TLS block is filled.
>> Isn't __rseq_size always at least 32 ?
>
> It's at least 32 when the registration succeeded, however if the
> registration failed or was disabled by tunable then it's set to 0.
Are we sure?
/* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
bytes when rseq is disabled by tunables. */
size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
bool do_rseq = true;
do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
if (do_rseq)
{
rseq_align = GLRO(dl_tls_rseq_align);
/* Make sure the rseq area size is at least the minimum ABI size and a
multiple of the requested aligment. */
rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
TLS_DL_RSEQ_MIN_SIZE), rseq_align);
}
static inline bool
rseq_register_current_thread (struct pthread *self, bool do_rseq)
{
if (do_rseq)
{
/* The kernel expects 'rseq_area->rseq_cs == NULL' on registration, zero
the whole rseq area. */
memset(rseq_get_area(), 0, GLRO (dl_tls_rseq_size));
int ret = INTERNAL_SYSCALL_CALL (rseq, rseq_get_area(),
GLRO (dl_tls_rseq_size),
0, RSEQ_SIG);
if (!INTERNAL_SYSCALL_ERROR_P (ret))
return true;
}
RSEQ_SETMEM (rseq_get_area(), cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
return false;
}
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-16 20:37 ` DJ Delorie
@ 2024-02-16 21:10 ` Michael Jeanson
2024-02-16 21:17 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-16 21:10 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-16 15 h 37, DJ Delorie wrote:
> Michael Jeanson <mjeanson@efficios.com> writes:
>>> The TLS doesn't get resized, does it? Or am I thinking of the DTV?
>>
>> The terminology is a bit confusing and could be clarified, my
>> understanding is that the 'static TLS block' is the area where the 'TLS
>> blocks' from shared objects and the main executable are allocated and
>> the DTV is the array of descriptors that point to each of these 'TLS
>> blocks' inside the 'static TLS block'.
>
> I might be thinking of where TLS data for dlopen()'d objects go, after
> the static TLS block is filled.
>
>>> Isn't __rseq_size always at least 32 ?
>>
>> It's at least 32 when the registration succeeded, however if the
>> registration failed or was disabled by tunable then it's set to 0.
>
> Are we sure?
>
> /* Default to the rseq ABI minimum sizes, this will reduce TLS usage to 32
> bytes when rseq is disabled by tunables. */
> size_t rseq_size = TLS_DL_RSEQ_MIN_SIZE;
> size_t rseq_align = TLS_DL_RSEQ_MIN_ALIGN;
> bool do_rseq = true;
> do_rseq = TUNABLE_GET_FULL (glibc, pthread, rseq, int, NULL);
> if (do_rseq)
> {
> rseq_align = GLRO(dl_tls_rseq_align);
> /* Make sure the rseq area size is at least the minimum ABI size and a
> multiple of the requested aligment. */
> rseq_size = roundup (MAX (GLRO(dl_tls_rseq_feature_size),
> TLS_DL_RSEQ_MIN_SIZE), rseq_align);
> }
>
>
> static inline bool
> rseq_register_current_thread (struct pthread *self, bool do_rseq)
> {
> if (do_rseq)
> {
> /* The kernel expects 'rseq_area->rseq_cs == NULL' on registration, zero
> the whole rseq area. */
> memset(rseq_get_area(), 0, GLRO (dl_tls_rseq_size));
> int ret = INTERNAL_SYSCALL_CALL (rseq, rseq_get_area(),
> GLRO (dl_tls_rseq_size),
> 0, RSEQ_SIG);
> if (!INTERNAL_SYSCALL_ERROR_P (ret))
> return true;
> }
> RSEQ_SETMEM (rseq_get_area(), cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
> return false;
> }
When rseq registration is disabled by tunable we still need to allocate
an rseq block because application code is allowed to check for the
registration status by reading 'rseq->cpu_id'. So in this case we
allocate the minimum ABI size of 32. Internally this is recorded in
'dl_tls_rseq_size' but the application visible '_rseq_size' is left at 0
(this happens in sysdeps/nptl/dl-tls_init_tp.c).
I'll try making this more clear with additional comments and finding a
name for 'rseq_size' that is not so close to '__rseq_size'.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-16 21:10 ` Michael Jeanson
@ 2024-02-16 21:17 ` DJ Delorie
2024-02-16 21:24 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-16 21:17 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
> When rseq registration is disabled by tunable we still need to allocate
> an rseq block because application code is allowed to check for the
> registration status by reading 'rseq->cpu_id'. So in this case we
> allocate the minimum ABI size of 32.
Ok, I admit that was confusing ;-)
> Internally this is recorded in 'dl_tls_rseq_size' but the application
> visible '_rseq_size' is left at 0 (this happens in
> sysdeps/nptl/dl-tls_init_tp.c).
A comment in sysdeps/nptl/dl-tls_init_tp.c after the if{} noting what
the variables would be otherwise might be useful. At least that was one
of the places I looked at when trying to figure this out.
> I'll try making this more clear with additional comments and finding a
> name for 'rseq_size' that is not so close to '__rseq_size'.
Perhaps "rseq_alloc_size" ?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 2/8] Add rseq extensible ABI support
2024-02-16 21:17 ` DJ Delorie
@ 2024-02-16 21:24 ` Michael Jeanson
0 siblings, 0 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-16 21:24 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-16 16 h 17, DJ Delorie wrote:
>
> Michael Jeanson <mjeanson@efficios.com> writes:
>> When rseq registration is disabled by tunable we still need to allocate
>> an rseq block because application code is allowed to check for the
>> registration status by reading 'rseq->cpu_id'. So in this case we
>> allocate the minimum ABI size of 32.
>
> Ok, I admit that was confusing ;-)
Yeah, I had to think for a minute to remember that, it should be documented.
>
>> Internally this is recorded in 'dl_tls_rseq_size' but the application
>> visible '_rseq_size' is left at 0 (this happens in
>> sysdeps/nptl/dl-tls_init_tp.c).
>
> A comment in sysdeps/nptl/dl-tls_init_tp.c after the if{} noting what
> the variables would be otherwise might be useful. At least that was one
> of the places I looked at when trying to figure this out.
Ack.
>
>> I'll try making this more clear with additional comments and finding a
>> name for 'rseq_size' that is not so close to '__rseq_size'.
>
> Perhaps "rseq_alloc_size" ?
Sounds good.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-06 16:27 ` [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol Michael Jeanson
@ 2024-02-16 22:07 ` DJ Delorie
2024-02-19 19:25 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-16 22:07 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mjeanson
* Suggestion for manual entry
* attributes on externs
* comment about _rseq_feature_size default
Michael Jeanson <mjeanson@efficios.com> writes:
> +RSEQ_FEATURE_SIZE_SIZE sizeof (unsigned int)
> +RSEQ_FEATURE_SIZE_ALIGN __alignof (unsigned int)
> +
Ok.
> diff --git a/elf/dl-rseq-symbols.S b/elf/dl-rseq-symbols.S
> index 2d8e88367f..709188ae22 100644
> --- a/elf/dl-rseq-symbols.S
> +++ b/elf/dl-rseq-symbols.S
> @@ -38,6 +38,23 @@ __rseq_size:
> _rseq_size:
> .zero RSEQ_SIZE_SIZE
>
> +/* Define 2 symbols, __rseq_feature_size is public const and
> + _rseq_feature_size, which is an alias of __rseq_feature_size, but hidden and
> + writable for internal use. */
> +
> + .globl __rseq_feature_size
> + .type __rseq_feature_size, %object
> + .size __rseq_feature_size, RSEQ_FEATURE_SIZE_SIZE
> + .hidden _rseq_feature_size
> + .globl _rseq_feature_size
> + .type _rseq_feature_size, %object
> + .size _rseq_feature_size, RSEQ_FEATURE_SIZE_SIZE
> + .section .data.rel.ro
> + .balign RSEQ_FEATURE_SIZE_ALIGN
> +__rseq_feature_size:
> +_rseq_feature_size:
> + .zero RSEQ_FEATURE_SIZE_SIZE
> +
Ok.
> diff --git a/manual/threads.texi b/manual/threads.texi
> +@deftypevar {unsigned int} __rseq_feature_size
> +@standards{Linux, sys/rseq.h}
> +This variable is either zero (if restartable sequence registration
> +failed or has been disabled) or the size of the restartable sequence
> +features. If registration is successful, @code{__rseq_feature_size}
> +is at least 20 (the initial feature size of @code{struct rseq}).
> +@end deftypevar
Perhaps a note about where the magic value '20' comes from, and how to
change it? (Yes, *I* know it comes from the kernel, but the reader
might not)
> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
> +extern unsigned int _rseq_feature_size attribute_relro attribute_hidden;
Again with attributes on externs.
> @@ -108,6 +109,7 @@ __tls_init_tp (void)
> if (rseq_register_current_thread (pd, do_rseq))
> {
> _rseq_size = GLRO (dl_tls_rseq_size);
> + _rseq_feature_size = GLRO (dl_tls_rseq_feature_size);
> }
Ok. Note "comment needed here" from our other mail thread ;-)
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> + GLIBC_2.40 {
> + __rseq_feature_size;
> + }
Ok.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/ld.abilist b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
> index 5151c0781d..93039b756d 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/aarch64/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/alpha/ld.abilist b/sysdeps/unix/sysv/linux/alpha/ld.abilist
> index 3e296c5473..fc67e31293 100644
> --- a/sysdeps/unix/sysv/linux/alpha/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/alpha/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x8
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/arc/ld.abilist b/sysdeps/unix/sysv/linux/arc/ld.abilist
> index 55f0c2ab9c..31156b6ee1 100644
> --- a/sysdeps/unix/sysv/linux/arc/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/arc/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/arm/be/ld.abilist b/sysdeps/unix/sysv/linux/arm/be/ld.abilist
> index f1da2c636d..3d16fa60db 100644
> --- a/sysdeps/unix/sysv/linux/arm/be/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/arm/be/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.4 __stack_chk_guard D 0x4
> GLIBC_2.4 __tls_get_addr F
> GLIBC_2.4 _dl_mcount F
> GLIBC_2.4 _r_debug D 0x14
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/arm/le/ld.abilist b/sysdeps/unix/sysv/linux/arm/le/ld.abilist
> index f1da2c636d..3d16fa60db 100644
> --- a/sysdeps/unix/sysv/linux/arm/le/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/arm/le/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.4 __stack_chk_guard D 0x4
> GLIBC_2.4 __tls_get_addr F
> GLIBC_2.4 _dl_mcount F
> GLIBC_2.4 _r_debug D 0x14
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/csky/ld.abilist b/sysdeps/unix/sysv/linux/csky/ld.abilist
> index 7f482276ed..088f000c4e 100644
> --- a/sysdeps/unix/sysv/linux/csky/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/csky/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/hppa/ld.abilist b/sysdeps/unix/sysv/linux/hppa/ld.abilist
> index 7f5527fb30..8ae26c46c1 100644
> --- a/sysdeps/unix/sysv/linux/hppa/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/hppa/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/i386/ld.abilist b/sysdeps/unix/sysv/linux/i386/ld.abilist
> index 9c4a45d8dc..d9761c34e3 100644
> --- a/sysdeps/unix/sysv/linux/i386/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/i386/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist b/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist
> index 93fcd64eee..f5dbb17ec9 100644
> --- a/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/loongarch/lp64/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.36 __stack_chk_guard D 0x8
> GLIBC_2.36 __tls_get_addr F
> GLIBC_2.36 _dl_mcount F
> GLIBC_2.36 _r_debug D 0x28
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist
> index f1da2c636d..3d16fa60db 100644
> --- a/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/m68k/coldfire/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.4 __stack_chk_guard D 0x4
> GLIBC_2.4 __tls_get_addr F
> GLIBC_2.4 _dl_mcount F
> GLIBC_2.4 _r_debug D 0x14
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist
> index dadbf852d0..3888e39812 100644
> --- a/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/m68k/m680x0/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/microblaze/ld.abilist b/sysdeps/unix/sysv/linux/microblaze/ld.abilist
> index 89a0b7e4fd..c83b62dce7 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/microblaze/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist b/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist
> index e304d1bb46..9710fdb941 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist
> index e304d1bb46..9710fdb941 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist
> index f26e594a13..ec1bdfd965 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x8
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/nios2/ld.abilist b/sysdeps/unix/sysv/linux/nios2/ld.abilist
> index 811ae9da2f..d2b742ec25 100644
> --- a/sysdeps/unix/sysv/linux/nios2/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/nios2/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/or1k/ld.abilist b/sysdeps/unix/sysv/linux/or1k/ld.abilist
> index cff2ffd23b..eb225ca4c5 100644
> --- a/sysdeps/unix/sysv/linux/or1k/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/or1k/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.35 __stack_chk_guard D 0x4
> GLIBC_2.35 __tls_get_addr F
> GLIBC_2.35 _dl_mcount F
> GLIBC_2.35 _r_debug D 0x14
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
> index b1073f0942..7a94751723 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
> @@ -9,3 +9,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.39 __parse_hwcap_3_4_and_convert_at_platform F
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist
> index 40942a2cc6..f6bdd89083 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/ld.abilist
> @@ -9,3 +9,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.39 __parse_hwcap_3_4_and_convert_at_platform F
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist
> index 01f2694a4d..672bade9ba 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ld.abilist
> @@ -9,3 +9,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.39 __parse_hwcap_3_4_and_convert_at_platform F
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist
> index 068368878e..d3be9236c2 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv32/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist
> index a7758a0e52..46b22238ce 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv64/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist
> index c15288394a..f438808c08 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist
> index 78d071600b..f5fbc6de43 100644
> --- a/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/s390/s390-64/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sh/be/ld.abilist b/sysdeps/unix/sysv/linux/sh/be/ld.abilist
> index 7f5527fb30..8ae26c46c1 100644
> --- a/sysdeps/unix/sysv/linux/sh/be/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/sh/be/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sh/le/ld.abilist b/sysdeps/unix/sysv/linux/sh/le/ld.abilist
> index 7f5527fb30..8ae26c46c1 100644
> --- a/sysdeps/unix/sysv/linux/sh/le/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/sh/le/ld.abilist
> @@ -7,3 +7,4 @@ GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> GLIBC_2.4 __stack_chk_guard D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist
> index 3aac73f3df..df3f7fefb3 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist
> index 5471b24d59..3bf61e1210 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h b/sysdeps/unix/sysv/linux/sys/rseq.h
> index b8afab8945..a048ba8958 100644
> --- a/sysdeps/unix/sysv/linux/sys/rseq.h
> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
> @@ -163,6 +163,10 @@ extern const ptrdiff_t __rseq_offset;
> unsuccessful. */
> extern const unsigned int __rseq_size;
>
> +/* Size of the registered rseq features. 0 if the registration was
> + unsuccessful. */
> +extern const unsigned int __rseq_feature_size;
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq-disable.c b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
> index ae4143916b..eb4b6278dc 100644
> --- a/sysdeps/unix/sysv/linux/tst-rseq-disable.c
> +++ b/sysdeps/unix/sysv/linux/tst-rseq-disable.c
> @@ -39,6 +39,7 @@ check_rseq_disabled (void)
>
> TEST_COMPARE (__rseq_flags, 0);
> TEST_COMPARE (__rseq_size, 0);
> + TEST_COMPARE (__rseq_feature_size, 0);
Ok.
> diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
> index c8c0518a5d..c5d9afbb0a 100644
> --- a/sysdeps/unix/sysv/linux/tst-rseq.c
> +++ b/sysdeps/unix/sysv/linux/tst-rseq.c
> @@ -38,12 +38,14 @@ static void
> do_rseq_main_test (void)
> {
> size_t rseq_align = MAX (getauxval (AT_RSEQ_ALIGN), RSEQ_TEST_MIN_ALIGN);
> - size_t rseq_size = roundup (MAX (getauxval (AT_RSEQ_FEATURE_SIZE), RSEQ_TEST_MIN_SIZE), rseq_align);
> + size_t rseq_feature_size = MAX (getauxval (AT_RSEQ_FEATURE_SIZE), RSEQ_TEST_MIN_FEATURE_SIZE);
> + size_t rseq_size = roundup (MAX (rseq_feature_size, RSEQ_TEST_MIN_SIZE), rseq_align);
Ok.
> struct rseq *rseq = __thread_pointer () + __rseq_offset;
>
> TEST_VERIFY_EXIT (rseq_thread_registered ());
> TEST_COMPARE (__rseq_flags, 0);
> TEST_COMPARE (__rseq_size, rseq_size);
> + TEST_COMPARE (__rseq_feature_size, rseq_feature_size);
Ok.
> diff --git a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
> index 5a8bd322cd..25c2153c21 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x8
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
> index e17496d124..05327004e2 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
> @@ -6,3 +6,4 @@ GLIBC_2.34 __rtld_version_placeholder F
> GLIBC_2.35 __rseq_flags D 0x4
> GLIBC_2.35 __rseq_offset D 0x4
> GLIBC_2.35 __rseq_size D 0x4
> +GLIBC_2.40 __rseq_feature_size D 0x4
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 4/8] nptl: Add features to internal 'struct rseq_area'
2024-02-06 16:27 ` [PATCH v8 4/8] nptl: Add features to internal 'struct rseq_area' Michael Jeanson
@ 2024-02-17 2:04 ` DJ Delorie
0 siblings, 0 replies; 39+ messages in thread
From: DJ Delorie @ 2024-02-17 2:04 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mjeanson
Michael Jeanson <mjeanson@efficios.com> writes:
> Access to features following the original rseq ABI 20 bytes (after
> 'flags') starting with 'node_id' must be gated by an rseq feature size
> test.
Are there any kernels that still provide auxvals that result in a 8 byte
rseq area? I.e. perhaps an older kernel that doesn't support the
feature area at all?
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> index b6c9deea6b..a63e4afbdb 100644
> --- a/sysdeps/unix/sysv/linux/rseq-internal.h
> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
> @@ -28,13 +28,17 @@
> #include <ldsodefs.h>
>
> /* rseq area registered with the kernel. Use a custom definition
> - here to isolate from kernel struct rseq changes. The
> - implementation of sched_getcpu needs acccess to the cpu_id field;
> - the other fields are unused and not included here. */
> + here to isolate from kernel struct rseq changes. Access to fields
> + beyond the 20 bytes of the original ABI (after 'flags') must be gated
> + by a check of the feature size. */
If the answer to the first question is "yes" then this should be changed
to say "8 bytes" etc.
> struct rseq_area
> {
> uint32_t cpu_id_start;
> uint32_t cpu_id;
> + uint64_t rseq_cs;
> + uint32_t flags;
> + uint32_t node_id;
> + uint32_t mm_cid;
> };
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 5/8] nptl: Add rseq internal utils
2024-02-06 16:27 ` [PATCH v8 5/8] nptl: Add rseq internal utils Michael Jeanson
@ 2024-02-17 2:33 ` DJ Delorie
2024-02-19 20:25 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-17 2:33 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mjeanson, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
> Implement internal rseq utils in preparation for their use in
> rseq_load32_load32_relaxed().
>
> This implementation is imported from the librseq project.
*Which* librseq project, and what are the copyright/license terms of
that project? I assume this has been discussed and approved before, but
for archival reasons, that URL should be listed here explicitly, along
with some note of the copyright clearance.
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> +/*
> + * gcc prior to 4.8.2 miscompiles asm goto.
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
> + *
> + * gcc prior to 8.1.0 miscompiles asm goto at O1.
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908
> + *
> + * clang prior to version 13.0.1 miscompiles asm goto at O2.
> + * https://github.com/llvm/llvm-project/issues/52735
> + *
> + * Work around these issues by adding a volatile inline asm with
> + * memory clobber in the fallthrough after the asm goto and at each
> + * label target. Emit this for all compilers in case other similar
> + * issues are found in the future.
> + */
> +#define rseq_after_asm_goto() __asm__ __volatile__ ("" : : : "memory")
Ok.
> +#define __rseq_str_1(x) #x
> +#define __rseq_str(x) __rseq_str_1(x)
Ok.
> +/* Offset of rseq_cs field in struct rseq. */
> +#define RSEQ_CS_OFFSET 8
Ok.
> +#define rseq_sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
> +#define rseq_offsetofend(TYPE, MEMBER) \
> + (offsetof(TYPE, MEMBER) + rseq_sizeof_field(TYPE, MEMBER))
Technically offsetofjustafterend, but ok ;-)
> +/* Returns the int value of 'rseq_area->cpu_id'. */
> +static inline int
> +rseq_get_cpu_id(void)
> +{
> + return (int) RSEQ_GETMEM_VOLATILE (rseq_get_area(), cpu_id);
> +}
Ok.
> +/* Returns true if the rseq registration is active. */
> +static inline bool
> +rseq_is_registered(void)
> +{
> + return rseq_get_cpu_id() >= 0;
> +}
Ok, REGISTRATION_FAILED is -2
> +/* Returns true if the current rseq registration has the 'node_id' field. */
> +static inline bool
> +rseq_node_id_available(void)
> +{
> + return __rseq_feature_size >= rseq_offsetofend(struct rseq_area, node_id);
> +}
Ok.
> +/* Returns true if the current rseq registration has the 'mm_cid' field. */
> +static inline bool
> +rseq_mm_cid_available(void)
> +{
> + return __rseq_feature_size >= rseq_offsetofend(struct rseq_area, mm_cid);
> +}
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed
2024-02-06 16:27 ` [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed Michael Jeanson
@ 2024-02-17 3:08 ` DJ Delorie
2024-02-19 20:27 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-17 3:08 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers, mjeanson
* copyright year
* docs?
Michael Jeanson <mjeanson@efficios.com> writes:
> This implementation is imported from the librseq project.
Same comment as [5/8]
> diff --git a/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h b/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
> new file mode 100644
> index 0000000000..fdca1b6439
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
> @@ -0,0 +1,109 @@
> +/* Restartable Sequences internal API. x86_64 macros.
> + Copyright (C) 2023 Free Software Foundation, Inc.
Year? Or does this stay the same if it's a pure copy of somewhere else...
> +#include <sysdeps/unix/sysv/linux/rseq-internal.h>
> +
> +#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
> + start_ip, post_commit_offset, abort_ip) \
> + ".pushsection __rseq_cs, \"aw\"\n\t" \
> + ".balign 32\n\t" \
> + __rseq_str(label) ":\n\t" \
> + ".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
> + ".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
> + ".popsection\n\t" \
> + ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
> + ".quad " __rseq_str(label) "b\n\t" \
> + ".popsection\n\t"
Ok, but each one of these needs some documentation on when/how/why to
use them, even if just a comment that says "only used below".
> +#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
> + __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
> + (post_commit_ip - start_ip), abort_ip)
Ok.
> +/*
> + * Exit points of a rseq critical section consist of all instructions outside
> + * of the critical section where a critical section can either branch to or
> + * reach through the normal course of its execution. The abort IP and the
> + * post-commit IP are already part of the __rseq_cs section and should not be
> + * explicitly defined as additional exit points. Knowing all exit points is
> + * useful to assist debuggers stepping over the critical section.
> + */
> +#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
> + ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
> + ".quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n\t" \
> + ".popsection\n\t"
Ok.
> +#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
> + "leaq " __rseq_str(cs_label) "(%%rip), %%rax\n\t" \
> + "movq %%rax, " __rseq_str(rseq_cs) "\n\t" \
> + __rseq_str(label) ":\n\t"
Ok.
> +#define RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, label) \
> + "cmpl %[" __rseq_str(cpu_id) "], " __rseq_str(current_cpu_id) "\n\t" \
> + "jnz " __rseq_str(label) "\n\t"
Ok.
> +#define RSEQ_ASM_DEFINE_ABORT(label, teardown, abort_label) \
> + ".pushsection __rseq_failure, \"ax\"\n\t" \
> + /* Disassembler-friendly signature: ud1 <sig>(%rip),%edi. */ \
> + ".byte 0x0f, 0xb9, 0x3d\n\t" \
> + ".long " __rseq_str(RSEQ_SIG) "\n\t" \
> + __rseq_str(label) ":\n\t" \
> + teardown \
> + "jmp %l[" __rseq_str(abort_label) "]\n\t" \
> + ".popsection\n\t"
Ok.
> +#define RSEQ_ASM_DEFINE_CMPFAIL(label, teardown, cmpfail_label) \
> + ".pushsection __rseq_failure, \"ax\"\n\t" \
> + __rseq_str(label) ":\n\t" \
> + teardown \
> + "jmp %l[" __rseq_str(cmpfail_label) "]\n\t" \
> + ".popsection\n\t"
Ok.
> +/*
> + * Load @src1 (32-bit) into @dst1 and load @src2 (32-bit) into @dst2.
> + */
> +#define RSEQ_HAS_LOAD32_LOAD32_RELAXED 1
> +static __always_inline int
> +rseq_load32_load32_relaxed(uint32_t *dst1, uint32_t *src1,
> + uint32_t *dst2, uint32_t *src2)
> +{
> + __asm__ __volatile__ goto (
> + RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
> + /* Start rseq by storing table entry pointer into rseq_cs. */
> + RSEQ_ASM_STORE_RSEQ_CS(1, 3b, %%fs:RSEQ_CS_OFFSET(%[rseq_offset]))
> + "movl %[src1], %%ebx\n\t"
> + "movl %[src2], %%ecx\n\t"
> + "movl %%ebx, %[dst1]\n\t"
> + /* final store */
> + "movl %%ecx, %[dst2]\n\t"
> + "2:\n\t"
> + RSEQ_ASM_DEFINE_ABORT(4, "", abort)
> + : /* gcc asm goto does not allow outputs */
> + : [rseq_offset] "r" (__rseq_offset),
> + /* final store input */
> + [dst1] "m" (*dst1),
> + [dst2] "m" (*dst2),
> + [src1] "m" (*src1),
> + [src2] "m" (*src2)
> + : "memory", "cc", "ebx", "ecx", "rax"
> + : abort
> + );
> + rseq_after_asm_goto();
> + return 0;
> +abort:
> + rseq_after_asm_goto();
> + return -1;
> +}
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 7/8] aarch64: Add rseq_load32_load32_relaxed
2024-02-06 16:28 ` [PATCH v8 7/8] aarch64: " Michael Jeanson
@ 2024-02-17 3:53 ` DJ Delorie
2024-02-19 20:29 ` Michael Jeanson
2024-02-20 15:07 ` Mathieu Desnoyers
0 siblings, 2 replies; 39+ messages in thread
From: DJ Delorie @ 2024-02-17 3:53 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers, mjeanson
Michael Jeanson <mjeanson@efficios.com> writes:
> This implementation is imported from the librseq project.
Same comments as [6/8] wrt origin URL
> diff --git a/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h b/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h
> + Copyright (C) 2023 Free Software Foundation, Inc.
Year?
> +#define RSEQ_ASM_TMP_REG32 "w15"
> +#define RSEQ_ASM_TMP_REG "x15"
> +#define RSEQ_ASM_TMP_REG_2 "x14"
> +
> +#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, start_ip, \
> + post_commit_offset, abort_ip) \
> + " .pushsection __rseq_cs, \"aw\"\n" \
> + " .balign 32\n" \
> + __rseq_str(label) ":\n" \
> + " .long " __rseq_str(version) ", " __rseq_str(flags) "\n" \
> + " .quad " __rseq_str(start_ip) ", " \
> + __rseq_str(post_commit_offset) ", " \
> + __rseq_str(abort_ip) "\n" \
> + " .popsection\n\t" \
> + " .pushsection __rseq_cs_ptr_array, \"aw\"\n" \
> + " .quad " __rseq_str(label) "b\n" \
> + " .popsection\n"
Ok.
> +#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
> + __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
> + (post_commit_ip - start_ip), abort_ip)
Ok.
> +/*
> + * Exit points of a rseq critical section consist of all instructions outside
> + * of the critical section where a critical section can either branch to or
> + * reach through the normal course of its execution. The abort IP and the
> + * post-commit IP are already part of the __rseq_cs section and should not be
> + * explicitly defined as additional exit points. Knowing all exit points is
> + * useful to assist debuggers stepping over the critical section.
> + */
> +#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
> + " .pushsection __rseq_exit_point_array, \"aw\"\n" \
> + " .quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n" \
> + " .popsection\n"
Ok.
> +#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
> + " adrp " RSEQ_ASM_TMP_REG ", " __rseq_str(cs_label) "\n" \
> + " add " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
> + ", :lo12:" __rseq_str(cs_label) "\n" \
> + " str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(rseq_cs) "]\n" \
> + __rseq_str(label) ":\n"
Ok.
> +#define RSEQ_ASM_DEFINE_ABORT(label, abort_label) \
> + " b 222f\n" \
> + " .inst " __rseq_str(RSEQ_SIG_CODE) "\n" \
> + __rseq_str(label) ":\n" \
> + " b %l[" __rseq_str(abort_label) "]\n" \
> + "222:\n"
Ok.
> +#define RSEQ_ASM_OP_STORE(value, var) \
> + " str %[" __rseq_str(value) "], %[" __rseq_str(var) "]\n"
Ok.
> +#define RSEQ_ASM_OP_STORE_RELEASE(value, var) \
> + " stlr %[" __rseq_str(value) "], %[" __rseq_str(var) "]\n"
Ok.
> +#define RSEQ_ASM_OP_FINAL_STORE(value, var, post_commit_label) \
> + RSEQ_ASM_OP_STORE(value, var) \
> + __rseq_str(post_commit_label) ":\n"
Ok.
> +#define RSEQ_ASM_OP_FINAL_STORE_RELEASE(value, var, post_commit_label) \
> + RSEQ_ASM_OP_STORE_RELEASE(value, var) \
> + __rseq_str(post_commit_label) ":\n"
Ok.
> +#define RSEQ_ASM_OP_CMPEQ(var, expect, label) \
> + " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
> + " sub " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
> + ", %[" __rseq_str(expect) "]\n" \
> + " cbnz " RSEQ_ASM_TMP_REG ", " __rseq_str(label) "\n"
This is why we need documentation; I would have guessed this was a CMPNE
operation, but it depends on how you define "label"
> +#define RSEQ_ASM_OP_CMPEQ32(var, expect, label) \
> + " ldr " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n" \
> + " sub " RSEQ_ASM_TMP_REG32 ", " RSEQ_ASM_TMP_REG32 \
> + ", %w[" __rseq_str(expect) "]\n" \
> + " cbnz " RSEQ_ASM_TMP_REG32 ", " __rseq_str(label) "\n"
Ok.
> +#define RSEQ_ASM_OP_CMPNE(var, expect, label) \
> + " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
> + " sub " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
> + ", %[" __rseq_str(expect) "]\n" \
> + " cbz " RSEQ_ASM_TMP_REG ", " __rseq_str(label) "\n"
And of course this one is the opposite way ;-)
> +#define RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, label) \
> + RSEQ_ASM_OP_CMPEQ32(current_cpu_id, cpu_id, label)
Ok.
> +#define RSEQ_ASM_OP_R_LOAD(var) \
> + " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n"
Ok.
> +#define RSEQ_ASM_OP_R_STORE(var) \
> + " str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n"
Ok.
> +#define RSEQ_ASM_OP_R_LOAD32(var) \
> + " ldr " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n"
Ok.
> +#define RSEQ_ASM_OP_R_STORE32(var) \
> + " str " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n"
Ok.
> +#define RSEQ_ASM_OP_R_LOAD_OFF(offset) \
> + " ldr " RSEQ_ASM_TMP_REG ", [" RSEQ_ASM_TMP_REG \
> + ", %[" __rseq_str(offset) "]]\n"
Ok.
> +#define RSEQ_ASM_OP_R_ADD(count) \
> + " add " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
> + ", %[" __rseq_str(count) "]\n"
Ok.
> +#define RSEQ_ASM_OP_R_FINAL_STORE(var, post_commit_label) \
> + " str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
> + __rseq_str(post_commit_label) ":\n"
Ok.
> +#define RSEQ_ASM_OP_R_FINAL_STORE32(var, post_commit_label) \
> + " str " RSEQ_ASM_TMP_REG32 ", %[" __rseq_str(var) "]\n" \
> + __rseq_str(post_commit_label) ":\n"
Ok.
> +#define RSEQ_ASM_OP_R_BAD_MEMCPY(dst, src, len) \
> + " cbz %[" __rseq_str(len) "], 333f\n" \
> + " mov " RSEQ_ASM_TMP_REG_2 ", %[" __rseq_str(len) "]\n" \
> + "222: sub " RSEQ_ASM_TMP_REG_2 ", " RSEQ_ASM_TMP_REG_2 ", #1\n" \
> + " ldrb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(src) "]" \
> + ", " RSEQ_ASM_TMP_REG_2 "]\n" \
> + " strb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(dst) "]" \
> + ", " RSEQ_ASM_TMP_REG_2 "]\n" \
> + " cbnz " RSEQ_ASM_TMP_REG_2 ", 222b\n" \
> + "333:\n"
Ok, but WHY?
> +/*
> + * Load @src1 (32-bit) into @dst1 and load @src2 (32-bit) into @dst2.
> + */
> +#define RSEQ_HAS_LOAD32_LOAD32_RELAXED 1
> +static __always_inline int
> +rseq_load32_load32_relaxed(uint32_t *dst1, uint32_t *src1,
> + uint32_t *dst2, uint32_t *src2)
> +{
> + __asm__ __volatile__ goto (
> + RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
> + RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
> + RSEQ_ASM_OP_R_LOAD32(src1)
> + RSEQ_ASM_OP_R_STORE32(dst1)
> + RSEQ_ASM_OP_R_LOAD32(src2)
> + RSEQ_ASM_OP_R_FINAL_STORE32(dst2, 3)
> + RSEQ_ASM_DEFINE_ABORT(4, abort)
> + : /* gcc asm goto does not allow outputs */
> + : [rseq_cs] "m" (rseq_get_area()->rseq_cs),
> + [dst1] "Qo" (*dst1),
> + [dst2] "Qo" (*dst2),
> + [src1] "Qo" (*src1),
> + [src2] "Qo" (*src2)
> + : "memory", RSEQ_ASM_TMP_REG
> + : abort
> + );
> + rseq_after_asm_goto();
> + return 0;
> +abort:
> + rseq_after_asm_goto();
> + return -1;
> +}
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu
2024-02-06 16:28 ` [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu Michael Jeanson
@ 2024-02-17 3:57 ` DJ Delorie
2024-02-19 22:14 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-17 3:57 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mjeanson, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
> diff --git a/sysdeps/unix/sysv/linux/getcpu.c b/sysdeps/unix/sysv/linux/getcpu.c
> index 0e7c3238c9..7e34d6d1eb 100644
> --- a/sysdeps/unix/sysv/linux/getcpu.c
> +++ b/sysdeps/unix/sysv/linux/getcpu.c
> @@ -19,9 +19,10 @@
> #include <sched.h>
> #include <sysdep.h>
> #include <sysdep-vdso.h>
> +#include <rseq-internal.h>
>
> -int
> -__getcpu (unsigned int *cpu, unsigned int *node)
> +static int
> +vsyscall_getcpu (unsigned int *cpu, unsigned int *node)
Ok; this is the original function. Any reason (other than "cleaner
diffs") to have this separate, and not just make it the #else clause
below?
Ah, it's called twice below. Got it. Ok. Do we want to tag this as
always inline for the better CPU performance?
> @@ -29,5 +30,32 @@ __getcpu (unsigned int *cpu, unsigned int *node)
> +#if defined (RSEQ_SIG) && defined (RSEQ_HAS_LOAD32_LOAD32_RELAXED)
> +int
> +__getcpu (unsigned int *cpu, unsigned int *node)
> +{
> + /* Check if rseq is registered and the node_id feature is available. */
> + if (__glibc_likely (rseq_node_id_available()))
> + {
> + struct rseq_area *rseq_area = rseq_get_area();
> +
> + if (rseq_load32_load32_relaxed(cpu, &rseq_area->cpu_id,
> + node, &rseq_area->node_id) == 0)
> + {
> + /* The critical section was not aborted, return 0. */
> + return 0;
> + }
> + }
> +
> + return vsyscall_getcpu (cpu, node);
> +}
> +#else
> +int
> +__getcpu (unsigned int *cpu, unsigned int *node)
> +{
> + return vsyscall_getcpu (cpu, node);
> +}
> +#endif
> weak_alias (__getcpu, getcpu)
> libc_hidden_def (__getcpu)
Ok.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-16 22:07 ` DJ Delorie
@ 2024-02-19 19:25 ` Michael Jeanson
2024-02-19 19:43 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 19:25 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, Mathieu Desnoyers
On 2024-02-16 17:07, DJ Delorie wrote:
>
> * Suggestion for manual entry
> * attributes on externs
> * comment about _rseq_feature_size default
>
>> diff --git a/manual/threads.texi b/manual/threads.texi
>
>> +@deftypevar {unsigned int} __rseq_feature_size
>> +@standards{Linux, sys/rseq.h}
>> +This variable is either zero (if restartable sequence registration
>> +failed or has been disabled) or the size of the restartable sequence
>> +features. If registration is successful, @code{__rseq_feature_size}
>> +is at least 20 (the initial feature size of @code{struct rseq}).
>> +@end deftypevar
>
> Perhaps a note about where the magic value '20' comes from, and how to
> change it? (Yes, *I* know it comes from the kernel, but the reader
> might not)
I'll add an explaination on where the magic value comes from.
Not sure about the 'change it' part, this minimum value is in the ABI and
should never change. Do you mean explaining why the 'feature_size' would change?
>> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
>> +extern unsigned int _rseq_feature_size attribute_relro attribute_hidden;
>
> Again with attributes on externs.
Ack.
>> @@ -108,6 +109,7 @@ __tls_init_tp (void)
>> if (rseq_register_current_thread (pd, do_rseq))
>> {
>> _rseq_size = GLRO (dl_tls_rseq_size);
>> + _rseq_feature_size = GLRO (dl_tls_rseq_feature_size);
>> }
>
> Ok. Note "comment needed here" from our other mail thread ;-)
Ack.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 19:25 ` Michael Jeanson
@ 2024-02-19 19:43 ` DJ Delorie
2024-02-19 20:15 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-19 19:43 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
> Not sure about the 'change it' part, this minimum value is in the ABI and
> should never change. Do you mean explaining why the 'feature_size' would change?
Dunno, I'm not familiar enough with the kernel side of this. *Is* it
something the user can change? Does it depend on kernel versions? Is
there some way of adding a user-specified region to this?
I was more worried about "magic numbers" than what they mean.
As for "it's ABI", the ABI used to have only 8 bytes in that struct ;-)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 19:43 ` DJ Delorie
@ 2024-02-19 20:15 ` Michael Jeanson
2024-02-19 20:24 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 20:15 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-19 14:43, DJ Delorie wrote:
> Michael Jeanson <mjeanson@efficios.com> writes:
>> Not sure about the 'change it' part, this minimum value is in the ABI and
>> should never change. Do you mean explaining why the 'feature_size' would change?
>
> Dunno, I'm not familiar enough with the kernel side of this. *Is* it
> something the user can change? Does it depend on kernel versions? Is
> there some way of adding a user-specified region to this?
>
> I was more worried about "magic numbers" than what they mean.
>
> As for "it's ABI", the ABI used to have only 8 bytes in that struct ;-)
Ah ok I think I understand the confusion here.
The original implementation of rseq in linux v4.18 had a fixed size 'struct
rseq' of 32 bytes and used 20 of those bytes, that's what we refer to as the
'original ABI'.
In linux v6.3, the 'Extensible rseq ABI' was merged which added the feature
size auxval and also added two 4 bytes fields, 'mm_cid' and 'node_id' bringing
the feature size to 28.
Currently a call to getauxval should either return 0 or 28 until new features
are added. If someone were to backport the feature size auxval to kernels
between v4.18 and v6.3 it should return 20.
Thus access to any fields past 20 bytes have to be gated by a check of the
feature size.
The initial glibc implementation of rseq used an internal 'struct rseq' of 8
bytes because it only needed to access the first two fields of that structure.
Users have no control over those values, they entirely depend on the currently
running kernel and the rseq features it implements.
That's why we floor the 'rseq_feature_size' to 20 when the auxval returns 0.
I hope this clarifies things, I'm unsure if this should be documented and if
so, where?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 20:15 ` Michael Jeanson
@ 2024-02-19 20:24 ` DJ Delorie
2024-02-19 22:06 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-19 20:24 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Ok, I'm clarified :-)
Perhaps some wording that the number comes from the kernel?
> If registration is successful, @code{__rseq_feature_size} is at least
> 20 (the initial feature size of @code{struct rseq}).
becomes
> If registration is successful, @code{__rseq_feature_size} is as
> reported by the kernel, and is at least 20 (the smallest @code{struct
> rseq} the kernel has ever supported).
?
It removes any ambiguity about the origin of the number and if it's
something the user has local control over.
Or perhaps change
> or the size of the restartable sequence features.
To
> or the size of the restartable sequence features supported by the
> running kernel.
?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 5/8] nptl: Add rseq internal utils
2024-02-17 2:33 ` DJ Delorie
@ 2024-02-19 20:25 ` Michael Jeanson
0 siblings, 0 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 20:25 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-16 21:33, DJ Delorie wrote:
> Michael Jeanson <mjeanson@efficios.com> writes:
>> Implement internal rseq utils in preparation for their use in
>> rseq_load32_load32_relaxed().
>>
>> This implementation is imported from the librseq project.
>
> *Which* librseq project, and what are the copyright/license terms of
> that project? I assume this has been discussed and approved before, but
> for archival reasons, that URL should be listed here explicitly, along
> with some note of the copyright clearance.
For reference, here is the URL to the librseq project :
https://git.kernel.org/pub/scm/libs/librseq/librseq.git
I'll double check the copyright clearance and licensing before sending the
next patchset.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed
2024-02-17 3:08 ` DJ Delorie
@ 2024-02-19 20:27 ` Michael Jeanson
0 siblings, 0 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 20:27 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-16 22:08, DJ Delorie wrote:
>
> * copyright year
> * docs?
>
> Michael Jeanson <mjeanson@efficios.com> writes:
>> This implementation is imported from the librseq project.
>
> Same comment as [5/8]
Ack.
>
>> diff --git a/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h b/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
>> new file mode 100644
>> index 0000000000..fdca1b6439
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86_64/rseq-internal.h
>> @@ -0,0 +1,109 @@
>> +/* Restartable Sequences internal API. x86_64 macros.
>> + Copyright (C) 2023 Free Software Foundation, Inc.
>
> Year? Or does this stay the same if it's a pure copy of somewhere else...
Again, will clarify before next patchset.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 7/8] aarch64: Add rseq_load32_load32_relaxed
2024-02-17 3:53 ` DJ Delorie
@ 2024-02-19 20:29 ` Michael Jeanson
2024-02-20 15:07 ` Mathieu Desnoyers
1 sibling, 0 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 20:29 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-16 22:53, DJ Delorie wrote:
> Michael Jeanson <mjeanson@efficios.com> writes:
>> This implementation is imported from the librseq project.
>
> Same comments as [6/8] wrt origin URL
Ack.
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h b/sysdeps/unix/sysv/linux/aarch64/rseq-internal.h
>> + Copyright (C) 2023 Free Software Foundation, Inc.
>
> Year?
Again, will clarify before next patchset.
I'll let Mathieu answer the macros / assembly questions.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 20:24 ` DJ Delorie
@ 2024-02-19 22:06 ` Michael Jeanson
2024-02-19 22:08 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 22:06 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-19 15:24, DJ Delorie wrote:
>
> Ok, I'm clarified :-)
>
> Perhaps some wording that the number comes from the kernel?
>
>> If registration is successful, @code{__rseq_feature_size} is at least
>> 20 (the initial feature size of @code{struct rseq}).
>
> becomes
>
>> If registration is successful, @code{__rseq_feature_size} is as
>> reported by the kernel, and is at least 20 (the smallest @code{struct
>> rseq} the kernel has ever supported).
>
> ?
>
> It removes any ambiguity about the origin of the number and if it's
> something the user has local control over.
>
> Or perhaps change
>
>> or the size of the restartable sequence features.
>
> To
>
>> or the size of the restartable sequence features supported by the
>> running kernel.
>
> ?
Would this make sense?
This variable is either zero (if restartable sequence registration
failed or has been disabled) or the size of the restartable sequence
features supported by the running kernel. If registration is
successful, @code{__rseq_feature_size} is at least 20 (the feature size of
@code{struct rseq} in the initial rseq kernel implementation).
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 22:06 ` Michael Jeanson
@ 2024-02-19 22:08 ` DJ Delorie
2024-02-19 22:18 ` Michael Jeanson
0 siblings, 1 reply; 39+ messages in thread
From: DJ Delorie @ 2024-02-19 22:08 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
> Would this make sense?
Yup!
> This variable is either zero (if restartable sequence registration
> failed or has been disabled) or the size of the restartable sequence
> features supported by the running kernel. If registration is
> successful, @code{__rseq_feature_size} is at least 20 (the feature size of
> @code{struct rseq} in the initial rseq kernel implementation).
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu
2024-02-17 3:57 ` DJ Delorie
@ 2024-02-19 22:14 ` Michael Jeanson
0 siblings, 0 replies; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 22:14 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-16 22:57, DJ Delorie wrote:
> Michael Jeanson <mjeanson@efficios.com> writes:
>> diff --git a/sysdeps/unix/sysv/linux/getcpu.c b/sysdeps/unix/sysv/linux/getcpu.c
>> index 0e7c3238c9..7e34d6d1eb 100644
>> --- a/sysdeps/unix/sysv/linux/getcpu.c
>> +++ b/sysdeps/unix/sysv/linux/getcpu.c
>> @@ -19,9 +19,10 @@
>> #include <sched.h>
>> #include <sysdep.h>
>> #include <sysdep-vdso.h>
>> +#include <rseq-internal.h>
>>
>> -int
>> -__getcpu (unsigned int *cpu, unsigned int *node)
>> +static int
>> +vsyscall_getcpu (unsigned int *cpu, unsigned int *node)
>
> Ok; this is the original function. Any reason (other than "cleaner
> diffs") to have this separate, and not just make it the #else clause
> below?
>
> Ah, it's called twice below. Got it. Ok. Do we want to tag this as
> always inline for the better CPU performance?
Yes, will do.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 22:08 ` DJ Delorie
@ 2024-02-19 22:18 ` Michael Jeanson
2024-02-19 22:21 ` DJ Delorie
0 siblings, 1 reply; 39+ messages in thread
From: Michael Jeanson @ 2024-02-19 22:18 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha, mathieu.desnoyers
On 2024-02-19 17:08, DJ Delorie wrote:
> Michael Jeanson <mjeanson@efficios.com> writes:
>> Would this make sense?
>
> Yup!
>
>> This variable is either zero (if restartable sequence registration
>> failed or has been disabled) or the size of the restartable sequence
>> features supported by the running kernel. If registration is
>> successful, @code{__rseq_feature_size} is at least 20 (the feature size of
>> @code{struct rseq} in the initial rseq kernel implementation).
>
Should I add a patch that does something similar for '__rseq_size'?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol
2024-02-19 22:18 ` Michael Jeanson
@ 2024-02-19 22:21 ` DJ Delorie
0 siblings, 0 replies; 39+ messages in thread
From: DJ Delorie @ 2024-02-19 22:21 UTC (permalink / raw)
To: Michael Jeanson; +Cc: libc-alpha, mathieu.desnoyers
Michael Jeanson <mjeanson@efficios.com> writes:
>>> This variable is either zero (if restartable sequence registration
>>> failed or has been disabled) or the size of the restartable sequence
>>> features supported by the running kernel. If registration is
>>> successful, @code{__rseq_feature_size} is at least 20 (the feature size of
>>> @code{struct rseq} in the initial rseq kernel implementation).
>>
>
> Should I add a patch that does something similar for '__rseq_size'?
Sure.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 7/8] aarch64: Add rseq_load32_load32_relaxed
2024-02-17 3:53 ` DJ Delorie
2024-02-19 20:29 ` Michael Jeanson
@ 2024-02-20 15:07 ` Mathieu Desnoyers
2024-02-20 17:55 ` DJ Delorie
1 sibling, 1 reply; 39+ messages in thread
From: Mathieu Desnoyers @ 2024-02-20 15:07 UTC (permalink / raw)
To: DJ Delorie, Michael Jeanson; +Cc: libc-alpha
On 2024-02-16 22:53, DJ Delorie wrote:
[...]
>
>> +#define RSEQ_ASM_OP_CMPEQ(var, expect, label) \
>> + " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
>> + " sub " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
>> + ", %[" __rseq_str(expect) "]\n" \
>> + " cbnz " RSEQ_ASM_TMP_REG ", " __rseq_str(label) "\n"
>
> This is why we need documentation; I would have guessed this was a CMPNE
> operation, but it depends on how you define "label"
This comes from the x86 implementation I originally did which has
"RSEQ_ASM_CMP_CPU_ID()" and static inline functions such as
"rseq_cmpeqv_storev()". The meaning of the "cmp" here is that the
critical section does _not_ abort (does not branch) if the comparison
matches.
But I understand how the ASM helpers that were contributed for other
architectures such as "RSEQ_ASM_OP_CMPEQ()", with the same semantic
of "do not branch to abort if the comparison matches" can be misleading
for someone used to reading assembler on pretty much any architecture,
where the conditional branch is expected to be taken if the condition
matches. So what I have here in librseq is backwards.
Fortunately, librseq is still just a master branch (no releases yet),
and the copy in the Linux kernel selftests is internal to that selftest,
so there are no stable API expectations at this stage.
So I don't think the semantic of e.g. "rseq_cmpeqv_storev()" is
misleading: it proceeds to do the store if the comparison matches.
However, the ASM macros would benefit from a logic flip. Even though
the API is not stable, I would like to introduce this in a way that
will allow users of the API to catch the change at compile-time. I
propose the following remapping of the macros for added clarity:
RSEQ_ASM_OP_CMPNE becomes RSEQ_ASM_OP_CBEQ (branch if equal)
RSEQ_ASM_OP_CMPEQ becomes RSEQ_ASM_OP_CBNE (branch if not equal)
RSEQ_ASM_CMP_CPU_ID becomes RSEQ_ASM_CBNE_CPU_ID (branch if cpu id is not equal)
I can do this change across all architectures in librseq to keep things in
sync. What do you think ?
[...]
>
>> +#define RSEQ_ASM_OP_R_BAD_MEMCPY(dst, src, len) \
>> + " cbz %[" __rseq_str(len) "], 333f\n" \
>> + " mov " RSEQ_ASM_TMP_REG_2 ", %[" __rseq_str(len) "]\n" \
>> + "222: sub " RSEQ_ASM_TMP_REG_2 ", " RSEQ_ASM_TMP_REG_2 ", #1\n" \
>> + " ldrb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(src) "]" \
>> + ", " RSEQ_ASM_TMP_REG_2 "]\n" \
>> + " strb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(dst) "]" \
>> + ", " RSEQ_ASM_TMP_REG_2 "]\n" \
>> + " cbnz " RSEQ_ASM_TMP_REG_2 ", 222b\n" \
>> + "333:\n"
>
> Ok, but WHY?
This is a memcpy from src to dst which can be aborted at any point
during the copy. Do you recommend we add documentation about what it does,
or that we remove it for now given that it is not used by the initial static
inline ?
Keeping all those helpers there simplifies the task of keeping librseq and
glibc in sync. But I would also understand if you prefer that we only introduce
what we use.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v8 7/8] aarch64: Add rseq_load32_load32_relaxed
2024-02-20 15:07 ` Mathieu Desnoyers
@ 2024-02-20 17:55 ` DJ Delorie
0 siblings, 0 replies; 39+ messages in thread
From: DJ Delorie @ 2024-02-20 17:55 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: mjeanson, libc-alpha
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
> On 2024-02-16 22:53, DJ Delorie wrote:
> [...]
>>
>>> +#define RSEQ_ASM_OP_CMPEQ(var, expect, label) \
>>> + " ldr " RSEQ_ASM_TMP_REG ", %[" __rseq_str(var) "]\n" \
>>> + " sub " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
>>> + ", %[" __rseq_str(expect) "]\n" \
>>> + " cbnz " RSEQ_ASM_TMP_REG ", " __rseq_str(label) "\n"
>>
>> This is why we need documentation; I would have guessed this was a CMPNE
>> operation, but it depends on how you define "label"
>
> This comes from the x86 implementation I originally did which has
> "RSEQ_ASM_CMP_CPU_ID()" and static inline functions such as
> "rseq_cmpeqv_storev()". The meaning of the "cmp" here is that the
> critical section does _not_ abort (does not branch) if the comparison
> matches.
Given that I'm looking at it as "someone not familiar with the RSEQ
API[*]", perhaps a one line comment that says "compare VAR and EXPECT
and ensure they're equal, else abort to LABEL" would have made me think
"Oh, that makes sense". Otherwise I have to read the inline asm, and my
familiarity with x86 asm would make me confused.
> However, the ASM macros would benefit from a logic flip. Even though
> the API is not stable, I would like to introduce this in a way that
> will allow users of the API to catch the change at compile-time.
I'm not pushing for an ABI change at this point. *Any* undocumented
inline assembler macro is going to be confusing to someone not familiar
with it, and needs a human-understandable comment or documentation for
it.
>>> +#define RSEQ_ASM_OP_R_BAD_MEMCPY(dst, src, len) \
>>> + " cbz %[" __rseq_str(len) "], 333f\n" \
>>> + " mov " RSEQ_ASM_TMP_REG_2 ", %[" __rseq_str(len) "]\n" \
>>> + "222: sub " RSEQ_ASM_TMP_REG_2 ", " RSEQ_ASM_TMP_REG_2 ", #1\n" \
>>> + " ldrb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(src) "]" \
>>> + ", " RSEQ_ASM_TMP_REG_2 "]\n" \
>>> + " strb " RSEQ_ASM_TMP_REG32 ", [%[" __rseq_str(dst) "]" \
>>> + ", " RSEQ_ASM_TMP_REG_2 "]\n" \
>>> + " cbnz " RSEQ_ASM_TMP_REG_2 ", 222b\n" \
>>> + "333:\n"
>>
>> Ok, but WHY?
>
> This is a memcpy from src to dst which can be aborted at any point
> during the copy. Do you recommend we add documentation about what it does,
Yes. A short comment that says "This is a slow dumb memcpy, but it can
be used in an rseq abortable code sequence." would be sufficient.
That's what I meant by my WHY? comment - there should be something that
explains why it exists and/or why you'd use it.
And for a bit of humorous snark, I'll point out that this mail thread is
already longer than the new documentation would need to be ;-)
[*] And someone old enough to realize that comments aren't just for your
peers, but also for your future self, to remind you what the heck
you were thinking when you wrote that ;-)
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2024-02-20 17:55 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-06 16:27 [PATCH v8 0/8] Extend rseq support Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 1/8] nptl: fix potential merge of __rseq_* relro symbols Michael Jeanson
2024-02-06 16:41 ` Mathieu Desnoyers
2024-02-16 2:02 ` DJ Delorie
2024-02-16 20:18 ` Michael Jeanson
2024-02-16 20:29 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 2/8] Add rseq extensible ABI support Michael Jeanson
2024-02-16 4:42 ` DJ Delorie
2024-02-16 20:19 ` Michael Jeanson
2024-02-16 20:37 ` DJ Delorie
2024-02-16 21:10 ` Michael Jeanson
2024-02-16 21:17 ` DJ Delorie
2024-02-16 21:24 ` Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 3/8] nptl: Add public __rseq_feature_size symbol Michael Jeanson
2024-02-16 22:07 ` DJ Delorie
2024-02-19 19:25 ` Michael Jeanson
2024-02-19 19:43 ` DJ Delorie
2024-02-19 20:15 ` Michael Jeanson
2024-02-19 20:24 ` DJ Delorie
2024-02-19 22:06 ` Michael Jeanson
2024-02-19 22:08 ` DJ Delorie
2024-02-19 22:18 ` Michael Jeanson
2024-02-19 22:21 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 4/8] nptl: Add features to internal 'struct rseq_area' Michael Jeanson
2024-02-17 2:04 ` DJ Delorie
2024-02-06 16:27 ` [PATCH v8 5/8] nptl: Add rseq internal utils Michael Jeanson
2024-02-17 2:33 ` DJ Delorie
2024-02-19 20:25 ` Michael Jeanson
2024-02-06 16:27 ` [PATCH v8 6/8] x86-64: Add rseq_load32_load32_relaxed Michael Jeanson
2024-02-17 3:08 ` DJ Delorie
2024-02-19 20:27 ` Michael Jeanson
2024-02-06 16:28 ` [PATCH v8 7/8] aarch64: " Michael Jeanson
2024-02-17 3:53 ` DJ Delorie
2024-02-19 20:29 ` Michael Jeanson
2024-02-20 15:07 ` Mathieu Desnoyers
2024-02-20 17:55 ` DJ Delorie
2024-02-06 16:28 ` [PATCH v8 8/8] Linux: Use rseq to accelerate getcpu Michael Jeanson
2024-02-17 3:57 ` DJ Delorie
2024-02-19 22:14 ` Michael Jeanson
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).