<!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>[180060] 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/180060">180060</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2015-02-13 10:57:57 -0800 (Fri, 13 Feb 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Google doc spreadsheet reproducibly crashes when sorting
https://bugs.webkit.org/show_bug.cgi?id=141098

Reviewed by Oliver Hunt.

Source/JavaScriptCore:

Moved the stack check to before the callee registers are allocated in the
prologue() by movving it from the functionInitialization() macro.  This
way we can check the stack before moving the stack pointer, avoiding a
crash during a &quot;call&quot; instruction.  Before this change, we weren't even
checking the stack for program and eval execution.

Made a couple of supporting changes.

* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::llint_stack_check): We can't just go up one frame as we
may be processing an exception to an entry frame.

* llint/LowLevelInterpreter.asm:

* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
(llint_throw_from_slow_path_trampoline): Changed method to get the vm
from the code block to not use the codeBlock, since we may need to
continue from an exception in a native function.

LayoutTests:

New test.

* js/regress-141098-expected.txt: Added.
* js/regress-141098.html: Added.
* js/script-tests/regress-141098.js: Added.
(probeAndRecurse):</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="#trunkSourceJavaScriptCorellintLLIntSlowPathscpp">trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLowLevelInterpreterasm">trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLowLevelInterpreter32_64asm">trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLowLevelInterpreter64asm">trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsjsregress141098expectedtxt">trunk/LayoutTests/js/regress-141098-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsregress141098html">trunk/LayoutTests/js/regress-141098.html</a></li>
<li><a href="#trunkLayoutTestsjsscripttestsregress141098js">trunk/LayoutTests/js/script-tests/regress-141098.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (180059 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/LayoutTests/ChangeLog        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2015-02-13  Michael Saboff  &lt;msaboff@apple.com&gt;
+
+        Google doc spreadsheet reproducibly crashes when sorting
+        https://bugs.webkit.org/show_bug.cgi?id=141098
+
+        Reviewed by Oliver Hunt.
+
+        New test.
+
+        * js/regress-141098-expected.txt: Added.
+        * js/regress-141098.html: Added.
+        * js/script-tests/regress-141098.js: Added.
+        (probeAndRecurse):
+
</ins><span class="cx"> 2015-02-13  ChangSeok Oh  &lt;changseok.oh@collabora.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Div having contentEditable and display:grid cannot be edited if it is empty.
</span></span></pre></div>
<a id="trunkLayoutTestsjsregress141098expectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress-141098-expected.txt (0 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/regress-141098-expected.txt                                (rev 0)
+++ trunk/LayoutTests/js/regress-141098-expected.txt        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -0,0 +1,9 @@
</span><ins>+Regression test for https://webkit.org/b/141098. Make sure eval() properly handles running out of stack space. This test should run without crashing.
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsregress141098html"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/regress-141098.html (0 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/regress-141098.html                                (rev 0)
+++ trunk/LayoutTests/js/regress-141098.html        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+&lt;!DOCTYPE HTML PUBLIC &quot;-//IETF//DTD HTML//EN&quot;&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script src=&quot;../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;script src=&quot;script-tests/regress-141098.js&quot;&gt;&lt;/script&gt;
+&lt;script src=&quot;../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsjsscripttestsregress141098js"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/script-tests/regress-141098.js (0 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/script-tests/regress-141098.js                                (rev 0)
+++ trunk/LayoutTests/js/script-tests/regress-141098.js        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -0,0 +1,46 @@
</span><ins>+description(&quot;Regression test for https://webkit.org/b/141098. Make sure eval() properly handles running out of stack space. This test should run without crashing.&quot;);
+
+function probeAndRecurse(depth)
+{
+    var result;
+
+    // Probe stack depth
+    try {
+        result = probeAndRecurse(depth+1);
+        if (result &lt; 0)
+            return result + 1;
+        else if (result &gt; 0)
+            return result;
+    } catch (e) {
+        // Go up a many frames and then create an expression to eval that will consume the stack using
+        // callee registers.
+        return -60;
+    }
+
+    try {
+        var count = 1;
+
+        for (var i = 0; i &lt; 40; count *= 10, i++) {
+            evalStringPrefix = &quot;{ var first = &quot; + count + &quot;; &quot;;
+            var evalStringBody = &quot;&quot;;
+
+            for (var varIndex = 0; varIndex &lt; count; varIndex++)
+                evalStringBody += &quot;var s&quot; + varIndex + &quot; = &quot; + varIndex + &quot;;&quot;;
+
+            evalStringBody += &quot;var value = [&quot;;
+            for (var varIndex = 0; varIndex &lt; count; varIndex++) {
+                if (varIndex &gt; 0)
+                    evalStringBody += &quot;, &quot;;
+                evalStringBody += &quot;s&quot; + varIndex;
+            }
+            evalStringBody +=  &quot;]; &quot;;
+
+           var evalResult = eval(&quot;{&quot; + evalStringBody + &quot;}&quot;);
+        }
+    } catch (e) {
+    }
+
+    return 1;
+}
+
+probeAndRecurse(0);
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (180059 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2015-02-13  Michael Saboff  &lt;msaboff@apple.com&gt;
+
+        Google doc spreadsheet reproducibly crashes when sorting
+        https://bugs.webkit.org/show_bug.cgi?id=141098
+
+        Reviewed by Oliver Hunt.
+
+        Moved the stack check to before the callee registers are allocated in the
+        prologue() by movving it from the functionInitialization() macro.  This
+        way we can check the stack before moving the stack pointer, avoiding a
+        crash during a &quot;call&quot; instruction.  Before this change, we weren't even
+        checking the stack for program and eval execution.
+
+        Made a couple of supporting changes.
+
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::llint_stack_check): We can't just go up one frame as we
+        may be processing an exception to an entry frame.
+
+        * llint/LowLevelInterpreter.asm:
+
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        (llint_throw_from_slow_path_trampoline): Changed method to get the vm
+        from the code block to not use the codeBlock, since we may need to
+        continue from an exception in a native function.
+
</ins><span class="cx"> 2015-02-12  Benjamin Poulain  &lt;benjamin@webkit.org&gt;
</span><span class="cx"> 
</span><span class="cx">         Simplify the initialization of BytecodeGenerator a bit
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLLIntSlowPathscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp (180059 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp        2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -490,7 +490,7 @@
</span><span class="cx">         LLINT_RETURN_TWO(pc, 0);
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-    exec = exec-&gt;callerFrame();
</del><ins>+    exec = exec-&gt;callerFrame(vm.topVMEntryFrame);
</ins><span class="cx">     vm.topCallFrame = exec;
</span><span class="cx">     ErrorHandlingScope errorScope(vm);
</span><span class="cx">     CommonSlowPaths::interpreterThrowInCaller(exec, createStackOverflowError(exec));
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLowLevelInterpreterasm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm (180059 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm        2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -445,21 +445,21 @@
</span><span class="cx">     subp entryFramePointer, VMEntryTotalFrameSize, resultReg
</span><span class="cx"> end
</span><span class="cx"> 
</span><del>-macro moveStackPointerForCodeBlock(codeBlock, scratch)
-    loadi CodeBlock::m_numCalleeRegisters[codeBlock], scratch
-    lshiftp 3, scratch
-    addp maxFrameExtentForSlowPathCall, scratch
-    if ARMv7
-        subp cfr, scratch, scratch
-        move scratch, sp
-    else
-        subp cfr, scratch, sp
-    end
</del><ins>+macro getFrameRegisterSizeForCodeBlock(codeBlock, size)
+    loadi CodeBlock::m_numCalleeRegisters[codeBlock], size
+    lshiftp 3, size
+    addp maxFrameExtentForSlowPathCall, size
</ins><span class="cx"> end
</span><span class="cx"> 
</span><span class="cx"> macro restoreStackPointerAfterCall()
</span><span class="cx">     loadp CodeBlock[cfr], t2
</span><del>-    moveStackPointerForCodeBlock(t2, t4)
</del><ins>+    getFrameRegisterSizeForCodeBlock(t2, t4)
+    if ARMv7
+        subp cfr, t4, t4
+        move t4, sp
+    else
+        subp cfr, t4, sp
+    end
</ins><span class="cx"> end
</span><span class="cx"> 
</span><span class="cx"> macro traceExecution()
</span><span class="lines">@@ -606,8 +606,6 @@
</span><span class="cx">     end
</span><span class="cx"> 
</span><span class="cx">     codeBlockSetter(t1)
</span><del>-    
-    moveStackPointerForCodeBlock(t1, t2)
</del><span class="cx"> 
</span><span class="cx">     # Set up the PC.
</span><span class="cx">     if JSVALUE64
</span><span class="lines">@@ -616,6 +614,29 @@
</span><span class="cx">     else
</span><span class="cx">         loadp CodeBlock::m_instructions[t1], PC
</span><span class="cx">     end
</span><ins>+
+    # Get new sp in t0 and check stack height.
+    getFrameRegisterSizeForCodeBlock(t1, t0)
+    subp cfr, t0, t0
+    loadp CodeBlock::m_vm[t1], t2
+    bpbeq VM::m_jsStackLimit[t2], t0, .stackHeightOK
+
+    # Stack height check failed - need to call a slow_path.
+    subp maxFrameExtentForSlowPathCall, sp # Set up temporary stack pointer for call
+    callSlowPath(_llint_stack_check)
+    bpeq t1, 0, .stackHeightOKGetCodeBlock
+    move t1, cfr
+    dispatch(0) # Go to exception handler in PC
+
+.stackHeightOKGetCodeBlock:
+    # Stack check slow path returned that the stack was ok.
+    # Since they were clobbered, need to get CodeBlock and new sp
+    codeBlockSetter(t1)
+    getFrameRegisterSizeForCodeBlock(t1, t0)
+    subp cfr, t0, t0
+
+.stackHeightOK:
+    move t0, sp
</ins><span class="cx"> end
</span><span class="cx"> 
</span><span class="cx"> # Expects that CodeBlock is in t1, which is what prologue() leaves behind.
</span><span class="lines">@@ -650,20 +671,6 @@
</span><span class="cx">     end
</span><span class="cx">     baddpnz -8, t0, .argumentProfileLoop
</span><span class="cx"> .argumentProfileDone:
</span><del>-        
-    # Check stack height.
-    loadi CodeBlock::m_numCalleeRegisters[t1], t0
-    loadp CodeBlock::m_vm[t1], t2
-    lshiftp 3, t0
-    addi maxFrameExtentForSlowPathCall, t0
-    subp cfr, t0, t0
-    bpbeq VM::m_jsStackLimit[t2], t0, .stackHeightOK
-
-    # Stack height check failed - need to call a slow_path.
-    callSlowPath(_llint_stack_check)
-    bpeq t1, 0, .stackHeightOK
-    move t1, cfr
-.stackHeightOK:
</del><span class="cx"> end
</span><span class="cx"> 
</span><span class="cx"> macro allocateJSObject(allocator, structure, result, scratch1, slowCase)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLowLevelInterpreter32_64asm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm (180059 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm        2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -2028,8 +2028,9 @@
</span><span class="cx">     # When throwing from the interpreter (i.e. throwing from LLIntSlowPaths), so
</span><span class="cx">     # the throw target is not necessarily interpreted code, we come to here.
</span><span class="cx">     # This essentially emulates the JIT's throwing protocol.
</span><del>-    loadp CodeBlock[cfr], t1
-    loadp CodeBlock::m_vm[t1], t1
</del><ins>+    loadp Callee[cfr], t1
+    andp MarkedBlockMask, t1
+    loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t1], t1
</ins><span class="cx">     jmp VM::targetMachinePCForThrow[t1]
</span><span class="cx"> 
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLowLevelInterpreter64asm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm (180059 => 180060)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm        2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm        2015-02-13 18:57:57 UTC (rev 180060)
</span><span class="lines">@@ -1886,8 +1886,9 @@
</span><span class="cx">     # When throwing from the interpreter (i.e. throwing from LLIntSlowPaths), so
</span><span class="cx">     # the throw target is not necessarily interpreted code, we come to here.
</span><span class="cx">     # This essentially emulates the JIT's throwing protocol.
</span><del>-    loadp CodeBlock[cfr], t1
-    loadp CodeBlock::m_vm[t1], t1
</del><ins>+    loadp Callee[cfr], t1
+    andp MarkedBlockMask, t1
+    loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t1], t1
</ins><span class="cx">     jmp VM::targetMachinePCForThrow[t1]
</span><span class="cx"> 
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>