[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/+/3300092Reviewed-by: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/main@{#78659}

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"
@@ -1738,6 +1739,69 @@ MaybeHandle<Object> StoreGlobalIC::Store(Handle<Name> name,
   return StoreIC::Store(global, name, value);
 }
 
+namespace {
+Maybe<bool> DefineOwnDataProperty(LookupIterator* it,
+                                  LookupIterator::State original_state,
+                                  Handle<Object> value,
+                                  Maybe<ShouldThrow> 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<JSProxy>(),
+                                        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<JSObject>()->IsAccessCheckNeeded());
+          V8_FALLTHROUGH;
+        }
+        case LookupIterator::NOT_FOUND:
+          return Object::AddDataProperty(it, value, NONE,
+                                         Nothing<ShouldThrow>(), 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<Object> StoreIC::Store(Handle<Object> object, Handle<Name> name,
                                    Handle<Object> value,
                                    StoreOrigin store_origin) {
@@ -1749,7 +1813,15 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> 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<ShouldThrow>()));
+    } else {
+      MAYBE_RETURN_NULL(Object::SetProperty(&it, value, StoreOrigin::kNamed));
+    }
     return value;
   }
 
@@ -1797,6 +1869,23 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> 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<JSObject>::cast(object)->HasNamedInterceptor()) {
+    Maybe<bool> can_define = JSReceiver::CheckIfCanDefine(
+        isolate(), &it, value, Nothing<ShouldThrow>());
+    if (can_define.IsNothing() || !can_define.FromJust()) {
+      return MaybeHandle<Object>();
+    }
+  }
+
   if (use_ic) {
     UpdateCaches(&it, value, store_origin);
   } else if (state() == NO_FEEDBACK) {
@@ -1805,7 +1894,21 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> 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<ShouldThrow>(), store_origin));
+  } else {
+    MAYBE_RETURN_NULL(Object::SetProperty(&it, value, store_origin));
+  }
   return value;
 }
 
@@ -1883,11 +1986,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()));
       }
 
@@ -2067,6 +2173,14 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
       Handle<JSReceiver> receiver =
           Handle<JSReceiver>::cast(lookup->GetReceiver());
       Handle<JSProxy> holder = lookup->GetHolder<JSProxy>();
+
+      // 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));
     }
@@ -2778,9 +2892,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<ShouldThrow>()),
-               ReadOnlyRoots(isolate).exception());
+
+  MAYBE_RETURN(
+      JSReceiver::CreateDataProperty(&it, value, Nothing<ShouldThrow>()),
+      ReadOnlyRoots(isolate).exception());
   return *value;
 }
 

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<String> 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<String> 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());
       }
     }
   }

src/objects/js-objects.cc

@@ -184,6 +184,27 @@ Maybe<bool> JSReceiver::HasInPrototypeChain(Isolate* isolate,
   }
 }
 
