<!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>[201207] 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/201207">201207</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-05-19 19:37:53 -0700 (Thu, 19 May 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC] FTL can crash on stack overflow
https://bugs.webkit.org/show_bug.cgi?id=157881
rdar://problem/24665964
Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-05-19
Reviewed by Michael Saboff.
The VM's m_largestFTLStackSize was never set anywhere (updateFTLLargestStackSize()
was never called). We forgot to change that when implementing B3.
Even when it is set, we still have a problem on OSR Exit.
If the last frame is a FTL frame and it OSR Exits, the space required for
that frame becomes significantly larger. What happens is we crash in the OSR Exit
instead of the FTL frame (this is what happens in rdar://problem/24665964).
This patch changes the stack boundary checks in FTL to be the same as DFG:
we verify that we have enough space for the current optimized function but
also for the baseline version (including inlining) in case of exit.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::lower):
(JSC::FTL::DFG::LowerDFGToB3::didOverflowStack): Deleted.
* runtime/VM.cpp:
(JSC::VM::VM): Deleted.
(JSC::VM::updateStackLimit): Deleted.
(JSC::VM::updateFTLLargestStackSize): Deleted.
* runtime/VM.h:
(JSC::VM::addressOfFTLStackLimit): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeVMcpp">trunk/Source/JavaScriptCore/runtime/VM.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeVMh">trunk/Source/JavaScriptCore/runtime/VM.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (201206 => 201207)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-05-20 02:37:53 UTC (rev 201207)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2016-05-19 Benjamin Poulain <bpoulain@apple.com>
+
+ [JSC] FTL can crash on stack overflow
+ https://bugs.webkit.org/show_bug.cgi?id=157881
+ rdar://problem/24665964
+
+ Reviewed by Michael Saboff.
+
+ The VM's m_largestFTLStackSize was never set anywhere (updateFTLLargestStackSize()
+ was never called). We forgot to change that when implementing B3.
+
+ Even when it is set, we still have a problem on OSR Exit.
+ If the last frame is a FTL frame and it OSR Exits, the space required for
+ that frame becomes significantly larger. What happens is we crash in the OSR Exit
+ instead of the FTL frame (this is what happens in rdar://problem/24665964).
+
+ This patch changes the stack boundary checks in FTL to be the same as DFG:
+ we verify that we have enough space for the current optimized function but
+ also for the baseline version (including inlining) in case of exit.
+
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::lower):
+ (JSC::FTL::DFG::LowerDFGToB3::didOverflowStack): Deleted.
+ * runtime/VM.cpp:
+ (JSC::VM::VM): Deleted.
+ (JSC::VM::updateStackLimit): Deleted.
+ (JSC::VM::updateFTLLargestStackSize): Deleted.
+ * runtime/VM.h:
+ (JSC::VM::addressOfFTLStackLimit): Deleted.
+
</ins><span class="cx"> 2016-05-18 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> DFG::LICMPhase shouldn't hoist type checks unless it knows that the check will succeed at the loop pre-header
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (201206 => 201207)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2016-05-20 02:37:53 UTC (rev 201207)
</span><span class="lines">@@ -157,10 +157,7 @@
</span><span class="cx"> m_out.setFrequency(1);
</span><span class="cx">
</span><span class="cx"> m_prologue = m_out.newBlock();
</span><del>- LBasicBlock stackOverflow = m_out.newBlock();
</del><span class="cx"> m_handleExceptions = m_out.newBlock();
</span><del>-
- LBasicBlock checkArguments = m_out.newBlock();
</del><span class="cx">
</span><span class="cx"> for (BlockIndex blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex) {
</span><span class="cx"> m_highBlock = m_graph.block(blockIndex);
</span><span class="lines">@@ -173,7 +170,7 @@
</span><span class="cx"> // Back to prologue frequency for any bocks that get sneakily created in the initialization code.
</span><span class="cx"> m_out.setFrequency(1);
</span><span class="cx">
</span><del>- m_out.appendTo(m_prologue, stackOverflow);
</del><ins>+ m_out.appendTo(m_prologue, m_handleExceptions);
</ins><span class="cx"> m_out.initializeConstants(m_proc, m_prologue);
</span><span class="cx"> createPhiVariables();
</span><span class="cx">
</span><span class="lines">@@ -194,44 +191,54 @@
</span><span class="cx"> m_proc.addFastConstant(m_tagMask->key());
</span><span class="cx">
</span><span class="cx"> m_out.storePtr(m_out.constIntPtr(codeBlock()), addressFor(JSStack::CodeBlock));
</span><del>-
- m_out.branch(
- didOverflowStack(), rarely(stackOverflow), usually(checkArguments));
-
- m_out.appendTo(stackOverflow, m_handleExceptions);
- m_out.call(m_out.voidType, m_out.operation(operationThrowStackOverflowError), m_callFrame, m_out.constIntPtr(codeBlock()));
- m_out.patchpoint(Void)->setGenerator(
- [=] (CCallHelpers& jit, const StackmapGenerationParams&) {
- // We are terminal, so we can clobber everything. That's why we don't claim to
- // clobber scratch.
</del><ins>+
+ // Stack Overflow Check.
+ unsigned exitFrameSize = m_graph.requiredRegisterCountForExit() * sizeof(Register);
+ MacroAssembler::AbsoluteAddress addressOfStackLimit(vm().addressOfStackLimit());
+ PatchpointValue* stackOverflowHandler = m_out.patchpoint(Void);
+ CallSiteIndex callSiteIndex = callSiteIndexForCodeOrigin(m_ftlState, CodeOrigin(0));
+ stackOverflowHandler->appendSomeRegister(m_callFrame);
+ stackOverflowHandler->clobber(RegisterSet::macroScratchRegisters());
+ stackOverflowHandler->numGPScratchRegisters = 1;
+ stackOverflowHandler->setGenerator(
+ [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
</ins><span class="cx"> AllowMacroScratchRegisterUsage allowScratch(jit);
</span><del>-
- jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
- jit.move(CCallHelpers::TrustedImmPtr(jit.vm()), GPRInfo::argumentGPR0);
- jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
- CCallHelpers::Call call = jit.call();
- jit.jumpToExceptionHandler();
</del><ins>+ GPRReg fp = params[0].gpr();
+ GPRReg scratch = params.gpScratch(0);
</ins><span class="cx">
</span><del>- jit.addLinkTask(
- [=] (LinkBuffer& linkBuffer) {
- linkBuffer.link(call, FunctionPtr(lookupExceptionHandlerFromCallerFrame));
</del><ins>+ unsigned ftlFrameSize = params.proc().frameSize();
+
+ jit.addPtr(MacroAssembler::TrustedImm32(-std::max(exitFrameSize, ftlFrameSize)), fp, scratch);
+ MacroAssembler::Jump stackOverflow = jit.branchPtr(MacroAssembler::Above, addressOfStackLimit, scratch);
+
+ params.addLatePath([=] (CCallHelpers& jit) {
+ AllowMacroScratchRegisterUsage allowScratch(jit);
+
+ stackOverflow.link(&jit);
+ jit.store32(
+ MacroAssembler::TrustedImm32(callSiteIndex.bits()),
+ CCallHelpers::tagFor(VirtualRegister(JSStack::ArgumentCount)));
+ jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
+
+ jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+ jit.move(CCallHelpers::TrustedImmPtr(jit.codeBlock()), GPRInfo::argumentGPR1);
+ CCallHelpers::Call throwCall = jit.call();
+
+ jit.move(CCallHelpers::TrustedImmPtr(jit.vm()), GPRInfo::argumentGPR0);
+ jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
+ CCallHelpers::Call lookupExceptionHandlerCall = jit.call();
+ jit.jumpToExceptionHandler();
+
+ jit.addLinkTask(
+ [=] (LinkBuffer& linkBuffer) {
+ linkBuffer.link(throwCall, FunctionPtr(operationThrowStackOverflowError));
+ linkBuffer.link(lookupExceptionHandlerCall, FunctionPtr(lookupExceptionHandlerFromCallerFrame));
</ins><span class="cx"> });
</span><ins>+ });
</ins><span class="cx"> });
</span><del>- m_out.unreachable();
-
- m_out.appendTo(m_handleExceptions, checkArguments);
- Box<CCallHelpers::Label> exceptionHandler = state->exceptionHandler;
- m_out.patchpoint(Void)->setGenerator(
- [=] (CCallHelpers& jit, const StackmapGenerationParams&) {
- CCallHelpers::Jump jump = jit.jump();
- jit.addLinkTask(
- [=] (LinkBuffer& linkBuffer) {
- linkBuffer.link(jump, linkBuffer.locationOf(*exceptionHandler));
- });
- });
- m_out.unreachable();
-
- m_out.appendTo(checkArguments, lowBlock(m_graph.block(0)));
</del><ins>+
+ LBasicBlock firstDFGBasicBlock = lowBlock(m_graph.block(0));
+ // Check Arguments.
</ins><span class="cx"> availabilityMap().clear();
</span><span class="cx"> availabilityMap().m_locals = Operands<Availability>(codeBlock()->numParameters(), 0);
</span><span class="cx"> for (unsigned i = codeBlock()->numParameters(); i--;) {
</span><span class="lines">@@ -273,8 +280,20 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>- m_out.jump(lowBlock(m_graph.block(0)));
-
</del><ins>+ m_out.jump(firstDFGBasicBlock);
+
+ m_out.appendTo(m_handleExceptions, firstDFGBasicBlock);
+ Box<CCallHelpers::Label> exceptionHandler = state->exceptionHandler;
+ m_out.patchpoint(Void)->setGenerator(
+ [=] (CCallHelpers& jit, const StackmapGenerationParams&) {
+ CCallHelpers::Jump jump = jit.jump();
+ jit.addLinkTask(
+ [=] (LinkBuffer& linkBuffer) {
+ linkBuffer.link(jump, linkBuffer.locationOf(*exceptionHandler));
+ });
+ });
+ m_out.unreachable();
+
</ins><span class="cx"> for (DFG::BasicBlock* block : preOrder)
</span><span class="cx"> compileBlock(block);
</span><span class="cx">
</span><span class="lines">@@ -7056,42 +7075,6 @@
</span><span class="cx"> m_out.constInt32(0),
</span><span class="cx"> m_out.address(constructor, m_heaps.RegExpConstructor_cachedResult_reified));
</span><span class="cx"> }
</span><del>-
- LValue didOverflowStack()
- {
- // This does a very simple leaf function analysis. The invariant of FTL call
- // frames is that the caller had already done enough of a stack check to
- // prove that this call frame has enough stack to run, and also enough stack
- // to make runtime calls. So, we only need to stack check when making calls
- // to other JS functions. If we don't find such calls then we don't need to
- // do any stack checks.
-
- for (BlockIndex blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex) {
- DFG::BasicBlock* block = m_graph.block(blockIndex);
- if (!block)
- continue;
-
- for (unsigned nodeIndex = block->size(); nodeIndex--;) {
- Node* node = block->at(nodeIndex);
-
- switch (node->op()) {
- case GetById:
- case PutById:
- case Call:
- case Construct:
- return m_out.below(
- m_callFrame,
- m_out.loadPtr(
- m_out.absolute(vm().addressOfFTLStackLimit())));
-
- default:
- break;
- }
- }
- }
-
- return m_out.booleanFalse;
- }
</del><span class="cx">
</span><span class="cx"> struct ArgumentsLength {
</span><span class="cx"> ArgumentsLength()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/VM.cpp (201206 => 201207)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/VM.cpp 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/JavaScriptCore/runtime/VM.cpp 2016-05-20 02:37:53 UTC (rev 201207)
</span><span class="lines">@@ -188,10 +188,6 @@
</span><span class="cx"> #if !ENABLE(JIT)
</span><span class="cx"> , m_jsStackLimit(0)
</span><span class="cx"> #endif
</span><del>-#if ENABLE(FTL_JIT)
- , m_ftlStackLimit(0)
- , m_largestFTLStackSize(0)
-#endif
</del><span class="cx"> , m_inDefineOwnProperty(false)
</span><span class="cx"> , m_codeCache(std::make_unique<CodeCache>())
</span><span class="cx"> , m_enabledProfiler(nullptr)
</span><span class="lines">@@ -666,19 +662,9 @@
</span><span class="cx"> if (m_stackPointerAtVMEntry) {
</span><span class="cx"> ASSERT(wtfThreadData().stack().isGrowingDownward());
</span><span class="cx"> char* startOfStack = reinterpret_cast<char*>(m_stackPointerAtVMEntry);
</span><del>-#if ENABLE(FTL_JIT)
- m_stackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_reservedZoneSize + m_largestFTLStackSize);
- m_ftlStackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_reservedZoneSize + 2 * m_largestFTLStackSize);
-#else
</del><span class="cx"> m_stackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_reservedZoneSize);
</span><del>-#endif
</del><span class="cx"> } else {
</span><del>-#if ENABLE(FTL_JIT)
- m_stackLimit = wtfThreadData().stack().recursionLimit(m_reservedZoneSize + m_largestFTLStackSize);
- m_ftlStackLimit = wtfThreadData().stack().recursionLimit(m_reservedZoneSize + 2 * m_largestFTLStackSize);
-#else
</del><span class="cx"> m_stackLimit = wtfThreadData().stack().recursionLimit(m_reservedZoneSize);
</span><del>-#endif
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if PLATFORM(WIN)
</span><span class="lines">@@ -687,16 +673,6 @@
</span><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#if ENABLE(FTL_JIT)
-void VM::updateFTLLargestStackSize(size_t stackSize)
-{
- if (stackSize > m_largestFTLStackSize) {
- m_largestFTLStackSize = stackSize;
- updateStackLimit();
- }
-}
-#endif
-
</del><span class="cx"> #if ENABLE(DFG_JIT)
</span><span class="cx"> void VM::gatherConservativeRoots(ConservativeRoots& conservativeRoots)
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeVMh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/VM.h (201206 => 201207)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/VM.h 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/JavaScriptCore/runtime/VM.h 2016-05-20 02:37:53 UTC (rev 201207)
</span><span class="lines">@@ -445,11 +445,6 @@
</span><span class="cx"> size_t reservedZoneSize() const { return m_reservedZoneSize; }
</span><span class="cx"> size_t updateReservedZoneSize(size_t reservedZoneSize);
</span><span class="cx">
</span><del>-#if ENABLE(FTL_JIT)
- void updateFTLLargestStackSize(size_t);
- void** addressOfFTLStackLimit() { return &m_ftlStackLimit; }
-#endif
-
</del><span class="cx"> #if !ENABLE(JIT)
</span><span class="cx"> void* jsStackLimit() { return m_jsStackLimit; }
</span><span class="cx"> void setJSStackLimit(void* limit) { m_jsStackLimit = limit; }
</span><span class="lines">@@ -644,11 +639,7 @@
</span><span class="cx"> void* m_stackLimit;
</span><span class="cx"> void* m_jsStackLimit;
</span><span class="cx"> };
</span><del>-#if ENABLE(FTL_JIT)
- void* m_ftlStackLimit;
- size_t m_largestFTLStackSize;
</del><span class="cx"> #endif
</span><del>-#endif
</del><span class="cx"> void* m_lastStackTop;
</span><span class="cx"> Exception* m_exception { nullptr };
</span><span class="cx"> Exception* m_lastException { nullptr };
</span></span></pre>
</div>
</div>
</body>
</html>