plasma-umass / doppio

Breaks the browser language barrier (includes a plugin-free JVM).
http://plasma-umass.github.io/doppio-demo
MIT License
2.17k stars 176 forks source link

Kawa is broken #164

Closed int3 closed 10 years ago

int3 commented 11 years ago

./console/runner.coffee kawa/repl

|kawa:1|# (+ 1 1)

Internal JVM Error: Error: Error in get_initialized_class: Class Lgnu/mapping/CallContext; is not initialized. Error: Error in get_initialized_class: Class Lgnu/mapping/CallContext; is not initialized. at CustomClassLoader.ClassLoader.get_initialized_class (/Users/jez/Dropbox/src/doppio/src/ClassLoader.coffee:235:13) at RuntimeState.root.RuntimeState.RuntimeState.get_class (/Users/jez/Dropbox/src/doppio/src/runtime.coffee:138:50) at FieldOpcode.root.opcodes.root.FieldOpcode.execute (/Users/jez/Dropbox/src/doppio/src/opcodes.coffee:1577:18) at Method.root.Method.Method.bytecode_loop (/Users/jez/Dropbox/src/doppio/src/methods.coffee:357:17) at Method.root.Method.Method.run_bytecode (/Users/jez/Dropbox/src/doppio/src/methods.coffee:325:21) at StackFrame.root.Method.Method.setup_stack.sf.runner (/Users/jez/Dropbox/src/doppio/src/methods.coffee:389:22) at Object.root.RuntimeState.RuntimeState.run_until_finished as _onTimeout at Timer.list.ontimeout (timers.js:101:19)

jvilk commented 11 years ago

Oh, good catch! I really wish we had this as a whole-system test so I knew during commit that it's broken.

(Meaning, we should have a more robust suite of whole-system tests, as mentioned in a previous issue I opened. :smile: )

perimosocordiae commented 11 years ago

We do have a set of whole-system tests, but you have to run it yourself: classes/special_test/EndToEnd

It doesn't test Kawa, though, because we don't bundle it with Doppio.

perimosocordiae commented 11 years ago

Speaking of, Rhino is broken, sometime between commit 978e9a99fd6c0d73451a6ecf0aae171c1d1451a4 and now (commit 97f494fb720e30bbb29930fb0e509c2dc7b400a0).

From EndToEnd:

Exception in thread "main" java.lang.ExceptionInInitializerError
    at sun.org.mozilla.javascript.internal.MemberBox.invoke(MemberBox.java:132)
    at sun.org.mozilla.javascript.internal.ScriptableObject.setBySetter(ScriptableObject.java:1693)
    at sun.org.mozilla.javascript.internal.ScriptableObject.put(ScriptableObject.java:230)
    at sun.org.mozilla.javascript.internal.IdScriptableObject.put(IdScriptableObject.java:407)
    at sun.org.mozilla.javascript.internal.ScriptableObject.putProperty(ScriptableObject.java:1386)
    at com.sun.script.javascript.JavaAdapter.init(JavaAdapter.java:51)
    at com.sun.script.javascript.RhinoTopLevel.(RhinoTopLevel.java:52)
    at com.sun.script.javascript.RhinoScriptEngine.(RhinoScriptEngine.java:118)
    at com.sun.script.javascript.RhinoScriptEngineFactory.getScriptEngine(RhinoScriptEngineFactory.java:57)
    at javax.script.ScriptEngineManager.getEngineByName(ScriptEngineManager.java:225)
    at com.sun.tools.script.shell.Main.getScriptEngine(Main.java:393)
    at com.sun.tools.script.shell.Main.processOptions(Main.java:114)
    at com.sun.tools.script.shell.Main.main(Main.java:26)
    at classes.special_test.EndToEnd.testRhino(EndToEnd.java:49)
    at classes.special_test.EndToEnd.main(EndToEnd.java:66)
Caused by: java.lang.InternalError: bouncer cannot be found
    at sun.reflect.misc.MethodUtil.getTrampoline(MethodUtil.java:282)
    at sun.reflect.misc.MethodUtil.(MethodUtil.java:47)
    ... 15 more
perimosocordiae commented 11 years ago

Looks like an issue with my old nemesis, java.util.Currency. Try untrapping the getInstance method that we've NOP'd to see the breakage. I think it has something to do with @jvilk's field access changes.

jvilk commented 11 years ago

I think Rhino is fixed now. Does Kawa-Scheme work as expected now?

jvilk commented 11 years ago

The repl boots, but I can easily break it:

#|kawa:1|# j
d
ddsf
dsf
/dev/stdin:1:1: warning - no declaration seen for j

