-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add GetCaller for use in error helpers #108
Conversation
This talk on mid-stack inliningwas helpful in understanding how Go tracks inlined PCs:
(and for reference, how the trace unwinder unwinds) |
This change is part of the following stack: Change managed by git-spice. |
bba5789
to
fd32937
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm besides comments
cd2c04b
to
b87e8e9
Compare
`errtrace.Wrap` captures the immediate caller, which doesn't compose with existing error-wrapping helpers. To support this use-case, add `errtrace.GetCaller()` that helpers can use to capture their caller and add that to the error trace. This approach was chosen over the typical `skip` callers as: * Skip requires a more complex assembly implementation to skip an arbitrary number of frames, which is more error-prone, e.g., handling the skip argument being too large. * Faster as each call only skips a fixed (and predictable) number of frames. This is especially important since it's expected that `errtrace` is used to annotate all frames in the return path. * Avoids skip calculations, and generally more flexible to pass around compared to skipping frames, which is tied to the stack. Regardless of which approach is chosen, there is a limitation: incompatibility with inlined callers. Go tracks inlined code and PCs of inlined code is correctly mapped back, but the stack does not contain inlined frames, so the skip calculation done cannot correctly skip inlined frames. This approach only requires the error helper to have a stack frame, which is typically the case when calling `errtrace.GetCaller` followed by `Wrap`, but callers should use `go:noinline` for correct stack frames.
fd32937
to
b1783c8
Compare
Fixes #70
errtrace.Wrap
captures the immediate caller, which doesn't composewith existing error-wrapping helpers. To support this use-case, add
errtrace.GetCaller()
that helpers can use to capture their callerand add that to the error trace.
This approach was chosen over the typical
skip
callers as:Skip requires a more complex assembly implementation to skip an
arbitrary number of frames, which is more error-prone, e.g.,
handling the skip argument being too large.
Faster as each call only skips a fixed (and predictable) number of
frames. This is especially important since it's expected that
errtrace
is used to annotate all frames in the return path.Avoids skip calculations, and generally more flexible to pass around
compared to skipping frames, which is tied to the stack.
Regardless of which approach is chosen, there is a limitation:
incompatibility with inlined callers. Go tracks inlined code
and PCs of inlined code is correctly mapped back, but the stack
does not contain inlined frames, so the skip calculation done
cannot correctly skip inlined frames. This approach only requires
the error helper to have a stack frame, which is typically the case
when calling
errtrace.GetCaller
followed byWrap
, but callersshould use
go:noinline
for correct stack frames.