Bug 1343375: Update RegExp.prototype.replace and .match to call ToLength(lastIndex) for non-global RegExp and handle recompilations

[Depends on] Bug 1317397: Implement RegExp.lastIndex changes from ES2017

5 files changed, 203 insertions, 51 deletions

diff --git a/js/src/builtin/RegExp.js b/js/src/builtin/RegExp.js
index 1ffea01050..f71ce4f759 100644
--- a/js/src/builtin/RegExp.js
+++ b/js/src/builtin/RegExp.js
@@ -122,8 +122,7 @@ function RegExpMatch(string) {
         }
 
         // Step 5.
-        var sticky = !!(flags & REGEXP_STICKY_FLAG);
-        return RegExpLocalMatchOpt(rx, S, sticky);
+        return RegExpBuiltinExec(rx, S, false);
     }
 
     // Stes 4-6
@@ -220,37 +219,6 @@ function RegExpGlobalMatchOpt(rx, S, fullUnicode) {
     }
 }
 
-// ES 2017 draft rev 6859bb9ccaea9c6ede81d71e5320e3833b92cb3e 21.2.5.6 step 5.
-// Optimized path for @@match without global flag.
-function RegExpLocalMatchOpt(rx, S, sticky) {
-    // Step 4.
-    var lastIndex = ToLength(rx.lastIndex);
-
-    // Step 8.
-    if (!sticky) {
-        lastIndex = 0;
-    } else {
-        if (lastIndex > S.length) {
-            // Steps 12.a.i-ii, 12.c.i.1-2.
-            rx.lastIndex = 0;
-            return null;
-        }
-    }
-
-    // Steps 3, 9-25, except 12.a.i-ii, 12.c.i.1-2, 15.
-    var result = RegExpMatcher(rx, S, lastIndex);
-    if (result === null) {
-        // Steps 12.a.i-ii, 12.c.i.1-2.
-        rx.lastIndex = 0;
-    } else {
-        // Step 15.
-        if (sticky)
-            rx.lastIndex = result.index + result[0].length;
-    }
-
-    return result;
-}
-
 // Checks if following properties and getters are not modified, and accessing
 // them not observed by content script:
 //   * flags
@@ -318,9 +286,10 @@ function RegExpReplace(string, replaceValue) {
 
             if (functionalReplace) {
                 var elemBase = GetElemBaseForLambda(replaceValue);
-                if (IsObject(elemBase))
+                if (IsObject(elemBase)) {
                     return RegExpGlobalReplaceOptElemBase(rx, S, lengthS, replaceValue,
                                                           fullUnicode, elemBase);
+                }
                 return RegExpGlobalReplaceOptFunc(rx, S, lengthS, replaceValue,
                                                   fullUnicode);
             }
@@ -336,18 +305,11 @@ function RegExpReplace(string, replaceValue) {
                                           fullUnicode);
         }
 
-        var sticky = !!(flags & REGEXP_STICKY_FLAG);
-
-        if (functionalReplace) {
-            return RegExpLocalReplaceOptFunc(rx, S, lengthS, replaceValue,
-                                             sticky);
-        }
-        if (firstDollarIndex !== -1) {
-            return RegExpLocalReplaceOptSubst(rx, S, lengthS, replaceValue,
-                                              sticky, firstDollarIndex);
-        }
-        return RegExpLocalReplaceOpt(rx, S, lengthS, replaceValue,
-                                     sticky);
+        if (functionalReplace)
+            return RegExpLocalReplaceOptFunc(rx, S, lengthS, replaceValue);
+        if (firstDollarIndex !== -1)
+            return RegExpLocalReplaceOptSubst(rx, S, lengthS, replaceValue, firstDollarIndex);
+        return RegExpLocalReplaceOpt(rx, S, lengthS, replaceValue);
     }
 
     // Steps 8-16.
diff --git a/js/src/builtin/RegExpLocalReplaceOpt.h.js b/js/src/builtin/RegExpLocalReplaceOpt.h.js
index edc2e2056c..96451d8a06 100644
--- a/js/src/builtin/RegExpLocalReplaceOpt.h.js
+++ b/js/src/builtin/RegExpLocalReplaceOpt.h.js
@@ -15,20 +15,33 @@
 // steps 11.a-16.
 // Optimized path for @@replace with the following conditions:
 //   * global flag is false
