From a0c9d153e0c177677701b8a4e6e5eba5a6c44a4f Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Tue, 17 Jan 2023 19:15:39 -0500 Subject: [PATCH] runtime: don't inject a sigpanic if not on user G stack If a panicking signal (e.g. SIGSEGV) happens on a g0 stack, we're either in the runtime or running C code. Either way we cannot recover and sigpanic will immediately throw. Further, injecting a sigpanic could make the C stack unwinder and the debugger fail to unwind the stack. So don't inject a sigpanic. If we have cgo traceback and symbolizer attached, if it panics in a C function ("CF" for the example below), previously it shows something like fatal error: unexpected signal during runtime execution [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x45f1ef] runtime stack: runtime.throw({0x485460?, 0x0?}) .../runtime/panic.go:1076 +0x5c fp=0x7ffd77f60f58 sp=0x7ffd77f60f28 pc=0x42e39c runtime.sigpanic() .../runtime/signal_unix.go:821 +0x3e9 fp=0x7ffd77f60fb8 sp=0x7ffd77f60f58 pc=0x442229 goroutine 1 [syscall]: CF /tmp/pp/c.c:6 pc=0x45f1ef runtime.asmcgocall .../runtime/asm_amd64.s:869 pc=0x458007 runtime.cgocall(0x45f1d0, 0xc000053f70) .../runtime/cgocall.go:158 +0x51 fp=0xc000053f48 sp=0xc000053f10 pc=0x404551 main._Cfunc_CF() _cgo_gotypes.go:39 +0x3f fp=0xc000053f70 sp=0xc000053f48 pc=0x45f0bf Now it shows SIGSEGV: segmentation violation PC=0x45f1ef m=0 sigcode=1 signal arrived during cgo execution goroutine 1 [syscall]: CF /tmp/pp/c.c:6 pc=0x45f1ef runtime.asmcgocall .../runtime/asm_amd64.s:869 pc=0x458007 runtime.cgocall(0x45f1d0, 0xc00004ef70) .../runtime/cgocall.go:158 +0x51 fp=0xc00004ef48 sp=0xc00004ef10 pc=0x404551 main._Cfunc_CF() _cgo_gotypes.go:39 +0x3f fp=0xc00004ef70 sp=0xc00004ef48 pc=0x45f0bf I think the new one is reasonable. For #57698. Change-Id: I4f7af91761374e9b569dce4c7587499d4799137e Reviewed-on: https://go-review.googlesource.com/c/go/+/462437 Reviewed-by: Michael Pratt TryBot-Result: Gopher Robot Run-TryBot: Cherry Mui --- src/runtime/crash_cgo_test.go | 10 ++++++++-- src/runtime/signal_unix.go | 6 +++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index ad15ebf9c2..fb34c7fe3e 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -518,15 +518,21 @@ func TestCgoTracebackSigpanic(t *testing.T) { t.Parallel() got := runTestProg(t, "testprogcgo", "TracebackSigpanic") t.Log(got) - want := "runtime.sigpanic" + // We should see the function that calls the C function. + want := "main.TracebackSigpanic" if !strings.Contains(got, want) { if runtime.GOOS == "android" && (runtime.GOARCH == "arm" || runtime.GOARCH == "arm64") { testenv.SkipFlaky(t, 58794) } t.Errorf("did not see %q in output", want) } + // We shouldn't inject a sigpanic call. (see issue 57698) + nowant := "runtime.sigpanic" + if strings.Contains(got, nowant) { + t.Errorf("unexpectedly saw %q in output", nowant) + } // No runtime errors like "runtime: unexpected return pc". - nowant := "runtime: " + nowant = "runtime: " if strings.Contains(got, nowant) { t.Errorf("unexpectedly saw %q in output", nowant) } diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 026a503eb1..d1719b22ff 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -673,9 +673,13 @@ func sighandler(sig uint32, info *siginfo, ctxt unsafe.Pointer, gp *g) { if sig < uint32(len(sigtable)) { flags = sigtable[sig].flags } - if !c.sigFromUser() && flags&_SigPanic != 0 && gp.throwsplit { + if !c.sigFromUser() && flags&_SigPanic != 0 && (gp.throwsplit || gp != mp.curg) { // We can't safely sigpanic because it may grow the // stack. Abort in the signal handler instead. + // + // Also don't inject a sigpanic if we are not on a + // user G stack. Either we're in the runtime, or we're + // running C code. Either way we cannot recover. flags = _SigThrow } if isAbortPC(c.sigpc()) { -- 2.50.0