[ic] Fix handling of elements kind transitions in polymorphic keyed ICs.

Ensure source map is not stable if elements kind transitions are expected.

BUG=chromium:700733

Change-Id: Ie937e7064127250b1100109986c3e9b411fae1d6
Reviewed-on: https://chromium-review.googlesource.com/483442Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44780}

src/compiler/access-info.cc

@@ -229,6 +229,7 @@ bool AccessInfoFactory::ComputeElementAccessInfos(
       if (transition_target == nullptr) {
         receiver_maps.Add(map);
       } else {
+        DCHECK(!map->is_stable());
         transitions.push_back(std::make_pair(map, handle(transition_target)));
       }
     }

src/crankshaft/hydrogen.cc

@@ -7180,6 +7180,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
     Map* transitioned_map =
         map->FindElementsKindTransitionedMap(&possible_transitioned_maps);
     if (transitioned_map != nullptr) {
+      DCHECK(!map->is_stable());
       transition_target.Add(handle(transitioned_map));
     } else {
       transition_target.Add(Handle<Map>());

src/ic/handler-compiler.cc

@@ -152,51 +152,5 @@ Handle<Code> NamedStoreHandlerCompiler::CompileStoreCallback(
 
 #undef __
 
-// static
-Handle<Object> ElementHandlerCompiler::GetKeyedLoadHandler(
-    Handle<Map> receiver_map, Isolate* isolate) {
-  if (receiver_map->has_indexed_interceptor() &&
-      !receiver_map->GetIndexedInterceptor()->getter()->IsUndefined(isolate) &&
-      !receiver_map->GetIndexedInterceptor()->non_masking()) {
-    TRACE_HANDLER_STATS(isolate, KeyedLoadIC_LoadIndexedInterceptorStub);
-    return LoadIndexedInterceptorStub(isolate).GetCode();
-  }
-  if (receiver_map->IsStringMap()) {
-    TRACE_HANDLER_STATS(isolate, KeyedLoadIC_LoadIndexedStringStub);
-    return isolate->builtins()->KeyedLoadIC_IndexedString();
-  }
-  InstanceType instance_type = receiver_map->instance_type();
-  if (instance_type < FIRST_JS_RECEIVER_TYPE) {
-    TRACE_HANDLER_STATS(isolate, KeyedLoadIC_SlowStub);
-    return isolate->builtins()->KeyedLoadIC_Slow();
-  }
-
-  ElementsKind elements_kind = receiver_map->elements_kind();
-  if (IsSloppyArgumentsElementsKind(elements_kind)) {
-    TRACE_HANDLER_STATS(isolate, KeyedLoadIC_KeyedLoadSloppyArgumentsStub);
-    return KeyedLoadSloppyArgumentsStub(isolate).GetCode();
-  }
-  bool is_js_array = instance_type == JS_ARRAY_TYPE;
-  if (elements_kind == DICTIONARY_ELEMENTS) {
-    TRACE_HANDLER_STATS(isolate, KeyedLoadIC_LoadElementDH);
-    return LoadHandler::LoadElement(isolate, elements_kind, false, is_js_array);
-  }
-  DCHECK(IsFastElementsKind(elements_kind) ||
-         IsFixedTypedArrayElementsKind(elements_kind));
-  // TODO(jkummerow): Use IsHoleyElementsKind(elements_kind).
-  bool convert_hole_to_undefined =
-      is_js_array && elements_kind == FAST_HOLEY_ELEMENTS &&
-      *receiver_map == isolate->get_initial_js_array_map(elements_kind);
-  TRACE_HANDLER_STATS(isolate, KeyedLoadIC_LoadElementDH);
-  return LoadHandler::LoadElement(isolate, elements_kind,
-                                  convert_hole_to_undefined, is_js_array);
-}
-
-void ElementHandlerCompiler::CompileElementHandlers(
-    MapHandleList* receiver_maps, List<Handle<Object>>* handlers) {
-  for (int i = 0; i < receiver_maps->length(); ++i) {
-    handlers->Add(GetKeyedLoadHandler(receiver_maps->at(i), isolate()));
-  }
-}
 }  // namespace internal
 }  // namespace v8

