Baseline JIT: Save accumulator in toInt32LhsAcc()

toInt32LhsAcc convertes both the lhs and the accumulator to int32. If
the accumulator is not saved, a GC run during the conversion of the lhs
might trash its value.

Fixes: QTBUG-74058
Change-Id: Ic42693061c7d483bb430d77bcc095de6ff9a6843
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

2 files changed, 28 insertions, 16 deletions

diff --git a/src/qml/jit/qv4baselineassembler.cpp b/src/qml/jit/qv4baselineassembler.cpp
index 5c08c42977..b13f646360 100644
--- a/src/qml/jit/qv4baselineassembler.cpp
+++ b/src/qml/jit/qv4baselineassembler.cpp
@@ -208,17 +208,20 @@ public:
         isNumber.link(this);
     }
 
+    // this converts both the lhs and the accumulator to int32
     void toInt32LhsAcc(Address lhs, RegisterID lhsTarget)
     {
         load64(lhs, lhsTarget);
         urshift64(lhsTarget, TrustedImm32(Value::QuickType_Shift), ScratchRegister2);
         auto lhsIsInt = branch32(Equal, TrustedImm32(Value::QT_Int), ScratchRegister2);
 
-        pushAligned(AccumulatorRegister);
+        const Address accumulatorStackAddress(JSStackFrameRegister,
+                                              offsetof(CallData, accumulator));
+        storeAccumulator(accumulatorStackAddress);
         move(lhsTarget, registerForArg(0));
         callHelper(toInt32Helper);
         move(ReturnValueRegister, lhsTarget);
-        popAligned(AccumulatorRegister);
+        loadAccumulator(accumulatorStackAddress);
 
         lhsIsInt.link(this);
         urshift64(AccumulatorRegister, TrustedImm32(Value::QuickType_Shift), ScratchRegister2);
@@ -498,6 +501,7 @@ public:
         isNumber.link(this);
     }
 
+    // this converts both the lhs and the accumulator to int32
     void toInt32LhsAcc(Address lhs, RegisterID lhsTarget)
     {
         bool accumulatorNeedsSaving = AccumulatorRegisterValue == ReturnValueRegisterValue
@@ -510,32 +514,28 @@ public:
         auto lhsIsInt = jump();
 
         lhsIsNotInt.link(this);
-        if (accumulatorNeedsSaving) {
-            push(AccumulatorRegisterTag);
-            push(AccumulatorRegisterValue);
-        }
+
+        // Save accumulator from being garbage collected, no matter if we will reuse the register.
+        const Address accumulatorStackAddress(JSStackFrameRegister,
+                                              offsetof(CallData, accumulator));
+        storeAccumulator(accumulatorStackAddress);
 
         if (ArgInRegCount < 2) {
-            if (!accumulatorNeedsSaving)
-                subPtr(TrustedImm32(2 * PointerSize), StackPointerRegister);
+            subPtr(TrustedImm32(2 * PointerSize), StackPointerRegister);
             push(lhsTarget);
             load32(lhs, lhsTarget);
             push(lhsTarget);
         } else {
-            if (accumulatorNeedsSaving)
-                subPtr(TrustedImm32(2 * PointerSize), StackPointerRegister);
             move(lhsTarget, registerForArg(1));
             load32(lhs, registerForArg(0));
         }
         callHelper(toInt32Helper);
         move(ReturnValueRegisterValue, lhsTarget);
-        if (accumulatorNeedsSaving) {
-            addPtr(TrustedImm32(2 * PointerSize), StackPointerRegister);
-            pop(AccumulatorRegisterValue);
-            pop(AccumulatorRegisterTag);
-        } else if (ArgInRegCount < 2) {
+        if (ArgInRegCount < 2)
             addPtr(TrustedImm32(4 * PointerSize), StackPointerRegister);
-        }
+
+        if (accumulatorNeedsSaving) // otherwise it's still the same
+            loadAccumulator(accumulatorStackAddress);
 
         lhsIsInt.link(this);
 
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index a67a503f82..f33f1d9125 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -365,6 +365,7 @@ private slots:
     void numberToStringWithRadix();
     void tailCallWithArguments();
     void deleteSparseInIteration();
+    void saveAccumulatorBeforeToInt32();
 
 private:
 //    static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@@ -8929,6 +8930,17 @@ void tst_qqmlecmascript::deleteSparseInIteration()
     QCOMPARE(value.property("2").toInt(), 4096);
 }
 
+void tst_qqmlecmascript::saveAccumulatorBeforeToInt32()
+{
+    QJSEngine engine;
+
+    // Infinite recursion produces a range error, but should not crash.
+    // Also, any GC runs in between should not trash the temporary results of "a+a".
+    const QJSValue value = engine.evaluate("function a(){a(a&a+a)}a()");
+    QVERIFY(value.isError());
+    QCOMPARE(value.toString(), QLatin1String("RangeError: Maximum call stack size exceeded."));
+}
+
 QTEST_MAIN(tst_qqmlecmascript)
 
 #include "tst_qqmlecmascript.moc"