From ca96f8db4e13e857be2f90e147c663e1277fb0e6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 5 Aug 2018 15:55:57 -0400 Subject: [PATCH 1/3] gdbstub: Replace PAddr alias with VAddr In all cases, a virtual address is being passed in, not a physical one. --- src/core/gdbstub/gdbstub.cpp | 20 ++++++++++---------- src/core/gdbstub/gdbstub.h | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 75f6b82353..ac53ba7520 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -171,7 +171,7 @@ WSADATA InitData; struct Breakpoint { bool active; - PAddr addr; + VAddr addr; u64 len; }; @@ -181,13 +181,13 @@ static std::map breakpoints_write; struct Module { std::string name; - PAddr beg; - PAddr end; + VAddr beg; + VAddr end; }; static std::vector modules; -void RegisterModule(std::string name, PAddr beg, PAddr end, bool add_elf_ext) { +void RegisterModule(std::string name, VAddr beg, VAddr end, bool add_elf_ext) { Module module; if (add_elf_ext) { Common::SplitPath(name, nullptr, &module.name, nullptr); @@ -441,7 +441,7 @@ static std::map& GetBreakpointList(BreakpointType type) { * @param type Type of breakpoint. * @param addr Address of breakpoint. */ -static void RemoveBreakpoint(BreakpointType type, PAddr addr) { +static void RemoveBreakpoint(BreakpointType type, VAddr addr) { std::map& p = GetBreakpointList(type); auto bp = p.find(static_cast(addr)); @@ -452,7 +452,7 @@ static void RemoveBreakpoint(BreakpointType type, PAddr addr) { } } -BreakpointAddress GetNextBreakpointFromAddress(PAddr addr, BreakpointType type) { +BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, BreakpointType type) { std::map& p = GetBreakpointList(type); auto next_breakpoint = p.lower_bound(static_cast(addr)); BreakpointAddress breakpoint; @@ -468,7 +468,7 @@ BreakpointAddress GetNextBreakpointFromAddress(PAddr addr, BreakpointType type) return breakpoint; } -bool CheckBreakpoint(PAddr addr, BreakpointType type) { +bool CheckBreakpoint(VAddr addr, BreakpointType type) { if (!IsConnected()) { return false; } @@ -975,7 +975,7 @@ static void Continue() { * @param addr Address of breakpoint. * @param len Length of breakpoint. */ -static bool CommitBreakpoint(BreakpointType type, PAddr addr, u64 len) { +static bool CommitBreakpoint(BreakpointType type, VAddr addr, u64 len) { std::map& p = GetBreakpointList(type); Breakpoint breakpoint; @@ -1015,7 +1015,7 @@ static void AddBreakpoint() { auto start_offset = command_buffer + 3; auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); - PAddr addr = HexToLong(start_offset, static_cast(addr_pos - start_offset)); + VAddr addr = HexToLong(start_offset, static_cast(addr_pos - start_offset)); start_offset = addr_pos + 1; u64 len = @@ -1064,7 +1064,7 @@ static void RemoveBreakpoint() { auto start_offset = command_buffer + 3; auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); - PAddr addr = HexToLong(start_offset, static_cast(addr_pos - start_offset)); + VAddr addr = HexToLong(start_offset, static_cast(addr_pos - start_offset)); if (type == BreakpointType::Access) { // Access is made up of Read and Write types, so add both breakpoints diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h index a6b50c26cf..5a36524b2e 100644 --- a/src/core/gdbstub/gdbstub.h +++ b/src/core/gdbstub/gdbstub.h @@ -22,7 +22,7 @@ enum class BreakpointType { }; struct BreakpointAddress { - PAddr address; + VAddr address; BreakpointType type; }; @@ -53,7 +53,7 @@ bool IsServerEnabled(); bool IsConnected(); /// Register module. -void RegisterModule(std::string name, PAddr beg, PAddr end, bool add_elf_ext = true); +void RegisterModule(std::string name, VAddr beg, VAddr end, bool add_elf_ext = true); /** * Signal to the gdbstub server that it should halt CPU execution. @@ -74,7 +74,7 @@ void HandlePacket(); * @param addr Address to search from. * @param type Type of breakpoint. */ -BreakpointAddress GetNextBreakpointFromAddress(PAddr addr, GDBStub::BreakpointType type); +BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, GDBStub::BreakpointType type); /** * Check if a breakpoint of the specified type exists at the given address. @@ -82,7 +82,7 @@ BreakpointAddress GetNextBreakpointFromAddress(PAddr addr, GDBStub::BreakpointTy * @param addr Address of breakpoint. * @param type Type of breakpoint. */ -bool CheckBreakpoint(PAddr addr, GDBStub::BreakpointType type); +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(); From 89c076b4b10a79244514e31466802b5b6fda07a4 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 5 Aug 2018 15:59:53 -0400 Subject: [PATCH 2/3] gdbstub: Move all file-static variables into the GDBStub namespace Keeps everything under the same namespace. While we're at it, enclose them all within an inner anonymous namespace. --- src/core/gdbstub/gdbstub.cpp | 71 ++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index ac53ba7520..22ea53e22c 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -41,40 +41,42 @@ #include "core/loader/loader.h" #include "core/memory.h" -const int GDB_BUFFER_SIZE = 10000; +namespace GDBStub { +namespace { +constexpr int GDB_BUFFER_SIZE = 10000; -const char GDB_STUB_START = '$'; -const char GDB_STUB_END = '#'; -const char GDB_STUB_ACK = '+'; -const char GDB_STUB_NACK = '-'; +constexpr char GDB_STUB_START = '$'; +constexpr char GDB_STUB_END = '#'; +constexpr char GDB_STUB_ACK = '+'; +constexpr char GDB_STUB_NACK = '-'; #ifndef SIGTRAP -const u32 SIGTRAP = 5; +constexpr u32 SIGTRAP = 5; #endif #ifndef SIGTERM -const u32 SIGTERM = 15; +constexpr u32 SIGTERM = 15; #endif #ifndef MSG_WAITALL -const u32 MSG_WAITALL = 8; +constexpr u32 MSG_WAITALL = 8; #endif -const u32 LR_REGISTER = 30; -const u32 SP_REGISTER = 31; -const u32 PC_REGISTER = 32; -const u32 CPSR_REGISTER = 33; -const u32 UC_ARM64_REG_Q0 = 34; -const u32 FPSCR_REGISTER = 66; +constexpr u32 LR_REGISTER = 30; +constexpr u32 SP_REGISTER = 31; +constexpr u32 PC_REGISTER = 32; +constexpr u32 CPSR_REGISTER = 33; +constexpr u32 UC_ARM64_REG_Q0 = 34; +constexpr u32 FPSCR_REGISTER = 66; // TODO/WiP - Used while working on support for FPU -const u32 TODO_DUMMY_REG_997 = 997; -const u32 TODO_DUMMY_REG_998 = 998; +constexpr u32 TODO_DUMMY_REG_997 = 997; +constexpr u32 TODO_DUMMY_REG_998 = 998; // For sample XML files see the GDB source /gdb/features // GDB also wants the l character at the start // This XML defines what the registers are for this specific ARM device -static const char* target_xml = +constexpr char target_xml[] = R"(l @@ -140,30 +142,28 @@ static const char* target_xml = )"; -namespace GDBStub { +int gdbserver_socket = -1; -static int gdbserver_socket = -1; +u8 command_buffer[GDB_BUFFER_SIZE]; +u32 command_length; -static u8 command_buffer[GDB_BUFFER_SIZE]; -static u32 command_length; +u32 latest_signal = 0; +bool memory_break = false; -static u32 latest_signal = 0; -static bool memory_break = false; - -static Kernel::Thread* current_thread = nullptr; -static u32 current_core = 0; +Kernel::Thread* current_thread = nullptr; +u32 current_core = 0; // 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; +u16 gdbstub_port = 24689; -static bool halt_loop = true; -static bool step_loop = false; -static bool send_trap = false; +bool halt_loop = true; +bool step_loop = false; +bool send_trap = false; // If set to false, the server will never be started and no // gdbstub-related functions will be executed. -static std::atomic server_enabled(false); +std::atomic server_enabled(false); #ifdef _WIN32 WSADATA InitData; @@ -175,9 +175,9 @@ struct Breakpoint { u64 len; }; -static std::map breakpoints_execute; -static std::map breakpoints_read; -static std::map breakpoints_write; +std::map breakpoints_execute; +std::map breakpoints_read; +std::map breakpoints_write; struct Module { std::string name; @@ -185,7 +185,8 @@ struct Module { VAddr end; }; -static std::vector modules; +std::vector modules; +} // Anonymous namespace void RegisterModule(std::string name, VAddr beg, VAddr end, bool add_elf_ext) { Module module; From 00f7e584cef1abbcec3f52c81275cce1e4c8f3d3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 5 Aug 2018 16:11:29 -0400 Subject: [PATCH 3/3] gdbstub: Use type alias for breakpoint maps Rather than having to type out the full std::map type signature, we can just use a straightforward alias. While we're at it, rename GetBreakpointList to GetBreakpointMap, which makes the name more accurate. We can also get rid of unnecessary u64 static_casts, since VAddr is an alias for a u64. --- src/core/gdbstub/gdbstub.cpp | 79 +++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 22ea53e22c..884e64e994 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -175,9 +175,10 @@ struct Breakpoint { u64 len; }; -std::map breakpoints_execute; -std::map breakpoints_read; -std::map breakpoints_write; +using BreakpointMap = std::map; +BreakpointMap breakpoints_execute; +BreakpointMap breakpoints_read; +BreakpointMap breakpoints_write; struct Module { std::string name; @@ -419,11 +420,11 @@ static u8 CalculateChecksum(const u8* buffer, size_t length) { } /** - * Get the list of breakpoints for a given breakpoint type. + * Get the map of breakpoints for a given breakpoint type. * - * @param type Type of breakpoint list. + * @param type Type of breakpoint map. */ -static std::map& GetBreakpointList(BreakpointType type) { +static BreakpointMap& GetBreakpointMap(BreakpointType type) { switch (type) { case BreakpointType::Execute: return breakpoints_execute; @@ -443,19 +444,21 @@ static std::map& GetBreakpointList(BreakpointType type) { * @param addr Address of breakpoint. */ static void RemoveBreakpoint(BreakpointType type, VAddr addr) { - std::map& p = GetBreakpointList(type); + BreakpointMap& p = GetBreakpointMap(type); - auto bp = p.find(static_cast(addr)); - if (bp != p.end()) { - LOG_DEBUG(Debug_GDBStub, "gdb: removed a breakpoint: {:016X} bytes at {:016X} of type {}", - bp->second.len, bp->second.addr, static_cast(type)); - p.erase(static_cast(addr)); + const auto bp = p.find(addr); + if (bp == p.end()) { + return; } + + LOG_DEBUG(Debug_GDBStub, "gdb: removed a breakpoint: {:016X} bytes at {:016X} of type {}", + bp->second.len, bp->second.addr, static_cast(type)); + p.erase(addr); } BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, BreakpointType type) { - std::map& p = GetBreakpointList(type); - auto next_breakpoint = p.lower_bound(static_cast(addr)); + const BreakpointMap& p = GetBreakpointMap(type); + const auto next_breakpoint = p.lower_bound(addr); BreakpointAddress breakpoint; if (next_breakpoint != p.end()) { @@ -474,31 +477,33 @@ bool CheckBreakpoint(VAddr addr, BreakpointType type) { return false; } - std::map& p = GetBreakpointList(type); + const BreakpointMap& p = GetBreakpointMap(type); + const auto bp = p.find(addr); - auto bp = p.find(static_cast(addr)); - if (bp != p.end()) { - u64 len = bp->second.len; + if (bp == p.end()) { + return false; + } - // IDA Pro defaults to 4-byte breakpoints for all non-hardware breakpoints - // no matter if it's a 4-byte or 2-byte instruction. When you execute a - // Thumb instruction with a 4-byte breakpoint set, it will set a breakpoint on - // two instructions instead of the single instruction you placed the breakpoint - // on. So, as a way to make sure that execution breakpoints are only breaking - // on the instruction that was specified, set the length of an execution - // breakpoint to 1. This should be fine since the CPU should never begin executing - // an instruction anywhere except the beginning of the instruction. - if (type == BreakpointType::Execute) { - len = 1; - } + u64 len = bp->second.len; - if (bp->second.active && (addr >= bp->second.addr && addr < bp->second.addr + len)) { - LOG_DEBUG(Debug_GDBStub, - "Found breakpoint type {} @ {:016X}, range: {:016X}" - " - {:016X} ({:X} bytes)", - static_cast(type), addr, bp->second.addr, bp->second.addr + len, len); - return true; - } + // IDA Pro defaults to 4-byte breakpoints for all non-hardware breakpoints + // no matter if it's a 4-byte or 2-byte instruction. When you execute a + // Thumb instruction with a 4-byte breakpoint set, it will set a breakpoint on + // two instructions instead of the single instruction you placed the breakpoint + // on. So, as a way to make sure that execution breakpoints are only breaking + // on the instruction that was specified, set the length of an execution + // breakpoint to 1. This should be fine since the CPU should never begin executing + // an instruction anywhere except the beginning of the instruction. + if (type == BreakpointType::Execute) { + len = 1; + } + + if (bp->second.active && (addr >= bp->second.addr && addr < bp->second.addr + len)) { + LOG_DEBUG(Debug_GDBStub, + "Found breakpoint type {} @ {:016X}, range: {:016X}" + " - {:016X} ({:X} bytes)", + static_cast(type), addr, bp->second.addr, bp->second.addr + len, len); + return true; } return false; @@ -977,7 +982,7 @@ static void Continue() { * @param len Length of breakpoint. */ static bool CommitBreakpoint(BreakpointType type, VAddr addr, u64 len) { - std::map& p = GetBreakpointList(type); + BreakpointMap& p = GetBreakpointMap(type); Breakpoint breakpoint; breakpoint.active = true;