src/ic/handler-compiler.h

@@ -186,21 +186,6 @@ class NamedStoreHandlerCompiler : public PropertyHandlerCompiler {
   static Register value();
 };
 
-
-class ElementHandlerCompiler : public PropertyHandlerCompiler {
- public:
-  explicit ElementHandlerCompiler(Isolate* isolate)
-      : PropertyHandlerCompiler(isolate, Code::KEYED_LOAD_IC,
-                                Handle<Map>::null(), Handle<JSObject>::null()) {
-  }
-
-  virtual ~ElementHandlerCompiler() {}
-
-  static Handle<Object> GetKeyedLoadHandler(Handle<Map> receiver_map,
-                                            Isolate* isolate);
-  void CompileElementHandlers(MapHandleList* receiver_maps,
-                              List<Handle<Object>>* handlers);
-};
 }  // namespace internal
 }  // namespace v8
 

src/ic/ic.cc

@@ -1320,8 +1320,7 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
   TargetMaps(&target_receiver_maps);
 
   if (target_receiver_maps.length() == 0) {
-    Handle<Object> handler =
-        ElementHandlerCompiler::GetKeyedLoadHandler(receiver_map, isolate());
+    Handle<Object> handler = LoadElementHandler(receiver_map);
     return ConfigureVectorState(Handle<Name>(), receiver_map, handler);
   }
 
@@ -1349,8 +1348,7 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
       IsMoreGeneralElementsKindTransition(
           target_receiver_maps.at(0)->elements_kind(),
           Handle<JSObject>::cast(receiver)->GetElementsKind())) {
-    Handle<Object> handler =
-        ElementHandlerCompiler::GetKeyedLoadHandler(receiver_map, isolate());
+    Handle<Object> handler = LoadElementHandler(receiver_map);
     return ConfigureVectorState(Handle<Name>(), receiver_map, handler);
   }
 
@@ -1373,11 +1371,67 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
   }
 
   List<Handle<Object>> handlers(target_receiver_maps.length());
-  ElementHandlerCompiler compiler(isolate());
-  compiler.CompileElementHandlers(&target_receiver_maps, &handlers);
+  LoadElementPolymorphicHandlers(&target_receiver_maps, &handlers);
   ConfigureVectorState(Handle<Name>(), &target_receiver_maps, &handlers);
 }
 
+Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map) {
+  if (receiver_map->has_indexed_interceptor() &&
+      !receiver_map->GetIndexedInterceptor()->getter()->IsUndefined(
+          isolate()) &&
+      !receiver_map->GetIndexedInterceptor()->non_masking()) {
+    TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadIndexedInterceptorStub);
+    return LoadIndexedInterceptorStub(isolate()).GetCode();
+  }
+  if (receiver_map->IsStringMap()) {
+    TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadIndexedStringStub);
+    return isolate()->builtins()->KeyedLoadIC_IndexedString();
+  }
+  InstanceType instance_type = receiver_map->instance_type();
+  if (instance_type < FIRST_JS_RECEIVER_TYPE) {
+    TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_SlowStub);
+    return isolate()->builtins()->KeyedLoadIC_Slow();
+  }
+
+  ElementsKind elements_kind = receiver_map->elements_kind();
+  if (IsSloppyArgumentsElementsKind(elements_kind)) {
+    TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_KeyedLoadSloppyArgumentsStub);
+    return KeyedLoadSloppyArgumentsStub(isolate()).GetCode();
+  }
+  bool is_js_array = instance_type == JS_ARRAY_TYPE;
+  if (elements_kind == DICTIONARY_ELEMENTS) {
+    TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadElementDH);
+    return LoadHandler::LoadElement(isolate(), elements_kind, false,
+                                    is_js_array);
+  }
+  DCHECK(IsFastElementsKind(elements_kind) ||
+         IsFixedTypedArrayElementsKind(elements_kind));
+  // TODO(jkummerow): Use IsHoleyElementsKind(elements_kind).
+  bool convert_hole_to_undefined =
+      is_js_array && elements_kind == FAST_HOLEY_ELEMENTS &&
+      *receiver_map == isolate()->get_initial_js_array_map(elements_kind);
+  TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadElementDH);
+  return LoadHandler::LoadElement(isolate(), elements_kind,
+                                  convert_hole_to_undefined, is_js_array);
+}
+
+void KeyedLoadIC::LoadElementPolymorphicHandlers(
+    MapHandleList* receiver_maps, List<Handle<Object>>* handlers) {
+  for (int i = 0; i < receiver_maps->length(); ++i) {
+    Handle<Map> receiver_map(receiver_maps->at(i));
+
+    // Mark all stable receiver maps that have elements kind transition map
+    // among receiver_maps as unstable because the optimizing compilers may
+    // generate an elements kind transition for this kind of receivers.
+    if (receiver_map->is_stable()) {
+      Map* tmap = receiver_map->FindElementsKindTransitionedMap(receiver_maps);
+      if (tmap != nullptr) {
+        receiver_map->NotifyLeafMapLayoutChange();
+      }
+    }
+    handlers->Add(LoadElementHandler(receiver_map));
+  }
+}
 
 MaybeHandle<Object> KeyedLoadIC::Load(Handle<Object> object,
                                       Handle<Object> key) {
@@ -2051,7 +2105,12 @@ void KeyedStoreIC::StoreElementPolymorphicHandlers(
       {
         Map* tmap =
             receiver_map->FindElementsKindTransitionedMap(receiver_maps);
-        if (tmap != nullptr) transitioned_map = handle(tmap);
+        if (tmap != nullptr) {
+          if (receiver_map->is_stable()) {
+            receiver_map->NotifyLeafMapLayoutChange();
+          }
+          transitioned_map = handle(tmap);
+        }
       }
 
       // TODO(mvstanton): The code below is doing pessimistic elements

src/ic/ic.h

@@ -325,6 +325,11 @@ class KeyedLoadIC : public LoadIC {
 
  private:
   friend class IC;
+
+  Handle<Object> LoadElementHandler(Handle<Map> receiver_map);
+
+  void LoadElementPolymorphicHandlers(MapHandleList* receiver_maps,
+                                      List<Handle<Object>>* handlers);
 };
 
 

test/mjsunit/regress/regress-crbug-700733.js

+// Copyright 2017 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 --verify-heap --expose-gc
+
+(function test_keyed_load() {
+  var smi_arr = [0];
+  smi_arr.load = 42;
+
+  var double_arr = [0.5];
+  double_arr.load = 42;
+
+  var obj_arr = [{}];
+  obj_arr.load = 42;
+
+  var arrs = [smi_arr, double_arr, obj_arr];
+
+  var tmp;
+  function do_keyed_load(arrs) {
+    for (var i = 0; i < arrs.length; i++) {
+      var arr = arrs[i];
+      tmp = arr[0];
+    }
+  }
+
+  var obj = {};
+  obj.load_boom = smi_arr;
+
+  do_keyed_load(arrs);
+  do_keyed_load(arrs);
+  %OptimizeFunctionOnNextCall(do_keyed_load);
+  do_keyed_load(arrs);
+
+  gc();
+})();
+
+(function test_keyed_store() {
+  var smi_arr = [0];
+  smi_arr.store = 42;
+
+  var double_arr = [0.5];
+  double_arr.store = 42;
+
+  var obj_arr = [{}];
+  obj_arr.store = 42;
+
+  var arrs = [smi_arr, double_arr, obj_arr];
+
+  function do_keyed_store(arrs) {
+    for (var i = 0; i < arrs.length; i++) {
+      var arr = arrs[i];
+      arr[0] = 0;
+    }
+  }
+
+  var obj = {};
+  obj.store_boom = smi_arr;
+
+  do_keyed_store(arrs);
+  do_keyed_store(arrs);
+  %OptimizeFunctionOnNextCall(do_keyed_store);
+  do_keyed_store(arrs);
+
+  gc();
+})();