Skip to content

Commit 11c86dd

Browse files
prattmicgopherbot
authored andcommitted
runtime: check for gsignal in asancall/msancall/racecall
asancall and msancall are reachable from the signal handler, where we are running on gsignal. Currently, these calls will use the g0 stack in this case, but if the interrupted code was running on g0 this will corrupt the stack and likely cause a crash. As far as I know, racecall is not reachable from the signal handler, but I have updated it as well for consistency. This is the most straightforward fix, though it would be nice to eventually migrate these wrappers to asmcgocall, which already handled this case. Fixes #71395. Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15,gotip-linux-amd64-msan-clang15,gotip-linux-amd64-race Change-Id: I6a6a636ccba826dd53e31c0e85b5d42fb1e98d12 Reviewed-on: https://go-review.googlesource.com/c/go/+/643875 Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent bc5aa2f commit 11c86dd

15 files changed

+155
-25
lines changed

src/runtime/asan_amd64.s

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,12 @@ TEXT asancall<>(SB), NOSPLIT, $0-0
100100
JE call // no g; still on a system stack
101101

102102
MOVQ g_m(R14), R13
103-
// Switch to g0 stack.
103+
104+
// Switch to g0 stack if we aren't already on g0 or gsignal.
105+
MOVQ m_gsignal(R13), R10
106+
CMPQ R10, R14
107+
JE call // already on gsignal
108+
104109
MOVQ m_g0(R13), R10
105110
CMPQ R10, R14
106111
JE call // already on g0

src/runtime/asan_arm64.s

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,22 @@ TEXT runtime·lsandoleakcheck(SB), NOSPLIT, $0-0
8383
// Switches SP to g0 stack and calls (FARG). Arguments already set.
8484
TEXT asancall<>(SB), NOSPLIT, $0-0
8585
MOVD RSP, R19 // callee-saved
86-
CBZ g, g0stack // no g, still on a system stack
86+
CBZ g, call // no g, still on a system stack
8787
MOVD g_m(g), R10
88+
89+
// Switch to g0 stack if we aren't already on g0 or gsignal.
90+
MOVD m_gsignal(R10), R11
91+
CMP R11, g
92+
BEQ call
93+
8894
MOVD m_g0(R10), R11
8995
CMP R11, g
90-
BEQ g0stack
96+
BEQ call
9197

9298
MOVD (g_sched+gobuf_sp)(R11), R5
9399
MOVD R5, RSP
94100

95-
g0stack:
101+
call:
96102
BL (FARG)
97103
MOVD R19, RSP
98104
RET

src/runtime/asan_loong64.s

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,20 @@ TEXT runtime·lsandoleakcheck(SB), NOSPLIT, $0-0
8383
// Switches SP to g0 stack and calls (FARG). Arguments already set.
8484
TEXT asancall<>(SB), NOSPLIT, $0-0
8585
MOVV R3, R23 // callee-saved
86-
BEQ g, g0stack // no g, still on a system stack
86+
BEQ g, call // no g, still on a system stack
8787
MOVV g_m(g), R14
88+
89+
// Switch to g0 stack if we aren't already on g0 or gsignal.
90+
MOVV m_gsignal(R14), R15
91+
BEQ R15, g, call
92+
8893
MOVV m_g0(R14), R15
89-
BEQ R15, g, g0stack
94+
BEQ R15, g, call
9095

9196
MOVV (g_sched+gobuf_sp)(R15), R9
9297
MOVV R9, R3
9398

94-
g0stack:
99+
call:
95100
JAL (FARG)
96101
MOVV R23, R3
97102
RET

src/runtime/asan_ppc64le.s

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,18 @@ TEXT asancall<>(SB), NOSPLIT, $0-0
8888
MOVD 0(R10), g
8989
MOVD g_m(g), R7 // m for g
9090
MOVD R1, R16 // callee-saved, preserved across C call
91-
MOVD m_g0(R7), R10 // g0 for m
92-
CMP R10, g // same g0?
93-
BEQ call // already on g0
91+
92+
// Switch to g0 stack if we aren't already on g0 or gsignal.
93+
MOVD m_gsignal(R7), R10
94+
CMP R10, g
95+
BEQ call
96+
97+
MOVD m_g0(R7), R10
98+
CMP R10, g
99+
BEQ call
100+
94101
MOVD (g_sched+gobuf_sp)(R10), R1 // switch R1
102+
95103
call:
96104
// prepare frame for C ABI
97105
SUB $32, R1 // create frame for callee saving LR, CR, R2 etc.

src/runtime/asan_riscv64.s

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,19 @@ TEXT runtime·lsandoleakcheck(SB), NOSPLIT, $0-0
7777
// Switches SP to g0 stack and calls (X14). Arguments already set.
7878
TEXT asancall<>(SB), NOSPLIT, $0-0
7979
MOV X2, X8 // callee-saved
80-
BEQZ g, g0stack // no g, still on a system stack
80+
BEQZ g, call // no g, still on a system stack
8181
MOV g_m(g), X21
82+
83+
// Switch to g0 stack if we aren't already on g0 or gsignal.
84+
MOV m_gsignal(X21), X21
85+
BEQ X21, g, call
86+
8287
MOV m_g0(X21), X21
83-
BEQ X21, g, g0stack
88+
BEQ X21, g, call
8489

