[runtime] Refactor interceptor handling

... and add regression test for contextual stores to JSGlobalObject
with interceptor in the prototype chain.

Bug: chromium:1216437
Change-Id: Ibd344288c6327b35f3276f59517995d591acb967
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2944895
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75038}

src/objects/objects.cc

@@ -2645,6 +2645,34 @@ Maybe<bool> Object::SetPropertyInternal(LookupIterator* it,
   return Nothing<bool>();
 }
 
+namespace {
+
+// If the receiver is the JSGlobalObject, the store was contextual. In case
+// the property did not exist yet on the global object itself, we have to
+// throw a reference error in strict mode.  In sloppy mode, we continue.
+// Returns false if the exception was thrown, otherwise true.
+bool CheckContextualStoreToJSGlobalObject(LookupIterator* it,
+                                          Maybe<ShouldThrow> should_throw) {
+  Isolate* isolate = it->isolate();
+
+  if (it->GetReceiver()->IsJSGlobalObject(isolate) &&
+      (GetShouldThrow(isolate, should_throw) == ShouldThrow::kThrowOnError)) {
+    if (it->state() == LookupIterator::TRANSITION) {
+      // The property cell that we have created is garbage because we are going
+      // to throw now instead of putting it into the global dictionary. However,
+      // the cell might already have been stored into the feedback vector, so
+      // we must invalidate it nevertheless.
+      it->transition_cell()->ClearAndInvalidate(ReadOnlyRoots(isolate));
+    }
+    isolate->Throw(*isolate->factory()->NewReferenceError(
+        MessageTemplate::kNotDefined, it->GetName()));
+    return false;
+  }
+  return true;
+}
+
+}  // namespace
+
 Maybe<bool> Object::SetProperty(LookupIterator* it, Handle<Object> value,
                                 StoreOrigin store_origin,
                                 Maybe<ShouldThrow> should_throw) {
@@ -2655,26 +2683,9 @@ Maybe<bool> Object::SetProperty(LookupIterator* it, Handle<Object> value,
     if (found) return result;
   }
 
-  // TODO(ishell): refactor this: both SetProperty and and SetSuperProperty have
+  if (!CheckContextualStoreToJSGlobalObject(it, should_throw)) {
-  // this piece of code.
-  // If the receiver is the JSGlobalObject, the store was contextual. In case
-  // the property did not exist yet on the global object itself, we have to
-  // throw a reference error in strict mode.  In sloppy mode, we continue.
-  if (it->GetReceiver()->IsJSGlobalObject() &&
-      (GetShouldThrow(it->isolate(), should_throw) ==
-       ShouldThrow::kThrowOnError)) {
-    if (it->state() == LookupIterator::TRANSITION) {
-      // The property cell that we have created is garbage because we are going
-      // to throw now instead of putting it into the global dictionary. However,
-      // the cell might already have been stored into the feedback vector, so
-      // we must invalidate it nevertheless.
-      it->transition_cell()->ClearAndInvalidate(ReadOnlyRoots(it->isolate()));
-    }
-    it->isolate()->Throw(*it->isolate()->factory()->NewReferenceError(
-        MessageTemplate::kNotDefined, it->GetName()));
     return Nothing<bool>();
   }
-
   return AddDataProperty(it, value, NONE, should_throw, store_origin);
 }
 
@@ -2764,25 +2775,9 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
     }
   }
 
-  // TODO(ishell): refactor this: both SetProperty and and SetSuperProperty have
+  if (!CheckContextualStoreToJSGlobalObject(&own_lookup, should_throw)) {
-  // this piece of code.
-  // If the receiver is the JSGlobalObject, the store was contextual. In case
-  // the property did not exist yet on the global object itself, we have to
-  // throw a reference error in strict mode.  In sloppy mode, we continue.
-  if (receiver->IsJSGlobalObject() &&
-      (GetShouldThrow(isolate, should_throw) == ShouldThrow::kThrowOnError)) {
-    if (own_lookup.state() == LookupIterator::TRANSITION) {
-      // The property cell that we have created is garbage because we are going
-      // to throw now instead of putting it into the global dictionary. However,
-      // the cell might already have been stored into the feedback vector, so
-      // we must invalidate it nevertheless.
-      own_lookup.transition_cell()->ClearAndInvalidate(ReadOnlyRoots(isolate));
-    }
-    isolate->Throw(*isolate->factory()->NewReferenceError(
-        MessageTemplate::kNotDefined, own_lookup.GetName()));
     return Nothing<bool>();
   }
-
   return AddDataProperty(&own_lookup, value, NONE, should_throw, store_origin);
 }
 

test/cctest/test-api-interceptors.cc

@@ -1698,6 +1698,37 @@ THREADED_TEST(EmptyInterceptorDoesNotShadowAccessors) {
   ExpectInt32("child.accessor_age", 10);
 }
 
+THREADED_TEST(EmptyInterceptorVsStoreGlobalICs) {
+  // In sloppy mode storing to global must succeed.
+  CheckInterceptorIC(EmptyInterceptorGetter,
+                     HasICQuery<Local<Name>, v8::internal::ABSENT>,
+                     "globalThis.__proto__ = o;"
+                     "let result = 0;"
+                     "for (var i = 0; i < 20; i++) {"
+                     "  try {"
+                     "    x = i;"
+                     "    result++;"
+                     "  } catch (e) {}"
+                     "}"
+                     "result + x",
+                     20 + 19);
+
+  // In strict mode storing to global must throw.
+  CheckInterceptorIC(EmptyInterceptorGetter,
+                     HasICQuery<Local<Name>, v8::internal::ABSENT>,
+                     "'use strict';"
+                     "globalThis.__proto__ = o;"
+                     "let result = 0;"
+                     "for (var i = 0; i < 20; i++) {"
+                     "  try {"
+                     "    x = i;"
+                     "  } catch (e) {"
+                     "    result++;"
+                     "  }"
+                     "}"
+                     "result + (typeof(x) === 'undefined' ? 100 : 0)",
+                     120);
+}
 
 THREADED_TEST(LegacyInterceptorDoesNotSeeSymbols) {
   LocalContext env;