From bfee76bdc96cfa8b4decf6d099ff690446d2f8eb Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Tue, 26 Aug 2014 14:21:01 +0200 Subject: [PATCH] V4 JIT: fix stack layout. Commit d9f33ccdef985badc56fd8940373748626beffc7 introduced an off-by-one in the calculation of the offset of a saved register (in StackLayout::savedRegPointer), resulting in overwriting a callee saved register with the tag of a QV4::Value. This method now calculates those pointers relative to the bottom of the stack frame. The off-by-one didn't happen before that patch, because there was a magical +1 used in the constructor for the number of callee saved registers, thereby prevented this from happening. However, that resulted in a frame size that was unnecessary big. Task-number: QTBUG-40927 Change-Id: If88fe9f3490a4d23a1e69c630c87219fcfef671f Reviewed-by: Simon Hausmann --- src/qml/jit/qv4assembler_p.h | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/qml/jit/qv4assembler_p.h b/src/qml/jit/qv4assembler_p.h index caa6986..87875f1 100644 --- a/src/qml/jit/qv4assembler_p.h +++ b/src/qml/jit/qv4assembler_p.h @@ -160,9 +160,9 @@ public: // callee saved reg n // ... // callee saved reg 0 - // saved reg arg 0 + // saved reg arg n // ... - // saved reg arg n <- SP + // saved reg arg 0 <- SP // // Stack layout for the JS stack: // function call argument n <- LocalsRegister @@ -205,15 +205,20 @@ public: int calculateStackFrameSize() const { + // sp was aligned before executing the call instruction. So, calculate all contents + // that were saved after that aligned stack...: const int stackSpaceAllocatedOtherwise = StackSpaceAllocatedUponFunctionEntry + RegisterSize; // saved StackFrameRegister - // space for the callee saved registers + // ... then calculate the stuff we want to store ...: int frameSize = RegisterSize * normalRegistersToSave + sizeof(double) * fpRegistersToSave; - frameSize += savedRegCount * sizeof(QV4::Value); // these get written out as Values, not as native registers + frameSize += savedRegCount * sizeof(QV4::Value); // (these get written out as Values, not as native registers) Q_ASSERT(frameSize + stackSpaceAllocatedOtherwise < INT_MAX); + // .. then align that chunk ..: frameSize = static_cast(WTF::roundUpToMultipleOf(StackAlignment, frameSize + stackSpaceAllocatedOtherwise)); + // ... which now holds our frame size + the extra stuff that was pushed due to the call. + // So subtract that extra stuff, and we have our frame size: frameSize -= stackSpaceAllocatedOtherwise; return frameSize; @@ -255,15 +260,12 @@ public: Q_ASSERT(offset >= 0); Q_ASSERT(offset < savedRegCount); - const int off = offset * sizeof(QV4::Value); - return Address(Assembler::StackFrameRegister, - calleeSavedRegisterSpace() - off); - } - - int calleeSavedRegisterSpace() const - { - // plus 1 for the old FP - return RegisterSize * (normalRegistersToSave + 1) - + sizeof(double) * fpRegistersToSave; + // Get the address of the bottom-most element of our frame: + Address ptr(Assembler::StackFrameRegister, -calculateStackFrameSize()); + // This now is the element with offset 0. So: + ptr.offset += offset * sizeof(QV4::Value); + // and we're done! + return ptr; } private: -- 2.7.4