[typedarray] Remove per-iteration detach check in TypedArray.prototype.set

Bug: v8:12750, v8:11111
Change-Id: I3e9947ec8e2883364178b497a49299a3a96332e4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3569879Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79792}

src/builtins/typed-array-set.tq

@@ -114,18 +114,16 @@ TypedArrayPrototypeSetArray(implicit context: Context, receiver: JSAny)(
     target: JSTypedArray, targetLength: uintptr, arrayArg: JSAny,
     targetOffset: uintptr,
     targetOffsetOverflowed: bool): void labels IfOffsetOutOfBounds {
-  // Steps 3-7 are not observable, do them later.
-
-  // 8. Let src be ? ToObject(source).
+  // 4. Let src be ? ToObject(source).
   const src: JSReceiver = ToObject_Inline(context, arrayArg);
 
-  // 9. Let srcLength be ? LengthOfArrayLike(src).
+  // 5. Let srcLength be ? LengthOfArrayLike(src).
   const srcLengthNum: Number = GetLengthProperty(src);
 
-  // 10. If targetOffset is +∞, throw a RangeError exception.
+  // 6. If targetOffset is +∞, throw a RangeError exception.
   if (targetOffsetOverflowed) goto IfOffsetOutOfBounds;
 
-  // 11. If srcLength + targetOffset > targetLength, throw a RangeError
+  // 7. If srcLength + targetOffset > targetLength, throw a RangeError
   //   exception.
   const srcLength = ChangeSafeIntegerNumberToUintPtr(srcLengthNum)
       otherwise IfOffsetOutOfBounds;
@@ -136,12 +134,6 @@ TypedArrayPrototypeSetArray(implicit context: Context, receiver: JSAny)(
   // to do with the empty source array.
   if (srcLength == 0) return;
 
-  // 4. Let targetName be the String value of target.[[TypedArrayName]].
-  // 5. Let targetElementSize be the Element Size value specified in
-  //   Table 69 for targetName.
-  // 6. Let targetType be the Element Type value in Table 69 for
-  //   targetName.
-
   try {
     // BigInt typed arrays are not handled by
     // CopyFastNumberJSArrayElementsToTypedArray.

src/objects/elements.cc

@@ -3822,42 +3822,52 @@ class TypedElementsAccessor
     return false;
   }
 
+  // ES#sec-settypedarrayfromarraylike
   static Object CopyElementsHandleSlow(Handle<Object> source,
                                        Handle<JSTypedArray> destination,
                                        size_t length, size_t offset) {
     Isolate* isolate = destination->GetIsolate();
+    // 8. Let k be 0.
+    // 9. Repeat, while k < srcLength,
     for (size_t i = 0; i < length; i++) {
       Handle<Object> elem;
+      // a. Let Pk be ! ToString(𝔽(k)).
+      // b. Let value be ? Get(src, Pk).
       LookupIterator it(isolate, source, i);
       ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, elem,
                                          Object::GetProperty(&it));
+      // c. Let targetIndex be 𝔽(targetOffset + k).
+      // d. Perform ? IntegerIndexedElementSet(target, targetIndex, value).
+      //
+      // Rest of loop body inlines ES#IntegerIndexedElementSet
       if (IsBigIntTypedArrayElementsKind(Kind)) {
+        // 1. If O.[[ContentType]] is BigInt, let numValue be ? ToBigInt(value).
         ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, elem,
                                            BigInt::FromObject(isolate, elem));
       } else {
+        // 2. Otherwise, let numValue be ? ToNumber(value).
         ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, elem,
                                            Object::ToNumber(isolate, elem));
       }
+      // 3. If IsValidIntegerIndex(O, index) is true, then
+      //   a. Let offset be O.[[ByteOffset]].
+      //   b. Let elementSize be TypedArrayElementSize(O).
+      //   c. Let indexedPosition be (ℝ(index) × elementSize) + offset.
+      //   d. Let elementType be TypedArrayElementType(O).
+      //   e. Perform SetValueInBuffer(O.[[ViewedArrayBuffer]],
+      //      indexedPosition, elementType, numValue, true, Unordered).
       bool out_of_bounds = false;
       size_t new_length = destination->GetLengthOrOutOfBounds(out_of_bounds);
-      if (V8_UNLIKELY(out_of_bounds || destination->WasDetached())) {
-        const char* op = "set";
-        const MessageTemplate message = MessageTemplate::kDetachedOperation;
-        Handle<String> operation =
-            isolate->factory()->NewStringFromAsciiChecked(op);
-        THROW_NEW_ERROR_RETURN_FAILURE(isolate,
-                                       NewTypeError(message, operation));
-      }
-      if (V8_UNLIKELY(new_length <= offset + i)) {
+      if (V8_UNLIKELY(out_of_bounds || destination->WasDetached() ||
+                      new_length <= offset + i)) {
         // Proceed with the loop so that we call get getters for the source even
         // though we don't set the values in the target.
-        // TODO(v8:11111): Maybe change this, depending on how
-        // https://github.com/tc39/proposal-resizablearraybuffer/issues/86 is
-        // resolved.
         continue;
       }
       SetImpl(destination, InternalIndex(offset + i), *elem);
+      // e. Set k to k + 1.
     }
+    // 10. Return unused.
     return *isolate->factory()->undefined_value();
   }
 

