Merged: [lookup] Robustify LookupIterator against own lookups

... on non-JSReceiver objects.

Bug: chromium:1447430
(cherry picked from commit 515f187)

Change-Id: Ib260f028eece91135860d09871ee769b834cd53e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4575070
Reviewed-by: Toon Verwaest <[email protected]>
Commit-Queue: Igor Sheludko <[email protected]>
Cr-Commit-Position: refs/branch-heads/11.5@{v8#4}
Cr-Branched-From: 0c4044b-refs/heads/11.5.150@{#1}
Cr-Branched-From: b71d303-refs/heads/main@{#87781}

src/objects/lookup-inl.h

@@ -206,7 +206,7 @@ Handle<Name> PropertyKey::GetName(Isolate* isolate) {
 }
 
 Handle<Name> LookupIterator::name() const {
-  DCHECK(!IsElement(*holder_));
+  DCHECK_IMPLIES(!holder_.is_null(), !IsElement(*holder_));
   return name_;
 }
 
@@ -302,13 +302,15 @@ void LookupIterator::UpdateProtector() {
 }
 
 InternalIndex LookupIterator::descriptor_number() const {
+  DCHECK(!holder_.is_null());
   DCHECK(!IsElement(*holder_));
   DCHECK(has_property_);
   DCHECK(holder_->HasFastProperties(isolate_));
   return number_;
 }
 
 InternalIndex LookupIterator::dictionary_entry() const {
+  DCHECK(!holder_.is_null());
   DCHECK(!IsElement(*holder_));
   DCHECK(has_property_);
   DCHECK(!holder_->HasFastProperties(isolate_));
@@ -323,13 +325,14 @@ LookupIterator::Configuration LookupIterator::ComputeConfiguration(
 }
 
 // static
-Handle<JSReceiver> LookupIterator::GetRoot(Isolate* isolate,
-                                           Handle<Object> lookup_start_object,
-                                           size_t index) {
+MaybeHandle<JSReceiver> LookupIterator::GetRoot(
+    Isolate* isolate, Handle<Object> lookup_start_object, size_t index,
+    Configuration configuration) {
   if (lookup_start_object->IsJSReceiver(isolate)) {
     return Handle<JSReceiver>::cast(lookup_start_object);
   }
-  return GetRootForNonJSReceiver(isolate, lookup_start_object, index);
+  return GetRootForNonJSReceiver(isolate, lookup_start_object, index,
+                                 configuration);
 }
 
 template <class T>

src/objects/lookup.cc

@@ -45,7 +45,17 @@ PropertyKey::PropertyKey(Isolate* isolate, Handle<Object> key, bool* success) {
 template <bool is_element>
 void LookupIterator::Start() {
   // GetRoot might allocate if lookup_start_object_ is a string.
-  holder_ = GetRoot(isolate_, lookup_start_object_, index_);
+  MaybeHandle<JSReceiver> maybe_holder =
+      GetRoot(isolate_, lookup_start_object_, index_, configuration_);
+  if (!maybe_holder.ToHandle(&holder_)) {
+    // This is an attempt to perform an own property lookup on a non-JSReceiver
+    // that doesn't have any properties.
+    DCHECK(!lookup_start_object_->IsJSReceiver());
+    DCHECK(!check_prototype_chain());
+    has_property_ = false;
+    state_ = NOT_FOUND;
+    return;
+  }
 
   {
     DisallowGarbageCollection no_gc;
@@ -118,19 +128,27 @@ template void LookupIterator::RestartInternal<true>(InterceptorState);
 template void LookupIterator::RestartInternal<false>(InterceptorState);
 
 // static
-Handle<JSReceiver> LookupIterator::GetRootForNonJSReceiver(
-    Isolate* isolate, Handle<Object> lookup_start_object, size_t index) {
-  // Strings are the only objects with properties (only elements) directly on
-  // the wrapper. Hence we can skip generating the wrapper for all other cases.
-  if (lookup_start_object->IsString(isolate) &&
-      index <
-          static_cast<size_t>(String::cast(*lookup_start_object).length())) {
-    // TODO(verwaest): Speed this up. Perhaps use a cached wrapper on the native
-    // context, ensuring that we don't leak it into JS?
-    Handle<JSFunction> constructor = isolate->string_function();
-    Handle<JSObject> result = isolate->factory()->NewJSObject(constructor);
-    Handle<JSPrimitiveWrapper>::cast(result)->set_value(*lookup_start_object);
-    return result;
+MaybeHandle<JSReceiver> LookupIterator::GetRootForNonJSReceiver(
+    Isolate* isolate, Handle<Object> lookup_start_object, size_t index,
+    Configuration configuration) {
+  // Strings are the only non-JSReceiver objects with properties (only elements
+  // and 'length') directly on the wrapper. Hence we can skip generating
+  // the wrapper for all other cases.
+  bool own_property_lookup = (configuration & kPrototypeChain) == 0;
+  if (lookup_start_object->IsString(isolate)) {
+    if (own_property_lookup ||
+        index <
+            static_cast<size_t>(String::cast(*lookup_start_object).length())) {
+      // TODO(verwaest): Speed this up. Perhaps use a cached wrapper on the
+      // native context, ensuring that we don't leak it into JS?
+      Handle<JSFunction> constructor = isolate->string_function();
+      Handle<JSObject> result = isolate->factory()->NewJSObject(constructor);
+      Handle<JSPrimitiveWrapper>::cast(result)->set_value(*lookup_start_object);
+      return result;
+    }
+  } else if (own_property_lookup) {
+    // Signal that the lookup will not find anything.
+    return {};
   }
   Handle<HeapObject> root(
       lookup_start_object->GetPrototypeChainRootMap(isolate).prototype(isolate),
@@ -902,6 +920,7 @@ Handle<Object> LookupIterator::FetchValue(
 }
 
 bool LookupIterator::CanStayConst(Object value) const {
+  DCHECK(!holder_.is_null());
   DCHECK(!IsElement(*holder_));
   DCHECK(holder_->HasFastProperties(isolate_));
   DCHECK_EQ(PropertyLocation::kField, property_details_.location());
@@ -935,6 +954,7 @@ bool LookupIterator::CanStayConst(Object value) const {
 }
 
 bool LookupIterator::DictCanStayConst(Object value) const {
+  DCHECK(!holder_.is_null());
   DCHECK(!IsElement(*holder_));
   DCHECK(!holder_->HasFastProperties(isolate_));
   DCHECK(!holder_->IsJSGlobalObject());
@@ -981,13 +1001,15 @@ int LookupIterator::GetAccessorIndex() const {
 
 FieldIndex LookupIterator::GetFieldIndex() const {
   DCHECK(has_property_);
+  DCHECK(!holder_.is_null());
   DCHECK(holder_->HasFastProperties(isolate_));
   DCHECK_EQ(PropertyLocation::kField, property_details_.location());
   DCHECK(!IsElement(*holder_));
   return FieldIndex::ForDetails(holder_->map(isolate_), property_details_);
 }
 
 Handle<PropertyCell> LookupIterator::GetPropertyCell() const {
+  DCHECK(!holder_.is_null());
   DCHECK(!IsElement(*holder_));
   Handle<JSGlobalObject> holder = GetHolder<JSGlobalObject>();
   return handle(holder->global_dictionary(isolate_, kAcquireLoad)

src/objects/lookup.h

@@ -293,12 +293,12 @@ class V8_EXPORT_PRIVATE LookupIterator final {
                                                    Configuration configuration,
                                                    Handle<Name> name);
 
-  static Handle<JSReceiver> GetRootForNonJSReceiver(
-      Isolate* isolate, Handle<Object> lookup_start_object,
-      size_t index = kInvalidIndex);
-  static inline Handle<JSReceiver> GetRoot(Isolate* isolate,
-                                           Handle<Object> lookup_start_object,
-                                           size_t index = kInvalidIndex);
+  static MaybeHandle<JSReceiver> GetRootForNonJSReceiver(
+      Isolate* isolate, Handle<Object> lookup_start_object, size_t index,
+      Configuration configuration);
+  static inline MaybeHandle<JSReceiver> GetRoot(
+      Isolate* isolate, Handle<Object> lookup_start_object, size_t index,
+      Configuration configuration);
 
   State NotFound(JSReceiver const holder) const;
 

test/mjsunit/regress/regress-crbug-1447430.js

@@ -0,0 +1,34 @@
+// Copyright 2023 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: --allow-natives-syntax
+
+var s = %CreatePrivateSymbol('x');
+
+(function TestSmi() {
+  function f(o, p) {
+    o[p] = 153;
+  }
+  (1).__proto__[s] = 42;
+  %PrepareFunctionForOptimization(f);
+  assertEquals(f(42, s), undefined);
+}());
+
+(function TestString() {
+  function f(o, p) {
+    o[p] = 153;
+  }
+  ('xyz').__proto__[s] = 42;
+  %PrepareFunctionForOptimization(f);
+  assertEquals(f('abc', s), undefined);
+}());
+
+(function TestSymbol() {
+  function f(o, p) {
+    o[p] = 153;
+  }
+  Symbol('xyz').__proto__[s] = 42;
+  %PrepareFunctionForOptimization(f);
+  assertEquals(f(Symbol('abc'), s), undefined);
+}());