From: Waldemar Kozaczuk <[email protected]>
Committer: Waldemar Kozaczuk <[email protected]>
Branch: master

sigaction: handle null sa_sigaction as SIG_DFL

As the comment of the issue #1047 - 
https://github.com/cloudius-systems/osv/issues/1047#issuecomment-846535939
- explains, Linux treats null value of the sa_sigaction parameter of
the sigaction() function as SIG_DFL even if SA_SIGINFO is set per sa_flags
parameter. This behavior does not seem to be standard as this stack
overflow - 
https://stackoverflow.com/questions/24079875/what-does-it-mean-if-sa-sigaction-is-set-to-null-when-calling-sigaction
- explains and probably stems from the fact that both sa_sigaction and
sa_handler are fields of the same union.

The above means, that if sa_sigaction is null we should treat
it the same same as if sa_handler == SIG_DFL and invoke default
signal handler if any when corresponding signal arrived. To that effect
this patch refines is_sig_dfl() helper function to implemented the
expected behavior.

Finally this patch adds new test - tst-sigaction.cc - that can be built
and run on both Linux and OSv to verify the same handling of null
sa_sigaction. Ideally, we would have wanted to expand existing
tst-kill.cc test, however we need a separate test to verify the
default action to terminate the app when SIGTERM is received is testable.

Fixes #1047

Signed-off-by: Waldemar Kozaczuk <[email protected]>

---
diff --git a/libc/signal.cc b/libc/signal.cc
--- a/libc/signal.cc
+++ b/libc/signal.cc
@@ -57,7 +57,11 @@ sigset* thread_signals(sched::thread *t)
 }
 
 inline bool is_sig_dfl(const struct sigaction &sa) {
-    return (!(sa.sa_flags & SA_SIGINFO) && sa.sa_handler == SIG_DFL);
+    if (sa.sa_flags & SA_SIGINFO) {
+         return sa.sa_sigaction == nullptr; // a non-standard Linux extension
+    } else {
+         return sa.sa_handler == SIG_DFL;
+    }
 }
 
 inline bool is_sig_ign(const struct sigaction &sa) {
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -111,7 +111,8 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so 
tst-bsd-evh.so \
        tst-calloc.so tst-crypt.so tst-non-fpic.so tst-small-malloc.so \
        tst-getopt.so tst-getopt-pie.so tst-non-pie.so tst-semaphore.so \
        tst-elf-init.so tst-realloc.so tst-setjmp.so \
-       libtls.so libtls_gold.so tst-tls.so tst-tls-gold.so tst-tls-pie.so
+       libtls.so libtls_gold.so tst-tls.so tst-tls-gold.so tst-tls-pie.so \
+       tst-sigaction.so
 #      libstatic-thread-variable.so tst-static-thread-variable.so \
 
 #TODO For now let us disable these tests for aarch64 until
diff --git a/tests/tst-sigaction.cc b/tests/tst-sigaction.cc
--- a/tests/tst-sigaction.cc
+++ b/tests/tst-sigaction.cc
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2021 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+// To compile on Linux, use: g++ -std=c++11 tests/tst-sigaction.cc
+#include <stdio.h>
+#include <unistd.h>
+#include <signal.h>
+#include <cassert>
+
+static int global = 0;
+void test_handler(int sig, siginfo_t *info, void *ucontext)
+{
+    printf("test_handler called, sig=%d, global=%d\n", sig, global);
+    global = 1;
+}
+
+void test_sigaction_with_handler(void (*handler)(int, siginfo_t *, void *))
+{
+    struct sigaction act = {};
+    act.sa_flags = SA_SIGINFO;
+    sigemptyset(&act.sa_mask);
+    act.sa_sigaction = handler;
+    assert(0 == sigaction(SIGTERM, &act, nullptr));
+    assert(0 == kill(getpid(), SIGTERM));
+}
+
+int main(int ac, char** av)
+{
+    test_sigaction_with_handler(test_handler);
+    for(int i = 0; i < 100; i++){
+        if(global == 1) break;
+        usleep(10000);
+    }
+    assert(global == 1);
+    printf("global now 1, test_handler called\n");
+
+    test_sigaction_with_handler(nullptr);
+    sleep(1);
+    assert(0); //We should not get here as the app and OSv should have already 
terminated by this time
+}

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/000000000000b9e19405c41fb5c2%40google.com.

Reply via email to