Changeset 243237 in webkit

Timestamp:

Mar 20, 2019, 2:04:10 PM (7 years ago)

Author:

msaboff@apple.com

Message:

JSC test crash: stress/dont-strength-reduce-regexp-with-compile-error.js.default
​https://bugs.webkit.org/show_bug.cgi?id=195906

Reviewed by Mark Lam.

The problem here as that we may successfully parsed a RegExp without running out of stack,
but later run out of stack when trying to JIT compile the same expression.

Added a check for available stack space when we call into one of the parenthesis compilation
functions that recurse. When we don't have enough stack space to recurse, we fail the JIT
compilation and let the interpreter handle the expression.

From code inspection of the YARR interpreter it has the same issue, but I couldn't cause a failure.
Filed a new bug and added a FIXME comment for the Interpreter to have similar checks.
Given that we can reproduce a failure, this is sufficient for now.

This change is covered by the previously added failing test,
JSTests/stress/dont-strength-reduce-regexp-with-compile-error.js.

• yarr/YarrInterpreter.cpp:

(JSC::Yarr::Interpreter::interpret):

• yarr/YarrJIT.cpp:

(JSC::Yarr::YarrGenerator::opCompileParenthesesSubpattern):
(JSC::Yarr::YarrGenerator::opCompileParentheticalAssertion):
(JSC::Yarr::YarrGenerator::opCompileBody):
(JSC::Yarr::dumpCompileFailure):

• yarr/YarrJIT.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• yarr/YarrInterpreter.cpp (modified) (1 diff)
• yarr/YarrJIT.cpp (modified) (4 diffs)
• yarr/YarrJIT.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-03-20  Michael Saboff  <msaboff@apple.com>
+
+        JSC test crash: stress/dont-strength-reduce-regexp-with-compile-error.js.default
+        https://bugs.webkit.org/show_bug.cgi?id=195906
+
+        Reviewed by Mark Lam.
+
+        The problem here as that we may successfully parsed a RegExp without running out of stack,
+        but later run out of stack when trying to JIT compile the same expression.
+
+        Added a check for available stack space when we call into one of the parenthesis compilation
+        functions that recurse.  When we don't have enough stack space to recurse, we fail the JIT
+        compilation and let the interpreter handle the expression.
+
+        From code inspection of the YARR interpreter it has the same issue, but I couldn't cause a failure.
+        Filed a new bug and added a FIXME comment for the Interpreter to have similar checks.
+        Given that we can reproduce a failure, this is sufficient for now.
+
+        This change is covered by the previously added failing test,
+        JSTests/stress/dont-strength-reduce-regexp-with-compile-error.js.
+
+        * yarr/YarrInterpreter.cpp:
+        (JSC::Yarr::Interpreter::interpret):
+        * yarr/YarrJIT.cpp:
+        (JSC::Yarr::YarrGenerator::opCompileParenthesesSubpattern):
+        (JSC::Yarr::YarrGenerator::opCompileParentheticalAssertion):
+        (JSC::Yarr::YarrGenerator::opCompileBody):
+        (JSC::Yarr::dumpCompileFailure):
+        * yarr/YarrJIT.h:
+
 2019-03-20  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/yarr/YarrInterpreter.cpp

@@ -1607 +1607 @@
     unsigned interpret()
     {
+        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=195970
+        // [Yarr Interpreter] The interpreter doesn't have checks for stack overflow due to deep recursion
         if (!input.isAvailableInput(0))
             return offsetNoMatch;

trunk/Source/JavaScriptCore/yarr/YarrJIT.cpp

@@ -3387 +3387 @@
         YarrOpCode alternativeEndOpCode = OpSimpleNestedAlternativeEnd;
 
+        if (UNLIKELY(!m_vm->isSafeToRecurse())) {
+            m_failureReason = JITFailureReason::ParenthesisNestedTooDeep;
+            return;
+        }
+
         // We can currently only compile quantity 1 subpatterns that are
         // not copies. We generate a copy in the case of a range quantifier,
@@ -3493 +3498 @@
     void opCompileParentheticalAssertion(PatternTerm* term)
     {
+        if (UNLIKELY(!m_vm->isSafeToRecurse())) {
+            m_failureReason = JITFailureReason::ParenthesisNestedTooDeep;
+            return;
+        }
+
         size_t parenBegin = m_ops.size();
         m_ops.append(OpParentheticalAssertionBegin);
@@ -3573 +3583 @@
     void opCompileBody(PatternDisjunction* disjunction)
     {
+        if (UNLIKELY(!m_vm->isSafeToRecurse())) {
+            m_failureReason = JITFailureReason::ParenthesisNestedTooDeep;
+            return;
+        }
+        
         Vector<std::unique_ptr<PatternAlternative>>& alternatives = disjunction->m_alternatives;
         size_t currentAlternativeIndex = 0;
@@ -4201 +4216 @@
         dataLog("Can't JIT a pattern containing fixed count parenthesized subpatterns\n");
         break;
+    case JITFailureReason::ParenthesisNestedTooDeep:
+        dataLog("Can't JIT pattern due to parentheses nested too deeply\n");
+        break;
     case JITFailureReason::ExecutableMemoryAllocationFailure:
         dataLog("Can't JIT because of failure of allocation of executable memory\n");

trunk/Source/JavaScriptCore/yarr/YarrJIT.h

@@ -57 +57 @@
     ParenthesizedSubpattern,
     FixedCountParenthesizedSubpattern,
+    ParenthesisNestedTooDeep,
     ExecutableMemoryAllocationFailure,
 };