From d30ac20fad4e9f384987046bdfbdb8ca7eef3d56 Mon Sep 17 00:00:00 2001 From: Eladash Date: Fri, 29 Jul 2022 18:58:38 +0300 Subject: [PATCH] Debugger: Fix and use centered PC by default * Fix instruction-selection dependent functionality. * Remove odd instruction position jumps in add/remove breakpoint. * Make PC fixate at 1/3 of the frame because knowing future instructions is more important than knowing the previous. --- rpcs3/rpcs3qt/breakpoint_list.cpp | 4 - rpcs3/rpcs3qt/debugger_frame.cpp | 15 ++-- rpcs3/rpcs3qt/debugger_list.cpp | 120 +++++++++++++++++++----------- rpcs3/rpcs3qt/debugger_list.h | 10 ++- rpcs3/rpcs3qt/gui_settings.h | 1 - 5 files changed, 92 insertions(+), 58 deletions(-) diff --git a/rpcs3/rpcs3qt/breakpoint_list.cpp b/rpcs3/rpcs3qt/breakpoint_list.cpp index 15cf48467e..a00b9c7ba2 100644 --- a/rpcs3/rpcs3qt/breakpoint_list.cpp +++ b/rpcs3/rpcs3qt/breakpoint_list.cpp @@ -68,8 +68,6 @@ void breakpoint_list::RemoveBreakpoint(u32 addr) } } - Q_EMIT RequestShowAddress(addr); - if (!count()) { hide(); @@ -94,8 +92,6 @@ bool breakpoint_list::AddBreakpoint(u32 pc) breakpoint_item->setData(Qt::UserRole, pc); addItem(breakpoint_item); - Q_EMIT RequestShowAddress(pc); - show(); return true; diff --git a/rpcs3/rpcs3qt/debugger_frame.cpp b/rpcs3/rpcs3qt/debugger_frame.cpp index b58b9999d7..a6d4f11803 100644 --- a/rpcs3/rpcs3qt/debugger_frame.cpp +++ b/rpcs3/rpcs3qt/debugger_frame.cpp @@ -360,7 +360,8 @@ void debugger_frame::keyPressEvent(QKeyEvent* event) } const u32 address_limits = (cpu->id_type() == 2 ? 0x3fffc : ~3); - const u32 pc = (row >= 0 ? m_debugger_list->m_pc + row * 4 : cpu->get_pc()) & address_limits; + const u32 pc = (m_debugger_list->m_pc & address_limits); + const u32 selected = (m_debugger_list->m_showing_selected_instruction ? m_debugger_list->m_selected_instruction : cpu->get_pc()) & address_limits; const auto modifiers = QApplication::keyboardModifiers(); @@ -568,7 +569,7 @@ void debugger_frame::keyPressEvent(QKeyEvent* event) { if (!m_inst_editor) { - m_inst_editor = new instruction_editor_dialog(this, pc, m_disasm.get(), make_check_cpu(cpu)); + m_inst_editor = new instruction_editor_dialog(this, selected, m_disasm.get(), make_check_cpu(cpu)); connect(m_inst_editor, &QDialog::finished, this, [this]() { m_inst_editor = nullptr; }); m_inst_editor->show(); } @@ -625,19 +626,21 @@ void debugger_frame::keyPressEvent(QKeyEvent* event) // Indirect branches (unknown targets, such as function return) do not proceed to any instruction std::array res{umax, umax}; + const u32 selected = (m_debugger_list->m_showing_selected_instruction ? m_debugger_list->m_selected_instruction : cpu->get_pc()) & address_limits; + switch (cpu->id_type()) { case 2: { - res = op_branch_targets(pc, spu_opcode_t{static_cast(cpu)->_ref(pc)}); + res = op_branch_targets(selected, spu_opcode_t{static_cast(cpu)->_ref(selected)}); break; } case 1: { be_t op{}; - if (vm::check_addr(pc, vm::page_executable) && vm::try_access(pc, &op, 4, false)) - res = op_branch_targets(pc, op); + if (vm::check_addr(selected, vm::page_executable) && vm::try_access(selected, &op, 4, false)) + res = op_branch_targets(selected, op); break; } @@ -645,7 +648,7 @@ void debugger_frame::keyPressEvent(QKeyEvent* event) } if (const usz pos = std::basic_string_view(res.data(), 2).find_last_not_of(umax); pos != umax) - m_debugger_list->ShowAddress(res[pos] - std::max(row, 0) * 4, true, true); + m_debugger_list->ShowAddress(res[pos] - std::max(row, 0) * 4, true); return; } diff --git a/rpcs3/rpcs3qt/debugger_list.cpp b/rpcs3/rpcs3qt/debugger_list.cpp index adc93cc1de..570857ec29 100644 --- a/rpcs3/rpcs3qt/debugger_list.cpp +++ b/rpcs3/rpcs3qt/debugger_list.cpp @@ -34,6 +34,26 @@ debugger_list::debugger_list(QWidget* parent, std::shared_ptr gui_ setSizeAdjustPolicy(QListWidget::AdjustToContents); setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + + connect(this, &QListWidget::currentRowChanged, this, [this](int row) + { + if (row < 0) + { + m_selected_instruction = -1; + m_showing_selected_instruction = false; + return; + } + + u32 pc = m_start_addr; + + for (; m_cpu && m_cpu->id_type() == 0x55 && row; row--) + { + // If scrolling forwards (downwards), we can skip entire commands + pc += std::max(m_disasm->disasm(pc), 4); + } + + m_selected_instruction = pc + row * 4; + }); } void debugger_list::UpdateCPUData(cpu_thread* cpu, CPUDisAsm* disasm) @@ -42,17 +62,40 @@ void debugger_list::UpdateCPUData(cpu_thread* cpu, CPUDisAsm* disasm) { m_cpu = cpu; m_selected_instruction = -1; + m_showing_selected_instruction = false; } m_disasm = disasm; } -u32 debugger_list::GetCenteredAddress(u32 address) const +u32 debugger_list::GetStartAddress(u32 address) { - return address - ((m_item_count / 2) * 4); + const u32 steps = m_item_count / 3; + + u32 result = address; + + if (m_cpu && m_cpu->id_type() == 0x55) + { + if (auto [count, res] = static_cast(m_cpu)->try_get_pc_of_x_cmds_backwards(steps, address); count == steps) + { + result = res; + } + } + else + { + result = address - (steps * 4); + } + + if (address > m_pc || m_start_addr > address) + { + m_pc = address; + m_start_addr = result; + } + + return m_start_addr; } -void debugger_list::ShowAddress(u32 addr, bool select_addr, bool force) +void debugger_list::ShowAddress(u32 addr, bool select_addr, bool direct) { const decltype(spu_thread::local_breakpoints)* spu_bps_list; @@ -71,32 +114,24 @@ void debugger_list::ShowAddress(u32 addr, bool select_addr, bool force) } }; - bool center_pc = m_gui_settings->GetValue(gui::d_centerPC).toBool(); - - // How many spaces addr can move down without us needing to move the entire view - const u32 addr_margin = (m_item_count / (center_pc ? 2 : 1) - 4); // 4 is just a buffer of 4 spaces at the bottom - - if (select_addr || force) + if (select_addr || direct) { // The user wants to survey a specific memory location, do not interfere from this point forth m_follow_thread = false; } - if (force || ((m_follow_thread || select_addr) && addr - m_pc > addr_margin * 4)) // 4 is the number of bytes in each instruction + u32 pc = m_start_addr; + + if (!direct && (m_follow_thread || select_addr)) { - if (center_pc) - { - m_pc = GetCenteredAddress(addr); - } - else - { - m_pc = addr; - } + pc = GetStartAddress(addr); } const auto& default_foreground = palette().color(foregroundRole()); const auto& default_background = palette().color(backgroundRole()); + m_showing_selected_instruction = false; + if (select_addr) { m_selected_instruction = addr; @@ -124,14 +159,14 @@ void debugger_list::ShowAddress(u32 addr, bool select_addr, bool force) { const bool is_spu = m_cpu->id_type() == 2; const u32 address_limits = (is_spu ? 0x3fffc : ~3); - m_pc &= address_limits; - u32 pc = m_pc; + m_start_addr &= address_limits; + u32 pc = m_start_addr; for (uint i = 0, count = 4; i < m_item_count; ++i, pc = (pc + count) & address_limits) { QListWidgetItem* list_item = item(i); - if (m_cpu->is_paused() && pc == m_cpu->get_pc()) + if (pc == m_cpu->get_pc()) { list_item->setForeground(m_text_color_pc); list_item->setBackground(m_color_pc); @@ -143,6 +178,8 @@ void debugger_list::ShowAddress(u32 addr, bool select_addr, bool force) { list_item->setSelected(true); } + + m_showing_selected_instruction = true; } else if (IsBreakpoint(pc)) { @@ -204,26 +241,25 @@ void debugger_list::EnableThreadFollowing(bool enable) void debugger_list::scroll(s32 steps) { - while (m_cpu && m_cpu->id_type() == 0x55 && steps > 0) + for (; m_cpu && m_cpu->id_type() == 0x55 && steps > 0; steps--) { // If scrolling forwards (downwards), we can skip entire commands - // Backwards is impossible though - m_pc += std::max(m_disasm->disasm(m_pc), 4); - steps--; + m_start_addr += std::max(m_disasm->disasm(m_start_addr), 4); } if (m_cpu && m_cpu->id_type() == 0x55 && steps < 0) { // If scrolling backwards (upwards), try to obtain the start of commands tail - if (auto [count, res] = static_cast(m_cpu)->try_get_pc_of_x_cmds_backwards(-steps, m_pc); count == 0u - steps) + if (auto [count, res] = static_cast(m_cpu)->try_get_pc_of_x_cmds_backwards(-steps, m_start_addr); count == 0u - steps) { steps = 0; - m_pc = res; + m_start_addr = res; } } EnableThreadFollowing(false); - ShowAddress(m_pc + (steps * 4), false, true); + m_start_addr += steps * 4; + ShowAddress(0, false, true); } void debugger_list::keyPressEvent(QKeyEvent* event) @@ -247,7 +283,7 @@ void debugger_list::keyPressEvent(QKeyEvent* event) } if (m_cpu && m_cpu->id_type() == 0x55) { - create_rsx_command_detail(m_pc, currentRow()); + create_rsx_command_detail(m_showing_selected_instruction ? m_selected_instruction : m_pc); return; } return; @@ -269,19 +305,8 @@ void debugger_list::hideEvent(QHideEvent* event) QListWidget::hideEvent(event); } -void debugger_list::create_rsx_command_detail(u32 pc, int row) +void debugger_list::create_rsx_command_detail(u32 pc) { - if (row < 0) - { - return; - } - - for (; row > 0; row--) - { - // Skip methods - pc += std::max(m_disasm->disasm(pc), 4); - } - RSXDisAsm rsx_dis = *static_cast(m_disasm); rsx_dis.change_mode(cpu_disasm_mode::list); @@ -321,10 +346,19 @@ void debugger_list::mouseDoubleClickEvent(QMouseEvent* event) { if (event->button() == Qt::LeftButton) { - const int i = currentRow(); + int i = currentRow(); if (i < 0) return; - const u32 pc = m_pc + i * 4; + u32 pc = m_start_addr; + + for (; m_cpu && m_cpu->id_type() == 0x55 && i; i--) + { + // If scrolling forwards (downwards), we can skip entire commands + pc += std::max(m_disasm->disasm(pc), 4); + } + + pc += i * 4; + m_selected_instruction = pc; // Let debugger_frame know about breakpoint. // Other option is to add to breakpoint manager directly and have a signal there instead. diff --git a/rpcs3/rpcs3qt/debugger_list.h b/rpcs3/rpcs3qt/debugger_list.h index e458f1542f..4f1b19d1e9 100644 --- a/rpcs3/rpcs3qt/debugger_list.h +++ b/rpcs3/rpcs3qt/debugger_list.h @@ -18,9 +18,11 @@ class debugger_list : public QListWidget public: u32 m_pc = 0; + u32 m_start_addr = 0; u32 m_item_count = 30; - bool m_follow_thread = true; // If true, follow the selected thread to wherever it goes in code u32 m_selected_instruction = -1; + bool m_follow_thread = true; // If true, follow the selected thread to wherever it goes in code + bool m_showing_selected_instruction = false; QColor m_color_bp; QColor m_color_pc; QColor m_text_color_bp; @@ -33,7 +35,7 @@ public: void UpdateCPUData(cpu_thread* cpu, CPUDisAsm* disasm); void EnableThreadFollowing(bool enable = true); public Q_SLOTS: - void ShowAddress(u32 addr, bool select_addr = true, bool force = false); + void ShowAddress(u32 addr, bool select_addr = true, bool direct = false); void RefreshView(); protected: void keyPressEvent(QKeyEvent* event) override; @@ -43,12 +45,12 @@ protected: void showEvent(QShowEvent* event) override; void hideEvent(QHideEvent* event) override; void scroll(s32 steps); - void create_rsx_command_detail(u32 pc, int row); + void create_rsx_command_detail(u32 pc); private: /** * It really upsetted me I had to copy this code to make debugger_list/frame not circularly dependent. */ - u32 GetCenteredAddress(u32 address) const; + u32 GetStartAddress(u32 address); std::shared_ptr m_gui_settings; diff --git a/rpcs3/rpcs3qt/gui_settings.h b/rpcs3/rpcs3qt/gui_settings.h index 49f2a1471f..782d035f18 100644 --- a/rpcs3/rpcs3qt/gui_settings.h +++ b/rpcs3/rpcs3qt/gui_settings.h @@ -196,7 +196,6 @@ namespace gui const gui_save l_limit_tty = gui_save(logger, "TTY_limit", 1000); const gui_save d_splitterState = gui_save(debugger, "splitterState", QByteArray()); - const gui_save d_centerPC = gui_save(debugger, "centerPC", false); const gui_save rsx_geometry = gui_save(rsx, "geometry", QByteArray()); const gui_save rsx_states = gui_save(rsx, "states", QVariantMap());