From eb1c9e2723d1d371699c6251aa56fd52535927a6 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Wed, 8 Jun 2016 23:22:07 -0700 Subject: [PATCH] [es6] Fix prototype chain walk for instanceof. When walking up the prototype chain during OrdinaryHasInstance, we first check if the current prototype equals the expected one, and only afterwards check the current prototype against null. That's obviously wrong if we check something like Proxy, whose prototype is null. R=yangguo@chromium.org BUG=v8:5085 Review-Url: https://codereview.chromium.org/2041103007 Cr-Commit-Position: refs/heads/master@{#36840} --- src/code-stub-assembler.cc | 2 +- src/compiler/js-typed-lowering.cc | 21 ++++++++++--------- src/crankshaft/arm/lithium-codegen-arm.cc | 4 ++-- src/crankshaft/arm64/lithium-codegen-arm64.cc | 4 ++-- src/crankshaft/ia32/lithium-codegen-ia32.cc | 4 ++-- src/crankshaft/mips/lithium-codegen-mips.cc | 2 +- .../mips64/lithium-codegen-mips64.cc | 2 +- src/crankshaft/ppc/lithium-codegen-ppc.cc | 4 ++-- src/crankshaft/s390/lithium-codegen-s390.cc | 4 ++-- src/crankshaft/x64/lithium-codegen-x64.cc | 5 ++--- src/crankshaft/x87/lithium-codegen-x87.cc | 4 ++-- test/mjsunit/regress/regress-5085.js | 14 +++++++++++++ 12 files changed, 42 insertions(+), 28 deletions(-) create mode 100644 test/mjsunit/regress/regress-5085.js diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index cdd36b30d1..4f2d5ad8ae 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -1956,8 +1956,8 @@ Node* CodeStubAssembler::OrdinaryHasInstance(Node* context, Node* callable, // Check the current {object} prototype. Node* object_prototype = LoadMapPrototype(object_map); - GotoIf(WordEqual(object_prototype, callable_prototype), &return_true); GotoIf(WordEqual(object_prototype, NullConstant()), &return_false); + GotoIf(WordEqual(object_prototype, callable_prototype), &return_true); // Continue with the prototype. var_object_map.Bind(LoadMap(object_prototype)); diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index faf1251f7d..32316bf323 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -1267,6 +1267,17 @@ Reduction JSTypedLowering::ReduceJSInstanceOf(Node* node) { simplified()->LoadField(AccessBuilder::ForMapPrototype()), loop_object_map, loop_effect, control); + // If not, check if object prototype is the null prototype. + Node* null_proto = + graph()->NewNode(simplified()->ReferenceEqual(r.right_type()), + object_prototype, jsgraph()->NullConstant()); + Node* branch_null_proto = graph()->NewNode( + common()->Branch(BranchHint::kFalse), null_proto, control); + Node* if_null_proto = graph()->NewNode(common()->IfTrue(), branch_null_proto); + Node* e_null_proto = effect; + + control = graph()->NewNode(common()->IfFalse(), branch_null_proto); + // Check if object prototype is equal to function prototype. Node* eq_proto = graph()->NewNode(simplified()->ReferenceEqual(r.right_type()), @@ -1278,16 +1289,6 @@ Reduction JSTypedLowering::ReduceJSInstanceOf(Node* node) { control = graph()->NewNode(common()->IfFalse(), branch_eq_proto); - // If not, check if object prototype is the null prototype. - Node* null_proto = - graph()->NewNode(simplified()->ReferenceEqual(r.right_type()), - object_prototype, jsgraph()->NullConstant()); - Node* branch_null_proto = graph()->NewNode( - common()->Branch(BranchHint::kFalse), null_proto, control); - Node* if_null_proto = graph()->NewNode(common()->IfTrue(), branch_null_proto); - Node* e_null_proto = effect; - - control = graph()->NewNode(common()->IfFalse(), branch_null_proto); Node* load_object_map = effect = graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()), object_prototype, effect, control); diff --git a/src/crankshaft/arm/lithium-codegen-arm.cc b/src/crankshaft/arm/lithium-codegen-arm.cc index 92ba70e587..4fd039554c 100644 --- a/src/crankshaft/arm/lithium-codegen-arm.cc +++ b/src/crankshaft/arm/lithium-codegen-arm.cc @@ -2525,10 +2525,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(eq, instr, Deoptimizer::kProxy); __ ldr(object_prototype, FieldMemOperand(object_map, Map::kPrototypeOffset)); - __ cmp(object_prototype, prototype); - EmitTrueBranch(instr, eq); __ CompareRoot(object_prototype, Heap::kNullValueRootIndex); EmitFalseBranch(instr, eq); + __ cmp(object_prototype, prototype); + EmitTrueBranch(instr, eq); __ ldr(object_map, FieldMemOperand(object_prototype, HeapObject::kMapOffset)); __ b(&loop); } diff --git a/src/crankshaft/arm64/lithium-codegen-arm64.cc b/src/crankshaft/arm64/lithium-codegen-arm64.cc index d939341466..5e6b905af0 100644 --- a/src/crankshaft/arm64/lithium-codegen-arm64.cc +++ b/src/crankshaft/arm64/lithium-codegen-arm64.cc @@ -2832,10 +2832,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(eq, instr, Deoptimizer::kProxy); __ Ldr(object_prototype, FieldMemOperand(object_map, Map::kPrototypeOffset)); - __ Cmp(object_prototype, prototype); - __ B(eq, instr->TrueLabel(chunk_)); __ CompareRoot(object_prototype, Heap::kNullValueRootIndex); __ B(eq, instr->FalseLabel(chunk_)); + __ Cmp(object_prototype, prototype); + __ B(eq, instr->TrueLabel(chunk_)); __ Ldr(object_map, FieldMemOperand(object_prototype, HeapObject::kMapOffset)); __ B(&loop); } diff --git a/src/crankshaft/ia32/lithium-codegen-ia32.cc b/src/crankshaft/ia32/lithium-codegen-ia32.cc index 203ae9e3b6..ff05c0ae6a 100644 --- a/src/crankshaft/ia32/lithium-codegen-ia32.cc +++ b/src/crankshaft/ia32/lithium-codegen-ia32.cc @@ -2316,10 +2316,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(equal, instr, Deoptimizer::kProxy); __ mov(object_prototype, FieldOperand(object_map, Map::kPrototypeOffset)); - __ cmp(object_prototype, prototype); - EmitTrueBranch(instr, equal); __ cmp(object_prototype, factory()->null_value()); EmitFalseBranch(instr, equal); + __ cmp(object_prototype, prototype); + EmitTrueBranch(instr, equal); __ mov(object_map, FieldOperand(object_prototype, HeapObject::kMapOffset)); __ jmp(&loop); } diff --git a/src/crankshaft/mips/lithium-codegen-mips.cc b/src/crankshaft/mips/lithium-codegen-mips.cc index c4faf69635..9a194772c6 100644 --- a/src/crankshaft/mips/lithium-codegen-mips.cc +++ b/src/crankshaft/mips/lithium-codegen-mips.cc @@ -2415,9 +2415,9 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( Operand(JS_PROXY_TYPE)); __ lw(object_prototype, FieldMemOperand(object_map, Map::kPrototypeOffset)); - EmitTrueBranch(instr, eq, object_prototype, Operand(prototype)); __ LoadRoot(at, Heap::kNullValueRootIndex); EmitFalseBranch(instr, eq, object_prototype, Operand(at)); + EmitTrueBranch(instr, eq, object_prototype, Operand(prototype)); __ Branch(USE_DELAY_SLOT, &loop); __ lw(object_map, FieldMemOperand(object_prototype, HeapObject::kMapOffset)); } diff --git a/src/crankshaft/mips64/lithium-codegen-mips64.cc b/src/crankshaft/mips64/lithium-codegen-mips64.cc index 12daa1031e..8c7f1dc641 100644 --- a/src/crankshaft/mips64/lithium-codegen-mips64.cc +++ b/src/crankshaft/mips64/lithium-codegen-mips64.cc @@ -2535,9 +2535,9 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( Operand(JS_PROXY_TYPE)); __ ld(object_prototype, FieldMemOperand(object_map, Map::kPrototypeOffset)); - EmitTrueBranch(instr, eq, object_prototype, Operand(prototype)); __ LoadRoot(at, Heap::kNullValueRootIndex); EmitFalseBranch(instr, eq, object_prototype, Operand(at)); + EmitTrueBranch(instr, eq, object_prototype, Operand(prototype)); __ Branch(&loop, USE_DELAY_SLOT); __ ld(object_map, FieldMemOperand(object_prototype, HeapObject::kMapOffset)); // In delay slot. diff --git a/src/crankshaft/ppc/lithium-codegen-ppc.cc b/src/crankshaft/ppc/lithium-codegen-ppc.cc index 3e2fa7f2fa..c7a5cd1c1c 100644 --- a/src/crankshaft/ppc/lithium-codegen-ppc.cc +++ b/src/crankshaft/ppc/lithium-codegen-ppc.cc @@ -2589,10 +2589,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(eq, instr, Deoptimizer::kProxy); __ LoadP(object_prototype, FieldMemOperand(object_map, Map::kPrototypeOffset)); - __ cmp(object_prototype, prototype); - EmitTrueBranch(instr, eq); __ CompareRoot(object_prototype, Heap::kNullValueRootIndex); EmitFalseBranch(instr, eq); + __ cmp(object_prototype, prototype); + EmitTrueBranch(instr, eq); __ LoadP(object_map, FieldMemOperand(object_prototype, HeapObject::kMapOffset)); __ b(&loop); diff --git a/src/crankshaft/s390/lithium-codegen-s390.cc b/src/crankshaft/s390/lithium-codegen-s390.cc index e34859334f..c9191e76f3 100644 --- a/src/crankshaft/s390/lithium-codegen-s390.cc +++ b/src/crankshaft/s390/lithium-codegen-s390.cc @@ -2567,10 +2567,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(eq, instr, Deoptimizer::kProxy); __ LoadP(object_prototype, FieldMemOperand(object_map, Map::kPrototypeOffset)); - __ CmpP(object_prototype, prototype); - EmitTrueBranch(instr, eq); __ CompareRoot(object_prototype, Heap::kNullValueRootIndex); EmitFalseBranch(instr, eq); + __ CmpP(object_prototype, prototype); + EmitTrueBranch(instr, eq); __ LoadP(object_map, FieldMemOperand(object_prototype, HeapObject::kMapOffset)); __ b(&loop); diff --git a/src/crankshaft/x64/lithium-codegen-x64.cc b/src/crankshaft/x64/lithium-codegen-x64.cc index 16adc2c66c..0746d3b434 100644 --- a/src/crankshaft/x64/lithium-codegen-x64.cc +++ b/src/crankshaft/x64/lithium-codegen-x64.cc @@ -2456,7 +2456,6 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( Label loop; __ bind(&loop); - // Deoptimize if the object needs to be access checked. __ testb(FieldOperand(object_map, Map::kBitFieldOffset), Immediate(1 << Map::kIsAccessCheckNeeded)); @@ -2466,10 +2465,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(equal, instr, Deoptimizer::kProxy); __ movp(object_prototype, FieldOperand(object_map, Map::kPrototypeOffset)); - __ cmpp(object_prototype, prototype); - EmitTrueBranch(instr, equal); __ CompareRoot(object_prototype, Heap::kNullValueRootIndex); EmitFalseBranch(instr, equal); + __ cmpp(object_prototype, prototype); + EmitTrueBranch(instr, equal); __ movp(object_map, FieldOperand(object_prototype, HeapObject::kMapOffset)); __ jmp(&loop); } diff --git a/src/crankshaft/x87/lithium-codegen-x87.cc b/src/crankshaft/x87/lithium-codegen-x87.cc index ccea6958ab..71f68d581e 100644 --- a/src/crankshaft/x87/lithium-codegen-x87.cc +++ b/src/crankshaft/x87/lithium-codegen-x87.cc @@ -2602,10 +2602,10 @@ void LCodeGen::DoHasInPrototypeChainAndBranch( DeoptimizeIf(equal, instr, Deoptimizer::kProxy); __ mov(object_prototype, FieldOperand(object_map, Map::kPrototypeOffset)); - __ cmp(object_prototype, prototype); - EmitTrueBranch(instr, equal); __ cmp(object_prototype, factory()->null_value()); EmitFalseBranch(instr, equal); + __ cmp(object_prototype, prototype); + EmitTrueBranch(instr, equal); __ mov(object_map, FieldOperand(object_prototype, HeapObject::kMapOffset)); __ jmp(&loop); } diff --git a/test/mjsunit/regress/regress-5085.js b/test/mjsunit/regress/regress-5085.js new file mode 100644 index 0000000000..0ed034dc2d --- /dev/null +++ b/test/mjsunit/regress/regress-5085.js @@ -0,0 +1,14 @@ +// Copyright 2016 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +function foo(x) { + return x instanceof Proxy; +} + +assertFalse(foo({})); +assertFalse(foo({})); +%OptimizeFunctionOnNextCall(foo); +assertFalse(foo({}));