<!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>[173282] trunk/Source/JavaScriptCore</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/173282">173282</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2014-09-04 14:23:38 -0700 (Thu, 04 Sep 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/173031">r173031</a>): crashes during run-layout-jsc on x86/Linux
https://bugs.webkit.org/show_bug.cgi?id=136436
Reviewed by Geoffrey Garen.
Instead of trying to calculate a stack pointer that allows for possible
stacked argument space, just use the "home" stack pointer location.
That stack pointer provides space for the worst case number of stacked
arguments on architectures that use stacked arguments. It also provides
stack space so that the return PC and caller frame pointer that are stored
as part of making the call to operationCallEval will not override any part
of the callee frame created on the stack.
Changed compileCallEval() to use the stackPointer value of the calling
function. That stack pointer is calculated to have enough space for
outgoing stacked arguments. By moving the stack pointer to its "home"
position, the caller frame and return PC are not set as part of making
the call to operationCallEval(). Moved the explicit setting of the
callerFrame field of the callee CallFrame from operationCallEval() to
compileCallEval() since it has been the artifact of making a call for
most architectures. Simplified the exception logic in compileCallEval()
as a result of the change. To be compliant with the stack state
expected by virtualCallThunkGenerator(), moved the stack pointer to
point above the CallerFrameAndPC of the callee CallFrame.
* jit/JIT.h: Changed callOperationNoExceptionCheck(J_JITOperation_EE, ...)
to callOperation(J_JITOperation_EE, ...) as it now can do a typical exception
check.
* jit/JITCall.cpp & jit/JITCall32_64.cpp:
(JSC::JIT::compileCallEval): Use the home stack pointer when making the call
to operationCallEval. Since the stack pointer adjustment no longer needs
to be done after making the call to operationCallEval(), the exception check
logic can be simplified.
(JSC::JIT::compileCallEvalSlowCase): Restored the stack pointer to point
to above the calleeFrame as this is what the generated thunk expects.
* jit/JITInlines.h:
(JSC::JIT::callOperation): Refactor of callOperationNoExceptionCheck
with the addition of a standard exception check.
(JSC::JIT::callOperationNoExceptionCheck): Deleted.
* jit/JITOperations.cpp:
(JSC::operationCallEval): Eliminated the explicit setting of caller frame
as that is now done in the code generated by compileCallEval().</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITh">trunk/Source/JavaScriptCore/jit/JIT.h</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITCallcpp">trunk/Source/JavaScriptCore/jit/JITCall.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITCall32_64cpp">trunk/Source/JavaScriptCore/jit/JITCall32_64.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITInlinesh">trunk/Source/JavaScriptCore/jit/JITInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITOperationscpp">trunk/Source/JavaScriptCore/jit/JITOperations.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (173281 => 173282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-09-04 21:23:38 UTC (rev 173282)
</span><span class="lines">@@ -1,3 +1,48 @@
</span><ins>+2014-09-04 Michael Saboff <msaboff@apple.com>
+
+ REGRESSION(r173031): crashes during run-layout-jsc on x86/Linux
+ https://bugs.webkit.org/show_bug.cgi?id=136436
+
+ Reviewed by Geoffrey Garen.
+
+ Instead of trying to calculate a stack pointer that allows for possible
+ stacked argument space, just use the "home" stack pointer location.
+ That stack pointer provides space for the worst case number of stacked
+ arguments on architectures that use stacked arguments. It also provides
+ stack space so that the return PC and caller frame pointer that are stored
+ as part of making the call to operationCallEval will not override any part
+ of the callee frame created on the stack.
+
+ Changed compileCallEval() to use the stackPointer value of the calling
+ function. That stack pointer is calculated to have enough space for
+ outgoing stacked arguments. By moving the stack pointer to its "home"
+ position, the caller frame and return PC are not set as part of making
+ the call to operationCallEval(). Moved the explicit setting of the
+ callerFrame field of the callee CallFrame from operationCallEval() to
+ compileCallEval() since it has been the artifact of making a call for
+ most architectures. Simplified the exception logic in compileCallEval()
+ as a result of the change. To be compliant with the stack state
+ expected by virtualCallThunkGenerator(), moved the stack pointer to
+ point above the CallerFrameAndPC of the callee CallFrame.
+
+ * jit/JIT.h: Changed callOperationNoExceptionCheck(J_JITOperation_EE, ...)
+ to callOperation(J_JITOperation_EE, ...) as it now can do a typical exception
+ check.
+ * jit/JITCall.cpp & jit/JITCall32_64.cpp:
+ (JSC::JIT::compileCallEval): Use the home stack pointer when making the call
+ to operationCallEval. Since the stack pointer adjustment no longer needs
+ to be done after making the call to operationCallEval(), the exception check
+ logic can be simplified.
+ (JSC::JIT::compileCallEvalSlowCase): Restored the stack pointer to point
+ to above the calleeFrame as this is what the generated thunk expects.
+ * jit/JITInlines.h:
+ (JSC::JIT::callOperation): Refactor of callOperationNoExceptionCheck
+ with the addition of a standard exception check.
+ (JSC::JIT::callOperationNoExceptionCheck): Deleted.
+ * jit/JITOperations.cpp:
+ (JSC::operationCallEval): Eliminated the explicit setting of caller frame
+ as that is now done in the code generated by compileCallEval().
+
</ins><span class="cx"> 2014-09-03 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Beef up the DFG's CFG analyses to include iterated dominance frontiers and more user-friendly BlockSets
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JIT.h (173281 => 173282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JIT.h 2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/JavaScriptCore/jit/JIT.h 2014-09-04 21:23:38 UTC (rev 173282)
</span><span class="lines">@@ -718,6 +718,7 @@
</span><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_EC, RegisterID);
</span><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_ECC, RegisterID, RegisterID);
</span><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_ECICC, RegisterID, const Identifier*, RegisterID, RegisterID);
</span><ins>+ MacroAssembler::Call callOperation(J_JITOperation_EE, RegisterID);
</ins><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_EIdJZ, const Identifier*, RegisterID, int32_t);
</span><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_EJ, RegisterID);
</span><span class="cx"> #if USE(JSVALUE64)
</span><span class="lines">@@ -738,7 +739,6 @@
</span><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_EPc, Instruction*);
</span><span class="cx"> MacroAssembler::Call callOperation(V_JITOperation_EZ, int32_t);
</span><span class="cx"> MacroAssembler::Call callOperationWithCallFrameRollbackOnException(J_JITOperation_E);
</span><del>- MacroAssembler::Call callOperationNoExceptionCheck(J_JITOperation_EE, RegisterID);
</del><span class="cx"> MacroAssembler::Call callOperationWithCallFrameRollbackOnException(V_JITOperation_ECb, CodeBlock*);
</span><span class="cx"> MacroAssembler::Call callOperationWithCallFrameRollbackOnException(Z_JITOperation_E);
</span><span class="cx"> #if USE(JSVALUE32_64)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITCallcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITCall.cpp (173281 => 173282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITCall.cpp 2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/JavaScriptCore/jit/JITCall.cpp 2014-09-04 21:23:38 UTC (rev 173282)
</span><span class="lines">@@ -136,18 +136,15 @@
</span><span class="cx"> void JIT::compileCallEval(Instruction* instruction)
</span><span class="cx"> {
</span><span class="cx"> addPtr(TrustedImm32(-static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC))), stackPointerRegister, regT1);
</span><del>- callOperationNoExceptionCheck(operationCallEval, regT1);
</del><ins>+ storePtr(callFrameRegister, Address(regT1, CallFrame::callerFrameOffset()));
</ins><span class="cx">
</span><del>- Jump noException = emitExceptionCheck(InvertedExceptionCheck);
- addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
- exceptionCheck(jump());
</del><ins>+ addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
+ checkStackPointerAlignment();
</ins><span class="cx">
</span><del>- noException.link(this);
</del><ins>+ callOperation(operationCallEval, regT1);
+
</ins><span class="cx"> addSlowCase(branch64(Equal, regT0, TrustedImm64(JSValue::encode(JSValue()))));
</span><span class="cx">
</span><del>- addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
- checkStackPointerAlignment();
-
</del><span class="cx"> sampleCodeBlock(m_codeBlock);
</span><span class="cx">
</span><span class="cx"> emitPutCallResult(instruction);
</span><span class="lines">@@ -156,7 +153,10 @@
</span><span class="cx"> void JIT::compileCallEvalSlowCase(Instruction* instruction, Vector<SlowCaseEntry>::iterator& iter)
</span><span class="cx"> {
</span><span class="cx"> linkSlowCase(iter);
</span><ins>+ int registerOffset = -instruction[4].u.operand;
</ins><span class="cx">
</span><ins>+ addPtr(TrustedImm32(registerOffset * sizeof(Register) + sizeof(CallerFrameAndPC)), callFrameRegister, stackPointerRegister);
+
</ins><span class="cx"> load64(Address(stackPointerRegister, sizeof(Register) * JSStack::Callee - sizeof(CallerFrameAndPC)), regT0);
</span><span class="cx"> move(TrustedImmPtr(&CallLinkInfo::dummy()), regT2);
</span><span class="cx"> emitNakedCall(m_vm->getCTIStub(virtualCallThunkGenerator).code());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITCall32_64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITCall32_64.cpp (173281 => 173282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITCall32_64.cpp 2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/JavaScriptCore/jit/JITCall32_64.cpp 2014-09-04 21:23:38 UTC (rev 173282)
</span><span class="lines">@@ -220,19 +220,14 @@
</span><span class="cx"> void JIT::compileCallEval(Instruction* instruction)
</span><span class="cx"> {
</span><span class="cx"> addPtr(TrustedImm32(-static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC))), stackPointerRegister, regT1);
</span><ins>+ storePtr(callFrameRegister, Address(regT1, CallFrame::callerFrameOffset()));
</ins><span class="cx">
</span><del>- callOperationNoExceptionCheck(operationCallEval, regT1);
-
- Jump noException = emitExceptionCheck(InvertedExceptionCheck);
</del><span class="cx"> addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
</span><del>- exceptionCheck(jump());
</del><span class="cx">
</span><del>- noException.link(this);
</del><ins>+ callOperation(operationCallEval, regT1);
+
</ins><span class="cx"> addSlowCase(branch32(Equal, regT1, TrustedImm32(JSValue::EmptyValueTag)));
</span><span class="cx">
</span><del>- addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
- checkStackPointerAlignment();
-
</del><span class="cx"> sampleCodeBlock(m_codeBlock);
</span><span class="cx">
</span><span class="cx"> emitPutCallResult(instruction);
</span><span class="lines">@@ -242,6 +237,10 @@
</span><span class="cx"> {
</span><span class="cx"> linkSlowCase(iter);
</span><span class="cx">
</span><ins>+ int registerOffset = -instruction[4].u.operand;
+
+ addPtr(TrustedImm32(registerOffset * sizeof(Register) + sizeof(CallerFrameAndPC)), callFrameRegister, stackPointerRegister);
+
</ins><span class="cx"> loadPtr(Address(stackPointerRegister, sizeof(Register) * JSStack::Callee - sizeof(CallerFrameAndPC)), regT0);
</span><span class="cx"> loadPtr(Address(stackPointerRegister, sizeof(Register) * JSStack::Callee - sizeof(CallerFrameAndPC)), regT1);
</span><span class="cx"> move(TrustedImmPtr(&CallLinkInfo::dummy()), regT2);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITInlines.h (173281 => 173282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITInlines.h 2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/JavaScriptCore/jit/JITInlines.h 2014-09-04 21:23:38 UTC (rev 173282)
</span><span class="lines">@@ -317,6 +317,13 @@
</span><span class="cx"> return appendCallWithExceptionCheck(operation);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(J_JITOperation_EE operation, RegisterID regOp)
+{
+ setupArgumentsWithExecState(regOp);
+ updateTopCallFrame();
+ return appendCallWithExceptionCheck(operation);
+}
+
</ins><span class="cx"> ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(V_JITOperation_EPc operation, Instruction* bytecodePC)
</span><span class="cx"> {
</span><span class="cx"> setupArgumentsWithExecState(TrustedImmPtr(bytecodePC));
</span><span class="lines">@@ -335,13 +342,6 @@
</span><span class="cx"> return appendCallWithCallFrameRollbackOnException(operation);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-ALWAYS_INLINE MacroAssembler::Call JIT::callOperationNoExceptionCheck(J_JITOperation_EE operation, RegisterID regOp)
-{
- setupArgumentsWithExecState(regOp);
- updateTopCallFrame();
- return appendCall(operation);
-}
-
</del><span class="cx"> ALWAYS_INLINE MacroAssembler::Call JIT::callOperationWithCallFrameRollbackOnException(V_JITOperation_ECb operation, CodeBlock* pointer)
</span><span class="cx"> {
</span><span class="cx"> setupArgumentsWithExecState(TrustedImmPtr(pointer));
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITOperationscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITOperations.cpp (173281 => 173282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITOperations.cpp 2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/JavaScriptCore/jit/JITOperations.cpp 2014-09-04 21:23:38 UTC (rev 173282)
</span><span class="lines">@@ -612,7 +612,6 @@
</span><span class="cx">
</span><span class="cx"> execCallee->setScope(exec->scope());
</span><span class="cx"> execCallee->setCodeBlock(0);
</span><del>- execCallee->setCallerFrame(exec);
</del><span class="cx">
</span><span class="cx"> if (!isHostFunction(execCallee->calleeAsValue(), globalFuncEval))
</span><span class="cx"> return JSValue::encode(JSValue());
</span></span></pre>
</div>
</div>
</body>
</html>