From c17261abf7a988e1cb38495c6f0ab50e20832733 Mon Sep 17 00:00:00 2001 From: emoose Date: Tue, 8 Oct 2024 22:10:57 +0100 Subject: [PATCH] [GDBStub] Return error for code / unimpl. reg writes, small fixups --- src/xenia/debug/gdb/gdbstub.cc | 136 ++++++++++++++++----------------- src/xenia/debug/gdb/gdbstub.h | 25 +++++- 2 files changed, 90 insertions(+), 71 deletions(-) diff --git a/src/xenia/debug/gdb/gdbstub.cc b/src/xenia/debug/gdb/gdbstub.cc index 7798c6b7e..106ded38b 100644 --- a/src/xenia/debug/gdb/gdbstub.cc +++ b/src/xenia/debug/gdb/gdbstub.cc @@ -40,21 +40,9 @@ using xe::kernel::XModule; using xe::kernel::XObject; using xe::kernel::XThread; -enum class GdbStubControl : char { - Ack = '+', - Nack = '-', - PacketStart = '$', - PacketEnd = '#', - Interrupt = '\03', -}; - constexpr const char* kGdbReplyOK = "OK"; constexpr const char* kGdbReplyError = "E01"; -constexpr int kSignalSigill = 4; // Illegal instruction -constexpr int kSignalSigtrap = 5; // Trace trap -constexpr int kSignalSigsegv = 11; // Segmentation fault - // must start with l for debugger to accept it constexpr char kMemoryMapXml[] = R"(l @@ -70,7 +58,7 @@ constexpr char kMemoryMapXml[] = )"; -// TODO: add power-altivec.xml (and update ReadRegister to support it) +// TODO: add power-altivec.xml (and update RegisterRead to support it) constexpr char kTargetXml[] = R"(l @@ -229,13 +217,13 @@ void GDBStub::Listen(std::unique_ptr& client) { cache_.cur_thread_id = cache_.notify_thread_id; } - int sig_num = kSignalSigtrap; + SignalCode signal = SignalCode::SIGTRAP; if (cache_.notify_exception_code.has_value()) { if (cache_.notify_exception_code == xe::Exception::Code::kIllegalInstruction) { - sig_num = kSignalSigill; + signal = SignalCode::SIGILL; } else { - sig_num = kSignalSigsegv; + signal = SignalCode::SIGSEGV; } cache_.notify_exception_code.reset(); @@ -243,7 +231,7 @@ void GDBStub::Listen(std::unique_ptr& client) { } SendPacket(client, - GetThreadStateReply(cache_.notify_thread_id, sig_num)); + GetThreadStateReply(cache_.notify_thread_id, signal)); cache_.notify_thread_id = -1; cache_.notify_stopped = false; } @@ -255,8 +243,7 @@ void GDBStub::Listen(std::unique_ptr& client) { void GDBStub::SendPacket(std::unique_ptr& client, const std::string& data) { std::stringstream ss; - ss << char(GdbStubControl::PacketStart) << data - << char(GdbStubControl::PacketEnd); + ss << char(ControlCode::PacketStart) << data << char(ControlCode::PacketEnd); uint8_t checksum = 0; for (char c : data) { @@ -271,7 +258,7 @@ void GDBStub::SendPacket(std::unique_ptr& client, #ifdef DEBUG std::string GetPacketFriendlyName(const std::string& packetCommand) { - static const std::unordered_map command_names = { + static const std::unordered_map kGdbCommandNames = { {"?", "StartupQuery"}, {"!", "EnableExtendedMode"}, {"p", "RegRead"}, @@ -297,8 +284,8 @@ std::string GetPacketFriendlyName(const std::string& packetCommand) { }; std::string packet_name = ""; - auto it = command_names.find(packetCommand); - if (it != command_names.end()) { + auto it = kGdbCommandNames.find(packetCommand); + if (it != kGdbCommandNames.end()) { packet_name = it->second; } @@ -319,8 +306,7 @@ bool GDBStub::ProcessIncomingData(std::unique_ptr& client, // Hacky interrupt '\03' packet handling, some reason checksum isn't // attached to this? - bool isInterrupt = - buffer[0] == char(GdbStubControl::Interrupt) && received == 1; + bool isInterrupt = buffer[0] == char(ControlCode::Interrupt) && received == 1; // Check if we've received a full packet yet, if not exit and allow caller // to try again @@ -330,7 +316,7 @@ bool GDBStub::ProcessIncomingData(std::unique_ptr& client, if (isInterrupt || packet_end + 2 < receive_buffer.length()) { std::string current_packet; if (isInterrupt) { - current_packet = char(GdbStubControl::Interrupt); + current_packet = char(ControlCode::Interrupt); receive_buffer = ""; isInterrupt = false; } else { @@ -348,12 +334,12 @@ bool GDBStub::ProcessIncomingData(std::unique_ptr& client, command.data); #endif - GdbStubControl result = GdbStubControl::Ack; + ControlCode result = ControlCode::Ack; client->Send(&result, 1); std::string response = HandleGDBCommand(command); SendPacket(client, response); } else { - GdbStubControl result = GdbStubControl::Nack; + ControlCode result = ControlCode::Nack; client->Send(&result, 1); } } else { @@ -390,23 +376,23 @@ bool GDBStub::ParsePacket(const std::string& packet, GDBCommand& out_cmd) { char c = ReadCharFromBuffer(); // Expecting start of packet '$' - if (c != char(GdbStubControl::PacketStart)) { + if (c != char(ControlCode::PacketStart)) { // gdb starts conversation with + for some reason - if (c == char(GdbStubControl::Ack)) { + if (c == char(ControlCode::Ack)) { c = ReadCharFromBuffer(); } // and IDA sometimes has double +, grr - if (c == char(GdbStubControl::Ack)) { + if (c == char(ControlCode::Ack)) { c = ReadCharFromBuffer(); } // Interrupt is special, handle it without checking checksum - if (c == char(GdbStubControl::Interrupt)) { - out_cmd.cmd = char(GdbStubControl::Interrupt); + if (c == char(ControlCode::Interrupt)) { + out_cmd.cmd = char(ControlCode::Interrupt); out_cmd.data = ""; out_cmd.checksum = 0; return true; } - if (c != char(GdbStubControl::PacketStart)) { + if (c != char(ControlCode::PacketStart)) { return false; } } @@ -423,7 +409,7 @@ bool GDBStub::ParsePacket(const std::string& packet, GDBCommand& out_cmd) { c = ReadCharFromBuffer(); // If we reach the end of the buffer or hit '#', stop - if (c == '\0' || c == char(GdbStubControl::PacketEnd)) { + if (c == '\0' || c == char(ControlCode::PacketEnd)) { break; } @@ -488,12 +474,14 @@ std::string GDBStub::RegisterRead(xe::cpu::ThreadDebugInfo* thread, // mode (eg. IDA will switch to 64-bit and refuse to allow decompiler to work // with it) // - // TODO: add altivec/VMX registers here... + // TODO: add altivec/VMX registers here + // TODO: add cvar to allow switch to 64-bit mode? (unsure if any x360 opcodes + // use the upper 32-bits?) // // ids from gdb/features/rs6000/powerpc-64.c - switch (rid) { - // pc - case 64: { + switch (RegisterIndex(rid)) { + case RegisterIndex::PC: { + // // If we recently hit a BP then debugger is likely asking for registers // for it // @@ -514,71 +502,69 @@ std::string GDBStub::RegisterRead(xe::cpu::ThreadDebugInfo* thread, } return string_util::to_hex_string((uint32_t)0); } - case 65: + case RegisterIndex::MSR: return string_util::to_hex_string((uint32_t)thread->guest_context.msr); - case 66: + case RegisterIndex::CR: return string_util::to_hex_string((uint32_t)thread->guest_context.cr()); - case 67: + case RegisterIndex::LR: return string_util::to_hex_string((uint32_t)thread->guest_context.lr); - case 68: + case RegisterIndex::CTR: return string_util::to_hex_string((uint32_t)thread->guest_context.ctr); - // xer - case 69: + case RegisterIndex::XER: return std::string(8, 'x'); - case 70: + case RegisterIndex::FPSCR: return string_util::to_hex_string(thread->guest_context.fpscr.value); } - if (rid > 70) { + if (rid >= int(RegisterIndex::PC)) { return ""; } // fpr - if (rid > 31) { + if (rid >= int(RegisterIndex::FPR0)) { return string_util::to_hex_string(thread->guest_context.f[rid - 32]); } // gpr return string_util::to_hex_string((uint32_t)thread->guest_context.r[rid]); } + std::string GDBStub::RegisterWrite(xe::cpu::ThreadDebugInfo* thread, uint32_t rid, const std::string_view value) { + // Have to update the main thread context, and the ThreadDebugInfo context auto* guest_context = thread->thread->thread_state()->context(); - switch (rid) { - // pc - case 64: - return kGdbReplyOK; // TODO: figure a way to change this - case 65: + switch (RegisterIndex(rid)) { + case RegisterIndex::PC: + return kGdbReplyError; // TODO: figure a way to change this + case RegisterIndex::MSR: guest_context->msr = string_util::from_string(value, true); thread->guest_context.msr = guest_context->msr; return kGdbReplyOK; - case 66: - // CR - return kGdbReplyOK; // TODO: figure a way to change this - case 67: + case RegisterIndex::CR: + return kGdbReplyError; // TODO: figure a way to change this + case RegisterIndex::LR: guest_context->lr = string_util::from_string(value, true); thread->guest_context.lr = guest_context->lr; return kGdbReplyOK; - case 68: + case RegisterIndex::CTR: guest_context->ctr = string_util::from_string(value, true); thread->guest_context.ctr = guest_context->ctr; return kGdbReplyOK; - // xer - case 69: - return kGdbReplyOK; - case 70: + case RegisterIndex::XER: + return kGdbReplyError; + case RegisterIndex::FPSCR: guest_context->fpscr.value = string_util::from_string(value, true); thread->guest_context.fpscr.value = guest_context->fpscr.value; return kGdbReplyOK; } - if (rid > 70) { + if (rid >= int(RegisterIndex::PC)) { return kGdbReplyError; } // fpr - if (rid > 31) { + if (rid >= int(RegisterIndex::FPR0)) { guest_context->f[rid - 32] = string_util::from_string(value, true); thread->guest_context.f[rid - 32] = guest_context->f[rid - 32]; return kGdbReplyOK; @@ -625,7 +611,7 @@ std::string GDBStub::RegisterReadAll() { return kGdbReplyError; } std::string result; - result.reserve(68 * 16 + 3 * 8); + result.reserve((39 * 8) + (32 * 16)); for (int i = 0; i < 71; ++i) { result += RegisterRead(thread, i); } @@ -638,6 +624,11 @@ std::string GDBStub::RegisterWriteAll(const std::string& data) { return kGdbReplyError; } + int expected_size = (39 * 8) + (32 * 16); + if (data.length() != expected_size) { + return kGdbReplyError; + } + int string_offset = 0; for (int i = 0; i < 71; ++i) { int reg_size = 8; // 8 hex-nibbles per 32-bit register @@ -746,6 +737,14 @@ std::string GDBStub::MemoryWrite(const std::string& data) { if (!heap) { return kGdbReplyError; } + + // Check if they're trying to write to an executable function + if (auto* exe_addr = processor_->LookupFunction(addr)) { + // TODO: allow the write and ask processor to recompile if no breakpoints + // are set there? + return kGdbReplyError; // error for now as writes here won't have an effect + } + uint32_t protect = 0; if (!heap->QueryProtect(addr, &protect) || (protect & kMemoryProtectRead) != kMemoryProtectRead) { @@ -791,10 +790,8 @@ std::string GDBStub::BuildThreadList() { return buffer; } -std::string GDBStub::GetThreadStateReply(uint32_t thread_id, uint8_t signal) { - constexpr int PC_REGISTER = 64; - constexpr int LR_REGISTER = 67; - +std::string GDBStub::GetThreadStateReply(uint32_t thread_id, + SignalCode signal) { auto* thread = cache_.thread_info(thread_id); if (thread_id != -1 && thread) { @@ -813,7 +810,8 @@ std::string GDBStub::GetThreadStateReply(uint32_t thread_id, uint8_t signal) { } return fmt::format("T{:02x}{:02x}:{:08x};{:02x}:{:08x};thread:{:x};", - signal, PC_REGISTER, uint32_t(pc_value), LR_REGISTER, + uint8_t(signal), int(RegisterIndex::PC), + uint32_t(pc_value), int(RegisterIndex::LR), uint32_t(thread->guest_context.lr), thread_id); } return "S05"; diff --git a/src/xenia/debug/gdb/gdbstub.h b/src/xenia/debug/gdb/gdbstub.h index 2e1d7f096..b8b71e02c 100644 --- a/src/xenia/debug/gdb/gdbstub.h +++ b/src/xenia/debug/gdb/gdbstub.h @@ -26,6 +26,28 @@ namespace debug { namespace gdb { class GDBStub : public cpu::DebugListener { + enum class ControlCode : char { + Ack = '+', + Nack = '-', + PacketStart = '$', + PacketEnd = '#', + Interrupt = '\03', + }; + + enum class SignalCode : uint8_t { SIGILL = 4, SIGTRAP = 5, SIGSEGV = 11 }; + + enum class RegisterIndex : int { + GPR0 = 0, + FPR0 = 32, + PC = 64, + MSR = 65, + CR = 66, + LR = 67, + CTR = 68, + XER = 69, + FPSCR = 70 + }; + public: virtual ~GDBStub(); @@ -77,7 +99,7 @@ class GDBStub : public cpu::DebugListener { std::string MemoryWrite(const std::string& data); std::string BuildThreadList(); - std::string GetThreadStateReply(uint32_t thread_id, uint8_t signal); + std::string GetThreadStateReply(uint32_t thread_id, SignalCode signal); bool CreateCodeBreakpoint(uint64_t address); void DeleteCodeBreakpoint(uint64_t address); @@ -112,7 +134,6 @@ class GDBStub : public cpu::DebugListener { std::vector thread_debug_infos; struct { - char kernel_call_filter[64] = {0}; std::vector> all_breakpoints; std::unordered_map code_breakpoints_by_guest_address;