From 07ba1be7f54ed5c44e0ee0b06839145aff801258 Mon Sep 17 00:00:00 2001 From: Ben Vanik Date: Fri, 1 Jan 2016 15:35:48 -0800 Subject: [PATCH] Switching debugger to not retain XThreads. --- src/xenia/debug/debugger.cc | 48 ++++++++++++++---------------- src/xenia/debug/debugger.h | 11 ++++--- src/xenia/debug/ui/debug_window.cc | 39 +++++++++++------------- src/xenia/debug/ui/debug_window.h | 5 ++-- 4 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/xenia/debug/debugger.cc b/src/xenia/debug/debugger.cc index 06b7a7724..0393649bc 100644 --- a/src/xenia/debug/debugger.cc +++ b/src/xenia/debug/debugger.cc @@ -183,10 +183,9 @@ std::vector Debugger::QueryThreadExecutionInfos() { return result; } -ThreadExecutionInfo* Debugger::QueryThreadExecutionInfo( - uint32_t thread_handle) { +ThreadExecutionInfo* Debugger::QueryThreadExecutionInfo(uint32_t thread_id) { auto global_lock = global_critical_region_.Acquire(); - const auto& it = thread_execution_infos_.find(thread_handle); + const auto& it = thread_execution_infos_.find(thread_id); if (it == thread_execution_infos_.end()) { return nullptr; } @@ -232,8 +231,7 @@ bool Debugger::SuspendAllThreads() { // Thread is a host thread, and we aren't suspending those (for now). continue; } else if (XThread::IsInThread() && - thread_info->thread_handle == - XThread::GetCurrentThreadHandle()) { + thread_info->thread_id == XThread::GetCurrentThreadId()) { // Can't suspend ourselves. continue; } @@ -244,9 +242,9 @@ bool Debugger::SuspendAllThreads() { return true; } -bool Debugger::ResumeThread(uint32_t thread_handle) { +bool Debugger::ResumeThread(uint32_t thread_id) { auto global_lock = global_critical_region_.Acquire(); - auto it = thread_execution_infos_.find(thread_handle); + auto it = thread_execution_infos_.find(thread_id); if (it == thread_execution_infos_.end()) { return false; } @@ -273,8 +271,7 @@ bool Debugger::ResumeAllThreads() { // Thread is a host thread, and we aren't suspending those (for now). continue; } else if (XThread::IsInThread() && - thread_info->thread_handle == - XThread::GetCurrentThreadHandle()) { + thread_info->thread_id == XThread::GetCurrentThreadId()) { // Can't resume ourselves. continue; } @@ -285,7 +282,7 @@ bool Debugger::ResumeAllThreads() { return true; } -void Debugger::UpdateThreadExecutionStates(uint32_t override_handle, +void Debugger::UpdateThreadExecutionStates(uint32_t override_thread_id, X64Context* override_context) { auto global_lock = global_critical_region_.Acquire(); auto stack_walker = emulator_->processor()->stack_walker(); @@ -310,7 +307,7 @@ void Debugger::UpdateThreadExecutionStates(uint32_t override_handle, // Grab stack trace and X64 context then resolve all symbols. uint64_t hash; X64Context* in_host_context = nullptr; - if (override_handle == thread_info->thread_handle) { + if (override_thread_id == thread_info->thread_id) { // If we were passed an override context we use that. Otherwise, ask the // stack walker for a new context. in_host_context = override_context; @@ -392,12 +389,12 @@ void Debugger::Continue() { } } -void Debugger::StepGuestInstruction(uint32_t thread_handle) { +void Debugger::StepGuestInstruction(uint32_t thread_id) { auto global_lock = global_critical_region_.Acquire(); assert_true(execution_state_ == ExecutionState::kPaused); execution_state_ = ExecutionState::kStepping; - auto thread_info = thread_execution_infos_[thread_handle].get(); + auto thread_info = thread_execution_infos_[thread_id].get(); uint32_t next_pc = CalculateNextGuestInstruction( thread_info, thread_info->frames[0].guest_pc); @@ -409,15 +406,15 @@ void Debugger::StepGuestInstruction(uint32_t thread_handle) { thread_info->step_breakpoint->Resume(); // ResumeAllBreakpoints(); - ResumeThread(thread_handle); + ResumeThread(thread_id); } -void Debugger::StepHostInstruction(uint32_t thread_handle) { +void Debugger::StepHostInstruction(uint32_t thread_id) { auto global_lock = global_critical_region_.Acquire(); assert_true(execution_state_ == ExecutionState::kPaused); execution_state_ = ExecutionState::kStepping; - auto thread_info = thread_execution_infos_[thread_handle].get(); + auto thread_info = thread_execution_infos_[thread_id].get(); uint64_t new_host_pc = CalculateNextHostInstruction(thread_info, thread_info->frames[0].host_pc); @@ -428,23 +425,24 @@ void Debugger::StepHostInstruction(uint32_t thread_handle) { thread_info->step_breakpoint->Resume(); // ResumeAllBreakpoints(); - ResumeThread(thread_handle); + ResumeThread(thread_id); } void Debugger::OnThreadCreated(XThread* thread) { auto global_lock = global_critical_region_.Acquire(); auto thread_info = std::make_unique(); thread_info->thread_handle = thread->handle(); + thread_info->thread_id = thread->thread_id(); thread_info->thread = thread; thread_info->state = ThreadExecutionInfo::State::kAlive; thread_info->suspended = false; - thread_execution_infos_.emplace(thread_info->thread_handle, + thread_execution_infos_.emplace(thread_info->thread_id, std::move(thread_info)); } void Debugger::OnThreadExit(XThread* thread) { auto global_lock = global_critical_region_.Acquire(); - auto it = thread_execution_infos_.find(thread->handle()); + auto it = thread_execution_infos_.find(thread->thread_id()); assert_true(it != thread_execution_infos_.end()); auto thread_info = it->second.get(); thread_info->state = ThreadExecutionInfo::State::kExited; @@ -452,7 +450,7 @@ void Debugger::OnThreadExit(XThread* thread) { void Debugger::OnThreadDestroyed(XThread* thread) { auto global_lock = global_critical_region_.Acquire(); - auto it = thread_execution_infos_.find(thread->handle()); + auto it = thread_execution_infos_.find(thread->thread_id()); assert_true(it != thread_execution_infos_.end()); auto thread_info = it->second.get(); // TODO(benvanik): retain the thread? @@ -462,7 +460,7 @@ void Debugger::OnThreadDestroyed(XThread* thread) { void Debugger::OnThreadEnteringWait(XThread* thread) { auto global_lock = global_critical_region_.Acquire(); - auto it = thread_execution_infos_.find(thread->handle()); + auto it = thread_execution_infos_.find(thread->thread_id()); assert_true(it != thread_execution_infos_.end()); auto thread_info = it->second.get(); thread_info->state = ThreadExecutionInfo::State::kWaiting; @@ -470,7 +468,7 @@ void Debugger::OnThreadEnteringWait(XThread* thread) { void Debugger::OnThreadLeavingWait(XThread* thread) { auto global_lock = global_critical_region_.Acquire(); - auto it = thread_execution_infos_.find(thread->handle()); + auto it = thread_execution_infos_.find(thread->thread_id()); assert_true(it != thread_execution_infos_.end()); auto thread_info = it->second.get(); if (thread_info->state == ThreadExecutionInfo::State::kWaiting) { @@ -531,7 +529,7 @@ bool Debugger::ExceptionCallback(Exception* ex) { SuspendAllThreads(); // Lookup thread info block. - auto it = thread_execution_infos_.find(XThread::GetCurrentThreadHandle()); + auto it = thread_execution_infos_.find(XThread::GetCurrentThreadId()); if (it == thread_execution_infos_.end()) { // Not found - exception on a thread we don't know about? assert_always("UD2 on a thread we don't track"); @@ -548,7 +546,7 @@ bool Debugger::ExceptionCallback(Exception* ex) { // Update all thread states with their latest values, using the context we got // from the exception instead of a sampled value (as it would just show the // exception handler). - UpdateThreadExecutionStates(thread_info->thread_handle, ex->thread_context()); + UpdateThreadExecutionStates(thread_info->thread_id, ex->thread_context()); // Whether we should block waiting for a continue event. bool wait_for_continue = false; @@ -673,7 +671,7 @@ bool Debugger::OnUnhandledException(Exception* ex) { // Suspend all guest threads (but this one). SuspendAllThreads(); - UpdateThreadExecutionStates(XThread::GetCurrentThreadHandle(), + UpdateThreadExecutionStates(XThread::GetCurrentThreadId(), ex->thread_context()); // Stop and notify the listener. diff --git a/src/xenia/debug/debugger.h b/src/xenia/debug/debugger.h index 8b1a77a9a..bf6ff3016 100644 --- a/src/xenia/debug/debugger.h +++ b/src/xenia/debug/debugger.h @@ -74,7 +74,10 @@ struct ThreadExecutionInfo { kZombie, }; + // XThread::thread_id(), unique to the thread for the run of the emulator. + uint32_t thread_id = 0; // XThread::handle() of the thread. + // This will be invalidated when the thread dies. uint32_t thread_handle = 0; // Target XThread, if it has not been destroyed. // TODO(benvanik): hold a ref here to keep zombie threads around? @@ -203,7 +206,7 @@ class Debugger { // the kernel. std::vector QueryThreadExecutionInfos(); // Returns the debugger info for the given thread. - ThreadExecutionInfo* QueryThreadExecutionInfo(uint32_t thread_handle); + ThreadExecutionInfo* QueryThreadExecutionInfo(uint32_t thread_id); // Adds a breakpoint to the debugger and activates it (if enabled). // The given breakpoint will not be owned by the debugger and must remain @@ -230,10 +233,10 @@ class Debugger { // Steps the given thread a single PPC guest instruction. // If the step is over a branch the branch will be followed. - void StepGuestInstruction(uint32_t thread_handle); + void StepGuestInstruction(uint32_t thread_id); // Steps the given thread a single x64 host instruction. // If the step is over a branch the branch will be followed. - void StepHostInstruction(uint32_t thread_handle); + void StepHostInstruction(uint32_t thread_id); public: // TODO(benvanik): hide. @@ -254,7 +257,7 @@ class Debugger { // Suspends all known threads (except the caller). bool SuspendAllThreads(); // Resumes the given thread. - bool ResumeThread(uint32_t thread_handle); + bool ResumeThread(uint32_t thread_id); // Resumes all known threads (except the caller). bool ResumeAllThreads(); // Updates all cached thread execution info (state, call stacks, etc). diff --git a/src/xenia/debug/ui/debug_window.cc b/src/xenia/debug/ui/debug_window.cc index cdb87afa4..77cdd45e1 100644 --- a/src/xenia/debug/ui/debug_window.cc +++ b/src/xenia/debug/ui/debug_window.cc @@ -293,8 +293,8 @@ void DebugWindow::DrawToolbar() { if (ImGui::Combo("##thread_combo", ¤t_thread_index, thread_combo.GetString(), 10)) { // Thread changed. - SelectThreadStackFrame( - cache_.thread_execution_infos[current_thread_index]->thread, 0, true); + SelectThreadStackFrame(cache_.thread_execution_infos[current_thread_index], + 0, true); } } @@ -347,12 +347,12 @@ void DebugWindow::DrawSourcePane() { ImGui::BeginGroup(); ImGui::PushButtonRepeat(true); - bool can_step = !cache_.is_running && state_.thread; + bool can_step = !cache_.is_running && state_.thread_info; if (ImGui::ButtonEx("Step PPC", ImVec2(0, 0), can_step ? 0 : ImGuiButtonFlags_Disabled)) { // By enabling the button when stepping we allow repeat behavior. if (debugger_->execution_state() != ExecutionState::kStepping) { - debugger_->StepGuestInstruction(state_.thread->handle()); + debugger_->StepGuestInstruction(state_.thread_info->thread_id); } } ImGui::PopButtonRepeat(); @@ -370,7 +370,7 @@ void DebugWindow::DrawSourcePane() { can_step ? 0 : ImGuiButtonFlags_Disabled)) { // By enabling the button when stepping we allow repeat behavior. if (debugger_->execution_state() != ExecutionState::kStepping) { - debugger_->StepHostInstruction(state_.thread->handle()); + debugger_->StepHostInstruction(state_.thread_info->thread_id); } } ImGui::PopButtonRepeat(); @@ -993,8 +993,8 @@ void DebugWindow::DrawThreadsPane() { ImGui::BeginChild("##threads_listing"); for (size_t i = 0; i < cache_.thread_execution_infos.size(); ++i) { auto thread_info = cache_.thread_execution_infos[i]; + bool is_current_thread = thread_info == state_.thread_info; auto thread = thread_info->thread; - bool is_current_thread = thread == state_.thread; if (!thread) { // TODO(benvanik): better display of zombie thread states. continue; @@ -1009,7 +1009,7 @@ void DebugWindow::DrawThreadsPane() { ImGui::PushStyleColor(ImGuiCol_Header, ImGui::GetStyle().Colors[ImGuiCol_HeaderActive]); } - ImGui::PushID(thread); + ImGui::PushID(thread_info); if (is_current_thread) { ImGui::SetNextTreeNodeOpened(true, ImGuiSetCond_Always); } @@ -1049,7 +1049,7 @@ void DebugWindow::DrawThreadsPane() { frame.host_pc, &frame); if (ImGui::Selectable(host_label, is_current_frame, ImGuiSelectableFlags_SpanAllColumns)) { - SelectThreadStackFrame(thread, j, true); + SelectThreadStackFrame(thread_info, j, true); } ImGui::SameLine(); ImGui::Dummy(ImVec2(8, 0)); @@ -1386,21 +1386,13 @@ void DebugWindow::DrawLogPane() { // if big, click to open dialog with contents } -void DebugWindow::SelectThreadStackFrame(XThread* thread, +void DebugWindow::SelectThreadStackFrame(ThreadExecutionInfo* thread_info, size_t stack_frame_index, bool always_navigate) { state_.has_changed_thread = false; - if (thread != state_.thread) { + if (thread_info != state_.thread_info) { state_.has_changed_thread = true; - state_.thread = thread; - - state_.thread_info = nullptr; - for (auto thread_info : cache_.thread_execution_infos) { - if (thread_info->thread == thread) { - state_.thread_info = thread_info; - break; - } - } + state_.thread_info = thread_info; } if (state_.thread_info) { stack_frame_index = @@ -1469,7 +1461,8 @@ void DebugWindow::UpdateCache() { cache_.thread_execution_infos = debugger_->QueryThreadExecutionInfos(); - SelectThreadStackFrame(state_.thread, state_.thread_stack_frame_index, false); + SelectThreadStackFrame(state_.thread_info, state_.thread_stack_frame_index, + false); } void DebugWindow::CreateCodeBreakpoint(CodeBreakpoint::AddressType address_type, @@ -1582,14 +1575,16 @@ void DebugWindow::OnExecutionEnded() { void DebugWindow::OnStepCompleted(xe::kernel::XThread* thread) { UpdateCache(); - SelectThreadStackFrame(thread, 0, true); + auto thread_info = debugger_->QueryThreadExecutionInfo(thread->thread_id()); + SelectThreadStackFrame(thread_info, 0, true); loop_->Post([this]() { window_->set_focus(true); }); } void DebugWindow::OnBreakpointHit(Breakpoint* breakpoint, xe::kernel::XThread* thread) { UpdateCache(); - SelectThreadStackFrame(thread, 0, true); + auto thread_info = debugger_->QueryThreadExecutionInfo(thread->thread_id()); + SelectThreadStackFrame(thread_info, 0, true); loop_->Post([this]() { window_->set_focus(true); }); } diff --git a/src/xenia/debug/ui/debug_window.h b/src/xenia/debug/ui/debug_window.h index f62c644ab..c67503831 100644 --- a/src/xenia/debug/ui/debug_window.h +++ b/src/xenia/debug/ui/debug_window.h @@ -69,8 +69,8 @@ class DebugWindow : public DebugListener { void DrawBreakpointsPane(); void DrawLogPane(); - void SelectThreadStackFrame(kernel::XThread* thread, size_t stack_frame_index, - bool always_navigate); + void SelectThreadStackFrame(ThreadExecutionInfo* thread_info, + size_t stack_frame_index, bool always_navigate); void NavigateToFunction(cpu::Function* function, uint32_t guest_pc = 0, uint64_t host_pc = 0); // void NavigateToMemory(uint64_t address, uint64_t length = 0); @@ -117,7 +117,6 @@ class DebugWindow : public DebugListener { static const int kRightPaneMemory = 1; int right_pane_tab = kRightPaneThreads; - xe::kernel::XThread* thread = nullptr; ThreadExecutionInfo* thread_info = nullptr; size_t thread_stack_frame_index = 0; bool has_changed_thread = false;