+// static
+Maybe<bool> JSReceiver::CheckIfCanDefine(Isolate* isolate, LookupIterator* it,
+                                         Handle<Object> value,
+                                         Maybe<ShouldThrow> should_throw) {
+  if (it->IsFound()) {
+    Maybe<PropertyAttributes> attributes = GetPropertyAttributes(it);
+    MAYBE_RETURN(attributes, Nothing<bool>());
+    if ((attributes.FromJust() & DONT_DELETE) != 0) {
+      RETURN_FAILURE(
+          isolate, GetShouldThrow(isolate, should_throw),
+          NewTypeError(MessageTemplate::kRedefineDisallowed, it->GetName()));
+    }
+  } else if (!JSObject::IsExtensible(
+                 Handle<JSObject>::cast(it->GetReceiver()))) {
+    RETURN_FAILURE(
+        isolate, GetShouldThrow(isolate, should_throw),
+        NewTypeError(MessageTemplate::kDefineDisallowed, it->GetName()));
+  }
+  return Just(true);
+}
+
 namespace {
 
 bool HasExcludedProperty(
@@ -3367,23 +3388,25 @@ void JSObject::AddProperty(Isolate* isolate, Handle<JSObject> object,
 // hidden prototypes.
 MaybeHandle<Object> JSObject::DefineOwnPropertyIgnoreAttributes(
     LookupIterator* it, Handle<Object> 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<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
     LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
-    Maybe<ShouldThrow> should_throw, AccessorInfoHandling handling) {
+    Maybe<ShouldThrow> should_throw, AccessorInfoHandling handling,
+    EnforceDefineSemantics semantics) {
   it->UpdateProtector();
   Handle<JSObject> object = Handle<JSObject>::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:
@@ -3402,13 +3425,36 @@ Maybe<bool> 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<bool> result =
-              JSObject::SetPropertyWithInterceptor(it, should_throw, value);
-          if (result.IsNothing() || result.FromJust()) return result;
+      case LookupIterator::INTERCEPTOR: {
+        Maybe<bool> 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<bool> 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<Object> accessors = it->GetAccessors();
@@ -3826,20 +3872,10 @@ Maybe<bool> JSObject::CreateDataProperty(LookupIterator* it,
   Handle<JSReceiver> receiver = Handle<JSReceiver>::cast(it->GetReceiver());
   Isolate* isolate = receiver->GetIsolate();
 
-  if (it->IsFound()) {
-    Maybe<PropertyAttributes> attributes = GetPropertyAttributes(it);
-    MAYBE_RETURN(attributes, Nothing<bool>());
-    if ((attributes.FromJust() & DONT_DELETE) != 0) {
-      RETURN_FAILURE(
-          isolate, GetShouldThrow(isolate, should_throw),
-          NewTypeError(MessageTemplate::kRedefineDisallowed, it->GetName()));
-    }
-  } else {
-    if (!JSObject::IsExtensible(Handle<JSObject>::cast(it->GetReceiver()))) {
-      RETURN_FAILURE(
-          isolate, GetShouldThrow(isolate, should_throw),
-          NewTypeError(MessageTemplate::kDefineDisallowed, it->GetName()));
-    }
+  Maybe<bool> 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(),

src/objects/js-objects.h

@@ -160,6 +160,12 @@ class JSReceiver : public TorqueGeneratedJSReceiver<JSReceiver, HeapObject> {
       Isolate* isolate, Handle<JSReceiver> object, Handle<Object> key,
       PropertyDescriptor* desc, Maybe<ShouldThrow> 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<bool> CheckIfCanDefine(
+      Isolate* isolate, LookupIterator* it, Handle<Object> value,
+      Maybe<ShouldThrow> should_throw);
+
   // ES6 7.3.4 (when passed kDontThrow)
   V8_WARN_UNUSED_RESULT static Maybe<bool> CreateDataProperty(
       Isolate* isolate, Handle<JSReceiver> object, Handle<Name> key,
@@ -399,15 +405,27 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
   // 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<Object>
   DefineOwnPropertyIgnoreAttributes(
       LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
-      AccessorInfoHandling handling = DONT_FORCE_FIELD);
+      AccessorInfoHandling handling = DONT_FORCE_FIELD,
+      EnforceDefineSemantics semantics = EnforceDefineSemantics::kSet);
 
   V8_WARN_UNUSED_RESULT static Maybe<bool> DefineOwnPropertyIgnoreAttributes(
       LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
       Maybe<ShouldThrow> should_throw,
-      AccessorInfoHandling handling = DONT_FORCE_FIELD);
+      AccessorInfoHandling handling = DONT_FORCE_FIELD,
+      EnforceDefineSemantics semantics = EnforceDefineSemantics::kSet);
 
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> V8_EXPORT_PRIVATE
   SetOwnPropertyIgnoreAttributes(Handle<JSObject> object, Handle<Name> name,

src/objects/lookup.cc

@@ -167,7 +167,8 @@ Handle<Map> 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<JSObject>());
 }

src/objects/objects.cc

@@ -2593,14 +2593,8 @@ Maybe<bool> Object::SetPropertyInternal(LookupIterator* it,
   return Nothing<bool>();
 }
 
-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<ShouldThrow> should_throw) {
+bool Object::CheckContextualStoreToJSGlobalObject(
+    LookupIterator* it, Maybe<ShouldThrow> should_throw) {
   Isolate* isolate = it->isolate();
 
   if (it->GetReceiver()->IsJSGlobalObject(isolate) &&
@@ -2619,8 +2613,6 @@ bool CheckContextualStoreToJSGlobalObject(LookupIterator* it,
   return true;
 }
 
-}  // namespace
-
 Maybe<bool> Object::SetProperty(LookupIterator* it, Handle<Object> value,
                                 StoreOrigin store_origin,
                                 Maybe<ShouldThrow> should_throw) {

src/objects/objects.h

@@ -731,6 +731,13 @@ class Object : public TaggedImpl<HeapObjectReferenceType::STRONG, Address> {
   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<ShouldThrow> should_throw);
+
  protected:
   inline Address field_address(size_t offset) const {
     return ptr() + offset - kHeapObjectTag;

test/cctest/test-api-interceptors.cc

@@ -2270,6 +2270,203 @@ THREADED_TEST(PropertyDefinerCallbackWithSetter) {
                    .FromJust());
 }
 
+namespace {
+std::vector<std::string> definer_calls;
+void LogDefinerCallsAndContinueCallback(
+    Local<Name> name, const v8::PropertyDescriptor& desc,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  String::Utf8Value utf8(info.GetIsolate(), name);
+  definer_calls.push_back(*utf8);
+}
+void LogDefinerCallsAndStopCallback(
+    Local<Name> name, const v8::PropertyDescriptor& desc,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  String::Utf8Value utf8(info.GetIsolate(), name);
+  definer_calls.push_back(*utf8);
+  info.GetReturnValue().Set(name);
+}
+
+struct StoreOwnICInterceptorConfig {
+  std::string code;
+  std::vector<std::string> intercepted_defines;
+};
+
+std::vector<StoreOwnICInterceptorConfig> 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> 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<v8::FunctionTemplate> templ =
+        v8::FunctionTemplate::New(CcTest::isolate());
+    templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
+        nullptr, nullptr, nullptr, nullptr, nullptr,
+        stop ? LogDefinerCallsAndStopCallback
+             : LogDefinerCallsAndContinueCallback,
+        nullptr));
+    Local<Object> 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> name, const v8::PropertyCallbackInfo<v8::Value>& info) {

test/mjsunit/proxy-super-return-define-property-trap.js

+// 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);

test/mjsunit/regress/regress-v8-12421-no-lazy-feedback.js

+// 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');

test/mjsunit/regress/regress-v8-12421.js

+// 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);
+  }
+}