define_method with empty body throws RuntimeError in interpreter #3483

annawinkler · 2015-11-20T23:15:26Z

If you do the following in jruby 9.0.4.0:

> define_method(:foo) { |*| }
> foo("hi")

You get this RuntimeError:

RuntimeError: BUG: interpreter fell through to end unexpectedly
    from (irb):2:in `foo'
    from (irb):2:in `<eval>'
    from org/jruby/RubyKernel.java:978:in `eval'
    from org/jruby/RubyKernel.java:1291:in `loop'
    from org/jruby/RubyKernel.java:1098:in `catch'
    from org/jruby/RubyKernel.java:1098:in `catch'
    from /home/vagrant/.rbenv/versions/jruby-9.0.4.0/bin/irb:13:in `<top>'

I tried the same in jruby-9.0.1.0, and it works as follows:

irb(main):005:0> define_method(:foo) { |*| }
=> :foo
irb(main):006:0> foo("hi")
=> nil

The text was updated successfully, but these errors were encountered:

headius · 2015-11-20T23:30:51Z

Ah-ha! This makes sense. In 9030 we made a change that converts define_method methods into regular methods. It seems something about this conversion is causing the code to fall off the end of the method.

This is definitely enough for us to reproduce and find the problem.

You can work around the issue by adding an explicit return to the method, so it seems like this is related to how blocks return. For example, this works:

define_method(:foo) {|*| return}
foo("hi")

annawinkler · 2015-11-20T23:33:30Z

oh i'll try that - thanks!

annawinkler · 2015-11-20T23:34:32Z

And for the record, this is why we're doing what we're doing for testing:

class NullLogger
  [ :add, :debug, :info, :warn, :error, :fatal, :unknown, :write ].each do |level|
    define_method(level) { |*| }
  end
end

So... one could argue that that's terrible :)

annawinkler · 2015-11-20T23:35:00Z

The addition of the return statement fixes the problem! Thanks very much!

headius · 2015-11-20T23:35:28Z

@annafw I knew it would be something like that...basically stubbing out methods you don't want to make noise. Glad to hear the workaround works. We'll get this fixed up for 9.0.5.0. Thanks for the report!

headius · 2015-11-23T21:06:55Z

Looking into this now.

headius · 2015-11-23T21:12:59Z

This only appears to affect running in the StartupInterpreterEngine:

[] ~/projects/jruby $ jruby -X-C -Xjit.threshold=-1 -e "define_method(:foo) { }; foo"

[] ~/projects/jruby $ jruby -X-C -Xjit.threshold=0 -e "define_method(:foo) { }; foo"

[] ~/projects/jruby $ jruby -Xjit.threshold=-1 -e "define_method(:foo) { }; foo"
RuntimeError: BUG: interpreter fell through to end unexpectedly
    foo at -e:1
  <top> at -e:1

Here's the full backtrace:

RuntimeError: BUG: interpreter fell through to end unexpectedly
        getStackTrace at java/lang/Thread.java:1552
     getBacktraceData at org/jruby/runtime/backtrace/TraceType.java:183
         getBacktrace at org/jruby/runtime/backtrace/TraceType.java:47
     prepareBacktrace at org/jruby/RubyException.java:223
             preRaise at org/jruby/exceptions/RaiseException.java:229
             preRaise at org/jruby/exceptions/RaiseException.java:196
               <init> at org/jruby/exceptions/RaiseException.java:111
    newRaiseException at org/jruby/Ruby.java:4058
      newRuntimeError at org/jruby/Ruby.java:3535
            interpret at org/jruby/ir/interpreter/StartupInterpreterEngine.java:142
            interpret at org/jruby/ir/interpreter/InterpreterEngine.java:77
     INTERPRET_METHOD at org/jruby/internal/runtime/methods/MixedModeIRMethod.java:162
                  foo at -e:1
                 call at org/jruby/internal/runtime/methods/MixedModeIRMethod.java:148
                 call at org/jruby/internal/runtime/methods/DefineMethodMethod.java:32
                 call at org/jruby/internal/runtime/methods/DynamicMethod.java:189
         cacheAndCall at org/jruby/runtime/callsite/CachingCallSite.java:293
                 call at org/jruby/runtime/callsite/CachingCallSite.java:131
                <top> at -e:1
  invokeWithArguments at java/lang/invoke/MethodHandle.java:627
                 load at org/jruby/ir/Compiler.java:111
            runScript at org/jruby/Ruby.java:821
            runScript at org/jruby/Ruby.java:813
          runNormally at org/jruby/Ruby.java:751
          runFromMain at org/jruby/Ruby.java:573
        doRunFromMain at org/jruby/Main.java:409
          internalRun at org/jruby/Main.java:304
                  run at org/jruby/Main.java:233
                 main at org/jruby/Main.java:200

headius · 2015-11-23T21:17:26Z

Ah-ha...I think I see the problem. In the startup interpreter, blocks are allowed to fall off the end without an explicit return instructions. Here's a screencap from my debug session:

So there's a few ways to fix this, and I'm not sure how to proceed.

Modify the startup interpreter/compiler to add this missing return instruction, so we don't fall off the end.
Force the full interpreter's passes to run before turning the define_method block into a regular method.
Fix up the define_method block when converting it to a method.

What do you think, @enebo and @subbuss?

headius · 2015-11-23T21:23:00Z

Ok, my naive forcing of a full build did not work.

Patch:

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
index d487251..e06610b 100644
--- a/core/src/main/java/org/jruby/RubyModule.java
+++ b/core/src/main/java/org/jruby/RubyModule.java
@@ -1800,6 +1800,9 @@ public class RubyModule extends RubyObject {
             // Ask closure to give us a method equivalent.
             IRMethod method = closure.convertToMethod(name);
             if (method != null) {
+                // Run passes against the method to fix #3483
+                method.prepareFullBuild();
+                
                 newMethod = new DefineMethodMethod(method, visibility, this, context.getFrameBlock());
                 Helpers.addInstanceMethod(this, name, newMethod, visibility, context, runtime);
                 return nameSym;

NPE:

Unhandled Java exception: java.lang.NullPointerException
java.lang.NullPointerException: null
             cloneInstrs at org/jruby/ir/IRScope.java:522
  prepareFullBuildCommon at org/jruby/ir/IRScope.java:539
        prepareFullBuild at org/jruby/ir/IRScope.java:558
           define_method at org/jruby/RubyModule.java:1804

headius · 2015-11-23T21:31:39Z

Ok, something is culling out the return before we even get to StartupInterpreter.

Here's the full instructions immediately after build:

I'll keep tracing through to figure out where this is happening.

headius · 2015-11-23T21:54:44Z

Bleh, ok. I'm stumped for now.

enebo · 2015-11-23T21:55:21Z

This should be fully converting to a method where we make a new IRMethod and args and body nodes are being treated as if they were method args and body node. I am guessing some presence of some node in AST is setting a flag in a bad way. We should not need to force full build prep since first access of this new "method" should lazilyAcquire the startup interp instrs and it should also run basic passes on it. If a return is getting stripped it is because of the presence of something normally a method would not have. At least that is my current hunch.

enebo · 2015-11-23T22:51:43Z

The weird thing about this bug involved println and displaying the AST. The optimization would compile the argsNode and the bodyNode as if the iternode was a def{n,s}node but def nodes will add an implicitnilnode whereas iter nodes would not if the body is empty. For some reason the ast command would not display nilimplicitnode in the output so it was not possible to see the difference. So adding an implicitnilnode for iternodes empty bodies fixes thing.

* master: (29 commits) [travis-ci] revert jdk 9 testing from 2fa04aa Eliminate Block.Type arg in BlockBody yield/call signatures Pass Block instead of Binding in BlockBody.yield/call Minor: fix typo in name of runtime helper instr method name Update to jnr-unixsocket 0.10. Improve CamelCase package splitting. Fixes #3493. Update to jnr-unixsocket 0.10-SNAPSHOT for forFD. [Truffle] j+tr: support for passing any extra ruby options each_object(cls.singleton_class) should not walk special classes. This test has been moved to RubySpec. [Truffle] Typo. [Truffle] Tag failing regex specs. [Truffle] Tag failing Time.at spec. The bin200 distribution has grown in size. [Truffle] Fixed two typos of the same word on the same line of code. [Truffle] Simplified the fix in 5446cc2. [Truffle] Fixed an NPE when a SharedMethodInfo has a null argumentsDescriptors. [Truffle] Fixed the interpreter_path when the root JRuby distribution directory is not named "jruby." Fixes #3483. define_method with empty body throws RuntimeError in interpreter Test main on Java 9 ...

westlakem · 2015-12-04T18:52:22Z

Do we know when we are planning on this going live? I believe it's effecting RAKE tasks as well (at least cucumber::task rake tasks)

EDIT: I found where the issue was, it's in the cucumber rerun formatter

headius added ir JRuby 9000 labels Nov 20, 2015

headius added this to the JRuby 9.0.5.0 milestone Nov 20, 2015

headius changed the title ~~define_method with * args throws RuntimeError~~ define_method with empty body throws RuntimeError in interpreter Nov 23, 2015

enebo closed this as completed in 1ee9007 Nov 23, 2015

headius mentioned this issue Jan 19, 2016

define_method doesn't work with empty blocks #3606

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

define_method with empty body throws RuntimeError in interpreter #3483

define_method with empty body throws RuntimeError in interpreter #3483

annawinkler commented Nov 20, 2015

headius commented Nov 20, 2015

annawinkler commented Nov 20, 2015

annawinkler commented Nov 20, 2015

annawinkler commented Nov 20, 2015

headius commented Nov 20, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

enebo commented Nov 23, 2015

enebo commented Nov 23, 2015

westlakem commented Dec 4, 2015

define_method with empty body throws RuntimeError in interpreter #3483

define_method with empty body throws RuntimeError in interpreter #3483

Comments

annawinkler commented Nov 20, 2015

headius commented Nov 20, 2015

annawinkler commented Nov 20, 2015

annawinkler commented Nov 20, 2015

annawinkler commented Nov 20, 2015

headius commented Nov 20, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

headius commented Nov 23, 2015

enebo commented Nov 23, 2015

enebo commented Nov 23, 2015

westlakem commented Dec 4, 2015