[runtime] Ensure elements transitions don't interfere with field type tracking.

This CL ensures that elements kind transitions don't cause silent
mutable-to-constant or any-to-class-type migrations of in-place
generalizable fields.

Bug: v8:5495, chromium:783132
Change-Id: Ie60224db62bd45d27148ae0469c7af5a3fe944fd
Reviewed-on: https://chromium-review.googlesource.com/785190Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49583}

src/api-natives.cc

@@ -558,7 +558,7 @@ MaybeHandle<JSObject> ApiNatives::InstantiateRemoteObject(
   Handle<Map> object_map = isolate->factory()->NewMap(
       JS_SPECIAL_API_OBJECT_TYPE,
       JSObject::kHeaderSize + data->embedder_field_count() * kPointerSize,
-      HOLEY_SMI_ELEMENTS);
+      TERMINAL_FAST_ELEMENTS_KIND);
   object_map->SetConstructor(*constructor);
   object_map->set_is_access_check_needed(true);
   object_map->set_may_have_interesting_symbols(true);

src/bootstrapper.cc

@@ -1056,8 +1056,8 @@ void Genesis::CreateJSProxyMaps() {
   // Allocate maps for all Proxy types.
   // Next to the default proxy, we need maps indicating callable and
   // constructable proxies.
-  Handle<Map> proxy_map =
-      factory()->NewMap(JS_PROXY_TYPE, JSProxy::kSize, PACKED_ELEMENTS);
+  Handle<Map> proxy_map = factory()->NewMap(JS_PROXY_TYPE, JSProxy::kSize,
+                                            TERMINAL_FAST_ELEMENTS_KIND);
   proxy_map->set_dictionary_map(true);
   proxy_map->set_may_have_interesting_symbols(true);
   native_context()->set_proxy_map(*proxy_map);
@@ -5510,7 +5510,7 @@ Genesis::Genesis(Isolate* isolate,
   DCHECK_EQ(global_proxy_data->embedder_field_count(),
             global_proxy_template->InternalFieldCount());
   Handle<Map> global_proxy_map = isolate->factory()->NewMap(
-      JS_GLOBAL_PROXY_TYPE, proxy_size, HOLEY_SMI_ELEMENTS);
+      JS_GLOBAL_PROXY_TYPE, proxy_size, TERMINAL_FAST_ELEMENTS_KIND);
   global_proxy_map->set_is_access_check_needed(true);
   global_proxy_map->set_has_hidden_prototype(true);
   global_proxy_map->set_may_have_interesting_symbols(true);

src/heap/heap.cc

@@ -2416,6 +2416,7 @@ AllocationResult Heap::AllocatePartialMap(InstanceType instance_type,
                    Map::ConstructionCounter::encode(Map::kNoSlackTracking);
   map->set_bit_field3(bit_field3);
   map->set_weak_cell_cache(Smi::kZero);
+  map->set_elements_kind(TERMINAL_FAST_ELEMENTS_KIND);
   return map;
 }
 
@@ -2423,6 +2424,11 @@ AllocationResult Heap::AllocateMap(InstanceType instance_type,
                                    int instance_size,
                                    ElementsKind elements_kind,
                                    int inobject_properties) {
+  STATIC_ASSERT(LAST_JS_OBJECT_TYPE == LAST_TYPE);
+  DCHECK_IMPLIES(instance_type >= FIRST_JS_OBJECT_TYPE &&
+                     !Map::CanHaveFastTransitionableElementsKind(instance_type),
+                 IsDictionaryElementsKind(elements_kind) ||
+                     IsTerminalElementsKind(elements_kind));
   HeapObject* result = nullptr;
   AllocationResult allocation = AllocateRaw(Map::kSize, MAP_SPACE);
   if (!allocation.To(&result)) return allocation;

src/heap/setup-heap-internal.cc

@@ -106,15 +106,12 @@ bool Heap::CreateInitialMaps() {
     }
 
     ALLOCATE_PARTIAL_MAP(FIXED_ARRAY_TYPE, kVariableSizeSentinel, fixed_array);
-    fixed_array_map()->set_elements_kind(HOLEY_ELEMENTS);
     ALLOCATE_PARTIAL_MAP(FIXED_ARRAY_TYPE, kVariableSizeSentinel,
                          fixed_cow_array)
-    fixed_cow_array_map()->set_elements_kind(HOLEY_ELEMENTS);
     DCHECK_NE(fixed_array_map(), fixed_cow_array_map());
 
     ALLOCATE_PARTIAL_MAP(FIXED_ARRAY_TYPE, kVariableSizeSentinel,
                          descriptor_array)
-    descriptor_array_map()->set_elements_kind(PACKED_ELEMENTS);
 
     ALLOCATE_PARTIAL_MAP(ODDBALL_TYPE, Oddball::kSize, undefined);
     ALLOCATE_PARTIAL_MAP(ODDBALL_TYPE, Oddball::kSize, null);

src/map-updater.cc

@@ -123,9 +123,9 @@ Handle<Map> MapUpdater::ReconfigureToDataField(int descriptor,
     new_field_type_ = field_type;
   }
 
-  Map::GeneralizeIfTransitionableFastElementsKind(
-      isolate_, new_elements_kind_, &new_constness_, &new_representation_,
-      &new_field_type_);
+  Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
+      isolate_, old_map_->instance_type(), &new_constness_,
+      &new_representation_, &new_field_type_);
 
   if (TryRecofigureToDataFieldInplace() == kEnd) return result_map_;
   if (FindRootMap() == kEnd) return result_map_;
@@ -416,6 +416,7 @@ MapUpdater::State MapUpdater::FindTargetMap() {
 }
 
 Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
+  InstanceType instance_type = old_map_->instance_type();
   int target_nof = target_map_->NumberOfOwnDescriptors();
   Handle<DescriptorArray> target_descriptors(
       target_map_->instance_descriptors(), isolate_);
@@ -497,8 +498,8 @@ Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
           old_details.representation(), old_field_type, next_representation,
           target_field_type, isolate_);
 
