From 59a0fee5aba73e0cbc74a037ebb5e3d59d024a6c Mon Sep 17 00:00:00 2001 From: Thomas Knauth Date: Thu, 26 Sep 2019 10:03:39 -0700 Subject: [PATCH] [LibOS] Fix return code on exit due to signal Previously, exit due to signal produced a return code of 0. This commit correctly propagates the return code on exiting due to a signal. This commit also adds two tests and updates graphene-tests submodule to disable clone02 (this fix exposed an exotic unsupported combination of clone flags). --- LibOS/shim/src/sys/shim_exit.c | 20 ++++++++++- LibOS/shim/test/apps | 2 +- LibOS/shim/test/regression/Makefile | 1 + LibOS/shim/test/regression/abort.c | 8 +++++ LibOS/shim/test/regression/exit_group.c | 33 +++++++++++++++++++ .../regression/exit_group.manifest.template | 15 +++++++++ LibOS/shim/test/regression/test_libos.py | 12 +++++++ 7 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 LibOS/shim/test/regression/abort.c create mode 100644 LibOS/shim/test/regression/exit_group.c create mode 100644 LibOS/shim/test/regression/exit_group.manifest.template diff --git a/LibOS/shim/src/sys/shim_exit.c b/LibOS/shim/src/sys/shim_exit.c index 6af5d277..e75519d5 100644 --- a/LibOS/shim/src/sys/shim_exit.c +++ b/LibOS/shim/src/sys/shim_exit.c @@ -135,7 +135,7 @@ int try_process_exit (int error_code, int term_signal) struct shim_thread * cur_thread = get_cur_thread(); cur_thread->exit_code = -error_code; - cur_process.exit_code = error_code; + cur_process.exit_code = term_signal ? term_signal : error_code; cur_thread->term_signal = term_signal; if (cur_thread->in_vm) @@ -170,6 +170,15 @@ noreturn int shim_do_exit_group (int error_code) struct shim_thread * cur_thread = get_cur_thread(); assert(!is_internal(cur_thread)); + /* If exit_group() is invoked multiple times, only a single invocation proceeds past this + * point. Kill signals are delivered asynchronously, which will eventually kick the execution + * out of this loop.*/ + static struct atomic_int first = ATOMIC_INIT(0); + if (atomic_cmpxchg(&first, 0, 1) == 1) { + while (1) + DkThreadYieldExecution(); + } + if (debug_handle) sysparser_printf("---- shim_exit_group (returning %d)\n", error_code); @@ -183,6 +192,15 @@ noreturn int shim_do_exit_group (int error_code) debug("now kill other threads in the process\n"); do_kill_proc(cur_thread->tgid, cur_thread->tgid, SIGKILL, false); + /* This loop ensures that the current thread, which issues exit_group(), wins in setting the + * process's exit code. try_process_exit() first sets the exit_code before updating the thread's + * state to "dead". Once check_last_thread() indicates that the current thread is the last + * thread, all the children will already have set thread->exit_code. Hence, this thread's + * execution of try_process_exit() gets to determine the final exit_code, which is the desired + * outcome. */ + while (check_last_thread(cur_thread)) { + DkThreadYieldExecution(); + } debug("now exit the process\n"); try_process_exit(error_code, 0); diff --git a/LibOS/shim/test/apps b/LibOS/shim/test/apps index 4ccdb249..0ad4576c 160000 --- a/LibOS/shim/test/apps +++ b/LibOS/shim/test/apps @@ -1 +1 @@ -Subproject commit 4ccdb249fd8550d519dcc4f743ed88b95730d6e4 +Subproject commit 0ad4576c9ba6ca32b783cf3f24fc9ff55f0dafda diff --git a/LibOS/shim/test/regression/Makefile b/LibOS/shim/test/regression/Makefile index 0eb0dfe6..e1f54013 100644 --- a/LibOS/shim/test/regression/Makefile +++ b/LibOS/shim/test/regression/Makefile @@ -20,6 +20,7 @@ CFLAGS-shared_object = -fPIC -pie CFLAGS-syscall += -I$(PALDIR)/../include -I$(PALDIR)/host/$(PAL_HOST) CFLAGS-openmp = -fopenmp CFLAGS-multi_pthread = -pthread +CFLAGS-exit_group = -pthread %: %.c $(call cmd,csingle) diff --git a/LibOS/shim/test/regression/abort.c b/LibOS/shim/test/regression/abort.c new file mode 100644 index 00000000..fc8e8278 --- /dev/null +++ b/LibOS/shim/test/regression/abort.c @@ -0,0 +1,8 @@ +#include + +/* Test if the correct error code is propagated on a signal-induced exit. This should report 134 + * (128 + 6 where 6 is SIGABRT) as its return code. */ + +int main(int argc, char* arvg[]) { + abort(); +} diff --git a/LibOS/shim/test/regression/exit_group.c b/LibOS/shim/test/regression/exit_group.c new file mode 100644 index 00000000..69b44d16 --- /dev/null +++ b/LibOS/shim/test/regression/exit_group.c @@ -0,0 +1,33 @@ +#include +#include +#include +#include +#include +#include + +#define THREAD_NUM 4 + +atomic_int counter = 1; + +pthread_barrier_t barrier; + +/* Test the process exit logic in Graphene. Multiple threads race to execute + * exit()/exit_group(). Expected return code is 0 .. 4, depending on which thread wins. */ + +void* inc(void* arg) { + int a = counter++; + pthread_barrier_wait(&barrier); + exit(a); +} + +int main(int argc, char** argv) { + pthread_t thread[THREAD_NUM]; + pthread_barrier_init(&barrier, NULL, THREAD_NUM + 1); + + for (int j = 0; j < THREAD_NUM; j++) { + pthread_create(&thread[j], NULL, inc, NULL); + } + + pthread_barrier_wait(&barrier); + return 0; +} diff --git a/LibOS/shim/test/regression/exit_group.manifest.template b/LibOS/shim/test/regression/exit_group.manifest.template new file mode 100644 index 00000000..03f9e330 --- /dev/null +++ b/LibOS/shim/test/regression/exit_group.manifest.template @@ -0,0 +1,15 @@ +loader.preload = file:../../src/libsysdb.so +loader.env.LD_LIBRARY_PATH = /lib +loader.debug_type = none +loader.syscall_symbol = syscalldb + +fs.mount.lib.type = chroot +fs.mount.lib.path = /lib +fs.mount.lib.uri = file:../../../../Runtime + +sgx.trusted_files.ld = file:../../../../Runtime/ld-linux-x86-64.so.2 +sgx.trusted_files.libc = file:../../../../Runtime/libc.so.6 +sgx.trusted_files.libpthread = file:../../../../Runtime/libpthread.so.0 + +# Test uses 5 threads plus Graphene has up to two internal threads. +sgx.thread_num = 7 diff --git a/LibOS/shim/test/regression/test_libos.py b/LibOS/shim/test/regression/test_libos.py index ecb53ad8..505d56aa 100644 --- a/LibOS/shim/test/regression/test_libos.py +++ b/LibOS/shim/test/regression/test_libos.py @@ -118,6 +118,18 @@ class TC_00_Bootstrap(RegressionTestCase): with self.expect_returncode(113): self.run_binary(['exit']) + @unittest.skipIf(HAS_SGX, + 'Exposes a rare memory corruption on SGX PAL. Disable for now.') + def test_401_exit_group(self): + try: + self.run_binary(['exit_group']) + except subprocess.CalledProcessError as e: + self.assertTrue(1 <= e.returncode and e.returncode <= 4) + + def test_401_signalexit(self): + with self.expect_returncode(134): + self.run_binary(['abort']) + def test_500_init_fail(self): try: self.run_binary(['init_fail'])