From 62af07e96173fb03b504f60b01e67d9f2dda695b Mon Sep 17 00:00:00 2001
From: Igor Sheludko <[email protected]>
Date: Mon, 13 Oct 2025 14:22:20 +0200
Subject: [PATCH] [ic] Cleanup AccessorAssembler::CallGetterIfAccessor()

This CL
 - reorders parameters to make |expected_receiver_mode| a mandatory
   one and properly computed,
 - makes sure we don't pass PropertyCell as a holder when JSReceiver is
   expected.

Bug: 450328966
Change-Id: I921dfbd99245d01143600b4f4713fe602c817657
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7036691
Commit-Queue: Igor Sheludko <[email protected]>
Reviewed-by: Leszek Swirski <[email protected]>
Cr-Commit-Position: refs/heads/main@{#103085}
---

diff --git a/src/codegen/code-stub-assembler.cc b/src/codegen/code-stub-assembler.cc
index a8e0c85..b749310 100644
--- a/src/codegen/code-stub-assembler.cc
+++ b/src/codegen/code-stub-assembler.cc
@@ -11662,7 +11662,8 @@
 
               var_value = CallGetterIfAccessor(
                   value_or_accessor, object, var_details.value(), context,
-                  object, next_key, &slow_load, kCallJSGetterUseCachedName);
+                  object, kExpectingJSReceiver, next_key, &slow_load,
+                  kCallJSGetterUseCachedName);
               Goto(&value_ready);
 
               BIND(&slow_load);
@@ -12158,15 +12159,11 @@
     TNode<SwissNameDictionary> dictionary, TNode<IntPtrT> name_index,
     TVariable<Uint32T>* var_details, TVariable<Object>* var_value);
 
-// |value| is the property backing store's contents, which is either a value or
-// an accessor pair, as specified by |details|. |holder| is a JSReceiver or a
-// PropertyCell. Returns either the original value, or the result of the getter
-// call.
 TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
-    TNode<Object> value, TNode<Union<JSReceiver, PropertyCell>> holder,
+    TNode<Object> value, std::optional<TNode<JSReceiver>> holder,
     TNode<Uint32T> details, TNode<Context> context, TNode<JSAny> receiver,
