This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 4/4] rseq registration tests (v2)
- From: Carlos O'Donell <codonell at redhat dot com>
- To: Mathieu Desnoyers <mathieu dot desnoyers at efficios dot com>, Carlos O'Donell <carlos at redhat dot com>
- Cc: Florian Weimer <fweimer at redhat dot com>, Joseph Myers <joseph at codesourcery dot com>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, libc-alpha at sourceware dot org, Thomas Gleixner <tglx at linutronix dot de>, Ben Maurer <bmaurer at fb dot com>, Peter Zijlstra <peterz at infradead dot org>, "Paul E. McKenney" <paulmck at linux dot vnet dot ibm dot com>, Boqun Feng <boqun dot feng at gmail dot com>, Will Deacon <will dot deacon at arm dot com>, Dave Watson <davejwatson at fb dot com>, Paul Turner <pjt at google dot com>
- Date: Wed, 27 Mar 2019 17:21:16 -0400
- Subject: Re: [PATCH 4/4] rseq registration tests (v2)
- References: <20190212194253.1951-1-mathieu.desnoyers@efficios.com> <20190212194253.1951-5-mathieu.desnoyers@efficios.com>
On 2/12/19 2:42 PM, Mathieu Desnoyers wrote:
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).
Thanks, this set of tests looks good, and covers the main uses.
tst-rseq.c only links against libc.so, testing registration of rseq in
a non-multithreaded environment.
OK.
tst-rseq-nptl.c also links against libpthread.so, testing registration
of rseq in a multithreaded environment.
OK.
See the Linux kernel selftests for extensive rseq stress-tests.
Yeah, that's where they should be, here we just want to exercise the
basic set of tests to ensure that integration with glibc is working
as expected.
This patch looks good to me, but is blocked on patch 1 acceptance,
Cleanup the following:
- One line short dedscriptions for files.
- Removal of contributed by lines.
- Answer small thread stack question.
and I'd say we're ready with this patch too.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Paul Turner <pjt@google.com>
CC: libc-alpha@sourceware.org
---
Changes since v1:
- Rename tst-rseq.c to tst-rseq-nptl.c.
- Introduce tst-rseq.c testing rseq registration in a non-multithreaded
environment.
---
sysdeps/unix/sysv/linux/Makefile | 4 +-
sysdeps/unix/sysv/linux/tst-rseq-nptl.c | 381 ++++++++++++++++++++++++
sysdeps/unix/sysv/linux/tst-rseq.c | 110 +++++++
3 files changed, 493 insertions(+), 2 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-nptl.c
create mode 100644 sysdeps/unix/sysv/linux/tst-rseq.c
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 5b541469ec..5f69f644a8 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -53,7 +53,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
test-errno-linux tst-memfd_create tst-mlock2 tst-pkey \
- tst-rlimit-infinity tst-ofdlocks
+ tst-rlimit-infinity tst-ofdlocks tst-rseq
OK.
tests-internal += tst-ofdlocks-compat
@@ -230,5 +230,5 @@ ifeq ($(subdir),nptl)
tests += tst-align-clone tst-getpid1 \
tst-thread-affinity-pthread tst-thread-affinity-pthread2 \
tst-thread-affinity-sched
-tests-internal += tst-setgetname
+tests-internal += tst-setgetname tst-rseq-nptl
OK.
endif
diff --git a/sysdeps/unix/sysv/linux/tst-rseq-nptl.c b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
new file mode 100644
index 0000000000..f4be4d2ae1
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq-nptl.c
@@ -0,0 +1,381 @@
Need a one line short description.
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
As noted in the other reviews, please remove the contributed by lines.
+
+ 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. */
OK.
+
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <support/check.h>
+
+#ifdef __NR_rseq
+#define HAS_RSEQ
+#endif
+
+#ifdef HAS_RSEQ
+#include <sys/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;
OK.
+
+static int
+rseq_thread_registered (void)
+{
+ return (int32_t) __rseq_abi.cpu_id >= 0;
OK.
+}
+
+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");
OK.
+ }
+ return 0;
+}
+
+static void
+cancel_routine (void *arg)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in cancel routine\n");
+ support_record_failure ();
+ }
+}
OK.
+
+static int cancel_thread_ready;
+
+static void
+test_cancel_thread (void)
+{
+ pthread_cleanup_push (cancel_routine, NULL);
+ atomic_store_release (&cancel_thread_ready, 1);
+ for (;;)
+ usleep (100);
+ pthread_cleanup_pop (0);
OK.
+}
+
+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;
+}
OK.
+
+static void
+sighandler (int sig)
+{
+ if (!rseq_thread_registered ())
+ {
+ printf ("rseq not registered in signal handler\n");
+ support_record_failure ();
+ }
+}
OK.
+
+static void
+setup_signals (void)
+{
+ struct sigaction sa;
+
+ sigemptyset (&sa.sa_mask);
+ sigaddset (&sa.sa_mask, SIGUSR1);
+ sa.sa_flags = 0;
+ sa.sa_handler = sighandler;
+ if (sigaction (SIGUSR1, &sa, NULL) != 0)
+ {
+ FAIL_EXIT1 ("sigaction failure: %s", strerror (errno));
OK.
+ }
+}
+
+#define N 7
+static const int t[N] = { 1, 2, 6, 5, 4, 3, 50 };
OK.
+
+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)
Why set it that small? Why set it at all?
+ {
+ 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);
OK. Decided in other mail that rseq should not be public.
+}
+
+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));
+ }
OK
+}
+
+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;
OK.
+}
+
+static int
+do_rseq_test (void)
+{
+ int i, result = 0;
+
+ if (!rseq_available ())
+ {
+ FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
OK.
+ }
+ 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;
OK.
+}
+
+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");
+}
OK. These two could do with becoming xthread_key_create/xthread_key_delete
wrappers in support/*, but it's not critical.
+
+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");
+}
OK.
+
+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");
OK.
+ return 0;
+}
+#endif
+
+static int
+do_test (void)
+{
+ return do_rseq_test ();
OK.
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/tst-rseq.c b/sysdeps/unix/sysv/linux/tst-rseq.c
new file mode 100644
index 0000000000..aaa1ad79fe
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-rseq.c
@@ -0,0 +1,110 @@
Need a one-line short description.
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
Remove please.
+
+ 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 main in an executable
+ not linked against libpthread. */
+
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <support/check.h>
+
+#ifdef __NR_rseq
+#define HAS_RSEQ
+#endif
+
+#ifdef HAS_RSEQ
+#include <sys/rseq.h>
+#include <syscall.h>
+#include <stdlib.h>
+#include <error.h>
+#include <errno.h>
+#include <stdint.h>
+#include <string.h>
+
+static int
+rseq_thread_registered (void)
+{
+ return (int32_t) __rseq_abi.cpu_id >= 0;
+}
+
+static int
+do_rseq_main_test (void)
+{
+ if (!rseq_thread_registered ())
+ {
+ FAIL_RET ("rseq not registered in main thread");
+ }
+ return 0;
+}
+
+static int
+sys_rseq (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_test (void)
+{
+ int result = 0;
+
+ if (!rseq_available ())
+ {
+ FAIL_UNSUPPORTED ("kernel does not support rseq, skipping test");
+ }
+ if (do_rseq_main_test ())
+ result = 1;
+ return result;
+}
+#else
+static int
+do_rseq_test (void)
+{
+ FAIL_UNSUPPORTED ("kernel headers do not support rseq, skipping test");
+ return 0;
+}
+#endif
+
+static int
+do_test (void)
+{
+ return do_rseq_test ();
OK.
+}
+
+#include <support/test-driver.c>
--
Cheers,
Carlos.