From d7abfe4f0dc91568648a66495b9f5d7ebc0f22b5 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Fri, 30 May 2025 17:05:41 -0400 Subject: runtime: acquire/release C TSAN lock when calling cgo symbolizer/tracebacker When calling into C via cmd/cgo, the generated code calls _cgo_tsan_acquire / _cgo_tsan_release around the C call to report a dummy lock to the C/C++ TSAN runtime. This is necessary because the C/C++ TSAN runtime does not understand synchronization within Go and would otherwise report false positive race reports. See the comment in cmd/cgo/out.go for more details. Various C functions in runtime/cgo also contain manual calls to _cgo_tsan_acquire/release where necessary to suppress race reports. However, the cgo symbolizer and cgo traceback functions called from callCgoSymbolizer and cgoContextPCs, respectively, do not have any instrumentation [1]. They call directly into user C functions with no TSAN instrumentation. This means they have an opportunity to report false race conditions. The most direct way is via their argument. Both are passed a pointer to a struct stored on the Go stack, and both write to fields of the struct. If two calls are passed the same pointer from different threads, the C TSAN runtime will think this is a race. This is simple to achieve for the cgo symbolizer function, which the new regression test does. callCgoSymbolizer is called on the standard goroutine stack, so the argument is a pointer into the goroutine stack. If the goroutine moves Ms between two calls, it will look like a race. On the other hand, cgoContextPCs is called on the system stack. Each M has a unique system stack, so for it to pass the same argument pointer on different threads would require the first M to exit, free its stack, and the same region of address space to be used as the stack for a new M. Theoretically possible, but quite unlikely. Both of these are addressed by providing a C wrapper in runtime/cgo that calls _cgo_tsan_acquire/_cgo_tsan_release around calls to the symbolizer and traceback functions. There is a lot of room for future cleanup here. Most runtime/cgo functions have manual instrumentation in their C implementation. That could be removed in favor of instrumentation in the runtime. We could even theoretically remove the instrumentation from cmd/cgo and move it to cgocall. None of these are necessary, but may make things more consistent and easier to follow. [1] Note that the cgo traceback function called from the signal handler via x_cgo_callers _does_ have manual instrumentation. Fixes #73949. Cq-Include-Trybots: luci.golang.try:gotip-freebsd-amd64,gotip-linux-amd64-longtest,gotip-windows-amd64-longtest Change-Id: I6a6a636c9daa38f7fd00694af76b75cb93ba1886 Reviewed-on: https://go-review.googlesource.com/c/go/+/677955 Reviewed-by: Michael Knyszek Auto-Submit: Michael Pratt Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI --- src/runtime/testdata/testprog/setcgotraceback.go | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/runtime/testdata/testprog/setcgotraceback.go (limited to 'src/runtime/testdata') diff --git a/src/runtime/testdata/testprog/setcgotraceback.go b/src/runtime/testdata/testprog/setcgotraceback.go new file mode 100644 index 0000000000..de005027ec --- /dev/null +++ b/src/runtime/testdata/testprog/setcgotraceback.go @@ -0,0 +1,45 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + "internal/abi" + "runtime" + "unsafe" +) + +func init() { + register("SetCgoTracebackNoCgo", SetCgoTracebackNoCgo) +} + +func cgoTraceback() { + panic("unexpectedly reached cgo traceback function") +} + +func cgoContext() { + panic("unexpectedly reached cgo context function") +} + +func cgoSymbolizer() { + panic("unexpectedly reached cgo symbolizer function") +} + +// SetCgoTraceback is a no-op in non-cgo binaries. +func SetCgoTracebackNoCgo() { + traceback := unsafe.Pointer(abi.FuncPCABIInternal(cgoTraceback)) + context := unsafe.Pointer(abi.FuncPCABIInternal(cgoContext)) + symbolizer := unsafe.Pointer(abi.FuncPCABIInternal(cgoSymbolizer)) + runtime.SetCgoTraceback(0, traceback, context, symbolizer) + + // In a cgo binary, runtime.(*Frames).Next calls the cgo symbolizer for + // any non-Go frames. Pass in a bogus frame to verify that Next does + // not attempt to call the cgo symbolizer, which would crash in a + // non-cgo binary like this one. + frames := runtime.CallersFrames([]uintptr{0x12345678}) + frames.Next() + + fmt.Println("OK") +} -- cgit v1.3