Skip to content

Commit

Permalink
sigaction: handle null sa_sigaction as SIG_DFL
Browse files Browse the repository at this point in the history
As the comment of the issue #1047 - #1047 (comment)
- 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]>
  • Loading branch information
wkozaczuk committed Jun 6, 2021
1 parent 7272f35 commit f9ba533
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 2 deletions.
6 changes: 5 additions & 1 deletion libc/signal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion modules/tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions tests/tst-sigaction.cc
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit f9ba533

Please sign in to comment.