Skip to content

Commit d7abfe4

Browse files
prattmicgopherbot
authored andcommitted
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 <[email protected]> Auto-Submit: Michael Pratt <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 393d91a commit d7abfe4

File tree

13 files changed

+462
-54
lines changed

13 files changed

+462
-54
lines changed
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
/*
8+
// Defined in tracebackctxt_c.c.
9+
extern void C1(void);
10+
extern void C2(void);
11+
extern void tcContext(void*);
12+
extern void tcTraceback(void*);
13+
extern void tcSymbolizer(void*);
14+
*/
15+
import "C"
16+
17+
import (
18+
"fmt"
19+
"runtime"
20+
"sync"
21+
"unsafe"
22+
)
23+
24+
// Regression test for https://go.dev/issue/73949. TSAN should not report races
25+
// on writes to the argument passed to the symbolizer function.
26+
//
27+
// Triggering this race requires calls to the symbolizer function with the same
28+
// argument pointer on multiple threads. The runtime passes a stack variable to
29+
// this function, so that means we need to get a single goroutine to execute on
30+
// two threads, calling the symbolizer function on each.
31+
//
32+
// runtime.CallersFrames / Next will call the symbolizer function (if there are
33+
// C frames). So the approach here is, with GOMAXPROCS=2, have 2 goroutines
34+
// that use CallersFrames over and over, both frequently calling Gosched in an
35+
// attempt to get picked up by the other P.
36+
37+
var tracebackOK bool
38+
39+
func main() {
40+
runtime.GOMAXPROCS(2)
41+
runtime.SetCgoTraceback(0, unsafe.Pointer(C.tcTraceback), unsafe.Pointer(C.tcContext), unsafe.Pointer(C.tcSymbolizer))
42+
C.C1()
43+
if tracebackOK {
44+
fmt.Println("OK")
45+
}
46+
}
47+
48+
//export G1
49+
func G1() {
50+
C.C2()
51+
}
52+
53+
//export G2
54+
func G2() {
55+
pc := make([]uintptr, 32)
56+
n := runtime.Callers(0, pc)
57+
58+
var wg sync.WaitGroup
59+
for range 2 {
60+
wg.Go(func() {
61+
for range 1000 {
62+
cf := runtime.CallersFrames(pc[:n])
63+
var frames []runtime.Frame
64+
for {
65+
frame, more := cf.Next()
66+
frames = append(frames, frame)
67+
if !more {
68+
break
69+
}
70+
}
71+
runtime.Gosched()
72+
}
73+
})
74+
}
75+
wg.Wait()
76+
77+
tracebackOK = true
78+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// The C definitions for tracebackctxt.go. That file uses //export so
6+
// it can't put function definitions in the "C" import comment.
7+
8+
#include <stdint.h>
9+
#include <stdio.h>
10+
11+
// Functions exported from Go.
12+
extern void G1(void);
13+
extern void G2(void);
14+
15+
void C1() {
16+
G1();
17+
}
18+
19+
void C2() {
20+
G2();
21+
}
22+
23+
struct cgoContextArg {
24+
uintptr_t context;
25+
};
26+
27+
struct cgoTracebackArg {
28+
uintptr_t context;
29+
uintptr_t sigContext;
30+
uintptr_t* buf;
31+
uintptr_t max;
32+
};
33+
34+
struct cgoSymbolizerArg {
35+
uintptr_t pc;
36+
const char* file;
37+
uintptr_t lineno;
38+
const char* func;
39+
uintptr_t entry;
40+
uintptr_t more;
41+
uintptr_t data;
42+
};
43+
44+
void tcContext(void* parg) {
45+
struct cgoContextArg* arg = (struct cgoContextArg*)(parg);
46+
if (arg->context == 0) {
47+
arg->context = 1;
48+
}
49+
}
50+
51+
void tcTraceback(void* parg) {
52+
int base, i;
53+
struct cgoTracebackArg* arg = (struct cgoTracebackArg*)(parg);
54+
if (arg->max < 1) {
55+
return;
56+
}
57+
arg->buf[0] = 6; // Chosen by fair dice roll.
58+
}
59+
60+
void tcSymbolizer(void *parg) {
61+
struct cgoSymbolizerArg* arg = (struct cgoSymbolizerArg*)(parg);
62+
if (arg->pc == 0) {
63+
return;
64+
}
65+
// Report two lines per PC returned by traceback, to test more handling.
66+
arg->more = arg->file == NULL;
67+
arg->file = "tracebackctxt.go";
68+
arg->func = "cFunction";
69+
arg->lineno = arg->pc + (arg->more << 16);
70+
}

src/cmd/cgo/internal/testsanitizers/tsan_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ func TestTSAN(t *testing.T) {
5656
{src: "tsan13.go", needsRuntime: true},
5757
{src: "tsan14.go", needsRuntime: true},
5858
{src: "tsan15.go", needsRuntime: true},
59+
{src: "tsan_tracebackctxt", needsRuntime: true}, // Subdirectory
5960
}
6061
for _, tc := range cases {
6162
tc := tc
@@ -67,7 +68,7 @@ func TestTSAN(t *testing.T) {
6768
defer dir.RemoveAll(t)
6869

6970
outPath := dir.Join(name)
70-
mustRun(t, config.goCmd("build", "-o", outPath, srcPath(tc.src)))
71+
mustRun(t, config.goCmd("build", "-o", outPath, "./"+srcPath(tc.src)))
7172

7273
cmdArgs := []string{outPath}
7374
if goos == "linux" {

src/runtime/cgo.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import "unsafe"
1515
//go:linkname _cgo_sys_thread_create _cgo_sys_thread_create
1616
//go:linkname _cgo_notify_runtime_init_done _cgo_notify_runtime_init_done
1717
//go:linkname _cgo_callers _cgo_callers
18-
//go:linkname _cgo_set_context_function _cgo_set_context_function
18+
//go:linkname _cgo_set_traceback_functions _cgo_set_traceback_functions
19+
//go:linkname _cgo_call_traceback_function _cgo_call_traceback_function
20+
//go:linkname _cgo_call_symbolizer_function _cgo_call_symbolizer_function
1921
//go:linkname _cgo_yield _cgo_yield
2022
//go:linkname _cgo_pthread_key_created _cgo_pthread_key_created
2123
//go:linkname _cgo_bindm _cgo_bindm
@@ -27,7 +29,9 @@ var (
2729
_cgo_sys_thread_create unsafe.Pointer
2830
_cgo_notify_runtime_init_done unsafe.Pointer
2931
_cgo_callers unsafe.Pointer
30-
_cgo_set_context_function unsafe.Pointer
32+
_cgo_set_traceback_functions unsafe.Pointer
33+
_cgo_call_traceback_function unsafe.Pointer
34+
_cgo_call_symbolizer_function unsafe.Pointer
3135
_cgo_yield unsafe.Pointer
3236
_cgo_pthread_key_created unsafe.Pointer
3337
_cgo_bindm unsafe.Pointer

src/runtime/cgo/callbacks.go

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,30 @@ var _cgo_bindm = &x_cgo_bindm
121121
var x_cgo_notify_runtime_init_done byte
122122
var _cgo_notify_runtime_init_done = &x_cgo_notify_runtime_init_done
123123

124-
// Sets the traceback context function. See runtime.SetCgoTraceback.
125-
126-
//go:cgo_import_static x_cgo_set_context_function
127-
//go:linkname x_cgo_set_context_function x_cgo_set_context_function
128-
//go:linkname _cgo_set_context_function _cgo_set_context_function
129-
var x_cgo_set_context_function byte
130-
var _cgo_set_context_function = &x_cgo_set_context_function
124+
// Sets the traceback, context, and symbolizer functions. See
125+
// runtime.SetCgoTraceback.
126+
127+
//go:cgo_import_static x_cgo_set_traceback_functions
128+
//go:linkname x_cgo_set_traceback_functions x_cgo_set_traceback_functions
129+
//go:linkname _cgo_set_traceback_functions _cgo_set_traceback_functions
130+
var x_cgo_set_traceback_functions byte
131+
var _cgo_set_traceback_functions = &x_cgo_set_traceback_functions
132+
133+
// Call the traceback function registered with x_cgo_set_traceback_functions.
134+
135+
//go:cgo_import_static x_cgo_call_traceback_function
136+
//go:linkname x_cgo_call_traceback_function x_cgo_call_traceback_function
137+
//go:linkname _cgo_call_traceback_function _cgo_call_traceback_function
138+
var x_cgo_call_traceback_function byte
139+
var _cgo_call_traceback_function = &x_cgo_call_traceback_function
140+
141+
// Call the symbolizer function registered with x_cgo_set_symbolizer_functions.
142+
143+
//go:cgo_import_static x_cgo_call_symbolizer_function
144+
//go:linkname x_cgo_call_symbolizer_function x_cgo_call_symbolizer_function
145+
//go:linkname _cgo_call_symbolizer_function _cgo_call_symbolizer_function
146+
var x_cgo_call_symbolizer_function byte
147+
var _cgo_call_symbolizer_function = &x_cgo_call_symbolizer_function
131148

132149
// Calls a libc function to execute background work injected via libc
133150
// interceptors, such as processing pending signals under the thread

src/runtime/cgo/gcc_context.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88

99
// Releases the cgo traceback context.
1010
void _cgo_release_context(uintptr_t ctxt) {
11-
void (*pfn)(struct context_arg*);
11+
void (*pfn)(struct cgoContextArg*);
1212

1313
pfn = _cgo_get_context_function();
1414
if (ctxt != 0 && pfn != nil) {
15-
struct context_arg arg;
15+
struct cgoContextArg arg;
1616

1717
arg.Context = ctxt;
1818
(*pfn)(&arg);

src/runtime/cgo/gcc_libinit.c

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,14 @@ static void pthread_key_destructor(void* g);
3232
uintptr_t x_cgo_pthread_key_created;
3333
void (*x_crosscall2_ptr)(void (*fn)(void *), void *, int, size_t);
3434

35+
// The traceback function, used when tracing C calls.
36+
static void (*cgo_traceback_function)(struct cgoTracebackArg*);
37+
3538
// The context function, used when tracing back C calls into Go.
36-
static void (*cgo_context_function)(struct context_arg*);
39+
static void (*cgo_context_function)(struct cgoContextArg*);
40+
41+
// The symbolizer function, used when symbolizing C frames.
42+
static void (*cgo_symbolizer_function)(struct cgoSymbolizerArg*);
3743

3844
void
3945
x_cgo_sys_thread_create(void* (*func)(void*), void* arg) {
@@ -52,7 +58,7 @@ x_cgo_sys_thread_create(void* (*func)(void*), void* arg) {
5258

5359
uintptr_t
5460
_cgo_wait_runtime_init_done(void) {
55-
void (*pfn)(struct context_arg*);
61+
void (*pfn)(struct cgoContextArg*);
5662
int done;
5763

5864
pfn = __atomic_load_n(&cgo_context_function, __ATOMIC_CONSUME);
@@ -70,7 +76,6 @@ _cgo_wait_runtime_init_done(void) {
7076
x_cgo_pthread_key_created = 1;
7177
}
7278

73-
7479
// TODO(iant): For the case of a new C thread calling into Go, such
7580
// as when using -buildmode=c-archive, we know that Go runtime
7681
// initialization is complete but we do not know that all Go init
@@ -87,7 +92,7 @@ _cgo_wait_runtime_init_done(void) {
8792
}
8893

8994
if (pfn != nil) {
90-
struct context_arg arg;
95+
struct cgoContextArg arg;
9196

9297
arg.Context = 0;
9398
(*pfn)(&arg);
@@ -138,17 +143,71 @@ x_cgo_notify_runtime_init_done(void* dummy __attribute__ ((unused))) {
138143
pthread_mutex_unlock(&runtime_init_mu);
139144
}
140145

141-
// Sets the context function to call to record the traceback context
142-
// when calling a Go function from C code. Called from runtime.SetCgoTraceback.
143-
void x_cgo_set_context_function(void (*context)(struct context_arg*)) {
144-
__atomic_store_n(&cgo_context_function, context, __ATOMIC_RELEASE);
146+
// Sets the traceback, context, and symbolizer functions. Called from
147+
// runtime.SetCgoTraceback.
148+
void x_cgo_set_traceback_functions(struct cgoSetTracebackFunctionsArg* arg) {
149+
__atomic_store_n(&cgo_traceback_function, arg->Traceback, __ATOMIC_RELEASE);
150+
__atomic_store_n(&cgo_context_function, arg->Context, __ATOMIC_RELEASE);
151+
__atomic_store_n(&cgo_symbolizer_function, arg->Symbolizer, __ATOMIC_RELEASE);
152+
}
153+
154+
// Gets the traceback function to call to trace C calls.
155+
void (*(_cgo_get_traceback_function(void)))(struct cgoTracebackArg*) {
156+
return __atomic_load_n(&cgo_traceback_function, __ATOMIC_CONSUME);
157+
}
158+
159+
// Call the traceback function registered with x_cgo_set_traceback_functions.
160+
//
161+
// The traceback function is an arbitrary user C function which may be built
162+
// with TSAN, and thus must be wrapped with TSAN acquire/release calls. For
163+
// normal cgo calls, cmd/cgo automatically inserts TSAN acquire/release calls.
164+
// Since the traceback, context, and symbolizer functions are registered at
165+
// startup and called via the runtime, they do not get automatic TSAN
166+
// acquire/release calls.
167+
//
168+
// The only purpose of this wrapper is to perform TSAN acquire/release.
169+
// Alternatively, if the runtime arranged to safely call TSAN acquire/release,
170+
// it could perform the call directly.
171+
void x_cgo_call_traceback_function(struct cgoTracebackArg* arg) {
172+
void (*pfn)(struct cgoTracebackArg*);
173+
174+
pfn = _cgo_get_traceback_function();
175+
if (pfn == nil) {
176+
return;
177+
}
178+
179+
_cgo_tsan_acquire();
180+
(*pfn)(arg);
181+
_cgo_tsan_release();
145182
}
146183

147-
// Gets the context function.
148-
void (*(_cgo_get_context_function(void)))(struct context_arg*) {
184+
// Gets the context function to call to record the traceback context
185+
// when calling a Go function from C code.
186+
void (*(_cgo_get_context_function(void)))(struct cgoContextArg*) {
149187
return __atomic_load_n(&cgo_context_function, __ATOMIC_CONSUME);
150188
}
151189

190+
// Gets the symbolizer function to call to symbolize C frames.
191+
void (*(_cgo_get_symbolizer_function(void)))(struct cgoSymbolizerArg*) {
192+
return __atomic_load_n(&cgo_symbolizer_function, __ATOMIC_CONSUME);
193+
}
194+
195+
// Call the symbolizer function registered with x_cgo_set_traceback_functions.
196+
//
197+
// See comment on x_cgo_call_traceback_function.
198+
void x_cgo_call_symbolizer_function(struct cgoSymbolizerArg* arg) {
199+
void (*pfn)(struct cgoSymbolizerArg*);
200+
201+
pfn = _cgo_get_symbolizer_function();
202+
if (pfn == nil) {
203+
return;
204+
}
205+
206+
_cgo_tsan_acquire();
207+
(*pfn)(arg);
208+
_cgo_tsan_release();
209+
}
210+
152211
// _cgo_try_pthread_create retries pthread_create if it fails with
153212
// EAGAIN.
154213
int

0 commit comments

Comments
 (0)