<!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>[201473] 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/201473">201473</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2016-05-27 16:42:08 -0700 (Fri, 27 May 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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/JavaScriptCore:
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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredebuggerDebuggerCallFramecpp">trunk/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsinspectordebuggerbreakpointsyntaxerrortoplevelexpectedtxt">trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level-expected.txt</a></li>
<li><a href="#trunkLayoutTestsinspectordebuggerbreakpointsyntaxerrortoplevelhtml">trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html</a></li>
<li><a href="#trunkLayoutTestsinspectordebuggerresourcesfilewithsyntaxerrorjs">trunk/LayoutTests/inspector/debugger/resources/file-with-syntax-error.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (201472 => 201473)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-05-27 23:34:25 UTC (rev 201472)
+++ trunk/LayoutTests/ChangeLog 2016-05-27 23:42:08 UTC (rev 201473)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2016-05-27 Saam barati <sbarati@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.
+
</ins><span class="cx"> 2016-05-27 Brent Fulgham <bfulgham@apple.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed test fix after r201468.
</span></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerbreakpointsyntaxerrortoplevelexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level-expected.txt (0 => 201473)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,8 @@
</span><ins>+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.
+
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerbreakpointsyntaxerrortoplevelhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/breakpoint-syntax-error-top-level.html (0 => 201473)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,42 @@
</span><ins>+<html>
+<head>
+<script src="../../http/tests/inspector/resources/protocol-test.js"></script>
+<script>
+function appendBadScript()
+{
+ let script = document.createElement("script");
+ script.src = "resources/file-with-syntax-error.js";
+ 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>
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerresourcesfilewithsyntaxerrorjs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/resources/file-with-syntax-error.js (0 => 201473)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,2 @@
</span><ins>+let duplicate;
+let duplicate;
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (201472 => 201473)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-05-27 23:34:25 UTC (rev 201472)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-05-27 23:42:08 UTC (rev 201473)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2016-05-27 Saam barati <sbarati@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):
+
</ins><span class="cx"> 2016-05-27 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> DFG::LazyJSValue::tryGetStringImpl() crashes for empty values
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredebuggerDebuggerCallFramecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp (201472 => 201473)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp 2016-05-27 23:34:25 UTC (rev 201472)
+++ trunk/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp 2016-05-27 23:42:08 UTC (rev 201473)
</span><span class="lines">@@ -62,6 +62,12 @@
</span><span class="cx">
</span><span class="cx"> Ref<DebuggerCallFrame> DebuggerCallFrame::create(CallFrame* callFrame)
</span><span class="cx"> {
</span><ins>+ if (UNLIKELY(callFrame == callFrame->lexicalGlobalObject()->globalExec())) {
+ ShadowChicken::Frame emptyFrame;
+ RELEASE_ASSERT(!emptyFrame.isTailDeleted);
+ return adoptRef(*new DebuggerCallFrame(callFrame, emptyFrame));
+ }
+
</ins><span class="cx"> Vector<ShadowChicken::Frame> frames;
</span><span class="cx"> callFrame->vm().shadowChicken().iterate(callFrame->vm(), callFrame, [&] (const ShadowChicken::Frame& frame) -> bool {
</span><span class="cx"> frames.append(frame);
</span></span></pre>
</div>
</div>
</body>
</html>