Internal JVM Error: Error: Error in get_initialized_class: Class Lgnu/mapping/CallContext; is not initialized.
Error: Error in get_initialized_class: Class Lgnu/mapping/CallContext; is not initialized.
    at Error (unknown source)
    at CustomClassLoader.ClassLoader.get_initialized_class (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/ClassLoader.js:87:19)
    at RuntimeState.root.RuntimeState.RuntimeState.get_class (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/runtime.js:52:99)
    at FieldOpcode.root.opcodes.root.FieldOpcode.execute (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/opcodes.js:1049:26)
    at Method.root.Method.Method.bytecode_loop (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/methods.js:137:43)
    at Method.root.Method.Method.run_bytecode (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/methods.js:126:29)
    at StackFrame.root.Method.Method.setup_stack.sf.runner (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/methods.js:148:30)
    at root.RuntimeState.RuntimeState.run_until_finished (/Users/jvilk/Code/doppio_cacheremove/doppio/build/opt/src/runtime.js:234:54)
    at process.startup.processNextTick.process._tickCallback (node.js:244:9)
jvilk commented 11 years ago

The issue is in here:

180: new root.FieldOpcode 'getfield', { execute: (rs) ->
    cls = rs.get_class(@field_spec.class) <-- Error here
    field = cls.field_lookup(rs, @field_spec)
    name = field.cls.get_type() + @field_spec.name
    new_execute =
      if @field_spec.type not in ['J','D']
        (rs) ->
          val = rs.check_null(rs.pop()).get_field rs, name
          rs.push val
      else
        (rs) ->
          val = rs.check_null(rs.pop()).get_field rs, name
          rs.push2 val, null
    new_execute.call(@, rs)
    @execute = new_execute
    return
  }

The class associated with the getfield should be initialized, but it isn't. The error indicates that it is loaded, but not initialized.

The logic being that the class of the object on the stack should have field_spec.class in its inheritance chain; thus, since there exists an object of a class with field_spec.class in its inheritance chain, field_spec.class must be initialized.

One way you might be able to betray this is if the object is null and is the first object of that type... I'll have to check that.

jvilk commented 11 years ago

My hunch was correct. We will have to do the null object check before any class assumptions. I updated Inheritance with a test that triggers this failure.

jvilk commented 11 years ago

Hm. I fixed that error, but it's still failing in the same place.

jvilk commented 11 years ago

I think I figured out the issue.

First off, the error message is incorrect: The class is not loaded in the classloader. I forgot to appropriately propagate up the null_handled argument from get_initialized_class to get_loaded_class.

Second, I realized I'm synchronously fetching classes in some places in the opcodes where it needs to be asynchronous. While the class is guaranteed to be loaded or initialized somewhere, it's not guaranteed to be loaded or initialized in the current classloader.

So, Kawa-Scheme is currently failing at the following opcode: https://github.com/int3/doppio/blob/master/src/opcodes.coffee#L649

This class is going to be in the inheritance hierarchy of the object we get off of the stack. However, we are supposed to resolve the class reference through the class's classloader, so we do so anyway.

Unfortunately, the classloader that loaded the class is different than the classloader that loaded the class we are looking up, so it fails. We are supposed to trigger loadClass through the current class's classloader, which would invoke the classloader that actually loaded the class we are looking up.

While I could just peep the inheritance hierarchy of the object we pop off of the stack and keep the first run synchronous, I think I will call the classloader instead.

tl;dr: Gotta make the first run of get/setfield potentially asynchronous.

perimosocordiae commented 11 years ago

We may have been premature. While the REPL works fine, the -e and -f script evaluation options are still busted. The -f option produces the same error, but in an infinite loop:

./doppio kawa/repl -- -e '(+ 5 2)'
java.lang.ExceptionInInitializerError
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:247)
    at gnu.expr.Language.getCompilation(Language.java:607)
    at gnu.expr.Language.parse(Language.java:683)
    at kawa.Shell.run(Shell.java:252)
    at kawa.Shell.run(Shell.java:194)
    at kawa.repl.processArgs(repl.java:228)
    at kawa.repl.main(repl.java:869)
Caused by: java.lang.NullPointerException:
    at gnu.bytecode.ArrayClassLoader.loadClass(ArrayClassLoader.java:142)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:247)
    at gnu.bytecode.ObjectType.getContextClass(ObjectType.java:90)
    at gnu.bytecode.ObjectType.getReflectClass(ObjectType.java:122)
    at gnu.bytecode.ClassType.getDeclaredMethods(ClassType.java:748)
    at gnu.bytecode.ClassType.getDeclaredMethod(ClassType.java:937)
    at gnu.bytecode.ClassType.getDeclaredMethod(ClassType.java:957)
    at gnu.expr.Compilation.(Compilation.java:296)
    at kawa.lang.Translator.(Translator.java:73)
    at kawa.standard.SchemeCompilation.(SchemeCompilation.java:20)
    ... 8 more
Exited with code -1

