332369 - Wrong results when using compiled Javascript Functions with JRockit VM

Status: RESOLVED INVALID

(Rhino Graveyard :: Compiler, defect)

Description

[Oliver Weise]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Rhino 1.6R2

When using JRockit VM there is a certain script constellation that returns wrong results, when Rhino is not in interpreter mode (optimization set to 0-9). The problem occurs, when a function returns a boolean value, that is calculated inside the return statement. It seems, that this always returns true.

Reproducible: Always

Steps to Reproduce:
1. Start Rhino using BEA JRockit VM (I'm using jdk 1.5.0_03) with optimization level 0-9
2. Compile the following script into a Rhino function object:

function theFunc() { return (true==false); }

3. Call the function and see return value.

Actual Results:  
true

Expected Results:  
false

Minimal proof-of-failure code:

String code ="function theFunc() { return (true==false); }";            
Context cx =Context.enter();
Scriptable scope = new ImporterTopLevel();
Function func = cx.compileFunction(scope, code, "theFunc", 0, null);
System.out.println(func.call(cx, scope, func, new Object[] {}));

Comment 1

[Igor Bukanov]

Reassigning to the reporter as I have neither interest nor time for Rhino.

See also bug 288433.

Comment 2

[Attila Szegedi]

You could try decompiling the class with JAD, recompile the resulting .java file with plain javac, and debug it to see where does it go wrong. 

Unfortunately, I only have Mac OS X on a PowerPC machine, so I can't run JRockit myself (as it's only available for Intel, AMD, and Sparc CPUs, and only for Windows, Linux, and Solaris).

Comment 3

[Oliver Weise]

Attachment: Decompiled proof-of-failure code

I uploaded the decompiled proof-of-failure code to this bug. As you see, there is one JVM instruction "tableswitch" in method "call", that JAD cannot correctly decompile and which is - as I think - the source of trouble.  Unfortunately it prevents me from effectively debugging the code.

The class file, that is the origin of this code, returns correct results when run via Sun VM (false) and wrong results via JRockit VM (true).

As I understand it (which might not be very well), this tableswitch does something like the java "switch" statement, using the field _id as parameter. When I modify this code to use a "real" switch statement it returns correct results running on JRockit VM (as expected).

So taking the facts, there is
a) a tableswitch VM instruction, that somehow cannot be decompiled correctly (and therefor might not be 100% correct?) that
b) the Sun VM understands as intended and
c) the JRockit VM does not

More details to come as I dig deeper into the magic...

Comment 4

[Attila Szegedi]

Well, if JAD fails to decompile it, you might as well just look at the raw bytecode, as produced by "javap -c" (javap is standard JDK tool) -- if you atach the output here, I can look at it too as I'm quite good at reading raw bytecode :-)

Comment 5

[Oliver Weise]

This is the bytecode of the faulting call method:

 0 aload_1
 1 invokestatic #45 <org/mozilla/javascript/ScriptRuntime.hasTopCall>
 4 ifne 17 (+13)
 7 aload_0
 8 aload_1
 9 aload_2
10 aload_3
11 aload 4
13 invokestatic #49 <org/mozilla/javascript/ScriptRuntime.doTopCall>
16 areturn
17 aload_0
18 aload_1
19 aload_2
20 aload_3
21 aload 4
23 aload_0
24 getfield #17 <thefunc._id>
27 tableswitch 1 to 1
          1:  48 (+21)
          default:  44 (+17)
44 invokestatic #53 <thefunc._c0>
47 areturn
48 invokestatic #56 <thefunc._c1>
51 areturn

This is the byte code of the modified call method, where I replaced the undecompilable tableswitch with a java switch statememt, and which works in both VMs.

 0 aload_1
 1 invokestatic #54 <org/mozilla/javascript/ScriptRuntime.hasTopCall>
 4 ifne 17 (+13)
 7 aload_0
 8 aload_1
 9 aload_2
10 aload_3
11 aload 4
13 invokestatic #58 <org/mozilla/javascript/ScriptRuntime.doTopCall>
16 areturn
17 aload_0
18 getfield #20 <thefunc._id>
21 tableswitch 1 to 1
          1:  50 (+29)
          default:  40 (+19)
40 aload_0
41 aload_1
42 aload_2
43 aload_3
44 aload 4
46 invokestatic #62 <thefunc._c0>
49 areturn
50 aload_0
51 aload_1
52 aload_2
53 aload_3
54 aload 4
56 invokestatic #65 <thefunc._c1>
59 areturn

I'm no bytecode augur. I can see/guess, that they differ by the parameters being loaded into the operand stack before and after the tableswitch. I cannot see what might make different VMs behave differently (but I need to study the VM spec). Can you?

Comment 6

[Attila Szegedi]

Well, in Java source code, "switch" is a statement, not an expression, so you can't have either "lookupswitch" or "tableswitch" bytecodes occur as part of loading the argument list of a method call onto a stack, since you can't use "switch" within the argument list of someMethodCall(...) -- on source level. While this bytecode sequence is perfectly legal, it can never occur in code compiled from a .java source file. 

You basically uncovered a bug in JRockit - while this bytecode sequence is admittedly a bit exotic, it is completely legal, as there's no restriction in JVM spec saying that lookupswitch/tableswitch can only occur when there's nothing on the operand stack (except their sole operand, that is). So any conformant JVM - inclusing JRockit - should be able to process it. My guess is that JRockit probably does JIT compilation by looking for typical patterns produced by Java compilers and, well, this is not one of those :-)

With more and more technologies out there that rely on on-the-fly code generation and instrumentation, BEA should make sure that JRockit can handle any legal bytecode thrown at it, and not just the "usual" sequences produced by javac...

While we could probably address this in Rhino as well - namely, change our code generator so it emits something that doesn't make JRockit choke on it - there is unfortunately two problems with that.

1. There's currently no active committer around who knows the code generator well enough to dare to touch it.

2. The problem ultimately lies in JRockit, and even if we fixed it in Rhino there's no telling when JRockit will choke on something similar coming from another code generator, anything using Apache BCEL, ASM, or CGLIB - i.e. Hibernate, any AOP bytecode transformers, you name it.

Comment 7

[Oliver Weise]

Agreed. VMs should be able to correctly run anything that matches the bytecode spec, not just things typical for java. In the end all this chatter about java the platform might be true and they must cope with classes generated by Groovy, JRuby or even Rhino :-) I will try to insert the problem at BEA.