-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Wrong line numbers in Kernel#caller inside set_trace_func (with code to reproduce) #4051
Comments
Commit 44a2aa6 fixes us reporting top vs main. Throwing it on this issue versus making a new one. |
@ivoanjo heya. I think I fixed the rest of this. I can see two potential failure paths though:
@ivoanjo If you can kick the tires it might be the best we can do for verifying this for the next JRuby release. |
Wow, nice to see you picked up the hard part after I did the easy one ;) I'll try |
Ok so I did a few tests and it seems like the lines are correct between the backtrace and what The exception I saw was |
@ivoanjo If you can distill some snippets then I will try and knock these down. I did run into something where an extra keyword in a line can cause it to bump forward by one (which is super confusing output from the parser). Open them as new issues. |
Fixes #4051. Wrong line numbers in Kernel#caller inside set_trace_func (with code to reproduce) #4051 was largely already fixed but the backtrace elements themselves had the wrong line (which was why #4060 was opened). The problem was mri changes their position of class/module/sclass from start line to end line. We just store start line. They will also look at first instr/node within the class/module for start line. That solution is memory efficient but then makes those nodes report wrong lines with -S ast. Our solution was to just add an endLine field to those nodes. The memory increase from that is very small so I think it is ok. The second part of this was that we build backtrace from setting line number via a line number instr. We had no end line so we couldn't emit one. Now that we have one we do....but only if --debug is supplied since the only instr after this point is return and that cannot raise (so we cannot generate a backtrace).
Fixes #4051. Wrong line numbers in Kernel#caller inside set_trace_func (with code to reproduce) #4051 was largely already fixed but the backtrace elements themselves had the wrong line (which was why #4060 was opened). The problem was mri changes their position of class/module/sclass from start line to end line. We just store start line. They will also look at first instr/node within the class/module for start line. That solution is memory efficient but then makes those nodes report wrong lines with -S ast. Our solution was to just add an endLine field to those nodes. The memory increase from that is very small so I think it is ok. The second part of this was that we build backtrace from setting line number via a line number instr. We had no end line so we couldn't emit one. Now that we have one we do....but only if --debug is supplied since the only instr after this point is return and that cannot raise (so we cannot generate a backtrace).
I was trying to see if I could get
pry-nav
to work with JRuby. It doesn't have a lot of features, but it's a straightforward use ofset_trace_func
and I wanted to experiment with it a bit.I hit some issues, and investigating them led me to discover that when running code inside
set_trace_func
and then callingKernel#caller
(such as when stepping) leads to line numbers being wrong in the stack trace, which confusespry
.Environment
Running
jruby 9.1.2.0 (2.3.0) 2016-05-26 7357c8f OpenJDK 64-Bit Server VM 25.91-b14 on 1.8.0_91-8u91-b14-3ubuntu1~15.10.1-b14 [linux-x86_64]
onUbuntu 15.10
.Expected Behavior
Example code:
Output in MRI 2.3.1:
With MRI,
caller.first
matches the line we get inset_trace_func
.Actual Behavior
Output in JRuby:
With JRuby, the line numbers don't match the line we get in
set_trace_func
.There is also a minor difference where MRI calls the top-level
<main>
while JRuby uses<top>
. This seems minor, but I can report it as a separate issue if needed.The text was updated successfully, but these errors were encountered: