REGRESSION (iOS 12.2): Webpage using CoffeeScript crashes
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Mar 2019 01:26:29 +0000 (01:26 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Mar 2019 01:26:29 +0000 (01:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195613

Reviewed by Mark Lam.

JSTests:

New regression test.

* stress/regexp-backref-inbounds.js: Added.
(testRegExp):

Source/JavaScriptCore:

The bug here is in Yarr JIT backreference matching code.  We are incorrectly
using a checkedOffset / inputPosition correction when checking for the available
length left in a string.  It is improper to do these corrections as a backreference's
match length is based on what was matched in the referenced capture group and not
part of the checkedOffset and inputPosition computed when we compiled the RegExp.
In some cases, the resulting incorrect calculation would allow us to go past
the subject string's length.  Removed these adjustments.

After writing tests for the first bug, found another bug where the non-greedy
backreference backtracking code didn't do an "are we at the end of the input?" check.
This caused an infinite loop as we'd jump from the backtracking code back to
try matching one more backreference, fail and then backtrack.

* yarr/YarrJIT.cpp:
(JSC::Yarr::YarrGenerator::generateBackReference):
(JSC::Yarr::YarrGenerator::backtrackBackReference):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242838 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/regexp-backref-inbounds.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/yarr/YarrJIT.cpp

index b7f9581..1d0fe28 100644 (file)
@@ -1,3 +1,15 @@
+2019-03-12  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION (iOS 12.2): Webpage using CoffeeScript crashes
+        https://bugs.webkit.org/show_bug.cgi?id=195613
+
+        Reviewed by Mark Lam.
+
+        New regression test.
+
+        * stress/regexp-backref-inbounds.js: Added.
+        (testRegExp):
+
 2019-03-12  Mark Lam  <mark.lam@apple.com>
 
         The HasIndexedProperty node does GC.
diff --git a/JSTests/stress/regexp-backref-inbounds.js b/JSTests/stress/regexp-backref-inbounds.js
new file mode 100644 (file)
index 0000000..ac57b94
--- /dev/null
@@ -0,0 +1,46 @@
+// This test should pass without crashing.
+
+function testRegExp(re, str, expected)
+{
+    let match = re.exec(str);
+
+    let errors = "";
+    
+    if (match) {
+        if (!expected)
+            errors += "\nExpected no match, but got: " + match;
+        else {
+            if (match.length != expected.length)
+                errors += "\nExpected to match " + expected.length - 1 + " groups, but matched " + match.length - 1 +  " groups.\n";
+            if (match[0] != expected[0])
+                errors += "\nExpected results \"" + expected[0] + "\", but got \"" + match[0] + "\"";
+
+            let checkLength = Math.min(match.length, expected.length);
+            for (i = 1; i < checkLength; ++i) {
+                if (match[i] != expected[i])
+                    errors += "\nExpected group " + (i - 1) + " to be \"" + expected[i] + "\", but got \"" + match[i] + "\"";
+            }
+        }
+    } else if (expected)
+        errors += "\nExpected a match of " + expected + ", but didn't match";
+
+    if (errors.length)
+        throw errors.substring(1);
+}
+
+testRegExp(/^(.)\1*(\1.)/, "    ", ["    ", " ", "  "]);
+testRegExp(/^(.)\1*(\1+?)a/, "    ", undefined);
+
+testRegExp(/^(.)\1*?(.+)/, "xxxx", ["xxxx", "x", "xxx"]);
+
+testRegExp(/^(.{2})\1*(.+)/, "xxxx", ["xxxx", "xx", "xx"]);
+testRegExp(/^(.{2})\1*?(.+)/, "xxxx", ["xxxx", "xx", "xx"]);
+
+testRegExp(/^(.{2})\1*(.+)/, "xxx", ["xxx", "xx", "x"]);
+testRegExp(/^(.{2})\1*?(.+)/, "xxx", ["xxx", "xx", "x"]);
+
+testRegExp(/^(.)\1*(.+)/s, "=======", ["=======", "=", "="]);
+testRegExp(/^(.)\1*?(.+)/s, "=======", ["=======", "=", "======"]);
+
+testRegExp(/^(.)\1*(X)/s, "======X", ["======X", "=", "X"]);
+testRegExp(/^(.)\1*?(X)/s, "======X", ["======X", "=", "X"]);
index a83c8a1..08bdaaf 100644 (file)
@@ -1,3 +1,27 @@
+2019-03-12  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION (iOS 12.2): Webpage using CoffeeScript crashes
+        https://bugs.webkit.org/show_bug.cgi?id=195613
+
+        Reviewed by Mark Lam.
+
+        The bug here is in Yarr JIT backreference matching code.  We are incorrectly
+        using a checkedOffset / inputPosition correction when checking for the available
+        length left in a string.  It is improper to do these corrections as a backreference's
+        match length is based on what was matched in the referenced capture group and not
+        part of the checkedOffset and inputPosition computed when we compiled the RegExp.
+        In some cases, the resulting incorrect calculation would allow us to go past
+        the subject string's length.  Removed these adjustments.
+
+        After writing tests for the first bug, found another bug where the non-greedy
+        backreference backtracking code didn't do an "are we at the end of the input?" check.
+        This caused an infinite loop as we'd jump from the backtracking code back to
+        try matching one more backreference, fail and then backtrack.
+
+        * yarr/YarrJIT.cpp:
+        (JSC::Yarr::YarrGenerator::generateBackReference):
+        (JSC::Yarr::YarrGenerator::backtrackBackReference):
+
 2019-03-12  Robin Morisset  <rmorisset@apple.com>
 
         A lot more classes have padding that can be reduced by reordering their fields
index 40851d6..4281016 100644 (file)
@@ -1198,8 +1198,6 @@ class YarrGenerator : public YarrJITInfo, private MacroAssembler {
 
             // PatternTemp should contain pattern end index at this point
             sub32(patternIndex, patternTemp);
-            if (m_checkedOffset - term->inputPosition)
-                sub32(Imm32((m_checkedOffset - term->inputPosition).unsafeGet()), patternTemp);
             op.m_jumps.append(checkNotEnoughInput(patternTemp));
 
             matchBackreference(opIndex, op.m_jumps, characterOrTemp, patternIndex, patternTemp);
@@ -1224,8 +1222,6 @@ class YarrGenerator : public YarrJITInfo, private MacroAssembler {
 
             // PatternTemp should contain pattern end index at this point
             sub32(patternIndex, patternTemp);
-            if (m_checkedOffset - term->inputPosition)
-                sub32(Imm32((m_checkedOffset - term->inputPosition).unsafeGet()), patternTemp);
             matches.append(checkNotEnoughInput(patternTemp));
 
             matchBackreference(opIndex, incompleteMatches, characterOrTemp, patternIndex, patternTemp);
@@ -1270,8 +1266,6 @@ class YarrGenerator : public YarrJITInfo, private MacroAssembler {
 
             // Check if we have input remaining to match
             sub32(patternIndex, patternTemp);
-            if (m_checkedOffset - term->inputPosition)
-                sub32(Imm32((m_checkedOffset - term->inputPosition).unsafeGet()), patternTemp);
             matches.append(checkNotEnoughInput(patternTemp));
 
             storeToFrame(index, parenthesesFrameLocation + BackTrackInfoBackReference::beginIndex());
@@ -1328,6 +1322,7 @@ class YarrGenerator : public YarrJITInfo, private MacroAssembler {
         case QuantifierNonGreedy: {
             const RegisterID matchAmount = regT0;
 
+            failures.append(atEndOfInput());
             loadFromFrame(parenthesesFrameLocation + BackTrackInfoBackReference::matchAmountIndex(), matchAmount);
             if (term->quantityMaxCount != quantifyInfinite)
                 failures.append(branch32(AboveOrEqual, Imm32(term->quantityMaxCount.unsafeGet()), matchAmount));