-    TNode<Object> name, Label* if_bailout, GetOwnPropertyMode mode,
-    ExpectedReceiverMode expected_receiver_mode) {
+    ExpectedReceiverMode expected_receiver_mode, TNode<Object> name,
+    Label* if_bailout, GetOwnPropertyMode mode) {
   TVARIABLE(Object, var_value, value);
   Label done(this), if_accessor_info(this, Label::kDeferred);
 
@@ -12207,44 +12204,51 @@
 
       BIND(&if_function_template_info);
       {
-        Label use_cached_property(this);
-        TNode<HeapObject> cached_property_name = LoadObjectField<HeapObject>(
-            getter, FunctionTemplateInfo::kCachedPropertyNameOffset);
+        if (holder.has_value()) {
+          Label use_cached_property(this);
+          TNode<HeapObject> cached_property_name = LoadObjectField<HeapObject>(
+              getter, FunctionTemplateInfo::kCachedPropertyNameOffset);
 
-        Label* has_cached_property = mode == kCallJSGetterUseCachedName
-                                         ? &use_cached_property
-                                         : if_bailout;
-        GotoIfNot(IsTheHole(cached_property_name), has_cached_property);
+          Label* has_cached_property = mode == kCallJSGetterUseCachedName
+                                           ? &use_cached_property
+                                           : if_bailout;
+          GotoIfNot(IsTheHole(cached_property_name), has_cached_property);
 
-        TNode<JSReceiver> js_receiver;
-        switch (expected_receiver_mode) {
-          case kExpectingJSReceiver:
-            js_receiver = CAST(receiver);
-            break;
-          case kExpectingAnyReceiver:
-            // TODO(ishell): in case the function template info has a signature
-            // and receiver is not a JSReceiver the signature check in
-            // CallFunctionTemplate builtin will fail anyway, so we can short
-            // cut it here and throw kIllegalInvocation immediately.
-            js_receiver = ToObject_Inline(context, receiver);
-            break;
-        }
-        TNode<JSReceiver> holder_receiver = CAST(holder);
-        TNode<NativeContext> creation_context =
-            GetCreationContext(holder_receiver, if_bailout);
-        TNode<Context> caller_context = context;
-        var_value = CallBuiltin(
-            Builtin::kCallFunctionTemplate_Generic, creation_context, getter,
-            Int32Constant(i::JSParameterCount(0)), caller_context, js_receiver);
-        Goto(&done);
-
-        if (mode == kCallJSGetterUseCachedName) {
-          Bind(&use_cached_property);
-
-          var_value =
-              GetProperty(context, holder_receiver, cached_property_name);
-
+          TNode<JSReceiver> js_receiver;
+          switch (expected_receiver_mode) {
+            case kExpectingJSReceiver:
+              js_receiver = CAST(receiver);
+              break;
+            case kExpectingAnyReceiver:
+              // TODO(ishell): in case the function template info has a
+              // signature and receiver is not a JSReceiver the signature check
+              // in CallFunctionTemplate builtin will fail anyway, so we can
+              // short cut it here and throw kIllegalInvocation immediately.
+              js_receiver = ToObject_Inline(context, receiver);
+              break;
+          }
+          TNode<JSReceiver> holder_receiver = *holder;
+          TNode<NativeContext> creation_context =
+              GetCreationContext(holder_receiver, if_bailout);
+          TNode<Context> caller_context = context;
+          var_value = CallBuiltin(Builtin::kCallFunctionTemplate_Generic,
+                                  creation_context, getter,
+                                  Int32Constant(i::JSParameterCount(0)),
+                                  caller_context, js_receiver);
           Goto(&done);
+
+          if (mode == kCallJSGetterUseCachedName) {
+            Bind(&use_cached_property);
+
+            var_value =
+                GetProperty(context, holder_receiver, cached_property_name);
+
+            Goto(&done);
+          }
+        } else {
+          // |holder| must be available in order to handle lazy AccessorPair
+          // case (we need it for computing the function's context).
+          Unreachable();
         }
       }
     } else {
@@ -12256,56 +12260,61 @@
   // AccessorInfo case.
   BIND(&if_accessor_info);
   {
-    TNode<AccessorInfo> accessor_info = CAST(value);
-    Label if_array(this), if_function(this), if_wrapper(this);
+    if (holder.has_value()) {
+      TNode<AccessorInfo> accessor_info = CAST(value);
+      Label if_array(this), if_function(this), if_wrapper(this);
+      // Dispatch based on {holder} instance type.
+      TNode<Map> holder_map = LoadMap(*holder);
+      TNode<Uint16T> holder_instance_type = LoadMapInstanceType(holder_map);
+      GotoIf(IsJSArrayInstanceType(holder_instance_type), &if_array);
+      GotoIf(IsJSFunctionInstanceType(holder_instance_type), &if_function);
+      Branch(IsJSPrimitiveWrapperInstanceType(holder_instance_type),
+             &if_wrapper, if_bailout);
 
-    // Dispatch based on {holder} instance type.
-    TNode<Map> holder_map = LoadMap(holder);
-    TNode<Uint16T> holder_instance_type = LoadMapInstanceType(holder_map);
-    GotoIf(IsJSArrayInstanceType(holder_instance_type), &if_array);
-    GotoIf(IsJSFunctionInstanceType(holder_instance_type), &if_function);
-    Branch(IsJSPrimitiveWrapperInstanceType(holder_instance_type), &if_wrapper,
-           if_bailout);
+      // JSArray AccessorInfo case.
+      BIND(&if_array);
+      {
+        // We only deal with the "length" accessor on JSArray.
+        GotoIfNot(IsLengthString(LoadObjectField(accessor_info,
+                                                 AccessorInfo::kNameOffset)),
+                  if_bailout);
+        TNode<JSArray> array = CAST(*holder);
+        var_value = LoadJSArrayLength(array);
+        Goto(&done);
+      }
 
-    // JSArray AccessorInfo case.
-    BIND(&if_array);
-    {
-      // We only deal with the "length" accessor on JSArray.
-      GotoIfNot(IsLengthString(
-                    LoadObjectField(accessor_info, AccessorInfo::kNameOffset)),
-                if_bailout);
-      TNode<JSArray> array = CAST(holder);
-      var_value = LoadJSArrayLength(array);
-      Goto(&done);
-    }
+      // JSFunction AccessorInfo case.
+      BIND(&if_function);
+      {
+        // We only deal with the "prototype" accessor on JSFunction here.
+        GotoIfNot(IsPrototypeString(LoadObjectField(accessor_info,
+                                                    AccessorInfo::kNameOffset)),
+                  if_bailout);
 
-    // JSFunction AccessorInfo case.
-    BIND(&if_function);
-    {
-      // We only deal with the "prototype" accessor on JSFunction here.
-      GotoIfNot(IsPrototypeString(
-                    LoadObjectField(accessor_info, AccessorInfo::kNameOffset)),