[constant-tracking] Properly check regexp proto symbols in string search.

This updates fast path checks in string's search/match/replace/split/matchAll
methods.

Bug: v8:8361
Change-Id: I0377aff21e380d6c718e7471f8964e10c030281b
Reviewed-on: https://chromium-review.googlesource.com/c/1333668
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57509}

src/bootstrapper.cc

@@ -2573,18 +2573,26 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
       SimpleInstallFunction(isolate_, prototype, factory->match_symbol(),
                             "[Symbol.match]", Builtins::kRegExpPrototypeMatch,
                             1, true);
+      DCHECK_EQ(JSRegExp::kSymbolMatchFunctionDescriptorIndex,
+                prototype->map()->LastAdded());
 
       SimpleInstallFunction(isolate_, prototype, factory->replace_symbol(),
                             "[Symbol.replace]",
                             Builtins::kRegExpPrototypeReplace, 2, false);
+      DCHECK_EQ(JSRegExp::kSymbolReplaceFunctionDescriptorIndex,
+                prototype->map()->LastAdded());
 
       SimpleInstallFunction(isolate_, prototype, factory->search_symbol(),
                             "[Symbol.search]", Builtins::kRegExpPrototypeSearch,
                             1, true);
+      DCHECK_EQ(JSRegExp::kSymbolSearchFunctionDescriptorIndex,
+                prototype->map()->LastAdded());
 
       SimpleInstallFunction(isolate_, prototype, factory->split_symbol(),
                             "[Symbol.split]", Builtins::kRegExpPrototypeSplit,
                             2, false);
+      DCHECK_EQ(JSRegExp::kSymbolSplitFunctionDescriptorIndex,
+                prototype->map()->LastAdded());
 
       Handle<Map> prototype_map(prototype->map(), isolate());
       Map::SetShouldBeFastPrototypeMap(prototype_map, true, isolate_);