-function FUNC_NAME(rx, S, lengthS, replaceValue, sticky
+function FUNC_NAME(rx, S, lengthS, replaceValue
 #ifdef SUBSTITUTION
                    , firstDollarIndex
 #endif
                   )
 {
-    var lastIndex;
-    if (sticky) {
-        lastIndex = ToLength(rx.lastIndex);
+    // 21.2.5.2.2 RegExpBuiltinExec, step 4.
+    var lastIndex = ToLength(rx.lastIndex);
+
+    // 21.2.5.2.2 RegExpBuiltinExec, step 5.
+    // Side-effects in step 4 can recompile the RegExp, so we need to read the
+    // flags again and handle the case when global was enabled even though this
+    // function is optimized for non-global RegExps.
+    var flags = UnsafeGetInt32FromReservedSlot(rx, REGEXP_FLAGS_SLOT);
+
+    // 21.2.5.2.2 RegExpBuiltinExec, steps 6-7.
+    var globalOrSticky = !!(flags & (REGEXP_GLOBAL_FLAG | REGEXP_STICKY_FLAG));
+
+    if (globalOrSticky) {
+        // 21.2.5.2.2 RegExpBuiltinExec, step 12.a.
         if (lastIndex > lengthS) {
+            // FIXME: Implement changes for bug 1317397.
             rx.lastIndex = 0;
             return S;
         }
     } else {
+        // 21.2.5.2.2 RegExpBuiltinExec, step 8.
         lastIndex = 0;
     }
 
@@ -37,7 +50,11 @@ function FUNC_NAME(rx, S, lengthS, replaceValue, sticky
 
     // Step 11.b.
     if (result === null) {
+        // 21.2.5.2.2 RegExpBuiltinExec, steps 12.a.i, 12.c.i.
+        // FIXME: Implement changes for bug 1317397.
         rx.lastIndex = 0;
+
+        // Steps 12-16.
         return S;
     }
 
@@ -61,7 +78,8 @@ function FUNC_NAME(rx, S, lengthS, replaceValue, sticky
     // To set rx.lastIndex before RegExpGetComplexReplacement.
     var nextSourcePosition = position + matchLength;
 
-    if (sticky)
+    // 21.2.5.2.2 RegExpBuiltinExec, step 15.
+    if (globalOrSticky)
        rx.lastIndex = nextSourcePosition;
 
     var replacement;
diff --git a/js/src/tests/ecma_6/RegExp/match-local-tolength-recompilation.js b/js/src/tests/ecma_6/RegExp/match-local-tolength-recompilation.js
new file mode 100644
index 0000000000..6cb286b8c2
--- /dev/null
+++ b/js/src/tests/ecma_6/RegExp/match-local-tolength-recompilation.js
@@ -0,0 +1,75 @@
+// Side-effects when calling ToLength(regExp.lastIndex) in
+// RegExp.prototype[@@match] for non-global RegExp can recompile the RegExp.
+
+for (var flag of ["", "y"]) {
+    var regExp = new RegExp("a", flag);
+
+    regExp.lastIndex = {
+        valueOf() {
+            regExp.compile("b");
+            return 0;
+        }
+    };
+
+    var result = regExp[Symbol.match]("b");
+    assertEq(result !== null, true);
+}
+
+// Recompilation modifies flag:
+// Case 1: Adds global flag, validate by checking lastIndex.
+var regExp = new RegExp("a", "");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is now in global mode, RegExpBuiltinExec should update the
+        // lastIndex property to reflect last match.
+        regExp.compile("a", "g");
+        return 0;
+    }
+};
+regExp[Symbol.match]("a");
+assertEq(regExp.lastIndex, 1);
+
+// Case 2: Removes sticky flag with match, validate by checking lastIndex.
+var regExp = new RegExp("a", "y");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is no longer sticky, RegExpBuiltinExec shouldn't modify the
+        // lastIndex property.
+        regExp.compile("a", "");
+        regExp.lastIndex = 9000;
+        return 0;
+    }
+};
+regExp[Symbol.match]("a");
+assertEq(regExp.lastIndex, 9000);
+
+// Case 3.a: Removes sticky flag without match, validate by checking lastIndex.
+var regExp = new RegExp("a", "y");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is no longer sticky, RegExpBuiltinExec shouldn't modify the
+        // lastIndex property.
+        regExp.compile("b", "");
+        regExp.lastIndex = 9001;
+        return 0;
+    }
+};
+regExp[Symbol.match]("a");
+assertEq(regExp.lastIndex, 0, "Update the expected value to |9001| after fixing 1317397");
+
+// Case 3.b: Removes sticky flag without match, validate by checking lastIndex.
+var regExp = new RegExp("a", "y");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is no longer sticky, RegExpBuiltinExec shouldn't modify the
+        // lastIndex property.
+        regExp.compile("b", "");
+        regExp.lastIndex = 9002;
+        return 10000;
+    }
+};
+regExp[Symbol.match]("a");
+assertEq(regExp.lastIndex, 0, "Update the expected value to |9002| after fixing 1317397");
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
diff --git a/js/src/tests/ecma_6/RegExp/replace-local-tolength-lastindex.js b/js/src/tests/ecma_6/RegExp/replace-local-tolength-lastindex.js
new file mode 100644
index 0000000000..7ba840e000
--- /dev/null
+++ b/js/src/tests/ecma_6/RegExp/replace-local-tolength-lastindex.js
@@ -0,0 +1,22 @@
+// RegExp.prototype[@@replace] always executes ToLength(regExp.lastIndex) for
+// non-global RegExps.
+
+for (var flag of ["", "g", "y", "gy"]) {
+    var regExp = new RegExp("a", flag);
+
+    var called = false;
+    regExp.lastIndex = {
+        valueOf() {
+            assertEq(called, false);
+            called = true;
+            return 0;
+        }
+    };
+
+    assertEq(called, false);
+    regExp[Symbol.replace]("");
+    assertEq(called, !flag.includes("g"));
+}
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
diff --git a/js/src/tests/ecma_6/RegExp/replace-local-tolength-recompilation.js b/js/src/tests/ecma_6/RegExp/replace-local-tolength-recompilation.js
new file mode 100644
index 0000000000..30002c3ed3
--- /dev/null
+++ b/js/src/tests/ecma_6/RegExp/replace-local-tolength-recompilation.js
@@ -0,0 +1,75 @@
+// Side-effects when calling ToLength(regExp.lastIndex) in
+// RegExp.prototype[@@replace] for non-global RegExp can recompile the RegExp.
+
+for (var flag of ["", "y"]) {
+    var regExp = new RegExp("a", flag);
+
+    regExp.lastIndex = {
+        valueOf() {
+            regExp.compile("b");
+            return 0;
+        }
+    };
+
+    var result = regExp[Symbol.replace]("b", "pass");
+    assertEq(result, "pass");
+}
+
+// Recompilation modifies flag:
+// Case 1: Adds global flag, validate by checking lastIndex.
+var regExp = new RegExp("a", "");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is now in global mode, RegExpBuiltinExec should update the
+        // lastIndex property to reflect last match.
+        regExp.compile("a", "g");
+        return 0;
+    }
+};
+regExp[Symbol.replace]("a", "");
+assertEq(regExp.lastIndex, 1);
+
+// Case 2: Removes sticky flag with match, validate by checking lastIndex.
+var regExp = new RegExp("a", "y");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is no longer sticky, RegExpBuiltinExec shouldn't modify the
+        // lastIndex property.
+        regExp.compile("a", "");
+        regExp.lastIndex = 9000;
+        return 0;
+    }
+};
+regExp[Symbol.replace]("a", "");
+assertEq(regExp.lastIndex, 9000);
+
+// Case 3.a: Removes sticky flag without match, validate by checking lastIndex.
+var regExp = new RegExp("a", "y");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is no longer sticky, RegExpBuiltinExec shouldn't modify the
+        // lastIndex property.
+        regExp.compile("b", "");
+        regExp.lastIndex = 9001;
+        return 0;
+    }
+};
+regExp[Symbol.replace]("a", "");
+assertEq(regExp.lastIndex, 0, "Update the expected value to |9001| after fixing 1317397");
+
+// Case 3.b: Removes sticky flag without match, validate by checking lastIndex.
+var regExp = new RegExp("a", "y");
+regExp.lastIndex = {
+    valueOf() {
+        // |regExp| is no longer sticky, RegExpBuiltinExec shouldn't modify the
+        // lastIndex property.
+        regExp.compile("b", "");
+        regExp.lastIndex = 9002;
+        return 10000;
+    }
+};
+regExp[Symbol.replace]("a", "");
+assertEq(regExp.lastIndex, 0, "Update the expected value to |9002| after fixing 1317397");
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);