From 60ef4b2c1e931069d81f053358a4023e09359e7f Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Tue, 6 Sep 2022 11:42:12 -0400 Subject: [PATCH] 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 Run-TryBot: Austin Clements TryBot-Result: Gopher Robot Auto-Submit: Austin Clements --- src/runtime/traceback.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 4cc5eb91c8..286e9c610e 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -159,7 +159,10 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in if frame.fp == 0 { // Jump over system stack transitions. If we're on g0 and there's a user // goroutine, try to jump. Otherwise this is a regular call. - if flags&_TraceJumpStack != 0 && gp == gp.m.g0 && gp.m.curg != nil { + // We also defensively check that this won't switch M's on us, + // which could happen at critical points in the scheduler. + // This ensures gp.m doesn't change from a stack jump. + if flags&_TraceJumpStack != 0 && gp == gp.m.g0 && gp.m.curg != nil && gp.m.curg.m == gp.m { switch f.funcID { case funcID_morestack: // morestack does not return normally -- newstack() -- 2.48.1