[ic] Don't handle stores with TypedArrays in the prototype chain in ICs

We don't want to handle even non-growing stores when there are TypedArrays
in the prototype chain. Typed arrays handle the out-of-bounds accesses by
ignoring the stores unlike the regular array writes. We just let runtime
handle these cases instead of making ICs more complex.

There was an earlier cl (https://chromium-review.googlesource.com/c/v8/v8/+/1609790)
that fixed it for growing stores. This cl extends it for non-growing stores
as well to handle more cases.

Bug: chromium:961709
Change-Id: I65e079b88c10d2ba343f69a67134893319cd8f8a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1662305
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62243}

src/ic/ic.cc

@@ -1935,13 +1935,9 @@ bool MayHaveTypedArrayInPrototypeChain(Handle<JSObject> object) {
 KeyedAccessStoreMode GetStoreMode(Handle<JSObject> receiver, uint32_t index) {
   bool oob_access = IsOutOfBoundsAccess(receiver, index);
   // Don't consider this a growing store if the store would send the receiver to
-  // dictionary mode. Also make sure we don't consider this a growing store if
-  // there's any JSTypedArray in the {receiver}'s prototype chain, since that
-  // prototype is going to swallow all stores that are out-of-bounds for said
-  // prototype, and we just let the runtime deal with the complexity of this.
+  // dictionary mode.
   bool allow_growth = receiver->IsJSArray() && oob_access &&
-                      !receiver->WouldConvertToSlowElements(index) &&
-                      !MayHaveTypedArrayInPrototypeChain(receiver);
+                      !receiver->WouldConvertToSlowElements(index);
   if (allow_growth) {
     return STORE_AND_GROW_HANDLE_COW;
   }
@@ -2046,6 +2042,13 @@ MaybeHandle<Object> KeyedStoreIC::Store(Handle<Object> object,
       } else if (object->IsJSArray() && IsGrowStoreMode(store_mode) &&
                  JSArray::HasReadOnlyLength(Handle<JSArray>::cast(object))) {
         set_slow_stub_reason("array has read only length");
+      } else if (object->IsJSArray() && MayHaveTypedArrayInPrototypeChain(
+                                            Handle<JSObject>::cast(object))) {
+        // Make sure we don't handle this in IC if there's any JSTypedArray in
+        // the {receiver}'s prototype chain, since that prototype is going to
+        // swallow all stores that are out-of-bounds for said prototype, and we
+        // just let the runtime deal with the complexity of this.
+        set_slow_stub_reason("typed array in the prototype chain of an Array");
       } else if (key_is_valid_index) {
         if (old_receiver_map->is_abandoned_prototype_map()) {
           set_slow_stub_reason("receiver with prototype map");

test/mjsunit/regress/regress-961709-classes-opt.js

+// Copyright 2019 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 --opt --no-always-opt
+
+function foo(a, i) {
+  a[i] = 1;
+  return a[i];
+}
+
+class MyArray extends (class C extends Array {
+}){};
+
+o = new MyArray;
+
+%PrepareFunctionForOptimization(foo);
+assertEquals(1, foo(o, 0));
+assertEquals(1, foo(o, 1));
+%OptimizeFunctionOnNextCall(foo);
+assertEquals(1, foo(o, 2));
+assertOptimized(foo);
+
+// Change prototype
+o.__proto__.__proto__ = new Int32Array(3);
+
+
+// Check it still works
+assertEquals(undefined, foo(o, 3));
+assertUnoptimized(foo);
+%PrepareFunctionForOptimization(foo);
+assertEquals(undefined, foo(o, 3));
+%OptimizeFunctionOnNextCall(foo);
+assertEquals(undefined, foo(o, 3));

test/mjsunit/regress/regress-961709-classes.js

+// Copyright 2019 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
+
+function foo(a, i) {
+  a[i] = 1;
+  return a[i];
+}
+
+class MyArray extends (class C extends Array {
+}){};
+
+o = new MyArray;
+
+%EnsureFeedbackVectorForFunction(foo);
+// initialize IC
+assertEquals(1, foo(o, 0));
+assertEquals(1, foo(o, 1));
+
+// Change prototype
+o.__proto__.__proto__ = new Int32Array(2);
+
+
+// Check it still works
+assertEquals(undefined, foo(o, 2));
+assertEquals(undefined, foo(o, 2));

test/mjsunit/regress/regress-crbug-961709-1.js

@@ -10,6 +10,17 @@ function foo() {
   return a[0];
 }
 
+function bar() {
+  const a = new Array(10);
+  a[0] = 1;
+  return a[0];
+}
+
 Object.setPrototypeOf(Array.prototype, new Int8Array());
+%EnsureFeedbackVectorForFunction(foo);
 assertEquals(undefined, foo());
 assertEquals(undefined, foo());
+
+%EnsureFeedbackVectorForFunction(bar);
+assertEquals(undefined, bar());
+assertEquals(undefined, bar());

test/mjsunit/regress/regress-crbug-961709-2.js

@@ -9,11 +9,24 @@ function foo() {
   a[0] = 1;
   return a[0];
 };
-%PrepareFunctionForOptimization(foo);
-%EnsureFeedbackVectorForFunction(foo);
+
+function bar() {
+  const a = new Array(10);
+  a[0] = 1;
+  return a[0];
+};
+
 Object.setPrototypeOf(Array.prototype, new Int8Array());
+%PrepareFunctionForOptimization(foo);
 assertEquals(undefined, foo());
 assertEquals(undefined, foo());
 %OptimizeFunctionOnNextCall(foo);
 assertEquals(undefined, foo());
 assertOptimized(foo);
+
+%PrepareFunctionForOptimization(bar);
+assertEquals(undefined, bar());
+assertEquals(undefined, bar());
+%OptimizeFunctionOnNextCall(bar);
+assertEquals(undefined, bar());
+assertOptimized(bar);