[typedarray] Check for a detached buffer before each iteration of TypedArray.p.join.

Bug: chromium:917980
Change-Id: Ia9b68f492bb9f0769dc6ee1706baf8b09de49968
Reviewed-on: https://chromium-review.googlesource.com/c/1392070
Commit-Queue: Peter Wong <peter.wm.wong@gmail.com>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58490}

src/builtins/array-join.tq

@@ -58,6 +58,7 @@ namespace array {
   builtin LoadJoinTypedElement<T: type>(
       context: Context, receiver: JSReceiver, k: Number): Object {
     const typedArray: JSTypedArray = UnsafeCast<JSTypedArray>(receiver);
+    assert(!IsDetachedBuffer(typedArray.buffer));
     return typed_array::LoadFixedTypedArrayElementAsTagged(
         typedArray.data_ptr, UnsafeCast<Smi>(k),
         typed_array::KindForArrayType<T>(), SMI_PARAMETERS);
@@ -89,13 +90,13 @@ namespace array {
   // Verifies the current element JSArray accessor can still be safely used
   // (see LoadJoinElement<ElementsAccessor>).
   macro CannotUseSameArrayAccessor<T: type>(implicit context: Context)(
-      useToLocaleString: constexpr bool, loadFn: LoadJoinElementFn,
-      receiver: JSReceiver, originalMap: Map, originalLen: Number): never
+      loadFn: LoadJoinElementFn, receiver: JSReceiver, originalMap: Map,
+      originalLen: Number): never
       labels Cannot, Can;
 
   CannotUseSameArrayAccessor<JSArray>(implicit context: Context)(
-      useToLocaleString: constexpr bool, loadFn: LoadJoinElementFn,
-      receiver: JSReceiver, originalMap: Map, originalLen: Number): never
+      loadFn: LoadJoinElementFn, receiver: JSReceiver, originalMap: Map,
+      originalLen: Number): never
       labels Cannot, Can {
     if (loadFn == LoadJoinElement<GenericElementsAccessor>) goto Can;
 
@@ -107,17 +108,11 @@ namespace array {
   }
 
   CannotUseSameArrayAccessor<JSTypedArray>(implicit context: Context)(
-      useToLocaleString: constexpr bool, loadFn: LoadJoinElementFn,
-      receiver: JSReceiver, initialMap: Map, initialLen: Number): never
+      loadFn: LoadJoinElementFn, receiver: JSReceiver, initialMap: Map,
+      initialLen: Number): never
       labels Cannot, Can {
     const typedArray: JSTypedArray = UnsafeCast<JSTypedArray>(receiver);
-    // It is assumed that calling toLocaleString is the only possible source
-    // of side-effects. Otherwise, it is safe to use the initial LoadJoinElement
-    // specialization and it cannot change through out the join call.
-    if constexpr (useToLocaleString) {
-      if (IsDetachedBuffer(typedArray.buffer)) goto Cannot;
-    }
-    assert(!IsDetachedBuffer(typedArray.buffer));
+    if (IsDetachedBuffer(typedArray.buffer)) goto Cannot;
     goto Can;
   }
 
@@ -292,14 +287,15 @@ namespace array {
 
     // 7. Repeat, while k < len
     while (k < len) {
+      if (CannotUseSameArrayAccessor<T>(
+              loadFn, receiver, initialMap, lengthNumber))
+        deferred {
+          loadFn = LoadJoinElement<GenericElementsAccessor>;
+        }
+
       if (k > 0) {
         // a. If k > 0, let R be the string-concatenation of R and sep.
         nofSeparators = nofSeparators + 1;
-        if (CannotUseSameArrayAccessor<T>(
-                useToLocaleString, loadFn, receiver, initialMap, lengthNumber))
-          deferred {
-            loadFn = LoadJoinElement<GenericElementsAccessor>;
-          }
       }
 
       // b. Let element be ? Get(O, ! ToString(k)).

test/mjsunit/regress/regress-crbug-917980.js

+// Copyright 2018 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
+
+const constructors = [
+  [Uint8Array, [0, 1]],
+  [Int8Array, [0, 1]],
+  [Uint16Array, [0, 1]],
+  [Int16Array, [0, 1]],
+  [Uint32Array, [0, 1]],
+  [Int32Array, [0, 1]],
+  [Float32Array, [0, 1]],
+  [Float64Array, [0, 1]],
+  [Uint8ClampedArray, [0, 1]],
+  [BigInt64Array, [0n, 1n]],
+  [BigUint64Array, [0n, 1n]]
+];
+
+let typedArray;
+
+const separator = {
+  toString() {
+    %ArrayBufferDetach(typedArray.buffer);
+    return '*';
+  }
+};
+
+constructors.forEach(([constructor, arr]) => {
+  typedArray = new constructor(arr);
+  assertSame(typedArray.join(separator), '*');
+});