Adding checks for the cases when array grows too big.

Adding checks for the cases when array grows too big.

Committed: http://code.google.com/p/v8/source/detail?r=3887

[antonm, 2010-02-17 12:04:06]

Mads,

here promised CL.

Technically that fixes a bug in ArrayPush (then you have an array in fast
element mode and try to add some elements so that new length doesn't fit
Smi...), but I doubt we ever see this in practice.  Still probably needs a
treatment.

I hope it have negligible perf impact, but would double check later.

http://codereview.chromium.org/601092/diff/1/2
File src/builtins.cc (right):

http://codereview.chromium.org/601092/diff/1/2#newcode624
src/builtins.cc:624: if ((itemCount - actualDeleteCount) > (Smi::kMaxValue -
len)) {
calling that late, after result array has been created and populated might be a
bad idea, but this case hopefully shouldn't be hit often

http://codereview.chromium.org/601092/diff/1/3
File test/mjsunit/array-splice.js (right):

http://codereview.chromium.org/601092/diff/1/3#newcode283
test/mjsunit/array-splice.js:283: // Check smi boundary
Technically we should have grown array to this boundary keeping it in fast
elements mode, but, alas, that takes too long.

[antonm, 2010-02-17 12:06:39]

Resending---first time I forgot to add you as a reviewer, sorry.

[Mads Ager (chromium), 2010-02-17 12:35:49]

As discussed offline, we don't actually need the checks because we cannot
allocate a fixed array that is so large that you will get a fast case elements
backing store with a length that does not fit in a smi.  You could add ASSERTS
to be safe.

Keeping the tests still seems like a good idea.

http://codereview.chromium.org/601092/diff/1/3
File test/mjsunit/array-splice.js (right):

http://codereview.chromium.org/601092/diff/1/3#newcode289
test/mjsunit/array-splice.js:289: if (false)  // Disabled, takes too long to
execute.
Please remove it all then.

http://codereview.chromium.org/601092/diff/1/4
File test/mjsunit/array-unshift.js (right):

http://codereview.chromium.org/601092/diff/1/4#newcode132
test/mjsunit/array-unshift.js:132: if (false)  // Disabled, takes too long to
execute.
Please remove it all then.

[antonm, 2010-02-17 12:50:15]

Thanks a lot, Mads.

http://codereview.chromium.org/601092/diff/1/3
File test/mjsunit/array-splice.js (right):

http://codereview.chromium.org/601092/diff/1/3#newcode289
test/mjsunit/array-splice.js:289: if (false)  // Disabled, takes too long to
execute.
On 2010/02/17 12:35:49, Mads Ager wrote:
> Please remove it all then.

Done.

http://codereview.chromium.org/601092/diff/1/4
File test/mjsunit/array-unshift.js (right):

http://codereview.chromium.org/601092/diff/1/4#newcode132
test/mjsunit/array-unshift.js:132: if (false)  // Disabled, takes too long to
execute.
On 2010/02/17 12:35:49, Mads Ager wrote:
> Please remove it all then.

Done.

[Mads Ager (chromium), 2010-02-17 13:02:01]

LGTM, thanks.