From 07b171baffb6bca8be6799ef477713094ae8316f Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Wed, 29 Apr 2026 17:24:44 +0200 Subject: [PATCH 1/9] gdb: Fix register write from debugger --- src/core/gdbstub/gdbstub.cpp | 57 +++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 660168bc1..a2c4ff7ec 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -38,6 +38,9 @@ #include "core/hle/kernel/process.h" #include "core/memory.h" +// Uncomment to log all GDB traffic +// #define PRINT_GDB_TRAFFIC + namespace GDBStub { namespace { constexpr int GDB_BUFFER_SIZE = 10000; @@ -228,6 +231,13 @@ static void FpuWrite(std::size_t id, u64 val, Kernel::Thread* thread = nullptr) } } +// Clear instruction cache for all cores. +static void ClearAllInstructionCache() { + for (int i = 0; i < Core::GetNumCores(); i++) { + Core::GetCore(i).ClearInstructionCache(); + } +} + /** * Turns hex string character into the equivalent byte. * @@ -493,6 +503,15 @@ void SendReply(const char* reply) { std::memset(command_buffer, 0, sizeof(command_buffer)); command_length = static_cast(strlen(reply)); + +#ifdef PRINT_GDB_TRAFFIC + if (command_length > 0) { + LOG_INFO(Debug_GDBStub, "Req: {} {}", *reply, reply + 1); + } else { + LOG_INFO(Debug_GDBStub, "Req:"); + } +#endif + if (command_length + 4 > sizeof(command_buffer)) { LOG_ERROR(Debug_GDBStub, "command_buffer overflow in SendReply"); return; @@ -762,6 +781,17 @@ static void ReadRegisters() { SendReply(reinterpret_cast(buffer)); } +static void UpdateCPUThreadContext() { + u32 core_id = current_thread->core_id; + auto& system = Core::System::GetInstance(); + auto& thread_manager = system.Kernel().GetThreadManager(core_id); + if (thread_manager.GetCurrentThread() == current_thread) { + // Only update CPU context if current thread is active, + // otherwise it will be updated when the thread is selected + Core::GetCore(current_thread->core_id).LoadContext(current_thread->context); + } +} + /// Modify data of register specified by gdb client. static void WriteRegister() { const u8* buffer_ptr = command_buffer + 3; @@ -785,7 +815,7 @@ static void WriteRegister() { return SendReply("E01"); } - Core::GetRunningCore().LoadContext(current_thread->context); + UpdateCPUThreadContext(); SendReply("OK"); } @@ -799,23 +829,23 @@ static void WriteRegisters() { for (u32 i = 0, reg = 0; reg <= FPSCR_REGISTER; i++, reg++) { if (reg <= PC_REGISTER) { - RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8)); + RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8), current_thread); } else if (reg == CPSR_REGISTER) { - RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8)); + RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8), current_thread); } else if (reg == CPSR_REGISTER - 1) { // Dummy FPA register, ignore } else if (reg < CPSR_REGISTER) { // Dummy FPA registers, ignore i += 2; } else if (reg >= D0_REGISTER && reg < FPSCR_REGISTER) { - FpuWrite(reg, GdbHexToLong(buffer_ptr + i * 16)); + FpuWrite(reg, GdbHexToLong(buffer_ptr + i * 16), current_thread); i++; // Skip padding } else if (reg == FPSCR_REGISTER) { - FpuWrite(reg, GdbHexToInt(buffer_ptr + i * 8)); + FpuWrite(reg, GdbHexToInt(buffer_ptr + i * 8), current_thread); } } - Core::GetRunningCore().LoadContext(current_thread->context); + UpdateCPUThreadContext(); SendReply("OK"); } @@ -876,7 +906,7 @@ static void WriteMemory() { GdbHexToMem(data.data(), len_pos + 1, len); memory.WriteBlock(addr, data.data(), len); - Core::GetRunningCore().ClearInstructionCache(); + ClearAllInstructionCache(); SendReply("OK"); } @@ -890,12 +920,12 @@ void Break(bool is_memory_break) { static void Step() { if (command_length > 1) { RegWrite(PC_REGISTER, GdbHexToInt(command_buffer + 1), current_thread); - Core::GetRunningCore().LoadContext(current_thread->context); + UpdateCPUThreadContext(); } step_loop = true; halt_loop = true; send_trap = true; - Core::GetRunningCore().ClearInstructionCache(); + ClearAllInstructionCache(); } bool IsMemoryBreak() { @@ -911,7 +941,7 @@ static void Continue() { memory_break = false; step_loop = false; halt_loop = false; - Core::GetRunningCore().ClearInstructionCache(); + ClearAllInstructionCache(); } /** @@ -937,7 +967,7 @@ static bool CommitBreakpoint(BreakpointType type, VAddr addr, u32 len) { Core::System::GetInstance().Memory().WriteBlock( *Core::System::GetInstance().Kernel().GetCurrentProcess(), addr, btrap.data(), btrap.size()); - Core::GetRunningCore().ClearInstructionCache(); + ClearAllInstructionCache(); } p.insert({addr, breakpoint}); @@ -1057,6 +1087,11 @@ void HandlePacket(Core::System& system) { return; } +#ifdef PRINT_GDB_TRAFFIC + std::string cmd_str(command_buffer + 1, command_buffer + command_length); + LOG_INFO(Debug_GDBStub, "Res: {:c} {}", command_buffer[0], cmd_str); +#endif + LOG_DEBUG(Debug_GDBStub, "Packet: {0:d} ('{0:c}')", command_buffer[0]); switch (command_buffer[0]) { From e9540b2e39b0f59baa5320d24fa533e6d9f4f4ca Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Wed, 29 Apr 2026 17:47:47 +0200 Subject: [PATCH 2/9] gdb: Remove all stepping support and full CPU halt --- src/core/arm/skyeye_common/armstate.cpp | 2 +- src/core/core.cpp | 14 -------- src/core/gdbstub/gdbstub.cpp | 48 ++----------------------- src/core/gdbstub/gdbstub.h | 20 ----------- src/core/gdbstub/hio.cpp | 10 ------ 5 files changed, 3 insertions(+), 91 deletions(-) diff --git a/src/core/arm/skyeye_common/armstate.cpp b/src/core/arm/skyeye_common/armstate.cpp index 9de68dc11..8ea916dbf 100644 --- a/src/core/arm/skyeye_common/armstate.cpp +++ b/src/core/arm/skyeye_common/armstate.cpp @@ -612,7 +612,7 @@ void ARMul_State::ServeBreak() { Kernel::Thread* thread = system.Kernel().GetCurrentThreadManager().GetCurrentThread(); system.GetRunningCore().SaveContext(thread->context); - if (last_bkpt_hit || GDBStub::IsMemoryBreak() || GDBStub::GetCpuStepFlag()) { + if (last_bkpt_hit || GDBStub::IsMemoryBreak()) { last_bkpt_hit = false; GDBStub::Break(); GDBStub::SendTrap(thread, 5); diff --git a/src/core/core.cpp b/src/core/core.cpp index 0132e24bd..c22d5674a 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -89,16 +89,6 @@ System::ResultStatus System::RunLoop(bool tight_loop) { running_core->SaveContext(thread->context); } GDBStub::HandlePacket(*this); - - // If the loop is halted and we want to step, use a tiny (1) number of instructions to - // execute. Otherwise, get out of the loop function. - if (GDBStub::GetCpuHaltFlag()) { - if (GDBStub::GetCpuStepFlag()) { - tight_loop = false; - } else { - return ResultStatus::Success; - } - } } Signal signal{Signal::None}; @@ -269,10 +259,6 @@ System::ResultStatus System::RunLoop(bool tight_loop) { } } - if (GDBStub::IsServerEnabled()) { - GDBStub::SetCpuStepFlag(false); - } - Reschedule(); return status; diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index a2c4ff7ec..114ac9bab 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -137,8 +137,6 @@ static Kernel::Thread* current_thread = nullptr; // so default to a port outside of that range. u16 gdbstub_port = 24689; -bool halt_loop = true; -bool step_loop = false; bool send_trap = false; // If set to false, the server will never be started and no @@ -666,7 +664,6 @@ static void ReadCommand() { return; } else if (c == 0x03) { LOG_INFO(Debug_GDBStub, "gdb: found break command\n"); - halt_loop = true; SendSignal(current_thread, SIGTRAP); return; } else if (c != GDB_STUB_START) { @@ -916,18 +913,6 @@ void Break(bool is_memory_break) { 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); - UpdateCPUThreadContext(); - } - step_loop = true; - halt_loop = true; - send_trap = true; - ClearAllInstructionCache(); -} - bool IsMemoryBreak() { if (!IsConnected()) { return false; @@ -939,8 +924,6 @@ bool IsMemoryBreak() { /// Tell the CPU to continue executing. static void Continue() { memory_break = false; - step_loop = false; - halt_loop = false; ClearAllInstructionCache(); } @@ -1132,7 +1115,8 @@ void HandlePacket(Core::System& system) { WriteMemory(); break; case 's': - Step(); + // Single step, return ENOTSUP + SendReply("E5F"); return; case 'C': case 'c': @@ -1181,17 +1165,9 @@ void DeferStart() { static void Init(u16 port) { if (!server_enabled) { - // Set the halt loop to false in case the user enabled the gdbstub mid-execution. - // This way the CPU can still execute normally. - halt_loop = false; - step_loop = false; return; } - // Setup initial gdbstub status - halt_loop = true; - step_loop = false; - breakpoints_execute.clear(); breakpoints_read.clear(); breakpoints_write.clear(); @@ -1239,9 +1215,6 @@ static void Init(u16 port) { if (gdbserver_socket < 0) { // In the case that we couldn't start the server for whatever reason, just start CPU // execution like normal. - halt_loop = false; - step_loop = false; - LOG_ERROR(Debug_GDBStub, "Failed to accept gdb client"); } else { LOG_INFO(Debug_GDBStub, "Client connected.\n"); @@ -1285,22 +1258,6 @@ bool IsConnected() { return IsServerEnabled() && gdbserver_socket != -1; } -bool GetCpuHaltFlag() { - return halt_loop; -} - -void SetCpuHaltFlag(bool halt) { - halt_loop = halt; -} - -bool GetCpuStepFlag() { - return step_loop; -} - -void SetCpuStepFlag(bool is_step) { - step_loop = is_step; -} - void SendTrap(Kernel::Thread* thread, int trap) { if (!send_trap) { return; @@ -1310,7 +1267,6 @@ void SendTrap(Kernel::Thread* thread, int trap) { 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 fc0c7df23..8a021580b 100644 --- a/src/core/gdbstub/gdbstub.h +++ b/src/core/gdbstub/gdbstub.h @@ -92,26 +92,6 @@ BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, GDBStub::BreakpointTy */ 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(); - -/** - * If set to true, the CPU will halt at the beginning of the next CPU loop. - * - * @param halt whether to halt on the next loop - */ -void SetCpuHaltFlag(bool halt); - -// If set to true and the CPU is halted, the CPU will step one instruction. -bool GetCpuStepFlag(); - -/** - * When set to true, the CPU will step one instruction when the CPU is halted next. - * - * @param is_step - */ -void SetCpuStepFlag(bool is_step); - /** * Send trap signal from thread back to the gdbstub server. * diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp index 00eab9061..cbc528672 100644 --- a/src/core/gdbstub/hio.cpp +++ b/src/core/gdbstub/hio.cpp @@ -23,9 +23,6 @@ enum class Status { static std::atomic request_status{Status::NoRequest}; -static std::atomic was_halted = false; -static std::atomic was_stepping = false; - } // namespace /** @@ -97,14 +94,9 @@ void SetHioRequest(Core::System& system, const VAddr addr) { current_hio_request_addr = addr; request_status = Status::NotSent; - was_halted = GetCpuHaltFlag(); - was_stepping = GetCpuStepFlag(); - // Now halt, so that no further instructions are executed until the request // is processed by the client. We will continue after the reply comes back Break(); - SetCpuHaltFlag(true); - SetCpuStepFlag(false); system.GetRunningCore().ClearInstructionCache(); } @@ -195,8 +187,6 @@ void HandleHioReply(Core::System& system, const u8* const command_buffer, request_status = Status::NoRequest; // Restore state from before the request came in - SetCpuStepFlag(was_stepping); - SetCpuHaltFlag(was_halted); system.GetRunningCore().ClearInstructionCache(); } From 362b993008b0644fc97f19bba6e47c70fafefa73 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Sun, 3 May 2026 00:11:03 +0200 Subject: [PATCH 3/9] gdb: Revamp the entire GDB implementation and add debug features --- CMakeModules/GenerateSettingKeys.cmake | 1 + src/citra_qt/citra_qt.cpp | 12 + src/citra_qt/configuration/config.cpp | 2 + .../configuration/configure_debug.cpp | 5 + src/citra_qt/configuration/configure_debug.ui | 10 + src/common/memory_ref.h | 6 +- src/common/settings.cpp | 2 + src/common/settings.h | 1 + src/common/string_util.cpp | 13 +- src/common/string_util.h | 1 + src/core/arm/arm_interface.h | 20 +- src/core/arm/dynarmic/arm_dynarmic.cpp | 63 +- src/core/arm/dynarmic/arm_dynarmic.h | 4 +- src/core/arm/dyncom/arm_dyncom.cpp | 8 +- .../arm/dyncom/arm_dyncom_interpreter.cpp | 12 +- src/core/arm/skyeye_common/armstate.cpp | 41 +- src/core/core.cpp | 8 + src/core/core.h | 6 +- src/core/gdbstub/gdbstub.cpp | 632 ++++++++++++++---- src/core/gdbstub/gdbstub.h | 25 +- src/core/gdbstub/hio.cpp | 20 +- src/core/gdbstub/hio.h | 11 +- src/core/hle/kernel/process.cpp | 36 + src/core/hle/kernel/process.h | 6 +- src/core/hle/kernel/svc.cpp | 2 +- src/core/hle/kernel/thread.cpp | 19 +- src/core/hle/kernel/thread.h | 10 + src/core/memory.cpp | 283 +++++++- src/core/memory.h | 36 +- 29 files changed, 1039 insertions(+), 256 deletions(-) diff --git a/CMakeModules/GenerateSettingKeys.cmake b/CMakeModules/GenerateSettingKeys.cmake index 90110cd33..d61d8d6c3 100644 --- a/CMakeModules/GenerateSettingKeys.cmake +++ b/CMakeModules/GenerateSettingKeys.cmake @@ -115,6 +115,7 @@ foreach(KEY IN ITEMS "log_filter" "log_regex_filter" "toggle_unique_data_console_type" + "break_on_unmapped_memory_access" "use_integer_scaling" "layouts_to_cycle" "camera_inner_flip" diff --git a/src/citra_qt/citra_qt.cpp b/src/citra_qt/citra_qt.cpp index 62fb24b9b..aa2c90228 100644 --- a/src/citra_qt/citra_qt.cpp +++ b/src/citra_qt/citra_qt.cpp @@ -3880,6 +3880,18 @@ void GMainWindow::OnCoreError(Core::System::ResultStatus result, std::string det .c_str()); error_severity_icon = QMessageBox::Icon::Critical; can_continue = false; + } else if (result == Core::System::ResultStatus::ErrorCoreExceptionRaised) { + title = tr("An exception occurred"); + message = tr("An exception occurred while executing the emulated application.\n\n"); + message += QString::fromStdString(details); + error_severity_icon = QMessageBox::Icon::Critical; + can_continue = false; + } else if (result == Core::System::ResultStatus::ErrorMemoryExceptionRaised) { + title = tr("An invalid memory access occurred"); + message = + tr("An invalid memory access occurred while executing the emulated application.\n\n"); + message += QString::fromStdString(details); + error_severity_icon = QMessageBox::Icon::Critical; } else { title = tr("Fatal Error"); message = tr("A fatal error occurred. " diff --git a/src/citra_qt/configuration/config.cpp b/src/citra_qt/configuration/config.cpp index a5aa9896b..e51a175d0 100644 --- a/src/citra_qt/configuration/config.cpp +++ b/src/citra_qt/configuration/config.cpp @@ -510,6 +510,7 @@ void QtConfig::ReadDebuggingValues() { ReadBasicSetting(Settings::values.instant_debug_log); ReadBasicSetting(Settings::values.enable_rpc_server); ReadBasicSetting(Settings::values.toggle_unique_data_console_type); + ReadBasicSetting(Settings::values.break_on_unmapped_memory_access); qt_config->beginGroup(QStringLiteral("LLE")); for (const auto& service_module : Service::service_module_map) { @@ -1094,6 +1095,7 @@ void QtConfig::SaveDebuggingValues() { WriteBasicSetting(Settings::values.instant_debug_log); WriteBasicSetting(Settings::values.enable_rpc_server); WriteBasicSetting(Settings::values.toggle_unique_data_console_type); + WriteBasicSetting(Settings::values.break_on_unmapped_memory_access); qt_config->beginGroup(QStringLiteral("LLE")); for (const auto& service_module : Settings::values.lle_modules) { diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index 0fe4f49cf..db09c7b93 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -112,6 +112,8 @@ void ConfigureDebug::SetConfiguration() { #endif // !ENABLE_SCRIPTING ui->toggle_unique_data_console_type->setChecked( Settings::values.toggle_unique_data_console_type.GetValue()); + ui->break_on_unmapped_memory_access->setChecked( + Settings::values.break_on_unmapped_memory_access.GetValue()); ui->toggle_renderer_debug->setChecked(Settings::values.renderer_debug.GetValue()); ui->toggle_dump_command_buffers->setChecked(Settings::values.dump_command_buffers.GetValue()); @@ -153,6 +155,8 @@ void ConfigureDebug::ApplyConfiguration() { Settings::values.enable_rpc_server = ui->enable_rpc_server->isChecked(); Settings::values.toggle_unique_data_console_type = ui->toggle_unique_data_console_type->isChecked(); + Settings::values.break_on_unmapped_memory_access = + ui->break_on_unmapped_memory_access->isChecked(); Settings::values.renderer_debug = ui->toggle_renderer_debug->isChecked(); Settings::values.dump_command_buffers = ui->toggle_dump_command_buffers->isChecked(); Settings::values.instant_debug_log = ui->instant_debug_log->isChecked(); @@ -178,6 +182,7 @@ void ConfigureDebug::SetupPerGameUI() { ui->groupBox_2->setVisible(false); ui->enable_rpc_server->setVisible(false); ui->toggle_unique_data_console_type->setVisible(false); + ui->break_on_unmapped_memory_access->setVisible(false); ui->toggle_cpu_jit->setVisible(false); } diff --git a/src/citra_qt/configuration/configure_debug.ui b/src/citra_qt/configuration/configure_debug.ui index 990835a80..d9f66de38 100644 --- a/src/citra_qt/configuration/configure_debug.ui +++ b/src/citra_qt/configuration/configure_debug.ui @@ -299,6 +299,16 @@ + + + + <html><head/><body><p>Pauses emulation and shows an error message if an unmapped memory access is detected.</p></body></html> + + + Break on unmapped memory access + + + diff --git a/src/common/memory_ref.h b/src/common/memory_ref.h index f5935bf4c..3e4ddc67f 100644 --- a/src/common/memory_ref.h +++ b/src/common/memory_ref.h @@ -1,4 +1,4 @@ -// Copyright 2020 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -153,7 +153,9 @@ private: void serialize(Archive& ar, const unsigned int) { ar & backing_mem; ar & offset; - Init(); + if (Archive::is_loading::value) { + Init(); + } } friend class boost::serialization::access; }; diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 2ac231f50..9ee701caf 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -165,6 +165,8 @@ void LogSettings() { log_setting("Debugging_InstantDebugLog", values.instant_debug_log.GetValue()); log_setting("Debugging_ToggleUniqueDataConsoleType", values.toggle_unique_data_console_type.GetValue()); + log_setting("Debugging_BreakOnUnmappedMemoryAccess", + values.break_on_unmapped_memory_access.GetValue()); } bool IsConfiguringGlobal() { diff --git a/src/common/settings.h b/src/common/settings.h index 5eca53421..315197cea 100644 --- a/src/common/settings.h +++ b/src/common/settings.h @@ -642,6 +642,7 @@ struct Values { Setting instant_debug_log{false, Keys::instant_debug_log}; Setting enable_rpc_server{false, Keys::enable_rpc_server}; Setting toggle_unique_data_console_type{false, Keys::toggle_unique_data_console_type}; + Setting break_on_unmapped_memory_access{false, Keys::break_on_unmapped_memory_access}; // Miscellaneous Setting log_filter{"*:Info", Keys::log_filter}; diff --git a/src/common/string_util.cpp b/src/common/string_util.cpp index 117c19df2..64effc9e2 100644 --- a/src/common/string_util.cpp +++ b/src/common/string_util.cpp @@ -122,8 +122,7 @@ void BuildCompleteFilename(std::string& _CompleteFilename, const std::string& _P _CompleteFilename += _Filename; } -std::vector SplitString(const std::string& str, const char delim) { - std::istringstream iss(str); +static std::vector SplitString(std::istringstream& iss, const char delim) { std::vector output(1); while (std::getline(iss, *output.rbegin(), delim)) { @@ -134,6 +133,16 @@ std::vector SplitString(const std::string& str, const char delim) { return output; } +std::vector SplitString(std::string_view str, const char delim) { + std::istringstream iss{std::string(str)}; + return SplitString(iss, delim); +} + +std::vector SplitString(const std::string& str, const char delim) { + std::istringstream iss(str); + return SplitString(iss, delim); +} + std::string TabsToSpaces(int tab_size, std::string in) { std::size_t i = 0; diff --git a/src/common/string_util.h b/src/common/string_util.h index 033c3ddb1..5c6d0bdac 100644 --- a/src/common/string_util.h +++ b/src/common/string_util.h @@ -36,6 +36,7 @@ namespace Common { [[nodiscard]] bool EndsWith(const std::string& value, const std::string& ending); +[[nodiscard]] std::vector SplitString(std::string_view str, const char delim); [[nodiscard]] std::vector SplitString(const std::string& str, const char delim); // "C:/Windows/winhelp.exe" to "C:/Windows/", "winhelp", ".exe" diff --git a/src/core/arm/arm_interface.h b/src/core/arm/arm_interface.h index 1f6ddc9e9..0aeb10de2 100644 --- a/src/core/arm/arm_interface.h +++ b/src/core/arm/arm_interface.h @@ -1,4 +1,4 @@ -// Copyright 2014 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -198,12 +198,28 @@ public: return id; } + /** + * Sets the core to not being runnable until its break condition is handled. + */ + void SetBreakFlag() { + break_flag = true; + } + + /* + * Sets the core being runnable after its break condition was handled. + */ + void ClearBreakFlag() { + break_flag = false; + } + protected: // This us used for serialization. Returning nullptr is valid if page tables are not used. virtual std::shared_ptr GetPageTable() const = 0; std::shared_ptr timer; + bool break_flag{}; + private: u32 id; @@ -213,6 +229,7 @@ private: void save(Archive& ar, const unsigned int file_version) const { ar << timer; ar << id; + ar << break_flag; const auto page_table = GetPageTable(); ar << page_table; for (int i = 0; i < 15; i++) { @@ -258,6 +275,7 @@ private: ClearInstructionCache(); ar >> timer; ar >> id; + ar >> break_flag; std::shared_ptr page_table{}; ar >> page_table; SetPageTable(page_table); diff --git a/src/core/arm/dynarmic/arm_dynarmic.cpp b/src/core/arm/dynarmic/arm_dynarmic.cpp index 2cdc9c119..002c5488d 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.cpp +++ b/src/core/arm/dynarmic/arm_dynarmic.cpp @@ -1,7 +1,8 @@ -// Copyright 2016 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include #include #include @@ -17,6 +18,14 @@ #include "core/hle/kernel/svc.h" #include "core/memory.h" +#ifndef SIGILL +constexpr u32 SIGILL = 4; +#endif + +#ifndef SIGTRAP +constexpr u32 SIGTRAP = 5; +#endif + namespace Core { class DynarmicUserCallbacks final : public Dynarmic::A32::UserCallbacks { @@ -25,6 +34,21 @@ public: : parent(parent), svc_context(parent.system), memory(parent.memory) {} ~DynarmicUserCallbacks() = default; + std::optional MemoryReadCode(VAddr vaddr) override { + constexpr VAddr low_page_limit = 0x1000; + + // This check prevents a common cascading error that results from the + // memory system allowing unmapped memory accesses (in some situations, + // a vtable is read from an invalid address and execution jumps to the + // zero page). On real hw, it's not normally possible to map the zero page. + if (vaddr < low_page_limit) [[unlikely]] { + LOG_CRITICAL(Debug, "Tried to read code from low address 0x{:08x}", vaddr); + return {}; + } + + return MemoryRead32(vaddr); + } + std::uint8_t MemoryRead8(VAddr vaddr) override { return memory.Read8(vaddr); } @@ -83,9 +107,8 @@ public: break; case Dynarmic::A32::Exception::Breakpoint: if (GDBStub::IsConnected()) { - parent.jit->HaltExecution(); parent.SetPC(pc); - parent.ServeBreak(); + parent.ServeBreak(SIGTRAP); return; } break; @@ -99,11 +122,19 @@ public: case Dynarmic::A32::Exception::PreloadInstruction: return; } - for (int i = 0; i < 16; i++) { - LOG_CRITICAL(Debug, "r{:02d} = {:08X}", i, parent.GetReg(i)); + + parent.SetPC(pc); + if (GDBStub::IsConnected()) { + parent.ServeBreak(SIGILL); + } else { + std::string error; + for (int i = 0; i < 16; i++) { + error += fmt::format("r{:02d} = {:08X}\n", i, parent.GetReg(i)); + } + error += fmt::format("ExceptionRaised(exception = {}, pc = {:08X})", exception, pc); + parent.system.SetStatus(Core::System::ResultStatus::ErrorCoreExceptionRaised, + error.c_str()); } - ASSERT_MSG(false, "ExceptionRaised(exception = {}, pc = {:08X}, code = {:08X})", exception, - pc, MemoryReadCode(pc).value()); } void AddTicks(std::uint64_t ticks) override { @@ -138,16 +169,19 @@ MICROPROFILE_DEFINE(ARM_Jit, "ARM JIT", "ARM JIT", MP_RGB(255, 64, 64)); void ARM_Dynarmic::Run() { ASSERT(memory.GetCurrentPageTable() == current_page_table); MICROPROFILE_SCOPE(ARM_Jit); + if (break_flag) [[unlikely]] { + return; + } jit->Run(); } void ARM_Dynarmic::Step() { - jit->Step(); - - if (GDBStub::IsConnected()) { - ServeBreak(); + if (break_flag) [[unlikely]] { + return; } + + jit->Step(); } void ARM_Dynarmic::SetPC(u32 pc) { @@ -294,11 +328,8 @@ void ARM_Dynarmic::SetPageTable(const std::shared_ptr& page_t jits.emplace(current_page_table, std::move(new_jit)); } -void ARM_Dynarmic::ServeBreak() { - Kernel::Thread* thread = system.Kernel().GetCurrentThreadManager().GetCurrentThread(); - SaveContext(thread->context); - GDBStub::Break(); - GDBStub::SendTrap(thread, 5); +void ARM_Dynarmic::ServeBreak(int signal) { + GDBStub::Break(signal); } std::unique_ptr ARM_Dynarmic::MakeJit() { diff --git a/src/core/arm/dynarmic/arm_dynarmic.h b/src/core/arm/dynarmic/arm_dynarmic.h index ec0c13390..e854df376 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.h +++ b/src/core/arm/dynarmic/arm_dynarmic.h @@ -1,4 +1,4 @@ -// Copyright 2016 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -60,7 +60,7 @@ protected: std::shared_ptr GetPageTable() const override; private: - void ServeBreak(); + void ServeBreak(int signal); friend class DynarmicUserCallbacks; Core::System& system; diff --git a/src/core/arm/dyncom/arm_dyncom.cpp b/src/core/arm/dyncom/arm_dyncom.cpp index c9bb66e38..63db2f9c4 100644 --- a/src/core/arm/dyncom/arm_dyncom.cpp +++ b/src/core/arm/dyncom/arm_dyncom.cpp @@ -1,4 +1,4 @@ -// Copyright 2014 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -24,10 +24,16 @@ ARM_DynCom::ARM_DynCom(Core::System& system_, Memory::MemorySystem& memory, ARM_DynCom::~ARM_DynCom() {} void ARM_DynCom::Run() { + if (break_flag) [[unlikely]] { + return; + } ExecuteInstructions(std::max(timer->GetDowncount(), 0)); } void ARM_DynCom::Step() { + if (break_flag) [[unlikely]] { + return; + } ExecuteInstructions(1); } diff --git a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp index 862c422bc..794197590 100644 --- a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp +++ b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp @@ -1,3 +1,7 @@ +// Copyright Citra Emulator Project / Azahar Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + // Copyright 2012 Michael Kang, 2014 Citra Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -954,11 +958,9 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { #define GDB_BP_CHECK \ cpu->Cpsr &= ~(1 << 5); \ cpu->Cpsr |= cpu->TFlag << 5; \ - if (GDBStub::IsServerEnabled()) { \ - if (GDBStub::IsMemoryBreak()) { \ - goto END; \ - } else if (breakpoint_data.type != GDBStub::BreakpointType::None && \ - PC == breakpoint_data.address) { \ + if (GDBStub::IsServerEnabled()) [[unlikely]] { \ + if (breakpoint_data.type != GDBStub::BreakpointType::None && \ + PC == breakpoint_data.address) { \ cpu->RecordBreak(breakpoint_data); \ goto END; \ } \ diff --git a/src/core/arm/skyeye_common/armstate.cpp b/src/core/arm/skyeye_common/armstate.cpp index 8ea916dbf..d7349b59a 100644 --- a/src/core/arm/skyeye_common/armstate.cpp +++ b/src/core/arm/skyeye_common/armstate.cpp @@ -1,8 +1,9 @@ -// Copyright 2015 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. #include +#include #include "common/logging/log.h" #include "common/swap.h" #include "core/arm/skyeye_common/armstate.h" @@ -10,6 +11,10 @@ #include "core/core.h" #include "core/memory.h" +#ifndef SIGTRAP +constexpr u32 SIGTRAP = 5; +#endif + ARMul_State::ARMul_State(Core::System& system_, Memory::MemorySystem& memory_, PrivilegeMode initial_mode) : system{system_}, memory{memory_} { @@ -182,26 +187,12 @@ void ARMul_State::ResetMPCoreCP15Registers() { CP15[CP15_MAIN_TLB_LOCKDOWN_ATTRIBUTE] = 0x00000000; CP15[CP15_TLB_DEBUG_CONTROL] = 0x00000000; } -#ifdef ANDROID -static void CheckMemoryBreakpoint(u32 address, GDBStub::BreakpointType type) {} -#else -static void CheckMemoryBreakpoint(u32 address, GDBStub::BreakpointType type) { - if (GDBStub::IsServerEnabled() && GDBStub::CheckBreakpoint(address, type)) { - LOG_DEBUG(Debug, "Found memory breakpoint @ {:08x}", address); - GDBStub::Break(true); - } -} -#endif u8 ARMul_State::ReadMemory8(u32 address) const { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - return memory.Read8(address); } u16 ARMul_State::ReadMemory16(u32 address) const { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - u16 data = memory.Read16(address); if (InBigEndianMode()) @@ -211,8 +202,6 @@ u16 ARMul_State::ReadMemory16(u32 address) const { } u32 ARMul_State::ReadMemory32(u32 address) const { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - u32 data = memory.Read32(address); if (InBigEndianMode()) @@ -222,8 +211,6 @@ u32 ARMul_State::ReadMemory32(u32 address) const { } u64 ARMul_State::ReadMemory64(u32 address) const { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - u64 data = memory.Read64(address); if (InBigEndianMode()) @@ -233,14 +220,10 @@ u64 ARMul_State::ReadMemory64(u32 address) const { } void ARMul_State::WriteMemory8(u32 address, u8 data) { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Write); - memory.Write8(address, data); } void ARMul_State::WriteMemory16(u32 address, u16 data) { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Write); - if (InBigEndianMode()) data = Common::swap16(data); @@ -248,8 +231,6 @@ void ARMul_State::WriteMemory16(u32 address, u16 data) { } void ARMul_State::WriteMemory32(u32 address, u32 data) { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Write); - if (InBigEndianMode()) data = Common::swap32(data); @@ -257,8 +238,6 @@ void ARMul_State::WriteMemory32(u32 address, u32 data) { } void ARMul_State::WriteMemory64(u32 address, u64 data) { - CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Write); - if (InBigEndianMode()) data = Common::swap64(data); @@ -609,12 +588,8 @@ void ARMul_State::ServeBreak() { DEBUG_ASSERT(Reg[15] == last_bkpt.address); } - Kernel::Thread* thread = system.Kernel().GetCurrentThreadManager().GetCurrentThread(); - system.GetRunningCore().SaveContext(thread->context); - - if (last_bkpt_hit || GDBStub::IsMemoryBreak()) { + if (last_bkpt_hit) { last_bkpt_hit = false; - GDBStub::Break(); - GDBStub::SendTrap(thread, 5); + GDBStub::Break(SIGTRAP); } } diff --git a/src/core/core.cpp b/src/core/core.cpp index c22d5674a..e76fd213a 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -88,6 +88,12 @@ System::ResultStatus System::RunLoop(bool tight_loop) { if (thread && running_core) { running_core->SaveContext(thread->context); } + // The break flag is only set if GDB is connected, + // we can do clearing here safely. If it is ever + // used outside, move the clearing outside the if. + for (auto& cpu_core : cpu_cores) { + cpu_core->ClearBreakFlag(); + } GDBStub::HandlePacket(*this); } @@ -249,6 +255,8 @@ System::ResultStatus System::RunLoop(bool tight_loop) { cpu_core->GetTimer().Idle(); PrepareReschedule(); } else { + // In the rare case the break flag is set (due to exception thrown) + // there is probably no need to adjust the timer accordingly. if (tight_loop) { cpu_core->Run(); } else { diff --git a/src/core/core.h b/src/core/core.h index 05620da32..89df9b00e 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -103,8 +103,10 @@ public: ErrorSavestate, ///< Error saving or loading ErrorArticDisconnected, ///< Error when artic base disconnects ErrorN3DSApplication, ///< Error launching New 3DS application in Old 3DS mode - ShutdownRequested, ///< Emulated program requested a system shutdown - ErrorUnknown ///< Any other error + ErrorCoreExceptionRaised, ///< The CPU emulation raised an exception + ErrorMemoryExceptionRaised, ///< Unmmaped memory was accessed + ShutdownRequested, ///< Emulated program requested a system shutdown + ErrorUnknown ///< Any other error }; explicit System(); diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 114ac9bab..a1d0f8e73 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -1,3 +1,7 @@ +// Copyright Citra Emulator Project / Azahar Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + // Copyright 2013 Dolphin Emulator Project // Licensed under GPLv2+ // Refer to the license.txt file included. @@ -36,10 +40,13 @@ #include "core/gdbstub/gdbstub.h" #include "core/gdbstub/hio.h" #include "core/hle/kernel/process.h" +#include "core/loader/loader.h" #include "core/memory.h" +#pragma optimize("", off) + // Uncomment to log all GDB traffic -// #define PRINT_GDB_TRAFFIC +#define PRINT_GDB_TRAFFIC namespace GDBStub { namespace { @@ -62,6 +69,10 @@ constexpr u32 SIGTERM = 15; constexpr u32 MSG_WAITALL = 8; #endif +#ifndef SIGSEGV +constexpr u32 SIGSEGV = 11; +#endif + constexpr u32 SP_REGISTER = 13; constexpr u32 LR_REGISTER = 14; constexpr u32 PC_REGISTER = 15; @@ -76,6 +87,8 @@ constexpr char target_xml[] = R"(l + arm + 3DS @@ -129,19 +142,25 @@ u8 command_buffer[GDB_BUFFER_SIZE]; u32 command_length; u32 latest_signal = 0; -bool memory_break = false; static Kernel::Thread* current_thread = nullptr; +static Kernel::Process* current_process = nullptr; // Binding to a port within the reserved ports range (0-1023) requires root permissions, // so default to a port outside of that range. u16 gdbstub_port = 24689; -bool send_trap = false; +constexpr bool supports_extended_mode = true; +bool is_extended_mode = false; // If set to false, the server will never be started and no // gdbstub-related functions will be executed. std::atomic server_enabled(false); +int accept_socket = -1; +int continue_thread = -1; + +static Kernel::Thread* break_thread = nullptr; +static int break_signal = 0; #ifdef _WIN32 WSADATA InitData; @@ -161,16 +180,17 @@ BreakpointMap breakpoints_write; } // Anonymous namespace static Kernel::Thread* FindThreadById(int id) { - u32 num_cores = Core::GetNumCores(); - for (u32 i = 0; i < num_cores; ++i) { - const auto& threads = - Core::System::GetInstance().Kernel().GetThreadManager(i).GetThreadList(); - for (auto& thread : threads) { - if (thread->GetThreadId() == static_cast(id)) { - return thread.get(); - } + if (!current_process) { + return nullptr; + } + + auto thread_list = current_process->GetThreadList(); + for (auto& thread : thread_list) { + if (thread->GetThreadId() == static_cast(id)) { + return thread.get(); } } + return nullptr; } @@ -364,12 +384,40 @@ static u64 GdbHexToLong(const u8* src) { return output; } +static int GetErrno() { +#ifdef _WIN32 + return WSAGetLastError(); +#else + return errno; +#endif +} + +static bool SetNonBlock(int socket, bool nonblock) { +#ifdef _WIN32 + unsigned long nonblocking = nonblock ? 1 : 0; + int ret = ioctlsocket(socket, FIONBIO, &nonblocking); + if (ret < 0) { + LOG_ERROR(Debug_GDBStub, "Failed to set non-blocking gdb socket"); + return false; + } +#else + int flags = nonblock ? O_NONBLOCK : 0; + + const int ret = ::fcntl(socket, F_SETFL, flags); + if (ret < 0) { + LOG_ERROR(Debug_GDBStub, "Failed to set non-blocking gdb socket"); + return false; + } +#endif + return true; +} + /// Read a byte from the gdb client. static u8 ReadByte() { u8 c; std::size_t received_size = recv(gdbserver_socket, reinterpret_cast(&c), 1, MSG_WAITALL); if (received_size != 1) { - LOG_ERROR(Debug_GDBStub, "recv failed : {}", received_size); + LOG_ERROR(Debug_GDBStub, "recv failed : {}", GetErrno()); Shutdown(); } @@ -417,13 +465,15 @@ static void RemoveBreakpoint(BreakpointType type, VAddr addr) { bp->second.len, bp->second.addr, type); if (type == BreakpointType::Execute) { - Core::System::GetInstance().Memory().WriteBlock( - *Core::System::GetInstance().Kernel().GetCurrentProcess(), bp->second.addr, - bp->second.inst.data(), bp->second.inst.size()); + Core::System::GetInstance().Memory().WriteBlock(*current_process, bp->second.addr, + bp->second.inst.data(), bp->second.len); u32 num_cores = Core::GetNumCores(); for (u32 i = 0; i < num_cores; ++i) { Core::GetCore(i).ClearInstructionCache(); } + } else { + Core::System::GetInstance().Memory().UnregisterWatchpoint(*current_process, bp->second.addr, + bp->second.len); } p.erase(addr); } @@ -444,38 +494,58 @@ BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, BreakpointType type) return breakpoint; } -bool CheckBreakpoint(VAddr addr, BreakpointType type) { +bool CheckBreakpoint(VAddr addr, u32 access_len, BreakpointType type) { if (!IsConnected()) { return false; } const BreakpointMap& p = GetBreakpointMap(type); - const auto bp = p.find(addr); - if (bp == p.end()) { - return false; - } + // Access range: [addr, access_end) + const VAddr access_end = addr + access_len; - u32 len = bp->second.len; + for (const auto& [base_addr, bp] : p) { + if (!bp.active) { + continue; + } - // 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; - } + u32 bp_len = bp.len; - if (bp->second.active && (addr >= bp->second.addr && addr < bp->second.addr + len)) { - LOG_DEBUG(Debug_GDBStub, - "Found breakpoint type {} @ {:08x}, range: {:08x}" - " - {:08x} ({:x} bytes)", - 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) { + bp_len = 1; + } + + // Breakpoint/watchpoint range: [bp.addr, bp_end) + const VAddr bp_end = bp.addr + bp_len; + + bool hit = false; + + if (type == BreakpointType::Execute) { + // Execute breakpoints should only trigger on exact PC match. + hit = (addr == bp.addr); + } else { + // Range overlap test: + // [addr, access_end) overlaps [bp.addr, bp_end) + hit = (addr < bp_end) && (bp.addr < access_end); + } + + if (hit) { + LOG_DEBUG(Debug_GDBStub, + "Found breakpoint type {}, " + "access range: {:08x} - {:08x}, " + "breakpoint range: {:08x} - {:08x}", + type, addr, addr, access_end, bp.addr, bp_end); + + return true; + } } return false; @@ -503,11 +573,7 @@ void SendReply(const char* reply) { command_length = static_cast(strlen(reply)); #ifdef PRINT_GDB_TRAFFIC - if (command_length > 0) { - LOG_INFO(Debug_GDBStub, "Req: {} {}", *reply, reply + 1); - } else { - LOG_INFO(Debug_GDBStub, "Req:"); - } + LOG_INFO(Debug_GDBStub, "Res: {}", reply); #endif if (command_length + 4 > sizeof(command_buffer)) { @@ -545,60 +611,103 @@ static void HandleQuery() { if (strcmp(query, "TStatus") == 0) { SendReply("T0"); + } else if (strncmp(query, "Attached", strlen("Attached")) == 0) { + SendReply("1"); } else if (strncmp(query, "Supported", strlen("Supported")) == 0) { // PacketSize needs to be large enough for target xml - SendReply("PacketSize=2000;qXfer:features:read+;qXfer:threads:read+"); + SendReply("PacketSize=2000;qXfer:features:read+;qXfer:osdata:read+;qXfer:threads:read+;" + "vContSupported+"); } 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"; - u32 num_cores = Core::GetNumCores(); - for (u32 i = 0; i < num_cores; ++i) { - const auto& threads = - Core::System::GetInstance().Kernel().GetThreadManager(i).GetThreadList(); - for (const auto& thread : threads) { - val += fmt::format("{:x},", thread->GetThreadId()); - } + if (!current_process) { + SendReply("E01"); + return; } + + auto thread_list = current_process->GetThreadList(); + std::string val = "m"; + for (const auto& thread : thread_list) { + val += fmt::format("{:x},", thread->GetThreadId()); + } + val.pop_back(); SendReply(val.c_str()); } else if (strncmp(query, "sThreadInfo", strlen("sThreadInfo")) == 0) { SendReply("l"); + } else if (strncmp(query, "Xfer:osdata:read:processes", strlen("Xfer:osdata:read:processes")) == + 0) { + std::string buffer; + buffer += "l"; + auto process_list = Core::System::GetInstance().Kernel().GetProcessList(); + for (const auto& p : process_list) { + buffer += fmt::format("" + "{}" + "{}" + "", + p->process_id, p->codeset->name); + } + buffer += ""; + SendReply(buffer.c_str()); } else if (strncmp(query, "Xfer:threads:read", strlen("Xfer:threads:read")) == 0) { + if (!current_process) { + SendReply("E01"); + return; + } + std::string buffer; buffer += "l"; buffer += ""; - u32 num_cores = Core::GetNumCores(); - for (u32 i = 0; i < num_cores; ++i) { - const auto& threads = - Core::System::GetInstance().Kernel().GetThreadManager(i).GetThreadList(); - for (const auto& thread : threads) { - buffer += fmt::format(R"*()*", - thread->GetThreadId(), thread->GetThreadId()); - } + auto thread_list = current_process->GetThreadList(); + for (const auto& thread : thread_list) { + buffer += fmt::format(R"*()*", + thread->GetThreadId(), thread->GetThreadId()); } buffer += ""; + SendReply(buffer.c_str()); } else { SendReply(""); } } +static bool SetThread(int thread_id) { + if (!current_process) { + // The process has not been selected yet + return false; + } + if (thread_id >= 1) { + current_thread = FindThreadById(thread_id); + } + if (!current_thread) { + auto thread_list = current_process->GetThreadList(); + if (thread_list.size() > 0) { + // Select the lowest thread ID, which is the main thread + std::sort(thread_list.begin(), thread_list.end(), + [](const std::shared_ptr& a, + const std::shared_ptr& b) { + return a->thread_id < b->thread_id; + }); + current_thread = thread_list[0].get(); + } + } + return current_thread != nullptr; +} /// Handle set thread command from gdb client. static void HandleSetThread() { 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 (command_buffer[1] == 'c') { + continue_thread = thread_id; + SendReply("OK"); + return; } - if (!current_thread) { - thread_id = 1; - current_thread = FindThreadById(thread_id); - } - if (current_thread) { + + if (SetThread(thread_id)) { SendReply("OK"); return; } @@ -607,6 +716,11 @@ static void HandleSetThread() { /// Handle thread alive command from gdb client. static void HandleThreadAlive() { + if (!current_process) { + SendReply("E01"); + return; + } + int thread_id = static_cast(HexToInt(command_buffer + 1, command_length - 1)); if (thread_id == 0) { thread_id = 1; @@ -618,6 +732,15 @@ static void HandleThreadAlive() { SendReply("E01"); } +static void HandleExtendedMode() { + if (supports_extended_mode) { + is_extended_mode = true; + SendReply("OK"); + } else { + SendReply(""); + } +} + /** * Send signal packet to client. * @@ -636,11 +759,16 @@ static void SendSignal(Kernel::Thread* thread, u32 signal, bool full = true) { std::string buffer; if (full) { - + Core::ARM_Interface::ThreadContext ctx{}; + if (thread) { + ctx = thread->context; + } else { + Core::GetRunningCore().SaveContext(ctx); + } buffer = fmt::format("T{:02x}{:02x}:{:08x};{:02x}:{:08x};{:02x}:{:08x}", latest_signal, - PC_REGISTER, htonl(Core::GetRunningCore().GetPC()), SP_REGISTER, - htonl(Core::GetRunningCore().GetReg(SP_REGISTER)), LR_REGISTER, - htonl(Core::GetRunningCore().GetReg(LR_REGISTER))); + PC_REGISTER, htonl(ctx.cpu_registers[PC_REGISTER]), SP_REGISTER, + htonl(ctx.cpu_registers[SP_REGISTER]), LR_REGISTER, + htonl(ctx.cpu_registers[LR_REGISTER])); } else { buffer = fmt::format("T{:02x}", latest_signal); } @@ -653,6 +781,53 @@ static void SendSignal(Kernel::Thread* thread, u32 signal, bool full = true) { SendReply(buffer.c_str()); } +static void HandleGetStopReason() { + if (is_extended_mode) { + // In extended mode, tell the debugger that there is no selected process yet. + // That way the debugger can ask for the process list and attack to the right PID. + if (!current_process) { + // The process has not been selected yet. + SendReply("W00"); + } else { + SendSignal(current_thread, latest_signal); + } + } else { + // In non extended mode, select the process corresponding to the "main" + // launched application. + u64 program_id = 0; + Core::System::GetInstance().GetAppLoader().ReadProgramId(program_id); + auto process_list = Core::System::GetInstance().Kernel().GetProcessList(); + for (const auto& process : process_list) { + if (process->codeset->program_id == program_id) { + current_process = process.get(); + current_process->SetDebugBreak(true); + if (SetThread(0)) { + SendSignal(current_thread, 0); + } else { + // Should never happen + SendReply("W00"); + } + return; + } + } + + // No process, should never happen + SendReply("W00"); + } +} + +static void BreakImpl(int signal) { + if (signal == SIGSEGV) { + LOG_WARNING(Debug_GDBStub, "Segmentation fault signals may not be fully accurate"); + } + + current_process->SetDebugBreak(true); + + latest_signal = signal; + + SendSignal(current_thread, signal); +} + /// Read command from gdb client. static void ReadCommand() { command_length = 0; @@ -664,7 +839,7 @@ static void ReadCommand() { return; } else if (c == 0x03) { LOG_INFO(Debug_GDBStub, "gdb: found break command\n"); - SendSignal(current_thread, SIGTRAP); + BreakImpl(SIGTRAP); return; } else if (c != GDB_STUB_START) { LOG_DEBUG(Debug_GDBStub, "gdb: read invalid byte {:02x}\n", c); @@ -726,6 +901,11 @@ static bool IsDataAvailable() { /// Send requested register to gdb client. static void ReadRegister() { + if (!current_process || !current_thread) { + SendReply("E01"); + return; + } + static u8 reply[64]; std::memset(reply, 0, sizeof(reply)); @@ -752,6 +932,11 @@ static void ReadRegister() { /// Send all registers to the gdb client. static void ReadRegisters() { + if (!current_process || !current_thread) { + SendReply("E01"); + return; + } + static u8 buffer[GDB_BUFFER_SIZE - 4]; std::memset(buffer, 0, sizeof(buffer)); @@ -791,6 +976,11 @@ static void UpdateCPUThreadContext() { /// Modify data of register specified by gdb client. static void WriteRegister() { + if (!current_process || !current_thread) { + SendReply("E01"); + return; + } + const u8* buffer_ptr = command_buffer + 3; u32 id = HexCharToValue(command_buffer[1]); @@ -819,6 +1009,11 @@ static void WriteRegister() { /// Modify all registers with data received from the client. static void WriteRegisters() { + if (!current_process || !current_thread) { + SendReply("E01"); + return; + } + const u8* buffer_ptr = command_buffer + 1; if (command_buffer[0] != 'G') @@ -849,6 +1044,11 @@ static void WriteRegisters() { /// Read location in memory specified by gdb client. static void ReadMemory() { + if (!current_process) { + SendReply("E01"); + return; + } + static u8 reply[GDB_BUFFER_SIZE - 4]; auto start_offset = command_buffer + 1; @@ -866,13 +1066,12 @@ static void ReadMemory() { } auto& memory = Core::System::GetInstance().Memory(); - if (!memory.IsValidVirtualAddress(*Core::System::GetInstance().Kernel().GetCurrentProcess(), - addr)) { - return SendReply("E00"); + if (!memory.IsValidVirtualAddress(*current_process, addr)) { + return SendReply("E14"); } std::vector data(len); - memory.ReadBlock(addr, data.data(), len); + memory.ReadBlock(*current_process, addr, data.data(), len); MemToGdbHex(reply, data.data(), len); reply[len * 2] = '\0'; @@ -885,6 +1084,11 @@ static void ReadMemory() { /// Modify location in memory with data received from the gdb client. static void WriteMemory() { + if (!current_process) { + SendReply("E01"); + return; + } + auto start_offset = command_buffer + 1; auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); VAddr addr = HexToInt(start_offset, static_cast(addr_pos - start_offset)); @@ -894,36 +1098,67 @@ static void WriteMemory() { u32 len = HexToInt(start_offset, static_cast(len_pos - start_offset)); auto& memory = Core::System::GetInstance().Memory(); - if (!memory.IsValidVirtualAddress(*Core::System::GetInstance().Kernel().GetCurrentProcess(), - addr)) { + if (!memory.IsValidVirtualAddress(*current_process, addr)) { return SendReply("E00"); } std::vector data(len); GdbHexToMem(data.data(), len_pos + 1, len); - memory.WriteBlock(addr, data.data(), len); + memory.WriteBlock(*current_process, addr, data.data(), len); ClearAllInstructionCache(); SendReply("OK"); } -void Break(bool is_memory_break) { - send_trap = true; - - memory_break = is_memory_break; -} - -bool IsMemoryBreak() { - if (!IsConnected()) { - return false; +void Break(int signal) { + if (!IsConnected() || !current_process || + Core::System::GetInstance().Kernel().GetCurrentProcess().get() != current_process) { + LOG_ERROR(Debug_GDBStub, "Got signal for un-attached process, ignoring..."); + return; } - return memory_break; + if (break_thread) { + LOG_ERROR(Debug_GDBStub, + "Got multiple break signals in quick succession, latest may be lost"); + return; + } + + break_thread = + Core::System::GetInstance().Kernel().GetCurrentThreadManager().GetCurrentThread(); + if (break_thread) { + break_signal = signal; + } + + // Try to break CPU asap + Core::GetRunningCore().SetBreakFlag(); + Core::GetRunningCore().ClearInstructionCache(); + Core::GetRunningCore().PrepareReschedule(); } /// Tell the CPU to continue executing. static void Continue() { - memory_break = false; + if (!current_process) { + return; + } + + u32 thread_id = -1; + if (continue_thread == 0) { + thread_id = current_process->GetThreadList()[0]->thread_id; + } else { + thread_id = continue_thread; + } + + // There is no documentation anywhere if continue should + // reset the continue thread value. Luma3DS implementation + // does this, and looks like IDA Pro expects it that way too. + continue_thread = -1; + + std::vector continue_list{}; + if (thread_id != -1) { + continue_list.push_back(thread_id); + } + + current_process->SetDebugBreak(false, continue_list); ClearAllInstructionCache(); } @@ -937,20 +1172,26 @@ static void Continue() { static bool CommitBreakpoint(BreakpointType type, VAddr addr, u32 len) { BreakpointMap& p = GetBreakpointMap(type); + if (type == BreakpointType::Execute && len != 2 && len != 4) { + return false; + } + Breakpoint breakpoint; breakpoint.active = true; breakpoint.addr = addr; breakpoint.len = len; - Core::System::GetInstance().Memory().ReadBlock( - *Core::System::GetInstance().Kernel().GetCurrentProcess(), addr, breakpoint.inst.data(), - breakpoint.inst.size()); + Core::System::GetInstance().Memory().ReadBlock(*current_process, addr, breakpoint.inst.data(), + len); static constexpr std::array btrap{0x70, 0x00, 0x20, 0xe1}; + static constexpr std::array btrap_thumb{0x00, 0xBE}; + if (type == BreakpointType::Execute) { Core::System::GetInstance().Memory().WriteBlock( - *Core::System::GetInstance().Kernel().GetCurrentProcess(), addr, btrap.data(), - btrap.size()); + *current_process, addr, (len == 2) ? btrap_thumb.data() : btrap.data(), len); ClearAllInstructionCache(); + } else { + Core::System::GetInstance().Memory().RegisterWatchpoint(*current_process, addr, len); } p.insert({addr, breakpoint}); @@ -962,6 +1203,11 @@ static bool CommitBreakpoint(BreakpointType type, VAddr addr, u32 len) { /// Handle add breakpoint command from gdb client. static void AddBreakpoint() { + if (!current_process) { + SendReply("E01"); + return; + } + BreakpointType type; u8 type_id = HexCharToValue(command_buffer[1]); @@ -1011,6 +1257,11 @@ static void AddBreakpoint() { /// Handle remove breakpoint command from gdb client. static void RemoveBreakpoint() { + if (!current_process) { + SendReply("E01"); + return; + } + BreakpointType type; u8 type_id = HexCharToValue(command_buffer[1]); @@ -1048,11 +1299,122 @@ static void RemoveBreakpoint() { SendReply("OK"); } +void HandleVCommand() { + std::string_view cmd_view((const char*)command_buffer, command_length); + + if (cmd_view.size() <= 1) { + SendReply("E01"); + return; + } + size_t delimiter_pos = cmd_view.find(';'); + std::string_view command = + cmd_view.substr(1, delimiter_pos == cmd_view.npos ? cmd_view.npos : delimiter_pos); + if (command == "Attach;") { + if (!is_extended_mode) { + SendReply("E01"); + return; + } + + std::string_view arg = cmd_view.substr(delimiter_pos + 1); + u32 pid = HexToInt(reinterpret_cast(arg.data()), arg.size()); + auto process = Core::System::GetInstance().Kernel().GetProcessById(pid); + if (!process) { + SendReply("E02"); + } else { + current_process = process.get(); + current_process->SetDebugBreak(true); + if (SetThread(0)) { + SendSignal(current_thread, 0); + } else { + // Should never happen + SendReply("W00"); + } + } + return; + } else if (command == "Cont?") { + SendReply("vCont;c;C"); + } else if (command == "Cont;") { + if (!current_process) { + SendReply("E01"); + return; + } + + std::string_view arg = cmd_view.substr(delimiter_pos + 1); + auto actions = Common::SplitString(arg, ';'); + + if (actions.empty()) { + SendReply("E01"); + return; + } + for (auto& action : actions) { + auto threads = Common::SplitString(action, ':'); + if (threads.empty()) { + SendReply("E01"); + return; + } + char action_type = threads[0][0]; + if (action_type != 'c') { + SendReply("E01"); + return; + } + std::vector thread_ids; + for (size_t i = 1; i < threads.size(); i++) { + thread_ids.push_back( + HexToInt(reinterpret_cast(threads[i].c_str()), threads[i].size())); + } + + current_process->SetDebugBreak(false, thread_ids); + } + } else { + SendReply(""); + } +} + void HandlePacket(Core::System& system) { + if (!IsConnected()) { if (defer_start) { ToggleServer(true); + defer_start = false; } + + // Handle accept new GDB connection + if (accept_socket != -1) { + sockaddr_in saddr_client; + sockaddr* client_addr = reinterpret_cast(&saddr_client); + socklen_t client_addrlen = sizeof(saddr_client); + gdbserver_socket = + static_cast(accept(accept_socket, client_addr, &client_addrlen)); + if (gdbserver_socket < 0) { +#ifdef _WIN32 + if (GetErrno() == WSAEWOULDBLOCK) { + // Nothing connected yet + return; + } +#else + if (GetErrno() == EAGAIN || GetErrno() == EWOULDBLOCK) { + // Nothing connected yet + return; + } +#endif + LOG_ERROR(Debug_GDBStub, "Failed to accept gdb client"); + } else { + LOG_INFO(Debug_GDBStub, "Client connected.\n"); + SetNonBlock(gdbserver_socket, false); + } + + shutdown(accept_socket, SHUT_RDWR); + accept_socket = -1; + } + return; + } + + if (break_thread) { + current_thread = break_thread; + break_thread = nullptr; + int signal = break_signal; + break_signal = 0; + BreakImpl(signal); return; } @@ -1072,7 +1434,7 @@ void HandlePacket(Core::System& system) { #ifdef PRINT_GDB_TRAFFIC std::string cmd_str(command_buffer + 1, command_buffer + command_length); - LOG_INFO(Debug_GDBStub, "Res: {:c} {}", command_buffer[0], cmd_str); + LOG_INFO(Debug_GDBStub, "Req: {:c} {}", command_buffer[0], cmd_str); #endif LOG_DEBUG(Debug_GDBStub, "Packet: {0:d} ('{0:c}')", command_buffer[0]); @@ -1085,16 +1447,28 @@ void HandlePacket(Core::System& system) { HandleSetThread(); break; case '?': - SendSignal(current_thread, latest_signal); + HandleGetStopReason(); + break; + case '!': + HandleExtendedMode(); + break; + case 'D': + SendReply("OK"); + ToggleServer(false); + // Continue execution + continue_thread = -1; + Continue(); break; case 'k': LOG_INFO(Debug_GDBStub, "killed by gdb"); ToggleServer(false); - // Continue execution so we don't hang forever after shutting down the server + // Continue execution and stop emulation + continue_thread = -1; Continue(); + system.RequestShutdown(); return; case 'F': - HandleHioReply(system, command_buffer, command_length); + HandleHioReply(system, current_process, command_buffer, command_length); break; case 'g': ReadRegisters(); @@ -1115,7 +1489,7 @@ void HandlePacket(Core::System& system) { WriteMemory(); break; case 's': - // Single step, return ENOTSUP + // Single step not supported, return ENOTSUP SendReply("E5F"); return; case 'C': @@ -1131,6 +1505,9 @@ void HandlePacket(Core::System& system) { case 'T': HandleThreadAlive(); break; + case 'v': + HandleVCommand(); + break; default: SendReply(""); break; @@ -1146,12 +1523,12 @@ void ToggleServer(bool status) { server_enabled = status; // Start server - if (!IsConnected() && Core::System::GetInstance().IsPoweredOn()) { + if (!IsInitialized() && Core::System::GetInstance().IsPoweredOn()) { Init(); } } else { // Stop server - if (IsConnected()) { + if (IsInitialized()) { Shutdown(); } @@ -1184,47 +1561,32 @@ static void Init(u16 port) { WSAStartup(MAKEWORD(2, 2), &InitData); #endif - int tmpsock = static_cast(socket(PF_INET, SOCK_STREAM, 0)); - if (tmpsock == -1) { + accept_socket = static_cast(socket(PF_INET, SOCK_STREAM, 0)); + if (accept_socket == -1) { LOG_ERROR(Debug_GDBStub, "Failed to create gdb socket"); } // Set socket to SO_REUSEADDR so it can always bind on the same port int reuse_enabled = 1; - if (setsockopt(tmpsock, SOL_SOCKET, SO_REUSEADDR, (const char*)&reuse_enabled, + if (setsockopt(accept_socket, SOL_SOCKET, SO_REUSEADDR, (const char*)&reuse_enabled, sizeof(reuse_enabled)) < 0) { LOG_ERROR(Debug_GDBStub, "Failed to set gdb socket option"); } const sockaddr* server_addr = reinterpret_cast(&saddr_server); socklen_t server_addrlen = sizeof(saddr_server); - if (bind(tmpsock, server_addr, server_addrlen) < 0) { + if (bind(accept_socket, server_addr, server_addrlen) < 0) { LOG_ERROR(Debug_GDBStub, "Failed to bind gdb socket"); } - if (listen(tmpsock, 1) < 0) { + if (listen(accept_socket, 1) < 0) { LOG_ERROR(Debug_GDBStub, "Failed to listen to gdb socket"); } + SetNonBlock(accept_socket, true); + // Wait for gdb to connect LOG_INFO(Debug_GDBStub, "Waiting for gdb to connect...\n"); - sockaddr_in saddr_client; - sockaddr* client_addr = reinterpret_cast(&saddr_client); - socklen_t client_addrlen = sizeof(saddr_client); - gdbserver_socket = static_cast(accept(tmpsock, client_addr, &client_addrlen)); - if (gdbserver_socket < 0) { - // In the case that we couldn't start the server for whatever reason, just start CPU - // execution like normal. - LOG_ERROR(Debug_GDBStub, "Failed to accept gdb client"); - } else { - LOG_INFO(Debug_GDBStub, "Client connected.\n"); - saddr_client.sin_addr.s_addr = ntohl(saddr_client.sin_addr.s_addr); - } - - // Clean up temporary socket if it's still alive at this point. - if (tmpsock != -1) { - shutdown(tmpsock, SHUT_RDWR); - } } void Init() { @@ -1236,6 +1598,7 @@ void Shutdown() { return; } defer_start = false; + is_extended_mode = false; LOG_INFO(Debug_GDBStub, "Stopping GDB ..."); if (gdbserver_socket != -1) { @@ -1243,6 +1606,11 @@ void Shutdown() { gdbserver_socket = -1; } + if (accept_socket != -1) { + shutdown(accept_socket, SHUT_RDWR); + accept_socket = -1; + } + #ifdef _WIN32 WSACleanup(); #endif @@ -1254,19 +1622,11 @@ bool IsServerEnabled() { return server_enabled; } +bool IsInitialized() { + return IsServerEnabled() && (accept_socket != -1 || gdbserver_socket != -1); +} + bool IsConnected() { return IsServerEnabled() && gdbserver_socket != -1; } - -void SendTrap(Kernel::Thread* thread, int trap) { - if (!send_trap) { - return; - } - - current_thread = thread; - - SendSignal(thread, trap); - - send_trap = false; -} }; // namespace GDBStub diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h index 8a021580b..14345aa77 100644 --- a/src/core/gdbstub/gdbstub.h +++ b/src/core/gdbstub/gdbstub.h @@ -1,3 +1,7 @@ +// Copyright Citra Emulator Project / Azahar Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + // Copyright 2013 Dolphin Emulator Project // Licensed under GPLv2+ // Refer to the license.txt file included. @@ -60,18 +64,18 @@ void Shutdown(); /// Checks if the gdbstub server is enabled. bool IsServerEnabled(); +/// Returns true if the GDB server is initialized +bool IsInitialized(); + /// Returns true if there is an active socket connection. bool IsConnected(); /** * Signal to the gdbstub server that it should halt CPU execution. * - * @param is_memory_break If true, the break resulted from a memory breakpoint. + * @param signal Signal that produced the break (SIGTRAP by default) */ -void Break(bool is_memory_break = false); - -/// Determine if there was a memory breakpoint. -bool IsMemoryBreak(); +void Break(int signal); /// Read and handle packet from gdb client. void HandlePacket(Core::System& system); @@ -88,17 +92,10 @@ BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, GDBStub::BreakpointTy * Check if a breakpoint of the specified type exists at the given address. * * @param addr Address of breakpoint. + * @param access_len Access size in bytes. * @param type Type of breakpoint. */ -bool CheckBreakpoint(VAddr addr, GDBStub::BreakpointType type); - -/** - * 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); +bool CheckBreakpoint(VAddr addr, u32 access_len, BreakpointType type); /** * Send reply to gdb client. diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp index cbc528672..2d6c8058a 100644 --- a/src/core/gdbstub/hio.cpp +++ b/src/core/gdbstub/hio.cpp @@ -1,13 +1,18 @@ -// Copyright 2023 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include #include "common/string_util.h" #include "core/core.h" #include "core/gdbstub/gdbstub.h" #include "core/gdbstub/hio.h" +#ifndef SIGTRAP +constexpr u32 SIGTRAP = 5; +#endif + namespace GDBStub { namespace { @@ -51,7 +56,7 @@ static void SendErrorReply(int error_code, int retval = -1) { SendReply(packet.data()); } -void SetHioRequest(Core::System& system, const VAddr addr) { +void SetHioRequest(Core::System& system, Kernel::Process* process, const VAddr addr) { if (!IsServerEnabled()) { LOG_WARNING(Debug_GDBStub, "HIO requested but GDB stub is not running"); return; @@ -67,14 +72,13 @@ void SetHioRequest(Core::System& system, const VAddr addr) { } auto& memory = system.Memory(); - const auto process = system.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)); + memory.ReadBlock(*process, addr, ¤t_hio_request, sizeof(PackedGdbHioRequest)); if (current_hio_request.magic != std::array{'G', 'D', 'B', '\0'}) { std::string_view bad_magic{ @@ -96,11 +100,11 @@ void SetHioRequest(Core::System& system, const VAddr addr) { // Now halt, so that no further instructions are executed until the request // is processed by the client. We will continue after the reply comes back - Break(); + Break(SIGTRAP); system.GetRunningCore().ClearInstructionCache(); } -void HandleHioReply(Core::System& system, const u8* const command_buffer, +void HandleHioReply(Core::System& system, Kernel::Process* process, const u8* const command_buffer, const u32 command_length) { if (!IsWaitingForHioReply()) { LOG_WARNING(Debug_GDBStub, "Got HIO reply but never sent a request"); @@ -169,7 +173,6 @@ void HandleHioReply(Core::System& system, const u8* const command_buffer, current_hio_request.retval, current_hio_request.gdb_errno, current_hio_request.ctrl_c); - const auto process = system.Kernel().GetCurrentProcess(); auto& memory = system.Memory(); // should have been checked when we first initialized the request, @@ -180,7 +183,8 @@ void HandleHioReply(Core::System& system, const u8* const command_buffer, return; } - memory.WriteBlock(current_hio_request_addr, ¤t_hio_request, sizeof(PackedGdbHioRequest)); + memory.WriteBlock(*process, current_hio_request_addr, ¤t_hio_request, + sizeof(PackedGdbHioRequest)); current_hio_request = {}; current_hio_request_addr = 0; diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h index 827521841..5fe366bb4 100644 --- a/src/core/gdbstub/hio.h +++ b/src/core/gdbstub/hio.h @@ -1,4 +1,4 @@ -// Copyright 2023 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -10,6 +10,10 @@ namespace Core { class System; } +namespace Kernel { +class Process; +} + namespace GDBStub { /** @@ -51,7 +55,7 @@ static_assert(sizeof(PackedGdbHioRequest) == 152, * * @param address The memory address of the \ref PackedGdbHioRequest. */ -void SetHioRequest(Core::System& system, const VAddr address); +void SetHioRequest(Core::System& system, Kernel::Process* process, const VAddr address); /** * If there is a pending HIO request, send it to the client. @@ -63,6 +67,7 @@ bool HandlePendingHioRequestPacket(); /** * Process an HIO reply from the client. */ -void HandleHioReply(Core::System& system, const u8* const command_buffer, const u32 command_length); +void HandleHioReply(Core::System& system, Kernel::Process* process, const u8* const command_buffer, + const u32 command_length); } // namespace GDBStub diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index 5e0d3810f..a808bb2ab 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -592,6 +592,42 @@ Result Process::Unmap(VAddr target, VAddr source, u32 size, VMAPermission perms, return ResultSuccess; } +std::vector> Kernel::Process::GetThreadList() { + std::vector> ret; + for (u32 core = 0; core < Core::GetNumCores(); core++) { + auto thread_list = kernel.GetThreadManager(core).GetThreadList(); + for (auto& thread : thread_list) { + if (thread->owner_process.lock().get() == this) { + ret.push_back(thread); + } + } + } + return ret; +} + +void Kernel::Process::SetDebugBreak(bool debug_break, std::vector thread_ids) { + auto thread_list = GetThreadList(); + bool needs_reschedule = false; + for (auto& t : thread_list) { + + if (!thread_ids.empty()) { + u32 thread_id = t->thread_id; + if (std::find(thread_ids.begin(), thread_ids.end(), thread_id) == thread_ids.end()) { + continue; + } + } + + needs_reschedule |= t->SetDebugBreak(debug_break); + } + + if (needs_reschedule) { + for (u32 i = 0; i < Core::GetNumCores(); i++) { + Core::GetCore(i).PrepareReschedule(); + kernel.GetThreadManager(i).Reschedule(); + } + } +} + void Process::FreeAllMemory() { if (memory_region == nullptr || resource_limit == nullptr) { return; diff --git a/src/core/hle/kernel/process.h b/src/core/hle/kernel/process.h index 7ec1adfbb..4067839cd 100644 --- a/src/core/hle/kernel/process.h +++ b/src/core/hle/kernel/process.h @@ -1,4 +1,4 @@ -// Copyright 2015 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. @@ -226,6 +226,10 @@ public: Result Unmap(VAddr target, VAddr source, u32 size, VMAPermission perms, bool privileged = false); + std::vector> GetThreadList(); + + void SetDebugBreak(bool debug_break, std::vector thread_ids = {}); + private: void FreeAllMemory(); diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 833543d9a..a02e91427 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1171,7 +1171,7 @@ void SVC::OutputDebugString(VAddr address, s32 len) { } if (len == 0) { - GDBStub::SetHioRequest(system, address); + GDBStub::SetHioRequest(system, kernel.GetCurrentProcess().get(), address); return; } diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 41a706fdd..a3cf78ec6 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -77,6 +77,7 @@ void Thread::serialize(Archive& ar, const unsigned int file_version) { } } ar & wakeup_callback; + ar & debug_break; } SERIALIZE_IMPL(Thread) @@ -191,7 +192,7 @@ Thread* ThreadManager::PopNextReadyThread() { u32 next_priority{}; next = nullptr; - if (thread && thread->status == ThreadStatus::Running) { + if (thread && thread->status == ThreadStatus::Running && thread->CanSchedule()) { do { // We have to do better than the current thread. // This call returns null when that's not possible. @@ -201,18 +202,18 @@ Thread* ThreadManager::PopNextReadyThread() { // Otherwise just keep going with the current thread next = thread; break; - } else if (!next->can_schedule) { + } else if (!next->CanSchedule()) { skipped.push_back({next_priority, next}); } - } while (!next->can_schedule); + } while (!next->CanSchedule()); } else { do { std::tie(next_priority, next) = ready_queue.pop_first(); - if (next && !next->can_schedule) { + if (next && !next->CanSchedule()) { skipped.push_back({next_priority, next}); } - } while (next && !next->can_schedule); + } while (next && !next->CanSchedule()); } for (auto it = skipped.rbegin(); it != skipped.rend(); it++) { @@ -537,6 +538,14 @@ VAddr Thread::GetCommandBufferAddress() const { return GetTLSAddress() + command_header_offset; } +bool Thread::SetDebugBreak(bool _debug_break) { + if (debug_break == _debug_break) { + return false; + } + debug_break = _debug_break; + return true; +} + CpuLimiter::~CpuLimiter() {} CpuLimiterMulti::CpuLimiterMulti(Kernel::KernelSystem& _kernel) : kernel(_kernel) {} diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index be515319b..49891b31a 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -365,6 +365,15 @@ public: return status == ThreadStatus::WaitSynchAll; } + bool CanSchedule() { + // TODO(PabloMK7): This may not be the proper way + // threads are marked as non-schedulable when they + // are in debug break. Figure out and fix. + return can_schedule && !debug_break; + } + + bool SetDebugBreak(bool debug_break); + Core::ARM_Interface::ThreadContext context{}; u32 thread_id; @@ -410,6 +419,7 @@ public: private: ThreadManager& thread_manager; + bool debug_break{}; friend class boost::serialization::access; template diff --git a/src/core/memory.cpp b/src/core/memory.cpp index 41ad049e5..47416ab87 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include #include #include @@ -16,6 +17,7 @@ #include "common/swap.h" #include "core/arm/arm_interface.h" #include "core/core.h" +#include "core/gdbstub/gdbstub.h" #include "core/global.h" #include "core/hle/kernel/process.h" #include "core/hle/service/plgldr/plgldr.h" @@ -28,6 +30,14 @@ SERIALIZE_EXPORT_IMPL(Memory::MemorySystem::BackingMemImpl SERIALIZE_EXPORT_IMPL(Memory::MemorySystem::BackingMemImpl) SERIALIZE_EXPORT_IMPL(Memory::MemorySystem::BackingMemImpl) +#ifndef SIGTRAP +constexpr u32 SIGTRAP = 5; +#endif + +#ifndef SIGSEGV +constexpr u32 SIGSEGV = 11; +#endif + namespace Memory { void PageTable::Clear() { @@ -187,7 +197,17 @@ public: std::memcpy(dest_buffer, src_ptr, copy_amount); break; } - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = page_table.watchpoint_pages_map.find(page_index); + ASSERT_MSG(it != page_table.watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + const u8* src_ptr = it->second.memory.GetPtr() + page_offset; + std::memcpy(dest_buffer, src_ptr, copy_amount); + break; + } + case PageType::RasterizerCachedMemory: + case PageType::RasterizerCachedMemoryWatchpoint: { if constexpr (!UNSAFE) { RasterizerFlushVirtualRegion(current_vaddr, static_cast(copy_amount), FlushMode::Flush); @@ -235,7 +255,17 @@ public: std::memcpy(dest_ptr, src_buffer, copy_amount); break; } - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = page_table.watchpoint_pages_map.find(page_index); + ASSERT_MSG(it != page_table.watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + u8* dest_ptr = it->second.memory.GetPtr() + page_offset; + std::memcpy(dest_ptr, src_buffer, copy_amount); + break; + } + case PageType::RasterizerCachedMemory: + case PageType::RasterizerCachedMemoryWatchpoint: { if constexpr (!UNSAFE) { RasterizerFlushVirtualRegion(current_vaddr, static_cast(copy_amount), FlushMode::Invalidate); @@ -392,6 +422,90 @@ PAddr& Memory::MemorySystem::Plugin3GXFramebufferAddress() { return impl->plugin_fb_address; } +#pragma optimize("", off) + +void MemorySystem::RegisterWatchpoint(const Kernel::Process& process, VAddr addr, u32 size) { + auto& page_table = *process.vm_manager.page_table; + + VAddr current = addr; + VAddr end = addr + size; + + while (current < end) { + const VAddr page_base = (current & ~CITRA_PAGE_MASK); + const VAddr page_index = page_base >> CITRA_PAGE_BITS; + + auto it = page_table.watchpoint_pages_map.find(page_index); + if (it != page_table.watchpoint_pages_map.end()) { + // Nothing to do, only increment count. + it->second.watchpoint_count++; + } else { + MemoryRef mem; + PageType& type = page_table.attributes[page_index]; + + switch (type) { + case PageType::Memory: + mem = page_table.pointers.Ref(page_index); + type = PageType::MemoryWatchpoint; + page_table.pointers[page_index] = nullptr; + break; + case PageType::RasterizerCachedMemory: + mem = GetPointerForRasterizerCache(page_base); + type = PageType::RasterizerCachedMemoryWatchpoint; + break; + default: + LOG_ERROR(HW_Memory, "Cannot get pointer to register watchpoint for page 0x{:08X}", + page_base); + continue; + } + + page_table.watchpoint_pages_map.insert( + {page_index, + PageTable::WatchpointPageInfo{.watchpoint_count = 1, .memory = std::move(mem)}}); + } + + current = page_base + CITRA_PAGE_SIZE; + } +} + +void MemorySystem::UnregisterWatchpoint(const Kernel::Process& process, VAddr addr, u32 size) { + auto& page_table = *process.vm_manager.page_table; + + VAddr current = addr; + VAddr end = addr + size; + + while (current < end) { + const VAddr page_base = (current & ~CITRA_PAGE_MASK); + const VAddr page_index = page_base >> CITRA_PAGE_BITS; + + auto it = page_table.watchpoint_pages_map.find(page_index); + if (it != page_table.watchpoint_pages_map.end()) { + if (--it->second.watchpoint_count == 0) { + + PageType& type = page_table.attributes[page_index]; + + switch (type) { + case PageType::MemoryWatchpoint: + type = PageType::Memory; + page_table.pointers[page_index] = it->second.memory; + break; + case PageType::RasterizerCachedMemoryWatchpoint: + type = PageType::RasterizerCachedMemory; + break; + default: + LOG_ERROR(HW_Memory, "Invalid watchpoint page type for page 0x{:08X}: {}", + page_base, static_cast(type)); + } + + page_table.watchpoint_pages_map.erase(page_index); + } + } else { + LOG_ERROR(HW_Memory, "No watchpoint found on page 0x{:08X}", page_base); + } + + current = page_base + CITRA_PAGE_SIZE; + } +} + void MemorySystem::MapPages(PageTable& page_table, u32 base, u32 size, MemoryRef memory, PageType type) { LOG_DEBUG(HW_Memory, "Mapping {} onto {:08X}-{:08X}", (void*)memory.GetPtr(), @@ -449,6 +563,23 @@ void MemorySystem::UnregisterPageTable(std::shared_ptr page_table) { } } +template +T MemorySystem::UnmappedAccess(const VAddr vaddr, const T value, bool read) { + const std::string mode = (read ? "Read" : "Write"); + const std::string value_str = read ? std::string("") : fmt::format(" 0x{:08X}", value); + const std::string message = fmt::format("unmapped {}{}{} @ 0x{:08X} at PC 0x{:08X}", mode, + sizeof(T) * 8, value_str, vaddr, impl->GetPC()); + if (GDBStub::IsConnected()) { + GDBStub::Break(SIGSEGV); + } else if (Settings::values.break_on_unmapped_memory_access) { + impl->system.SetStatus(Core::System::ResultStatus::ErrorMemoryExceptionRaised, + message.c_str()); + } + + LOG_ERROR(HW_Memory, "{}", message); + return {}; +} + template T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr vaddr) { const u8* page_pointer = page_table->pointers[vaddr >> CITRA_PAGE_BITS]; @@ -476,20 +607,45 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v PageType type = page_table->attributes[vaddr >> CITRA_PAGE_BITS]; switch (type) { - case PageType::Unmapped: - LOG_ERROR(HW_Memory, "unmapped Read{} @ 0x{:08X} at PC 0x{:08X}", sizeof(T) * 8, vaddr, - impl->GetPC()); - return 0; + case PageType::Unmapped: { + return UnmappedAccess(vaddr, 0, true); + } case PageType::Memory: ASSERT_MSG(false, "Mapped memory page without a pointer @ {:08X}", vaddr); break; - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = page_table->watchpoint_pages_map.find(vaddr >> CITRA_PAGE_BITS); + ASSERT_MSG(it != page_table->watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + T value; + std::memcpy(&value, it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK), sizeof(T)); + + if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Read)) { + GDBStub::Break(SIGTRAP); + } + + return value; + } + [[likely]] case PageType::RasterizerCachedMemory: { RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Flush); T value; std::memcpy(&value, GetPointerForRasterizerCache(vaddr), sizeof(T)); return value; } + case PageType::RasterizerCachedMemoryWatchpoint: { + RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Flush); + + T value; + std::memcpy(&value, GetPointerForRasterizerCache(vaddr), sizeof(T)); + + if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Read)) { + GDBStub::Break(SIGTRAP); + } + + return value; + } default: UNREACHABLE(); } @@ -527,17 +683,39 @@ void MemorySystem::Write(const std::shared_ptr& page_table, const VAd PageType type = page_table->attributes[vaddr >> CITRA_PAGE_BITS]; switch (type) { case PageType::Unmapped: - LOG_ERROR(HW_Memory, "unmapped Write{} 0x{:08X} @ 0x{:08X} at PC 0x{:08X}", - sizeof(data) * 8, (u32)data, vaddr, impl->GetPC()); + (void)UnmappedAccess(vaddr, data, false); return; case PageType::Memory: ASSERT_MSG(false, "Mapped memory page without a pointer @ {:08X}", vaddr); break; - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = page_table->watchpoint_pages_map.find(vaddr >> CITRA_PAGE_BITS); + ASSERT_MSG(it != page_table->watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + std::memcpy(it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK), &data, sizeof(T)); + + if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { + GDBStub::Break(SIGTRAP); + } + + break; + } + [[likely]] case PageType::RasterizerCachedMemory: { RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Invalidate); std::memcpy(GetPointerForRasterizerCache(vaddr), &data, sizeof(T)); break; } + case PageType::RasterizerCachedMemoryWatchpoint: { + RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Invalidate); + std::memcpy(GetPointerForRasterizerCache(vaddr), &data, sizeof(T)); + + if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { + GDBStub::Break(SIGTRAP); + } + + break; + } default: UNREACHABLE(); } @@ -556,18 +734,44 @@ bool MemorySystem::WriteExclusive(const VAddr vaddr, const T data, const T expec PageType type = impl->current_page_table->attributes[vaddr >> CITRA_PAGE_BITS]; switch (type) { case PageType::Unmapped: - LOG_ERROR(HW_Memory, "unmapped Write{} 0x{:08X} @ 0x{:08X} at PC 0x{:08X}", - sizeof(data) * 8, static_cast(data), vaddr, impl->GetPC()); + (void)UnmappedAccess(vaddr, data, false); return true; case PageType::Memory: ASSERT_MSG(false, "Mapped memory page without a pointer @ {:08X}", vaddr); return true; - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = impl->current_page_table->watchpoint_pages_map.find(vaddr >> CITRA_PAGE_BITS); + ASSERT_MSG(it != impl->current_page_table->watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + const auto volatile_pointer = + reinterpret_cast(it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK)); + + bool ret = Common::AtomicCompareAndSwap(volatile_pointer, data, expected); + + if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { + GDBStub::Break(SIGTRAP); + } + + return ret; + } + [[likely]] case PageType::RasterizerCachedMemory: { RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Invalidate); const auto volatile_pointer = reinterpret_cast(GetPointerForRasterizerCache(vaddr).GetPtr()); return Common::AtomicCompareAndSwap(volatile_pointer, data, expected); } + case PageType::RasterizerCachedMemoryWatchpoint: { + RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Invalidate); + const auto volatile_pointer = + reinterpret_cast(GetPointerForRasterizerCache(vaddr).GetPtr()); + + if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { + GDBStub::Break(SIGTRAP); + } + + return Common::AtomicCompareAndSwap(volatile_pointer, data, expected); + } default: UNREACHABLE(); } @@ -582,7 +786,7 @@ bool MemorySystem::IsValidVirtualAddress(const Kernel::Process& process, const V return true; } - if (page_table.attributes[vaddr >> CITRA_PAGE_BITS] == PageType::RasterizerCachedMemory) { + if (page_table.attributes[vaddr >> CITRA_PAGE_BITS] != PageType::Unmapped) { return true; } @@ -600,7 +804,9 @@ u8* MemorySystem::GetPointer(const VAddr vaddr) { } if (impl->current_page_table->attributes[vaddr >> CITRA_PAGE_BITS] == - PageType::RasterizerCachedMemory) { + PageType::RasterizerCachedMemory || + impl->current_page_table->attributes[vaddr >> CITRA_PAGE_BITS] == + PageType::RasterizerCachedMemoryWatchpoint) { return GetPointerForRasterizerCache(vaddr); } @@ -615,7 +821,9 @@ const u8* MemorySystem::GetPointer(const VAddr vaddr) const { } if (impl->current_page_table->attributes[vaddr >> CITRA_PAGE_BITS] == - PageType::RasterizerCachedMemory) { + PageType::RasterizerCachedMemory || + impl->current_page_table->attributes[vaddr >> CITRA_PAGE_BITS] == + PageType::RasterizerCachedMemoryWatchpoint) { return GetPointerForRasterizerCache(vaddr); } @@ -755,7 +963,10 @@ void MemorySystem::RasterizerMarkRegionCached(PAddr start, u32 size, bool cached // address space, for example, a system module need not have a VRAM mapping. break; case PageType::Memory: - page_type = PageType::RasterizerCachedMemory; + case PageType::MemoryWatchpoint: + page_type = (page_type == PageType::Memory) + ? PageType::RasterizerCachedMemory + : PageType::RasterizerCachedMemoryWatchpoint; page_table->pointers[vaddr >> CITRA_PAGE_BITS] = nullptr; break; default: @@ -768,10 +979,16 @@ void MemorySystem::RasterizerMarkRegionCached(PAddr start, u32 size, bool cached // It is not necessary for a process to have this region mapped into its // address space, for example, a system module need not have a VRAM mapping. break; - case PageType::RasterizerCachedMemory: { - page_type = PageType::Memory; - page_table->pointers[vaddr >> CITRA_PAGE_BITS] = - GetPointerForRasterizerCache(vaddr & ~CITRA_PAGE_MASK); + case PageType::RasterizerCachedMemory: + case PageType::RasterizerCachedMemoryWatchpoint: { + page_type = (page_type == PageType::RasterizerCachedMemory) + ? PageType::Memory + : PageType::MemoryWatchpoint; + + if (page_type == PageType::Memory) { + page_table->pointers[vaddr >> CITRA_PAGE_BITS] = + GetPointerForRasterizerCache(vaddr & ~CITRA_PAGE_MASK); + } break; } default: @@ -911,7 +1128,17 @@ void MemorySystem::ZeroBlock(const Kernel::Process& process, const VAddr dest_ad std::memset(dest_ptr, 0, copy_amount); break; } - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = page_table.watchpoint_pages_map.find(page_index); + ASSERT_MSG(it != page_table.watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + u8* dest_ptr = it->second.memory.GetPtr() + page_offset; + std::memset(dest_ptr, 0, copy_amount); + break; + } + case PageType::RasterizerCachedMemory: + case PageType::RasterizerCachedMemoryWatchpoint: { RasterizerFlushVirtualRegion(current_vaddr, static_cast(copy_amount), FlushMode::Invalidate); std::memset(GetPointerForRasterizerCache(current_vaddr), 0, copy_amount); @@ -960,7 +1187,17 @@ void MemorySystem::CopyBlock(const Kernel::Process& dest_process, WriteBlock(dest_process, dest_addr, src_ptr, copy_amount); break; } - case PageType::RasterizerCachedMemory: { + case PageType::MemoryWatchpoint: { + auto it = page_table.watchpoint_pages_map.find(page_index); + ASSERT_MSG(it != page_table.watchpoint_pages_map.end(), + "Missing memory for watchpoint page"); + + const u8* src_ptr = it->second.memory.GetPtr() + page_offset; + WriteBlock(dest_process, dest_addr, src_ptr, copy_amount); + break; + } + case PageType::RasterizerCachedMemory: + case PageType::RasterizerCachedMemoryWatchpoint: { RasterizerFlushVirtualRegion(current_vaddr, static_cast(copy_amount), FlushMode::Flush); WriteBlock(dest_process, dest_addr, GetPointerForRasterizerCache(current_vaddr), diff --git a/src/core/memory.h b/src/core/memory.h index 5c215b3f2..1433ad7d8 100644 --- a/src/core/memory.h +++ b/src/core/memory.h @@ -34,7 +34,7 @@ constexpr u32 CITRA_PAGE_MASK = CITRA_PAGE_SIZE - 1; constexpr int CITRA_PAGE_BITS = 12; constexpr std::size_t PAGE_TABLE_NUM_ENTRIES = 1 << (32 - CITRA_PAGE_BITS); -enum class PageType { +enum class PageType : u8 { /// Page is unmapped and should cause an access error. Unmapped, /// Page is mapped to regular memory. This is the only type you can get pointers to. @@ -42,6 +42,12 @@ enum class PageType { /// Page is mapped to regular memory, but also needs to check for rasterizer cache flushing and /// invalidation RasterizerCachedMemory, + /// Page is mapped to regular memory. Furthermore a debug watchpoint is set to an address within + /// the page. + MemoryWatchpoint, + /// Page is mapped to regular memory, but also needs to check for rasterizer cache flushing and + /// invalidation. Furthermore a debug watchpoint is set to an address within the page. + RasterizerCachedMemoryWatchpoint, }; /** @@ -82,6 +88,10 @@ struct PageTable { return Entry(*this, static_cast(idx)); } + const MemoryRef& Ref(std::size_t idx) { + return refs[idx]; + } + private: std::array raw; std::array refs; @@ -100,6 +110,22 @@ struct PageTable { return pointers.raw; } + struct WatchpointPageInfo { + u32 watchpoint_count{}; + MemoryRef memory; + + template + void serialize(Archive& ar, const unsigned int) { + ar & watchpoint_count; + ar & memory; + } + }; + + // Map holding pages that are marked to contain watchpoints. We don't need + // any fancy performance tricks here, as watchpoints are only used rarely + // while debugging and performance is not a priority in such cases. + std::unordered_map watchpoint_pages_map{}; + void Clear(); private: @@ -107,6 +133,7 @@ private: void serialize(Archive& ar, const unsigned int) { ar & pointers.refs; ar & attributes; + ar & watchpoint_pages_map; for (std::size_t i = 0; i < PAGE_TABLE_NUM_ENTRIES; i++) { pointers.raw[i] = pointers.refs[i].GetPtr(); } @@ -649,7 +676,14 @@ public: /// Returns a reference to the framebuffer address of the currently loaded 3GX plugin. PAddr& Plugin3GXFramebufferAddress(); + void RegisterWatchpoint(const Kernel::Process& process, VAddr addr, u32 size); + + void UnregisterWatchpoint(const Kernel::Process& process, VAddr addr, u32 size); + private: + template + T UnmappedAccess(const VAddr vaddr, const T value, bool read); + template T Read(const std::shared_ptr& page_table, const VAddr vaddr); From 46b118fbeb9ae8e3fd1df58a3075302ac0334c26 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Sun, 3 May 2026 13:42:49 +0200 Subject: [PATCH 4/9] gdb: Cleanup code --- src/core/arm/arm_interface.h | 7 + src/core/arm/dynarmic/arm_dynarmic.h | 4 + src/core/arm/dyncom/arm_dyncom.h | 4 + src/core/core.cpp | 4 - src/core/gdbstub/gdbstub.cpp | 218 ++++++++++++++++++--------- 5 files changed, 162 insertions(+), 75 deletions(-) diff --git a/src/core/arm/arm_interface.h b/src/core/arm/arm_interface.h index 0aeb10de2..ceb945148 100644 --- a/src/core/arm/arm_interface.h +++ b/src/core/arm/arm_interface.h @@ -186,6 +186,13 @@ public: /// Prepare core for thread reschedule (if needed to correctly handle state) virtual void PrepareReschedule() = 0; + /** + * Whether the backend allows to break with single instruction accuracy + * when Run() is used. If false is returned, the user should expect + * innaccuracies with memory watchpoints access exceptions. + */ + virtual bool HasSingleInstructionBreakAccuracy() = 0; + Core::Timing::Timer& GetTimer() { return *timer; } diff --git a/src/core/arm/dynarmic/arm_dynarmic.h b/src/core/arm/dynarmic/arm_dynarmic.h index e854df376..713bd99db 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.h +++ b/src/core/arm/dynarmic/arm_dynarmic.h @@ -56,6 +56,10 @@ public: void ClearExclusiveState() override; void SetPageTable(const std::shared_ptr& page_table) override; + bool HasSingleInstructionBreakAccuracy() override { + return false; + } + protected: std::shared_ptr GetPageTable() const override; diff --git a/src/core/arm/dyncom/arm_dyncom.h b/src/core/arm/dyncom/arm_dyncom.h index 3fd37989a..c13828176 100644 --- a/src/core/arm/dyncom/arm_dyncom.h +++ b/src/core/arm/dyncom/arm_dyncom.h @@ -51,6 +51,10 @@ public: void SetPageTable(const std::shared_ptr& page_table) override; void PrepareReschedule() override; + bool HasSingleInstructionBreakAccuracy() override { + return true; + } + protected: std::shared_ptr GetPageTable() const override; diff --git a/src/core/core.cpp b/src/core/core.cpp index e76fd213a..a40a93c54 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -84,10 +84,6 @@ System::ResultStatus System::RunLoop(bool tight_loop) { } if (GDBStub::IsServerEnabled()) { - Kernel::Thread* thread = kernel->GetCurrentThreadManager().GetCurrentThread(); - if (thread && running_core) { - running_core->SaveContext(thread->context); - } // The break flag is only set if GDB is connected, // we can do clearing here safely. If it is ever // used outside, move the clearing outside the if. diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index a1d0f8e73..3272e3ba7 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -73,16 +73,27 @@ constexpr u32 MSG_WAITALL = 8; constexpr u32 SIGSEGV = 11; #endif +#ifdef _WIN32 +using SOCKET = UINT_PTR; +#else +using SOCKET = int; +#endif // _WIN32 + +#define INVALID_SOCKET ((SOCKET)(~0)) + constexpr u32 SP_REGISTER = 13; constexpr u32 LR_REGISTER = 14; constexpr u32 PC_REGISTER = 15; constexpr u32 CPSR_REGISTER = 25; constexpr u32 D0_REGISTER = 26; constexpr u32 FPSCR_REGISTER = 42; +constexpr u32 FPEXC_REGISTER = 43; // 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 +// The CPSR is register 25, rather than register 16, because the FPA registers historically were +// placed between the PC and the CPSR in the "g" packet. constexpr char target_xml[] = R"(l @@ -106,11 +117,6 @@ constexpr char target_xml[] = - - - @@ -131,15 +137,17 @@ constexpr char target_xml[] = + )"; -int gdbserver_socket = -1; +SOCKET gdbserver_socket = INVALID_SOCKET; bool defer_start = false; u8 command_buffer[GDB_BUFFER_SIZE]; -u32 command_length; +u32 recv_command_length; +u32 send_command_length; u32 latest_signal = 0; @@ -156,7 +164,7 @@ bool is_extended_mode = false; // If set to false, the server will never be started and no // gdbstub-related functions will be executed. std::atomic server_enabled(false); -int accept_socket = -1; +SOCKET accept_socket = INVALID_SOCKET; int continue_thread = -1; static Kernel::Thread* break_thread = nullptr; @@ -179,6 +187,32 @@ BreakpointMap breakpoints_read; BreakpointMap breakpoints_write; } // Anonymous namespace +static void ResetState() { + gdbserver_socket = INVALID_SOCKET; + defer_start = false; + + memset(command_buffer, 0, GDB_BUFFER_SIZE); + recv_command_length = 0; + send_command_length = 0; + + latest_signal = 0; + + current_thread = nullptr; + current_process = nullptr; + + is_extended_mode = false; + + accept_socket = INVALID_SOCKET; + continue_thread = -1; + + break_thread = nullptr; + break_signal = 0; + + breakpoints_execute.clear(); + breakpoints_read.clear(); + breakpoints_write.clear(); +} + static Kernel::Thread* FindThreadById(int id) { if (!current_process) { return nullptr; @@ -231,6 +265,8 @@ static u64 FpuRead(std::size_t id, Kernel::Thread* thread = nullptr) { return ret; } else if (id == FPSCR_REGISTER) { return thread->context.fpscr; + } else if (id == FPEXC_REGISTER) { + return thread->context.fpexc; } else { return 0; } @@ -246,6 +282,8 @@ static void FpuWrite(std::size_t id, u64 val, Kernel::Thread* thread = nullptr) thread->context.fpu_registers[2 * (id - D0_REGISTER) + 1] = static_cast(val >> 32); } else if (id == FPSCR_REGISTER) { thread->context.fpscr = static_cast(val); + } else if (id == FPEXC_REGISTER) { + thread->context.fpexc = static_cast(val); } } @@ -392,7 +430,7 @@ static int GetErrno() { #endif } -static bool SetNonBlock(int socket, bool nonblock) { +static bool SetNonBlock(SOCKET socket, bool nonblock) { #ifdef _WIN32 unsigned long nonblocking = nonblock ? 1 : 0; int ret = ioctlsocket(socket, FIONBIO, &nonblocking); @@ -478,6 +516,21 @@ static void RemoveBreakpoint(BreakpointType type, VAddr addr) { p.erase(addr); } +void RemoveAllBreakpoints() { + std::vector> trash_bin; + + for (auto type : {BreakpointType::Execute, BreakpointType::Read, BreakpointType::Write}) { + auto& map = GetBreakpointMap(type); + for (auto b : map) { + trash_bin.push_back({type, b.first}); + } + } + + for (auto& p : trash_bin) { + RemoveBreakpoint(p.first, p.second); + } +} + BreakpointAddress GetNextBreakpointFromAddress(VAddr addr, BreakpointType type) { const BreakpointMap& p = GetBreakpointMap(type); const auto next_breakpoint = p.lower_bound(addr); @@ -544,6 +597,13 @@ bool CheckBreakpoint(VAddr addr, u32 access_len, BreakpointType type) { "breakpoint range: {:08x} - {:08x}", type, addr, addr, access_end, bp.addr, bp_end); + if (type != BreakpointType::Execute && + !Core::GetCore(0).HasSingleInstructionBreakAccuracy()) { + LOG_WARNING(Debug_GDBStub, + "The current CPU backend does not support accurate watchpoints and " + "memory exceptions. Disable CPU JIT for more accuracy."); + } + return true; } } @@ -570,27 +630,27 @@ void SendReply(const char* reply) { std::memset(command_buffer, 0, sizeof(command_buffer)); - command_length = static_cast(strlen(reply)); + send_command_length = static_cast(strlen(reply)); #ifdef PRINT_GDB_TRAFFIC LOG_INFO(Debug_GDBStub, "Res: {}", reply); #endif - if (command_length + 4 > sizeof(command_buffer)) { + if (send_command_length + 4 > sizeof(command_buffer)) { LOG_ERROR(Debug_GDBStub, "command_buffer overflow in SendReply"); return; } - std::memcpy(command_buffer + 1, reply, command_length); + std::memcpy(command_buffer + 1, reply, send_command_length); - u8 checksum = CalculateChecksum(command_buffer, command_length + 1); + u8 checksum = CalculateChecksum(command_buffer, send_command_length + 1); command_buffer[0] = GDB_STUB_START; - command_buffer[command_length + 1] = GDB_STUB_END; - command_buffer[command_length + 2] = NibbleToHex(checksum >> 4); - command_buffer[command_length + 3] = NibbleToHex(checksum); + command_buffer[send_command_length + 1] = GDB_STUB_END; + command_buffer[send_command_length + 2] = NibbleToHex(checksum >> 4); + command_buffer[send_command_length + 3] = NibbleToHex(checksum); u8* ptr = command_buffer; - u32 left = command_length + 4; + u32 left = send_command_length + 4; while (left > 0) { s32 sent_size = static_cast(send(gdbserver_socket, reinterpret_cast(ptr), left, 0)); @@ -615,7 +675,7 @@ static void HandleQuery() { SendReply("1"); } else if (strncmp(query, "Supported", strlen("Supported")) == 0) { // PacketSize needs to be large enough for target xml - SendReply("PacketSize=2000;qXfer:features:read+;qXfer:osdata:read+;qXfer:threads:read+;" + SendReply("PacketSize=9800;qXfer:features:read+;qXfer:osdata:read+;qXfer:threads:read+;" "vContSupported+"); } else if (strncmp(query, "Xfer:features:read:target.xml:", strlen("Xfer:features:read:target.xml:")) == 0) { @@ -698,7 +758,7 @@ static bool SetThread(int thread_id) { static void HandleSetThread() { int thread_id = -1; if (command_buffer[2] != '-') { - thread_id = static_cast(HexToInt(command_buffer + 2, command_length - 2)); + thread_id = static_cast(HexToInt(command_buffer + 2, recv_command_length - 2)); } if (command_buffer[1] == 'c') { @@ -721,7 +781,7 @@ static void HandleThreadAlive() { return; } - int thread_id = static_cast(HexToInt(command_buffer + 1, command_length - 1)); + int thread_id = static_cast(HexToInt(command_buffer + 1, recv_command_length - 1)); if (thread_id == 0) { thread_id = 1; } @@ -746,8 +806,8 @@ static void HandleExtendedMode() { * * @param signal Signal to be sent to client. */ -static void SendSignal(Kernel::Thread* thread, u32 signal, bool full = true) { - if (gdbserver_socket == -1) { +static void SendTStopReply(Kernel::Thread* thread, u32 signal, bool full = true) { + if (gdbserver_socket == INVALID_SOCKET) { return; } @@ -784,12 +844,12 @@ static void SendSignal(Kernel::Thread* thread, u32 signal, bool full = true) { static void HandleGetStopReason() { if (is_extended_mode) { // In extended mode, tell the debugger that there is no selected process yet. - // That way the debugger can ask for the process list and attack to the right PID. + // That way the debugger can ask for the process list and attach to the right one. if (!current_process) { // The process has not been selected yet. SendReply("W00"); } else { - SendSignal(current_thread, latest_signal); + SendTStopReply(current_thread, latest_signal); } } else { // In non extended mode, select the process corresponding to the "main" @@ -802,7 +862,7 @@ static void HandleGetStopReason() { current_process = process.get(); current_process->SetDebugBreak(true); if (SetThread(0)) { - SendSignal(current_thread, 0); + SendTStopReply(current_thread, 0); } else { // Should never happen SendReply("W00"); @@ -817,20 +877,21 @@ static void HandleGetStopReason() { } static void BreakImpl(int signal) { - if (signal == SIGSEGV) { - LOG_WARNING(Debug_GDBStub, "Segmentation fault signals may not be fully accurate"); + if (signal == SIGSEGV && !Core::GetCore(0).HasSingleInstructionBreakAccuracy()) { + LOG_WARNING(Debug_GDBStub, "The current CPU backend does not support accurate watchpoints " + "and memory exceptions. Disable CPU JIT for more accuracy."); } current_process->SetDebugBreak(true); latest_signal = signal; - SendSignal(current_thread, signal); + SendTStopReply(current_thread, signal); } /// Read command from gdb client. static void ReadCommand() { - command_length = 0; + recv_command_length = 0; std::memset(command_buffer, 0, sizeof(command_buffer)); u8 c = ReadByte(); @@ -847,27 +908,27 @@ static void ReadCommand() { } while ((c = ReadByte()) != GDB_STUB_END) { - if (command_length >= sizeof(command_buffer)) { + if (recv_command_length >= sizeof(command_buffer)) { LOG_ERROR(Debug_GDBStub, "gdb: command_buffer overflow\n"); SendPacket(GDB_STUB_NACK); return; } - command_buffer[command_length++] = c; + command_buffer[recv_command_length++] = c; } u8 checksum_received = HexCharToValue(ReadByte()) << 4; checksum_received |= HexCharToValue(ReadByte()); - u8 checksum_calculated = CalculateChecksum(command_buffer, command_length); + u8 checksum_calculated = CalculateChecksum(command_buffer, recv_command_length); if (checksum_received != checksum_calculated) { LOG_ERROR( Debug_GDBStub, "gdb: invalid checksum: calculated {:02x} and read {:02x} for ${}# (length: {})\n", checksum_calculated, checksum_received, reinterpret_cast(command_buffer), - command_length); + recv_command_length); - command_length = 0; + recv_command_length = 0; SendPacket(GDB_STUB_NACK); return; @@ -885,7 +946,7 @@ static bool IsDataAvailable() { fd_set fd_socket; FD_ZERO(&fd_socket); - FD_SET(static_cast(gdbserver_socket), &fd_socket); + FD_SET(gdbserver_socket, &fd_socket); struct timeval t; t.tv_sec = 0; @@ -923,6 +984,8 @@ static void ReadRegister() { LongToGdbHex(reply, FpuRead(id, current_thread)); } else if (id == FPSCR_REGISTER) { IntToGdbHex(reply, static_cast(FpuRead(id, current_thread))); + } else if (id == FPEXC_REGISTER) { + IntToGdbHex(reply, static_cast(FpuRead(id, current_thread))); } else { return SendReply("E01"); } @@ -960,6 +1023,10 @@ static void ReadRegisters() { IntToGdbHex(bufptr, static_cast(FpuRead(FPSCR_REGISTER, current_thread))); + bufptr += 8; + + IntToGdbHex(bufptr, static_cast(FpuRead(FPEXC_REGISTER, current_thread))); + SendReply(reinterpret_cast(buffer)); } @@ -998,6 +1065,8 @@ static void WriteRegister() { FpuWrite(id, GdbHexToLong(buffer_ptr), current_thread); } else if (id == FPSCR_REGISTER) { FpuWrite(id, GdbHexToInt(buffer_ptr), current_thread); + } else if (id == FPEXC_REGISTER) { + FpuWrite(id, GdbHexToInt(buffer_ptr), current_thread); } else { return SendReply("E01"); } @@ -1019,7 +1088,7 @@ static void WriteRegisters() { if (command_buffer[0] != 'G') return SendReply("E01"); - for (u32 i = 0, reg = 0; reg <= FPSCR_REGISTER; i++, reg++) { + for (u32 i = 0, reg = 0; reg <= FPEXC_REGISTER; i++, reg++) { if (reg <= PC_REGISTER) { RegWrite(reg, GdbHexToInt(buffer_ptr + i * 8), current_thread); } else if (reg == CPSR_REGISTER) { @@ -1034,6 +1103,8 @@ static void WriteRegisters() { i++; // Skip padding } else if (reg == FPSCR_REGISTER) { FpuWrite(reg, GdbHexToInt(buffer_ptr + i * 8), current_thread); + } else if (reg == FPEXC_REGISTER) { + FpuWrite(reg, GdbHexToInt(buffer_ptr + i * 8), current_thread); } } @@ -1052,12 +1123,12 @@ static void ReadMemory() { static u8 reply[GDB_BUFFER_SIZE - 4]; auto start_offset = command_buffer + 1; - auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); + auto addr_pos = std::find(start_offset, command_buffer + recv_command_length, ','); VAddr addr = HexToInt(start_offset, static_cast(addr_pos - start_offset)); start_offset = addr_pos + 1; - u32 len = - HexToInt(start_offset, static_cast((command_buffer + command_length) - start_offset)); + u32 len = HexToInt(start_offset, + static_cast((command_buffer + recv_command_length) - start_offset)); LOG_DEBUG(Debug_GDBStub, "ReadMemory addr: {:08x} len: {:08x}", addr, len); @@ -1090,11 +1161,11 @@ static void WriteMemory() { } auto start_offset = command_buffer + 1; - auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); + auto addr_pos = std::find(start_offset, command_buffer + recv_command_length, ','); VAddr addr = HexToInt(start_offset, static_cast(addr_pos - start_offset)); start_offset = addr_pos + 1; - auto len_pos = std::find(start_offset, command_buffer + command_length, ':'); + auto len_pos = std::find(start_offset, command_buffer + recv_command_length, ':'); u32 len = HexToInt(start_offset, static_cast(len_pos - start_offset)); auto& memory = Core::System::GetInstance().Memory(); @@ -1230,12 +1301,12 @@ static void AddBreakpoint() { } auto start_offset = command_buffer + 3; - auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); + auto addr_pos = std::find(start_offset, command_buffer + recv_command_length, ','); VAddr addr = HexToInt(start_offset, static_cast(addr_pos - start_offset)); start_offset = addr_pos + 1; - u32 len = - HexToInt(start_offset, static_cast((command_buffer + command_length) - start_offset)); + u32 len = HexToInt(start_offset, + static_cast((command_buffer + recv_command_length) - start_offset)); if (type == BreakpointType::Access) { // Access is made up of Read and Write types, so add both breakpoints @@ -1284,7 +1355,7 @@ static void RemoveBreakpoint() { } auto start_offset = command_buffer + 3; - auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); + auto addr_pos = std::find(start_offset, command_buffer + recv_command_length, ','); VAddr addr = HexToInt(start_offset, static_cast(addr_pos - start_offset)); if (type == BreakpointType::Access) { @@ -1300,7 +1371,7 @@ static void RemoveBreakpoint() { } void HandleVCommand() { - std::string_view cmd_view((const char*)command_buffer, command_length); + std::string_view cmd_view((const char*)command_buffer, recv_command_length); if (cmd_view.size() <= 1) { SendReply("E01"); @@ -1324,7 +1395,7 @@ void HandleVCommand() { current_process = process.get(); current_process->SetDebugBreak(true); if (SetThread(0)) { - SendSignal(current_thread, 0); + SendTStopReply(current_thread, 0); } else { // Should never happen SendReply("W00"); @@ -1353,7 +1424,7 @@ void HandleVCommand() { return; } char action_type = threads[0][0]; - if (action_type != 'c') { + if (action_type != 'c' && action_type != 'C') { SendReply("E01"); return; } @@ -1379,13 +1450,13 @@ void HandlePacket(Core::System& system) { } // Handle accept new GDB connection - if (accept_socket != -1) { + if (accept_socket != INVALID_SOCKET) { sockaddr_in saddr_client; sockaddr* client_addr = reinterpret_cast(&saddr_client); socklen_t client_addrlen = sizeof(saddr_client); gdbserver_socket = - static_cast(accept(accept_socket, client_addr, &client_addrlen)); - if (gdbserver_socket < 0) { + static_cast(accept(accept_socket, client_addr, &client_addrlen)); + if (gdbserver_socket == INVALID_SOCKET) { #ifdef _WIN32 if (GetErrno() == WSAEWOULDBLOCK) { // Nothing connected yet @@ -1404,7 +1475,7 @@ void HandlePacket(Core::System& system) { } shutdown(accept_socket, SHUT_RDWR); - accept_socket = -1; + accept_socket = INVALID_SOCKET; } return; } @@ -1428,12 +1499,12 @@ void HandlePacket(Core::System& system) { } ReadCommand(); - if (command_length == 0) { + if (recv_command_length == 0) { return; } #ifdef PRINT_GDB_TRAFFIC - std::string cmd_str(command_buffer + 1, command_buffer + command_length); + std::string cmd_str(command_buffer + 1, command_buffer + recv_command_length); LOG_INFO(Debug_GDBStub, "Req: {:c} {}", command_buffer[0], cmd_str); #endif @@ -1455,6 +1526,7 @@ void HandlePacket(Core::System& system) { case 'D': SendReply("OK"); ToggleServer(false); + ToggleServer(true); // Continue execution continue_thread = -1; Continue(); @@ -1468,7 +1540,7 @@ void HandlePacket(Core::System& system) { system.RequestShutdown(); return; case 'F': - HandleHioReply(system, current_process, command_buffer, command_length); + HandleHioReply(system, current_process, command_buffer, recv_command_length); break; case 'g': ReadRegisters(); @@ -1528,9 +1600,7 @@ void ToggleServer(bool status) { } } else { // Stop server - if (IsInitialized()) { - Shutdown(); - } + Shutdown(); server_enabled = status; } @@ -1545,10 +1615,6 @@ static void Init(u16 port) { return; } - breakpoints_execute.clear(); - breakpoints_read.clear(); - breakpoints_write.clear(); - // Start gdb server LOG_INFO(Debug_GDBStub, "Starting GDB server on port {}...", port); @@ -1562,8 +1628,9 @@ static void Init(u16 port) { #endif accept_socket = static_cast(socket(PF_INET, SOCK_STREAM, 0)); - if (accept_socket == -1) { + if (accept_socket == INVALID_SOCKET) { LOG_ERROR(Debug_GDBStub, "Failed to create gdb socket"); + return; } // Set socket to SO_REUSEADDR so it can always bind on the same port @@ -1571,16 +1638,22 @@ static void Init(u16 port) { if (setsockopt(accept_socket, SOL_SOCKET, SO_REUSEADDR, (const char*)&reuse_enabled, sizeof(reuse_enabled)) < 0) { LOG_ERROR(Debug_GDBStub, "Failed to set gdb socket option"); + Shutdown(); + return; } const sockaddr* server_addr = reinterpret_cast(&saddr_server); socklen_t server_addrlen = sizeof(saddr_server); if (bind(accept_socket, server_addr, server_addrlen) < 0) { LOG_ERROR(Debug_GDBStub, "Failed to bind gdb socket"); + Shutdown(); + return; } if (listen(accept_socket, 1) < 0) { LOG_ERROR(Debug_GDBStub, "Failed to listen to gdb socket"); + Shutdown(); + return; } SetNonBlock(accept_socket, true); @@ -1597,24 +1670,26 @@ void Shutdown() { if (!server_enabled) { return; } - defer_start = false; - is_extended_mode = false; + + RemoveAllBreakpoints(); LOG_INFO(Debug_GDBStub, "Stopping GDB ..."); - if (gdbserver_socket != -1) { + if (gdbserver_socket != INVALID_SOCKET) { shutdown(gdbserver_socket, SHUT_RDWR); - gdbserver_socket = -1; + gdbserver_socket = INVALID_SOCKET; } - if (accept_socket != -1) { + if (accept_socket != INVALID_SOCKET) { shutdown(accept_socket, SHUT_RDWR); - accept_socket = -1; + accept_socket = INVALID_SOCKET; } #ifdef _WIN32 WSACleanup(); #endif + ResetState(); + LOG_INFO(Debug_GDBStub, "GDB stopped."); } @@ -1623,10 +1698,11 @@ bool IsServerEnabled() { } bool IsInitialized() { - return IsServerEnabled() && (accept_socket != -1 || gdbserver_socket != -1); + return IsServerEnabled() && + (accept_socket != INVALID_SOCKET || gdbserver_socket != INVALID_SOCKET); } bool IsConnected() { - return IsServerEnabled() && gdbserver_socket != -1; + return IsServerEnabled() && gdbserver_socket != INVALID_SOCKET; } }; // namespace GDBStub From 359b92d14b015e3a65a7c027e7f6e69a070d2953 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Sun, 3 May 2026 14:52:32 +0200 Subject: [PATCH 5/9] gdb: Add pause process at start and minor fixes --- .../configuration/configure_debug.cpp | 20 ++++++ src/citra_qt/configuration/configure_debug.ui | 7 ++ src/core/core.h | 14 ++++ src/core/gdbstub/gdbstub.cpp | 69 +++++++++++++++---- src/core/gdbstub/gdbstub.h | 10 +++ src/core/hle/kernel/process.cpp | 13 ++++ src/core/hle/kernel/thread.cpp | 2 + 7 files changed, 123 insertions(+), 12 deletions(-) diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index db09c7b93..313df5d36 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include "citra_qt/configuration/configuration_shared.h" #include "citra_qt/configuration/configure_debug.h" @@ -12,6 +13,7 @@ #include "common/file_util.h" #include "common/logging/backend.h" #include "common/settings.h" +#include "core/core.h" #include "ui_configure_debug.h" #ifdef ENABLE_VULKAN #include "video_core/renderer_vulkan/vk_instance.h" @@ -33,6 +35,17 @@ ConfigureDebug::ConfigureDebug(bool is_powered_on_, QWidget* parent) ui->setupUi(this); SetConfiguration(); + connect(ui->toggle_gdbstub, &QCheckBox::clicked, + [this](bool checked) { ui->debug_next_process->setEnabled(checked); }); + + connect(ui->debug_next_process, &QCheckBox::clicked, [](bool checked) { + if (checked) { + Core::System::GetInstance().SetDebugNextProcessFlag(); + } else { + Core::System::GetInstance().ClearDebugNextProcessFlag(); + } + }); + connect(ui->open_log_button, &QPushButton::clicked, []() { QString path = QString::fromStdString(FileUtil::GetUserPath(FileUtil::UserPath::LogDir)); QDesktopServices::openUrl(QUrl::fromLocalFile(path)); @@ -94,6 +107,9 @@ ConfigureDebug::~ConfigureDebug() = default; void ConfigureDebug::SetConfiguration() { ui->toggle_gdbstub->setChecked(Settings::values.use_gdbstub.GetValue()); + if (!ui->toggle_gdbstub->isChecked()) { + ui->debug_next_process->setEnabled(false); + } ui->gdbport_spinbox->setEnabled(Settings::values.use_gdbstub.GetValue()); ui->gdbport_spinbox->setValue(Settings::values.gdbstub_port.GetValue()); ui->toggle_console->setEnabled(!is_powered_on); @@ -135,6 +151,10 @@ void ConfigureDebug::SetConfiguration() { ui->clock_display_label->setText( QStringLiteral("%1%").arg(Settings::values.cpu_clock_percentage.GetValue())); ui->instant_debug_log->setChecked(Settings::values.instant_debug_log.GetValue()); + + if (Core::System::GetInstance().GetDebugNextProcessFlag()) { + ui->debug_next_process->setChecked(true); + } } void ConfigureDebug::ApplyConfiguration() { diff --git a/src/citra_qt/configuration/configure_debug.ui b/src/citra_qt/configuration/configure_debug.ui index d9f66de38..1a470fb6f 100644 --- a/src/citra_qt/configuration/configure_debug.ui +++ b/src/citra_qt/configuration/configure_debug.ui @@ -60,6 +60,13 @@ + + + + Pause next non-sysmodule process at start + + + diff --git a/src/core/core.h b/src/core/core.h index 89df9b00e..971bd7b32 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -405,6 +405,18 @@ public: info_led_color_changed = func; } + void SetDebugNextProcessFlag() { + debug_next_process = true; + } + + bool GetDebugNextProcessFlag() { + return debug_next_process; + } + + void ClearDebugNextProcessFlag() { + debug_next_process = false; + } + private: /** * Initialize the emulated system. @@ -514,6 +526,8 @@ private: Common::Vec3 info_led_color; std::function info_led_color_changed; + bool debug_next_process; + friend class boost::serialization::access; template void serialize(Archive& ar, const unsigned int file_version); diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 3272e3ba7..9189c3a69 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -77,6 +77,7 @@ constexpr u32 SIGSEGV = 11; using SOCKET = UINT_PTR; #else using SOCKET = int; +#define closesocket(x) close(x) #endif // _WIN32 #define INVALID_SOCKET ((SOCKET)(~0)) @@ -161,6 +162,9 @@ u16 gdbstub_port = 24689; constexpr bool supports_extended_mode = true; bool is_extended_mode = false; +bool is_running = false; +bool current_process_finished = false; + // If set to false, the server will never be started and no // gdbstub-related functions will be executed. std::atomic server_enabled(false); @@ -202,6 +206,9 @@ static void ResetState() { is_extended_mode = false; + is_running = false; + current_process_finished = false; + accept_socket = INVALID_SOCKET; continue_thread = -1; @@ -452,11 +459,12 @@ static bool SetNonBlock(SOCKET socket, bool nonblock) { /// Read a byte from the gdb client. static u8 ReadByte() { - u8 c; + u8 c{}; std::size_t received_size = recv(gdbserver_socket, reinterpret_cast(&c), 1, MSG_WAITALL); if (received_size != 1) { LOG_ERROR(Debug_GDBStub, "recv failed : {}", GetErrno()); - Shutdown(); + ToggleServer(false); + ToggleServer(true); } return c; @@ -656,7 +664,9 @@ void SendReply(const char* reply) { static_cast(send(gdbserver_socket, reinterpret_cast(ptr), left, 0)); if (sent_size < 0) { LOG_ERROR(Debug_GDBStub, "gdb: send failed"); - return Shutdown(); + ToggleServer(false); + ToggleServer(true); + return; } left -= sent_size; @@ -806,11 +816,16 @@ static void HandleExtendedMode() { * * @param signal Signal to be sent to client. */ -static void SendTStopReply(Kernel::Thread* thread, u32 signal, bool full = true) { +static void SendStopReply(Kernel::Thread* thread, u32 signal, bool full = true) { if (gdbserver_socket == INVALID_SOCKET) { return; } + if (current_process_finished) { + SendReply("W00"); + return; + } + latest_signal = signal; if (!thread) { @@ -849,7 +864,7 @@ static void HandleGetStopReason() { // The process has not been selected yet. SendReply("W00"); } else { - SendTStopReply(current_thread, latest_signal); + SendStopReply(current_thread, latest_signal); } } else { // In non extended mode, select the process corresponding to the "main" @@ -861,8 +876,9 @@ static void HandleGetStopReason() { if (process->codeset->program_id == program_id) { current_process = process.get(); current_process->SetDebugBreak(true); + is_running = false; if (SetThread(0)) { - SendTStopReply(current_thread, 0); + SendStopReply(current_thread, 0); } else { // Should never happen SendReply("W00"); @@ -883,10 +899,11 @@ static void BreakImpl(int signal) { } current_process->SetDebugBreak(true); + is_running = false; latest_signal = signal; - SendTStopReply(current_thread, signal); + SendStopReply(current_thread, signal); } /// Read command from gdb client. @@ -1116,7 +1133,7 @@ static void WriteRegisters() { /// Read location in memory specified by gdb client. static void ReadMemory() { if (!current_process) { - SendReply("E01"); + SendReply(""); return; } @@ -1133,12 +1150,12 @@ static void ReadMemory() { LOG_DEBUG(Debug_GDBStub, "ReadMemory addr: {:08x} len: {:08x}", addr, len); if (len * 2 > sizeof(reply)) { - SendReply("E01"); + SendReply(""); } auto& memory = Core::System::GetInstance().Memory(); if (!memory.IsValidVirtualAddress(*current_process, addr)) { - return SendReply("E14"); + return SendReply(""); } std::vector data(len); @@ -1170,7 +1187,7 @@ static void WriteMemory() { auto& memory = Core::System::GetInstance().Memory(); if (!memory.IsValidVirtualAddress(*current_process, addr)) { - return SendReply("E00"); + return SendReply("E0E"); } std::vector data(len); @@ -1230,6 +1247,8 @@ static void Continue() { } current_process->SetDebugBreak(false, continue_list); + is_running = true; + ClearAllInstructionCache(); } @@ -1394,8 +1413,9 @@ void HandleVCommand() { } else { current_process = process.get(); current_process->SetDebugBreak(true); + is_running = false; if (SetThread(0)) { - SendTStopReply(current_thread, 0); + SendStopReply(current_thread, 0); } else { // Should never happen SendReply("W00"); @@ -1435,12 +1455,34 @@ void HandleVCommand() { } current_process->SetDebugBreak(false, thread_ids); + is_running = true; } } else { SendReply(""); } } +void OnProcessExit(u32 process_id) { + if (!GDBStub::IsConnected || !current_process || current_process->process_id != process_id) { + return; + } + + current_process_finished = true; + if (is_running) { + SendStopReply(nullptr, 0); + } + current_process = nullptr; + current_thread = nullptr; +} + +void OnThreadExit(u32 thread_id) { + if (!GDBStub::IsConnected || !current_thread || current_thread->thread_id != thread_id) { + return; + } + + current_thread = nullptr; +} + void HandlePacket(Core::System& system) { if (!IsConnected()) { @@ -1475,6 +1517,7 @@ void HandlePacket(Core::System& system) { } shutdown(accept_socket, SHUT_RDWR); + closesocket(accept_socket); accept_socket = INVALID_SOCKET; } return; @@ -1676,11 +1719,13 @@ void Shutdown() { LOG_INFO(Debug_GDBStub, "Stopping GDB ..."); if (gdbserver_socket != INVALID_SOCKET) { shutdown(gdbserver_socket, SHUT_RDWR); + closesocket(gdbserver_socket); gdbserver_socket = INVALID_SOCKET; } if (accept_socket != INVALID_SOCKET) { shutdown(accept_socket, SHUT_RDWR); + closesocket(accept_socket); accept_socket = INVALID_SOCKET; } diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h index 14345aa77..fec95d5aa 100644 --- a/src/core/gdbstub/gdbstub.h +++ b/src/core/gdbstub/gdbstub.h @@ -77,6 +77,16 @@ bool IsConnected(); */ void Break(int signal); +/** + * Signal to the GDB stub that the specified process ID is exiting + */ +void OnProcessExit(u32 process_id); + +/** + * Signal to the GDB stub that the specified thread ID is exiting + */ +void OnThreadExit(u32 thread_id); + /// Read and handle packet from gdb client. void HandlePacket(Core::System& system); diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index a808bb2ab..32d9a46cd 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -16,6 +16,7 @@ #include "common/logging/log.h" #include "common/serialization/boost_vector.hpp" #include "core/core.h" +#include "core/gdbstub/gdbstub.h" #include "core/hle/kernel/errors.h" #include "core/hle/kernel/memory.h" #include "core/hle/kernel/process.h" @@ -260,9 +261,21 @@ void Process::Run(s32 main_thread_priority, u32 stack_size) { vm_manager.LogLayout(Common::Log::Level::Debug); Kernel::SetupMainThread(kernel, codeset->entrypoint, main_thread_priority, SharedFrom(this)); + + // Pause process at start if flag enabled and we are not a sysmodule + if (Core::System::GetInstance().GetDebugNextProcessFlag() && + resource_limit->GetCategory() != Kernel::ResourceLimitCategory::Other) { + if (GDBStub::IsServerEnabled()) { + LOG_INFO(Loader, "Pausing process {} at start", process_id); + SetDebugBreak(true); + } + Core::System::GetInstance().ClearDebugNextProcessFlag(); + } } void Process::Exit() { + GDBStub::OnProcessExit(process_id); + auto plgldr = Service::PLGLDR::GetService(Core::System::GetInstance()); if (plgldr) { plgldr->OnProcessExit(*this, kernel); diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index a3cf78ec6..73fced736 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -134,6 +134,8 @@ void Thread::Stop() { process->tls_slots[tls_page].reset(tls_slot); process->resource_limit->Release(ResourceLimitType::Thread, 1); } + + GDBStub::OnThreadExit(thread_id); } void ThreadManager::SwitchContext(Thread* new_thread) { From b28c7c1e5c2d3d4699e5402fad9a9ae9165ae2d4 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Sun, 3 May 2026 17:51:17 +0200 Subject: [PATCH 6/9] frontends: Adapt UIs to new changes --- .../java/org/citra/citra_emu/NativeLibrary.kt | 66 ++++++++++--------- src/android/app/src/main/jni/config.cpp | 1 + src/android/app/src/main/jni/default_ini.h | 3 +- .../app/src/main/res/values/strings.xml | 3 +- src/citra_qt/citra_qt.cpp | 25 ++++++- src/citra_qt/configuration/config.cpp | 5 +- src/citra_qt/configuration/config.h | 2 +- src/citra_qt/main.ui | 34 ++++++++++ src/core/arm/dyncom/arm_dyncom.h | 2 +- 9 files changed, 103 insertions(+), 38 deletions(-) diff --git a/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt b/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt index 018fea670..32ec8671b 100644 --- a/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt +++ b/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt @@ -317,6 +317,12 @@ object NativeLibrary { canContinue = false } + CoreError.ErrorCoreExceptionRaised -> { + title = emulationActivity.getString(R.string.fatal_error) + message = emulationActivity.getString(R.string.fatal_error_message) + canContinue = false + } + CoreError.ErrorUnknown -> { title = emulationActivity.getString(R.string.fatal_error) message = emulationActivity.getString(R.string.fatal_error_message) @@ -439,7 +445,7 @@ object NativeLibrary { return } - if (resultCode == EmulationErrorDialogFragment.ShutdownRequested) { + if (resultCode == CoreError.ShutdownRequested.value) { emulationActivity.finish() return } @@ -458,23 +464,25 @@ object NativeLibrary { override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { emulationActivity = requireActivity() as EmulationActivity - var captionId = R.string.loader_error_invalid_format val result = requireArguments().getInt(RESULT_CODE) - if (result == ErrorLoader_ErrorEncrypted) { - captionId = R.string.loader_error_encrypted + var captionString = getString(R.string.loader_error_invalid_format) + if (result == CoreError.ErrorLoader_ErrorEncrypted.value) { + captionString = getString(R.string.loader_error_encrypted) } - if (result == ErrorArticDisconnected) { - captionId = R.string.artic_base + if (result == CoreError.ErrorArticDisconnected.value) { + captionString = getString(R.string.artic_base) } val alert = MaterialAlertDialogBuilder(requireContext()) - .setTitle(captionId) + .setTitle(captionString) .setMessage( Html.fromHtml( - if (result == ErrorArticDisconnected) - CitraApplication.appContext.resources.getString(R.string.artic_server_comm_error) + if (result == CoreError.ErrorArticDisconnected.value) + getString(R.string.artic_server_comm_error) + else if (result == CoreError.ErrorLoader_ErrorEncrypted.value) + getString(R.string.loader_error_encrypted_desc) else - CitraApplication.appContext.resources.getString(R.string.redump_games), + getString(R.string.loader_error_generic, result), Html.FROM_HTML_MODE_LEGACY ) ) @@ -496,21 +504,6 @@ object NativeLibrary { const val RESULT_CODE = "resultcode" - const val Success = 0 - const val ErrorNotInitialized = 1 - const val ErrorGetLoader = 2 - const val ErrorSystemMode = 3 - const val ErrorLoader = 4 - const val ErrorLoader_ErrorEncrypted = 5 - const val ErrorLoader_ErrorInvalidFormat = 6 - const val ErrorLoader_ErrorGBATitle = 7 - const val ErrorSystemFiles = 8 - const val ErrorSavestate = 9 - const val ErrorArticDisconnected = 10 - const val ErrorN3DSApplication = 11 - const val ShutdownRequested = 12 - const val ErrorUnknown = 13 - fun newInstance(resultCode: Int): EmulationErrorDialogFragment { val args = Bundle() args.putInt(RESULT_CODE, resultCode) @@ -857,12 +850,23 @@ object NativeLibrary { FileUtil.deleteDocument(path) } - enum class CoreError { - ErrorSystemFiles, - ErrorSavestate, - ErrorArticDisconnected, - ErrorN3DSApplication, - ErrorUnknown + enum class CoreError(val value: Int) { + Success(0), + ErrorNotInitialized(1), + ErrorGetLoader(2), + ErrorSystemMode(3), + ErrorLoader(4), + ErrorLoader_ErrorEncrypted(5), + ErrorLoader_ErrorInvalidFormat(6), + ErrorLoader_ErrorGBATitle(7), + ErrorSystemFiles(8), + ErrorSavestate(9), + ErrorArticDisconnected(10), + ErrorN3DSApplication(11), + ErrorCoreExceptionRaised(12), + ErrorMemoryExceptionRaised(13), + ShutdownRequested(14), + ErrorUnknown(15) } enum class InstallStatus { diff --git a/src/android/app/src/main/jni/config.cpp b/src/android/app/src/main/jni/config.cpp index e0fa260c5..90c157626 100644 --- a/src/android/app/src/main/jni/config.cpp +++ b/src/android/app/src/main/jni/config.cpp @@ -316,6 +316,7 @@ void Config::ReadValues() { ReadSetting("Debugging", Settings::values.instant_debug_log); ReadSetting("Debugging", Settings::values.enable_rpc_server); ReadSetting("Debugging", Settings::values.toggle_unique_data_console_type); + ReadSetting("Debugging", Settings::values.break_on_unmapped_memory_access); for (const auto& service_module : Service::service_module_map) { bool use_lle = diff --git a/src/android/app/src/main/jni/default_ini.h b/src/android/app/src/main/jni/default_ini.h index ce93d0e6f..d96140518 100644 --- a/src/android/app/src/main/jni/default_ini.h +++ b/src/android/app/src/main/jni/default_ini.h @@ -35,7 +35,8 @@ constexpr std::array android_config_omitted_keys = { Keys::audio_encoder, Keys::audio_encoder_options, Keys::audio_bitrate, - Keys::last_artic_base_addr, // On Android, this value is stored as a "preference" + Keys::last_artic_base_addr, // On Android, this value is stored as a "preference" + Keys::break_on_unmapped_memory_access, // Does nothing as the error is ignored }; // clang-format off diff --git a/src/android/app/src/main/res/values/strings.xml b/src/android/app/src/main/res/values/strings.xml index 08e927081..0cc45048b 100644 --- a/src/android/app/src/main/res/values/strings.xml +++ b/src/android/app/src/main/res/values/strings.xml @@ -391,7 +391,6 @@ Learn More Close Reset to Default - game cartridges or installed titles.]]> Default None Auto @@ -435,9 +434,11 @@ Your ROM is Encrypted + blog post for more information.]]> Invalid ROM format ROM file does not exist No bootable game present! + An error occurred while loading ROM: %d Press Back to access the menu. diff --git a/src/citra_qt/citra_qt.cpp b/src/citra_qt/citra_qt.cpp index aa2c90228..180591c9f 100644 --- a/src/citra_qt/citra_qt.cpp +++ b/src/citra_qt/citra_qt.cpp @@ -855,10 +855,11 @@ void GMainWindow::InitializeHotkeys() { // QAction Hotkeys const auto link_action_shortcut = [&](QAction* action, const QString& action_name, - const bool primary_only = false) { + const bool primary_only = false, + const bool auto_repeat = false) { static const QString main_window = QStringLiteral("Main Window"); action->setShortcut(hotkey_registry.GetKeySequence(main_window, action_name)); - action->setAutoRepeat(false); + action->setAutoRepeat(auto_repeat); this->addAction(action); if (!primary_only) secondary_window->addAction(action); @@ -875,6 +876,9 @@ void GMainWindow::InitializeHotkeys() { link_action_shortcut(ui->action_Show_Status_Bar, QStringLiteral("Toggle Status Bar")); link_action_shortcut(ui->action_Fullscreen, fullscreen, true); link_action_shortcut(ui->action_Capture_Screenshot, QStringLiteral("Capture Screenshot")); + link_action_shortcut(ui->action_Debug_Pause, QStringLiteral("Debug Pause")); + link_action_shortcut(ui->action_Debug_Resume, QStringLiteral("Debug Resume")); + link_action_shortcut(ui->action_Debug_Step, QStringLiteral("Debug Step"), false, true); link_action_shortcut(ui->action_Screen_Layout_Swap_Screens, QStringLiteral("Swap Screens")); link_action_shortcut(ui->action_Screen_Layout_Upright_Screens, QStringLiteral("Rotate Screens Upright")); @@ -1189,6 +1193,23 @@ void GMainWindow::ConnectMenuEvents() { connect_menu(ui->action_Capture_Screenshot, &GMainWindow::OnCaptureScreenshot); connect_menu(ui->action_Dump_Video, &GMainWindow::OnDumpVideo); + // Tools debug + connect_menu(ui->action_Debug_Pause, [this] { + if (emu_thread) { + emu_thread->SetRunning(false); + } + }); + connect_menu(ui->action_Debug_Resume, [this] { + if (emu_thread) { + emu_thread->SetRunning(true); + } + }); + connect_menu(ui->action_Debug_Step, [this] { + if (emu_thread) { + emu_thread->ExecStep(); + } + }); + // Tools connect_menu(ui->action_Compress_ROM_File, &GMainWindow::OnCompressFile); connect_menu(ui->action_Decompress_ROM_File, &GMainWindow::OnDecompressFile); diff --git a/src/citra_qt/configuration/config.cpp b/src/citra_qt/configuration/config.cpp index e51a175d0..4fd9e046c 100644 --- a/src/citra_qt/configuration/config.cpp +++ b/src/citra_qt/configuration/config.cpp @@ -57,12 +57,15 @@ const std::array, Settings::NativeAnalog::NumAnalogs> QtConfi // This must be in alphabetical order according to action name as it must have the same order as // UISetting::values.shortcuts, which is alphabetically ordered. // clang-format off -const std::array QtConfig::default_hotkeys {{ +const std::array QtConfig::default_hotkeys {{ {QStringLiteral("Advance Frame"), QStringLiteral("Main Window"), {QStringLiteral(""), Qt::ApplicationShortcut}}, {QStringLiteral("Audio Mute/Unmute"), QStringLiteral("Main Window"), {QStringLiteral("Ctrl+M"), Qt::WindowShortcut}}, {QStringLiteral("Audio Volume Down"), QStringLiteral("Main Window"), {QStringLiteral(""), Qt::WindowShortcut}}, {QStringLiteral("Audio Volume Up"), QStringLiteral("Main Window"), {QStringLiteral(""), Qt::WindowShortcut}}, {QStringLiteral("Capture Screenshot"), QStringLiteral("Main Window"), {QStringLiteral("Ctrl+P"), Qt::WidgetWithChildrenShortcut}}, + {QStringLiteral("Debug Pause"), QStringLiteral("Main Window"), {QStringLiteral("Ctrl+F4"),Qt::WidgetWithChildrenShortcut}}, + {QStringLiteral("Debug Resume"), QStringLiteral("Main Window"), {QStringLiteral("Ctrl+F5"),Qt::WidgetWithChildrenShortcut}}, + {QStringLiteral("Debug Step"), QStringLiteral("Main Window"), {QStringLiteral("Ctrl+F6"),Qt::WidgetWithChildrenShortcut}}, {QStringLiteral("Continue/Pause Emulation"), QStringLiteral("Main Window"), {QStringLiteral("F4"), Qt::WindowShortcut}}, {QStringLiteral("Decrease 3D Factor"), QStringLiteral("Main Window"), {QStringLiteral("Ctrl+-"), Qt::ApplicationShortcut}}, {QStringLiteral("Decrease Speed Limit"), QStringLiteral("Main Window"), {QStringLiteral("-"), Qt::ApplicationShortcut}}, diff --git a/src/citra_qt/configuration/config.h b/src/citra_qt/configuration/config.h index 3fba498ed..6cc87f7f0 100644 --- a/src/citra_qt/configuration/config.h +++ b/src/citra_qt/configuration/config.h @@ -26,7 +26,7 @@ public: static const std::array default_buttons; static const std::array, Settings::NativeAnalog::NumAnalogs> default_analogs; - static const std::array default_hotkeys; + static const std::array default_hotkeys; private: void Initialize(const std::string& config_name); diff --git a/src/citra_qt/main.ui b/src/citra_qt/main.ui index acca5361d..1e5dbb4e5 100644 --- a/src/citra_qt/main.ui +++ b/src/citra_qt/main.ui @@ -206,6 +206,16 @@ + + + Debug + + + + + + + @@ -453,6 +463,30 @@ Capture Screenshot + + + true + + + Debug Pause + + + + + true + + + Debug Resume + + + + + true + + + Debug Step + + true diff --git a/src/core/arm/dyncom/arm_dyncom.h b/src/core/arm/dyncom/arm_dyncom.h index c13828176..cfe1d3e92 100644 --- a/src/core/arm/dyncom/arm_dyncom.h +++ b/src/core/arm/dyncom/arm_dyncom.h @@ -1,4 +1,4 @@ -// Copyright 2014 Citra Emulator Project +// Copyright Citra Emulator Project / Azahar Emulator Project // Licensed under GPLv2 or any later version // Refer to the license.txt file included. From 6a3597c23b88444304ba0360fd2c4410e4560051 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Mon, 4 May 2026 14:53:34 +0200 Subject: [PATCH 7/9] gdb: Add cmake toggle to enable GDB stub --- CMakeLists.txt | 3 ++- docker/azahar-room/Dockerfile | 1 + src/android/app/build.gradle.kts | 3 ++- src/android/app/src/main/jni/default_ini.h | 6 ++---- .../configuration/configure_debug.cpp | 6 +++++- src/citra_qt/configuration/configure_debug.ui | 2 +- src/core/CMakeLists.txt | 14 ++++++++++---- src/core/arm/dynarmic/arm_dynarmic.cpp | 13 +++++++++++-- .../arm/dyncom/arm_dyncom_interpreter.cpp | 8 ++++++-- src/core/arm/skyeye_common/armstate.cpp | 5 +++++ src/core/arm/skyeye_common/armstate.h | 10 ++++++++++ src/core/core.cpp | 10 ++++++++++ src/core/gdbstub/gdbstub.cpp | 9 ++++----- src/core/gdbstub/gdbstub.h | 4 ++++ src/core/gdbstub/hio.cpp | 4 ++++ src/core/gdbstub/hio.h | 4 ++++ src/core/hle/kernel/process.cpp | 6 ++++++ src/core/hle/kernel/svc.cpp | 4 ++++ src/core/hle/kernel/thread.cpp | 5 +++++ src/core/memory.cpp | 19 ++++++++++++++++++- 20 files changed, 114 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3d58cc70d..d98994384 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -99,7 +99,7 @@ endif() # Track which options were explicitly set by the user (for libretro conflict detection) set(_LIBRETRO_INCOMPATIBLE_OPTIONS - ENABLE_SDL2 ENABLE_QT ENABLE_WEB_SERVICE ENABLE_SCRIPTING + ENABLE_SDL2 ENABLE_QT ENABLE_WEB_SERVICE ENABLE_SCRIPTING ENABLE_GDBSTUB ENABLE_OPENAL ENABLE_ROOM ENABLE_ROOM_STANDALONE ENABLE_CUBEB ENABLE_LIBUSB) set(_USER_SET_OPTIONS "") foreach(_opt IN LISTS _LIBRETRO_INCOMPATIBLE_OPTIONS) @@ -122,6 +122,7 @@ CMAKE_DEPENDENT_OPTION(ENABLE_ROOM_STANDALONE "Enable generating a standalone de option(ENABLE_WEB_SERVICE "Enable web services (telemetry, etc.)" ON) option(ENABLE_SCRIPTING "Enable RPC server for scripting" ON) +option(ENABLE_GDBSTUB "Enable GDB stub for emulated applications" ON) CMAKE_DEPENDENT_OPTION(ENABLE_CUBEB "Enables the cubeb audio backend" ON "NOT IOS" OFF) option(ENABLE_OPENAL "Enables the OpenAL audio backend" ON) diff --git a/docker/azahar-room/Dockerfile b/docker/azahar-room/Dockerfile index c47896dd9..9a5f07820 100644 --- a/docker/azahar-room/Dockerfile +++ b/docker/azahar-room/Dockerfile @@ -9,6 +9,7 @@ COPY . /var/azahar-src RUN mkdir builddir && cd builddir && \ cmake /var/azahar-src -G Ninja \ -DENABLE_QT=OFF \ + -DENABLE_GDBSTUB=OFF \ -DENABLE_TESTS=OFF \ -DENABLE_ROOM=ON \ -DENABLE_ROOM_STANDALONE=ON \ diff --git a/src/android/app/build.gradle.kts b/src/android/app/build.gradle.kts index 733656c0e..2ee844985 100644 --- a/src/android/app/build.gradle.kts +++ b/src/android/app/build.gradle.kts @@ -79,7 +79,8 @@ android { "-DENABLE_QT=0", // Don't use QT "-DENABLE_SDL2=0", // Don't use SDL "-DANDROID_ARM_NEON=true", // cryptopp requires Neon to work - "-DANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES=ON" // Support Android 15 16KiB page sizes + "-DANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES=ON", // Support Android 15 16KiB page sizes + "-DENABLE_GDBSTUB=OFF", // Disable GDB stub ) } } diff --git a/src/android/app/src/main/jni/default_ini.h b/src/android/app/src/main/jni/default_ini.h index d96140518..374499864 100644 --- a/src/android/app/src/main/jni/default_ini.h +++ b/src/android/app/src/main/jni/default_ini.h @@ -37,6 +37,8 @@ constexpr std::array android_config_omitted_keys = { Keys::audio_bitrate, Keys::last_artic_base_addr, // On Android, this value is stored as a "preference" Keys::break_on_unmapped_memory_access, // Does nothing as the error is ignored + Keys::use_gdbstub, // GDB functionality disabled by deafult on Android + Keys::gdbstub_port, }; // clang-format off @@ -528,10 +530,6 @@ static const char* android_config_default_file_content = (BOOST_HANA_STRING(R"( # 0 (default): Off, 1: On )") DECLARE_KEY(renderer_debug) BOOST_HANA_STRING(R"( -# Port for listening to GDB connections. -)") DECLARE_KEY(use_gdbstub) BOOST_HANA_STRING(R"( -)") DECLARE_KEY(gdbstub_port) BOOST_HANA_STRING(R"( - # Flush log output on every message # Immediately commits the debug log to file. Use this if Azahar crashes and the log output is being cut. )") DECLARE_KEY(instant_debug_log) BOOST_HANA_STRING(R"( diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index 313df5d36..a1e26e456 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -100,6 +100,10 @@ ConfigureDebug::ConfigureDebug(bool is_powered_on_, QWidget* parent) ui->clock_speed_label->setVisible(Settings::IsConfiguringGlobal()); ui->clock_speed_combo->setVisible(!Settings::IsConfiguringGlobal()); +#ifndef ENABLE_GDBSTUB + ui->gdb_groupbox->setVisible(false); +#endif + SetupPerGameUI(); } @@ -198,7 +202,7 @@ void ConfigureDebug::SetupPerGameUI() { ConfigurationShared::SetHighlight(ui->clock_speed_widget, index == 1); }); - ui->groupBox->setVisible(false); + ui->gdb_groupbox->setVisible(false); ui->groupBox_2->setVisible(false); ui->enable_rpc_server->setVisible(false); ui->toggle_unique_data_console_type->setVisible(false); diff --git a/src/citra_qt/configuration/configure_debug.ui b/src/citra_qt/configuration/configure_debug.ui index 1a470fb6f..d9c833949 100644 --- a/src/citra_qt/configuration/configure_debug.ui +++ b/src/citra_qt/configuration/configure_debug.ui @@ -17,7 +17,7 @@ - + GDB diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index ec7a45bfe..e3f8e5275 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -126,10 +126,6 @@ add_library(citra_core STATIC frontend/image_interface.cpp frontend/image_interface.h frontend/input.h - gdbstub/gdbstub.cpp - gdbstub/gdbstub.h - gdbstub/hio.cpp - gdbstub/hio.h hle/applets/applet.cpp hle/applets/applet.h hle/applets/erreula.cpp @@ -529,6 +525,16 @@ if (ENABLE_SCRIPTING) ) endif() +if (ENABLE_GDBSTUB) + target_compile_definitions(citra_core PUBLIC -DENABLE_GDBSTUB) + target_sources(citra_core PRIVATE + gdbstub/gdbstub.cpp + gdbstub/gdbstub.h + gdbstub/hio.cpp + gdbstub/hio.h + ) +endif() + if ("x86_64" IN_LIST ARCHITECTURE OR "arm64" IN_LIST ARCHITECTURE) target_sources(citra_core PRIVATE arm/dynarmic/arm_dynarmic.cpp diff --git a/src/core/arm/dynarmic/arm_dynarmic.cpp b/src/core/arm/dynarmic/arm_dynarmic.cpp index 002c5488d..503092889 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.cpp +++ b/src/core/arm/dynarmic/arm_dynarmic.cpp @@ -14,7 +14,9 @@ #include "core/arm/dynarmic/arm_tick_counts.h" #include "core/core.h" #include "core/core_timing.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/gdbstub.h" +#endif #include "core/hle/kernel/svc.h" #include "core/memory.h" @@ -106,11 +108,13 @@ public: case Dynarmic::A32::Exception::NoExecuteFault: break; case Dynarmic::A32::Exception::Breakpoint: +#ifdef ENABLE_GDBSTUB if (GDBStub::IsConnected()) { parent.SetPC(pc); parent.ServeBreak(SIGTRAP); return; } +#endif break; case Dynarmic::A32::Exception::SendEvent: case Dynarmic::A32::Exception::SendEventLocal: @@ -124,9 +128,12 @@ public: } parent.SetPC(pc); +#ifdef ENABLE_GDBSTUB if (GDBStub::IsConnected()) { parent.ServeBreak(SIGILL); - } else { + } else +#endif + { std::string error; for (int i = 0; i < 16; i++) { error += fmt::format("r{:02d} = {:08X}\n", i, parent.GetReg(i)); @@ -328,8 +335,10 @@ void ARM_Dynarmic::SetPageTable(const std::shared_ptr& page_t jits.emplace(current_page_table, std::move(new_jit)); } -void ARM_Dynarmic::ServeBreak(int signal) { +void ARM_Dynarmic::ServeBreak([[maybe_unused]] int signal) { +#ifdef ENABLE_GDBSTUB GDBStub::Break(signal); +#endif } std::unique_ptr ARM_Dynarmic::MakeJit() { diff --git a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp index 794197590..af30fdad3 100644 --- a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp +++ b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp @@ -23,7 +23,9 @@ #include "core/arm/skyeye_common/vfp/vfp.h" #include "core/core.h" #include "core/core_timing.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/gdbstub.h" +#endif #include "core/hle/kernel/svc.h" #include "core/memory.h" @@ -922,9 +924,11 @@ MICROPROFILE_DEFINE(DynCom_Execute, "DynCom", "Execute", MP_RGB(255, 0, 0)); unsigned InterpreterMainLoop(ARMul_State* cpu) { MICROPROFILE_SCOPE(DynCom_Execute); +#ifdef ENABLE_GDBSTUB /// Nearest upcoming GDB code execution breakpoint, relative to the last dispatch's address. GDBStub::BreakpointAddress breakpoint_data; breakpoint_data.type = GDBStub::BreakpointType::None; +#endif #undef RM #undef RS @@ -952,7 +956,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { #define INC_PC(l) ptr += sizeof(arm_inst) + l #define INC_PC_STUB ptr += sizeof(arm_inst) -#ifdef ANDROID +#ifndef ENABLE_GDBSTUB #define GDB_BP_CHECK #else #define GDB_BP_CHECK \ @@ -1653,7 +1657,7 @@ DISPATCH: { goto END; } -#ifndef ANDROID +#ifdef ENABLE_GDBSTUB // Find breakpoint if one exists within the block if (GDBStub::IsConnected()) { breakpoint_data = diff --git a/src/core/arm/skyeye_common/armstate.cpp b/src/core/arm/skyeye_common/armstate.cpp index d7349b59a..9ff89c5a7 100644 --- a/src/core/arm/skyeye_common/armstate.cpp +++ b/src/core/arm/skyeye_common/armstate.cpp @@ -9,6 +9,9 @@ #include "core/arm/skyeye_common/armstate.h" #include "core/arm/skyeye_common/vfp/vfp.h" #include "core/core.h" +#ifdef ENABLE_GDBSTUB +#include "core/gdbstub/gdbstub.h" +#endif #include "core/memory.h" #ifndef SIGTRAP @@ -580,6 +583,7 @@ void ARMul_State::WriteCP15Register(u32 value, u32 crn, u32 opcode_1, u32 crm, u } void ARMul_State::ServeBreak() { +#ifdef ENABLE_GDBSTUB if (!GDBStub::IsServerEnabled()) { return; } @@ -592,4 +596,5 @@ void ARMul_State::ServeBreak() { last_bkpt_hit = false; GDBStub::Break(SIGTRAP); } +#endif } diff --git a/src/core/arm/skyeye_common/armstate.h b/src/core/arm/skyeye_common/armstate.h index f36c46c07..925881304 100644 --- a/src/core/arm/skyeye_common/armstate.h +++ b/src/core/arm/skyeye_common/armstate.h @@ -1,3 +1,7 @@ +// Copyright Citra Emulator Project / Azahar Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + /* armdefs.h -- ARMulator common definitions: ARM6 Instruction Emulator. Copyright (C) 1994 Advanced RISC Machines Ltd. @@ -21,7 +25,9 @@ #include #include "common/common_types.h" #include "core/arm/skyeye_common/arm_regformat.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/gdbstub.h" +#endif namespace Core { class System; @@ -199,10 +205,12 @@ public: return TFlag ? 2 : 4; } +#ifdef ENABLE_GDBSTUB void RecordBreak(GDBStub::BreakpointAddress bkpt) { last_bkpt = bkpt; last_bkpt_hit = true; } +#endif void ServeBreak(); @@ -267,6 +275,8 @@ private: u32 exclusive_tag; // The address for which the local monitor is in exclusive access mode bool exclusive_state; +#ifdef ENABLE_GDBSTUB GDBStub::BreakpointAddress last_bkpt{}; bool last_bkpt_hit = false; +#endif }; diff --git a/src/core/core.cpp b/src/core/core.cpp index a40a93c54..cc0e65ed9 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -26,7 +26,9 @@ #include "core/dumping/backend.h" #include "core/file_sys/ncch_container.h" #include "core/frontend/image_interface.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/gdbstub.h" +#endif #include "core/global.h" #include "core/hle/kernel/ipc_debugger/recorder.h" #include "core/hle/kernel/kernel.h" @@ -83,6 +85,7 @@ System::ResultStatus System::RunLoop(bool tight_loop) { return ResultStatus::ErrorNotInitialized; } +#ifdef ENABLE_GDBSTUB if (GDBStub::IsServerEnabled()) { // The break flag is only set if GDB is connected, // we can do clearing here safely. If it is ever @@ -92,6 +95,7 @@ System::ResultStatus System::RunLoop(bool tight_loop) { } GDBStub::HandlePacket(*this); } +#endif Signal signal{Signal::None}; u32 param{}; @@ -561,7 +565,9 @@ System::ResultStatus System::Init(Frontend::EmuWindow& emu_window, app_loader->ReadProgramId(loading_title_id); HW::AES::InitKeys(); Service::Init(*this, loading_title_id, lle_modules, !app_loader->DoingInitialSetup()); +#ifdef ENABLE_GDBSTUB GDBStub::DeferStart(); +#endif if (!registered_image_interface) { registered_image_interface = std::make_shared(); @@ -685,7 +691,9 @@ void System::Shutdown(bool is_deserializing) { gpu.reset(); if (!is_deserializing) { lle_modules.clear(); +#ifdef ENABLE_GDBSTUB GDBStub::Shutdown(); +#endif perf_stats.reset(); app_loader.reset(); } @@ -748,8 +756,10 @@ void System::Reset() { } void System::ApplySettings() { +#ifdef ENABLE_GDBSTUB GDBStub::SetServerPort(Settings::values.gdbstub_port.GetValue()); GDBStub::ToggleServer(Settings::values.use_gdbstub.GetValue()); +#endif if (gpu) { #ifndef ANDROID diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 9189c3a69..017677606 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -43,10 +43,12 @@ #include "core/loader/loader.h" #include "core/memory.h" -#pragma optimize("", off) +#ifndef ENABLE_GDBSTUB +#error "File was compiled with GDB stub support disabled" +#endif // Uncomment to log all GDB traffic -#define PRINT_GDB_TRAFFIC +// #define PRINT_GDB_TRAFFIC namespace GDBStub { namespace { @@ -1700,9 +1702,6 @@ static void Init(u16 port) { } SetNonBlock(accept_socket, true); - - // Wait for gdb to connect - LOG_INFO(Debug_GDBStub, "Waiting for gdb to connect...\n"); } void Init() { diff --git a/src/core/gdbstub/gdbstub.h b/src/core/gdbstub/gdbstub.h index fec95d5aa..0fd3c7c00 100644 --- a/src/core/gdbstub/gdbstub.h +++ b/src/core/gdbstub/gdbstub.h @@ -14,6 +14,10 @@ #include "common/common_types.h" #include "core/hle/kernel/thread.h" +#ifndef ENABLE_GDBSTUB +#error "File was included with GDB stub support disabled" +#endif + namespace Core { class System; } diff --git a/src/core/gdbstub/hio.cpp b/src/core/gdbstub/hio.cpp index 2d6c8058a..8a8a7adc6 100644 --- a/src/core/gdbstub/hio.cpp +++ b/src/core/gdbstub/hio.cpp @@ -9,6 +9,10 @@ #include "core/gdbstub/gdbstub.h" #include "core/gdbstub/hio.h" +#ifndef ENABLE_GDBSTUB +#error "File was compiled with GDB stub support disabled" +#endif + #ifndef SIGTRAP constexpr u32 SIGTRAP = 5; #endif diff --git a/src/core/gdbstub/hio.h b/src/core/gdbstub/hio.h index 5fe366bb4..066bef55a 100644 --- a/src/core/gdbstub/hio.h +++ b/src/core/gdbstub/hio.h @@ -6,6 +6,10 @@ #include "common/common_types.h" +#ifndef ENABLE_GDBSTUB +#error "File was included with GDB stub support disabled" +#endif + namespace Core { class System; } diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index 32d9a46cd..b895a1f13 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -16,7 +16,9 @@ #include "common/logging/log.h" #include "common/serialization/boost_vector.hpp" #include "core/core.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/gdbstub.h" +#endif #include "core/hle/kernel/errors.h" #include "core/hle/kernel/memory.h" #include "core/hle/kernel/process.h" @@ -265,16 +267,20 @@ void Process::Run(s32 main_thread_priority, u32 stack_size) { // Pause process at start if flag enabled and we are not a sysmodule if (Core::System::GetInstance().GetDebugNextProcessFlag() && resource_limit->GetCategory() != Kernel::ResourceLimitCategory::Other) { +#ifdef ENABLE_GDBSTUB if (GDBStub::IsServerEnabled()) { LOG_INFO(Loader, "Pausing process {} at start", process_id); SetDebugBreak(true); } +#endif Core::System::GetInstance().ClearDebugNextProcessFlag(); } } void Process::Exit() { +#ifdef ENABLE_GDBSTUB GDBStub::OnProcessExit(process_id); +#endif auto plgldr = Service::PLGLDR::GetService(Core::System::GetInstance()); if (plgldr) { diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index a02e91427..7392de779 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -14,7 +14,9 @@ #include "core/arm/arm_interface.h" #include "core/core.h" #include "core/core_timing.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/hio.h" +#endif #include "core/hle/kernel/address_arbiter.h" #include "core/hle/kernel/client_port.h" #include "core/hle/kernel/client_session.h" @@ -1171,7 +1173,9 @@ void SVC::OutputDebugString(VAddr address, s32 len) { } if (len == 0) { +#ifdef ENABLE_GDBSTUB GDBStub::SetHioRequest(system, kernel.GetCurrentProcess().get(), address); +#endif return; } diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 73fced736..5f982c9e6 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -17,6 +17,9 @@ #include "core/arm/arm_interface.h" #include "core/arm/skyeye_common/armstate.h" #include "core/core.h" +#ifdef ENABLE_GDBSTUB +#include "core/gdbstub/gdbstub.h" +#endif #include "core/hle/kernel/errors.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/mutex.h" @@ -135,7 +138,9 @@ void Thread::Stop() { process->resource_limit->Release(ResourceLimitType::Thread, 1); } +#ifdef ENABLE_GDBSTUB GDBStub::OnThreadExit(thread_id); +#endif } void ThreadManager::SwitchContext(Thread* new_thread) { diff --git a/src/core/memory.cpp b/src/core/memory.cpp index 47416ab87..d96e8651f 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -17,7 +17,9 @@ #include "common/swap.h" #include "core/arm/arm_interface.h" #include "core/core.h" +#ifdef ENABLE_GDBSTUB #include "core/gdbstub/gdbstub.h" +#endif #include "core/global.h" #include "core/hle/kernel/process.h" #include "core/hle/service/plgldr/plgldr.h" @@ -569,9 +571,12 @@ T MemorySystem::UnmappedAccess(const VAddr vaddr, const T value, bool read) { const std::string value_str = read ? std::string("") : fmt::format(" 0x{:08X}", value); const std::string message = fmt::format("unmapped {}{}{} @ 0x{:08X} at PC 0x{:08X}", mode, sizeof(T) * 8, value_str, vaddr, impl->GetPC()); +#ifdef ENABLE_GDBSTUB if (GDBStub::IsConnected()) { GDBStub::Break(SIGSEGV); - } else if (Settings::values.break_on_unmapped_memory_access) { + } else +#endif + if (Settings::values.break_on_unmapped_memory_access) { impl->system.SetStatus(Core::System::ResultStatus::ErrorMemoryExceptionRaised, message.c_str()); } @@ -621,9 +626,11 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v T value; std::memcpy(&value, it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK), sizeof(T)); +#ifdef ENABLE_GDBSTUB if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Read)) { GDBStub::Break(SIGTRAP); } +#endif return value; } @@ -640,9 +647,11 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v T value; std::memcpy(&value, GetPointerForRasterizerCache(vaddr), sizeof(T)); +#ifdef ENABLE_GDBSTUB if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Read)) { GDBStub::Break(SIGTRAP); } +#endif return value; } @@ -695,9 +704,11 @@ void MemorySystem::Write(const std::shared_ptr& page_table, const VAd std::memcpy(it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK), &data, sizeof(T)); +#ifdef ENABLE_GDBSTUB if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { GDBStub::Break(SIGTRAP); } +#endif break; } @@ -710,9 +721,11 @@ void MemorySystem::Write(const std::shared_ptr& page_table, const VAd RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Invalidate); std::memcpy(GetPointerForRasterizerCache(vaddr), &data, sizeof(T)); +#ifdef ENABLE_GDBSTUB if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { GDBStub::Break(SIGTRAP); } +#endif break; } @@ -749,9 +762,11 @@ bool MemorySystem::WriteExclusive(const VAddr vaddr, const T data, const T expec bool ret = Common::AtomicCompareAndSwap(volatile_pointer, data, expected); +#ifdef ENABLE_GDBSTUB if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { GDBStub::Break(SIGTRAP); } +#endif return ret; } @@ -766,9 +781,11 @@ bool MemorySystem::WriteExclusive(const VAddr vaddr, const T data, const T expec const auto volatile_pointer = reinterpret_cast(GetPointerForRasterizerCache(vaddr).GetPtr()); +#ifdef ENABLE_GDBSTUB if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Write)) { GDBStub::Break(SIGTRAP); } +#endif return Common::AtomicCompareAndSwap(volatile_pointer, data, expected); } From 76b862a726ea7492fdef231ad542582550645744 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Mon, 4 May 2026 17:41:24 +0200 Subject: [PATCH 8/9] gdb: Last fixes from review round --- src/citra_qt/configuration/configure_debug.cpp | 1 - src/core/memory.cpp | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index a1e26e456..bdb945eb9 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -4,7 +4,6 @@ #include #include -#include #include #include "citra_qt/configuration/configuration_shared.h" #include "citra_qt/configuration/configure_debug.h" diff --git a/src/core/memory.cpp b/src/core/memory.cpp index d96e8651f..dafe6d0b4 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -424,8 +424,6 @@ PAddr& Memory::MemorySystem::Plugin3GXFramebufferAddress() { return impl->plugin_fb_address; } -#pragma optimize("", off) - void MemorySystem::RegisterWatchpoint(const Kernel::Process& process, VAddr addr, u32 size) { auto& page_table = *process.vm_manager.page_table; From 0c01818f66794b60168f92a90a443641b69b4cbd Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Mon, 4 May 2026 20:25:41 +0200 Subject: [PATCH 9/9] memory: Add Read32OrNullopt to better detect unmapped memory execution --- src/common/CMakeLists.txt | 1 + src/common/optional_helper.h | 42 +++++++++++++++++ src/core/arm/dynarmic/arm_dynarmic.cpp | 33 +++++++------ src/core/memory.cpp | 64 ++++++++++++++++++-------- src/core/memory.h | 27 ++++++++++- 5 files changed, 133 insertions(+), 34 deletions(-) create mode 100644 src/common/optional_helper.h diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 4b6e74c49..931899f5f 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -59,6 +59,7 @@ add_library(citra_common STATIC microprofile.cpp microprofile.h microprofileui.h + optional_helper.h param_package.cpp param_package.h play_time_manager.cpp diff --git a/src/common/optional_helper.h b/src/common/optional_helper.h new file mode 100644 index 000000000..f057973d9 --- /dev/null +++ b/src/common/optional_helper.h @@ -0,0 +1,42 @@ +// Copyright Citra Emulator Project / Azahar Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once + +#include +#include + +namespace detail { +template +struct is_optional_trait : std::false_type { + using value_type = T; +}; + +template +struct is_optional_trait> : std::true_type { + using value_type = T; +}; +} // namespace detail + +/** + * Returns true if T is a std::optional, false otherwise. + * For example: + * using Test1 = u32; + * using Test2 = std::optional; + * is_optional_type -> false + * is_optional_type -> true + */ +template +inline constexpr bool is_optional_type = detail::is_optional_trait::value; + +/** + * Provides the inner type of T if it is a std::optional, or T itself if it is not. + * For example: + * using Test1 = u32; + * using Test2 = std::optional; + * optional_value_type -> u32 + * optional_value_type -> u32 + */ +template +using optional_inner_or_type = typename detail::is_optional_trait::value_type; diff --git a/src/core/arm/dynarmic/arm_dynarmic.cpp b/src/core/arm/dynarmic/arm_dynarmic.cpp index 503092889..64b740569 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.cpp +++ b/src/core/arm/dynarmic/arm_dynarmic.cpp @@ -37,18 +37,7 @@ public: ~DynarmicUserCallbacks() = default; std::optional MemoryReadCode(VAddr vaddr) override { - constexpr VAddr low_page_limit = 0x1000; - - // This check prevents a common cascading error that results from the - // memory system allowing unmapped memory accesses (in some situations, - // a vtable is read from an invalid address and execution jumps to the - // zero page). On real hw, it's not normally possible to map the zero page. - if (vaddr < low_page_limit) [[unlikely]] { - LOG_CRITICAL(Debug, "Tried to read code from low address 0x{:08x}", vaddr); - return {}; - } - - return MemoryRead32(vaddr); + return memory.Read32OrNullopt(vaddr); } std::uint8_t MemoryRead8(VAddr vaddr) override { @@ -127,6 +116,23 @@ public: return; } + static constexpr auto ExceptionToString = [](Dynarmic::A32::Exception e) -> std::string { + switch (e) { + case Dynarmic::A32::Exception::UndefinedInstruction: + return "UndefinedInstruction"; + case Dynarmic::A32::Exception::UnpredictableInstruction: + return "UnpredictableInstruction"; + case Dynarmic::A32::Exception::DecodeError: + return "DecodeError"; + case Dynarmic::A32::Exception::NoExecuteFault: + return "NoExecuteFault"; + case Dynarmic::A32::Exception::Breakpoint: + return "Breakpoint"; + default: + return fmt::format("Unknown({})", e); + } + }; + parent.SetPC(pc); #ifdef ENABLE_GDBSTUB if (GDBStub::IsConnected()) { @@ -138,7 +144,8 @@ public: for (int i = 0; i < 16; i++) { error += fmt::format("r{:02d} = {:08X}\n", i, parent.GetReg(i)); } - error += fmt::format("ExceptionRaised(exception = {}, pc = {:08X})", exception, pc); + error += fmt::format("ExceptionRaised(exception = {}, pc = {:08X})", + ExceptionToString(exception), pc); parent.system.SetStatus(Core::System::ResultStatus::ErrorCoreExceptionRaised, error.c_str()); } diff --git a/src/core/memory.cpp b/src/core/memory.cpp index dafe6d0b4..ee446eb6d 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -13,6 +13,7 @@ #include "common/atomic_ops.h" #include "common/common_types.h" #include "common/logging/log.h" +#include "common/optional_helper.h" #include "common/settings.h" #include "common/swap.h" #include "core/arm/arm_interface.h" @@ -564,7 +565,7 @@ void MemorySystem::UnregisterPageTable(std::shared_ptr page_table) { } template -T MemorySystem::UnmappedAccess(const VAddr vaddr, const T value, bool read) { +void MemorySystem::UnmappedAccess(const VAddr vaddr, const T value, bool read) { const std::string mode = (read ? "Read" : "Write"); const std::string value_str = read ? std::string("") : fmt::format(" 0x{:08X}", value); const std::string message = fmt::format("unmapped {}{}{} @ 0x{:08X} at PC 0x{:08X}", mode, @@ -580,16 +581,20 @@ T MemorySystem::UnmappedAccess(const VAddr vaddr, const T value, bool read) { } LOG_ERROR(HW_Memory, "{}", message); - return {}; } template T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr vaddr) { + constexpr bool is_optional = is_optional_type; + using ReadType = optional_inner_or_type; + + constexpr size_t read_size = sizeof(ReadType); + const u8* page_pointer = page_table->pointers[vaddr >> CITRA_PAGE_BITS]; if (page_pointer) { // NOTE: Avoid adding any extra logic to this fast-path block - T value; - std::memcpy(&value, &page_pointer[vaddr & CITRA_PAGE_MASK], sizeof(T)); + ReadType value; + std::memcpy(&value, &page_pointer[vaddr & CITRA_PAGE_MASK], read_size); return value; } @@ -598,20 +603,27 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v if (vaddr & (1 << 31)) { PAddr paddr = (vaddr & ~(1 << 31)); if ((paddr & 0xF0000000) == Memory::FCRAM_PADDR) { // Check FCRAM region - T value; - std::memcpy(&value, GetFCRAMPointer(paddr - Memory::FCRAM_PADDR), sizeof(T)); + ReadType value; + std::memcpy(&value, GetFCRAMPointer(paddr - Memory::FCRAM_PADDR), read_size); return value; } else if ((paddr & 0xF0000000) == 0x10000000 && paddr >= Memory::IO_AREA_PADDR) { // Check MMIO region - return impl->system.GPU().ReadReg(static_cast(paddr) - Memory::IO_AREA_PADDR + - 0x1EC00000); + return static_cast(impl->system.GPU().ReadReg( + static_cast(paddr) - Memory::IO_AREA_PADDR + 0x1EC00000)); } } PageType type = page_table->attributes[vaddr >> CITRA_PAGE_BITS]; switch (type) { case PageType::Unmapped: { - return UnmappedAccess(vaddr, 0, true); + + UnmappedAccess(vaddr, 0, true); + + if constexpr (is_optional) { + return std::nullopt; + } else { + return T{}; + } } case PageType::Memory: ASSERT_MSG(false, "Mapped memory page without a pointer @ {:08X}", vaddr); @@ -621,11 +633,11 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v ASSERT_MSG(it != page_table->watchpoint_pages_map.end(), "Missing memory for watchpoint page"); - T value; - std::memcpy(&value, it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK), sizeof(T)); + ReadType value; + std::memcpy(&value, it->second.memory.GetPtr() + (vaddr & CITRA_PAGE_MASK), read_size); #ifdef ENABLE_GDBSTUB - if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Read)) { + if (GDBStub::CheckBreakpoint(vaddr, read_size, GDBStub::BreakpointType::Read)) { GDBStub::Break(SIGTRAP); } #endif @@ -633,20 +645,20 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v return value; } [[likely]] case PageType::RasterizerCachedMemory: { - RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Flush); + RasterizerFlushVirtualRegion(vaddr, read_size, FlushMode::Flush); - T value; - std::memcpy(&value, GetPointerForRasterizerCache(vaddr), sizeof(T)); + ReadType value; + std::memcpy(&value, GetPointerForRasterizerCache(vaddr), read_size); return value; } case PageType::RasterizerCachedMemoryWatchpoint: { - RasterizerFlushVirtualRegion(vaddr, sizeof(T), FlushMode::Flush); + RasterizerFlushVirtualRegion(vaddr, read_size, FlushMode::Flush); - T value; - std::memcpy(&value, GetPointerForRasterizerCache(vaddr), sizeof(T)); + ReadType value; + std::memcpy(&value, GetPointerForRasterizerCache(vaddr), read_size); #ifdef ENABLE_GDBSTUB - if (GDBStub::CheckBreakpoint(vaddr, sizeof(T), GDBStub::BreakpointType::Read)) { + if (GDBStub::CheckBreakpoint(vaddr, read_size, GDBStub::BreakpointType::Read)) { GDBStub::Break(SIGTRAP); } #endif @@ -657,7 +669,11 @@ T MemorySystem::Read(const std::shared_ptr& page_table, const VAddr v UNREACHABLE(); } - return T{}; + if constexpr (is_optional) { + return std::nullopt; + } else { + return T{}; + } } template @@ -1047,6 +1063,14 @@ u64 MemorySystem::Read64(const Kernel::Process& process, VAddr addr) { return Read(process.vm_manager.page_table, addr); } +std::optional MemorySystem::Read32OrNullopt(VAddr addr) { + return Read>(impl->current_page_table, addr); +} + +std::optional MemorySystem::Read32OrNullopt(const Kernel::Process& process, VAddr addr) { + return Read>(process.vm_manager.page_table, addr); +} + void MemorySystem::ReadBlock(const Kernel::Process& process, const VAddr src_addr, void* dest_buffer, const std::size_t size) { return impl->ReadBlockImpl(process, src_addr, dest_buffer, size); diff --git a/src/core/memory.h b/src/core/memory.h index 1433ad7d8..d4f09fa9e 100644 --- a/src/core/memory.h +++ b/src/core/memory.h @@ -5,11 +5,13 @@ #pragma once #include #include +#include #include #include #include #include "common/common_types.h" #include "common/memory_ref.h" +#include "common/swap.h" namespace Kernel { class Process; @@ -387,6 +389,29 @@ public: */ u64 Read64(const Kernel::Process& process, VAddr addr); + /** + * Reads a 32-bit unsigned value from the current process' address space + * at the given virtual address. If the address is invalid std::nullopt + * is returned instead. + * + * @param addr The virtual address to read the 32-bit value from. + * + * @returns the read 32-bit unsigned value or std::nullopt. + */ + std::optional Read32OrNullopt(VAddr addr); + + /** + * Reads a 32-bit unsigned value from the process' address space + * at the given virtual address. If the address is invalid std::nullopt + * is returned instead. + * + * @param process The process to read from. + * @param addr The virtual address to read the 32-bit value from. + * + * @returns the read 32-bit unsigned value or std::nullopt. + */ + std::optional Read32OrNullopt(const Kernel::Process& process, VAddr addr); + /** * Writes an 8-bit unsigned integer to the given virtual address in * the current process' address space. @@ -682,7 +707,7 @@ public: private: template - T UnmappedAccess(const VAddr vaddr, const T value, bool read); + void UnmappedAccess(const VAddr vaddr, const T value, bool read); template T Read(const std::shared_ptr& page_table, const VAddr vaddr);