From d5a398e8c912330006f581a725dc305b7edc619f Mon Sep 17 00:00:00 2001 From: Igor Sheludko Date: Wed, 16 Aug 2017 13:53:05 +0200 Subject: [PATCH] Fix spec violation in Function.prototype.bind. '9. Let targetName be ? Get(Target, "name").' didn't produce required side effects. Bug: v8:6712 Change-Id: Iebf007b4e93ebbf9c6c85c9729d972a8c1a7b129 Reviewed-on: https://chromium-review.googlesource.com/616727 Reviewed-by: Camillo Bruni Commit-Queue: Igor Sheludko Cr-Commit-Position: refs/heads/master@{#47393} --- src/builtins/builtins-function.cc | 8 ++++---- src/lookup.cc | 6 ++++++ src/lookup.h | 1 + test/mjsunit/regress/regress-v8-6712.js | 16 ++++++++++++++++ 4 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 test/mjsunit/regress/regress-v8-6712.js diff --git a/src/builtins/builtins-function.cc b/src/builtins/builtins-function.cc index 4f5a82cf97..b94220603c 100644 --- a/src/builtins/builtins-function.cc +++ b/src/builtins/builtins-function.cc @@ -252,14 +252,14 @@ Object* DoFunctionBind(Isolate* isolate, BuiltinArguments args) { } // Setup the "name" property based on the "name" of the {target}. - // If the targets name is the default JSFunction accessor, we can keep the + // If the target's name is the default JSFunction accessor, we can keep the // accessor that's installed by default on the JSBoundFunction. It lazily // computes the value from the underlying internal name. - LookupIterator name_lookup(target, isolate->factory()->name_string(), target, - LookupIterator::OWN); + LookupIterator name_lookup(target, isolate->factory()->name_string(), target); if (!target->IsJSFunction() || name_lookup.state() != LookupIterator::ACCESSOR || - !name_lookup.GetAccessors()->IsAccessorInfo()) { + !name_lookup.GetAccessors()->IsAccessorInfo() || + (name_lookup.IsFound() && !name_lookup.HolderIsReceiver())) { Handle target_name; ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, target_name, Object::GetProperty(&name_lookup)); diff --git a/src/lookup.cc b/src/lookup.cc index 3281c9bf53..7c5b677d98 100644 --- a/src/lookup.cc +++ b/src/lookup.cc @@ -629,6 +629,12 @@ void LookupIterator::TransitionToAccessorPair(Handle pair, } } +bool LookupIterator::HolderIsReceiver() const { + DCHECK(has_property_ || state_ == INTERCEPTOR || state_ == JSPROXY); + // Optimization that only works if configuration_ is not mutable. + if (!check_prototype_chain()) return true; + return *receiver_ == *holder_; +} bool LookupIterator::HolderIsReceiverOrHiddenPrototype() const { DCHECK(has_property_ || state_ == INTERCEPTOR || state_ == JSPROXY); diff --git a/src/lookup.h b/src/lookup.h index d08c8e1d3d..b7b73d5ded 100644 --- a/src/lookup.h +++ b/src/lookup.h @@ -197,6 +197,7 @@ class V8_EXPORT_PRIVATE LookupIterator final BASE_EMBEDDED { return Handle::cast(holder_); } + bool HolderIsReceiver() const; bool HolderIsReceiverOrHiddenPrototype() const; bool check_prototype_chain() const { diff --git a/test/mjsunit/regress/regress-v8-6712.js b/test/mjsunit/regress/regress-v8-6712.js new file mode 100644 index 0000000000..0c5d5442fa --- /dev/null +++ b/test/mjsunit/regress/regress-v8-6712.js @@ -0,0 +1,16 @@ +// Copyright 2017 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. + +var log = []; + +function f() {} +Object.defineProperty(Function.prototype, "name", { + get() { log.push("getter"); return "ok"; } +}); +delete f.name; +var b = f.bind(); +assertEquals("bound ok", b.name); +assertEquals("bound ok", b.name); +assertEquals("bound ok", b.name); +assertEquals(["getter"], log);