Skip to content

Commit c5737dc

Browse files
ianlancetaylorgopherbot
authored andcommitted
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 <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]>
1 parent b9a4a09 commit c5737dc

File tree

8 files changed

+53
-19
lines changed

8 files changed

+53
-19
lines changed

src/runtime/cgo/gcc_sigaction.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build linux && (amd64 || arm64 || loong64 || ppc64le)
5+
//go:build linux && (386 || amd64 || arm64 || loong64 || ppc64le)
66

77
#include <errno.h>
88
#include <stddef.h>
@@ -17,7 +17,7 @@
1717
// to and from struct sigaction — are specific to ${goos}/${goarch}.
1818
typedef struct {
1919
uintptr_t handler;
20-
uint64_t flags;
20+
unsigned long flags;
2121
#ifdef __loongarch__
2222
uint64_t mask;
2323
uintptr_t restorer;
@@ -57,7 +57,7 @@ x_cgo_sigaction(intptr_t signum, const go_sigaction_t *goact, go_sigaction_t *ol
5757
sigaddset(&act.sa_mask, (int)(i+1));
5858
}
5959
}
60-
act.sa_flags = (int)(goact->flags & ~(uint64_t)SA_RESTORER);
60+
act.sa_flags = (int)(goact->flags & ~(unsigned long)SA_RESTORER);
6161
}
6262

6363
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
7979
oldgoact->mask |= (uint64_t)(1)<<i;
8080
}
8181
}
82-
oldgoact->flags = (uint64_t)oldact.sa_flags;
82+
oldgoact->flags = (unsigned long)oldact.sa_flags;
8383
}
8484

8585
_cgo_tsan_release();

src/runtime/cgo/sigaction.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build (linux && (amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64)
5+
//go:build (linux && (386 || amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64)
66

77
package cgo
88

src/runtime/cgo_sigaction.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
// license that can be found in the LICENSE file.
44

55
// Support for sanitizers. See runtime/cgo/sigaction.go.
6+
// Also used on linux/386 to clear the SA_RESTORER flag
7+
// when using cgo; see issue #75253.
68

7-
//go:build (linux && (amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64)
9+
//go:build (linux && (386 || amd64 || arm64 || loong64 || ppc64le)) || (freebsd && amd64)
810

911
package runtime
1012

@@ -42,6 +44,8 @@ func sigaction(sig uint32, new, old *sigactiont) {
4244

4345
var ret int32
4446

47+
fixSigactionForCgo(new)
48+
4549
var g *g
4650
if mainStarted {
4751
g = getg()

src/runtime/crash_cgo_test.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -842,17 +842,6 @@ func TestEINTR(t *testing.T) {
842842
switch runtime.GOOS {
843843
case "plan9", "windows":
844844
t.Skipf("no EINTR on %s", runtime.GOOS)
845-
case "linux":
846-
if runtime.GOARCH == "386" {
847-
// On linux-386 the Go signal handler sets
848-
// a restorer function that is not preserved
849-
// by the C sigaction call in the test,
850-
// causing the signal handler to crash when
851-
// returning the normal code. The test is not
852-
// architecture-specific, so just skip on 386
853-
// rather than doing a complicated workaround.
854-
t.Skip("skipping on linux-386; C sigaction does not preserve Go restorer")
855-
}
856845
}
857846
if runtime.GOOS == "freebsd" && race.Enabled {
858847
t.Skipf("race + cgo freebsd not supported. See https://go.dev/issue/73788.")

src/runtime/os_freebsd.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,12 @@ func sysSigaction(sig uint32, new, old *sigactiont) {
457457
}
458458
}
459459

460+
// fixSigactionForCgo is needed for Linux.
461+
//
462+
//go:nosplit
463+
func fixSigactionForCgo(new *sigactiont) {
464+
}
465+
460466
// asmSigaction is implemented in assembly.
461467
//
462468
//go:noescape

src/runtime/os_linux.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,8 @@ func setsig(i uint32, fn uintptr) {
486486
sigfillset(&sa.sa_mask)
487487
// Although Linux manpage says "sa_restorer element is obsolete and
488488
// should not be used". x86_64 kernel requires it. Only use it on
489-
// x86.
489+
// x86. Note that on 386 this is cleared when using the C sigaction
490+
// function via cgo; see fixSigactionForCgo.
490491
if GOARCH == "386" || GOARCH == "amd64" {
491492
sa.sa_restorer = abi.FuncPCABI0(sigreturn__sigaction)
492493
}
@@ -562,6 +563,21 @@ func sysSigaction(sig uint32, new, old *sigactiont) {
562563
//go:noescape
563564
func rt_sigaction(sig uintptr, new, old *sigactiont, size uintptr) int32
564565

566+
// fixSigactionForCgo is called when we are using cgo to call the
567+
// C sigaction function. On 386 the C function does not expect the
568+
// SA_RESTORER flag to be set, and in some cases will fail if it is set:
569+
// it will pass the SA_RESTORER flag to the kernel without passing
570+
// the sa_restorer field. Since the C function will handle SA_RESTORER
571+
// for us, we need not pass it. See issue #75253.
572+
//
573+
//go:nosplit
574+
func fixSigactionForCgo(new *sigactiont) {
575+
if GOARCH == "386" && new != nil {
576+
new.sa_flags &^= _SA_RESTORER
577+
new.sa_restorer = 0
578+
}
579+
}
580+
565581
func getpid() int
566582
func tgkill(tgid, tid, sig int)
567583

src/runtime/sigaction.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build (linux && !amd64 && !arm64 && !loong64 && !ppc64le) || (freebsd && !amd64)
5+
//go:build (linux && !386 && !amd64 && !arm64 && !loong64 && !ppc64le) || (freebsd && !amd64)
66

77
package runtime
88

src/runtime/sys_linux_386.s

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,25 @@ TEXT runtime·rt_sigaction(SB),NOSPLIT,$0
410410
MOVL AX, ret+16(FP)
411411
RET
412412

413+
// Call the function stored in _cgo_sigaction using the GCC calling convention.
414+
TEXT runtime·callCgoSigaction(SB),NOSPLIT,$0-16
415+
MOVL _cgo_sigaction(SB), AX
416+
MOVL sig+0(FP), BX
417+
MOVL new+4(FP), CX
418+
MOVL old+8(FP), DX
419+
MOVL SP, SI // align stack to call C function
420+
SUBL $32, SP
421+
ANDL $~15, SP
422+
MOVL BX, 0(SP)
423+
MOVL CX, 4(SP)
424+
MOVL DX, 8(SP)
425+
MOVL SI, 12(SP)
426+
CALL AX
427+
MOVL 12(SP), BX
428+
MOVL BX, SP
429+
MOVL AX, ret+12(FP)
430+
RET
431+
413432
TEXT runtime·sigfwd(SB),NOSPLIT,$12-16
414433
MOVL fn+0(FP), AX
415434
MOVL sig+4(FP), BX

0 commit comments

Comments
 (0)