- Revision
- 182167
- Author
- [email protected]
- Date
- 2015-03-30 17:43:49 -0700 (Mon, 30 Mar 2015)
Log Message
REGRESSION (r181993): inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes.
<https://webkit.org/b/143105>
Reviewed by Filip Pizlo.
Source/_javascript_Core:
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.
LayoutTests:
* TestExpectations:
- Undid test skipped in r182072.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (182166 => 182167)
--- trunk/LayoutTests/ChangeLog 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/LayoutTests/ChangeLog 2015-03-31 00:43:49 UTC (rev 182167)
@@ -1,3 +1,13 @@
+2015-03-30 Mark Lam <[email protected]>
+
+ 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.
+
2015-03-30 Chris Dumez <[email protected]>
Cached "Expires" header is not updated upon successful resource revalidation
Modified: trunk/LayoutTests/TestExpectations (182166 => 182167)
--- trunk/LayoutTests/TestExpectations 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/LayoutTests/TestExpectations 2015-03-31 00:43:49 UTC (rev 182167)
@@ -127,7 +127,7 @@
webkit.org/b/142548 editing/selection/extend-by-character-007.html [ Failure ]
webkit.org/b/128736 inspector-protocol/debugger/setBreakpoint-dfg.html [ Failure Pass ]
-webkit.org/b/143105 inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html [ Skip ] # Crashing
+webkit.org/b/134982 inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html [ Failure Pass ]
# CSS Font Loading is not yet enabled on all platforms
webkit.org/b/135390 fast/css/fontloader-download-error.html [ Skip ]
Modified: trunk/Source/_javascript_Core/ChangeLog (182166 => 182167)
--- trunk/Source/_javascript_Core/ChangeLog 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-03-31 00:43:49 UTC (rev 182167)
@@ -1,3 +1,38 @@
+2015-03-30 Mark Lam <[email protected]>
+
+ 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.
+
2015-03-30 Michael Saboff <[email protected]>
Fix flakey float32-repeat-out-of-bounds.js and int8-repeat-out-of-bounds.js tests for ARM64
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (182166 => 182167)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-03-31 00:43:49 UTC (rev 182167)
@@ -147,6 +147,7 @@
, m_inlineStackTop(0)
, m_haveBuiltOperandMaps(false)
, m_currentInstruction(0)
+ , m_hasDebuggerEnabled(graph.hasDebuggerEnabled())
{
ASSERT(m_profiledBlock);
}
@@ -385,6 +386,8 @@
ArgumentPosition* argumentPosition = findArgumentPositionForLocal(operand);
if (argumentPosition)
flushDirect(operand, argumentPosition);
+ else if (m_hasDebuggerEnabled && operand == m_codeBlock->scopeRegister())
+ flush(operand);
}
VariableAccessData* variableAccessData = newVariableAccessData(operand);
@@ -522,6 +525,7 @@
{
int numArguments;
if (InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame) {
+ ASSERT(!m_hasDebuggerEnabled);
numArguments = inlineCallFrame->arguments.size();
if (inlineCallFrame->isClosureCall)
flushDirect(inlineStackEntry->remapOperand(VirtualRegister(JSStack::Callee)));
@@ -531,6 +535,8 @@
numArguments = inlineStackEntry->m_codeBlock->numParameters();
for (unsigned argument = numArguments; argument-- > 1;)
flushDirect(inlineStackEntry->remapOperand(virtualRegisterForArgument(argument)));
+ if (m_hasDebuggerEnabled)
+ flush(m_codeBlock->scopeRegister());
}
void flushForTerminal()
@@ -997,6 +1003,7 @@
StubInfoMap m_dfgStubInfos;
Instruction* m_currentInstruction;
+ bool m_hasDebuggerEnabled;
};
#define NEXT_OPCODE(name) \
@@ -1168,6 +1175,12 @@
if (verbose)
dataLog("Considering inlining ", callee, " into ", currentCodeOrigin(), "\n");
+ if (m_hasDebuggerEnabled) {
+ if (verbose)
+ dataLog(" Failing because the debugger is in use.\n");
+ return UINT_MAX;
+ }
+
FunctionExecutable* executable = callee.functionExecutable();
if (!executable) {
if (verbose)
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (182166 => 182167)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2015-03-31 00:43:49 UTC (rev 182167)
@@ -74,6 +74,9 @@
for (unsigned i = m_mustHandleValues.size(); i--;)
m_mustHandleValues[i] = freezeFragile(plan.mustHandleValues[i]);
+
+ m_hasDebuggerEnabled = m_profiledBlock->globalObject()->hasDebugger()
+ || Options::forceDebuggerBytecodeGeneration();
}
Graph::~Graph()
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (182166 => 182167)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2015-03-31 00:43:49 UTC (rev 182167)
@@ -709,7 +709,9 @@
NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
BasicBlock*, const char* file, int line, const char* function,
const char* assertion);
-
+
+ bool hasDebuggerEnabled() const { return m_hasDebuggerEnabled; }
+
VM& m_vm;
Plan& m_plan;
CodeBlock* m_codeBlock;
@@ -791,6 +793,7 @@
UnificationState m_unificationState;
PlanStage m_planStage { PlanStage::Initial };
RefCountState m_refCountState;
+ bool m_hasDebuggerEnabled;
private:
void handleSuccessor(Vector<BasicBlock*, 16>& worklist, BasicBlock*, BasicBlock* successor);
Modified: trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp (182166 => 182167)
--- trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp 2015-03-31 00:43:49 UTC (rev 182167)
@@ -175,7 +175,10 @@
// This register is never valid for DFG code blocks.
codeBlock()->setActivationRegister(VirtualRegister());
- codeBlock()->setScopeRegister(VirtualRegister());
+ if (LIKELY(!m_graph.hasDebuggerEnabled()))
+ codeBlock()->setScopeRegister(VirtualRegister());
+ else
+ codeBlock()->setScopeRegister(assign(allocation, codeBlock()->scopeRegister()));
for (unsigned i = m_graph.m_inlineVariableData.size(); i--;) {
InlineVariableData data = ""
Modified: trunk/Source/_javascript_Core/ftl/FTLCompile.cpp (182166 => 182167)
--- trunk/Source/_javascript_Core/ftl/FTLCompile.cpp 2015-03-31 00:33:19 UTC (rev 182166)
+++ trunk/Source/_javascript_Core/ftl/FTLCompile.cpp 2015-03-31 00:43:49 UTC (rev 182167)
@@ -322,6 +322,9 @@
inlineCallFrame->calleeRecovery =
inlineCallFrame->calleeRecovery.withLocalsOffset(localsOffset);
}
+
+ if (graph.hasDebuggerEnabled())
+ codeBlock->setScopeRegister(codeBlock->scopeRegister() + localsOffset);
}
MacroAssembler::Label stackOverflowException;
