From 312f7c1bd300d374f7078c4449c5ad142e0c3a5e Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Fri, 6 Dec 2024 15:37:34 -0500 Subject: [PATCH] runtime: add note that Callers never returns an entry PC The presence of a pc > entry check in CallersFrame implies we might actually see pc == entry, when in reality Callers will never return such a PC. This check is actually just a safety check for avoid reporting completely nonsensical from bad input. all.bash reports two violations to this invariant: TestCallersFromWrapper, which explicitly constructs a CallersFrame input with an entry PC. runtime/pprof.printStackRecord, which passes pprof stacks to CallersFrame (technically not a valid use of CallersFrames!). runtime/pprof.(*Profile).Add can add the entry PC of runtime/pprof.lostProfileEvent to samples. (CPU profiles do lostProfileEvent + 1. I will send a second CL to fix Add.) Change-Id: Iac2a2f0c15117d4a383bd84cddf0413b2d7dd3ef Reviewed-on: https://go-review.googlesource.com/c/go/+/634315 Auto-Submit: Michael Pratt LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Knyszek --- src/runtime/symtab.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index ea048832c7..c78b044264 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -118,11 +118,16 @@ func (ci *Frames) Next() (frame Frame, more bool) { } f := funcInfo._Func() entry := f.Entry() + // We store the pc of the start of the instruction following + // the instruction in question (the call or the inline mark). + // This is done for historical reasons, and to make FuncForPC + // work correctly for entries in the result of runtime.Callers. + // Decrement to get back to the instruction we care about. + // + // It is not possible to get pc == entry from runtime.Callers, + // but if the caller does provide one, provide best-effort + // results by avoiding backing out of the function entirely. if pc > entry { - // We store the pc of the start of the instruction following - // the instruction in question (the call or the inline mark). - // This is done for historical reasons, and to make FuncForPC - // work correctly for entries in the result of runtime.Callers. pc-- } // It's important that interpret pc non-strictly as cgoTraceback may -- 2.48.1