From 382435480d682ea35c804c954ffc6f4fef78346b Mon Sep 17 00:00:00 2001 From: conradw Date: Wed, 6 May 2015 08:04:39 -0700 Subject: [PATCH] [es6] Fix symbol comparison on some architectures https://codereview.chromium.org/1125783002 did not handle all cases for some architectures. These cases are now covered, and tests have been extended to check them. BUG=v8:4073 LOG=N Review URL: https://codereview.chromium.org/1128143002 Cr-Commit-Position: refs/heads/master@{#28266} --- src/arm/code-stubs-arm.cc | 4 +++- src/arm64/code-stubs-arm64.cc | 11 +++++++---- src/mips/code-stubs-mips.cc | 4 ++-- src/mips64/code-stubs-mips64.cc | 4 ++-- src/ppc/code-stubs-ppc.cc | 4 +++- test/mjsunit/es6/symbols.js | 42 +++++++++++++++++------------------------ 6 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index c69b382..6133281 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -253,7 +253,7 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, if (cond == lt || cond == gt) { __ CompareObjectType(r0, r4, r4, FIRST_SPEC_OBJECT_TYPE); __ b(ge, slow); - __ CompareObjectType(r0, r4, r4, SYMBOL_TYPE); + __ cmp(r4, Operand(SYMBOL_TYPE)); __ b(eq, slow); } else { __ CompareObjectType(r0, r4, r4, HEAP_NUMBER_TYPE); @@ -262,6 +262,8 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, if (cond != eq) { __ cmp(r4, Operand(FIRST_SPEC_OBJECT_TYPE)); __ b(ge, slow); + __ cmp(r4, Operand(SYMBOL_TYPE)); + __ b(eq, slow); // Normally here we fall through to return_equal, but undefined is // special: (undefined == undefined) == true, but // (undefined <= undefined) == false! See ECMAScript 11.8.5. diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index f71822e..9ce5a05 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -221,19 +221,22 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, // so we do the second best thing - test it ourselves. // They are both equal and they are not both Smis so both of them are not // Smis. If it's not a heap number, then return equal. + Register right_type = scratch; if ((cond == lt) || (cond == gt)) { - __ JumpIfObjectType(right, scratch, scratch, FIRST_SPEC_OBJECT_TYPE, slow, - ge); - __ JumpIfObjectType(right, scratch, scratch, SYMBOL_TYPE, slow, eq); + __ JumpIfObjectType(right, right_type, right_type, FIRST_SPEC_OBJECT_TYPE, + slow, ge); + __ Cmp(right_type, SYMBOL_TYPE); + __ B(eq, slow); } else if (cond == eq) { __ JumpIfHeapNumber(right, &heap_number); } else { - Register right_type = scratch; __ JumpIfObjectType(right, right_type, right_type, HEAP_NUMBER_TYPE, &heap_number); // Comparing JS objects with <=, >= is complicated. __ Cmp(right_type, FIRST_SPEC_OBJECT_TYPE); __ B(ge, slow); + __ Cmp(right_type, SYMBOL_TYPE); + __ B(eq, slow); // Normally here we fall through to return_equal, but undefined is // special: (undefined == undefined) == true, but // (undefined <= undefined) == false! See ECMAScript 11.8.5. diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index 484c81a..5b5e050 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -291,16 +291,16 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, // so we do the second best thing - test it ourselves. // They are both equal and they are not both Smis so both of them are not // Smis. If it's not a heap number, then return equal. + __ GetObjectType(a0, t4, t4); if (cc == less || cc == greater) { - __ GetObjectType(a0, t4, t4); __ Branch(slow, greater, t4, Operand(FIRST_SPEC_OBJECT_TYPE)); __ Branch(slow, eq, t4, Operand(SYMBOL_TYPE)); } else { - __ GetObjectType(a0, t4, t4); __ Branch(&heap_number, eq, t4, Operand(HEAP_NUMBER_TYPE)); // Comparing JS objects with <=, >= is complicated. if (cc != eq) { __ Branch(slow, greater, t4, Operand(FIRST_SPEC_OBJECT_TYPE)); + __ Branch(slow, eq, t4, Operand(SYMBOL_TYPE)); // Normally here we fall through to return_equal, but undefined is // special: (undefined == undefined) == true, but // (undefined <= undefined) == false! See ECMAScript 11.8.5. diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 0a872c1..fb9c4ee 100644 --- a/src/mips64/code-stubs-mips64.cc +++ b/src/mips64/code-stubs-mips64.cc @@ -287,16 +287,16 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, // so we do the second best thing - test it ourselves. // They are both equal and they are not both Smis so both of them are not // Smis. If it's not a heap number, then return equal. + __ GetObjectType(a0, t0, t0); if (cc == less || cc == greater) { - __ GetObjectType(a0, t0, t0); __ Branch(slow, greater, t0, Operand(FIRST_SPEC_OBJECT_TYPE)); __ Branch(slow, eq, t0, Operand(SYMBOL_TYPE)); } else { - __ GetObjectType(a0, t0, t0); __ Branch(&heap_number, eq, t0, Operand(HEAP_NUMBER_TYPE)); // Comparing JS objects with <=, >= is complicated. if (cc != eq) { __ Branch(slow, greater, t0, Operand(FIRST_SPEC_OBJECT_TYPE)); + __ Branch(slow, eq, t0, Operand(SYMBOL_TYPE)); // Normally here we fall through to return_equal, but undefined is // special: (undefined == undefined) == true, but // (undefined <= undefined) == false! See ECMAScript 11.8.5. diff --git a/src/ppc/code-stubs-ppc.cc b/src/ppc/code-stubs-ppc.cc index 94a2ae0..abcad08 100644 --- a/src/ppc/code-stubs-ppc.cc +++ b/src/ppc/code-stubs-ppc.cc @@ -262,7 +262,7 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, Label* slow, if (cond == lt || cond == gt) { __ CompareObjectType(r3, r7, r7, FIRST_SPEC_OBJECT_TYPE); __ bge(slow); - __ CompareObjectType(r3, r7, r7, SYMBOL_TYPE); + __ cmpi(r7, Operand(SYMBOL_TYPE)); __ beq(slow); } else { __ CompareObjectType(r3, r7, r7, HEAP_NUMBER_TYPE); @@ -271,6 +271,8 @@ static void EmitIdenticalObjectComparison(MacroAssembler* masm, Label* slow, if (cond != eq) { __ cmpi(r7, Operand(FIRST_SPEC_OBJECT_TYPE)); __ bge(slow); + __ cmpi(r7, Operand(SYMBOL_TYPE)); + __ beq(slow); // Normally here we fall through to return_equal, but undefined is // special: (undefined == undefined) == true, but // (undefined <= undefined) == false! See ECMAScript 11.8.5. diff --git a/test/mjsunit/es6/symbols.js b/test/mjsunit/es6/symbols.js index 7203f67..05601bd 100644 --- a/test/mjsunit/es6/symbols.js +++ b/test/mjsunit/es6/symbols.js @@ -507,30 +507,22 @@ TestGetOwnPropertySymbolsOnPrimitives(); function TestComparison() { - function f() { - var a = Symbol(); - a < a; - a > a; - a <= a; - a >= a; - } - - assertThrows(f, TypeError); - %OptimizeFunctionOnNextCall(f); - assertThrows(f, TypeError); - assertThrows(f, TypeError); - - function g() { - var a = Symbol(); - var b = Symbol(); - a < b; - a > b; - a <= b; - a >= b; - } - assertThrows(g, TypeError); - %OptimizeFunctionOnNextCall(g); - assertThrows(g, TypeError); - assertThrows(g, TypeError); + function lt() { var a = Symbol(); var b = Symbol(); a < b; } + function gt() { var a = Symbol(); var b = Symbol(); a > b; } + function le() { var a = Symbol(); var b = Symbol(); a <= b; } + function ge() { var a = Symbol(); var b = Symbol(); a >= b; } + function lt_same() { var a = Symbol(); a < a; } + function gt_same() { var a = Symbol(); a > a; } + function le_same() { var a = Symbol(); a <= a; } + function ge_same() { var a = Symbol(); a >= a; } + + var throwFuncs = [lt, gt, le, ge, lt_same, gt_same, le_same, ge_same]; + + for (var f of throwFuncs) { + assertThrows(f, TypeError); + %OptimizeFunctionOnNextCall(f); + assertThrows(f, TypeError); + assertThrows(f, TypeError); + } } TestComparison(); -- 2.7.4