[runtime] perform type conversion earlier in IntegerIndexedElementSet

When storing an indexed property in a typed array, it's necessary to
convert the value to a Number (or to a Bigint) before performing the
bounds check, per
https://tc39.github.io/ecma262/#sec-integerindexedelementset.

This CL adds appropriate type conversions in
Object::SetPropertyInternal (which technically is reached after the
bounds check has already occurred, but this isn't observable yet ---
In the future, once OOB accesses on TypedArrays actually throw, this
will need to be refactored again), and in StoreFastElementStub, and
ElementsTransitionAndStoreStub (via CSA::EmitElementStore).

The change was not necessary in TurboFan, as
JSNativeContextSpecialization already performs the value conversion
before the boundscheck.

The result is some fixed test262 tests, and some new test coverage
for this behaviour in mjsunit.

BUG=v8:7896, v8:5327
R=neis@chromium.org, jkummerow@chromium.org, gsathya@chromium.org

Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ibe6bec24c72ef6a4fd3e77d5bcafa03737f4c5e3
Reviewed-on: https://chromium-review.googlesource.com/1117372
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54096}

src/code-stub-assembler.cc

@@ -9084,6 +9084,9 @@ Node* CodeStubAssembler::PrepareValueForWriteToTypedArray(
     case FLOAT64_ELEMENTS:
       rep = MachineRepresentation::kFloat64;
       break;
+    case BIGINT64_ELEMENTS:
+    case BIGUINT64_ELEMENTS:
+      return ToBigInt(context, input);
     default:
       UNREACHABLE();
   }
@@ -9245,6 +9248,21 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
 
   if (IsFixedTypedArrayElementsKind(elements_kind)) {
     Label done(this);
+
+    // IntegerIndexedElementSet converts value to a Number/BigInt prior to the
+    // bounds check.
+    value = PrepareValueForWriteToTypedArray(CAST(value), elements_kind,
+                                             CAST(context));
+
+    // There must be no allocations between the buffer load and
+    // and the actual store to backing store, because GC may decide that
+    // the buffer is not alive or move the elements.
+    // TODO(ishell): introduce DisallowHeapAllocationCode scope here.
+
+    // Check if buffer has been neutered.
+    Node* buffer = LoadObjectField(object, JSArrayBufferView::kBufferOffset);
+    GotoIf(IsDetachedBuffer(buffer), bailout);
+
     // Bounds check.
     Node* length =
         TaggedToParameter(LoadTypedArrayLength(CAST(object)), parameter_mode);
@@ -9258,24 +9276,17 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
       GotoIfNot(UintPtrLessThan(intptr_key, length), bailout);
     }
 
-    TNode<Object> value_obj = UncheckedCast<Object>(value);
     if (elements_kind == BIGINT64_ELEMENTS ||
         elements_kind == BIGUINT64_ELEMENTS) {
-      EmitBigTypedArrayElementStore(CAST(object), CAST(elements), intptr_key,
-                                    value_obj, CAST(context), bailout);
+      TNode<BigInt> bigint_value = UncheckedCast<BigInt>(value);
+
+      TNode<RawPtrT> backing_store =
+          LoadFixedTypedArrayBackingStore(CAST(elements));
+      TNode<IntPtrT> offset = ElementOffsetFromIndex(
+          intptr_key, BIGINT64_ELEMENTS, INTPTR_PARAMETERS, 0);
+      EmitBigTypedArrayElementStore(CAST(elements), backing_store, offset,
+                                    bigint_value);
     } else {
-      value = PrepareValueForWriteToTypedArray(value_obj, elements_kind,
-                                               CAST(context));
-
-      // There must be no allocations between the buffer load and
-      // and the actual store to backing store, because GC may decide that
-      // the buffer is not alive or move the elements.
-      // TODO(ishell): introduce DisallowHeapAllocationCode scope here.
-
-      // Check if buffer has been neutered.
-      Node* buffer = LoadObjectField(object, JSArrayBufferView::kBufferOffset);
-      GotoIf(IsDetachedBuffer(buffer), bailout);
-
       Node* backing_store = LoadFixedTypedArrayBackingStore(CAST(elements));
       StoreElement(backing_store, elements_kind, intptr_key, value,
                    parameter_mode);

src/objects.cc

@@ -4895,9 +4895,33 @@ Maybe<bool> Object::SetPropertyInternal(LookupIterator* it,
         }
         return SetPropertyWithAccessor(it, value, should_throw);
       }
