Title: [242838] trunk
Revision
242838
Author
msab...@apple.com
Date
2019-03-12 18:26:29 -0700 (Tue, 12 Mar 2019)

Log Message

REGRESSION (iOS 12.2): Webpage using CoffeeScript crashes
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/_javascript_Core:

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):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (242837 => 242838)


--- trunk/JSTests/ChangeLog	2019-03-13 01:19:20 UTC (rev 242837)
+++ trunk/JSTests/ChangeLog	2019-03-13 01:26:29 UTC (rev 242838)
@@ -1,3 +1,15 @@
+2019-03-12  Michael Saboff  <msab...@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....@apple.com>
 
         The HasIndexedProperty node does GC.

Added: trunk/JSTests/stress/regexp-backref-inbounds.js (0 => 242838)


--- trunk/JSTests/stress/regexp-backref-inbounds.js	                        (rev 0)
+++ trunk/JSTests/stress/regexp-backref-inbounds.js	2019-03-13 01:26:29 UTC (rev 242838)
@@ -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"]);

Modified: trunk/Source/_javascript_Core/ChangeLog (242837 => 242838)


--- trunk/Source/_javascript_Core/ChangeLog	2019-03-13 01:19:20 UTC (rev 242837)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-03-13 01:26:29 UTC (rev 242838)
@@ -1,3 +1,27 @@
+2019-03-12  Michael Saboff  <msab...@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  <rmoris...@apple.com>
 
         A lot more classes have padding that can be reduced by reordering their fields

Modified: trunk/Source/_javascript_Core/yarr/YarrJIT.cpp (242837 => 242838)


--- trunk/Source/_javascript_Core/yarr/YarrJIT.cpp	2019-03-13 01:19:20 UTC (rev 242837)
+++ trunk/Source/_javascript_Core/yarr/YarrJIT.cpp	2019-03-13 01:26:29 UTC (rev 242838)
@@ -1198,8 +1198,6 @@
 
             // 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 @@
 
             // 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 @@
 
             // 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 @@
         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));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to