[api] Cached properties are read of the receiver

The optimization was initially designed to support only the case where
the receiver is the holder, so make this explicit:

Cached properties were implemented before super property access and
Reflect.get, or at least around the same time, not realising it
conflicted. Cached properties are optimizations for known accessors
globalThis.window and globalThis.document. They store the result of
calling those accessors. The result of calling those accessors depends
on the receiver passed to the call, so we shouldn't simply read the
cached property off of the _holder_ of the accessor, but only do so if
the holder is the same as the receiver.

Bug: chromium:1305302
Change-Id: Iea6f4437e09d5a293798041adcb310469589d00f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3738744Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81498}

src/codegen/code-stub-assembler.cc

@@ -9880,7 +9880,9 @@ void CodeStubAssembler::TryGetOwnProperty(
     Label* if_bailout) {
   TryGetOwnProperty(context, receiver, object, map, instance_type, unique_name,
                     if_found_value, var_value, nullptr, nullptr, if_not_found,
-                    if_bailout, kCallJSGetterUseCachedName);
+                    if_bailout,
+                    receiver == object ? kCallJSGetterUseCachedName
+                                       : kCallJSGetterDontUseCachedName);
 }
 
 void CodeStubAssembler::TryGetOwnProperty(

src/objects/lookup.cc

@@ -1416,16 +1416,32 @@ bool LookupIterator::TryLookupCachedProperty() {
 }
 
 bool LookupIterator::LookupCachedProperty(Handle<AccessorPair> accessor_pair) {
+  if (!HolderIsReceiverOrHiddenPrototype()) return false;
+  if (!lookup_start_object_.is_identical_to(receiver_) &&
+      !lookup_start_object_.is_identical_to(holder_)) {
+    return false;
+  }
+
   DCHECK_EQ(state(), LookupIterator::ACCESSOR);
   DCHECK(GetAccessors()->IsAccessorPair(isolate_));
 
+  Object getter = accessor_pair->getter(isolate_);
   base::Optional<Name> maybe_name =
-      FunctionTemplateInfo::TryGetCachedPropertyName(
-          isolate(), accessor_pair->getter(isolate_));
+      FunctionTemplateInfo::TryGetCachedPropertyName(isolate(), getter);
   if (!maybe_name.has_value()) return false;
 
-  // We have found a cached property! Modify the iterator accordingly.
+  if (getter.IsJSFunction()) {
+    // If the getter was a JSFunction there's no guarantee that the holder
+    // actually has a property with the cached name. In that case look it up to
+    // make sure.
+    LookupIterator it(isolate_, holder_, handle(maybe_name.value(), isolate_));
+    if (it.state() != DATA) return false;
+    name_ = it.name();
+  } else {
     name_ = handle(maybe_name.value(), isolate_);
+  }
+
+  // We have found a cached property! Modify the iterator accordingly.
   Restart();
   CHECK_EQ(state(), LookupIterator::DATA);
   return true;

src/objects/templates.cc

@@ -130,7 +130,12 @@ FunctionTemplateRareData FunctionTemplateInfo::AllocateFunctionTemplateRareData(
 base::Optional<Name> FunctionTemplateInfo::TryGetCachedPropertyName(
     Isolate* isolate, Object getter) {
   DisallowGarbageCollection no_gc;
-  if (!getter.IsFunctionTemplateInfo()) return {};
+  if (!getter.IsFunctionTemplateInfo()) {
+    if (!getter.IsJSFunction()) return {};
+    SharedFunctionInfo info = JSFunction::cast(getter).shared();
+    if (!info.IsApiFunction()) return {};
+    getter = info.get_api_func_data();
+  }
   // Check if the accessor uses a cached property.
   Object maybe_name = FunctionTemplateInfo::cast(getter).cached_property_name();
   if (maybe_name.IsTheHole(isolate)) return {};