diff options
5 files changed, 203 insertions, 51 deletions
diff --git a/js/src/builtin/RegExp.js b/js/src/builtin/RegExp.js index 1ffea0105..f71ce4f75 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 edc2e2056..96451d8a0 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 000000000..6cb286b8c --- /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 000000000..7ba840e00 --- /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 000000000..30002c3ed --- /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); |