Modified: trunk/Source/_javascript_Core/ChangeLog (201206 => 201207)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-20 02:37:53 UTC (rev 201207)
@@ -1,3 +1,33 @@
+2016-05-19 Benjamin Poulain <[email protected]>
+
+ [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.
+
2016-05-18 Filip Pizlo <[email protected]>
DFG::LICMPhase shouldn't hoist type checks unless it knows that the check will succeed at the loop pre-header
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (201206 => 201207)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-05-20 02:37:53 UTC (rev 201207)
@@ -157,10 +157,7 @@
m_out.setFrequency(1);
m_prologue = m_out.newBlock();
- LBasicBlock stackOverflow = m_out.newBlock();
m_handleExceptions = m_out.newBlock();
-
- LBasicBlock checkArguments = m_out.newBlock();
for (BlockIndex blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex) {
m_highBlock = m_graph.block(blockIndex);
@@ -173,7 +170,7 @@
// Back to prologue frequency for any bocks that get sneakily created in the initialization code.
m_out.setFrequency(1);
- m_out.appendTo(m_prologue, stackOverflow);
+ m_out.appendTo(m_prologue, m_handleExceptions);
m_out.initializeConstants(m_proc, m_prologue);
createPhiVariables();
@@ -194,44 +191,54 @@
m_proc.addFastConstant(m_tagMask->key());
m_out.storePtr(m_out.constIntPtr(codeBlock()), addressFor(JSStack::CodeBlock));
-
- 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.
+
+ // 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) {
AllowMacroScratchRegisterUsage allowScratch(jit);
-
- jit.copyCalleeSavesToVMEntryFrameCalleeSavesBuffer();
- jit.move(CCallHelpers::TrustedImmPtr(jit.vm()), GPRInfo::argumentGPR0);
- jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
- CCallHelpers::Call call = jit.call();
- jit.jumpToExceptionHandler();
+ GPRReg fp = params[0].gpr();
+ GPRReg scratch = params.gpScratch(0);
- jit.addLinkTask(
- [=] (LinkBuffer& linkBuffer) {
- linkBuffer.link(call, FunctionPtr(lookupExceptionHandlerFromCallerFrame));
+ 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));
});
+ });
});
- 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)));
+
+ LBasicBlock firstDFGBasicBlock = lowBlock(m_graph.block(0));
+ // Check Arguments.
availabilityMap().clear();
availabilityMap().m_locals = Operands<Availability>(codeBlock()->numParameters(), 0);
for (unsigned i = codeBlock()->numParameters(); i--;) {
@@ -273,8 +280,20 @@
break;
}
}
- m_out.jump(lowBlock(m_graph.block(0)));
-
+ 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();
+
for (DFG::BasicBlock* block : preOrder)
compileBlock(block);
@@ -7056,42 +7075,6 @@
m_out.constInt32(0),
m_out.address(constructor, m_heaps.RegExpConstructor_cachedResult_reified));
}
-
- 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;
- }
struct ArgumentsLength {
ArgumentsLength()
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (201206 => 201207)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2016-05-20 02:37:53 UTC (rev 201207)
@@ -188,10 +188,6 @@
#if !ENABLE(JIT)
, m_jsStackLimit(0)
#endif
-#if ENABLE(FTL_JIT)
- , m_ftlStackLimit(0)
- , m_largestFTLStackSize(0)
-#endif
, m_inDefineOwnProperty(false)
, m_codeCache(std::make_unique<CodeCache>())
, m_enabledProfiler(nullptr)
@@ -666,19 +662,9 @@
if (m_stackPointerAtVMEntry) {
ASSERT(wtfThreadData().stack().isGrowingDownward());
char* startOfStack = reinterpret_cast<char*>(m_stackPointerAtVMEntry);
-#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
m_stackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_reservedZoneSize);
-#endif
} else {
-#if ENABLE(FTL_JIT)
- m_stackLimit = wtfThreadData().stack().recursionLimit(m_reservedZoneSize + m_largestFTLStackSize);
- m_ftlStackLimit = wtfThreadData().stack().recursionLimit(m_reservedZoneSize + 2 * m_largestFTLStackSize);
-#else
m_stackLimit = wtfThreadData().stack().recursionLimit(m_reservedZoneSize);
-#endif
}
#if PLATFORM(WIN)
@@ -687,16 +673,6 @@
#endif
}
-#if ENABLE(FTL_JIT)
-void VM::updateFTLLargestStackSize(size_t stackSize)
-{
- if (stackSize > m_largestFTLStackSize) {
- m_largestFTLStackSize = stackSize;
- updateStackLimit();
- }
-}
-#endif
-
#if ENABLE(DFG_JIT)
void VM::gatherConservativeRoots(ConservativeRoots& conservativeRoots)
{
Modified: trunk/Source/_javascript_Core/runtime/VM.h (201206 => 201207)
--- trunk/Source/_javascript_Core/runtime/VM.h 2016-05-20 02:25:51 UTC (rev 201206)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2016-05-20 02:37:53 UTC (rev 201207)
@@ -445,11 +445,6 @@
size_t reservedZoneSize() const { return m_reservedZoneSize; }
size_t updateReservedZoneSize(size_t reservedZoneSize);
-#if ENABLE(FTL_JIT)
- void updateFTLLargestStackSize(size_t);
- void** addressOfFTLStackLimit() { return &m_ftlStackLimit; }
-#endif
-
#if !ENABLE(JIT)
void* jsStackLimit() { return m_jsStackLimit; }
void setJSStackLimit(void* limit) { m_jsStackLimit = limit; }
@@ -644,11 +639,7 @@
void* m_stackLimit;
void* m_jsStackLimit;
};
-#if ENABLE(FTL_JIT)
- void* m_ftlStackLimit;
- size_t m_largestFTLStackSize;
#endif
-#endif
void* m_lastStackTop;
Exception* m_exception { nullptr };
Exception* m_lastException { nullptr };
