Bug 940635 - Don't allow Int32 conversions of both LHS and RHS in comparisons when their types are not actually known, r=shu.

This commit is contained in:
Brian Hackett 2013-12-07 18:18:16 -08:00
parent 8187c0ae71
commit 627564f755
10 changed files with 112 additions and 41 deletions

View File

@ -0,0 +1,7 @@
function f(y) {
return (y > 0) == y;
}
assertEq(f(0), true);
assertEq(f(0), true);
assertEq(f(null), false);
assertEq(f(null), false);

View File

@ -226,8 +226,20 @@ BaselineInspector::expectedCompareType(jsbytecode *pc)
if (!first && !dimorphicStub(pc, &first, &second))
return MCompare::Compare_Unknown;
if (CanUseInt32Compare(first->kind()) && (!second || CanUseInt32Compare(second->kind())))
if (CanUseInt32Compare(first->kind()) && (!second || CanUseInt32Compare(second->kind()))) {
ICCompare_Int32WithBoolean *coerce =
first->isCompare_Int32WithBoolean()
? first->toCompare_Int32WithBoolean()
: ((second && second->isCompare_Int32WithBoolean())
? second->toCompare_Int32WithBoolean()
: nullptr);
if (coerce) {
return coerce->lhsIsInt32()
? MCompare::Compare_Int32MaybeCoerceRHS
: MCompare::Compare_Int32MaybeCoerceLHS;
}
return MCompare::Compare_Int32;
}
if (CanUseDoubleCompare(first->kind()) && (!second || CanUseDoubleCompare(second->kind()))) {
ICCompare_NumberWithUndefined *coerce =

View File

@ -201,7 +201,8 @@ CodeGenerator::visitValueToInt32(LValueToInt32 *lir)
masm.bind(oolDouble->rejoin());
} else {
masm.convertValueToInt32(operand, input, temp, output, &fails,
lir->mirNormal()->canBeNegativeZero());
lir->mirNormal()->canBeNegativeZero(),
lir->mirNormal()->conversion());
}
return bailoutFrom(&fails, lir->snapshot());

View File

@ -1449,7 +1449,7 @@ jit::ExtractLinearInequality(MTest *test, BranchDirection direction,
MDefinition *rhs = compare->getOperand(1);
// TODO: optimize Compare_UInt32
if (compare->compareType() != MCompare::Compare_Int32)
if (!compare->isInt32Comparison())
return false;
JS_ASSERT(lhs->type() == MIRType_Int32);

View File

@ -1563,7 +1563,8 @@ MacroAssembler::convertValueToInt(ValueOperand value, MDefinition *maybeInput,
Label *handleStringEntry, Label *handleStringRejoin,
Label *truncateDoubleSlow,
Register stringReg, FloatRegister temp, Register output,
Label *fail, IntConversionBehavior behavior)
Label *fail, IntConversionBehavior behavior,
IntConversionInputKind conversion)
{
Register tag = splitTagForTest(value);
bool handleStrings = (behavior == IntConversion_Truncate ||
@ -1572,29 +1573,36 @@ MacroAssembler::convertValueToInt(ValueOperand value, MDefinition *maybeInput,
handleStringRejoin;
bool zeroObjects = behavior == IntConversion_ClampToUint8;
JS_ASSERT_IF(handleStrings || zeroObjects, conversion == IntConversion_Any);
Label done, isInt32, isBool, isDouble, isNull, isString;
branchEqualTypeIfNeeded(MIRType_Int32, maybeInput, tag, &isInt32);
branchEqualTypeIfNeeded(MIRType_Boolean, maybeInput, tag, &isBool);
if (conversion == IntConversion_Any)
branchEqualTypeIfNeeded(MIRType_Boolean, maybeInput, tag, &isBool);
branchEqualTypeIfNeeded(MIRType_Double, maybeInput, tag, &isDouble);
// If we are not truncating, we fail for anything that's not
// null. Otherwise we might be able to handle strings and objects.
switch (behavior) {
case IntConversion_Normal:
case IntConversion_NegativeZeroCheck:
branchTestNull(Assembler::NotEqual, tag, fail);
break;
if (conversion == IntConversion_Any) {
// If we are not truncating, we fail for anything that's not
// null. Otherwise we might be able to handle strings and objects.
switch (behavior) {
case IntConversion_Normal:
case IntConversion_NegativeZeroCheck:
branchTestNull(Assembler::NotEqual, tag, fail);
break;
case IntConversion_Truncate:
case IntConversion_ClampToUint8:
branchEqualTypeIfNeeded(MIRType_Null, maybeInput, tag, &isNull);
if (handleStrings)
branchEqualTypeIfNeeded(MIRType_String, maybeInput, tag, &isString);
if (zeroObjects)
branchEqualTypeIfNeeded(MIRType_Object, maybeInput, tag, &isNull);
branchTestUndefined(Assembler::NotEqual, tag, fail);
break;
case IntConversion_Truncate:
case IntConversion_ClampToUint8:
branchEqualTypeIfNeeded(MIRType_Null, maybeInput, tag, &isNull);
if (handleStrings)
branchEqualTypeIfNeeded(MIRType_String, maybeInput, tag, &isString);
if (zeroObjects)
branchEqualTypeIfNeeded(MIRType_Object, maybeInput, tag, &isNull);
branchTestUndefined(Assembler::NotEqual, tag, fail);
break;
}
} else {
jump(fail);
}
// The value is null or undefined in truncation contexts - just emit 0.

View File

@ -1174,6 +1174,11 @@ class MacroAssembler : public MacroAssemblerSpecific
IntConversion_ClampToUint8,
};
enum IntConversionInputKind {
IntConversion_NumbersOnly,
IntConversion_Any
};
//
// Functions for converting values to int.
//
@ -1188,7 +1193,8 @@ class MacroAssembler : public MacroAssemblerSpecific
Label *handleStringEntry, Label *handleStringRejoin,
Label *truncateDoubleSlow,
Register stringReg, FloatRegister temp, Register output,
Label *fail, IntConversionBehavior behavior);
Label *fail, IntConversionBehavior behavior,
IntConversionInputKind conversion = IntConversion_Any);
void convertValueToInt(ValueOperand value, FloatRegister temp, Register output, Label *fail,
IntConversionBehavior behavior)
{
@ -1214,12 +1220,13 @@ class MacroAssembler : public MacroAssemblerSpecific
}
void convertValueToInt32(ValueOperand value, MDefinition *input,
FloatRegister temp, Register output, Label *fail,
bool negativeZeroCheck)
bool negativeZeroCheck, IntConversionInputKind conversion = IntConversion_Any)
{
convertValueToInt(value, input, nullptr, nullptr, nullptr, InvalidReg, temp, output, fail,
negativeZeroCheck
? IntConversion_NegativeZeroCheck
: IntConversion_Normal);
: IntConversion_Normal,
conversion);
}
bool convertValueToInt32(JSContext *cx, const Value &v, Register output, Label *fail,
bool negativeZeroCheck)