-      Map::GeneralizeIfTransitionableFastElementsKind(
-          isolate_, new_elements_kind_, &next_constness, &next_representation,
+      Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
+          isolate_, instance_type, &next_constness, &next_representation,
           &next_field_type);
 
       Handle<Object> wrapped_type(Map::WrapFieldType(next_field_type));

src/objects-debug.cc

@@ -426,6 +426,9 @@ void Map::MapVerify() {
   CHECK_IMPLIES(has_named_interceptor(), may_have_interesting_symbols());
   CHECK_IMPLIES(is_dictionary_map(), may_have_interesting_symbols());
   CHECK_IMPLIES(is_access_check_needed(), may_have_interesting_symbols());
+  CHECK_IMPLIES(IsJSObjectMap() && !CanHaveFastTransitionableElementsKind(),
+                IsDictionaryElementsKind(elements_kind()) ||
+                    IsTerminalElementsKind(elements_kind()));
 }
 
 

src/objects.cc

@@ -3807,11 +3807,11 @@ MaybeHandle<Map> Map::CopyWithField(Handle<Map> map, Handle<Name> name,
     constness = kMutable;
     representation = Representation::Tagged();
     type = FieldType::Any(isolate);
+  } else {
+    Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
+        isolate, map->instance_type(), &constness, &representation, &type);
   }
 
-  Map::GeneralizeIfTransitionableFastElementsKind(
-      isolate, map->elements_kind(), &constness, &representation, &type);
-
   Handle<Object> wrapped_type(WrapFieldType(type));
 
   DCHECK_IMPLIES(!FLAG_track_constant_fields, constness == kMutable);
