[builtins] typed array detaching in builtin iterations

%TypedArray.prototype% methods that receive a user callback fn should not break in the mid-way of the iteration when the backing array buffer was been detached. Instead, the iteration should continue with the value set to undefined. Notably, %TypedArray.prototype%.filter was throwing when the backing buffer was detached during iteration. This should not throw now. Refs: tc39/ecma262#2164 Bug: v8:4895 Change-Id: Ia7fab63264c8148a11f8f123b43c7b3ee0893300 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3066941 Reviewed-by: Shu-yu Guo <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/main@{#76611}
p-g-krish · Aug 31, 2021 · 3926d6c · 3926d6c
1 parent 9cc4140
commit 3926d6c
Show file tree

Hide file tree

Showing 16 changed files with 218 additions and 183 deletions.
diff --git a/src/builtins/builtins-array-gen.cc b/src/builtins/builtins-array-gen.cc
@@ -45,23 +45,23 @@ void ArrayBuiltinsAssembler::TypedArrayMapResultGenerator() {
 // See tc39.github.io/ecma262/#sec-%typedarray%.prototype.map.
 TNode<Object> ArrayBuiltinsAssembler::TypedArrayMapProcessor(
     TNode<Object> k_value, TNode<UintPtrT> k) {
-  // 8. c. Let mapped_value be ? Call(callbackfn, T, « kValue, k, O »).
+  // 7c. Let mapped_value be ? Call(callbackfn, T, « kValue, k, O »).
   TNode<Number> k_number = ChangeUintPtrToTagged(k);
   TNode<Object> mapped_value =
       Call(context(), callbackfn(), this_arg(), k_value, k_number, o());
   Label fast(this), slow(this), done(this), detached(this, Label::kDeferred);
 
-  // 8. d. Perform ? Set(A, Pk, mapped_value, true).
+  // 7d. Perform ? Set(A, Pk, mapped_value, true).
   // Since we know that A is a TypedArray, this always ends up in
   // #sec-integer-indexed-exotic-objects-set-p-v-receiver and then
   // tc39.github.io/ecma262/#sec-integerindexedelementset .
   Branch(fast_typed_array_target_, &fast, &slow);
 
   BIND(&fast);
   // #sec-integerindexedelementset
-  // 5. If arrayTypeName is "BigUint64Array" or "BigInt64Array", let
+  // 2. If arrayTypeName is "BigUint64Array" or "BigInt64Array", let
   // numValue be ? ToBigInt(v).
-  // 6. Otherwise, let numValue be ? ToNumber(value).
+  // 3. Otherwise, let numValue be ? ToNumber(value).
   TNode<Object> num_value;
   if (source_elements_kind_ == BIGINT64_ELEMENTS ||
       source_elements_kind_ == BIGUINT64_ELEMENTS) {
@@ -175,24 +175,15 @@ void ArrayBuiltinsAssembler::GenerateIteratingTypedArrayBuiltinBody(
   size_t i = 0;
   for (auto it = labels.begin(); it != labels.end(); ++i, ++it) {
     BIND(&*it);
-    Label done(this);
     source_elements_kind_ = static_cast<ElementsKind>(elements_kinds[i]);
-    // TODO(turbofan): Silently cancelling the loop on buffer detachment is a
-    // spec violation. Should go to &throw_detached and throw a TypeError
-    // instead.
-    VisitAllTypedArrayElements(array_buffer, processor, &done, direction,
-                               typed_array);
-    Goto(&done);
-    // No exception, return success
-    BIND(&done);
+    VisitAllTypedArrayElements(array_buffer, processor, direction, typed_array);
     ReturnFromBuiltin(a_.value());
   }
 }
 
 void ArrayBuiltinsAssembler::VisitAllTypedArrayElements(
     TNode<JSArrayBuffer> array_buffer, const CallResultProcessor& processor,
-    Label* detached, ForEachDirection direction,
-    TNode<JSTypedArray> typed_array) {
+    ForEachDirection direction, TNode<JSTypedArray> typed_array) {
   VariableList list({&a_, &k_}, zone());
 
   TNode<UintPtrT> start = UintPtrConstant(0);
@@ -208,12 +199,28 @@ void ArrayBuiltinsAssembler::VisitAllTypedArrayElements(
   BuildFastLoop<UintPtrT>(
       list, start, end,
       [&](TNode<UintPtrT> index) {
-        GotoIf(IsDetachedBuffer(array_buffer), detached);
-        TNode<RawPtrT> data_ptr = LoadJSTypedArrayDataPtr(typed_array);
-        TNode<Numeric> value = LoadFixedTypedArrayElementAsTagged(
-            data_ptr, index, source_elements_kind_);
-        k_ = index;
-        a_ = processor(this, value, index);
+        TVARIABLE(Object, value);
+        Label detached(this, Label::kDeferred);
+        Label process(this);
+        GotoIf(IsDetachedBuffer(array_buffer), &detached);
+        {
+          TNode<RawPtrT> data_ptr = LoadJSTypedArrayDataPtr(typed_array);
+          value = LoadFixedTypedArrayElementAsTagged(data_ptr, index,
+                                                     source_elements_kind_);
+          Goto(&process);
+        }
+
+        BIND(&detached);
+        {
+          value = UndefinedConstant();
+          Goto(&process);
+        }
+
+        BIND(&process);
+        {
+          k_ = index;
+          a_ = processor(this, value.value(), index);
+        }
       },
       incr, advance_mode);
 }

diff --git a/src/builtins/builtins-array-gen.h b/src/builtins/builtins-array-gen.h
@@ -104,7 +104,7 @@ class ArrayBuiltinsAssembler : public CodeStubAssembler {
  private:
   void VisitAllTypedArrayElements(TNode<JSArrayBuffer> array_buffer,
                                   const CallResultProcessor& processor,
-                                  Label* detached, ForEachDirection direction,
+                                  ForEachDirection direction,
                                   TNode<JSTypedArray> typed_array);
 
   TNode<Object> callbackfn_;

diff --git a/src/builtins/typed-array-every.tq b/src/builtins/typed-array-every.tq
@@ -7,24 +7,43 @@
 namespace typed_array {
 const kBuiltinNameEvery: constexpr string = '%TypedArray%.prototype.every';
 
+// https://tc39.github.io/ecma262/#sec-%typedarray%.prototype.every
 transitioning macro EveryAllElements(implicit context: Context)(
     array: typed_array::AttachedJSTypedArray, callbackfn: Callable,
     thisArg: JSAny): Boolean {
   let witness = typed_array::NewAttachedJSTypedArrayWitness(array);
   const length: uintptr = witness.Get().length;
+
+  // 6. Repeat, while k < len
   for (let k: uintptr = 0; k < length; k++) {
-    // BUG(4895): We should throw on detached buffers rather than simply exit.
-    witness.Recheck() otherwise break;
-    const value: JSAny = witness.Load(k);
+    // 6a. Let Pk be ! ToString(𝔽(k)).
+    // There is no need to cast ToString to load elements.
+
+    // 6b. Let kValue be ! Get(O, Pk).
+    // kValue must be undefined when the buffer is detached.
+    let value: JSAny;
+    try {
+      witness.Recheck() otherwise goto IsDetached;
+      value = witness.Load(k);
+    } label IsDetached deferred {
+      value = Undefined;
+    }
+
+    // 6c. Let testResult be ! ToBoolean(? Call(callbackfn, thisArg, « kValue,
+    // 𝔽(k), O »)).
     // TODO(v8:4153): Consider versioning this loop for Smi and non-Smi
     // indices to optimize Convert<Number>(k) for the most common case.
     const result = Call(
         context, callbackfn, thisArg, value, Convert<Number>(k),
         witness.GetStable());
+    // 6d. If testResult is false, return false.
     if (!ToBoolean(result)) {
       return False;
     }
+    // 6e. Set k to k + 1. (done by the loop).
   }
+
+  // 7. Return true.
   return True;
 }
 

diff --git a/src/builtins/typed-array-filter.tq b/src/builtins/typed-array-filter.tq
@@ -38,11 +38,15 @@ transitioning javascript builtin TypedArrayPrototypeFilter(
     // 8. Let captured be 0.
     // 9. Repeat, while k < len
     for (let k: uintptr = 0; k < len; k++) {
-      witness.Recheck() otherwise IsDetached;
-
+      let value: JSAny;
       // a. Let Pk be ! ToString(k).
       // b. Let kValue be ? Get(O, Pk).
-      const value: JSAny = witness.Load(k);
+      try {
+        witness.Recheck() otherwise goto IsDetached;
+        value = witness.Load(k);
+      } label IsDetached deferred {
+        value = Undefined;
+      }
 
       // c. Let selected be ToBoolean(? Call(callbackfn, T, « kValue, k, O
       // »)).
@@ -57,7 +61,7 @@ transitioning javascript builtin TypedArrayPrototypeFilter(
       //   ii. Increase captured by 1.
       if (ToBoolean(selected)) kept.Push(value);
 
-      // e.Increase k by 1.
+      // e. Increase k by 1. (done by the loop)
     }
 
     // 10. Let A be ? TypedArraySpeciesCreate(O, captured).

diff --git a/src/builtins/typed-array-find.tq b/src/builtins/typed-array-find.tq
@@ -7,24 +7,45 @@
 namespace typed_array {
 const kBuiltinNameFind: constexpr string = '%TypedArray%.prototype.find';
 
+// https://tc39.github.io/ecma262/#sec-%typedarray%.prototype.find
 transitioning macro FindAllElements(implicit context: Context)(
-    array: typed_array::AttachedJSTypedArray, callbackfn: Callable,
+    array: typed_array::AttachedJSTypedArray, predicate: Callable,
     thisArg: JSAny): JSAny {
   let witness = typed_array::NewAttachedJSTypedArrayWitness(array);
   const length: uintptr = witness.Get().length;
+
+  // 6. Repeat, while k < len
   for (let k: uintptr = 0; k < length; k++) {
-    // BUG(4895): We should throw on detached buffers rather than simply exit.
-    witness.Recheck() otherwise break;
-    const value: JSAny = witness.Load(k);
+    // 6a. Let Pk be ! ToString(𝔽(k)).
+    // There is no need to cast ToString to load elements.
+
+    // 6b. Let kValue be ! Get(O, Pk).
+    // kValue must be undefined when the buffer is detached.
+    let value: JSAny;
+    try {
+      witness.Recheck() otherwise goto IsDetached;
+      value = witness.Load(k);
+    } label IsDetached deferred {
+      value = Undefined;
+    }
+
+    // 6c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue,
+    // 𝔽(k), O »)).
     // TODO(v8:4153): Consider versioning this loop for Smi and non-Smi
     // indices to optimize Convert<Number>(k) for the most common case.
     const result = Call(
-        context, callbackfn, thisArg, value, Convert<Number>(k),
+        context, predicate, thisArg, value, Convert<Number>(k),
         witness.GetStable());
+
+    // 6d. If testResult is true, return kValue.
     if (ToBoolean(result)) {
       return value;
     }
+
+    // 6e. Set k to k + 1. (done by the loop).
   }
+
+  // 7. Return undefined.
   return Undefined;
 }
 
@@ -39,9 +60,9 @@ TypedArrayPrototypeFind(
         otherwise NotTypedArray;
     const uarray = typed_array::EnsureAttached(array) otherwise IsDetached;
 
-    const callbackfn = Cast<Callable>(arguments[0]) otherwise NotCallable;
+    const predicate = Cast<Callable>(arguments[0]) otherwise NotCallable;
     const thisArg = arguments[1];
-    return FindAllElements(uarray, callbackfn, thisArg);
+    return FindAllElements(uarray, predicate, thisArg);
   } label NotCallable deferred {
     ThrowTypeError(MessageTemplate::kCalledNonCallable, arguments[0]);
   } label NotTypedArray deferred {

diff --git a/src/builtins/typed-array-findindex.tq b/src/builtins/typed-array-findindex.tq
@@ -9,19 +9,33 @@ const kBuiltinNameFindIndex: constexpr string =
     '%TypedArray%.prototype.findIndex';
 
 transitioning macro FindIndexAllElements(implicit context: Context)(
-    array: typed_array::AttachedJSTypedArray, callbackfn: Callable,
+    array: typed_array::AttachedJSTypedArray, predicate: Callable,
     thisArg: JSAny): Number {
   let witness = typed_array::NewAttachedJSTypedArrayWitness(array);
   const length: uintptr = witness.Get().length;
+
+  // 6. Repeat, while k < len
   for (let k: uintptr = 0; k < length; k++) {
-    // BUG(4895): We should throw on detached buffers rather than simply exit.
-    witness.Recheck() otherwise break;
-    const value: JSAny = witness.Load(k);
+    // 6a. Let Pk be ! ToString(𝔽(k)).
+    // There is no need to cast ToString to load elements.
+
+    // 6b. Let kValue be ! Get(O, Pk).
+    // kValue must be undefined when the buffer is detached.
+    let value: JSAny;
+    try {
+      witness.Recheck() otherwise goto IsDetached;
+      value = witness.Load(k);
+    } label IsDetached deferred {
+      value = Undefined;
+    }
+
+    // 6c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue,
+    // 𝔽(k), O »)).
     // TODO(v8:4153): Consider versioning this loop for Smi and non-Smi
     // indices to optimize Convert<Number>(k) for the most common case.
     const indexNumber: Number = Convert<Number>(k);
     const result = Call(
-        context, callbackfn, thisArg, value, indexNumber, witness.GetStable());
+        context, predicate, thisArg, value, indexNumber, witness.GetStable());
     if (ToBoolean(result)) {
       return indexNumber;
     }
@@ -40,9 +54,9 @@ TypedArrayPrototypeFindIndex(
         otherwise NotTypedArray;
     const uarray = typed_array::EnsureAttached(array) otherwise IsDetached;
 
-    const callbackfn = Cast<Callable>(arguments[0]) otherwise NotCallable;
+    const predicate = Cast<Callable>(arguments[0]) otherwise NotCallable;
     const thisArg = arguments[1];
-    return FindIndexAllElements(uarray, callbackfn, thisArg);
+    return FindIndexAllElements(uarray, predicate, thisArg);
   } label NotCallable deferred {
     ThrowTypeError(MessageTemplate::kCalledNonCallable, arguments[0]);
   } label NotTypedArray deferred {

diff --git a/src/builtins/typed-array-findlast.tq b/src/builtins/typed-array-findlast.tq
@@ -8,56 +8,28 @@ namespace typed_array {
 const kBuiltinNameFindLast: constexpr string =
     '%TypedArray%.prototype.findLast';
 
-// Continuation part of
-// https://tc39.es/proposal-array-find-from-last/index.html#sec-%typedarray%.prototype.findlast
-// when array buffer was detached.
-transitioning builtin FindLastAllElementsDetachedContinuation(
-    implicit context: Context)(
-    array: JSTypedArray, predicate: Callable, thisArg: JSAny,
-    initialK: Number): JSAny {
-  // 6. Repeat, while k ≥ 0
-  for (let k: Number = initialK; k >= 0; k--) {
-    // 6a. Let Pk be ! ToString(𝔽(k)).
-    // there is no need to cast ToString to load elements.
-
-    // 6b. Let kValue be ! Get(O, Pk).
-    // kValue must be undefined when the buffer was detached.
-
-    // 6c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue,
-    // 𝔽(k), O »)).
-    // TODO(v8:4153): Consider versioning this loop for Smi and non-Smi
-    // indices to optimize Convert<Number>(k) for the most common case.
-    const result =
-        Call(context, predicate, thisArg, Undefined, Convert<Number>(k), array);
-    // 6d. If testResult is true, return kValue.
-    if (ToBoolean(result)) {
-      return Undefined;
-    }
-
-    // 6e. Set k to k - 1. (done by the loop).
-  }
-
-  // 7. Return undefined.
-  return Undefined;
-}
-
 // https://tc39.es/proposal-array-find-from-last/index.html#sec-%typedarray%.prototype.findlast
 transitioning macro FindLastAllElements(implicit context: Context)(
     array: typed_array::AttachedJSTypedArray, predicate: Callable,
-    thisArg: JSAny): JSAny labels
-Bailout(Number) {
+    thisArg: JSAny): JSAny {
   let witness = typed_array::NewAttachedJSTypedArrayWitness(array);
   // 3. Let len be O.[[ArrayLength]].
   const length: uintptr = witness.Get().length;
   // 5. Let k be len - 1.
   // 6. Repeat, while k ≥ 0
   for (let k: uintptr = length; k-- > 0;) {
-    witness.Recheck() otherwise goto Bailout(Convert<Number>(k));
     // 6a. Let Pk be ! ToString(𝔽(k)).
-    // there is no need to cast ToString to load elements.
+    // There is no need to cast ToString to load elements.
 
     // 6b. Let kValue be ! Get(O, Pk).
-    const value: JSAny = witness.Load(k);
+    // kValue must be undefined when the buffer was detached.
+    let value: JSAny;
+    try {
+      witness.Recheck() otherwise goto IsDetached;
+      value = witness.Load(k);
+    } label IsDetached deferred {
+      value = Undefined;
+    }
 
     // 6c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue,
     // 𝔽(k), O »)).
@@ -94,13 +66,7 @@ TypedArrayPrototypeFindLast(
     // 4. If IsCallable(predicate) is false, throw a TypeError exception.
     const predicate = Cast<Callable>(arguments[0]) otherwise NotCallable;
     const thisArg = arguments[1];
-    try {
-      return FindLastAllElements(uarray, predicate, thisArg)
-          otherwise Bailout;
-    } label Bailout(k: Number) deferred {
-      return FindLastAllElementsDetachedContinuation(
-          uarray, predicate, thisArg, k);
-    }
+    return FindLastAllElements(uarray, predicate, thisArg);
   } label NotCallable deferred {
     ThrowTypeError(MessageTemplate::kCalledNonCallable, arguments[0]);
   } label NotTypedArray deferred {