Make guest debugger usable

src/xenia/cpu/breakpoint.cc

@@ -84,29 +84,37 @@ void Breakpoint::ForEachHostAddress(
     // Lookup all functions that contain this guest address and patch them.
     auto functions = processor_->FindFunctionsWithAddress(guest_address);
 
-    if (functions.empty()) {
-      // If function does not exist demand it, as we need someplace to put our
-      // breakpoint. Note that this follows the same resolution rules as the
-      // JIT, so what's returned is the function the JIT would have jumped to.
-      auto fn = processor_->ResolveFunction(guest_address);
-      if (!fn) {
-        // TODO(benvanik): error out better with 'invalid breakpoint'?
-        assert_not_null(fn);
-        return;
-      }
-      functions.push_back(fn);
+    // If function does not exist demand it, as we need someplace to put our
+    // breakpoint. Note that this follows the same resolution rules as the
+    // JIT, so what's returned is the function the JIT would have jumped to.
+    auto fn = processor_->ResolveFunction(guest_address);
+    if (!fn) {
+      // TODO(benvanik): error out better with 'invalid breakpoint'?
+      assert_not_null(fn);
+      return;
     }
+
+    functions.push_back(fn);
     assert_false(functions.empty());
+    uintptr_t host_address = 0;
 
     for (auto function : functions) {
       // TODO(benvanik): other function types.
       assert_true(function->is_guest());
       auto guest_function = reinterpret_cast<GuestFunction*>(function);
-      uintptr_t host_address =
+      host_address =
           guest_function->MapGuestAddressToMachineCode(guest_address);
-      assert_not_zero(host_address);
-      callback(host_address);
+
+      // Functions that jump around another function can be misinterpreted as
+      // containing an address. Try each eligible function and use the one that
+      // works.
+      if (host_address != 0) {
+        callback(host_address);
+        break;
+      }
     }
+
+    assert_not_zero(host_address);
   } else {
     // Direct host address patching.
     callback(host_address());

src/xenia/cpu/processor.cc

@@ -447,6 +447,7 @@ bool Processor::Restore(ByteStream* stream) {
   std::vector<uint32_t> to_delete;
   for (auto& it : thread_debug_infos_) {
     if (it.second->state == ThreadDebugInfo::State::kZombie) {
+      it.second->thread_handle = NULL;
       to_delete.push_back(it.first);
     }
   }
@@ -481,11 +482,11 @@ void Processor::OnThreadCreated(uint32_t thread_handle,
                                 ThreadState* thread_state, Thread* thread) {
   auto global_lock = global_critical_region_.Acquire();
   auto thread_info = std::make_unique<ThreadDebugInfo>();
-  thread_info->thread_handle = thread_handle;
   thread_info->thread_id = thread_state->thread_id();
   thread_info->thread = thread;
   thread_info->state = ThreadDebugInfo::State::kAlive;
   thread_info->suspended = false;
+  thread_info->thread_handle = thread_handle;
   thread_debug_infos_.emplace(thread_info->thread_id, std::move(thread_info));
 }
 
@@ -501,6 +502,7 @@ void Processor::OnThreadDestroyed(uint32_t thread_id) {
   auto global_lock = global_critical_region_.Acquire();
   auto it = thread_debug_infos_.find(thread_id);
   assert_true(it != thread_debug_infos_.end());
+  it->second->thread_handle = NULL;
   thread_debug_infos_.erase(it);
 }
 
@@ -667,14 +669,15 @@ bool Processor::OnThreadBreakpointHit(Exception* ex) {
     debug_listener_->OnExecutionPaused();
   }
 
+  ResumeAllThreads();
   thread_info->thread->thread()->Suspend();
 
   // Apply thread context changes.
   // TODO(benvanik): apply to all threads?
 #if XE_ARCH_AMD64
-  ex->set_resume_pc(thread_info->host_context.rip);
+  ex->set_resume_pc(thread_info->host_context.rip + 2);
 #elif XE_ARCH_ARM64
-  ex->set_resume_pc(thread_info->host_context.pc);
+  ex->set_resume_pc(thread_info->host_context.pc + 2);
 #else
 #error Instruction pointer not specified for the target CPU architecture.
 #endif  // XE_ARCH
@@ -902,6 +905,7 @@ void Processor::Continue() {
   execution_state_ = ExecutionState::kRunning;
   ResumeAllBreakpoints();
   ResumeAllThreads();
+
   if (debug_listener_) {
     debug_listener_->OnExecutionContinued();
   }

src/xenia/cpu/stack_walker_win.cc

@@ -240,10 +240,13 @@ class Win32StackWalker : public StackWalker {
           // displacement in x64 from the JIT'ed code start to the PC.
           if (function->is_guest()) {
             auto guest_function = static_cast<GuestFunction*>(function);
-            // Adjust the host PC by -1 so that we will go back into whatever
-            // instruction was executing before the capture (like a call).
+
+            // GaryFrazier: Removed -1 as that does not reflect the guest pc of
+            // the host address Adjust the host PC by -1 so that we will go back
+            // into whatever instruction was executing before the capture (like
+            // a call).
             frame.guest_pc =
-                guest_function->MapMachineCodeToGuestAddress(frame.host_pc - 1);
+                guest_function->MapMachineCodeToGuestAddress(frame.host_pc);
           }
         } else {
           frame.guest_symbol.function = nullptr;

src/xenia/debug/ui/debug_window.cc

@@ -300,11 +300,26 @@ void DebugWindow::DrawToolbar() {
     if (thread_info == state_.thread_info) {
       current_thread_index = i;
     }
-    if (thread_info->state != cpu::ThreadDebugInfo::State::kZombie) {
-      thread_combo.Append(thread_info->thread->thread_name());
-    } else {
-      thread_combo.Append("(zombie)");
+
+    // Threads can be briefly invalid once destroyed and before a cache update.
+    // This ensures we are accessing threads that are still valid.
+    switch (thread_info->state) {
+      case cpu::ThreadDebugInfo::State::kAlive:
+      case cpu::ThreadDebugInfo::State::kExited:
+      case cpu::ThreadDebugInfo::State::kWaiting:
+        if (thread_info->thread_handle == NULL || thread_info->thread == NULL) {
+          thread_combo.Append("(invalid)");
+        } else {
+          thread_combo.Append(thread_info->thread->thread_name());
+        }
+        break;
+      case cpu::ThreadDebugInfo::State::kZombie:
+        thread_combo.Append("(zombie)");
+        break;
+      default:
+        thread_combo.Append("(invalid)");
     }
+
     thread_combo.Append('\0');
     ++i;
   }