@@ -4505,6 +4513,8 @@ void Genesis::InitializeGlobal_harmony_string_matchall() {
     Handle<Map> regexp_prototype_map(regexp_prototype->map(), isolate());
     Map::SetShouldBeFastPrototypeMap(regexp_prototype_map, true, isolate());
     native_context()->set_regexp_prototype_map(*regexp_prototype_map);
+    DCHECK_EQ(JSRegExp::kSymbolMatchAllFunctionDescriptorIndex,
+              regexp_prototype->map()->LastAdded());
   }
 
   {  // --- R e g E x p S t r i n g  I t e r a t o r ---

src/builtins/builtins-regexp-gen.cc

@@ -907,11 +907,10 @@ Node* RegExpBuiltinsAssembler::IsFastRegExpNoPrototype(Node* const context,
 // We use a fairly coarse granularity for this and simply check whether both
 // the regexp itself is unmodified (i.e. its map has not changed), its
 // prototype is unmodified, and lastIndex is a non-negative smi.
-void RegExpBuiltinsAssembler::BranchIfFastRegExp(Node* const context,
-                                                 Node* const object,
-                                                 Node* const map,
-                                                 Label* const if_isunmodified,
-                                                 Label* const if_ismodified) {
+void RegExpBuiltinsAssembler::BranchIfFastRegExp(
+    Node* const context, Node* const object, Node* const map,
+    base::Optional<DescriptorIndexAndName> additional_property_to_check,
+    Label* const if_isunmodified, Label* const if_ismodified) {
   CSA_ASSERT(this, WordEqual(LoadMap(object), map));
 
   GotoIfForceSlowPath(if_ismodified);
@@ -928,9 +927,17 @@ void RegExpBuiltinsAssembler::BranchIfFastRegExp(Node* const context,
   Node* const initial_proto_initial_map =
       LoadContextElement(native_context, Context::REGEXP_PROTOTYPE_MAP_INDEX);
 
-  GotoIfInitialPrototypePropertyModified(
+  DescriptorIndexAndName properties_to_check[2];
+  int property_count = 0;
+  properties_to_check[property_count++] = DescriptorIndexAndName{
+      JSRegExp::kExecFunctionDescriptorIndex, RootIndex::kexec_string};
+  if (additional_property_to_check) {
+    properties_to_check[property_count++] = *additional_property_to_check;
+  }
+
+  GotoIfInitialPrototypePropertiesModified(
       CAST(map), CAST(initial_proto_initial_map),
-      JSRegExp::kExecFunctionDescriptorIndex, RootIndex::kexec_string,
+      Vector<DescriptorIndexAndName>(properties_to_check, property_count),
       if_ismodified);
 
   // The smi check is required to omit ToLength(lastIndex) calls with possible
@@ -944,8 +951,8 @@ void RegExpBuiltinsAssembler::BranchIfFastRegExp(Node* const context,
                                                  Label* const if_isunmodified,
                                                  Label* const if_ismodified) {
   CSA_ASSERT(this, TaggedIsNotSmi(object));
-  BranchIfFastRegExp(context, object, LoadMap(object), if_isunmodified,
-                     if_ismodified);
+  BranchIfFastRegExp(context, object, LoadMap(object), base::nullopt,
+                     if_isunmodified, if_ismodified);
 }
 
 TNode<BoolT> RegExpBuiltinsAssembler::IsFastRegExp(SloppyTNode<Context> context,
@@ -1260,7 +1267,8 @@ TF_BUILTIN(RegExpPrototypeFlagsGetter, RegExpBuiltinsAssembler) {
   TNode<JSReceiver> receiver = CAST(maybe_receiver);
 
   Label if_isfastpath(this), if_isslowpath(this, Label::kDeferred);
-  BranchIfFastRegExp(context, receiver, map, &if_isfastpath, &if_isslowpath);
+  BranchIfFastRegExp(context, receiver, map, base::nullopt, &if_isfastpath,
+                     &if_isslowpath);
 
   BIND(&if_isfastpath);
   Return(FlagsGetter(context, receiver, true));

src/builtins/builtins-regexp-gen.h

@@ -5,6 +5,7 @@
 #ifndef V8_BUILTINS_BUILTINS_REGEXP_GEN_H_
 #define V8_BUILTINS_BUILTINS_REGEXP_GEN_H_
 
+#include "src/base/optional.h"
 #include "src/code-stub-assembler.h"
 #include "src/message-template.h"
 
@@ -16,9 +17,10 @@ class RegExpBuiltinsAssembler : public CodeStubAssembler {
   explicit RegExpBuiltinsAssembler(compiler::CodeAssemblerState* state)
       : CodeStubAssembler(state) {}
 
-  void BranchIfFastRegExp(Node* const context, Node* const object,
-                          Node* const map, Label* const if_isunmodified,
-                          Label* const if_ismodified);
+  void BranchIfFastRegExp(
+      Node* const context, Node* const object, Node* const map,
+      base::Optional<DescriptorIndexAndName> additional_property_to_check,
+      Label* const if_isunmodified, Label* const if_ismodified);
 
   // Create and initialize a RegExp object.
   TNode<Object> RegExpCreate(TNode<Context> context,

src/builtins/builtins-string-gen.cc

@@ -1057,8 +1057,8 @@ void StringBuiltinsAssembler::RequireObjectCoercible(Node* const context,
 
 void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
     Node* const context, Node* const object, Node* const maybe_string,
-    Handle<Symbol> symbol, const NodeFunction0& regexp_call,
-    const NodeFunction1& generic_call) {
+    Handle<Symbol> symbol, DescriptorIndexAndName symbol_index,
+    const NodeFunction0& regexp_call, const NodeFunction1& generic_call) {
   Label out(this);
 
   // Smis definitely don't have an attached symbol.
@@ -1075,8 +1075,8 @@ void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
     GotoIfNot(IsString(maybe_string), &slow_lookup);
 
     RegExpBuiltinsAssembler regexp_asm(state());
-    regexp_asm.BranchIfFastRegExp(context, object, LoadMap(object), &stub_call,
-                                  &slow_lookup);
+    regexp_asm.BranchIfFastRegExp(context, object, LoadMap(object),
+                                  symbol_index, &stub_call, &slow_lookup);
 
     BIND(&stub_call);
     // TODO(jgruber): Add a no-JS scope once it exists.
@@ -1290,6 +1290,8 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) {
 
   MaybeCallFunctionAtSymbol(
       context, search, receiver, isolate()->factory()->replace_symbol(),
+      DescriptorIndexAndName{JSRegExp::kSymbolReplaceFunctionDescriptorIndex,
+                             RootIndex::kreplace_symbol},
       [=]() {
         Return(CallBuiltin(Builtins::kRegExpReplace, context, search, receiver,
                            replace));
@@ -1440,18 +1442,25 @@ class StringMatchSearchAssembler : public StringBuiltinsAssembler {
 
     Builtins::Name builtin;
     Handle<Symbol> symbol;
+    DescriptorIndexAndName property_to_check;
     if (variant == kMatch) {
       builtin = Builtins::kRegExpMatchFast;
       symbol = isolate()->factory()->match_symbol();
+      property_to_check =
+          DescriptorIndexAndName{JSRegExp::kSymbolMatchFunctionDescriptorIndex,
+                                 RootIndex::kmatch_symbol};
     } else {
       builtin = Builtins::kRegExpSearchFast;
       symbol = isolate()->factory()->search_symbol();
+      property_to_check =
+          DescriptorIndexAndName{JSRegExp::kSymbolSearchFunctionDescriptorIndex,
+                                 RootIndex::ksearch_symbol};
     }
 
     RequireObjectCoercible(context, receiver, method_name);
 
     MaybeCallFunctionAtSymbol(
-        context, maybe_regexp, receiver, symbol,
+        context, maybe_regexp, receiver, symbol, property_to_check,
         [=] { Return(CallBuiltin(builtin, context, maybe_regexp, receiver)); },
         [=](Node* fn) {
           Callable call_callable = CodeFactory::Call(isolate());
@@ -1472,8 +1481,8 @@ class StringMatchSearchAssembler : public StringBuiltinsAssembler {
           context, initial_map, maybe_regexp, EmptyStringConstant());
 
       Label fast_path(this), slow_path(this);
-      regexp_asm.BranchIfFastRegExp(context, regexp, initial_map, &fast_path,
-                                    &slow_path);
+      regexp_asm.BranchIfFastRegExp(context, regexp, initial_map,
+                                    property_to_check, &fast_path, &slow_path);
 
       BIND(&fast_path);
       Return(CallBuiltin(builtin, context, regexp, receiver_string));
@@ -1533,9 +1542,12 @@ TF_BUILTIN(StringPrototypeMatchAll, StringBuiltinsAssembler) {
       Callable call_callable = CodeFactory::Call(isolate());
       Return(CallJS(call_callable, context, fn, maybe_regexp, receiver));
     };
-    MaybeCallFunctionAtSymbol(context, maybe_regexp, receiver,
-                              isolate()->factory()->match_all_symbol(),
-                              if_regexp_call, if_generic_call);
+    MaybeCallFunctionAtSymbol(
+        context, maybe_regexp, receiver,
+        isolate()->factory()->match_all_symbol(),
+        DescriptorIndexAndName{JSRegExp::kSymbolMatchAllFunctionDescriptorIndex,
+                               RootIndex::kmatch_all_symbol},
+        if_regexp_call, if_generic_call);
     Goto(&tostring_and_create_regexp_string_iterator);
   }
   BIND(&tostring_and_create_regexp_string_iterator);
@@ -1852,6 +1864,8 @@ TF_BUILTIN(StringPrototypeSplit, StringBuiltinsAssembler) {
 
   MaybeCallFunctionAtSymbol(
       context, separator, receiver, isolate()->factory()->split_symbol(),
+      DescriptorIndexAndName{JSRegExp::kSymbolSplitFunctionDescriptorIndex,
+                             RootIndex::ksplit_symbol},
       [&]() {
         args.PopAndReturn(CallBuiltin(Builtins::kRegExpSplit, context,
                                       separator, receiver, limit));

src/builtins/builtins-string-gen.h

@@ -112,6 +112,7 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
   void MaybeCallFunctionAtSymbol(Node* const context, Node* const object,
                                  Node* const maybe_string,
                                  Handle<Symbol> symbol,
+                                 DescriptorIndexAndName symbol_index,
                                  const NodeFunction0& regexp_call,
                                  const NodeFunction1& generic_call);
 };

src/code-stub-assembler.cc

@@ -13577,32 +13577,50 @@ void CodeStubAssembler::SetPropertyLength(TNode<Context> context,
 void CodeStubAssembler::GotoIfInitialPrototypePropertyModified(
     TNode<Map> object_map, TNode<Map> initial_prototype_map, int descriptor,
     RootIndex field_name_root_index, Label* if_modified) {
+  DescriptorIndexAndName index_name{descriptor, field_name_root_index};
+  GotoIfInitialPrototypePropertiesModified(
+      object_map, initial_prototype_map,
+      Vector<DescriptorIndexAndName>(&index_name, 1), if_modified);
+}
+
+void CodeStubAssembler::GotoIfInitialPrototypePropertiesModified(
+    TNode<Map> object_map, TNode<Map> initial_prototype_map,
+    Vector<DescriptorIndexAndName> properties, 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
+    // With constant field tracking, we need to make sure that important
+    // properties in the prototype has not been tampered with. We do this by
+    // checking that their slots in the prototype's descriptor array are 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> combined_details;
+    for (int i = 0; i < properties.length(); i++) {
+      // Assert the descriptor index is in-bounds.
+      int descriptor = properties[i].descriptor_index;
+      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(properties[i].name_root_index)));
+
+      TNode<Uint32T> details =
+          DescriptorArrayGetDetails(descriptors, Uint32Constant(descriptor));
+      if (i == 0) {
+        combined_details = details;
+      } else {
+        combined_details = Unsigned(Word32And(combined_details, details));
+      }
+    }
 
     TNode<Uint32T> constness =
-        DecodeWord32<PropertyDetails::ConstnessField>(details);
+        DecodeWord32<PropertyDetails::ConstnessField>(combined_details);
 
     GotoIfNot(
         Word32Equal(constness,

src/code-stub-assembler.h

@@ -3169,6 +3169,18 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                                               int descfriptor,
                                               RootIndex field_name_root_index,
                                               Label* if_modified);
+  struct DescriptorIndexAndName {
+    DescriptorIndexAndName() {}
+    DescriptorIndexAndName(int descriptor_index, RootIndex name_root_index)
+        : descriptor_index(descriptor_index),
+          name_root_index(name_root_index) {}
+
+    int descriptor_index;
+    RootIndex name_root_index;
+  };
+  void GotoIfInitialPrototypePropertiesModified(
+      TNode<Map> object_map, TNode<Map> initial_prototype_map,
+      Vector<DescriptorIndexAndName> properties, Label* if_modified);
 
   // Implements DescriptorArray::Search().
   void DescriptorLookup(SloppyTNode<Name> unique_name,

src/objects/js-regexp.h

@@ -157,8 +157,13 @@ class JSRegExp : public JSObject {
   static const int kLastIndexFieldIndex = 0;
   static const int kInObjectFieldCount = 1;
 
-  // Descriptor array index to the exec method in the prototype.
+  // Descriptor array index to important methods in the prototype.
   static const int kExecFunctionDescriptorIndex = 1;
+  static const int kSymbolMatchFunctionDescriptorIndex = 13;
+  static const int kSymbolReplaceFunctionDescriptorIndex = 14;
+  static const int kSymbolSearchFunctionDescriptorIndex = 15;
+  static const int kSymbolSplitFunctionDescriptorIndex = 16;
+  static const int kSymbolMatchAllFunctionDescriptorIndex = 17;
 
   // The uninitialized value for a regexp code object.
   static const int kUninitializedValue = -1;

test/mjsunit/regexp-override-exec.js

+// Copyright 2018 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.
+
+var s = "baa";
+
+assertEquals(1, s.search(/a/));
+assertEquals(["aa"], s.match(/a./));
+assertEquals(["b", "", ""], s.split(/a/));
+
+let o = { index : 3, 0 : "x" };
+
+RegExp.prototype.exec = () => { return o; }
+assertEquals(3, s.search(/a/));
+assertEquals(o, s.match(/a./));
+assertEquals("baar", s.replace(/a./, "r"));
+
+RegExp.prototype.exec = () => { return null; }
+assertEquals(["baa"], s.split(/a/));

test/mjsunit/regexp-override-symbol-match-all.js

+// Copyright 2018 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: --harmony-string-matchall
+
+var s = "baa";
+
+assertEquals([["b"]], [...s.matchAll(/./)]);
+
+RegExp.prototype[Symbol.matchAll] = () => 42;
+assertEquals(42, s.matchAll(/a./));

test/mjsunit/regexp-override-symbol-match.js

+// Copyright 2018 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.
+
+var s = "baa";
+
+assertEquals(["aa"], s.match(/a./));
+
+RegExp.prototype[Symbol.match] = () => 42;
+assertEquals(42, s.match(/a./));

test/mjsunit/regexp-override-symbol-replace.js

+// Copyright 2018 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.
+
+var s = "baa";
+
+assertEquals("bca", s.replace(/a/, "c"));
+
+RegExp.prototype[Symbol.replace] = () => 42;
+assertEquals(42, s.replace(/a./));

test/mjsunit/regexp-override-symbol-search.js

+// Copyright 2018 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.
+
+var s = "baa";
+
+assertEquals(1, s.search(/a/));
+
+RegExp.prototype[Symbol.search] = () => 42;
+assertEquals(42, s.search(/a/));

test/mjsunit/regexp-override-symbol-split.js

+// Copyright 2018 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.
+
+var s = "baa";
+
+assertEquals(["b", "", ""], s.split(/a/));
+
+RegExp.prototype[Symbol.split] = () => 42;
+assertEquals(42, s.split(/a./));