From: Ian Lance Taylor Date: Sat, 6 Sep 2025 05:24:37 +0000 (-0700) Subject: runtime: when using cgo on 386, call C sigaction function X-Git-Tag: go1.26rc1~917 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c5737dc21bbac9fbefc35ac9313e66291d66b382;p=gostls13.git runtime: when using cgo on 386, call C sigaction function On 386 the C sigaction function assumes that the caller does not set the SA_RESTORER flag. It does not copy the C sa_restorer field to the kernel sa_restorer field. The effect is that the kernel sees the SA_RESTORER flag but a NULL sa_restorer field, and the program crashes when returning from a signal handler. On the other hand, the C sigaction function will return the SA_RESTORER flag and the sa_restorer field stored in the kernel. This means that if the Go runtime installs a signal handler, with SA_RESTORER as is required when calling the kernel, and the Go program calls C code that calls the C sigaction function to query the current signal handler, that C code will get a result that it can't pass back to sigaction. This CL fixes the problem by using the C sigaction function for 386 programs that use cgo. This reuses the functionality used on amd64 and other GOARCHs to support the race detector. See #75253, or runtime/testdata/testprogcgo/eintr.go, for sample code that used to fail on 386. No new test case is required, we just remove the skip we used to have for eintr.go. Fixes #75253 Change-Id: I803059b1fb9e09e9fbb43f68eccb6a59a92c2991 Reviewed-on: https://go-review.googlesource.com/c/go/+/701375 LUCI-TryBot-Result: Go LUCI Reviewed-by: Cherry Mui Reviewed-by: Dmitri Shuralyov Auto-Submit: Ian Lance Taylor --- diff --git a/src/runtime/cgo/gcc_sigaction.c b/src/runtime/cgo/gcc_sigaction.c index 7cbef7db11..ad48a88dc1 100644 --- a/src/runtime/cgo/gcc_sigaction.c +++ b/src/runtime/cgo/gcc_sigaction.c @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build linux && (amd64 || arm64 || loong64 || ppc64le) +//go:build linux && (386 || amd64 || arm64 || loong64 || ppc64le) #include #include @@ -17,7 +17,7 @@ // to and from struct sigaction — are specific to ${goos}/${goarch}. typedef struct { uintptr_t handler; - uint64_t flags; + unsigned long flags; #ifdef __loongarch__ uint64_t mask; uintptr_t restorer; @@ -57,7 +57,7 @@ x_cgo_sigaction(intptr_t signum, const go_sigaction_t *goact, go_sigaction_t *ol sigaddset(&act.sa_mask, (int)(i+1)); } } - act.sa_flags = (int)(goact->flags & ~(uint64_t)SA_RESTORER); + act.sa_flags = (int)(goact->flags & ~(unsigned long)SA_RESTORER); } ret = sigaction((int)signum, goact ? &act : NULL, oldgoact ? &oldact : NULL); @@ -79,7 +79,7 @@ x_cgo_sigaction(intptr_t signum, const go_sigaction_t *goact, go_sigaction_t *ol oldgoact->mask |= (uint64_t)(1)<flags = (uint64_t)oldact.sa_flags; + oldgoact->flags = (unsigned long)oldact.sa_flags; } _cgo_tsan_release(); diff --git a/src/runtime/cgo/sigaction.go b/src/runtime/cgo/sigaction.go index dc3f5fd255..90034bad32 100644 --- a/src/runtime/cgo/sigaction.go +++ b/src/runtime/cgo/sigaction.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build (linux && (amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64) +//go:build (linux && (386 || amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64) package cgo diff --git a/src/runtime/cgo_sigaction.go b/src/runtime/cgo_sigaction.go index 5c644587f0..f725dbef4d 100644 --- a/src/runtime/cgo_sigaction.go +++ b/src/runtime/cgo_sigaction.go @@ -3,8 +3,10 @@ // license that can be found in the LICENSE file. // Support for sanitizers. See runtime/cgo/sigaction.go. +// Also used on linux/386 to clear the SA_RESTORER flag +// when using cgo; see issue #75253. -//go:build (linux && (amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64) +//go:build (linux && (386 || amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64) package runtime @@ -42,6 +44,8 @@ func sigaction(sig uint32, new, old *sigactiont) { var ret int32 + fixSigactionForCgo(new) + var g *g if mainStarted { g = getg() diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index c5d7303261..b77ff8dafd 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -842,17 +842,6 @@ func TestEINTR(t *testing.T) { switch runtime.GOOS { case "plan9", "windows": t.Skipf("no EINTR on %s", runtime.GOOS) - case "linux": - if runtime.GOARCH == "386" { - // On linux-386 the Go signal handler sets - // a restorer function that is not preserved - // by the C sigaction call in the test, - // causing the signal handler to crash when - // returning the normal code. The test is not - // architecture-specific, so just skip on 386 - // rather than doing a complicated workaround. - t.Skip("skipping on linux-386; C sigaction does not preserve Go restorer") - } } if runtime.GOOS == "freebsd" && race.Enabled { t.Skipf("race + cgo freebsd not supported. See https://go.dev/issue/73788.") diff --git a/src/runtime/os_freebsd.go b/src/runtime/os_freebsd.go index ab859cfb47..68d895b95d 100644 --- a/src/runtime/os_freebsd.go +++ b/src/runtime/os_freebsd.go @@ -457,6 +457,12 @@ func sysSigaction(sig uint32, new, old *sigactiont) { } } +// fixSigactionForCgo is needed for Linux. +// +//go:nosplit +func fixSigactionForCgo(new *sigactiont) { +} + // asmSigaction is implemented in assembly. // //go:noescape diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go index c9d25a5be8..f9fe1b5f33 100644 --- a/src/runtime/os_linux.go +++ b/src/runtime/os_linux.go @@ -486,7 +486,8 @@ func setsig(i uint32, fn uintptr) { sigfillset(&sa.sa_mask) // Although Linux manpage says "sa_restorer element is obsolete and // should not be used". x86_64 kernel requires it. Only use it on - // x86. + // x86. Note that on 386 this is cleared when using the C sigaction + // function via cgo; see fixSigactionForCgo. if GOARCH == "386" || GOARCH == "amd64" { sa.sa_restorer = abi.FuncPCABI0(sigreturn__sigaction) } @@ -562,6 +563,21 @@ func sysSigaction(sig uint32, new, old *sigactiont) { //go:noescape func rt_sigaction(sig uintptr, new, old *sigactiont, size uintptr) int32 +// fixSigactionForCgo is called when we are using cgo to call the +// C sigaction function. On 386 the C function does not expect the +// SA_RESTORER flag to be set, and in some cases will fail if it is set: +// it will pass the SA_RESTORER flag to the kernel without passing +// the sa_restorer field. Since the C function will handle SA_RESTORER +// for us, we need not pass it. See issue #75253. +// +//go:nosplit +func fixSigactionForCgo(new *sigactiont) { + if GOARCH == "386" && new != nil { + new.sa_flags &^= _SA_RESTORER + new.sa_restorer = 0 + } +} + func getpid() int func tgkill(tgid, tid, sig int) diff --git a/src/runtime/sigaction.go b/src/runtime/sigaction.go index 2027ae80bf..1a99f7f3ec 100644 --- a/src/runtime/sigaction.go +++ b/src/runtime/sigaction.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build (linux && !amd64 && !arm64 && !loong64 && !ppc64le) || (freebsd && !amd64) +//go:build (linux && !386 && !amd64 && !arm64 && !loong64 && !ppc64le) || (freebsd && !amd64) package runtime diff --git a/src/runtime/sys_linux_386.s b/src/runtime/sys_linux_386.s index d53be243fe..8e832687e0 100644 --- a/src/runtime/sys_linux_386.s +++ b/src/runtime/sys_linux_386.s @@ -410,6 +410,25 @@ TEXT runtime·rt_sigaction(SB),NOSPLIT,$0 MOVL AX, ret+16(FP) RET +// Call the function stored in _cgo_sigaction using the GCC calling convention. +TEXT runtime·callCgoSigaction(SB),NOSPLIT,$0-16 + MOVL _cgo_sigaction(SB), AX + MOVL sig+0(FP), BX + MOVL new+4(FP), CX + MOVL old+8(FP), DX + MOVL SP, SI // align stack to call C function + SUBL $32, SP + ANDL $~15, SP + MOVL BX, 0(SP) + MOVL CX, 4(SP) + MOVL DX, 8(SP) + MOVL SI, 12(SP) + CALL AX + MOVL 12(SP), BX + MOVL BX, SP + MOVL AX, ret+12(FP) + RET + TEXT runtime·sigfwd(SB),NOSPLIT,$12-16 MOVL fn+0(FP), AX MOVL sig+4(FP), BX