Deprecate some signature checks

Deprecate signature checks in
* Template::SetNativeDataProperty
* ObjectTemplate::SetAccessor
These are not used in Chrome and require some complicated check in the IC code, which we want to remove.

Change-Id: I413fafc8658e922fd590e7fe200600a624f019a6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3557253
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Stephen Röttger <sroettger@google.com>
Cr-Commit-Position: refs/heads/main@{#79689}
This commit is contained in:
Stephen Roettger 2022-03-30 14:39:57 +02:00 committed by V8 LUCI CQ
parent 0df9606dca
commit a8beac553b
5 changed files with 98 additions and 217 deletions

View File

@ -89,11 +89,26 @@ class V8_EXPORT Template : public Data {
* defined by FunctionTemplate::HasInstance()), an implicit TypeError is
* thrown and no callback is invoked.
*/
V8_DEPRECATE_SOON("Do signature check in accessor")
void SetNativeDataProperty(
Local<String> name, AccessorGetterCallback getter,
AccessorSetterCallback setter, Local<Value> data,
PropertyAttribute attribute, Local<AccessorSignature> signature,
AccessControl settings = DEFAULT,
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
V8_DEPRECATE_SOON("Do signature check in accessor")
void SetNativeDataProperty(
Local<Name> name, AccessorNameGetterCallback getter,
AccessorNameSetterCallback setter, Local<Value> data,
PropertyAttribute attribute, Local<AccessorSignature> signature,
AccessControl settings = DEFAULT,
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
void SetNativeDataProperty(
Local<String> name, AccessorGetterCallback getter,
AccessorSetterCallback setter = nullptr,
Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
Local<AccessorSignature> signature = Local<AccessorSignature>(),
AccessControl settings = DEFAULT,
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
@ -101,7 +116,6 @@ class V8_EXPORT Template : public Data {
Local<Name> name, AccessorNameGetterCallback getter,
AccessorNameSetterCallback setter = nullptr,
Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
Local<AccessorSignature> signature = Local<AccessorSignature>(),
AccessControl settings = DEFAULT,
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
@ -813,12 +827,26 @@ class V8_EXPORT ObjectTemplate : public Template {
* defined by FunctionTemplate::HasInstance()), an implicit TypeError is
* thrown and no callback is invoked.
*/
V8_DEPRECATE_SOON("Do signature check in accessor")
void SetAccessor(
Local<String> name, AccessorGetterCallback getter,
AccessorSetterCallback setter, Local<Value> data, AccessControl settings,
PropertyAttribute attribute, Local<AccessorSignature> signature,
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
V8_DEPRECATE_SOON("Do signature check in accessor")
void SetAccessor(
Local<Name> name, AccessorNameGetterCallback getter,
AccessorNameSetterCallback setter, Local<Value> data,
AccessControl settings, PropertyAttribute attribute,
Local<AccessorSignature> signature,
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
void SetAccessor(
Local<String> name, AccessorGetterCallback getter,
AccessorSetterCallback setter = nullptr,
Local<Value> data = Local<Value>(), AccessControl settings = DEFAULT,
PropertyAttribute attribute = None,
Local<AccessorSignature> signature = Local<AccessorSignature>(),
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
void SetAccessor(
@ -826,7 +854,6 @@ class V8_EXPORT ObjectTemplate : public Template {
AccessorNameSetterCallback setter = nullptr,
Local<Value> data = Local<Value>(), AccessControl settings = DEFAULT,
PropertyAttribute attribute = None,
Local<AccessorSignature> signature = Local<AccessorSignature>(),
SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);

View File

@ -1630,6 +1630,19 @@ static void TemplateSetAccessor(
i::ApiNatives::AddNativeDataProperty(isolate, info, accessor_info);
}
void Template::SetNativeDataProperty(v8::Local<String> name,
AccessorGetterCallback getter,
AccessorSetterCallback setter,
v8::Local<Value> data,
PropertyAttribute attribute,
AccessControl settings,
SideEffectType getter_side_effect_type,
SideEffectType setter_side_effect_type) {
TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
Local<AccessorSignature>(), true, false,
getter_side_effect_type, setter_side_effect_type);
}
void Template::SetNativeDataProperty(
v8::Local<String> name, AccessorGetterCallback getter,
AccessorSetterCallback setter, v8::Local<Value> data,
@ -1641,6 +1654,19 @@ void Template::SetNativeDataProperty(
setter_side_effect_type);
}
void Template::SetNativeDataProperty(v8::Local<Name> name,
AccessorNameGetterCallback getter,
AccessorNameSetterCallback setter,
v8::Local<Value> data,
PropertyAttribute attribute,
AccessControl settings,
SideEffectType getter_side_effect_type,
SideEffectType setter_side_effect_type) {
TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
Local<AccessorSignature>(), true, false,
getter_side_effect_type, setter_side_effect_type);
}
void Template::SetNativeDataProperty(
v8::Local<Name> name, AccessorNameGetterCallback getter,
AccessorNameSetterCallback setter, v8::Local<Value> data,
@ -1675,6 +1701,32 @@ void Template::SetIntrinsicDataProperty(Local<Name> name, Intrinsic intrinsic,
static_cast<i::PropertyAttributes>(attribute));
}
void ObjectTemplate::SetAccessor(v8::Local<String> name,
AccessorGetterCallback getter,
AccessorSetterCallback setter,
v8::Local<Value> data, AccessControl settings,
PropertyAttribute attribute,
SideEffectType getter_side_effect_type,
SideEffectType setter_side_effect_type) {
TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
Local<AccessorSignature>(),
i::FLAG_disable_old_api_accessors, false,
getter_side_effect_type, setter_side_effect_type);
}
void ObjectTemplate::SetAccessor(v8::Local<Name> name,
AccessorNameGetterCallback getter,
AccessorNameSetterCallback setter,
v8::Local<Value> data, AccessControl settings,
PropertyAttribute attribute,
SideEffectType getter_side_effect_type,
SideEffectType setter_side_effect_type) {
TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
Local<AccessorSignature>(),
i::FLAG_disable_old_api_accessors, false,
getter_side_effect_type, setter_side_effect_type);
}
void ObjectTemplate::SetAccessor(v8::Local<String> name,
AccessorGetterCallback getter,
AccessorSetterCallback setter,

View File

@ -294,9 +294,9 @@ TEST(AccessCheckWithInterceptor) {
IndexedEnumerator));
global_template->SetNativeDataProperty(
v8_str("cross_context_int"), GetCrossContextInt, SetCrossContextInt);
global_template->SetNativeDataProperty(
v8_str("all_can_read"), Return42, nullptr, v8::Local<v8::Value>(),
v8::None, v8::Local<v8::AccessorSignature>(), v8::ALL_CAN_READ);
global_template->SetNativeDataProperty(v8_str("all_can_read"), Return42,
nullptr, v8::Local<v8::Value>(),
v8::None, v8::ALL_CAN_READ);
v8::Local<v8::Context> context0 =
v8::Context::New(isolate, nullptr, global_template);
@ -386,9 +386,9 @@ TEST(NewRemoteContext) {
IndexedEnumerator));
global_template->SetNativeDataProperty(
v8_str("cross_context_int"), GetCrossContextInt, SetCrossContextInt);
global_template->SetNativeDataProperty(
v8_str("all_can_read"), Return42, nullptr, v8::Local<v8::Value>(),
v8::None, v8::Local<v8::AccessorSignature>(), v8::ALL_CAN_READ);
global_template->SetNativeDataProperty(v8_str("all_can_read"), Return42,
nullptr, v8::Local<v8::Value>(),
v8::None, v8::ALL_CAN_READ);
v8::Local<v8::Object> global0 =
v8::Context::NewRemoteContext(isolate, global_template).ToLocalChecked();
@ -451,9 +451,9 @@ TEST(NewRemoteInstance) {
v8::IndexedPropertyHandlerConfiguration(IndexedGetter, IndexedSetter,
IndexedQuery, IndexedDeleter,
IndexedEnumerator));
tmpl->SetNativeDataProperty(
v8_str("all_can_read"), Return42, nullptr, v8::Local<v8::Value>(),
v8::None, v8::Local<v8::AccessorSignature>(), v8::ALL_CAN_READ);
tmpl->SetNativeDataProperty(v8_str("all_can_read"), Return42, nullptr,
v8::Local<v8::Value>(), v8::None,
v8::ALL_CAN_READ);
v8::Local<v8::Object> obj = tmpl->NewRemoteInstance().ToLocalChecked();

View File

@ -411,11 +411,9 @@ TEST(NativeTemplateAccessorWithSideEffects) {
v8::Local<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(isolate);
templ->SetAccessor(v8_str("get"), Getter, nullptr, v8::Local<v8::Value>(),
v8::AccessControl::DEFAULT, v8::PropertyAttribute::None,
v8::Local<v8::AccessorSignature>(),
v8::SideEffectType::kHasSideEffect);
templ->SetAccessor(v8_str("set"), Getter, Setter, v8::Local<v8::Value>(),
v8::AccessControl::DEFAULT, v8::PropertyAttribute::None,
v8::Local<v8::AccessorSignature>(),
v8::SideEffectType::kHasNoSideEffect,
v8::SideEffectType::kHasSideEffect);
@ -551,7 +549,6 @@ TEST(SetAccessorSetSideEffectReceiverCheck2) {
templ->InstanceTemplate()->SetAccessor(
v8_str("bar"), Getter, Setter, v8::Local<v8::Value>(),
v8::AccessControl::DEFAULT, v8::PropertyAttribute::None,
v8::Local<v8::AccessorSignature>(),
v8::SideEffectType::kHasSideEffectToReceiver,
v8::SideEffectType::kHasSideEffectToReceiver);
CHECK(env->Global()
@ -668,10 +665,10 @@ TEST(ObjectTemplateSetAccessorHasNoSideEffect) {
v8::Local<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(isolate);
templ->SetAccessor(v8_str("foo"), StringGetter);
templ->SetAccessor(
v8_str("foo2"), StringGetter, nullptr, v8::Local<v8::Value>(),
v8::AccessControl::DEFAULT, v8::PropertyAttribute::None,
v8::Local<v8::AccessorSignature>(), v8::SideEffectType::kHasNoSideEffect);
templ->SetAccessor(v8_str("foo2"), StringGetter, nullptr,
v8::Local<v8::Value>(), v8::AccessControl::DEFAULT,
v8::PropertyAttribute::None,
v8::SideEffectType::kHasNoSideEffect);
v8::Local<v8::Object> obj = templ->NewInstance(env.local()).ToLocalChecked();
CHECK(env->Global()->Set(env.local(), v8_str("obj"), obj).FromJust());
@ -711,8 +708,8 @@ TEST(ObjectTemplateSetNativePropertyHasNoSideEffect) {
templ->SetNativeDataProperty(v8_str("foo"), Getter);
templ->SetNativeDataProperty(
v8_str("foo2"), Getter, nullptr, v8::Local<v8::Value>(),
v8::PropertyAttribute::None, v8::Local<v8::AccessorSignature>(),
v8::AccessControl::DEFAULT, v8::SideEffectType::kHasNoSideEffect);
v8::PropertyAttribute::None, v8::AccessControl::DEFAULT,
v8::SideEffectType::kHasNoSideEffect);
v8::Local<v8::Object> obj = templ->NewInstance(env.local()).ToLocalChecked();
CHECK(env->Global()->Set(env.local(), v8_str("obj"), obj).FromJust());

View File

@ -20712,201 +20712,6 @@ TEST(StringEmpty) {
CHECK(*v8::Utils::OpenHandle(*v8::String::Empty(isolate)) == *empty_string);
}
static int instance_checked_getter_count = 0;
static void InstanceCheckedGetter(
Local<String> name,
const v8::PropertyCallbackInfo<v8::Value>& info) {
CHECK(name->Equals(info.GetIsolate()->GetCurrentContext(), v8_str("foo"))
.FromJust());
instance_checked_getter_count++;
info.GetReturnValue().Set(v8_num(11));
}
static int instance_checked_setter_count = 0;
static void InstanceCheckedSetter(Local<String> name,
Local<Value> value,
const v8::PropertyCallbackInfo<void>& info) {
CHECK(name->Equals(info.GetIsolate()->GetCurrentContext(), v8_str("foo"))
.FromJust());
CHECK(value->Equals(info.GetIsolate()->GetCurrentContext(), v8_num(23))
.FromJust());
instance_checked_setter_count++;
}
static void CheckInstanceCheckedResult(int getters, int setters,
bool expects_callbacks,
TryCatch* try_catch) {
if (expects_callbacks) {
CHECK(!try_catch->HasCaught());
CHECK_EQ(getters, instance_checked_getter_count);
CHECK_EQ(setters, instance_checked_setter_count);
} else {
CHECK(try_catch->HasCaught());
CHECK_EQ(0, instance_checked_getter_count);
CHECK_EQ(0, instance_checked_setter_count);
}
try_catch->Reset();
}
static void CheckInstanceCheckedAccessors(bool expects_callbacks) {
instance_checked_getter_count = 0;
instance_checked_setter_count = 0;
TryCatch try_catch(CcTest::isolate());
// Test path through generic runtime code.
CompileRun("obj.foo");
CheckInstanceCheckedResult(1, 0, expects_callbacks, &try_catch);
CompileRun("obj.foo = 23");
CheckInstanceCheckedResult(1, 1, expects_callbacks, &try_catch);
// Test path through generated LoadIC and StoredIC.
CompileRun(
"function test_get(o) { o.foo; };"
"%PrepareFunctionForOptimization(test_get);"
"test_get(obj);");
CheckInstanceCheckedResult(2, 1, expects_callbacks, &try_catch);
CompileRun("test_get(obj);");
CheckInstanceCheckedResult(3, 1, expects_callbacks, &try_catch);
CompileRun("test_get(obj);");
CheckInstanceCheckedResult(4, 1, expects_callbacks, &try_catch);
CompileRun(
"function test_set(o) { o.foo = 23; }"
"%PrepareFunctionForOptimization(test_set);"
"test_set(obj);");
CheckInstanceCheckedResult(4, 2, expects_callbacks, &try_catch);
CompileRun("test_set(obj);");
CheckInstanceCheckedResult(4, 3, expects_callbacks, &try_catch);
CompileRun("test_set(obj);");
CheckInstanceCheckedResult(4, 4, expects_callbacks, &try_catch);
// Test path through optimized code.
CompileRun("%OptimizeFunctionOnNextCall(test_get);"
"test_get(obj);");
CheckInstanceCheckedResult(5, 4, expects_callbacks, &try_catch);
CompileRun("%OptimizeFunctionOnNextCall(test_set);"
"test_set(obj);");
CheckInstanceCheckedResult(5, 5, expects_callbacks, &try_catch);
// Cleanup so that closures start out fresh in next check.
CompileRun(
"%DeoptimizeFunction(test_get);"
"%ClearFunctionFeedback(test_get);"
"%DeoptimizeFunction(test_set);"
"%ClearFunctionFeedback(test_set);");
}
THREADED_TEST(InstanceCheckOnInstanceAccessor) {
v8::internal::FLAG_allow_natives_syntax = true;
LocalContext context;
v8::HandleScope scope(context->GetIsolate());
Local<FunctionTemplate> templ = FunctionTemplate::New(context->GetIsolate());
Local<ObjectTemplate> inst = templ->InstanceTemplate();
inst->SetAccessor(v8_str("foo"), InstanceCheckedGetter, InstanceCheckedSetter,
Local<Value>(), v8::DEFAULT, v8::None,
v8::AccessorSignature::New(context->GetIsolate(), templ));
CHECK(context->Global()
->Set(context.local(), v8_str("f"),
templ->GetFunction(context.local()).ToLocalChecked())
.FromJust());
printf("Testing positive ...\n");
CompileRun("var obj = new f();");
CHECK(templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(true);
printf("Testing negative ...\n");
CompileRun("var obj = {};"
"obj.__proto__ = new f();");
CHECK(!templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(false);
}
static void EmptyInterceptorGetter(
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {}
static void EmptyInterceptorSetter(
Local<Name> name, Local<Value> value,
const v8::PropertyCallbackInfo<v8::Value>& info) {}
THREADED_TEST(InstanceCheckOnInstanceAccessorWithInterceptor) {
v8::internal::FLAG_allow_natives_syntax = true;
LocalContext context;
v8::HandleScope scope(context->GetIsolate());
Local<FunctionTemplate> templ = FunctionTemplate::New(context->GetIsolate());
Local<ObjectTemplate> inst = templ->InstanceTemplate();
templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
EmptyInterceptorGetter, EmptyInterceptorSetter));
inst->SetAccessor(v8_str("foo"), InstanceCheckedGetter, InstanceCheckedSetter,
Local<Value>(), v8::DEFAULT, v8::None,
v8::AccessorSignature::New(context->GetIsolate(), templ));
CHECK(context->Global()
->Set(context.local(), v8_str("f"),
templ->GetFunction(context.local()).ToLocalChecked())
.FromJust());
printf("Testing positive ...\n");
CompileRun("var obj = new f();");
CHECK(templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(true);
printf("Testing negative ...\n");
CompileRun("var obj = {};"
"obj.__proto__ = new f();");
CHECK(!templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(false);
}
THREADED_TEST(InstanceCheckOnPrototypeAccessor) {
v8::internal::FLAG_allow_natives_syntax = true;
LocalContext context;
v8::HandleScope scope(context->GetIsolate());
Local<FunctionTemplate> templ = FunctionTemplate::New(context->GetIsolate());
Local<ObjectTemplate> proto = templ->PrototypeTemplate();
proto->SetAccessor(v8_str("foo"), InstanceCheckedGetter,
InstanceCheckedSetter, Local<Value>(), v8::DEFAULT,
v8::None,
v8::AccessorSignature::New(context->GetIsolate(), templ));
CHECK(context->Global()
->Set(context.local(), v8_str("f"),
templ->GetFunction(context.local()).ToLocalChecked())
.FromJust());
printf("Testing positive ...\n");
CompileRun("var obj = new f();");
CHECK(templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(true);
printf("Testing negative ...\n");
CompileRun("var obj = {};"
"obj.__proto__ = new f();");
CHECK(!templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(false);
printf("Testing positive with modified prototype chain ...\n");
CompileRun("var obj = new f();"
"var pro = {};"
"pro.__proto__ = obj.__proto__;"
"obj.__proto__ = pro;");
CHECK(templ->HasInstance(
context->Global()->Get(context.local(), v8_str("obj")).ToLocalChecked()));
CheckInstanceCheckedAccessors(true);
}
THREADED_TEST(CheckIsLeafTemplateForApiObject) {
LocalContext context;
v8::HandleScope scope(context->GetIsolate());