Skip to content
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

Core method call missing in exception backtrace on JRuby 9.2.0.0 #5200

Closed
nilsding opened this issue May 28, 2018 · 4 comments · Fixed by #6653
Closed

Core method call missing in exception backtrace on JRuby 9.2.0.0 #5200

nilsding opened this issue May 28, 2018 · 4 comments · Fixed by #6653
Milestone

Comments

@nilsding
Copy link

I noticed some very minor issue with the exception backtraces in JRuby 9.2.0.0: there is a core method call missing in the output of this script:

puts RUBY_DESCRIPTION

def make_error
  1 / 0
end

begin
  make_error
rescue => e
  puts e
  puts e.backtrace
end

See Actual Behavior below for the program output when running with JRuby 9.2.0.0.

Normal Ruby exceptions (via raise) show up correctly, and so do pure Java exceptions.

Environment

jruby -v:

jruby 9.2.0.0 (2.5.0) 2018-05-24 81156a8 Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [darwin-x86_64]

No JRUBY_OPTS set

OS:

Darwin MS-DOS6.local 16.7.0 Darwin Kernel Version 16.7.0: Thu Jan 11 22:59:40 PST 2018; root:xnu-3789.73.8~1/RELEASE_X86_64 x86_64

Expected Behavior

Same as in JRuby 9.1.17.0 -- there should be a call to the / method somewhere in the backtrace:

jruby 9.1.17.0 (2.3.3) 2018-04-20 d8b1ff9 Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [darwin-x86_64]
divided by 0
org/jruby/RubyFixnum.java:564:in `/'
./thing.rb:4:in `make_error'
./thing.rb:8:in `<main>'

For comparison, here's the same script run with MRI 2.5.0:

ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin16]
divided by 0
./thing.rb:4:in `/'
./thing.rb:4:in `make_error'
./thing.rb:8:in `<main>'

Actual Behavior

jruby 9.2.0.0 (2.5.0) 2018-05-24 81156a8 Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [darwin-x86_64]
divided by 0
./thing.rb:4:in `make_error'
./thing.rb:8:in `<main>'
@headius
Copy link
Member

headius commented May 28, 2018

I suspect this is due to a numeric optimization that bypasses the known Ruby core method and goes straight to other logic. We would just want to register those other method names so that they are transformed into a core method backtrace line.

If you specify -Xbacktrace.style=full, you'll see all the intermediate and Ruby lines together in a single trace.

@headius
Copy link
Member

headius commented Apr 12, 2019

This may originally have been because we had some special-case methods for faster math operations. Now the root cause is that all these methods are defined on RubyInteger, which is how they're entered into the backtrace scrubber. But when dispatched against RubyFixnum or RubyBignum, they don't ever pass through RubyInteger, so that trace line is lost.

Brainstorming a fix...

We could make the Ruby-bound methods redispatch to a different name, so they would not be overridable (and could be marked as final to ensure they don't get overridden. However all dispatches from within RubyInteger would be bimorphic against RubyFixnum and RubyBignum, rather than the original call site seeing only one or the other.

We could modify the JRubyMethod annotation, or the processing of it, to list or find subclass overrides, and also add those classes to the mapping table for method names. If this were done manually, it would be error-prone, and done automatically would require digging up all child classes during populator generation. It's doable though.

We could introduce another annotation used to generate a populator for the name mapping, It would be another build step and another generated piece of code to run on boot, but it could accurately reflect all class/method combinations we want to map to a different Ruby name, including specialized locations such as our "helpers" classes or utility methods in the interpreter. This seems like the best idea, but has the most work involved.

@enebo Gimme your $0.02!

@headius
Copy link
Member

headius commented Apr 7, 2021

There's a bit more complication here with the Integer integration.

Our backtraces are based on matching an exact class plus method name in the Java trace, which works fine for classes that implement their own Ruby-facing methods. RubyInteger, however, has mostly abstract implementations of these methods, implemented in the two subclasses RubyBignum and RubyFixnum. That prevents the stack trace from reflecting the RubyInteger class and as a result none of these methods will show up in a Ruby trace.

This is a specific example of the general problem that methods not directly bound via @JRubyMethod will not show up in Ruby traces.

headius added a commit to headius/jruby that referenced this issue Apr 7, 2021
Our Ruby backtrace-generation logic depends on a mapping of class
and method names to their equivalent Ruby name, but this fails
when the JRubyMethod annotation is on an abstract method
implemented by one or more subclasses. The subclasses are not
registered in the mapping and when their overridden methods appear
in the Java stack trace they will not be replaced with their Ruby
equivalent.

This patch adds support for JRubyMethod.implementers, a list of
additional classes to include in the mapping table. This is a
naive change currently, in that it assumes all methods on class A
should also be mapped for class B < A regardless of whether they
are actually overridden or not.

This is a first pass at fixing the missing trace lines reported in
issue jruby#5200.
@headius
Copy link
Member

headius commented Apr 9, 2021

I have pushed a PR that should fix this, at least for Integer, Method/UnboundMethod, and the specialized Array subtypes for one and two-element lists. We will land that for 9.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants