experience report: runtime: misunderstood Frames.Next documentation

report.md

I'm adding a feature to the Go compiler to make debugging it easier. That is, debugging of the Go compiler itself, not of end-user code. (I believe further details are irrelevant, but for concreteness this is CL 328909.)

This feature captures a lot of stack traces, but the expected number of unique frames across all traces is modest. For performance, I expected I would want to use runtime.Callers, and then manually process and memoize the uintptr values. I haven't had a need for runtime.Callers since the introduction of runtime.CallersFrames so I read the docs to catch up.

Issue 1: Iterating over the runtime.Callers results

I started by looking at the runtime.Callers docs, which state:

To translate these PCs into symbolic information such as function names and line numbers, use CallersFrames. CallersFrames accounts for inlined functions and adjusts the return program counters into call program counters. Iterating over the returned slice of PCs directly is discouraged, as is using FuncForPC on any of the returned PCs, since these cannot account for inlining or return program counter adjustment.

I was incidentally aware that we used to store skipPleaseUseCallersFrames pseudo-frames in the results from runtime.Callers. However, it appears that was removed in CL 152537.

As far as I can tell, the warning that "iterating over the returned slice of PCs [...] cannot account for inlining or return program counter adjustment" is no longer true. As long as runtime.CallersFrames is used, it appears safe to iterate over the PCs slice directly.

This has performance implicitations for my use case, because it affects whether it's safe to memoize the uintptr->[]runtime.Frame expansion.

Issue 2: I misunderstood how to use Frames.Next

Continuing from the docs above, I looked at runtime.CallersFrames:

func CallersFrames(callers []uintptr) *Frames

CallersFrames takes a slice of PC values returned by Callers and prepares to return function/file/line information. Do not change the slice until you are done with the Frames.

and then runtime.(*Frames).Next immediately below it:

func (ci *Frames) Next() (frame Frame, more bool)

Next returns frame information for the next caller. If more is false, there are no more callers (the Frame value is valid).

The API seemed a bit clumsy to use, because I couldn't just write something like for frames.Next() { ... }, like I can with bufio.(*Scanner).Scan. But still seemed straight forward.

I wrote roughly the following code:

frames := runtime.CallersFrames(callers)
for {
  frame, ok := frames.Next()
  if !ok {
    break
  }
  use(frame)
}

However, this code is wrong: it stops one frame early. I should have written something like:

if len(callers) == 0 {
  return
}
frames := runtime.CallersFrames(callers)
for {
  frame, more := frames.Next()
  use(frame)
  if !more {
    break
  }
}

Re-reading the runtime.(*Frames).Next documentation now, I think I see how it explains the actual semantics. However, upon my initial reading, that's not how I understood it.

I think the documentation could be revised and the subtleties called out explicitly.

Additionally, I think a higher-level API that's more intuitive and harder to misuse could be nice. For example, something like:

// WalkFrames calls visit(frame) for each frame represented by callers.
// If any call returns a non-nil error, WalkFrames stops and returns that error.
// Otherwise, WalkFrames returns nil.
func WalkFrames(callers []uintptr, visit func(Frame) error) error {
  if len(callers) == 0 {
    return nil
  }
  frames := CallersFrames(callers)
  for {
    frame, more := frames.Next()
    if err := visit(frame); err != nil {
      return err
    }
    if !more {
      return nil
    }
  }
}