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((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(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 sent_request{false}; + +enum class Status { + NoRequest, + NotSent, + SentWaitingReply, +}; + +static std::atomic 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, ¤t_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 + #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;