@@ -9443,6 +9443,13 @@ void Map::InstallDescriptors(Handle<Map> parent, Handle<Map> child,
 
 Handle<Map> Map::CopyAsElementsKind(Handle<Map> map, ElementsKind kind,
                                     TransitionFlag flag) {
+  // Only certain objects are allowed to have non-terminal fast transitional
+  // elements kinds.
+  DCHECK(map->IsJSObjectMap());
+  DCHECK_IMPLIES(
+      !map->CanHaveFastTransitionableElementsKind(),
+      IsDictionaryElementsKind(kind) || IsTerminalElementsKind(kind));
+
   Map* maybe_elements_transition_map = nullptr;
   if (flag == INSERT_TRANSITION) {
     // Ensure we are requested to add elements kind transition "near the root".
@@ -15218,6 +15225,9 @@ bool JSObject::WouldConvertToSlowElements(uint32_t index) {
 
 
 static ElementsKind BestFittingFastElementsKind(JSObject* object) {
+  if (!object->map()->CanHaveFastTransitionableElementsKind()) {
+    return HOLEY_ELEMENTS;
+  }
   if (object->HasSloppyArgumentsElements()) {
     return FAST_SLOPPY_ARGUMENTS_ELEMENTS;
   }

src/objects/map-inl.h

@@ -42,11 +42,20 @@ bool Map::IsInplaceGeneralizableField(PropertyConstness constness,
   return false;
 }
 
+bool Map::CanHaveFastTransitionableElementsKind(InstanceType instance_type) {
+  return instance_type == JS_ARRAY_TYPE || instance_type == JS_VALUE_TYPE ||
+         instance_type == JS_ARGUMENTS_TYPE;
+}
+
+bool Map::CanHaveFastTransitionableElementsKind() const {
+  return CanHaveFastTransitionableElementsKind(instance_type());
+}
+
 // static
-void Map::GeneralizeIfTransitionableFastElementsKind(
-    Isolate* isolate, ElementsKind elements_kind, PropertyConstness* constness,
+void Map::GeneralizeIfCanHaveTransitionableFastElementsKind(
+    Isolate* isolate, InstanceType instance_type, PropertyConstness* constness,
     Representation* representation, Handle<FieldType>* field_type) {
-  if (IsTransitionableFastElementsKind(elements_kind)) {
+  if (CanHaveFastTransitionableElementsKind(instance_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

src/objects/map.h

@@ -457,14 +457,15 @@ class Map : public HeapObject {
                                                  Representation representation,
                                                  FieldType* field_type);
 
-  // Generalizes constness, representation and field_type if the given elements
-  // kind is a fast and transitionable by stubs / optimized code.
+  // Generalizes constness, representation and field_type if objects with given
+  // instance type can have fast elements that can be transitioned by stubs or
+  // optimized code to more general elements kind.
   // This generalization is necessary in order to ensure that elements kind
   // transitions performed by stubs / optimized code don't silently transition
   // kMutable fields back to kConst state or fields with HeapObject
   // representation and "Any" type back to "Class" type.
-  static inline void GeneralizeIfTransitionableFastElementsKind(
-      Isolate* isolate, ElementsKind elements_kind,
+  static inline void GeneralizeIfCanHaveTransitionableFastElementsKind(
+      Isolate* isolate, InstanceType instance_type,
       PropertyConstness* constness, Representation* representation,
       Handle<FieldType>* field_type);
 
@@ -797,6 +798,16 @@ class Map : public HeapObject {
 
   static VisitorId GetVisitorId(Map* map);
 
+  // Returns true if objects with given instance type are allowed to have
+  // fast transitionable elements kinds. This predicate is used to ensure
+  // that objects that can have transitionable fast elements kind will not
+  // get in-place generalizable fields because the elements kind transition
+  // performed by stubs or optimized code can't properly generalize such
+  // fields.
+  static inline bool CanHaveFastTransitionableElementsKind(
+      InstanceType instance_type);
+  inline bool CanHaveFastTransitionableElementsKind() const;
+
  private:
   // This byte encodes either the instance size without the in-object slack or
   // the slack size in properties backing store.

test/cctest/test-field-type-tracking.cc

@@ -1717,6 +1717,7 @@ static void TestReconfigureElementsKind_GeneralizeField(
 
   // Create a map, add required properties to it and initialize expectations.
   Handle<Map> initial_map = Map::Create(isolate, 0);
+  initial_map->set_instance_type(JS_ARRAY_TYPE);
   initial_map->set_elements_kind(PACKED_SMI_ELEMENTS);
 
   Handle<Map> map = initial_map;
@@ -1810,6 +1811,7 @@ static void TestReconfigureElementsKind_GeneralizeFieldTrivial(
 
   // Create a map, add required properties to it and initialize expectations.
   Handle<Map> initial_map = Map::Create(isolate, 0);
+  initial_map->set_instance_type(JS_ARRAY_TYPE);
   initial_map->set_elements_kind(PACKED_SMI_ELEMENTS);
 
   Handle<Map> map = initial_map;

test/mjsunit/regress/regress-crbug-783132.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: --verify-heap
+
+function f(o, v) {
+  try {
+    f(o, v + 1);
+  } catch (e) {
+  }
+  o[v] = 43.35 + v * 5.3;
+}
+
+f(Array.prototype, 0);

test/mjsunit/unbox-double-arrays.js

@@ -50,11 +50,6 @@ function force_to_fast_double_array(a) {
   assertTrue(%HasDoubleElements(a));
 }
 
-function make_object_like_array(size) {
-  obj = new Object();
-  obj.length = size;
-  return obj;
-}
 
 function testOneArrayType(allocator) {
   var large_array = new allocator(large_array_size);
@@ -349,11 +344,18 @@ function testOneArrayType(allocator) {
   assertTrue(%HasDoubleElements(large_array));
 }
 
+class ArraySubclass extends Array {
+  constructor(...args) {
+    super(...args);
+    this.marker = 42;
+  }
+}
+
 // Force gc here to start with a clean heap if we repeat this test multiple
 // times.
 gc();
-testOneArrayType(make_object_like_array);
 testOneArrayType(Array);
+testOneArrayType(ArraySubclass);
 
 var large_array = new Array(large_array_size);
 force_to_fast_double_array(large_array);