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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partially fix set_trace_func line numbers #4052

Merged
merged 2 commits into from Aug 8, 2016

Conversation

Projects
None yet
2 participants
@ivoanjo
Contributor

ivoanjo commented Aug 6, 2016

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_func did not have the correct line numbers.

For the LINE and CALL events the fix is straightforward: just add a line number instruction before the trace instruction, as the line number was already known statically by the IRBuilder.

Unfortunately, for the RETURN and END events 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 some FIXMEs so at least this doesn't get forgotten.

Nevertheless, and happily, the partial fix seems to be enough to get pry-nav to 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

ivoanjo added some commits Aug 6, 2016

Add line numbers before adding LINE and CALL trace instructions
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
Add FIXME and link to issue 4051 to RETURN and END trace instructions
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
@ivoanjo

This comment has been minimized.

Show comment
Hide comment
@ivoanjo

ivoanjo Aug 7, 2016

Contributor

Update: Along with pry-nav, I have now gotten pry-debugger to work too.

Woohoo, finally breakpoints and flow control inside pry! 馃帀

Contributor

ivoanjo commented Aug 7, 2016

Update: Along with pry-nav, I have now gotten pry-debugger to work too.

Woohoo, finally breakpoints and flow control inside pry! 馃帀

@enebo enebo added this to the JRuby 9.1.3.0 milestone Aug 8, 2016

@enebo enebo added the ir label Aug 8, 2016

@enebo enebo merged commit fe34564 into jruby:master Aug 8, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Aug 8, 2016

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.

Member

enebo commented Aug 8, 2016

@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.

@ivoanjo

This comment has been minimized.

Show comment
Hide comment
@ivoanjo

ivoanjo Aug 9, 2016

Contributor

Awesome, thanks for the quick review!

Contributor

ivoanjo commented Aug 9, 2016

Awesome, thanks for the quick review!

nomadium added a commit to nomadium/jruby that referenced this pull request Jan 12, 2018

Implement File.lutime
For more information, please see feature #4052.

@nomadium nomadium referenced this pull request Jan 12, 2018

Merged

Add File.lutime #4969

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment