Title: [201473] trunk
- Revision
- 201473
- Author
- sbar...@apple.com
- Date
- 2016-05-27 16:42:08 -0700 (Fri, 27 May 2016)
Log Message
DebuggerCallFrame crashes when updated with the globalExec because neither ShadowChicken's algorithm nor StackVisitor's algorithm reasons about the globalExec
https://bugs.webkit.org/show_bug.cgi?id=158104
Reviewed by Filip Pizlo.
Source/_javascript_Core:
I think globalExec is a special enough case that it should be handled
at the layers above ShadowChicken and StackVisitor. Those APIs should
deal with real stack frames on the machine stack, not a heap constructed frame.
This patch makes DebuggerCallFrame::create aware that it may be
created with the globalObject->globalExec() by having it construct
a single DebuggerCallFrame that wraps the globalExec.
This fixes a crasher because we will construct a DebuggerCallFrame
with the globalExec when the Inspector is set to pause on all uncaught
exceptions and the JS program has a syntax error. Because the program
hasn't begun execution, there is no machine JS stack frame yet. So
DebuggerCallFrame is created with globalExec, which will cause it
to hit an assertion that dictates that the stack have size greater
than zero.
* debugger/DebuggerCallFrame.cpp:
(JSC::DebuggerCallFrame::create):
LayoutTests:
* inspector/debugger/breakpoint-syntax-error-top-level-expected.txt: Added.
* inspector/debugger/breakpoint-syntax-error-top-level.html: Added.
* inspector/debugger/resources/file-with-syntax-error.js: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (201472 => 201473)
--- trunk/LayoutTests/ChangeLog 2016-05-27 23:34:25 UTC (rev 201472)
+++ trunk/LayoutTests/ChangeLog 2016-05-27 23:42:08 UTC (rev 201473)
@@ -1,3 +1,14 @@
+2016-05-27 Saam barati <sbar...@apple.com>
+
+ DebuggerCallFrame crashes when updated with the globalExec because neither ShadowChicken's algorithm nor StackVisitor's algorithm reasons about the globalExec
+ https://bugs.webkit.org/show_bug.cgi?id=158104
+
+ Reviewed by Filip Pizlo.
+
+ * inspector/debugger/breakpoint-syntax-error-top-level-expected.txt: Added.
+ * inspector/debugger/breakpoint-syntax-error-top-level.html: Added.
+ * inspector/debugger/resources/file-with-syntax-error.js: Added.
+
2016-05-27 Brent Fulgham <bfulg...@apple.com>
Unreviewed test fix after r201468.
Added: trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level-expected.txt (0 => 201473)
--- trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level-expected.txt 2016-05-27 23:42:08 UTC (rev 201473)
@@ -0,0 +1,8 @@
+CONSOLE MESSAGE: line 2: SyntaxError: Cannot declare a let variable twice: 'duplicate'.
+Making sure we don't crash when having a top-level syntax error.
+
+
+== Running test suite: TopLevelSyntaxError
+-- Running test case: TopLevelSyntaxErrorDontCrash
+PASS: Paused on the breakpoint after syntax error at top level without crashing.
+
Added: trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html (0 => 201473)
--- trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html 2016-05-27 23:42:08 UTC (rev 201473)
@@ -0,0 +1,42 @@
+<html>
+<head>
+<script src=""
+<script>
+function appendBadScript()
+{
+ let script = document.createElement("script");
+ script.src = ""
+ document.body.appendChild(script);
+}
+
+function test()
+{
+ InspectorProtocol.sendCommand("Debugger.enable", {});
+ InspectorProtocol.sendCommand("Debugger.setPauseOnExceptions", {state: "all"}, InspectorProtocol.checkForError);
+ InspectorProtocol.sendCommand("Debugger.setBreakpointsActive", {active: true});
+
+ let suite = ProtocolTest.createAsyncSuite("TopLevelSyntaxError");
+
+ suite.addTestCase({
+ name: "TopLevelSyntaxErrorDontCrash",
+ description: "Make sure exceptions from top-level syntax errors don't cause us to crash.",
+ test: (resolve, reject) => {
+ InspectorProtocol.eventHandler["Debugger.paused"] = function(messageObject) {
+ InspectorProtocol.sendCommand("Debugger.resume");
+
+ ProtocolTest.pass("Paused on the breakpoint after syntax error at top level without crashing.");
+ resolve();
+ }
+
+ ProtocolTest.evaluateInPage("appendBadScript()");
+ }
+ });
+
+ suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p> Making sure we don't crash when having a top-level syntax error. </p>
+</body>
+</html>
Added: trunk/LayoutTests/inspector/debugger/resources/file-with-syntax-error.js (0 => 201473)
--- trunk/LayoutTests/inspector/debugger/resources/file-with-syntax-error.js (rev 0)
+++ trunk/LayoutTests/inspector/debugger/resources/file-with-syntax-error.js 2016-05-27 23:42:08 UTC (rev 201473)
@@ -0,0 +1,2 @@
+let duplicate;
+let duplicate;
Modified: trunk/Source/_javascript_Core/ChangeLog (201472 => 201473)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-27 23:34:25 UTC (rev 201472)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-27 23:42:08 UTC (rev 201473)
@@ -1,3 +1,29 @@
+2016-05-27 Saam barati <sbar...@apple.com>
+
+ DebuggerCallFrame crashes when updated with the globalExec because neither ShadowChicken's algorithm nor StackVisitor's algorithm reasons about the globalExec
+ https://bugs.webkit.org/show_bug.cgi?id=158104
+
+ Reviewed by Filip Pizlo.
+
+ I think globalExec is a special enough case that it should be handled
+ at the layers above ShadowChicken and StackVisitor. Those APIs should
+ deal with real stack frames on the machine stack, not a heap constructed frame.
+
+ This patch makes DebuggerCallFrame::create aware that it may be
+ created with the globalObject->globalExec() by having it construct
+ a single DebuggerCallFrame that wraps the globalExec.
+
+ This fixes a crasher because we will construct a DebuggerCallFrame
+ with the globalExec when the Inspector is set to pause on all uncaught
+ exceptions and the JS program has a syntax error. Because the program
+ hasn't begun execution, there is no machine JS stack frame yet. So
+ DebuggerCallFrame is created with globalExec, which will cause it
+ to hit an assertion that dictates that the stack have size greater
+ than zero.
+
+ * debugger/DebuggerCallFrame.cpp:
+ (JSC::DebuggerCallFrame::create):
+
2016-05-27 Filip Pizlo <fpi...@apple.com>
DFG::LazyJSValue::tryGetStringImpl() crashes for empty values
Modified: trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.cpp (201472 => 201473)
--- trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.cpp 2016-05-27 23:34:25 UTC (rev 201472)
+++ trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.cpp 2016-05-27 23:42:08 UTC (rev 201473)
@@ -62,6 +62,12 @@
Ref<DebuggerCallFrame> DebuggerCallFrame::create(CallFrame* callFrame)
{
+ if (UNLIKELY(callFrame == callFrame->lexicalGlobalObject()->globalExec())) {
+ ShadowChicken::Frame emptyFrame;
+ RELEASE_ASSERT(!emptyFrame.isTailDeleted);
+ return adoptRef(*new DebuggerCallFrame(callFrame, emptyFrame));
+ }
+
Vector<ShadowChicken::Frame> frames;
callFrame->vm().shadowChicken().iterate(callFrame->vm(), callFrame, [&] (const ShadowChicken::Frame& frame) -> bool {
frames.append(frame);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
