Repository navigation
Add GetCaller for use in error helpers - #108
Merged
Merged
Conversation
Contributor
Author
|
This talk on mid-stack inliningwas helpful in understanding how Go tracks inlined PCs:
(and for reference, how the trace unwinder unwinds) |
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
prashantv
force-pushed
the
prashant/pc-caller
branch
from
November 10, 2024 04:12
bba5789 to
fd32937
Compare
abhinav
reviewed
Nov 10, 2024
prashantv
force-pushed
the
prashant/version
branch
from
November 10, 2024 05:08
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.
prashantv
force-pushed
the
prashant/pc-caller
branch
from
November 10, 2024 05:49
fd32937 to
b1783c8
Compare
prashantv
added a commit
that referenced
this pull request
Nov 10, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #70
errtrace.Wrapcaptures 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
skipcallers 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
errtraceis 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.GetCallerfollowed byWrap, but callersshould use
go:noinlinefor correct stack frames.