Make TypedArray elements configurable

This implements the spec change in tc39/ecma262#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ä <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#71955}
ashu8912 · Jan 7, 2021 · d485af5 · d485af5
1 parent 96c41c3
commit d485af5
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 16 deletions.
diff --git 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<JSObject> 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
@@ -227,12 +227,12 @@ Maybe<bool> 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<bool> 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<Object> value = desc->value();

diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc
@@ -853,8 +853,11 @@ Maybe<bool> JSReceiver::DeleteProperty(LookupIterator* it,
         return Just(true);
       case LookupIterator::DATA:
       case LookupIterator::ACCESSOR: {
-        if (!it->IsConfigurable()) {
-          // Fail if the property is not configurable.
+        Handle<JSObject> holder = it->GetHolder<JSObject>();
+        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
@@ -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
@@ -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
@@ -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
@@ -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],