From e23e345b6c6b66c7fac5c76741bbfef722953e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Tue, 18 Jan 2022 14:12:53 +0100 Subject: [PATCH] deps: V8: cherry-pick 80bbbb143c24 Original commit message: [class] handle existing readonly properties in StoreOwnIC Previously, StoreOwnIC incorrectly reuses the [[Set]] semantics when initializing public literal class fields and object literals in certain cases (e.g. when there's no feedback). This was less of an issue for object literals, but with public class fields it's possible to define property attributes while the instance is still being initialized, or to encounter existing static "name" or "length" properties that should be readonly. This patch fixes it by 1) Emitting code that calls into the slow stub when handling StoreOwnIC with existing read-only properties. 2) Adding extra steps in StoreIC::Store to handle such stores properly with [[DefineOwnProperty]] semantics. Bug: v8:12421, v8:9888 Change-Id: I6547320a1caba58c66ee1043cd3183a2de7cefef Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3300092 Reviewed-by: Shu-yu Guo Reviewed-by: Toon Verwaest Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#78659} Refs: https://github.com/v8/v8/commit/80bbbb143c2449f7a2109424a937d1887043c9ee PR-URL: https://github.com/nodejs/node/pull/40907 Reviewed-By: Jiawen Geng Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- common.gypi | 2 +- deps/v8/src/ic/ic.cc | 131 ++++++++- deps/v8/src/ic/keyed-store-generic.cc | 32 ++- deps/v8/src/objects/js-objects.cc | 82 ++++-- deps/v8/src/objects/js-objects.h | 22 +- deps/v8/src/objects/lookup.cc | 3 +- deps/v8/src/objects/objects.cc | 12 +- deps/v8/src/objects/objects.h | 7 + deps/v8/test/cctest/test-api-interceptors.cc | 197 +++++++++++++ ...proxy-super-return-define-property-trap.js | 31 +++ .../regress-v8-12421-no-lazy-feedback.js | 7 + .../test/mjsunit/regress/regress-v8-12421.js | 258 ++++++++++++++++++ 12 files changed, 727 insertions(+), 57 deletions(-) create mode 100644 deps/v8/test/mjsunit/proxy-super-return-define-property-trap.js create mode 100644 deps/v8/test/mjsunit/regress/regress-v8-12421-no-lazy-feedback.js create mode 100644 deps/v8/test/mjsunit/regress/regress-v8-12421.js diff --git a/common.gypi b/common.gypi index 8b323a5f10a4ea..4a71d03b9b1df8 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.11', + 'v8_embedder_string': '-node.12', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/ic/ic.cc b/deps/v8/src/ic/ic.cc index 6141377bfea5c0..b9f3d040a5f8f3 100644 --- a/deps/v8/src/ic/ic.cc +++ b/deps/v8/src/ic/ic.cc @@ -35,6 +35,7 @@ #include "src/objects/js-array-inl.h" #include "src/objects/megadom-handler.h" #include "src/objects/module-inl.h" +#include "src/objects/property-descriptor.h" #include "src/objects/prototype.h" #include "src/objects/struct-inl.h" #include "src/runtime/runtime-utils.h" @@ -1726,6 +1727,69 @@ MaybeHandle StoreGlobalIC::Store(Handle name, return StoreIC::Store(global, name, value); } +namespace { +Maybe DefineOwnDataProperty(LookupIterator* it, + LookupIterator::State original_state, + Handle value, + Maybe should_throw, + StoreOrigin store_origin) { + // It should not be possible to call DefineOwnDataProperty in a + // contextual store (indicated by IsJSGlobalObject()). + DCHECK(!it->GetReceiver()->IsJSGlobalObject(it->isolate())); + + // Handle special cases that can't be handled by + // DefineOwnPropertyIgnoreAttributes first. + switch (it->state()) { + case LookupIterator::JSPROXY: { + PropertyDescriptor new_desc; + new_desc.set_value(value); + new_desc.set_writable(true); + new_desc.set_enumerable(true); + new_desc.set_configurable(true); + DCHECK_EQ(original_state, LookupIterator::JSPROXY); + // TODO(joyee): this will start the lookup again. Ideally we should + // implement something that reuses the existing LookupIterator. + return JSProxy::DefineOwnProperty(it->isolate(), it->GetHolder(), + it->GetName(), &new_desc, should_throw); + } + // When lazy feedback is disabled, the original state could be different + // while the object is already prepared for TRANSITION. + case LookupIterator::TRANSITION: { + switch (original_state) { + case LookupIterator::JSPROXY: + case LookupIterator::TRANSITION: + case LookupIterator::DATA: + case LookupIterator::INTERCEPTOR: + case LookupIterator::ACCESSOR: + case LookupIterator::INTEGER_INDEXED_EXOTIC: + UNREACHABLE(); + case LookupIterator::ACCESS_CHECK: { + DCHECK(!it->GetHolder()->IsAccessCheckNeeded()); + V8_FALLTHROUGH; + } + case LookupIterator::NOT_FOUND: + return Object::AddDataProperty(it, value, NONE, + Nothing(), store_origin); + } + } + case LookupIterator::ACCESS_CHECK: + case LookupIterator::NOT_FOUND: + case LookupIterator::DATA: + case LookupIterator::ACCESSOR: + case LookupIterator::INTERCEPTOR: + case LookupIterator::INTEGER_INDEXED_EXOTIC: + break; + } + + // We need to restart to handle interceptors properly. + it->Restart(); + + return JSObject::DefineOwnPropertyIgnoreAttributes( + it, value, NONE, should_throw, JSObject::DONT_FORCE_FIELD, + JSObject::EnforceDefineSemantics::kDefine); +} +} // namespace + MaybeHandle StoreIC::Store(Handle object, Handle name, Handle value, StoreOrigin store_origin) { @@ -1737,7 +1801,15 @@ MaybeHandle StoreIC::Store(Handle object, Handle name, isolate(), object, key, IsAnyStoreOwn() ? LookupIterator::OWN : LookupIterator::DEFAULT); DCHECK_IMPLIES(IsStoreOwnIC(), it.IsFound() && it.HolderIsReceiver()); - MAYBE_RETURN_NULL(Object::SetProperty(&it, value, StoreOrigin::kNamed)); + // TODO(joyee): IsStoreOwnIC() is used in [[DefineOwnProperty]] + // operations during initialization of object literals and class + // fields. Rename them or separate them out. + if (IsStoreOwnIC()) { + MAYBE_RETURN_NULL( + JSReceiver::CreateDataProperty(&it, value, Nothing())); + } else { + MAYBE_RETURN_NULL(Object::SetProperty(&it, value, StoreOrigin::kNamed)); + } return value; } @@ -1785,6 +1857,23 @@ MaybeHandle StoreIC::Store(Handle object, Handle name, use_ic = false; } } + + // For IsStoreOwnIC(), we can't simply do CreateDataProperty below + // because we need to check the attributes before UpdateCaches updates + // the state of the LookupIterator. + LookupIterator::State original_state = it.state(); + // We'll defer the check for JSProxy and objects with named interceptors, + // because the defineProperty traps need to be called first if they are + // present. + if (IsStoreOwnIC() && !object->IsJSProxy() && + !Handle::cast(object)->HasNamedInterceptor()) { + Maybe can_define = JSReceiver::CheckIfCanDefine( + isolate(), &it, value, Nothing()); + if (can_define.IsNothing() || !can_define.FromJust()) { + return MaybeHandle(); + } + } + if (use_ic) { UpdateCaches(&it, value, store_origin); } else if (state() == NO_FEEDBACK) { @@ -1793,7 +1882,21 @@ MaybeHandle StoreIC::Store(Handle object, Handle name, : TraceIC("StoreIC", name); } - MAYBE_RETURN_NULL(Object::SetProperty(&it, value, store_origin)); + // TODO(joyee): IsStoreOwnIC() is true in [[DefineOwnProperty]] + // operations during initialization of object literals and class + // fields. In both paths, Rename the operations properly to avoid + // confusion. + // ES #sec-definefield + // ES #sec-runtime-semantics-propertydefinitionevaluation + if (IsStoreOwnIC()) { + // Private property should be defined via DefineOwnIC (as private names) or + // stored via other store ICs through private symbols. + DCHECK(!name->IsPrivate()); + MAYBE_RETURN_NULL(DefineOwnDataProperty( + &it, original_state, value, Nothing(), store_origin)); + } else { + MAYBE_RETURN_NULL(Object::SetProperty(&it, value, store_origin)); + } return value; } @@ -1871,11 +1974,14 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) { // If the interceptor is on the receiver... if (lookup->HolderIsReceiverOrHiddenPrototype() && !info.non_masking()) { - // ...return a store interceptor Smi handler if there is one... - if (!info.setter().IsUndefined(isolate())) { + // ...return a store interceptor Smi handler if there is a setter + // interceptor and it's not StoreOwnIC (which should call the + // definer)... + if (!info.setter().IsUndefined(isolate()) && !IsStoreOwnIC()) { return MaybeObjectHandle(StoreHandler::StoreInterceptor(isolate())); } - // ...otherwise return a slow-case Smi handler. + // ...otherwise return a slow-case Smi handler, which invokes the + // definer for StoreOwnIC. return MaybeObjectHandle(StoreHandler::StoreSlow(isolate())); } @@ -2055,6 +2161,14 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) { Handle receiver = Handle::cast(lookup->GetReceiver()); Handle holder = lookup->GetHolder(); + + // IsStoreOwnIC() is true when we are defining public fields on a Proxy. + // In that case use the slow stub to invoke the define trap. + if (IsStoreOwnIC()) { + TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub); + return MaybeObjectHandle(StoreHandler::StoreSlow(isolate())); + } + return MaybeObjectHandle(StoreHandler::StoreProxy( isolate(), lookup_start_object_map(), holder, receiver)); } @@ -2766,9 +2880,10 @@ RUNTIME_FUNCTION(Runtime_StoreOwnIC_Slow) { PropertyKey lookup_key(isolate, key); LookupIterator it(isolate, object, lookup_key, LookupIterator::OWN); - MAYBE_RETURN(JSObject::DefineOwnPropertyIgnoreAttributes( - &it, value, NONE, Nothing()), - ReadOnlyRoots(isolate).exception()); + + MAYBE_RETURN( + JSReceiver::CreateDataProperty(&it, value, Nothing()), + ReadOnlyRoots(isolate).exception()); return *value; } diff --git a/deps/v8/src/ic/keyed-store-generic.cc b/deps/v8/src/ic/keyed-store-generic.cc index 8f92911910d92c..cbb48ea201f490 100644 --- a/deps/v8/src/ic/keyed-store-generic.cc +++ b/deps/v8/src/ic/keyed-store-generic.cc @@ -151,7 +151,7 @@ class KeyedStoreGenericAssembler : public AccessorAssembler { bool ShouldCheckPrototype() const { return IsKeyedStore(); } bool ShouldReconfigureExisting() const { return IsStoreInLiteral(); } - bool ShouldCallSetter() const { return IsKeyedStore() || IsKeyedStoreOwn(); } + bool ShouldCallSetter() const { return IsKeyedStore(); } bool ShouldCheckPrototypeValidity() const { // We don't do this for "in-literal" stores, because it is impossible for // the target object to be a "prototype". @@ -1008,20 +1008,28 @@ void KeyedStoreGenericAssembler::EmitGenericPropertyStore( if (!ShouldReconfigureExisting()) { BIND(&readonly); { - LanguageMode language_mode; - if (maybe_language_mode.To(&language_mode)) { - if (language_mode == LanguageMode::kStrict) { - TNode type = Typeof(p->receiver()); - ThrowTypeError(p->context(), MessageTemplate::kStrictReadOnlyProperty, - name, type, p->receiver()); + // FIXME(joyee): IsKeyedStoreOwn is actually true from + // StaNamedOwnProperty, which implements [[DefineOwnProperty]] + // semantics. Rename them. + if (IsKeyedDefineOwn() || IsKeyedStoreOwn()) { + Goto(slow); + } else { + LanguageMode language_mode; + if (maybe_language_mode.To(&language_mode)) { + if (language_mode == LanguageMode::kStrict) { + TNode type = Typeof(p->receiver()); + ThrowTypeError(p->context(), + MessageTemplate::kStrictReadOnlyProperty, name, type, + p->receiver()); + } else { + exit_point->Return(p->value()); + } } else { + CallRuntime(Runtime::kThrowTypeErrorIfStrict, p->context(), + SmiConstant(MessageTemplate::kStrictReadOnlyProperty), + name, Typeof(p->receiver()), p->receiver()); exit_point->Return(p->value()); } - } else { - CallRuntime(Runtime::kThrowTypeErrorIfStrict, p->context(), - SmiConstant(MessageTemplate::kStrictReadOnlyProperty), name, - Typeof(p->receiver()), p->receiver()); - exit_point->Return(p->value()); } } } diff --git a/deps/v8/src/objects/js-objects.cc b/deps/v8/src/objects/js-objects.cc index 4c6809b56f9549..933c3cda0e217e 100644 --- a/deps/v8/src/objects/js-objects.cc +++ b/deps/v8/src/objects/js-objects.cc @@ -184,6 +184,27 @@ Maybe JSReceiver::HasInPrototypeChain(Isolate* isolate, } } +// static +Maybe JSReceiver::CheckIfCanDefine(Isolate* isolate, LookupIterator* it, + Handle value, + Maybe should_throw) { + if (it->IsFound()) { + Maybe attributes = GetPropertyAttributes(it); + MAYBE_RETURN(attributes, Nothing()); + if ((attributes.FromJust() & DONT_DELETE) != 0) { + RETURN_FAILURE( + isolate, GetShouldThrow(isolate, should_throw), + NewTypeError(MessageTemplate::kRedefineDisallowed, it->GetName())); + } + } else if (!JSObject::IsExtensible( + Handle::cast(it->GetReceiver()))) { + RETURN_FAILURE( + isolate, GetShouldThrow(isolate, should_throw), + NewTypeError(MessageTemplate::kDefineDisallowed, it->GetName())); + } + return Just(true); +} + namespace { bool HasExcludedProperty( @@ -3365,23 +3386,25 @@ void JSObject::AddProperty(Isolate* isolate, Handle object, // hidden prototypes. MaybeHandle JSObject::DefineOwnPropertyIgnoreAttributes( LookupIterator* it, Handle value, PropertyAttributes attributes, - AccessorInfoHandling handling) { + AccessorInfoHandling handling, EnforceDefineSemantics semantics) { MAYBE_RETURN_NULL(DefineOwnPropertyIgnoreAttributes( - it, value, attributes, Just(ShouldThrow::kThrowOnError), handling)); + it, value, attributes, Just(ShouldThrow::kThrowOnError), handling, + semantics)); return value; } Maybe JSObject::DefineOwnPropertyIgnoreAttributes( LookupIterator* it, Handle value, PropertyAttributes attributes, - Maybe should_throw, AccessorInfoHandling handling) { + Maybe should_throw, AccessorInfoHandling handling, + EnforceDefineSemantics semantics) { it->UpdateProtector(); Handle object = Handle::cast(it->GetReceiver()); for (; it->IsFound(); it->Next()) { switch (it->state()) { case LookupIterator::JSPROXY: - case LookupIterator::NOT_FOUND: case LookupIterator::TRANSITION: + case LookupIterator::NOT_FOUND: UNREACHABLE(); case LookupIterator::ACCESS_CHECK: @@ -3400,13 +3423,36 @@ Maybe JSObject::DefineOwnPropertyIgnoreAttributes( // TODO(verwaest): JSProxy afterwards verify the attributes that the // JSProxy claims it has, and verifies that they are compatible. If not, // they throw. Here we should do the same. - case LookupIterator::INTERCEPTOR: - if (handling == DONT_FORCE_FIELD) { - Maybe result = - JSObject::SetPropertyWithInterceptor(it, should_throw, value); - if (result.IsNothing() || result.FromJust()) return result; + case LookupIterator::INTERCEPTOR: { + Maybe result = Just(false); + if (semantics == EnforceDefineSemantics::kDefine) { + PropertyDescriptor descriptor; + descriptor.set_configurable((attributes & DONT_DELETE) != 0); + descriptor.set_enumerable((attributes & DONT_ENUM) != 0); + descriptor.set_writable((attributes & READ_ONLY) != 0); + descriptor.set_value(value); + result = DefinePropertyWithInterceptorInternal( + it, it->GetInterceptor(), should_throw, &descriptor); + } else { + DCHECK_EQ(semantics, EnforceDefineSemantics::kSet); + if (handling == DONT_FORCE_FIELD) { + result = + JSObject::SetPropertyWithInterceptor(it, should_throw, value); + } + } + if (result.IsNothing() || result.FromJust()) return result; + + if (semantics == EnforceDefineSemantics::kDefine) { + it->Restart(); + Maybe can_define = JSReceiver::CheckIfCanDefine( + it->isolate(), it, value, should_throw); + if (can_define.IsNothing() || !can_define.FromJust()) { + return can_define; + } + it->Restart(); } break; + } case LookupIterator::ACCESSOR: { Handle accessors = it->GetAccessors(); @@ -3824,20 +3870,10 @@ Maybe JSObject::CreateDataProperty(LookupIterator* it, Handle receiver = Handle::cast(it->GetReceiver()); Isolate* isolate = receiver->GetIsolate(); - if (it->IsFound()) { - Maybe attributes = GetPropertyAttributes(it); - MAYBE_RETURN(attributes, Nothing()); - if ((attributes.FromJust() & DONT_DELETE) != 0) { - RETURN_FAILURE( - isolate, GetShouldThrow(isolate, should_throw), - NewTypeError(MessageTemplate::kRedefineDisallowed, it->GetName())); - } - } else { - if (!JSObject::IsExtensible(Handle::cast(it->GetReceiver()))) { - RETURN_FAILURE( - isolate, GetShouldThrow(isolate, should_throw), - NewTypeError(MessageTemplate::kDefineDisallowed, it->GetName())); - } + Maybe can_define = + JSReceiver::CheckIfCanDefine(isolate, it, value, should_throw); + if (can_define.IsNothing() || !can_define.FromJust()) { + return can_define; } RETURN_ON_EXCEPTION_VALUE(it->isolate(), diff --git a/deps/v8/src/objects/js-objects.h b/deps/v8/src/objects/js-objects.h index 8eff066bb17893..3904144e4084f8 100644 --- a/deps/v8/src/objects/js-objects.h +++ b/deps/v8/src/objects/js-objects.h @@ -160,6 +160,12 @@ class JSReceiver : public TorqueGeneratedJSReceiver { Isolate* isolate, Handle object, Handle key, PropertyDescriptor* desc, Maybe should_throw); + // Check if a data property can be created on the object. It will fail with + // an error when it cannot. + V8_WARN_UNUSED_RESULT static Maybe CheckIfCanDefine( + Isolate* isolate, LookupIterator* it, Handle value, + Maybe should_throw); + // ES6 7.3.4 (when passed kDontThrow) V8_WARN_UNUSED_RESULT static Maybe CreateDataProperty( Isolate* isolate, Handle object, Handle key, @@ -399,15 +405,27 @@ class JSObject : public TorqueGeneratedJSObject { // to the default behavior that calls the setter. enum AccessorInfoHandling { FORCE_FIELD, DONT_FORCE_FIELD }; + // Currently DefineOwnPropertyIgnoreAttributes invokes the setter + // interceptor and user-defined setters during define operations, + // even in places where it makes more sense to invoke the definer + // interceptor and not invoke the setter: e.g. both the definer and + // the setter interceptors are called in Object.defineProperty(). + // kDefine allows us to implement the define semantics correctly + // in selected locations. + // TODO(joyee): see if we can deprecate the old behavior. + enum class EnforceDefineSemantics { kSet, kDefine }; + V8_WARN_UNUSED_RESULT static MaybeHandle DefineOwnPropertyIgnoreAttributes( LookupIterator* it, Handle value, PropertyAttributes attributes, - AccessorInfoHandling handling = DONT_FORCE_FIELD); + AccessorInfoHandling handling = DONT_FORCE_FIELD, + EnforceDefineSemantics semantics = EnforceDefineSemantics::kSet); V8_WARN_UNUSED_RESULT static Maybe DefineOwnPropertyIgnoreAttributes( LookupIterator* it, Handle value, PropertyAttributes attributes, Maybe should_throw, - AccessorInfoHandling handling = DONT_FORCE_FIELD); + AccessorInfoHandling handling = DONT_FORCE_FIELD, + EnforceDefineSemantics semantics = EnforceDefineSemantics::kSet); V8_WARN_UNUSED_RESULT static MaybeHandle V8_EXPORT_PRIVATE SetOwnPropertyIgnoreAttributes(Handle object, Handle name, diff --git a/deps/v8/src/objects/lookup.cc b/deps/v8/src/objects/lookup.cc index b53bbeb0e7d183..5b10337e41ec4b 100644 --- a/deps/v8/src/objects/lookup.cc +++ b/deps/v8/src/objects/lookup.cc @@ -167,7 +167,8 @@ Handle LookupIterator::GetReceiverMap() const { } bool LookupIterator::HasAccess() const { - DCHECK_EQ(ACCESS_CHECK, state_); + // TRANSITION is true when being called from StoreOwnIC. + DCHECK(state_ == ACCESS_CHECK || state_ == TRANSITION); return isolate_->MayAccess(handle(isolate_->context(), isolate_), GetHolder()); } diff --git a/deps/v8/src/objects/objects.cc b/deps/v8/src/objects/objects.cc index 2da580ea4bd310..3d4e6cf39948ec 100644 --- a/deps/v8/src/objects/objects.cc +++ b/deps/v8/src/objects/objects.cc @@ -2579,14 +2579,8 @@ Maybe Object::SetPropertyInternal(LookupIterator* it, return Nothing(); } -namespace { - -// If the receiver is the JSGlobalObject, the store was contextual. In case -// the property did not exist yet on the global object itself, we have to -// throw a reference error in strict mode. In sloppy mode, we continue. -// Returns false if the exception was thrown, otherwise true. -bool CheckContextualStoreToJSGlobalObject(LookupIterator* it, - Maybe should_throw) { +bool Object::CheckContextualStoreToJSGlobalObject( + LookupIterator* it, Maybe should_throw) { Isolate* isolate = it->isolate(); if (it->GetReceiver()->IsJSGlobalObject(isolate) && @@ -2605,8 +2599,6 @@ bool CheckContextualStoreToJSGlobalObject(LookupIterator* it, return true; } -} // namespace - Maybe Object::SetProperty(LookupIterator* it, Handle value, StoreOrigin store_origin, Maybe should_throw) { diff --git a/deps/v8/src/objects/objects.h b/deps/v8/src/objects/objects.h index 4daaafead35dd6..9fc636365deee9 100644 --- a/deps/v8/src/objects/objects.h +++ b/deps/v8/src/objects/objects.h @@ -721,6 +721,13 @@ class Object : public TaggedImpl { inline void WriteExternalPointerField(size_t offset, Isolate* isolate, Address value, ExternalPointerTag tag); + // If the receiver is the JSGlobalObject, the store was contextual. In case + // the property did not exist yet on the global object itself, we have to + // throw a reference error in strict mode. In sloppy mode, we continue. + // Returns false if the exception was thrown, otherwise true. + static bool CheckContextualStoreToJSGlobalObject( + LookupIterator* it, Maybe should_throw); + protected: inline Address field_address(size_t offset) const { return ptr() + offset - kHeapObjectTag; diff --git a/deps/v8/test/cctest/test-api-interceptors.cc b/deps/v8/test/cctest/test-api-interceptors.cc index 25a5bdd4f58319..fe5524a0ee1a29 100644 --- a/deps/v8/test/cctest/test-api-interceptors.cc +++ b/deps/v8/test/cctest/test-api-interceptors.cc @@ -2270,6 +2270,203 @@ THREADED_TEST(PropertyDefinerCallbackWithSetter) { .FromJust()); } +namespace { +std::vector definer_calls; +void LogDefinerCallsAndContinueCallback( + Local name, const v8::PropertyDescriptor& desc, + const v8::PropertyCallbackInfo& info) { + String::Utf8Value utf8(info.GetIsolate(), name); + definer_calls.push_back(*utf8); +} +void LogDefinerCallsAndStopCallback( + Local name, const v8::PropertyDescriptor& desc, + const v8::PropertyCallbackInfo& info) { + String::Utf8Value utf8(info.GetIsolate(), name); + definer_calls.push_back(*utf8); + info.GetReturnValue().Set(name); +} + +struct StoreOwnICInterceptorConfig { + std::string code; + std::vector intercepted_defines; +}; + +std::vector configs{ + { + R"( + class ClassWithNormalField extends Base { + field = (() => { + Object.defineProperty( + this, + 'normalField', + { writable: true, configurable: true, value: 'initial'} + ); + return 1; + })(); + normalField = 'written'; + constructor(arg) { + super(arg); + } + } + new ClassWithNormalField(obj); + stop ? (obj.field === undefined && obj.normalField === undefined) + : (obj.field === 1 && obj.normalField === 'written'))", + {"normalField", "field", "normalField"}, // intercepted defines + }, + { + R"( + let setterCalled = false; + class ClassWithSetterField extends Base { + field = (() => { + Object.defineProperty( + this, + 'setterField', + { configurable: true, set(val) { setterCalled = true; } } + ); + return 1; + })(); + setterField = 'written'; + constructor(arg) { + super(arg); + } + } + new ClassWithSetterField(obj); + !setterCalled && + (stop ? (obj.field === undefined && obj.setterField === undefined) + : (obj.field === 1 && obj.setterField === 'written')))", + {"setterField", "field", "setterField"}, // intercepted defines + }, + { + R"( + class ClassWithReadOnlyField extends Base { + field = (() => { + Object.defineProperty( + this, + 'readOnlyField', + { writable: false, configurable: true, value: 'initial'} + ); + return 1; + })(); + readOnlyField = 'written'; + constructor(arg) { + super(arg); + } + } + new ClassWithReadOnlyField(obj); + stop ? (obj.field === undefined && obj.readOnlyField === undefined) + : (obj.field === 1 && obj.readOnlyField === 'written'))", + {"readOnlyField", "field", "readOnlyField"}, // intercepted defines + }, + { + R"( + class ClassWithNonConfigurableField extends Base { + field = (() => { + Object.defineProperty( + this, + 'nonConfigurableField', + { writable: false, configurable: false, value: 'initial'} + ); + return 1; + })(); + nonConfigurableField = 'configured'; + constructor(arg) { + super(arg); + } + } + let nonConfigurableThrown = false; + try { new ClassWithNonConfigurableField(obj); } + catch { nonConfigurableThrown = true; } + stop ? (!nonConfigurableThrown && obj.field === undefined + && obj.nonConfigurableField === undefined) + : (nonConfigurableThrown && obj.field === 1 + && obj.nonConfigurableField === 'initial'))", + // intercepted defines + {"nonConfigurableField", "field", "nonConfigurableField"}} + // We don't test non-extensible objects here because objects with + // interceptors cannot prevent extensions. +}; +} // namespace + +void CheckPropertyDefinerCallbackInStoreOwnIC(Local context, + bool stop) { + v8_compile(R"( + class Base { + constructor(arg) { + return arg; + } + })") + ->Run(context) + .ToLocalChecked(); + + v8_compile(stop ? "var stop = true;" : "var stop = false;") + ->Run(context) + .ToLocalChecked(); + + for (auto& config : configs) { + printf("stop = %s, running...\n%s\n", stop ? "true" : "false", + config.code.c_str()); + + definer_calls.clear(); + + // Create the object with interceptors. + v8::Local templ = + v8::FunctionTemplate::New(CcTest::isolate()); + templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration( + nullptr, nullptr, nullptr, nullptr, nullptr, + stop ? LogDefinerCallsAndStopCallback + : LogDefinerCallsAndContinueCallback, + nullptr)); + Local obj = templ->GetFunction(context) + .ToLocalChecked() + ->NewInstance(context) + .ToLocalChecked(); + context->Global()->Set(context, v8_str("obj"), obj).FromJust(); + + CHECK(v8_compile(config.code.c_str()) + ->Run(context) + .ToLocalChecked() + ->IsTrue()); + for (size_t i = 0; i < definer_calls.size(); ++i) { + printf("define %s\n", definer_calls[i].c_str()); + } + + CHECK_EQ(config.intercepted_defines.size(), definer_calls.size()); + for (size_t i = 0; i < config.intercepted_defines.size(); ++i) { + CHECK_EQ(config.intercepted_defines[i], definer_calls[i]); + } + } +} + +THREADED_TEST(PropertyDefinerCallbackInStoreOwnIC) { + { + LocalContext env; + v8::HandleScope scope(env->GetIsolate()); + CheckPropertyDefinerCallbackInStoreOwnIC(env.local(), true); + } + + { + LocalContext env; + v8::HandleScope scope(env->GetIsolate()); + CheckPropertyDefinerCallbackInStoreOwnIC(env.local(), false); + } + + { + i::FLAG_lazy_feedback_allocation = false; + i::FlagList::EnforceFlagImplications(); + LocalContext env; + v8::HandleScope scope(env->GetIsolate()); + CheckPropertyDefinerCallbackInStoreOwnIC(env.local(), true); + } + + { + i::FLAG_lazy_feedback_allocation = false; + i::FlagList::EnforceFlagImplications(); + LocalContext env; + v8::HandleScope scope(env->GetIsolate()); + CheckPropertyDefinerCallbackInStoreOwnIC(env.local(), false); + } +} + namespace { void EmptyPropertyDescriptorCallback( Local name, const v8::PropertyCallbackInfo& info) { diff --git a/deps/v8/test/mjsunit/proxy-super-return-define-property-trap.js b/deps/v8/test/mjsunit/proxy-super-return-define-property-trap.js new file mode 100644 index 00000000000000..34a011b17a82bf --- /dev/null +++ b/deps/v8/test/mjsunit/proxy-super-return-define-property-trap.js @@ -0,0 +1,31 @@ +// Copyright 2022 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: --no-lazy-feedback-allocation +Proxy.prototype = Object.prototype; + +let logs = []; + +class Z extends Proxy { + constructor() { + super({}, { + set() { + logs.push("set"); + return true; + }, + defineProperty() { + logs.push("defineProperty"); + return true; + } + }) + } + a = 1; +} + +new Z(); +assertEquals(["defineProperty"], logs); + +logs = []; +new Z(); +assertEquals(["defineProperty"], logs); diff --git a/deps/v8/test/mjsunit/regress/regress-v8-12421-no-lazy-feedback.js b/deps/v8/test/mjsunit/regress/regress-v8-12421-no-lazy-feedback.js new file mode 100644 index 00000000000000..5f8f900311c821 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-v8-12421-no-lazy-feedback.js @@ -0,0 +1,7 @@ +// Copyright 2021 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: --no-lazy-feedback-allocation + +d8.file.execute('test/mjsunit/regress/regress-v8-12421.js'); diff --git a/deps/v8/test/mjsunit/regress/regress-v8-12421.js b/deps/v8/test/mjsunit/regress/regress-v8-12421.js new file mode 100644 index 00000000000000..ba0dba2542985c --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-v8-12421.js @@ -0,0 +1,258 @@ +// Copyright 2021 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. + +{ + class X { + static name = "name"; + static length = 15; + } + + assertEquals({ + "value": "name", + "writable": true, + "enumerable": true, + "configurable": true + }, Object.getOwnPropertyDescriptor(X, "name")); + + assertEquals({ + "value": 15, + "writable": true, + "enumerable": true, + "configurable": true + }, Object.getOwnPropertyDescriptor(X, "length")); +} + +{ + class X { + field = Object.preventExtensions(this); + } + + assertThrows(() => { + new X(); + }, TypeError, /Cannot define property field, object is not extensible/); +} + +{ + class X { + field = Object.defineProperty( + this, + "field2", + { writable: false, configurable: true, value: 1} + ); + field2 = 2; + } + + let x = new X(); + assertEquals(2, x.field2); +} + +{ + class X { + field = Object.defineProperty( + this, + "field2", + { writable: false, configurable: false, value: 1} + ); + field2 = true; + } + + assertThrows(() => { + new X(); + }, TypeError, /Cannot redefine property: field2/); +} + +{ + let setterCalled = false; + class X { + field = Object.defineProperty( + this, + "field2", + { + configurable: true, + set(val) { + setterCalled = true; + } + } + ); + field2 = 2; + } + + let x = new X(); + assertFalse(setterCalled); +} + +{ + class Base { + constructor(arg) { + return arg; + } + } + + class ClassWithNormalField extends Base { + field = (() => { + Object.defineProperty( + this, + "normalField", + { writable: true, configurable: true, value: "initial"} + ); + return 1; + })(); + normalField = "written"; + constructor(arg) { + super(arg); + } + } + + let setterCalled = false; + class ClassWithSetterField extends Base { + field = (() => { + Object.defineProperty( + this, + "setterField", + { configurable: true, set(val) { setterCalled = true; } } + ); + return 1; + })(); + setterField = "written"; + constructor(arg) { + super(arg); + } + } + + class ClassWithReadOnlyField extends Base { + field = (() => { + Object.defineProperty( + this, + "readOnlyField", + { writable: false, configurable: true, value: "initial"} + ); + return 1; + })(); + readOnlyField = "written"; + constructor(arg) { + super(arg); + } + } + + class ClassWithNonConfigurableField extends Base { + field = (() => { + Object.defineProperty( + this, + "nonConfigurableField", + { writable: false, configurable: false, value: "initial"} + ); + return 1; + })(); + nonConfigurableField = "configured"; + constructor(arg) { + super(arg); + } + } + + class ClassNonExtensible extends Base { + field = (() => { + Object.preventExtensions(this); + return 1; + })(); + nonExtensible = 4; + constructor(arg) { + super(arg); + } + } + + // Test dictionary objects. + { + let dict = Object.create(null); + + new ClassWithNormalField(dict); + assertEquals(1, dict.field); + assertEquals("written", dict.normalField); + + new ClassWithSetterField(dict); + assertFalse(setterCalled); + + new ClassWithReadOnlyField(dict); + assertEquals("written", dict.readOnlyField); + + assertThrows(() => { + new ClassWithNonConfigurableField(dict); + }, TypeError, /Cannot redefine property: nonConfigurableField/); + assertEquals("initial", dict.nonConfigurableField); + + assertThrows(() => { + new ClassNonExtensible(dict); + }, TypeError, /Cannot define property nonExtensible, object is not extensible/); + assertEquals(undefined, dict.nonExtensible); + } + + // Test proxies. + { + let trapCalls = []; + let target = {}; + let proxy = new Proxy(target, { + get(oTarget, sKey) { + return oTarget[sKey]; + }, + defineProperty(oTarget, sKey, oDesc) { + trapCalls.push(sKey); + Object.defineProperty(oTarget, sKey, oDesc); + return oTarget; + } + }); + + new ClassWithNormalField(proxy); + assertEquals(1, proxy.field); + assertEquals("written", proxy.normalField); + assertEquals(["normalField", "field", "normalField"], trapCalls); + + trapCalls = []; + new ClassWithSetterField(proxy); + assertFalse(setterCalled); + assertEquals("written", proxy.setterField); + assertEquals(["setterField", "field", "setterField"], trapCalls); + + trapCalls = []; + new ClassWithReadOnlyField(proxy); + assertEquals("written", proxy.readOnlyField); + assertEquals(["readOnlyField", "field", "readOnlyField"], trapCalls); + + trapCalls = []; + assertThrows(() => { + new ClassWithNonConfigurableField(proxy); + }, TypeError, /Cannot redefine property: nonConfigurableField/); + assertEquals("initial", proxy.nonConfigurableField); + assertEquals(["nonConfigurableField", "field", "nonConfigurableField"], trapCalls); + + trapCalls = []; + assertThrows(() => { + new ClassNonExtensible(proxy); + }, TypeError, /Cannot define property nonExtensible, object is not extensible/); + assertEquals(undefined, proxy.nonExtensible); + assertEquals(["field", "nonExtensible"], trapCalls); + } + + // Test globalThis. + { + new ClassWithNormalField(globalThis); + assertEquals(1, field); + assertEquals("written", normalField); + + new ClassWithSetterField(globalThis); + assertFalse(setterCalled); + assertEquals("written", setterField); + + new ClassWithReadOnlyField(globalThis); + assertEquals("written", readOnlyField); + + assertThrows(() => { + new ClassWithNonConfigurableField(globalThis); + }, TypeError, /Cannot redefine property: nonConfigurableField/); + assertEquals("initial", nonConfigurableField); + + assertThrows(() => { + new ClassNonExtensible(globalThis); + }, TypeError, /Cannot add property nonExtensible, object is not extensible/); + assertEquals("undefined", typeof nonExtensible); + } +}