From 4f4bd57fe94ee94bf25236b733641c7684ed4c48 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 1 Dec 2022 16:04:53 -0800 Subject: [PATCH 1/5] Fix crash when stopping emulation while the JIT widget is in use The call to analyzer.Analyze breaks when it attempts to read an instruction, as it eventually tries to read memory when Memory::m_pRAM is nullptr. Trying to read when execution is not paused in general seems like a bad idea (especially as analyzer.Analyze uses PowerPC::TryReadInstruction which can update icache - this is probably still a problem). --- Source/Core/DolphinQt/Debugger/JITWidget.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Debugger/JITWidget.cpp b/Source/Core/DolphinQt/Debugger/JITWidget.cpp index 8a5f97c15f..99d4249504 100644 --- a/Source/Core/DolphinQt/Debugger/JITWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/JITWidget.cpp @@ -12,6 +12,7 @@ #include #include "Common/GekkoDisassembler.h" +#include "Core/Core.h" #include "Core/PowerPC/PPCAnalyst.h" #include "UICommon/Disassembler.h" @@ -130,7 +131,7 @@ void JITWidget::Update() if (!isVisible()) return; - if (!m_address) + if (!m_address || (Core::GetState() != Core::State::Paused)) { m_ppc_asm_widget->setHtml(QStringLiteral("%1").arg(tr("(ppc)"))); m_host_asm_widget->setHtml(QStringLiteral("%1").arg(tr("(host)"))); From 0ccfa31ec89581cc06fc3bc2fc5c60f11acb4486 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 1 Dec 2022 16:43:41 -0800 Subject: [PATCH 2/5] Fix code widget not becoming visible when selecting 'view code' or similar This affected the memory and registers widgets (and possibly others). I'm pretty sure it regressed in 5f629abd8b89971872b0c2c2cdc8ea0035e8998d. The SetCodeVisible line is a new fix, but the equivalent already existed in the memory widget. --- Source/Core/DolphinQt/Debugger/CodeWidget.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp index 92e068fada..60f74e52ad 100644 --- a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp @@ -291,8 +291,10 @@ void CodeWidget::SetAddress(u32 address, CodeViewWidget::SetAddressUpdate update { m_code_view->SetAddress(address, update); - if (update == CodeViewWidget::SetAddressUpdate::WithUpdate) + if (update == CodeViewWidget::SetAddressUpdate::WithUpdate || + update == CodeViewWidget::SetAddressUpdate::WithDetailedUpdate) { + Settings::Instance().SetCodeVisible(true); raise(); m_code_view->setFocus(); } From 5842b90bee7fcd11196ceaeb8e98843095aedcef Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 1 Dec 2022 16:47:32 -0800 Subject: [PATCH 3/5] Show JIT blocks widget when selecting 'PPC vs Host' in code widget Before, I just assumed this feature was broken since I didn't know what widget it used. Now, it behaves like show memory and show code elsewhere. --- Source/Core/DolphinQt/Debugger/JITWidget.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Source/Core/DolphinQt/Debugger/JITWidget.cpp b/Source/Core/DolphinQt/Debugger/JITWidget.cpp index 99d4249504..68c39eb662 100644 --- a/Source/Core/DolphinQt/Debugger/JITWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/JITWidget.cpp @@ -123,6 +123,11 @@ void JITWidget::ConnectWidgets() void JITWidget::Compare(u32 address) { m_address = address; + + Settings::Instance().SetJITVisible(true); + raise(); + m_host_asm_widget->setFocus(); + Update(); } From 6a6d24550e0115df508d56d9ffc3764075e1e144 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 1 Dec 2022 17:16:11 -0800 Subject: [PATCH 4/5] Clean up DisassembleBlock and JitInterface::GetHostCode --- Source/Core/Core/PowerPC/JitInterface.cpp | 27 +++++------ Source/Core/Core/PowerPC/JitInterface.h | 15 +++++- Source/Core/DolphinQt/Debugger/JITWidget.cpp | 22 +++++---- Source/Core/UICommon/Disassembler.cpp | 50 ++++++++++++++------ Source/Core/UICommon/Disassembler.h | 11 ++++- 5 files changed, 83 insertions(+), 42 deletions(-) diff --git a/Source/Core/Core/PowerPC/JitInterface.cpp b/Source/Core/Core/PowerPC/JitInterface.cpp index 8e31441b4f..c91d46237a 100644 --- a/Source/Core/Core/PowerPC/JitInterface.cpp +++ b/Source/Core/Core/PowerPC/JitInterface.cpp @@ -146,43 +146,42 @@ void GetProfileResults(Profiler::ProfileStats* prof_stats) }); } -int GetHostCode(u32* address, const u8** code, u32* code_size) +std::variant GetHostCode(u32 address) { if (!g_jit) { - *code_size = 0; - return 1; + return GetHostCodeError::NoJitActive; } - JitBlock* block = g_jit->GetBlockCache()->GetBlockFromStartAddress(*address, MSR.Hex); + JitBlock* block = g_jit->GetBlockCache()->GetBlockFromStartAddress(address, MSR.Hex); if (!block) { for (int i = 0; i < 500; i++) { - block = g_jit->GetBlockCache()->GetBlockFromStartAddress(*address - 4 * i, MSR.Hex); + block = g_jit->GetBlockCache()->GetBlockFromStartAddress(address - 4 * i, MSR.Hex); if (block) break; } if (block) { - if (!(block->effectiveAddress <= *address && - block->originalSize + block->effectiveAddress >= *address)) + if (!(block->effectiveAddress <= address && + block->originalSize + block->effectiveAddress >= address)) block = nullptr; } - // Do not merge this "if" with the above - block_num changes inside it. + // Do not merge this "if" with the above - block changes inside it. if (!block) { - *code_size = 0; - return 2; + return GetHostCodeError::NoTranslation; } } - *code = block->checkedEntry; - *code_size = block->codeSize; - *address = block->effectiveAddress; - return 0; + GetHostCodeResult result; + result.code = block->checkedEntry; + result.code_size = block->codeSize; + result.entry_address = block->effectiveAddress; + return result; } bool HandleFault(uintptr_t access_address, SContext* ctx) diff --git a/Source/Core/Core/PowerPC/JitInterface.h b/Source/Core/Core/PowerPC/JitInterface.h index 1b876cc63a..18b60d5296 100644 --- a/Source/Core/Core/PowerPC/JitInterface.h +++ b/Source/Core/Core/PowerPC/JitInterface.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include "Common/CommonTypes.h" #include "Core/MachineContext.h" @@ -43,10 +44,22 @@ enum class ProfilingState Disabled }; +enum class GetHostCodeError +{ + NoJitActive, + NoTranslation, +}; +struct GetHostCodeResult +{ + const u8* code; + u32 code_size; + u32 entry_address; +}; + void SetProfilingState(ProfilingState state); void WriteProfileResults(const std::string& filename); void GetProfileResults(Profiler::ProfileStats* prof_stats); -int GetHostCode(u32* address, const u8** code, u32* code_size); +std::variant GetHostCode(u32 address); // Memory Utilities bool HandleFault(uintptr_t access_address, SContext* ctx); diff --git a/Source/Core/DolphinQt/Debugger/JITWidget.cpp b/Source/Core/DolphinQt/Debugger/JITWidget.cpp index 68c39eb662..97e57d0b3a 100644 --- a/Source/Core/DolphinQt/Debugger/JITWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/JITWidget.cpp @@ -146,14 +146,11 @@ void JITWidget::Update() // TODO: Actually do something with the table (Wx doesn't) // Get host side code disassembly - u32 host_instructions_count = 0; - u32 host_code_size = 0; - std::string host_instructions_disasm; - host_instructions_disasm = - DisassembleBlock(m_disassembler.get(), &m_address, &host_instructions_count, &host_code_size); + auto host_instructions_disasm = DisassembleBlock(m_disassembler.get(), m_address); + m_address = host_instructions_disasm.entry_address; m_host_asm_widget->setHtml( - QStringLiteral("
%1
").arg(QString::fromStdString(host_instructions_disasm))); + QStringLiteral("
%1
").arg(QString::fromStdString(host_instructions_disasm.text))); // == Fill in ppc box u32 ppc_addr = m_address; @@ -191,19 +188,24 @@ void JITWidget::Update() ppc_disasm << st.numCycles << " estimated cycles" << std::endl; ppc_disasm << "Num instr: PPC: " << code_block.m_num_instructions - << " Host: " << host_instructions_count; + << " Host: " << host_instructions_disasm.instruction_count; if (code_block.m_num_instructions != 0) { ppc_disasm << " (blowup: " - << 100 * host_instructions_count / code_block.m_num_instructions - 100 << "%)"; + << 100 * host_instructions_disasm.instruction_count / + code_block.m_num_instructions - + 100 + << "%)"; } ppc_disasm << std::endl; ppc_disasm << "Num bytes: PPC: " << code_block.m_num_instructions * 4 - << " Host: " << host_code_size; + << " Host: " << host_instructions_disasm.code_size; if (code_block.m_num_instructions != 0) { - ppc_disasm << " (blowup: " << 100 * host_code_size / (4 * code_block.m_num_instructions) - 100 + ppc_disasm << " (blowup: " + << 100 * host_instructions_disasm.code_size / (4 * code_block.m_num_instructions) - + 100 << "%)"; } ppc_disasm << std::endl; diff --git a/Source/Core/UICommon/Disassembler.cpp b/Source/Core/UICommon/Disassembler.cpp index 7cc9d8e8f3..590611b12a 100644 --- a/Source/Core/UICommon/Disassembler.cpp +++ b/Source/Core/UICommon/Disassembler.cpp @@ -16,6 +16,8 @@ #include // Bochs #endif +#include "Common/Assert.h" +#include "Common/VariantUtil.h" #include "Core/PowerPC/JitInterface.h" #if defined(HAVE_LLVM) @@ -169,21 +171,39 @@ std::unique_ptr GetNewDisassembler(const std::string& arch) return std::make_unique(); } -std::string DisassembleBlock(HostDisassembler* disasm, u32* address, u32* host_instructions_count, - u32* code_size) +DisassembleResult DisassembleBlock(HostDisassembler* disasm, u32 address) { - const u8* code; - int res = JitInterface::GetHostCode(address, &code, code_size); + auto res = JitInterface::GetHostCode(address); - if (res == 1) - { - *host_instructions_count = 0; - return "(No JIT active)"; - } - else if (res == 2) - { - *host_instructions_count = 0; - return "(No translation)"; - } - return disasm->DisassembleHostBlock(code, *code_size, host_instructions_count, (u64)code); + return std::visit(overloaded{[&](JitInterface::GetHostCodeError error) { + DisassembleResult result; + switch (error) + { + case JitInterface::GetHostCodeError::NoJitActive: + result.text = "(No JIT active)"; + break; + case JitInterface::GetHostCodeError::NoTranslation: + result.text = "(No translation)"; + break; + default: + ASSERT(false); + break; + } + result.entry_address = address; + result.instruction_count = 0; + result.code_size = 0; + return result; + }, + [&](JitInterface::GetHostCodeResult host_result) { + DisassembleResult new_result; + u32 instruction_count = 0; + new_result.text = disasm->DisassembleHostBlock( + host_result.code, host_result.code_size, &instruction_count, + (u64)host_result.code); + new_result.entry_address = host_result.entry_address; + new_result.code_size = host_result.code_size; + new_result.instruction_count = instruction_count; + return new_result; + }}, + res); } diff --git a/Source/Core/UICommon/Disassembler.h b/Source/Core/UICommon/Disassembler.h index f5765c6c8a..5f135aa30d 100644 --- a/Source/Core/UICommon/Disassembler.h +++ b/Source/Core/UICommon/Disassembler.h @@ -18,6 +18,13 @@ public: } }; +struct DisassembleResult +{ + std::string text; + u32 entry_address; + u32 instruction_count; + u32 code_size; +}; + std::unique_ptr GetNewDisassembler(const std::string& arch); -std::string DisassembleBlock(HostDisassembler* disasm, u32* address, u32* host_instructions_count, - u32* code_size); +DisassembleResult DisassembleBlock(HostDisassembler* disasm, u32 address); From 3d6bfcd236c7fe8d0432072aeeb6e2825c43b3cc Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 1 Dec 2022 17:40:49 -0800 Subject: [PATCH 5/5] JITWidget: Convert to fmt --- Source/Core/DolphinQt/Debugger/JITWidget.cpp | 42 ++++++++------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/JITWidget.cpp b/Source/Core/DolphinQt/Debugger/JITWidget.cpp index 97e57d0b3a..898901f22c 100644 --- a/Source/Core/DolphinQt/Debugger/JITWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/JITWidget.cpp @@ -173,45 +173,37 @@ void JITWidget::Update() if (analyzer.Analyze(ppc_addr, &code_block, &code_buffer, code_buffer.size()) != 0xFFFFFFFF) { - std::ostringstream ppc_disasm; + std::string ppc_disasm_str; + auto ppc_disasm = std::back_inserter(ppc_disasm_str); for (u32 i = 0; i < code_block.m_num_instructions; i++) { const PPCAnalyst::CodeOp& op = code_buffer[i]; const std::string opcode = Common::GekkoDisassembler::Disassemble(op.inst.hex, op.address); - ppc_disasm << std::setfill('0') << std::setw(8) << std::hex << op.address; - ppc_disasm << " " << opcode << std::endl; + fmt::format_to(ppc_disasm, "{:08x} {}\n", op.address, opcode); } // Add stats to the end of the ppc box since it's generally the shortest. - ppc_disasm << std::dec << std::endl; - - ppc_disasm << st.numCycles << " estimated cycles" << std::endl; - - ppc_disasm << "Num instr: PPC: " << code_block.m_num_instructions - << " Host: " << host_instructions_disasm.instruction_count; - if (code_block.m_num_instructions != 0) + fmt::format_to(ppc_disasm, "\n{} estimated cycles", st.numCycles); + fmt::format_to(ppc_disasm, "\nNum instr: PPC: {} Host: {}", code_block.m_num_instructions, + host_instructions_disasm.instruction_count); + if (code_block.m_num_instructions != 0 && host_instructions_disasm.instruction_count != 0) { - ppc_disasm << " (blowup: " - << 100 * host_instructions_disasm.instruction_count / - code_block.m_num_instructions - - 100 - << "%)"; + fmt::format_to( + ppc_disasm, " (blowup: {}%)", + 100 * host_instructions_disasm.instruction_count / code_block.m_num_instructions - 100); } - ppc_disasm << std::endl; - ppc_disasm << "Num bytes: PPC: " << code_block.m_num_instructions * 4 - << " Host: " << host_instructions_disasm.code_size; - if (code_block.m_num_instructions != 0) + fmt::format_to(ppc_disasm, "\nNum bytes: PPC: {} Host: {}", code_block.m_num_instructions * 4, + host_instructions_disasm.code_size); + if (code_block.m_num_instructions != 0 && host_instructions_disasm.code_size != 0) { - ppc_disasm << " (blowup: " - << 100 * host_instructions_disasm.code_size / (4 * code_block.m_num_instructions) - - 100 - << "%)"; + fmt::format_to( + ppc_disasm, " (blowup: {}%)", + 100 * host_instructions_disasm.code_size / (4 * code_block.m_num_instructions) - 100); } - ppc_disasm << std::endl; m_ppc_asm_widget->setHtml( - QStringLiteral("
%1
").arg(QString::fromStdString(ppc_disasm.str()))); + QStringLiteral("
%1
").arg(QString::fromStdString(ppc_disasm_str))); } else {