203841 - Bad code generation to merge "if" and boolean expressions

Status: VERIFIED FIXED

(Rhino Graveyard :: Compiler, defect)

Description

[Brian Gugliemetti]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20021216
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20021216

When using a comma operator inside an if statement in non-interpreted mode, as
in the following example,

if (a = 5, b == 6) { c = 1; }

Rhino throws this exception:

Exception in thread "main" java.lang.IllegalStateException: Stack underflow: -1
        at org.mozilla.classfile.ClassFileWriter.badStack(ClassFileWriter.java:1154)
        at org.mozilla.classfile.ClassFileWriter.add(ClassFileWriter.java:401)
        at org.mozilla.javascript.optimizer.Codegen.addByteCode(Codegen.java:3568)
        at org.mozilla.javascript.optimizer.Codegen.visitGOTO(Codegen.java:1574)
        at
org.mozilla.javascript.optimizer.Codegen.generateCodeFromNode(Codegen.java:1172)
        at
org.mozilla.javascript.optimizer.Codegen.generateCodeFromNode(Codegen.java:1044)
        at
org.mozilla.javascript.optimizer.Codegen.generateCodeFromNode(Codegen.java:1044)
        at org.mozilla.javascript.optimizer.Codegen.generateCode(Codegen.java:409)
        at org.mozilla.javascript.optimizer.Codegen.compile(Codegen.java:137)
        at org.mozilla.javascript.Context.compile(Context.java:1916)
        at org.mozilla.javascript.Context.compileReader(Context.java:886)
        at org.mozilla.javascript.Context.evaluateReader(Context.java:803)
        at org.mozilla.javascript.tools.shell.Main.evaluateReader(Main.java:363)
        at org.mozilla.javascript.tools.shell.Main.processSource(Main.java:260)
        at org.mozilla.javascript.tools.shell.Main.exec(Main.java:103)
        at org.mozilla.javascript.tools.shell.Main.main(Main.java:76)

Reproducible: Always

Steps to Reproduce:
1. use the js> shell
2. Enter the following:

js> Packages.org.mozilla.javascript.Context.enter().setOptimizationLevel(1);
js> if (a = 5, b==6} { return false; }

Actual Results:  
Got this exception:

org.mozilla.javascript.optimizer.Codegen.generateCode(Codegen.java:409)
        at org.mozilla.javascript.optimizer.Codegen.compile(Codegen.java:137)
        at org.mozilla.javascript.Context.compile(Context.java:1916)
        at org.mozilla.javascript.Context.compileReader(Context.java:886)
        at org.mozilla.javascript.Context.evaluateReader(Context.java:803)
        at org.mozilla.javascript.tools.shell.Main.evaluateReader(Main.java:363)
        at org.mozilla.javascript.tools.shell.Main.processSource(Main.java:260)
        at org.mozilla.javascript.tools.shell.Main.exec(Main.java:103)
        at org.mozilla.javascript.tools.shell.Main.main(Main.java:76)

Expected Results:  
No exception

This isn't a problem in interpreted mode.

Comment 1

[Phil Schwartau]

Confirming bug; cc'ing Igor -

Comment 2

[Phil Schwartau]

Testcase added to JS testsuite:

      mozilla/js/tests/js1_5/Regress/regress-203841.js


Currently passing in SpiderMonkey and Rhino interpreted mode.
But crashing in any optimized Rhino mode, with the above stack.

Comment 3

[Igor Bukanov]

I changed the subject since the same problem persists not only with comma
operator cases like:

if (1, 1 == 6) ;

but also with the following cases"

// get elem case
x=[];
if (x[1==2]) ;

// set elem case
if (x[1==2]=1) ;

// delete elem case
if (delete x[1==2]) ;


All these cases caused by incorrect code generation optimization to merger
evaluation of boolean expressions like x == y, a && b etc. with generation code
for conditional jumps like code for if statements, ? operator etc.

In fact these test cases are produced by looking for the code of
generateCodeFromNode in  omj/optimizer/Codegen.java.

Comment 4

[Igor Bukanov]

Attachment: Fix for Codegen.java: conditional jumps are processed in new generateIfJump method

To fix the issue I moved away from generateCodeFromNode code to merge boolean
checks and conditional jumps into separated generateIfJump method that tries to
apply this optimization and if it is not possible, it calls
generateCodeFromNode and adds a generic jump.

Comment 5

[Phil Schwartau]

I've added Igor's new examples from Comment #3 to the testcase.
Once again, passing in SpiderMonkey and in Rhino interpreted mode,
but failing in Rhino compiled modes.

Comment 6

[Igor Bukanov]

I committed the fix. By "if and boolean checks merger" I mean special code in
the optimizer not to generate Boolean object just to convert them to boolean on
JVM stack later when generating bytecode for conditional jumps but rather use
IFLE etc. bytecode to compare and jump skipping intermediate steps. 

Hopefully the fix closed all the problems with the optimization.

Comment 7

[Phil Schwartau]

Verified FIXED.

The above testcase now passes not only in Rhino interpreted mode,
but in optimized modes as well.

Comment 8

[Igor Bukanov]

Targeting as resolved against 1.5R5