Avoid making maps unstable in keyed store IC.

If the runtime does not transition in keyed store IC miss handler,
avoid generating transitioning handler since this could make
the receiver map non-stable. (The optimizing compiler does not like
non-stable fast prototype maps.)

Bug: chromium:950328
Change-Id: I113880d2033518e3eb8fd11df1599e56a67d7fd0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1559867
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60752}

src/compiler/access-info.cc

@@ -440,7 +440,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
   // Remember the receiver map. We use {map} as loop variable.
   Handle<Map> receiver_map = map;
   MaybeHandle<JSObject> holder;
-  do {
+  while (true) {
     // Lookup the named property on the {map}.
     Handle<DescriptorArray> descriptors(map->instance_descriptors(), isolate());
     int const number = descriptors->Search(*name, *map);
@@ -538,8 +538,14 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
     }
     map = handle(map_prototype->map(), isolate());
     holder = map_prototype;
-  } while (CanInlinePropertyAccess(map));
-  return false;
+
+    if (!CanInlinePropertyAccess(map)) return false;
+
+    // Successful lookup on prototype chain needs to guarantee that all
+    // the prototypes up to the holder have stable maps. Let us make sure
+    // the prototype maps are stable here.
+    CHECK(map->is_stable());
+  }
 }
 
 bool AccessInfoFactory::ComputePropertyAccessInfo(

src/ic/ic.cc

@@ -2175,6 +2175,18 @@ MaybeHandle<Object> KeyedStoreIC::Store(Handle<Object> object,
           set_slow_stub_reason("receiver with prototype map");
         } else if (!old_receiver_map->DictionaryElementsInPrototypeChainOnly(
                        isolate())) {
+          // If the SetObjectProperty call did not transition, avoid adding
+          // a transition just for the ICs. We want to avoid making
+          // the receiver map unnecessarily non-stable (crbug.com/950328).
+          //
+          // TODO(jarin) We should make this more robust so that the IC system
+          // does not duplicate the logic implemented in runtime
+          // (Runtime::SetObjectProperty).
+          if (old_receiver_map->elements_kind() ==
+              Handle<HeapObject>::cast(object)->map()->elements_kind()) {
+            store_mode =
+                GetNonTransitioningStoreMode(store_mode, receiver_was_cow);
+          }
           // We should go generic if receiver isn't a dictionary, but our
           // prototype chain does have dictionary elements. This ensures that
           // other non-dictionary receivers in the polymorphic case benefit

test/mjsunit/regress/regress-950328.js

@@ -0,0 +1,45 @@
+// 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 NoStoreBecauseReadonlyLength() {
+  var a = [];
+  Object.defineProperty(a, 'length', { writable: false });
+
+
+  function f() {
+    var o = { __proto__ : a };
+    o.push;
+  }
+
+  f();
+  f();
+  %OptimizeFunctionOnNextCall(f);
+
+  a[0] = 1.1;
+  f();
+  assertEquals(undefined, a[0]);
+})();
+
+(function NoStoreBecauseTypedArrayProto() {
+  const arr_proto = [].__proto__;
+  const arr = [];
+
+  function f() {
+    const i32arr = new Int32Array();
+
+    const obj = {};
+    obj.__proto__ = arr;
+    arr_proto.__proto__ = i32arr;
+    obj.__proto__ = arr;
+    arr_proto.__proto__ = i32arr;
+  }
+
+  f();
+  %OptimizeFunctionOnNextCall(f);
+  arr[1024] = [];
+  f();
+  assertEquals(undefined, arr[1024]);
+})();