Reviewers: Michael Starzinger,
Message:
Feel free to tell me this SparseMove() is getting too hacky. This is mostly
an
attempt to see what it would take to be spec-compliant.
Description:
Correctly handle Array unshift/splices that move elements past the max
length of
an Array
BUG=v8:2615
LOG=n
Please review this at https://codereview.chromium.org/679113003/
Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+17, -41 lines):
M src/array.js
D test/mjsunit/bugs/bug-2615.js
A + test/mjsunit/regress/regress-splice-large-index.js
Index: test/mjsunit/bugs/bug-2615.js
diff --git a/test/mjsunit/bugs/bug-2615.js b/test/mjsunit/bugs/bug-2615.js
deleted file mode 100644
index
5da17eecf1fa57699af86521adefc3fbe62a262f..0000000000000000000000000000000000000000
--- a/test/mjsunit/bugs/bug-2615.js
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2013 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-// * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-// * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following
-// disclaimer in the documentation and/or other materials provided
-// with the distribution.
-// * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived
-// from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-var a = [];
-a[0xfffffffe] = 10;
-assertThrows("a.unshift(1);", RangeError);
-assertEquals(0xffffffff, a.length);
-assertEquals(10, a[0xffffffff]);
-assertEquals(undefined, a[0xfffffffe]);
-
-a = [1,2,3];
-a[0xfffffffe] = 10;
-assertThrows("a.splice(1,1,7,7,7,7,7);", RangeError);
-assertEquals([1,7,7,7,7,7,3], a.slice(0, 7));
-assertEquals(0xffffffff, a.length);
-assertEquals(10, a[0xfffffffe + 5 - 1]);
Index: src/array.js
diff --git a/src/array.js b/src/array.js
index
29fa8318e2bfee8c8cfa4ba95af4da5bd777590e..ff14b3583f2ba5ee08b2f37015eff889a642a53e
100644
--- a/src/array.js
+++ b/src/array.js
@@ -238,7 +238,10 @@ function SparseMove(array, start_i, del_count, len,
num_additional_args) {
// Bail out if no moving is necessary.
if (num_additional_args === del_count) return;
// Move data to new array.
- var new_array = new InternalArray(len - del_count + num_additional_args);
+ var new_array = new InternalArray(
+ // Clamp array length to 2^32-1 to avoid early RangeError.
+ MathMin(len - del_count + num_additional_args, 0xffffffff));
+ var big_indices;
var indices = %GetArrayKeys(array, len);
if (IS_NUMBER(indices)) {
var limit = indices;
@@ -267,7 +270,12 @@ function SparseMove(array, start_i, del_count, len,
num_additional_args) {
} else if (key >= start_i + del_count) {
var current = array[key];
if (!IS_UNDEFINED(current) || key in array) {
- new_array[key - del_count + num_additional_args] = current;
+ var new_key = key - del_count + num_additional_args;
+ new_array[new_key] = current;
+ if (new_key > 0xffffffff) {
+ big_indices = big_indices || new InternalArray();
+ big_indices.push(new_key);
+ }
}
}
}
@@ -275,6 +283,14 @@ function SparseMove(array, start_i, del_count, len,
num_additional_args) {
}
// Move contents of new_array into this array
%MoveArrayContents(new_array, array);
+ // Add any moved values that aren't elements anymore.
+ if (!IS_UNDEFINED(big_indices)) {
+ var length = big_indices.length;
+ for (var i = 0; i < length; ++i) {
+ var key = big_indices[i];
+ array[key] = new_array[key];
+ }
+ }
}
Index: test/mjsunit/regress/regress-splice-large-index.js
diff --git a/test/mjsunit/bugs/bug-2615.js
b/test/mjsunit/regress/regress-splice-large-index.js
similarity index 100%
rename from test/mjsunit/bugs/bug-2615.js
rename to test/mjsunit/regress/regress-splice-large-index.js
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
