0001-Fix-Interceptor-being-reset-by-LookupIterator-change.patch

From f498f86cecc69cf82cb316452f33505204c2df5a Mon Sep 17 00:00:00 2001
From: Divy Srivastava <dj.srivastava23@gmail.com>
Date: Fri, 8 Mar 2024 06:16:46 +0000
Subject: [PATCH] Fix Interceptor being reset by LookupIterator changes

---
 src/objects/js-objects.cc | 48 ++++++++++++---------------------------
 1 file changed, 14 insertions(+), 34 deletions(-)

diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc
index af21b605e3..7f6513d925 100644
--- a/src/objects/js-objects.cc
+++ b/src/objects/js-objects.cc
@@ -1380,52 +1380,32 @@ Maybe<bool> JSReceiver::OrdinaryDefineOwnProperty(
     PropertyDescriptor* desc, Maybe<ShouldThrow> should_throw) {
   LookupIterator it(isolate, object, key, LookupIterator::OWN);

-  // Deal with access checks first.
-  if (it.state() == LookupIterator::ACCESS_CHECK) {
-    if (!it.HasAccess()) {
-      RETURN_ON_EXCEPTION_VALUE(
-          isolate, isolate->ReportFailedAccessCheck(it.GetHolder<JSObject>()),
-          Nothing<bool>());
-      UNREACHABLE();
-    }
-    it.Next();
-  }
-
   // 1. Let current be O.[[GetOwnProperty]](P).
   // 2. ReturnIfAbrupt(current).
   PropertyDescriptor current;
   MAYBE_RETURN(GetOwnPropertyDescriptor(&it, &current), Nothing<bool>());

-  // TODO(jkummerow/verwaest): It would be nice if we didn't have to reset
-  // the iterator every time. Currently, the reasons why we need it are because
-  // GetOwnPropertyDescriptor can have side effects, namely:
-  // - Interceptors
-  // - Accessors (which might change the holder's map)
   it.Restart();
-
-  // Skip over the access check after restarting -- we've already checked it.
-  if (it.state() == LookupIterator::ACCESS_CHECK) {
-    DCHECK(it.HasAccess());
-    it.Next();
-  }
-
-  // Handle interceptor.
-  if (it.state() == LookupIterator::INTERCEPTOR) {
-    if (it.HolderIsReceiverOrHiddenPrototype()) {
-      Maybe<bool> result = DefinePropertyWithInterceptorInternal(
-          &it, it.GetInterceptor(), should_throw, desc);
-      if (result.IsNothing() || result.FromJust()) {
-        return result;
+  // Handle interceptor
+  for (; it.IsFound(); it.Next()) {
+    if (it.state() == LookupIterator::INTERCEPTOR) {
+      if (it.HolderIsReceiverOrHiddenPrototype()) {
+        Maybe<bool> result = DefinePropertyWithInterceptorInternal(
+            &it, it.GetInterceptor(), should_throw, desc);
+        if (result.IsNothing() || result.FromJust()) {
+          return result;
+        }
       }
-      // We need to restart the lookup in case the accessor ran with side
-      // effects.
-      it.Restart();
     }
   }

+  // TODO(jkummerow/verwaest): It would be nice if we didn't have to reset
+  // the iterator every time. Currently, the reasons why we need it are:
+  // - handle interceptors correctly
+  // - handle accessors correctly (which might change the holder's map)
+  it.Restart();
   // 3. Let extensible be the value of the [[Extensible]] internal slot of O.
   bool extensible = JSObject::IsExtensible(isolate, object);
-
   return ValidateAndApplyPropertyDescriptor(
       isolate, &it, extensible, desc, &current, should_throw, Handle<Name>());
 }
--
2.34.1