From 4baf7e5e3d6eab6971138cef0c64d1577a517a69 Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Wed, 17 Mar 2021 18:03:14 +0000 Subject: [PATCH] [vm/concurrency] Introduce GroupDebugger that takes CodeBreakpoints management away from isolate Debugger. CodeBreakpoint class tracks patching of the call targets for the purpose of routing the call to the debugger runtime. With this CL CodeBreakpoint instance is managed by GroupDebugger, debugger class owned by IsolateGroup, and can be associated with multiple BreakpointLocation, one location per isolate that set a breakpoint at particular code location. TEST=observatory/tests/service/break_on_function_child_isolate_test Issue https://github.com/dart-lang/sdk/issues/36097 Change-Id: I3b3d0974a0fb9668742e5c1e2a2ce13b84b1d630 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190732 Commit-Queue: Alexander Aprelev Reviewed-by: Martin Kustermann --- .../break_on_function_child_isolate_test.dart | 84 ++++++ .../tests/service/service_kernel.status | 1 + .../tests/service_2/service_2_kernel.status | 1 + runtime/vm/debugger.cc | 257 ++++++++++++------ runtime/vm/debugger.h | 104 +++++-- runtime/vm/isolate.cc | 20 +- runtime/vm/isolate.h | 7 + runtime/vm/object.cc | 12 +- runtime/vm/runtime_entry.cc | 3 +- 9 files changed, 370 insertions(+), 119 deletions(-) create mode 100644 runtime/observatory/tests/service/break_on_function_child_isolate_test.dart diff --git a/runtime/observatory/tests/service/break_on_function_child_isolate_test.dart b/runtime/observatory/tests/service/break_on_function_child_isolate_test.dart new file mode 100644 index 00000000000..d30310afe6b --- /dev/null +++ b/runtime/observatory/tests/service/break_on_function_child_isolate_test.dart @@ -0,0 +1,84 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +// VMOptions=--verbose_debug --enable-isolate-groups --experimental-enable-isolate-groups-jit +import 'dart:async'; +import 'dart:isolate' as dart_isolate; + +import 'package:observatory/service_io.dart'; +import 'package:test/test.dart'; +import 'service_test_common.dart'; +import 'test_helper.dart'; +import 'dart:developer'; + +const int LINE_A = 18; +const int LINE_B = 25; +const int LINE_C = 29; + +foo(args) { // LINE_A + final dart_isolate.SendPort sendPort = args[0] as dart_isolate.SendPort; + sendPort.send('reply from foo'); +} + +testMain() async { + final rpResponse = dart_isolate.ReceivePort(); + debugger(); // LINE_B + await dart_isolate.Isolate.spawn(foo, [rpResponse.sendPort]); + await rpResponse.first; + rpResponse.close(); + debugger(); // LINE_C +} + +final completerAtFoo = Completer(); + +final tests = [ + hasPausedAtStart, + resumeIsolate, + hasStoppedAtBreakpoint, + stoppedAtLine(LINE_B + 1), + (Isolate isolate) async { + // Set up a listener to wait for child isolate launch and breakpoint events. + final stream = await isolate.vm.getEventStream(VM.kDebugStream); + var childIsolate; + var subscription; + subscription = stream.listen((ServiceEvent event) async { + switch (event.kind) { + case ServiceEvent.kPauseStart: + childIsolate = event.isolate!; + await childIsolate.reload(); + + Library rootLib = await childIsolate.rootLibrary.load() as Library; + final foo = rootLib.functions.singleWhere((f) => f.name == 'foo'); + final bpt = await childIsolate.addBreakpointAtEntry(foo); + + expect(bpt is Breakpoint, isTrue); + childIsolate.resume(); + break; + case ServiceEvent.kPauseBreakpoint: + if (childIsolate == event.isolate) { + ServiceMap stack = await childIsolate.getStack(); + Frame top = stack['frames'][0]; + Script script = await top.location!.script.load() as Script; + expect(script.tokenToLine(top.location!.tokenPos), equals(LINE_A)); + + childIsolate.resume(); + subscription.cancel(); + completerAtFoo.complete(); + } + break; + } + }); + }, + resumeIsolate, + (Isolate isolate) async { + await completerAtFoo.future; + }, + hasStoppedAtBreakpoint, + stoppedAtLine(LINE_C + 1), + resumeIsolate, +]; + +main(args) async { + runIsolateTests(args, tests, + testeeConcurrent: testMain, pause_on_start: true); +} diff --git a/runtime/observatory/tests/service/service_kernel.status b/runtime/observatory/tests/service/service_kernel.status index 16e2d93e88b..600d2dfc532 100644 --- a/runtime/observatory/tests/service/service_kernel.status +++ b/runtime/observatory/tests/service/service_kernel.status @@ -58,6 +58,7 @@ bad_reload_test: SkipByDesign # Hot reload is disabled in AOT mode. break_on_activation_test: SkipByDesign # Debugger is disabled in AOT mode. break_on_async_function_test: SkipByDesign # Debugger is disabled in AOT mode. break_on_default_constructor_test: SkipByDesign # Debugger is disabled in AOT mode. +break_on_function_child_isolate_test: SkipByDesign # Debugger is disabled in AOT mode. break_on_function_test: SkipByDesign # Debugger is disabled in AOT mode. breakpoint_async_break_test: SkipByDesign # Debugger is disabled in AOT mode. breakpoint_in_package_parts_class_file_uri_test: SkipByDesign # Debugger is disabled in AOT mode. diff --git a/runtime/observatory_2/tests/service_2/service_2_kernel.status b/runtime/observatory_2/tests/service_2/service_2_kernel.status index f352aa6a1cf..a04fc9c55a6 100644 --- a/runtime/observatory_2/tests/service_2/service_2_kernel.status +++ b/runtime/observatory_2/tests/service_2/service_2_kernel.status @@ -58,6 +58,7 @@ bad_reload_test: SkipByDesign # Hot reload is disabled in AOT mode. break_on_activation_test: SkipByDesign # Debugger is disabled in AOT mode. break_on_async_function_test: SkipByDesign # Debugger is disabled in AOT mode. break_on_default_constructor_test: SkipByDesign # Debugger is disabled in AOT mode. +break_on_function_child_isolate_test: SkipByDesign # Debugger is disabled in AOT mode. break_on_function_test: SkipByDesign # Debugger is disabled in AOT mode. breakpoint_async_break_test: SkipByDesign # Debugger is disabled in AOT mode. breakpoint_in_package_parts_class_file_uri_test: SkipByDesign # Debugger is disabled in AOT mode. diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index 2b5db6a6042..643e324a630 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -57,12 +57,14 @@ DECLARE_FLAG(bool, warn_on_pause_with_no_debugger); #ifndef PRODUCT // Create an unresolved breakpoint in given token range and script. -BreakpointLocation::BreakpointLocation(const Script& script, +BreakpointLocation::BreakpointLocation(Debugger* debugger, + const Script& script, TokenPosition token_pos, TokenPosition end_token_pos, intptr_t requested_line_number, intptr_t requested_column_number) - : script_(script.ptr()), + : debugger_(debugger), + script_(script.ptr()), url_(script.url()), token_pos_(token_pos), end_token_pos_(end_token_pos), @@ -77,10 +79,12 @@ BreakpointLocation::BreakpointLocation(const Script& script, } // Create a latent breakpoint at given url and line number. -BreakpointLocation::BreakpointLocation(const String& url, +BreakpointLocation::BreakpointLocation(Debugger* debugger, + const String& url, intptr_t requested_line_number, intptr_t requested_column_number) - : script_(Script::null()), + : debugger_(debugger), + script_(Script::null()), url_(url.ptr()), token_pos_(TokenPosition::kNoSource), end_token_pos_(TokenPosition::kNoSource), @@ -323,7 +327,7 @@ void BreakpointLocation::AddBreakpoint(Breakpoint* bpt, Debugger* dbg) { bpt->set_next(breakpoints()); set_breakpoints(bpt); - dbg->SyncBreakpointLocation(this); + dbg->group_debugger()->SyncBreakpointLocation(this); dbg->SendBreakpointEvent(ServiceEvent::kBreakpointAdded, bpt); } @@ -441,6 +445,11 @@ bool Debugger::HasBreakpoint(const Function& func, Zone* zone) { } return false; } + return group_debugger()->HasCodeBreakpointInFunction(func); +} + +bool GroupDebugger::HasCodeBreakpointInFunction(const Function& func) { + SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); CodeBreakpoint* cbpt = code_breakpoints_; while (cbpt != NULL) { if (func.ptr() == cbpt->function()) { @@ -451,7 +460,8 @@ bool Debugger::HasBreakpoint(const Function& func, Zone* zone) { return false; } -bool Debugger::HasBreakpoint(const Code& code) { +bool GroupDebugger::HasBreakpointInCode(const Code& code) { + SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); CodeBreakpoint* cbpt = code_breakpoints_; while (cbpt != NULL) { if (code.ptr() == cbpt->code_) { @@ -1446,7 +1456,6 @@ CodeBreakpoint::CodeBreakpoint(const Code& code, pc_(pc), line_number_(-1), is_enabled_(false), - bpt_location_(NULL), next_(NULL), breakpoint_kind_(kind), saved_value_(Code::null()) { @@ -1463,7 +1472,6 @@ CodeBreakpoint::~CodeBreakpoint() { #ifdef DEBUG code_ = Code::null(); pc_ = 0ul; - bpt_location_ = NULL; next_ = NULL; breakpoint_kind_ = UntaggedPcDescriptors::kOther; #endif @@ -1506,12 +1514,57 @@ void CodeBreakpoint::Disable() { ASSERT(!is_enabled_); } +bool CodeBreakpoint::HasBreakpointLocation( + BreakpointLocation* breakpoint_location) { + for (intptr_t i = 0; i < breakpoint_locations_.length(); i++) { + if (breakpoint_locations_[i] == breakpoint_location) { + return true; + } + } + return false; +} + +bool CodeBreakpoint::FindAndDeleteBreakpointLocation( + BreakpointLocation* breakpoint_location) { + for (intptr_t i = 0; i < breakpoint_locations_.length(); i++) { + if (breakpoint_locations_[i] == breakpoint_location) { + breakpoint_locations_.EraseAt(i); + return true; + } + } + return false; +} + +BreakpointLocation* CodeBreakpoint::FindBreakpointForDebugger( + Debugger* debugger) { + for (intptr_t i = 0; i < breakpoint_locations_.length(); i++) { + if (breakpoint_locations_[i]->debugger() == debugger) { + return breakpoint_locations_[i]; + } + } + return nullptr; +} + +GroupDebugger::GroupDebugger(IsolateGroup* isolate_group) + : isolate_group_(isolate_group), + code_breakpoints_lock_(new SafepointRwLock()), + code_breakpoints_(nullptr), + needs_breakpoint_cleanup_(false) {} + +GroupDebugger::~GroupDebugger() { + while (code_breakpoints_ != nullptr) { + CodeBreakpoint* cbpt = code_breakpoints_; + code_breakpoints_ = code_breakpoints_->next(); + ASSERT(!cbpt->IsEnabled()); + delete cbpt; + } +} + Debugger::Debugger(Isolate* isolate) : isolate_(isolate), next_id_(1), latent_locations_(NULL), breakpoint_locations_(NULL), - code_breakpoints_(NULL), resume_action_(kContinue), resume_frame_index_(-1), post_deopt_frame_index_(-1), @@ -1526,7 +1579,6 @@ Debugger::Debugger(Isolate* isolate) async_stepping_fp_(0), top_frame_awaiter_(Object::null()), skip_next_step_(false), - needs_breakpoint_cleanup_(false), synthetic_async_breakpoint_(NULL), exc_pause_info_(kNoPauseOnExceptions) {} @@ -1534,7 +1586,6 @@ Debugger::~Debugger() { ASSERT(!IsPaused()); ASSERT(latent_locations_ == NULL); ASSERT(breakpoint_locations_ == NULL); - ASSERT(code_breakpoints_ == NULL); ASSERT(stack_trace_ == NULL); ASSERT(async_causal_stack_trace_ == NULL); ASSERT(synthetic_async_breakpoint_ == NULL); @@ -1546,22 +1597,18 @@ void Debugger::Shutdown() { if (Isolate::IsSystemIsolate(isolate_)) { return; } - while (breakpoint_locations_ != NULL) { + while (breakpoint_locations_ != nullptr) { BreakpointLocation* loc = breakpoint_locations_; + group_debugger()->DisableCodeBreakpointsFor(loc); breakpoint_locations_ = breakpoint_locations_->next(); delete loc; } - while (latent_locations_ != NULL) { + while (latent_locations_ != nullptr) { BreakpointLocation* loc = latent_locations_; + group_debugger()->DisableCodeBreakpointsFor(loc); latent_locations_ = latent_locations_->next(); delete loc; } - while (code_breakpoints_ != NULL) { - CodeBreakpoint* cbpt = code_breakpoints_; - code_breakpoints_ = code_breakpoints_->next(); - cbpt->Disable(); - delete cbpt; - } if (NeedsIsolateEvents()) { ServiceEvent event(isolate_, ServiceEvent::kIsolateExit); InvokeEventHandler(&event); @@ -2439,8 +2486,8 @@ static TokenPosition ResolveBreakpointPos(const Function& func, return TokenPosition::kNoSource; } -void Debugger::MakeCodeBreakpointAt(const Function& func, - BreakpointLocation* loc) { +void GroupDebugger::MakeCodeBreakpointAt(const Function& func, + BreakpointLocation* loc) { ASSERT(loc->token_pos_.IsReal()); ASSERT((loc != NULL) && loc->IsResolved()); ASSERT(!func.HasOptimizedCode()); @@ -2464,19 +2511,31 @@ void Debugger::MakeCodeBreakpointAt(const Function& func, if (lowest_pc_offset != kUwordMax) { uword lowest_pc = code.PayloadStart() + lowest_pc_offset; CodeBreakpoint* code_bpt = GetCodeBreakpoint(lowest_pc); - if (code_bpt == NULL) { - // No code breakpoint for this code exists; create one. - code_bpt = - new CodeBreakpoint(code, loc->token_pos_, lowest_pc, lowest_kind); - if (FLAG_verbose_debug) { - OS::PrintErr("Setting code breakpoint at pos %s pc %#" Px - " offset %#" Px "\n", - loc->token_pos_.ToCString(), lowest_pc, - lowest_pc - code.PayloadStart()); + if (code_bpt == nullptr) { + SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock()); + code_bpt = GetCodeBreakpoint(lowest_pc); + if (code_bpt == nullptr) { + // No code breakpoint for this code exists; create one. + code_bpt = + new CodeBreakpoint(code, loc->token_pos_, lowest_pc, lowest_kind); + if (FLAG_verbose_debug) { + OS::PrintErr("Setting code breakpoint at pos %s pc %#" Px + " offset %#" Px "\n", + loc->token_pos_.ToCString(), lowest_pc, + lowest_pc - code.PayloadStart()); + } + RegisterCodeBreakpoint(code_bpt); + } else { + if (FLAG_verbose_debug) { + OS::PrintErr( + "Adding location to existing code breakpoint at pos %s pc %#" Px + " offset %#" Px "\n", + loc->token_pos_.ToCString(), lowest_pc, + lowest_pc - code.PayloadStart()); + } } - RegisterCodeBreakpoint(code_bpt); } - code_bpt->set_bpt_location(loc); + code_bpt->AddBreakpointLocation(loc); if (loc->AnyEnabled()) { code_bpt->Enable(); } @@ -2734,7 +2793,7 @@ BreakpointLocation* Debugger::SetCodeBreakpoints( requested_column); } if (loc == NULL) { - loc = new BreakpointLocation(script, breakpoint_pos, breakpoint_pos, + loc = new BreakpointLocation(this, script, breakpoint_pos, breakpoint_pos, requested_line, requested_column); RegisterBreakpointLocation(loc); } @@ -2751,7 +2810,7 @@ BreakpointLocation* Debugger::SetCodeBreakpoints( for (intptr_t i = 0; i < num_functions; i++) { func ^= functions.At(i); ASSERT(func.HasCode()); - MakeCodeBreakpointAt(func, loc); + group_debugger()->MakeCodeBreakpointAt(func, loc); } if (FLAG_verbose_debug) { intptr_t line_number = -1; @@ -2842,7 +2901,7 @@ BreakpointLocation* Debugger::SetBreakpoint(const Script& script, BreakpointLocation* loc = GetBreakpointLocation(script, token_pos, -1, requested_column); if (loc == NULL) { - loc = new BreakpointLocation(script, token_pos, last_token_pos, + loc = new BreakpointLocation(this, script, token_pos, last_token_pos, requested_line, requested_column); RegisterBreakpointLocation(loc); } @@ -2851,12 +2910,13 @@ BreakpointLocation* Debugger::SetBreakpoint(const Script& script, // Synchronize the enabled/disabled state of all code breakpoints // associated with the breakpoint location loc. -void Debugger::SyncBreakpointLocation(BreakpointLocation* loc) { +void GroupDebugger::SyncBreakpointLocation(BreakpointLocation* loc) { bool any_enabled = loc->AnyEnabled(); + SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); CodeBreakpoint* cbpt = code_breakpoints_; while (cbpt != NULL) { - if (loc == cbpt->bpt_location()) { + if (cbpt->HasBreakpointLocation(loc)) { if (any_enabled) { cbpt->Enable(); } else { @@ -2867,6 +2927,17 @@ void Debugger::SyncBreakpointLocation(BreakpointLocation* loc) { } } +void GroupDebugger::DisableCodeBreakpointsFor(BreakpointLocation* location) { + SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); + CodeBreakpoint* cbpt = code_breakpoints_; + while (cbpt != nullptr) { + if (cbpt->HasBreakpointLocation(location)) { + cbpt->Disable(); + } + cbpt = cbpt->next(); + } +} + Breakpoint* Debugger::SetBreakpointAtEntry(const Function& target_function, bool single_shot) { ASSERT(!target_function.IsNull()); @@ -3044,6 +3115,14 @@ BreakpointLocation* Debugger::BreakpointLocationAtLineCol( return loc; } +void GroupDebugger::VisitObjectPointers(ObjectPointerVisitor* visitor) { + CodeBreakpoint* cbpt = code_breakpoints_; + while (cbpt != nullptr) { + cbpt->VisitObjectPointers(visitor); + cbpt = cbpt->next(); + } +} + // static void Debugger::VisitObjectPointers(ObjectPointerVisitor* visitor) { ASSERT(visitor != NULL); @@ -3057,11 +3136,6 @@ void Debugger::VisitObjectPointers(ObjectPointerVisitor* visitor) { loc->VisitObjectPointers(visitor); loc = loc->next(); } - CodeBreakpoint* cbpt = code_breakpoints_; - while (cbpt != NULL) { - cbpt->VisitObjectPointers(visitor); - cbpt = cbpt->next(); - } visitor->VisitPointer(reinterpret_cast(&top_frame_awaiter_)); } @@ -3102,10 +3176,14 @@ void Debugger::Pause(ServiceEvent* event) { } } + group_debugger()->Pause(); + pause_event_ = nullptr; +} + +void GroupDebugger::Pause() { if (needs_breakpoint_cleanup_) { RemoveUnlinkedCodeBreakpoints(); } - pause_event_ = NULL; } void Debugger::EnterSingleStepMode() { @@ -3664,7 +3742,7 @@ ErrorPtr Debugger::PauseStepping() { // If there is an active breakpoint at this pc, then we should have // already bailed out of this function in the skip_next_step_ test // above. - ASSERT(!HasActiveBreakpoint(frame->pc())); + ASSERT(!group_debugger()->HasActiveBreakpoint(frame->pc())); if (FLAG_verbose_debug) { OS::PrintErr(">>> single step break at %s:%" Pd ":%" Pd @@ -3701,17 +3779,19 @@ ErrorPtr Debugger::PauseBreakpoint() { ASSERT(stack_trace->Length() > 0); ActivationFrame* top_frame = stack_trace->FrameAt(0); ASSERT(top_frame != nullptr); - CodeBreakpoint* cbpt = GetCodeBreakpoint(top_frame->pc()); - ASSERT(cbpt != nullptr); - if (!Library::Handle(top_frame->Library()).IsDebuggable()) { return Error::null(); } - BreakpointLocation* bpt_location = cbpt->bpt_location_; + CodeBreakpoint* cbpt = nullptr; + BreakpointLocation* bpt_location = + group_debugger()->GetBreakpointLocationFor(this, top_frame->pc(), &cbpt); if (bpt_location == nullptr) { + // There might be no breakpoint locations for this isolate/debugger. return Error::null(); } + ASSERT(cbpt != nullptr); + Breakpoint* bpt_hit = bpt_location->FindHitBreakpoint(top_frame); if (bpt_hit == nullptr) { return Error::null(); @@ -3967,7 +4047,7 @@ void Debugger::NotifyCompilation(const Function& func) { bpt = bpt->next(); } } - MakeCodeBreakpointAt(func, loc); + group_debugger()->MakeCodeBreakpointAt(func, loc); } } } @@ -4039,9 +4119,9 @@ void Debugger::NotifyDoneLoading() { if (existing_loc == NULL) { // Create and register a new source breakpoint for the // latent breakpoint. - BreakpointLocation* unresolved_loc = - new BreakpointLocation(script, first_token_pos, last_token_pos, - line_number, column_number); + BreakpointLocation* unresolved_loc = new BreakpointLocation( + this, script, first_token_pos, last_token_pos, line_number, + column_number); RegisterBreakpointLocation(unresolved_loc); // Move breakpoints over. @@ -4058,7 +4138,7 @@ void Debugger::NotifyDoneLoading() { } bpt = bpt->next(); } - SyncBreakpointLocation(unresolved_loc); + group_debugger()->SyncBreakpointLocation(unresolved_loc); } delete matched_loc; // Break out of the iteration over loaded libraries. If the @@ -4093,12 +4173,12 @@ void Debugger::NotifyDoneLoading() { // TODO(hausner): Could potentially make this faster by checking // whether the call target at pc is a debugger stub. -bool Debugger::HasActiveBreakpoint(uword pc) { +bool GroupDebugger::HasActiveBreakpoint(uword pc) { CodeBreakpoint* cbpt = GetCodeBreakpoint(pc); - return (cbpt != NULL) && (cbpt->IsEnabled()); + return (cbpt != nullptr) && (cbpt->IsEnabled()); } -CodeBreakpoint* Debugger::GetCodeBreakpoint(uword breakpoint_address) { +CodeBreakpoint* GroupDebugger::GetCodeBreakpoint(uword breakpoint_address) { CodeBreakpoint* cbpt = code_breakpoints_; while (cbpt != NULL) { if (cbpt->pc() == breakpoint_address) { @@ -4106,10 +4186,33 @@ CodeBreakpoint* Debugger::GetCodeBreakpoint(uword breakpoint_address) { } cbpt = cbpt->next(); } - return NULL; + return nullptr; } -CodePtr Debugger::GetPatchedStubAddress(uword breakpoint_address) { +BreakpointLocation* GroupDebugger::GetBreakpointLocationFor( + Debugger* debugger, + uword breakpoint_address, + CodeBreakpoint** pcbpt) { + ASSERT(pcbpt != nullptr); + SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); + *pcbpt = code_breakpoints_; + while (*pcbpt != nullptr) { + if ((*pcbpt)->pc() == breakpoint_address) { + return (*pcbpt)->FindBreakpointForDebugger(debugger); + } + *pcbpt = (*pcbpt)->next(); + } + return nullptr; +} + +void GroupDebugger::RegisterCodeBreakpoint(CodeBreakpoint* cbpt) { + ASSERT(cbpt->next() == NULL); + DEBUG_ASSERT(code_breakpoints_lock()->IsCurrentThreadWriter()); + cbpt->set_next(code_breakpoints_); + code_breakpoints_ = cbpt; +} + +CodePtr GroupDebugger::GetPatchedStubAddress(uword breakpoint_address) { CodeBreakpoint* cbpt = GetCodeBreakpoint(breakpoint_address); if (cbpt != NULL) { return cbpt->OrigStubAddress(); @@ -4174,7 +4277,7 @@ bool Debugger::RemoveBreakpointFromTheList(intptr_t bp_id, // Remove references from code breakpoints to this breakpoint // location and disable them. // Latent breakpoint locations won't have code breakpoints. - UnlinkCodeBreakpoints(curr_loc); + group_debugger()->UnlinkCodeBreakpoints(curr_loc); } BreakpointLocation* next_loc = curr_loc->next(); delete curr_loc; @@ -4200,14 +4303,16 @@ bool Debugger::RemoveBreakpointFromTheList(intptr_t bp_id, // They will later be deleted when control returns from the pause event // callback. Also, disable the breakpoint so it no longer fires if it // should be hit before it gets deleted. -void Debugger::UnlinkCodeBreakpoints(BreakpointLocation* bpt_location) { - ASSERT(bpt_location != NULL); +void GroupDebugger::UnlinkCodeBreakpoints(BreakpointLocation* bpt_location) { + ASSERT(bpt_location != nullptr); + SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); CodeBreakpoint* curr_bpt = code_breakpoints_; - while (curr_bpt != NULL) { - if (curr_bpt->bpt_location() == bpt_location) { - curr_bpt->Disable(); - curr_bpt->set_bpt_location(NULL); - needs_breakpoint_cleanup_ = true; + while (curr_bpt != nullptr) { + if (curr_bpt->FindAndDeleteBreakpointLocation(bpt_location)) { + if (curr_bpt->HasNoBreakpointLocations()) { + curr_bpt->Disable(); + needs_breakpoint_cleanup_ = true; + } } curr_bpt = curr_bpt->next(); } @@ -4215,19 +4320,19 @@ void Debugger::UnlinkCodeBreakpoints(BreakpointLocation* bpt_location) { // Remove and delete unlinked code breakpoints, i.e. breakpoints that // are not associated with a breakpoint location. -void Debugger::RemoveUnlinkedCodeBreakpoints() { - CodeBreakpoint* prev_bpt = NULL; +void GroupDebugger::RemoveUnlinkedCodeBreakpoints() { + SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock()); + CodeBreakpoint* prev_bpt = nullptr; CodeBreakpoint* curr_bpt = code_breakpoints_; - while (curr_bpt != NULL) { - if (curr_bpt->bpt_location() == NULL) { - if (prev_bpt == NULL) { + while (curr_bpt != nullptr) { + if (curr_bpt->HasNoBreakpointLocations()) { + if (prev_bpt == nullptr) { code_breakpoints_ = code_breakpoints_->next(); } else { prev_bpt->set_next(curr_bpt->next()); } CodeBreakpoint* temp_bpt = curr_bpt; curr_bpt = curr_bpt->next(); - temp_bpt->Disable(); delete temp_bpt; } else { prev_bpt = curr_bpt; @@ -4317,7 +4422,7 @@ BreakpointLocation* Debugger::GetLatentBreakpoint(const String& url, loc = loc->next(); } // No breakpoint for this location requested. Allocate new one. - loc = new BreakpointLocation(url, line, column); + loc = new BreakpointLocation(this, url, line, column); loc->set_next(latent_locations_); latent_locations_ = loc; return loc; @@ -4329,12 +4434,6 @@ void Debugger::RegisterBreakpointLocation(BreakpointLocation* loc) { breakpoint_locations_ = loc; } -void Debugger::RegisterCodeBreakpoint(CodeBreakpoint* cbpt) { - ASSERT(cbpt->next() == NULL); - cbpt->set_next(code_breakpoints_); - code_breakpoints_ = cbpt; -} - #endif // !PRODUCT } // namespace dart diff --git a/runtime/vm/debugger.h b/runtime/vm/debugger.h index 6378fb1f69b..c833e5dd5c0 100644 --- a/runtime/vm/debugger.h +++ b/runtime/vm/debugger.h @@ -5,6 +5,8 @@ #ifndef RUNTIME_VM_DEBUGGER_H_ #define RUNTIME_VM_DEBUGGER_H_ +#include + #include "include/dart_tools_api.h" #include "vm/kernel_isolate.h" @@ -16,6 +18,8 @@ #include "vm/stack_frame.h" #include "vm/stack_trace.h" +#if !defined(PRODUCT) + DECLARE_FLAG(bool, verbose_debug); // 'Trace Debugger' TD_Print. @@ -120,13 +124,15 @@ class Breakpoint { class BreakpointLocation { public: // Create a new unresolved breakpoint. - BreakpointLocation(const Script& script, + BreakpointLocation(Debugger* debugger, + const Script& script, TokenPosition token_pos, TokenPosition end_token_pos, intptr_t requested_line_number, intptr_t requested_column_number); // Create a new latent breakpoint. - BreakpointLocation(const String& url, + BreakpointLocation(Debugger* debugger, + const String& url, intptr_t requested_line_number, intptr_t requested_column_number); @@ -154,6 +160,8 @@ class BreakpointLocation { bool IsResolved() const { return code_token_pos_.IsReal(); } bool IsLatent() const { return !token_pos_.IsReal(); } + Debugger* debugger() { return debugger_; } + private: void VisitObjectPointers(ObjectPointerVisitor* visitor); @@ -170,6 +178,7 @@ class BreakpointLocation { // Finds the breakpoint we hit at |location|. Breakpoint* FindHitBreakpoint(ActivationFrame* top_frame); + Debugger* debugger_; ScriptPtr script_; StringPtr url_; TokenPosition token_pos_; @@ -184,12 +193,16 @@ class BreakpointLocation { TokenPosition code_token_pos_; friend class Debugger; + friend class GroupDebugger; DISALLOW_COPY_AND_ASSIGN(BreakpointLocation); }; // CodeBreakpoint represents a location in compiled code. // There may be more than one CodeBreakpoint for one BreakpointLocation, // e.g. when a function gets compiled as a regular function and as a closure. +// There may be more than one BreakpointLocation associated with CodeBreakpoint, +// one for for every isolate in a group that sets a breakpoint at particular +// code location represented by the CodeBreakpoint. class CodeBreakpoint { public: CodeBreakpoint(const Code& code, @@ -201,6 +214,11 @@ class CodeBreakpoint { FunctionPtr function() const; uword pc() const { return pc_; } TokenPosition token_pos() const { return token_pos_; } + bool HasBreakpointLocation(BreakpointLocation* breakpoint_location); + bool FindAndDeleteBreakpointLocation(BreakpointLocation* breakpoint_location); + bool HasNoBreakpointLocations() { + return breakpoint_locations_.length() == 0; + } ScriptPtr SourceCode(); StringPtr SourceUrl(); @@ -215,8 +233,10 @@ class CodeBreakpoint { private: void VisitObjectPointers(ObjectPointerVisitor* visitor); - BreakpointLocation* bpt_location() const { return bpt_location_; } - void set_bpt_location(BreakpointLocation* value) { bpt_location_ = value; } + BreakpointLocation* FindBreakpointForDebugger(Debugger* debugger); + void AddBreakpointLocation(BreakpointLocation* value) { + breakpoint_locations_.Add(value); + } void set_next(CodeBreakpoint* value) { next_ = value; } CodeBreakpoint* next() const { return this->next_; } @@ -230,13 +250,16 @@ class CodeBreakpoint { intptr_t line_number_; bool is_enabled_; - BreakpointLocation* bpt_location_; + // Breakpoint locations from different debuggers/isolates that + // point to this code breakpoint. + MallocGrowableArray breakpoint_locations_; CodeBreakpoint* next_; UntaggedPcDescriptors::Kind breakpoint_kind_; CodePtr saved_value_; friend class Debugger; + friend class GroupDebugger; DISALLOW_COPY_AND_ASSIGN(CodeBreakpoint); }; @@ -473,6 +496,52 @@ typedef enum { kInvalidExceptionPauseInfo } Dart_ExceptionPauseInfo; +class GroupDebugger { + public: + explicit GroupDebugger(IsolateGroup* isolate_group); + ~GroupDebugger(); + + void MakeCodeBreakpointAt(const Function& func, BreakpointLocation* bpt); + // Returns [nullptr] if no breakpoint exists for the given address. + CodeBreakpoint* GetCodeBreakpoint(uword breakpoint_address); + BreakpointLocation* GetBreakpointLocationFor(Debugger* debugger, + uword breakpoint_address, + CodeBreakpoint** pcbpt); + CodePtr GetPatchedStubAddress(uword breakpoint_address); + + void UnlinkCodeBreakpoints(BreakpointLocation* bpt_location); + + // Returns true if the call at address pc is patched to point to + // a debugger stub. + bool HasActiveBreakpoint(uword pc); + bool HasCodeBreakpointInFunction(const Function& func); + bool HasCodeBreakpointInCode(const Code& code); + + bool HasBreakpointInFunction(const Function& func); + bool HasBreakpointInCode(const Code& code); + + void SyncBreakpointLocation(BreakpointLocation* loc); + void DisableCodeBreakpointsFor(BreakpointLocation* loc); + + void Pause(); + + void VisitObjectPointers(ObjectPointerVisitor* visitor); + + private: + IsolateGroup* isolate_group_; + + std::unique_ptr code_breakpoints_lock_; + CodeBreakpoint* code_breakpoints_; + + SafepointRwLock* code_breakpoints_lock() { + return code_breakpoints_lock_.get(); + } + void RemoveUnlinkedCodeBreakpoints(); + void RegisterCodeBreakpoint(CodeBreakpoint* bpt); + + bool needs_breakpoint_cleanup_; +}; + class Debugger { public: enum ResumeAction { @@ -560,11 +629,6 @@ class Debugger { // This may be called from different threads, therefore do not use the, // debugger's zone. bool HasBreakpoint(const Function& func, Zone* zone); - bool HasBreakpoint(const Code& code); - - // Returns true if the call at address pc is patched to point to - // a debugger stub. - bool HasActiveBreakpoint(uword pc); // Returns a stack trace with frames corresponding to invisible functions // omitted. CurrentStackTrace always returns a new trace on the current stack. @@ -595,14 +659,11 @@ class Debugger { // Dart. void PauseDeveloper(const String& msg); - CodePtr GetPatchedStubAddress(uword breakpoint_address); - void PrintBreakpointsToJSONArray(JSONArray* jsarr) const; void PrintSettingsToJSONObject(JSONObject* jsobj) const; static bool IsDebuggable(const Function& func); - // IsolateGroupDebugger:: static bool IsDebugging(Thread* thread, const Function& func); intptr_t limitBreakpointId() { return next_id_; } @@ -621,17 +682,14 @@ class Debugger { void SendBreakpointEvent(ServiceEvent::EventKind kind, Breakpoint* bpt); - // IsolateGroupDebugger:: void FindCompiledFunctions(const Script& script, TokenPosition start_pos, TokenPosition end_pos, GrowableObjectArray* code_function_list); - // IsolateGroupDebugger:: bool FindBestFit(const Script& script, TokenPosition token_pos, TokenPosition last_token_pos, Function* best_fit); - // IsolateGroupDebugger:: void DeoptimizeWorld(); void NotifySingleStepping(bool value) const; BreakpointLocation* SetCodeBreakpoints(const Script& script, @@ -649,24 +707,17 @@ class Debugger { const Function& function); bool RemoveBreakpointFromTheList(intptr_t bp_id, BreakpointLocation** list); Breakpoint* GetBreakpointByIdInTheList(intptr_t id, BreakpointLocation* list); - void RemoveUnlinkedCodeBreakpoints(); - void UnlinkCodeBreakpoints(BreakpointLocation* bpt_location); BreakpointLocation* GetLatentBreakpoint(const String& url, intptr_t line, intptr_t column); void RegisterBreakpointLocation(BreakpointLocation* bpt); - void RegisterCodeBreakpoint(CodeBreakpoint* bpt); BreakpointLocation* GetBreakpointLocation( const Script& script, TokenPosition token_pos, intptr_t requested_line, intptr_t requested_column, TokenPosition code_token_pos = TokenPosition::kNoSource); - void MakeCodeBreakpointAt(const Function& func, BreakpointLocation* bpt); - // Returns NULL if no breakpoint exists for the given address. - CodeBreakpoint* GetCodeBreakpoint(uword breakpoint_address); - void SyncBreakpointLocation(BreakpointLocation* loc); void PrintBreakpointsListToJSONArray(BreakpointLocation* sbpt, JSONArray* jsarr) const; @@ -702,6 +753,8 @@ class Debugger { void SetAsyncSteppingFramePointer(DebuggerStackTrace* stack_trace); void SetSyncSteppingFramePointer(DebuggerStackTrace* stack_trace); + GroupDebugger* group_debugger() { return isolate_->group()->debugger(); } + Isolate* isolate_; // ID number generator. @@ -709,7 +762,6 @@ class Debugger { BreakpointLocation* latent_locations_; BreakpointLocation* breakpoint_locations_; - CodeBreakpoint* code_breakpoints_; // Tells debugger what to do when resuming execution after a breakpoint. ResumeAction resume_action_; @@ -751,8 +803,6 @@ class Debugger { // breakpoint. bool skip_next_step_; - bool needs_breakpoint_cleanup_; - // We keep this breakpoint alive until after the debugger does the step over // async continuation machinery so that we can report that we've stopped // at the breakpoint. @@ -787,4 +837,6 @@ class DisableBreakpointsScope : public ValueObject { } // namespace dart +#endif // !defined(PRODUCT) + #endif // RUNTIME_VM_DEBUGGER_H_ diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 1cb889682f8..495d026d8e0 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -386,7 +386,12 @@ IsolateGroup::IsolateGroup(std::shared_ptr source, boxed_field_list_(GrowableObjectArray::null()), program_lock_(new SafepointRwLock()), active_mutators_monitor_(new Monitor()), - max_active_mutators_(Scavenger::MaxMutatorThreadCount()) { + max_active_mutators_(Scavenger::MaxMutatorThreadCount()) +#if !defined(PRODUCT) + , + debugger_(new GroupDebugger(this)) +#endif +{ FlagsCopyFrom(api_flags); const bool is_vm_isolate = Dart::VmIsolateNameEquals(source_->name); if (!is_vm_isolate) { @@ -425,6 +430,11 @@ IsolateGroup::~IsolateGroup() { } delete[] obfuscation_map_; } + +#if !defined(PRODUCT) + delete debugger_; + debugger_ = nullptr; +#endif } void IsolateGroup::RegisterIsolate(Isolate* isolate) { @@ -2528,7 +2538,7 @@ void Isolate::LowLevelCleanup(Isolate* isolate) { } // Cache these two fields, since they are no longer available after the - // `delete this` further down. + // `delete isolate` further down. IsolateGroup* isolate_group = isolate->isolate_group_; Dart_IsolateCleanupCallback cleanup = isolate->on_cleanup_callback(); auto callback_data = isolate->init_callback_data_; @@ -2811,6 +2821,12 @@ void IsolateGroup::VisitObjectPointers(ObjectPointerVisitor* visitor, NOT_IN_PRECOMPILED(background_compiler()->VisitPointers(visitor)); +#if !defined(PRODUCT) + if (debugger() != nullptr) { + debugger()->VisitObjectPointers(visitor); + } +#endif + #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME) // Visit objects that are being used for isolate reload. if (program_reload_context() != nullptr) { diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index d146eac24db..90f557ad43d 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -51,6 +51,7 @@ class CodeIndexTable; class Debugger; class DeoptContext; class ExternalTypedData; +class GroupDebugger; class HandleScope; class HandleVisitor; class Heap; @@ -344,6 +345,10 @@ class IsolateGroup : public IntrusiveDListEntry { #endif } +#if !defined(PRODUCT) + GroupDebugger* debugger() const { return debugger_; } +#endif + IdleTimeHandler* idle_time_handler() { return &idle_time_handler_; } // Returns true if this is the first isolate registered. @@ -938,6 +943,8 @@ class IsolateGroup : public IntrusiveDListEntry { intptr_t active_mutators_ = 0; intptr_t waiting_mutators_ = 0; intptr_t max_active_mutators_ = 0; + + NOT_IN_PRODUCT(GroupDebugger* debugger_ = nullptr); }; // When an isolate sends-and-exits this class represent things that it passed diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index e1e0d8248ce..37da0f55d93 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -16333,17 +16333,7 @@ bool Code::HasBreakpoint() const { #if defined(PRODUCT) return false; #else - // TODO(dartbug.com/36097): We might need to adjust this once we start adding - // debugging support to --enable-isolate-groups. - auto isolate_group = Thread::Current()->isolate_group(); - - bool has_breakpoint = false; - isolate_group->ForEachIsolate([&](Isolate* isolate) { - if (isolate->debugger()->HasBreakpoint(*this)) { - has_breakpoint = true; - } - }); - return has_breakpoint; + return IsolateGroup::Current()->debugger()->HasBreakpointInCode(*this); #endif } diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 5b562acc02a..95d3875d6ae 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -1146,7 +1146,8 @@ DEFINE_RUNTIME_ENTRY(BreakpointRuntimeHandler, 0) { StackFrame* caller_frame = iterator.NextFrame(); ASSERT(caller_frame != NULL); Code& orig_stub = Code::Handle(zone); - orig_stub = isolate->debugger()->GetPatchedStubAddress(caller_frame->pc()); + orig_stub = + isolate->group()->debugger()->GetPatchedStubAddress(caller_frame->pc()); const Error& error = Error::Handle(zone, isolate->debugger()->PauseBreakpoint()); ThrowIfError(error);