From d485af5644ccf1f7fc1e34ede414f4299fc0c510 Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Mon, 28 Dec 2020 12:47:14 -0800 Subject: [PATCH] Make TypedArray elements configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This implements the spec change in https://github.com/tc39/ecma262/pull/2164 Making TA elements configurable has interaction with delete. While the elements are configurable, they are only "deletable" via detaching the underlying ArrayBuffer, not via `delete`. That is, `delete ta[idx]` for an in-bounds `idx` still returns false. Bug: v8:11281 Change-Id: I2e9348a7ec3c3239a92cc35e51b7182423736834 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2605234 Reviewed-by: Marja Hölttä Commit-Queue: Shu-yu Guo Cr-Commit-Position: refs/heads/master@{#71955} --- src/objects/elements.cc | 9 ++++++--- src/objects/js-array-buffer.cc | 6 +++--- src/objects/js-objects.cc | 7 +++++-- test/mjsunit/element-accessor.js | 2 +- test/mjsunit/es6/typedarray-detached.js | 2 +- test/mjsunit/es6/typedarray.js | 2 +- test/test262/test262.status | 5 ----- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/objects/elements.cc b/src/objects/elements.cc index cabc110adfb6..f9ba145cf40b 100644 --- a/src/objects/elements.cc +++ b/src/objects/elements.cc @@ -3066,12 +3066,12 @@ class TypedElementsAccessor } static PropertyDetails GetDetailsImpl(JSObject holder, InternalIndex entry) { - return PropertyDetails(kData, DONT_DELETE, PropertyCellType::kNoCell); + return PropertyDetails(kData, NONE, PropertyCellType::kNoCell); } static PropertyDetails GetDetailsImpl(FixedArrayBase backing_store, InternalIndex entry) { - return PropertyDetails(kData, DONT_DELETE, PropertyCellType::kNoCell); + return PropertyDetails(kData, NONE, PropertyCellType::kNoCell); } static bool HasElementImpl(Isolate* isolate, JSObject holder, size_t index, @@ -3092,7 +3092,10 @@ class TypedElementsAccessor } static void DeleteImpl(Handle obj, InternalIndex entry) { - UNREACHABLE(); + // Do nothing. + // + // TypedArray elements are configurable to explain detaching, but cannot be + // deleted otherwise. } static InternalIndex GetEntryForIndexImpl(Isolate* isolate, JSObject holder, diff --git a/src/objects/js-array-buffer.cc b/src/objects/js-array-buffer.cc index 9a7348bd56cb..074a8dc1bf28 100644 --- a/src/objects/js-array-buffer.cc +++ b/src/objects/js-array-buffer.cc @@ -227,12 +227,12 @@ Maybe JSTypedArray::DefineOwnProperty(Isolate* isolate, NewTypeError(MessageTemplate::kRedefineDisallowed, key)); } // 3b vii. If Desc has a [[Configurable]] field and if - // Desc.[[Configurable]] is true, return false. + // Desc.[[Configurable]] is false, return false. // 3b viii. If Desc has an [[Enumerable]] field and if Desc.[[Enumerable]] // is false, return false. // 3b ix. If Desc has a [[Writable]] field and if Desc.[[Writable]] is // false, return false. - if ((desc->has_configurable() && desc->configurable()) || + if ((desc->has_configurable() && !desc->configurable()) || (desc->has_enumerable() && !desc->enumerable()) || (desc->has_writable() && !desc->writable())) { RETURN_FAILURE(isolate, GetShouldThrow(isolate, should_throw), @@ -242,7 +242,7 @@ Maybe JSTypedArray::DefineOwnProperty(Isolate* isolate, // 3b x 1. Let value be Desc.[[Value]]. // 3b x 2. Return ? IntegerIndexedElementSet(O, numericIndex, value). if (desc->has_value()) { - if (!desc->has_configurable()) desc->set_configurable(false); + if (!desc->has_configurable()) desc->set_configurable(true); if (!desc->has_enumerable()) desc->set_enumerable(true); if (!desc->has_writable()) desc->set_writable(true); Handle value = desc->value(); diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc index 4ad61c2748e3..e150a1339c60 100644 --- a/src/objects/js-objects.cc +++ b/src/objects/js-objects.cc @@ -853,8 +853,11 @@ Maybe JSReceiver::DeleteProperty(LookupIterator* it, return Just(true); case LookupIterator::DATA: case LookupIterator::ACCESSOR: { - if (!it->IsConfigurable()) { - // Fail if the property is not configurable. + Handle holder = it->GetHolder(); + if (!it->IsConfigurable() || + (holder->IsJSTypedArray() && it->IsElement(*holder))) { + // Fail if the property is not configurable if the property is a + // TypedArray element. if (is_strict(language_mode)) { isolate->Throw(*isolate->factory()->NewTypeError( MessageTemplate::kStrictDeleteProperty, it->GetName(), diff --git a/test/mjsunit/element-accessor.js b/test/mjsunit/element-accessor.js index 8d412ed12ff6..0fa30f23e986 100644 --- a/test/mjsunit/element-accessor.js +++ b/test/mjsunit/element-accessor.js @@ -38,7 +38,7 @@ assertThrows( () => Object.defineProperty(o, '0', {get: function() {}}), TypeError); assertEquals( - {value: 0, writable: true, enumerable: true, configurable: false}, + {value: 0, writable: true, enumerable: true, configurable: true}, Object.getOwnPropertyDescriptor(o, '0')); })(); diff --git a/test/mjsunit/es6/typedarray-detached.js b/test/mjsunit/es6/typedarray-detached.js index 8148e206b34c..fc5421b8b130 100644 --- a/test/mjsunit/es6/typedarray-detached.js +++ b/test/mjsunit/es6/typedarray-detached.js @@ -770,7 +770,7 @@ assertThrows(function() { DataView(new ArrayBuffer()); }, TypeError); function TestNonConfigurableProperties(constructor) { var arr = new constructor([100]) - assertFalse(Object.getOwnPropertyDescriptor(arr,"0").configurable) + assertTrue(Object.getOwnPropertyDescriptor(arr,"0").configurable) assertFalse(delete arr[0]) } diff --git a/test/mjsunit/es6/typedarray.js b/test/mjsunit/es6/typedarray.js index 4db5276c848b..9d1e9d782ca4 100644 --- a/test/mjsunit/es6/typedarray.js +++ b/test/mjsunit/es6/typedarray.js @@ -974,7 +974,7 @@ assertThrows(function() { DataView(new ArrayBuffer()); }, TypeError); function TestNonConfigurableProperties(constructor) { var arr = new constructor([100]) - assertFalse(Object.getOwnPropertyDescriptor(arr,"0").configurable) + assertTrue(Object.getOwnPropertyDescriptor(arr,"0").configurable) assertFalse(delete arr[0]) } diff --git a/test/test262/test262.status b/test/test262/test262.status index f7ca00f8e11f..52d7809a1a82 100644 --- a/test/test262/test262.status +++ b/test/test262/test262.status @@ -79,12 +79,7 @@ # https://bugs.chromium.org/p/v8/issues/detail?id=4895 'built-ins/TypedArrayConstructors/internals/DefineOwnProperty/tonumber-value-detached-buffer': [FAIL], 'built-ins/TypedArrayConstructors/internals/DefineOwnProperty/BigInt/tonumber-value-detached-buffer': [FAIL], - 'built-ins/TypedArrayConstructors/internals/DefineOwnProperty/BigInt/key-is-numericindex': [FAIL], - 'built-ins/TypedArrayConstructors/internals/DefineOwnProperty/BigInt/key-is-numericindex-desc-configurable': [FAIL], - 'built-ins/TypedArrayConstructors/internals/DefineOwnProperty/key-is-numericindex': [FAIL], - 'built-ins/TypedArrayConstructors/internals/DefineOwnProperty/key-is-numericindex-desc-configurable': [FAIL], 'built-ins/TypedArrayConstructors/internals/GetOwnProperty/BigInt/index-prop-desc': [FAIL], - 'built-ins/TypedArrayConstructors/internals/GetOwnProperty/index-prop-desc': [FAIL], # Some TypedArray methods throw due to the same bug, from Get 'built-ins/TypedArray/prototype/every/callbackfn-detachbuffer': [FAIL], 'built-ins/TypedArray/prototype/every/BigInt/callbackfn-detachbuffer': [FAIL],