Changeset 242838 in webkit

Timestamp:

Mar 12, 2019 6:26:29 PM (5 years ago)

Author:

msaboff@apple.com

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

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/regexp-backref-inbounds.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/yarr/YarrJIT.cpp (modified) (4 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/yarr/YarrJIT.cpp

@@ -1199 +1199 @@
             // 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));
 
@@ -1225 +1223 @@
             // 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));
 
@@ -1271 +1267 @@
             // 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));
 
@@ -1329 +1323 @@
             const RegisterID matchAmount = regT0;
 
+            failures.append(atEndOfInput());
             loadFromFrame(parenthesesFrameLocation + BackTrackInfoBackReference::matchAmountIndex(), matchAmount);
             if (term->quantityMaxCount != quantifyInfinite)