Skip to content

Commit

Permalink
deps: back-port 73ee7943 from v8 upstream
Browse files Browse the repository at this point in the history
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@{nodejs#36179}

Fixes: nodejs#9288
PR-URL: nodejs#9293
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
  • Loading branch information
bnoordhuis committed Mar 2, 2017
1 parent a38d091 commit 0eb4f1d
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 58 deletions.
57 changes: 39 additions & 18 deletions deps/v8/src/api-natives.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace {

MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
Handle<ObjectTemplateInfo> data,
Handle<JSReceiver> new_target,
bool is_hidden_prototype);

MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
Expand All @@ -31,7 +32,7 @@ MaybeHandle<Object> Instantiate(Isolate* isolate, Handle<Object> data,
Handle<FunctionTemplateInfo>::cast(data), name);
} else if (data->IsObjectTemplateInfo()) {
return InstantiateObject(isolate, Handle<ObjectTemplateInfo>::cast(data),
false);
Handle<JSReceiver>(), false);
} else {
return data;
}
Expand Down Expand Up @@ -288,11 +289,25 @@ void UncacheTemplateInstantiation(Isolate* isolate, uint32_t serial_number) {

MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
Handle<ObjectTemplateInfo> info,
Handle<JSReceiver> new_target,
bool is_hidden_prototype) {
// Fast path.
Handle<JSObject> result;
Handle<JSFunction> constructor;
uint32_t serial_number =
static_cast<uint32_t>(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<JSFunction>::cast(new_target);
} else {
// Disable caching for subclass instantiation.
serial_number = 0;
}
}
// Fast path.
Handle<JSObject> result;
if (serial_number) {
// Probe cache.
auto cache = isolate->template_instantiations_cache();
Expand All @@ -305,20 +320,27 @@ MaybeHandle<JSObject> 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<JSFunction> cons;
if (constructor->IsUndefined()) {
cons = isolate->object_function();
} else {
auto cons_templ = Handle<FunctionTemplateInfo>::cast(constructor);
ASSIGN_RETURN_ON_EXCEPTION(
isolate, cons, InstantiateFunction(isolate, cons_templ), JSFunction);

if (constructor.is_null()) {
Handle<Object> cons(info->constructor(), isolate);
if (cons->IsUndefined()) {
constructor = isolate->object_function();
} else {
auto cons_templ = Handle<FunctionTemplateInfo>::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<JSObject> 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");

Expand Down Expand Up @@ -356,7 +378,7 @@ MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
isolate, prototype,
InstantiateObject(isolate,
Handle<ObjectTemplateInfo>::cast(prototype_templ),
data->hidden_prototype()),
Handle<JSReceiver>(), data->hidden_prototype()),
JSFunction);
}
auto parent = handle(data->parent_template(), isolate);
Expand Down Expand Up @@ -448,12 +470,11 @@ MaybeHandle<JSFunction> ApiNatives::InstantiateFunction(
return ::v8::internal::InstantiateFunction(isolate, data);
}


MaybeHandle<JSObject> ApiNatives::InstantiateObject(
Handle<ObjectTemplateInfo> data) {
Handle<ObjectTemplateInfo> data, Handle<JSReceiver> 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);
}


Expand Down
3 changes: 2 additions & 1 deletion deps/v8/src/api-natives.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class ApiNatives {
Handle<FunctionTemplateInfo> data);

MUST_USE_RESULT static MaybeHandle<JSObject> InstantiateObject(
Handle<ObjectTemplateInfo> data);
Handle<ObjectTemplateInfo> data,
Handle<JSReceiver> new_target = Handle<JSReceiver>());

enum ApiInstanceType {
JavaScriptObjectType,
Expand Down
8 changes: 5 additions & 3 deletions deps/v8/src/builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4262,9 +4262,11 @@ MUST_USE_RESULT MaybeHandle<Object> HandleApiCallHelper(
}
Handle<ObjectTemplateInfo> 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<JSReceiver>::cast(new_target)),
Object);
args[0] = *receiver;
DCHECK_EQ(*receiver, *args.receiver());
} else {
Expand Down
61 changes: 31 additions & 30 deletions deps/v8/src/objects.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
95 changes: 89 additions & 6 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::FunctionTemplate> fun_A = v8::FunctionTemplate::New(isolate);
fun_A->SetClassName(v8_str("A"));

Local<ObjectTemplate> templ_A = fun_A->InstanceTemplate();
templ_A->SetNativeDataProperty(v8_str("nirk"), GetNirk);
templ_A->SetNativeDataProperty(v8_str("rino"), GetRino);

Local<v8::FunctionTemplate> fun_B = v8::FunctionTemplate::New(isolate);
v8::Local<v8::String> class_name = v8_str("B");
fun_B->SetClassName(class_name);
fun_B->Inherit(fun_A);

v8::Local<v8::String> subclass_name = v8_str("C");
v8::Local<v8::Object> b_proto;
v8::Local<v8::Object> c_proto;
// Perform several iterations to make sure the cache doesn't break
// subclassing.
for (int i = 0; i < 3; i++) {
Local<v8::Function> 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<v8::Object> 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> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
info.GetReturnValue().Set(v8_num(42));
}

THREADED_TEST(TestObjectTemplateReflectConstruct) {
LocalContext env;
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);

Local<v8::FunctionTemplate> fun_B = v8::FunctionTemplate::New(isolate);
fun_B->InstanceTemplate()->SetHandler(
v8::NamedPropertyHandlerConfiguration(NamedPropertyGetterWhichReturns42));
v8::Local<v8::String> class_name = v8_str("B");
fun_B->SetClassName(class_name);

v8::Local<v8::String> subclass_name = v8_str("C");
v8::Local<v8::Object> b_proto;
v8::Local<v8::Object> c_proto;
// Perform several iterations to make sure the cache doesn't break
// subclassing.
for (int i = 0; i < 3; i++) {
Local<v8::Function> 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<v8::Object> 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<v8::Value>& args) {
ApiTestFuzzer::Fuzz();
args.GetReturnValue().Set(v8_num(17.2));
Expand Down Expand Up @@ -18765,12 +18854,6 @@ TEST(SetterOnConstructorPrototype) {
}


static void NamedPropertyGetterWhichReturns42(
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
info.GetReturnValue().Set(v8_num(42));
}


static void NamedPropertySetterWhichSetsYOnThisTo23(
Local<Name> name, Local<Value> value,
const v8::PropertyCallbackInfo<v8::Value>& info) {
Expand Down

0 comments on commit 0eb4f1d

Please sign in to comment.