[regexp] Refactor Regexp.prototype[@@replace]

Tbr: jgruber@chromium.org
Bug: chromium:944971
Change-Id: I2dcbfae638848c11eac4e262c3d636f33c3f24a4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1541477
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60490}

src/regexp/regexp-utils.cc

@@ -36,7 +36,7 @@ Handle<String> RegExpUtils::GenericCaptureGetter(
 
 namespace {
 
-V8_INLINE bool HasInitialRegExpMap(Isolate* isolate, Handle<JSReceiver> recv) {
+V8_INLINE bool HasInitialRegExpMap(Isolate* isolate, JSReceiver recv) {
   return recv->map() == isolate->regexp_function()->initial_map();
 }
 
@@ -47,7 +47,7 @@ MaybeHandle<Object> RegExpUtils::SetLastIndex(Isolate* isolate,
                                               uint64_t value) {
   Handle<Object> value_as_object =
       isolate->factory()->NewNumberFromInt64(value);
-  if (HasInitialRegExpMap(isolate, recv)) {
+  if (HasInitialRegExpMap(isolate, *recv)) {
     JSRegExp::cast(*recv)->set_last_index(*value_as_object, SKIP_WRITE_BARRIER);
     return recv;
   } else {
@@ -59,7 +59,7 @@ MaybeHandle<Object> RegExpUtils::SetLastIndex(Isolate* isolate,
 
 MaybeHandle<Object> RegExpUtils::GetLastIndex(Isolate* isolate,
                                               Handle<JSReceiver> recv) {
-  if (HasInitialRegExpMap(isolate, recv)) {
+  if (HasInitialRegExpMap(isolate, *recv)) {
     return handle(JSRegExp::cast(*recv)->last_index(), isolate);
   } else {
     return Object::GetProperty(isolate, recv,
@@ -155,9 +155,7 @@ bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) {
 
   JSReceiver recv = JSReceiver::cast(*obj);
 
-  // Check the receiver's map.
-  Handle<JSFunction> regexp_function = isolate->regexp_function();
-  if (recv->map() != regexp_function->initial_map()) return false;
+  if (!HasInitialRegExpMap(isolate, recv)) return false;
 
   // Check the receiver's prototype's map.
   Object proto = recv->map()->prototype();

src/runtime/runtime-regexp.cc

@@ -1250,10 +1250,9 @@ static Object SearchRegExpMultiple(Isolate* isolate, Handle<String> subject,
 // doesn't properly call the underlying exec method.
 V8_WARN_UNUSED_RESULT MaybeHandle<String> RegExpReplace(
     Isolate* isolate, Handle<JSRegExp> regexp, Handle<String> string,
-    Handle<Object> replace_obj) {
+    Handle<String> replace) {
   // Functional fast-paths are dispatched directly by replace builtin.
   DCHECK(RegExpUtils::IsUnmodifiedRegExp(isolate, regexp));
-  DCHECK(!replace_obj->IsCallable());
 
   Factory* factory = isolate->factory();
 
@@ -1261,9 +1260,6 @@ V8_WARN_UNUSED_RESULT MaybeHandle<String> RegExpReplace(
   const bool global = (flags & JSRegExp::kGlobal) != 0;
   const bool sticky = (flags & JSRegExp::kSticky) != 0;
 
-  Handle<String> replace;
-  ASSIGN_RETURN_ON_EXCEPTION(isolate, replace,
-                             Object::ToString(isolate, replace_obj), String);
   replace = String::Flatten(isolate, replace);
 
   Handle<RegExpMatchInfo> last_match_info = isolate->regexp_last_match_info();
@@ -1364,18 +1360,23 @@ RUNTIME_FUNCTION(Runtime_RegExpExecMultiple) {
   CONVERT_ARG_HANDLE_CHECKED(String, subject, 1);
   CONVERT_ARG_HANDLE_CHECKED(RegExpMatchInfo, last_match_info, 2);
   CONVERT_ARG_HANDLE_CHECKED(JSArray, result_array, 3);
+
+  DCHECK(RegExpUtils::IsUnmodifiedRegExp(isolate, regexp));
   CHECK(result_array->HasObjectElements());
 
   subject = String::Flatten(isolate, subject);
   CHECK(regexp->GetFlags() & JSRegExp::kGlobal);
 
+  Object result;
   if (regexp->CaptureCount() == 0) {
-    return SearchRegExpMultiple<false>(isolate, subject, regexp,
-                                       last_match_info, result_array);
+    result = SearchRegExpMultiple<false>(isolate, subject, regexp,
+                                         last_match_info, result_array);
   } else {
-    return SearchRegExpMultiple<true>(isolate, subject, regexp, last_match_info,
-                                      result_array);
+    result = SearchRegExpMultiple<true>(isolate, subject, regexp,
+                                        last_match_info, result_array);
   }
+  DCHECK(RegExpUtils::IsUnmodifiedRegExp(isolate, regexp));
+  return result;
 }
 
 RUNTIME_FUNCTION(Runtime_StringReplaceNonGlobalRegExpWithFunction) {
@@ -1698,24 +1699,27 @@ RUNTIME_FUNCTION(Runtime_RegExpReplaceRT) {
 
   const bool functional_replace = replace_obj->IsCallable();
 
+  Handle<String> replace;
+  if (!functional_replace) {
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, replace,
+                                       Object::ToString(isolate, replace_obj));
+  }
+
   // Fast-path for unmodified JSRegExps (and non-functional replace).
   if (RegExpUtils::IsUnmodifiedRegExp(isolate, recv)) {
     // We should never get here with functional replace because unmodified
     // regexp and functional replace should be fully handled in CSA code.
     CHECK(!functional_replace);
-    RETURN_RESULT_OR_FAILURE(
-        isolate, RegExpReplace(isolate, Handle<JSRegExp>::cast(recv), string,
-                               replace_obj));
+    Handle<Object> result;
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, result,
+        RegExpReplace(isolate, Handle<JSRegExp>::cast(recv), string, replace));
+    DCHECK(RegExpUtils::IsUnmodifiedRegExp(isolate, recv));
+    return *result;
   }
 
   const uint32_t length = string->length();
 
-  Handle<String> replace;
-  if (!functional_replace) {
-    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, replace,
-                                       Object::ToString(isolate, replace_obj));
-  }
-
   Handle<Object> global_obj;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, global_obj,

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

+// Copyright 2019 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.
+
+let re = /x/y;
+let cnt = 0;
+let str = re[Symbol.replace]("x", {
+  toString: () => {
+    cnt++;
+    if (cnt == 2) {
+      re.lastIndex = {valueOf: () => {
+        re.x = 42;
+        return 0;
+      }};
+    }
+    return 'y$';
+  }
+});
+assertEquals("y$", str);