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 <simon.hausmann@digia.com>

diff --git a/src/qml/jit/qv4assembler_p.h b/src/qml/jit/qv4assembler_p.h
index caa6986..87875f1 100644 (file)
--- 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<int>(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: