From c9b2da3f28195fab2dd8fc4133efd22409e9e29b Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Mon, 13 Feb 2023 17:57:26 -0500
Subject: [PATCH] runtime: simplify traceback PC back-up logic

Updates #54466.

Change-Id: If070cf3f484e3e02b8e586bff466e0018b1a1845
Reviewed-on: https://go-review.googlesource.com/c/go/+/468298
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
---
 src/runtime/traceback.go | 49 +++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 665961f9b1..30f874bc73 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -45,6 +45,10 @@ const (
 	// unwindTrap indicates that the initial PC and SP are from a trap, not a
 	// return PC from a call.
 	//
+	// The unwindTrap flag is updated during unwinding. If set, frame.pc is the
+	// address of a faulting instruction instead of the return address of a
+	// call. It also means the liveness at pc may not be known.
+	//
 	// TODO: Distinguish frame.continpc, which is really the stack map PC, from
 	// the actual continuation PC, which is computed differently depending on
 	// this flag and a few other things.
@@ -461,6 +465,11 @@ func (u *unwinder) next() {
 	}
 
 	injectedCall := f.funcID == funcID_sigpanic || f.funcID == funcID_asyncPreempt || f.funcID == funcID_debugCallV2
+	if injectedCall {
+		u.flags |= unwindTrap
+	} else {
+		u.flags &^= unwindTrap
+	}
 
 	// Unwind to next frame.
 	u.calleeFuncID = f.funcID
@@ -541,6 +550,25 @@ func (u *unwinder) finishInternal() {
 	}
 }
 
+// symPC returns the PC that should be used for symbolizing the current frame.
+// Specifically, this is the PC of the last instruction executed in this frame.
+//
+// If this frame did a normal call, then frame.pc is a return PC, so this will
+// return frame.pc-1, which points into the CALL instruction. If the frame was
+// interrupted by a signal (e.g., profiler, segv, etc) then frame.pc is for the
+// trapped instruction, so this returns frame.pc. See issue #34123. Finally,
+// frame.pc can be at function entry when the frame is initialized without
+// actually running code, like in runtime.mstart, in which case this returns
+// frame.pc because that's the best we can do.
+func (u *unwinder) symPC() uintptr {
+	if u.flags&unwindTrap == 0 && u.frame.pc > u.frame.fn.entry() {
+		// Regular call.
+		return u.frame.pc - 1
+	}
+	// Trapping instruction or we're at the function entry point.
+	return u.frame.pc
+}
+
 // Generic traceback. Handles runtime stack prints (pcbuf == nil),
 // and the runtime.Callers function (pcbuf != nil).
 // A little clunky to merge these, but avoids
@@ -581,24 +609,9 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 		frame := &u.frame
 		f := frame.fn
 
-		// Backup to the CALL instruction to read inlining info
-		//
-		// Normally, pc is a return address. In that case, we want to look up
-		// file/line information using pc-1, because that is the pc of the
-		// call instruction (more precisely, the last byte of the call instruction).
-		// When the pc is from a signal (e.g. profiler or segv) then pc is for
-		// the trapped instruction, not a return address, so we use pc unchanged.
-		// See issue 34123.
-		// The pc can be at function entry when the frame is initialized without
-		// actually running code, like runtime.mstart.
-		callPC := frame.pc
-		if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry() && u.calleeFuncID != funcID_sigpanic {
-			callPC--
-		}
-
 		if pcbuf != nil {
 			// TODO: Why does cache escape? (Same below)
-			for iu, uf := newInlineUnwinder(f, callPC, noEscapePtr(&u.cache)); uf.valid(); uf = iu.next(uf) {
+			for iu, uf := newInlineUnwinder(f, u.symPC(), noEscapePtr(&u.cache)); uf.valid(); uf = iu.next(uf) {
 				sf := iu.srcFunc(uf)
 				if sf.funcID == funcID_wrapper && elideWrapperCalling(u.calleeFuncID) {
 					// ignore wrappers
@@ -623,7 +636,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 			// any frames. And don't elide wrappers that
 			// called panic rather than the wrapped
 			// function. Otherwise, leave them out.
-			for iu, uf := newInlineUnwinder(f, callPC, noEscapePtr(&u.cache)); uf.valid(); uf = iu.next(uf) {
+			for iu, uf := newInlineUnwinder(f, u.symPC(), noEscapePtr(&u.cache)); uf.valid(); uf = iu.next(uf) {
 				sf := iu.srcFunc(uf)
 				if (flags&_TraceRuntimeFrames) != 0 || showframe(sf, gp, nprint == 0, u.calleeFuncID) {
 					name := sf.name()
@@ -640,7 +653,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 						print("...")
 					} else {
 						argp := unsafe.Pointer(frame.argp)
-						printArgs(f, argp, callPC)
+						printArgs(f, argp, u.symPC())
 					}
 					print(")\n")
 					print("\t", file, ":", line)