Title: [237297] trunk
Revision
237297
Author
sbar...@apple.com
Date
2018-10-19 13:03:54 -0700 (Fri, 19 Oct 2018)

Log Message

vmCall should check if we exit before emitting an OSR exit due to exceptions
https://bugs.webkit.org/show_bug.cgi?id=190740
<rdar://problem/45220139>

Reviewed by Mark Lam.

JSTests:

* stress/dont-emit-osr-exits-for-every-call-ftl.js: Added.
(foo):

Source/_javascript_Core:

The bug we were seeing is the MovHint removal phase would
eliminate a superfluous MovHint. This left a certain range
of nodes in a state where they would not be able to reconstruct
values for an OSR exit. This is OK, since this phase proved those
nodes don't exit. However, some of these nodes may use the vmCall
construct in FTLLower. vmCall used to unconditionally emit an
exception check after each call. However, if such a call happens
in the range of nodes where we can't exit, we would end up generating
an invalid exit (and running with validateFTLOSRExitLiveness flag
would find this issue).

This patch makes vmCall check to see if the node can exit before
emitting an exception check. A node not being able to exit implies
that it can't exit for exceptions, therefore, by definition, it can't
throw an exception.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::vmCall):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (237296 => 237297)


--- trunk/JSTests/ChangeLog	2018-10-19 19:31:37 UTC (rev 237296)
+++ trunk/JSTests/ChangeLog	2018-10-19 20:03:54 UTC (rev 237297)
@@ -1,3 +1,14 @@
+2018-10-19  Saam Barati  <sbar...@apple.com>
+
+        vmCall should check if we exit before emitting an OSR exit due to exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=190740
+        <rdar://problem/45220139>
+
+        Reviewed by Mark Lam.
+
+        * stress/dont-emit-osr-exits-for-every-call-ftl.js: Added.
+        (foo):
+
 2018-10-19  Caio Lima  <ticaiol...@gmail.com>
 
         [ESNext][BigInt] Implement support for "^"

Added: trunk/JSTests/stress/dont-emit-osr-exits-for-every-call-ftl.js (0 => 237297)


--- trunk/JSTests/stress/dont-emit-osr-exits-for-every-call-ftl.js	                        (rev 0)
+++ trunk/JSTests/stress/dont-emit-osr-exits-for-every-call-ftl.js	2018-10-19 20:03:54 UTC (rev 237297)
@@ -0,0 +1,15 @@
+//@ runDefault("--useConcurrentJIT=0", "--validateFTLOSRExitLiveness=1")
+
+function foo(o, p) {
+    p = null;
+    try {
+        o.f = null;
+        p = null;
+    } catch (e) {
+    }
+}
+noInline(foo);
+
+for (var i = 0; i < 1000000; ++i) {
+    foo({});
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (237296 => 237297)


--- trunk/Source/_javascript_Core/ChangeLog	2018-10-19 19:31:37 UTC (rev 237296)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-10-19 20:03:54 UTC (rev 237297)
@@ -1,3 +1,30 @@
+2018-10-19  Saam Barati  <sbar...@apple.com>
+
+        vmCall should check if we exit before emitting an OSR exit due to exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=190740
+        <rdar://problem/45220139>
+
+        Reviewed by Mark Lam.
+
+        The bug we were seeing is the MovHint removal phase would
+        eliminate a superfluous MovHint. This left a certain range
+        of nodes in a state where they would not be able to reconstruct
+        values for an OSR exit. This is OK, since this phase proved those
+        nodes don't exit. However, some of these nodes may use the vmCall
+        construct in FTLLower. vmCall used to unconditionally emit an
+        exception check after each call. However, if such a call happens
+        in the range of nodes where we can't exit, we would end up generating
+        an invalid exit (and running with validateFTLOSRExitLiveness flag
+        would find this issue).
+        
+        This patch makes vmCall check to see if the node can exit before
+        emitting an exception check. A node not being able to exit implies
+        that it can't exit for exceptions, therefore, by definition, it can't
+        throw an exception.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::vmCall):
+
 2018-10-19  Caio Lima  <ticaiol...@gmail.com>
 
         [ESNext][BigInt] Implement support for "^"

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (237296 => 237297)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-10-19 19:31:37 UTC (rev 237296)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-10-19 20:03:54 UTC (rev 237297)
@@ -45,6 +45,7 @@
 #include "DFGCapabilities.h"
 #include "DFGDominators.h"
 #include "DFGInPlaceAbstractState.h"
+#include "DFGMayExit.h"
 #include "DFGOSRAvailabilityAnalysisPhase.h"
 #include "DFGOSRExitFuzz.h"
 #include "DirectArguments.h"
@@ -16333,7 +16334,26 @@
     {
         callPreflight();
         LValue result = m_out.call(type, function, std::forward<Args>(args)...);
-        callCheck();
+        if (mayExit(m_graph, m_node))
+            callCheck();
+        else {
+            // We can't exit due to an exception, so we also can't throw an exception.
+#ifndef NDEBUG
+            LBasicBlock crash = m_out.newBlock();
+            LBasicBlock continuation = m_out.newBlock();
+
+            LValue exception = m_out.load64(m_out.absolute(vm().addressOfException()));
+            LValue hadException = m_out.notZero64(exception);
+
+            m_out.branch(
+                hadException, rarely(crash), usually(continuation));
+
+            LBasicBlock lastNext = m_out.appendTo(crash, continuation);
+            m_out.unreachable();
+
+            m_out.appendTo(continuation, lastNext);
+#endif
+        }
         return result;
     }
     
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to