[builtins] Fixes TypedArray ops behavior when the buffer was detached

After the parameter processing, the arraybuffer may have been detached.
TypedArray copyWithin/fill should throw in that condition. TypedArray
includes should return false if the search element is not undefined.

Change-Id: If507d0efa1dafbe3dcefcd368e5ea27406bb3df8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3144315Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77103}

src/builtins/builtins-typed-array.cc

@@ -81,11 +81,13 @@ BUILTIN(TypedArrayPrototypeCopyWithin) {
   if (count <= 0) return *array;
 
   // TypedArray buffer may have been transferred/detached during parameter
-  // processing above. Return early in this case, to prevent potential UAF error
-  // TODO(caitp): throw here, as though the full algorithm were performed (the
-  // throw would have come from ecma262/#sec-integerindexedelementget)
-  // (see )
-  if (V8_UNLIKELY(array->WasDetached())) return *array;
+  // processing above.
+  if (V8_UNLIKELY(array->WasDetached())) {
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kDetachedOperation,
+                              isolate->factory()->NewStringFromAsciiChecked(
+                                  method_name)));
+  }
 
   if (V8_UNLIKELY(array->is_backed_by_rab())) {
     bool out_of_bounds = false;
@@ -174,7 +176,10 @@ BUILTIN(TypedArrayPrototypeFill) {
   }
 
   if (V8_UNLIKELY(array->WasDetached())) {
-    return *array;
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kDetachedOperation,
+                              isolate->factory()->NewStringFromAsciiChecked(
+                                  method_name)));
   }
 
   if (V8_UNLIKELY(array->IsVariableLength())) {
@@ -224,10 +229,6 @@ BUILTIN(TypedArrayPrototypeIncludes) {
     index = CapRelativeIndex(num, 0, len);
   }
 
-  // TODO(cwhan.tunz): throw. See the above comment in CopyWithin.
-  if (V8_UNLIKELY(array->WasDetached()))
-    return ReadOnlyRoots(isolate).false_value();
-
   Handle<Object> search_element = args.atOrUndefined(isolate, 1);
   ElementsAccessor* elements = array->GetElementsAccessor();
   Maybe<bool> result =
@@ -256,7 +257,6 @@ BUILTIN(TypedArrayPrototypeIndexOf) {
     index = CapRelativeIndex(num, 0, len);
   }
 
-  // TODO(cwhan.tunz): throw. See the above comment in CopyWithin.
   if (V8_UNLIKELY(array->WasDetached())) return Smi::FromInt(-1);
 
   Handle<Object> search_element = args.atOrUndefined(isolate, 1);

src/objects/elements.cc

@@ -3327,8 +3327,6 @@ class TypedElementsAccessor
     DisallowGarbageCollection no_gc;
     JSTypedArray typed_array = JSTypedArray::cast(*receiver);
 
-    // TODO(caitp): return Just(false) here when implementing strict throwing on
-    // detached views.
     if (typed_array.WasDetached()) {
       return Just(value->IsUndefined(isolate) && length > start_from);
     }

test/mjsunit/es6/regress/regress-5929-1.js

@@ -11,4 +11,6 @@ tmp[Symbol.toPrimitive] = function () {
   %ArrayBufferDetach(arr.buffer);
   return 50;
 }
-arr.copyWithin(tmp);
+assertThrows(function() {
+  arr.copyWithin(tmp);
+}, TypeError);

test/mjsunit/typedarray-resizablearraybuffer-detach.js

@@ -132,7 +132,9 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
     let evil = { valueOf: () => { %ArrayBufferDetach(rab); return 1;}};
     // The length is read after converting the first parameter ('value'), so the
     // detaching parameter has to be the 2nd ('start') or 3rd ('end').
-    FillHelper(fixedLength, 1, 0, evil);
+    assertThrows(function() {
+      FillHelper(fixedLength, 1, 0, evil);
+    }, TypeError);
   }
 })();
 
@@ -143,6 +145,8 @@ d8.file.execute('test/mjsunit/typedarray-helpers.js');
     const fixedLength = new ctor(rab, 0, 4);
 
     let evil = { valueOf: () => { %ArrayBufferDetach(rab); return 2;}};
-    fixedLength.copyWithin(evil, 0, 1);
+    assertThrows(function() {
+      fixedLength.copyWithin(evil, 0, 1);
+    }, TypeError);
   }
 })();

test/test262/test262.status

@@ -69,17 +69,6 @@
   # https://code.google.com/p/v8/issues/detail?id=10958
   'language/module-code/eval-gtbndng-indirect-faux-assertion': [FAIL],
 
-  # copyWithin should also throw on detached buffers
-  'built-ins/TypedArray/prototype/copyWithin/coerced-values-end-detached-prototype': [FAIL],
-  'built-ins/TypedArray/prototype/copyWithin/coerced-values-start-detached': [FAIL],
-  'built-ins/TypedArray/prototype/copyWithin/coerced-values-end-detached': [FAIL],
-  # fill should also throw on detached buffers
-  'built-ins/TypedArray/prototype/fill/coerced-value-detach': [FAIL],
-  'built-ins/TypedArray/prototype/fill/coerced-end-detach': [FAIL],
-  'built-ins/TypedArray/prototype/fill/coerced-start-detach': [FAIL],
-  'built-ins/TypedArray/prototype/includes/detached-buffer-during-fromIndex-returns-true-for-undefined': [FAIL],
-  'built-ins/TypedArray/prototype/includes/BigInt/detached-buffer-during-fromIndex-returns-true-for-undefined': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=4951
   'language/expressions/assignment/destructuring/iterator-destructuring-property-reference-target-evaluation-order': [FAIL],
   'language/expressions/assignment/destructuring/keyed-destructuring-property-reference-target-evaluation-order': [FAIL],