[IC] Clarify receiver vs holder some more

This is a follow up for
https://chromium-review.googlesource.com/c/v8/v8/+/2362918 .

The "slow" path in HandleLoadICSmiHandlerLoadNamedCase was using
only "receiver", even though it should've considered both "receiver"
and "holder".

Bug: v8:9237
Change-Id: I5d7ba1f72e8bf55f9533f648054abf5d25c85533
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2387576
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69671}

src/ic/accessor-assembler.cc

@@ -651,8 +651,8 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
                                     p->name(), p->slot(), p->vector());
 
     } else {
-      exit_point->ReturnCallRuntime(Runtime::kGetProperty, p->context(),
-                                    p->receiver(), p->name());
+      exit_point->ReturnCallRuntime(Runtime::kGetProperty, p->context(), holder,
+                                    p->name(), p->receiver());
     }
   }
 
@@ -3230,8 +3230,8 @@ void AccessorAssembler::KeyedLoadICGeneric(const LoadICParameters* p) {
     Comment("KeyedLoadGeneric_slow");
     IncrementCounter(isolate()->counters()->ic_keyed_load_generic_slow(), 1);
     // TODO(jkummerow): Should we use the GetProperty TF stub instead?
-    TailCallRuntime(Runtime::kGetProperty, p->context(), p->receiver(),
-                    var_name.value());
+    TailCallRuntime(Runtime::kGetProperty, p->context(),
+                    p->receiver_and_lookup_start_object(), var_name.value());
   }
 }
 

src/objects/lookup-inl.h