-      case LookupIterator::INTEGER_INDEXED_EXOTIC:
-        // TODO(verwaest): We should throw an exception if holder is receiver.
+      case LookupIterator::INTEGER_INDEXED_EXOTIC: {
+        // IntegerIndexedElementSet converts value to a Number/BigInt prior to
+        // the bounds check. The bounds check has already happened here, but
+        // perform the possibly effectful ToNumber (or ToBigInt) operation
+        // anyways.
+        auto holder = it->GetHolder<JSTypedArray>();
+        Handle<Object> throwaway_value;
+        if (holder->type() == kExternalBigInt64Array ||
+            holder->type() == kExternalBigUint64Array) {
+          ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+              it->isolate(), throwaway_value,
+              BigInt::FromObject(it->isolate(), value), Nothing<bool>());
+        } else {
+          ASSIGN_RETURN_ON_EXCEPTION_VALUE(it->isolate(), throwaway_value,
+                                           Object::ToNumber(value),
+                                           Nothing<bool>());
+        }
+
+        // FIXME: Throw a TypeError if the holder is neutered here
+        // (IntegerIndexedElementSpec step 5).
+
+        // TODO(verwaest): Per spec, we should return false here (steps 6-9
+        // in IntegerIndexedElementSpec), resulting in an exception being thrown
+        // on OOB accesses in strict code. Historically, v8 has not done made
+        // this change due to uncertainty about web compat. (v8:4901)
         return Just(true);
+      }
 
       case LookupIterator::DATA:
         if (it->IsReadOnly()) {

test/mjsunit/es6/typedarray-keyedstore-tonumber.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.
+
+let TypedArrayConstructors = [
+  Float64Array,
+  Float32Array,
+  Uint32Array,
+  Int32Array,
+  Uint16Array,
+  Int16Array,
+  Uint8Array,
+  Uint8ClampedArray,
+  Int8Array,
+];
+
+// Test KeyedStore in uninitialized and monomorphic states.
+for (let C of TypedArrayConstructors) {
+  let keyedSta = function(array) {
+    var didRun = false;
+    array[0] = {
+      valueOf() {
+        didRun = true;
+        return 42;
+      }
+    };
+
+    return { didRun, array };
+  };
+
+  for (var i = 0; i < 3; ++i) {
+    var { didRun, array } = keyedSta(new C(1));
+    assertTrue(didRun);
+    assertEquals(array[0], 42);
+
+    // OOB store
+    // FIXME: Throw a TypeError when storing OOB in a TypedArray.
+    var { didRun } = keyedSta(new C);
+    assertTrue(didRun);
+  }
+}
+
+// Test KeyedStore in polymorphic and megamorphic states.
+do {
+  let keyedSta = function(array) {
+    var didRun = false;
+    array[0] = {
+      valueOf() {
+        didRun = true;
+        return 42;
+      }
+    };
+
+    return { didRun, array };
+  };
+
+  for (var i = 0; i < 3; ++i) {
+    for (var C of TypedArrayConstructors) {
+      var { didRun, array } = keyedSta(new C(1));
+      assertTrue(didRun);
+      assertEquals(array[0], 42);
+
+      // OOB store
+      // FIXME: Throw a TypeError when storing OOB in a TypedArray.
+      var { didRun } = keyedSta(new C);
+      assertTrue(didRun);
+    }
+  }
+} while (false);

test/mjsunit/harmony/bigintarray-keyedstore-tobigint.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: --harmony-bigint
+
+let TypedArrayConstructors = [
+  BigUint64Array,
+  BigInt64Array,
+];
+
+// Test KeyedStore in uninitialized and monomorphic states.
+for (let C of TypedArrayConstructors) {
+  let keyedSta = function(array) {
+    var didRun = false;
+    array[0] = {
+      valueOf() {
+        didRun = true;
+        return 42n;
+      }
+    };
+
+    return { didRun, array };
+  };
+
+  for (var i = 0; i < 3; ++i) {
+    var { didRun, array } = keyedSta(new C(1));
+    assertTrue(didRun);
+    assertEquals(array[0], 42n);
+
+    // OOB store
+    // FIXME: Throw a TypeError when storing OOB in a TypedArray.
+    var { didRun } = keyedSta(new C);
+    assertTrue(didRun);
+  }
+}
+
+// Test KeyedStore in polymorphic and megamorphic states.
+do {
+  let keyedSta = function(array) {
+    var didRun = false;
+    array[0] = {
+      valueOf() {
+        didRun = true;
+        return 42n;
+      }
+    };
+
+    return { didRun, array };
+  };
+
+  for (var i = 0; i < 3; ++i) {
+    for (var C of TypedArrayConstructors) {
+      var { didRun, array } = keyedSta(new C(1));
+      assertTrue(didRun);
+      assertEquals(array[0], 42n);
+
+      // OOB store
+      // FIXME: Throw a TypeError when storing OOB in a TypedArray.
+      var { didRun } = keyedSta(new C);
+      assertTrue(didRun);
+    }
+  }
+} while (false);

test/test262/test262.status

@@ -299,8 +299,6 @@
   'built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-not-integer': [FAIL],
   'built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds': [FAIL],
   'built-ins/TypedArrayConstructors/internals/Set/BigInt/key-is-out-of-bounds': [FAIL],
-  'built-ins/TypedArrayConstructors/internals/Set/tonumber-value-throws': [FAIL],
-  'built-ins/TypedArrayConstructors/internals/Set/BigInt/tonumber-value-throws': [FAIL],
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=5329
   'built-ins/RegExp/prototype/source/value-line-terminator': [FAIL],