diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc index 77683fa6a16..c9f88837a73 100644 --- a/runtime/vm/compiler/jit/compiler.cc +++ b/runtime/vm/compiler/jit/compiler.cc @@ -233,7 +233,7 @@ DEFINE_RUNTIME_ENTRY(CompileFunction, 1) { bool Compiler::CanOptimizeFunction(Thread* thread, const Function& function) { #if !defined(PRODUCT) - if (Debugger::IsDebugging(thread, function)) { + if (thread->isolate_group()->debugger()->IsDebugging(thread, function)) { // We cannot set breakpoints and single step in optimized code, // so do not optimize the function. Bump usage counter down to avoid // repeatedly entering the runtime for an optimization attempt. diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index e90e6ad9116..d057516fd0a 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -332,7 +332,7 @@ ErrorPtr Debugger::PauseRequest(ServiceEvent::EventKind kind) { } CacheStackTraces(trace, DebuggerStackTrace::CollectAsyncCausal(), DebuggerStackTrace::CollectAwaiterReturn()); - resume_action_ = kContinue; + set_resume_action(kContinue); Pause(&event); HandleSteppingRequest(trace); ClearCachedStackTraces(); @@ -459,27 +459,6 @@ static bool IsImplicitFunction(const Function& func) { return false; } -bool Debugger::HasBreakpoint(const Function& func, Zone* zone) { - if (!func.HasCode()) { - // If the function is not compiled yet, just check whether there - // is a user-defined breakpoint that falls into the token - // range of the function. This may be a false positive: the breakpoint - // might be inside a local closure. - Script& script = Script::Handle(zone); - BreakpointLocation* sbpt = breakpoint_locations_; - while (sbpt != NULL) { - script = sbpt->script(); - if (FunctionOverlaps(func, script, sbpt->token_pos(), - sbpt->end_token_pos())) { - return true; - } - sbpt = sbpt->next_; - } - return false; - } - return group_debugger()->HasCodeBreakpointInFunction(func); -} - bool GroupDebugger::HasCodeBreakpointInFunction(const Function& func) { SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock()); CodeBreakpoint* cbpt = code_breakpoints_; @@ -1558,6 +1537,7 @@ GroupDebugger::GroupDebugger(IsolateGroup* isolate_group) code_breakpoints_lock_(new SafepointRwLock()), code_breakpoints_(nullptr), breakpoint_locations_lock_(new SafepointRwLock()), + single_stepping_set_lock_(new SafepointRwLock()), needs_breakpoint_cleanup_(false) {} GroupDebugger::~GroupDebugger() { @@ -1671,13 +1651,13 @@ bool Debugger::SetResumeAction(ResumeAction action, case kStepOver: case kStepOut: case kContinue: - resume_action_ = action; + set_resume_action(action); return true; case kStepRewind: if (!CanRewindFrame(frame_index, error)) { return false; } - resume_action_ = kStepRewind; + set_resume_action(kStepRewind); resume_frame_index_ = frame_index; return true; case kStepOverAsyncSuspension: @@ -3315,6 +3295,7 @@ void Debugger::Pause(ServiceEvent* event) { } void GroupDebugger::Pause() { + SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock()); if (needs_breakpoint_cleanup_) { RemoveUnlinkedCodeBreakpoints(); } @@ -3603,7 +3584,7 @@ void Debugger::RewindToUnoptimizedFrame(StackFrame* frame, const Code& code) { // We will be jumping out of the debugger rather than exiting this // function, so prepare the debugger state. ClearCachedStackTraces(); - resume_action_ = kContinue; + set_resume_action(kContinue); resume_frame_index_ = -1; EnterSingleStepMode(); @@ -3634,7 +3615,7 @@ void Debugger::RewindToOptimizedFrame(StackFrame* frame, // We will be jumping out of the debugger rather than exiting this // function, so prepare the debugger state. ClearCachedStackTraces(); - resume_action_ = kContinue; + set_resume_action(kContinue); resume_frame_index_ = -1; EnterSingleStepMode(); @@ -3707,28 +3688,67 @@ bool Debugger::IsDebuggable(const Function& func) { return lib.IsDebuggable(); } -bool Debugger::IsDebugging(Thread* thread, const Function& func) { - // TODO(dartbug.com/36097): We might need to adjust this once we start adding - // debugging support to --enable-isolate-groups. - auto isolate_group = thread->isolate_group(); +void GroupDebugger::RegisterSingleSteppingDebugger(Thread* thread, + const Debugger* debugger) { + ASSERT(single_stepping_set_lock()->IsCurrentThreadWriter()); + single_stepping_set_.Insert(debugger); +} - bool has_breakpoint = false; - bool is_single_stepping = false; - isolate_group->ForEachIsolate( - [&](Isolate* isolate) { - if (isolate->debugger()->IsStepping()) { - is_single_stepping = true; - } - if (isolate->debugger()->HasBreakpoint(func, thread->zone())) { - has_breakpoint = true; - } - }, - thread->IsAtSafepoint()); - return has_breakpoint || is_single_stepping; +void GroupDebugger::UnregisterSingleSteppingDebugger(Thread* thread, + const Debugger* debugger) { + ASSERT(single_stepping_set_lock()->IsCurrentThreadWriter()); + single_stepping_set_.Remove(debugger); +} + +bool GroupDebugger::HasBreakpoint(Thread* thread, const Function& function) { + { + // Check if function has any breakpoints. + SafepointReadRwLocker sl(thread, breakpoint_locations_lock()); + Script& script = Script::Handle(thread->zone()); + for (intptr_t i = 0; i < breakpoint_locations_.length(); i++) { + BreakpointLocation* location = breakpoint_locations_.At(i); + script = location->script(); + if (FunctionOverlaps(function, script, location->token_pos(), + location->end_token_pos())) { + return true; + } + } + } + // TODO(aam): do we have to iterate over both code breakpoints and + // breakpoint locations? Wouldn't be sufficient to iterate over only + // one list? Could you have a CodeBreakpoint without corresponding + // BreakpointLocation? + if (HasCodeBreakpointInFunction(function)) { + return true; + } + + return false; +} + +bool GroupDebugger::IsDebugging(Thread* thread, const Function& function) { + { + SafepointReadRwLocker sl(thread, single_stepping_set_lock()); + if (!single_stepping_set_.IsEmpty()) { + return true; + } + } + return HasBreakpoint(thread, function); +} + +void Debugger::set_resume_action(ResumeAction resume_action) { + auto thread = Thread::Current(); + SafepointWriteRwLocker sl(thread, + group_debugger()->single_stepping_set_lock()); + if (resume_action == kContinue) { + group_debugger()->UnregisterSingleSteppingDebugger(thread, this); + } else { + group_debugger()->RegisterSingleSteppingDebugger(thread, this); + } + resume_action_ = resume_action; } void Debugger::SignalPausedEvent(ActivationFrame* top_frame, Breakpoint* bpt) { - resume_action_ = kContinue; + set_resume_action(kContinue); ResetSteppingFramePointers(); NotifySingleStepping(false); ASSERT(!IsPaused()); @@ -4354,7 +4374,7 @@ void GroupDebugger::UnlinkCodeBreakpoints(BreakpointLocation* bpt_location) { // Remove and delete unlinked code breakpoints, i.e. breakpoints that // are not associated with a breakpoint location. void GroupDebugger::RemoveUnlinkedCodeBreakpoints() { - SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock()); + ASSERT(code_breakpoints_lock()->IsCurrentThreadWriter()); CodeBreakpoint* prev_bpt = nullptr; CodeBreakpoint* curr_bpt = code_breakpoints_; while (curr_bpt != nullptr) { diff --git a/runtime/vm/debugger.h b/runtime/vm/debugger.h index 29b7092e542..ccedca94af3 100644 --- a/runtime/vm/debugger.h +++ b/runtime/vm/debugger.h @@ -519,6 +519,44 @@ typedef enum { kInvalidExceptionPauseInfo } Dart_ExceptionPauseInfo; +class DebuggerKeyValueTrait : public AllStatic { + public: + typedef const Debugger* Key; + typedef bool Value; + + struct Pair { + Key key; + Value value; + Pair() : key(NULL), value(false) {} + Pair(const Key key, const Value& value) : key(key), value(value) {} + Pair(const Pair& other) : key(other.key), value(other.value) {} + Pair& operator=(const Pair&) = default; + }; + + static Key KeyOf(Pair kv) { return kv.key; } + static Value ValueOf(Pair kv) { return kv.value; } + static intptr_t Hashcode(Key key) { return reinterpret_cast(key); } + static bool IsKeyEqual(Pair kv, Key key) { return kv.key == key; } +}; + +class DebuggerSet : public MallocDirectChainedHashMap { + public: + typedef DebuggerKeyValueTrait::Key Key; + typedef DebuggerKeyValueTrait::Value Value; + typedef DebuggerKeyValueTrait::Pair Pair; + + virtual ~DebuggerSet() { Clear(); } + + void Insert(const Key& key) { + Pair pair(key, /*value=*/true); + MallocDirectChainedHashMap::Insert(pair); + } + + void Remove(const Key& key) { + MallocDirectChainedHashMap::Remove(key); + } +}; + class GroupDebugger { public: explicit GroupDebugger(IsolateGroup* isolate_group); @@ -563,6 +601,18 @@ class GroupDebugger { return breakpoint_locations_lock_.get(); } + SafepointRwLock* single_stepping_set_lock() { + return single_stepping_set_lock_.get(); + } + void RegisterSingleSteppingDebugger(Thread* thread, const Debugger* debugger); + void UnregisterSingleSteppingDebugger(Thread* thread, + const Debugger* debugger); + + // Returns true if there is at least one breakpoint set in function or code. + // Checks for both user-defined and internal temporary breakpoints. + bool HasBreakpoint(Thread* thread, const Function& function); + bool IsDebugging(Thread* thread, const Function& function); + private: IsolateGroup* isolate_group_; @@ -575,6 +625,9 @@ class GroupDebugger { std::unique_ptr breakpoint_locations_lock_; MallocGrowableArray breakpoint_locations_; + std::unique_ptr single_stepping_set_lock_; + DebuggerSet single_stepping_set_; + SafepointRwLock* code_breakpoints_lock() { return code_breakpoints_lock_.get(); } @@ -665,12 +718,6 @@ class Debugger { void VisitObjectPointers(ObjectPointerVisitor* visitor); - // Returns true if there is at least one breakpoint set in func or code. - // Checks for both user-defined and internal temporary breakpoints. - // This may be called from different threads, therefore do not use the, - // debugger's zone. - bool HasBreakpoint(const Function& func, Zone* zone); - // Returns a stack trace with frames corresponding to invisible functions // omitted. CurrentStackTrace always returns a new trace on the current stack. // The trace returned by StackTrace may have been cached; it is suitable for @@ -705,8 +752,6 @@ class Debugger { static bool IsDebuggable(const Function& func); - static bool IsDebugging(Thread* thread, const Function& func); - intptr_t limitBreakpointId() { return next_id_; } // Callback to the debugger to continue frame rewind, post-deoptimization. @@ -809,6 +854,7 @@ class Debugger { // Tells debugger what to do when resuming execution after a breakpoint. ResumeAction resume_action_; + void set_resume_action(ResumeAction action); intptr_t resume_frame_index_; intptr_t post_deopt_frame_index_; diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 2c1485453c6..1a6494edeaf 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -6820,19 +6820,8 @@ bool Function::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 thread = Thread::Current(); - auto zone = thread->zone(); - auto isolate_group = thread->isolate_group(); - - bool has_breakpoint = false; - isolate_group->ForEachIsolate([&](Isolate* isolate) { - if (isolate->debugger()->HasBreakpoint(*this, zone)) { - has_breakpoint = true; - } - }); - return has_breakpoint; + return thread->isolate_group()->debugger()->HasBreakpoint(thread, *this); #endif } diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 92778d35e61..23452906f9e 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -1274,7 +1274,10 @@ static void TrySwitchInstanceCall(Thread* thread, #if !defined(PRODUCT) // Skip functions that contain breakpoints or when debugger is in single // stepping mode. - if (Debugger::IsDebugging(thread, caller_function)) return; + if (thread->isolate_group()->debugger()->IsDebugging(thread, + caller_function)) { + return; + } #endif const intptr_t num_checks = ic_data.NumberOfChecks();