Commit 5138e02a authored by Hai Dang's avatar Hai Dang Committed by Commit Bot

Fix Array lastIndexOf to call [[HasProperty]] before [[Get]]

Also add more test cases of Array lastIndexOf with proxy, inspired by test262.

In the path for sparse arrays, no changes are needed because element accesses
are not observable there (thanks to UseSparseVariant).

Bug: v8:7813
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ifd47149f654e92f56d0a1ed6b3debc93718702be
Reviewed-on: https://chromium-review.googlesource.com/1160307
Commit-Queue: Hai Dang <dhai@google.com>
Reviewed-by: 's avatarSathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54887}
parent 02b85b19
......@@ -855,12 +855,12 @@ DEFINE_METHOD_LEN(
// Lookup through the array.
if (!IS_UNDEFINED(element)) {
for (var i = max; i >= min; i--) {
if (array[i] === element) return i;
if (i in array && array[i] === element) return i;
}
return -1;
}
for (var i = max; i >= min; i--) {
if (IS_UNDEFINED(array[i]) && i in array) {
if (i in array && IS_UNDEFINED(array[i])) {
return i;
}
}
......
......@@ -8,3 +8,76 @@ assertThrows(() => {
assertThrows(() => {
Array.prototype.lastIndexOf.call(undefined, 42);
}, TypeError);
/* Tests inspired by test262's
lastIndexOf/calls-only-has-on-prototype-after-length-zeroed.js */
// Stateful fromIndex that tries to empty the array
(function testFromIndex() {
var array = [5, undefined, 7];
var fromIndex = {
valueOf: function() {
array.length = 1;
return 2;
}
};
assertEquals(-1, array.lastIndexOf(undefined, fromIndex));
array = [5, undefined, 7];
assertEquals(0, array.lastIndexOf(5, fromIndex));
})();
// Stateful fromIndex and proxy as Prototype
// Must test for [[HasProperty]] before [[Get]]
var testHasProperty = function(value) {
var array = [5, undefined, 7];
var fromIndex = {
valueOf: function() {
array.length = 0;
return 2;
}
};
// Install a prototype that only has [[HasProperty]], and throws on [[Get]]
Object.setPrototypeOf(array,
new Proxy(Array.prototype, {
has: function(t, pk) { return pk in t; },
get: function () { throw new Error('[[Get]] trap called') },
}));
assertEquals(-1, Array.prototype.lastIndexOf.call(array, value, fromIndex));
}
testHasProperty(5);
testHasProperty(undefined);
// Test call order: [[HasProperty]] before [[Get]]
var testHasPropertyThenGet = function(value) {
var array = [5, , 7];
var log = [];
// Install a prototype with only [[HasProperty]] and [[Get]]
Object.setPrototypeOf(array,
new Proxy(Array.prototype, {
has: function() { log.push("HasProperty"); return true; },
get: function() { log.push("Get"); },
}));
// The 2nd element (index 1) will trigger the calls to the prototype
Array.prototype.lastIndexOf.call(array, value);
assertEquals(["HasProperty", "Get"], log);
}
testHasPropertyThenGet(5);
testHasPropertyThenGet(undefined);
// Test for sparse Arrays
/* This will not enter the fast path for sparse arrays, due to UseSparseVariant
excluding array elements with accessors */
(function() {
var array = new Array(10000);
array[0] = 5; array[9999] = 7;
var count = 0;
Object.defineProperty(array.__proto__, 9998, { get: () => ++count });
Array.prototype.lastIndexOf.call(array, 0);
assertEquals(1,count);
})();
......@@ -478,9 +478,6 @@
# https://bugs.chromium.org/p/v8/issues/detail?id=7993
'intl402/RelativeTimeFormat/prototype/toStringTag/toStringTag': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=7813
'built-ins/Array/prototype/lastIndexOf/calls-only-has-on-prototype-after-length-zeroed': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=7814
'built-ins/Array/prototype/splice/property-traps-order-with-species': [FAIL],
......
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