[protectors] Migrate ArraySpeciesProtector to Protectors

Bug: v8:9463
Change-Id: I4d9d35222597925a289a6c3055ef0ca0aaa43a2f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1775926
Commit-Queue: Joshua Litt <joshualitt@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63553}

src/builtins/builtins-array.cc

@@ -7,6 +7,7 @@
 #include "src/codegen/code-factory.h"
 #include "src/debug/debug.h"
 #include "src/execution/isolate.h"
+#include "src/execution/protectors-inl.h"
 #include "src/handles/global-handles.h"
 #include "src/logging/counters.h"
 #include "src/objects/contexts.h"
@@ -1468,7 +1469,7 @@ BUILTIN(ArrayConcat) {
   // Avoid a real species read to avoid extra lookups to the array constructor
   if (V8_LIKELY(receiver->IsJSArray() &&
                 Handle<JSArray>::cast(receiver)->HasArrayPrototype(isolate) &&
-                isolate->IsArraySpeciesLookupChainIntact())) {
+                Protectors::IsArraySpeciesLookupChainIntact(isolate))) {
     if (Fast_ArrayConcat(isolate, &args).ToHandle(&result_array)) {
       return *result_array;
     }

src/execution/isolate-inl.h

@@ -119,25 +119,6 @@ bool Isolate::IsArrayConstructorIntact() {
   return array_constructor_cell.value() == Smi::FromInt(kProtectorValid);
 }
 
-bool Isolate::IsArraySpeciesLookupChainIntact() {
-  // Note: It would be nice to have debug checks to make sure that the
-  // species protector is accurate, but this would be hard to do for most of
-  // what the protector stands for:
-  // - You'd need to traverse the heap to check that no Array instance has
-  //   a constructor property
-  // - To check that Array[Symbol.species] == Array, JS code has to execute,
-  //   but JS cannot be invoked in callstack overflow situations
-  // All that could be checked reliably is that
-  // Array.prototype.constructor == Array. Given that limitation, no check is
-  // done here. In place, there are mjsunit tests harmony/array-species* which
-  // ensure that behavior is correct in various invalid protector cases.
-
-  PropertyCell species_cell =
-      PropertyCell::cast(root(RootIndex::kArraySpeciesProtector));
-  return species_cell.value().IsSmi() &&
-         Smi::ToInt(species_cell.value()) == kProtectorValid;
-}
-
 bool Isolate::IsTypedArraySpeciesLookupChainIntact() {
   PropertyCell species_cell =
       PropertyCell::cast(root(RootIndex::kTypedArraySpeciesProtector));

src/execution/isolate.cc

@@ -3999,15 +3999,6 @@ void Isolate::InvalidateArrayConstructorProtector() {
   DCHECK(!IsArrayConstructorIntact());
 }
 
-void Isolate::InvalidateArraySpeciesProtector() {
-  DCHECK(factory()->array_species_protector()->value().IsSmi());
-  DCHECK(IsArraySpeciesLookupChainIntact());
-  PropertyCell::SetValueWithInvalidation(
-      this, "array_species_protector", factory()->array_species_protector(),
-      handle(Smi::FromInt(kProtectorInvalid), this));
-  DCHECK(!IsArraySpeciesLookupChainIntact());
-}
-
 void Isolate::InvalidateTypedArraySpeciesProtector() {
   DCHECK(factory()->typed_array_species_protector()->value().IsSmi());
   DCHECK(IsTypedArraySpeciesLookupChainIntact());

src/execution/isolate.h

@@ -1176,7 +1176,6 @@ class Isolate final : private HiddenFactory {
 
   bool IsArrayOrObjectOrStringPrototype(Object object);
 
-  inline bool IsArraySpeciesLookupChainIntact();
   inline bool IsTypedArraySpeciesLookupChainIntact();
 
   // Check that the @@species protector is intact, which guards the lookup of
@@ -1264,7 +1263,6 @@ class Isolate final : private HiddenFactory {
   void TraceProtectorInvalidation(const char* protector_name);
 
   void InvalidateArrayConstructorProtector();
-  void InvalidateArraySpeciesProtector();
   void InvalidateTypedArraySpeciesProtector();
   void InvalidateRegExpSpeciesProtector(Handle<NativeContext> native_context);
   void InvalidatePromiseSpeciesProtector();

src/execution/protectors-inl.h

@@ -13,13 +13,22 @@
 namespace v8 {
 namespace internal {
 
-#define DEFINE_PROTECTOR_CHECK(name, cell)                                  \
+#define DEFINE_PROTECTOR_ON_NATIVE_CONTEXT_CHECK(name, cell)                \
   bool Protectors::Is##name##Intact(Handle<NativeContext> native_context) { \
     PropertyCell species_cell = native_context->cell();                     \
     return species_cell.value().IsSmi() &&                                  \
            Smi::ToInt(species_cell.value()) == kProtectorValid;             \
   }
-DECLARED_PROTECTORS(DEFINE_PROTECTOR_CHECK)
+DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(DEFINE_PROTECTOR_ON_NATIVE_CONTEXT_CHECK)
+
+#define DEFINE_PROTECTOR_ON_ISOLATE_CHECK(name, root_index, unused_cell) \
+  bool Protectors::Is##name##Intact(Isolate* isolate) {                  \
+    PropertyCell cell =                                                  \
+        PropertyCell::cast(isolate->root(RootIndex::k##root_index));     \
+    return cell.value().IsSmi() &&                                       \
+           Smi::ToInt(cell.value()) == kProtectorValid;                  \
+  }
+DECLARED_PROTECTORS_ON_ISOLATE(DEFINE_PROTECTOR_ON_ISOLATE_CHECK)
 
 }  // namespace internal
 }  // namespace v8

src/execution/protectors.cc

@@ -16,7 +16,7 @@
 namespace v8 {
 namespace internal {
 
-#define INVALIDATE_PROTECTOR_DEFINITION(name, cell)                         \
+#define INVALIDATE_PROTECTOR_ON_NATIVE_CONTEXT_DEFINITION(name, cell)       \
   void Protectors::Invalidate##name(Isolate* isolate,                       \
                                     Handle<NativeContext> native_context) { \
     DCHECK_EQ(*native_context, isolate->raw_native_context());              \
@@ -28,8 +28,21 @@ namespace internal {
         handle(Smi::FromInt(kProtectorInvalid), isolate));                  \
     DCHECK(!Is##name##Intact(native_context));                              \
   }
-DECLARED_PROTECTORS(INVALIDATE_PROTECTOR_DEFINITION)
-#undef INVALIDATE_PROTECTOR_DEFINITION
+DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(
+    INVALIDATE_PROTECTOR_ON_NATIVE_CONTEXT_DEFINITION)
+#undef INVALIDATE_PROTECTOR_ON_NATIVE_CONTEXT_DEFINITION
+
+#define INVALIDATE_PROTECTOR_ON_ISOLATE_DEFINITION(name, unused_index, cell) \
+  void Protectors::Invalidate##name(Isolate* isolate) {                      \
+    DCHECK(isolate->factory()->cell()->value().IsSmi());                     \
+    DCHECK(Is##name##Intact(isolate));                                       \
+    PropertyCell::SetValueWithInvalidation(                                  \
+        isolate, #cell, isolate->factory()->cell(),                          \
+        handle(Smi::FromInt(kProtectorInvalid), isolate));                   \
+    DCHECK(!Is##name##Intact(isolate));                                      \
+  }
+DECLARED_PROTECTORS_ON_ISOLATE(INVALIDATE_PROTECTOR_ON_ISOLATE_DEFINITION)
+#undef INVALIDATE_PROTECTOR_ON_ISOLATE_DEFINITION
 
 }  // namespace internal
 }  // namespace v8

src/execution/protectors.h

@@ -15,15 +15,25 @@ class Protectors : public AllStatic {
   static const int kProtectorValid = 1;
   static const int kProtectorInvalid = 0;
 
-#define DECLARED_PROTECTORS(V) \
+#define DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(V) \
   V(RegExpSpeciesLookupChainProtector, regexp_species_protector)
 
-#define DECLARE_PROTECTOR(name, unused_cell)                                 \
+#define DECLARED_PROTECTORS_ON_ISOLATE(V) \
+  V(ArraySpeciesLookupChain, ArraySpeciesProtector, array_species_protector)
+
+#define DECLARE_PROTECTOR_ON_NATIVE_CONTEXT(name, unused_cell)               \
   static inline bool Is##name##Intact(Handle<NativeContext> native_context); \
   static void Invalidate##name(Isolate* isolate,                             \
                                Handle<NativeContext> native_context);
-  DECLARED_PROTECTORS(DECLARE_PROTECTOR)
-#undef DECLARE_PROTECTOR
+  DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(DECLARE_PROTECTOR_ON_NATIVE_CONTEXT)
+#undef DECLARE_PROTECTOR_ON_NATIVE_CONTEXT
+
+#define DECLARE_PROTECTOR_ON_ISOLATE(name, unused_root_index, unused_cell) \
+  static inline bool Is##name##Intact(Isolate* isolate);                   \
+  static void Invalidate##name(Isolate* isolate);
+
+  DECLARED_PROTECTORS_ON_ISOLATE(DECLARE_PROTECTOR_ON_ISOLATE)
+#undef DECLARE_PROTECTOR_ON_ISOLATE
 };
 
 }  // namespace internal

src/objects/lookup.cc

@@ -247,7 +247,7 @@ void LookupIterator::InternalUpdateProtector() {
       native_context = Handle<JSReceiver>::cast(receiver)->GetCreationContext();
     }
 
-    if (!isolate_->IsArraySpeciesLookupChainIntact() &&
+    if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_) &&
         !isolate_->IsPromiseSpeciesLookupChainIntact() &&
         !Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
             native_context) &&
@@ -256,10 +256,10 @@ void LookupIterator::InternalUpdateProtector() {
     }
     // Setting the constructor property could change an instance's @@species
     if (receiver->IsJSArray(isolate_)) {
-      if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
+      if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_)) return;
       isolate_->CountUsage(
           v8::Isolate::UseCounterFeature::kArrayInstanceConstructorModified);
-      isolate_->InvalidateArraySpeciesProtector();
+      Protectors::InvalidateArraySpeciesLookupChain(isolate_);
       return;
     } else if (receiver->IsJSPromise(isolate_)) {
       if (!isolate_->IsPromiseSpeciesLookupChainIntact()) return;
@@ -287,10 +287,10 @@ void LookupIterator::InternalUpdateProtector() {
       // prototype is pointing the same TYPED_ARRAY_PROTOTYPE.
       if (isolate_->IsInAnyContext(*receiver,
                                    Context::INITIAL_ARRAY_PROTOTYPE_INDEX)) {
-        if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
+        if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_)) return;
         isolate_->CountUsage(
             v8::Isolate::UseCounterFeature::kArrayPrototypeConstructorModified);
-        isolate_->InvalidateArraySpeciesProtector();
+        Protectors::InvalidateArraySpeciesLookupChain(isolate_);
       } else if (isolate_->IsInAnyContext(*receiver,
                                           Context::PROMISE_PROTOTYPE_INDEX)) {
         if (!isolate_->IsPromiseSpeciesLookupChainIntact()) return;
@@ -343,7 +343,7 @@ void LookupIterator::InternalUpdateProtector() {
       native_context = Handle<JSReceiver>::cast(receiver)->GetCreationContext();
     }
 
-    if (!isolate_->IsArraySpeciesLookupChainIntact() &&
+    if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_) &&
         !isolate_->IsPromiseSpeciesLookupChainIntact() &&
         !Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
             native_context) &&
@@ -353,10 +353,10 @@ void LookupIterator::InternalUpdateProtector() {
     // Setting the Symbol.species property of any Array, Promise or TypedArray
     // constructor invalidates the @@species protector
     if (isolate_->IsInAnyContext(*receiver, Context::ARRAY_FUNCTION_INDEX)) {
-      if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
+      if (!Protectors::IsArraySpeciesLookupChainIntact(isolate_)) return;
       isolate_->CountUsage(
           v8::Isolate::UseCounterFeature::kArraySpeciesModified);
-      isolate_->InvalidateArraySpeciesProtector();
+      Protectors::InvalidateArraySpeciesLookupChain(isolate_);
     } else if (isolate_->IsInAnyContext(*receiver,
                                         Context::PROMISE_FUNCTION_INDEX)) {
       if (!isolate_->IsPromiseSpeciesLookupChainIntact()) return;

src/objects/objects.cc

@@ -33,6 +33,7 @@
 #include "src/execution/frames-inl.h"
 #include "src/execution/isolate-inl.h"
 #include "src/execution/microtask-queue.h"
+#include "src/execution/protectors-inl.h"
 #include "src/heap/heap-inl.h"
 #include "src/heap/read-only-heap.h"
 #include "src/ic/ic.h"
@@ -1667,7 +1668,7 @@ MaybeHandle<Object> Object::ArraySpeciesConstructor(
   Handle<Object> default_species = isolate->array_function();
   if (original_array->IsJSArray() &&
       Handle<JSArray>::cast(original_array)->HasArrayPrototype(isolate) &&
-      isolate->IsArraySpeciesLookupChainIntact()) {
+      Protectors::IsArraySpeciesLookupChainIntact(isolate)) {
     return default_species;
   }
   Handle<Object> constructor = isolate->factory()->undefined_value();

src/runtime/runtime-test.cc

@@ -17,6 +17,7 @@
 #include "src/execution/arguments-inl.h"
 #include "src/execution/frames-inl.h"
 #include "src/execution/isolate-inl.h"
+#include "src/execution/protectors-inl.h"
 #include "src/execution/runtime-profiler.h"
 #include "src/heap/heap-inl.h"  // For ToBoolean. TODO(jkummerow): Drop.
 #include "src/heap/heap-write-barrier-inl.h"
@@ -1099,7 +1100,8 @@ TYPED_ARRAYS(FIXED_TYPED_ARRAYS_CHECK_RUNTIME_FUNCTION)
 RUNTIME_FUNCTION(Runtime_ArraySpeciesProtector) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(0, args.length());
-  return isolate->heap()->ToBoolean(isolate->IsArraySpeciesLookupChainIntact());
+  return isolate->heap()->ToBoolean(
+      Protectors::IsArraySpeciesLookupChainIntact(isolate));
 }
 
 RUNTIME_FUNCTION(Runtime_MapIteratorProtector) {