From 7d6ada27147d7f3136aeca8d0400232b4883e155 Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Wed, 16 Oct 2019 19:06:32 -0700 Subject: [PATCH] [protectors] Update protectors in DefineClass DefineClass uses the ClassBoilerplate to directly construct the property descriptor array or dictionary for defining the class constructor and prototype, skipping use of the LookupIterator and the encapsulated protector update logic. This patch adds manual calls to UpdateProtector(), which is in particular relevant for the isConcatSpreadable protector. Bug: v8:9837 Change-Id: I7b9d8105d41f5f0f826ca2ce35d6bf3d1aeee6e7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1863644 Commit-Queue: Shu-yu Guo Reviewed-by: Jakob Kummerow Cr-Commit-Position: refs/heads/master@{#64368} --- src/objects/lookup-inl.h | 22 ++-- src/objects/lookup.cc | 213 +++++++++++++++++---------------- src/objects/lookup.h | 5 +- src/runtime/runtime-classes.cc | 25 ++++ test/mjsunit/mjsunit.status | 3 - 5 files changed, 151 insertions(+), 117 deletions(-) diff --git a/src/objects/lookup-inl.h b/src/objects/lookup-inl.h index fe206b91c2..994a961162 100644 --- a/src/objects/lookup-inl.h +++ b/src/objects/lookup-inl.h @@ -123,19 +123,25 @@ bool LookupIterator::IsCacheableTransition() { transition_map()->GetBackPointer(isolate_).IsMap(isolate_); } -void LookupIterator::UpdateProtector() { - if (IsElement()) return; +// static +void LookupIterator::UpdateProtector(Isolate* isolate, Handle receiver, + Handle name) { // This list must be kept in sync with // CodeStubAssembler::CheckForAssociatedProtector! - ReadOnlyRoots roots(isolate_); - if (*name_ == roots.is_concat_spreadable_symbol() || - *name_ == roots.constructor_string() || *name_ == roots.next_string() || - *name_ == roots.species_symbol() || *name_ == roots.iterator_symbol() || - *name_ == roots.resolve_string() || *name_ == roots.then_string()) { - InternalUpdateProtector(); + ReadOnlyRoots roots(isolate); + if (*name == roots.is_concat_spreadable_symbol() || + *name == roots.constructor_string() || *name == roots.next_string() || + *name == roots.species_symbol() || *name == roots.iterator_symbol() || + *name == roots.resolve_string() || *name == roots.then_string()) { + InternalUpdateProtector(isolate, receiver, name); } } +void LookupIterator::UpdateProtector() { + if (IsElement()) return; + UpdateProtector(isolate_, receiver_, name_); +} + InternalIndex LookupIterator::descriptor_number() const { DCHECK(!IsElement()); DCHECK(has_property_); diff --git a/src/objects/lookup.cc b/src/objects/lookup.cc index 018b86918d..df53635e77 100644 --- a/src/objects/lookup.cc +++ b/src/objects/lookup.cc @@ -231,195 +231,198 @@ bool IsTypedArrayFunctionInAnyContext(Isolate* isolate, HeapObject object) { } // namespace -void LookupIterator::InternalUpdateProtector() { - if (isolate_->bootstrapper()->IsActive()) return; - if (!receiver_->IsHeapObject()) return; - Handle receiver = Handle::cast(receiver_); +// static +void LookupIterator::InternalUpdateProtector(Isolate* isolate, + Handle receiver_generic, + Handle name) { + if (isolate->bootstrapper()->IsActive()) return; + if (!receiver_generic->IsHeapObject()) return; + Handle receiver = Handle::cast(receiver_generic); // Getting the native_context from the isolate as a fallback. If possible, we // use the receiver's creation context instead. - Handle native_context = isolate_->native_context(); + Handle native_context = isolate->native_context(); - ReadOnlyRoots roots(isolate_); - if (*name_ == roots.constructor_string()) { + ReadOnlyRoots roots(isolate); + if (*name == roots.constructor_string()) { // Fetching the context in here since the operation is rather expensive. if (receiver->IsJSReceiver()) { native_context = Handle::cast(receiver)->GetCreationContext(); } - if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_) && - !Protectors::IsPromiseSpeciesLookupChainIntact(isolate_) && + if (!Protectors::IsArraySpeciesLookupChainIntact(isolate) && + !Protectors::IsPromiseSpeciesLookupChainIntact(isolate) && !Protectors::IsRegExpSpeciesLookupChainProtectorIntact( native_context) && - !Protectors::IsTypedArraySpeciesLookupChainIntact(isolate_)) { + !Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) { return; } // Setting the constructor property could change an instance's @@species - if (receiver->IsJSArray(isolate_)) { - if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_)) return; - isolate_->CountUsage( + if (receiver->IsJSArray(isolate)) { + if (!Protectors::IsArraySpeciesLookupChainIntact(isolate)) return; + isolate->CountUsage( v8::Isolate::UseCounterFeature::kArrayInstanceConstructorModified); - Protectors::InvalidateArraySpeciesLookupChain(isolate_); + Protectors::InvalidateArraySpeciesLookupChain(isolate); return; - } else if (receiver->IsJSPromise(isolate_)) { - if (!Protectors::IsPromiseSpeciesLookupChainIntact(isolate_)) return; - Protectors::InvalidatePromiseSpeciesLookupChain(isolate_); + } else if (receiver->IsJSPromise(isolate)) { + if (!Protectors::IsPromiseSpeciesLookupChainIntact(isolate)) return; + Protectors::InvalidatePromiseSpeciesLookupChain(isolate); return; - } else if (receiver->IsJSRegExp(isolate_)) { + } else if (receiver->IsJSRegExp(isolate)) { if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact( native_context)) { return; } - Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate_, + Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate, native_context); return; - } else if (receiver->IsJSTypedArray(isolate_)) { - if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate_)) return; - Protectors::InvalidateTypedArraySpeciesLookupChain(isolate_); + } else if (receiver->IsJSTypedArray(isolate)) { + if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) return; + Protectors::InvalidateTypedArraySpeciesLookupChain(isolate); return; } - if (receiver->map(isolate_).is_prototype_map()) { + if (receiver->map(isolate).is_prototype_map()) { DisallowHeapAllocation no_gc; // Setting the constructor of any prototype with the @@species protector // (of any realm) also needs to invalidate the protector. // For typed arrays, we check a prototype of this receiver since // TypedArrays have different prototypes for each type, and their parent // prototype is pointing the same TYPED_ARRAY_PROTOTYPE. - if (isolate_->IsInAnyContext(*receiver, - Context::INITIAL_ARRAY_PROTOTYPE_INDEX)) { - if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_)) return; - isolate_->CountUsage( + if (isolate->IsInAnyContext(*receiver, + Context::INITIAL_ARRAY_PROTOTYPE_INDEX)) { + if (!Protectors::IsArraySpeciesLookupChainIntact(isolate)) return; + isolate->CountUsage( v8::Isolate::UseCounterFeature::kArrayPrototypeConstructorModified); - Protectors::InvalidateArraySpeciesLookupChain(isolate_); - } else if (isolate_->IsInAnyContext(*receiver, - Context::PROMISE_PROTOTYPE_INDEX)) { - if (!Protectors::IsPromiseSpeciesLookupChainIntact(isolate_)) return; - Protectors::InvalidatePromiseSpeciesLookupChain(isolate_); - } else if (isolate_->IsInAnyContext(*receiver, - Context::REGEXP_PROTOTYPE_INDEX)) { + Protectors::InvalidateArraySpeciesLookupChain(isolate); + } else if (isolate->IsInAnyContext(*receiver, + Context::PROMISE_PROTOTYPE_INDEX)) { + if (!Protectors::IsPromiseSpeciesLookupChainIntact(isolate)) return; + Protectors::InvalidatePromiseSpeciesLookupChain(isolate); + } else if (isolate->IsInAnyContext(*receiver, + Context::REGEXP_PROTOTYPE_INDEX)) { if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact( native_context)) { return; } - Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate_, + Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate, native_context); - } else if (isolate_->IsInAnyContext( - receiver->map(isolate_).prototype(isolate_), + } else if (isolate->IsInAnyContext( + receiver->map(isolate).prototype(isolate), Context::TYPED_ARRAY_PROTOTYPE_INDEX)) { - if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate_)) return; - Protectors::InvalidateTypedArraySpeciesLookupChain(isolate_); + if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) return; + Protectors::InvalidateTypedArraySpeciesLookupChain(isolate); } } - } else if (*name_ == roots.next_string()) { + } else if (*name == roots.next_string()) { if (receiver->IsJSArrayIterator() || - isolate_->IsInAnyContext( + isolate->IsInAnyContext( *receiver, Context::INITIAL_ARRAY_ITERATOR_PROTOTYPE_INDEX)) { // Setting the next property of %ArrayIteratorPrototype% also needs to // invalidate the array iterator protector. - if (!Protectors::IsArrayIteratorLookupChainIntact(isolate_)) return; - Protectors::InvalidateArrayIteratorLookupChain(isolate_); + if (!Protectors::IsArrayIteratorLookupChainIntact(isolate)) return; + Protectors::InvalidateArrayIteratorLookupChain(isolate); } else if (receiver->IsJSMapIterator() || - isolate_->IsInAnyContext( + isolate->IsInAnyContext( *receiver, Context::INITIAL_MAP_ITERATOR_PROTOTYPE_INDEX)) { - if (!Protectors::IsMapIteratorLookupChainIntact(isolate_)) return; - Protectors::InvalidateMapIteratorLookupChain(isolate_); + if (!Protectors::IsMapIteratorLookupChainIntact(isolate)) return; + Protectors::InvalidateMapIteratorLookupChain(isolate); } else if (receiver->IsJSSetIterator() || - isolate_->IsInAnyContext( + isolate->IsInAnyContext( *receiver, Context::INITIAL_SET_ITERATOR_PROTOTYPE_INDEX)) { - if (!Protectors::IsSetIteratorLookupChainIntact(isolate_)) return; - Protectors::InvalidateSetIteratorLookupChain(isolate_); + if (!Protectors::IsSetIteratorLookupChainIntact(isolate)) return; + Protectors::InvalidateSetIteratorLookupChain(isolate); } else if (receiver->IsJSStringIterator() || - isolate_->IsInAnyContext( + isolate->IsInAnyContext( *receiver, Context::INITIAL_STRING_ITERATOR_PROTOTYPE_INDEX)) { // Setting the next property of %StringIteratorPrototype% invalidates the // string iterator protector. - if (!Protectors::IsStringIteratorLookupChainIntact(isolate_)) return; - Protectors::InvalidateStringIteratorLookupChain(isolate_); + if (!Protectors::IsStringIteratorLookupChainIntact(isolate)) return; + Protectors::InvalidateStringIteratorLookupChain(isolate); } - } else if (*name_ == roots.species_symbol()) { + } else if (*name == roots.species_symbol()) { // Fetching the context in here since the operation is rather expensive. if (receiver->IsJSReceiver()) { native_context = Handle::cast(receiver)->GetCreationContext(); } - if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_) && - !Protectors::IsPromiseSpeciesLookupChainIntact(isolate_) && + if (!Protectors::IsArraySpeciesLookupChainIntact(isolate) && + !Protectors::IsPromiseSpeciesLookupChainIntact(isolate) && !Protectors::IsRegExpSpeciesLookupChainProtectorIntact( native_context) && - !Protectors::IsTypedArraySpeciesLookupChainIntact(isolate_)) { + !Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) { return; } // Setting the Symbol.species property of any Array, Promise or TypedArray // constructor invalidates the @@species protector - if (isolate_->IsInAnyContext(*receiver, Context::ARRAY_FUNCTION_INDEX)) { - if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_)) return; - isolate_->CountUsage( + if (isolate->IsInAnyContext(*receiver, Context::ARRAY_FUNCTION_INDEX)) { + if (!Protectors::IsArraySpeciesLookupChainIntact(isolate)) return; + isolate->CountUsage( v8::Isolate::UseCounterFeature::kArraySpeciesModified); - Protectors::InvalidateArraySpeciesLookupChain(isolate_); - } else if (isolate_->IsInAnyContext(*receiver, - Context::PROMISE_FUNCTION_INDEX)) { - if (!Protectors::IsPromiseSpeciesLookupChainIntact(isolate_)) return; - Protectors::InvalidatePromiseSpeciesLookupChain(isolate_); - } else if (isolate_->IsInAnyContext(*receiver, - Context::REGEXP_FUNCTION_INDEX)) { + Protectors::InvalidateArraySpeciesLookupChain(isolate); + } else if (isolate->IsInAnyContext(*receiver, + Context::PROMISE_FUNCTION_INDEX)) { + if (!Protectors::IsPromiseSpeciesLookupChainIntact(isolate)) return; + Protectors::InvalidatePromiseSpeciesLookupChain(isolate); + } else if (isolate->IsInAnyContext(*receiver, + Context::REGEXP_FUNCTION_INDEX)) { if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact( native_context)) { return; } - Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate_, + Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate, native_context); - } else if (IsTypedArrayFunctionInAnyContext(isolate_, *receiver)) { - if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate_)) return; - Protectors::InvalidateTypedArraySpeciesLookupChain(isolate_); + } else if (IsTypedArrayFunctionInAnyContext(isolate, *receiver)) { + if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) return; + Protectors::InvalidateTypedArraySpeciesLookupChain(isolate); } - } else if (*name_ == roots.is_concat_spreadable_symbol()) { - if (!Protectors::IsIsConcatSpreadableLookupChainIntact(isolate_)) return; - Protectors::InvalidateIsConcatSpreadableLookupChain(isolate_); - } else if (*name_ == roots.iterator_symbol()) { - if (receiver->IsJSArray(isolate_)) { - if (!Protectors::IsArrayIteratorLookupChainIntact(isolate_)) return; - Protectors::InvalidateArrayIteratorLookupChain(isolate_); - } else if (receiver->IsJSSet(isolate_) || receiver->IsJSSetIterator() || - isolate_->IsInAnyContext( + } else if (*name == roots.is_concat_spreadable_symbol()) { + if (!Protectors::IsIsConcatSpreadableLookupChainIntact(isolate)) return; + Protectors::InvalidateIsConcatSpreadableLookupChain(isolate); + } else if (*name == roots.iterator_symbol()) { + if (receiver->IsJSArray(isolate)) { + if (!Protectors::IsArrayIteratorLookupChainIntact(isolate)) return; + Protectors::InvalidateArrayIteratorLookupChain(isolate); + } else if (receiver->IsJSSet(isolate) || receiver->IsJSSetIterator() || + isolate->IsInAnyContext( *receiver, Context::INITIAL_SET_ITERATOR_PROTOTYPE_INDEX) || - isolate_->IsInAnyContext(*receiver, - Context::INITIAL_SET_PROTOTYPE_INDEX)) { - if (Protectors::IsSetIteratorLookupChainIntact(isolate_)) { - Protectors::InvalidateSetIteratorLookupChain(isolate_); + isolate->IsInAnyContext(*receiver, + Context::INITIAL_SET_PROTOTYPE_INDEX)) { + if (Protectors::IsSetIteratorLookupChainIntact(isolate)) { + Protectors::InvalidateSetIteratorLookupChain(isolate); } } else if (receiver->IsJSMapIterator() || - isolate_->IsInAnyContext( + isolate->IsInAnyContext( *receiver, Context::INITIAL_MAP_ITERATOR_PROTOTYPE_INDEX)) { - if (Protectors::IsMapIteratorLookupChainIntact(isolate_)) { - Protectors::InvalidateMapIteratorLookupChain(isolate_); + if (Protectors::IsMapIteratorLookupChainIntact(isolate)) { + Protectors::InvalidateMapIteratorLookupChain(isolate); } - } else if (isolate_->IsInAnyContext( + } else if (isolate->IsInAnyContext( *receiver, Context::INITIAL_ITERATOR_PROTOTYPE_INDEX)) { - if (Protectors::IsMapIteratorLookupChainIntact(isolate_)) { - Protectors::InvalidateMapIteratorLookupChain(isolate_); + if (Protectors::IsMapIteratorLookupChainIntact(isolate)) { + Protectors::InvalidateMapIteratorLookupChain(isolate); } - if (Protectors::IsSetIteratorLookupChainIntact(isolate_)) { - Protectors::InvalidateSetIteratorLookupChain(isolate_); + if (Protectors::IsSetIteratorLookupChainIntact(isolate)) { + Protectors::InvalidateSetIteratorLookupChain(isolate); } - } else if (isolate_->IsInAnyContext( + } else if (isolate->IsInAnyContext( *receiver, Context::INITIAL_STRING_PROTOTYPE_INDEX)) { // Setting the Symbol.iterator property of String.prototype invalidates // the string iterator protector. Symbol.iterator can also be set on a // String wrapper, but not on a primitive string. We only support // protector for primitive strings. - if (!Protectors::IsStringIteratorLookupChainIntact(isolate_)) return; - Protectors::InvalidateStringIteratorLookupChain(isolate_); + if (!Protectors::IsStringIteratorLookupChainIntact(isolate)) return; + Protectors::InvalidateStringIteratorLookupChain(isolate); } - } else if (*name_ == roots.resolve_string()) { - if (!Protectors::IsPromiseResolveLookupChainIntact(isolate_)) return; + } else if (*name == roots.resolve_string()) { + if (!Protectors::IsPromiseResolveLookupChainIntact(isolate)) return; // Setting the "resolve" property on any %Promise% intrinsic object // invalidates the Promise.resolve protector. - if (isolate_->IsInAnyContext(*receiver, Context::PROMISE_FUNCTION_INDEX)) { - Protectors::InvalidatePromiseResolveLookupChain(isolate_); + if (isolate->IsInAnyContext(*receiver, Context::PROMISE_FUNCTION_INDEX)) { + Protectors::InvalidatePromiseResolveLookupChain(isolate); } - } else if (*name_ == roots.then_string()) { - if (!Protectors::IsPromiseThenLookupChainIntact(isolate_)) return; + } else if (*name == roots.then_string()) { + if (!Protectors::IsPromiseThenLookupChainIntact(isolate)) return; // Setting the "then" property on any JSPromise instance or on the // initial %PromisePrototype% invalidates the Promise#then protector. // Also setting the "then" property on the initial %ObjectPrototype% @@ -427,11 +430,11 @@ void LookupIterator::InternalUpdateProtector() { // to guard the fast-path in AsyncGeneratorResolve, where we can skip // the ResolvePromise step and go directly to FulfillPromise if we // know that the Object.prototype doesn't contain a "then" method. - if (receiver->IsJSPromise(isolate_) || - isolate_->IsInAnyContext(*receiver, - Context::INITIAL_OBJECT_PROTOTYPE_INDEX) || - isolate_->IsInAnyContext(*receiver, Context::PROMISE_PROTOTYPE_INDEX)) { - Protectors::InvalidatePromiseThenLookupChain(isolate_); + if (receiver->IsJSPromise(isolate) || + isolate->IsInAnyContext(*receiver, + Context::INITIAL_OBJECT_PROTOTYPE_INDEX) || + isolate->IsInAnyContext(*receiver, Context::PROMISE_PROTOTYPE_INDEX)) { + Protectors::InvalidatePromiseThenLookupChain(isolate); } } } diff --git a/src/objects/lookup.h b/src/objects/lookup.h index fbec76f8f7..145bceb1c3 100644 --- a/src/objects/lookup.h +++ b/src/objects/lookup.h @@ -182,6 +182,8 @@ class V8_EXPORT_PRIVATE LookupIterator final { Handle GetDataValue() const; void WriteDataValue(Handle value, bool initializing_store); inline void UpdateProtector(); + static inline void UpdateProtector(Isolate* isolate, Handle receiver, + Handle name); // Lookup a 'cached' private property for an accessor. // If not found returns false and leaves the LookupIterator unmodified. @@ -194,7 +196,8 @@ class V8_EXPORT_PRIVATE LookupIterator final { Handle transition_map, PropertyDetails details, bool has_property); - void InternalUpdateProtector(); + static void InternalUpdateProtector(Isolate* isolate, Handle receiver, + Handle name); enum class InterceptorState { kUninitialized, diff --git a/src/runtime/runtime-classes.cc b/src/runtime/runtime-classes.cc index 49d82e8d40..c10fb6ab3b 100644 --- a/src/runtime/runtime-classes.cc +++ b/src/runtime/runtime-classes.cc @@ -17,6 +17,7 @@ #include "src/objects/elements.h" #include "src/objects/hash-table-inl.h" #include "src/objects/literal-objects-inl.h" +#include "src/objects/lookup-inl.h" #include "src/objects/smi.h" #include "src/objects/struct-inl.h" #include "src/runtime/runtime.h" @@ -282,6 +283,26 @@ bool SubstituteValues(Isolate* isolate, Handle dictionary, return true; } +void UpdateProtectors(Isolate* isolate, Handle receiver, + Handle properties_dictionary) { + ReadOnlyRoots roots(isolate); + for (InternalIndex i : properties_dictionary->IterateEntries()) { + Object maybe_key = properties_dictionary->KeyAt(i); + if (!NameDictionary::IsKey(roots, maybe_key)) continue; + Handle name(Name::cast(maybe_key), isolate); + LookupIterator::UpdateProtector(isolate, receiver, name); + } +} + +void UpdateProtectors(Isolate* isolate, Handle receiver, + Handle properties_template) { + int nof_descriptors = properties_template->number_of_descriptors(); + for (InternalIndex i : InternalIndex::Range(nof_descriptors)) { + Handle name(properties_template->GetKey(i), isolate); + LookupIterator::UpdateProtector(isolate, receiver, name); + } +} + bool AddDescriptorsByTemplate( Isolate* isolate, Handle map, Handle descriptors_template, @@ -368,6 +389,8 @@ bool AddDescriptorsByTemplate( } } + UpdateProtectors(isolate, receiver, descriptors_template); + map->InitializeDescriptors(isolate, *descriptors, LayoutDescriptor::FastPointerLayout()); if (elements_dictionary->NumberOfElements() > 0) { @@ -448,6 +471,8 @@ bool AddDescriptorsByTemplate( CHECK_EQ(*dict, *properties_dictionary); } + UpdateProtectors(isolate, receiver, properties_dictionary); + if (elements_dictionary->NumberOfElements() > 0) { if (!SubstituteValues(isolate, elements_dictionary, receiver, args)) { diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 623cba33e9..f2549fe68d 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -256,9 +256,6 @@ # Test doesn't work on 32-bit architectures (it would require a # regexp pattern with too many captures). 'regress/regress-976627': [FAIL, ['arch == x64 or arch == arm64 or arch == mips64el or arch == ppc64 or arch == s390x', PASS]], - - # BUG(v8:9837) - 'es6/array-concat-unspreadable-array-subclass': [FAIL], }], # ALWAYS ['novfp3 == True', {