94257 - Computation within a array index fails.

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[wtam]

This following javascript fails to execute:
var a = new Array(4);
a[1+3] = "test";  // this is the line that fails.

The problem is in the generation of the parse tree in the class
org.mozilla.javascript.IRFactory in the method "createSetProp".  When the above
code is parsed, "1+3" is parsed into a node/branch that is separate from the
array accessor node.  It is later cloned and appended to the array accessor
node.  The problem is, only a shallow clone is performed on the parent node of
computation branch, which means that none of the children nodes, ie. the actual
nodes representing the computation itself, is cloned.

The solution was simply to do a deep clone, but the Node object did not include
a deep clonging method.  Not wanting break anything in the Node object, I put
the following deep cloning method into IRFactory:

    private Node deepClone(Node n)
    {
        Node first = n.getFirst();
        if(first != null){
            if(first.getType() == TokenStream.IFNE){
                Node ifTrue = first.getNext();
                Node ifFalse = ifTrue.getNext().getNext().getNext();
                return (Node)createIf(deepClone(first.getFirst()),
                                      deepClone(ifTrue),
                                      deepClone(ifFalse),-1);
            }else{
                Node result = n.cloneNode();
                while(first != null){
                    result.addChildToBack(deepClone(first));
                    first = first.getNext();
                }
                return result;
            }
        }
        return n.cloneNode();
    }

I used this method in "createSetProp" to change this:

        } else {
            tmp1 = obj.cloneNode();
            tmp2 = id.cloneNode();
            opLeft = new Node(nodeType, obj, id);
        }

to this:

        } else {
            tmp1 = obj.cloneNode();
            tmp2 = deepClone(id); 
            opLeft = new Node(nodeType, obj, id);
        }

I found this bug in 1.5R1, but I think R2 also has this problem.

Comment 1

[Phil Schwartau]

This is working correctly for me with the latest Rhino source:

[c:/] java org.mozilla.javascript.tools.shell.Main

js> var a = new Array(4);

js> a.length
4

js> a[1+3] = "test";
test

js> a[4]
test

js> a[1+3]
test

js> a[2+2]
test


[e-mail]: can you download the latest tarball (Rhino 1.5r2)
at ftp://ftp.mozilla.org/pub/js, or the latest Rhino source from CVS,
and see if the problem has gone away for you, too?

Comment 2

[wtam]

I gave the wrong test code with my original submission.  I found the problem
almost a year ago, so I forgot what the exact problem was.  Here's the original
code I used to test for the problem:

var testArray = new Array(6);
testArray[1+1] = 1;
testArray[1+1]+=2;

Comment 3

[Phil Schwartau]

[e-mail]: thank you; this is a bug with my source, too:

[c/] java org.mozilla.javascript.tools.shell.Main

js> var testArray = new Array(6);

js> testArray.length
6

js> testArray[1+1] = 1;
1

js> testArray.length
6

js> testArray[1+1]
1

js> testArray[2]
1

js> testArray[2]+=2;                          <<<<<<<<<<<<<<<<<< (THIS IS OK)
3

js> testArray[1+1]+=2;                        <<<<<<<<<<<<<<<<<< (THIS CRASHES)
Exception in thread "main" java.lang.NullPointerException
at org.mozilla.javascript.Interpreter.generateICode(Interpreter.java:258)
at org.mozilla.javascript.Interpreter.generateICode(Interpreter.java:579)
at org.mozilla.javascript.Interpreter.generateICode(Interpreter.java:660)
at org.mozilla.javascript.Interpreter.generateICode(Interpreter.java:810)
at org.mozilla.javascript.Interpreter.generateICode(Interpreter.java:279)
at org.mozilla.javascript.Interpreter.generateICodeFromTree(Interpreter.java:89)
at org.mozilla.javascript.Interpreter.generateScriptICode(Interpreter.java:134)
at org.mozilla.javascript.Interpreter.compile(Interpreter.java:78)
at org.mozilla.javascript.Context.compile(Context.java:1810)
at org.mozilla.javascript.Context.compile(Context.java:1735)
at org.mozilla.javascript.Context.compileReader(Context.java:852)
at org.mozilla.javascript.Context.evaluateReader(Context.java:770)
at org.mozilla.javascript.tools.shell.Main.evaluateReader(Main.java:293)
at org.mozilla.javascript.tools.shell.Main.processSource(Main.java:217)
at org.mozilla.javascript.tools.shell.Main.exec(Main.java:104)
at org.mozilla.javascript.tools.shell.Main.main(Main.java:66)

Comment 4

[Norris Boyd]

Thanks, I just checked in a fix to create a temp rather than the deep clone.

Comment 5

[Phil Schwartau]

Marking VERIFIED. Testcase added to JS testsuite: 

           mozilla/js/tests/js1_5/Array/regress-94257.js

Testcase passing in Rhino compiled and interpreted modes on WinNT.

Comment 6

[Norris Boyd]

*** Bug 109438 has been marked as a duplicate of this bug. ***

Comment 7

[Igor Bukanov]

Targeting as resolved against 1.5R3