Fix buffer-detached check in TypedArray.of/from

The assert-guarded comment claiming that ToNumber could not
possibly neuter the target array unfortunately turns out to
have been wishful thinking.

Bug: chromium:816961
Change-Id: Ib98f96f4cd7f33414c0b5a6037bfb881938cc15e
Reviewed-on: https://chromium-review.googlesource.com/939767
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51637}

src/builtins/builtins-typedarray-gen.cc

@@ -1512,18 +1512,6 @@ TF_BUILTIN(TypedArrayPrototypeKeys, TypedArrayBuiltinsAssembler) {
       context, receiver, "%TypedArray%.prototype.keys()", IterationKind::kKeys);
 }
 
-void TypedArrayBuiltinsAssembler::DebugSanityCheckTypedArrayIndex(
-    TNode<JSTypedArray> array, TNode<IntPtrT> index) {
-#ifdef DEBUG
-  TNode<JSArrayBuffer> buffer =
-      LoadObjectField<JSArrayBuffer>(array, JSArrayBufferView::kBufferOffset);
-  CSA_ASSERT(this, Word32BinaryNot(IsDetachedBuffer(buffer)));
-  TNode<IntPtrT> array_length =
-      LoadAndUntagObjectField(array, JSTypedArray::kLengthOffset);
-  CSA_ASSERT(this, IntPtrLessThan(index, array_length));
-#endif
-}
-
 // ES6 #sec-%typedarray%.of
 TF_BUILTIN(TypedArrayOf, TypedArrayBuiltinsAssembler) {
   TNode<Context> context = CAST(Parameter(BuiltinDescriptor::kContext));
@@ -1535,7 +1523,8 @@ TF_BUILTIN(TypedArrayOf, TypedArrayBuiltinsAssembler) {
   CodeStubArguments args(this, length, nullptr, INTPTR_PARAMETERS,
                          CodeStubArguments::ReceiverMode::kHasReceiver);
 
-  Label if_not_constructor(this, Label::kDeferred);
+  Label if_not_constructor(this, Label::kDeferred),
+      if_neutered(this, Label::kDeferred);
 
   // 3. Let C be the this value.
   // 4. If IsConstructor(C) is false, throw a TypeError exception.
@@ -1566,13 +1555,19 @@ TF_BUILTIN(TypedArrayOf, TypedArrayBuiltinsAssembler) {
               TNode<Object> item = args.AtIndex(index, INTPTR_PARAMETERS);
               TNode<IntPtrT> intptr_index = UncheckedCast<IntPtrT>(index);
               if (kind == BIGINT64_ELEMENTS || kind == BIGUINT64_ELEMENTS) {
-                EmitBigTypedArrayElementStore(
-                    new_typed_array, elements, intptr_index, item, context,
-                    nullptr /* no need to check for neutered buffer */);
+                EmitBigTypedArrayElementStore(new_typed_array, elements,
+                                              intptr_index, item, context,
+                                              &if_neutered);
               } else {
                 Node* value =
                     PrepareValueForWriteToTypedArray(item, kind, context);
 
+                // ToNumber may execute JavaScript code, which could neuter
+                // the array's buffer.
+                Node* buffer = LoadObjectField(new_typed_array,
+                                               JSTypedArray::kBufferOffset);
+                GotoIf(IsDetachedBuffer(buffer), &if_neutered);
+
                 // GC may move backing store in ToNumber, thus load backing
                 // store everytime in this loop.
                 TNode<RawPtrT> backing_store =
@@ -1580,9 +1575,6 @@ TF_BUILTIN(TypedArrayOf, TypedArrayBuiltinsAssembler) {
                 StoreElement(backing_store, kind, index, value,
                              INTPTR_PARAMETERS);
               }
-              // ToNumber/ToBigInt may execute JavaScript code, but they cannot
-              // access arguments array and new typed array.
-              DebugSanityCheckTypedArrayIndex(new_typed_array, intptr_index);
             },
             1, ParameterMode::INTPTR_PARAMETERS, IndexAdvanceMode::kPost);
       });
