]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: improve last ditch signal forwarding for Unix libraries
authorJoe Sylve <joe.sylve@gmail.com>
Wed, 23 Mar 2016 03:11:42 +0000 (22:11 -0500)
committerIan Lance Taylor <iant@golang.org>
Thu, 24 Mar 2016 19:34:17 +0000 (19:34 +0000)
The current runtime attempts to forward signals generated by non-Go
code to the original signal handler.  If it can't call the original
handler directly, it currently attempts to re-raise the signal after
resetting the handler.  In this case, the original context is lost.

This fix prevents that problem by simply returning from the go signal
handler after resetting the original handler.  It only does this when
the original handler is the system default handler, which in all cases
is known to not recover.  The signal is not reset, so it is retriggered
and the original handler takes over with the proper context.

Fixes #14899

Change-Id: Ib1c19dfa4b50d9732d7a453de3784c8141e1cbb3
Reviewed-on: https://go-review.googlesource.com/21006
Reviewed-by: Ian Lance Taylor <iant@golang.org>
misc/cgo/testcarchive/carchive_test.go
misc/cgo/testcarchive/main5.c [new file with mode: 0644]
src/runtime/os1_nacl.go
src/runtime/signal1_unix.go
src/runtime/signal_darwin.go
src/runtime/signal_freebsd.go
src/runtime/signal_openbsd.go
src/runtime/signal_sigtramp.go
src/runtime/signal_solaris_amd64.go
src/runtime/sigqueue.go
src/runtime/sys_solaris_amd64.s

index d5cd43391333ee55a45fd237296813c7ef57f5bb..b941397cf15b3730cf6c75cee4d865798064ebd0 100644 (file)
@@ -5,6 +5,7 @@
 package carchive_test
 
 import (
+       "bufio"
        "fmt"
        "io/ioutil"
        "os"
@@ -12,6 +13,7 @@ import (
        "path/filepath"
        "runtime"
        "strings"
+       "syscall"
        "testing"
        "unicode"
 )
@@ -221,6 +223,127 @@ func TestEarlySignalHandler(t *testing.T) {
        }
 }
 
