diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 7be52f499c3d58..0e79a8e8fe6147 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -143,9 +143,18 @@ func buildTestProg(t *testing.T, binary string, flags ...string) (string, error) return exe, nil } -func TestVDSO(t *testing.T) { +func TestSIGPROFInVDSO(t *testing.T) { t.Parallel() - output := runTestProg(t, "testprog", "SignalInVDSO") + output := runTestProg(t, "testprog", "SIGPROFInVDSO") + want := "success\n" + if output != want { + t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want) + } +} + +func TestSIGUSR1InVDSO(t *testing.T) { + t.Parallel() + output := runTestProg(t, "testprog", "SIGUSR1InVDSO") want := "success\n" if output != want { t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want) diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 6a8b5b7ace5072..e3140ebb3837b2 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -274,19 +274,56 @@ func sigpipe() { dieFromSignal(_SIGPIPE) } -// sigFetchG fetches the value of G safely when running in a signal handler. // On some architectures, the g value may be clobbered when running in a VDSO. // See issue #32912. // //go:nosplit -func sigFetchG(c *sigctxt) *g { +func sigClobbered(c *sigctxt) bool { switch GOARCH { case "arm", "arm64": - if inVDSOPage(c.sigpc()) { - return nil + return inVDSOPage(c.sigpc()); + } + return false +} + +// sigpending stores signals during the Go signal handler when the g value is clobbered. +// See issue #34391. +var sigpending [(_NSIG + 31) / 32]uint32 + +func sigAddPending(s uint32) { + for { + p := sigpending[s/32] + q := p | (1 << (s & 31)) + if atomic.Cas(&sigpending[s/32], p, q) { + return } } - return getg() +} + +// sigClearPending is called from outside the signal handler context. +// It should be called just after the clobbered G value is restored. +//go:nosplit +//go:nowritebarrierrec +func sigClearPending() { + for s := 0; s < _NSIG; s++ { + // steal signal from pending queue + steal := false + for { + p := sigpending[s/32] + if p & (1 << (s & 31)) == 0 { + break + } + q := p &^ (1 << (s & 31)) + if atomic.Cas(&sigpending[s/32], p, q) { + steal = true + break + } + } + if !steal { + continue + } + raise(uint32(s)) + } } // sigtrampgo is called from the signal handler function, sigtramp, @@ -305,7 +342,16 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) { return } c := &sigctxt{info, ctx} - g := sigFetchG(c) + if sigClobbered(c) { + if sig == _SIGPROF { + sigprofNonGoPC(c.sigpc()) + return + } + // at this point iscgo must be true + sigAddPending(sig) + return + } + g := getg() if g == nil { if sig == _SIGPROF { sigprofNonGoPC(c.sigpc()) @@ -670,13 +716,19 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool { if (c.sigcode() == _SI_USER || flags&_SigPanic == 0) && sig != _SIGPIPE { return false } - // Determine if the signal occurred inside Go code. We test that: - // (1) we weren't in VDSO page, - // (2) we were in a goroutine (i.e., m.curg != nil), and - // (3) we weren't in CGO. - g := sigFetchG(c) - if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo { - return false + if sigClobbered(c) { + // There is no handler to be forwarded to. + if !iscgo { + return false + } + } else { + // Determine if the signal occurred inside Go code. We test that: + // (1) we were in a goroutine (i.e., m.curg != nil), and + // (2) we weren't in CGO. + g := getg() + if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo { + return false + } } // Signal not handled by Go, forward it. diff --git a/src/runtime/sys_linux_arm.s b/src/runtime/sys_linux_arm.s index c7d83a6d3aced8..86581118e29ae4 100644 --- a/src/runtime/sys_linux_arm.s +++ b/src/runtime/sys_linux_arm.s @@ -247,6 +247,15 @@ noswitch: B.EQ fallback BL (R11) + + SUB $24, R13 + MOVW R4, 8(R13) + MOVW R5, 12(R13) + BL runtime·sigClearPending(SB) + MOVW 12(R13), R5 + MOVW 8(R13), R4 + ADD $24, R13 + JMP finish fallback: @@ -298,6 +307,15 @@ noswitch: B.EQ fallback BL (R11) + + SUB $24, R13 + MOVW R4, 8(R13) + MOVW R5, 12(R13) + BL runtime·sigClearPending(SB) + MOVW 12(R13), R5 + MOVW 8(R13), R4 + ADD $24, R13 + JMP finish fallback: diff --git a/src/runtime/sys_linux_arm64.s b/src/runtime/sys_linux_arm64.s index bce948a91e0427..5d2ee9b61e8c65 100644 --- a/src/runtime/sys_linux_arm64.s +++ b/src/runtime/sys_linux_arm64.s @@ -208,6 +208,13 @@ noswitch: MOVD runtime·vdsoClockgettimeSym(SB), R2 CBZ R2, fallback BL (R2) + + SUB $24, RSP + MOVD R20, 16(RSP) + BL runtime·sigClearPending(SB) + MOVD 16(RSP), R20 + ADD $24, RSP + B finish fallback: @@ -251,6 +258,13 @@ noswitch: MOVD runtime·vdsoClockgettimeSym(SB), R2 CBZ R2, fallback BL (R2) + + SUB $24, RSP + MOVD R20, 16(RSP) + BL runtime·sigClearPending(SB) + MOVD 16(RSP), R20 + ADD $24, RSP + B finish fallback: diff --git a/src/runtime/testdata/testprog/vdso.go b/src/runtime/testdata/testprog/vdso.go index ef92f487581ab3..03c9aaa1fa207c 100644 --- a/src/runtime/testdata/testprog/vdso.go +++ b/src/runtime/testdata/testprog/vdso.go @@ -2,7 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Invoke signal hander in the VDSO context (see issue 32912). +// Invoke signal hander in the VDSO context. +// See issue 32912 and 34391. package main @@ -11,14 +12,16 @@ import ( "io/ioutil" "os" "runtime/pprof" + "syscall" "time" ) func init() { - register("SignalInVDSO", signalInVDSO) + register("SIGPROFInVDSO", signalSIGPROFInVDSO) + register("SIGUSR1InVDSO", signalSIGUSR1InVDSO) } -func signalInVDSO() { +func signalSIGPROFInVDSO() { f, err := ioutil.TempFile("", "timeprofnow") if err != nil { fmt.Fprintln(os.Stderr, err) @@ -53,3 +56,28 @@ func signalInVDSO() { fmt.Println("success") } + +func signalSIGUSR1InVDSO() { + done := make(chan struct {}) + + p, _ := os.FindProcess(os.Getpid()) + go func() { + for { + p.Signal(syscall.SIGUSR1) + select { + case <- done: + return + default: + } + } + }() + + t0 := time.Now() + t1 := t0 + for t1.Sub(t0) < time.Second { + t1 = time.Now() + } + close(done) + + fmt.Println("success") +}