From 679d9503cffe631cb3b938627274aea10893069c Mon Sep 17 00:00:00 2001 From: bmeurer Date: Wed, 9 Mar 2016 02:42:15 -0800 Subject: [PATCH] [undetectable] Really get comparisons of document.all right now. According to https://www.w3.org/TR/html5/obsolete.html#dom-document-all, comparisons of document.all to other values such as strings or objects, are unaffected. In fact document.all only gets special treatment in comparisons with null or undefined according to HTML. Especially setting the undetectable doesn't make two distinct JSReceivers equal. R=jarin@chromium.org Review URL: https://codereview.chromium.org/1774273002 Cr-Commit-Position: refs/heads/master@{#34608} --- src/arm/code-stubs-arm.cc | 16 ++++++++++++++-- src/arm64/code-stubs-arm64.cc | 20 ++++++++++++++++---- src/ia32/code-stubs-ia32.cc | 14 ++++++++++++-- src/mips/code-stubs-mips.cc | 12 +++++++++++- src/mips64/code-stubs-mips64.cc | 12 +++++++++++- src/objects.cc | 6 +++--- src/x64/code-stubs-x64.cc | 18 ++++++++++++++---- test/mjsunit/undetectable-compare.js | 11 ++++++----- 8 files changed, 87 insertions(+), 22 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index f0fb38fe51..61512c5635 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -477,7 +477,9 @@ static void EmitCheckForTwoHeapNumbers(MacroAssembler* masm, } -// Fast negative check for internalized-to-internalized equality. +// Fast negative check for internalized-to-internalized equality or receiver +// equality. Also handles the undetectable receiver to null/undefined +// comparison. static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, Register lhs, Register rhs, Label* possible_strings, @@ -486,7 +488,7 @@ static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, (lhs.is(r1) && rhs.is(r0))); // r2 is object type of rhs. - Label object_test, return_unequal, undetectable; + Label object_test, return_equal, return_unequal, undetectable; STATIC_ASSERT(kInternalizedTag == 0 && kStringTag == 0); __ tst(r2, Operand(kIsNotStringMask)); __ b(ne, &object_test); @@ -524,6 +526,16 @@ static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, __ bind(&undetectable); __ tst(r5, Operand(1 << Map::kIsUndetectable)); __ b(eq, &return_unequal); + + // If both sides are JSReceivers, then the result is false according to + // the HTML specification, which says that only comparisons with null or + // undefined are affected by special casing for document.all. + __ CompareInstanceType(r2, r2, ODDBALL_TYPE); + __ b(eq, &return_equal); + __ CompareInstanceType(r3, r3, ODDBALL_TYPE); + __ b(ne, &return_unequal); + + __ bind(&return_equal); __ mov(r0, Operand(EQUAL)); __ Ret(); } diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index 27ce16120d..96d34d5bdb 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -425,7 +425,9 @@ static void EmitSmiNonsmiComparison(MacroAssembler* masm, } -// Fast negative check for internalized-to-internalized equality. +// Fast negative check for internalized-to-internalized equality or receiver +// equality. Also handles the undetectable receiver to null/undefined +// comparison. // See call site for description. static void EmitCheckForInternalizedStringsOrObjects( MacroAssembler* masm, Register left, Register right, Register left_map, @@ -435,7 +437,7 @@ static void EmitCheckForInternalizedStringsOrObjects( Register result = x0; DCHECK(left.is(x0) || right.is(x0)); - Label object_test, return_unequal, undetectable; + Label object_test, return_equal, return_unequal, undetectable; STATIC_ASSERT((kInternalizedTag == 0) && (kStringTag == 0)); // TODO(all): reexamine this branch sequence for optimisation wrt branch // prediction. @@ -463,12 +465,22 @@ static void EmitCheckForInternalizedStringsOrObjects( __ CompareInstanceType(left_map, left_type, FIRST_JS_RECEIVER_TYPE); __ B(lt, runtime_call); - __ bind(&return_unequal); + __ Bind(&return_unequal); // Return non-equal by returning the non-zero object pointer in x0. __ Ret(); - __ bind(&undetectable); + __ Bind(&undetectable); __ Tbz(left_bitfield, MaskToBit(1 << Map::kIsUndetectable), &return_unequal); + + // If both sides are JSReceivers, then the result is false according to + // the HTML specification, which says that only comparisons with null or + // undefined are affected by special casing for document.all. + __ CompareInstanceType(right_map, right_type, ODDBALL_TYPE); + __ B(eq, &return_equal); + __ CompareInstanceType(left_map, left_type, ODDBALL_TYPE); + __ B(ne, &return_unequal); + + __ Bind(&return_equal); __ Mov(result, EQUAL); __ Ret(); } diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 47b93d601a..6c070891d4 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -1404,7 +1404,7 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) { // Non-strict equality. Objects are unequal if // they are both JSObjects and not undetectable, // and their pointers are different. - Label return_unequal, undetectable; + Label return_equal, return_unequal, undetectable; // At most one is a smi, so we can test for smi by adding the two. // A smi plus a heap object has the low bit set, a heap object plus // a heap object has the low bit clear. @@ -1412,7 +1412,7 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) { STATIC_ASSERT(kSmiTagMask == 1); __ lea(ecx, Operand(eax, edx, times_1, 0)); __ test(ecx, Immediate(kSmiTagMask)); - __ j(not_zero, &runtime_call, Label::kNear); + __ j(not_zero, &runtime_call); __ mov(ecx, FieldOperand(eax, HeapObject::kMapOffset)); __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset)); @@ -1437,6 +1437,16 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) { __ test_b(FieldOperand(ecx, Map::kBitFieldOffset), 1 << Map::kIsUndetectable); __ j(zero, &return_unequal, Label::kNear); + + // If both sides are JSReceivers, then the result is false according to + // the HTML specification, which says that only comparisons with null or + // undefined are affected by special casing for document.all. + __ CmpInstanceType(ebx, ODDBALL_TYPE); + __ j(zero, &return_equal, Label::kNear); + __ CmpInstanceType(ecx, ODDBALL_TYPE); + __ j(not_zero, &return_unequal, Label::kNear); + + __ bind(&return_equal); __ Move(eax, Immediate(EQUAL)); __ ret(0); // eax, edx were pushed } diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index 80b984a39d..58379c0b64 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -506,7 +506,7 @@ static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, (lhs.is(a1) && rhs.is(a0))); // a2 is object type of rhs. - Label object_test, return_unequal, undetectable; + Label object_test, return_equal, return_unequal, undetectable; STATIC_ASSERT(kInternalizedTag == 0 && kStringTag == 0); __ And(at, a2, Operand(kIsNotStringMask)); __ Branch(&object_test, ne, at, Operand(zero_reg)); @@ -546,6 +546,16 @@ static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, __ bind(&undetectable); __ And(at, t1, Operand(1 << Map::kIsUndetectable)); __ Branch(&return_unequal, eq, at, Operand(zero_reg)); + + // If both sides are JSReceivers, then the result is false according to + // the HTML specification, which says that only comparisons with null or + // undefined are affected by special casing for document.all. + __ GetInstanceType(a2, a2); + __ Branch(&return_equal, eq, a2, Operand(ODDBALL_TYPE)); + __ GetInstanceType(a3, a3); + __ Branch(&return_unequal, ne, a3, Operand(ODDBALL_TYPE)); + + __ bind(&return_equal); __ Ret(USE_DELAY_SLOT); __ li(v0, Operand(EQUAL)); // In delay slot. } diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 390969d070..b2b4aec4d1 100644 --- a/src/mips64/code-stubs-mips64.cc +++ b/src/mips64/code-stubs-mips64.cc @@ -502,7 +502,7 @@ static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, (lhs.is(a1) && rhs.is(a0))); // a2 is object type of rhs. - Label object_test, return_unequal, undetectable; + Label object_test, return_equal, return_unequal, undetectable; STATIC_ASSERT(kInternalizedTag == 0 && kStringTag == 0); __ And(at, a2, Operand(kIsNotStringMask)); __ Branch(&object_test, ne, at, Operand(zero_reg)); @@ -542,6 +542,16 @@ static void EmitCheckForInternalizedStringsOrObjects(MacroAssembler* masm, __ bind(&undetectable); __ And(at, t1, Operand(1 << Map::kIsUndetectable)); __ Branch(&return_unequal, eq, at, Operand(zero_reg)); + + // If both sides are JSReceivers, then the result is false according to + // the HTML specification, which says that only comparisons with null or + // undefined are affected by special casing for document.all. + __ GetInstanceType(a2, a2); + __ Branch(&return_equal, eq, a2, Operand(ODDBALL_TYPE)); + __ GetInstanceType(a3, a3); + __ Branch(&return_unequal, ne, a3, Operand(ODDBALL_TYPE)); + + __ bind(&return_equal); __ Ret(USE_DELAY_SLOT); __ li(v0, Operand(EQUAL)); // In delay slot. } diff --git a/src/objects.cc b/src/objects.cc index d669943db9..2114a9c1a1 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -359,10 +359,10 @@ Maybe Object::Equals(Handle x, Handle y) { return Just(false); } } else if (x->IsJSReceiver()) { - if (y->IsUndetectable()) { - return Just(x->IsUndetectable()); - } else if (y->IsJSReceiver()) { + if (y->IsJSReceiver()) { return Just(x.is_identical_to(y)); + } else if (y->IsUndetectable()) { + return Just(x->IsUndetectable()); } else if (y->IsBoolean()) { y = Oddball::ToNumber(Handle::cast(y)); } else if (!JSReceiver::ToPrimitive(Handle::cast(x)) diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index b7923a7fbc..19ca8d6a74 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -1283,7 +1283,7 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) { // Not strict equality. Objects are unequal if // they are both JSObjects and not undetectable, // and their pointers are different. - Label return_unequal, undetectable; + Label return_equal, return_unequal, undetectable; // At most one is a smi, so we can test for smi by adding the two. // A smi plus a heap object has the low bit set, a heap object plus // a heap object has the low bit clear. @@ -1297,10 +1297,10 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) { __ movp(rcx, FieldOperand(rdx, HeapObject::kMapOffset)); __ testb(FieldOperand(rbx, Map::kBitFieldOffset), Immediate(1 << Map::kIsUndetectable)); - __ j(not_zero, &undetectable); + __ j(not_zero, &undetectable, Label::kNear); __ testb(FieldOperand(rcx, Map::kBitFieldOffset), Immediate(1 << Map::kIsUndetectable)); - __ j(not_zero, &return_unequal); + __ j(not_zero, &return_unequal, Label::kNear); __ CmpInstanceType(rbx, FIRST_JS_RECEIVER_TYPE); __ j(below, &runtime_call, Label::kNear); @@ -1314,7 +1314,17 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) { __ bind(&undetectable); __ testb(FieldOperand(rcx, Map::kBitFieldOffset), Immediate(1 << Map::kIsUndetectable)); - __ j(zero, &return_unequal); + __ j(zero, &return_unequal, Label::kNear); + + // If both sides are JSReceivers, then the result is false according to + // the HTML specification, which says that only comparisons with null or + // undefined are affected by special casing for document.all. + __ CmpInstanceType(rbx, ODDBALL_TYPE); + __ j(zero, &return_equal, Label::kNear); + __ CmpInstanceType(rcx, ODDBALL_TYPE); + __ j(not_zero, &return_unequal, Label::kNear); + + __ bind(&return_equal); __ Set(rax, EQUAL); __ ret(0); } diff --git a/test/mjsunit/undetectable-compare.js b/test/mjsunit/undetectable-compare.js index 2887f2f53b..c78593439c 100644 --- a/test/mjsunit/undetectable-compare.js +++ b/test/mjsunit/undetectable-compare.js @@ -92,15 +92,16 @@ for (var i = 0; i < 5; i++) { } -assertTrue(undetectable == %GetUndetectable()); +assertFalse(undetectable == %GetUndetectable()); assertFalse(undetectable === %GetUndetectable()); function test2(a, b) { - assertTrue(a == b); + return a == b; } -test2(1, 1); -test2(undetectable, undetectable); +test2(0, 1); +test2(undetectable, {}); +%OptimizeFunctionOnNextCall(test2); for (var i = 0; i < 5; ++i) { - test2(undetectable, %GetUndetectable()); + assertFalse(test2(undetectable, %GetUndetectable())); }