[runtime] Don't create class field types for arrays' fields.

... when generalizing const fields to mutable fields.

Bug: chromium:748539, chromium:747979, chromium:738763
Change-Id: Iee772a5d0cddd23599f1f68bca00b8beecb76da0
Reviewed-on: https://chromium-review.googlesource.com/586709Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46921}

src/map-updater.cc

@@ -123,21 +123,8 @@ Handle<Map> MapUpdater::ReconfigureToDataField(int descriptor,
     new_field_type_ = field_type;
   }
 
-  if (IsTransitionableFastElementsKind(new_elements_kind_) &&
-      Map::IsInplaceGeneralizableField(new_constness_, new_representation_,
-                                       *new_field_type_)) {
-    // We don't support propagation of field generalization through elements
-    // kind transitions because they are inserted into the transition tree
-    // before field transitions. In order to avoid complexity of handling
-    // such a case we ensure that all maps with transitionable elements kinds
-    // do not have fields that can be generalized in-place (without creation
-    // of a new map).
-    if (FLAG_track_constant_fields && FLAG_modify_map_inplace) {
-      new_constness_ = kMutable;
-    }
-    DCHECK(representation.IsHeapObject());
-    new_field_type_ = FieldType::Any(isolate_);
-  }
+  GeneralizeIfTransitionableFastElementsKind(
+      &new_constness_, &new_representation_, &new_field_type_);
 
   if (TryRecofigureToDataFieldInplace() == kEnd) return result_map_;
   if (FindRootMap() == kEnd) return result_map_;
