Commit 700bbdc6 authored by littledan's avatar littledan Committed by Commit bot

Avoid calling %AddElement with a number out of array index range

This patch wraps callsites to %AddElement to fall back to adding a
named property in case it is given an argument of 2**32 or greater.
The change is needed because %AddElement is called by Array functions
in various places, and ES2015 changes these Array functions to use
ToLength rather than ToUint32, so several callsites of %AddElement
which used to be reliable array indices may be larger numbers. While
the proper long-term solution may be to call out to
Object.defineProperty, this fix should allow the ToLength semantics
to be shipped while preserving correctness and not requiring a
rewrite.

BUG=v8:4516
LOG=Y
R=adamk
TEST=Interactively ran Array.prototype.slice on an Array-like which
exceeded array bounds, and found that this did not check-fail at
runtime as it did before.
Microbenchmarked this technique against the previous version on a
simple reverse implementation and found at most a 1% slowdown, as
opposed to other techniques, like calling %DefineDataPropertyUnchecked,
which had a 20% slowdown or Object.defineProperty with a 80% slowdown.

Review URL: https://codereview.chromium.org/1420663003

Cr-Commit-Position: refs/heads/master@{#31640}
parent 1243ce0b
......@@ -11,6 +11,7 @@
// -------------------------------------------------------------------
// Imports
var AddIndexedProperty;
var Delete;
var FLAG_harmony_tolength;
var GlobalArray = global.Array;
......@@ -28,6 +29,7 @@ var ObserveEnqueueSpliceRecord;
var unscopablesSymbol = utils.ImportNow("unscopables_symbol");
utils.Import(function(from) {
AddIndexedProperty = from.AddIndexedProperty;
Delete = from.Delete;
MakeTypeError = from.MakeTypeError;
MinSimple = from.MinSimple;
......@@ -245,7 +247,7 @@ function SparseSlice(array, start_i, del_count, len, deleted_elements) {
for (var i = start_i; i < limit; ++i) {
var current = array[i];
if (!IS_UNDEFINED(current) || i in array) {
%AddElement(deleted_elements, i - start_i, current);
AddIndexedProperty(deleted_elements, i - start_i, current);
}
}
} else {
......@@ -256,7 +258,7 @@ function SparseSlice(array, start_i, del_count, len, deleted_elements) {
if (key >= start_i) {
var current = array[key];
if (!IS_UNDEFINED(current) || key in array) {
%AddElement(deleted_elements, key - start_i, current);
AddIndexedProperty(deleted_elements, key - start_i, current);
}
}
}
......@@ -336,9 +338,9 @@ function SimpleSlice(array, start_i, del_count, len, deleted_elements) {
var index = start_i + i;
if (HAS_INDEX(array, index, is_array)) {
var current = array[index];
// The spec requires [[DefineOwnProperty]] here, %AddElement is close
// enough (in that it ignores the prototype).
%AddElement(deleted_elements, i, current);
// The spec requires [[DefineOwnProperty]] here, AddIndexedProperty is
// close enough (in that it ignores the prototype).
AddIndexedProperty(deleted_elements, i, current);
}
}
}
......
......@@ -11,6 +11,7 @@
// -------------------------------------------------------------------
// Imports
var AddIndexedProperty;
var FLAG_harmony_tolength;
var GetIterator;
var GetMethod;
......@@ -23,6 +24,7 @@ var ObjectIsFrozen;
var ObjectDefineProperty;
utils.Import(function(from) {
AddIndexedProperty = from.AddIndexedProperty;
FLAG_harmony_tolength = from.FLAG_harmony_tolength;
GetIterator = from.GetIterator;
GetMethod = from.GetMethod;
......@@ -182,7 +184,7 @@ function ArrayFill(value, start, end) {
function AddArrayElement(constructor, array, i, value) {
if (constructor === GlobalArray) {
%AddElement(array, i, value);
AddIndexedProperty(array, i, value);
} else {
ObjectDefineProperty(array, i, {
value: value, writable: true, configurable: true, enumerable: true
......
......@@ -194,7 +194,7 @@ function SameValueZero(x, y) {
function ConcatIterableToArray(target, iterable) {
var index = target.length;
for (var element of iterable) {
%AddElement(target, index++, element);
AddIndexedProperty(target, index++, element);
}
return target;
}
......@@ -206,6 +206,19 @@ function ConcatIterableToArray(target, iterable) {
*/
// This function should be called rather than %AddElement in contexts where the
// argument might not be less than 2**32-1. ES2015 ToLength semantics mean that
// this is a concern at basically all callsites.
function AddIndexedProperty(obj, index, value) {
if (index === TO_UINT32(index)) {
%AddElement(obj, index, value);
} else {
%AddNamedProperty(obj, TO_STRING(index), value, NONE);
}
}
%SetForceInlineFlag(AddIndexedProperty);
// ES6, draft 10-14-14, section 22.1.3.1.1
function IsConcatSpreadable(O) {
if (!IS_SPEC_OBJECT(O)) return false;
......@@ -248,6 +261,7 @@ function MinSimple(a, b) {
// Exports
utils.Export(function(to) {
to.AddIndexedProperty = AddIndexedProperty;
to.MaxSimple = MaxSimple;
to.MinSimple = MinSimple;
to.SameValue = SameValue;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment