Modified: branches/jsCStack/Source/_javascript_Core/ChangeLog (162957 => 162958)
--- branches/jsCStack/Source/_javascript_Core/ChangeLog 2014-01-28 22:03:57 UTC (rev 162957)
+++ branches/jsCStack/Source/_javascript_Core/ChangeLog 2014-01-28 22:10:30 UTC (rev 162958)
@@ -1,5 +1,21 @@
2014-01-28 Filip Pizlo <fpi...@apple.com>
+ Arity fixup clobbers callee-saves, causing FTL->FTL arity fixed-up calls to corrupt state
+ https://bugs.webkit.org/show_bug.cgi?id=127791
+
+ Reviewed by Michael Saboff.
+
+ Make the 64-bit path - i.e. the only path where the FTL matters - use regT6 instead
+ of regT3. regT6 is not a callee save.
+
+ * jit/ThunkGenerators.cpp:
+ (JSC::arityFixup):
+ * tests/stress/ftl-to-ftl-arity-fixup.js: Added.
+ (foo):
+ (bar):
+
+2014-01-28 Filip Pizlo <fpi...@apple.com>
+
FTL should really, *really* support NotCellUse
https://bugs.webkit.org/show_bug.cgi?id=127790
Modified: branches/jsCStack/Source/_javascript_Core/jit/ThunkGenerators.cpp (162957 => 162958)
--- branches/jsCStack/Source/_javascript_Core/jit/ThunkGenerators.cpp 2014-01-28 22:03:57 UTC (rev 162957)
+++ branches/jsCStack/Source/_javascript_Core/jit/ThunkGenerators.cpp 2014-01-28 22:10:30 UTC (rev 162958)
@@ -435,15 +435,15 @@
# endif
jit.lshift32(JSInterfaceJIT::TrustedImm32(logStackAlignmentRegisters()), JSInterfaceJIT::regT0);
jit.neg64(JSInterfaceJIT::regT0);
- jit.move(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::regT3);
+ jit.move(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::regT6);
jit.load32(JSInterfaceJIT::Address(JSInterfaceJIT::callFrameRegister, JSStack::ArgumentCount * sizeof(Register)), JSInterfaceJIT::regT2);
jit.add32(JSInterfaceJIT::TrustedImm32(JSStack::CallFrameHeaderSize), JSInterfaceJIT::regT2);
// Move current frame down regT0 number of slots
JSInterfaceJIT::Label copyLoop(jit.label());
- jit.load64(JSInterfaceJIT::regT3, JSInterfaceJIT::regT1);
- jit.store64(JSInterfaceJIT::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT3, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
- jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::regT3);
+ jit.load64(JSInterfaceJIT::regT6, JSInterfaceJIT::regT1);
+ jit.store64(JSInterfaceJIT::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT6, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
+ jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::regT6);
jit.branchSub32(MacroAssembler::NonZero, JSInterfaceJIT::TrustedImm32(1), JSInterfaceJIT::regT2).linkTo(copyLoop, &jit);
// Fill in regT0 - 1 missing arg slots with undefined
@@ -451,8 +451,8 @@
jit.move(JSInterfaceJIT::TrustedImm64(ValueUndefined), JSInterfaceJIT::regT1);
jit.add32(JSInterfaceJIT::TrustedImm32(1), JSInterfaceJIT::regT2);
JSInterfaceJIT::Label fillUndefinedLoop(jit.label());
- jit.store64(JSInterfaceJIT::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT3, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
- jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::regT3);
+ jit.store64(JSInterfaceJIT::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT6, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
+ jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::regT6);
jit.branchAdd32(MacroAssembler::NonZero, JSInterfaceJIT::TrustedImm32(1), JSInterfaceJIT::regT2).linkTo(fillUndefinedLoop, &jit);
// Adjust call frame register and stack pointer to account for missing args
@@ -463,7 +463,7 @@
// Save the original return PC.
jit.loadPtr(JSInterfaceJIT::Address(JSInterfaceJIT::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT1);
- jit.storePtr(GPRInfo::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT3, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
+ jit.storePtr(GPRInfo::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT6, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
// Install the new return PC.
jit.storePtr(GPRInfo::regT5, JSInterfaceJIT::Address(JSInterfaceJIT::callFrameRegister, CallFrame::returnPCOffset()));
Added: branches/jsCStack/Source/_javascript_Core/tests/stress/ftl-to-ftl-arity-fixup.js (0 => 162958)
--- branches/jsCStack/Source/_javascript_Core/tests/stress/ftl-to-ftl-arity-fixup.js (rev 0)
+++ branches/jsCStack/Source/_javascript_Core/tests/stress/ftl-to-ftl-arity-fixup.js 2014-01-28 22:10:30 UTC (rev 162958)
@@ -0,0 +1,36 @@
+function foo(a, b, c) {
+ return (a|0) + (b|0) + (c|0);
+}
+
+function bar(o) {
+ // Save a bunch of state in local variables.
+ var a = o.f;
+ var b = o.g;
+ var c = o.h;
+ var d = o.i;
+ var e = o.j;
+ var f = o.k;
+ var g = o.l;
+ // Make a call that will be subject to arity fixup and then use the saved state. We're
+ // counting on LLVM to put those variables in callee-saves, since that's pretty much the
+ // only sensible choice.
+ return foo(42) + a + b + c + d + e + f + g;
+}
+
+noInline(foo);
+noInline(bar);
+
+for (var i = 0; i < 100000; ++i) {
+ // Call bar() in such a way that all of those callee-save variables have fairly unique
+ // looking values, to maximize the chances of foo() clobbering them in a recognizable
+ // way.
+ var result = bar({
+ f:i * 3, g:i - 1, h:(i / 2)|0, i:-i, j:13 + ((i / 5)|0), k:14 - ((i / 6)|0),
+ l:1 - i});
+
+ var expected = 42 + i * 3 + i - 1 + ((i / 2)|0) - i + 13 + ((i / 5)|0) + 14 -
+ ((i / 6)|0) + 1 - i;
+
+ if (result != expected)
+ throw "Error: for iteration " + i + " expected " + expected + " but got " + result;
+}
