From bb004a179a034c799809f42d525801ec4a791987 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Thu, 24 Feb 2022 16:54:13 -0500 Subject: syscall: define Syscall in terms of RawSyscall on linux This is a re-do of CL 388477, fixing #52472. It is unsafe to call syscall.RawSyscall from syscall.Syscall with -coverpkg=all and -race. This is because: 1. Coverage adds a sync/atomic call in RawSyscall to increment the coverage counter. 2. Race mode instruments sync/atomic calls with TSAN runtime calls. TSAN eventually calls runtime.racecallbackfunc, which expects getg().m.p != 0, which is no longer true after entersyscall(). cmd/go actually avoids adding coverage instrumention to package runtime in race mode entirely to avoid these kinds of problems. Rather than also excluding all of syscall for this one function, work around by calling RawSyscall6 instead, which avoids coverage instrumention both by being written in assembly and in package runtime/*. For #51087 Fixes #52472 Change-Id: Iaffd27df03753020c4716059a455d6ca7b62f347 Reviewed-on: https://go-review.googlesource.com/c/go/+/401654 Run-TryBot: Michael Pratt TryBot-Result: Gopher Robot Auto-Submit: Michael Pratt Reviewed-by: Cherry Mui --- src/syscall/syscall_linux.go | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) (limited to 'src/syscall/syscall_linux.go') diff --git a/src/syscall/syscall_linux.go b/src/syscall/syscall_linux.go index a9a8ecbefd..999ca5bb7f 100644 --- a/src/syscall/syscall_linux.go +++ b/src/syscall/syscall_linux.go @@ -16,7 +16,6 @@ import ( "unsafe" ) -func Syscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno) func Syscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno) // N.B. RawSyscall6 is provided via linkname by runtime/internal/syscall. @@ -26,6 +25,18 @@ func Syscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno) func RawSyscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno) +// Pull in entersyscall/exitsyscall for Syscall/Syscall6. +// +// Note that this can't be a push linkname because the runtime already has a +// nameless linkname to export to assembly here and in x/sys. Additionally, +// entersyscall fetches the caller PC and SP and thus can't have a wrapper +// inbetween. + +//go:linkname runtime_entersyscall runtime.entersyscall +func runtime_entersyscall() +//go:linkname runtime_exitsyscall runtime.exitsyscall +func runtime_exitsyscall() + // N.B. For the Syscall functions below: // // //go:uintptrkeepalive because the uintptr argument may be converted pointers @@ -47,6 +58,28 @@ func RawSyscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno) { return RawSyscall6(trap, a1, a2, a3, 0, 0, 0) } +//go:uintptrkeepalive +//go:nosplit +//go:linkname Syscall +func Syscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno) { + runtime_entersyscall() + // N.B. Calling RawSyscall here is unsafe with atomic coverage + // instrumentation and race mode. + // + // Coverage instrumentation will add a sync/atomic call to RawSyscall. + // Race mode will add race instrumentation to sync/atomic. Race + // instrumentation requires a P, which we no longer have. + // + // RawSyscall6 is fine because it is implemented in assembly and thus + // has no coverage instrumentation. + // + // This is typically not a problem in the runtime because cmd/go avoids + // adding coverage instrumentation to the runtime in race mode. + r1, r2, err = RawSyscall6(trap, a1, a2, a3, 0, 0, 0) + runtime_exitsyscall() + return +} + func rawSyscallNoError(trap, a1, a2, a3 uintptr) (r1, r2 uintptr) /* -- cgit v1.3