694884 - unshift discrepancy between sparse/dense arrays

Status: RESOLVED FIXED

(Tamarin Graveyard :: Library, defect)

Description

[Felix S. Klock II [:pnkfelix, :fklock]]

(uncovered by Bug 693431)

    var a1 = [];
    var a2 = [];
    var b1 = [];
    var b2 = [];
    
    function sparsify(a) { a[0] = 1; a[10000] = 1; a.length = 0; }
    
    sparsify(a1);
    sparsify(a2);
    
    a1[1] = 10;
    a2[1] = 10;
    b1[1] = 10;
    b2[1] = 10;
    
    print("pre  a's: ["+a1+"] ["+a2+"]");
    print("pre  b's: ["+b1+"] ["+b2+"]");
    
    a1.public::unshift(1);
    b1.public::unshift(1);
    a2.AS3::unshift(1);
    b2.AS3::unshift(1);
    
    print("post a's: ["+a1+"] ["+a2+"]");
    print("post b's: ["+b1+"] ["+b2+"]");

For 32-bit DebugDebugger avmshell (rev 6653:5968a1240330), prints:

  pre  a's: [,10] [,10]
  pre  b's: [,10] [,10]
  post a's: [1,,10] [1,,10]
  post b's: [,1,] [,1,]

In the above, the a's are sparse arrays and the b's are dense arrays.  Prior to the same unshift invocation, they both look the same, but after the unshift they appear different in content and length.

This indicates that the distinction between sparse/dense is exposed.

(This may be somehow related to Bug 685204, except that 1. this bug does not appear to be related to AS3 (since the outcome is the same for both AS3:: and public:: qualified unshift invocations, and 2. it is a little hard for me to believe that this was not injected by the dense array rewrite.  we will see.)

Comment 1

[Felix S. Klock II [:pnkfelix, :fklock]]

This appears to be an injection.

At TR rev 5711 (immediately prior to dense-array rewrite), 32-bit DebugDebugger build, the output is:

  pre  a's: [,10] [,10]
  pre  b's: [,10] [,10]
  post a's: [1,,10] [1,,10]
  post b's: [1,,10] [1,,10]

So this indicates that we have injected a different behavior for dense arrays (that's what the b's are) with regards to unshift.

(Also, it is the same for AS3:: and for public:: qualified arrays because for single argument unshift, the generic public::unshift just delegates to the unshift method on the ArrayObject which in turns delegates to AS3_unshift.  I suspect Bug 685204 is a different issue that is exposing a semantic difference by passing multiple arguments during a single unshift invcation.)

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: patch A v1: denseStart is 0 after prefix-filling with atomNotFounds

(Still need to send through acceptance tests, but seems like a no-brainer.)

QRB: Where is this fix targeted, Anza or Brannan?

Comment 3

[Lars T Hansen]

Sweet test case BTW.

Comment 4

[Tamarin Bot]

changeset: 6659:0e53174609f3
user:      Felix S Klock II <[e-mail]>
summary:   Bug 694884: denseStart is 0 after prefix-filling with atomNotFounds (r=lhansen).

http://hg.mozilla.org/tamarin-redux/rev/0e53174609f3

Comment 5

[Felix S. Klock II [:pnkfelix, :fklock]]

(whoops, reopening; still want to know if this should be targeted for Anza or for Brannan.)

Comment 6

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to Felix S Klock II from comment #5)
> (whoops, reopening; still want to know if this should be targeted for Anza
> or for Brannan.)

that question is irrelevant at this point; so I can close this now.