AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[debug] Instantiate accessors only once.

Browse Source 
When retrieving an API accessor function (i.e. either the getter or the
setter) for which the lazy accessor mechanism is used (i.e. where the
actual JSFunction is created lazily and only the FunctionTemplateInfo)
is around, we thus far created a fresh JSFunction every time the
accessor function is requested, but that's observably wrong behavior,
since the accessors are JavaScript objects with identity. We currently
rely on the instantiation cache to guarantee identity, but there's no
reason why we couldn't instead just put the instantiated JSFunction into
the AccessorPair.

Fixing this to only instantiate the lazy accessor pair only once, upon
first time it's requested, coincidentally also simplifies (and fixes)
the API accessor breakpoint machinery. This was previously lacking
support for walking dictionary prototype objects and forcibly
instantiating the lazy accessor pairs with break points. However, all
this magic in the debugger is no longer necessary when we ensure that
the lazy accessor pair component is generally only instantiated once.

Bug: v8:178, v8:7596, chromium:986063, chromium:496666
Change-Id: I41d28378010716c96c8ecf7c3f1247765f8bc669
Fixed: chromium:1163547
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2731527
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73163}
This commit is contained in:
Benedikt Meurer 2021-03-03 08:30:08 +01:00 committed by Commit Bot
parent fab754ff56
commit e9873bf129
3 changed files with 53 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/debug/debug.cc
View File

@ -1369,12 +1369,7 @@ void Debug::InstallDebugBreakTrampoline() {
Handle<Code> trampoline = BUILTIN_CODE(isolate_, DebugBreakTrampoline);
std::vector<Handle<JSFunction>> needs_compile;
using AccessorPairWithContext =
std::pair<Handle<AccessorPair>, Handle<NativeContext>>;
std::vector<AccessorPairWithContext> needs_instantiate;
{
// Deduplicate {needs_instantiate} by recording all collected AccessorPairs.
std::set<AccessorPair> recorded;
HeapObjectIterator iterator(isolate_->heap());
for (HeapObject obj = iterator.Next(); !obj.is_null();
obj = iterator.Next()) {
@ -1391,58 +1386,10 @@ void Debug::InstallDebugBreakTrampoline() {
} else {
fun.set_code(*trampoline);
}
} else if (obj.IsJSObject()) {
JSObject object = JSObject::cast(obj);
DescriptorArray descriptors =
object.map().instance_descriptors(kRelaxedLoad);
for (InternalIndex i : object.map().IterateOwnDescriptors()) {
if (descriptors.GetDetails(i).kind() == PropertyKind::kAccessor) {
Object value = descriptors.GetStrongValue(i);
if (!value.IsAccessorPair()) continue;
AccessorPair accessor_pair = AccessorPair::cast(value);
if (!accessor_pair.getter().IsFunctionTemplateInfo() &&
!accessor_pair.setter().IsFunctionTemplateInfo()) {
continue;
}
if (recorded.find(accessor_pair) != recorded.end()) continue;
needs_instantiate.emplace_back(
handle(accessor_pair, isolate_),
object.GetCreationContext().ToHandleChecked());
recorded.insert(accessor_pair);
}
}
}
}
}
// Forcibly instantiate all lazy accessor pairs to make sure that they
// properly hit the debug break trampoline.
for (AccessorPairWithContext tuple : needs_instantiate) {
Handle<AccessorPair> accessor_pair = tuple.first;
Handle<NativeContext> native_context = tuple.second;
if (accessor_pair->getter().IsFunctionTemplateInfo()) {
Handle<JSFunction> fun =
ApiNatives::InstantiateFunction(
isolate_, native_context,
handle(FunctionTemplateInfo::cast(accessor_pair->getter()),
isolate_))
.ToHandleChecked();
accessor_pair->set_getter(*fun);
}
if (accessor_pair->setter().IsFunctionTemplateInfo()) {
Handle<JSFunction> fun =
ApiNatives::InstantiateFunction(
isolate_, native_context,
handle(FunctionTemplateInfo::cast(accessor_pair->setter()),
isolate_))
.ToHandleChecked();
accessor_pair->set_setter(*fun);
}
}
// By overwriting the function code with DebugBreakTrampoline, which tailcalls
// to shared code, we bypass CompileLazy. Perform CompileLazy here instead.
for (Handle<JSFunction> fun : needs_compile) {

18
src/objects/objects.cc
View File

@ -4434,17 +4434,19 @@ Handle<Object> AccessorPair::GetComponent(Isolate* isolate,
Handle<NativeContext> native_context,
Handle<AccessorPair> accessor_pair,
AccessorComponent component) {
Object accessor = accessor_pair->get(component);
if (accessor.IsFunctionTemplateInfo()) {
return ApiNatives::InstantiateFunction(
isolate, native_context,
handle(FunctionTemplateInfo::cast(accessor), isolate))
.ToHandleChecked();
Handle<Object> accessor(accessor_pair->get(component), isolate);
if (accessor->IsFunctionTemplateInfo()) {
auto function = ApiNatives::InstantiateFunction(
isolate, native_context,
Handle<FunctionTemplateInfo>::cast(accessor))
.ToHandleChecked();
accessor_pair->set(component, *function);
return function;
}
if (accessor.IsNull(isolate)) {
if (accessor->IsNull(isolate)) {
return isolate->factory()->undefined_value();
}
return handle(accessor, isolate);
return accessor;
}
#ifdef DEBUG

43
test/cctest/test-debug.cc
View File

@ -1354,6 +1354,49 @@ TEST(BreakPointApiAccessor) {
CheckDebuggerUnloaded();
}
TEST(Regress1163547) {
LocalContext env;
v8::HandleScope scope(env->GetIsolate());
DebugEventCounter delegate;
v8::debug::SetDebugDelegate(env->GetIsolate(), &delegate);
i::Handle<i::BreakPoint> bp;
auto constructor_tmpl = v8::FunctionTemplate::New(env->GetIsolate());
auto prototype_tmpl = constructor_tmpl->PrototypeTemplate();
auto accessor_tmpl =
v8::FunctionTemplate::New(env->GetIsolate(), NoOpFunctionCallback);
prototype_tmpl->SetAccessorProperty(v8_str("f"), accessor_tmpl);
auto constructor =
constructor_tmpl->GetFunction(env.local()).ToLocalChecked();
env->Global()->Set(env.local(), v8_str("C"), constructor).ToChecked();
CompileRun("o = new C();");
v8::Local<v8::Function> function =
CompileRun("Object.getOwnPropertyDescriptor(C.prototype, 'f').get")
.As<v8::Function>();
// === Test API accessor ===
break_point_hit_count = 0;
// At this point, the C.prototype - which holds the "f" accessor - is in
// dictionary mode.
auto constructor_fun =
Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*constructor));
CHECK(!i::JSObject::cast(constructor_fun->prototype()).HasFastProperties());
// Run with breakpoint.
bp = SetBreakPoint(function, 0);
CompileRun("o.f");
CHECK_EQ(1, break_point_hit_count);
v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr);
CheckDebuggerUnloaded();
}
TEST(BreakPointInlineApiFunction) {
i::FLAG_allow_natives_syntax = true;
LocalContext env;