@@ -56,6 +56,15 @@ LookupIterator::LookupIterator(Isolate* isolate, Handle<Object> receiver,
     : LookupIterator(isolate, receiver, key.name(), key.index(), holder,
                      configuration) {}
 
+LookupIterator LookupIterator::LookupWithReceiver(Isolate* isolate,
+                                                  Handle<Object> receiver,
+                                                  const Key& key,
+                                                  Handle<Object> holder,
+                                                  Configuration configuration) {
+  return LookupIterator(isolate, receiver, key,
+                        GetRoot(isolate, holder, key.index()), configuration);
+}
+
 // This private constructor is the central bottleneck that all the other
 // constructors use.
 LookupIterator::LookupIterator(Isolate* isolate, Handle<Object> receiver,

src/objects/lookup.h

@@ -85,6 +85,12 @@ class V8_EXPORT_PRIVATE LookupIterator final {
                         const Key& key, Handle<JSReceiver> holder,
                         Configuration configuration = DEFAULT);
 
+  // Usable for cases where "holder" is not necessarily a JSReceiver (a separate
+  // overloaded constructor is not possible).
+  static inline LookupIterator LookupWithReceiver(
+      Isolate* isolate, Handle<Object> receiver, const Key& key,
+      Handle<Object> holder, Configuration configuration = DEFAULT);
+
   void Restart() {
     InterceptorState state = InterceptorState::kUninitialized;
     IsElement() ? RestartInternal<true>(state) : RestartInternal<false>(state);

src/runtime/runtime-object.cc

@@ -24,21 +24,26 @@ namespace v8 {
 namespace internal {
 
 MaybeHandle<Object> Runtime::GetObjectProperty(Isolate* isolate,
-                                               Handle<Object> object,
+                                               Handle<Object> holder,
                                                Handle<Object> key,
-                                               bool* is_found_out) {
-  if (object->IsNullOrUndefined(isolate)) {
-    ErrorUtils::ThrowLoadFromNullOrUndefined(isolate, object, key);
+                                               Handle<Object> receiver,
+                                               bool* is_found) {
+  if (receiver.is_null()) {
+    receiver = holder;
+  }
+  if (holder->IsNullOrUndefined(isolate)) {
+    ErrorUtils::ThrowLoadFromNullOrUndefined(isolate, holder, key);
     return MaybeHandle<Object>();
   }
 
   bool success = false;
   LookupIterator::Key lookup_key(isolate, key, &success);
   if (!success) return MaybeHandle<Object>();
-  LookupIterator it(isolate, object, lookup_key);
+  LookupIterator it =
+      LookupIterator::LookupWithReceiver(isolate, receiver, lookup_key, holder);
 
   MaybeHandle<Object> result = Object::GetProperty(&it);
-  if (is_found_out) *is_found_out = it.IsFound();
+  if (is_found) *is_found = it.IsFound();
 
   if (!it.IsFound() && key->IsSymbol() &&
       Symbol::cast(*key).is_private_name()) {
@@ -52,12 +57,12 @@ MaybeHandle<Object> Runtime::GetObjectProperty(Isolate* isolate,
                                       : name_string;
       THROW_NEW_ERROR(isolate,
                       NewTypeError(MessageTemplate::kInvalidPrivateBrand,
-                                   class_name, object),
+                                   class_name, holder),
                       Object);
     }
     THROW_NEW_ERROR(isolate,
                     NewTypeError(MessageTemplate::kInvalidPrivateMemberRead,
-                                 name_string, object),
+                                 name_string, holder),
                     Object);
   }
   return result;
@@ -582,11 +587,16 @@ RUNTIME_FUNCTION(Runtime_JSReceiverSetPrototypeOfDontThrow) {
 
 RUNTIME_FUNCTION(Runtime_GetProperty) {
   HandleScope scope(isolate);
-  DCHECK_EQ(2, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(Object, receiver_obj, 0);
+  DCHECK(args.length() == 3 || args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(Object, holder_obj, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, key_obj, 1);
+  Handle<Object> receiver_obj = holder_obj;
+  if (args.length() == 3) {
+    CHECK(args[2].IsObject());
+    receiver_obj = args.at<Object>(2);
+  }
 
-  // Fast cases for getting named properties of the receiver JSObject
+  // Fast cases for getting named properties of the holder JSObject
   // itself.
   //
   // The global proxy objects has to be excluded since LookupOwn on
@@ -604,18 +614,18 @@ RUNTIME_FUNCTION(Runtime_GetProperty) {
   if (key_obj->IsString() && String::cast(*key_obj).AsArrayIndex(&index)) {
     key_obj = isolate->factory()->NewNumberFromUint(index);
   }
-  if (receiver_obj->IsJSObject()) {
-    if (!receiver_obj->IsJSGlobalProxy() &&
-        !receiver_obj->IsAccessCheckNeeded() && key_obj->IsName()) {
-      Handle<JSObject> receiver = Handle<JSObject>::cast(receiver_obj);
+  if (holder_obj->IsJSObject()) {
+    if (!holder_obj->IsJSGlobalProxy() && !holder_obj->IsAccessCheckNeeded() &&
+        key_obj->IsName()) {
+      Handle<JSObject> holder = Handle<JSObject>::cast(holder_obj);
       Handle<Name> key = Handle<Name>::cast(key_obj);
       key_obj = key = isolate->factory()->InternalizeName(key);
 
       DisallowHeapAllocation no_allocation;
-      if (receiver->IsJSGlobalObject()) {
+      if (holder->IsJSGlobalObject()) {
         // Attempt dictionary lookup.
         GlobalDictionary dictionary =
-            JSGlobalObject::cast(*receiver).global_dictionary();
+            JSGlobalObject::cast(*holder).global_dictionary();
         InternalIndex entry = dictionary.FindEntry(isolate, key);
         if (entry.is_found()) {
           PropertyCell cell = dictionary.CellAt(entry);
@@ -625,9 +635,9 @@ RUNTIME_FUNCTION(Runtime_GetProperty) {
             // If value is the hole (meaning, absent) do the general lookup.
           }
         }
-      } else if (!receiver->HasFastProperties()) {
+      } else if (!holder->HasFastProperties()) {
         // Attempt dictionary lookup.
-        NameDictionary dictionary = receiver->property_dictionary();
+        NameDictionary dictionary = holder->property_dictionary();
         InternalIndex entry = dictionary.FindEntry(isolate, key);
         if ((entry.is_found()) &&
             (dictionary.DetailsAt(entry).kind() == kData)) {
@@ -641,7 +651,7 @@ RUNTIME_FUNCTION(Runtime_GetProperty) {
       // transition elements to FAST_*_ELEMENTS to avoid excessive boxing of
       // doubles for those future calls in the case that the elements would
       // become PACKED_DOUBLE_ELEMENTS.
-      Handle<JSObject> js_object = Handle<JSObject>::cast(receiver_obj);
+      Handle<JSObject> js_object = Handle<JSObject>::cast(holder_obj);
       ElementsKind elements_kind = js_object->GetElementsKind();
       if (IsDoubleElementsKind(elements_kind)) {
         if (Smi::ToInt(*key_obj) >= js_object->elements().length()) {
@@ -654,9 +664,9 @@ RUNTIME_FUNCTION(Runtime_GetProperty) {
                !IsFastElementsKind(elements_kind));
       }
     }
-  } else if (receiver_obj->IsString() && key_obj->IsSmi()) {
+  } else if (holder_obj->IsString() && key_obj->IsSmi()) {
     // Fast case for string indexing using [] with a smi index.
-    Handle<String> str = Handle<String>::cast(receiver_obj);
+    Handle<String> str = Handle<String>::cast(holder_obj);
     int index = Handle<Smi>::cast(key_obj)->value();
     if (index >= 0 && index < str->length()) {
       Factory* factory = isolate->factory();
@@ -667,7 +677,8 @@ RUNTIME_FUNCTION(Runtime_GetProperty) {
 
   // Fall back to GetObjectProperty.
   RETURN_RESULT_OR_FAILURE(
-      isolate, Runtime::GetObjectProperty(isolate, receiver_obj, key_obj));
+      isolate,
+      Runtime::GetObjectProperty(isolate, holder_obj, key_obj, receiver_obj));
 }
 
 RUNTIME_FUNCTION(Runtime_SetKeyedProperty) {

src/runtime/runtime.h

@@ -11,6 +11,7 @@
 #include "src/base/bit-field.h"
 #include "src/base/platform/time.h"
 #include "src/common/globals.h"
+#include "src/handles/handles.h"
 #include "src/objects/elements-kind.h"
 #include "src/strings/unicode.h"
 #include "src/utils/allocation.h"
@@ -304,7 +305,7 @@ namespace internal {
   F(GetFunctionName, 1, 1)                                      \
   F(GetOwnPropertyDescriptor, 2, 1)                             \
   F(GetOwnPropertyKeys, 2, 1)                                   \
-  F(GetProperty, 2, 1)                                          \
+  F(GetProperty, -1 /* [2, 3] */, 1)                            \
   F(HasFastPackedElements, 1, 1)                                \
   F(HasInPrototypeChain, 2, 1)                                  \
   I(HasProperty, 2, 1)                                          \
@@ -754,9 +755,11 @@ class Runtime : public AllStatic {
                     Handle<Object> value, StoreOrigin store_origin,
                     Maybe<ShouldThrow> should_throw = Nothing<ShouldThrow>());
 
+  // When "receiver" is not passed, it defaults to "holder".
   V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle<Object>
-  GetObjectProperty(Isolate* isolate, Handle<Object> object, Handle<Object> key,
-                    bool* is_found_out = nullptr);
+  GetObjectProperty(Isolate* isolate, Handle<Object> holder, Handle<Object> key,
+                    Handle<Object> receiver = Handle<Object>(),
+                    bool* is_found = nullptr);
 
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> HasProperty(
       Isolate* isolate, Handle<Object> object, Handle<Object> key);