diff --git a/src/core/cpu_core.cpp b/src/core/cpu_core.cpp index 743878f5c..83c9ced25 100644 --- a/src/core/cpu_core.cpp +++ b/src/core/cpu_core.cpp @@ -48,7 +48,12 @@ static bool IsCop0ExecutionBreakpointUnmasked(); static void Cop0ExecutionBreakpointCheck(); template static void Cop0DataBreakpointCheck(VirtualMemoryAddress address); -static void BreakpointCheck(); + +static BreakpointList& GetBreakpointList(BreakpointType type); +static bool CheckBreakpointList(BreakpointType type, VirtualMemoryAddress address); +static void ExecutionBreakpointCheck(); +template +static void MemoryBreakpointCheck(VirtualMemoryAddress address); #ifdef _DEBUG static void TracePrintInstruction(); @@ -94,10 +99,11 @@ static bool s_log_file_opened = false; static bool s_trace_to_log = false; static constexpr u32 INVALID_BREAKPOINT_PC = UINT32_C(0xFFFFFFFF); -static std::vector s_breakpoints; +static std::array, static_cast(BreakpointType::Count)> s_breakpoints; static u32 s_breakpoint_counter = 1; static u32 s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC; -static u8 s_single_step = 0; // 0 - off, 1 - executing one, 2 - done +static bool s_single_step = false; +static bool s_break_after_instruction = false; } // namespace CPU bool CPU::IsTraceEnabled() @@ -156,10 +162,12 @@ void CPU::Initialize() g_state.cop0_regs.PRID = UINT32_C(0x00000002); g_state.use_debug_dispatcher = false; - s_breakpoints.clear(); + for (BreakpointList& bps : s_breakpoints) + bps.clear(); s_breakpoint_counter = 1; s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC; - s_single_step = 0; + s_single_step = false; + s_break_after_instruction = false; UpdateMemoryPointers(); @@ -1436,7 +1444,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } u8 value; if (!ReadMemoryByte(addr, &value)) @@ -1455,7 +1466,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } u16 value; if (!ReadMemoryHalfWord(addr, &value)) @@ -1473,7 +1487,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } u32 value; if (!ReadMemoryWord(addr, &value)) @@ -1490,7 +1507,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } u8 value; if (!ReadMemoryByte(addr, &value)) @@ -1508,7 +1528,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } u16 value; if (!ReadMemoryHalfWord(addr, &value)) @@ -1528,7 +1551,10 @@ restart_instruction: const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); const VirtualMemoryAddress aligned_addr = addr & ~UINT32_C(3); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } u32 aligned_value; if (!ReadMemoryWord(aligned_addr, &aligned_value)) @@ -1560,7 +1586,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } const u32 value = ReadReg(inst.i.rt); WriteMemoryByte(addr, value); @@ -1574,7 +1603,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } const u32 value = ReadReg(inst.i.rt); WriteMemoryHalfWord(addr, value); @@ -1588,7 +1620,10 @@ restart_instruction: { const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); if constexpr (debug) + { Cop0DataBreakpointCheck(addr); + MemoryBreakpointCheck(addr); + } const u32 value = ReadReg(inst.i.rt); WriteMemoryWord(addr, value); @@ -1604,7 +1639,10 @@ restart_instruction: const VirtualMemoryAddress addr = ReadReg(inst.i.rs) + inst.i.imm_sext32(); const VirtualMemoryAddress aligned_addr = addr & ~UINT32_C(3); if constexpr (debug) + { Cop0DataBreakpointCheck(aligned_addr); + MemoryBreakpointCheck(aligned_addr); + } const u32 reg_value = ReadReg(inst.i.rt); const u8 shift = (Truncate8(addr) & u8(3)) * u8(8); @@ -1933,7 +1971,7 @@ void CPU::DispatchInterrupt() bool CPU::UpdateDebugDispatcherFlag() { - const bool has_any_breakpoints = !s_breakpoints.empty() || (s_single_step != 0); + const bool has_any_breakpoints = HasAnyBreakpoints() || s_single_step; // TODO: cop0 breakpoints const auto& dcic = g_state.cop0_regs.dcic; @@ -1962,12 +2000,26 @@ void CPU::ExitExecution() bool CPU::HasAnyBreakpoints() { - return !s_breakpoints.empty(); + return (GetBreakpointList(BreakpointType::Execute).size() + GetBreakpointList(BreakpointType::Read).size() + + GetBreakpointList(BreakpointType::Write).size()) > 0; } -bool CPU::HasBreakpointAtAddress(VirtualMemoryAddress address) +ALWAYS_INLINE CPU::BreakpointList& CPU::GetBreakpointList(BreakpointType type) { - for (const Breakpoint& bp : s_breakpoints) + return s_breakpoints[static_cast(type)]; +} + +const char* CPU::GetBreakpointTypeName(BreakpointType type) +{ + static constexpr std::array(BreakpointType::Count)> names = { + {TRANSLATE_NOOP("DebuggerWindow", "Execute"), TRANSLATE_NOOP("DebuggerWindow", "Read"), + TRANSLATE_NOOP("DebuggerWindow", "Write")}}; + return Host::TranslateToCString("DebuggerWindow", names[static_cast(type)]); +} + +bool CPU::HasBreakpointAtAddress(BreakpointType type, VirtualMemoryAddress address) +{ + for (const Breakpoint& bp : GetBreakpointList(type)) { if (bp.address == address) return true; @@ -1976,68 +2028,79 @@ bool CPU::HasBreakpointAtAddress(VirtualMemoryAddress address) return false; } -CPU::BreakpointList CPU::GetBreakpointList(bool include_auto_clear, bool include_callbacks) +CPU::BreakpointList CPU::CopyBreakpointList(bool include_auto_clear, bool include_callbacks) { BreakpointList bps; - bps.reserve(s_breakpoints.size()); - for (const Breakpoint& bp : s_breakpoints) + size_t total = 0; + for (const BreakpointList& bplist : s_breakpoints) + total += bplist.size(); + + bps.reserve(total); + + for (const BreakpointList& bplist : s_breakpoints) { - if (bp.callback && !include_callbacks) - continue; - if (bp.auto_clear && !include_auto_clear) - continue; + for (const Breakpoint& bp : bplist) + { + if (bp.callback && !include_callbacks) + continue; + if (bp.auto_clear && !include_auto_clear) + continue; - bps.push_back(bp); + bps.push_back(bp); + } } return bps; } -bool CPU::AddBreakpoint(VirtualMemoryAddress address, bool auto_clear, bool enabled) +bool CPU::AddBreakpoint(BreakpointType type, VirtualMemoryAddress address, bool auto_clear, bool enabled) { - if (HasBreakpointAtAddress(address)) + if (HasBreakpointAtAddress(type, address)) return false; - Log_InfoPrintf("Adding breakpoint at %08X, auto clear = %u", address, static_cast(auto_clear)); + Log_InfoFmt("Adding {} breakpoint at {:08X}, auto clear = %u", GetBreakpointTypeName(type), address, + static_cast(auto_clear)); - Breakpoint bp{address, nullptr, auto_clear ? 0 : s_breakpoint_counter++, 0, auto_clear, enabled}; - s_breakpoints.push_back(std::move(bp)); + Breakpoint bp{address, nullptr, auto_clear ? 0 : s_breakpoint_counter++, 0, type, auto_clear, enabled}; + GetBreakpointList(type).push_back(std::move(bp)); if (UpdateDebugDispatcherFlag()) System::InterruptExecution(); if (!auto_clear) { - Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerMessage", "Added breakpoint at 0x%08X."), address); + Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerWindow", "Added breakpoint at 0x%08X."), address); } return true; } -bool CPU::AddBreakpointWithCallback(VirtualMemoryAddress address, BreakpointCallback callback) +bool CPU::AddBreakpointWithCallback(BreakpointType type, VirtualMemoryAddress address, BreakpointCallback callback) { - if (HasBreakpointAtAddress(address)) + if (HasBreakpointAtAddress(type, address)) return false; - Log_InfoPrintf("Adding breakpoint with callback at %08X", address); + Log_InfoFmt("Adding {} breakpoint with callback at {:08X}", GetBreakpointTypeName(type), address); - Breakpoint bp{address, callback, 0, 0, false, true}; - s_breakpoints.push_back(std::move(bp)); + Breakpoint bp{address, callback, 0, 0, type, false, true}; + GetBreakpointList(type).push_back(std::move(bp)); if (UpdateDebugDispatcherFlag()) System::InterruptExecution(); return true; } -bool CPU::RemoveBreakpoint(VirtualMemoryAddress address) +bool CPU::RemoveBreakpoint(BreakpointType type, VirtualMemoryAddress address) { - auto it = std::find_if(s_breakpoints.begin(), s_breakpoints.end(), - [address](const Breakpoint& bp) { return bp.address == address; }); - if (it == s_breakpoints.end()) + BreakpointList& bplist = GetBreakpointList(type); + auto it = + std::find_if(bplist.begin(), bplist.end(), [address](const Breakpoint& bp) { return bp.address == address; }); + if (it == bplist.end()) return false; - Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerMessage", "Removed breakpoint at 0x%08X."), address); + Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerWindow", "Removed %s breakpoint at 0x%08X."), + GetBreakpointTypeName(type), address); - s_breakpoints.erase(it); + bplist.erase(it); if (UpdateDebugDispatcherFlag()) System::InterruptExecution(); @@ -2049,7 +2112,8 @@ bool CPU::RemoveBreakpoint(VirtualMemoryAddress address) void CPU::ClearBreakpoints() { - s_breakpoints.clear(); + for (BreakpointList& bplist : s_breakpoints) + bplist.clear(); s_breakpoint_counter = 0; s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC; if (UpdateDebugDispatcherFlag()) @@ -2068,7 +2132,7 @@ bool CPU::AddStepOverBreakpoint() if (!IsCallInstruction(inst)) { - Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerMessage", "0x%08X is not a call instruction."), g_state.pc); + Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerWindow", "0x%08X is not a call instruction."), g_state.pc); return false; } @@ -2077,7 +2141,7 @@ bool CPU::AddStepOverBreakpoint() if (IsBranchInstruction(inst)) { - Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerMessage", "Can't step over double branch at 0x%08X"), + Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerWindow", "Can't step over double branch at 0x%08X"), g_state.pc); return false; } @@ -2085,9 +2149,9 @@ bool CPU::AddStepOverBreakpoint() // skip the delay slot bp_pc += sizeof(Instruction); - Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerMessage", "Stepping over to 0x%08X."), bp_pc); + Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerWindow", "Stepping over to 0x%08X."), bp_pc); - return AddBreakpoint(bp_pc, true); + return AddBreakpoint(BreakpointType::Execute, bp_pc, true); } bool CPU::AddStepOutBreakpoint(u32 max_instructions_to_search) @@ -2102,42 +2166,36 @@ bool CPU::AddStepOutBreakpoint(u32 max_instructions_to_search) if (!SafeReadInstruction(ret_pc, &inst.bits)) { Host::ReportFormattedDebuggerMessage( - TRANSLATE("DebuggerMessage", "Instruction read failed at %08X while searching for function end."), ret_pc); + TRANSLATE("DebuggerWindow", "Instruction read failed at %08X while searching for function end."), ret_pc); return false; } if (IsReturnInstruction(inst)) { - Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerMessage", "Stepping out to 0x%08X."), ret_pc); + Host::ReportFormattedDebuggerMessage(TRANSLATE("DebuggerWindow", "Stepping out to 0x%08X."), ret_pc); - return AddBreakpoint(ret_pc, true); + return AddBreakpoint(BreakpointType::Execute, ret_pc, true); } } Host::ReportFormattedDebuggerMessage( - TRANSLATE("DebuggerMessage", "No return instruction found after %u instructions for step-out at %08X."), + TRANSLATE("DebuggerWindow", "No return instruction found after %u instructions for step-out at %08X."), max_instructions_to_search, g_state.pc); return false; } -ALWAYS_INLINE_RELEASE void CPU::BreakpointCheck() +ALWAYS_INLINE_RELEASE bool CPU::CheckBreakpointList(BreakpointType type, VirtualMemoryAddress address) { - const u32 pc = g_state.pc; + BreakpointList& bplist = GetBreakpointList(type); + size_t count = bplist.size(); + if (count == 0) [[likely]] + return false; - if (pc == s_last_breakpoint_check_pc && !s_single_step) [[unlikely]] + for (size_t i = 0; i < count;) { - // we don't want to trigger the same breakpoint which just paused us repeatedly. - return; - } - - s_last_breakpoint_check_pc = pc; - - u32 count = static_cast(s_breakpoints.size()); - for (u32 i = 0; i < count;) - { - Breakpoint& bp = s_breakpoints[i]; - if (!bp.enabled || bp.address != pc) + Breakpoint& bp = bplist[i]; + if (!bp.enabled || bp.address != address) { i++; continue; @@ -2145,12 +2203,14 @@ ALWAYS_INLINE_RELEASE void CPU::BreakpointCheck() bp.hit_count++; + const u32 pc = g_state.pc; + if (bp.callback) { // if callback returns false, the bp is no longer recorded - if (!bp.callback(pc)) + if (!bp.callback(BreakpointType::Execute, pc, address)) { - s_breakpoints.erase(s_breakpoints.begin() + i); + bplist.erase(bplist.begin() + i); count--; UpdateDebugDispatcherFlag(); } @@ -2166,41 +2226,60 @@ ALWAYS_INLINE_RELEASE void CPU::BreakpointCheck() if (bp.auto_clear) { Host::ReportFormattedDebuggerMessage("Stopped execution at 0x%08X.", pc); - s_breakpoints.erase(s_breakpoints.begin() + i); + bplist.erase(bplist.begin() + i); count--; UpdateDebugDispatcherFlag(); } else { - Host::ReportFormattedDebuggerMessage("Hit breakpoint %u at 0x%08X.", bp.number, pc); + Host::ReportFormattedDebuggerMessage("Hit %s breakpoint %u at 0x%08X.", GetBreakpointTypeName(type), bp.number, + address); i++; } - s_single_step = 0; - ExitExecution(); + return true; } } - // single step - we want to break out after this instruction, so set a pending exit - // the bp check happens just before execution, so this is fine - if (s_single_step != 0) [[unlikely]] + return false; +} + +ALWAYS_INLINE_RELEASE void CPU::ExecutionBreakpointCheck() +{ + if (s_single_step) [[unlikely]] { - // not resetting this will break double single stepping, because we broke on the next instruction. - s_last_breakpoint_check_pc = INVALID_BREAKPOINT_PC; - - if (s_single_step == 2) - { - s_single_step = 0; - System::PauseSystem(true); - - Host::ReportFormattedDebuggerMessage("Stepped to 0x%08X.", g_state.pc); - UpdateDebugDispatcherFlag(); - ExitExecution(); - } - - // if there's a bp on the instruction we're about to execute, we should've paused already - s_single_step = 2; + // single step ignores breakpoints, since it stops anyway + s_single_step = false; + s_break_after_instruction = true; + Host::ReportFormattedDebuggerMessage("Stepped to 0x%08X.", g_state.npc); + return; } + + if (s_breakpoints[static_cast(BreakpointType::Execute)].empty()) [[likely]] + return; + + const u32 pc = g_state.pc; + if (pc == s_last_breakpoint_check_pc) [[unlikely]] + { + // we don't want to trigger the same breakpoint which just paused us repeatedly. + return; + } + + s_last_breakpoint_check_pc = pc; + + if (CheckBreakpointList(BreakpointType::Execute, pc)) [[unlikely]] + { + s_single_step = false; + ExitExecution(); + } +} + +template +ALWAYS_INLINE_RELEASE void CPU::MemoryBreakpointCheck(VirtualMemoryAddress address) +{ + const BreakpointType bptype = (type == MemoryAccessType::Read) ? BreakpointType::Read : BreakpointType::Write; + if (CheckBreakpointList(bptype, address)) + s_break_after_instruction = true; } template @@ -2215,7 +2294,7 @@ template if constexpr (debug) { Cop0ExecutionBreakpointCheck(); - BreakpointCheck(); + ExecutionBreakpointCheck(); } g_state.pending_ticks++; @@ -2258,6 +2337,17 @@ template // next load delay UpdateLoadDelay(); + + if constexpr (debug) + { + if (s_break_after_instruction) + { + s_break_after_instruction = false; + System::PauseSystem(true); + UpdateDebugDispatcherFlag(); + ExitExecution(); + } + } } } } @@ -2334,7 +2424,7 @@ void CPU::Execute() void CPU::SetSingleStepFlag() { - s_single_step = 1; + s_single_step = true; if (UpdateDebugDispatcherFlag()) System::InterruptExecution(); } diff --git a/src/core/cpu_core.h b/src/core/cpu_core.h index 5b7e1984e..961d2f616 100644 --- a/src/core/cpu_core.h +++ b/src/core/cpu_core.h @@ -191,8 +191,17 @@ bool IsTraceEnabled(); void StartTrace(); void StopTrace(); +// Breakpoint types - execute => breakpoint, read/write => watchpoints +enum class BreakpointType : u8 +{ + Execute, + Read, + Write, + Count +}; + // Breakpoint callback - if the callback returns false, the breakpoint will be removed. -using BreakpointCallback = bool (*)(VirtualMemoryAddress address); +using BreakpointCallback = bool (*)(BreakpointType type, VirtualMemoryAddress pc, VirtualMemoryAddress memaddr); struct Breakpoint { @@ -200,6 +209,7 @@ struct Breakpoint BreakpointCallback callback; u32 number; u32 hit_count; + BreakpointType type; bool auto_clear; bool enabled; }; @@ -207,12 +217,13 @@ struct Breakpoint using BreakpointList = std::vector; // Breakpoints +const char* GetBreakpointTypeName(BreakpointType type); bool HasAnyBreakpoints(); -bool HasBreakpointAtAddress(VirtualMemoryAddress address); -BreakpointList GetBreakpointList(bool include_auto_clear = false, bool include_callbacks = false); -bool AddBreakpoint(VirtualMemoryAddress address, bool auto_clear = false, bool enabled = true); -bool AddBreakpointWithCallback(VirtualMemoryAddress address, BreakpointCallback callback); -bool RemoveBreakpoint(VirtualMemoryAddress address); +bool HasBreakpointAtAddress(BreakpointType type, VirtualMemoryAddress address); +BreakpointList CopyBreakpointList(bool include_auto_clear = false, bool include_callbacks = false); +bool AddBreakpoint(BreakpointType type, VirtualMemoryAddress address, bool auto_clear = false, bool enabled = true); +bool AddBreakpointWithCallback(BreakpointType type, VirtualMemoryAddress address, BreakpointCallback callback); +bool RemoveBreakpoint(BreakpointType type, VirtualMemoryAddress address); void ClearBreakpoints(); bool AddStepOverBreakpoint(); bool AddStepOutBreakpoint(u32 max_instructions_to_search = 1000); diff --git a/src/core/gdb_protocol.cpp b/src/core/gdb_protocol.cpp index dc9af1cd8..b7792f262 100644 --- a/src/core/gdb_protocol.cpp +++ b/src/core/gdb_protocol.cpp @@ -219,7 +219,7 @@ static std::optional Cmd$z1(const std::string_view& data) { auto address = StringUtil::FromChars(data, 16); if (address) { - CPU::RemoveBreakpoint(*address); + CPU::RemoveBreakpoint(CPU::BreakpointType::Execute, *address); return { "OK" }; } else { @@ -232,7 +232,7 @@ static std::optional Cmd$Z1(const std::string_view& data) { auto address = StringUtil::FromChars(data, 16); if (address) { - CPU::AddBreakpoint(*address, false); + CPU::AddBreakpoint(CPU::BreakpointType::Execute, *address, false); return { "OK" }; } else { diff --git a/src/duckstation-qt/CMakeLists.txt b/src/duckstation-qt/CMakeLists.txt index ca6972170..32bf78998 100644 --- a/src/duckstation-qt/CMakeLists.txt +++ b/src/duckstation-qt/CMakeLists.txt @@ -64,6 +64,7 @@ set(SRCS coverdownloaddialog.cpp coverdownloaddialog.h coverdownloaddialog.ui + debuggeraddbreakpointdialog.ui debuggermodels.cpp debuggermodels.h debuggerwindow.cpp diff --git a/src/duckstation-qt/debuggeraddbreakpointdialog.ui b/src/duckstation-qt/debuggeraddbreakpointdialog.ui new file mode 100644 index 000000000..d02201ce4 --- /dev/null +++ b/src/duckstation-qt/debuggeraddbreakpointdialog.ui @@ -0,0 +1,93 @@ + + + DebuggerAddBreakpointDialog + + + + 0 + 0 + 400 + 149 + + + + Add Breakpoint + + + + + + Address: + + + + + + + + + + Type: + + + + + + + + + Execute + + + true + + + + + + + Read + + + + + + + Write + + + + + + + + + Qt::Horizontal + + + QDialogButtonBox::Cancel|QDialogButtonBox::Ok + + + + + + + + + buttonBox + rejected() + DebuggerAddBreakpointDialog + reject() + + + 316 + 260 + + + 286 + 274 + + + + + diff --git a/src/duckstation-qt/debuggermodels.cpp b/src/duckstation-qt/debuggermodels.cpp index c429df2ed..eff69bd2e 100644 --- a/src/duckstation-qt/debuggermodels.cpp +++ b/src/duckstation-qt/debuggermodels.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2019-2022 Connor McLaughlin +// SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin // SPDX-License-Identifier: (GPL-3.0 OR CC-BY-NC-ND-4.0) #include "debuggermodels.h" @@ -13,6 +13,8 @@ #include #include #include +#include +#include static constexpr int NUM_COLUMNS = 5; static constexpr int STACK_RANGE = 128; @@ -449,3 +451,44 @@ void DebuggerStackModel::invalidateView() beginResetModel(); endResetModel(); } + +DebuggerAddBreakpointDialog::DebuggerAddBreakpointDialog(QWidget* parent /*= nullptr*/) : QDialog(parent) +{ + m_ui.setupUi(this); + connect(m_ui.buttonBox->button(QDialogButtonBox::Ok), &QAbstractButton::clicked, this, + &DebuggerAddBreakpointDialog::okClicked); +} + +DebuggerAddBreakpointDialog::~DebuggerAddBreakpointDialog() = default; + +void DebuggerAddBreakpointDialog::okClicked() +{ + const QString address_str = m_ui.address->text(); + m_address = 0; + bool ok = false; + + if (!address_str.isEmpty()) + { + if (address_str.startsWith("0x")) + m_address = address_str.mid(2).toUInt(&ok, 16); + else + m_address = address_str.toUInt(&ok, 16); + + if (!ok) + { + QMessageBox::critical( + this, qApp->translate("DebuggerWindow", "Error"), + qApp->translate("DebuggerWindow", "Invalid address. It should be in hex (0x12345678 or 12345678)")); + return; + } + + if (m_ui.read->isChecked()) + m_type = CPU::BreakpointType::Read; + else if (m_ui.write->isChecked()) + m_type = CPU::BreakpointType::Write; + else + m_type = CPU::BreakpointType::Execute; + + accept(); + } +} diff --git a/src/duckstation-qt/debuggermodels.h b/src/duckstation-qt/debuggermodels.h index b3ab07d8a..1d0e0c93c 100644 --- a/src/duckstation-qt/debuggermodels.h +++ b/src/duckstation-qt/debuggermodels.h @@ -1,27 +1,32 @@ -// SPDX-FileCopyrightText: 2019-2022 Connor McLaughlin +// SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin // SPDX-License-Identifier: (GPL-3.0 OR CC-BY-NC-ND-4.0) #pragma once + +#include "ui_debuggeraddbreakpointdialog.h" + #include "core/bus.h" #include "core/cpu_core.h" #include "core/cpu_types.h" + #include #include #include +#include #include -class DebuggerCodeModel : public QAbstractTableModel +class DebuggerCodeModel final : public QAbstractTableModel { Q_OBJECT public: DebuggerCodeModel(QObject* parent = nullptr); - virtual ~DebuggerCodeModel(); + ~DebuggerCodeModel() override; - virtual int rowCount(const QModelIndex& parent = QModelIndex()) const override; - virtual int columnCount(const QModelIndex& parent = QModelIndex()) const override; - virtual QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; - virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; + int rowCount(const QModelIndex& parent = QModelIndex()) const override; + int columnCount(const QModelIndex& parent = QModelIndex()) const override; + QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; + QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; // Returns the row for this instruction pointer void resetCodeView(VirtualMemoryAddress start_address); @@ -51,39 +56,59 @@ private: QPixmap m_breakpoint_pixmap; }; -class DebuggerRegistersModel : public QAbstractListModel +class DebuggerRegistersModel final : public QAbstractListModel { Q_OBJECT public: DebuggerRegistersModel(QObject* parent = nullptr); - virtual ~DebuggerRegistersModel(); + ~DebuggerRegistersModel() override; - virtual int rowCount(const QModelIndex& parent = QModelIndex()) const override; - virtual int columnCount(const QModelIndex& parent = QModelIndex()) const override; - virtual QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; - virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; + int rowCount(const QModelIndex& parent = QModelIndex()) const override; + int columnCount(const QModelIndex& parent = QModelIndex()) const override; + QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; + QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; void updateValues(); void saveCurrentValues(); private: - std::array m_reg_values = {}; + std::array m_reg_values = {}; std::array m_old_reg_values = {}; }; -class DebuggerStackModel : public QAbstractListModel +class DebuggerStackModel final : public QAbstractListModel { Q_OBJECT public: DebuggerStackModel(QObject* parent = nullptr); - virtual ~DebuggerStackModel(); + ~DebuggerStackModel() override; - virtual int rowCount(const QModelIndex& parent = QModelIndex()) const override; - virtual int columnCount(const QModelIndex& parent = QModelIndex()) const override; - virtual QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; - virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; + int rowCount(const QModelIndex& parent = QModelIndex()) const override; + int columnCount(const QModelIndex& parent = QModelIndex()) const override; + QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; + QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; void invalidateView(); }; + +class DebuggerAddBreakpointDialog final : public QDialog +{ + Q_OBJECT + +public: + DebuggerAddBreakpointDialog(QWidget* parent = nullptr); + ~DebuggerAddBreakpointDialog() override; + + u32 getAddress() const { return m_address; } + CPU::BreakpointType getType() const { return m_type; } + +private Q_SLOTS: + void okClicked(); + +private: + Ui::DebuggerAddBreakpointDialog m_ui; + u32 m_address = 0; + CPU::BreakpointType m_type = CPU::BreakpointType::Execute; +}; diff --git a/src/duckstation-qt/debuggerwindow.cpp b/src/duckstation-qt/debuggerwindow.cpp index f6bec4822..34f86efcf 100644 --- a/src/duckstation-qt/debuggerwindow.cpp +++ b/src/duckstation-qt/debuggerwindow.cpp @@ -10,6 +10,7 @@ #include "core/cpu_core_private.h" #include +#include #include #include #include @@ -41,7 +42,6 @@ void DebuggerWindow::onSystemPaused() { setUIEnabled(true, true); refreshAll(); - refreshBreakpointList(); { QSignalBlocker sb(m_ui.actionPause); @@ -111,7 +111,7 @@ void DebuggerWindow::onRunToCursorTriggered() return; } - CPU::AddBreakpoint(addr.value(), true, true); + CPU::AddBreakpoint(CPU::BreakpointType::Execute, addr.value(), true, true); g_emu_thread->setSystemPaused(false); } @@ -163,18 +163,11 @@ void DebuggerWindow::onFollowAddressTriggered() void DebuggerWindow::onAddBreakpointTriggered() { - std::optional address = - QtUtils::PromptForAddress(this, windowTitle(), tr("Enter code address:"), true); - if (!address.has_value()) + DebuggerAddBreakpointDialog dlg(this); + if (!dlg.exec()) return; - if (CPU::HasBreakpointAtAddress(address.value())) - { - QMessageBox::critical(this, windowTitle(), tr("A breakpoint already exists at this address.")); - return; - } - - toggleBreakpoint(address.value()); + addBreakpoint(dlg.getType(), dlg.getAddress()); } void DebuggerWindow::onToggleBreakpointTriggered() @@ -191,6 +184,22 @@ void DebuggerWindow::onClearBreakpointsTriggered() clearBreakpoints(); } +void DebuggerWindow::onBreakpointListContextMenuRequested() +{ + const QList selected = m_ui.breakpointsWidget->selectedItems(); + if (selected.size() != 1) + return; + + const QTreeWidgetItem* item = selected[0]; + const u32 address = item->data(1, Qt::UserRole).toUInt(); + const CPU::BreakpointType type = static_cast(item->data(2, Qt::UserRole).toUInt()); + + QMenu menu(this); + connect(menu.addAction(tr("&Remove")), &QAction::triggered, this, + [this, address, type]() { removeBreakpoint(type, address); }); + menu.exec(QCursor::pos()); +} + void DebuggerWindow::onStepIntoActionTriggered() { Assert(System::IsPaused()); @@ -268,7 +277,7 @@ void DebuggerWindow::onCodeViewContextMenuRequested(const QPoint& pt) action = menu.addAction(QIcon::fromTheme("debugger-go-to-cursor"), tr("&Run To Cursor")); connect(action, &QAction::triggered, this, [address]() { Host::RunOnCPUThread([address]() { - CPU::AddBreakpoint(address, true, true); + CPU::AddBreakpoint(CPU::BreakpointType::Execute, address, true, true); g_emu_thread->setSystemPaused(false); }); }); @@ -426,6 +435,7 @@ void DebuggerWindow::setupAdditionalUi() m_ui.stackView->setFont(fixedFont); m_ui.codeView->setContextMenuPolicy(Qt::CustomContextMenu); + m_ui.breakpointsWidget->setContextMenuPolicy(Qt::CustomContextMenu); setCentralWidget(nullptr); delete m_ui.centralwidget; @@ -454,6 +464,8 @@ void DebuggerWindow::connectSignals() connect(m_ui.actionClose, &QAction::triggered, this, &DebuggerWindow::close); connect(m_ui.codeView, &QTreeView::activated, this, &DebuggerWindow::onCodeViewItemActivated); connect(m_ui.codeView, &QTreeView::customContextMenuRequested, this, &DebuggerWindow::onCodeViewContextMenuRequested); + connect(m_ui.breakpointsWidget, &QTreeWidget::customContextMenuRequested, this, + &DebuggerWindow::onBreakpointListContextMenuRequested); connect(m_ui.memoryRegionRAM, &QRadioButton::clicked, [this]() { setMemoryViewRegion(Bus::MemoryRegion::RAM); }); connect(m_ui.memoryRegionEXP1, &QRadioButton::clicked, [this]() { setMemoryViewRegion(Bus::MemoryRegion::EXP1); }); @@ -492,7 +504,8 @@ void DebuggerWindow::createModels() m_ui.breakpointsWidget->setColumnWidth(0, 50); m_ui.breakpointsWidget->setColumnWidth(1, 80); - m_ui.breakpointsWidget->setColumnWidth(2, 40); + m_ui.breakpointsWidget->setColumnWidth(2, 50); + m_ui.breakpointsWidget->setColumnWidth(3, 40); m_ui.breakpointsWidget->setRootIsDecorated(false); } @@ -553,19 +566,19 @@ void DebuggerWindow::setMemoryViewRegion(Bus::MemoryRegion region) void DebuggerWindow::toggleBreakpoint(VirtualMemoryAddress address) { Host::RunOnCPUThread([this, address]() { - const bool new_bp_state = !CPU::HasBreakpointAtAddress(address); + const bool new_bp_state = !CPU::HasBreakpointAtAddress(CPU::BreakpointType::Execute, address); if (new_bp_state) { - if (!CPU::AddBreakpoint(address, false)) + if (!CPU::AddBreakpoint(CPU::BreakpointType::Execute, address, false)) return; } else { - if (!CPU::RemoveBreakpoint(address)) + if (!CPU::RemoveBreakpoint(CPU::BreakpointType::Execute, address)) return; } - QtHost::RunOnUIThread([this, address, new_bp_state, bps = CPU::GetBreakpointList()]() { + QtHost::RunOnUIThread([this, address, new_bp_state, bps = CPU::CopyBreakpointList()]() { m_code_model->setBreakpointState(address, new_bp_state); refreshBreakpointList(bps); }); @@ -618,7 +631,8 @@ bool DebuggerWindow::scrollToMemoryAddress(VirtualMemoryAddress address) void DebuggerWindow::refreshBreakpointList() { - refreshBreakpointList(CPU::GetBreakpointList()); + Host::RunOnCPUThread( + [this]() { QtHost::RunOnUIThread([this, bps = CPU::CopyBreakpointList()]() { refreshBreakpointList(bps); }); }); } void DebuggerWindow::refreshBreakpointList(const CPU::BreakpointList& bps) @@ -633,7 +647,50 @@ void DebuggerWindow::refreshBreakpointList(const CPU::BreakpointList& bps) item->setCheckState(0, bp.enabled ? Qt::Checked : Qt::Unchecked); item->setText(0, QString::asprintf("%u", bp.number)); item->setText(1, QString::asprintf("0x%08X", bp.address)); - item->setText(2, QString::asprintf("%u", bp.hit_count)); + item->setText(2, QString::fromUtf8(CPU::GetBreakpointTypeName(bp.type))); + item->setText(3, QString::asprintf("%u", bp.hit_count)); + item->setData(0, Qt::UserRole, bp.number); + item->setData(1, Qt::UserRole, bp.address); + item->setData(2, Qt::UserRole, static_cast(bp.type)); m_ui.breakpointsWidget->addTopLevelItem(item); } } + +void DebuggerWindow::addBreakpoint(CPU::BreakpointType type, u32 address) +{ + Host::RunOnCPUThread([this, address, type]() { + const bool result = CPU::AddBreakpoint(type, address); + QtHost::RunOnUIThread([this, address, type, result, bps = CPU::CopyBreakpointList()]() { + if (!result) + { + QMessageBox::critical(this, windowTitle(), + tr("Failed to add breakpoint. A breakpoint may already exist at this address.")); + return; + } + + if (type == CPU::BreakpointType::Execute) + m_code_model->setBreakpointState(address, true); + + refreshBreakpointList(bps); + }); + }); +} + +void DebuggerWindow::removeBreakpoint(CPU::BreakpointType type, u32 address) +{ + Host::RunOnCPUThread([this, address, type]() { + const bool result = CPU::RemoveBreakpoint(type, address); + QtHost::RunOnUIThread([this, address, type, result, bps = CPU::CopyBreakpointList()]() { + if (!result) + { + QMessageBox::critical(this, windowTitle(), tr("Failed to remove breakpoint. This breakpoint may not exist.")); + return; + } + + if (type == CPU::BreakpointType::Execute) + m_code_model->setBreakpointState(address, false); + + refreshBreakpointList(bps); + }); + }); +} diff --git a/src/duckstation-qt/debuggerwindow.h b/src/duckstation-qt/debuggerwindow.h index ac68d056e..83816f45c 100644 --- a/src/duckstation-qt/debuggerwindow.h +++ b/src/duckstation-qt/debuggerwindow.h @@ -55,6 +55,7 @@ private Q_SLOTS: void onAddBreakpointTriggered(); void onToggleBreakpointTriggered(); void onClearBreakpointsTriggered(); + void onBreakpointListContextMenuRequested(); void onStepIntoActionTriggered(); void onStepOverActionTriggered(); void onStepOutActionTriggered(); @@ -78,6 +79,8 @@ private: bool scrollToMemoryAddress(VirtualMemoryAddress address); void refreshBreakpointList(); void refreshBreakpointList(const CPU::BreakpointList& bps); + void addBreakpoint(CPU::BreakpointType type, u32 address); + void removeBreakpoint(CPU::BreakpointType type, u32 address); Ui::DebuggerWindow m_ui; diff --git a/src/duckstation-qt/debuggerwindow.ui b/src/duckstation-qt/debuggerwindow.ui index 019b86fb2..5ecfc10d7 100644 --- a/src/duckstation-qt/debuggerwindow.ui +++ b/src/duckstation-qt/debuggerwindow.ui @@ -6,7 +6,7 @@ 0 0 - 1210 + 1270 800 @@ -233,7 +233,7 @@ - 200 + 290 524287 @@ -269,6 +269,11 @@ Address + + + Type + + Hit Count diff --git a/src/duckstation-qt/duckstation-qt.vcxproj b/src/duckstation-qt/duckstation-qt.vcxproj index 5b62a3d88..7a0b22aa9 100644 --- a/src/duckstation-qt/duckstation-qt.vcxproj +++ b/src/duckstation-qt/duckstation-qt.vcxproj @@ -332,6 +332,9 @@ Document + + Document + diff --git a/src/duckstation-qt/duckstation-qt.vcxproj.filters b/src/duckstation-qt/duckstation-qt.vcxproj.filters index e68a43847..b54f2db03 100644 --- a/src/duckstation-qt/duckstation-qt.vcxproj.filters +++ b/src/duckstation-qt/duckstation-qt.vcxproj.filters @@ -292,6 +292,7 @@ +