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.

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<int>(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<int>(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<u32>(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<u32>(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<MemCheckResult>(result);
 	}

pcsx2-qt/Debugger/Breakpoints/BreakpointDialog.ui

@@ -283,7 +283,7 @@
     </item>
    </layout>
   </widget>
-  <widget class="QGroupBox" name="grpInstrumentation">
+  <widget class="QGroupBox" name="grpLogWhenHit">
    <property name="enabled">
     <bool>true</bool>
    </property>
@@ -296,7 +296,7 @@
     </rect>
    </property>
    <property name="title">
-    <string>Instrumentation</string>
+    <string>Log When Hit</string>
    </property>
    <property name="checkable">
     <bool>true</bool>
@@ -347,19 +347,6 @@
     <string/>
    </property>
    <layout class="QVBoxLayout" name="verticalLayout">
-    <item>
-     <widget class="QCheckBox" name="chkLog">
-      <property name="enabled">
-       <bool>true</bool>
-      </property>
-      <property name="text">
-       <string>Log</string>
-      </property>
-      <property name="checked">
-       <bool>true</bool>
-      </property>
-     </widget>
-    </item>
     <item>
      <widget class="QCheckBox" name="chkEnable">
       <property name="text">
@@ -425,7 +412,6 @@
   <tabstop>rdoExecute</tabstop>
   <tabstop>txtAddress</tabstop>
   <tabstop>txtDescription</tabstop>
-  <tabstop>chkLog</tabstop>
   <tabstop>chkEnable</tabstop>
   <tabstop>spnMaxHitCount</tabstop>
   <tabstop>chkRead</tabstop>
@@ -433,6 +419,7 @@
   <tabstop>chkChange</tabstop>
   <tabstop>txtSize</tabstop>
   <tabstop>txtCondition</tabstop>
+  <tabstop>grpLogWhenHit</tabstop>
   <tabstop>txtLogFormat</tabstop>
   <tabstop>chkContinueOnHit</tabstop>
  </tabstops>

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<DebuggerEvents::Refresh>([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;
 	});
 }

pcsx2/DebugTools/Breakpoints.cpp

@@ -6,7 +6,10 @@
 #include "MIPSAnalyst.h"
 #include <cstdio>
 #include "R5900.h"
+#include "R5900OpcodeTables.h"
 #include "R3000A.h"
+#include "Memory.h"
+#include "IopMem.h"
 #include "common/Console.h"
 
 std::vector<BreakPoint> 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)

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);

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);

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;