proposal: add interp.FilterStack() method to get interpreter stacktrace #1348
Conversation
This file contains 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
|
|
By placing a handle value on several strategic calls in the runtime we can then later parse a runtime stacktrace, look for the magic values in function parameters, and reconstruct the calling nodes. We can use that to filter out all the yaegi runtime calls and present the user with a stacktrace that includes only interpreted frames.
bailsman
force-pushed
the
FilterStack
branch
from
7648ac0
to
0ff627e
Compare
this makes the filtered stack look identical to the regular go stacktrace
this allows calling code to interchange both types
interpreter may be entered (for example) from genFunctionWrapper
bailsman
force-pushed
the
FilterStack
branch
2 times, most recently
from
eb2cc02
to
fd96a6c
Compare
When a panic happens, we want to get the stack trace from the oldest panic, before runCfg unwinds everything. However, at that point we don't know yet whether we will be recovered. As a silly kludge, currently storing the oldest panic in a list on the Interpreter struct which can then be queried once we're ready. The approach taken is not strictly correct: if a panic is recovered, and never queried, and later the same error occurs again and then is not recovered, the wrong call stack will be returned.
|
@mvertes Excuse me, when will this feature be officially released? |
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.
Related to the following tickets:
detailed call information on panic #531
Capture call stack #1040
Interpreter-level stack traces are now available using the new debug interface #1188 #1225
(using, I think, a much saner approach than I am about to propose)
However, it could still be useful to instead convert a runtime stack trace showing the full code path through both interpreted and binary code, without using the debug interface. For example:
Run with
go run sample.goThe normal runtime interpreter stacktrace is not ideal:
With the proposed FilterStack method, you can get a stacktrace that looks like this instead:
You can follow the call path through both binary frames and interpreted frames. Yaegi runtime frames are filtered out and replaced by the interpreted code, making it easier to see what happened. This example now shows calling a debug function deliberately, but you can capture a panic in the same way.
How it works: by placing a handle value in the parameters to several strategic calls in the runtime we can then later parse a runtime stacktrace, look for the magic values in function parameters, and reconstruct the calling nodes.
Aside from being totally insane, this approach has several other disadvantages:
(1) I'm not sure if we can be confident that the runtime stack trace can always be parsed in this way (it might change, and might vary by platform and runtime version)
(2) Interpreter behavior and stack trace parsing code are now tightly coupled, perhaps forcing you go to back and fix the stack trace parsing with every little change to the interpreter.
(3) I don't know what effects this has on performance.
Also, FuncName() turned out to be much more complicated than I expected. I'm convinced there must be a simpler way to do this - perhaps with some help at parse time. I haven't extensively tested correctness either.
I've also modified runCfg so that it captures a filtered stack trace on every panic, which can then be retrieved by calling i.GetOldestPanicForErr(err), which returns an interp.Panic object. In this branch, yaegi now prints the filtered stacktrace rather than the normal runtime stacktrace by default, which you can try out by running it on anything that panics. The runtime stacktrace is still available programmatically as i.GetOldestPanicForErr(err).Stack.
We want to capture the oldest panic, before runCfg unwinds everything and we lose important context. However, at that point, we don't yet know whether it will be recovered or not. We store the panic in the Interpreter struct. This will cause incorrect behavior if the panic is recovered and GetOldestPanicForErr is never called, and then later the exact same error happens again but is not recovered. In that case we will return the wrong (old) stacktrace.
All in all, using the new debug functionality to achieve something like this would probably work out better in the long run, but because this might be useful to someone I figured I'd put it up for discussion.