diff --git a/Core/Core.cpp b/Core/Core.cpp index bbb372291c..139493770c 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -360,6 +360,10 @@ void Core_RunLoopUntil(u64 globalticks) { break; } break; + case CORE_REENTER_DISPATCH: + _dbg_assert_(false); + coreState = CORE_STEPPING_CPU; + break; } } } @@ -423,6 +427,7 @@ static void Core_PerformCPUStep(MIPSDebugInterface *cpu, CPUStepType stepType) { breakpointAddress = currentPc + 2 * cpu->getInstructionSize(0); } } + // Add a temporary breakpoint. g_breakpoints.AddBreakPoint(breakpointAddress, true); Core_Resume(); } else { @@ -481,6 +486,9 @@ static bool Core_ProcessStepping(MIPSDebugInterface *cpu) { case CORE_RUNNING_GE: // All good break; + case CORE_REENTER_DISPATCH: + _dbg_assert_(false); + return true; default: // Nothing to do. return true; diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index a37d8fc402..91b5367038 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -144,6 +144,10 @@ bool BreakpointManager::RangeContainsBreakPoint(u32 addr, u32 size) } int BreakpointManager::AddBreakPoint(u32 addr, bool temp) { + if (addr & 3) { + WARN_LOG(Log::Debugger, "Breakpoint added at %08x will not be effective - unaligned address.", addr); + } + size_t bp = FindBreakpoint(addr, true, temp); if (bp == INVALID_BREAKPOINT) { BreakPoint pt; @@ -659,19 +663,24 @@ BreakAction BreakpointManager::ExecRegBreakpoint(int reg, u32 pc) { NOTICE_LOG(Log::JIT, "BKP reg write r%d, PC=%08x: %s", reg, pc, formatted.c_str()); } } - if (info.result & BREAK_ACTION_PAUSE) { + if ((info.result & BREAK_ACTION_PAUSE) && g_breakpoints.CheckSkipFirst() != pc) { Core_Break(BreakReason::RegBreakpoint, pc); } return info.result; } +void BreakpointManager::ClearSkipFirst() { + breakSkipFirstAt_ = 0; + breakSkipFirstTicks_ = 0; +} + void BreakpointManager::SetSkipFirst(u32 pc) { breakSkipFirstAt_ = pc; breakSkipFirstTicks_ = CoreTiming::GetTicks(currentMIPS); } -u32 BreakpointManager::CheckSkipFirst() { +u32 BreakpointManager::CheckSkipFirst() const { u32 pc = breakSkipFirstAt_; if (breakSkipFirstTicks_ == CoreTiming::GetTicks(currentMIPS)) return pc; diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index 05f3db9ab5..0f728e2973 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -213,7 +213,8 @@ public: BreakAction ExecRegBreakpoint(int reg, u32 pc); void SetSkipFirst(u32 pc); - u32 CheckSkipFirst(); + u32 CheckSkipFirst() const; + void ClearSkipFirst(); // Includes uncached addresses. std::vector GetMemCheckRanges(bool write); diff --git a/Core/MIPS/MIPSTables.cpp b/Core/MIPS/MIPSTables.cpp index fec07b393a..53b5e3c216 100644 --- a/Core/MIPS/MIPSTables.cpp +++ b/Core/MIPS/MIPSTables.cpp @@ -1086,28 +1086,6 @@ void MIPSDisAsm(MIPSOpcode op, u32 pc, char *out, size_t outSize, bool tabsToSpa } } -static inline void InterpretInstruction(MIPSState *mips, const MIPSInstruction *instr, MIPSOpcode op) { - if (instr && instr->interpret) { - instr->interpret(mips, op); - } else { - Core_ExecException(mips->pc, mips->pc, ExecExceptionType::ILLEGAL); - } -} - -inline int GetInstructionCycleEstimate(const MIPSInstruction *instr) { - return instr ? instr->flags.cycles : 1; -} - -void MIPSInterpret(MIPSState *mips, MIPSOpcode op) { - const MIPSInstruction *instr = MIPSGetInstruction(op); - InterpretInstruction(mips, instr, op); -} - -// See the declaration comment in MIPSTables.h. -void CDECL MIPSInterpretTrampoline(MIPSOpcode op) { - MIPSInterpret(currentMIPS, op); -} - // This is the fast runloop for the interpreter. // When making changes, always make sure it's in sync with the fallback loop, RunUntilDowncountZeroWithChecks, which is // far less efficient but supports breakpoints of all kinds etc. @@ -1157,6 +1135,84 @@ static inline int GetGPRWriteTarget(const MIPSInstruction *instr, MIPSOpcode op) return -1; } +static bool CheckExecBreakpoints(const MIPSState *mips, u32 pc) { + if (g_breakpoints.HasBreakPoints() && g_breakpoints.IsAddressBreakPoint(pc) && g_breakpoints.CheckSkipFirst() != mips->pc) { + g_breakpoints.ExecBreakPoint(pc); + if (coreState == CORE_STEPPING_CPU) { // after ExecBreakPoint. + if (g_breakpoints.IsTempBreakPoint(mips->pc)) + g_breakpoints.RemoveBreakPoint(mips->pc); + return true; + } + } + return false; +} + +static bool CheckMemBreakpoints(const MIPSState *mips, const MIPSInstruction *instr, const MIPSOpcode op) { + if ((instr->flags & (IN_MEM | OUT_MEM)) != 0 && g_breakpoints.CheckSkipFirst() != mips->pc && instr->interpret != &Int_Syscall) { + // This is common for all IN_MEM/OUT_MEM funcs. + int offset = (instr->flags & IS_VFPU) != 0 ? SignExtend16ToS32(op & 0xFFFC) : SignExtend16ToS32(op); + u32 addr = (mips->r[_RS(op)] + offset) & 0xFFFFFFFC; + int sz = MIPSGetMemoryAccessSize(op); + + if ((instr->flags & IN_MEM) != 0) + g_breakpoints.ExecMemCheck(addr, false, sz, mips->pc, "interpret"); + if ((instr->flags & OUT_MEM) != 0) + g_breakpoints.ExecMemCheck(addr, true, sz, mips->pc, "interpret"); + + // If it tripped, bail without running. + return coreState == CORE_STEPPING_CPU; + } + return false; +} + +static bool CheckRegBreakpoints(const MIPSState *mips, const MIPSInstruction *instr, const MIPSOpcode op, u32 regBPMask) { + if ((instr->flags & (OUT_RT | OUT_RD | OUT_RA)) != 0 && g_breakpoints.CheckSkipFirst() != mips->pc) { + int regTarget = GetGPRWriteTarget(instr, op); + if (regTarget >= 0 && (regBPMask & (1u << regTarget)) != 0) { + g_breakpoints.ExecRegBreakpoint(regTarget, mips->pc); + // If it tripped, bail without running. + if (coreState == CORE_STEPPING_CPU) + return true; + } + } + return false; +} + +static inline void InterpretInstruction(MIPSState *mips, const MIPSInstruction *instr, MIPSOpcode op) { + if (instr && instr->interpret) { + instr->interpret(mips, op); + } else { + Core_ExecException(mips->pc, mips->pc, ExecExceptionType::ILLEGAL); + } +} + +inline int GetInstructionCycleEstimate(const MIPSInstruction *instr) { + return instr ? instr->flags.cycles : 1; +} + +void MIPSInterpret(MIPSState *mips, MIPSOpcode op) { + const MIPSInstruction *instr = MIPSGetInstruction(op); + + // Check/trigger breakpoints. NOTE: We set coreState to CORE_STEPPING_CPU if a breakpoint was hit (this function + // is used to skip delay slots), but unlike when running free, we don't avoid executing the instruction (so you + // can actually step through). + if (g_breakpoints.HasBreakPoints()) { + CheckExecBreakpoints(mips, mips->pc); + } + if (g_breakpoints.HasMemChecks()) { + CheckMemBreakpoints(mips, instr, op); + } + if (g_breakpoints.GetRegBreakpointMask()) { + CheckRegBreakpoints(mips, instr, op, g_breakpoints.GetRegBreakpointMask()); + } + InterpretInstruction(mips, instr, op); +} + +// See the declaration comment in MIPSTables.h. +void CDECL MIPSInterpretTrampoline(MIPSOpcode op) { + MIPSInterpret(currentMIPS, op); +} + // This is the slow, feature-rich runloop for the interpreter. // When making changes, always make sure it's in sync with the fast loop, RunUntilDowncountZeroFast, which is // much more efficient. @@ -1180,43 +1236,20 @@ static void RunUntilDowncountZeroWithChecks(MIPSState *mips, u64 globalTicks) { // backends and the IR interpreter, via JitBreakpoint()/IRRunBreakpoint()) rather // than breaking unconditionally - it's what actually respects BREAK_ACTION_LOG vs // BREAK_ACTION_PAUSE (a log-only breakpoint, added with log=true and no/false - // enabled, must not stop execution here). The old code called Core_Break() - // directly whenever IsAddressBreakPoint() was true - true for *any* non-ignored - // breakpoint, log-only included - so log-only address breakpoints always paused - // too, contradicting their own documented behavior. - if (hasBPs && g_breakpoints.IsAddressBreakPoint(mips->pc) && g_breakpoints.CheckSkipFirst() != mips->pc) { - g_breakpoints.ExecBreakPoint(mips->pc); - // If it tripped, bail without running - same convention as memchecks/reg - // breakpoints below. - if (coreState == CORE_STEPPING_CPU) { - if (g_breakpoints.IsTempBreakPoint(mips->pc)) - g_breakpoints.RemoveBreakPoint(mips->pc); - break; - } - } - if (hasMCs && (instr->flags & (IN_MEM | OUT_MEM)) != 0 && g_breakpoints.CheckSkipFirst() != mips->pc && instr->interpret != &Int_Syscall) { - // This is common for all IN_MEM/OUT_MEM funcs. - int offset = (instr->flags & IS_VFPU) != 0 ? SignExtend16ToS32(op & 0xFFFC) : SignExtend16ToS32(op); - u32 addr = (mips->r[_RS(op)] + offset) & 0xFFFFFFFC; - int sz = MIPSGetMemoryAccessSize(op); - - if ((instr->flags & IN_MEM) != 0) - g_breakpoints.ExecMemCheck(addr, false, sz, mips->pc, "interpret"); - if ((instr->flags & OUT_MEM) != 0) - g_breakpoints.ExecMemCheck(addr, true, sz, mips->pc, "interpret"); - + // enabled, must not stop execution here). + bool breakExec = false; // We use this mechanism so we always check all breakpoint types - they can overlap. + if (hasBPs && CheckExecBreakpoints(mips, mips->pc)) { // If it tripped, bail without running. - if (coreState == CORE_STEPPING_CPU) - break; + breakExec = true; } - if (regBPMask != 0 && (instr->flags & (OUT_RT | OUT_RD | OUT_RA)) != 0 && g_breakpoints.CheckSkipFirst() != mips->pc) { - int regTarget = GetGPRWriteTarget(instr, op); - if (regTarget >= 0 && (regBPMask & (1u << regTarget)) != 0) { - g_breakpoints.ExecRegBreakpoint(regTarget, mips->pc); - // If it tripped, bail without running - same convention as memchecks above. - if (coreState == CORE_STEPPING_CPU) - break; - } + if (hasMCs && CheckMemBreakpoints(mips, instr, op)) { + breakExec = true; + } + if (regBPMask != 0 && CheckRegBreakpoints(mips, instr, op, regBPMask)) { + breakExec = true; + } + if (breakExec) { + break; } const bool wasInDelaySlot = mips->inDelaySlot; diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index 4b5ed3d4a8..3a2008b819 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -1034,7 +1034,7 @@ static void DrawBreakpointsView(MIPSDebugInterface *mipsDebug, ImConfig &cfg) { ImGui::TableHeadersRow(); for (int i = 0; i < (int)bps.size(); i++) { - auto &bp = bps[i]; + BreakPoint &bp = bps[i]; bool temp = bp.temporary; if (temp) { continue; @@ -1048,6 +1048,7 @@ static void DrawBreakpointsView(MIPSDebugInterface *mipsDebug, ImConfig &cfg) { if (ImGui::Selectable("", cfg.selectedBreakpoint == i, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap) && !bp.temporary) { cfg.selectedBreakpoint = i; cfg.selectedMemCheck = -1; + cfg.selectedRegBreakpoint = -1; } ImGui::SameLine(); ImGui::CheckboxFlags("##enabled", (int *)&bp.action, (int)BREAK_ACTION_PAUSE); @@ -1087,6 +1088,7 @@ static void DrawBreakpointsView(MIPSDebugInterface *mipsDebug, ImConfig &cfg) { if (ImGui::Selectable("##memcheck", cfg.selectedMemCheck == i, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap)) { cfg.selectedBreakpoint = -1; cfg.selectedMemCheck = i; + cfg.selectedRegBreakpoint = -1; } ImGui::SameLine(); ImGui::CheckboxFlags("##enabled", (int *)&mc.action, (int)BREAK_ACTION_PAUSE); @@ -1107,6 +1109,37 @@ static void DrawBreakpointsView(MIPSDebugInterface *mipsDebug, ImConfig &cfg) { ImGui::PopID(); } + // Finally, list register breakpoints. + for (auto &rbp : g_breakpoints.GetRegBreakpoints()) { + ImGui::TableNextRow(); + ImGui::TableNextColumn(); + ImGui::PushID(&rbp - &g_breakpoints.GetRegBreakpoints()[0] + 20000); + if (ImGui::Selectable("##regbp", cfg.selectedRegBreakpoint == &rbp - &g_breakpoints.GetRegBreakpoints()[0], ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap)) { + cfg.selectedBreakpoint = -1; + cfg.selectedMemCheck = -1; + cfg.selectedRegBreakpoint = &rbp - &g_breakpoints.GetRegBreakpoints()[0]; + } + ImGui::SameLine(); + ImGui::CheckboxFlags("", (int *)&rbp.result, BREAK_ACTION_PAUSE); + ImGui::TableNextColumn(); + ImGui::TextUnformatted("Reg"); + ImGui::TableNextColumn(); + ImGui::TextUnformatted(mipsDebug->GetRegName(0, rbp.reg).c_str()); + ImGui::TableNextColumn(); + ImGui::TextUnformatted("-"); // size/label + ImGui::TableNextColumn(); + ImGui::TextUnformatted("-"); // opcode + ImGui::TableNextColumn(); + if (rbp.hasCond) { + ImGui::TextUnformatted(rbp.cond.expressionString.c_str()); + } else { + ImGui::TextUnformatted("-"); // condition + } + ImGui::TableNextColumn(); + ImGui::Text("%d", rbp.numHits); + ImGui::PopID(); + } + ImGui::EndTable(); } diff --git a/UI/ImDebugger/ImDebugger.h b/UI/ImDebugger/ImDebugger.h index 6f80655188..9fc31b2382 100644 --- a/UI/ImDebugger/ImDebugger.h +++ b/UI/ImDebugger/ImDebugger.h @@ -106,6 +106,7 @@ struct ImConfig { int selectedFramebuffer = -1; int selectedBreakpoint = -1; int selectedMemCheck = -1; + int selectedRegBreakpoint = -1; int selectedAtracCtx = 0; int selectedMp3Ctx = 0; int selectedAacCtx = 0;