Throw TypeError if a proxy's [[OwnPropertyKeys]] returns dupes

Adding implementation for step 9 which is missing for spec:
https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys
Update bytecode_expectations as well.

Bug v8:6776

Change-Id: Id191f9604e2dc08e71cbcff8ebd5707c233af193
Reviewed-on: https://chromium-review.googlesource.com/c/1419779
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#59180}
This commit is contained in:
Z Duong Nguyen-Huu 2019-01-28 10:53:22 -08:00 committed by Commit Bot
parent 026ce2c105
commit 0cabc54666
13 changed files with 110 additions and 149 deletions

View File

@ -801,7 +801,7 @@ class NameComparator {
} // namespace
// ES6 9.5.12
// ES6 #sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys
// Returns |true| on success, |nothing| in case of exception.
Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
Handle<JSProxy> proxy) {
@ -843,51 +843,9 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
Object::CreateListFromArrayLike(isolate_, trap_result_array,
ElementTypes::kStringAndSymbol),
Nothing<bool>());
// 9. Let extensibleTarget be ? IsExtensible(target).
Maybe<bool> maybe_extensible = JSReceiver::IsExtensible(target);
MAYBE_RETURN(maybe_extensible, Nothing<bool>());
bool extensible_target = maybe_extensible.FromJust();
// 10. Let targetKeys be ? target.[[OwnPropertyKeys]]().
Handle<FixedArray> target_keys;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate_, target_keys,
JSReceiver::OwnPropertyKeys(target),
Nothing<bool>());
// 11. (Assert)
// 12. Let targetConfigurableKeys be an empty List.
// To save memory, we're re-using target_keys and will modify it in-place.
Handle<FixedArray> target_configurable_keys = target_keys;
// 13. Let targetNonconfigurableKeys be an empty List.
Handle<FixedArray> target_nonconfigurable_keys =
isolate_->factory()->NewFixedArray(target_keys->length());
int nonconfigurable_keys_length = 0;
// 14. Repeat, for each element key of targetKeys:
for (int i = 0; i < target_keys->length(); ++i) {
// 14a. Let desc be ? target.[[GetOwnProperty]](key).
PropertyDescriptor desc;
Maybe<bool> found = JSReceiver::GetOwnPropertyDescriptor(
isolate_, target, handle(target_keys->get(i), isolate_), &desc);
MAYBE_RETURN(found, Nothing<bool>());
// 14b. If desc is not undefined and desc.[[Configurable]] is false, then
if (found.FromJust() && !desc.configurable()) {
// 14b i. Append key as an element of targetNonconfigurableKeys.
target_nonconfigurable_keys->set(nonconfigurable_keys_length,
target_keys->get(i));
nonconfigurable_keys_length++;
// The key was moved, null it out in the original list.
target_keys->set(i, Smi::kZero);
} else {
// 14c. Else,
// 14c i. Append key as an element of targetConfigurableKeys.
// (No-op, just keep it in |target_keys|.)
}
}
// 15. If extensibleTarget is true and targetNonconfigurableKeys is empty,
// then:
if (extensible_target && nonconfigurable_keys_length == 0) {
// 15a. Return trapResult.
return AddKeysFromJSProxy(proxy, trap_result);
}
// 16. Let uncheckedResultKeys be a new List which is a copy of trapResult.
// 9. If trapResult contains any duplicate entries, throw a TypeError
// exception. Combine with step 18
// 18. Let uncheckedResultKeys be a new List which is a copy of trapResult.
Zone set_zone(isolate_->allocator(), ZONE_NAME);
ZoneAllocationPolicy alloc(&set_zone);
const int kPresent = 1;
@ -903,32 +861,61 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
if (entry->value != kPresent) {
entry->value = kPresent;
unchecked_result_keys_size++;
}
}
// 17. Repeat, for each key that is an element of targetNonconfigurableKeys:
for (int i = 0; i < nonconfigurable_keys_length; ++i) {
Object raw_key = target_nonconfigurable_keys->get(i);
Handle<Name> key(Name::cast(raw_key), isolate_);
// 17a. If key is not an element of uncheckedResultKeys, throw a
// TypeError exception.
auto found = unchecked_result_keys.Lookup(key, key->Hash());
if (found == nullptr || found->value == kGone) {
} else {
// found dupes, throw exception
isolate_->Throw(*isolate_->factory()->NewTypeError(
MessageTemplate::kProxyOwnKeysMissing, key));
MessageTemplate::kProxyOwnKeysDuplicateEntries));
return Nothing<bool>();
}
// 17b. Remove key from uncheckedResultKeys.
found->value = kGone;
unchecked_result_keys_size--;
}
// 18. If extensibleTarget is true, return trapResult.
if (extensible_target) {
// 10. Let extensibleTarget be ? IsExtensible(target).
Maybe<bool> maybe_extensible = JSReceiver::IsExtensible(target);
MAYBE_RETURN(maybe_extensible, Nothing<bool>());
bool extensible_target = maybe_extensible.FromJust();
// 11. Let targetKeys be ? target.[[OwnPropertyKeys]]().
Handle<FixedArray> target_keys;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate_, target_keys,
JSReceiver::OwnPropertyKeys(target),
Nothing<bool>());
// 12, 13. (Assert)
// 14. Let targetConfigurableKeys be an empty List.
// To save memory, we're re-using target_keys and will modify it in-place.
Handle<FixedArray> target_configurable_keys = target_keys;
// 15. Let targetNonconfigurableKeys be an empty List.
Handle<FixedArray> target_nonconfigurable_keys =
isolate_->factory()->NewFixedArray(target_keys->length());
int nonconfigurable_keys_length = 0;
// 16. Repeat, for each element key of targetKeys:
for (int i = 0; i < target_keys->length(); ++i) {
// 16a. Let desc be ? target.[[GetOwnProperty]](key).
PropertyDescriptor desc;
Maybe<bool> found = JSReceiver::GetOwnPropertyDescriptor(
isolate_, target, handle(target_keys->get(i), isolate_), &desc);
MAYBE_RETURN(found, Nothing<bool>());
// 16b. If desc is not undefined and desc.[[Configurable]] is false, then
if (found.FromJust() && !desc.configurable()) {
// 16b i. Append key as an element of targetNonconfigurableKeys.
target_nonconfigurable_keys->set(nonconfigurable_keys_length,
target_keys->get(i));
nonconfigurable_keys_length++;
// The key was moved, null it out in the original list.
target_keys->set(i, Smi::kZero);
} else {
// 16c. Else,
// 16c i. Append key as an element of targetConfigurableKeys.
// (No-op, just keep it in |target_keys|.)
}
}
// 17. If extensibleTarget is true and targetNonconfigurableKeys is empty,
// then:
if (extensible_target && nonconfigurable_keys_length == 0) {
// 17a. Return trapResult.
return AddKeysFromJSProxy(proxy, trap_result);
}
// 19. Repeat, for each key that is an element of targetConfigurableKeys:
for (int i = 0; i < target_configurable_keys->length(); ++i) {
Object raw_key = target_configurable_keys->get(i);
if (raw_key->IsSmi()) continue; // Zapped entry, was nonconfigurable.
// 18. (Done in step 9)
// 19. Repeat, for each key that is an element of targetNonconfigurableKeys:
for (int i = 0; i < nonconfigurable_keys_length; ++i) {
Object raw_key = target_nonconfigurable_keys->get(i);
Handle<Name> key(Name::cast(raw_key), isolate_);
// 19a. If key is not an element of uncheckedResultKeys, throw a
// TypeError exception.
@ -942,14 +929,35 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
found->value = kGone;
unchecked_result_keys_size--;
}
// 20. If uncheckedResultKeys is not empty, throw a TypeError exception.
// 20. If extensibleTarget is true, return trapResult.
if (extensible_target) {
return AddKeysFromJSProxy(proxy, trap_result);
}
// 21. Repeat, for each key that is an element of targetConfigurableKeys:
for (int i = 0; i < target_configurable_keys->length(); ++i) {
Object raw_key = target_configurable_keys->get(i);
if (raw_key->IsSmi()) continue; // Zapped entry, was nonconfigurable.
Handle<Name> key(Name::cast(raw_key), isolate_);
// 21a. If key is not an element of uncheckedResultKeys, throw a
// TypeError exception.
auto found = unchecked_result_keys.Lookup(key, key->Hash());
if (found == nullptr || found->value == kGone) {
isolate_->Throw(*isolate_->factory()->NewTypeError(
MessageTemplate::kProxyOwnKeysMissing, key));
return Nothing<bool>();
}
// 21b. Remove key from uncheckedResultKeys.
found->value = kGone;
unchecked_result_keys_size--;
}
// 22. If uncheckedResultKeys is not empty, throw a TypeError exception.
if (unchecked_result_keys_size != 0) {
DCHECK_GT(unchecked_result_keys_size, 0);
isolate_->Throw(*isolate_->factory()->NewTypeError(
MessageTemplate::kProxyOwnKeysNonExtensible));
return Nothing<bool>();
}
// 21. Return trapResult.
// 23. Return trapResult.
return AddKeysFromJSProxy(proxy, trap_result);
}

