From 53cf78d413cb4e4cc546f027423a6a011cd30a64 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Thu, 17 Mar 2022 12:53:38 -0700 Subject: [PATCH] Gekko constistancy changes. Add context item to codeview to show or copy a load/store target memory address from instructions at or near PC when paused. --- Source/Core/Common/DebugInterface.h | 5 ++ Source/Core/Common/GekkoDisassembler.cpp | 87 ++++++++----------- Source/Core/Common/GekkoDisassembler.h | 4 +- .../Core/Core/Debugger/PPCDebugInterface.cpp | 50 +++++++++++ Source/Core/Core/Debugger/PPCDebugInterface.h | 1 + .../DolphinQt/Debugger/CodeViewWidget.cpp | 56 +++++++++++- .../Core/DolphinQt/Debugger/CodeViewWidget.h | 2 + 7 files changed, 149 insertions(+), 56 deletions(-) diff --git a/Source/Core/Common/DebugInterface.h b/Source/Core/Common/DebugInterface.h index 43d917e975..a59ac79f69 100644 --- a/Source/Core/Common/DebugInterface.h +++ b/Source/Core/Common/DebugInterface.h @@ -72,6 +72,11 @@ public: virtual void WriteExtraMemory(int /*memory*/, u32 /*value*/, u32 /*address*/) {} virtual u32 ReadExtraMemory(int /*memory*/, u32 /*address*/) const { return 0; } virtual u32 ReadInstruction(u32 /*address*/) const { return 0; } + virtual std::optional + GetMemoryAddressFromInstruction(const std::string& /*instruction*/) const + { + return std::nullopt; + } virtual u32 GetPC() const { return 0; } virtual void SetPC(u32 /*address*/) {} virtual void Step() {} diff --git a/Source/Core/Common/GekkoDisassembler.cpp b/Source/Core/Common/GekkoDisassembler.cpp index 17997f4f89..8d849b35c5 100644 --- a/Source/Core/Common/GekkoDisassembler.cpp +++ b/Source/Core/Common/GekkoDisassembler.cpp @@ -177,24 +177,23 @@ static u32 HelperRotateMask(int r, int mb, int me) static std::string ldst_offs(u32 val) { if (val == 0) - { return "0"; - } if (val & 0x8000) - { return fmt::format("-0x{:04X}", ((~val) & 0xffff) + 1); - } return fmt::format("0x{:04X}", val); } -static int SEX12(u32 x) +static std::string psq_offs(u32 val) { - if ((x & 0x800) != 0) - return static_cast(x | 0xFFFFF000); + if (val == 0) + return "0"; - return static_cast(x); + if ((val & 0x800) != 0) + return fmt::format("-0x{:04X}", ((~val) & 0xfff) + 1); + + return fmt::format("0x{:04X}", val); } static std::string spr_name(int i) @@ -452,24 +451,9 @@ std::string GekkoDisassembler::rd_ra_rb(u32 in, int mask) return result; } -std::string GekkoDisassembler::fd_ra_rb(u32 in, int mask) +std::string GekkoDisassembler::fd_ra_rb(u32 in) { - std::string result; - - if (mask) - { - if (mask & 4) - result += fmt::format("f{},", PPCGETD(in)); - if (mask & 2) - result += fmt::format("{},", regnames[PPCGETA(in)]); - if (mask & 1) - result += fmt::format("{},", regnames[PPCGETB(in)]); - - // Drop the trailing comma - result.pop_back(); - } - - return result; + return fmt::format("f{}, {}, {}", PPCGETD(in), regnames[PPCGETA(in)], regnames[PPCGETB(in)]); } void GekkoDisassembler::trapi(u32 in, unsigned char dmode) @@ -972,26 +956,23 @@ void GekkoDisassembler::fdabc(u32 in, std::string_view name, int mask, unsigned m_flags |= dmode; m_opcode = fmt::format("f{}{}", name, rcsel[in & 1]); - m_operands += fmt::format("f{},", PPCGETD(in)); + m_operands += fmt::format("f{}", PPCGETD(in)); if (mask & 4) - m_operands += fmt::format("f{},", PPCGETA(in)); + m_operands += fmt::format(", f{}", PPCGETA(in)); else if ((mask & 8) == 0) err |= (int)PPCGETA(in); if (mask & 2) - m_operands += fmt::format("f{},", PPCGETC(in)); + m_operands += fmt::format(", f{}", PPCGETC(in)); else if (PPCGETC(in) && (mask & 8) == 0) err |= (int)PPCGETC(in); if (mask & 1) - m_operands += fmt::format("f{},", PPCGETB(in)); + m_operands += fmt::format(", f{}", PPCGETB(in)); else if (!(mask & 8)) err |= (int)PPCGETB(in); - // Drop the trailing comma - m_operands.pop_back(); - if (err) ill(in); } @@ -1003,10 +984,10 @@ void GekkoDisassembler::fmr(u32 in) } // Indexed float instruction: xxxx fD,rA,rB -void GekkoDisassembler::fdab(u32 in, std::string_view name, int mask) +void GekkoDisassembler::fdab(u32 in, std::string_view name) { m_opcode = name; - m_operands = fd_ra_rb(in, mask); + m_operands = fd_ra_rb(in); } void GekkoDisassembler::fcmp(u32 in, char c) @@ -1018,7 +999,7 @@ void GekkoDisassembler::fcmp(u32 in, char c) else { m_opcode = fmt::format("fcmp{}", c); - m_operands = fmt::format("cr{},f{},f{}", PPCGETCRD(in), PPCGETA(in), PPCGETB(in)); + m_operands = fmt::format("cr{}, f{}, f{}", PPCGETCRD(in), PPCGETA(in), PPCGETB(in)); } } @@ -1092,7 +1073,7 @@ void GekkoDisassembler::ps(u32 inst) { case 6: m_opcode = inst & 0x40 ? "psq_lux" : "psq_lx"; - m_operands = fmt::format("p{}, (r{} + r{}), {}, qr{}", FD, RA, RB, WX, IX); + m_operands = fmt::format("p{}, r{}, r{}, {}, qr{}", FD, RA, RB, WX, IX); return; case 7: @@ -1224,22 +1205,22 @@ void GekkoDisassembler::ps(u32 inst) } case 528: m_opcode = "ps_merge00"; - m_operands = fmt::format("p{}, p{}[0],p{}[0]", FD, FA, FB); + m_operands = fmt::format("p{}, p{}[0], p{}[0]", FD, FA, FB); return; case 560: m_opcode = "ps_merge01"; - m_operands = fmt::format("p{}, p{}[0],p{}[1]", FD, FA, FB); + m_operands = fmt::format("p{}, p{}[0], p{}[1]", FD, FA, FB); return; case 592: m_opcode = "ps_merge10"; - m_operands = fmt::format("p{}, p{}[1],p{}[0]", FD, FA, FB); + m_operands = fmt::format("p{}, p{}[1], p{}[0]", FD, FA, FB); return; case 624: m_opcode = "ps_merge11"; - m_operands = fmt::format("p{}, p{}[1],p{}[1]", FD, FA, FB); + m_operands = fmt::format("p{}, p{}[1], p{}[1]", FD, FA, FB); return; case 1014: @@ -1261,23 +1242,23 @@ void GekkoDisassembler::ps_mem(u32 inst) { case 56: m_opcode = "psq_l"; - m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, SEX12(inst & 0xFFF), RA, W, I); + m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, psq_offs(inst & 0xFFF), RA, W, I); break; case 57: m_opcode = "psq_lu"; - m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, SEX12(inst & 0xFFF), RA, W, I); + m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, psq_offs(inst & 0xFFF), RA, W, I); ; break; case 60: m_opcode = "psq_st"; - m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, SEX12(inst & 0xFFF), RA, W, I); + m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, psq_offs(inst & 0xFFF), RA, W, I); break; case 61: m_opcode = "psq_stu"; - m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, SEX12(inst & 0xFFF), RA, W, I); + m_operands = fmt::format("p{}, {}(r{}), {}, qr{}", RS, psq_offs(inst & 0xFFF), RA, W, I); break; } } @@ -1924,7 +1905,7 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 535: - fdab(in, "lfsx", 7); + fdab(in, "lfsx"); break; case 536: @@ -1940,7 +1921,7 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 567: - fdab(in, "lfsux", 7); + fdab(in, "lfsux"); break; case 595: @@ -1956,11 +1937,11 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 599: - fdab(in, "lfdx", 7); + fdab(in, "lfdx"); break; case 631: - fdab(in, "lfdux", 7); + fdab(in, "lfdux"); break; case 659: @@ -1979,11 +1960,11 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 663: - fdab(in, "stfsx", 7); + fdab(in, "stfsx"); break; case 695: - fdab(in, "stfsux", 7); + fdab(in, "stfsux"); break; case 725: @@ -1991,11 +1972,11 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 727: - fdab(in, "stfdx", 7); + fdab(in, "stfdx"); break; case 759: - fdab(in, "stfdux", 7); + fdab(in, "stfdux"); break; case 790: @@ -2044,7 +2025,7 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian) break; case 983: - fdab(in, "stfiwx", 7); + fdab(in, "stfiwx"); break; case 986: diff --git a/Source/Core/Common/GekkoDisassembler.h b/Source/Core/Common/GekkoDisassembler.h index 8bed9e32a0..34991a28c4 100644 --- a/Source/Core/Common/GekkoDisassembler.h +++ b/Source/Core/Common/GekkoDisassembler.h @@ -56,7 +56,7 @@ private: static std::string ra_rb(u32 in); static std::string rd_ra_rb(u32 in, int mask); - static std::string fd_ra_rb(u32 in, int mask); + static std::string fd_ra_rb(u32 in); static void trapi(u32 in, unsigned char dmode); static void cmpi(u32 in, int uimm); @@ -84,7 +84,7 @@ private: static void ldst(u32 in, std::string_view name, char reg, unsigned char dmode); static void fdabc(u32 in, std::string_view name, int mask, unsigned char dmode); static void fmr(u32 in); - static void fdab(u32 in, std::string_view name, int mask); + static void fdab(u32 in, std::string_view name); static void fcmp(u32 in, char c); static void mtfsb(u32 in, int n); static void ps(u32 inst); diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.cpp b/Source/Core/Core/Debugger/PPCDebugInterface.cpp index 42082a5cfb..03d8331a11 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.cpp +++ b/Source/Core/Core/Debugger/PPCDebugInterface.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -375,6 +376,55 @@ std::string PPCDebugInterface::GetDescription(u32 address) const return g_symbolDB.GetDescription(address); } +std::optional +PPCDebugInterface::GetMemoryAddressFromInstruction(const std::string& instruction) const +{ + std::regex re(",[^r0-]*(-?)(0[xX]?[0-9a-fA-F]*|r\\d+)[^r^s]*.(p|toc|\\d+)"); + std::smatch match; + + // Instructions should be identified as a load or store before using this function. This error + // check should never trigger. + if (!std::regex_search(instruction, match, re)) + return std::nullopt; + + // Output: match.str(1): negative sign for offset or no match. match.str(2): 0xNNNN, 0, or + // rNN. Check next for 'r' to see if a gpr needs to be loaded. match.str(3): will either be p, + // toc, or NN. Always a gpr. + const std::string offset_match = match.str(2); + const std::string register_match = match.str(3); + constexpr char is_reg = 'r'; + u32 offset = 0; + + if (is_reg == offset_match[0]) + { + const int register_index = std::stoi(offset_match.substr(1), nullptr, 10); + offset = (register_index == 0 ? 0 : GPR(register_index)); + } + else + { + offset = static_cast(std::stoi(offset_match, nullptr, 16)); + } + + // sp and rtoc need to be converted to 1 and 2. + constexpr char is_sp = 'p'; + constexpr char is_rtoc = 't'; + u32 i = 0; + + if (is_sp == register_match[0]) + i = 1; + else if (is_rtoc == register_match[0]) + i = 2; + else + i = std::stoi(register_match, nullptr, 10); + + const u32 base_address = GPR(i); + + if (!match.str(1).empty()) + return base_address - offset; + + return base_address + offset; +} + u32 PPCDebugInterface::GetPC() const { return PowerPC::ppcState.pc; diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.h b/Source/Core/Core/Debugger/PPCDebugInterface.h index a323c5f5bd..4c5dfb0fec 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.h +++ b/Source/Core/Core/Debugger/PPCDebugInterface.h @@ -76,6 +76,7 @@ public: u32 ReadExtraMemory(int memory, u32 address) const override; u32 ReadInstruction(u32 address) const override; + std::optional GetMemoryAddressFromInstruction(const std::string& instruction) const override; u32 GetPC() const override; void SetPC(u32 address) override; void Step() override {} diff --git a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp index fb300a370f..76e43d50ad 100644 --- a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp @@ -235,6 +235,15 @@ static bool IsBranchInstructionWithLink(std::string_view ins) StringEndsWith(ins, "la+") || StringEndsWith(ins, "l-") || StringEndsWith(ins, "la-"); } +static bool IsInstructionLoadStore(std::string_view ins) +{ + // Could add check for context address being near PC, because we need gprs to be correct for the + // load/store. + return (StringBeginsWith(ins, "l") && !StringBeginsWith(ins, "li")) || + StringBeginsWith(ins, "st") || StringBeginsWith(ins, "psq_l") || + StringBeginsWith(ins, "psq_s"); +} + void CodeViewWidget::Update() { if (!isVisible()) @@ -530,7 +539,10 @@ void CodeViewWidget::OnContextMenu() auto* copy_hex_action = menu->addAction(tr("Copy &hex"), this, &CodeViewWidget::OnCopyHex); menu->addAction(tr("Show in &memory"), this, &CodeViewWidget::OnShowInMemory); - + auto* show_target_memory = + menu->addAction(tr("Show target in memor&y"), this, &CodeViewWidget::OnShowTargetInMemory); + auto* copy_target_memory = + menu->addAction(tr("Copy tar&get address"), this, &CodeViewWidget::OnCopyTargetAddress); menu->addSeparator(); auto* symbol_rename_action = @@ -561,6 +573,14 @@ void CodeViewWidget::OnContextMenu() for (auto* action : {symbol_rename_action, symbol_size_action, symbol_end_action}) action->setEnabled(has_symbol); + const bool valid_load_store = Core::GetState() == Core::State::Paused && + IsInstructionLoadStore(PowerPC::debug_interface.Disassemble(addr)); + + for (auto* action : {copy_target_memory, show_target_memory}) + { + action->setEnabled(valid_load_store); + } + restore_action->setEnabled(running && PowerPC::debug_interface.HasEnabledPatch(addr)); menu->exec(QCursor::pos()); @@ -574,11 +594,45 @@ void CodeViewWidget::OnCopyAddress() QApplication::clipboard()->setText(QStringLiteral("%1").arg(addr, 8, 16, QLatin1Char('0'))); } +void CodeViewWidget::OnCopyTargetAddress() +{ + if (Core::GetState() != Core::State::Paused) + return; + + const std::string code_line = PowerPC::debug_interface.Disassemble(GetContextAddress()); + + if (!IsInstructionLoadStore(code_line)) + return; + + const std::optional addr = + PowerPC::debug_interface.GetMemoryAddressFromInstruction(code_line); + + if (addr) + QApplication::clipboard()->setText(QStringLiteral("%1").arg(*addr, 8, 16, QLatin1Char('0'))); +} + void CodeViewWidget::OnShowInMemory() { emit ShowMemory(GetContextAddress()); } +void CodeViewWidget::OnShowTargetInMemory() +{ + if (Core::GetState() != Core::State::Paused) + return; + + const std::string code_line = PowerPC::debug_interface.Disassemble(GetContextAddress()); + + if (!IsInstructionLoadStore(code_line)) + return; + + const std::optional addr = + PowerPC::debug_interface.GetMemoryAddressFromInstruction(code_line); + + if (addr) + emit ShowMemory(*addr); +} + void CodeViewWidget::OnCopyCode() { const u32 addr = GetContextAddress(); diff --git a/Source/Core/DolphinQt/Debugger/CodeViewWidget.h b/Source/Core/DolphinQt/Debugger/CodeViewWidget.h index 64124b7268..c962971cb8 100644 --- a/Source/Core/DolphinQt/Debugger/CodeViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/CodeViewWidget.h @@ -70,7 +70,9 @@ private: void OnFollowBranch(); void OnCopyAddress(); + void OnCopyTargetAddress(); void OnShowInMemory(); + void OnShowTargetInMemory(); void OnCopyFunction(); void OnCopyCode(); void OnCopyHex();