View File

@ -769,20 +769,17 @@ LIRGenerator::visitTest(MTest *test)
}
// Compare and branch Int32 or Object pointers.
if (comp->compareType() == MCompare::Compare_Int32 ||
if (comp->isInt32Comparison() ||
comp->compareType() == MCompare::Compare_UInt32 ||
comp->compareType() == MCompare::Compare_Object)
{
JSOp op = ReorderComparison(comp->jsop(), &left, &right);
LAllocation lhs = useRegister(left);
LAllocation rhs;
if (comp->compareType() == MCompare::Compare_Int32 ||
comp->compareType() == MCompare::Compare_UInt32)
{
if (comp->isInt32Comparison() || comp->compareType() == MCompare::Compare_UInt32)
rhs = useAnyOrConstant(right);
} else {
else
rhs = useRegister(right);
}
LCompareAndBranch *lir = new(alloc()) LCompareAndBranch(op, lhs, rhs, ifTrue, ifFalse);
return add(lir, comp);
}
@ -958,14 +955,14 @@ LIRGenerator::visitCompare(MCompare *comp)
}
// Compare Int32 or Object pointers.
if (comp->compareType() == MCompare::Compare_Int32 ||
if (comp->isInt32Comparison() ||
comp->compareType() == MCompare::Compare_UInt32 ||
comp->compareType() == MCompare::Compare_Object)
{
JSOp op = ReorderComparison(comp->jsop(), &left, &right);
LAllocation lhs = useRegister(left);
LAllocation rhs;
if (comp->compareType() == MCompare::Compare_Int32 ||
if (comp->isInt32Comparison() ||
comp->compareType() == MCompare::Compare_UInt32)
{
rhs = useAnyOrConstant(right);

View File

@ -1723,6 +1723,9 @@ MCompare::inputType()
return MIRType_Boolean;
case Compare_UInt32:
case Compare_Int32:
case Compare_Int32MaybeCoerceBoth:
case Compare_Int32MaybeCoerceLHS:
case Compare_Int32MaybeCoerceRHS:
return MIRType_Int32;
case Compare_Double:
case Compare_DoubleMaybeCoerceLHS:
@ -1809,7 +1812,7 @@ MCompare::infer(BaselineInspector *inspector, jsbytecode *pc)
if ((lhs == MIRType_Int32 && rhs == MIRType_Int32) ||
(lhs == MIRType_Boolean && rhs == MIRType_Boolean))
{
compareType_ = Compare_Int32;
compareType_ = Compare_Int32MaybeCoerceBoth;
return;
}
@ -1818,7 +1821,7 @@ MCompare::infer(BaselineInspector *inspector, jsbytecode *pc)
(lhs == MIRType_Int32 || lhs == MIRType_Boolean) &&
(rhs == MIRType_Int32 || rhs == MIRType_Boolean))
{
compareType_ = Compare_Int32;
compareType_ = Compare_Int32MaybeCoerceBoth;
return;
}

View File

@ -2192,6 +2192,9 @@ class MCompare
// Int32 compared to Int32
// Boolean compared to Boolean
Compare_Int32,
Compare_Int32MaybeCoerceBoth,
Compare_Int32MaybeCoerceLHS,
Compare_Int32MaybeCoerceRHS,
// Int32 compared as unsigneds
Compare_UInt32,
@ -2258,6 +2261,12 @@ class MCompare
CompareType compareType() const {
return compareType_;
}
bool isInt32Comparison() const {
return compareType() == Compare_Int32 ||
compareType() == Compare_Int32MaybeCoerceBoth ||
compareType() == Compare_Int32MaybeCoerceLHS ||
compareType() == Compare_Int32MaybeCoerceRHS;
}
bool isDoubleComparison() const {
return compareType() == Compare_Double ||
compareType() == Compare_DoubleMaybeCoerceLHS ||
@ -3041,10 +3050,12 @@ class MToInt32
public ToInt32Policy
{
bool canBeNegativeZero_;
MacroAssembler::IntConversionInputKind conversion_;
MToInt32(MDefinition *def)
MToInt32(MDefinition *def, MacroAssembler::IntConversionInputKind conversion)
: MUnaryInstruction(def),
canBeNegativeZero_(true)
canBeNegativeZero_(true),
conversion_(conversion)
{
setResultType(MIRType_Int32);
setMovable();
@ -3052,9 +3063,11 @@ class MToInt32
public:
INSTRUCTION_HEADER(ToInt32)
static MToInt32 *New(TempAllocator &alloc, MDefinition *def)
static MToInt32 *New(TempAllocator &alloc, MDefinition *def,
MacroAssembler::IntConversionInputKind conversion =
MacroAssembler::IntConversion_Any)
{
return new(alloc) MToInt32(def);
return new(alloc) MToInt32(def, conversion);
}
MDefinition *foldsTo(TempAllocator &alloc, bool useValueNumbers);
@ -3073,6 +3086,10 @@ class MToInt32
return this;
}
MacroAssembler::IntConversionInputKind conversion() const {
return conversion_;
}
bool congruentTo(MDefinition *ins) const {
return congruentIfOperandsEqual(ins);
}

View File

@ -141,7 +141,7 @@ ComparePolicy::adjustInputs(TempAllocator &alloc, MInstruction *def)
if (compare->compareType() == MCompare::Compare_Boolean &&
def->getOperand(0)->type() == MIRType_Boolean)
{
compare->setCompareType(MCompare::Compare_Int32);
compare->setCompareType(MCompare::Compare_Int32MaybeCoerceBoth);
}
// Compare_Boolean specialization is done for "Anything === Bool"
@ -242,9 +242,28 @@ ComparePolicy::adjustInputs(TempAllocator &alloc, MInstruction *def)
replace = MToFloat32::New(alloc, in, convert);
break;
}
case MIRType_Int32:
replace = MToInt32::New(alloc, in);
case MIRType_Int32: {
MacroAssembler::IntConversionInputKind convert = MacroAssembler::IntConversion_NumbersOnly;
if (compare->compareType() == MCompare::Compare_Int32MaybeCoerceBoth ||
(compare->compareType() == MCompare::Compare_Int32MaybeCoerceLHS && i == 0) ||
(compare->compareType() == MCompare::Compare_Int32MaybeCoerceRHS && i == 1))
{
convert = MacroAssembler::IntConversion_Any;
}
if (convert == MacroAssembler::IntConversion_NumbersOnly) {
if (in->type() != MIRType_Int32 && in->type() != MIRType_Value)
in = boxAt(alloc, def, in);
} else {
if (in->type() == MIRType_Undefined ||
in->type() == MIRType_String ||
in->type() == MIRType_Object)
{
in = boxAt(alloc, def, in);
}
}
replace = MToInt32::New(alloc, in, convert);
break;
}
case MIRType_Object:
replace = MUnbox::New(alloc, in, MIRType_Object, MUnbox::Infallible);
break;