Title: [242955] trunk
Revision
242955
Author
[email protected]
Date
2019-03-14 12:31:52 -0700 (Thu, 14 Mar 2019)

Log Message

ASSERTION FAILED: regexp->isValid() or ASSERTION FAILED: !isCompilationThread()
https://bugs.webkit.org/show_bug.cgi?id=195735

Reviewed by Mark Lam.

JSTests:

New regression test.

* stress/dont-strength-reduce-regexp-with-compile-error.js: Added.
(foo):
(bar):

Source/_javascript_Core:

There are two bug fixes here.

The first bug happens due to a race condition when we are compiling on a separate thread while the
main thread is compiling the RegExp at a place where it can run out of stack.  When that happens,
the RegExp becomes invalid due to the out of stack error.  If we check the ASSERT condition in the DFG
compilation thread, we crash.  After the main thread throws an exception, it resets the RegExp as
it might compile successfully the next time we try to execute it on a shallower stack.
The main thread will see the regular _expression_ as valid when it executes the JIT'ed code we are compiling
or any slow path we call out to.  Therefore ASSERTs like this in compilation code can be eliminated.

The second bug is due to incorrect logic when we go to run the regexp in the Strength Reduction phase.
The current check for "do we have code to run the RegExp?" only checks that the RegExp's state
is != NotCompiled.  We also can't run the RegExp if there the state is ParseError.
Changing hasCode() to take this into account fixes the second issue.

(JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp):
* runtime/RegExp.h:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileNewRegexp):
* runtime/RegExp.h:

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (242954 => 242955)


--- trunk/JSTests/ChangeLog	2019-03-14 19:27:28 UTC (rev 242954)
+++ trunk/JSTests/ChangeLog	2019-03-14 19:31:52 UTC (rev 242955)
@@ -1,3 +1,16 @@
+2019-03-13  Michael Saboff  <[email protected]>
+
+        ASSERTION FAILED: regexp->isValid() or ASSERTION FAILED: !isCompilationThread()
+        https://bugs.webkit.org/show_bug.cgi?id=195735
+
+        Reviewed by Mark Lam.
+
+        New regression test.
+
+        * stress/dont-strength-reduce-regexp-with-compile-error.js: Added.
+        (foo):
+        (bar):
+
 2019-03-14  Saam barati  <[email protected]>
 
         Fixup uses KnownInt32 incorrectly in some nodes

Added: trunk/JSTests/stress/dont-strength-reduce-regexp-with-compile-error.js (0 => 242955)


--- trunk/JSTests/stress/dont-strength-reduce-regexp-with-compile-error.js	                        (rev 0)
+++ trunk/JSTests/stress/dont-strength-reduce-regexp-with-compile-error.js	2019-03-14 19:31:52 UTC (rev 242955)
@@ -0,0 +1,23 @@
+//@ runDefault("--jitPolicyScale=0")
+
+let tryCount = 10000;
+
+function foo(a) {
+  if (tryCount == 0)
+     return;
+
+  tryCount--;
+
+  try {
+    eval('bar(/' + a[0].source + '/)');
+  } catch(e) {
+  }
+}
+
+function bar(r) {
+  foo([r]);
+  foo([r]);
+  r.exec('x');
+}
+
+bar(/((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((x))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))/);

Modified: trunk/Source/_javascript_Core/ChangeLog (242954 => 242955)


--- trunk/Source/_javascript_Core/ChangeLog	2019-03-14 19:27:28 UTC (rev 242954)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-03-14 19:31:52 UTC (rev 242955)
@@ -1,3 +1,31 @@
+2019-03-14  Michael Saboff  <[email protected]>
+
+        ASSERTION FAILED: regexp->isValid() or ASSERTION FAILED: !isCompilationThread()
+        https://bugs.webkit.org/show_bug.cgi?id=195735
+
+        Reviewed by Mark Lam.
+
+        There are two bug fixes here.
+
+        The first bug happens due to a race condition when we are compiling on a separate thread while the
+        main thread is compiling the RegExp at a place where it can run out of stack.  When that happens,
+        the RegExp becomes invalid due to the out of stack error.  If we check the ASSERT condition in the DFG
+        compilation thread, we crash.  After the main thread throws an exception, it resets the RegExp as
+        it might compile successfully the next time we try to execute it on a shallower stack.
+        The main thread will see the regular _expression_ as valid when it executes the JIT'ed code we are compiling
+        or any slow path we call out to.  Therefore ASSERTs like this in compilation code can be eliminated.
+
+        The second bug is due to incorrect logic when we go to run the regexp in the Strength Reduction phase.
+        The current check for "do we have code to run the RegExp?" only checks that the RegExp's state
+        is != NotCompiled.  We also can't run the RegExp if there the state is ParseError.
+        Changing hasCode() to take this into account fixes the second issue.
+
+        (JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp):
+        * runtime/RegExp.h:
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileNewRegexp):
+        * runtime/RegExp.h:
+
 2019-03-14  Saam barati  <[email protected]>
 
         Fixup uses KnownInt32 incorrectly in some nodes

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (242954 => 242955)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2019-03-14 19:27:28 UTC (rev 242954)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2019-03-14 19:31:52 UTC (rev 242955)
@@ -9756,7 +9756,6 @@
 void SpeculativeJIT::compileNewRegexp(Node* node)
 {
     RegExp* regexp = node->castOperand<RegExp*>();
-    ASSERT(regexp->isValid());
 
     GPRTemporary result(this);
     GPRTemporary scratch1(this);

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (242954 => 242955)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2019-03-14 19:27:28 UTC (rev 242954)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2019-03-14 19:31:52 UTC (rev 242955)
@@ -11274,7 +11274,6 @@
         FrozenValue* regexp = m_node->cellOperand();
         LValue lastIndex = lowJSValue(m_node->child1());
         ASSERT(regexp->cell()->inherits<RegExp>(vm()));
-        ASSERT(m_node->castOperand<RegExp*>()->isValid());
 
         LBasicBlock slowCase = m_out.newBlock();
         LBasicBlock continuation = m_out.newBlock();

Modified: trunk/Source/_javascript_Core/runtime/RegExp.h (242954 => 242955)


--- trunk/Source/_javascript_Core/runtime/RegExp.h	2019-03-14 19:27:28 UTC (rev 242954)
+++ trunk/Source/_javascript_Core/runtime/RegExp.h	2019-03-14 19:31:52 UTC (rev 242955)
@@ -108,7 +108,7 @@
 
     bool hasCode()
     {
-        return m_state != NotCompiled;
+        return m_state == JITCode || m_state == ByteCode;
     }
 
     bool hasCodeFor(Yarr::YarrCharSize);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to