@@ -1592,6 +1584,10 @@ TF_BUILTIN(TypedArrayOf, TypedArrayBuiltinsAssembler) {
 
   BIND(&if_not_constructor);
   ThrowTypeError(context, MessageTemplate::kNotConstructor, receiver);
+
+  BIND(&if_neutered);
+  ThrowTypeError(context, MessageTemplate::kDetachedOperation,
+                 "%TypedArray%.of");
 }
 
 TF_BUILTIN(IterableToList, TypedArrayBuiltinsAssembler) {
@@ -1666,7 +1662,8 @@ TF_BUILTIN(TypedArrayFrom, TypedArrayBuiltinsAssembler) {
       slow_path(this), create_typed_array(this),
       if_not_constructor(this, Label::kDeferred),
       if_map_fn_not_callable(this, Label::kDeferred),
-      if_iterator_fn_not_callable(this, Label::kDeferred);
+      if_iterator_fn_not_callable(this, Label::kDeferred),
+      if_neutered(this, Label::kDeferred);
 
   CodeStubArguments args(
       this, ChangeInt32ToIntPtr(Parameter(BuiltinDescriptor::kArgumentsCount)));
@@ -1797,14 +1794,19 @@ TF_BUILTIN(TypedArrayFrom, TypedArrayBuiltinsAssembler) {
             elements_kind,
             [&](ElementsKind kind, int size, int typed_array_fun_index) {
               if (kind == BIGINT64_ELEMENTS || kind == BIGUINT64_ELEMENTS) {
-                EmitBigTypedArrayElementStore(
-                    target_obj.value(), elements, intptr_index, mapped_value,
-                    context,
-                    nullptr /* no need to check for neutered buffer */);
+                EmitBigTypedArrayElementStore(target_obj.value(), elements,
+                                              intptr_index, mapped_value,
+                                              context, &if_neutered);
               } else {
                 Node* const final_value = PrepareValueForWriteToTypedArray(
                     mapped_value, kind, context);
 
+                // ToNumber may execute JavaScript code, which could neuter
+                // the array's buffer.
+                Node* buffer = LoadObjectField(target_obj.value(),
+                                               JSTypedArray::kBufferOffset);
+                GotoIf(IsDetachedBuffer(buffer), &if_neutered);
+
                 // GC may move backing store in map_fn, thus load backing
                 // store in each iteration of this loop.
                 TNode<RawPtrT> backing_store =
@@ -1813,9 +1815,6 @@ TF_BUILTIN(TypedArrayFrom, TypedArrayBuiltinsAssembler) {
                              SMI_PARAMETERS);
               }
             });
-        // map_fn or ToNumber or ToBigint could have executed JavaScript code,
-        // but they could not have accessed the new typed array.
-        DebugSanityCheckTypedArrayIndex(target_obj.value(), intptr_index);
       },
       1, ParameterMode::SMI_PARAMETERS, IndexAdvanceMode::kPost);
 
@@ -1829,6 +1828,10 @@ TF_BUILTIN(TypedArrayFrom, TypedArrayBuiltinsAssembler) {
 
   BIND(&if_iterator_fn_not_callable);
   ThrowTypeError(context, MessageTemplate::kIteratorSymbolNonCallable);
+
+  BIND(&if_neutered);
+  ThrowTypeError(context, MessageTemplate::kDetachedOperation,
+                 "%TypedArray%.from");
 }
 
 // ES %TypedArray%.prototype.filter

src/builtins/builtins-typedarray-gen.h

@@ -125,9 +125,6 @@ class TypedArrayBuiltinsAssembler : public CodeStubAssembler {
 
   void DispatchTypedArrayByElementsKind(
       TNode<Word32T> elements_kind, const TypedArraySwitchCase& case_function);
-
-  void DebugSanityCheckTypedArrayIndex(TNode<JSTypedArray> array,
-                                       TNode<IntPtrT> index);
 };
 
 }  // namespace internal

test/mjsunit/regress/regress-crbug-816961.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.
+
+assertThrows(function() {
+  var memory = new WebAssembly.Memory({initial: 64 * 1024 * 1024 / 0x10000});
+  var array = new Uint8Array(memory.buffer);
+  Uint8Array.of.call(function() { return array },
+                    {valueOf() { memory.grow(1); } });
+}, TypeError);
+
+assertThrows(function() {
+  var memory = new WebAssembly.Memory({initial: 64 * 1024 * 1024 / 0x10000});
+  var array = new Uint8Array(memory.buffer);
+  Uint8Array.from.call(function() { return array },
+                       [{valueOf() { memory.grow(1); } }],
+                       x => x);
+}, TypeError);