989586 - new Array(length) return wrong size

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[swediomurital]

User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.152 Safari/537.36

Steps to reproduce:

Firefox Linux i686; rv:28.0

HTML:

Array.length: <input type="text" id="arw1" />
Demanded length:<input type="text" id="arw2" />

JS:

var arw1 = document.getElementById('arw1'),
      arw2 = document.getElementById('arw2');

function arrMany(w, show) {
    var a = new Array(w);
    if (show) {
        arw1.value = a.length;
        arw2.value = w;
    }
    /* optionally something like
    if (a.length !== w) {
         console.log(a.length, w);
    }
    */
}

function tick() {
    arrMany(3, false);
    arrMany(4, false);
    arrMany(5, false);
    arrMany(10, true);
}

setInterval(tick, 1000 / 300);


Actual results:

After some time, 20+ seconds, length of new Array(10) returns size of *previous* call. I.e. 5 instead of 10.


Expected results:

Length should be the one specified.

Notes:

1. No action in Browser Console or Web Console.
2. If Web Console is open the error never occurs.
3. If Web Console is opened when the error manifests, it corrects itself at once.
4. Given pt. 3. it is rather hard to debug in a sane way using the browser itself.

Comment 1

[André Bargull [:anba]]

Still reproducible in latest trunk (tip 0e19655e93df). Not reproducible with ion turned off.

Expected: throws "ok"
Actual: typein:5:4 Error: Assertion failed: got 5, expected 3: iter: 4725

shell test case:
---
function t() {
  var iter = 0;
  function a(w) {
    var a = new Array(w);
    assertEq(a.length, w, "iter: " + ++iter);
    if (iter === 1000000) throw "ok";
  }
  function r() {
    a(3); a(4); a(5); a(10);
  }
  for(;;) r();
}
---

Comment 2

[Till Schneidereit [:till]]

Curious. Note that additional invocations of t() in the shell testcase assert on iteration 1 or 2. Always with "got 3, expected 4".

Except if the shell is started with --ion-eager, then it's always "got 3, expected 4".

The iteration in which the assert happens first varies: Without --ion-eager I saw values between 5293 and 8053, with --ion-eager between 1098 and 1338.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

It starts being wrong after compilation of 'r', which inlines 'a' and uses MNewArray instead of a call to Array's constructor. Looking at visitNewArray in CodeGenerator + the fact that it shows up after different iterations counts lets me think it is GC related, if not a GC bug. Adding a call to gc() anywhere in r() seems to make the problem disappear.


function a(w) {
    var a = new Array(w);
    assertEq(a.length, w)
}
function r() {
    a(3); a(4);
}
for(;;) r();

Comment 4

[Jan de Mooij [:jandem]]

Regression from bug 922270. Consider this line:

    var a = new Array(w);

The Baseline IC will create the array template object with length == 3, and we'll use this template object in Ion, even for the w == 4 case.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

Might as well fix this myself.

Comment 6

[Jan de Mooij [:jandem]]

Thanks a lot for the bug report and shell testcase btw.

Comment 7

[Jan de Mooij [:jandem]]

Very safe fix, we should backport this as it can break real-world websites.

Comment 8

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/37d8a465a7b8

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/37d8a465a7b8

Comment 10

[Jan de Mooij [:jandem]]

Comment on attachment 8399586 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 922270.
User impact if declined: Broken websites.
Testing completed (on m-c, etc.): On m-i and m-c.
Risk to taking this patch (and alternatives if risky): Very low.
String or IDL/UUID changes made by this patch: None.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/36c50045602c
https://hg.mozilla.org/releases/mozilla-beta/rev/37ab641216bd

Comment 12

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

https://hg.mozilla.org/releases/mozilla-beta/diff/37ab641216bd/js/src/jit-test/tests/ion/bug989586.js