697745 - ArrayObject: denseStart needs to be zeroed when denseArray emptied.

Status: RESOLVED FIXED

(Tamarin Graveyard :: Library, defect)

Description

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

Code:

  var a = new Array();
  a[1] = 2;
  print(a);
  a.splice(1,1);
  print(a);

If you turn on the current internal Array checking code via this define in ArrayObject.h:
    #define DEBUG_ARRAY_VERIFY

then you get this in debug build of the shell:

,2
Assertion failed: "((m_denseStart == 0))" ("../core/ArrayObject.cpp":189)
Trace/BPT trap

At first I thought this meant that we had gotten our reasoning wrong in Bug 692534, but that's not the real issue.

The real issue is that the internal verify check is asserting that if we have an empty m_denseArray, then m_denseStart must be 0.  That means that at each point in the code where m_denseArray can become empty (e.g. splice), we must check for that and set m_denseStart accordingly.  Well, that, or loosen up the verify check, assuming we double-check that the code is not relying on that strong an invariant.  Both options involve some amont of effort; the first is probably easier to get right.

Comment 1

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

Attachment: patch A v1: fix

(Its a tiny patch, but I know you (Lars) are swamped.  feel free to suggest reassignment.)

Comment 2

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

Attachment: corner cases addressed by patch A

This code illustrates the two corner cases patch A is fixing.

(They both independently assert fail without the two respective fixes in patch A, assuming one has turned on the internal debug checks as described in comment 0.)

Comment 3

[Lars T Hansen]

Comment on attachment 570025 [details] [diff] [review]
patch A v1: fix

OK.  I looked for other cases where this was missing but found that they correctly set denseStart to zero, so my gut feeling is that you've fixed a few stragglers, not an endemic problem.

Comment 4

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

changeset:   6689:db5b02e60575
user:        Felix S Klock II <[e-mail]>
date:        Mon Oct 31 12:19:20 2011 +0100
summary:     Bug 697745: zero denseStart when denseArray emptied (r=lhansen).

http://hg.mozilla.org/tamarin-redux/rev/db5b02e60575