test/mjsunit/es6/typedarray.js

@@ -642,7 +642,7 @@ function TestTypedArraySet() {
       return 1;
     }
   };
-  assertThrows(() => a111.set(evilarr), TypeError);
+  a111.set(evilarr);
   assertEquals(true, detached);
 
   // Check if the target is a typed array before converting offset to integer

test/mjsunit/harmony/typedarray-set-length-detach.js

-// Copyright 2020 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Flags: --allow-natives-syntax
-
-
-let ta = new Int32Array(10);
-assertThrows(() => {
-  ta.set({
-    get length() {
-      %ArrayBufferDetach(ta.buffer);
-      return 1;
-    },
-    get 0() {
-      return 100;
-    },
-  });
-}, TypeError);

test/mjsunit/typedarray-resizablearraybuffer-detach.js

@@ -1207,32 +1207,29 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
     });
   }
 
-  // Tests where the length getter returns a non-zero value -> these throw.
+  // Tests where the length getter detaches -> these are no-op.
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const fixedLength = new ctor(rab, 0, 4);
-    assertThrows(() => { fixedLength.set(CreateSourceProxy(1)); }, TypeError);
+    fixedLength.set(CreateSourceProxy(1));
   }
 
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const fixedLengthWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT, 2);
-    assertThrows(() => { fixedLengthWithOffset.set(CreateSourceProxy(1)); },
-                         TypeError);
+    fixedLengthWithOffset.set(CreateSourceProxy(1));
   }
 
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const lengthTracking = new ctor(rab, 0);
-    assertThrows(() => { lengthTracking.set(CreateSourceProxy(1)); },
-                 TypeError);
+    lengthTracking.set(CreateSourceProxy(1));
   }
 
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const lengthTrackingWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT);
-    assertThrows(() => { lengthTrackingWithOffset.set(CreateSourceProxy(1)); },
-                 TypeError);
+    lengthTrackingWithOffset.set(CreateSourceProxy(1));
   }
 
   // Tests where the length getter returns a zero -> these don't throw.
@@ -1302,31 +1299,28 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
     rab = CreateRabForTest(ctor);
     const fixedLength = new ctor(rab, 0, 4);
     detachAt = 2;
-    assertThrows(() => { fixedLength.set(CreateSourceProxy(4)); }, TypeError);
+    fixedLength.set(CreateSourceProxy(4));
   }
 
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const fixedLengthWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT, 2);
     detachAt = 2;
-    assertThrows(() => { fixedLengthWithOffset.set(CreateSourceProxy(2)); },
-                         TypeError);
+    fixedLengthWithOffset.set(CreateSourceProxy(2));
   }
 
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const lengthTracking = new ctor(rab, 0);
     detachAt = 2;
