[constant-tracking] Refactor the proto function checks, fix regexp fast path check.

This introduces a utility function on code stub assembler to check that
a map has given prototype object map. In addition, if constant field
tracking is active, it will check that a given field is still constant
(typically a function that is assumed not to be patched).

This also fixes a fast path check in regexp C++ code to properly detect
that "exec" function was modified on RegExp's prototype when constant
field tracking is on.

Bug: v8:5495, v8:8361
Change-Id: I98476db46ba4633124cf1437be90aea3585ce978
Reviewed-on: https://chromium-review.googlesource.com/c/1297954
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57290}

src/builtins/builtins-collections-gen.cc

@@ -103,13 +103,13 @@ class BaseCollectionsAssembler : public BaseBuiltinsFromDSLAssembler {
 
   // Checks whether {collection}'s initial add/set function has been modified
   // (depending on {variant}, loaded from {native_context}).
-  void CheckIfInitialAddFunctionModified(Variant variant,
-                                         TNode<Context> native_context,
-                                         TNode<Object> collection,
-                                         Label* if_modified);
+  void GotoIfInitialAddFunctionModified(Variant variant,
+                                        TNode<Context> native_context,
+                                        TNode<Object> collection,
+                                        Label* if_modified);
 
   // Gets root index for the name of the add/set function.
-  TNode<Object> GetAddFunctionName(Variant variant);
+  RootIndex GetAddFunctionNameIndex(Variant variant);
 
   // Retrieves the offset to access the backing table from the collection.
   int GetTableOffset(Variant variant);
@@ -189,8 +189,8 @@ void BaseCollectionsAssembler::AddConstructorEntries(
     TNode<Object> table = AllocateTable(variant, context, at_least_space_for);
     StoreObjectField(collection, GetTableOffset(variant), table);
     GotoIf(IsNullOrUndefined(initial_entries), &exit);
-    CheckIfInitialAddFunctionModified(variant, native_context, collection,
-                                      &slow_loop);
+    GotoIfInitialAddFunctionModified(variant, native_context, collection,
+                                     &slow_loop);
     Branch(use_fast_loop.value(), &fast_loop, &slow_loop);
   }
   BIND(&fast_loop);
@@ -215,8 +215,8 @@ void BaseCollectionsAssembler::AddConstructorEntries(
       {
         // Check that add/set function has not been modified.
         Label if_not_modified(this), if_modified(this);
-        CheckIfInitialAddFunctionModified(variant, native_context, collection,
-                                          &if_modified);
+        GotoIfInitialAddFunctionModified(variant, native_context, collection,
+                                         &if_modified);
         Goto(&if_not_modified);
         BIND(&if_modified);
         Unreachable();
@@ -345,63 +345,28 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromIterable(
   BIND(&exit);
 }
 
-TNode<Object> BaseCollectionsAssembler::GetAddFunctionName(Variant variant) {
+RootIndex BaseCollectionsAssembler::GetAddFunctionNameIndex(Variant variant) {
   switch (variant) {
     case kMap:
     case kWeakMap:
-      return LoadRoot(RootIndex::kset_string);
+      return RootIndex::kset_string;
     case kSet:
     case kWeakSet:
-      return LoadRoot(RootIndex::kadd_string);
+      return RootIndex::kadd_string;
   }
   UNREACHABLE();
 }
 
-void BaseCollectionsAssembler::CheckIfInitialAddFunctionModified(
+void BaseCollectionsAssembler::GotoIfInitialAddFunctionModified(
     Variant variant, TNode<Context> native_context, TNode<Object> collection,
     Label* if_modified) {
-  Label done(this);
-  TVARIABLE(BoolT, result, Int32FalseConstant());
-
-  TNode<Map> prototype_map =
-      LoadMap(LoadMapPrototype(LoadMap(CAST(collection))));
-  GotoIfNot(WordEqual(prototype_map,
-                      GetInitialCollectionPrototype(variant, native_context)),
-            if_modified);
-
-  if (FLAG_track_constant_fields) {
-    // With constant field tracking, we need to make sure that the add/set
-    // function in the prototype has not been tampered with. We do this by
-    // checking that the slot in the prototype's descriptor array is still
-    // marked as const.
-    TNode<DescriptorArray> descriptors = LoadMapDescriptors(prototype_map);
-
-    STATIC_ASSERT(JSCollection::kAddFunctionDescriptorIndex ==
-                  JSWeakCollection::kAddFunctionDescriptorIndex);
-    int index = JSCollection::kAddFunctionDescriptorIndex;
-
-    // Assert the index is in-bounds.
-    CSA_ASSERT(this, SmiLessThan(SmiConstant(index),
-                                 LoadWeakFixedArrayLength(descriptors)));
-    // Assert that the name is correct. This essentially checks that
-    // kAddFunctionDescriptorArrayIndex corresponds to the insertion order in
-    // the bootstrapper.
-    CSA_ASSERT(this,
-               WordEqual(LoadWeakFixedArrayElement(
-                             descriptors, DescriptorArray::ToKeyIndex(index)),
-                         GetAddFunctionName(variant)));
-
-    TNode<Uint32T> details =
-        DescriptorArrayGetDetails(descriptors, Uint32Constant(index));
-
-    TNode<Uint32T> constness =
-        DecodeWord32<PropertyDetails::ConstnessField>(details);
-
-    GotoIfNot(
-        Word32Equal(constness,
-                    Int32Constant(static_cast<int>(PropertyConstness::kConst))),
-        if_modified);
-  }
+  STATIC_ASSERT(JSCollection::kAddFunctionDescriptorIndex ==
+                JSWeakCollection::kAddFunctionDescriptorIndex);
+  GotoIfInitialPrototypePropertyModified(
+      LoadMap(CAST(collection)),
+      GetInitialCollectionPrototype(variant, native_context),
+      JSCollection::kAddFunctionDescriptorIndex,
+      GetAddFunctionNameIndex(variant), if_modified);
 }
 
 TNode<Object> BaseCollectionsAssembler::AllocateJSCollection(

src/builtins/builtins-regexp-gen.cc

@@ -927,42 +927,11 @@ void RegExpBuiltinsAssembler::BranchIfFastRegExp(Node* const context,
 
   Node* const initial_proto_initial_map =
       LoadContextElement(native_context, Context::REGEXP_PROTOTYPE_MAP_INDEX);
-  Node* const proto_map = LoadMap(LoadMapPrototype(map));
-
-  Node* const proto_has_initialmap =
-      WordEqual(proto_map, initial_proto_initial_map);
-
-  GotoIfNot(proto_has_initialmap, if_ismodified);
-
-  if (FLAG_track_constant_fields) {
-    // With constant field tracking, we need to make sure that the exec
-    // function in the prototype has not been tampered with. We do this by
-    // checking that the slot in the prototype's descriptor array is still
-    // marked as const.
-    TNode<DescriptorArray> descriptors = LoadMapDescriptors(proto_map);
-
-    const int index = JSRegExp::kExecFunctionDescriptorIndex;
-    // Assert the index is in-bounds.
-    CSA_ASSERT(this, SmiLessThan(SmiConstant(index),
-                                 LoadWeakFixedArrayLength(descriptors)));
-    // Assert that the name is correct. This essentially checks that
-    // the index corresponds to the insertion order in the bootstrapper.
-    CSA_ASSERT(this,
-               WordEqual(LoadWeakFixedArrayElement(
-                             descriptors, DescriptorArray::ToKeyIndex(index)),
-                         LoadRoot(RootIndex::kexec_string)));
-
-    TNode<Uint32T> details =
-        DescriptorArrayGetDetails(descriptors, Uint32Constant(index));
 
-    TNode<Uint32T> constness =
-        DecodeWord32<PropertyDetails::ConstnessField>(details);
-
-    GotoIfNot(
-        Word32Equal(constness,
-                    Int32Constant(static_cast<int>(PropertyConstness::kConst))),
-        if_ismodified);
-  }
+  GotoIfInitialPrototypePropertyModified(
+      CAST(map), CAST(initial_proto_initial_map),
+      JSRegExp::kExecFunctionDescriptorIndex, RootIndex::kexec_string,
+      if_ismodified);
 
   // The smi check is required to omit ToLength(lastIndex) calls with possible
   // user-code execution on the fast path.

src/code-stub-assembler.cc

@@ -13531,5 +13531,42 @@ void CodeStubAssembler::SetPropertyLength(TNode<Context> context,
   BIND(&done);
 }
 
+void CodeStubAssembler::GotoIfInitialPrototypePropertyModified(
+    TNode<Map> object_map, TNode<Map> initial_prototype_map, int descriptor,
+    RootIndex field_name_root_index, Label* if_modified) {
+  TNode<Map> prototype_map = LoadMap(LoadMapPrototype(object_map));
+  GotoIfNot(WordEqual(prototype_map, initial_prototype_map), if_modified);
+
+  if (FLAG_track_constant_fields) {
+    // With constant field tracking, we need to make sure that the property
+    // in the prototype has not been tampered with. We do this by
+    // checking that the slot in the prototype's descriptor array is still
+    // marked as const.
+    TNode<DescriptorArray> descriptors = LoadMapDescriptors(prototype_map);
+
+    // Assert the index is in-bounds.
+    CSA_ASSERT(this, SmiLessThan(SmiConstant(descriptor),
+                                 LoadWeakFixedArrayLength(descriptors)));
+    // Assert that the name is correct. This essentially checks that
+    // the {descriptor} index corresponds to the insertion order in
+    // the bootstrapper.
+    CSA_ASSERT(this, WordEqual(LoadWeakFixedArrayElement(
+                                   descriptors,
+                                   DescriptorArray::ToKeyIndex(descriptor)),
+                               LoadRoot(field_name_root_index)));
+
+    TNode<Uint32T> details =
+        DescriptorArrayGetDetails(descriptors, Uint32Constant(descriptor));
+
+    TNode<Uint32T> constness =
+        DecodeWord32<PropertyDetails::ConstnessField>(details);
+
+    GotoIfNot(
+        Word32Equal(constness,
+                    Int32Constant(static_cast<int>(PropertyConstness::kConst))),
+        if_modified);
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/code-stub-assembler.h

@@ -3130,6 +3130,22 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   void SetPropertyLength(TNode<Context> context, TNode<Object> array,
                          TNode<Number> length);
 
+  // Checks that {object_map}'s prototype map is the {initial_prototype_map} and
+  // makes sure that the field with name at index {descriptor} is still
+  // constant. If it is not, go to label {if_modified}.
+  //
+  // To make the checks robust, the method also asserts that the descriptor has
+  // the right key, the caller must pass the root index of the key
+  // in {field_name_root_index}.
+  //
+  // This is useful for checking that given function has not been patched
+  // on the prototype.
+  void GotoIfInitialPrototypePropertyModified(TNode<Map> object_map,
+                                              TNode<Map> initial_prototype_map,
+                                              int descfriptor,
+                                              RootIndex field_name_root_index,
+                                              Label* if_modified);
+
   // Implements DescriptorArray::Search().
   void DescriptorLookup(SloppyTNode<Name> unique_name,
                         SloppyTNode<DescriptorArray> descriptors,

src/regexp/regexp-utils.cc

@@ -136,9 +136,6 @@ Maybe<bool> RegExpUtils::IsRegExp(Isolate* isolate, Handle<Object> object) {
 }
 
 bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) {
-  // TODO(ishell): Update this check once map changes for constant field
-  // tracking are landing.
-
 #ifdef V8_ENABLE_FORCE_SLOW_PATH
   if (isolate->force_slow_path()) return false;
 #endif
@@ -156,10 +153,25 @@ bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) {
   if (!proto->IsJSReceiver()) return false;
 
   Handle<Map> initial_proto_initial_map = isolate->regexp_prototype_map();
-  if (JSReceiver::cast(proto)->map() != *initial_proto_initial_map) {
+  Map* proto_map = JSReceiver::cast(proto)->map();
+  if (proto_map != *initial_proto_initial_map) {
     return false;
   }
 
+  // Check that the "exec" method is unmodified.
+  if (FLAG_track_constant_fields) {
+    // Check that the index refers to "exec" method (this has to be consistent
+    // with the init order in the bootstrapper).
+    DCHECK_EQ(*(isolate->factory()->exec_string()),
+              proto_map->instance_descriptors()->GetKey(
+                  JSRegExp::kExecFunctionDescriptorIndex));
+    if (proto_map->instance_descriptors()
+            ->GetDetails(JSRegExp::kExecFunctionDescriptorIndex)
+            .constness() != PropertyConstness::kConst) {
+      return false;
+    }
+  }
+
   // The smi check is required to omit ToLength(lastIndex) calls with possible
   // user-code execution on the fast path.
   Object* last_index = JSRegExp::cast(recv)->last_index();

src/regexp/regexp-utils.h

@@ -36,7 +36,8 @@ class RegExpUtils : public AllStatic {
   static Maybe<bool> IsRegExp(Isolate* isolate, Handle<Object> object);
 
   // Checks whether the given object is an unmodified JSRegExp instance.
-  // Neither the object's map, nor its prototype's map may be modified.
+  // Neither the object's map, nor its prototype's map, nor any relevant
+  // method on the prototype may be modified.
   static bool IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj);
 
   // ES#sec-advancestringindex