Title: [238437] trunk
Revision
238437
Author
sbar...@apple.com
Date
2018-11-21 19:43:30 -0800 (Wed, 21 Nov 2018)

Log Message

DFGSpeculativeJIT should not &= exitOK with mayExit(node)
https://bugs.webkit.org/show_bug.cgi?id=191897
<rdar://problem/45871998>

Reviewed by Mark Lam.

JSTests:

* stress/exitok-is-not-the-same-as-mayExit.js: Added.
(bar):
(foo):

Source/_javascript_Core:

exitOK is a statement about it being legal to exit. mayExit() is about being
conservative and returning false only if an OSR exit *could never* happen.
mayExit() tries to be as smart as possible to see if it can return false.
It can't return false if a runtime exit *could* happen. However, there is
code in the compiler where mayExit() returns false (because it uses data
generated from AI about type checks being proved), but the code we emit in the
compiler backend unconditionally generates an OSR exit, even if that exit may
never execute. For example, let's say we have this IR:

SomeNode(Boolean:@input)

And we always emit code like this as a way of emitting a boolean type check:

jump L1 if input == true
jump L1 if input == false
emit an OSR exit

In such a program, when we generate the above OSR exit, in a validationEnabled()
build, and if @input is proved to be a boolean, we'll end up crashing because we
have the bogus assertion saying !exitOK. This is one reason why things are cleaner
if we don't conflate mayExit() with exitOK.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCurrentBlock):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (238436 => 238437)


--- trunk/JSTests/ChangeLog	2018-11-22 03:39:54 UTC (rev 238436)
+++ trunk/JSTests/ChangeLog	2018-11-22 03:43:30 UTC (rev 238437)
@@ -1,5 +1,17 @@
 2018-11-21  Saam barati  <sbar...@apple.com>
 
+        DFGSpeculativeJIT should not &= exitOK with mayExit(node)
+        https://bugs.webkit.org/show_bug.cgi?id=191897
+        <rdar://problem/45871998>
+
+        Reviewed by Mark Lam.
+
+        * stress/exitok-is-not-the-same-as-mayExit.js: Added.
+        (bar):
+        (foo):
+
+2018-11-21  Saam barati  <sbar...@apple.com>
+
         Fix assertion in KnownCellUse inside SpeculativeJIT::speculate
         https://bugs.webkit.org/show_bug.cgi?id=191895
         <rdar://problem/46167406>

Added: trunk/JSTests/stress/exitok-is-not-the-same-as-mayExit.js (0 => 238437)


--- trunk/JSTests/stress/exitok-is-not-the-same-as-mayExit.js	                        (rev 0)
+++ trunk/JSTests/stress/exitok-is-not-the-same-as-mayExit.js	2018-11-22 03:43:30 UTC (rev 238437)
@@ -0,0 +1,19 @@
+//@ runDefault("--useAccessInlining=0")
+
+function bar(ranges) {
+    for (const [z] of ranges) {
+        let ys = [];
+        for (y = 0; y <= 100000; y++) {
+            ys[y] = false;
+        }
+    }
+}
+
+function foo() {
+    let iterator = [][Symbol.iterator]();
+    iterator.x = 1;
+}
+
+bar([ [], [], [], [], [], [], [], [], [], [], [] ]);
+foo();
+bar([ [], [] ]);

Modified: trunk/Source/_javascript_Core/ChangeLog (238436 => 238437)


--- trunk/Source/_javascript_Core/ChangeLog	2018-11-22 03:39:54 UTC (rev 238436)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-11-22 03:43:30 UTC (rev 238437)
@@ -1,5 +1,38 @@
 2018-11-21  Saam barati  <sbar...@apple.com>
 
+        DFGSpeculativeJIT should not &= exitOK with mayExit(node)
+        https://bugs.webkit.org/show_bug.cgi?id=191897
+        <rdar://problem/45871998>
+
+        Reviewed by Mark Lam.
+
+        exitOK is a statement about it being legal to exit. mayExit() is about being
+        conservative and returning false only if an OSR exit *could never* happen.
+        mayExit() tries to be as smart as possible to see if it can return false.
+        It can't return false if a runtime exit *could* happen. However, there is
+        code in the compiler where mayExit() returns false (because it uses data
+        generated from AI about type checks being proved), but the code we emit in the
+        compiler backend unconditionally generates an OSR exit, even if that exit may
+        never execute. For example, let's say we have this IR:
+        
+        SomeNode(Boolean:@input)
+        
+        And we always emit code like this as a way of emitting a boolean type check:
+        
+        jump L1 if input == true
+        jump L1 if input == false
+        emit an OSR exit
+        
+        In such a program, when we generate the above OSR exit, in a validationEnabled()
+        build, and if @input is proved to be a boolean, we'll end up crashing because we
+        have the bogus assertion saying !exitOK. This is one reason why things are cleaner
+        if we don't conflate mayExit() with exitOK.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileCurrentBlock):
+
+2018-11-21  Saam barati  <sbar...@apple.com>
+
         Fix assertion in KnownCellUse inside SpeculativeJIT::speculate
         https://bugs.webkit.org/show_bug.cgi?id=191895
         <rdar://problem/46167406>

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (238436 => 238437)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2018-11-22 03:39:54 UTC (rev 238436)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2018-11-22 03:43:30 UTC (rev 238437)
@@ -1840,8 +1840,6 @@
         m_interpreter.executeKnownEdgeTypes(m_currentNode);
         m_jit.setForNode(m_currentNode);
         m_origin = m_currentNode->origin;
-        if (validationEnabled())
-            m_origin.exitOK &= mayExit(m_jit.graph(), m_currentNode) == Exits;
         m_lastGeneratedNode = m_currentNode->op();
         
         ASSERT(m_currentNode->shouldGenerate());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to