Looks like yet another classloader bug, @jvilk. The relevant bits of the vtrace are here: http://pastebin.com/ckQe2bvP

jvilk commented 11 years ago

Thanks for the info. I'll take a look into this...

jvilk commented 11 years ago

Aha! I had a feeling this would come to bite me.

The ClassLoader specification allows us to use null to represent the bootstrap class loader. Despite this, Kawa-Scheme is calling getParent() to get its parent classloader object, which is the bootstrap classloader, and then is invoking loadClass on it. However, it is null.

This is a bug in Kawa-Scheme tightly coupling itself with HotSpot's behavior. Changing the bootstrap classloader to a non-null object would break the various checks around the code that use the null value to identify the bootstrap classloader. Eventually, we should move toward having an actual Java object for the bootstrap classloader, which would allow us to treat custom and non-custom classloaders in an identical manner and should simplify classloading code.

The easiest thing to do now is to modify Kawa to use the static bootstrap classloader methods when getParent returns null, as I would prefer to not refactor classloading again until after the deadline.

jvilk commented 11 years ago

I forgot to reference #159, which is the solution to this problem.

jvilk commented 11 years ago

The solution is to change getParent().loadClass(name) to Class.forName(classname, false, getParent()), which is a static method that appropriately handles a null bootstrap classloader. I'm compiling a JAR file that I will test and upload.

jvilk commented 11 years ago

You can download a patched version here.

We can keep this issue open until I fix the classloader issue.

jvilk commented 11 years ago

It's broken even further!

$ ./doppio -classpath kawa/ kawa/repl -f nqueens2.sch
/Users/jvilk/PLASMADOPPIO2/doppio/nqueens2.sch:8:5: evaluating syntax transformer 'letrec' threw java.lang.NoSuchFieldError: Lkawa/lib/prim_syntax$frame;out$Mnbindings
/Users/jvilk/PLASMADOPPIO2/doppio/nqueens2.sch:31:3: warning - no declaration seen for nqueens
/Users/jvilk/PLASMADOPPIO2/doppio/nqueens2.sch:31:3: unbound location nqueens
    at gnu.mapping.SharedLocation.get(SharedLocation.java:22)
    at gnu.mapping.ThreadLocation.get(ThreadLocation.java:105)
    at atInteractiveLevel$3.main(nqueens2.sch:31)
    at atInteractiveLevel$3.apply0(nqueens2.sch:30)
    at gnu.expr.ModuleMethod.apply0(ModuleMethod.java:186)
    at gnu.expr.ModuleMethod.apply(ModuleMethod.java:160)
    at gnu.mapping.CallContext.runUntilDone(CallContext.java:235)
    at gnu.expr.ModuleExp.evalModule2(ModuleExp.java:377)
    at gnu.expr.ModuleExp.evalModule(ModuleExp.java:211)
    at kawa.Shell.run(Shell.java:279)
    at kawa.Shell.runFile(Shell.java:486)
    at kawa.Shell.runFileOrClass(Shell.java:420)
    at kawa.repl.processArgs(repl.java:248)
    at kawa.repl.main(repl.java:869)

Contents of nqueens2.sch:

;;; NQUEENS -- Compute number of solutions to 8-queens problem. Doppio benchmark edition.

