From 6f23ee43ae812a57c62b81efd3993b3f4823ae35 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Fri, 2 Dec 2022 19:31:31 -0600
Subject: [PATCH 01/13] Initial port of luma3ds' gdb_hio to Citra

---
 src/core/CMakeLists.txt      |   2 +
 src/core/gdbstub/gdbstub.cpp |  19 +++++
 src/core/gdbstub/hio.cpp     | 150 +++++++++++++++++++++++++++++++++++
 src/core/gdbstub/hio.h       |  32 ++++++++
 src/core/hle/kernel/svc.cpp  |  11 ++-
 5 files changed, 212 insertions(+), 2 deletions(-)
 create mode 100644 src/core/gdbstub/hio.cpp
 create mode 100644 src/core/gdbstub/hio.h

diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt
index 8aadcad81..c97062ed7 100644
--- a/src/core/CMakeLists.txt
+++ b/src/core/CMakeLists.txt
@@ -115,6 +115,8 @@ add_library(core STATIC
     frontend/mic.h
     gdbstub/gdbstub.cpp
     gdbstub/gdbstub.h
+    gdbstub/hio.cpp
+    gdbstub/hio.h
     hle/applets/applet.cpp
     hle/applets/applet.h
     hle/applets/erreula.cpp
diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index 932d1f06d..8e73898b8 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -35,6 +35,7 @@
 #include "core/arm/arm_interface.h"
 #include "core/core.h"
 #include "core/gdbstub/gdbstub.h"
+#include "core/gdbstub/hio.h"
 #include "core/hle/kernel/process.h"
 #include "core/loader/loader.h"
 #include "core/memory.h"
@@ -1061,6 +1062,19 @@ void HandlePacket() {
 
     LOG_DEBUG(Debug_GDBStub, "Packet: {}", command_buffer[0]);
 
+    // HACK: instead of polling DebugEvents properly via SVC, just check for
+    // whether there's a pending a request, and send it if so.
+    switch (command_buffer[0]) {
+    case 'c':
+    case 'C':
+    case 's':
+        if (HasHioRequest()) {
+            const auto reply = BuildHioReply();
+            SendReply(reply.data());
+            return;
+        }
+    }
+
     switch (command_buffer[0]) {
     case 'q':
         HandleQuery();
@@ -1075,6 +1089,11 @@ void HandlePacket() {
         Shutdown();
         LOG_INFO(Debug_GDBStub, "killed by gdb");
         return;
+    case 'F':
+        if (HandleHioRequest(command_buffer, command_length)) {
+            Continue();
+        };
+        break;
     case 'g':
         ReadRegisters();
         break;
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
new file mode 100644
index 000000000..da120f6c3
--- /dev/null
+++ b/src/core/gdbstub/hio.cpp
@@ -0,0 +1,150 @@
+#include "core/core.h"
+#include "core/gdbstub/gdbstub.h"
+#include "core/gdbstub/hio.h"
+
+namespace GDBStub {
+
+namespace {
+
+static VAddr current_hio_request_addr;
+static PackedGdbHioRequest current_hio_request;
+
+} // namespace
+
+void SetHioRequest(const VAddr addr) {
+    if (!IsServerEnabled()) {
+        LOG_WARNING(Debug_GDBStub, "HIO requested but GDB stub is not running");
+        return;
+    }
+
+    if (current_hio_request_addr != 0) {
+        LOG_WARNING(Debug_GDBStub, "HIO requested while already in progress!");
+        return;
+    }
+
+    auto& memory = Core::System::GetInstance().Memory();
+    if (!memory.IsValidVirtualAddress(*Core::System::GetInstance().Kernel().GetCurrentProcess(),
+                                      addr)) {
+        LOG_WARNING(Debug_GDBStub, "Invalid address for HIO request");
+        return;
+    }
+
+    memory.ReadBlock(addr, &current_hio_request, sizeof(PackedGdbHioRequest));
+    current_hio_request_addr = addr;
+
+    LOG_DEBUG(Debug_GDBStub, "HIO request initiated");
+}
+
+bool HandleHioRequest(const u8* const command_buffer, const u32 command_length) {
+    if (!HasHioRequest()) {
+        // TODO send error reply packet?
+        return false;
+    }
+
+    u64 retval{0};
+
+    auto* command_pos = command_buffer;
+    ++command_pos;
+
+    // TODO: not totally sure what's going on here...
+    if (*command_pos == 0 || *command_pos == ',') {
+        // return GDB_ReplyErrno(ctx, EILSEQ);
+        return false;
+    } else if (*command_pos == '-') {
+        command_pos++;
+        current_hio_request.retval = -1;
+    } else if (*command_pos == '+') {
+        command_pos++;
+        current_hio_request.retval = 1;
+    } else {
+        current_hio_request.retval = 1;
+    }
+
+    // TODO:
+    // pos = GDB_ParseHexIntegerList64(&retval, pos, 1, ',');
+
+    if (command_pos == nullptr) {
+        // return GDB_ReplyErrno(ctx, EILSEQ);
+        return false;
+    }
+
+    current_hio_request.retval *= retval;
+    current_hio_request.gdb_errno = 0;
+    current_hio_request.ctrl_c = 0;
+
+    if (*command_pos != 0) {
+        u32 errno_;
+        // GDB protocol technically allows errno to have a +/- prefix but this will never happen.
+        // TODO:
+        // pos = GDB_ParseHexIntegerList(&errno_, ++pos, 1, ',');
+        current_hio_request.gdb_errno = (int)errno_;
+        if (command_pos == nullptr) {
+            return false;
+            // return GDB_ReplyErrno(ctx, EILSEQ);
+        }
+
+        if (*command_pos != 0) {
+            if (*command_pos != 'C') {
+                return false;
+                // return GDB_ReplyErrno(ctx, EILSEQ);
+            }
+
+            current_hio_request.ctrl_c = true;
+        }
+    }
+
+    std::fill(std::begin(current_hio_request.param_format),
+              std::end(current_hio_request.param_format), 0);
+
+    auto& memory = Core::System::GetInstance().Memory();
+    // should have been checked when we first initialized the request:
+    assert(memory.IsValidVirtualAddress(*Core::System::GetInstance().Kernel().GetCurrentProcess(),
+                                        current_hio_request_addr));
+
+    memory.WriteBlock(current_hio_request_addr, &current_hio_request, sizeof(PackedGdbHioRequest));
+
+    current_hio_request = PackedGdbHioRequest{};
+    current_hio_request_addr = 0;
+
+    return true;
+}
+
+bool HasHioRequest() {
+    return current_hio_request_addr != 0;
+}
+
+std::string BuildHioReply() {
+    char buf[256 + 1];
+    char tmp[32 + 1];
+    u32 nStr = 0;
+
+    // TODO: c++ify this and use the IntToGdbHex funcs instead of snprintf
+
+    snprintf(buf, 256, "F%s", current_hio_request.function_name);
+
+    for (u32 i = 0; i < 8 && current_hio_request.param_format[i] != 0; i++) {
+        switch (current_hio_request.param_format[i]) {
+        case 'i':
+        case 'I':
+        case 'p':
+            snprintf(tmp, 32, ",%x", (u32)current_hio_request.parameters[i]);
+            break;
+        case 'l':
+        case 'L':
+            snprintf(tmp, 32, ",%llx", current_hio_request.parameters[i]);
+            break;
+        case 's':
+            snprintf(tmp, 32, ",%x/%zx", (u32)current_hio_request.parameters[i],
+                     current_hio_request.string_lengths[nStr++]);
+            break;
+        default:
+            tmp[0] = 0;
+            break;
+        }
+        strcat(buf, tmp);
+    }
+
+    return std::string{buf, strlen(buf)};
+}
+
+} // namespace GDBStub
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
new file mode 100644
index 000000000..5b4bce8a6
--- /dev/null
+++ b/src/core/gdbstub/hio.h
@@ -0,0 +1,32 @@
+#pragma once
+
+#include "common/common_types.h"
+
+namespace GDBStub {
+
+struct PackedGdbHioRequest {
+    char magic[4]; // "GDB\x00"
+    u32 version;
+
+    // Request
+    char function_name[16 + 1];
+    char param_format[8 + 1];
+
+    u64 parameters[8];
+    size_t string_lengths[8];
+
+    // Return
+    s64 retval;
+    int gdb_errno;
+    bool ctrl_c;
+};
+
+void SetHioRequest(const VAddr address);
+
+bool HandleHioRequest(const u8* const command_buffer, const u32 command_length);
+
+bool HasHioRequest();
+
+std::string BuildHioReply();
+
+} // namespace GDBStub
diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp
index 2603bd03f..8157d92ea 100644
--- a/src/core/hle/kernel/svc.cpp
+++ b/src/core/hle/kernel/svc.cpp
@@ -13,6 +13,7 @@
 #include "core/arm/arm_interface.h"
 #include "core/core.h"
 #include "core/core_timing.h"
+#include "core/gdbstub/hio.h"
 #include "core/hle/kernel/address_arbiter.h"
 #include "core/hle/kernel/client_port.h"
 #include "core/hle/kernel/client_session.h"
@@ -1140,8 +1141,14 @@ void SVC::Break(u8 break_reason) {
     system.SetStatus(Core::System::ResultStatus::ErrorUnknown);
 }
 
-/// Used to output a message on a debug hardware unit - does nothing on a retail unit
+/// Used to output a message on a debug hardware unit, or for the GDB HIO
+// protocol - does nothing on a retail unit.
 void SVC::OutputDebugString(VAddr address, s32 len) {
+    if (len == 0) {
+        GDBStub::SetHioRequest(address);
+        return;
+    }
+
     if (len <= 0) {
         return;
     }
@@ -2212,7 +2219,7 @@ const std::array<SVC::FunctionDef, 180> SVC::SVC_Table{{
     {0x60, nullptr, "DebugActiveProcess"},
     {0x61, nullptr, "BreakDebugProcess"},
     {0x62, nullptr, "TerminateDebugProcess"},
-    {0x63, nullptr, "GetProcessDebugEvent"},
+    {0x63, nullptr, "GetProcessDebugEvent"}, // TODO: do we need this for HIO to work?
     {0x64, nullptr, "ContinueDebugEvent"},
     {0x65, &SVC::Wrap<&SVC::GetProcessList>, "GetProcessList"},
     {0x66, nullptr, "GetThreadList"},

From 7de1bf374630aeddb3c6bb69c0a83a492800ae17 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Sat, 3 Dec 2022 16:09:13 -0500
Subject: [PATCH 02/13] Properly parse incoming hio packet

This also does kind of a hacky way of sending HIO requests, since we
don't have a direct way of signaling a request should be sent like the
Rosalina implementation.

To improve this, it could probably do some kind of signal sending which
the main run loop handles instead of GDBStub::HandlePacket();
---
 src/core/gdbstub/gdbstub.cpp | 38 ++++++-----------
 src/core/gdbstub/gdbstub.h   | 23 ++++++++++
 src/core/gdbstub/hio.cpp     | 82 +++++++++++++++++++++++-------------
 src/core/gdbstub/hio.h       |  8 +++-
 4 files changed, 94 insertions(+), 57 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index 8e73898b8..225f92d69 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -262,13 +262,7 @@ static u8 NibbleToHex(u8 n) {
     }
 }
 
-/**
- * Converts input hex string characters into an array of equivalent of u8 bytes.
- *
- * @param src Pointer to array of output hex string characters.
- * @param len Length of src array.
- */
-static u32 HexToInt(const u8* src, std::size_t len) {
+u32 HexToInt(const u8* src, std::size_t len) {
     u32 output = 0;
     while (len-- > 0) {
         output = (output << 4) | HexCharToValue(src[0]);
@@ -319,12 +313,7 @@ static void IntToGdbHex(u8* dest, u32 v) {
     }
 }
 
-/**
- * Convert a gdb-formatted hex string into a u32.
- *
- * @param src Pointer to hex string.
- */
-static u32 GdbHexToInt(const u8* src) {
+u32 GdbHexToInt(const u8* src) {
     u32 output = 0;
 
     for (int i = 0; i < 8; i += 2) {
@@ -348,12 +337,7 @@ static void LongToGdbHex(u8* dest, u64 v) {
     }
 }
 
-/**
- * Convert a gdb-formatted hex string into a u64.
- *
- * @param src Pointer to hex string.
- */
-static u64 GdbHexToLong(const u8* src) {
+u64 GdbHexToLong(const u8* src) {
     u64 output = 0;
 
     for (int i = 0; i < 16; i += 2) {
@@ -1051,6 +1035,12 @@ void HandlePacket() {
         return;
     }
 
+    if (HasHioRequest()) {
+        const auto reply = BuildHioRequestPacket();
+        SendReply(reply.data());
+        return;
+    }
+
     if (!IsDataAvailable()) {
         return;
     }
@@ -1064,15 +1054,13 @@ void HandlePacket() {
 
     // HACK: instead of polling DebugEvents properly via SVC, just check for
     // whether there's a pending a request, and send it if so.
+    // ...This doesn't seem good enough for the general case
     switch (command_buffer[0]) {
     case 'c':
     case 'C':
     case 's':
-        if (HasHioRequest()) {
-            const auto reply = BuildHioReply();
-            SendReply(reply.data());
-            return;
-        }
+        //
+        ;
     }
 
     switch (command_buffer[0]) {
@@ -1090,7 +1078,7 @@ void HandlePacket() {
         LOG_INFO(Debug_GDBStub, "killed by gdb");
         return;
     case 'F':
-        if (HandleHioRequest(command_buffer, command_length)) {
+        if (HandleHioReply(command_buffer, command_length)) {
             Continue();
         };
         break;
diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h
index b878b7957..2eb092758 100644
--- a/src/core/gdbstub/gdbstub.h
+++ b/src/core/gdbstub/gdbstub.h
@@ -107,4 +107,27 @@ void SetCpuStepFlag(bool is_step);
  * @param trap Trap no.
  */
 void SendTrap(Kernel::Thread* thread, int trap);
+
+/**
+ * Converts input hex string characters into an array of equivalent of u8 bytes.
+ *
+ * @param src Pointer to array of output hex string characters.
+ * @param len Length of src array.
+ */
+u32 HexToInt(const u8* src, std::size_t len);
+
+/**
+ * Convert a gdb-formatted hex string into a u32.
+ *
+ * @param src Pointer to hex string.
+ */
+u32 GdbHexToInt(const u8* src);
+
+/**
+ * Convert a gdb-formatted hex string into a u64.
+ *
+ * @param src Pointer to hex string.
+ */
+u64 GdbHexToLong(const u8* src);
+
 } // namespace GDBStub
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index da120f6c3..b66fe09aa 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -1,3 +1,7 @@
+// Copyright 2022 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
 #include "core/core.h"
 #include "core/gdbstub/gdbstub.h"
 #include "core/gdbstub/hio.h"
@@ -8,6 +12,7 @@ namespace {
 
 static VAddr current_hio_request_addr;
 static PackedGdbHioRequest current_hio_request;
+static std::atomic<bool> sent_request{false};
 
 } // namespace
 
@@ -22,35 +27,39 @@ void SetHioRequest(const VAddr addr) {
         return;
     }
 
-    auto& memory = Core::System::GetInstance().Memory();
-    if (!memory.IsValidVirtualAddress(*Core::System::GetInstance().Kernel().GetCurrentProcess(),
-                                      addr)) {
+    const auto process = Core::System::GetInstance().Kernel().GetCurrentProcess();
+    if (!Memory::IsValidVirtualAddress(*process, addr)) {
         LOG_WARNING(Debug_GDBStub, "Invalid address for HIO request");
         return;
     }
 
-    memory.ReadBlock(addr, &current_hio_request, sizeof(PackedGdbHioRequest));
+    auto& memory = Core::System::GetInstance().Memory();
+    memory.ReadBlock(*process, addr, &current_hio_request, sizeof(PackedGdbHioRequest));
+
+    // TODO read + check request magic header
+
     current_hio_request_addr = addr;
+    sent_request = false;
 
     LOG_DEBUG(Debug_GDBStub, "HIO request initiated");
 }
 
-bool HandleHioRequest(const u8* const command_buffer, const u32 command_length) {
-    if (!HasHioRequest()) {
+bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
+    if (current_hio_request_addr == 0 || !sent_request) {
+        LOG_WARNING(Debug_GDBStub, "Got HIO reply but never sent a request");
         // TODO send error reply packet?
         return false;
     }
 
-    u64 retval{0};
+    auto* command_pos = command_buffer + 1;
 
-    auto* command_pos = command_buffer;
-    ++command_pos;
-
-    // TODO: not totally sure what's going on here...
     if (*command_pos == 0 || *command_pos == ',') {
         // return GDB_ReplyErrno(ctx, EILSEQ);
         return false;
-    } else if (*command_pos == '-') {
+    }
+
+    // Set the sign of the retval
+    if (*command_pos == '-') {
         command_pos++;
         current_hio_request.retval = -1;
     } else if (*command_pos == '+') {
@@ -60,13 +69,13 @@ bool HandleHioRequest(const u8* const command_buffer, const u32 command_length)
         current_hio_request.retval = 1;
     }
 
-    // TODO:
-    // pos = GDB_ParseHexIntegerList64(&retval, pos, 1, ',');
-
-    if (command_pos == nullptr) {
+    const auto retval_end = std::find(command_pos, command_buffer + command_length, ',');
+    if (retval_end >= command_buffer + command_length) {
         // return GDB_ReplyErrno(ctx, EILSEQ);
         return false;
     }
+    u64 retval = (u64)HexToInt(command_pos, retval_end - command_pos);
+    command_pos = retval_end + 1;
 
     current_hio_request.retval *= retval;
     current_hio_request.gdb_errno = 0;
@@ -75,13 +84,11 @@ bool HandleHioRequest(const u8* const command_buffer, const u32 command_length)
     if (*command_pos != 0) {
         u32 errno_;
         // GDB protocol technically allows errno to have a +/- prefix but this will never happen.
-        // TODO:
-        // pos = GDB_ParseHexIntegerList(&errno_, ++pos, 1, ',');
+        const auto errno_end = std::find(command_pos, command_buffer + command_length, ',');
+        errno_ = HexToInt(command_pos, errno_end - command_pos);
+        command_pos = errno_end + 1;
+
         current_hio_request.gdb_errno = (int)errno_;
-        if (command_pos == nullptr) {
-            return false;
-            // return GDB_ReplyErrno(ctx, EILSEQ);
-        }
 
         if (*command_pos != 0) {
             if (*command_pos != 'C') {
@@ -96,24 +103,35 @@ bool HandleHioRequest(const u8* const command_buffer, const u32 command_length)
     std::fill(std::begin(current_hio_request.param_format),
               std::end(current_hio_request.param_format), 0);
 
-    auto& memory = Core::System::GetInstance().Memory();
-    // should have been checked when we first initialized the request:
-    assert(memory.IsValidVirtualAddress(*Core::System::GetInstance().Kernel().GetCurrentProcess(),
-                                        current_hio_request_addr));
+    LOG_DEBUG(Debug_GDBStub, "HIO reply: {{retval = {}, errno = {}, ctrl_c = {}}}",
+              current_hio_request.retval, current_hio_request.gdb_errno,
+              current_hio_request.ctrl_c);
 
-    memory.WriteBlock(current_hio_request_addr, &current_hio_request, sizeof(PackedGdbHioRequest));
+    const auto process = Core::System::GetInstance().Kernel().GetCurrentProcess();
+    // should have been checked when we first initialized the request,
+    // but just double check again before we write to memory
+    if (!Memory::IsValidVirtualAddress(*process, current_hio_request_addr)) {
+        LOG_WARNING(Debug_GDBStub, "Invalid address {:X} to write HIO request",
+                    current_hio_request_addr);
+        return false;
+    }
+
+    auto& memory = Core::System::GetInstance().Memory();
+    memory.WriteBlock(*process, current_hio_request_addr, &current_hio_request,
+                      sizeof(PackedGdbHioRequest));
 
     current_hio_request = PackedGdbHioRequest{};
     current_hio_request_addr = 0;
+    sent_request = false;
 
     return true;
 }
 
 bool HasHioRequest() {
-    return current_hio_request_addr != 0;
+    return current_hio_request_addr != 0 && !sent_request;
 }
 
-std::string BuildHioReply() {
+std::string BuildHioRequestPacket() {
     char buf[256 + 1];
     char tmp[32 + 1];
     u32 nStr = 0;
@@ -144,7 +162,11 @@ std::string BuildHioReply() {
         strcat(buf, tmp);
     }
 
-    return std::string{buf, strlen(buf)};
+    auto packet = std::string{buf, strlen(buf)};
+    LOG_DEBUG(Debug_GDBStub, "HIO request packet: {}", packet);
+    sent_request = true;
+
+    return packet;
 }
 
 } // namespace GDBStub
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index 5b4bce8a6..62559f05a 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -1,3 +1,7 @@
+// Copyright 2022 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
 #pragma once
 
 #include "common/common_types.h"
@@ -23,10 +27,10 @@ struct PackedGdbHioRequest {
 
 void SetHioRequest(const VAddr address);
 
-bool HandleHioRequest(const u8* const command_buffer, const u32 command_length);
+bool HandleHioReply(const u8* const command_buffer, const u32 command_length);
 
 bool HasHioRequest();
 
-std::string BuildHioReply();
+std::string BuildHioRequestPacket();
 
 } // namespace GDBStub

From 874bfebaf94245cff3cae92407a3131562c735c2 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Sat, 3 Dec 2022 16:48:21 -0500
Subject: [PATCH 03/13] Add some notes + use stringstream to build packet

---
 src/core/gdbstub/gdbstub.cpp | 14 ++++++------
 src/core/gdbstub/hio.cpp     | 41 ++++++++++++++++++------------------
 2 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index 225f92d69..f6f5c4154 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -1035,12 +1035,6 @@ void HandlePacket() {
         return;
     }
 
-    if (HasHioRequest()) {
-        const auto reply = BuildHioRequestPacket();
-        SendReply(reply.data());
-        return;
-    }
-
     if (!IsDataAvailable()) {
         return;
     }
@@ -1059,8 +1053,12 @@ void HandlePacket() {
     case 'c':
     case 'C':
     case 's':
-        //
-        ;
+        if (HasHioRequest()) {
+            // Really, this request needs to get sent _after_ the step or continue
+            // began, but not sure how to schedule that...
+            const auto request_packet = BuildHioRequestPacket();
+            SendReply(request_packet.data());
+        }
     }
 
     switch (command_buffer[0]) {
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index b66fe09aa..7e3c6d61e 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -36,12 +36,15 @@ void SetHioRequest(const VAddr addr) {
     auto& memory = Core::System::GetInstance().Memory();
     memory.ReadBlock(*process, addr, &current_hio_request, sizeof(PackedGdbHioRequest));
 
-    // TODO read + check request magic header
-
-    current_hio_request_addr = addr;
-    sent_request = false;
-
-    LOG_DEBUG(Debug_GDBStub, "HIO request initiated");
+    if (std::string_view{current_hio_request.magic} != "GDB") {
+        LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application");
+        current_hio_request_addr = 0;
+        current_hio_request = {};
+    } else {
+        current_hio_request_addr = addr;
+        sent_request = false;
+        LOG_DEBUG(Debug_GDBStub, "HIO request initiated");
+    }
 }
 
 bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
@@ -132,41 +135,37 @@ bool HasHioRequest() {
 }
 
 std::string BuildHioRequestPacket() {
-    char buf[256 + 1];
-    char tmp[32 + 1];
+    std::stringstream packet;
+    // TODO:use the IntToGdbHex funcs instead std::hex ?
+    packet << "F" << current_hio_request.function_name << std::hex;
+
     u32 nStr = 0;
 
-    // TODO: c++ify this and use the IntToGdbHex funcs instead of snprintf
-
-    snprintf(buf, 256, "F%s", current_hio_request.function_name);
-
     for (u32 i = 0; i < 8 && current_hio_request.param_format[i] != 0; i++) {
         switch (current_hio_request.param_format[i]) {
         case 'i':
         case 'I':
         case 'p':
-            snprintf(tmp, 32, ",%x", (u32)current_hio_request.parameters[i]);
+            packet << "," << (u32)current_hio_request.parameters[i];
             break;
         case 'l':
         case 'L':
-            snprintf(tmp, 32, ",%llx", current_hio_request.parameters[i]);
+            packet << "," << current_hio_request.parameters[i];
             break;
         case 's':
-            snprintf(tmp, 32, ",%x/%zx", (u32)current_hio_request.parameters[i],
-                     current_hio_request.string_lengths[nStr++]);
+            packet << "," << (u32)current_hio_request.parameters[i] << "/"
+                   << current_hio_request.string_lengths[nStr++];
             break;
         default:
-            tmp[0] = 0;
+            packet << '\0';
             break;
         }
-        strcat(buf, tmp);
     }
 
-    auto packet = std::string{buf, strlen(buf)};
-    LOG_DEBUG(Debug_GDBStub, "HIO request packet: {}", packet);
+    LOG_DEBUG(Debug_GDBStub, "HIO request packet: {}", packet.str());
     sent_request = true;
 
-    return packet;
+    return packet.str();
 }
 
 } // namespace GDBStub

From b9c11e71d7a71c80d5636120b102fda426249d86 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Sun, 4 Dec 2022 15:03:29 -0500
Subject: [PATCH 04/13] Fix ABI mismatch in packed HIO request

Using size_t etc. is incorrect, since these definitions are based on the
build machine which most likely has a different arch than the 3DS.
---
 src/core/gdbstub/hio.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index 62559f05a..5fbf45d93 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -17,11 +17,11 @@ struct PackedGdbHioRequest {
     char param_format[8 + 1];
 
     u64 parameters[8];
-    size_t string_lengths[8];
+    u32 string_lengths[8];
 
     // Return
     s64 retval;
-    int gdb_errno;
+    s32 gdb_errno;
     bool ctrl_c;
 };
 

From f92f494cab96856b45d6e4f3fe94e84b46489a76 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Sun, 4 Dec 2022 15:34:06 -0500
Subject: [PATCH 05/13] Stop execution when performing HIO request

Since the HIO request is basically treated like a syscall, we need to
stop emulation while waiting for the GDB client to reply with the
result. This ensures any memory queries etc. that GDB makes to fulfill
the HIO request are accessing memory as it was at the time of the
request, instead of afterwards.
---
 src/core/gdbstub/gdbstub.cpp | 38 +++++++++++++++----------------
 src/core/gdbstub/gdbstub.h   |  2 ++
 src/core/gdbstub/hio.cpp     | 44 ++++++++++++++++++++++++------------
 src/core/gdbstub/hio.h       |  6 ++++-
 src/core/hle/kernel/svc.cpp  |  5 ++++
 5 files changed, 60 insertions(+), 35 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index f6f5c4154..d700309a5 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -638,7 +638,7 @@ static void ReadCommand() {
     memset(command_buffer, 0, sizeof(command_buffer));
 
     u8 c = ReadByte();
-    if (c == '+') {
+    if (c == GDB_STUB_ACK) {
         // ignore ack
         return;
     } else if (c == 0x03) {
@@ -828,7 +828,7 @@ static void ReadMemory() {
     u32 len =
         HexToInt(start_offset, static_cast<u32>((command_buffer + command_length) - start_offset));
 
-    LOG_DEBUG(Debug_GDBStub, "gdb: addr: {:08x} len: {:08x}\n", addr, len);
+    LOG_DEBUG(Debug_GDBStub, "ReadMemory addr: {:08x} len: {:08x}", addr, len);
 
     if (len * 2 > sizeof(reply)) {
         SendReply("E01");
@@ -845,6 +845,9 @@ static void ReadMemory() {
 
     MemToGdbHex(reply, data.data(), len);
     reply[len * 2] = '\0';
+
+    LOG_DEBUG(Debug_GDBStub, "ReadMemory result: {}", reply);
+
     SendReply(reinterpret_cast<char*>(reply));
 }
 
@@ -1046,21 +1049,6 @@ void HandlePacket() {
 
     LOG_DEBUG(Debug_GDBStub, "Packet: {}", command_buffer[0]);
 
-    // HACK: instead of polling DebugEvents properly via SVC, just check for
-    // whether there's a pending a request, and send it if so.
-    // ...This doesn't seem good enough for the general case
-    switch (command_buffer[0]) {
-    case 'c':
-    case 'C':
-    case 's':
-        if (HasHioRequest()) {
-            // Really, this request needs to get sent _after_ the step or continue
-            // began, but not sure how to schedule that...
-            const auto request_packet = BuildHioRequestPacket();
-            SendReply(request_packet.data());
-        }
-    }
-
     switch (command_buffer[0]) {
     case 'q':
         HandleQuery();
@@ -1077,8 +1065,8 @@ void HandlePacket() {
         return;
     case 'F':
         if (HandleHioReply(command_buffer, command_length)) {
-            Continue();
-        };
+            // TODO figure out how to "resume" the last command
+        }
         break;
     case 'g':
         ReadRegisters();
@@ -1256,6 +1244,10 @@ bool GetCpuHaltFlag() {
     return halt_loop;
 }
 
+void SetCpuHaltFlag(bool halt) {
+    halt_loop = halt;
+}
+
 bool GetCpuStepFlag() {
     return step_loop;
 }
@@ -1270,7 +1262,13 @@ void SendTrap(Kernel::Thread* thread, int trap) {
     }
 
     current_thread = thread;
-    SendSignal(thread, trap);
+
+    if (HasPendingHioRequest()) {
+        const auto request_packet = BuildHioRequestPacket();
+        SendReply(request_packet.data());
+    } else {
+        SendSignal(thread, trap);
+    }
 
     halt_loop = true;
     send_trap = false;
diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h
index 2eb092758..e11d31324 100644
--- a/src/core/gdbstub/gdbstub.h
+++ b/src/core/gdbstub/gdbstub.h
@@ -90,6 +90,8 @@ bool CheckBreakpoint(VAddr addr, GDBStub::BreakpointType type);
 // If set to true, the CPU will halt at the beginning of the next CPU loop.
 bool GetCpuHaltFlag();
 
+void SetCpuHaltFlag(bool halt);
+
 // If set to true and the CPU is halted, the CPU will step one instruction.
 bool GetCpuStepFlag();
 
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index 7e3c6d61e..6be94b7e5 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -12,7 +12,14 @@ namespace {
 
 static VAddr current_hio_request_addr;
 static PackedGdbHioRequest current_hio_request;
-static std::atomic<bool> sent_request{false};
+
+enum class Status {
+    NoRequest,
+    NotSent,
+    SentWaitingReply,
+};
+
+static std::atomic<Status> request_status{Status::NoRequest};
 
 } // namespace
 
@@ -40,15 +47,23 @@ void SetHioRequest(const VAddr addr) {
         LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application");
         current_hio_request_addr = 0;
         current_hio_request = {};
+        request_status = Status::NoRequest;
     } else {
+        LOG_DEBUG(Debug_GDBStub, "HIO request initiated at 0x{:X}", addr);
         current_hio_request_addr = addr;
-        sent_request = false;
-        LOG_DEBUG(Debug_GDBStub, "HIO request initiated");
+        request_status = Status::NotSent;
+
+        // Now halt, so that no further instructions are executed until the request
+        // is processed by the client. We auto-continue after the reply comes back
+        Break();
+        SetCpuHaltFlag(true);
+        SetCpuStepFlag(false);
+        Core::GetRunningCore().ClearInstructionCache();
     }
 }
 
 bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
-    if (current_hio_request_addr == 0 || !sent_request) {
+    if (!WaitingForHioReply()) {
         LOG_WARNING(Debug_GDBStub, "Got HIO reply but never sent a request");
         // TODO send error reply packet?
         return false;
@@ -73,10 +88,6 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     }
 
     const auto retval_end = std::find(command_pos, command_buffer + command_length, ',');
-    if (retval_end >= command_buffer + command_length) {
-        // return GDB_ReplyErrno(ctx, EILSEQ);
-        return false;
-    }
     u64 retval = (u64)HexToInt(command_pos, retval_end - command_pos);
     command_pos = retval_end + 1;
 
@@ -86,7 +97,7 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
 
     if (*command_pos != 0) {
         u32 errno_;
-        // GDB protocol technically allows errno to have a +/- prefix but this will never happen.
+        // GDB protocol technically allows errno to have a +/- prefix but this should never happen.
         const auto errno_end = std::find(command_pos, command_buffer + command_length, ',');
         errno_ = HexToInt(command_pos, errno_end - command_pos);
         command_pos = errno_end + 1;
@@ -123,15 +134,19 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     memory.WriteBlock(*process, current_hio_request_addr, &current_hio_request,
                       sizeof(PackedGdbHioRequest));
 
-    current_hio_request = PackedGdbHioRequest{};
+    current_hio_request = {};
     current_hio_request_addr = 0;
-    sent_request = false;
+    request_status = Status::NoRequest;
 
     return true;
 }
 
-bool HasHioRequest() {
-    return current_hio_request_addr != 0 && !sent_request;
+bool HasPendingHioRequest() {
+    return current_hio_request_addr != 0 && request_status == Status::NotSent;
+}
+
+bool WaitingForHioReply() {
+    return current_hio_request_addr != 0 && request_status == Status::SentWaitingReply;
 }
 
 std::string BuildHioRequestPacket() {
@@ -163,7 +178,8 @@ std::string BuildHioRequestPacket() {
     }
 
     LOG_DEBUG(Debug_GDBStub, "HIO request packet: {}", packet.str());
-    sent_request = true;
+
+    request_status = Status::SentWaitingReply;
 
     return packet.str();
 }
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index 5fbf45d93..58c7d1e66 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -4,6 +4,8 @@
 
 #pragma once
 
+#include <string>
+
 #include "common/common_types.h"
 
 namespace GDBStub {
@@ -29,7 +31,9 @@ void SetHioRequest(const VAddr address);
 
 bool HandleHioReply(const u8* const command_buffer, const u32 command_length);
 
-bool HasHioRequest();
+bool HasPendingHioRequest();
+
+bool WaitingForHioReply();
 
 std::string BuildHioRequestPacket();
 
diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp
index 8157d92ea..f3f00a8b7 100644
--- a/src/core/hle/kernel/svc.cpp
+++ b/src/core/hle/kernel/svc.cpp
@@ -1144,6 +1144,11 @@ void SVC::Break(u8 break_reason) {
 /// Used to output a message on a debug hardware unit, or for the GDB HIO
 // protocol - does nothing on a retail unit.
 void SVC::OutputDebugString(VAddr address, s32 len) {
+    if (!Memory::IsValidVirtualAddress(*kernel.GetCurrentProcess(), address)) {
+        LOG_WARNING(Kernel_SVC, "OutputDebugString called with invalid address {:X}", address);
+        return;
+    }
+
     if (len == 0) {
         GDBStub::SetHioRequest(address);
         return;

From 33dde0d06bcdb39292fa5a65dca98e4d064d6a2a Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Fri, 31 Mar 2023 13:54:47 -0400
Subject: [PATCH 06/13] Update default config example for log_filter

As far as I can tell, this log filter does not actually support
wildcards in the subclass.
---
 src/citra/default_ini.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/citra/default_ini.h b/src/citra/default_ini.h
index 692134590..afb59a375 100644
--- a/src/citra/default_ini.h
+++ b/src/citra/default_ini.h
@@ -322,7 +322,7 @@ camera_inner_flip =
 
 [Miscellaneous]
 # A filter which removes logs below a certain logging level.
-# Examples: *:Debug Kernel.SVC:Trace Service.*:Critical
+# Examples: *:Debug Kernel.SVC:Trace Service:Critical
 log_filter = *:Info
 
 [Debugging]

From 83138e0c6377fb0d00f663b8612d2af7ffb13813 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Fri, 31 Mar 2023 14:05:57 -0400
Subject: [PATCH 07/13] Fix some build + impl errors with HIO

After rebase there were some API changes, also fix continuation logic in
the main gdbstub.
---
 src/core/gdbstub/gdbstub.cpp |  24 ++++---
 src/core/gdbstub/hio.cpp     | 127 ++++++++++++++++++++---------------
 src/core/gdbstub/hio.h       |   4 ++
 src/core/hle/kernel/svc.cpp  |   2 +-
 4 files changed, 94 insertions(+), 63 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index d700309a5..09f29be46 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -846,7 +846,7 @@ static void ReadMemory() {
     MemToGdbHex(reply, data.data(), len);
     reply[len * 2] = '\0';
 
-    LOG_DEBUG(Debug_GDBStub, "ReadMemory result: {}", reply);
+    LOG_DEBUG(Debug_GDBStub, "ReadMemory result: {}", (char*)reply);
 
     SendReply(reinterpret_cast<char*>(reply));
 }
@@ -1038,6 +1038,12 @@ void HandlePacket() {
         return;
     }
 
+    if (HasPendingHioRequest()) {
+        const auto request_packet = BuildHioRequestPacket();
+        SendReply(request_packet.data());
+        return;
+    }
+
     if (!IsDataAvailable()) {
         return;
     }
@@ -1047,7 +1053,7 @@ void HandlePacket() {
         return;
     }
 
-    LOG_DEBUG(Debug_GDBStub, "Packet: {}", command_buffer[0]);
+    LOG_DEBUG(Debug_GDBStub, "Packet: {0:d} ('{0:c}')", command_buffer[0]);
 
     switch (command_buffer[0]) {
     case 'q':
@@ -1065,7 +1071,12 @@ void HandlePacket() {
         return;
     case 'F':
         if (HandleHioReply(command_buffer, command_length)) {
-            // TODO figure out how to "resume" the last command
+            // TODO: technically if we were paused when the reply came in, we
+            // shouldn't continue here. Could recurse back into HandlePacket() maybe??
+            Continue();
+        } else {
+            // TODO reply with errno if relevant. Maybe that code should live in
+            // HandleHioReply
         }
         break;
     case 'g':
@@ -1263,12 +1274,7 @@ void SendTrap(Kernel::Thread* thread, int trap) {
 
     current_thread = thread;
 
-    if (HasPendingHioRequest()) {
-        const auto request_packet = BuildHioRequestPacket();
-        SendReply(request_packet.data());
-    } else {
-        SendSignal(thread, trap);
-    }
+    SendSignal(thread, trap);
 
     halt_loop = true;
     send_trap = false;
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index 6be94b7e5..06eb3eeaa 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -2,6 +2,8 @@
 // Licensed under GPLv2 or any later version
 // Refer to the license.txt file included.
 
+#include <fmt/ranges.h>
+#include "common/string_util.h"
 #include "core/core.h"
 #include "core/gdbstub/gdbstub.h"
 #include "core/gdbstub/hio.h"
@@ -29,37 +31,40 @@ void SetHioRequest(const VAddr addr) {
         return;
     }
 
-    if (current_hio_request_addr != 0) {
+    if (HasPendingHioRequest() || WaitingForHioReply()) {
         LOG_WARNING(Debug_GDBStub, "HIO requested while already in progress!");
         return;
     }
 
+    auto& memory = Core::System::GetInstance().Memory();
     const auto process = Core::System::GetInstance().Kernel().GetCurrentProcess();
-    if (!Memory::IsValidVirtualAddress(*process, addr)) {
+
+    if (!memory.IsValidVirtualAddress(*process, addr)) {
         LOG_WARNING(Debug_GDBStub, "Invalid address for HIO request");
         return;
     }
 
-    auto& memory = Core::System::GetInstance().Memory();
     memory.ReadBlock(*process, addr, &current_hio_request, sizeof(PackedGdbHioRequest));
 
-    if (std::string_view{current_hio_request.magic} != "GDB") {
-        LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application");
+    std::string_view magic{current_hio_request.magic};
+    if (magic != "GDB") {
+        LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application: magic '{}'", magic);
         current_hio_request_addr = 0;
         current_hio_request = {};
         request_status = Status::NoRequest;
-    } else {
-        LOG_DEBUG(Debug_GDBStub, "HIO request initiated at 0x{:X}", addr);
-        current_hio_request_addr = addr;
-        request_status = Status::NotSent;
-
-        // Now halt, so that no further instructions are executed until the request
-        // is processed by the client. We auto-continue after the reply comes back
-        Break();
-        SetCpuHaltFlag(true);
-        SetCpuStepFlag(false);
-        Core::GetRunningCore().ClearInstructionCache();
+        return;
     }
+
+    LOG_DEBUG(Debug_GDBStub, "HIO request initiated at 0x{:X}", addr);
+    current_hio_request_addr = addr;
+    request_status = Status::NotSent;
+
+    // Now halt, so that no further instructions are executed until the request
+    // is processed by the client. We will continue after the reply comes back
+    Break();
+    SetCpuHaltFlag(true);
+    SetCpuStepFlag(false);
+    Core::GetRunningCore().ClearInstructionCache();
 }
 
 bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
@@ -69,6 +74,7 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
         return false;
     }
 
+    // Skip 'F' header
     auto* command_pos = command_buffer + 1;
 
     if (*command_pos == 0 || *command_pos == ',') {
@@ -80,57 +86,65 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     if (*command_pos == '-') {
         command_pos++;
         current_hio_request.retval = -1;
-    } else if (*command_pos == '+') {
-        command_pos++;
-        current_hio_request.retval = 1;
     } else {
+        if (*command_pos == '+') {
+            command_pos++;
+        }
+
         current_hio_request.retval = 1;
     }
 
-    const auto retval_end = std::find(command_pos, command_buffer + command_length, ',');
-    u64 retval = (u64)HexToInt(command_pos, retval_end - command_pos);
-    command_pos = retval_end + 1;
+    const std::string command_str{command_pos, command_buffer + command_length};
+    std::vector<std::string> command_parts;
+    Common::SplitString(command_str, ',', command_parts);
 
-    current_hio_request.retval *= retval;
-    current_hio_request.gdb_errno = 0;
-    current_hio_request.ctrl_c = 0;
+    if (command_parts.empty() || command_parts.size() > 3) {
+        LOG_WARNING(Debug_GDBStub, "unexpected reply packet size: {}", command_parts);
+        // return GDB_ReplyErrno(ctx, EILSEQ);
+        return false;
+    }
 
-    if (*command_pos != 0) {
-        u32 errno_;
-        // GDB protocol technically allows errno to have a +/- prefix but this should never happen.
-        const auto errno_end = std::find(command_pos, command_buffer + command_length, ',');
-        errno_ = HexToInt(command_pos, errno_end - command_pos);
-        command_pos = errno_end + 1;
+    u64 unsigned_retval = HexToInt((u8*)command_parts[0].data(), command_parts[0].size());
+    current_hio_request.retval *= unsigned_retval;
 
-        current_hio_request.gdb_errno = (int)errno_;
+    if (command_parts.size() > 1) {
+        // Technically the errno could be signed but in practice this doesn't happen
+        current_hio_request.gdb_errno =
+            HexToInt((u8*)command_parts[1].data(), command_parts[1].size());
+    } else {
+        current_hio_request.gdb_errno = 0;
+    }
 
-        if (*command_pos != 0) {
-            if (*command_pos != 'C') {
-                return false;
-                // return GDB_ReplyErrno(ctx, EILSEQ);
-            }
+    current_hio_request.ctrl_c = false;
 
-            current_hio_request.ctrl_c = true;
+    if (command_parts.size() > 2 && !command_parts[2].empty()) {
+        if (command_parts[2][0] != 'C') {
+            return false;
+            // return GDB_ReplyErrno(ctx, EILSEQ);
         }
+
+        // for now we just ignore any trailing ";..." attachments
+        current_hio_request.ctrl_c = true;
     }
 
     std::fill(std::begin(current_hio_request.param_format),
               std::end(current_hio_request.param_format), 0);
 
-    LOG_DEBUG(Debug_GDBStub, "HIO reply: {{retval = {}, errno = {}, ctrl_c = {}}}",
+    LOG_TRACE(Debug_GDBStub, "HIO reply: {{retval = {}, errno = {}, ctrl_c = {}}}",
               current_hio_request.retval, current_hio_request.gdb_errno,
               current_hio_request.ctrl_c);
 
     const auto process = Core::System::GetInstance().Kernel().GetCurrentProcess();
+    auto& memory = Core::System::GetInstance().Memory();
+
     // should have been checked when we first initialized the request,
     // but just double check again before we write to memory
-    if (!Memory::IsValidVirtualAddress(*process, current_hio_request_addr)) {
-        LOG_WARNING(Debug_GDBStub, "Invalid address {:X} to write HIO request",
+    if (!memory.IsValidVirtualAddress(*process, current_hio_request_addr)) {
+        LOG_WARNING(Debug_GDBStub, "Invalid address {:#X} to write HIO reply",
                     current_hio_request_addr);
         return false;
     }
 
-    auto& memory = Core::System::GetInstance().Memory();
     memory.WriteBlock(*process, current_hio_request_addr, &current_hio_request,
                       sizeof(PackedGdbHioRequest));
 
@@ -150,38 +164,45 @@ bool WaitingForHioReply() {
 }
 
 std::string BuildHioRequestPacket() {
-    std::stringstream packet;
-    // TODO:use the IntToGdbHex funcs instead std::hex ?
-    packet << "F" << current_hio_request.function_name << std::hex;
+    std::string packet = fmt::format("F{}", current_hio_request.function_name);
+    // TODO: should we use the IntToGdbHex funcs instead of fmt::format_to ?
 
     u32 nStr = 0;
 
     for (u32 i = 0; i < 8 && current_hio_request.param_format[i] != 0; i++) {
+        u64 param = current_hio_request.parameters[i];
+
         switch (current_hio_request.param_format[i]) {
         case 'i':
         case 'I':
         case 'p':
-            packet << "," << (u32)current_hio_request.parameters[i];
-            break;
+            // For pointers and 32-bit ints, truncate before sending
+            param = static_cast<u32>(param);
+
         case 'l':
         case 'L':
-            packet << "," << current_hio_request.parameters[i];
+            fmt::format_to(std::back_inserter(packet), ",{:x}", param);
             break;
+
         case 's':
-            packet << "," << (u32)current_hio_request.parameters[i] << "/"
-                   << current_hio_request.string_lengths[nStr++];
+            // strings are written as {pointer}/{length}
+            fmt::format_to(std::back_inserter(packet), ",{:x}/{:x}",
+                           (u32)current_hio_request.parameters[i],
+                           current_hio_request.string_lengths[nStr++]);
             break;
+
         default:
-            packet << '\0';
+            // TODO: early return? Or break out of this loop?
             break;
         }
     }
 
-    LOG_DEBUG(Debug_GDBStub, "HIO request packet: {}", packet.str());
+    LOG_TRACE(Debug_GDBStub, "HIO request packet: {}", packet);
 
+    // technically we should set this _after_ the reply is sent...
     request_status = Status::SentWaitingReply;
 
-    return packet.str();
+    return packet;
 }
 
 } // namespace GDBStub
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index 58c7d1e66..ec6804df0 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -10,6 +10,10 @@
 
 namespace GDBStub {
 
+/**
+ * Based on the Rosalina implementation:
+ * https://github.com/LumaTeam/Luma3DS/blob/master/sysmodules/rosalina/include/gdb.h#L46C27-L62
+ */
 struct PackedGdbHioRequest {
     char magic[4]; // "GDB\x00"
     u32 version;
diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp
index f3f00a8b7..e2ddb1e91 100644
--- a/src/core/hle/kernel/svc.cpp
+++ b/src/core/hle/kernel/svc.cpp
@@ -1144,7 +1144,7 @@ void SVC::Break(u8 break_reason) {
 /// Used to output a message on a debug hardware unit, or for the GDB HIO
 // protocol - does nothing on a retail unit.
 void SVC::OutputDebugString(VAddr address, s32 len) {
-    if (!Memory::IsValidVirtualAddress(*kernel.GetCurrentProcess(), address)) {
+    if (!memory.IsValidVirtualAddress(*kernel.GetCurrentProcess(), address)) {
         LOG_WARNING(Kernel_SVC, "OutputDebugString called with invalid address {:X}", address);
         return;
     }

From 351730d585a85f97c26bcabe93b2fddea19107ec Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Fri, 31 Mar 2023 14:06:43 -0400
Subject: [PATCH 08/13] Fix infinite hang if GDB client kills gdbstub

---
 src/core/gdbstub/gdbstub.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index 09f29be46..281ba3d62 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -1066,12 +1066,15 @@ void HandlePacket() {
         SendSignal(current_thread, latest_signal);
         break;
     case 'k':
-        Shutdown();
+        ToggleServer(false);
+        // Continue execution so we don't hang forever after shutting down the
+        // server
+        Continue();
         LOG_INFO(Debug_GDBStub, "killed by gdb");
         return;
     case 'F':
         if (HandleHioReply(command_buffer, command_length)) {
-            // TODO: technically if we were paused when the reply came in, we
+            // TODO: technically if we were paused when the request came in, we
             // shouldn't continue here. Could recurse back into HandlePacket() maybe??
             Continue();
         } else {

From 0d4c93d1c28e7014873d248fd7eadceafa48dc07 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Tue, 4 Apr 2023 12:20:41 -0400
Subject: [PATCH 09/13] Undo some changes exposing unused functions

---
 src/core/gdbstub/gdbstub.cpp | 14 ++++++++++++--
 src/core/gdbstub/gdbstub.h   | 16 +---------------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index 281ba3d62..c8012dda2 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -313,7 +313,12 @@ static void IntToGdbHex(u8* dest, u32 v) {
     }
 }
 
-u32 GdbHexToInt(const u8* src) {
+/**
+ * Convert a gdb-formatted hex string into a u32.
+ *
+ * @param src Pointer to hex string.
+ */
+static u32 GdbHexToInt(const u8* src) {
     u32 output = 0;
 
     for (int i = 0; i < 8; i += 2) {
@@ -337,7 +342,12 @@ static void LongToGdbHex(u8* dest, u64 v) {
     }
 }
 
-u64 GdbHexToLong(const u8* src) {
+/**
+ * Convert a gdb-formatted hex string into a u64.
+ *
+ * @param src Pointer to hex string.
+ */
+static u64 GdbHexToLong(const u8* src) {
     u64 output = 0;
 
     for (int i = 0; i < 16; i += 2) {
diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h
index e11d31324..70aa0e2db 100644
--- a/src/core/gdbstub/gdbstub.h
+++ b/src/core/gdbstub/gdbstub.h
@@ -111,25 +111,11 @@ void SetCpuStepFlag(bool is_step);
 void SendTrap(Kernel::Thread* thread, int trap);
 
 /**
+u32 HexToInt(const u8* src, std::size_t len) {
  * Converts input hex string characters into an array of equivalent of u8 bytes.
  *
  * @param src Pointer to array of output hex string characters.
  * @param len Length of src array.
  */
 u32 HexToInt(const u8* src, std::size_t len);
-
-/**
- * Convert a gdb-formatted hex string into a u32.
- *
- * @param src Pointer to hex string.
- */
-u32 GdbHexToInt(const u8* src);
-
-/**
- * Convert a gdb-formatted hex string into a u64.
- *
- * @param src Pointer to hex string.
- */
-u64 GdbHexToLong(const u8* src);
-
 } // namespace GDBStub

From 6e45de760eb8ea75d68f003138156e0d1a2ad52e Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Tue, 4 Apr 2023 13:09:34 -0400
Subject: [PATCH 10/13] Cleanup new code, add docs and error handling

---
 src/core/gdbstub/gdbstub.cpp | 28 +++--------
 src/core/gdbstub/gdbstub.h   | 13 ++++-
 src/core/gdbstub/hio.cpp     | 92 ++++++++++++++++++++++++++----------
 src/core/gdbstub/hio.h       | 29 ++++++++----
 src/core/hle/kernel/svc.cpp  |  9 ++--
 5 files changed, 112 insertions(+), 59 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index c8012dda2..0a4f8769c 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -487,12 +487,7 @@ static void SendPacket(const char packet) {
     }
 }
 
-/**
- * Send reply to gdb client.
- *
- * @param reply Reply to be sent to client.
- */
-static void SendReply(const char* reply) {
+void SendReply(const char* reply) {
     if (!IsConnected()) {
         return;
     }
@@ -1048,9 +1043,8 @@ void HandlePacket() {
         return;
     }
 
-    if (HasPendingHioRequest()) {
-        const auto request_packet = BuildHioRequestPacket();
-        SendReply(request_packet.data());
+    if (HandlePendingHioRequestPacket()) {
+        // Don't do anything else while we wait for the client to respond
         return;
     }
 
@@ -1076,21 +1070,13 @@ void HandlePacket() {
         SendSignal(current_thread, latest_signal);
         break;
     case 'k':
-        ToggleServer(false);
-        // Continue execution so we don't hang forever after shutting down the
-        // server
-        Continue();
         LOG_INFO(Debug_GDBStub, "killed by gdb");
+        ToggleServer(false);
+        // Continue execution so we don't hang forever after shutting down the server
+        Continue();
         return;
     case 'F':
-        if (HandleHioReply(command_buffer, command_length)) {
-            // TODO: technically if we were paused when the request came in, we
-            // shouldn't continue here. Could recurse back into HandlePacket() maybe??
-            Continue();
-        } else {
-            // TODO reply with errno if relevant. Maybe that code should live in
-            // HandleHioReply
-        }
+        HandleHioReply(command_buffer, command_length);
         break;
     case 'g':
         ReadRegisters();
diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h
index 70aa0e2db..33d6bd8f1 100644
--- a/src/core/gdbstub/gdbstub.h
+++ b/src/core/gdbstub/gdbstub.h
@@ -90,6 +90,11 @@ bool CheckBreakpoint(VAddr addr, GDBStub::BreakpointType type);
 // If set to true, the CPU will halt at the beginning of the next CPU loop.
 bool GetCpuHaltFlag();
 
+/**
+ * If set to true, the CPU will halt at the beginning of the next CPU loop.
+ *
+ * @param halt whether to halt on the next loop
+ */
 void SetCpuHaltFlag(bool halt);
 
 // If set to true and the CPU is halted, the CPU will step one instruction.
@@ -111,7 +116,13 @@ void SetCpuStepFlag(bool is_step);
 void SendTrap(Kernel::Thread* thread, int trap);
 
 /**
-u32 HexToInt(const u8* src, std::size_t len) {
+ * Send reply to gdb client.
+ *
+ * @param reply Reply to be sent to client.
+ */
+void SendReply(const char* reply);
+
+/**
  * Converts input hex string characters into an array of equivalent of u8 bytes.
  *
  * @param src Pointer to array of output hex string characters.
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index 06eb3eeaa..50fd1a0fb 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Citra Emulator Project
+// Copyright 2023 Citra Emulator Project
 // Licensed under GPLv2 or any later version
 // Refer to the license.txt file included.
 
@@ -23,19 +23,52 @@ enum class Status {
 
 static std::atomic<Status> request_status{Status::NoRequest};
 
+static std::atomic<bool> was_halted = false;
+static std::atomic<bool> was_stepping = false;
+
 } // namespace
 
+/**
+ * @return Whether the application has requested I/O, and it has not been sent.
+ */
+static bool HasPendingHioRequest() {
+    return current_hio_request_addr != 0 && request_status == Status::NotSent;
+}
+
+/**
+ * @return Whether the GDB stub is awaiting a reply from the client.
+ */
+static bool WaitingForHioReply() {
+    return current_hio_request_addr != 0 && request_status == Status::SentWaitingReply;
+}
+
+/**
+ * Send a response indicating an error back to the application.
+ *
+ * @param error_code The error code to respond back to the app. This typically corresponds to errno.
+ *
+ * @param retval The return value of the syscall the app requested.
+ */
+static void SendErrorReply(int error_code, int retval = -1) {
+    auto packet = fmt::format("F{:x},{:x}", retval, error_code);
+    SendReply(packet.data());
+}
+
 void SetHioRequest(const VAddr addr) {
     if (!IsServerEnabled()) {
         LOG_WARNING(Debug_GDBStub, "HIO requested but GDB stub is not running");
         return;
     }
 
-    if (HasPendingHioRequest() || WaitingForHioReply()) {
+    if (WaitingForHioReply()) {
         LOG_WARNING(Debug_GDBStub, "HIO requested while already in progress!");
         return;
     }
 
+    if (HasPendingHioRequest()) {
+        LOG_INFO(Debug_GDBStub, "overwriting existing HIO request that was not sent yet");
+    }
+
     auto& memory = Core::System::GetInstance().Memory();
     const auto process = Core::System::GetInstance().Kernel().GetCurrentProcess();
 
@@ -59,6 +92,9 @@ void SetHioRequest(const VAddr addr) {
     current_hio_request_addr = addr;
     request_status = Status::NotSent;
 
+    was_halted = GetCpuHaltFlag();
+    was_stepping = GetCpuStepFlag();
+
     // Now halt, so that no further instructions are executed until the request
     // is processed by the client. We will continue after the reply comes back
     Break();
@@ -67,19 +103,19 @@ void SetHioRequest(const VAddr addr) {
     Core::GetRunningCore().ClearInstructionCache();
 }
 
-bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
+void HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     if (!WaitingForHioReply()) {
         LOG_WARNING(Debug_GDBStub, "Got HIO reply but never sent a request");
-        // TODO send error reply packet?
-        return false;
+        return;
     }
 
     // Skip 'F' header
     auto* command_pos = command_buffer + 1;
 
     if (*command_pos == 0 || *command_pos == ',') {
-        // return GDB_ReplyErrno(ctx, EILSEQ);
-        return false;
+        LOG_WARNING(Debug_GDBStub, "bad HIO packet format position 0: {}", *command_pos);
+        SendErrorReply(EILSEQ);
+        return;
     }
 
     // Set the sign of the retval
@@ -100,8 +136,8 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
 
     if (command_parts.empty() || command_parts.size() > 3) {
         LOG_WARNING(Debug_GDBStub, "unexpected reply packet size: {}", command_parts);
-        // return GDB_ReplyErrno(ctx, EILSEQ);
-        return false;
+        SendErrorReply(EILSEQ);
+        return;
     }
 
     u64 unsigned_retval = HexToInt((u8*)command_parts[0].data(), command_parts[0].size());
@@ -119,8 +155,9 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
 
     if (command_parts.size() > 2 && !command_parts[2].empty()) {
         if (command_parts[2][0] != 'C') {
-            return false;
-            // return GDB_ReplyErrno(ctx, EILSEQ);
+            LOG_WARNING(Debug_GDBStub, "expected ctrl-c flag got '{}'", command_parts[2][0]);
+            SendErrorReply(EILSEQ);
+            return;
         }
 
         // for now we just ignore any trailing ";..." attachments
@@ -142,7 +179,7 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     if (!memory.IsValidVirtualAddress(*process, current_hio_request_addr)) {
         LOG_WARNING(Debug_GDBStub, "Invalid address {:#X} to write HIO reply",
                     current_hio_request_addr);
-        return false;
+        return;
     }
 
     memory.WriteBlock(*process, current_hio_request_addr, &current_hio_request,
@@ -152,18 +189,22 @@ bool HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     current_hio_request_addr = 0;
     request_status = Status::NoRequest;
 
-    return true;
+    // Restore state from before the request came in
+    SetCpuStepFlag(was_stepping);
+    SetCpuHaltFlag(was_halted);
+    Core::GetRunningCore().ClearInstructionCache();
 }
 
-bool HasPendingHioRequest() {
-    return current_hio_request_addr != 0 && request_status == Status::NotSent;
-}
+bool HandlePendingHioRequestPacket() {
+    if (!HasPendingHioRequest()) {
+        return false;
+    }
 
-bool WaitingForHioReply() {
-    return current_hio_request_addr != 0 && request_status == Status::SentWaitingReply;
-}
+    if (WaitingForHioReply()) {
+        // We already sent it, continue waiting for a reply
+        return true;
+    }
 
-std::string BuildHioRequestPacket() {
     std::string packet = fmt::format("F{}", current_hio_request.function_name);
     // TODO: should we use the IntToGdbHex funcs instead of fmt::format_to ?
 
@@ -192,17 +233,18 @@ std::string BuildHioRequestPacket() {
             break;
 
         default:
-            // TODO: early return? Or break out of this loop?
-            break;
+            LOG_WARNING(Debug_GDBStub, "unexpected hio request param format '{}'",
+                        current_hio_request.param_format[i]);
+            SendErrorReply(EILSEQ);
+            return false;
         }
     }
 
     LOG_TRACE(Debug_GDBStub, "HIO request packet: {}", packet);
 
-    // technically we should set this _after_ the reply is sent...
+    SendReply(packet.data());
     request_status = Status::SentWaitingReply;
-
-    return packet;
+    return true;
 }
 
 } // namespace GDBStub
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index ec6804df0..dda4e8ef1 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -1,4 +1,4 @@
-// Copyright 2022 Citra Emulator Project
+// Copyright 2023 Citra Emulator Project
 // Licensed under GPLv2 or any later version
 // Refer to the license.txt file included.
 
@@ -11,11 +11,14 @@
 namespace GDBStub {
 
 /**
+ * A request from a debugged application to perform some I/O with the GDB client.
+ * This structure is also used to encode the reply back to the application.
+ *
  * Based on the Rosalina implementation:
  * https://github.com/LumaTeam/Luma3DS/blob/master/sysmodules/rosalina/include/gdb.h#L46C27-L62
  */
 struct PackedGdbHioRequest {
-    char magic[4]; // "GDB\x00"
+    char magic[4]; // "GDB\0"
     u32 version;
 
     // Request
@@ -31,14 +34,24 @@ struct PackedGdbHioRequest {
     bool ctrl_c;
 };
 
+/**
+ * Set the current HIO request to the given address. This is how the debugged
+ * app indicates to the gdbstub that it wishes to perform a request.
+ *
+ *  @param address The memory address of the \ref PackedGdbHioRequest.
+ */
 void SetHioRequest(const VAddr address);
 
-bool HandleHioReply(const u8* const command_buffer, const u32 command_length);
+/**
+ * If there is a pending HIO request, send it to the client.
+ *
+ * @returns whethere any request was sent to the client.
+ */
+bool HandlePendingHioRequestPacket();
 
-bool HasPendingHioRequest();
-
-bool WaitingForHioReply();
-
-std::string BuildHioRequestPacket();
+/**
+ * Process an HIO reply from the client.
+ */
+void HandleHioReply(const u8* const command_buffer, const u32 command_length);
 
 } // namespace GDBStub
diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp
index e2ddb1e91..6bdc72535 100644
--- a/src/core/hle/kernel/svc.cpp
+++ b/src/core/hle/kernel/svc.cpp
@@ -1141,8 +1141,8 @@ void SVC::Break(u8 break_reason) {
     system.SetStatus(Core::System::ResultStatus::ErrorUnknown);
 }
 
-/// Used to output a message on a debug hardware unit, or for the GDB HIO
-// protocol - does nothing on a retail unit.
+/// Used to output a message on a debug hardware unit, or for the GDB file I/O
+/// (HIO) protocol - does nothing on a retail unit.
 void SVC::OutputDebugString(VAddr address, s32 len) {
     if (!memory.IsValidVirtualAddress(*kernel.GetCurrentProcess(), address)) {
         LOG_WARNING(Kernel_SVC, "OutputDebugString called with invalid address {:X}", address);
@@ -1154,7 +1154,8 @@ void SVC::OutputDebugString(VAddr address, s32 len) {
         return;
     }
 
-    if (len <= 0) {
+    if (len < 0) {
+        LOG_WARNING(Kernel_SVC, "OutputDebugString called with invalid length {}", len);
         return;
     }
 
@@ -2224,7 +2225,7 @@ const std::array<SVC::FunctionDef, 180> SVC::SVC_Table{{
     {0x60, nullptr, "DebugActiveProcess"},
     {0x61, nullptr, "BreakDebugProcess"},
     {0x62, nullptr, "TerminateDebugProcess"},
-    {0x63, nullptr, "GetProcessDebugEvent"}, // TODO: do we need this for HIO to work?
+    {0x63, nullptr, "GetProcessDebugEvent"},
     {0x64, nullptr, "ContinueDebugEvent"},
     {0x65, &SVC::Wrap<&SVC::GetProcessList>, "GetProcessList"},
     {0x66, nullptr, "GetThreadList"},

From 696c0904f8ab56d08f23ec23291bc742ee792f43 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Tue, 4 Apr 2023 13:21:25 -0400
Subject: [PATCH 11/13] Update some c-style casts -> reinterpret_cast

---
 src/core/gdbstub/gdbstub.cpp | 5 +++--
 src/core/gdbstub/hio.cpp     | 7 ++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp
index 0a4f8769c..073ecdc0e 100644
--- a/src/core/gdbstub/gdbstub.cpp
+++ b/src/core/gdbstub/gdbstub.cpp
@@ -851,9 +851,10 @@ static void ReadMemory() {
     MemToGdbHex(reply, data.data(), len);
     reply[len * 2] = '\0';
 
-    LOG_DEBUG(Debug_GDBStub, "ReadMemory result: {}", (char*)reply);
+    auto reply_str = reinterpret_cast<char*>(reply);
 
-    SendReply(reinterpret_cast<char*>(reply));
+    LOG_DEBUG(Debug_GDBStub, "ReadMemory result: {}", reply_str);
+    SendReply(reply_str);
 }
 
 /// Modify location in memory with data received from the gdb client.
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index 50fd1a0fb..b19535c21 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -140,13 +140,14 @@ void HandleHioReply(const u8* const command_buffer, const u32 command_length) {
         return;
     }
 
-    u64 unsigned_retval = HexToInt((u8*)command_parts[0].data(), command_parts[0].size());
+    u64 unsigned_retval =
+        HexToInt(reinterpret_cast<const u8*>(command_parts[0].data()), command_parts[0].size());
     current_hio_request.retval *= unsigned_retval;
 
     if (command_parts.size() > 1) {
         // Technically the errno could be signed but in practice this doesn't happen
         current_hio_request.gdb_errno =
-            HexToInt((u8*)command_parts[1].data(), command_parts[1].size());
+            HexToInt(reinterpret_cast<const u8*>(command_parts[1].data()), command_parts[1].size());
     } else {
         current_hio_request.gdb_errno = 0;
     }
@@ -228,7 +229,7 @@ bool HandlePendingHioRequestPacket() {
         case 's':
             // strings are written as {pointer}/{length}
             fmt::format_to(std::back_inserter(packet), ",{:x}/{:x}",
-                           (u32)current_hio_request.parameters[i],
+                           static_cast<u32>(current_hio_request.parameters[i]),
                            current_hio_request.string_lengths[nStr++]);
             break;
 

From 4cc181e5b8382d1351dbb047dcd1186d1c5ce40c Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Wed, 5 Apr 2023 12:41:17 -0400
Subject: [PATCH 12/13] Address review comments

Use more modern C++ style casts + std::array, clean up some log
messages, and rename some functions.
---
 src/citra/default_ini.h  |  2 +-
 src/core/gdbstub/hio.cpp | 40 +++++++++++++++++++++++++---------------
 src/core/gdbstub/hio.h   | 23 ++++++++++++++++-------
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/src/citra/default_ini.h b/src/citra/default_ini.h
index afb59a375..692134590 100644
--- a/src/citra/default_ini.h
+++ b/src/citra/default_ini.h
@@ -322,7 +322,7 @@ camera_inner_flip =
 
 [Miscellaneous]
 # A filter which removes logs below a certain logging level.
-# Examples: *:Debug Kernel.SVC:Trace Service:Critical
+# Examples: *:Debug Kernel.SVC:Trace Service.*:Critical
 log_filter = *:Info
 
 [Debugging]
diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index b19535c21..02d149b0f 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -38,7 +38,7 @@ static bool HasPendingHioRequest() {
 /**
  * @return Whether the GDB stub is awaiting a reply from the client.
  */
-static bool WaitingForHioReply() {
+static bool IsWaitingForHioReply() {
     return current_hio_request_addr != 0 && request_status == Status::SentWaitingReply;
 }
 
@@ -60,8 +60,8 @@ void SetHioRequest(const VAddr addr) {
         return;
     }
 
-    if (WaitingForHioReply()) {
-        LOG_WARNING(Debug_GDBStub, "HIO requested while already in progress!");
+    if (IsWaitingForHioReply()) {
+        LOG_WARNING(Debug_GDBStub, "HIO requested while already in progress");
         return;
     }
 
@@ -79,9 +79,14 @@ void SetHioRequest(const VAddr addr) {
 
     memory.ReadBlock(*process, addr, &current_hio_request, sizeof(PackedGdbHioRequest));
 
-    std::string_view magic{current_hio_request.magic};
-    if (magic != "GDB") {
-        LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application: magic '{}'", magic);
+    if (current_hio_request.magic != std::array{'G', 'D', 'B', '\0'}) {
+        std::string_view bad_magic{
+            std::begin(current_hio_request.magic),
+            std::end(current_hio_request.magic),
+        };
+        LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application: bad magic '{}'",
+                    bad_magic);
+
         current_hio_request_addr = 0;
         current_hio_request = {};
         request_status = Status::NoRequest;
@@ -104,7 +109,7 @@ void SetHioRequest(const VAddr addr) {
 }
 
 void HandleHioReply(const u8* const command_buffer, const u32 command_length) {
-    if (!WaitingForHioReply()) {
+    if (!IsWaitingForHioReply()) {
         LOG_WARNING(Debug_GDBStub, "Got HIO reply but never sent a request");
         return;
     }
@@ -135,7 +140,7 @@ void HandleHioReply(const u8* const command_buffer, const u32 command_length) {
     Common::SplitString(command_str, ',', command_parts);
 
     if (command_parts.empty() || command_parts.size() > 3) {
-        LOG_WARNING(Debug_GDBStub, "unexpected reply packet size: {}", command_parts);
+        LOG_WARNING(Debug_GDBStub, "Unexpected reply packet size: {}", command_parts);
         SendErrorReply(EILSEQ);
         return;
     }
@@ -201,25 +206,30 @@ bool HandlePendingHioRequestPacket() {
         return false;
     }
 
-    if (WaitingForHioReply()) {
+    if (IsWaitingForHioReply()) {
         // We already sent it, continue waiting for a reply
         return true;
     }
 
-    std::string packet = fmt::format("F{}", current_hio_request.function_name);
-    // TODO: should we use the IntToGdbHex funcs instead of fmt::format_to ?
+    // We need a null-terminated string from char* instead of using
+    // the full length of the array (like {.begin(), .end()} constructor would)
+    std::string_view function_name{current_hio_request.function_name.data()};
 
-    u32 nStr = 0;
+    std::string packet = fmt::format("F{}", function_name);
+
+    u32 str_length_idx = 0;
 
     for (u32 i = 0; i < 8 && current_hio_request.param_format[i] != 0; i++) {
         u64 param = current_hio_request.parameters[i];
 
+        // TODO: should we use the IntToGdbHex funcs instead of fmt::format_to ?
         switch (current_hio_request.param_format[i]) {
         case 'i':
         case 'I':
         case 'p':
-            // For pointers and 32-bit ints, truncate before sending
+            // For pointers and 32-bit ints, truncate down to size before sending
             param = static_cast<u32>(param);
+            [[fallthrough]];
 
         case 'l':
         case 'L':
@@ -230,7 +240,7 @@ bool HandlePendingHioRequestPacket() {
             // strings are written as {pointer}/{length}
             fmt::format_to(std::back_inserter(packet), ",{:x}/{:x}",
                            static_cast<u32>(current_hio_request.parameters[i]),
-                           current_hio_request.string_lengths[nStr++]);
+                           current_hio_request.string_lengths[str_length_idx++]);
             break;
 
         default:
@@ -241,7 +251,7 @@ bool HandlePendingHioRequestPacket() {
         }
     }
 
-    LOG_TRACE(Debug_GDBStub, "HIO request packet: {}", packet);
+    LOG_TRACE(Debug_GDBStub, "HIO request packet: '{}'", packet);
 
     SendReply(packet.data());
     request_status = Status::SentWaitingReply;
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index dda4e8ef1..f85f1e7dc 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -14,19 +14,25 @@ namespace GDBStub {
  * A request from a debugged application to perform some I/O with the GDB client.
  * This structure is also used to encode the reply back to the application.
  *
- * Based on the Rosalina implementation:
+ * Based on the Rosalina + libctru implementations:
  * https://github.com/LumaTeam/Luma3DS/blob/master/sysmodules/rosalina/include/gdb.h#L46C27-L62
+ * https://github.com/devkitPro/libctru/blob/master/libctru/source/gdbhio.c#L71-L87
  */
 struct PackedGdbHioRequest {
-    char magic[4]; // "GDB\0"
+    std::array<char, 4> magic; // "GDB\0"
     u32 version;
 
-    // Request
-    char function_name[16 + 1];
-    char param_format[8 + 1];
+private:
+    static inline constexpr std::size_t MAX_FUNCNAME_LEN = 16;
+    static inline constexpr std::size_t PARAM_COUNT = 8;
 
-    u64 parameters[8];
-    u32 string_lengths[8];
+public:
+    // Request. Char arrays have +1 entry for null terminator
+    std::array<char, MAX_FUNCNAME_LEN + 1> function_name;
+    std::array<char, PARAM_COUNT + 1> param_format;
+
+    std::array<u64, PARAM_COUNT> parameters;
+    std::array<u32, PARAM_COUNT> string_lengths;
 
     // Return
     s64 retval;
@@ -34,6 +40,9 @@ struct PackedGdbHioRequest {
     bool ctrl_c;
 };
 
+static_assert(sizeof(PackedGdbHioRequest) == 152,
+              "HIO request size must match libctru implementation");
+
 /**
  * Set the current HIO request to the given address. This is how the debugged
  * app indicates to the gdbstub that it wishes to perform a request.

From 025913a7342c085c246b471aa884749773cc04e5 Mon Sep 17 00:00:00 2001
From: Ian Chamberlain <ian.h.chamberlain@gmail.com>
Date: Wed, 5 Apr 2023 13:27:46 -0400
Subject: [PATCH 13/13] Fix string_view constructor for macOS clang

Some versions of clang 14 (macOS+android) don't implement
`string_view(It first, It last)`, so let's use
`string_view(const CharT*, size_type)` instead.

Also remove unused header leftover from old code that uses std::string.
---
 src/core/gdbstub/hio.cpp | 4 ++--
 src/core/gdbstub/hio.h   | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp
index 02d149b0f..ac085d5b7 100644
--- a/src/core/gdbstub/hio.cpp
+++ b/src/core/gdbstub/hio.cpp
@@ -81,8 +81,8 @@ void SetHioRequest(const VAddr addr) {
 
     if (current_hio_request.magic != std::array{'G', 'D', 'B', '\0'}) {
         std::string_view bad_magic{
-            std::begin(current_hio_request.magic),
-            std::end(current_hio_request.magic),
+            current_hio_request.magic.data(),
+            current_hio_request.magic.size(),
         };
         LOG_WARNING(Debug_GDBStub, "Invalid HIO request sent by application: bad magic '{}'",
                     bad_magic);
diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h
index f85f1e7dc..d498a2737 100644
--- a/src/core/gdbstub/hio.h
+++ b/src/core/gdbstub/hio.h
@@ -4,8 +4,6 @@
 
 #pragma once
 
-#include <string>
-
 #include "common/common_types.h"
 
 namespace GDBStub {