<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[182167] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/182167">182167</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-03-30 17:43:49 -0700 (Mon, 30 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/181993">r181993</a>): inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes.
<https://webkit.org/b/143105>
Reviewed by Filip Pizlo.
Source/JavaScriptCore:
With <a href="http://trac.webkit.org/projects/webkit/changeset/181993">r181993</a>, the DFG and FTL may elide the storing of the scope register. As a result,
on OSR exits from DFG / FTL frames where this elision has take place, we may get baseline
JIT frames that may have its scope register not set. The Debugger's current implementation
which relies on the scope register is not happy about this. For example, this results in a
crash in the layout test inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html.
The fix is to disable inlining when the debugger is in use. Also, we add Flush nodes to
ensure that the scope register value is flushed to the register in the stack frame.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::ByteCodeParser):
(JSC::DFG::ByteCodeParser::setLocal):
(JSC::DFG::ByteCodeParser::flush):
- Add code to flush the scope register.
(JSC::DFG::ByteCodeParser::inliningCost):
- Pretend that all codeBlocks are too expensive to inline if the debugger is in use, thereby
disabling inlining whenever the debugger is in use.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::Graph):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::hasDebuggerEnabled):
* dfg/DFGStackLayoutPhase.cpp:
(JSC::DFG::StackLayoutPhase::run):
- Update the DFG codeBlock's scopeRegister since it can be moved during stack layout.
* ftl/FTLCompile.cpp:
(JSC::FTL::mmAllocateDataSection):
- Update the FTL codeBlock's scopeRegister since it can be moved during stack layout.
LayoutTests:
* TestExpectations:
- Undid test skipped in <a href="http://trac.webkit.org/projects/webkit/changeset/182072">r182072</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsTestExpectations">trunk/LayoutTests/TestExpectations</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp">trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphcpp">trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphh">trunk/Source/JavaScriptCore/dfg/DFGGraph.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGStackLayoutPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLCompilecpp">trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/LayoutTests/ChangeLog 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2015-03-30 Mark Lam <mark.lam@apple.com>
+
+ REGRESSION (r181993): inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes.
+ <https://webkit.org/b/143105>
+
+ Reviewed by Filip Pizlo.
+
+ * TestExpectations:
+ - Undid test skipped in r182072.
+
</ins><span class="cx"> 2015-03-30 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> Cached "Expires" header is not updated upon successful resource revalidation
</span></span></pre></div>
<a id="trunkLayoutTestsTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/TestExpectations (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/TestExpectations 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/LayoutTests/TestExpectations 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -127,7 +127,7 @@
</span><span class="cx"> webkit.org/b/142548 editing/selection/extend-by-character-007.html [ Failure ]
</span><span class="cx">
</span><span class="cx"> webkit.org/b/128736 inspector-protocol/debugger/setBreakpoint-dfg.html [ Failure Pass ]
</span><del>-webkit.org/b/143105 inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html [ Skip ] # Crashing
</del><ins>+webkit.org/b/134982 inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html [ Failure Pass ]
</ins><span class="cx">
</span><span class="cx"> # CSS Font Loading is not yet enabled on all platforms
</span><span class="cx"> webkit.org/b/135390 fast/css/fontloader-download-error.html [ Skip ]
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2015-03-30 Mark Lam <mark.lam@apple.com>
+
+ REGRESSION (r181993): inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes.
+ <https://webkit.org/b/143105>
+
+ Reviewed by Filip Pizlo.
+
+ With r181993, the DFG and FTL may elide the storing of the scope register. As a result,
+ on OSR exits from DFG / FTL frames where this elision has take place, we may get baseline
+ JIT frames that may have its scope register not set. The Debugger's current implementation
+ which relies on the scope register is not happy about this. For example, this results in a
+ crash in the layout test inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html.
+
+ The fix is to disable inlining when the debugger is in use. Also, we add Flush nodes to
+ ensure that the scope register value is flushed to the register in the stack frame.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::ByteCodeParser):
+ (JSC::DFG::ByteCodeParser::setLocal):
+ (JSC::DFG::ByteCodeParser::flush):
+ - Add code to flush the scope register.
+ (JSC::DFG::ByteCodeParser::inliningCost):
+ - Pretend that all codeBlocks are too expensive to inline if the debugger is in use, thereby
+ disabling inlining whenever the debugger is in use.
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::Graph):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::hasDebuggerEnabled):
+ * dfg/DFGStackLayoutPhase.cpp:
+ (JSC::DFG::StackLayoutPhase::run):
+ - Update the DFG codeBlock's scopeRegister since it can be moved during stack layout.
+ * ftl/FTLCompile.cpp:
+ (JSC::FTL::mmAllocateDataSection):
+ - Update the FTL codeBlock's scopeRegister since it can be moved during stack layout.
+
</ins><span class="cx"> 2015-03-30 Michael Saboff <msaboff@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix flakey float32-repeat-out-of-bounds.js and int8-repeat-out-of-bounds.js tests for ARM64
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -147,6 +147,7 @@
</span><span class="cx"> , m_inlineStackTop(0)
</span><span class="cx"> , m_haveBuiltOperandMaps(false)
</span><span class="cx"> , m_currentInstruction(0)
</span><ins>+ , m_hasDebuggerEnabled(graph.hasDebuggerEnabled())
</ins><span class="cx"> {
</span><span class="cx"> ASSERT(m_profiledBlock);
</span><span class="cx"> }
</span><span class="lines">@@ -385,6 +386,8 @@
</span><span class="cx"> ArgumentPosition* argumentPosition = findArgumentPositionForLocal(operand);
</span><span class="cx"> if (argumentPosition)
</span><span class="cx"> flushDirect(operand, argumentPosition);
</span><ins>+ else if (m_hasDebuggerEnabled && operand == m_codeBlock->scopeRegister())
+ flush(operand);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> VariableAccessData* variableAccessData = newVariableAccessData(operand);
</span><span class="lines">@@ -522,6 +525,7 @@
</span><span class="cx"> {
</span><span class="cx"> int numArguments;
</span><span class="cx"> if (InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame) {
</span><ins>+ ASSERT(!m_hasDebuggerEnabled);
</ins><span class="cx"> numArguments = inlineCallFrame->arguments.size();
</span><span class="cx"> if (inlineCallFrame->isClosureCall)
</span><span class="cx"> flushDirect(inlineStackEntry->remapOperand(VirtualRegister(JSStack::Callee)));
</span><span class="lines">@@ -531,6 +535,8 @@
</span><span class="cx"> numArguments = inlineStackEntry->m_codeBlock->numParameters();
</span><span class="cx"> for (unsigned argument = numArguments; argument-- > 1;)
</span><span class="cx"> flushDirect(inlineStackEntry->remapOperand(virtualRegisterForArgument(argument)));
</span><ins>+ if (m_hasDebuggerEnabled)
+ flush(m_codeBlock->scopeRegister());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void flushForTerminal()
</span><span class="lines">@@ -997,6 +1003,7 @@
</span><span class="cx"> StubInfoMap m_dfgStubInfos;
</span><span class="cx">
</span><span class="cx"> Instruction* m_currentInstruction;
</span><ins>+ bool m_hasDebuggerEnabled;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> #define NEXT_OPCODE(name) \
</span><span class="lines">@@ -1168,6 +1175,12 @@
</span><span class="cx"> if (verbose)
</span><span class="cx"> dataLog("Considering inlining ", callee, " into ", currentCodeOrigin(), "\n");
</span><span class="cx">
</span><ins>+ if (m_hasDebuggerEnabled) {
+ if (verbose)
+ dataLog(" Failing because the debugger is in use.\n");
+ return UINT_MAX;
+ }
+
</ins><span class="cx"> FunctionExecutable* executable = callee.functionExecutable();
</span><span class="cx"> if (!executable) {
</span><span class="cx"> if (verbose)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -74,6 +74,9 @@
</span><span class="cx">
</span><span class="cx"> for (unsigned i = m_mustHandleValues.size(); i--;)
</span><span class="cx"> m_mustHandleValues[i] = freezeFragile(plan.mustHandleValues[i]);
</span><ins>+
+ m_hasDebuggerEnabled = m_profiledBlock->globalObject()->hasDebugger()
+ || Options::forceDebuggerBytecodeGeneration();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> Graph::~Graph()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.h (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.h 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.h 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -709,7 +709,9 @@
</span><span class="cx"> NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
</span><span class="cx"> BasicBlock*, const char* file, int line, const char* function,
</span><span class="cx"> const char* assertion);
</span><del>-
</del><ins>+
+ bool hasDebuggerEnabled() const { return m_hasDebuggerEnabled; }
+
</ins><span class="cx"> VM& m_vm;
</span><span class="cx"> Plan& m_plan;
</span><span class="cx"> CodeBlock* m_codeBlock;
</span><span class="lines">@@ -791,6 +793,7 @@
</span><span class="cx"> UnificationState m_unificationState;
</span><span class="cx"> PlanStage m_planStage { PlanStage::Initial };
</span><span class="cx"> RefCountState m_refCountState;
</span><ins>+ bool m_hasDebuggerEnabled;
</ins><span class="cx"> private:
</span><span class="cx">
</span><span class="cx"> void handleSuccessor(Vector<BasicBlock*, 16>& worklist, BasicBlock*, BasicBlock* successor);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGStackLayoutPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -175,7 +175,10 @@
</span><span class="cx">
</span><span class="cx"> // This register is never valid for DFG code blocks.
</span><span class="cx"> codeBlock()->setActivationRegister(VirtualRegister());
</span><del>- codeBlock()->setScopeRegister(VirtualRegister());
</del><ins>+ if (LIKELY(!m_graph.hasDebuggerEnabled()))
+ codeBlock()->setScopeRegister(VirtualRegister());
+ else
+ codeBlock()->setScopeRegister(assign(allocation, codeBlock()->scopeRegister()));
</ins><span class="cx">
</span><span class="cx"> for (unsigned i = m_graph.m_inlineVariableData.size(); i--;) {
</span><span class="cx"> InlineVariableData data = m_graph.m_inlineVariableData[i];
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLCompilecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp (182166 => 182167)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp 2015-03-31 00:43:49 UTC (rev 182167)
</span><span class="lines">@@ -322,6 +322,9 @@
</span><span class="cx"> inlineCallFrame->calleeRecovery =
</span><span class="cx"> inlineCallFrame->calleeRecovery.withLocalsOffset(localsOffset);
</span><span class="cx"> }
</span><ins>+
+ if (graph.hasDebuggerEnabled())
+ codeBlock->setScopeRegister(codeBlock->scopeRegister() + localsOffset);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> MacroAssembler::Label stackOverflowException;
</span></span></pre>
</div>
</div>
</body>
</html>