Diff
Modified: branches/jsCStack/Source/_javascript_Core/ChangeLog (160957 => 160958)
--- branches/jsCStack/Source/_javascript_Core/ChangeLog 2013-12-21 03:50:12 UTC (rev 160957)
+++ branches/jsCStack/Source/_javascript_Core/ChangeLog 2013-12-21 04:58:15 UTC (rev 160958)
@@ -1,5 +1,23 @@
2013-12-20 Filip Pizlo <[email protected]>
+ FTL OSR exit should be able to handle the arity check fail case
+ https://bugs.webkit.org/show_bug.cgi?id=126111
+
+ Not yet reviewed.
+
+ * ftl/FTLOSRExitCompiler.cpp:
+ (JSC::FTL::compileStub):
+ * jit/RegisterPreservationWrapperGenerator.cpp:
+ (JSC::generateRegisterRestoration):
+ * tests/stress: Added.
+ * tests/stress/exit-from-ftl-with-arity-check-fail.js: Added.
+ (foo):
+ (bar):
+ * tests/stress/repeated-arity-check-fail.js: Added.
+ (bar):
+
+2013-12-20 Filip Pizlo <[email protected]>
+
Arity check stack restoration should preserve the ArgumentCount in case there is a register restoration thunk below it
https://bugs.webkit.org/show_bug.cgi?id=126106
Modified: branches/jsCStack/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp (160957 => 160958)
--- branches/jsCStack/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2013-12-21 03:50:12 UTC (rev 160957)
+++ branches/jsCStack/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2013-12-21 04:58:15 UTC (rev 160958)
@@ -162,7 +162,44 @@
}
jit.load32(CCallHelpers::payloadFor(JSStack::ArgumentCount), GPRInfo::regT2);
+
+ // Let's say that the FTL function had failed its arity check. In that case, the stack will
+ // contain some extra stuff.
+ //
+ // First we compute the padded stack space:
+ //
+ // paddedStackSpace = roundUp(codeBlock->numParameters - regT2 + 1)
+ //
+ // The stack will have regT2 + CallFrameHeaderSize stuff, but above it there will be
+ // paddedStackSpace gunk used by the arity check fail restoration thunk. When that happens
+ // we want to make the stack look like this, from higher addresses down:
+ //
+ // - register preservation return PC
+ // - preserved registers
+ // - arity check fail return PC
+ // - argument padding
+ // - actual arguments
+ // - call frame header
+ //
+ // So that the actual call frame header appears to return to the arity check fail return
+ // PC, and that then returns to the register preservation thunk. The arity check thunk that
+ // we return to will have the padding size encoded into it. It will then know to return
+ // into the register preservation thunk, which uses the argument count to figure out where
+ // registers are preserved.
+ // This code assumes that we're dealing with FunctionCode.
+ RELEASE_ASSERT(codeBlock->codeType() == FunctionCode);
+
+ jit.add32(
+ MacroAssembler::TrustedImm32(-codeBlock->numParameters()), GPRInfo::regT2,
+ GPRInfo::regT3);
+ MacroAssembler::Jump arityIntact = jit.branchTest32(MacroAssembler::Zero, GPRInfo::regT3);
+ jit.neg32(GPRInfo::regT3);
+ jit.add32(MacroAssembler::TrustedImm32(1 + stackAlignmentRegisters() - 1), GPRInfo::regT3);
+ jit.and32(MacroAssembler::TrustedImm32(-stackAlignmentRegisters()), GPRInfo::regT3);
+ jit.add32(GPRInfo::regT3, GPRInfo::regT2);
+ arityIntact.link(&jit);
+
// First set up SP so that our data doesn't get clobbered by signals.
jit.addPtr(
MacroAssembler::TrustedImm32(
@@ -207,13 +244,34 @@
jit.store64(GPRInfo::regT0, AssemblyHelpers::Address(GPRInfo::regT1, currentOffset));
}
- // We need to make sure that we return into the register restoration thunk.
+ // We need to make sure that we return into the register restoration thunk. This works
+ // differently depending on whether or not we had arity issues.
+ MacroAssembler::Jump arityIntactForReturnPC =
+ jit.branchTest32(MacroAssembler::Zero, GPRInfo::regT3);
+
+ // The return PC in the call frame header points at exactly the right arity restoration
+ // thunk. We don't want to change that. But the arity restoration thunk's frame has a
+ // return PC and we want to reroute that to our register restoration thunk. The arity
+ // restoration's return PC just just below regT1, and the register restoration's return PC
+ // is right at regT1.
+ jit.loadPtr(MacroAssembler::Address(GPRInfo::regT1, -static_cast<ptrdiff_t>(sizeof(Register))), GPRInfo::regT0);
+ jit.storePtr(GPRInfo::regT0, GPRInfo::regT1);
+ jit.storePtr(
+ MacroAssembler::TrustedImmPtr(vm->getCTIStub(registerRestorationThunkGenerator).code().executableAddress()),
+ MacroAssembler::Address(GPRInfo::regT1, -static_cast<ptrdiff_t>(sizeof(Register))));
+
+ MacroAssembler::Jump arityReturnPCReady = jit.jump();
+
+ arityIntactForReturnPC.link(&jit);
+
jit.loadPtr(MacroAssembler::Address(MacroAssembler::framePointerRegister, CallFrame::returnPCOffset()), GPRInfo::regT0);
jit.storePtr(GPRInfo::regT0, GPRInfo::regT1);
jit.storePtr(
MacroAssembler::TrustedImmPtr(vm->getCTIStub(registerRestorationThunkGenerator).code().executableAddress()),
MacroAssembler::Address(MacroAssembler::framePointerRegister, CallFrame::returnPCOffset()));
+ arityReturnPCReady.link(&jit);
+
// Now get state out of the scratch buffer and place it back into the stack. This part does
// all reboxing.
for (unsigned index = exit.m_values.size(); index--;) {
Modified: branches/jsCStack/Source/_javascript_Core/jit/RegisterPreservationWrapperGenerator.cpp (160957 => 160958)
--- branches/jsCStack/Source/_javascript_Core/jit/RegisterPreservationWrapperGenerator.cpp 2013-12-21 03:50:12 UTC (rev 160957)
+++ branches/jsCStack/Source/_javascript_Core/jit/RegisterPreservationWrapperGenerator.cpp 2013-12-21 04:58:15 UTC (rev 160958)
@@ -147,13 +147,21 @@
AssemblyHelpers::Address(
AssemblyHelpers::stackPointerRegister,
(JSStack::ArgumentCount - JSStack::CallerFrameAndPCSize) * sizeof(Register) + PayloadOffset),
- GPRInfo::regT2);
+ GPRInfo::regT1);
+ jit.move(GPRInfo::regT1, GPRInfo::regT2);
jit.lshift32(AssemblyHelpers::TrustedImm32(3), GPRInfo::regT2);
jit.addPtr(AssemblyHelpers::TrustedImm32(offset), AssemblyHelpers::stackPointerRegister);
jit.addPtr(AssemblyHelpers::stackPointerRegister, GPRInfo::regT2);
+ // Thunks like this rely on the ArgumentCount being intact. Pay it forward.
+ jit.store32(
+ GPRInfo::regT1,
+ AssemblyHelpers::Address(
+ AssemblyHelpers::stackPointerRegister,
+ (JSStack::ArgumentCount - JSStack::CallerFrameAndPCSize) * sizeof(Register) + PayloadOffset));
+
// We saved things at:
//
// adjSP + (JSStack::CallFrameHeaderSize - JSStack::CallerFrameAndPCSize + NumArgs) * 8
Added: branches/jsCStack/Source/_javascript_Core/tests/stress/exit-from-ftl-with-arity-check-fail.js (0 => 160958)
--- branches/jsCStack/Source/_javascript_Core/tests/stress/exit-from-ftl-with-arity-check-fail.js (rev 0)
+++ branches/jsCStack/Source/_javascript_Core/tests/stress/exit-from-ftl-with-arity-check-fail.js 2013-12-21 04:58:15 UTC (rev 160958)
@@ -0,0 +1,18 @@
+function foo(o, a, b, c, d, e, f, g, h, i, j) {
+ return o.f;
+}
+
+function bar(o) {
+ return foo(o);
+}
+
+noInline(foo);
+noInline(bar);
+
+for (var i = 0; i < 100000; ++i)
+ bar({f:42});
+
+var result = bar({g:24, f:43});
+if (result != 43)
+ throw "Error: bad result: " + result;
+
Added: branches/jsCStack/Source/_javascript_Core/tests/stress/repeated-arity-check-fail.js (0 => 160958)
--- branches/jsCStack/Source/_javascript_Core/tests/stress/repeated-arity-check-fail.js (rev 0)
+++ branches/jsCStack/Source/_javascript_Core/tests/stress/repeated-arity-check-fail.js 2013-12-21 04:58:15 UTC (rev 160958)
@@ -0,0 +1,8 @@
+function bar(a,b,c,d,e,f,g,h,i,j,k) {
+}
+
+noInline(bar);
+
+for (var i = 0; i < 10000000; ++i)
+ bar();
+
