Bug 43401 – REGRESSION: calling unshift passing more than 1 argument causes array corruption

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Heather]

Created attachment 63295 [details]
array unshift test case

In the latest nightly calling unshift on an array and passing more than one argument causes the array to become corrupted after the 3rd call to unshift.

[Michael Saboff]

Created attachment 63406 [details]
Patch to fix initialization of array contents as part of unshift.

[Gavin Barraclough]

Comment on attachment 63406 [details]
Patch to fix initialization of array contents as part of unshift.

Looks fine but please remove tabs & add layout test as discussed.
cheers, G.

[Michael Saboff]

Created attachment 63467 [details]
Updated patch with fix for <tabs> in source and inclusion of new regression tests.

[Darin Adler]

Comment on attachment 63467 [details]
Updated patch with fix for <tabs> in source and inclusion of new regression tests.

Looks good. Nice test too.

> +    for (unsigned i = 0; i < (unsigned)count; i++)

We normally don't use C-style casts, so this should be a static_cast, or even better you could just use the same type for "i" as for "count" and avoid the cast entirely.

I’ll leave this on commit-queue? so you can decide whether to fix this. If you choose to upload a new patch, then I'll set commit-queue+ on that, or I can set commit-queue+ on this if you want to land it without changing the typecast above.

Not sure why you are using shouldEvaluateTo instead of shouldBe in the test. What's the difference?

The patch was uploaded with the wrong MIME type and without the "is a patch" check box set; I had to fix those before I could do this review.

[Michael Saboff]

Created attachment 63470 [details]
Updated patch with changes suggested in review of attachment 63467 [details]

[Eric Seidel (no email)]

Comment on attachment 63467 [details]
Updated patch with fix for <tabs> in source and inclusion of new regression tests.

Cleared Darin Adler's review+ from obsolete attachment 63467 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[WebKit Commit Bot]

Comment on attachment 63470 [details]
Updated patch with changes suggested in review of attachment 63467 [details]

Clearing flags on attachment: 63470

Committed r64773: <http://trac.webkit.org/changeset/64773>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.