[Debug] Processor: breakpoint address fixes

- Update OnThreadBreakpointHit to use bp->ContainsHostAddress

Sometimes guest PC doesn't map to the x64 exactly, so ResolveStack may return
a guest PC different to the breakpoint that triggered it

Processor code would just skip the breakpoint entirely if PC didn't match
so any debugger wouldn't know what BP caused it

Luckily BP does have a ContainsHostAddress func which seems perfect, but was
somehow left unused

Also added a hack to nudge the PC we tell GDB about to the BP's PC

- Update StepGuest/HostInstruction to add step BP to front of map

Fixes an issue where a different BP might get triggered before the step BP
which could cause debugger to step again, causing assert error as a step BP
already existed

I noticed one more issue, if BPs are set on instructions very close together
it might act on the same x64 instruction, causing assert when a BP is already
placed and another BP tries to set on the same addr...

Not really sure what best way to fix that is yet, for now just don't place
BPs too close together!

src/xenia/cpu/processor.cc

@@ -644,7 +644,8 @@ bool Processor::OnThreadBreakpointHit(Exception* ex) {
       if ((scan_breakpoint->address_type() == Breakpoint::AddressType::kGuest &&
            scan_breakpoint->guest_address() == frame.guest_pc) ||
           (scan_breakpoint->address_type() == Breakpoint::AddressType::kHost &&
-           scan_breakpoint->host_address() == frame.host_pc)) {
+           scan_breakpoint->host_address() == frame.host_pc) ||
+          scan_breakpoint->ContainsHostAddress(frame.host_pc)) {
         breakpoint = scan_breakpoint;
         break;
       }
@@ -937,7 +938,10 @@ void Processor::StepHostInstruction(uint32_t thread_id) {
                        thread_info->step_breakpoint.reset();
                        OnStepCompleted(thread_info);
                      }));
-  AddBreakpoint(thread_info->step_breakpoint.get());
+
+  // Add to front of breakpoints map, so this should get evaluated first
+  breakpoints_.insert(breakpoints_.begin(), thread_info->step_breakpoint.get());
+
   thread_info->step_breakpoint->Resume();
 
   // ResumeAllBreakpoints();
@@ -970,7 +974,10 @@ void Processor::StepGuestInstruction(uint32_t thread_id) {
                        thread_info->step_breakpoint.reset();
                        OnStepCompleted(thread_info);
                      }));
-  AddBreakpoint(thread_info->step_breakpoint.get());
+
+  // Add to front of breakpoints map, so this should get evaluated first
+  breakpoints_.insert(breakpoints_.begin(), thread_info->step_breakpoint.get());
+
   thread_info->step_breakpoint->Resume();
 
   // ResumeAllBreakpoints();

src/xenia/debug/gdb/gdbstub.cc

@@ -184,7 +184,8 @@ uint8_t from_hexchar(char c) {
   return 0;
 }
 
-std::string get_reg(xe::cpu::ThreadDebugInfo* thread, uint32_t rid) {
+std::string GDBStub::ReadRegister(xe::cpu::ThreadDebugInfo* thread,
+                                  uint32_t rid) {
   // Send registers as 32-bit, otherwise some debuggers will switch to 64-bit
   // mode (eg. IDA will switch to 64-bit and refuse to allow decompiler to work
   // with it)
@@ -195,6 +196,16 @@ std::string get_reg(xe::cpu::ThreadDebugInfo* thread, uint32_t rid) {
   switch (rid) {
     // pc
     case 64: {
+      // If we recently hit a BP then debugger is likely asking for registers
+      // for it
+      //
+      // Lie about the PC and say it's the BP address, since PC might not always
+      // match
+      if (cache_.notify_bp_guest_address != -1) {
+        auto ret = u32_to_padded_hex((uint32_t)cache_.notify_bp_guest_address);
+        cache_.notify_bp_guest_address = -1;
+        return ret;
+      }
       // Search for first frame that has guest_pc attached, GDB doesn't care
       // about host
       for (auto& frame : thread->frames) {
@@ -560,7 +571,7 @@ void GDBStub::UpdateCache() {
 
 std::string GDBStub::ReadRegister(const std::string& data) {
   uint32_t rid = hex_to_u32(data);
-  std::string result = get_reg(cache_.cur_thread_info(), rid);
+  std::string result = ReadRegister(cache_.cur_thread_info(), rid);
   if (result.empty()) {
     return kGdbReplyError;  // TODO: is this error correct?
   }
@@ -571,7 +582,7 @@ std::string GDBStub::ReadRegisters() {
   std::string result;
   result.reserve(68 * 16 + 3 * 8);
   for (int i = 0; i < 71; ++i) {
-    result += get_reg(cache_.cur_thread_info(), i);
+    result += ReadRegister(cache_.cur_thread_info(), i);
   }
   return result;
 }
@@ -594,7 +605,8 @@ std::string GDBStub::ExecutionContinue() {
 
 std::string GDBStub::ExecutionStep() {
 #ifdef DEBUG
-  debugging::DebugPrint("GDBStub: ExecutionStep (thread {})\n", cache_.last_bp_thread_id);
+  debugging::DebugPrint("GDBStub: ExecutionStep (thread {})\n",
+                        cache_.last_bp_thread_id);
 #endif
 
   if (cache_.last_bp_thread_id != -1)
@@ -666,6 +678,12 @@ std::string GDBStub::GetThreadStateReply(uint32_t thread_id, uint8_t signal) {
       }
     }
 
+    // If BP was hit use the address of it, so debugger can match it up to its
+    // BP list
+    if (cache_.notify_bp_guest_address != -1) {
+      pc_value = cache_.notify_bp_guest_address;
+    }
+
     return fmt::format(
         "T{:02x}{:02x}:{};{:02x}:{};thread:{:x};", signal, PC_REGISTER,
         u32_to_padded_hex((uint32_t)pc_value), LR_REGISTER,
@@ -786,6 +804,7 @@ void GDBStub::OnBreakpointHit(Breakpoint* breakpoint,
                         breakpoint->address(), thread_info->thread_id);
 #endif
 
+  cache_.notify_bp_guest_address = breakpoint->address();
   cache_.notify_bp_thread_id = thread_info->thread_id;
   cache_.last_bp_thread_id = thread_info->thread_id;
   UpdateCache();

src/xenia/debug/gdb/gdbstub.h

@@ -61,6 +61,7 @@ class GDBStub : public cpu::DebugListener {
 
   void UpdateCache();
 
+  std::string ReadRegister(xe::cpu::ThreadDebugInfo* thread, uint32_t rid);
   std::string ReadRegister(const std::string& data);
   std::string ReadRegisters();
   std::string ExecutionPause();
@@ -93,6 +94,7 @@ class GDBStub : public cpu::DebugListener {
     uint32_t cur_thread_id = -1;
     uint32_t last_bp_thread_id = -1;
 
+    uint64_t notify_bp_guest_address = -1;
     uint32_t notify_bp_thread_id = -1;
     bool notify_stopped = false;