From 644e457a6205b2f6d61a472a170bb2757d859bca Mon Sep 17 00:00:00 2001 From: archshift Date: Fri, 4 Sep 2015 23:29:12 -0700 Subject: [PATCH] Fix the callstack widget by keeping track of branches Co-opts the interpreter to create a callstack, by checking for branches and returns from branches. --- src/citra_qt/debugger/callstack.cpp | 58 ++----------------- src/core/arm/arm_interface.h | 4 ++ src/core/arm/dyncom/arm_dyncom.cpp | 13 ++++- src/core/arm/dyncom/arm_dyncom.h | 1 + .../arm/dyncom/arm_dyncom_interpreter.cpp | 10 ++++ src/core/arm/skyeye_common/armstate.h | 2 + src/core/core.h | 4 ++ 7 files changed, 39 insertions(+), 53 deletions(-) diff --git a/src/citra_qt/debugger/callstack.cpp b/src/citra_qt/debugger/callstack.cpp index 793944639..30fdce653 100644 --- a/src/citra_qt/debugger/callstack.cpp +++ b/src/citra_qt/debugger/callstack.cpp @@ -7,69 +7,27 @@ #include "citra_qt/debugger/callstack.h" #include "common/common_types.h" -#include "common/symbols.h" #include "core/core.h" -#include "core/memory.h" #include "core/arm/arm_interface.h" -#include "core/arm/disassembler/arm_disasm.h" CallstackWidget::CallstackWidget(QWidget* parent): QDockWidget(parent) { ui.setupUi(this); callstack_model = new QStandardItemModel(this); - callstack_model->setColumnCount(4); - callstack_model->setHeaderData(0, Qt::Horizontal, "Stack Pointer"); - callstack_model->setHeaderData(2, Qt::Horizontal, "Return Address"); - callstack_model->setHeaderData(1, Qt::Horizontal, "Call Address"); - callstack_model->setHeaderData(3, Qt::Horizontal, "Function"); + callstack_model->setColumnCount(1); + callstack_model->setHeaderData(0, Qt::Horizontal, "Caller address"); ui.treeView->setModel(callstack_model); } void CallstackWidget::OnDebugModeEntered() { - // Stack pointer - const u32 sp = Core::g_app_core->GetReg(13); - Clear(); - int counter = 0; - for (u32 addr = 0x10000000; addr >= sp; addr -= 4) - { - const u32 ret_addr = Memory::Read32(addr); - const u32 call_addr = ret_addr - 4; //get call address??? - - if (Memory::GetPointer(call_addr) == nullptr) - break; - - /* TODO (mattvail) clean me, move to debugger interface */ - u32 insn = Memory::Read32(call_addr); - if (ARM_Disasm::Decode(insn) == OP_BL) - { - std::string name; - // ripped from disasm - u8 cond = (insn >> 28) & 0xf; - u32 i_offset = insn & 0xffffff; - // Sign-extend the 24-bit offset - if ((i_offset >> 23) & 1) - i_offset |= 0xff000000; - - // Pre-compute the left-shift and the prefetch offset - i_offset <<= 2; - i_offset += 8; - const u32 func_addr = call_addr + i_offset; - - callstack_model->setItem(counter, 0, new QStandardItem(QString("0x%1").arg(addr, 8, 16, QLatin1Char('0')))); - callstack_model->setItem(counter, 1, new QStandardItem(QString("0x%1").arg(ret_addr, 8, 16, QLatin1Char('0')))); - callstack_model->setItem(counter, 2, new QStandardItem(QString("0x%1").arg(call_addr, 8, 16, QLatin1Char('0')))); - - name = Symbols::HasSymbol(func_addr) ? Symbols::GetSymbol(func_addr).name : "unknown"; - callstack_model->setItem(counter, 3, new QStandardItem(QString("%1_%2").arg(QString::fromStdString(name)) - .arg(QString("0x%1").arg(func_addr, 8, 16, QLatin1Char('0'))))); - - counter++; - } + std::vector stack_trace = Core::g_app_core->GetStackTrace(); + for (auto entry : stack_trace) { + callstack_model->setItem(callstack_model->rowCount(), new QStandardItem(QString("0x%1").arg(entry, 8, 16, QLatin1Char('0')))); } } @@ -80,9 +38,5 @@ void CallstackWidget::OnDebugModeLeft() void CallstackWidget::Clear() { - for (int row = 0; row < callstack_model->rowCount(); row++) { - for (int column = 0; column < callstack_model->columnCount(); column++) { - callstack_model->setItem(row, column, new QStandardItem()); - } - } + callstack_model->removeRows(0, callstack_model->rowCount()); } diff --git a/src/core/arm/arm_interface.h b/src/core/arm/arm_interface.h index 533067d4f..cc87ddc08 100644 --- a/src/core/arm/arm_interface.h +++ b/src/core/arm/arm_interface.h @@ -4,6 +4,8 @@ #pragma once +#include + #include "common/common_types.h" #include "core/arm/skyeye_common/arm_regformat.h" @@ -111,6 +113,8 @@ public: */ virtual void SetCP15Register(CP15Register reg, u32 value) = 0; + virtual std::vector GetStackTrace() const = 0; + /** * Advance the CPU core by the specified number of ticks (e.g. to simulate CPU execution time) * @param ticks Number of ticks to advance the CPU core diff --git a/src/core/arm/dyncom/arm_dyncom.cpp b/src/core/arm/dyncom/arm_dyncom.cpp index f3be2c857..f8355a60d 100644 --- a/src/core/arm/dyncom/arm_dyncom.cpp +++ b/src/core/arm/dyncom/arm_dyncom.cpp @@ -72,6 +72,10 @@ void ARM_DynCom::SetCP15Register(CP15Register reg, u32 value) { state->CP15[reg] = value; } +std::vector ARM_DynCom::GetStackTrace() const { + return state->stack_trace; +} + void ARM_DynCom::AddTicks(u64 ticks) { down_count -= ticks; if (down_count < 0) @@ -89,12 +93,15 @@ void ARM_DynCom::ExecuteInstructions(int num_instructions) { } void ARM_DynCom::ResetContext(Core::ThreadContext& context, u32 stack_top, u32 entry_point, u32 arg) { - memset(&context, 0, sizeof(Core::ThreadContext)); + context = Core::ThreadContext(); context.cpu_registers[0] = arg; context.pc = entry_point; context.sp = stack_top; context.cpsr = 0x1F; // Usermode + + context.stack_trace.clear(); + context.stack_trace.reserve(128); } void ARM_DynCom::SaveContext(Core::ThreadContext& ctx) { @@ -108,6 +115,8 @@ void ARM_DynCom::SaveContext(Core::ThreadContext& ctx) { ctx.fpscr = state->VFP[1]; ctx.fpexc = state->VFP[2]; + + ctx.stack_trace = state->stack_trace; } void ARM_DynCom::LoadContext(const Core::ThreadContext& ctx) { @@ -121,6 +130,8 @@ void ARM_DynCom::LoadContext(const Core::ThreadContext& ctx) { state->VFP[1] = ctx.fpscr; state->VFP[2] = ctx.fpexc; + + state->stack_trace = ctx.stack_trace; } void ARM_DynCom::PrepareReschedule() { diff --git a/src/core/arm/dyncom/arm_dyncom.h b/src/core/arm/dyncom/arm_dyncom.h index 3664fd728..8a5d2a637 100644 --- a/src/core/arm/dyncom/arm_dyncom.h +++ b/src/core/arm/dyncom/arm_dyncom.h @@ -33,6 +33,7 @@ public: void SetCPSR(u32 cpsr) override; u32 GetCP15Register(CP15Register reg) override; void SetCP15Register(CP15Register reg, u32 value) override; + std::vector GetStackTrace() const override; void AddTicks(u64 ticks) override; diff --git a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp index 5f8826034..ae4e76679 100644 --- a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp +++ b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp @@ -3816,6 +3816,11 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { #define CurrentModeHasSPSR (cpu->Mode != SYSTEM32MODE) && (cpu->Mode != USER32MODE) #define PC (cpu->Reg[15]) + #define PUSH_STACK_TRACE \ + cpu->stack_trace.emplace_back(cpu->Reg[15]) + #define UPDATE_STACK_TRACE \ + if (cpu->stack_trace.size() > 0 && cpu->stack_trace.back() == PC - cpu->GetInstructionSize()) \ + cpu->stack_trace.pop_back() // GCC and Clang have a C++ extension to support a lookup table of labels. Otherwise, fallback // to a clunky switch statement. @@ -3867,6 +3872,8 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { else cpu->Reg[15] &= 0xfffffffc; + UPDATE_STACK_TRACE; + // Find the cached instruction cream, otherwise translate it... auto itr = cpu->instruction_cache.find(cpu->Reg[15]); if (itr != cpu->instruction_cache.end()) { @@ -3993,6 +4000,7 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { if ((inst_base->cond == ConditionCode::AL) || CondPassed(cpu, inst_base->cond)) { bbl_inst *inst_cream = (bbl_inst *)inst_base->component; if (inst_cream->L) { + PUSH_STACK_TRACE; LINK_RTN_ADDR; } SET_PC; @@ -4049,6 +4057,8 @@ unsigned InterpreterMainLoop(ARMul_State* cpu) { { blx_inst *inst_cream = (blx_inst *)inst_base->component; if ((inst_base->cond == ConditionCode::AL) || CondPassed(cpu, inst_base->cond)) { + PUSH_STACK_TRACE; + unsigned int inst = inst_cream->inst; if (BITS(inst, 20, 27) == 0x12 && BITS(inst, 4, 7) == 0x3) { cpu->Reg[14] = (cpu->Reg[15] + cpu->GetInstructionSize()); diff --git a/src/core/arm/skyeye_common/armstate.h b/src/core/arm/skyeye_common/armstate.h index d42ff2669..8cde29a06 100644 --- a/src/core/arm/skyeye_common/armstate.h +++ b/src/core/arm/skyeye_common/armstate.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include "common/common_types.h" @@ -239,6 +240,7 @@ public: // TODO(bunnei): Move this cache to a better place - it should be per codeset (likely per // process for our purposes), not per ARMul_State (which tracks CPU core state). std::unordered_map instruction_cache; + std::vector stack_trace; private: void ResetMPCoreCP15Registers(); diff --git a/src/core/core.h b/src/core/core.h index ad26dca3f..30e2f409c 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -5,6 +5,8 @@ #pragma once #include +#include + #include "common/common_types.h" class ARM_Interface; @@ -22,6 +24,8 @@ struct ThreadContext { u32 fpu_registers[64]; u32 fpscr; u32 fpexc; + + std::vector stack_trace; }; extern std::unique_ptr g_app_core; ///< ARM11 application core