@@ -150,6 +137,8 @@ Handle<Map> MapUpdater::ReconfigureToDataField(int descriptor,
 Handle<Map> MapUpdater::ReconfigureElementsKind(ElementsKind elements_kind) {
   DCHECK_EQ(kInitialized, state_);
   new_elements_kind_ = elements_kind;
+  is_transitionable_fast_elements_kind_ =
+      IsTransitionableFastElementsKind(new_elements_kind_);
 
   if (FindRootMap() == kEnd) return result_map_;
   if (FindTargetMap() == kEnd) return result_map_;
@@ -169,6 +158,28 @@ Handle<Map> MapUpdater::Update() {
   return result_map_;
 }
 
+void MapUpdater::GeneralizeIfTransitionableFastElementsKind(
+    PropertyConstness* constness, Representation* representation,
+    Handle<FieldType>* field_type) {
+  DCHECK_EQ(is_transitionable_fast_elements_kind_,
+            IsTransitionableFastElementsKind(new_elements_kind_));
+  if (is_transitionable_fast_elements_kind_ &&
+      Map::IsInplaceGeneralizableField(*constness, *representation,
+                                       **field_type)) {
+    // We don't support propagation of field generalization through elements
+    // kind transitions because they are inserted into the transition tree
+    // before field transitions. In order to avoid complexity of handling
+    // such a case we ensure that all maps with transitionable elements kinds
+    // do not have fields that can be generalized in-place (without creation
+    // of a new map).
+    if (FLAG_track_constant_fields && FLAG_modify_map_inplace) {
+      *constness = kMutable;
+    }
+    DCHECK(representation->IsHeapObject());
+    *field_type = FieldType::Any(isolate_);
+  }
+}
+
 void MapUpdater::GeneralizeField(Handle<Map> map, int modify_index,
                                  PropertyConstness new_constness,
                                  Representation new_representation,
@@ -505,6 +516,9 @@ Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
           old_details.representation(), old_field_type, next_representation,
           target_field_type, isolate_);
 
+      GeneralizeIfTransitionableFastElementsKind(
+          &next_constness, &next_representation, &next_field_type);
+
       Handle<Object> wrapped_type(Map::WrapFieldType(next_field_type));
       Descriptor d;
       if (next_kind == kData) {
@@ -548,10 +562,17 @@ Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
 
     Descriptor d;
     if (next_location == kField) {
-      Handle<FieldType> old_field_type =
+      Handle<FieldType> next_field_type =
           GetOrComputeFieldType(i, old_details.location(), next_representation);
 
-      Handle<Object> wrapped_type(Map::WrapFieldType(old_field_type));
+      // If the |new_elements_kind_| is still transitionable then the old map's
+      // elements kind is also transitionable and therefore the old descriptors
+      // array must already have non in-place generalizable fields.
+      CHECK_IMPLIES(is_transitionable_fast_elements_kind_,
+                    !Map::IsInplaceGeneralizableField(
+                        next_constness, next_representation, *next_field_type));
+
+      Handle<Object> wrapped_type(Map::WrapFieldType(next_field_type));
       Descriptor d;
       if (next_kind == kData) {
         DCHECK_IMPLIES(!FLAG_track_constant_fields, next_constness == kMutable);

src/map-updater.h

@@ -49,7 +49,9 @@ class MapUpdater {
         old_map_(old_map),
         old_descriptors_(old_map->instance_descriptors(), isolate_),
         old_nof_(old_map_->NumberOfOwnDescriptors()),
-        new_elements_kind_(old_map_->elements_kind()) {
+        new_elements_kind_(old_map_->elements_kind()),
+        is_transitionable_fast_elements_kind_(
+            IsTransitionableFastElementsKind(new_elements_kind_)) {
     // We shouldn't try to update remote objects.
     DCHECK(!old_map->FindRootMap()->GetConstructor()->IsFunctionTemplateInfo());
   }
@@ -146,6 +148,10 @@ class MapUpdater {
       Handle<DescriptorArray> descriptors, int descriptor,
       PropertyLocation location, Representation representation);
 
+  inline void GeneralizeIfTransitionableFastElementsKind(
+      PropertyConstness* constness, Representation* representation,
+      Handle<FieldType>* field_type);
+
   void GeneralizeField(Handle<Map> map, int modify_index,
                        PropertyConstness new_constness,
                        Representation new_representation,
@@ -161,8 +167,9 @@ class MapUpdater {
 
   State state_ = kInitialized;
   ElementsKind new_elements_kind_;
+  bool is_transitionable_fast_elements_kind_;
 
-  // If |modified_descriptor_| is not equal to -1 them the fields below form
+  // If |modified_descriptor_| is not equal to -1 then the fields below form
   // an "update" of the |old_map_|'s descriptors.
   int modified_descriptor_ = -1;
   PropertyKind new_kind_ = kData;

src/objects.cc

@@ -4495,20 +4495,6 @@ void Map::GeneralizeField(Handle<Map> map, int modify_index,
   }
 }
 
-bool Map::IsInplaceGeneralizableField(PropertyConstness constness,
-                                      Representation representation,
-                                      FieldType* field_type) {
-  if (FLAG_track_constant_fields && FLAG_modify_map_inplace &&
-      (constness == kConst)) {
-    // kConst -> kMutable field generalization may happen in-place.
-    return true;
-  }
-  if (representation.IsHeapObject() && !field_type->IsAny()) {
-    return true;
-  }
-  return false;
-}
-
 // TODO(ishell): remove.
 // static
 Handle<Map> Map::ReconfigureProperty(Handle<Map> map, int modify_index,

src/objects/map-inl.h

@@ -5,6 +5,7 @@
 #ifndef V8_OBJECTS_MAP_INL_H_
 #define V8_OBJECTS_MAP_INL_H_
 
+#include "src/field-type.h"
 #include "src/objects/map.h"
 
 // Has to be the last include (doesn't have include guards):
@@ -27,6 +28,20 @@ InterceptorInfo* Map::GetIndexedInterceptor() {
   return InterceptorInfo::cast(info->indexed_property_handler());
 }
 
+bool Map::IsInplaceGeneralizableField(PropertyConstness constness,
+                                      Representation representation,
+                                      FieldType* field_type) {
+  if (FLAG_track_constant_fields && FLAG_modify_map_inplace &&
+      (constness == kConst)) {
+    // kConst -> kMutable field generalization may happen in-place.
+    return true;
+  }
+  if (representation.IsHeapObject() && !field_type->IsAny()) {
+    return true;
+  }
+  return false;
+}
+
 int NormalizedMapCache::GetIndex(Handle<Map> map) {
   return map->Hash() % NormalizedMapCache::kEntries;
 }

src/objects/map.h

@@ -346,7 +346,7 @@ class Map : public HeapObject {
                               Handle<FieldType> new_field_type);
   // Returns true if |descriptor|'th property is a field that may be generalized
   // by just updating current map.
-  static bool IsInplaceGeneralizableField(PropertyConstness constness,
+  static inline bool IsInplaceGeneralizableField(PropertyConstness constness,
                                                  Representation representation,
                                                  FieldType* field_type);
 

test/mjsunit/regress/regress-crbug-748539.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
+
+function f1() {}
+function f2() {}
+
+var o1 = [];
+o1.a = 0;
+o1.f = f1;
+%HeapObjectVerify(o1);
+
+var o2 = [];
+o2.a = 4.2;
+o2.f = f2;
+%HeapObjectVerify(o2);
+
+o1.a;
+%HeapObjectVerify(o1);
+%HeapObjectVerify(o2);