From 0862e3e999fe6f41d7379968e183761287bc7a68 Mon Sep 17 00:00:00 2001 From: mmvest <53498772+mmvest@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:11:50 -0400 Subject: [PATCH] Debugger: Merged memcheck log and instrumentation into Log When Hit. Add breakpoint handling to psxDynarecMemcheck function. Add fix for window refreshing clobbering inputs in the breakpoint list. --- .../Debugger/Breakpoints/BreakpointDialog.cpp | 12 +++---- .../Debugger/Breakpoints/BreakpointDialog.ui | 19 ++---------- .../Debugger/Breakpoints/BreakpointView.cpp | 11 +++++-- pcsx2/DebugTools/Breakpoints.cpp | 31 +++++++++++++++++-- pcsx2/DebugTools/Breakpoints.h | 3 +- pcsx2/x86/iR3000A.cpp | 9 ++---- pcsx2/x86/ix86-32/iR5900.cpp | 8 ----- 7 files changed, 48 insertions(+), 45 deletions(-) diff --git a/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.cpp b/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.cpp index 691b9869df..170ae85726 100644 --- a/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.cpp +++ b/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.cpp @@ -46,7 +46,7 @@ BreakpointDialog::BreakpointDialog(QWidget* parent, DebugInterface* cpu, Breakpo m_ui.txtDescription->setText(QString::fromStdString(bp->description)); m_ui.spnMaxHitCount->setValue(static_cast(bp->maxHits)); - m_ui.grpInstrumentation->setChecked(bp->instrumentationEnabled); + m_ui.grpLogWhenHit->setChecked(bp->instrumentationEnabled); m_ui.txtLogFormat->setText(QString::fromStdString(bp->logFormat)); m_ui.chkContinueOnHit->setChecked(bp->continueOnHit); @@ -67,10 +67,9 @@ BreakpointDialog::BreakpointDialog(QWidget* parent, DebugInterface* cpu, Breakpo m_ui.chkChange->setChecked(mc->memCond & MEMCHECK_WRITE_ONCHANGE); m_ui.chkEnable->setChecked(mc->result & MEMCHECK_BREAK); - m_ui.chkLog->setChecked(mc->result & MEMCHECK_LOG); m_ui.spnMaxHitCount->setValue(static_cast(mc->maxHits)); - m_ui.grpInstrumentation->setChecked(mc->instrumentationEnabled); + m_ui.grpLogWhenHit->setChecked(mc->instrumentationEnabled); m_ui.txtLogFormat->setText(QString::fromStdString(mc->logFormat)); m_ui.chkContinueOnHit->setChecked(mc->continueOnHit); @@ -89,7 +88,6 @@ void BreakpointDialog::onRdoButtonToggled() m_ui.grpMemory->setEnabled(!isExecute); - m_ui.chkLog->setEnabled(!isExecute); } void BreakpointDialog::accept() @@ -124,7 +122,7 @@ void BreakpointDialog::accept() bp->maxHits = static_cast(m_ui.spnMaxHitCount->value()); bp->enabled = m_ui.chkEnable->isChecked(); - const bool instrumentationEnabled = m_ui.grpInstrumentation->isChecked(); + const bool instrumentationEnabled = m_ui.grpLogWhenHit->isChecked(); const std::string logFormat = m_ui.txtLogFormat->text().toStdString(); if (instrumentationEnabled && !logFormat.empty() && !ValidateInstrumentationLogFormat(*m_cpu, logFormat, error)) { @@ -180,7 +178,7 @@ void BreakpointDialog::accept() mc->maxHits = static_cast(m_ui.spnMaxHitCount->value()); mc->description = m_ui.txtDescription->text().toStdString(); - const bool instrumentationEnabled = m_ui.grpInstrumentation->isChecked(); + const bool instrumentationEnabled = m_ui.grpLogWhenHit->isChecked(); const std::string logFormat = m_ui.txtLogFormat->text().toStdString(); if (instrumentationEnabled && !logFormat.empty() && !ValidateInstrumentationLogFormat(*m_cpu, logFormat, error)) { @@ -225,8 +223,6 @@ void BreakpointDialog::accept() int result = 0; if (m_ui.chkEnable->isChecked()) result |= MEMCHECK_BREAK; - if (m_ui.chkLog->isChecked()) - result |= MEMCHECK_LOG; mc->result = static_cast(result); } diff --git a/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.ui b/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.ui index d6d37bb3a3..44891dc4d9 100644 --- a/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.ui +++ b/pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.ui @@ -283,7 +283,7 @@ - + true @@ -296,7 +296,7 @@ - Instrumentation + Log When Hit true @@ -347,19 +347,6 @@ - - - - true - - - Log - - - true - - - @@ -425,7 +412,6 @@ rdoExecute txtAddress txtDescription - chkLog chkEnable spnMaxHitCount chkRead @@ -433,6 +419,7 @@ chkChange txtSize txtCondition + grpLogWhenHit txtLogFormat chkContinueOnHit diff --git a/pcsx2-qt/Debugger/Breakpoints/BreakpointView.cpp b/pcsx2-qt/Debugger/Breakpoints/BreakpointView.cpp index b1c0606de7..d1f1c2840d 100644 --- a/pcsx2-qt/Debugger/Breakpoints/BreakpointView.cpp +++ b/pcsx2-qt/Debugger/Breakpoints/BreakpointView.cpp @@ -50,10 +50,17 @@ BreakpointView::BreakpointView(const DebuggerViewParameters& parameters) // Continue-on-hit breakpoints update their hit counts without ever pausing the VM, // so onVMActuallyPaused never fires. Refresh periodically off the debugger's - // refresh timer so the UI can update regularly. + // refresh timer so the UI can update regularly. We don't refresh if the VM is paused + // or if a child of the breakpoint list is being edited. receiveEvent([this](const DebuggerEvents::Refresh& event) -> bool { - if (!QtHost::IsVMPaused()) + // If focus is on a child of the breakpoint list and not the list itself, + // we assume an inline editor is active and we should not refresh. + bool isEditing = m_ui.breakpointList->isAncestorOf(QApplication::focusWidget()) + && QApplication::focusWidget() != m_ui.breakpointList; + + if (!QtHost::IsVMPaused() && !isEditing) m_model->refreshData(); + return true; }); } diff --git a/pcsx2/DebugTools/Breakpoints.cpp b/pcsx2/DebugTools/Breakpoints.cpp index 6031bc97b3..a9677eb12e 100644 --- a/pcsx2/DebugTools/Breakpoints.cpp +++ b/pcsx2/DebugTools/Breakpoints.cpp @@ -6,7 +6,10 @@ #include "MIPSAnalyst.h" #include #include "R5900.h" +#include "R5900OpcodeTables.h" #include "R3000A.h" +#include "Memory.h" +#include "IopMem.h" #include "common/Console.h" std::vector CBreakPoints::breakPoints_; @@ -159,6 +162,26 @@ static void LogInstrumentation(BreakPointCpu cpu, const std::string& logFormat) Console.WriteLn(color, "%s", EvaluateInstrumentationLogFormat(debug, logFormat).c_str()); } +std::string GetMemCheckDefaultLogFormat(BreakPointCpu cpu) +{ + const u32 pc = (cpu == BREAKPOINT_IOP) ? psxRegs.pc : cpuRegs.pc; + + // We need to handle the branch delay slot!!! When the memory access happens in a branch + // delay slot, PC points at the branch but the actual load/store is one instruction later. + // To handle that, we can check the return of isMemCheckNeeded and handle pc appropriately. + const int needed = (cpu == BREAKPOINT_IOP) ? psxIsMemcheckNeeded(pc) : isMemcheckNeeded(pc); + const bool inDelaySlot = (needed == 2); + const u32 accessPc = inDelaySlot ? pc + 4 : pc; + const u32 op = (cpu == BREAKPOINT_IOP) ? iopMemRead32(accessPc) : memRead32(accessPc); + + // The R5900 opcode table is used for both CPUs... confusing, I know! + const char* const pcExpr = inDelaySlot ? "{pc + 4}" : "{pc}"; + const std::string defaultLogFormat = (R5900::GetInstruction(op).flags & IS_STORE) + ? std::string("Hit write breakpoint @ ") + pcExpr + : std::string("Hit read breakpoint @ ") + pcExpr; + return defaultLogFormat; +} + MemCheck::MemCheck() : start(0) , end(0) @@ -471,7 +494,9 @@ bool CBreakPoints::HandleBreakpointHit(BreakPointCpu cpu, u32 addr) breakpoint.totalHits++; const bool instrumentationEnabled = breakpoint.instrumentationEnabled; - const std::string logFormat = breakpoint.logFormat; + const std::string logFormat = breakpoint.logFormat.empty() + ? "Hit execute breakpoint @ {pc}" + : breakpoint.logFormat; const bool continueOnHit = breakpoint.continueOnHit; if (instrumentationEnabled) @@ -499,7 +524,9 @@ bool CBreakPoints::HandleMemCheckHit(BreakPointCpu cpu, u32 start, u32 end) memCheck.totalHits++; const bool instrumentationEnabled = memCheck.instrumentationEnabled; - const std::string logFormat = memCheck.logFormat; + const std::string logFormat = memCheck.logFormat.empty() + ? GetMemCheckDefaultLogFormat(cpu) + : memCheck.logFormat; const bool continueOnHit = memCheck.continueOnHit; if (instrumentationEnabled) diff --git a/pcsx2/DebugTools/Breakpoints.h b/pcsx2/DebugTools/Breakpoints.h index 1a3c04e222..27ff2c2a76 100644 --- a/pcsx2/DebugTools/Breakpoints.h +++ b/pcsx2/DebugTools/Breakpoints.h @@ -70,7 +70,6 @@ enum MemCheckCondition enum MemCheckResult { MEMCHECK_IGNORE = 0x00, - MEMCHECK_LOG = 0x01, MEMCHECK_BREAK = 0x02, MEMCHECK_BOTH = 0x03, @@ -212,4 +211,4 @@ private: u32 standardizeBreakpointAddress(u32 addr); std::string EvaluateInstrumentationLogFormat(DebugInterface& debug, const std::string& format); -bool ValidateInstrumentationLogFormat(DebugInterface& debug, const std::string& format, std::string& error); +bool ValidateInstrumentationLogFormat(DebugInterface& debug, const std::string& format, std::string& error); \ No newline at end of file diff --git a/pcsx2/x86/iR3000A.cpp b/pcsx2/x86/iR3000A.cpp index 0c458f067c..65db3a9428 100644 --- a/pcsx2/x86/iR3000A.cpp +++ b/pcsx2/x86/iR3000A.cpp @@ -1383,13 +1383,8 @@ static bool psxDynarecMemcheck(size_t i) return false; } - if (mc.result & MEMCHECK_LOG) - { - if (opcode.flags & IS_STORE) - DevCon.WriteLn("Hit R3000 store breakpoint @0x%x", pc); - else - DevCon.WriteLn("Hit R3000 load breakpoint @0x%x", pc); - } + if (!CBreakPoints::HandleMemCheckHit(BREAKPOINT_IOP, mc.start, mc.end)) + return false; CBreakPoints::SetBreakpointTriggered(true, BREAKPOINT_IOP); VMManager::SetPaused(true); diff --git a/pcsx2/x86/ix86-32/iR5900.cpp b/pcsx2/x86/ix86-32/iR5900.cpp index 72aeef5a08..318d30e5ef 100644 --- a/pcsx2/x86/ix86-32/iR5900.cpp +++ b/pcsx2/x86/ix86-32/iR5900.cpp @@ -1580,14 +1580,6 @@ void dynarecMemcheck(size_t i) return; } - if (mc.result & MEMCHECK_LOG) - { - if (opcode.flags & IS_STORE) - DevCon.WriteLn("Hit store breakpoint @0x%x", cpuRegs.pc); - else - DevCon.WriteLn("Hit load breakpoint @0x%x", cpuRegs.pc); - } - if (!CBreakPoints::HandleMemCheckHit(BREAKPOINT_EE, mc.start, mc.end)) return;