Partially fix set_trace_func line numbers#4052
Merged
enebo merged 2 commits intojruby:masterfrom Aug 8, 2016
ivoanjo:partially-fix-set-trace-func-line-numbers
Merged
Partially fix set_trace_func line numbers#4052enebo merged 2 commits intojruby:masterfrom ivoanjo:partially-fix-set-trace-func-line-numbers
enebo merged 2 commits intojruby:masterfrom
ivoanjo:partially-fix-set-trace-func-line-numbers
Conversation
This makes it so that stack traces gathered inside the trace instruction have accurate line numbers (and is especially important because some debuggers use line numbers from the stack trace, rather than the ones received by set_trace_func). Issue #4051
Unlike the LINE and CALL trace instructions, currently the parser does not supply line numbers for RETURN and END trace instructions. This is worked around by by the TraceInstr by computing line numbers dynamically, rather than statically, but the same approach cannot be applied to line number instructions, which means that line numbers in stack traces generated inside the trace function will not be correct. As the fix seems to be quite hard, I think it's important to document this bug. Issue #4051
Contributor
Author
|
Update: Along with Woohoo, finally breakpoints and flow control inside pry! 🎉 |
Member
|
@ivoanjo both of those changes make total sense to me so that is good :) I am happy to hear some libraries are working. I will try and better understand the end/return line number problems to see if I can totally put this to bed. |
Contributor
Author
|
Awesome, thanks for the quick review! |
nomadium
added a commit
to nomadium/jruby
that referenced
this pull request
Jan 12, 2018
For more information, please see feature jruby#4052.
Merged
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
2 participants
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.
After reporting issue #4051 I decided to look at the generated IR and noticed that trace instructions were being added without line numbers. This led to the observed issue where stack traces generated inside
set_trace_funcdid not have the correct line numbers.For the
LINEandCALLevents the fix is straightforward: just add a line number instruction before the trace instruction, as the line number was already known statically by theIRBuilder.Unfortunately, for the
RETURNandENDevents the parser does not include this information and thus there's no simple way of adding line number instructions before those. I think that particular fix is a bit beyond my capabilities so I added someFIXMEs so at least this doesn't get forgotten.Nevertheless, and happily, the partial fix seems to be enough to get
pry-navto work!On a side note, I must admit that
but nevertheless for a first time diving inside JRuby's codebase, this went better than expected 😄
Please let me know if there's some way I can help fix this further!
Issue #4051