From 9c0e2a6723e3f4ea1c181d235a08b1115663d39a Mon Sep 17 00:00:00 2001 From: machenbach Date: Tue, 29 Nov 2016 00:49:23 -0800 Subject: [PATCH] =?UTF-8?q?Revert=20of=20[ic]=20Use=20validity=20cells=20t?= =?UTF-8?q?o=20protect=20keyed=20element=20stores=20against=20object's=20p?= =?UTF-8?q?rototype=20chain=20modificati=E2=80=A6=20(patchset=20#2=20id:40?= =?UTF-8?q?001=20of=20https://codereview.chromium.org/2534613002/=20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reason for revert: Layout test crashes: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/11691 Original issue's description: > [ic] Use validity cells to protect keyed element stores against object's prototype chain modifications. > > ... instead of clearing of all the KeyedStoreICs which didn't always work. > > BUG=chromium:662907, v8:5561 > TBR=verwaest@chromium.org, bmeurer@chromium.org > > Committed: https://crrev.com/a39522f44f7e0be4686831688917e9675255dcaf > Cr-Commit-Position: refs/heads/master@{#41332} TBR=jkummerow@chromium.org,ishell@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=chromium:662907, v8:5561 Review-Url: https://codereview.chromium.org/2538693002 Cr-Commit-Position: refs/heads/master@{#41337} --- include/v8.h | 4 +- src/ast/ast-types.cc | 1 - src/builtins/builtins-array.cc | 8 +-- src/code-stub-assembler.h | 2 - src/compiler/types.cc | 1 - src/elements.cc | 17 +++-- src/factory.cc | 8 --- src/factory.h | 3 - src/ic/accessor-assembler-impl.h | 11 ++- src/ic/accessor-assembler.cc | 73 ++++---------------- src/ic/ic-compiler.cc | 39 +++++------ src/ic/ic-compiler.h | 8 +-- src/ic/ic-inl.h | 4 +- src/ic/ic.cc | 11 +-- src/ic/ic.h | 2 +- src/lookup.cc | 7 +- src/objects-debug.cc | 6 -- src/objects-inl.h | 4 +- src/objects-printer.cc | 7 -- src/objects.cc | 47 ++++++++----- src/objects.h | 36 +++------- src/type-feedback-vector.cc | 56 ++++++++++++--- src/type-feedback-vector.h | 5 +- src/value-serializer.cc | 4 +- test/mjsunit/regress/regress-crbug-662907.js | 53 -------------- 25 files changed, 160 insertions(+), 257 deletions(-) delete mode 100644 test/mjsunit/regress/regress-crbug-662907.js diff --git a/include/v8.h b/include/v8.h index 9f4e8531e1..d0dda84c10 100644 --- a/include/v8.h +++ b/include/v8.h @@ -8286,8 +8286,8 @@ class Internals { static const int kNodeIsIndependentShift = 3; static const int kNodeIsActiveShift = 4; - static const int kJSObjectType = 0xbd; - static const int kJSApiObjectType = 0xbc; + static const int kJSObjectType = 0xbc; + static const int kJSApiObjectType = 0xbb; static const int kFirstNonstringType = 0x80; static const int kOddballType = 0x83; static const int kForeignType = 0x87; diff --git a/src/ast/ast-types.cc b/src/ast/ast-types.cc index bb50a15f01..49551dd7fa 100644 --- a/src/ast/ast-types.cc +++ b/src/ast/ast-types.cc @@ -315,7 +315,6 @@ AstType::bitset AstBitsetType::Lub(i::Map* map) { case CELL_TYPE: case WEAK_CELL_TYPE: case PROTOTYPE_INFO_TYPE: - case TUPLE2_TYPE: case TUPLE3_TYPE: case CONTEXT_EXTENSION_TYPE: UNREACHABLE(); diff --git a/src/builtins/builtins-array.cc b/src/builtins/builtins-array.cc index f56ee74be0..9bfd68ff2f 100644 --- a/src/builtins/builtins-array.cc +++ b/src/builtins/builtins-array.cc @@ -457,9 +457,8 @@ class ArrayConcatVisitor { SeededNumberDictionary::cast(*storage_)); // The object holding this backing store has just been allocated, so // it cannot yet be used as a prototype. - Handle not_a_prototype_holder; - Handle result = SeededNumberDictionary::AtNumberPut( - dict, index, elm, not_a_prototype_holder); + Handle result = + SeededNumberDictionary::AtNumberPut(dict, index, elm, false); if (!result.is_identical_to(dict)) { // Dictionary needed to grow. clear_storage(); @@ -526,10 +525,9 @@ class ArrayConcatVisitor { if (!element->IsTheHole(isolate_)) { // The object holding this backing store has just been allocated, so // it cannot yet be used as a prototype. - Handle not_a_prototype_holder; Handle new_storage = SeededNumberDictionary::AtNumberPut(slow_storage, i, element, - not_a_prototype_holder); + false); if (!new_storage.is_identical_to(slow_storage)) { slow_storage = loop_scope.CloseAndEscape(new_storage); } diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index d6cfad035a..29afdfe4a0 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -38,8 +38,6 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol }; V(SymbolMap, SymbolMap) \ V(TheHoleValue, TheHole) \ V(TrueValue, True) \ - V(Tuple2Map, Tuple2Map) \ - V(Tuple3Map, Tuple3Map) \ V(UndefinedValue, Undefined) // Provides JavaScript-specific "macro-assembler" functionality on top of the diff --git a/src/compiler/types.cc b/src/compiler/types.cc index a66e15a9ab..806bd8f2c5 100644 --- a/src/compiler/types.cc +++ b/src/compiler/types.cc @@ -310,7 +310,6 @@ Type::bitset BitsetType::Lub(i::Map* map) { case CELL_TYPE: case WEAK_CELL_TYPE: case PROTOTYPE_INFO_TYPE: - case TUPLE2_TYPE: case TUPLE3_TYPE: case CONTEXT_EXTENSION_TYPE: UNREACHABLE(); diff --git a/src/elements.cc b/src/elements.cc index ffea033c93..748b420cda 100644 --- a/src/elements.cc +++ b/src/elements.cc @@ -1431,8 +1431,9 @@ class DictionaryElementsAccessor ? JSObject::NormalizeElements(object) : handle(SeededNumberDictionary::cast(object->elements())); Handle new_dictionary = - SeededNumberDictionary::AddNumberEntry(dictionary, index, value, - details, object); + SeededNumberDictionary::AddNumberEntry( + dictionary, index, value, details, + object->map()->is_prototype_map()); if (attributes != NONE) object->RequireSlowElements(*new_dictionary); if (dictionary.is_identical_to(new_dictionary)) return; object->set_elements(*new_dictionary); @@ -1772,14 +1773,15 @@ class FastElementsAccessor : public ElementsAccessorBase { SeededNumberDictionary::New(isolate, capacity); PropertyDetails details = PropertyDetails::Empty(); + bool used_as_prototype = object->map()->is_prototype_map(); int j = 0; for (int i = 0; j < capacity; i++) { if (IsHoleyElementsKind(kind)) { if (BackingStore::cast(*store)->is_the_hole(isolate, i)) continue; } Handle value = Subclass::GetImpl(isolate, *store, i); - dictionary = SeededNumberDictionary::AddNumberEntry(dictionary, i, value, - details, object); + dictionary = SeededNumberDictionary::AddNumberEntry( + dictionary, i, value, details, used_as_prototype); j++; } return dictionary; @@ -3274,8 +3276,9 @@ class SlowSloppyArgumentsElementsAccessor : JSObject::NormalizeElements(object); PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell); Handle new_dictionary = - SeededNumberDictionary::AddNumberEntry(dictionary, index, value, - details, object); + SeededNumberDictionary::AddNumberEntry( + dictionary, index, value, details, + object->map()->is_prototype_map()); if (attributes != NONE) object->RequireSlowElements(*new_dictionary); if (*dictionary != *new_dictionary) { FixedArray::cast(object->elements())->set(1, *new_dictionary); @@ -3308,7 +3311,7 @@ class SlowSloppyArgumentsElementsAccessor Handle arguments( SeededNumberDictionary::cast(parameter_map->get(1)), isolate); arguments = SeededNumberDictionary::AddNumberEntry( - arguments, entry, value, details, object); + arguments, entry, value, details, object->map()->is_prototype_map()); // If the attributes were NONE, we would have called set rather than // reconfigure. DCHECK_NE(NONE, attributes); diff --git a/src/factory.cc b/src/factory.cc index 844880c209..e38b2f6730 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -103,14 +103,6 @@ Handle Factory::NewPrototypeInfo() { return result; } -Handle Factory::NewTuple2(Handle value1, - Handle value2) { - Handle result = Handle::cast(NewStruct(TUPLE2_TYPE)); - result->set_value1(*value1); - result->set_value2(*value2); - return result; -} - Handle Factory::NewTuple3(Handle value1, Handle value2, Handle value3) { Handle result = Handle::cast(NewStruct(TUPLE3_TYPE)); diff --git a/src/factory.h b/src/factory.h index dc258701bc..d059b10888 100644 --- a/src/factory.h +++ b/src/factory.h @@ -85,9 +85,6 @@ class V8_EXPORT_PRIVATE Factory final { // Create a new PrototypeInfo struct. Handle NewPrototypeInfo(); - // Create a new Tuple2 struct. - Handle NewTuple2(Handle value1, Handle value2); - // Create a new Tuple3 struct. Handle NewTuple3(Handle value1, Handle value2, Handle value3); diff --git a/src/ic/accessor-assembler-impl.h b/src/ic/accessor-assembler-impl.h index 41b6e06a43..01e9bf1820 100644 --- a/src/ic/accessor-assembler-impl.h +++ b/src/ic/accessor-assembler-impl.h @@ -81,12 +81,12 @@ class AccessorAssemblerImpl : public CodeStubAssembler { Node* value; }; - enum ElementSupport { kOnlyProperties, kSupportElements }; - void HandleStoreICHandlerCase( - const StoreICParameters* p, Node* handler, Label* miss, - ElementSupport support_elements = kOnlyProperties); + void HandleStoreICHandlerCase(const StoreICParameters* p, Node* handler, + Label* miss); private: + enum ElementSupport { kOnlyProperties, kSupportElements }; + // Stub generation entry points. void LoadIC(const LoadICParameters* p); @@ -142,9 +142,6 @@ class AccessorAssemblerImpl : public CodeStubAssembler { // StoreIC implementation. - void HandleStoreICElementHandlerCase(const StoreICParameters* p, - Node* handler, Label* miss); - void HandleStoreICProtoHandler(const StoreICParameters* p, Node* handler, Label* miss); // If |transition| is nullptr then the normal field store is generated or diff --git a/src/ic/accessor-assembler.cc b/src/ic/accessor-assembler.cc index 024fba4038..db59ed00e5 100644 --- a/src/ic/accessor-assembler.cc +++ b/src/ic/accessor-assembler.cc @@ -472,13 +472,13 @@ void AccessorAssemblerImpl::HandleLoadGlobalICHandlerCase( miss, kOnlyProperties); } -void AccessorAssemblerImpl::HandleStoreICHandlerCase( - const StoreICParameters* p, Node* handler, Label* miss, - ElementSupport support_elements) { - Label if_smi_handler(this), if_nonsmi_handler(this); - Label if_proto_handler(this), if_element_handler(this), call_handler(this); +void AccessorAssemblerImpl::HandleStoreICHandlerCase(const StoreICParameters* p, + Node* handler, + Label* miss) { + Label if_smi_handler(this); + Label try_proto_handler(this), call_handler(this); - Branch(TaggedIsSmi(handler), &if_smi_handler, &if_nonsmi_handler); + Branch(TaggedIsSmi(handler), &if_smi_handler, &try_proto_handler); // |handler| is a Smi, encoding what to do. See SmiHandler methods // for the encoding format. @@ -491,22 +491,9 @@ void AccessorAssemblerImpl::HandleStoreICHandlerCase( HandleStoreICSmiHandlerCase(handler_word, holder, p->value, nullptr, miss); } - Bind(&if_nonsmi_handler); - { - Node* handler_map = LoadMap(handler); - if (support_elements == kSupportElements) { - GotoIf(IsTuple2Map(handler_map), &if_element_handler); - } - Branch(IsCodeMap(handler_map), &call_handler, &if_proto_handler); - } - - if (support_elements == kSupportElements) { - Bind(&if_element_handler); - { HandleStoreICElementHandlerCase(p, handler, miss); } - } - - Bind(&if_proto_handler); + Bind(&try_proto_handler); { + GotoIf(IsCodeMap(LoadMap(handler)), &call_handler); HandleStoreICProtoHandler(p, handler, miss); } @@ -519,23 +506,6 @@ void AccessorAssemblerImpl::HandleStoreICHandlerCase( } } -void AccessorAssemblerImpl::HandleStoreICElementHandlerCase( - const StoreICParameters* p, Node* handler, Label* miss) { - Comment("HandleStoreICElementHandlerCase"); - Node* validity_cell = LoadObjectField(handler, Tuple2::kValue1Offset); - Node* cell_value = LoadObjectField(validity_cell, Cell::kValueOffset); - GotoIf(WordNotEqual(cell_value, - SmiConstant(Smi::FromInt(Map::kPrototypeChainValid))), - miss); - - Node* code_handler = LoadObjectField(handler, Tuple2::kValue2Offset); - CSA_ASSERT(this, IsCodeMap(LoadMap(code_handler))); - - StoreWithVectorDescriptor descriptor(isolate()); - TailCallStub(descriptor, code_handler, p->context, p->receiver, p->name, - p->value, p->slot, p->vector); -} - void AccessorAssemblerImpl::HandleStoreICProtoHandler( const StoreICParameters* p, Node* handler, Label* miss) { // IC dispatchers rely on these assumptions to be held. @@ -1536,7 +1506,7 @@ void AccessorAssemblerImpl::KeyedStoreIC(const StoreICParameters* p, Bind(&if_handler); { Comment("KeyedStoreIC_if_handler"); - HandleStoreICHandlerCase(p, var_handler.value(), &miss, kSupportElements); + HandleStoreICHandlerCase(p, var_handler.value(), &miss); } Bind(&try_polymorphic); @@ -1553,26 +1523,11 @@ void AccessorAssemblerImpl::KeyedStoreIC(const StoreICParameters* p, &var_transition_map_cell, &miss); Bind(&if_transition_handler); Comment("KeyedStoreIC_polymorphic_transition"); - { - Node* handler = var_handler.value(); - CSA_ASSERT(this, IsTuple2Map(LoadMap(handler))); - - // Check validity cell. - Node* validity_cell = LoadObjectField(handler, Tuple2::kValue1Offset); - Node* cell_value = LoadObjectField(validity_cell, Cell::kValueOffset); - GotoIf(WordNotEqual(cell_value, - SmiConstant(Smi::FromInt(Map::kPrototypeChainValid))), - &miss); - - Node* code_handler = LoadObjectField(handler, Tuple2::kValue2Offset); - CSA_ASSERT(this, IsCodeMap(LoadMap(code_handler))); - - Node* transition_map = - LoadWeakCellValue(var_transition_map_cell.value(), &miss); - StoreTransitionDescriptor descriptor(isolate()); - TailCallStub(descriptor, code_handler, p->context, p->receiver, p->name, - transition_map, p->value, p->slot, p->vector); - } + Node* transition_map = + LoadWeakCellValue(var_transition_map_cell.value(), &miss); + StoreTransitionDescriptor descriptor(isolate()); + TailCallStub(descriptor, var_handler.value(), p->context, p->receiver, + p->name, transition_map, p->value, p->slot, p->vector); } Bind(&try_megamorphic); diff --git a/src/ic/ic-compiler.cc b/src/ic/ic-compiler.cc index 1d71b57ef3..750c88daa9 100644 --- a/src/ic/ic-compiler.cc +++ b/src/ic/ic-compiler.cc @@ -10,7 +10,7 @@ namespace v8 { namespace internal { -Handle PropertyICCompiler::ComputeKeyedStoreMonomorphicHandler( +Handle PropertyICCompiler::ComputeKeyedStoreMonomorphicHandler( Handle receiver_map, KeyedAccessStoreMode store_mode) { Isolate* isolate = receiver_map->GetIsolate(); @@ -20,14 +20,14 @@ Handle PropertyICCompiler::ComputeKeyedStoreMonomorphicHandler( store_mode == STORE_NO_TRANSITION_HANDLE_COW); PropertyICCompiler compiler(isolate); - Handle handler = + Handle code = compiler.CompileKeyedStoreMonomorphicHandler(receiver_map, store_mode); - return handler; + return code; } void PropertyICCompiler::ComputeKeyedStorePolymorphicHandlers( MapHandleList* receiver_maps, MapHandleList* transitioned_maps, - List>* handlers, KeyedAccessStoreMode store_mode) { + CodeHandleList* handlers, KeyedAccessStoreMode store_mode) { Isolate* isolate = receiver_maps->at(0)->GetIsolate(); DCHECK(store_mode == STANDARD_STORE || store_mode == STORE_AND_GROW_NO_TRANSITION || @@ -38,12 +38,13 @@ void PropertyICCompiler::ComputeKeyedStorePolymorphicHandlers( receiver_maps, transitioned_maps, handlers, store_mode); } + void PropertyICCompiler::CompileKeyedStorePolymorphicHandlers( MapHandleList* receiver_maps, MapHandleList* transitioned_maps, - List>* handlers, KeyedAccessStoreMode store_mode) { + CodeHandleList* handlers, KeyedAccessStoreMode store_mode) { for (int i = 0; i < receiver_maps->length(); ++i) { Handle receiver_map(receiver_maps->at(i)); - Handle handler; + Handle cached_stub; Handle transitioned_map; { Map* tmap = receiver_map->FindElementsKindTransitionedMap(receiver_maps); @@ -60,25 +61,21 @@ void PropertyICCompiler::CompileKeyedStorePolymorphicHandlers( ElementsKind elements_kind = receiver_map->elements_kind(); TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_ElementsTransitionAndStoreStub); - Handle stub = + cached_stub = ElementsTransitionAndStoreStub(isolate(), elements_kind, transitioned_map->elements_kind(), - is_js_array, store_mode) - .GetCode(); - Handle validity_cell = - Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate()); - handler = isolate()->factory()->NewTuple2(validity_cell, stub); - + is_js_array, store_mode).GetCode(); } else if (receiver_map->instance_type() < FIRST_JS_RECEIVER_TYPE) { // TODO(mvstanton): Consider embedding store_mode in the state of the slow // keyed store ic for uniformity. TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_SlowStub); - handler = isolate()->builtins()->KeyedStoreIC_Slow(); + cached_stub = isolate()->builtins()->KeyedStoreIC_Slow(); } else { - handler = CompileKeyedStoreMonomorphicHandler(receiver_map, store_mode); + cached_stub = + CompileKeyedStoreMonomorphicHandler(receiver_map, store_mode); } - DCHECK(!handler.is_null()); - handlers->Add(handler); + DCHECK(!cached_stub.is_null()); + handlers->Add(cached_stub); transitioned_maps->Add(transitioned_map); } } @@ -86,7 +83,8 @@ void PropertyICCompiler::CompileKeyedStorePolymorphicHandlers( #define __ ACCESS_MASM(masm()) -Handle PropertyICCompiler::CompileKeyedStoreMonomorphicHandler( + +Handle PropertyICCompiler::CompileKeyedStoreMonomorphicHandler( Handle receiver_map, KeyedAccessStoreMode store_mode) { ElementsKind elements_kind = receiver_map->elements_kind(); bool is_jsarray = receiver_map->instance_type() == JS_ARRAY_TYPE; @@ -103,10 +101,7 @@ Handle PropertyICCompiler::CompileKeyedStoreMonomorphicHandler( TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_StoreElementStub); stub = StoreElementStub(isolate(), elements_kind, store_mode).GetCode(); } - Handle validity_cell = - Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate()); - Handle handler = isolate()->factory()->NewTuple2(validity_cell, stub); - return handler; + return stub; } diff --git a/src/ic/ic-compiler.h b/src/ic/ic-compiler.h index b8d6635ae0..e0fa60ab01 100644 --- a/src/ic/ic-compiler.h +++ b/src/ic/ic-compiler.h @@ -14,22 +14,22 @@ namespace internal { class PropertyICCompiler : public PropertyAccessCompiler { public: // Keyed - static Handle ComputeKeyedStoreMonomorphicHandler( + static Handle ComputeKeyedStoreMonomorphicHandler( Handle receiver_map, KeyedAccessStoreMode store_mode); static void ComputeKeyedStorePolymorphicHandlers( MapHandleList* receiver_maps, MapHandleList* transitioned_maps, - List>* handlers, KeyedAccessStoreMode store_mode); + CodeHandleList* handlers, KeyedAccessStoreMode store_mode); private: explicit PropertyICCompiler(Isolate* isolate) : PropertyAccessCompiler(isolate, Code::KEYED_STORE_IC, kCacheOnReceiver) {} - Handle CompileKeyedStoreMonomorphicHandler( + Handle CompileKeyedStoreMonomorphicHandler( Handle receiver_map, KeyedAccessStoreMode store_mode); void CompileKeyedStorePolymorphicHandlers(MapHandleList* receiver_maps, MapHandleList* transitioned_maps, - List>* handlers, + CodeHandleList* handlers, KeyedAccessStoreMode store_mode); }; diff --git a/src/ic/ic-inl.h b/src/ic/ic-inl.h index b286315c01..1b5d063270 100644 --- a/src/ic/ic-inl.h +++ b/src/ic/ic-inl.h @@ -93,8 +93,8 @@ Code* IC::target() const { } bool IC::IsHandler(Object* object) { - return (object->IsSmi() && (object != nullptr)) || object->IsTuple2() || - object->IsTuple3() || object->IsFixedArray() || + return (object->IsSmi() && (object != nullptr)) || object->IsTuple3() || + object->IsFixedArray() || (object->IsCode() && Code::cast(object)->is_handler()); } diff --git a/src/ic/ic.cc b/src/ic/ic.cc index feef309d7a..38fe3314b0 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -603,9 +603,10 @@ void IC::ConfigureVectorState(Handle name, MapHandleList* maps, OnTypeFeedbackChanged(isolate(), get_host()); } + void IC::ConfigureVectorState(MapHandleList* maps, MapHandleList* transitioned_maps, - List>* handlers) { + CodeHandleList* handlers) { DCHECK(UseVector()); DCHECK(kind() == Code::KEYED_STORE_IC); KeyedStoreICNexus* nexus = casted_nexus(); @@ -2207,7 +2208,7 @@ void KeyedStoreIC::UpdateStoreElement(Handle receiver_map, Handle monomorphic_map = ComputeTransitionedMap(receiver_map, store_mode); store_mode = GetNonTransitioningStoreMode(store_mode); - Handle handler = + Handle handler = PropertyICCompiler::ComputeKeyedStoreMonomorphicHandler(monomorphic_map, store_mode); return ConfigureVectorState(Handle(), monomorphic_map, handler); @@ -2241,7 +2242,7 @@ void KeyedStoreIC::UpdateStoreElement(Handle receiver_map, // if they at least come from the same origin for a transitioning store, // stay MONOMORPHIC and use the map for the most generic ElementsKind. store_mode = GetNonTransitioningStoreMode(store_mode); - Handle handler = + Handle handler = PropertyICCompiler::ComputeKeyedStoreMonomorphicHandler( transitioned_receiver_map, store_mode); ConfigureVectorState(Handle(), transitioned_receiver_map, handler); @@ -2255,7 +2256,7 @@ void KeyedStoreIC::UpdateStoreElement(Handle receiver_map, // A "normal" IC that handles stores can switch to a version that can // grow at the end of the array, handle OOB accesses or copy COW arrays // and still stay MONOMORPHIC. - Handle handler = + Handle handler = PropertyICCompiler::ComputeKeyedStoreMonomorphicHandler(receiver_map, store_mode); return ConfigureVectorState(Handle(), receiver_map, handler); @@ -2316,7 +2317,7 @@ void KeyedStoreIC::UpdateStoreElement(Handle receiver_map, } MapHandleList transitioned_maps(target_receiver_maps.length()); - List> handlers(target_receiver_maps.length()); + CodeHandleList handlers(target_receiver_maps.length()); PropertyICCompiler::ComputeKeyedStorePolymorphicHandlers( &target_receiver_maps, &transitioned_maps, &handlers, store_mode); ConfigureVectorState(&target_receiver_maps, &transitioned_maps, &handlers); diff --git a/src/ic/ic.h b/src/ic/ic.h index 407e1cd30d..63429bca6a 100644 --- a/src/ic/ic.h +++ b/src/ic/ic.h @@ -120,7 +120,7 @@ class IC { // keyed stores). void ConfigureVectorState(MapHandleList* maps, MapHandleList* transitioned_maps, - List>* handlers); + CodeHandleList* handlers); char TransitionMarkFromState(IC::State state); void TraceIC(const char* type, Handle name); diff --git a/src/lookup.cc b/src/lookup.cc index 593e6928f9..186823df84 100644 --- a/src/lookup.cc +++ b/src/lookup.cc @@ -526,8 +526,11 @@ void LookupIterator::TransitionToAccessorPair(Handle pair, Handle dictionary = JSObject::NormalizeElements(receiver); - dictionary = SeededNumberDictionary::Set(dictionary, index_, pair, details, - receiver); + // We unconditionally pass used_as_prototype=false here because the call + // to RequireSlowElements takes care of the required IC clearing and + // we don't want to walk the heap twice. + dictionary = + SeededNumberDictionary::Set(dictionary, index_, pair, details, false); receiver->RequireSlowElements(*dictionary); if (receiver->HasSlowArgumentsElements()) { diff --git a/src/objects-debug.cc b/src/objects-debug.cc index ca23f201c8..db50cfba75 100644 --- a/src/objects-debug.cc +++ b/src/objects-debug.cc @@ -1078,12 +1078,6 @@ void PrototypeInfo::PrototypeInfoVerify() { CHECK(validity_cell()->IsCell() || validity_cell()->IsSmi()); } -void Tuple2::Tuple2Verify() { - CHECK(IsTuple2()); - VerifyObjectField(kValue1Offset); - VerifyObjectField(kValue2Offset); -} - void Tuple3::Tuple3Verify() { CHECK(IsTuple3()); VerifyObjectField(kValue1Offset); diff --git a/src/objects-inl.h b/src/objects-inl.h index 970d521a94..8ec95cd613 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -5764,8 +5764,8 @@ ACCESSORS(PrototypeInfo, validity_cell, Object, kValidityCellOffset) SMI_ACCESSORS(PrototypeInfo, bit_field, kBitFieldOffset) BOOL_ACCESSORS(PrototypeInfo, bit_field, should_be_fast_map, kShouldBeFastBit) -ACCESSORS(Tuple2, value1, Object, kValue1Offset) -ACCESSORS(Tuple2, value2, Object, kValue2Offset) +ACCESSORS(Tuple3, value1, Object, kValue1Offset) +ACCESSORS(Tuple3, value2, Object, kValue2Offset) ACCESSORS(Tuple3, value3, Object, kValue3Offset) ACCESSORS(ContextExtension, scope_info, ScopeInfo, kScopeInfoOffset) diff --git a/src/objects-printer.cc b/src/objects-printer.cc index 18c4045676..94e0b99e30 100644 --- a/src/objects-printer.cc +++ b/src/objects-printer.cc @@ -1272,13 +1272,6 @@ void PrototypeInfo::PrototypeInfoPrint(std::ostream& os) { // NOLINT os << "\n"; } -void Tuple2::Tuple2Print(std::ostream& os) { // NOLINT - HeapObject::PrintHeader(os, "Tuple2"); - os << "\n - value1: " << Brief(value1()); - os << "\n - value2: " << Brief(value2()); - os << "\n"; -} - void Tuple3::Tuple3Print(std::ostream& os) { // NOLINT HeapObject::PrintHeader(os, "Tuple3"); os << "\n - value1: " << Brief(value1()); diff --git a/src/objects.cc b/src/objects.cc index a4e2cda8ec..199eeb3874 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -6163,10 +6163,9 @@ void JSObject::ResetElements(Handle object) { void JSObject::RequireSlowElements(SeededNumberDictionary* dictionary) { if (dictionary->requires_slow_elements()) return; dictionary->set_requires_slow_elements(); + // TODO(verwaest): Remove this hack. if (map()->is_prototype_map()) { - // If this object is a prototype (the callee will check), invalidate any - // prototype chains involving it. - InvalidatePrototypeChains(map()); + TypeFeedbackVector::ClearAllKeyedStoreICs(GetIsolate()); } } @@ -15621,6 +15620,9 @@ Maybe JSObject::SetPrototype(Handle object, // SpiderMonkey behaves this way. if (!value->IsJSReceiver() && !value->IsNull(isolate)) return Just(true); + bool dictionary_elements_in_chain = + object->map()->DictionaryElementsInPrototypeChainOnly(); + bool all_extensible = object->map()->is_extensible(); Handle real_receiver = object; if (from_javascript) { @@ -15686,6 +15688,14 @@ Maybe JSObject::SetPrototype(Handle object, DCHECK(new_map->prototype() == *value); JSObject::MigrateToMap(real_receiver, new_map); + if (from_javascript && !dictionary_elements_in_chain && + new_map->DictionaryElementsInPrototypeChainOnly()) { + // If the prototype chain didn't previously have element callbacks, then + // KeyedStoreICs need to be cleared to ensure any that involve this + // map go generic. + TypeFeedbackVector::ClearAllKeyedStoreICs(isolate); + } + heap->ClearInstanceofCache(); DCHECK(size == object->Size()); return Just(true); @@ -17382,7 +17392,7 @@ Handle JSObject::PrepareSlowElementsForSort( return bailout; } else { Handle result = SeededNumberDictionary::AddNumberEntry( - new_dict, pos, value, details, object); + new_dict, pos, value, details, object->map()->is_prototype_map()); DCHECK(result.is_identical_to(new_dict)); USE(result); pos++; @@ -17393,7 +17403,7 @@ Handle JSObject::PrepareSlowElementsForSort( return bailout; } else { Handle result = SeededNumberDictionary::AddNumberEntry( - new_dict, key, value, details, object); + new_dict, key, value, details, object->map()->is_prototype_map()); DCHECK(result.is_identical_to(new_dict)); USE(result); } @@ -17410,7 +17420,7 @@ Handle JSObject::PrepareSlowElementsForSort( HandleScope scope(isolate); Handle result = SeededNumberDictionary::AddNumberEntry( new_dict, pos, isolate->factory()->undefined_value(), no_details, - object); + object->map()->is_prototype_map()); DCHECK(result.is_identical_to(new_dict)); USE(result); pos++; @@ -18274,8 +18284,8 @@ bool SeededNumberDictionary::HasComplexElements() { return false; } -void SeededNumberDictionary::UpdateMaxNumberKey( - uint32_t key, Handle dictionary_holder) { +void SeededNumberDictionary::UpdateMaxNumberKey(uint32_t key, + bool used_as_prototype) { DisallowHeapAllocation no_allocation; // If the dictionary requires slow elements an element has already // been added at a high index. @@ -18283,8 +18293,9 @@ void SeededNumberDictionary::UpdateMaxNumberKey( // Check if this index is high enough that we should require slow // elements. if (key > kRequiresSlowElementsLimit) { - if (!dictionary_holder.is_null()) { - dictionary_holder->RequireSlowElements(this); + if (used_as_prototype) { + // TODO(verwaest): Remove this hack. + TypeFeedbackVector::ClearAllKeyedStoreICs(GetIsolate()); } set_requires_slow_elements(); return; @@ -18297,11 +18308,11 @@ void SeededNumberDictionary::UpdateMaxNumberKey( } } + Handle SeededNumberDictionary::AddNumberEntry( Handle dictionary, uint32_t key, - Handle value, PropertyDetails details, - Handle dictionary_holder) { - dictionary->UpdateMaxNumberKey(key, dictionary_holder); + Handle value, PropertyDetails details, bool used_as_prototype) { + dictionary->UpdateMaxNumberKey(key, used_as_prototype); SLOW_DCHECK(dictionary->FindEntry(key) == kNotFound); return Add(dictionary, key, value, details); } @@ -18329,8 +18340,8 @@ Handle UnseededNumberDictionary::DeleteKey( Handle SeededNumberDictionary::AtNumberPut( Handle dictionary, uint32_t key, - Handle value, Handle dictionary_holder) { - dictionary->UpdateMaxNumberKey(key, dictionary_holder); + Handle value, bool used_as_prototype) { + dictionary->UpdateMaxNumberKey(key, used_as_prototype); return AtPut(dictionary, key, value); } @@ -18342,13 +18353,13 @@ Handle UnseededNumberDictionary::AtNumberPut( return AtPut(dictionary, key, value); } + Handle SeededNumberDictionary::Set( Handle dictionary, uint32_t key, - Handle value, PropertyDetails details, - Handle dictionary_holder) { + Handle value, PropertyDetails details, bool used_as_prototype) { int entry = dictionary->FindEntry(key); if (entry == kNotFound) { - return AddNumberEntry(dictionary, key, value, details, dictionary_holder); + return AddNumberEntry(dictionary, key, value, details, used_as_prototype); } // Preserve enumeration index. details = details.set_index(dictionary->DetailsAt(entry).dictionary_index()); diff --git a/src/objects.h b/src/objects.h index 1754a0a37d..1de78c7e34 100644 --- a/src/objects.h +++ b/src/objects.h @@ -402,7 +402,6 @@ const int kStubMinorKeyBits = kSmiValueSize - kStubMajorKeyBits - 1; V(PROMISE_RESOLVE_THENABLE_JOB_INFO_TYPE) \ V(PROMISE_REACTION_JOB_INFO_TYPE) \ V(PROTOTYPE_INFO_TYPE) \ - V(TUPLE2_TYPE) \ V(TUPLE3_TYPE) \ V(CONTEXT_EXTENSION_TYPE) \ V(MODULE_TYPE) \ @@ -570,7 +569,6 @@ const int kStubMinorKeyBits = kSmiValueSize - kStubMajorKeyBits - 1; V(DEBUG_INFO, DebugInfo, debug_info) \ V(BREAK_POINT_INFO, BreakPointInfo, break_point_info) \ V(PROTOTYPE_INFO, PrototypeInfo, prototype_info) \ - V(TUPLE2, Tuple2, tuple2) \ V(TUPLE3, Tuple3, tuple3) \ V(MODULE, Module, module) \ V(MODULE_INFO_ENTRY, ModuleInfoEntry, module_info_entry) \ @@ -750,7 +748,6 @@ enum InstanceType { TRANSITION_ARRAY_TYPE, PROPERTY_CELL_TYPE, PROTOTYPE_INFO_TYPE, - TUPLE2_TYPE, TUPLE3_TYPE, CONTEXT_EXTENSION_TYPE, MODULE_TYPE, @@ -4028,20 +4025,18 @@ class SeededNumberDictionary // Type specific at put (default NONE attributes is used when adding). MUST_USE_RESULT static Handle AtNumberPut( Handle dictionary, uint32_t key, - Handle value, Handle dictionary_holder); + Handle value, bool used_as_prototype); MUST_USE_RESULT static Handle AddNumberEntry( Handle dictionary, uint32_t key, - Handle value, PropertyDetails details, - Handle dictionary_holder); + Handle value, PropertyDetails details, bool used_as_prototype); // Set an existing entry or add a new one if needed. // Return the updated dictionary. MUST_USE_RESULT static Handle Set( Handle dictionary, uint32_t key, - Handle value, PropertyDetails details, - Handle dictionary_holder); + Handle value, PropertyDetails details, bool used_as_prototype); - void UpdateMaxNumberKey(uint32_t key, Handle dictionary_holder); + void UpdateMaxNumberKey(uint32_t key, bool used_as_prototype); // Returns true if the dictionary contains any elements that are non-writable, // non-configurable, non-enumerable, or have getters/setters. @@ -6975,27 +6970,10 @@ class PrototypeInfo : public Struct { DISALLOW_IMPLICIT_CONSTRUCTORS(PrototypeInfo); }; -class Tuple2 : public Struct { +class Tuple3 : public Struct { public: DECL_ACCESSORS(value1, Object) DECL_ACCESSORS(value2, Object) - - DECLARE_CAST(Tuple2) - - // Dispatched behavior. - DECLARE_PRINTER(Tuple2) - DECLARE_VERIFIER(Tuple2) - - static const int kValue1Offset = HeapObject::kHeaderSize; - static const int kValue2Offset = kValue1Offset + kPointerSize; - static const int kSize = kValue2Offset + kPointerSize; - - private: - DISALLOW_IMPLICIT_CONSTRUCTORS(Tuple2); -}; - -class Tuple3 : public Tuple2 { - public: DECL_ACCESSORS(value3, Object) DECLARE_CAST(Tuple3) @@ -7004,7 +6982,9 @@ class Tuple3 : public Tuple2 { DECLARE_PRINTER(Tuple3) DECLARE_VERIFIER(Tuple3) - static const int kValue3Offset = Tuple2::kSize; + static const int kValue1Offset = HeapObject::kHeaderSize; + static const int kValue2Offset = kValue1Offset + kPointerSize; + static const int kValue3Offset = kValue2Offset + kPointerSize; static const int kSize = kValue3Offset + kPointerSize; private: diff --git a/src/type-feedback-vector.cc b/src/type-feedback-vector.cc index ac584ad8a7..04459acf15 100644 --- a/src/type-feedback-vector.cc +++ b/src/type-feedback-vector.cc @@ -326,6 +326,50 @@ void TypeFeedbackVector::ClearSlotsImpl(SharedFunctionInfo* shared, } +// static +void TypeFeedbackVector::ClearAllKeyedStoreICs(Isolate* isolate) { + SharedFunctionInfo::Iterator iterator(isolate); + SharedFunctionInfo* shared; + while ((shared = iterator.Next())) { + if (!shared->OptimizedCodeMapIsCleared()) { + FixedArray* optimized_code_map = shared->optimized_code_map(); + int length = optimized_code_map->length(); + for (int i = SharedFunctionInfo::kEntriesStart; i < length; + i += SharedFunctionInfo::kEntryLength) { + WeakCell* cell = WeakCell::cast( + optimized_code_map->get(i + SharedFunctionInfo::kLiteralsOffset)); + if (cell->value()->IsLiteralsArray()) { + TypeFeedbackVector* vector = + LiteralsArray::cast(cell->value())->feedback_vector(); + vector->ClearKeyedStoreICs(shared); + } + } + } + } +} + + +void TypeFeedbackVector::ClearKeyedStoreICs(SharedFunctionInfo* shared) { + Isolate* isolate = GetIsolate(); + + Code* host = shared->code(); + Object* uninitialized_sentinel = + TypeFeedbackVector::RawUninitializedSentinel(isolate); + + TypeFeedbackMetadataIterator iter(metadata()); + while (iter.HasNext()) { + FeedbackVectorSlot slot = iter.Next(); + FeedbackVectorSlotKind kind = iter.kind(); + if (kind != FeedbackVectorSlotKind::KEYED_STORE_IC) continue; + Object* obj = Get(slot); + if (obj != uninitialized_sentinel) { + KeyedStoreICNexus nexus(this, slot); + nexus.Clear(host); + } + } +} + + // static Handle TypeFeedbackVector::DummyVector(Isolate* isolate) { return isolate->factory()->dummy_vector(); @@ -716,9 +760,10 @@ void KeyedStoreICNexus::ConfigurePolymorphic(Handle name, InstallHandlers(array, maps, handlers); } + void KeyedStoreICNexus::ConfigurePolymorphic(MapHandleList* maps, MapHandleList* transitioned_maps, - List>* handlers) { + CodeHandleList* handlers) { int receiver_count = maps->length(); DCHECK(receiver_count > 1); Handle array = EnsureArrayOfSize(receiver_count * 3); @@ -915,14 +960,7 @@ KeyedAccessStoreMode KeyedStoreICNexus::GetKeyedAccessStoreMode() const { FindHandlers(&handlers, maps.length()); for (int i = 0; i < handlers.length(); i++) { // The first handler that isn't the slow handler will have the bits we need. - Handle maybe_code_handler = handlers.at(i); - Handle handler; - if (maybe_code_handler->IsTuple2()) { - Handle data_handler = Handle::cast(maybe_code_handler); - handler = handle(Code::cast(data_handler->value2())); - } else { - handler = Handle::cast(maybe_code_handler); - } + Handle handler = Handle::cast(handlers.at(i)); CodeStub::Major major_key = CodeStub::MajorKeyFromKey(handler->stub_key()); uint32_t minor_key = CodeStub::MinorKeyFromKey(handler->stub_key()); CHECK(major_key == CodeStub::KeyedStoreSloppyArguments || diff --git a/src/type-feedback-vector.h b/src/type-feedback-vector.h index 4b62ee86b6..069c831858 100644 --- a/src/type-feedback-vector.h +++ b/src/type-feedback-vector.h @@ -264,6 +264,9 @@ class TypeFeedbackVector : public FixedArray { ClearSlotsImpl(shared, false); } + static void ClearAllKeyedStoreICs(Isolate* isolate); + void ClearKeyedStoreICs(SharedFunctionInfo* shared); + // The object that indicates an uninitialized cache. static inline Handle UninitializedSentinel(Isolate* isolate); @@ -603,7 +606,7 @@ class KeyedStoreICNexus : public FeedbackNexus { List>* handlers); void ConfigurePolymorphic(MapHandleList* maps, MapHandleList* transitioned_maps, - List>* handlers); + CodeHandleList* handlers); void ConfigureMegamorphicKeyed(IcCheckType property_type); KeyedAccessStoreMode GetKeyedAccessStoreMode() const; diff --git a/src/value-serializer.cc b/src/value-serializer.cc index 81301fd553..c6abb8a85c 100644 --- a/src/value-serializer.cc +++ b/src/value-serializer.cc @@ -1006,10 +1006,10 @@ void ValueDeserializer::TransferArrayBuffer( } Handle dictionary = array_buffer_transfer_map_.ToHandleChecked(); - Handle not_a_prototype_holder; + const bool used_as_prototype = false; Handle new_dictionary = SeededNumberDictionary::AtNumberPut(dictionary, transfer_id, array_buffer, - not_a_prototype_holder); + used_as_prototype); if (!new_dictionary.is_identical_to(dictionary)) { GlobalHandles::Destroy(Handle::cast(dictionary).location()); array_buffer_transfer_map_ = Handle::cast( diff --git a/test/mjsunit/regress/regress-crbug-662907.js b/test/mjsunit/regress/regress-crbug-662907.js deleted file mode 100644 index 3cc3b600e2..0000000000 --- a/test/mjsunit/regress/regress-crbug-662907.js +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2016 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 --expose-gc - -(function() { - function foo() { - var a = new Array(); - a[0] = 10; - return a; - } - - assertEquals(1, foo().length); - - gc(); - gc(); - gc(); - gc(); - - // Change prototype elements from fast smi to slow elements dictionary. - // The validity cell is invalidated by the change of Array.prototype's - // map. - Array.prototype.__defineSetter__("0", function() {}); - - assertEquals(0, foo().length); -})(); - - -(function() { - function foo() { - var a = new Array(); - a[0] = 10; - return a; - } - - // Change prototype elements from fast smi to dictionary. - Array.prototype[123456789] = 42; - Array.prototype.length = 0; - - assertEquals(1, foo().length); - - gc(); - gc(); - gc(); - gc(); - - // Change prototype elements from dictionary to slow elements dictionary. - // The validity cell is invalidated by making the elements dictionary slow. - Array.prototype.__defineSetter__("0", function() {}); - - assertEquals(0, foo().length); -})();