8590
MOV (g_sched+gobuf_sp)(X21), X2
8691

87-
g0stack:
92+
call:
8893
JALR RA, X14
8994
MOV X8, X2
9095
RET

src/runtime/crash_cgo_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,15 @@ func TestCgoTracebackContextPreemption(t *testing.T) {
310310
}
311311
}
312312

313+
func TestCgoTracebackContextProfile(t *testing.T) {
314+
t.Parallel()
315+
got := runTestProg(t, "testprogcgo", "TracebackContextProfile")
316+
want := "OK\n"
317+
if got != want {
318+
t.Errorf("expected %q got %v", want, got)
319+
}
320+
}
321+
313322
func testCgoPprof(t *testing.T, buildArg, runArg, top, bottom string) {
314323
t.Parallel()
315324
if runtime.GOOS != "linux" || (runtime.GOARCH != "amd64" && runtime.GOARCH != "ppc64le" && runtime.GOARCH != "arm64" && runtime.GOARCH != "loong64") {

src/runtime/msan_amd64.s

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ TEXT msancall<>(SB), NOSPLIT, $0-0
7676
JE call // no g; still on a system stack
7777

7878
MOVQ g_m(R14), R13
79-
// Switch to g0 stack.
79+
80+
// Switch to g0 stack if we aren't already on g0 or gsignal.
81+
MOVQ m_gsignal(R13), R10
82+
CMPQ R10, R14
83+
JE call // already on gsignal
84+
8085
MOVQ m_g0(R13), R10
8186
CMPQ R10, R14
8287
JE call // already on g0

src/runtime/msan_arm64.s

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,22 @@ TEXT runtime·msanmove(SB), NOSPLIT, $0-24
5858
// Switches SP to g0 stack and calls (FARG). Arguments already set.
5959
TEXT msancall<>(SB), NOSPLIT, $0-0
6060
MOVD RSP, R19 // callee-saved
61-
CBZ g, g0stack // no g, still on a system stack
61+
CBZ g, call // no g, still on a system stack
6262
MOVD g_m(g), R10
63+
64+
// Switch to g0 stack if we aren't already on g0 or gsignal.
65+
MOVD m_gsignal(R10), R11
66+
CMP R11, g
67+
BEQ call
68+
6369
MOVD m_g0(R10), R11
6470
CMP R11, g
65-
BEQ g0stack
71+
BEQ call
6672

6773
MOVD (g_sched+gobuf_sp)(R11), R4
6874
MOVD R4, RSP
6975

70-
g0stack:
76+
call:
7177
BL (FARG)
7278
MOVD R19, RSP
7379
RET

src/runtime/msan_loong64.s

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,15 @@ TEXT runtime·msanmove(SB), NOSPLIT, $0-24
5858
// Switches SP to g0 stack and calls (FARG). Arguments already set.
5959
TEXT msancall<>(SB), NOSPLIT, $0-0
6060
MOVV R3, R23 // callee-saved
61-
BEQ g, g0stack // no g, still on a system stack
61+
BEQ g, call // no g, still on a system stack
6262
MOVV g_m(g), R14
63+
64+
// Switch to g0 stack if we aren't already on g0 or gsignal.
65+
MOVV m_gsignal(R14), R15
66+
BEQ R15, g, call
67+
6368
MOVV m_g0(R14), R15
64-
BEQ R15, g, g0stack
69+
BEQ R15, g, call
6570

6671
MOVV (g_sched+gobuf_sp)(R15), R9
6772
MOVV R9, R3

src/runtime/race_amd64.s

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,9 +438,16 @@ TEXT racecall<>(SB), NOSPLIT|NOFRAME, $0-0
438438
MOVQ g_m(R14), R13
439439
// Switch to g0 stack.
440440
MOVQ SP, R12 // callee-saved, preserved across the CALL
441+
442+
// Switch to g0 stack if we aren't already on g0 or gsignal.
443+
MOVQ m_gsignal(R13), R10
444+
CMPQ R10, R14
445+
JE call // already on gsignal
446+
441447
MOVQ m_g0(R13), R10
442448
CMPQ R10, R14
443449
JE call // already on g0
450+
444451
MOVQ (g_sched+gobuf_sp)(R10), SP
445452
call:
446453
ANDQ $~15, SP // alignment for gcc ABI

src/runtime/race_arm64.s

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,9 +463,16 @@ TEXT racecall<>(SB), NOSPLIT|NOFRAME, $0-0
463463
// Switch to g0 stack.
464464
MOVD RSP, R19 // callee-saved, preserved across the CALL
465465
MOVD R30, R20 // callee-saved, preserved across the CALL
466+
467+
// Switch to g0 stack if we aren't already on g0 or gsignal.
468+
MOVD m_gsignal(R10), R11
469+
CMP R11, g
470+
BEQ call
471+
466472
MOVD m_g0(R10), R11
467473
CMP R11, g
468-
BEQ call // already on g0
474+
BEQ call
475+
469476
MOVD (g_sched+gobuf_sp)(R11), R12
470477
MOVD R12, RSP
471478
call:

src/runtime/race_ppc64le.s

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,16 @@ TEXT racecall<>(SB), NOSPLIT, $0-0
484484
MOVD 0(R10), g
485485
MOVD g_m(g), R7 // m for g
486486
MOVD R1, R16 // callee-saved, preserved across C call
487-
MOVD m_g0(R7), R10 // g0 for m
488-
CMP R10, g // same g0?
489-
BEQ call // already on g0
487+
488+
// Switch to g0 stack if we aren't already on g0 or gsignal.
489+
MOVD m_gsignal(R7), R10
490+
CMP R10, g
491+
BEQ call
492+
493+
MOVD m_g0(R7), R10
494+
CMP R10, g
495+
BEQ call
496+
490497
MOVD (g_sched+gobuf_sp)(R10), R1 // switch R1
491498
call:
492499
// prepare frame for C ABI

src/runtime/race_s390x.s

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,16 @@ TEXT racecall<>(SB), NOSPLIT, $0-0
410410
BL runtime·save_g(SB) // Save g for callbacks.
411411
MOVD R15, R7 // Save SP.
412412
MOVD g_m(g), R8 // R8 = thread.
413-
MOVD m_g0(R8), R8 // R8 = g0.
414-
CMPBEQ R8, g, call // Already on g0?
413+
414+
// Switch to g0 stack if we aren't already on g0 or gsignal.
415+
MOVD m_gsignal(R8), R8
416+
CMPBEQ R8, g, call
417+
418+
MOVD m_g0(R8), R8
419+
CMPBEQ R8, g, call
420+
415421
MOVD (g_sched+gobuf_sp)(R8), R15 // Switch SP to g0.
422+
416423
call: SUB $160, R15 // Allocate C frame.
417424
BL R1 // Call C code.
418425
MOVD R7, R15 // Restore SP.

src/runtime/testdata/testprogcgo/tracebackctxt.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,24 @@ extern void tcTraceback(void*);
1717
extern void tcSymbolizer(void*);
1818
extern int getContextCount(void);
1919
extern void TracebackContextPreemptionCallGo(int);
20+
extern void TracebackContextProfileCallGo(void);
2021
*/
2122
import "C"
2223

2324
import (
2425
"fmt"
26+
"io"
2527
"runtime"
28+
"runtime/pprof"
2629
"sync"
30+
"sync/atomic"
2731
"unsafe"
2832
)
2933

3034
func init() {
3135
register("TracebackContext", TracebackContext)
3236
register("TracebackContextPreemption", TracebackContextPreemption)
37+
register("TracebackContextProfile", TracebackContextProfile)
3338
}
3439

3540
var tracebackOK bool
@@ -134,3 +139,41 @@ func TracebackContextPreemptionGoFunction(i C.int) {
134139
// Do some busy work.
135140
fmt.Sprintf("%d\n", i)
136141
}
142+
143+
// Regression test for issue 71395.
144+
//
145+
// The SIGPROF handler can call the SetCgoTraceback traceback function if the
146+
// context function is also provided. Ensure that call is safe.
147+
func TracebackContextProfile() {
148+
runtime.SetCgoTraceback(0, unsafe.Pointer(C.tcTraceback), unsafe.Pointer(C.tcContextSimple), unsafe.Pointer(C.tcSymbolizer))
149+
150+
if err := pprof.StartCPUProfile(io.Discard); err != nil {
151+
panic(fmt.Sprintf("error starting CPU profile: %v", err))
152+
}
153+
defer pprof.StopCPUProfile()
154+
155+
const calls = 1e5
156+
var wg sync.WaitGroup
157+
for i := 0; i < runtime.GOMAXPROCS(0); i++ {
158+
wg.Add(1)
159+
go func() {
160+
defer wg.Done()
161+
for j := 0; j < calls; j++ {
162+
C.TracebackContextProfileCallGo()
163+
}
164+
}()
165+
}
166+
wg.Wait()
167+
168+
fmt.Println("OK")
169+
}
170+
171+
var sink atomic.Pointer[byte]
172+
173+
//export TracebackContextProfileGoFunction
174+
func TracebackContextProfileGoFunction() {
175+
// Issue 71395 occurs when SIGPROF lands on code running on the system
176+
// stack in a cgo callback. The allocator uses the system stack.
177+
b := make([]byte, 128)
178+
sink.Store(&b[0])
179+
}

src/runtime/testdata/testprogcgo/tracebackctxt_c.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
extern void G1(void);
1313
extern void G2(void);
1414
extern void TracebackContextPreemptionGoFunction(int);
15+
extern void TracebackContextProfileGoFunction(void);
1516

1617
void C1() {
1718
G1();
@@ -101,3 +102,7 @@ void tcSymbolizer(void *parg) {
101102
void TracebackContextPreemptionCallGo(int i) {
102103
TracebackContextPreemptionGoFunction(i);
103104
}
105+
106+
void TracebackContextProfileCallGo(void) {
107+
TracebackContextProfileGoFunction();
108+
}

0 commit comments

Comments
 (0)