Discussion:
[RFC PATCH glibc 3/4] support record failure: allow use from constructor
(too old to reply)
Mathieu Desnoyers
2018-12-04 19:21:40 UTC
Permalink
Expose support_record_failure_init () so constructors can explicitly
initialize the record failure API.

This is preferred to lazy initialization at first use, because
lazy initialization does not cover use in constructors within
forked children processes (forked from parent constructor).

Signed-off-by: Mathieu Desnoyers <***@efficios.com>
CC: Carlos O'Donell <***@redhat.com>
CC: Florian Weimer <***@redhat.com>
CC: Joseph Myers <***@codesourcery.com>
CC: Szabolcs Nagy <***@arm.com>
CC: libc-***@sourceware.org
---
support/check.h | 4 ++++
support/support_record_failure.c | 18 +++++++++++++-----
2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/support/check.h b/support/check.h
index e6765289f2..9ccd73884c 100644
--- a/support/check.h
+++ b/support/check.h
@@ -88,6 +88,10 @@ void support_test_verify_exit_impl (int status, const char *file, int line,
does not support reporting failures from a DSO. */
void support_record_failure (void);

+/* Initialize record failure. Calling this is only needed when
+ recording failures from constructors. */
+void support_record_failure_init (void);
+
/* Static assertion, under a common name for both C++ and C11. */
#ifdef __cplusplus
# define support_static_assert static_assert
diff --git a/support/support_record_failure.c b/support/support_record_failure.c
index 356798f556..947551d2c8 100644
--- a/support/support_record_failure.c
+++ b/support/support_record_failure.c
@@ -32,8 +32,12 @@
zero, the failure of a test can be detected.

The init constructor function below puts *state on a shared
- annonymous mapping, so that failure reports from subprocesses
- propagate to the parent process. */
+ anonymous mapping, so that failure reports from subprocesses
+ propagate to the parent process.
+
+ support_record_failure_init is exposed so it can be called explicitly
+ in case this API needs to be used from a constructor. */
+
struct test_failures
{
unsigned int counter;
@@ -41,10 +45,14 @@ struct test_failures
};
static struct test_failures *state;

-static __attribute__ ((constructor)) void
-init (void)
+__attribute__ ((constructor)) void
+support_record_failure_init (void)
{
- void *ptr = mmap (NULL, sizeof (*state), PROT_READ | PROT_WRITE,
+ void *ptr;
+
+ if (state != NULL)
+ return;
+ ptr = mmap (NULL, sizeof (*state), PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_SHARED, -1, 0);
if (ptr == MAP_FAILED)
{
--
2.17.1
Mathieu Desnoyers
2018-12-04 19:21:39 UTC
Permalink
When available, use the cpu_id field from __rseq_abi on Linux to
implement sched_getcpu(). Fall-back on the vgetcpu vDSO if unavailable.

Benchmarks:

x86-64: Intel E5-2630 ***@2.40GHz, 16-core, hyperthreading

glibc sched_getcpu(): 13.7 ns (baseline)
glibc sched_getcpu() using rseq: 2.5 ns (speedup: 5.5x)
inline load cpuid from __rseq_abi TLS: 0.8 ns (speedup: 17.1x)

Signed-off-by: Mathieu Desnoyers <***@efficios.com>
CC: Carlos O'Donell <***@redhat.com>
CC: Florian Weimer <***@redhat.com>
CC: Joseph Myers <***@codesourcery.com>
CC: Szabolcs Nagy <***@arm.com>
CC: Thomas Gleixner <***@linutronix.de>
CC: Ben Maurer <***@fb.com>
CC: Peter Zijlstra <***@infradead.org>
CC: "Paul E. McKenney" <***@linux.vnet.ibm.com>
CC: Boqun Feng <***@gmail.com>
CC: Will Deacon <***@arm.com>
CC: Dave Watson <***@fb.com>
CC: Paul Turner <***@google.com>
CC: libc-***@sourceware.org
CC: linux-***@vger.kernel.org
CC: linux-***@vger.kernel.org
---
sysdeps/unix/sysv/linux/sched_getcpu.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
index b69eeda15c..37fda59d36 100644
--- a/sysdeps/unix/sysv/linux/sched_getcpu.c
+++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
@@ -24,8 +24,8 @@
#endif
#include <sysdep-vdso.h>

-int
-sched_getcpu (void)
+static int
+vsyscall_sched_getcpu (void)
{
#ifdef __NR_getcpu
unsigned int cpu;
@@ -37,3 +37,24 @@ sched_getcpu (void)
return -1;
#endif
}
+
+#ifdef __NR_rseq
+#include <linux/rseq.h>
+
+extern __attribute__ ((tls_model ("initial-exec")))
+__thread volatile struct rseq __rseq_abi;
+
+int
+sched_getcpu (void)
+{
+ int cpu_id = __rseq_abi.cpu_id;
+
+ return cpu_id >= 0 ? cpu_id : vsyscall_sched_getcpu ();
+}
+#else
+int
+sched_getcpu (void)
+{
+ return vsyscall_sched_getcpu ();
+}
+#endif
--
2.17.1
Mathieu Desnoyers
2018-12-04 19:21:41 UTC
Permalink
These tests validate that rseq is registered from various execution
contexts (main thread, constructor, destructor, other threads, other
threads created from constructor and destructor, forked process
(without exec), pthread_atfork handlers, pthread setspecific
destructors, C++ thread and process destructors, signal handlers,
atexit handlers).

See the Linux kernel selftests for extensive rseq stress-tests.

Signed-off-by: Mathieu Desnoyers <***@efficios.com>
CC: Carlos O'Donell <***@redhat.com>
CC: Florian Weimer <***@redhat.com>
CC: Joseph Myers <***@codesourcery.com>
CC: Szabolcs Nagy <***@arm.com>
CC: Thomas Gleixner <***@linutronix.de>
CC: Ben Maurer <***@fb.com>
CC: Peter Zijlstra <***@infradead.org>
CC: "Paul E. McKenney" <***@linux.vnet.ibm.com>
CC: Boqun Feng <***@gmail.com>
CC: Will Deacon <***@arm.com>
CC: Dave Watson <***@fb.com>
CC: Paul Turner <***@google.com>
CC: libc-***@sourceware.org
---
nptl/Makefile | 2 +-
nptl/tst-rseq.c | 384 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 385 insertions(+), 1 deletion(-)
create mode 100644 nptl/tst-rseq.c

diff --git a/nptl/Makefile b/nptl/Makefile
index 3a5dc80c65..797309f7a8 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -323,7 +323,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
tests-internal := tst-rwlock19 tst-rwlock20 \
tst-sem11 tst-sem12 tst-sem13 \
tst-barrier5 tst-signal7 tst-mutex8 tst-mutex8-static \
- tst-mutexpi8 tst-mutexpi8-static
+ tst-mutexpi8 tst-mutexpi8-static tst-rseq

xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \
tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
diff --git a/nptl/tst-rseq.c b/nptl/tst-rseq.c
new file mode 100644
index 0000000000..218acbee51
--- /dev/null
+++ b/nptl/tst-rseq.c
@@ -0,0 +1,384 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <***@efficios.com>, 2018.
+
+ 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
+ <http://www.gnu.org/licenses/>. */
+
+/* These tests validate that rseq is registered from various execution
+ contexts (main thread, constructor, destructor, other threads, other
+ threads created from constructor and destructor, forked process
+ (without exec), pthread_atfork handlers, pthread setspecific
+ destructors, C++ thread and process destructors, signal handlers,
+ atexit handlers).
+
+ See the Linux kernel selftests for extensive rseq stress-tests. */
+
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <support/check.h>
+
+#if defined (__linux__) && defined (__NR_rseq)
+#define HAS_RSEQ
+#endif
+
+#ifdef HAS_RSEQ
+#include <linux/rseq.h>
+#include <pthread.h>
+#include <syscall.h>
+#include <stdlib.h>
+#include <error.h>
+#include <errno.h>
+#include <string.h>
+#include <stdint.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <atomic.h>
+
+static pthread_key_t rseq_test_key;
+
+extern __thread volatile struct rseq __rseq_abi
+__attribute__ ((tls_model ("initial-exec")));
+
+static int
+rseq_thread_registered (void)
+{
+ return __rseq_abi.cpu_id >= 0;
+}
+
+static int
+do_rseq_main_test (void)
+{
+ if (raise (SIGUSR1))
+ FAIL_EXIT1 ("error raising signal");
+ if (pthread_setspecific (rseq_test_key, (void *) 1l))
+ FAIL_EXIT1 ("error in pthread_setspecific");
+ if (!rseq_thread_registered ())
+ {
+ FAIL_RET ("rseq not registered in main thread");
+ }
+ return 0;
+}
+
+static void
+cancel_routine (void *arg)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in cancel routine\n");
+ support_record_failure ();
+ }
+}
+
+static int cancel_thread_ready;
+
+static void
+test_cancel_thread (void)
+{
+ pthread_cleanup_push (cancel_routine, NULL);
+ atomic_store_release (&cancel_thread_ready, 1);
+ for (;;)
+ usleep (100);
+ pthread_cleanup_pop (0);
+}
+
+static void *
+thread_function (void * arg)
+{
+ int i = (int) (intptr_t) arg;
+
+ if (raise (SIGUSR1))
+ FAIL_EXIT1 ("error raising signal");
+ if (i == 0)
+ test_cancel_thread ();
+ if (pthread_setspecific (rseq_test_key, (void *) 1l))
+ FAIL_EXIT1 ("error in pthread_setspecific");
+ return rseq_thread_registered () ? NULL : (void *) 1l;
+}
+
+static void
+sighandler (int sig)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in signal handler\n");
+ support_record_failure ();
+ }
+}
+
+static void
+setup_signals (void)
+{
+ struct sigaction sa;
+
+ sigemptyset (&sa.sa_mask);
+ sigaddset (&sa.sa_mask, SIGUSR1);
+ sa.sa_flags = 0;
+ sa.sa_handler = sighandler;
+ if (sigaction (SIGUSR1, &sa, NULL) != 0)
+ {
+ FAIL_EXIT1 ("sigaction failure: %s", strerror (errno));
+ }
+}
+
+#define N 7
+static const int t[N] = { 1, 2, 6, 5, 4, 3, 50 };
+
+static int
+do_rseq_threads_test (int nr_threads)
+{
+ pthread_t th[nr_threads];
+ int i;
+ int result = 0;
+ pthread_attr_t at;
+
+ if (pthread_attr_init (&at) != 0)
+ {
+ FAIL_EXIT1 ("attr_init failed");
+ }
+
+ if (pthread_attr_setstacksize (&at, 1 * 1024 * 1024) != 0)
+ {
+ FAIL_EXIT1 ("attr_setstacksize failed");
+ }
+
+ cancel_thread_ready = 0;
+ for (i = 0; i < nr_threads; ++i)
+ if (pthread_create (&th[i], NULL, thread_function,
+ (void *) (intptr_t) i) != 0)
+ {
+ FAIL_EXIT1 ("creation of thread %d failed", i);
+ }
+
+ if (pthread_attr_destroy (&at) != 0)
+ {
+ FAIL_EXIT1 ("attr_destroy failed");
+ }
+
+ while (!atomic_load_acquire (&cancel_thread_ready))
+ usleep (100);
+
+ if (pthread_cancel (th[0]))
+ FAIL_EXIT1 ("error in pthread_cancel");
+
+ for (i = 0; i < nr_threads; ++i)
+ {
+ void *v;
+ if (pthread_join (th[i], &v) != 0)
+ {
+ printf ("join of thread %d failed\n", i);
+ result = 1;
+ }
+ else if (i != 0 && v != NULL)
+ {
+ printf ("join %d successful, but child failed\n", i);
+ result = 1;
+ }
+ else if (i == 0 && v == NULL)
+ {
+ printf ("join %d successful, child did not fail as expected\n", i);
+ result = 1;
+ }
+ }
+ return result;
+}
+
+static int
+sys_rseq (volatile struct rseq *rseq_abi, uint32_t rseq_len,
+ int flags, uint32_t sig)
+{
+ return syscall (__NR_rseq, rseq_abi, rseq_len, flags, sig);
+}
+
+static int
+rseq_available (void)
+{
+ int rc;
+
+ rc = sys_rseq (NULL, 0, 0, 0);
+ if (rc != -1)
+ FAIL_EXIT1 ("Unexpected rseq return value %d", rc);
+ switch (errno)
+ {
+ case ENOSYS:
+ return 0;
+ case EINVAL:
+ return 1;
+ default:
+ FAIL_EXIT1 ("Unexpected rseq error %s", strerror (errno));
+ }
+}
+
+static int
+do_rseq_fork_test (void)
+{
+ int status;
+ pid_t pid, retpid;
+
+ pid = fork ();
+ switch (pid)
+ {
+ case 0:
+ exit (do_rseq_main_test ());
+ case -1:
+ FAIL_EXIT1 ("Unexpected fork error %s", strerror (errno));
+ }
+ retpid = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
+ if (retpid != pid)
+ {
+ FAIL_EXIT1 ("waitpid returned %ld, expected %ld",
+ (long int) retpid, (long int) pid);
+ }
+ if (WEXITSTATUS (status))
+ {
+ printf ("rseq not registered in child\n");
+ return 1;
+ }
+ return 0;
+}
+
+static int
+do_rseq_test (void)
+{
+ int i, result = 0;
+
+ if (!rseq_available ())
+ {
+ FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
+ }
+ setup_signals ();
+ if (raise (SIGUSR1))
+ FAIL_EXIT1 ("error raising signal");
+ if (do_rseq_main_test ())
+ result = 1;
+ for (i = 0; i < N; i++)
+ {
+ if (do_rseq_threads_test (t[i]))
+ result = 1;
+ }
+ if (do_rseq_fork_test ())
+ result = 1;
+ return result;
+}
+
+static void
+atfork_prepare (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in pthread atfork prepare\n");
+ support_record_failure ();
+ }
+}
+
+static void
+atfork_parent (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in pthread atfork parent\n");
+ support_record_failure ();
+ }
+}
+
+static void
+atfork_child (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in pthread atfork child\n");
+ support_record_failure ();
+ }
+}
+
+static void
+rseq_key_destructor (void *arg)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (!rseq_thread_registered ())
+ FAIL_EXIT1 ("rseq not registered in pthread key destructor");
+}
+
+static void
+do_rseq_create_key (void)
+{
+ if (pthread_key_create (&rseq_test_key, rseq_key_destructor))
+ FAIL_EXIT1 ("error in pthread_key_create");
+}
+
+static void
+do_rseq_delete_key (void)
+{
+ if (pthread_key_delete (rseq_test_key))
+ FAIL_EXIT1 ("error in pthread_key_delete");
+}
+
+static void
+atexit_handler (void)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (!rseq_thread_registered ())
+ FAIL_EXIT1 ("rseq not registered in atexit handler");
+}
+
+static void __attribute__ ((constructor))
+do_rseq_constructor_test (void)
+{
+ support_record_failure_init ();
+ if (atexit (atexit_handler))
+ {
+ FAIL_EXIT1 ("error calling atexit");
+ }
+ do_rseq_create_key ();
+ if (pthread_atfork (atfork_prepare, atfork_parent, atfork_child))
+ FAIL_EXIT1 ("error calling pthread_atfork");
+ if (do_rseq_test ())
+ FAIL_EXIT1 ("rseq not registered within constructor");
+}
+
+static void __attribute__ ((destructor))
+do_rseq_destructor_test (void)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (do_rseq_test ())
+ FAIL_EXIT1 ("rseq not registered within destructor");
+ do_rseq_delete_key ();
+}
+
+/* Test C++ destructor called at thread and process exit. */
+void
+__call_tls_dtors (void)
+{
+ /* Cannot use deferred failure reporting after main () returns. */
+ if (!rseq_thread_registered ())
+ FAIL_EXIT1 ("rseq not registered in C++ thread/process exit destructor");
+}
+#else
+static int
+do_rseq_test (void)
+{
+ FAIL_UNSUPPORTED ("kernel headers do not support rseq, skipping test");
+ return 0;
+}
+#endif
+
+static int
+do_test (void)
+{
+ return do_rseq_test ();
+}
+
+#include <support/test-driver.c>
--
2.17.1
Florian Weimer
2018-12-04 19:44:00 UTC
Permalink
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
index c370fda73d..9da78d59d2 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
@@ -235,3 +235,4 @@ GLIBC_2.28 tss_create F
GLIBC_2.28 tss_delete F
GLIBC_2.28 tss_get F
GLIBC_2.28 tss_set F
+GLIBC_2.29 __rseq_refcount T 0x4
This part looks buggy.

It's unclear based on this patch whether you actually want to get rid of
the __rseq_refcount symbol.

Thanks,
Florian
Mathieu Desnoyers
2018-12-04 19:50:54 UTC
Permalink
Post by Florian Weimer
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
b/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
index c370fda73d..9da78d59d2 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libpthread.abilist
@@ -235,3 +235,4 @@ GLIBC_2.28 tss_create F
GLIBC_2.28 tss_delete F
GLIBC_2.28 tss_get F
GLIBC_2.28 tss_set F
+GLIBC_2.29 __rseq_refcount T 0x4
This part looks buggy.
It's unclear based on this patch whether you actually want to get rid of
the __rseq_refcount symbol.
I want to keep the __rseq_refcount symbol so out-of-libc users can
register rseq if they are linked against a pre-2.29 libc.

Why does it look buggy ?

Thanks,

Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Loading...