Provide limited implementation of the tgkill syscall needed by Golang PIE apps

[wkozaczuk]

The commit 33bea60 fixed the original crash reported by issue #1026. However, when a user presses Ctrl-C on running golang-pie-httpserver app, OSv prints message about unimplemented tgkill syscall instead of exiting gracefully.
The goal of this issue is to provide a limited but reasonable implementation of this syscall to let Golang apps like the one mentioned here exit gracefully.
For more details please see this discussion on OSv dev list.

[wkozaczuk]

As I was researching other Golang related issues, I came back at this one, and here is what I am suggesting we do.

First of the tgkill syscall is used by Golang to implement sys.raise():

TEXT runtime·raise(SB),NOSPLIT,$0
        MOVL    $SYS_getpid, AX
        SYSCALL
        MOVL    AX, R12
        MOVL    $SYS_gettid, AX
        SYSCALL
        MOVL    AX, SI  // arg 2 tid
        MOVL    R12, DI // arg 1 pid
        MOVL    sig+0(FP), DX   // arg 3
        MOVL    $SYS_tgkill, AX
        SYSCALL
        RET

When Ctrl-C is called, Golang calls raise() to send signal 2 to the current thread. The registered handler for 2 resets the handler with null sa_sigaction and sends another signal 2. Here is how it looks like after adding some debug statements in OSv:

OSv v0.55.0-247-g1433c08b
eth0: 192.168.122.15
Booted up in 160.71 ms
Cmdline: /httpserver-external-pie
-->tgkill syscall, tgid:2, tid:55, sig:23
kill: user defined signal handler: 23
Go version: go1.15.8



kill: user defined signal handler: 2
--> sigaction: setting new for signum:2, act->sa_handler:0, act->sa_sigaction:0, is SA_SIGINFO:4
-->tgkill syscall, tgid:2, tid:62, sig:2

So I think we have to do:

1. Implement the SYS_tgkill as a simple passthrough to kill() in libc/signal.cc like so (or maybe delegate it to kill() only if the current thread id == tid, otherwise report a warning or fail; will OSv ever implement sending signals to specific threads, should it?):

+extern "C" int tgkill(int tgid, int tid, int sig)
+{
+    printf("-->tgkill syscall, tgid:%ld, tid:%ld, sig:%ld\n", tgid, tid, sig);
+    return kill(tgid,sig);
+}
+
 long syscall(long number, ...)
 {
     // Save FPU state and restore it at the end of this function
@@ -451,6 +459,7 @@ long syscall(long number, ...)
     SYSCALL2(mkdir, char*, mode_t);
 #endif
     SYSCALL3(mkdirat, int, char*, mode_t);
+    SYSCALL3(tgkill, int, int, int);
     }

2. I believe we have a bug in is_sig_dfl():

 inline bool is_sig_dfl(const struct sigaction &sa) {
-    return (!(sa.sa_flags & SA_SIGINFO) && sa.sa_handler == SIG_DFL);
+    return sa.sa_handler == SIG_DFL;
 }

I do not understand the original reasoning, but I do not think we should treat the sa.sa_handler == SIG_DFL differently if SA_SIGINFO is set in sa_flags. The sa_handler and sa_sigaction are in part of the same union and I think the sigaction() specification can be interpreted as either of the two can be SIG_DFL (see this discussion - https://stackoverflow.com/questions/24079875/what-does-it-mean-if-sa-sigaction-is-set-to-null-when-calling-sigaction).

[nyh]

1. Seems reasonable, you can do a patch and test it on your golang example.
2. Nice catch. If sigaction(2) is used with SA_SIGINFO, then the sa_handler field is ignored and instead sa_sigaction is used. Only sa_handler may be SIG_DFL - there is sense in which sa_sigaction may be the default. If SA_SIGINFO is specified, it means there must be some non-default function to handle it. But that being said, I didn't realize that a null sa_sigaction is also the same as setting sa_handler to SIG_DFL. As far as I know this behavior isn't standard, but the stackoverflow thread you pointed to suggests it works on Linux so we need to support it too. If this is the case (we should have a unit test to verify this, and try it on both Linux and OSv) then indeed is_sig_dfl needs to be expanded to something like:

inline bool is_sig_dfl(const struct sigaction &sa) {
    if (sa.sa_flags & SA_SIGINFO) {
         return sa.sa_sigaction == nullptr; // a non-standard Linux extension
    } else {
         return sa.sa_handler == SIG_DFL;
    }
}