From 7de1bf374630aeddb3c6bb69c0a83a492800ae17 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 3 Dec 2022 16:09:13 -0500 Subject: [PATCH] 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 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, ¤t_hio_request, sizeof(PackedGdbHioRequest)); + auto& memory = Core::System::GetInstance().Memory(); + memory.ReadBlock(*process, addr, ¤t_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, ¤t_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, ¤t_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