614070 - Array.prototype.unshift does not set new length if argc == 0

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Test case:
---
var a = {};
a.length = 4294967296;
Array.prototype.unshift.call(a);
assertEq(a.length, 0);
---
Easy fix, patch tomorrow (don't have time to run tests atm)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this bug 612260, or different?

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Fix

Comment 3

[Jan de Mooij [:jandem]]

(In reply to comment #1)
> Is this bug 612260, or different?

No this is only an edge case in unshift.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 492735 [details] [diff] [review]
Fix

This seems generally reasonable, but I admit I found the test behavior a bit surprising, until I remembered the 32-bit cast in ToUint32 -- plus, your test neglects testing the other mode of failure.  Could you also add a test that checks for correct behavior using a getter and setter for the length property?  With that all should be good.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch v2

Good idea, added more tests using getters/setters.

Comment 6

[Jeff Walden [:Waldo]]

Comment on attachment 493118 [details] [diff] [review]
Patch v2

Excellent.

(I'm queueing up your patches for a push in a bit -- probably not today but most likely before Monday, or Monday at latest -- just FYI.)

Comment 7

[Jan de Mooij [:jandem]]

(In reply to comment #6)
> (I'm queueing up your patches for a push in a bit -- probably not today but
> most likely before Monday, or Monday at latest -- just FYI.)

Thanks, most of these bugs have gone unnoticed for years (spec nitpickery), so another week shouldn't hurt ;)

Comment 8

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/2000c21b7910

Oh, also, I noticed the |if (argc > 0)| guard on reassigning existing elements of this isn't in the spec -- sent mail to es5-discuss about it:

https://mail.mozilla.org/pipermail/es5-discuss/2010-November/003836.html

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/2000c21b7910