Skip to content

Commit

Permalink
Fix JITted code for jump strict-not-equal undefined on 32bit
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Erik Verbruggen committed Mar 15, 2018
1 parent 10647ce commit e2218f8
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 12 deletions.
41 changes: 29 additions & 12 deletions src/qml/jit/qv4isel_masm.cpp
Expand Up @@ -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;
}

Expand Down
21 changes: 21 additions & 0 deletions tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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"

0 comments on commit e2218f8

Please sign in to comment.