From 6f22d42c741c88c08b5df1a77831b6646e368fd1 Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Sun, 17 Jul 2022 15:57:09 -0400
Subject: [PATCH] runtime: resolve caller funcInfo after processing current
 frame
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently, gentraceback resolves the funcInfo of the caller prior to
processing the current frame (calling the callback, printing it, etc).
As a result, if this lookup fails in a verbose context, it will print
the failure before printing the frame that it's already resolved.

To fix this, move the resolution of LR to a funcInfo to after current
frame processing.

This also has the advantage that we can reduce the scope of "flr" (the
caller's funcInfo) to only the post-frame part of the loop, which will
make it easier to stack-rip gentraceback into an iterator.

For #54466.

Change-Id: I8be44d4eac598a686c32936ab37018b8aa97c00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/458217
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
---
 src/runtime/traceback.go | 50 +++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index b3c1404b8f..d04bbf2d57 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -202,11 +202,10 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 				frame.fp += goarch.PtrSize
 			}
 		}
-		var flr funcInfo
+		var lrPtr uintptr
 		if flag&funcFlag_TOPFRAME != 0 {
 			// This function marks the top of the stack. Stop the traceback.
 			frame.lr = 0
-			flr = funcInfo{}
 		} else if flag&funcFlag_SPWRITE != 0 && (callback == nil || n > 0) {
 			// The function we are in does a write to SP that we don't know
 			// how to encode in the spdelta table. Examples include context
@@ -230,9 +229,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 				throw("traceback")
 			}
 			frame.lr = 0
-			flr = funcInfo{}
 		} else {
-			var lrPtr uintptr
 			if usesLR {
 				if n == 0 && frame.sp < frame.fp || frame.lr == 0 {
 					lrPtr = frame.sp
@@ -244,28 +241,6 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 					frame.lr = uintptr(*(*uintptr)(unsafe.Pointer(lrPtr)))
 				}
 			}
-			flr = findfunc(frame.lr)
-			if !flr.valid() {
-				// This happens if you get a profiling interrupt at just the wrong time.
-				// In that context it is okay to stop early.
-				// But if callback is set, we're doing a garbage collection and must
-				// get everything, so crash loudly.
-				doPrint := printing
-				if doPrint && gp.m.incgo && f.funcID == funcID_sigpanic {
-					// We can inject sigpanic
-					// calls directly into C code,
-					// in which case we'll see a C
-					// return PC. Don't complain.
-					doPrint = false
-				}
-				if callback != nil || doPrint {
-					print("runtime: g ", gp.goid, ": unexpected return pc for ", funcname(f), " called from ", hex(frame.lr), "\n")
-					tracebackHexdump(gp.stack, &frame, lrPtr)
-				}
-				if callback != nil {
-					throw("unknown caller pc")
-				}
-			}
 		}
 
 		frame.varp = frame.fp
@@ -469,7 +444,30 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 		injectedCall := f.funcID == funcID_sigpanic || f.funcID == funcID_asyncPreempt || f.funcID == funcID_debugCallV2
 
 		// Do not unwind past the bottom of the stack.
+		if frame.lr == 0 {
+			break
+		}
+		flr := findfunc(frame.lr)
 		if !flr.valid() {
+			// This happens if you get a profiling interrupt at just the wrong time.
+			// In that context it is okay to stop early.
+			// But if callback is set, we're doing a garbage collection and must
+			// get everything, so crash loudly.
+			doPrint := printing
+			if doPrint && gp.m.incgo && f.funcID == funcID_sigpanic {
+				// We can inject sigpanic
+				// calls directly into C code,
+				// in which case we'll see a C
+				// return PC. Don't complain.
+				doPrint = false
+			}
+			if callback != nil || doPrint {
+				print("runtime: g ", gp.goid, ": unexpected return pc for ", funcname(f), " called from ", hex(frame.lr), "\n")
+				tracebackHexdump(gp.stack, &frame, lrPtr)
+			}
+			if callback != nil {
+				throw("unknown caller pc")
+			}
 			break
 		}