From a409356c548133e188acd7873e73a66ff5982b57 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 21 Jul 2022 16:49:48 -0700 Subject: [PATCH] misc/cgo/testcarchive: permit SIGQUIT for TestSignalForwardingExternal Occasionally the signal will be sent to a Go thread, which will cause the program to exit with SIGQUIT rather than SIGSEGV. Add TestSignalForwardingGo to test the case where the signal is expected to be delivered to a Go thread. Fixes #53907 Change-Id: Iaefb964c2be4a815c11c507fa89648f8a7740ba9 Reviewed-on: https://go-review.googlesource.com/c/go/+/419014 Run-TryBot: Ian Lance Taylor Auto-Submit: Ian Lance Taylor Reviewed-by: Austin Clements TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor --- misc/cgo/testcarchive/carchive_test.go | 237 +++++++++++------- .../testcarchive/testdata/libgo2/libgo2.go | 6 + misc/cgo/testcarchive/testdata/main5.c | 13 +- 3 files changed, 158 insertions(+), 98 deletions(-) diff --git a/misc/cgo/testcarchive/carchive_test.go b/misc/cgo/testcarchive/carchive_test.go index b959bc6cfa3..ed0e84d6807 100644 --- a/misc/cgo/testcarchive/carchive_test.go +++ b/misc/cgo/testcarchive/carchive_test.go @@ -19,6 +19,7 @@ import ( "runtime" "strconv" "strings" + "sync" "syscall" "testing" "time" @@ -526,38 +527,13 @@ func TestEarlySignalHandler(t *testing.T) { func TestSignalForwarding(t *testing.T) { checkSignalForwardingTest(t) + buildSignalForwardingTest(t) - if !testWork { - defer func() { - os.Remove("libgo2.a") - os.Remove("libgo2.h") - os.Remove("testp" + exeSuffix) - os.RemoveAll(filepath.Join(GOPATH, "pkg")) - }() - } - - cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "./libgo2") - if out, err := cmd.CombinedOutput(); err != nil { - t.Logf("%s", out) - t.Fatal(err) - } - checkLineComments(t, "libgo2.h") - checkArchive(t, "libgo2.a") - - ccArgs := append(cc, "-o", "testp"+exeSuffix, "main5.c", "libgo2.a") - if runtime.Compiler == "gccgo" { - ccArgs = append(ccArgs, "-lgo") - } - 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")...) + cmd := exec.Command(bin[0], append(bin[1:], "1")...) out, err := cmd.CombinedOutput() t.Logf("%v\n%s", cmd.Args, out) - expectSignal(t, err, syscall.SIGSEGV) + expectSignal(t, err, syscall.SIGSEGV, 0) // SIGPIPE is never forwarded on darwin. See golang.org/issue/33384. if runtime.GOOS != "darwin" && runtime.GOOS != "ios" { @@ -568,7 +544,7 @@ func TestSignalForwarding(t *testing.T) { if len(out) > 0 { t.Logf("%s", out) } - expectSignal(t, err, syscall.SIGPIPE) + expectSignal(t, err, syscall.SIGPIPE, 0) } } @@ -579,32 +555,7 @@ func TestSignalForwardingExternal(t *testing.T) { t.Skipf("skipping on %s/%s: runtime does not permit SI_USER SIGSEGV", GOOS, GOARCH) } checkSignalForwardingTest(t) - - if !testWork { - defer func() { - os.Remove("libgo2.a") - os.Remove("libgo2.h") - os.Remove("testp" + exeSuffix) - os.RemoveAll(filepath.Join(GOPATH, "pkg")) - }() - } - - cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "./libgo2") - if out, err := cmd.CombinedOutput(); err != nil { - t.Logf("%s", out) - t.Fatal(err) - } - checkLineComments(t, "libgo2.h") - checkArchive(t, "libgo2.a") - - ccArgs := append(cc, "-o", "testp"+exeSuffix, "main5.c", "libgo2.a") - if runtime.Compiler == "gccgo" { - ccArgs = append(ccArgs, "-lgo") - } - if out, err := exec.Command(ccArgs[0], ccArgs[1:]...).CombinedOutput(); err != nil { - t.Logf("%s", out) - t.Fatal(err) - } + buildSignalForwardingTest(t) // We want to send the process a signal and see if it dies. // Normally the signal goes to the C thread, the Go signal @@ -617,42 +568,27 @@ func TestSignalForwardingExternal(t *testing.T) { // fail. const tries = 20 for i := 0; i < tries; i++ { - 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") - } - - // Give the program a chance to enter the sleep function. - time.Sleep(time.Millisecond) - - cmd.Process.Signal(syscall.SIGSEGV) - - err = cmd.Wait() - + err := runSignalForwardingTest(t, "2") if err == nil { continue } - if expectSignal(t, err, syscall.SIGSEGV) { + // If the signal is delivered to a C thread, as expected, + // the Go signal handler will disable itself and re-raise + // the signal, causing the program to die with SIGSEGV. + // + // It is also possible that the signal will be + // delivered to a Go thread, such as a GC thread. + // Currently when the Go runtime sees that a SIGSEGV was + // sent from a different program, it first tries to send + // the signal to the os/signal API. If nothing is looking + // for (or explicitly ignoring) SIGSEGV, then it crashes. + // Because the Go runtime is invoked via a c-archive, + // it treats this as GOTRACEBACK=crash, meaning that it + // dumps a stack trace for all goroutines, which it does + // by raising SIGQUIT. The effect is that we will see the + // program die with SIGQUIT in that case, not SIGSEGV. + if expectSignal(t, err, syscall.SIGSEGV, syscall.SIGQUIT) { return } } @@ -660,6 +596,16 @@ func TestSignalForwardingExternal(t *testing.T) { t.Errorf("program succeeded unexpectedly %d times", tries) } +func TestSignalForwardingGo(t *testing.T) { + checkSignalForwardingTest(t) + buildSignalForwardingTest(t) + err := runSignalForwardingTest(t, "4") + + // Occasionally the signal will be delivered to a C thread, + // and the program will crash with SIGSEGV. + expectSignal(t, err, syscall.SIGQUIT, syscall.SIGSEGV) +} + // checkSignalForwardingTest calls t.Skip if the SignalForwarding test // doesn't work on this platform. func checkSignalForwardingTest(t *testing.T) { @@ -674,18 +620,121 @@ func checkSignalForwardingTest(t *testing.T) { } } +// buildSignalForwardingTest builds the executable used by the various +// signal forwarding tests. +func buildSignalForwardingTest(t *testing.T) { + if !testWork { + t.Cleanup(func() { + os.Remove("libgo2.a") + os.Remove("libgo2.h") + os.Remove("testp" + exeSuffix) + os.RemoveAll(filepath.Join(GOPATH, "pkg")) + }) + } + + t.Log("go build -buildmode=c-archive -o libgo2.a ./libgo2") + cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo2.a", "./libgo2") + out, err := cmd.CombinedOutput() + if len(out) > 0 { + t.Logf("%s", out) + } + if err != nil { + t.Fatal(err) + } + + checkLineComments(t, "libgo2.h") + checkArchive(t, "libgo2.a") + + ccArgs := append(cc, "-o", "testp"+exeSuffix, "main5.c", "libgo2.a") + if runtime.Compiler == "gccgo" { + ccArgs = append(ccArgs, "-lgo") + } + t.Log(ccArgs) + out, err = exec.Command(ccArgs[0], ccArgs[1:]...).CombinedOutput() + if len(out) > 0 { + t.Logf("%s", out) + } + if err != nil { + t.Fatal(err) + } +} + +func runSignalForwardingTest(t *testing.T, arg string) error { + t.Logf("%v %s", bin, arg) + cmd := exec.Command(bin[0], append(bin[1:], arg)...) + + var out strings.Builder + cmd.Stdout = &out + + 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 process is started. + ok, err := r.ReadString('\n') + + // Verify trigger. + if err != nil || ok != "OK\n" { + t.Fatal("Did not receive OK signal") + } + + var wg sync.WaitGroup + wg.Add(1) + var errsb strings.Builder + go func() { + defer wg.Done() + io.Copy(&errsb, r) + }() + + // Give the program a chance to enter the function. + // If the program doesn't get there the test will still + // pass, although it doesn't quite test what we intended. + // This is fine as long as the program normally makes it. + time.Sleep(time.Millisecond) + + cmd.Process.Signal(syscall.SIGSEGV) + + err = cmd.Wait() + + s := out.String() + if len(s) > 0 { + t.Log(s) + } + wg.Wait() + s = errsb.String() + if len(s) > 0 { + t.Log(s) + } + + return err +} + // expectSignal checks that err, the exit status of a test program, -// shows a failure due to a specific signal. Returns whether we found -// the expected signal. -func expectSignal(t *testing.T, err error, sig syscall.Signal) bool { +// shows a failure due to a specific signal or two. Returns whether we +// found an expected signal. +func expectSignal(t *testing.T, err error, sig1, sig2 syscall.Signal) bool { + t.Helper() 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() != sig { - t.Errorf("got %v; expected signal %v", ee, sig) + } else if !ws.Signaled() || (ws.Signal() != sig1 && ws.Signal() != sig2) { + if sig2 == 0 { + t.Errorf("got %q; expected signal %q", ee, sig1) + } else { + t.Errorf("got %q; expected signal %q or %q", ee, sig1, sig2) + } } else { return true } @@ -1022,14 +1071,14 @@ func TestCompileWithoutShared(t *testing.T) { binArgs := append(cmdToRun(exe), "1") out, err = exec.Command(binArgs[0], binArgs[1:]...).CombinedOutput() t.Logf("%v\n%s", binArgs, out) - expectSignal(t, err, syscall.SIGSEGV) + expectSignal(t, err, syscall.SIGSEGV, 0) // SIGPIPE is never forwarded on darwin. See golang.org/issue/33384. if runtime.GOOS != "darwin" && runtime.GOOS != "ios" { binArgs := append(cmdToRun(exe), "3") out, err = exec.Command(binArgs[0], binArgs[1:]...).CombinedOutput() t.Logf("%v\n%s", binArgs, out) - expectSignal(t, err, syscall.SIGPIPE) + expectSignal(t, err, syscall.SIGPIPE, 0) } } diff --git a/misc/cgo/testcarchive/testdata/libgo2/libgo2.go b/misc/cgo/testcarchive/testdata/libgo2/libgo2.go index 19c8e1a6dcb..35c89ae92bb 100644 --- a/misc/cgo/testcarchive/testdata/libgo2/libgo2.go +++ b/misc/cgo/testcarchive/testdata/libgo2/libgo2.go @@ -49,6 +49,12 @@ func RunGoroutines() { } } +// Block blocks the current thread while running Go code. +//export Block +func Block() { + select {} +} + var P *byte // TestSEGV makes sure that an invalid address turns into a run-time Go panic. diff --git a/misc/cgo/testcarchive/testdata/main5.c b/misc/cgo/testcarchive/testdata/main5.c index d431ce01ce5..c64c246fdea 100644 --- a/misc/cgo/testcarchive/testdata/main5.c +++ b/misc/cgo/testcarchive/testdata/main5.c @@ -29,10 +29,6 @@ int main(int argc, char** argv) { verbose = (argc > 2); - if (verbose) { - printf("calling RunGoroutines\n"); - } - Noop(); switch (test) { @@ -90,6 +86,15 @@ int main(int argc, char** argv) { printf("did not receive SIGPIPE\n"); return 0; } + case 4: { + fprintf(stderr, "OK\n"); + fflush(stderr); + + if (verbose) { + printf("calling Block\n"); + } + Block(); + } default: printf("Unknown test: %d\n", test); return 0;