+func TestSignalForwarding(t *testing.T) {
+       switch runtime.GOOS {
+       case "darwin":
+               switch runtime.GOARCH {
+               case "arm", "arm64":
+                       t.Skipf("skipping on %s/%s; see https://golang.org/issue/13701", runtime.GOOS, runtime.GOARCH)
+               }
+       case "windows":
+               t.Skip("skipping signal test on Windows")
+       }
+
+       defer func() {
+               os.Remove("libgo2.a")
+               os.Remove("libgo2.h")
+               os.Remove("testp")
+               os.RemoveAll("pkg")
+       }()
+
+       cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "libgo2")
+       cmd.Env = gopathEnv
+       if out, err := cmd.CombinedOutput(); err != nil {
+               t.Logf("%s", out)
+               t.Fatal(err)
+       }
+
+       ccArgs := append(cc, "-o", "testp"+exeSuffix, "main5.c", "libgo2.a")
+       if out, err := exec.Command(ccArgs[0], ccArgs[1:]...).CombinedOutput(); err != nil {
+               t.Logf("%s", out)
+               t.Fatal(err)
+       }
+
+       cmd = exec.Command(bin[0], append(bin[1:], "1")...)
+
+       out, err := cmd.CombinedOutput()
+
+       if err == nil {
+               t.Logf("%s", out)
+               t.Error("test program succeeded unexpectedly")
+       } else if ee, ok := err.(*exec.ExitError); !ok {
+               t.Logf("%s", out)
+               t.Errorf("error (%v) has type %T; expected exec.ExitError", err, err)
+       } else if ws, ok := ee.Sys().(syscall.WaitStatus); !ok {
+               t.Logf("%s", out)
+               t.Errorf("error.Sys (%v) has type %T; expected syscall.WaitStatus", ee.Sys(), ee.Sys())
+       } else if !ws.Signaled() || ws.Signal() != syscall.SIGSEGV {
+               t.Logf("%s", out)
+               t.Errorf("got %v; expected SIGSEGV", ee)
+       }
+}
+
+func TestSignalForwardingExternal(t *testing.T) {
+       switch runtime.GOOS {
+       case "darwin":
+               switch runtime.GOARCH {
+               case "arm", "arm64":
+                       t.Skipf("skipping on %s/%s; see https://golang.org/issue/13701", runtime.GOOS, runtime.GOARCH)
+               }
+       case "windows":
+               t.Skip("skipping signal test on Windows")
+       }
+
+       defer func() {
+               os.Remove("libgo2.a")
+               os.Remove("libgo2.h")
+               os.Remove("testp")
+               os.RemoveAll("pkg")
+       }()
+
+       cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "libgo2")
+       cmd.Env = gopathEnv
+       if out, err := cmd.CombinedOutput(); err != nil {
+               t.Logf("%s", out)
+               t.Fatal(err)
+       }
+
+       ccArgs := append(cc, "-o", "testp"+exeSuffix, "main5.c", "libgo2.a")
+       if out, err := exec.Command(ccArgs[0], ccArgs[1:]...).CombinedOutput(); err != nil {
+               t.Logf("%s", out)
+               t.Fatal(err)
+       }
+
+       cmd = exec.Command(bin[0], append(bin[1:], "2")...)
+
+       stderr, err := cmd.StderrPipe()
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer stderr.Close()
+
+       r := bufio.NewReader(stderr)
+
+       err = cmd.Start()
+
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // Wait for trigger to ensure that the process is started.
+       ok, err := r.ReadString('\n')
+
+       // Verify trigger.
+       if err != nil || ok != "OK\n" {
+               t.Fatalf("Did not receive OK signal")
+       }
+
+       // Trigger an interrupt external to the process.
+       cmd.Process.Signal(syscall.SIGSEGV)
+
+       err = cmd.Wait()
+
+       if err == nil {
+               t.Error("test program succeeded unexpectedly")
+       } else if ee, ok := err.(*exec.ExitError); !ok {
+               t.Errorf("error (%v) has type %T; expected exec.ExitError", err, err)
+       } else if ws, ok := ee.Sys().(syscall.WaitStatus); !ok {
+               t.Errorf("error.Sys (%v) has type %T; expected syscall.WaitStatus", ee.Sys(), ee.Sys())
+       } else if !ws.Signaled() || ws.Signal() != syscall.SIGSEGV {
+               t.Errorf("got %v; expected SIGSEGV", ee)
+       }
+}
+
 func TestOsSignal(t *testing.T) {
        switch runtime.GOOS {
        case "windows":
diff --git a/misc/cgo/testcarchive/main5.c b/misc/cgo/testcarchive/main5.c
new file mode 100644 (file)
index 0000000..556abdf
--- /dev/null
@@ -0,0 +1,63 @@
+// Copyright 2015 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test for verifying that the Go runtime properly forwards
+// signals when non-Go signals are raised.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "libgo2.h"
+
+int main(int argc, char** argv) {
+       int verbose;
+       int test;
+
+       if (argc < 2) {
+               printf("Missing argument");
+               return 1;
+       }
+
+       test = atoi(argv[1]);
+
+       verbose = (argc > 2);
+
+       if (verbose) {
+               printf("calling RunGoroutines\n");
+       }
+
+       RunGoroutines();
+
+       switch (test) {
+               case 1: {
+                       if (verbose) {
+                               printf("attempting segfault\n");
+                       }
+
+                       volatile int crash = *(int *) 0;
+                       break;
+               }
+
+               case 2: {
+                       if (verbose) {
+                               printf("attempting external signal test\n");
+                       }
+
+                       fprintf(stderr, "OK\n");
+                       fflush(stderr);
+
+                       // The program should be interrupted before this sleep finishes.
+                       sleep(60);
+
+                       break;
+               }
+               default:
+                       printf("Unknown test: %d\n", test);
+                       return 0;
+       }
+
+       printf("FAIL\n");
+       return 0;
+}
index 6fc2819cddf338bfc82dd1ea0c11fd348af96d53..622755119d0336b2edb1536dc97f2a4874baa13b 100644 (file)
@@ -172,6 +172,22 @@ func memlimit() uintptr {
        return 0
 }
 
+// This runs on a foreign stack, without an m or a g. No stack split.
+//go:nosplit
+//go:norace
+//go:nowritebarrierrec
+func badsignal(sig uintptr) {
+       cgocallback(unsafe.Pointer(funcPC(badsignalgo)), noescape(unsafe.Pointer(&sig)), unsafe.Sizeof(sig))
+}
+
+func badsignalgo(sig uintptr) {
+       if !sigsend(uint32(sig)) {
+               // A foreign thread received the signal sig, and the
+               // Go code does not want to handle it.
+               raisebadsignal(int32(sig))
+       }
+}
+
 // This runs on a foreign stack, without an m or a g. No stack split.
 //go:nosplit
 func badsignal2() {
index 8e4d425fde95d19eafb38623f87cbb098e7ccf9d..31c1f2c4e58e9347fb64d5f83caf20ce65bd4a7c 100644 (file)
@@ -6,7 +6,10 @@
 
 package runtime
 
-import "runtime/internal/sys"
+import (
+       "runtime/internal/sys"
+       "unsafe"
+)
 
 const (
        _SIG_DFL uintptr = 0
@@ -197,7 +200,7 @@ func dieFromSignal(sig int32) {
 // raisebadsignal is called when a signal is received on a non-Go
 // thread, and the Go program does not want to handle it (that is, the
 // program has not called os/signal.Notify for the signal).
-func raisebadsignal(sig int32) {
+func raisebadsignal(sig int32, c *sigctxt) {
        if sig == _SIGPROF {
                // Ignore profiling signals that arrive on non-Go threads.
                return
@@ -220,6 +223,18 @@ func raisebadsignal(sig int32) {
        // again.
        unblocksig(sig)
        setsig(sig, handler, false)
+
+       // If we're linked into a non-Go program we want to try to
+       // avoid modifying the original context in which the signal
+       // was raised. If the handler is the default, we know it
+       // is non-recoverable, so we don't have to worry about
+       // re-installing sighandler. At this point we can just
+       // return and the signal will be re-raised and caught by
+       // the default handler with the correct context.
+       if (isarchive || islibrary) && handler == _SIG_DFL && c.sigcode() != _SI_USER {
+               return
+       }
+
        raise(sig)
 
        // If the signal didn't cause the program to exit, restore the
@@ -307,3 +322,19 @@ func sigNotOnStack(sig uint32) {
        println("signal", sig, "received but handler not on signal stack")
        throw("non-Go code set up signal handler without SA_ONSTACK flag")
 }
+
+// This runs on a foreign stack, without an m or a g. No stack split.
+//go:nosplit
+//go:norace
+//go:nowritebarrierrec
+func badsignal(sig uintptr, c *sigctxt) {
+       cgocallback(unsafe.Pointer(funcPC(badsignalgo)), noescape(unsafe.Pointer(&sig)), unsafe.Sizeof(sig)+unsafe.Sizeof(c))
+}
+
+func badsignalgo(sig uintptr, c *sigctxt) {
+       if !sigsend(uint32(sig)) {
+               // A foreign thread received the signal sig, and the
+               // Go code does not want to handle it.
+               raisebadsignal(int32(sig), c)
+       }
+}
index 8d43724e2fe136117fce960e30d29068aea2fe04..c8534ff09bad785c3b768040e5969423b0120b08 100644 (file)
@@ -58,7 +58,7 @@ func sigtrampgo(fn uintptr, infostyle, sig uint32, info *siginfo, ctx unsafe.Poi
        }
        g := getg()
        if g == nil {
-               badsignal(uintptr(sig))
+               badsignal(uintptr(sig), &sigctxt{info, ctx})
                sigreturn(ctx, infostyle)
                return
        }
index f3411aac6afd2121b109acf4846e51d64b649f3f..cd2068a62c04985567fd43b9ec51d57c7b63b87d 100644 (file)
@@ -55,7 +55,7 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
        }
        g := getg()
        if g == nil {
-               badsignal(uintptr(sig))
+               badsignal(uintptr(sig), &sigctxt{info, ctx})
                return
        }
 
index d0239b1d91f2c7ca89fb2b2d1a5eb0b96f2c9221..3c50190da44449ffe936647d5b359b7667c8c7d3 100644 (file)
@@ -55,7 +55,7 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
        }
        g := getg()
        if g == nil {
-               badsignal(uintptr(sig))
+               badsignal(uintptr(sig), &sigctxt{info, ctx})
                return
        }
 
index 00ab03846e1b8946f21a232da7501a66edf32005..18fc375e247c9eb4892f95178d40bcabe4df9ca2 100644 (file)
@@ -18,7 +18,7 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
        }
        g := getg()
        if g == nil {
-               badsignal(uintptr(sig))
+               badsignal(uintptr(sig), &sigctxt{info, ctx})
                return
        }
 
index a577c8c1994b2667d965394446de85be7189fcc8..7ba368f25b2b8cf115cc9c8f0eab337f759f6398 100644 (file)
@@ -11,6 +11,10 @@ type sigctxt struct {
        ctxt unsafe.Pointer
 }
 
+func makesigctxt(info *siginfo, ctxt unsafe.Pointer) *sigctxt {
+       return &sigctxt{info, ctxt}
+}
+
 func (c *sigctxt) regs() *mcontext {
        return (*mcontext)(unsafe.Pointer(&(*ucontext)(c.ctxt).uc_mcontext))
 }
index e86e6a56369a7edc6a0039cf752a44974d56bc6b..0162ffa04fbb6ecdc9497778e91461a6ef30e8a1 100644 (file)
@@ -30,7 +30,7 @@ package runtime
 
 import (
        "runtime/internal/atomic"
-       "unsafe"
+       _ "unsafe" // for go:linkname
 )
 
 var sig struct {
@@ -176,19 +176,3 @@ func signal_ignore(s uint32) {
 func signal_ignored(s uint32) bool {
        return sig.ignored[s/32]&(1<<(s&31)) != 0
 }
-
-// This runs on a foreign stack, without an m or a g. No stack split.
-//go:nosplit
-//go:norace
-//go:nowritebarrierrec
-func badsignal(sig uintptr) {
-       cgocallback(unsafe.Pointer(funcPC(badsignalgo)), noescape(unsafe.Pointer(&sig)), unsafe.Sizeof(sig))
-}
-
-func badsignalgo(sig uintptr) {
-       if !sigsend(uint32(sig)) {
-               // A foreign thread received the signal sig, and the
-               // Go code does not want to handle it.
-               raisebadsignal(int32(sig))
-       }
-}
index 09f22636a9f7f6835ef6482e2d009a08c47ecc5e..f8b7da5c6216902036e02bf774cbecde63d78101 100644 (file)
@@ -173,7 +173,12 @@ TEXT runtime·sigtramp(SB),NOSPLIT,$0
        MOVQ    g(BX), R10
        CMPQ    R10, $0
        JNE     allgood
+       MOVQ    SI, 0(SP)
+       MOVQ    DX, 8(SP)
+       CALL    runtime·makesigctxt(SB)
+       MOVQ    16(SP), AX
        MOVQ    DI, 0(SP)
+       MOVQ    AX, 8(SP)
        MOVQ    $runtime·badsignal(SB), AX
        CALL    AX
        JMP     exit