Skip to content

Commit 60ef4b2

Browse files
aclementsgopherbot
authored andcommitted
runtime: in traceback, only jump stack if M doesn't change
CL 424257 modified gentraceback to switch gp when jumping from a system stack to a user stack to simplify reasoning through the rest of the function. This has the unintended side-effect of also switching all references to gp.m. The vast majority of the time, g0.m and curg.m are the same across a stack switch, making this a no-op, but there's at least one case where this isn't true: if a profiling signal happens in execute between setting mp.curg and setting gp.m. In this case, mp.curg.m is briefly nil, which can cause gentraceback to crash with a nil pointer dereference. We see this failure (surprisingly frequently!) in profiling tests in the morestack=mayMoreStackPreempt testing mode (#48297). Fix this by making only jumping stacks if doing so will not switch Ms. This restores the original property that gp.m doesn't change across the stack jump, and makes gentraceback a little more conservative about jumping stacks. Fixes #54885. Change-Id: Ib1524c41c748eeff35896e0f3abf9a7efbe5969f Reviewed-on: https://go-review.googlesource.com/c/go/+/428656 Reviewed-by: Michael Pratt <[email protected]> Run-TryBot: Austin Clements <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Austin Clements <[email protected]>
1 parent e3885c4 commit 60ef4b2

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

src/runtime/traceback.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
159159
if frame.fp == 0 {
160160
// Jump over system stack transitions. If we're on g0 and there's a user
161161
// goroutine, try to jump. Otherwise this is a regular call.
162-
if flags&_TraceJumpStack != 0 && gp == gp.m.g0 && gp.m.curg != nil {
162+
// We also defensively check that this won't switch M's on us,
163+
// which could happen at critical points in the scheduler.
164+
// This ensures gp.m doesn't change from a stack jump.
165+
if flags&_TraceJumpStack != 0 && gp == gp.m.g0 && gp.m.curg != nil && gp.m.curg.m == gp.m {
163166
switch f.funcID {
164167
case funcID_morestack:
165168
// morestack does not return normally -- newstack()

0 commit comments

Comments
 (0)