From aae3a117342e8959c074ea9f36cefac772f608d3 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sat, 24 Mar 2018 15:54:49 +0100 Subject: 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 --- js/src/builtin/RegExp.js | 54 +++++-------------------------- js/src/builtin/RegExpLocalReplaceOpt.h.js | 28 +++++++++++++--- 2 files changed, 31 insertions(+), 51 deletions(-) (limited to 'js/src/builtin') 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; -- cgit v1.2.3 From 5fd5b2ac2f396eb1d8707a691aa26ad159ad25e3 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sat, 24 Mar 2018 16:01:06 +0100 Subject: Bug 1317397: Only set lastIndex for global or sticky RegExps in RegExpBuiltinExec per ES2017 --- js/src/builtin/RegExp.js | 80 ++++++++++++++++++++++--------- js/src/builtin/RegExpLocalReplaceOpt.h.js | 12 +++-- js/src/builtin/Utilities.js | 12 ++++- 3 files changed, 75 insertions(+), 29 deletions(-) (limited to 'js/src/builtin') diff --git a/js/src/builtin/RegExp.js b/js/src/builtin/RegExp.js index f71ce4f75..0b849292c 100644 --- a/js/src/builtin/RegExp.js +++ b/js/src/builtin/RegExp.js @@ -609,7 +609,8 @@ function RegExpGlobalReplaceShortOpt(rx, S, lengthS, replaceValue, fullUnicode) #undef SUBSTITUTION #undef FUNC_NAME -// ES 2017 draft 6859bb9ccaea9c6ede81d71e5320e3833b92cb3e 21.2.5.9. +// ES2017 draft rev 6390c2f1b34b309895d31d8c0512eac8660a0210 +// 21.2.5.9 RegExp.prototype [ @@search ] ( string ) function RegExpSearch(string) { // Step 1. var rx = this; @@ -621,41 +622,69 @@ function RegExpSearch(string) { // Step 3. var S = ToString(string); + // Step 4. + var previousLastIndex = rx.lastIndex; + + // Step 5. + var lastIndexIsZero = SameValue(previousLastIndex, 0); + if (!lastIndexIsZero) + rx.lastIndex = 0; + if (IsRegExpMethodOptimizable(rx) && S.length < 0x7fff) { // Step 6. var result = RegExpSearcher(rx, S, 0); - // Step 8. + // We need to consider two cases: + // + // 1. Neither global nor sticky is set: + // RegExpBuiltinExec doesn't modify lastIndex for local RegExps, that + // means |SameValue(rx.lastIndex, 0)| is true after calling exec. The + // comparison in steps 7-8 |SameValue(rx.lastIndex, previousLastIndex)| + // is therefore equal to the already computed |lastIndexIsZero| value. + // + // 2. Global or sticky flag is set. + // RegExpBuiltinExec will always update lastIndex and we need to + // restore the property to its original value. + + // Steps 7-8. + if (!lastIndexIsZero) { + rx.lastIndex = previousLastIndex; + } else { + var flags = UnsafeGetInt32FromReservedSlot(rx, REGEXP_FLAGS_SLOT); + if (flags & (REGEXP_GLOBAL_FLAG | REGEXP_STICKY_FLAG)) + rx.lastIndex = previousLastIndex; + } + + // Step 9. if (result === -1) return -1; - // Step 9. + // Step 10. return result & 0x7fff; } - return RegExpSearchSlowPath(rx, S); + return RegExpSearchSlowPath(rx, S, previousLastIndex); } -// ES 2017 draft 6859bb9ccaea9c6ede81d71e5320e3833b92cb3e 21.2.5.9 -// steps 4-9. -function RegExpSearchSlowPath(rx, S) { - // Step 4. - var previousLastIndex = rx.lastIndex; - - // Step 5. - rx.lastIndex = 0; - +// ES2017 draft rev 6390c2f1b34b309895d31d8c0512eac8660a0210 +// 21.2.5.9 RegExp.prototype [ @@search ] ( string ) +// Steps 6-10. +function RegExpSearchSlowPath(rx, S, previousLastIndex) { // Step 6. var result = RegExpExec(rx, S, false); // Step 7. - rx.lastIndex = previousLastIndex; + var currentLastIndex = rx.lastIndex; // Step 8. + if (!SameValue(currentLastIndex, previousLastIndex)) + rx.lastIndex = previousLastIndex; + + // Step 9. if (result === null) return -1; - // Step 9. + // Step 10. return result.index; } @@ -904,15 +933,16 @@ function RegExpExec(R, S, forTest) { return forTest ? result !== null : result; } -// ES 2017 draft rev 6a13789aa9e7c6de4e96b7d3e24d9e6eba6584ad 21.2.5.2.2. +// ES2017 draft rev 6390c2f1b34b309895d31d8c0512eac8660a0210 +// 21.2.5.2.2 Runtime Semantics: RegExpBuiltinExec ( R, S ) function RegExpBuiltinExec(R, S, forTest) { - // ES6 21.2.5.2.1 step 6. + // 21.2.5.2.1 Runtime Semantics: RegExpExec, step 5. // This check is here for RegExpTest. RegExp_prototype_Exec does same // thing already. if (!IsRegExpObject(R)) return UnwrapAndCallRegExpBuiltinExec(R, S, forTest); - // Steps 1-2 (skipped). + // Steps 1-3 (skipped). // Step 4. var lastIndex = ToLength(R.lastIndex); @@ -927,9 +957,11 @@ function RegExpBuiltinExec(R, S, forTest) { if (!globalOrSticky) { lastIndex = 0; } else { + // Step 12.a. if (lastIndex > S.length) { - // Steps 12.a.i-ii, 12.c.i.1-2. - R.lastIndex = 0; + // Steps 12.a.i-ii. + if (globalOrSticky) + R.lastIndex = 0; return forTest ? false : null; } } @@ -939,7 +971,8 @@ function RegExpBuiltinExec(R, S, forTest) { var endIndex = RegExpTester(R, S, lastIndex); if (endIndex == -1) { // Steps 12.a.i-ii, 12.c.i.1-2. - R.lastIndex = 0; + if (globalOrSticky) + R.lastIndex = 0; return false; } @@ -953,8 +986,9 @@ function RegExpBuiltinExec(R, S, forTest) { // Steps 3, 9-25, except 12.a.i-ii, 12.c.i.1-2, 15. var result = RegExpMatcher(R, S, lastIndex); if (result === null) { - // Steps 12.a.i-ii, 12.c.i.1-2. - R.lastIndex = 0; + // Steps 12.a.i, 12.c.i. + if (globalOrSticky) + R.lastIndex = 0; } else { // Step 15. if (globalOrSticky) diff --git a/js/src/builtin/RegExpLocalReplaceOpt.h.js b/js/src/builtin/RegExpLocalReplaceOpt.h.js index 96451d8a0..1acd6a73a 100644 --- a/js/src/builtin/RegExpLocalReplaceOpt.h.js +++ b/js/src/builtin/RegExpLocalReplaceOpt.h.js @@ -11,7 +11,7 @@ // * FUNCTIONAL -- replaceValue is a function // * neither of above -- replaceValue is a string without "$" -// ES 2017 draft 03bfda119d060aca4099d2b77cf43f6d4f11cfa2 21.2.5.8 +// ES 2017 draft 6390c2f1b34b309895d31d8c0512eac8660a0210 21.2.5.8 // steps 11.a-16. // Optimized path for @@replace with the following conditions: // * global flag is false @@ -36,8 +36,10 @@ function FUNC_NAME(rx, S, lengthS, replaceValue if (globalOrSticky) { // 21.2.5.2.2 RegExpBuiltinExec, step 12.a. if (lastIndex > lengthS) { - // FIXME: Implement changes for bug 1317397. - rx.lastIndex = 0; + if (globalOrSticky) + rx.lastIndex = 0; + + // Steps 12-16. return S; } } else { @@ -51,8 +53,8 @@ function FUNC_NAME(rx, S, lengthS, replaceValue // 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; + if (globalOrSticky) + rx.lastIndex = 0; // Steps 12-16. return S; diff --git a/js/src/builtin/Utilities.js b/js/src/builtin/Utilities.js index 2dece3801..3145b4d51 100644 --- a/js/src/builtin/Utilities.js +++ b/js/src/builtin/Utilities.js @@ -106,7 +106,17 @@ function ToLength(v) { return std_Math_min(v, 0x1fffffffffffff); } -/* Spec: ECMAScript Draft, 6th edition Oct 14, 2014, 7.2.4 */ +// ES2017 draft rev aebf014403a3e641fb1622aec47c40f051943527 +// 7.2.9 SameValue ( x, y ) +function SameValue(x, y) { + if (x === y) { + return (x !== 0) || (1 / x === 1 / y); + } + return (x !== x && y !== y); +} + +// ES2017 draft rev aebf014403a3e641fb1622aec47c40f051943527 +// 7.2.10 SameValueZero ( x, y ) function SameValueZero(x, y) { return x === y || (x !== x && y !== y); } -- cgit v1.2.3