Commit b959ece4 authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

[array] Enable copying from the prototype chain when sorting JSArrays

This CL enables the pre-processing step of copying from the
prototype chain for JSArrays. Previously, this was done for everything
BUT JSArrays. This brings Array#sort more in line with other engines
in the case of undefined behavior.

R=jgruber@chromium.org

Bug: v8:8666
Change-Id: I832d470dc02111b64dc4919e84e7e3e47c8fdd47
Reviewed-on: https://chromium-review.googlesource.com/c/1426119
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarMathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58999}
parent f25656ae
......@@ -393,10 +393,10 @@ RUNTIME_FUNCTION(Runtime_PrepareElementsForSort) {
// Counter for sorting arrays that have non-packed elements and where either
// the ElementsProtector is invalid or the prototype does not match
// Array.prototype.
JSObject initial_array_proto = JSObject::cast(
isolate->native_context()->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX));
if (object->IsJSArray() &&
!Handle<JSArray>::cast(object)->HasFastPackedElements()) {
JSObject initial_array_proto = JSObject::cast(
isolate->native_context()->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX));
if (!isolate->IsNoElementsProtectorIntact() ||
object->map()->prototype() != initial_array_proto) {
isolate->CountUsage(
......@@ -404,7 +404,10 @@ RUNTIME_FUNCTION(Runtime_PrepareElementsForSort) {
}
}
if (!object->IsJSArray()) {
// Skip copying from prototype for JSArrays with ElementsProtector intact and
// the original array prototype.
if (!object->IsJSArray() || !isolate->IsNoElementsProtectorIntact() ||
object->map()->prototype() != initial_array_proto) {
RETURN_FAILURE_ON_EXCEPTION(isolate,
CopyFromPrototype(isolate, object, length));
}
......
......@@ -567,6 +567,27 @@ function TestPrototypeHoles() {
}
TestPrototypeHoles();
// The following test ensures that elements on the prototype are also copied
// for JSArrays and not only JSObjects.
function TestArrayPrototypeHasElements() {
let array = [1, 2, 3, 4, 5];
for (let i = 0; i < array.length; i++) {
delete array[i];
Object.prototype[i] = 42;
}
let comparator_called = false;
array.sort(function (a, b) {
if (a === 42 || b === 42) {
comparator_called = true;
}
return a - b;
});
assertTrue(comparator_called);
}
TestArrayPrototypeHasElements();
// The following Tests make sure that there is no crash when the element kind
// or the array length changes. Since comparison functions like this are not
// consistent, we do not have to make sure that the array is actually sorted
......
......@@ -59,7 +59,7 @@ PASS showHoles([0, , 2].concat([3, , 5])) is '[0, peekaboo, 2, 3, peekaboo, 5]'
PASS showHoles([0, , 2, 3].reverse()) is '[3, 2, peekaboo, 0]'
PASS a = [0, , 2, 3]; a.shift(); showHoles(a) is '[peekaboo, 2, 3]'
PASS showHoles([0, , 2, 3].slice(0, 3)) is '[0, peekaboo, 2]'
PASS showHoles([0, , 2, 3].sort()) is '[0, 2, 3, hole]'
PASS showHoles([0, , 2, 3].sort()) is '[0, 2, 3, peekaboo]'
PASS showHoles([0, undefined, 2, 3].sort()) is '[0, 2, 3, undefined]'
PASS a = [0, , 2, 3]; a.splice(2, 3, 5, 6); showHoles(a) is '[0, hole, 5, 6]'
PASS a = [0, , 2, 3]; a.unshift(4); showHoles(a) is '[4, 0, peekaboo, 2, 3]'
......
......@@ -110,7 +110,7 @@ shouldBe("showHoles([0, , 2].concat([3, , 5]))", "'[0, peekaboo, 2, 3, peekaboo,
shouldBe("showHoles([0, , 2, 3].reverse())", "'[3, 2, peekaboo, 0]'");
shouldBe("a = [0, , 2, 3]; a.shift(); showHoles(a)", "'[peekaboo, 2, 3]'");
shouldBe("showHoles([0, , 2, 3].slice(0, 3))", "'[0, peekaboo, 2]'");
shouldBe("showHoles([0, , 2, 3].sort())", "'[0, 2, 3, hole]'");
shouldBe("showHoles([0, , 2, 3].sort())", "'[0, 2, 3, peekaboo]'");
shouldBe("showHoles([0, undefined, 2, 3].sort())", "'[0, 2, 3, undefined]'");
shouldBe("a = [0, , 2, 3]; a.splice(2, 3, 5, 6); showHoles(a)", "'[0, hole, 5, 6]'");
shouldBe("a = [0, , 2, 3]; a.unshift(4); showHoles(a)", "'[4, 0, peekaboo, 2, 3]'");
......
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