-    assertThrows(() => { lengthTracking.set(CreateSourceProxy(2)); },
-                 TypeError);
+    lengthTracking.set(CreateSourceProxy(2));
   }
 
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const lengthTrackingWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT);
     detachAt = 2;
-    assertThrows(() => { lengthTrackingWithOffset.set(CreateSourceProxy(2)); },
-                 TypeError);
+    lengthTrackingWithOffset.set(CreateSourceProxy(2));
   }
 })();
 

test/mjsunit/typedarray-resizablearraybuffer.js

@@ -5570,13 +5570,13 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
     });
   }
 
-  // Tests where the length getter returns a non-zero value -> these throw if
+  // Tests where the length getter returns a non-zero value -> these are nop if
   // the TA went OOB.
   for (let ctor of ctors) {
     rab = CreateRabForTest(ctor);
     const fixedLength = new ctor(rab, 0, 4);
     resizeTo = 3 * ctor.BYTES_PER_ELEMENT;
-    assertThrows(() => { fixedLength.set(CreateSourceProxy(1)); }, TypeError);
+    fixedLength.set(CreateSourceProxy(1));
     assertEquals([0, 2, 4], ToNumbers(new ctor(rab)));
   }
 
@@ -5584,8 +5584,7 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
     rab = CreateRabForTest(ctor);
     const fixedLengthWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT, 2);
     resizeTo = 3 * ctor.BYTES_PER_ELEMENT;
-    assertThrows(() => { fixedLengthWithOffset.set(CreateSourceProxy(1)); },
-                         TypeError);
+    fixedLengthWithOffset.set(CreateSourceProxy(1));
     assertEquals([0, 2, 4], ToNumbers(new ctor(rab)));
   }
 
@@ -5612,8 +5611,7 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
     rab = CreateRabForTest(ctor);
     const lengthTrackingWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT);
     resizeTo = 1 * ctor.BYTES_PER_ELEMENT;
-    assertThrows(() => { lengthTrackingWithOffset.set(CreateSourceProxy(1)); },
-                 TypeError);
+    lengthTrackingWithOffset.set(CreateSourceProxy(1));
     assertEquals([0], ToNumbers(new ctor(rab)));
   }
 
@@ -5757,7 +5755,7 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
     const fixedLength = new ctor(rab, 0, 4);
     resizeAt = 2;
     resizeTo = 3 * ctor.BYTES_PER_ELEMENT;
-    assertThrows(() => { fixedLength.set(CreateSourceProxy(4)); }, TypeError);
+    fixedLength.set(CreateSourceProxy(4));
     assertEquals([1, 2, 4], ToNumbers(new ctor(rab)));
   }
 
@@ -5766,8 +5764,7 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
     const fixedLengthWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT, 2);
     resizeAt = 2;
     resizeTo = 3 * ctor.BYTES_PER_ELEMENT;
-    assertThrows(() => { fixedLengthWithOffset.set(CreateSourceProxy(2)); },
-                         TypeError);
+    fixedLengthWithOffset.set(CreateSourceProxy(2));
     assertEquals([0, 2, 1], ToNumbers(new ctor(rab)));
   }
 
@@ -5797,8 +5794,7 @@ function TestIterationAndResize(ta, expected, rab, resize_after,
     const lengthTrackingWithOffset = new ctor(rab, 2 * ctor.BYTES_PER_ELEMENT);
     resizeAt = 1;
     resizeTo = 1 * ctor.BYTES_PER_ELEMENT;
-    assertThrows(() => { lengthTrackingWithOffset.set(CreateSourceProxy(2)); },
-                 TypeError);
+    lengthTrackingWithOffset.set(CreateSourceProxy(2));
     assertEquals([0], ToNumbers(new ctor(rab)));
   }
 })();

test/test262/test262.status

@@ -2905,9 +2905,6 @@
   # https://bugs.chromium.org/p/v8/issues/detail?id=12744
   'built-ins/TypedArrayConstructors/ctors/no-species': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=12750
-  'built-ins/TypedArray/prototype/set/array-arg-targetbuffer-detached-on-get-src-value-no-throw': [FAIL],
-
   ######################## NEEDS INVESTIGATION ###########################
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=7833