This is sort of related to (and is blocked by) bug 126092, but it's related more to the alignment of the stack at the time of the call on x86. x86 needs two extra slots to store arguments during the call. It also needs SP to be 16-byte aligned after the call pushes the return address onto the stack. Right now we're just subtracting 3 * sizeof(void*), which gives us space for the arguments and the correct alignment assuming SP was correctly aligned to start with. Unfortunately, when the ScratchRegisterAllocator saves reused registers it breaks the correct starting alignment guarantee.
Correction to the above: the stack must be aligned at the time of the call (i.e. 16-byte aligned, then return address is pushed).
Created attachment 220535 [details] Patch
Comment on attachment 220535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220535&action=review r=me > Source/JavaScriptCore/jit/Repatch.cpp:798 > + unsigned desiredAlignment = 16; Please use StackAlignment.h instead. > Source/JavaScriptCore/jit/Repatch.cpp:897 > #if ENABLE(GGC) > - MacroAssembler::Call writeBarrierOperation = writeBarrier(stubJit, baseGPR, scratchGPR1, scratchGPR2, allocator); > + MacroAssembler::Call writeBarrierOperation = writeBarrier(stubJit, baseGPR, scratchGPR1, scratchGPR2, callFrameRegister, allocator); > #endif For #ifdefs like this, I prefer to make the call site unconditional, and make the implementation of writeBarrier() conditional on #ENABLE(GGC). That removes distractions from the code.
> For #ifdefs like this, I prefer to make the call site unconditional, and make the implementation of writeBarrier() conditional on #ENABLE(GGC). That removes distractions from the code. My main reasoning for putting it here rather than in the body is to make so as much code compiles when GGC is disabled.
(In reply to comment #4) > My main reasoning for putting it here rather than in the body is to make so as much code compiles when GGC is disabled. ...as possible?
Committed r161450: <http://trac.webkit.org/changeset/161450>