]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix cgo signals detection
authorMichael Pratt <mpratt@google.com>
Wed, 4 Aug 2021 15:24:28 +0000 (11:24 -0400)
committerMichael Pratt <mpratt@google.com>
Tue, 26 Oct 2021 21:17:38 +0000 (21:17 +0000)
CL 64070 removed lockOSThread from the cgocall path, but didn't update
the signal-in-cgo detection in sighandler. As a result, signals that
arrive during a cgo call are treated like they arrived during Go
execution, breaking the traceback.

Update the cgo detection to fix the backtrace.

Fixes #47522

Change-Id: I61d77ba6465f55e3e6187246d79675ba8467ec23
Reviewed-on: https://go-review.googlesource.com/c/go/+/339989
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
src/runtime/crash_cgo_test.go
src/runtime/signal_unix.go
src/runtime/signal_windows.go
src/runtime/testdata/testprogcgo/panic.go [new file with mode: 0644]
src/runtime/testdata/testprogcgo/sigthrow.go [new file with mode: 0644]

index ce7bed920ffda9a22d33557e526bb83886f05191..9df6fcd48b95aabf98a3657d578176d59f573ae4 100644 (file)
@@ -536,6 +536,29 @@ func TestCgoTracebackSigpanic(t *testing.T) {
        }
 }
 
+func TestCgoPanicCallback(t *testing.T) {
+       t.Parallel()
+       got := runTestProg(t, "testprogcgo", "PanicCallback")
+       t.Log(got)
+       want := "panic: runtime error: invalid memory address or nil pointer dereference"
+       if !strings.Contains(got, want) {
+               t.Errorf("did not see %q in output", want)
+       }
+       want = "panic_callback"
+       if !strings.Contains(got, want) {
+               t.Errorf("did not see %q in output", want)
+       }
+       want = "PanicCallback"
+       if !strings.Contains(got, want) {
+               t.Errorf("did not see %q in output", want)
+       }
+       // No runtime errors like "runtime: unexpected return pc".
+       nowant := "runtime: "
+       if strings.Contains(got, nowant) {
+               t.Errorf("did not see %q in output", want)
+       }
+}
+
 // Test that C code called via cgo can use large Windows thread stacks
 // and call back in to Go without crashing. See issue #20975.
 //
@@ -603,6 +626,28 @@ func TestSegv(t *testing.T) {
        }
 }
 
+func TestAbortInCgo(t *testing.T) {
+       switch runtime.GOOS {
+       case "plan9", "windows":
+               // N.B. On Windows, C abort() causes the program to exit
+               // without going through the runtime at all.
+               t.Skipf("no signals on %s", runtime.GOOS)
+       }
+
+       t.Parallel()
+       got := runTestProg(t, "testprogcgo", "Abort")
+       t.Log(got)
+       want := "SIGABRT"
+       if !strings.Contains(got, want) {
+               t.Errorf("did not see %q in output", want)
+       }
+       // No runtime errors like "runtime: unknown pc".
+       nowant := "runtime: "
+       if strings.Contains(got, nowant) {
+               t.Errorf("did not see %q in output", want)
+       }
+}
+
 // TestEINTR tests that we handle EINTR correctly.
 // See issue #20400 and friends.
 func TestEINTR(t *testing.T) {
index 07f371cefe0c402afa267ac62362e0bfb74fdd4d..8854629224ddf70e1e4173bbbff21890da45620e 100644 (file)
@@ -688,9 +688,11 @@ func sighandler(sig uint32, info *siginfo, ctxt unsafe.Pointer, gp *g) {
        }
 
        print("PC=", hex(c.sigpc()), " m=", _g_.m.id, " sigcode=", c.sigcode(), "\n")
-       if _g_.m.lockedg != 0 && _g_.m.ncgo > 0 && gp == _g_.m.g0 {
+       if _g_.m.incgo && gp == _g_.m.g0 && _g_.m.curg != nil {
                print("signal arrived during cgo execution\n")
-               gp = _g_.m.lockedg.ptr()
+               // Switch to curg so that we get a traceback of the Go code
+               // leading up to the cgocall, which switched from curg to g0.
+               gp = _g_.m.curg
        }
        if sig == _SIGILL || sig == _SIGFPE {
                // It would be nice to know how long the instruction is.
index 3fe352ef575bb780a2705292c905c5798390e1e1..16c36d07f14cd7a976f17ec95384ee5273b7ddce 100644 (file)
@@ -218,11 +218,11 @@ func winthrow(info *exceptionrecord, r *context, gp *g) {
        print("Exception ", hex(info.exceptioncode), " ", hex(info.exceptioninformation[0]), " ", hex(info.exceptioninformation[1]), " ", hex(r.ip()), "\n")
 
        print("PC=", hex(r.ip()), "\n")
-       if _g_.m.lockedg != 0 && _g_.m.ncgo > 0 && gp == _g_.m.g0 {
+       if _g_.m.incgo && gp == _g_.m.g0 && _g_.m.curg != nil {
                if iscgo {
                        print("signal arrived during external code execution\n")
                }
-               gp = _g_.m.lockedg.ptr()
+               gp = _g_.m.curg
        }
        print("\n")
 
diff --git a/src/runtime/testdata/testprogcgo/panic.go b/src/runtime/testdata/testprogcgo/panic.go
new file mode 100644 (file)
index 0000000..4ddef3a
--- /dev/null
@@ -0,0 +1,29 @@
+package main
+
+import "C"
+
+// This program will crash.
+// We want to test unwinding from a cgo callback.
+
+/*
+void panic_callback();
+
+static void call_callback(void) {
+       panic_callback();
+}
+*/
+import "C"
+
+func init() {
+       register("PanicCallback", PanicCallback)
+}
+
+//export panic_callback
+func panic_callback() {
+       var i *int
+       *i = 42
+}
+
+func PanicCallback() {
+       C.call_callback()
+}
diff --git a/src/runtime/testdata/testprogcgo/sigthrow.go b/src/runtime/testdata/testprogcgo/sigthrow.go
new file mode 100644 (file)
index 0000000..665e3b0
--- /dev/null
@@ -0,0 +1,20 @@
+// Copyright 2021 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.
+
+package main
+
+// This program will abort.
+
+/*
+#include <stdlib.h>
+*/
+import "C"
+
+func init() {
+       register("Abort", Abort)
+}
+
+func Abort() {
+       C.abort()
+}