Browse Source

[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).
Thomas Knauth 4 years ago
parent
commit
59a0fee5ab

+ 19 - 1
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);

+ 1 - 1
LibOS/shim/test/apps

@@ -1 +1 @@
-Subproject commit 4ccdb249fd8550d519dcc4f743ed88b95730d6e4
+Subproject commit 0ad4576c9ba6ca32b783cf3f24fc9ff55f0dafda

+ 1 - 0
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)

+ 8 - 0
LibOS/shim/test/regression/abort.c

@@ -0,0 +1,8 @@
+#include <stdlib.h>
+
+/* 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();
+}

+ 33 - 0
LibOS/shim/test/regression/exit_group.c

@@ -0,0 +1,33 @@
+#include <linux/unistd.h>
+#include <pthread.h>
+#include <stdatomic.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#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;
+}

+ 15 - 0
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

+ 12 - 0
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'])