From 091db6392da5913e4bd4806215102e461dc5649c Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Wed, 4 Aug 2021 11:24:28 -0400 Subject: [PATCH] runtime: fix cgo signals detection 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 Run-TryBot: Michael Pratt TryBot-Result: Go Bot Reviewed-by: Cherry Mui Reviewed-by: Ian Lance Taylor Reviewed-by: Austin Clements --- src/runtime/crash_cgo_test.go | 45 ++++++++++++++++++++ src/runtime/signal_unix.go | 6 ++- src/runtime/signal_windows.go | 4 +- src/runtime/testdata/testprogcgo/panic.go | 29 +++++++++++++ src/runtime/testdata/testprogcgo/sigthrow.go | 20 +++++++++ 5 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 src/runtime/testdata/testprogcgo/panic.go create mode 100644 src/runtime/testdata/testprogcgo/sigthrow.go diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index ce7bed920ff..9df6fcd48b9 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -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) { diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 07f371cefe0..8854629224d 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -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. diff --git a/src/runtime/signal_windows.go b/src/runtime/signal_windows.go index 3fe352ef575..16c36d07f14 100644 --- a/src/runtime/signal_windows.go +++ b/src/runtime/signal_windows.go @@ -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 index 00000000000..4ddef3abcde --- /dev/null +++ b/src/runtime/testdata/testprogcgo/panic.go @@ -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 index 00000000000..665e3b02df1 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/sigthrow.go @@ -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 +*/ +import "C" + +func init() { + register("Abort", Abort) +} + +func Abort() { + C.abort() +}