View File

@ -228,6 +228,8 @@ namespace internal {
T(ProxyOwnKeysNonExtensible, \
"'ownKeys' on proxy: trap returned extra keys but proxy target is " \
"non-extensible") \
T(ProxyOwnKeysDuplicateEntries, \
"'ownKeys' on proxy: trap returned duplicate entries") \
T(ProxyPreventExtensionsExtensible, \
"'preventExtensions' on proxy: trap returned truish but the proxy target " \
"is extensible") \

View File

@ -336,7 +336,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(18),
B(LdaConstant), U8(13),
B(Star), R(19),

View File

@ -66,7 +66,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(13),
B(LdaConstant), U8(6),
B(Star), R(14),
@ -204,7 +204,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(14),
B(LdaConstant), U8(6),
B(Star), R(15),
@ -328,7 +328,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(15),
B(LdaConstant), U8(7),
B(Star), R(16),

View File

@ -94,7 +94,7 @@ bytecodes: [
B(JumpIfNull), U8(86),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(17),
B(LdaConstant), U8(9),
B(Star), R(18),
@ -266,7 +266,7 @@ bytecodes: [
B(JumpIfNull), U8(86),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(17),
B(LdaConstant), U8(9),
B(Star), R(18),
@ -454,7 +454,7 @@ bytecodes: [
B(JumpIfNull), U8(86),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(17),
B(LdaConstant), U8(9),
B(Star), R(18),
@ -609,7 +609,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(14),
B(LdaConstant), U8(8),
B(Star), R(15),

View File

@ -62,7 +62,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(12),
B(LdaConstant), U8(6),
B(Star), R(13),
@ -165,7 +165,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(13),
B(LdaConstant), U8(6),
B(Star), R(14),
@ -278,7 +278,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(12),
B(LdaConstant), U8(6),
B(Star), R(13),
@ -384,7 +384,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(12),
B(LdaConstant), U8(8),
B(Star), R(13),

View File

@ -66,7 +66,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(14),
B(LdaConstant), U8(5),
B(Star), R(15),
@ -203,7 +203,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(15),
B(LdaConstant), U8(10),
B(Star), R(16),
@ -318,7 +318,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(13),
B(LdaConstant), U8(7),
B(Star), R(14),
@ -430,7 +430,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(16),
B(LdaConstant), U8(7),
B(Star), R(17),
@ -547,7 +547,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(15),
B(LdaConstant), U8(8),
B(Star), R(16),
@ -679,7 +679,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(14),
B(LdaConstant), U8(11),
B(Star), R(15),
@ -795,7 +795,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(16),
B(LdaConstant), U8(5),
B(Star), R(17),
@ -938,7 +938,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(15),
B(LdaConstant), U8(6),
B(Star), R(16),

View File

@ -180,7 +180,7 @@ bytecodes: [
B(JumpIfNull), U8(50),
B(TestTypeOf), U8(6),
B(JumpIfTrue), U8(18),
B(Wide), B(LdaSmi), I16(154),
B(Wide), B(LdaSmi), I16(155),
B(Star), R(14),
B(LdaConstant), U8(12),
B(Star), R(15),

View File

@ -54,9 +54,9 @@ assertEquals(["a", "b", "c"], Reflect.ownKeys(proxy));
keys.length = Math.pow(2, 33);
assertThrows("Reflect.ownKeys(proxy)", RangeError);
// Check that we allow duplicated keys.
// Check that we don't allow duplicated keys.
keys = ['a', 'a', 'a']
assertEquals(keys, Reflect.ownKeys(proxy));
assertThrows("Reflect.ownKeys(proxy)", TypeError);
// Non-Name results throw.
keys = [1];
@ -75,9 +75,9 @@ assertThrows("Reflect.ownKeys(proxy)", TypeError);
keys = ["nonconf"];
assertEquals(keys, Reflect.ownKeys(proxy));
// Check that we allow duplicated keys.
// Check that we don't allow duplicated keys.
keys = ['nonconf', 'nonconf', 'nonconf']
assertEquals(keys, Reflect.ownKeys(proxy));
assertThrows("Reflect.ownKeys(proxy)", TypeError);
// Step 19a: The trap result must all keys of a non-extensible target.
Object.preventExtensions(target);
@ -89,6 +89,6 @@ assertEquals(keys, Reflect.ownKeys(proxy));
keys = ["nonconf", "target_one", "fantasy"];
assertThrows("Reflect.ownKeys(proxy)", TypeError);
// Check that we allow duplicated keys.
// Check that we don't allow duplicated keys.
keys = ['nonconf', 'target_one', 'nonconf', 'nonconf', 'target_one',]
assertEquals(keys, Reflect.ownKeys(proxy));
assertThrows("Reflect.ownKeys(proxy)", TypeError);

View File

@ -144,29 +144,11 @@ function TestOrderWithDuplicates(withWarmup) {
});
if (withWarmup) {
for (const key in P) {}
for (const key in O) {};
try { for (const key in P) {} } catch {};
}
log = [];
assertEquals([
["a", 1],
["a", 1],
["456", 123],
["456", 123]
], Object.entries(P));
assertEquals([
"[[OwnPropertyKeys]]",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")"
], log);
assertThrows(() => Object.entries(P), TypeError);
}
TestOrderWithDuplicates();
TestOrderWithDuplicates(true);

View File

@ -193,21 +193,7 @@ function TestDuplicateKeys() {
defineProperty(target, name, desc) { assertUnreachable(); }
});
var result = Object.getOwnPropertyDescriptors(P);
assertEquals({
"A": {
"value": "VALUE",
"writable": false,
"enumerable": false,
"configurable": true
}
}, result);
assertTrue(result.hasOwnProperty("A"));
assertEquals([
"ownKeys()",
"getOwnPropertyDescriptor(A)",
"getOwnPropertyDescriptor(A)"
], log);
assertThrows(() => Object.getOwnPropertyDescriptors(P), TypeError);
}
TestDuplicateKeys();

View File

@ -121,20 +121,7 @@ function TestOrderWithDuplicates() {
}
});
assertEquals([1, 1, 123, 123], Object.values(P));
assertEquals([
"[[OwnPropertyKeys]]",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
], log);
assertThrows(() => Object.values(P), TypeError);
}
TestOrderWithDuplicates();

View File

@ -483,10 +483,6 @@
'built-ins/Object/internals/DefineOwnProperty/consistent-value-function-caller': [FAIL_SLOPPY],
'built-ins/Object/internals/DefineOwnProperty/consistent-value-function-arguments': [FAIL_SLOPPY],
# https://bugs.chromium.org/p/v8/issues/detail?id=6776
'built-ins/Proxy/ownKeys/return-duplicate-entries-throws': [FAIL],
'built-ins/Proxy/ownKeys/return-duplicate-symbol-entries-throws': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=7184
'annexB/language/expressions/yield/star-iterable-return-emulates-undefined-throws-when-called': [FAIL],
'annexB/language/statements/for-await-of/iterator-close-return-emulates-undefined-throws-when-called': [FAIL],