Fix JITted code for jump strict-not-equal undefined on 32bit

The generated code for jump-on-strict-not-equal-undefined used the
same logic (but with inverted conditions) as the equal case. For
equality, one can jump to else if the value parts are not the same.
So, for not-equal, if the value parts are the same, it would jump
to the else block if they are the same. Meaning, an encoded int
value of 0 (which is strict-not-equal to undefined) would end up
being evaluated as equal.

Task-number: QTBUG-66832
Change-Id: Id27bb44eccbf39608ae8cebab634c8bcd4c8adfc
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

2 files changed, 50 insertions, 12 deletions

diff --git a/src/qml/jit/qv4isel_masm.cpp b/src/qml/jit/qv4isel_masm.cpp
index 4a201200fa..ad0f850847 100644
--- a/src/qml/jit/qv4isel_masm.cpp
+++ b/src/qml/jit/qv4isel_masm.cpp
@@ -1869,24 +1869,41 @@ bool InstructionSelection::visitCJumpStrictUndefined(IR::Binop *binop,
         return true;
     }
 
+    const Assembler::RegisterID varReg = Assembler::ReturnValueRegister;
+#ifdef QV4_USE_64_BIT_VALUE_ENCODING
     Assembler::RelationalCondition cond = binop->op == IR::OpStrictEqual ? Assembler::Equal
                                                                          : Assembler::NotEqual;
-    const Assembler::RegisterID tagReg = Assembler::ReturnValueRegister;
-#ifdef QV4_USE_64_BIT_VALUE_ENCODING
-    Assembler::Pointer addr = _as->loadAddress(Assembler::ScratchRegister, varSrc);
-    _as->load64(addr, tagReg);
-    const Assembler::TrustedImm64 tag(0);
+    Assembler::Pointer varAddr = _as->loadAddress(Assembler::ScratchRegister, varSrc);
+    _as->load64(varAddr, varReg);
+    const Assembler::TrustedImm64 undefined(0);
+    _as->generateCJumpOnCompare(cond, varReg, undefined, _block, trueBlock, falseBlock);
 #else // !QV4_USE_64_BIT_VALUE_ENCODING
-    Assembler::Pointer tagAddr = _as->loadAddress(Assembler::ScratchRegister, varSrc);
-    _as->load32(tagAddr, tagReg);
-    Assembler::Jump j = _as->branch32(Assembler::invert(cond), tagReg, Assembler::TrustedImm32(0));
-    _as->addPatch(falseBlock, j);
+    const Assembler::TrustedImm32 undefinedTag(QV4::Value::Managed_Type_Internal);
+    const Assembler::TrustedImm32 undefinedValue(0);
 
+    Assembler::Pointer varAddr = _as->loadAddress(Assembler::ScratchRegister, varSrc);
+    Assembler::Pointer tagAddr = varAddr;
     tagAddr.offset += 4;
-    _as->load32(tagAddr, tagReg);
-    const Assembler::TrustedImm32 tag(QV4::Value::Managed_Type_Internal);
+    const Assembler::RegisterID tagReg = varReg;
+
+    if (binop->op == IR::OpStrictEqual) {
+        _as->load32(tagAddr, tagReg);
+        // if the tags are not the same, we can fail already:
+        Assembler::Jump j = _as->branch32(Assembler::NotEqual, tagReg, undefinedTag);
+        _as->addPatch(falseBlock, j);
+        _as->load32(varAddr, varReg);
+        // ok, tags are the same, so if the values are the same then we're done
+        _as->generateCJumpOnCompare(Assembler::Equal, varReg, undefinedValue, _block, trueBlock, falseBlock);
+    } else { // strict not equal:
+        _as->load32(varAddr, varReg);
+        // if the values are not the same, we're done
+        Assembler::Jump j = _as->branch32(Assembler::NotEqual, varReg, undefinedValue);
+        _as->addPatch(trueBlock, j);
+        _as->load32(tagAddr, tagReg);
+        // ok, so the values are the same, now check the tags
+        _as->generateCJumpOnCompare(Assembler::NotEqual, tagReg, undefinedTag, _block, trueBlock, falseBlock);
+    }
 #endif
-    _as->generateCJumpOnCompare(cond, tagReg, tag, _block, trueBlock, falseBlock);
     return true;
 }
 
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 491e62f1d2..cafda83cea 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -334,6 +334,7 @@ private slots:
     void qtbug_54589();
     void qtbug_54687();
     void stringify_qtbug_50592();
+    void jumpStrictNotEqualUndefined();
 
 private:
 //    static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@@ -8061,6 +8062,26 @@ void tst_qqmlecmascript::stringify_qtbug_50592()
     QCOMPARE(obj->property("source").toString(), QString::fromLatin1("http://example.org/some_nonexistant_image.png"));
 }
 
+void tst_qqmlecmascript::jumpStrictNotEqualUndefined()
+{
+    QJSEngine engine;
+    QJSValue v = engine.evaluate(QString::fromLatin1(
+        "var ok = 0\n"
+        "var foo = 0\n"
+        "if (foo !== void 1)\n"
+        "    ++ok;\n"
+        "else\n"
+        "    --ok;\n"
+        "if (foo === void 1)\n"
+        "    --ok;\n"
+        "else\n"
+        "    ++ok;\n"
+        "ok\n"
+    ));
+    QVERIFY(!v.isError());
+    QCOMPARE(v.toInt(), 2);
+}
+
 QTEST_MAIN(tst_qqmlecmascript)
 
 #include "tst_qqmlecmascript.moc"