Title: [200906] trunk/Source/_javascript_Core
Revision
200906
Author
[email protected]
Date
2016-05-13 19:03:10 -0700 (Fri, 13 May 2016)

Log Message

DFG/FTL have a few bugs in their reasoning about the scope
https://bugs.webkit.org/show_bug.cgi?id=157696

Reviewed by Benjamin Poulain.

1. When the debugger is enabled, it is easier for the DFG to reason
about the scope register by simply claiming all nodes read the scope
register. This prevents us from ever entering the runtime where we
may take a stack trace but there isn't a scope on the stack.

2. This patch fixes a bug where the FTL compilation wasn't properly
setting the CodeBlock register. It was only doing this when there
was inline data, but when the debugger is enabled, we never inline.
So this code just needed to be removed from that loop. It was never
right for it to be inside the loop.

* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* ftl/FTLCompile.cpp:
(JSC::FTL::compile):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (200905 => 200906)


--- trunk/Source/_javascript_Core/ChangeLog	2016-05-14 00:20:24 UTC (rev 200905)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-05-14 02:03:10 UTC (rev 200906)
@@ -1,3 +1,26 @@
+2016-05-13  Saam barati  <[email protected]>
+
+        DFG/FTL have a few bugs in their reasoning about the scope
+        https://bugs.webkit.org/show_bug.cgi?id=157696
+
+        Reviewed by Benjamin Poulain.
+
+        1. When the debugger is enabled, it is easier for the DFG to reason
+        about the scope register by simply claiming all nodes read the scope
+        register. This prevents us from ever entering the runtime where we
+        may take a stack trace but there isn't a scope on the stack.
+
+        2. This patch fixes a bug where the FTL compilation wasn't properly
+        setting the CodeBlock register. It was only doing this when there
+        was inline data, but when the debugger is enabled, we never inline.
+        So this code just needed to be removed from that loop. It was never
+        right for it to be inside the loop.
+
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * ftl/FTLCompile.cpp:
+        (JSC::FTL::compile):
+
 2016-05-13  Benjamin Poulain  <[email protected]>
 
         [JSC] SetLocal without exit do not need phantoms

Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (200905 => 200906)


--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2016-05-14 00:20:24 UTC (rev 200905)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2016-05-14 02:03:10 UTC (rev 200906)
@@ -117,6 +117,17 @@
         if (inlineCallFrame->isVarargs())
             read(AbstractHeap(Stack, inlineCallFrame->stackOffset + JSStack::ArgumentCount));
     }
+
+    // We don't want to specifically account which nodes can read from the scope
+    // when the debugger is enabled. It's helpful to just claim all nodes do.
+    // Specifically, if a node allocates, this may call into the debugger's machinery.
+    // The debugger's machinery is free to take a stack trace and try to read from
+    // a scope which is expected to be flushed to the stack.
+    if (graph.hasDebuggerEnabled()) {
+        ASSERT(!node->origin.semantic.inlineCallFrame);
+        read(AbstractHeap(Stack, graph.m_codeBlock->scopeRegister()));
+    }
+        
     
     switch (node->op()) {
     case JSConstant:

Modified: trunk/Source/_javascript_Core/ftl/FTLCompile.cpp (200905 => 200906)


--- trunk/Source/_javascript_Core/ftl/FTLCompile.cpp	2016-05-14 00:20:24 UTC (rev 200905)
+++ trunk/Source/_javascript_Core/ftl/FTLCompile.cpp	2016-05-14 02:03:10 UTC (rev 200906)
@@ -103,9 +103,11 @@
                 inlineCallFrame->calleeRecovery.withLocalsOffset(localsOffset);
         }
 
-        if (graph.hasDebuggerEnabled())
-            codeBlock->setScopeRegister(codeBlock->scopeRegister() + localsOffset);
     }
+
+    if (graph.hasDebuggerEnabled())
+        codeBlock->setScopeRegister(codeBlock->scopeRegister() + localsOffset);
+
     for (OSRExitDescriptor& descriptor : state.jitCode->osrExitDescriptors) {
         for (unsigned i = descriptor.m_values.size(); i--;)
             descriptor.m_values[i] = descriptor.m_values[i].withLocalsOffset(localsOffset);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to