From 7cee52948fc2ef0f9f5d0981dc9028b3f001b200 Mon Sep 17 00:00:00 2001 From: "dcarney@chromium.org" Date: Fri, 17 Jan 2014 10:34:43 +0000 Subject: [PATCH] Fix stub-invoked setter callback handling. When invoking a setter callback for a property using JSObject::SetPropertyWithCallback(),the callback arguments includes a correct pair of receiver and holder objects. Such a pair of _possibly different_ arguments (receiver, holder) must also be supplied when invoking the same setter callback from JITed code, when the setter is invoked through the StoreCallbackProperty stub. An example where this matters are the accessor properties kept on the global scope of Worker (i.e., properties kept on the global object itself, and not on its prototype.) Conflating the receiver with the holder leads to general confusion when attempting to fetch out the wrapper object. LOG=N R=dcarney@chromium.org, dcarney BUG=239669 Review URL: https://codereview.chromium.org/139263008 Patch from Sigbjorn Finne . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18658 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 7 ++++--- src/ia32/stub-cache-ia32.cc | 7 ++++--- src/mips/stub-cache-mips.cc | 7 ++++--- src/stub-cache.cc | 15 ++++++++------- src/x64/stub-cache-x64.cc | 7 ++++--- test/cctest/test-api.cc | 24 ++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index a20702c22c..0fddbedca8 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -2518,13 +2518,14 @@ Handle StoreStubCompiler::CompileStoreCallback( Handle holder, Handle name, Handle callback) { - HandlerFrontend(IC::CurrentTypeOf(object, isolate()), - receiver(), holder, name); + Register holder_reg = HandlerFrontend( + IC::CurrentTypeOf(object, isolate()), receiver(), holder, name); // Stub never generated for non-global objects that require access checks. ASSERT(holder->IsJSGlobalProxy() || !holder->IsAccessCheckNeeded()); __ push(receiver()); // receiver + __ push(holder_reg); __ mov(ip, Operand(callback)); // callback info __ push(ip); __ mov(ip, Operand(name)); @@ -2533,7 +2534,7 @@ Handle StoreStubCompiler::CompileStoreCallback( // Do tail-call to the runtime system. ExternalReference store_callback_property = ExternalReference(IC_Utility(IC::kStoreCallbackProperty), isolate()); - __ TailCallExternalReference(store_callback_property, 4, 1); + __ TailCallExternalReference(store_callback_property, 5, 1); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index d974940d7c..800add864d 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -2638,11 +2638,12 @@ Handle StoreStubCompiler::CompileStoreCallback( Handle holder, Handle name, Handle callback) { - HandlerFrontend(IC::CurrentTypeOf(object, isolate()), - receiver(), holder, name); + Register holder_reg = HandlerFrontend( + IC::CurrentTypeOf(object, isolate()), receiver(), holder, name); __ pop(scratch1()); // remove the return address __ push(receiver()); + __ push(holder_reg); __ Push(callback); __ Push(name); __ push(value()); @@ -2651,7 +2652,7 @@ Handle StoreStubCompiler::CompileStoreCallback( // Do tail-call to the runtime system. ExternalReference store_callback_property = ExternalReference(IC_Utility(IC::kStoreCallbackProperty), isolate()); - __ TailCallExternalReference(store_callback_property, 4, 1); + __ TailCallExternalReference(store_callback_property, 5, 1); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/src/mips/stub-cache-mips.cc b/src/mips/stub-cache-mips.cc index b7948b14d6..3fb322b503 100644 --- a/src/mips/stub-cache-mips.cc +++ b/src/mips/stub-cache-mips.cc @@ -2514,14 +2514,15 @@ Handle StoreStubCompiler::CompileStoreCallback( Handle holder, Handle name, Handle callback) { - HandlerFrontend(IC::CurrentTypeOf(object, isolate()), - receiver(), holder, name); + Register holder_reg = HandlerFrontend( + IC::CurrentTypeOf(object, isolate()), receiver(), holder, name); // Stub never generated for non-global objects that require access // checks. ASSERT(holder->IsJSGlobalProxy() || !holder->IsAccessCheckNeeded()); __ push(receiver()); // Receiver. + __ push(holder_reg); __ li(at, Operand(callback)); // Callback info. __ push(at); __ li(at, Operand(name)); @@ -2530,7 +2531,7 @@ Handle StoreStubCompiler::CompileStoreCallback( // Do tail-call to the runtime system. ExternalReference store_callback_property = ExternalReference(IC_Utility(IC::kStoreCallbackProperty), isolate()); - __ TailCallExternalReference(store_callback_property, 4, 1); + __ TailCallExternalReference(store_callback_property, 5, 1); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/src/stub-cache.cc b/src/stub-cache.cc index cb8d29402b..eb422a1046 100644 --- a/src/stub-cache.cc +++ b/src/stub-cache.cc @@ -810,24 +810,25 @@ void StubCache::CollectMatchingMaps(SmallMapList* types, RUNTIME_FUNCTION(MaybeObject*, StoreCallbackProperty) { - JSObject* recv = JSObject::cast(args[0]); - ExecutableAccessorInfo* callback = ExecutableAccessorInfo::cast(args[1]); + JSObject* receiver = JSObject::cast(args[0]); + JSObject* holder = JSObject::cast(args[1]); + ExecutableAccessorInfo* callback = ExecutableAccessorInfo::cast(args[2]); Address setter_address = v8::ToCData
(callback->setter()); v8::AccessorSetterCallback fun = FUNCTION_CAST(setter_address); ASSERT(fun != NULL); - ASSERT(callback->IsCompatibleReceiver(recv)); - Handle name = args.at(2); - Handle value = args.at(3); + ASSERT(callback->IsCompatibleReceiver(receiver)); + Handle name = args.at(3); + Handle value = args.at(4); HandleScope scope(isolate); // TODO(rossberg): Support symbols in the API. if (name->IsSymbol()) return *value; Handle str = Handle::cast(name); - LOG(isolate, ApiNamedPropertyAccess("store", recv, *name)); + LOG(isolate, ApiNamedPropertyAccess("store", receiver, *name)); PropertyCallbackArguments - custom_args(isolate, callback->data(), recv, recv); + custom_args(isolate, callback->data(), receiver, holder); custom_args.Call(fun, v8::Utils::ToLocal(str), v8::Utils::ToLocal(value)); RETURN_IF_SCHEDULED_EXCEPTION(isolate); return *value; diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index b0bdf70a4e..f7dc2cb316 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -2542,11 +2542,12 @@ Handle StoreStubCompiler::CompileStoreCallback( Handle holder, Handle name, Handle callback) { - HandlerFrontend(IC::CurrentTypeOf(object, isolate()), - receiver(), holder, name); + Register holder_reg = HandlerFrontend( + IC::CurrentTypeOf(object, isolate()), receiver(), holder, name); __ PopReturnAddressTo(scratch1()); __ push(receiver()); + __ push(holder_reg); __ Push(callback); // callback info __ Push(name); __ push(value()); @@ -2555,7 +2556,7 @@ Handle StoreStubCompiler::CompileStoreCallback( // Do tail-call to the runtime system. ExternalReference store_callback_property = ExternalReference(IC_Utility(IC::kStoreCallbackProperty), isolate()); - __ TailCallExternalReference(store_callback_property, 4, 1); + __ TailCallExternalReference(store_callback_property, 5, 1); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index d8b714c050..fc4d630ecc 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -21735,3 +21735,27 @@ TEST(EscapeableHandleScope) { } } } + + +static void SetterWhichExpectsThisAndHolderToDiffer( + Local, Local, const v8::PropertyCallbackInfo& info) { + CHECK(info.Holder() != info.This()); +} + + +TEST(Regress239669) { + LocalContext context; + v8::Isolate* isolate = context->GetIsolate(); + v8::HandleScope scope(isolate); + Local templ = ObjectTemplate::New(isolate); + templ->SetAccessor(v8_str("x"), 0, SetterWhichExpectsThisAndHolderToDiffer); + context->Global()->Set(v8_str("P"), templ->NewInstance()); + CompileRun( + "function C1() {" + " this.x = 23;" + "};" + "C1.prototype = P;" + "for (var i = 0; i < 4; i++ ) {" + " new C1();" + "}"); +}