plasma-umass / doppio

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

Error in trace creation #470

Closed marcosscriven closed 7 years ago

marcosscriven commented 7 years ago

Running Doppio from build off master, I got an Unexpected Integer Javascript error in Chrome.

It appears to be in this emitted function:

(function(f,t,u
/**/) {
var s0 = f.opStack.pop();f.locals[6]=s0;var s1=null;f.locals[7]=s1;var s2=f.locals[4];if(s2>=-17&&s2<=-1){f.pc=17+f.method.getCodeAttribute().getCode().readInt32BE(32+((s2--17)*4))}else{f.pc=1838}
})

Which formatted looks like:

(function(f, t, u
    /**/
) {
    var s0 = f.opStack.pop();
    f.locals[6] = s0;
    var s1 = null;
    f.locals[7] = s1;
    var s2 = f.locals[4];
    if (s2 >= -17 && s2 <= -1) {
        f.pc = 17 + f.method.getCodeAttribute().getCode().readInt32BE(32 + ((s2--17) * 4))
    } else {
        f.pc = 1838
    }
})

Making it clearer the double-minus in the readInt32BE(...) args is the issue.

I searched for when f.opStack.pop() first appeared, and found this commit by @hrj : https://github.com/plasma-umass/doppio/commit/4cf67d724547c0e9c2afe72f0781a98ac6cbdd85

Running against the parent of that fixes the issue.

Unfortunately the code in question is third party, so I can't easily provide a test case, but do let me know if there's any further debug info I can provide.

hrj commented 7 years ago

Ah, this is easy to solve. Constants should have a space preceding them in the emitted code.

I will try to locate the opcode that causes this.. but a test-case would help fix and validate faster.

marcosscriven commented 7 years ago

Thanks @hrj - all I know is it's somewhere in the execution of code in a third-party jar.

If you can briefly describe how I find out what class/method is running at the time, I could probably decompile it and create a test case.

hrj commented 7 years ago

The emitted code is for the tableswitch opcode (which doesn't have a unit-test I think). I have pushed a tentative fix to this branch. Can you try it @marcosscriven ?

To figure out the source of error, on CLI you can add the -XX:+PrintCompilation flag.

In the library, you can call JVM.setPrintJITCompilation(true) and use the dev build.

marcosscriven commented 7 years ago

Hi @hrj - that worked thanks. 🏆

Incidentally I tried -XX:+PrintCompilation, but didn't seem to have any effect. I deliberately misspelt it too as a sanity check, and it's definitely being picked up (complains of no such option if I misspell it).

I've got stdout and stderr correctly piping to the console, so not sure what the issue there is.

jvilk commented 7 years ago

@marcosscriven I believe that flag only works in development or fast-development builds. I'll open an issue, since we shouldn't list flags that aren't operational in a build.