From 5ecd9e34dfe0491f1d76372e272d782578ad5bdb Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 28 Jan 2021 17:17:38 -0500 Subject: [PATCH] runtime: do not treat mcall as a topofstack I added mcall to this list in 2013 without explaining why. (https://codereview.appspot.com/11085043/diff/61001/src/pkg/runtime/traceback_x86.c) I suspect I was stopping crashes during profiling where the unwind tried to walk up past mcall and got confused. mcall is not something you can unwind past, because it switches stacks, but it's also not something you should expect as a standard top-of-stack frame. So if you do see it during say a garbage collection stack walk, it would be important to crash instead of silently stopping the walk prematurely. This CL removes it from the topofstack list to avoid the silent stop. Now that mcall is detected as SPWRITE, that will stop the unwind (with a crash if encountered during GC, which we want). This CL is part of a stack adding windows/arm64 support (#36439), intended to land in the Go 1.17 cycle. This CL is, however, not windows/arm64-specific. It is cleanup meant to make the port (and future ports) easier. Change-Id: I666487ce24efd72292f2bc3eac7fe0477e16bddd Reviewed-on: https://go-review.googlesource.com/c/go/+/288803 Trust: Russ Cox Reviewed-by: Cherry Zhang --- src/runtime/symtab.go | 14 ++++++++++++++ src/runtime/traceback.go | 1 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index d7da255e435..00f802aaa77 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -337,7 +337,21 @@ const ( type funcFlag uint8 const ( + // TOPFRAME indicates a function that appears at the top of its stack. + // The traceback routine stop at such a function and consider that a + // successful, complete traversal of the stack. + // Examples of TOPFRAME functions include goexit, which appears + // at the top of a user goroutine stack, and mstart, which appears + // at the top of a system goroutine stack. funcFlag_TOPFRAME funcFlag = 1 << iota + + // SPWRITE indicates a function that writes an arbitrary value to SP + // (any write other than adding or subtracting a constant amount). + // The traceback routines cannot encode such changes into the + // pcsp tables, so the function traceback cannot safely unwind past + // SPWRITE functions. Stopping at an SPWRITE function is considered + // to be an incomplete unwinding of the stack. In certain contexts + // (in particular garbage collector stack scans) that is a fatal error. funcFlag_SPWRITE ) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index e2bd9689192..c89a8913ae4 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -1003,7 +1003,6 @@ func tracebackHexdump(stk stack, frame *stkframe, bad uintptr) { // Does f mark the top of a goroutine stack? func topofstack(f funcInfo, g0 bool) bool { return f.flag&funcFlag_TOPFRAME != 0 || - f.funcID == funcID_mcall || f.funcID == funcID_morestack || // asmcgocall is TOS on the system stack because it // switches to the system stack, but in this case we