From bd658a88014152551c2c833205bd2ac27b816537 Mon Sep 17 00:00:00 2001 From: Jarek Syrylak Date: Thu, 16 Aug 2018 10:40:52 +0100 Subject: [PATCH 1/3] GDB Modernization: - Can be used in either DynCom or Dynarmic mode - Added support for threads - Proper support for FPU registers - Fix for NibbleToHex conversion that used to produce false error codes - Fix for clang-format failing under Windows --- CMakeLists.txt | 2 +- src/citra_qt/configuration/configure_debug.ui | 7 - src/core/arm/dynarmic/arm_dynarmic.cpp | 3 + src/core/arm/dyncom/arm_dyncom.cpp | 1 + .../arm/dyncom/arm_dyncom_interpreter.cpp | 2 +- src/core/arm/skyeye_common/armstate.h | 25 ++ src/core/core.cpp | 7 +- src/core/gdbstub/gdbstub.cpp | 287 ++++++++++++++---- src/core/gdbstub/gdbstub.h | 9 + 9 files changed, 281 insertions(+), 62 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4d67916fb..b19f1147f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -321,7 +321,7 @@ if (CLANG_FORMAT) set(CCOMMENT "Running clang format against all the .h and .cpp files in src/") if (WIN32) add_custom_target(clang-format - COMMAND powershell.exe -Command "${CLANG_FORMAT} -i @(Get-ChildItem -Recurse ${SRCS}/* -Include \'*.h\', \'*.cpp\')" + COMMAND powershell.exe -Command "Get-ChildItem ${SRCS}/* -Include *.cpp,*.h -Recurse | Foreach {${CLANG_FORMAT} -i $_.fullname}" COMMENT ${CCOMMENT}) elseif(MINGW) add_custom_target(clang-format diff --git a/src/citra_qt/configuration/configure_debug.ui b/src/citra_qt/configuration/configure_debug.ui index 85593f6c7..24023106c 100644 --- a/src/citra_qt/configuration/configure_debug.ui +++ b/src/citra_qt/configuration/configure_debug.ui @@ -22,13 +22,6 @@ GDB - - - - The GDB Stub only works correctly when the CPU JIT is off. - - - diff --git a/src/core/arm/dynarmic/arm_dynarmic.cpp b/src/core/arm/dynarmic/arm_dynarmic.cpp index f324b7e09..461f1e564 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.cpp +++ b/src/core/arm/dynarmic/arm_dynarmic.cpp @@ -87,6 +87,8 @@ static void InterpreterFallback(u32 pc, Dynarmic::Jit* jit, void* user_arg) { jit->SetCpsr(state->Cpsr); jit->ExtRegs() = state->ExtReg; jit->SetFpscr(state->VFP[VFP_FPSCR]); + + state->ServeBreak(); } static bool IsReadOnlyMemory(u32 vaddr) { @@ -233,6 +235,7 @@ void ARM_Dynarmic::ClearInstructionCache() { for (const auto& j : jits) { j.second->ClearCache(); } + interpreter_state->instruction_cache.clear(); } void ARM_Dynarmic::InvalidateCacheRange(u32 start_address, size_t length) { diff --git a/src/core/arm/dyncom/arm_dyncom.cpp b/src/core/arm/dyncom/arm_dyncom.cpp index c06ffc4de..b984a62cd 100644 --- a/src/core/arm/dyncom/arm_dyncom.cpp +++ b/src/core/arm/dyncom/arm_dyncom.cpp @@ -147,6 +147,7 @@ void ARM_DynCom::ExecuteInstructions(u64 num_instructions) { state->NumInstrsToExecute = num_instructions; unsigned ticks_executed = InterpreterMainLoop(state.get()); CoreTiming::AddTicks(ticks_executed); + state.get()->ServeBreak(); } std::unique_ptr ARM_DynCom::NewContext() const { diff --git a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp index 0fa392b4b..cd70b8da3 100644 --- a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp +++ b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp @@ -956,7 +956,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { if (GDBStub::IsServerEnabled()) { \ if (GDBStub::IsMemoryBreak() || (breakpoint_data.type != GDBStub::BreakpointType::None && \ PC == breakpoint_data.address)) { \ - GDBStub::Break(); \ + cpu->RecordBreak(breakpoint_data); \ goto END; \ } \ } diff --git a/src/core/arm/skyeye_common/armstate.h b/src/core/arm/skyeye_common/armstate.h index 60880caf9..cca754b31 100644 --- a/src/core/arm/skyeye_common/armstate.h +++ b/src/core/arm/skyeye_common/armstate.h @@ -21,6 +21,8 @@ #include #include "common/common_types.h" #include "core/arm/skyeye_common/arm_regformat.h" +#include "core/core.h" +#include "core/gdbstub/gdbstub.h" // Signal levels enum { LOW = 0, HIGH = 1, LOWHIGH = 1, HIGHLOW = 2 }; @@ -189,6 +191,26 @@ public: return TFlag ? 2 : 4; } + void RecordBreak(GDBStub::BreakpointAddress bkpt) { + last_bkpt = bkpt; + last_bkpt_hit = true; + } + + void ServeBreak() { + if (GDBStub::IsServerEnabled()) { + if (last_bkpt_hit) { + Reg[15] = last_bkpt.address; + } + Kernel::Thread* thread = Kernel::GetCurrentThread(); + Core::CPU().SaveContext(thread->context); + if (last_bkpt_hit || GDBStub::GetCpuStepFlag()) { + last_bkpt_hit = false; + GDBStub::Break(); + GDBStub::SendTrap(thread, 5); + } + } + } + std::array Reg{}; // The current register file std::array Reg_usr{}; std::array Reg_svc{}; // R13_SVC R14_SVC @@ -246,4 +268,7 @@ private: u32 exclusive_tag; // The address for which the local monitor is in exclusive access mode bool exclusive_state; + + GDBStub::BreakpointAddress last_bkpt{}; + bool last_bkpt_hit; }; diff --git a/src/core/core.cpp b/src/core/core.cpp index 013ef4c2e..2b7bcc7d4 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -46,8 +46,7 @@ System::ResultStatus System::RunLoop(bool tight_loop) { // execute. Otherwise, get out of the loop function. if (GDBStub::GetCpuHaltFlag()) { if (GDBStub::GetCpuStepFlag()) { - GDBStub::SetCpuStepFlag(false); - tight_loop = 1; + tight_loop = false; } else { return ResultStatus::Success; } @@ -70,6 +69,10 @@ System::ResultStatus System::RunLoop(bool tight_loop) { } } + if (GDBStub::IsServerEnabled()) { + GDBStub::SetCpuStepFlag(false); + } + HW::Update(); Reschedule(); diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 57e683c49..ddf306207 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/hle/kernel/process.h" #include "core/loader/loader.h" #include "core/memory.h" @@ -57,9 +58,12 @@ const u32 SIGTERM = 15; const u32 MSG_WAITALL = 8; #endif -const u32 R15_REGISTER = 15; +const u32 SP_REGISTER = 13; +const u32 LR_REGISTER = 14; +const u32 PC_REGISTER = 15; const u32 CPSR_REGISTER = 25; -const u32 FPSCR_REGISTER = 58; +const u32 D0_REGISTER = 26; +const u32 FPSCR_REGISTER = 42; // For sample XML files see the GDB source /gdb/features // GDB also wants the l character at the start @@ -122,15 +126,17 @@ static u8 command_buffer[GDB_BUFFER_SIZE]; static u32 command_length; static u32 latest_signal = 0; -static bool step_break = false; static bool memory_break = false; +Kernel::Thread* current_thread = nullptr; + // Binding to a port within the reserved ports range (0-1023) requires root permissions, // so default to a port outside of that range. static u16 gdbstub_port = 24689; static bool halt_loop = true; static bool step_loop = false; +static bool send_trap = false; // If set to false, the server will never be started and no // gdbstub-related functions will be executed. @@ -144,12 +150,79 @@ struct Breakpoint { bool active; PAddr addr; u32 len; + std::array inst; }; static std::map breakpoints_execute; static std::map breakpoints_read; static std::map breakpoints_write; +static Kernel::Thread* FindThreadById(int id) { + const auto& threads = Kernel::GetThreadList(); + for (auto& thread : threads) { + if (thread->GetThreadId() == static_cast(id)) { + return thread.get(); + } + } + return nullptr; +} + +static u32 RegRead(std::size_t id, Kernel::Thread* thread = nullptr) { + if (!thread) { + return 0; + } + + if (id <= PC_REGISTER) { + return thread->context.get()->GetCpuRegister(id); + } else if (id == CPSR_REGISTER) { + return thread->context.get()->GetCpsr(); + } else { + return 0; + } +} + +static void RegWrite(std::size_t id, u32 val, Kernel::Thread* thread = nullptr) { + if (!thread) { + return; + } + + if (id <= PC_REGISTER) { + return thread->context.get()->SetCpuRegister(id, val); + } else if (id == CPSR_REGISTER) { + return thread->context.get()->SetCpsr(val); + } +} + +static u64 FpuRead(std::size_t id, Kernel::Thread* thread = nullptr) { + if (!thread) { + return 0; + } + + if (id >= D0_REGISTER && id < FPSCR_REGISTER) { + u64 ret = thread->context.get()->GetFpuRegister(2 * (id - D0_REGISTER)); + ret |= static_cast(thread->context.get()->GetFpuRegister(2 * (id - D0_REGISTER) + 1)) + << 32; + return ret; + } else if (id == FPSCR_REGISTER) { + return thread->context.get()->GetFpscr(); + } else { + return 0; + } +} + +static void FpuWrite(std::size_t id, u64 val, Kernel::Thread* thread = nullptr) { + if (!thread) { + return; + } + + if (id >= D0_REGISTER && id < FPSCR_REGISTER) { + thread->context.get()->SetFpuRegister(2 * (id - D0_REGISTER), (u32)val); + thread->context.get()->SetFpuRegister(2 * (id - D0_REGISTER) + 1, val >> 32); + } else if (id == FPSCR_REGISTER) { + return thread->context.get()->SetFpscr(static_cast(val)); + } +} + /** * Turns hex string character into the equivalent byte. * @@ -178,7 +251,7 @@ static u8 NibbleToHex(u8 n) { if (n < 0xA) { return '0' + n; } else { - return 'A' + n - 0xA; + return 'a' + n - 0xA; } } @@ -255,6 +328,35 @@ static u32 GdbHexToInt(const u8* src) { return output; } +/** + * Convert a u64 into a gdb-formatted hex string. + * + * @param dest Pointer to buffer to store output hex string characters. + * @param v Value to convert. + */ +static void LongToGdbHex(u8* dest, u64 v) { + for (int i = 0; i < 16; i += 2) { + dest[i + 1] = NibbleToHex(static_cast(v >> (4 * i))); + dest[i] = NibbleToHex(static_cast(v >> (4 * (i + 1)))); + } +} + +/** + * 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) { + output = (output << 4) | HexCharToValue(src[15 - i - 1]); + output = (output << 4) | HexCharToValue(src[15 - i]); + } + + return output; +} + /// Read a byte from the gdb client. static u8 ReadByte() { u8 c; @@ -303,6 +405,8 @@ static void RemoveBreakpoint(BreakpointType type, PAddr addr) { if (bp != p.end()) { LOG_DEBUG(Debug_GDBStub, "gdb: removed a breakpoint: {:08x} bytes at {:08x} of type {}\n", bp->second.len, bp->second.addr, static_cast(type)); + Memory::WriteBlock(bp->second.addr, bp->second.inst.data(), bp->second.inst.size()); + Core::CPU().ClearInstructionCache(); p.erase(addr); } } @@ -419,10 +523,32 @@ static void HandleQuery() { SendReply("T0"); } else if (strncmp(query, "Supported", strlen("Supported")) == 0) { // PacketSize needs to be large enough for target xml - SendReply("PacketSize=800;qXfer:features:read+"); + SendReply("PacketSize=2000;qXfer:features:read+;qXfer:threads:read+"); } else if (strncmp(query, "Xfer:features:read:target.xml:", strlen("Xfer:features:read:target.xml:")) == 0) { SendReply(target_xml); + } else if (strncmp(query, "fThreadInfo", strlen("fThreadInfo")) == 0) { + std::string val = "m"; + const auto& threads = Kernel::GetThreadList(); + for (const auto& thread : threads) { + val += fmt::format("{:x}", thread->GetThreadId()); + val += ","; + } + val.pop_back(); + SendReply(val.c_str()); + } else if (strncmp(query, "sThreadInfo", strlen("sThreadInfo")) == 0) { + SendReply("l"); + } else if (strncmp(query, "Xfer:threads:read", strlen("Xfer:threads:read")) == 0) { + std::string buffer; + buffer += "l"; + buffer += ""; + const auto& threads = Kernel::GetThreadList(); + for (const auto& thread : threads) { + buffer += fmt::format(R"*()*", + thread->GetThreadId(), thread->GetThreadId()); + } + buffer += ""; + SendReply(buffer.c_str()); } else { SendReply(""); } @@ -430,11 +556,34 @@ static void HandleQuery() { /// Handle set thread command from gdb client. static void HandleSetThread() { - if (memcmp(command_buffer, "Hg0", 3) == 0 || memcmp(command_buffer, "Hc-1", 4) == 0 || - memcmp(command_buffer, "Hc0", 4) == 0 || memcmp(command_buffer, "Hc1", 4) == 0) { - return SendReply("OK"); + int thread_id = -1; + if (command_buffer[2] != '-') { + thread_id = static_cast(HexToInt(command_buffer + 2, command_length - 2)); } + if (thread_id >= 1) { + current_thread = FindThreadById(thread_id); + } + if (!current_thread) { + thread_id = 1; + current_thread = FindThreadById(thread_id); + } + if (current_thread) { + SendReply("OK"); + return; + } + SendReply("E01"); +} +/// Handle thread alive command from gdb client. +static void HandleThreadAlive() { + int thread_id = static_cast(HexToInt(command_buffer + 1, command_length - 1)); + if (thread_id == 0) { + thread_id = 1; + } + if (FindThreadById(thread_id)) { + SendReply("OK"); + return; + } SendReply("E01"); } @@ -443,16 +592,31 @@ static void HandleSetThread() { * * @param signal Signal to be sent to client. */ -static void SendSignal(u32 signal) { +static void SendSignal(Kernel::Thread* thread, u32 signal, bool full = true) { if (gdbserver_socket == -1) { return; } latest_signal = signal; - std::string buffer = - Common::StringFromFormat("T%02x%02x:%08x;%02x:%08x;", latest_signal, 15, - htonl(Core::CPU().GetPC()), 13, htonl(Core::CPU().GetReg(13))); + if (!thread) { + full = false; + } + + std::string buffer; + if (full) { + buffer = Common::StringFromFormat("T%02x%02x:%08x;%02x:%08x;%02x:%08x", latest_signal, + PC_REGISTER, htonl(Core::CPU().GetPC()), SP_REGISTER, + htonl(Core::CPU().GetReg(SP_REGISTER)), LR_REGISTER, + htonl(Core::CPU().GetReg(LR_REGISTER))); + } else { + buffer = Common::StringFromFormat("T%02x", latest_signal); + } + + if (thread) { + buffer += Common::StringFromFormat(";thread:%x;", thread->GetThreadId()); + } + LOG_DEBUG(Debug_GDBStub, "Response: {}", buffer); SendReply(buffer.c_str()); } @@ -469,7 +633,7 @@ static void ReadCommand() { } else if (c == 0x03) { LOG_INFO(Debug_GDBStub, "gdb: found break command\n"); halt_loop = true; - SendSignal(SIGTRAP); + SendSignal(current_thread, SIGTRAP); return; } else if (c != GDB_STUB_START) { LOG_DEBUG(Debug_GDBStub, "gdb: read invalid byte {:02x}\n", c); @@ -539,17 +703,14 @@ static void ReadRegister() { id |= HexCharToValue(command_buffer[2]); } - if (id <= R15_REGISTER) { - IntToGdbHex(reply, Core::CPU().GetReg(id)); + if (id <= PC_REGISTER) { + IntToGdbHex(reply, RegRead(id, current_thread)); } else if (id == CPSR_REGISTER) { - IntToGdbHex(reply, Core::CPU().GetCPSR()); - } else if (id > CPSR_REGISTER && id < FPSCR_REGISTER) { - IntToGdbHex(reply, Core::CPU().GetVFPReg( - id - CPSR_REGISTER - - 1)); // VFP registers should start at 26, so one after CSPR_REGISTER + IntToGdbHex(reply, RegRead(id, current_thread)); + } else if (id >= D0_REGISTER && id < FPSCR_REGISTER) { + LongToGdbHex(reply, FpuRead(id, current_thread)); } else if (id == FPSCR_REGISTER) { - IntToGdbHex(reply, Core::CPU().GetVFPSystemReg(VFP_FPSCR)); // Get FPSCR - IntToGdbHex(reply + 8, 0); + IntToGdbHex(reply, static_cast(FpuRead(id, current_thread))); } else { return SendReply("E01"); } @@ -564,23 +725,23 @@ static void ReadRegisters() { u8* bufptr = buffer; - for (u32 reg = 0; reg <= R15_REGISTER; reg++) { - IntToGdbHex(bufptr + reg * CHAR_BIT, Core::CPU().GetReg(reg)); + for (u32 reg = 0; reg <= PC_REGISTER; reg++) { + IntToGdbHex(bufptr + reg * 8, RegRead(reg, current_thread)); } - bufptr += (16 * CHAR_BIT); + bufptr += 16 * 8; - IntToGdbHex(bufptr, Core::CPU().GetCPSR()); + IntToGdbHex(bufptr, RegRead(CPSR_REGISTER, current_thread)); - bufptr += CHAR_BIT; + bufptr += 8; - for (u32 reg = 0; reg <= 31; reg++) { - IntToGdbHex(bufptr + reg * CHAR_BIT, Core::CPU().GetVFPReg(reg)); + for (u32 reg = D0_REGISTER; reg < FPSCR_REGISTER; reg++) { + LongToGdbHex(bufptr + reg * 16, FpuRead(reg, current_thread)); } - bufptr += (32 * CHAR_BIT); + bufptr += 16 * 16; - IntToGdbHex(bufptr, Core::CPU().GetVFPSystemReg(VFP_FPSCR)); + IntToGdbHex(bufptr, static_cast(FpuRead(FPSCR_REGISTER, current_thread))); SendReply(reinterpret_cast(buffer)); } @@ -596,18 +757,20 @@ static void WriteRegister() { id |= HexCharToValue(command_buffer[2]); } - if (id <= R15_REGISTER) { - Core::CPU().SetReg(id, GdbHexToInt(buffer_ptr)); + if (id <= PC_REGISTER) { + RegWrite(id, GdbHexToInt(buffer_ptr), current_thread); } else if (id == CPSR_REGISTER) { - Core::CPU().SetCPSR(GdbHexToInt(buffer_ptr)); - } else if (id > CPSR_REGISTER && id < FPSCR_REGISTER) { - Core::CPU().SetVFPReg(id - CPSR_REGISTER - 1, GdbHexToInt(buffer_ptr)); + RegWrite(id, GdbHexToInt(buffer_ptr), current_thread); + } else if (id >= D0_REGISTER && id < FPSCR_REGISTER) { + FpuWrite(id, GdbHexToLong(buffer_ptr), current_thread); } else if (id == FPSCR_REGISTER) { - Core::CPU().SetVFPSystemReg(VFP_FPSCR, GdbHexToInt(buffer_ptr)); + FpuWrite(id, GdbHexToInt(buffer_ptr), current_thread); } else { return SendReply("E01"); } + Core::CPU().LoadContext(current_thread->context); + SendReply("OK"); } @@ -619,23 +782,25 @@ static void WriteRegisters() { return SendReply("E01"); for (u32 i = 0, reg = 0; reg <= FPSCR_REGISTER; i++, reg++) { - if (reg <= R15_REGISTER) { - Core::CPU().SetReg(reg, GdbHexToInt(buffer_ptr + i * CHAR_BIT)); + if (reg <= PC_REGISTER) { + RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8)); } else if (reg == CPSR_REGISTER) { - Core::CPU().SetCPSR(GdbHexToInt(buffer_ptr + i * CHAR_BIT)); + RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8)); } else if (reg == CPSR_REGISTER - 1) { // Dummy FPA register, ignore } else if (reg < CPSR_REGISTER) { // Dummy FPA registers, ignore i += 2; - } else if (reg > CPSR_REGISTER && reg < FPSCR_REGISTER) { - Core::CPU().SetVFPReg(reg - CPSR_REGISTER - 1, GdbHexToInt(buffer_ptr + i * CHAR_BIT)); + } else if (reg >= D0_REGISTER && reg < FPSCR_REGISTER) { + FpuWrite(reg, GdbHexToLong(buffer_ptr + i * 16)); i++; // Skip padding } else if (reg == FPSCR_REGISTER) { - Core::CPU().SetVFPSystemReg(VFP_FPSCR, GdbHexToInt(buffer_ptr + i * CHAR_BIT)); + FpuWrite(reg, GdbHexToInt(buffer_ptr + i * 8)); } } + Core::CPU().LoadContext(current_thread->context); + SendReply("OK"); } @@ -687,24 +852,26 @@ static void WriteMemory() { GdbHexToMem(data.data(), len_pos + 1, len); Memory::WriteBlock(addr, data.data(), len); + Core::CPU().ClearInstructionCache(); SendReply("OK"); } void Break(bool is_memory_break) { - if (!halt_loop) { - halt_loop = true; - SendSignal(SIGTRAP); - } + send_trap = true; memory_break = is_memory_break; } /// Tell the CPU that it should perform a single step. static void Step() { + if (command_length > 1) { + RegWrite(PC_REGISTER, GdbHexToInt(command_buffer + 1), current_thread); + Core::CPU().LoadContext(current_thread->context); + } step_loop = true; halt_loop = true; - step_break = true; - SendSignal(SIGTRAP); + send_trap = true; + Core::CPU().ClearInstructionCache(); } bool IsMemoryBreak() { @@ -718,9 +885,9 @@ bool IsMemoryBreak() { /// Tell the CPU to continue executing. static void Continue() { memory_break = false; - step_break = false; step_loop = false; halt_loop = false; + Core::CPU().ClearInstructionCache(); } /** @@ -737,6 +904,10 @@ static bool CommitBreakpoint(BreakpointType type, PAddr addr, u32 len) { breakpoint.active = true; breakpoint.addr = addr; breakpoint.len = len; + Memory::ReadBlock(addr, breakpoint.inst.data(), breakpoint.inst.size()); + static constexpr std::array btrap{0x70, 0x00, 0x20, 0xe1}; + Memory::WriteBlock(addr, btrap.data(), btrap.size()); + Core::CPU().ClearInstructionCache(); p.insert({addr, breakpoint}); LOG_DEBUG(Debug_GDBStub, "gdb: added {} breakpoint: {:08x} bytes at {:08x}\n", @@ -857,7 +1028,7 @@ void HandlePacket() { HandleSetThread(); break; case '?': - SendSignal(latest_signal); + SendSignal(current_thread, latest_signal); break; case 'k': Shutdown(); @@ -894,6 +1065,9 @@ void HandlePacket() { case 'Z': AddBreakpoint(); break; + case 'T': + HandleThreadAlive(); + break; default: SendReply(""); break; @@ -1038,4 +1212,15 @@ bool GetCpuStepFlag() { void SetCpuStepFlag(bool is_step) { step_loop = is_step; } + +void SendTrap(Kernel::Thread* thread, int trap) { + if (send_trap) { + if (!halt_loop || current_thread == thread) { + current_thread = thread; + SendSignal(thread, trap); + } + halt_loop = true; + send_trap = false; + } +} }; // namespace GDBStub diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h index da00ba5df..7f19fd33f 100644 --- a/src/core/gdbstub/gdbstub.h +++ b/src/core/gdbstub/gdbstub.h @@ -7,6 +7,7 @@ #pragma once #include "common/common_types.h" +#include "core/hle/kernel/thread.h" namespace GDBStub { @@ -91,4 +92,12 @@ bool GetCpuStepFlag(); * @param is_step */ void SetCpuStepFlag(bool is_step); + +/** + * Send trap signal from thread back to the gdbstub server. + * + * @param thread Sending thread. + * @param trap Trap no. + */ +void SendTrap(Kernel::Thread* thread, int trap); } // namespace GDBStub From a6ecb3c9135048b53b43ff17dde850095731315d Mon Sep 17 00:00:00 2001 From: Jarek Syrylak Date: Thu, 16 Aug 2018 16:24:16 +0100 Subject: [PATCH 2/3] Fixed as per PR feedback. --- src/core/arm/skyeye_common/armstate.cpp | 17 +++++++++- src/core/arm/skyeye_common/armstate.h | 16 +--------- src/core/gdbstub/gdbstub.cpp | 42 ++++++++++++------------- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/core/arm/skyeye_common/armstate.cpp b/src/core/arm/skyeye_common/armstate.cpp index dd388d38a..8a0e43a21 100644 --- a/src/core/arm/skyeye_common/armstate.cpp +++ b/src/core/arm/skyeye_common/armstate.cpp @@ -7,7 +7,7 @@ #include "common/swap.h" #include "core/arm/skyeye_common/armstate.h" #include "core/arm/skyeye_common/vfp/vfp.h" -#include "core/gdbstub/gdbstub.h" +#include "core/core.h" #include "core/memory.h" ARMul_State::ARMul_State(PrivilegeMode initial_mode) { @@ -595,3 +595,18 @@ void ARMul_State::WriteCP15Register(u32 value, u32 crn, u32 opcode_1, u32 crm, u CP15[CP15_THREAD_UPRW] = value; } } + +void ARMul_State::ServeBreak() { + if (GDBStub::IsServerEnabled()) { + if (last_bkpt_hit) { + Reg[15] = last_bkpt.address; + } + Kernel::Thread* thread = Kernel::GetCurrentThread(); + Core::CPU().SaveContext(thread->context); + if (last_bkpt_hit || GDBStub::GetCpuStepFlag()) { + last_bkpt_hit = false; + GDBStub::Break(); + GDBStub::SendTrap(thread, 5); + } + } +} diff --git a/src/core/arm/skyeye_common/armstate.h b/src/core/arm/skyeye_common/armstate.h index cca754b31..2f99b738b 100644 --- a/src/core/arm/skyeye_common/armstate.h +++ b/src/core/arm/skyeye_common/armstate.h @@ -21,7 +21,6 @@ #include #include "common/common_types.h" #include "core/arm/skyeye_common/arm_regformat.h" -#include "core/core.h" #include "core/gdbstub/gdbstub.h" // Signal levels @@ -196,20 +195,7 @@ public: last_bkpt_hit = true; } - void ServeBreak() { - if (GDBStub::IsServerEnabled()) { - if (last_bkpt_hit) { - Reg[15] = last_bkpt.address; - } - Kernel::Thread* thread = Kernel::GetCurrentThread(); - Core::CPU().SaveContext(thread->context); - if (last_bkpt_hit || GDBStub::GetCpuStepFlag()) { - last_bkpt_hit = false; - GDBStub::Break(); - GDBStub::SendTrap(thread, 5); - } - } - } + void ServeBreak(); std::array Reg{}; // The current register file std::array Reg_usr{}; diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index ddf306207..cc8fc9ffb 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -128,7 +128,7 @@ static u32 command_length; static u32 latest_signal = 0; static bool memory_break = false; -Kernel::Thread* current_thread = nullptr; +static Kernel::Thread* current_thread = nullptr; // Binding to a port within the reserved ports range (0-1023) requires root permissions, // so default to a port outside of that range. @@ -173,9 +173,9 @@ static u32 RegRead(std::size_t id, Kernel::Thread* thread = nullptr) { } if (id <= PC_REGISTER) { - return thread->context.get()->GetCpuRegister(id); + return thread->context->GetCpuRegister(id); } else if (id == CPSR_REGISTER) { - return thread->context.get()->GetCpsr(); + return thread->context->GetCpsr(); } else { return 0; } @@ -187,9 +187,9 @@ static void RegWrite(std::size_t id, u32 val, Kernel::Thread* thread = nullptr) } if (id <= PC_REGISTER) { - return thread->context.get()->SetCpuRegister(id, val); + return thread->context->SetCpuRegister(id, val); } else if (id == CPSR_REGISTER) { - return thread->context.get()->SetCpsr(val); + return thread->context->SetCpsr(val); } } @@ -199,12 +199,11 @@ static u64 FpuRead(std::size_t id, Kernel::Thread* thread = nullptr) { } if (id >= D0_REGISTER && id < FPSCR_REGISTER) { - u64 ret = thread->context.get()->GetFpuRegister(2 * (id - D0_REGISTER)); - ret |= static_cast(thread->context.get()->GetFpuRegister(2 * (id - D0_REGISTER) + 1)) - << 32; + u64 ret = thread->context->GetFpuRegister(2 * (id - D0_REGISTER)); + ret |= static_cast(thread->context->GetFpuRegister(2 * (id - D0_REGISTER) + 1)) << 32; return ret; } else if (id == FPSCR_REGISTER) { - return thread->context.get()->GetFpscr(); + return thread->context->GetFpscr(); } else { return 0; } @@ -216,10 +215,10 @@ static void FpuWrite(std::size_t id, u64 val, Kernel::Thread* thread = nullptr) } if (id >= D0_REGISTER && id < FPSCR_REGISTER) { - thread->context.get()->SetFpuRegister(2 * (id - D0_REGISTER), (u32)val); - thread->context.get()->SetFpuRegister(2 * (id - D0_REGISTER) + 1, val >> 32); + thread->context->SetFpuRegister(2 * (id - D0_REGISTER), (u32)val); + thread->context->SetFpuRegister(2 * (id - D0_REGISTER) + 1, val >> 32); } else if (id == FPSCR_REGISTER) { - return thread->context.get()->SetFpscr(static_cast(val)); + return thread->context->SetFpscr(static_cast(val)); } } @@ -531,8 +530,7 @@ static void HandleQuery() { std::string val = "m"; const auto& threads = Kernel::GetThreadList(); for (const auto& thread : threads) { - val += fmt::format("{:x}", thread->GetThreadId()); - val += ","; + val += fmt::format("{:x},", thread->GetThreadId()); } val.pop_back(); SendReply(val.c_str()); @@ -1214,13 +1212,15 @@ void SetCpuStepFlag(bool is_step) { } void SendTrap(Kernel::Thread* thread, int trap) { - if (send_trap) { - if (!halt_loop || current_thread == thread) { - current_thread = thread; - SendSignal(thread, trap); - } - halt_loop = true; - send_trap = false; + if (!send_trap) { + return; } + + if (!halt_loop || current_thread == thread) { + current_thread = thread; + SendSignal(thread, trap); + } + halt_loop = true; + send_trap = false; } }; // namespace GDBStub From 039fb95f8005596708129638a21d206f10960a91 Mon Sep 17 00:00:00 2001 From: Jarek Syrylak Date: Thu, 16 Aug 2018 19:44:31 +0100 Subject: [PATCH 3/3] More fixes as per PR feedback. --- src/core/arm/dyncom/arm_dyncom.cpp | 2 +- src/core/arm/skyeye_common/armstate.cpp | 24 +++++++++++++----------- src/core/gdbstub/gdbstub.cpp | 4 ++-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/core/arm/dyncom/arm_dyncom.cpp b/src/core/arm/dyncom/arm_dyncom.cpp index b984a62cd..a46a71682 100644 --- a/src/core/arm/dyncom/arm_dyncom.cpp +++ b/src/core/arm/dyncom/arm_dyncom.cpp @@ -147,7 +147,7 @@ void ARM_DynCom::ExecuteInstructions(u64 num_instructions) { state->NumInstrsToExecute = num_instructions; unsigned ticks_executed = InterpreterMainLoop(state.get()); CoreTiming::AddTicks(ticks_executed); - state.get()->ServeBreak(); + state->ServeBreak(); } std::unique_ptr ARM_DynCom::NewContext() const { diff --git a/src/core/arm/skyeye_common/armstate.cpp b/src/core/arm/skyeye_common/armstate.cpp index 8a0e43a21..88ad7f533 100644 --- a/src/core/arm/skyeye_common/armstate.cpp +++ b/src/core/arm/skyeye_common/armstate.cpp @@ -597,16 +597,18 @@ void ARMul_State::WriteCP15Register(u32 value, u32 crn, u32 opcode_1, u32 crm, u } void ARMul_State::ServeBreak() { - if (GDBStub::IsServerEnabled()) { - if (last_bkpt_hit) { - Reg[15] = last_bkpt.address; - } - Kernel::Thread* thread = Kernel::GetCurrentThread(); - Core::CPU().SaveContext(thread->context); - if (last_bkpt_hit || GDBStub::GetCpuStepFlag()) { - last_bkpt_hit = false; - GDBStub::Break(); - GDBStub::SendTrap(thread, 5); - } + if (!GDBStub::IsServerEnabled()) { + return; + } + + if (last_bkpt_hit) { + Reg[15] = last_bkpt.address; + } + Kernel::Thread* thread = Kernel::GetCurrentThread(); + Core::CPU().SaveContext(thread->context); + if (last_bkpt_hit || GDBStub::GetCpuStepFlag()) { + last_bkpt_hit = false; + GDBStub::Break(); + GDBStub::SendTrap(thread, 5); } } diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index cc8fc9ffb..007f0688c 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -215,8 +215,8 @@ static void FpuWrite(std::size_t id, u64 val, Kernel::Thread* thread = nullptr) } if (id >= D0_REGISTER && id < FPSCR_REGISTER) { - thread->context->SetFpuRegister(2 * (id - D0_REGISTER), (u32)val); - thread->context->SetFpuRegister(2 * (id - D0_REGISTER) + 1, val >> 32); + thread->context->SetFpuRegister(2 * (id - D0_REGISTER), static_cast(val)); + thread->context->SetFpuRegister(2 * (id - D0_REGISTER) + 1, static_cast(val >> 32)); } else if (id == FPSCR_REGISTER) { return thread->context->SetFpscr(static_cast(val)); }