Changeset 245939 in webkit

Timestamp:

May 30, 2019, 5:33:42 PM (7 years ago)

Author:

Alan Coon

Message:

Cherry-pick r243237. rdar://problem/51264876

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:

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

Location:

branches/safari-607-branch/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)

branches/safari-607-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-05-30  Alan Coon  <alancoon@apple.com>
+
+        Cherry-pick r243237. rdar://problem/51264876
+
+    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:
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243237 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-05-30  Kocsen Chung  <kocsen_chung@apple.com>
 

branches/safari-607-branch/Source/JavaScriptCore/yarr/YarrInterpreter.cpp

@@ -1609 +1609 @@
     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;

branches/safari-607-branch/Source/JavaScriptCore/yarr/YarrJIT.cpp

@@ -3390 +3390 @@
         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,
@@ -3496 +3501 @@
     void opCompileParentheticalAssertion(PatternTerm* term)
     {
+        if (UNLIKELY(!m_vm->isSafeToRecurse())) {
+            m_failureReason = JITFailureReason::ParenthesisNestedTooDeep;
+            return;
+        }
+
         size_t parenBegin = m_ops.size();
         m_ops.append(OpParentheticalAssertionBegin);
@@ -3576 +3586 @@
     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;
@@ -4209 +4224 @@
         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");

branches/safari-607-branch/Source/JavaScriptCore/yarr/YarrJIT.h

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