Diff
Modified: trunk/LayoutTests/ChangeLog (198581 => 198582)
--- trunk/LayoutTests/ChangeLog 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/LayoutTests/ChangeLog 2016-03-23 09:15:43 UTC (rev 198582)
@@ -1,3 +1,13 @@
+2016-03-23 Saam barati <[email protected]>
+
+ We should not disable inlining when the debugger is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=155741
+
+ Reviewed by Oliver Hunt.
+
+ * inspector/debugger/breakpoint-with-inlining-expected.txt: Added.
+ * inspector/debugger/breakpoint-with-inlining.html: Added.
+
2016-03-22 Darin Adler <[email protected]>
Tiny tweak to test I just landed.
Added: trunk/LayoutTests/inspector/debugger/breakpoint-with-inlining-expected.txt (0 => 198582)
--- trunk/LayoutTests/inspector/debugger/breakpoint-with-inlining-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-with-inlining-expected.txt 2016-03-23 09:15:43 UTC (rev 198582)
@@ -0,0 +1,7 @@
+Debugger.setBreakpoint on line:column in <script>
+
+Found <script>
+Running testInlining() without a breakpoint to get it DFG or FTL compiled with foo() inlined.
+Running testInlining() again with a breakpoint set at foo().
+We paused all 10 times as expected!
+
Added: trunk/LayoutTests/inspector/debugger/breakpoint-with-inlining.html (0 => 198582)
--- trunk/LayoutTests/inspector/debugger/breakpoint-with-inlining.html (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-with-inlining.html 2016-03-23 09:15:43 UTC (rev 198582)
@@ -0,0 +1,62 @@
+<html>
+<head>
+<script src=""
+<script>
+// Put this here instead of on <body onload> to prevent an extra Debugger.scriptParsed event.
+window._onload_ = runTest;
+
+function test()
+{
+ // This test sets a breakpoint on line:column in the <script> below.
+ // We first warm the function up to get foo inlined. Then we set a breakpoint
+ // and make sure that we hit it.
+
+ InspectorProtocol.sendCommand("Debugger.enable", {});
+
+ InspectorProtocol.eventHandler["Debugger.scriptParsed"] = function(messageObject)
+ {
+ if (/breakpoint-with-inlining\.html$/.test(messageObject.params.url) && messageObject.params.startLine > 10) {
+ ProtocolTest.log("Found <script>");
+ var scriptIdentifier = messageObject.params.scriptId;
+ var lineNumber = messageObject.params.startLine + 2;
+ var columnNumber = 4;
+ var location = {scriptId: scriptIdentifier, lineNumber: lineNumber, columnNumber: columnNumber};
+
+ ProtocolTest.log("Running testInlining() without a breakpoint to get it DFG or FTL compiled with foo() inlined.");
+
+ InspectorProtocol.sendCommand("Runtime.evaluate", {_expression_: "testInlining(100000)"}, function() {
+ InspectorProtocol.sendCommand("Debugger.setBreakpoint", {location: location}, function() {
+ ProtocolTest.log("Running testInlining() again with a breakpoint set at foo().");
+ InspectorProtocol.sendCommand("Runtime.evaluate", {_expression_: "testInlining(10)"});
+ });
+ });
+ }
+ }
+
+ let iters = 0;
+ InspectorProtocol.eventHandler["Debugger.paused"] = function(messageObject)
+ {
+ InspectorProtocol.sendCommand("Debugger.resume", {});
+
+ iters++;
+ if (iters === 10) {
+ ProtocolTest.log("We paused all 10 times as expected!");
+ ProtocolTest.completeTest();
+ }
+ }
+}
+</script>
+</head>
+<body>
+<p>Debugger.setBreakpoint on line:column in <script></p>
+<script> // Line 0
+function foo(i) { // Line 1;
+ return i*2; // Line 2;
+}
+function testInlining(iters) {
+ for (let i = 0; i < iters; i++)
+ foo(i);
+}
+</script>
+</body>
+</html>
Modified: trunk/Source/_javascript_Core/ChangeLog (198581 => 198582)
--- trunk/Source/_javascript_Core/ChangeLog 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-03-23 09:15:43 UTC (rev 198582)
@@ -1,3 +1,36 @@
+2016-03-23 Saam barati <[email protected]>
+
+ We should not disable inlining when the debugger is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=155741
+
+ Reviewed by Oliver Hunt.
+
+ We can enable inlining when the debugger is enabled as long
+ as we make sure we still jettison the proper CodeBlocks when
+ a breakpoint is set. This means that for any optimized CodeBlock,
+ we must ask if any of its inlinees contain the breakpoint that
+ is being set. If any inlinees do contain the breakpoint, we must
+ jettison the machine code block that they are a part of.
+
+ * debugger/Debugger.cpp:
+ (JSC::Debugger::toggleBreakpoint):
+ (JSC::Debugger::applyBreakpoints):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::ByteCodeParser):
+ (JSC::DFG::ByteCodeParser::setLocal):
+ (JSC::DFG::ByteCodeParser::flush):
+ (JSC::DFG::ByteCodeParser::flushForTerminal):
+ (JSC::DFG::ByteCodeParser::inliningCost):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::Graph):
+ (JSC::DFG::Graph::~Graph):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::hasDebuggerEnabled): Deleted.
+ * dfg/DFGStackLayoutPhase.cpp:
+ (JSC::DFG::StackLayoutPhase::run):
+ * ftl/FTLCompile.cpp:
+ (JSC::FTL::compile):
+
2016-03-23 Yusuke Suzuki <[email protected]>
[ES6] Allow undefined/null for Symbol.search and Symbol.match
Modified: trunk/Source/_javascript_Core/debugger/Debugger.cpp (198581 => 198582)
--- trunk/Source/_javascript_Core/debugger/Debugger.cpp 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/debugger/Debugger.cpp 2016-03-23 09:15:43 UTC (rev 198582)
@@ -23,9 +23,11 @@
#include "Debugger.h"
#include "CodeBlock.h"
+#include "DFGCommonData.h"
#include "DebuggerCallFrame.h"
#include "Error.h"
#include "HeapIterationScope.h"
+#include "InlineCallFrame.h"
#include "Interpreter.h"
#include "JSCJSValueInlines.h"
#include "JSFunction.h"
@@ -248,40 +250,64 @@
void Debugger::toggleBreakpoint(CodeBlock* codeBlock, Breakpoint& breakpoint, BreakpointState enabledOrNot)
{
- ScriptExecutable* executable = codeBlock->ownerScriptExecutable();
+ auto isBreakpointInCodeBlock = [&] (CodeBlock* codeBlock) -> bool {
+ ScriptExecutable* executable = codeBlock->ownerScriptExecutable();
- SourceID sourceID = static_cast<SourceID>(executable->sourceID());
- if (breakpoint.sourceID != sourceID)
- return;
+ SourceID sourceID = static_cast<SourceID>(executable->sourceID());
+ if (breakpoint.sourceID != sourceID)
+ return false;
- unsigned line = breakpoint.line;
- unsigned column = breakpoint.column;
+ unsigned line = breakpoint.line;
+ unsigned column = breakpoint.column;
+
+ unsigned startLine = executable->firstLine();
+ unsigned startColumn = executable->startColumn();
+ unsigned endLine = executable->lastLine();
+ unsigned endColumn = executable->endColumn();
+ line += 1;
+ column = column ? column + 1 : Breakpoint::unspecifiedColumn;
- unsigned startLine = executable->firstLine();
- unsigned startColumn = executable->startColumn();
- unsigned endLine = executable->lastLine();
- unsigned endColumn = executable->endColumn();
+ if (line < startLine || line > endLine)
+ return false;
+ if (column != Breakpoint::unspecifiedColumn) {
+ if (line == startLine && column < startColumn)
+ return false;
+ if (line == endLine && column > endColumn)
+ return false;
+ }
- // Inspector breakpoint line and column values are zero-based but the executable
- // and CodeBlock line and column values are one-based.
- line += 1;
- column = column ? column + 1 : Breakpoint::unspecifiedColumn;
+ if (!codeBlock->hasOpDebugForLineAndColumn(line, column))
+ return false;
- if (line < startLine || line > endLine)
+ return true;
+ };
+
+ if (isBreakpointInCodeBlock(codeBlock)) {
+ if (enabledOrNot == BreakpointEnabled)
+ codeBlock->addBreakpoint(1);
+ else
+ codeBlock->removeBreakpoint(1);
+
return;
- if (column != Breakpoint::unspecifiedColumn) {
- if (line == startLine && column < startColumn)
+ }
+
+#if ENABLE(DFG_JIT)
+ if (enabledOrNot == BreakpointEnabled) {
+ // See if any of our inlinees contain the breakpoint. We only care about this
+ // when we set a breakpoint.
+ if (!JITCode::isOptimizingJIT(codeBlock->jitType()))
return;
- if (line == endLine && column > endColumn)
+ InlineCallFrameSet* inlineCallFrameSet = codeBlock->jitCode()->dfgCommon()->inlineCallFrames.get();
+ if (!inlineCallFrameSet)
return;
+ for (InlineCallFrame* inlineCallFrame : *inlineCallFrameSet) {
+ if (isBreakpointInCodeBlock(inlineCallFrame->baselineCodeBlock.get())) {
+ codeBlock->jettison(Profiler::JettisonDueToDebuggerBreakpoint);
+ break;
+ }
+ }
}
- if (!codeBlock->hasOpDebugForLineAndColumn(line, column))
- return;
-
- if (enabledOrNot == BreakpointEnabled)
- codeBlock->addBreakpoint(1);
- else
- codeBlock->removeBreakpoint(1);
+#endif // ENABLE(DFG_JIT)
}
void Debugger::applyBreakpoints(CodeBlock* codeBlock)
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (198581 => 198582)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-03-23 09:15:43 UTC (rev 198582)
@@ -150,7 +150,6 @@
, m_numPassedVarArgs(0)
, m_inlineStackTop(0)
, m_currentInstruction(0)
- , m_hasDebuggerEnabled(graph.hasDebuggerEnabled())
{
ASSERT(m_profiledBlock);
}
@@ -447,8 +446,6 @@
ArgumentPosition* argumentPosition = findArgumentPositionForLocal(operand);
if (argumentPosition)
flushDirect(operand, argumentPosition);
- else if (m_hasDebuggerEnabled && operand == m_codeBlock->scopeRegister())
- flush(operand);
}
VariableAccessData* variableAccessData = newVariableAccessData(operand);
@@ -586,7 +583,6 @@
{
int numArguments;
if (InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame) {
- ASSERT(!m_hasDebuggerEnabled);
numArguments = inlineCallFrame->arguments.size();
if (inlineCallFrame->isClosureCall)
flushDirect(inlineStackEntry->remapOperand(VirtualRegister(JSStack::Callee)));
@@ -596,8 +592,6 @@
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()
@@ -1121,7 +1115,6 @@
StubInfoMap m_dfgStubInfos;
Instruction* m_currentInstruction;
- bool m_hasDebuggerEnabled;
};
#define NEXT_OPCODE(name) \
@@ -1294,12 +1287,6 @@
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 (198581 => 198582)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2016-03-23 09:15:43 UTC (rev 198582)
@@ -77,9 +77,6 @@
, m_refCountState(EverythingIsLive)
{
ASSERT(m_profiledBlock);
-
- m_hasDebuggerEnabled = m_profiledBlock->globalObject()->hasDebugger()
- || Options::forceDebuggerBytecodeGeneration();
}
Graph::~Graph()
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (198581 => 198582)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2016-03-23 09:15:43 UTC (rev 198582)
@@ -797,8 +797,6 @@
BasicBlock*, const char* file, int line, const char* function,
const char* assertion);
- bool hasDebuggerEnabled() const { return m_hasDebuggerEnabled; }
-
void ensureDominators();
void ensurePrePostNumbering();
void ensureNaturalLoops();
@@ -896,7 +894,6 @@
UnificationState m_unificationState;
PlanStage m_planStage { PlanStage::Initial };
RefCountState m_refCountState;
- bool m_hasDebuggerEnabled;
bool m_hasExceptionHandlers { false };
private:
Modified: trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp (198581 => 198582)
--- trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/dfg/DFGStackLayoutPhase.cpp 2016-03-23 09:15:43 UTC (rev 198582)
@@ -173,10 +173,7 @@
data->machineLocal = assign(allocation, data->local);
}
- if (LIKELY(!m_graph.hasDebuggerEnabled()))
- codeBlock()->setScopeRegister(VirtualRegister());
- else
- codeBlock()->setScopeRegister(assign(allocation, codeBlock()->scopeRegister()));
+ codeBlock()->setScopeRegister(VirtualRegister());
for (unsigned i = m_graph.m_inlineVariableData.size(); i--;) {
InlineVariableData data = ""
Modified: trunk/Source/_javascript_Core/ftl/FTLCompile.cpp (198581 => 198582)
--- trunk/Source/_javascript_Core/ftl/FTLCompile.cpp 2016-03-23 09:11:56 UTC (rev 198581)
+++ trunk/Source/_javascript_Core/ftl/FTLCompile.cpp 2016-03-23 09:15:43 UTC (rev 198582)
@@ -102,9 +102,6 @@
inlineCallFrame->calleeRecovery =
inlineCallFrame->calleeRecovery.withLocalsOffset(localsOffset);
}
-
- if (graph.hasDebuggerEnabled())
- codeBlock->setScopeRegister(codeBlock->scopeRegister() + localsOffset);
}
for (OSRExitDescriptor& descriptor : state.jitCode->osrExitDescriptors) {
for (unsigned i = descriptor.m_values.size(); i--;)
