diff --git a/src/api-natives.cc b/src/api-natives.cc index a3b3edcc75..93698c9f52 100644 --- a/src/api-natives.cc +++ b/src/api-natives.cc @@ -558,7 +558,7 @@ MaybeHandle ApiNatives::InstantiateRemoteObject( Handle 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); diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index cbff07fe96..ea663dd572 100644 --- a/src/bootstrapper.cc +++ b/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 proxy_map = - factory()->NewMap(JS_PROXY_TYPE, JSProxy::kSize, PACKED_ELEMENTS); + Handle 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 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); diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 6a434c18a3..1946c3ad6e 100644 --- a/src/heap/heap.cc +++ b/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; diff --git a/src/heap/setup-heap-internal.cc b/src/heap/setup-heap-internal.cc index 1f4e399358..770024690d 100644 --- a/src/heap/setup-heap-internal.cc +++ b/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); diff --git a/src/map-updater.cc b/src/map-updater.cc index ec61be1f23..1b8bdfdf12 100644 --- a/src/map-updater.cc +++ b/src/map-updater.cc @@ -123,9 +123,9 @@ Handle 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 MapUpdater::BuildDescriptorArray() { + InstanceType instance_type = old_map_->instance_type(); int target_nof = target_map_->NumberOfOwnDescriptors(); Handle target_descriptors( target_map_->instance_descriptors(), isolate_); @@ -497,8 +498,8 @@ Handle 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 wrapped_type(Map::WrapFieldType(next_field_type)); diff --git a/src/objects-debug.cc b/src/objects-debug.cc index 40a9cfd1c4..03fb853369 100644 --- a/src/objects-debug.cc +++ b/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())); } diff --git a/src/objects.cc b/src/objects.cc index a1ae42a7e1..9d9e078728 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3807,11 +3807,11 @@ MaybeHandle Map::CopyWithField(Handle map, Handle 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 wrapped_type(WrapFieldType(type)); DCHECK_IMPLIES(!FLAG_track_constant_fields, constness == kMutable); @@ -9443,6 +9443,13 @@ void Map::InstallDescriptors(Handle parent, Handle child, Handle Map::CopyAsElementsKind(Handle 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; } diff --git a/src/objects/map-inl.h b/src/objects/map-inl.h index a8c815a5c4..a5421a32ca 100644 --- a/src/objects/map-inl.h +++ b/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* 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 diff --git a/src/objects/map.h b/src/objects/map.h index ed551ce4cb..e3038f51e4 100644 --- a/src/objects/map.h +++ b/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* 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. diff --git a/test/cctest/test-field-type-tracking.cc b/test/cctest/test-field-type-tracking.cc index 5ec0cc2619..9622da53b8 100644 --- a/test/cctest/test-field-type-tracking.cc +++ b/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 initial_map = Map::Create(isolate, 0); + initial_map->set_instance_type(JS_ARRAY_TYPE); initial_map->set_elements_kind(PACKED_SMI_ELEMENTS); Handle map = initial_map; @@ -1810,6 +1811,7 @@ static void TestReconfigureElementsKind_GeneralizeFieldTrivial( // Create a map, add required properties to it and initialize expectations. Handle initial_map = Map::Create(isolate, 0); + initial_map->set_instance_type(JS_ARRAY_TYPE); initial_map->set_elements_kind(PACKED_SMI_ELEMENTS); Handle map = initial_map; diff --git a/test/mjsunit/regress/regress-crbug-783132.js b/test/mjsunit/regress/regress-crbug-783132.js new file mode 100644 index 0000000000..600a6bf5b6 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-783132.js @@ -0,0 +1,15 @@ +// 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); diff --git a/test/mjsunit/unbox-double-arrays.js b/test/mjsunit/unbox-double-arrays.js index 2bebddb449..d6fc0938f9 100644 --- a/test/mjsunit/unbox-double-arrays.js +++ b/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);