diff options
| author | Austin Clements <austin@google.com> | 2022-09-06 11:42:12 -0400 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2022-09-06 16:04:07 +0000 |
| commit | 60ef4b2c1e931069d81f053358a4023e09359e7f (patch) | |
| tree | 91095cae71bbfd27b7425cc906317c0a2334eb4a /src | |
| parent | e3885c4ee5f35a7012fa11d8fc84a27c3c1177a0 (diff) | |
| download | go-60ef4b2c1e931069d81f053358a4023e09359e7f.tar.xz | |
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 <mpratt@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Austin Clements <austin@google.com>
Diffstat (limited to 'src')
| -rw-r--r-- | src/runtime/traceback.go | 5 |
1 files changed, 4 insertions, 1 deletions
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() |