From 0eb4f1d84be5cfc7f8e71f43ad19a82a24d4922c Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 26 Oct 2016 04:49:01 +0200 Subject: [PATCH] deps: back-port 73ee7943 from v8 upstream Original commit message: When instantiating a subclassed API function, the instance cache is avoided. There is currently no direct API yet to instantiate a Template while passing in a new.target. It probably makes sense to extend ObjectTemplate::NewInstance to accept a new.target, in line with Reflect.construct. BUG=v8:3330, v8:5001 Review-Url: https://codereview.chromium.org/1972613002 Cr-Commit-Position: refs/heads/master@{#36179} Fixes: https://github.com/nodejs/node/issues/9288 PR-URL: https://github.com/nodejs/node/pull/9293 Reviewed-By: Ali Ijaz Sheikh Reviewed-By: James M Snell Reviewed-By: Myles Borins --- deps/v8/src/api-natives.cc | 57 +++++++++++++------- deps/v8/src/api-natives.h | 3 +- deps/v8/src/builtins.cc | 8 +-- deps/v8/src/objects.cc | 61 ++++++++++----------- deps/v8/test/cctest/test-api.cc | 95 ++++++++++++++++++++++++++++++--- 5 files changed, 166 insertions(+), 58 deletions(-) diff --git a/deps/v8/src/api-natives.cc b/deps/v8/src/api-natives.cc index adf4b6af576431..fa735e523be5b6 100644 --- a/deps/v8/src/api-natives.cc +++ b/deps/v8/src/api-natives.cc @@ -17,6 +17,7 @@ namespace { MaybeHandle InstantiateObject(Isolate* isolate, Handle data, + Handle new_target, bool is_hidden_prototype); MaybeHandle InstantiateFunction(Isolate* isolate, @@ -31,7 +32,7 @@ MaybeHandle Instantiate(Isolate* isolate, Handle data, Handle::cast(data), name); } else if (data->IsObjectTemplateInfo()) { return InstantiateObject(isolate, Handle::cast(data), - false); + Handle(), false); } else { return data; } @@ -288,11 +289,25 @@ void UncacheTemplateInstantiation(Isolate* isolate, uint32_t serial_number) { MaybeHandle InstantiateObject(Isolate* isolate, Handle info, + Handle new_target, bool is_hidden_prototype) { - // Fast path. - Handle result; + Handle constructor; uint32_t serial_number = static_cast(Smi::cast(info->serial_number())->value()); + if (!new_target.is_null()) { + if (new_target->IsJSFunction() && + JSFunction::cast(*new_target)->shared()->function_data() == + info->constructor() && + JSFunction::cast(*new_target)->context()->native_context() == + isolate->context()->native_context()) { + constructor = Handle::cast(new_target); + } else { + // Disable caching for subclass instantiation. + serial_number = 0; + } + } + // Fast path. + Handle result; if (serial_number) { // Probe cache. auto cache = isolate->template_instantiations_cache(); @@ -305,20 +320,27 @@ MaybeHandle InstantiateObject(Isolate* isolate, } // Enter a new scope. Recursion could otherwise create a lot of handles. HandleScope scope(isolate); - auto constructor = handle(info->constructor(), isolate); - Handle cons; - if (constructor->IsUndefined()) { - cons = isolate->object_function(); - } else { - auto cons_templ = Handle::cast(constructor); - ASSIGN_RETURN_ON_EXCEPTION( - isolate, cons, InstantiateFunction(isolate, cons_templ), JSFunction); + + if (constructor.is_null()) { + Handle cons(info->constructor(), isolate); + if (cons->IsUndefined()) { + constructor = isolate->object_function(); + } else { + auto cons_templ = Handle::cast(cons); + ASSIGN_RETURN_ON_EXCEPTION(isolate, constructor, + InstantiateFunction(isolate, cons_templ), + JSObject); + } + + if (new_target.is_null()) new_target = constructor; } - auto object = isolate->factory()->NewJSObject(cons); + + Handle object; + ASSIGN_RETURN_ON_EXCEPTION(isolate, object, + JSObject::New(constructor, new_target), JSObject); ASSIGN_RETURN_ON_EXCEPTION( isolate, result, - ConfigureInstance(isolate, object, info, is_hidden_prototype), - JSFunction); + ConfigureInstance(isolate, object, info, is_hidden_prototype), JSObject); // TODO(dcarney): is this necessary? JSObject::MigrateSlowToFast(result, 0, "ApiNatives::InstantiateObject"); @@ -356,7 +378,7 @@ MaybeHandle InstantiateFunction(Isolate* isolate, isolate, prototype, InstantiateObject(isolate, Handle::cast(prototype_templ), - data->hidden_prototype()), + Handle(), data->hidden_prototype()), JSFunction); } auto parent = handle(data->parent_template(), isolate); @@ -448,12 +470,11 @@ MaybeHandle ApiNatives::InstantiateFunction( return ::v8::internal::InstantiateFunction(isolate, data); } - MaybeHandle ApiNatives::InstantiateObject( - Handle data) { + Handle data, Handle new_target) { Isolate* isolate = data->GetIsolate(); InvokeScope invoke_scope(isolate); - return ::v8::internal::InstantiateObject(isolate, data, false); + return ::v8::internal::InstantiateObject(isolate, data, new_target, false); } diff --git a/deps/v8/src/api-natives.h b/deps/v8/src/api-natives.h index 91f0b168d92830..66901fe965e715 100644 --- a/deps/v8/src/api-natives.h +++ b/deps/v8/src/api-natives.h @@ -23,7 +23,8 @@ class ApiNatives { Handle data); MUST_USE_RESULT static MaybeHandle InstantiateObject( - Handle data); + Handle data, + Handle new_target = Handle()); enum ApiInstanceType { JavaScriptObjectType, diff --git a/deps/v8/src/builtins.cc b/deps/v8/src/builtins.cc index 602905ef24b390..01f103101ced02 100644 --- a/deps/v8/src/builtins.cc +++ b/deps/v8/src/builtins.cc @@ -4262,9 +4262,11 @@ MUST_USE_RESULT MaybeHandle HandleApiCallHelper( } Handle instance_template( ObjectTemplateInfo::cast(fun_data->instance_template()), isolate); - ASSIGN_RETURN_ON_EXCEPTION(isolate, receiver, - ApiNatives::InstantiateObject(instance_template), - Object); + ASSIGN_RETURN_ON_EXCEPTION( + isolate, receiver, + ApiNatives::InstantiateObject(instance_template, + Handle::cast(new_target)), + Object); args[0] = *receiver; DCHECK_EQ(*receiver, *args.receiver()); } else { diff --git a/deps/v8/src/objects.cc b/deps/v8/src/objects.cc index fa45a091b12dfc..e2eefa8078bb9e 100644 --- a/deps/v8/src/objects.cc +++ b/deps/v8/src/objects.cc @@ -13080,50 +13080,51 @@ namespace { bool CanSubclassHaveInobjectProperties(InstanceType instance_type) { switch (instance_type) { - case JS_OBJECT_TYPE: - case JS_CONTEXT_EXTENSION_OBJECT_TYPE: - case JS_GENERATOR_OBJECT_TYPE: - case JS_MODULE_TYPE: - case JS_VALUE_TYPE: - case JS_DATE_TYPE: - case JS_ARRAY_TYPE: - case JS_MESSAGE_OBJECT_TYPE: case JS_ARRAY_BUFFER_TYPE: - case JS_TYPED_ARRAY_TYPE: + case JS_ARRAY_TYPE: + case JS_CONTEXT_EXTENSION_OBJECT_TYPE: case JS_DATA_VIEW_TYPE: - case JS_SET_TYPE: + case JS_DATE_TYPE: + case JS_FUNCTION_TYPE: + case JS_GENERATOR_OBJECT_TYPE: + case JS_MAP_ITERATOR_TYPE: case JS_MAP_TYPE: + case JS_MESSAGE_OBJECT_TYPE: + case JS_MODULE_TYPE: + case JS_OBJECT_TYPE: + case JS_PROMISE_TYPE: + case JS_REGEXP_TYPE: case JS_SET_ITERATOR_TYPE: - case JS_MAP_ITERATOR_TYPE: + case JS_SET_TYPE: + case JS_SPECIAL_API_OBJECT_TYPE: + case JS_TYPED_ARRAY_TYPE: + case JS_VALUE_TYPE: case JS_WEAK_MAP_TYPE: case JS_WEAK_SET_TYPE: - case JS_PROMISE_TYPE: - case JS_REGEXP_TYPE: - case JS_FUNCTION_TYPE: return true; - case JS_BOUND_FUNCTION_TYPE: - case JS_PROXY_TYPE: - case JS_GLOBAL_PROXY_TYPE: - case JS_GLOBAL_OBJECT_TYPE: + case BYTECODE_ARRAY_TYPE: + case BYTE_ARRAY_TYPE: + case CELL_TYPE: + case CODE_TYPE: + case FILLER_TYPE: case FIXED_ARRAY_TYPE: case FIXED_DOUBLE_ARRAY_TYPE: - case ODDBALL_TYPE: case FOREIGN_TYPE: - case MAP_TYPE: - case CODE_TYPE: - case CELL_TYPE: - case PROPERTY_CELL_TYPE: - case WEAK_CELL_TYPE: - case SYMBOL_TYPE: - case BYTECODE_ARRAY_TYPE: + case FREE_SPACE_TYPE: case HEAP_NUMBER_TYPE: + case JS_BOUND_FUNCTION_TYPE: + case JS_GLOBAL_OBJECT_TYPE: + case JS_GLOBAL_PROXY_TYPE: + case JS_PROXY_TYPE: + case MAP_TYPE: case MUTABLE_HEAP_NUMBER_TYPE: - case SIMD128_VALUE_TYPE: - case FILLER_TYPE: - case BYTE_ARRAY_TYPE: - case FREE_SPACE_TYPE: + case ODDBALL_TYPE: + case PROPERTY_CELL_TYPE: case SHARED_FUNCTION_INFO_TYPE: + case SIMD128_VALUE_TYPE: + case SYMBOL_TYPE: + case WEAK_CELL_TYPE: #define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size) \ case FIXED_##TYPE##_ARRAY_TYPE: diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index b9cb9bd856cc39..a44239c244f4a5 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -2142,6 +2142,95 @@ THREADED_TEST(TestObjectTemplateInheritedWithPrototype2) { Constructor_GetFunction_New); } +THREADED_TEST(TestObjectTemplateClassInheritance) { + LocalContext env; + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + + Local fun_A = v8::FunctionTemplate::New(isolate); + fun_A->SetClassName(v8_str("A")); + + Local templ_A = fun_A->InstanceTemplate(); + templ_A->SetNativeDataProperty(v8_str("nirk"), GetNirk); + templ_A->SetNativeDataProperty(v8_str("rino"), GetRino); + + Local fun_B = v8::FunctionTemplate::New(isolate); + v8::Local class_name = v8_str("B"); + fun_B->SetClassName(class_name); + fun_B->Inherit(fun_A); + + v8::Local subclass_name = v8_str("C"); + v8::Local b_proto; + v8::Local c_proto; + // Perform several iterations to make sure the cache doesn't break + // subclassing. + for (int i = 0; i < 3; i++) { + Local function_B = + fun_B->GetFunction(env.local()).ToLocalChecked(); + if (i == 0) { + CHECK(env->Global()->Set(env.local(), class_name, function_B).FromJust()); + CompileRun("class C extends B {}"); + b_proto = + CompileRun("B.prototype")->ToObject(env.local()).ToLocalChecked(); + c_proto = + CompileRun("C.prototype")->ToObject(env.local()).ToLocalChecked(); + CHECK(b_proto->Equals(env.local(), c_proto->GetPrototype()).FromJust()); + } + Local instance = + CompileRun("new C()")->ToObject(env.local()).ToLocalChecked(); + CHECK(c_proto->Equals(env.local(), instance->GetPrototype()).FromJust()); + + CHECK(subclass_name->StrictEquals(instance->GetConstructorName())); + CHECK(env->Global()->Set(env.local(), v8_str("o"), instance).FromJust()); + + CHECK_EQ(900, CompileRun("o.nirk")->IntegerValue(env.local()).FromJust()); + CHECK_EQ(560, CompileRun("o.rino")->IntegerValue(env.local()).FromJust()); + } +} + +static void NamedPropertyGetterWhichReturns42( + Local name, const v8::PropertyCallbackInfo& info) { + info.GetReturnValue().Set(v8_num(42)); +} + +THREADED_TEST(TestObjectTemplateReflectConstruct) { + LocalContext env; + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + + Local fun_B = v8::FunctionTemplate::New(isolate); + fun_B->InstanceTemplate()->SetHandler( + v8::NamedPropertyHandlerConfiguration(NamedPropertyGetterWhichReturns42)); + v8::Local class_name = v8_str("B"); + fun_B->SetClassName(class_name); + + v8::Local subclass_name = v8_str("C"); + v8::Local b_proto; + v8::Local c_proto; + // Perform several iterations to make sure the cache doesn't break + // subclassing. + for (int i = 0; i < 3; i++) { + Local function_B = + fun_B->GetFunction(env.local()).ToLocalChecked(); + if (i == 0) { + CHECK(env->Global()->Set(env.local(), class_name, function_B).FromJust()); + CompileRun("function C() {}"); + c_proto = + CompileRun("C.prototype")->ToObject(env.local()).ToLocalChecked(); + } + Local instance = CompileRun("Reflect.construct(B, [], C)") + ->ToObject(env.local()) + .ToLocalChecked(); + CHECK(c_proto->Equals(env.local(), instance->GetPrototype()).FromJust()); + + CHECK(subclass_name->StrictEquals(instance->GetConstructorName())); + CHECK(env->Global()->Set(env.local(), v8_str("o"), instance).FromJust()); + + CHECK_EQ(42, CompileRun("o.nirk")->IntegerValue(env.local()).FromJust()); + CHECK_EQ(42, CompileRun("o.rino")->IntegerValue(env.local()).FromJust()); + } +} + static void GetFlabby(const v8::FunctionCallbackInfo& args) { ApiTestFuzzer::Fuzz(); args.GetReturnValue().Set(v8_num(17.2)); @@ -18765,12 +18854,6 @@ TEST(SetterOnConstructorPrototype) { } -static void NamedPropertyGetterWhichReturns42( - Local name, const v8::PropertyCallbackInfo& info) { - info.GetReturnValue().Set(v8_num(42)); -} - - static void NamedPropertySetterWhichSetsYOnThisTo23( Local name, Local value, const v8::PropertyCallbackInfo& info) {