(define trace? #f)

(define (nqueens n)

  (define (iota1 n)
    (let loop ((i n) (l '()))
      (if (= i 0) l (loop (- i 1) (cons i l)))))

  (define (my-try x y z)
    (if (null? x)
      (if (null? y)
        (begin (if trace? (begin (write z) (newline))) 1)
        0)
      (+ (if (ok? (car x) 1 z)
           (my-try (append (cdr x) y) '() (cons (car x) z))
           0)
         (my-try (cdr x) (cons (car x) y) z))))

  (define (ok? row dist placed)
    (if (null? placed)
      #t
      (and (not (= (car placed) (+ row dist)))
           (not (= (car placed) (- row dist)))
           (ok? row (+ dist 1) (cdr placed)))))

  (my-try (iota1 n) '() '()))

(define (main)
  (nqueens 8))

(main)
perimosocordiae commented 11 years ago

Interesting. I tried looking at the disassembly for the class in question, kawa/lib/prim_syntax$frame, and discovered some weirdness:

$ javap -v kawa.lib.prim_syntax$frame
Compiled from "prim_syntax.scm"
public class kawa.lib.prim_syntax extends gnu.expr.ModuleBody implements gnu.expr.RunnableModule
  InnerClass:
   public #641= #239; //prim_syntax$frame=class kawa/lib/prim_syntax$frame
...snip...
  Constant pool:
const #1 = Asciz        kawa/lib/prim_syntax;
const #2 = class        #1;     //  kawa/lib/prim_syntax

And with Doppio:

$ node build/release-cli/console/disassembler.js /tmp/doppio/kawa-1.13/kawa/lib/prim_syntax\$frame.class
Compiled from "prim_syntax.scm"
public class kawa.lib.prim_syntax$frame extends gnu.expr.ModuleBody
  InnerClass:
   public #132= #2; //prim_syntax$frame=class kawa/lib/prim_syntax$frame
...snip...
  Constant pool:
const #1 = Asciz        kawa/lib/prim_syntax$frame;
const #2 = class        #1;     //  Lkawa/lib/prim_syntax$frame;

Notice how javap sees class kawa.lib.prim_syntax, with a big long constant pool, while Doppio sees kawa.lib.prim_syntax$frame.

Later, we see with javap:

const #238 = Asciz      kawa/lib/prim_syntax$frame;
const #239 = class      #238;   //  kawa/lib/prim_syntax$frame
const #240 = Method     #239.#23;       //  kawa/lib/prim_syntax$frame."<init>":()V
const #241 = Asciz      gnu/lists/LList;
const #242 = class      #241;   //  gnu/lists/LList
const #243 = Asciz      Empty;

And the (I think) corresponding lines in the Doppio disasm:

const #1 = Asciz        kawa/lib/prim_syntax$frame;
const #2 = class        #1;     //  Lkawa/lib/prim_syntax$frame;
const #3 = Asciz        java/lang/Object;
const #4 = class        #3;     //  Ljava/lang/Object;
const #5 = Asciz        $unnamed$0;
const #6 = Asciz        [Ljava/lang/Object;;

Also, this is happening:

$ ./doppio -cp /tmp/doppio/kawa-1.13:. classes/util/Javap kawa.lib.prim_syntax$frame
ERROR:Could not find kawa.lib.prim_syntax
jvilk commented 11 years ago

Classpath doesn't work like that with javap. Passing -classpath to the javap shell command works differently than java -classpath.

We get the correctish output with:

./doppio-dev classes/util/Javap -c -private -verbose kawa/kawa/lib/prim_syntax\$frame

... where you replace kawa/kawa/lib/prim_syntax\$frame with the path to the class file.

... except one small thing is different at the start of the file.

Doppio:

Compiled from "prim_syntax.scm"
public class kawa.lib.prim_syntax$frame extends gnu.expr.ModuleBody
  InnerClass:
   public #132= #2; //prim_syntax$frame=class kawa/lib/prim_syntax$frame 
  EnclosingMethod: length = 0x4
   00 10 00 80

HotSpot:

Compiled from "prim_syntax.scm"
public class kawa.lib.prim_syntax$frame extends gnu.expr.ModuleBody
  InnerClass:
   public #132= #2; //prim_syntax$frame=class kawa/lib/prim_syntax$frame
  EnclosingMethod: length = 0x4
   00 10 00 FFFFFF80
jvilk commented 11 years ago

Also, it looks like you forgot to escape the $ on your java command.

perimosocordiae commented 11 years ago

Actually, you do still need the classpath. This is what worked for me:

./doppio classes/util/Javap -c -private -verbose -classpath /tmp/doppio/kawa-1.13 kawa/lib/prim_syntax\$frame.class

Interestingly, we do have a diff (between javap and ./doppio classes/util/Javap) for kawa.lib.prim_syntax:

$ diff javap.out doppio.out
2067c2067
<    2340:  sipush  -4095
---
>    2340:  sipush  61441
2683c2683
<    37:    iinc    3, -1
---
>    37:    iinc    3, 255
2685c2685
<    41:    ifge    29
---
>    41:    ifge    65565

I've seen something like this before, but it was a long time ago...

perimosocordiae commented 11 years ago

Our output now matches native java's for the diff, but the Kawa error persists.

jvilk commented 10 years ago

Current status:

$ java -jar kawa-1.13.jar
#|kawa:1|# (+ 1 1)
java.lang.NoSuchMethodError: No such method found in gnu.math.IntNum::writeObject(Ljava/lang/Object;)V
    at atInteractiveLevel$1.run(stdin:1)
    at gnu.expr.ModuleExp.evalModule2(ModuleExp.java:311)
    at gnu.expr.ModuleExp.evalModule(ModuleExp.java:211)
    at kawa.Shell.run(Shell.java:279)
    at kawa.Shell.run(Shell.java:194)
    at kawa.Shell.run(Shell.java:175)
    at kawa.repl.main(repl.java:890)
jvilk commented 10 years ago

Hahahaha, our swap opcode was broken! It was popping two things off, and then pushing them back on in the same order.

Now it's good and works!

int3 commented 10 years ago

Hah, I noticed the problem with swap when I was writing my D version. Never got around to fixing it since we'd established that javac doesn't generate that opcode. I guess the Kawa compiler is doing that here.

jvilk commented 10 years ago

Dammit, @int3! :tongue: