From de020978e409dc7c9a83a59d3ee44d305f751e18 Mon Sep 17 00:00:00 2001 From: Ty Lamontagne Date: Sun, 7 Jul 2024 21:41:00 -0400 Subject: [PATCH] Debugger: Use std::string for bp conditions. Implement memory bp conditions --- pcsx2-qt/Debugger/BreakpointDialog.cpp | 32 +++-- pcsx2-qt/Debugger/BreakpointDialog.ui | 5 +- pcsx2-qt/Debugger/Models/BreakpointModel.cpp | 136 +++++++++++++------ pcsx2/DebugTools/Breakpoints.cpp | 56 +++++--- pcsx2/DebugTools/Breakpoints.h | 51 ++++--- pcsx2/Interpreter.cpp | 10 +- pcsx2/R3000AInterpreter.cpp | 10 +- pcsx2/x86/iR3000A.cpp | 52 ++++--- pcsx2/x86/ix86-32/iR5900.cpp | 60 ++++---- 9 files changed, 252 insertions(+), 160 deletions(-) diff --git a/pcsx2-qt/Debugger/BreakpointDialog.cpp b/pcsx2-qt/Debugger/BreakpointDialog.cpp index 732958f928..3372911ac6 100644 --- a/pcsx2-qt/Debugger/BreakpointDialog.cpp +++ b/pcsx2-qt/Debugger/BreakpointDialog.cpp @@ -44,7 +44,7 @@ BreakpointDialog::BreakpointDialog(QWidget* parent, DebugInterface* cpu, Breakpo m_ui.txtAddress->setText(QtUtils::FilledQStringFromValue(bp->addr, 16)); if (bp->hasCond) - m_ui.txtCondition->setText(QString::fromLocal8Bit(bp->cond.expressionString)); + m_ui.txtCondition->setText(QString::fromStdString(bp->cond.expressionString)); } else if (const auto* mc = std::get_if(&bp_mc)) { @@ -53,12 +53,15 @@ BreakpointDialog::BreakpointDialog(QWidget* parent, DebugInterface* cpu, Breakpo m_ui.txtAddress->setText(QtUtils::FilledQStringFromValue(mc->start, 16)); m_ui.txtSize->setText(QtUtils::FilledQStringFromValue(mc->end - mc->start, 16)); - m_ui.chkRead->setChecked(mc->cond & MEMCHECK_READ); - m_ui.chkWrite->setChecked(mc->cond & MEMCHECK_WRITE); - m_ui.chkChange->setChecked(mc->cond & MEMCHECK_WRITE_ONCHANGE); + m_ui.chkRead->setChecked(mc->memCond & MEMCHECK_READ); + m_ui.chkWrite->setChecked(mc->memCond & MEMCHECK_WRITE); + 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); + + if (mc->hasCond) + m_ui.txtCondition->setText(QString::fromStdString(mc->cond.expressionString)); } } @@ -70,7 +73,6 @@ void BreakpointDialog::onRdoButtonToggled() { const bool isExecute = m_ui.rdoExecute->isChecked(); - m_ui.grpExecute->setEnabled(isExecute); m_ui.grpMemory->setEnabled(!isExecute); m_ui.chkLog->setEnabled(!isExecute); @@ -114,8 +116,7 @@ void BreakpointDialog::accept() } bp->cond.expression = expr; - strncpy(&bp->cond.expressionString[0], m_ui.txtCondition->text().toLocal8Bit().constData(), - sizeof(bp->cond.expressionString)); + bp->cond.expressionString = m_ui.txtCondition->text().toStdString(); } } if (auto* mc = std::get_if(&m_bp_mc)) @@ -141,6 +142,21 @@ void BreakpointDialog::accept() mc->start = startAddress; mc->end = startAddress + size; + if (!m_ui.txtCondition->text().isEmpty()) + { + mc->hasCond = true; + mc->cond.debug = m_cpu; + + if (!m_cpu->initExpression(m_ui.txtCondition->text().toLocal8Bit().constData(), expr)) + { + QMessageBox::warning(this, tr("Error"), tr("Invalid condition \"%1\"").arg(getExpressionError())); + return; + } + + mc->cond.expression = expr; + mc->cond.expressionString = m_ui.txtCondition->text().toStdString(); + } + int condition = 0; if (m_ui.chkRead->isChecked()) condition |= MEMCHECK_READ; @@ -149,7 +165,7 @@ void BreakpointDialog::accept() if (m_ui.chkChange->isChecked()) condition |= MEMCHECK_WRITE_ONCHANGE; - mc->cond = static_cast(condition); + mc->memCond = static_cast(condition); int result = 0; if (m_ui.chkEnable->isChecked()) diff --git a/pcsx2-qt/Debugger/BreakpointDialog.ui b/pcsx2-qt/Debugger/BreakpointDialog.ui index 76986c5621..a15212baf0 100644 --- a/pcsx2-qt/Debugger/BreakpointDialog.ui +++ b/pcsx2-qt/Debugger/BreakpointDialog.ui @@ -221,9 +221,6 @@ - - false - 110 @@ -233,7 +230,7 @@ - Execute + diff --git a/pcsx2-qt/Debugger/Models/BreakpointModel.cpp b/pcsx2-qt/Debugger/Models/BreakpointModel.cpp index d0546d9849..53fccf3c0a 100644 --- a/pcsx2-qt/Debugger/Models/BreakpointModel.cpp +++ b/pcsx2-qt/Debugger/Models/BreakpointModel.cpp @@ -52,7 +52,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const // Note: Fix up the disassemblymanager so we can use it here, instead of calling a function through the disassemblyview (yuck) return m_cpu.disasm(bp->addr, true).c_str(); case BreakpointColumns::CONDITION: - return bp->hasCond ? QString::fromLocal8Bit(bp->cond.expressionString) : ""; + return bp->hasCond ? QString::fromStdString(bp->cond.expressionString) : ""; case BreakpointColumns::HITS: return tr("--"); } @@ -66,10 +66,10 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const case BreakpointColumns::TYPE: { QString type(""); - type += (mc->cond & MEMCHECK_READ) ? tr("Read") : ""; - type += ((mc->cond & MEMCHECK_READWRITE) == MEMCHECK_READWRITE) ? ", " : " "; + type += (mc->memCond & MEMCHECK_READ) ? tr("Read") : ""; + type += ((mc->memCond & MEMCHECK_READWRITE) == MEMCHECK_READWRITE) ? ", " : " "; //: (C) = changes, as in "look for changes". - type += (mc->cond & MEMCHECK_WRITE) ? (mc->cond & MEMCHECK_WRITE_ONCHANGE) ? tr("Write(C)") : tr("Write") : ""; + type += (mc->memCond & MEMCHECK_WRITE) ? (mc->memCond & MEMCHECK_WRITE_ONCHANGE) ? tr("Write(C)") : tr("Write") : ""; return type; } case BreakpointColumns::OFFSET: @@ -79,7 +79,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const case BreakpointColumns::OPCODE: return tr("--"); // Our address is going to point to memory, no purpose in printing the op case BreakpointColumns::CONDITION: - return tr("--"); // No condition on memchecks + return mc->hasCond ? QString::fromStdString(mc->cond.expressionString) : ""; case BreakpointColumns::HITS: return QString::number(mc->numHits); } @@ -105,7 +105,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const // Note: Fix up the disassemblymanager so we can use it here, instead of calling a function through the disassemblyview (yuck) return m_cpu.disasm(bp->addr, false).c_str(); case BreakpointColumns::CONDITION: - return bp->hasCond ? QString::fromLocal8Bit(bp->cond.expressionString) : ""; + return bp->hasCond ? QString::fromStdString(bp->cond.expressionString) : ""; case BreakpointColumns::HITS: return 0; } @@ -117,7 +117,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const case BreakpointColumns::ENABLED: return (mc->result & MEMCHECK_BREAK); case BreakpointColumns::TYPE: - return mc->cond; + return mc->memCond; case BreakpointColumns::OFFSET: return mc->start; case BreakpointColumns::SIZE_LABEL: @@ -125,7 +125,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const case BreakpointColumns::OPCODE: return ""; case BreakpointColumns::CONDITION: - return ""; + return mc->hasCond ? QString::fromStdString(mc->cond.expressionString) : ""; case BreakpointColumns::HITS: return mc->numHits; } @@ -151,7 +151,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const // Note: Fix up the disassemblymanager so we can use it here, instead of calling a function through the disassemblyview (yuck) return m_cpu.disasm(bp->addr, false).c_str(); case BreakpointColumns::CONDITION: - return bp->hasCond ? QString::fromLocal8Bit(bp->cond.expressionString) : ""; + return bp->hasCond ? QString::fromStdString(bp->cond.expressionString) : ""; case BreakpointColumns::HITS: return 0; } @@ -161,7 +161,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const switch (index.column()) { case BreakpointColumns::TYPE: - return mc->cond; + return mc->memCond; case BreakpointColumns::OFFSET: return QtUtils::FilledQStringFromValue(mc->start, 16); case BreakpointColumns::SIZE_LABEL: @@ -169,7 +169,7 @@ QVariant BreakpointModel::data(const QModelIndex& index, int role) const case BreakpointColumns::OPCODE: return ""; case BreakpointColumns::CONDITION: - return ""; + return mc->hasCond ? QString::fromStdString(mc->cond.expressionString) : ""; case BreakpointColumns::HITS: return mc->numHits; case BreakpointColumns::ENABLED: @@ -256,14 +256,12 @@ Qt::ItemFlags BreakpointModel::flags(const QModelIndex& index) const { volatile const int row = index.row(); - bool is_breakpoint = std::holds_alternative(m_breakpoints.at(row)); + const bool is_breakpoint = std::holds_alternative(m_breakpoints.at(row)); switch (index.column()) { case BreakpointColumns::CONDITION: - if (is_breakpoint) - return Qt::ItemFlag::ItemIsEnabled | Qt::ItemFlag::ItemIsSelectable | Qt::ItemFlag::ItemIsEditable; - [[fallthrough]]; + return Qt::ItemFlag::ItemIsEnabled | Qt::ItemFlag::ItemIsSelectable | Qt::ItemFlag::ItemIsEditable; case BreakpointColumns::TYPE: case BreakpointColumns::OPCODE: case BreakpointColumns::HITS: @@ -292,7 +290,7 @@ bool BreakpointModel::setData(const QModelIndex& index, const QVariant& value, i else if (const auto* mc = std::get_if(&bp_mc)) { Host::RunOnCPUThread([cpu = this->m_cpu.getCpuType(), mc = *mc] { - CBreakPoints::ChangeMemCheck(cpu, mc.start, mc.end, mc.cond, + CBreakPoints::ChangeMemCheck(cpu, mc.start, mc.end, mc.memCond, MemCheckResult(mc.result ^ MEMCHECK_BREAK)); }); } @@ -303,40 +301,71 @@ bool BreakpointModel::setData(const QModelIndex& index, const QVariant& value, i { auto bp_mc = m_breakpoints.at(index.row()); - if (std::holds_alternative(bp_mc)) - return false; - - const auto bp = std::get(bp_mc); - - const QString condValue = value.toString(); - - if (condValue.isEmpty()) + if (auto* bp = std::get_if(&bp_mc)) { - if (bp.hasCond) + const QString condValue = value.toString(); + + if (condValue.isEmpty()) { - Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), bp] { - CBreakPoints::ChangeBreakPointRemoveCond(cpu, bp.addr); + if (bp->hasCond) + { + Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), bp] { + CBreakPoints::ChangeBreakPointRemoveCond(cpu, bp->addr); + }); + } + } + else + { + PostfixExpression expr; + + if (!m_cpu.initExpression(condValue.toLocal8Bit().constData(), expr)) + { + QMessageBox::warning(nullptr, "Condition Error", QString(getExpressionError())); + return false; + } + + BreakPointCond cond; + cond.debug = &m_cpu; + cond.expression = expr; + cond.expressionString = condValue.toStdString(); + + Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), bp, cond] { + CBreakPoints::ChangeBreakPointAddCond(cpu, bp->addr, cond); }); } } - else + else if (auto* mc = std::get_if(&bp_mc)) { - PostfixExpression expr; + const QString condValue = value.toString(); - if (!m_cpu.initExpression(condValue.toLocal8Bit().constData(), expr)) + if (condValue.isEmpty()) { - QMessageBox::warning(nullptr, "Condition Error", QString(getExpressionError())); - return false; + if (mc->hasCond) + { + Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), mc] { + CBreakPoints::ChangeMemCheckRemoveCond(cpu, mc->start, mc->end); + }); + } } + else + { + PostfixExpression expr; - BreakPointCond cond; - cond.debug = &m_cpu; - cond.expression = expr; - strcpy(&cond.expressionString[0], condValue.toLocal8Bit().constData()); + if (!m_cpu.initExpression(condValue.toLocal8Bit().constData(), expr)) + { + QMessageBox::warning(nullptr, "Condition Error", QString(getExpressionError())); + return false; + } - Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), bp, cond] { - CBreakPoints::ChangeBreakPointAddCond(cpu, bp.addr, cond); - }); + BreakPointCond cond; + cond.debug = &m_cpu; + cond.expression = expr; + cond.expressionString = condValue.toStdString(); + + Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), mc, cond] { + CBreakPoints::ChangeMemCheckAddCond(cpu, mc->start, mc->end, cond); + }); + } } emit dataChanged(index, index); return true; @@ -401,7 +430,11 @@ bool BreakpointModel::insertBreakpointRows(int row, int count, std::vector(&bp_mc)) { Host::RunOnCPUThread([cpu = m_cpu.getCpuType(), mc = *mc] { - CBreakPoints::AddMemCheck(cpu, mc.start, mc.end, mc.cond, mc.result); + CBreakPoints::AddMemCheck(cpu, mc.start, mc.end, mc.memCond, mc.result); + if (mc.hasCond) + { + CBreakPoints::ChangeMemCheckAddCond(cpu, mc.start, mc.end, mc.cond); + } }); } } @@ -413,18 +446,15 @@ bool BreakpointModel::insertBreakpointRows(int row, int count, std::vector all_breakpoints; std::ranges::move(CBreakPoints::GetBreakpoints(m_cpu.getCpuType(), false), std::back_inserter(all_breakpoints)); std::ranges::move(CBreakPoints::GetMemChecks(m_cpu.getCpuType()), std::back_inserter(all_breakpoints)); QtHost::RunOnUIThread([this, breakpoints = std::move(all_breakpoints)]() mutable { - beginResetModel(); m_breakpoints = std::move(breakpoints); endResetModel(); }); - }); } @@ -470,7 +500,7 @@ void BreakpointModel::loadBreakpointFromFieldList(QStringList fields) return; } bp.cond.expression = expr; - strncpy(&bp.cond.expressionString[0], fields[BreakpointModel::BreakpointColumns::CONDITION].toUtf8().constData(), sizeof(bp.cond.expressionString)); + bp.cond.expressionString = fields[BreakpointModel::BreakpointColumns::CONDITION].toStdString(); } // Enabled @@ -492,7 +522,7 @@ void BreakpointModel::loadBreakpointFromFieldList(QStringList fields) Console.WriteLn("Debugger Breakpoint Model: Failed to parse cond type '%s', skipping", fields[BreakpointModel::BreakpointColumns::TYPE].toUtf8().constData()); return; } - mc.cond = static_cast(type); + mc.memCond = static_cast(type); // Address QString test = fields[BreakpointModel::BreakpointColumns::OFFSET]; @@ -511,6 +541,22 @@ void BreakpointModel::loadBreakpointFromFieldList(QStringList fields) return; } + // Condition + if (!fields[BreakpointModel::BreakpointColumns::CONDITION].isEmpty()) + { + PostfixExpression expr; + mc.hasCond = true; + mc.cond.debug = &m_cpu; + + if (!m_cpu.initExpression(fields[BreakpointModel::BreakpointColumns::CONDITION].toUtf8().constData(), expr)) + { + Console.WriteLn("Debugger Breakpoint Model: Failed to parse cond '%s', skipping", fields[BreakpointModel::BreakpointColumns::CONDITION].toUtf8().constData()); + return; + } + mc.cond.expression = expr; + mc.cond.expressionString = fields[BreakpointModel::BreakpointColumns::CONDITION].toStdString(); + } + // Result const int result = fields[BreakpointModel::BreakpointColumns::ENABLED].toUInt(&ok); if (!ok) diff --git a/pcsx2/DebugTools/Breakpoints.cpp b/pcsx2/DebugTools/Breakpoints.cpp index 789e23ff20..bb38db2d5a 100644 --- a/pcsx2/DebugTools/Breakpoints.cpp +++ b/pcsx2/DebugTools/Breakpoints.cpp @@ -38,7 +38,8 @@ u32 standardizeBreakpointAddress(u32 addr) MemCheck::MemCheck() : start(0) , end(0) - , cond(MEMCHECK_READWRITE) + , hasCond(false) + , memCond(MEMCHECK_READWRITE) , result(MEMCHECK_BOTH) , cpu(BREAKPOINT_EE) , numHits(0) @@ -55,15 +56,15 @@ void MemCheck::Log(u32 addr, bool write, int size, u32 pc) void MemCheck::Action(u32 addr, bool write, int size, u32 pc) { int mask = write ? MEMCHECK_WRITE : MEMCHECK_READ; - if (cond & mask) + if (memCond & mask) { ++numHits; Log(addr, write, size, pc); if (result & MEMCHECK_BREAK) { - // Core_EnableStepping(true); - // host->SetDebugMode(true); + // Core_EnableStepping(true); + // host->SetDebugMode(true); } } } @@ -71,7 +72,7 @@ void MemCheck::Action(u32 addr, bool write, int size, u32 pc) void MemCheck::JitBefore(u32 addr, bool write, int size, u32 pc) { int mask = MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE; - if (write && (cond & mask) == mask) + if (write && (memCond & mask) == mask) { lastAddr = addr; lastPC = pc; @@ -157,7 +158,7 @@ bool CBreakPoints::IsAddressBreakPoint(BreakPointCpu cpu, u32 addr) bool CBreakPoints::IsAddressBreakPoint(BreakPointCpu cpu, u32 addr, bool* enabled) { - size_t bp = FindBreakpoint(cpu, addr); + const size_t bp = FindBreakpoint(cpu, addr); if (bp == INVALID_BREAKPOINT) return false; if (enabled != NULL) @@ -167,13 +168,13 @@ bool CBreakPoints::IsAddressBreakPoint(BreakPointCpu cpu, u32 addr, bool* enable bool CBreakPoints::IsTempBreakPoint(BreakPointCpu cpu, u32 addr) { - size_t bp = FindBreakpoint(cpu, addr, true, true); + const size_t bp = FindBreakpoint(cpu, addr, true, true); return bp != INVALID_BREAKPOINT; } void CBreakPoints::AddBreakPoint(BreakPointCpu cpu, u32 addr, bool temp, bool enabled) { - size_t bp = FindBreakpoint(cpu, addr, true, temp); + const size_t bp = FindBreakpoint(cpu, addr, true, temp); if (bp == INVALID_BREAKPOINT) { BreakPoint pt; @@ -211,7 +212,7 @@ void CBreakPoints::RemoveBreakPoint(BreakPointCpu cpu, u32 addr) void CBreakPoints::ChangeBreakPoint(BreakPointCpu cpu, u32 addr, bool status) { - size_t bp = FindBreakpoint(cpu, addr); + const size_t bp = FindBreakpoint(cpu, addr); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].enabled = status; @@ -245,7 +246,7 @@ void CBreakPoints::ClearTemporaryBreakPoints() void CBreakPoints::ChangeBreakPointAddCond(BreakPointCpu cpu, u32 addr, const BreakPointCond& cond) { - size_t bp = FindBreakpoint(cpu, addr, true, false); + const size_t bp = FindBreakpoint(cpu, addr, true, false); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].hasCond = true; @@ -256,7 +257,7 @@ void CBreakPoints::ChangeBreakPointAddCond(BreakPointCpu cpu, u32 addr, const Br void CBreakPoints::ChangeBreakPointRemoveCond(BreakPointCpu cpu, u32 addr) { - size_t bp = FindBreakpoint(cpu, addr, true, false); + const size_t bp = FindBreakpoint(cpu, addr, true, false); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].hasCond = false; @@ -282,13 +283,13 @@ void CBreakPoints::AddMemCheck(BreakPointCpu cpu, u32 start, u32 end, MemCheckCo // This will ruin any pending memchecks. cleanupMemChecks_.clear(); - size_t mc = FindMemCheck(cpu, start, end); + const size_t mc = FindMemCheck(cpu, start, end); if (mc == INVALID_MEMCHECK) { MemCheck check; check.start = start; check.end = end; - check.cond = cond; + check.memCond = cond; check.result = result; check.cpu = cpu; @@ -297,7 +298,7 @@ void CBreakPoints::AddMemCheck(BreakPointCpu cpu, u32 start, u32 end, MemCheckCo } else { - memChecks_[mc].cond = (MemCheckCondition)(memChecks_[mc].cond | cond); + memChecks_[mc].memCond = (MemCheckCondition)(memChecks_[mc].memCond | cond); memChecks_[mc].result = (MemCheckResult)(memChecks_[mc].result | result); Update(cpu); } @@ -308,7 +309,7 @@ void CBreakPoints::RemoveMemCheck(BreakPointCpu cpu, u32 start, u32 end) // This will ruin any pending memchecks. cleanupMemChecks_.clear(); - size_t mc = FindMemCheck(cpu, start, end); + const size_t mc = FindMemCheck(cpu, start, end); if (mc != INVALID_MEMCHECK) { memChecks_.erase(memChecks_.begin() + mc); @@ -318,15 +319,36 @@ void CBreakPoints::RemoveMemCheck(BreakPointCpu cpu, u32 start, u32 end) void CBreakPoints::ChangeMemCheck(BreakPointCpu cpu, u32 start, u32 end, MemCheckCondition cond, MemCheckResult result) { - size_t mc = FindMemCheck(cpu, start, end); + const size_t mc = FindMemCheck(cpu, start, end); if (mc != INVALID_MEMCHECK) { - memChecks_[mc].cond = cond; + memChecks_[mc].memCond = cond; memChecks_[mc].result = result; Update(cpu); } } +void CBreakPoints::ChangeMemCheckRemoveCond(BreakPointCpu cpu, u32 start, u32 end) +{ + const size_t mc = FindMemCheck(cpu, start, end); + if (mc != INVALID_MEMCHECK) + { + memChecks_[mc].hasCond = false; + Update(cpu); + } +} + +void CBreakPoints::ChangeMemCheckAddCond(BreakPointCpu cpu, u32 start, u32 end, const BreakPointCond& cond) +{ + const size_t mc = FindMemCheck(cpu, start, end); + if (mc != INVALID_MEMCHECK) + { + memChecks_[mc].hasCond = true; + memChecks_[mc].cond = cond; + Update(cpu); + } +} + void CBreakPoints::ClearAllMemChecks() { // This will ruin any pending memchecks. diff --git a/pcsx2/DebugTools/Breakpoints.h b/pcsx2/DebugTools/Breakpoints.h index 4919fdfe70..90a9af4850 100644 --- a/pcsx2/DebugTools/Breakpoints.h +++ b/pcsx2/DebugTools/Breakpoints.h @@ -12,29 +12,35 @@ struct BreakPointCond { - DebugInterface *debug; + DebugInterface* debug; PostfixExpression expression; - char expressionString[128]; + std::string expressionString; - BreakPointCond() : debug(NULL) + BreakPointCond() + : debug(NULL) { - expressionString[0] = '\0'; } u32 Evaluate() { u64 result; - if (!debug->parseExpression(expression,result) || result == 0) return 0; + if (!debug->parseExpression(expression, result) || result == 0) + return 0; return 1; } }; struct BreakPoint { - BreakPoint() : addr(0), enabled(false), temporary(false), hasCond(false) - {} + BreakPoint() + : addr(0) + , enabled(false) + , temporary(false) + , hasCond(false) + { + } - u32 addr; + u32 addr; bool enabled; bool temporary; @@ -42,10 +48,12 @@ struct BreakPoint BreakPointCond cond; BreakPointCpu cpu; - bool operator == (const BreakPoint &other) const { + bool operator==(const BreakPoint& other) const + { return addr == other.addr; } - bool operator < (const BreakPoint &other) const { + bool operator<(const BreakPoint& other) const + { return addr < other.addr; } }; @@ -75,7 +83,9 @@ struct MemCheck u32 start; u32 end; - MemCheckCondition cond; + bool hasCond; + BreakPointCond cond; + MemCheckCondition memCond; MemCheckResult result; BreakPointCpu cpu; @@ -91,7 +101,8 @@ struct MemCheck void Log(u32 addr, bool write, int size, u32 pc); - bool operator == (const MemCheck &other) const { + bool operator==(const MemCheck& other) const + { return start == other.start && end == other.end; } }; @@ -115,13 +126,15 @@ public: static void ClearTemporaryBreakPoints(); // Makes a copy. Temporary breakpoints can't have conditions. - static void ChangeBreakPointAddCond(BreakPointCpu cpu, u32 addr, const BreakPointCond &cond); + static void ChangeBreakPointAddCond(BreakPointCpu cpu, u32 addr, const BreakPointCond& cond); static void ChangeBreakPointRemoveCond(BreakPointCpu cpu, u32 addr); - static BreakPointCond *GetBreakPointCondition(BreakPointCpu cpu, u32 addr); + static BreakPointCond* GetBreakPointCondition(BreakPointCpu cpu, u32 addr); static void AddMemCheck(BreakPointCpu cpu, u32 start, u32 end, MemCheckCondition cond, MemCheckResult result); static void RemoveMemCheck(BreakPointCpu cpu, u32 start, u32 end); static void ChangeMemCheck(BreakPointCpu cpu, u32 start, u32 end, MemCheckCondition cond, MemCheckResult result); + static void ChangeMemCheckRemoveCond(BreakPointCpu cpu, u32 start, u32 end); + static void ChangeMemCheckAddCond(BreakPointCpu cpu, u32 start, u32 end, const BreakPointCond& cond); static void ClearAllMemChecks(); static void SetSkipFirst(BreakPointCpu cpu, u32 pc); @@ -135,14 +148,18 @@ public: static const std::vector GetBreakpoints(BreakPointCpu cpu, bool includeTemp); // Returns count of all non-temporary breakpoints static size_t GetNumBreakpoints() - { + { return std::count_if(breakPoints_.begin(), breakPoints_.end(), [](BreakPoint& bp) { return !bp.temporary; }); } static size_t GetNumMemchecks() { return memChecks_.size(); } static void Update(BreakPointCpu cpu = BREAKPOINT_IOP_AND_EE, u32 addr = 0); - static void SetBreakpointTriggered(bool triggered, BreakPointCpu cpu = BreakPointCpu::BREAKPOINT_IOP_AND_EE) { breakpointTriggered_ = triggered; breakpointTriggeredCpu_ = cpu; }; + static void SetBreakpointTriggered(bool triggered, BreakPointCpu cpu = BreakPointCpu::BREAKPOINT_IOP_AND_EE) + { + breakpointTriggered_ = triggered; + breakpointTriggeredCpu_ = cpu; + }; static bool GetBreakpointTriggered() { return breakpointTriggered_; }; static BreakPointCpu GetBreakpointTriggeredCpu() { return breakpointTriggeredCpu_; }; @@ -165,7 +182,7 @@ private: static bool corePaused; static std::vector memChecks_; - static std::vector cleanupMemChecks_; + static std::vector cleanupMemChecks_; }; diff --git a/pcsx2/Interpreter.cpp b/pcsx2/Interpreter.cpp index 56fe619630..331bad4a7a 100644 --- a/pcsx2/Interpreter.cpp +++ b/pcsx2/Interpreter.cpp @@ -99,11 +99,17 @@ void intMemcheck(u32 op, u32 bits, bool store) if (check.result == 0) continue; - if ((check.cond & MEMCHECK_WRITE) == 0 && store) + if ((check.memCond & MEMCHECK_WRITE) == 0 && store) continue; - if ((check.cond & MEMCHECK_READ) == 0 && !store) + if ((check.memCond & MEMCHECK_READ) == 0 && !store) continue; + if (check.hasCond) + { + if (!check.cond.Evaluate()) + continue; + } + if (start < check.end && check.start < end) intBreakpoint(true); } diff --git a/pcsx2/R3000AInterpreter.cpp b/pcsx2/R3000AInterpreter.cpp index 6aedd35c76..cf6fa00e7f 100644 --- a/pcsx2/R3000AInterpreter.cpp +++ b/pcsx2/R3000AInterpreter.cpp @@ -150,11 +150,17 @@ void psxMemcheck(u32 op, u32 bits, bool store) if (check.result == 0) continue; - if ((check.cond & MEMCHECK_WRITE) == 0 && store) + if ((check.memCond & MEMCHECK_WRITE) == 0 && store) continue; - if ((check.cond & MEMCHECK_READ) == 0 && !store) + if ((check.memCond & MEMCHECK_READ) == 0 && !store) continue; + if (check.hasCond) + { + if (!check.cond.Evaluate()) + continue; + } + if (start < check.end && check.start < end) psxBreakpoint(true); } diff --git a/pcsx2/x86/iR3000A.cpp b/pcsx2/x86/iR3000A.cpp index 902f20d070..53643a35a2 100644 --- a/pcsx2/x86/iR3000A.cpp +++ b/pcsx2/x86/iR3000A.cpp @@ -1302,12 +1302,30 @@ static bool psxDynarecCheckBreakpoint() return true; } -static bool psxDynarecMemcheck() +static bool psxDynarecMemcheck(size_t i) { - u32 pc = psxRegs.pc; + const u32 pc = psxRegs.pc; + const u32 op = iopMemRead32(pc); + const R5900::OPCODE& opcode = R5900::GetInstruction(op); + auto mc = CBreakPoints::GetMemChecks(BREAKPOINT_IOP)[i]; + if (CBreakPoints::CheckSkipFirst(BREAKPOINT_IOP, pc) == pc) return false; + if (mc.hasCond) + { + if (!mc.cond.Evaluate()) + 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); + } + CBreakPoints::SetBreakpointTriggered(true, BREAKPOINT_IOP); VMManager::SetPaused(true); @@ -1316,14 +1334,6 @@ static bool psxDynarecMemcheck() return true; } -static void psxDynarecMemLogcheck(u32 start, bool store) -{ - if (store) - DevCon.WriteLn("Hit store breakpoint @0x%x", start); - else - DevCon.WriteLn("Hit load breakpoint @0x%x", start); -} - static void psxRecMemcheck(u32 op, u32 bits, bool store) { _psxFlushCall(FLUSH_EVERYTHING | FLUSH_PC); @@ -1344,9 +1354,9 @@ static void psxRecMemcheck(u32 op, u32 bits, bool store) { if (checks[i].result == 0) continue; - if ((checks[i].cond & MEMCHECK_WRITE) == 0 && store) + if ((checks[i].memCond & MEMCHECK_WRITE) == 0 && store) continue; - if ((checks[i].cond & MEMCHECK_READ) == 0 && !store) + if ((checks[i].memCond & MEMCHECK_READ) == 0 && !store) continue; // logic: memAddress < bpEnd && bpStart < memAddress+memSize @@ -1360,25 +1370,11 @@ static void psxRecMemcheck(u32 op, u32 bits, bool store) xForwardJGE8 next2; // if start >= address+size then goto next2 // hit the breakpoint - if (checks[i].result & MEMCHECK_LOG) - { - xMOV(edx, store); - // Refer to the EE recompiler for an explaination - if(!(checks[i].result & MEMCHECK_BREAK)) - { - xPUSH(eax); xPUSH(ebx); xPUSH(ecx); xPUSH(edx); - xFastCall((void*)psxDynarecMemLogcheck, ecx, edx); - xPOP(edx); xPOP(ecx); xPOP(ebx); xPOP(eax); - } - else - { - xFastCall((void*)psxDynarecMemLogcheck, ecx, edx); - } - } if (checks[i].result & MEMCHECK_BREAK) { - xFastCall((void*)psxDynarecMemcheck); + xMOV(eax, i); + xFastCall((void*)psxDynarecMemcheck, eax); xTEST(al, al); xJNZ(iopExitRecompiledCode); } diff --git a/pcsx2/x86/ix86-32/iR5900.cpp b/pcsx2/x86/ix86-32/iR5900.cpp index bf4bebc3f3..db549f717c 100644 --- a/pcsx2/x86/ix86-32/iR5900.cpp +++ b/pcsx2/x86/ix86-32/iR5900.cpp @@ -1535,25 +1535,34 @@ void dynarecCheckBreakpoint() recExitExecution(); } -void dynarecMemcheck() +void dynarecMemcheck(size_t i) { - const u32 pc = cpuRegs.pc; + const u32 op = memRead32(cpuRegs.pc); + const OPCODE& opcode = GetInstruction(op); if (CBreakPoints::CheckSkipFirst(BREAKPOINT_EE, pc) != 0) return; + auto mc = CBreakPoints::GetMemChecks(BREAKPOINT_EE)[i]; + + if(mc.hasCond) + { + if (!mc.cond.Evaluate()) + 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); + } + CBreakPoints::SetBreakpointTriggered(true, BREAKPOINT_EE); VMManager::SetPaused(true); recExitExecution(); } -void dynarecMemLogcheck(u32 start, bool store) -{ - if (store) - DevCon.WriteLn("Hit store breakpoint @0x%x", start); - else - DevCon.WriteLn("Hit load breakpoint @0x%x", start); -} - void recMemcheck(u32 op, u32 bits, bool store) { iFlushCall(FLUSH_EVERYTHING | FLUSH_PC); @@ -1578,9 +1587,9 @@ void recMemcheck(u32 op, u32 bits, bool store) { if (checks[i].result == 0) continue; - if ((checks[i].cond & MEMCHECK_WRITE) == 0 && store) + if ((checks[i].memCond & MEMCHECK_WRITE) == 0 && store) continue; - if ((checks[i].cond & MEMCHECK_READ) == 0 && !store) + if ((checks[i].memCond & MEMCHECK_READ) == 0 && !store) continue; // logic: memAddress < bpEnd && bpStart < memAddress+memSize @@ -1594,33 +1603,10 @@ void recMemcheck(u32 op, u32 bits, bool store) xForwardJGE8 next2; // if start >= address+size then goto next2 // hit the breakpoint - if (checks[i].result & MEMCHECK_LOG) - { - xMOV(edx, store); - // Preserve ecx (address) and edx (address+size) because we aren't breaking - // out of this loops iteration and dynarecMemLogcheck will clobber them - // Also keep 16 byte stack alignment - if (!(checks[i].result & MEMCHECK_BREAK)) - { - xPUSH(eax); - xPUSH(ebx); - xPUSH(ecx); - xPUSH(edx); - xFastCall((void*)dynarecMemLogcheck, ecx, edx); - xPOP(edx); - xPOP(ecx); - xPOP(ebx); - xPOP(eax); - } - else - { - xFastCall((void*)dynarecMemLogcheck, ecx, edx); - } - } if (checks[i].result & MEMCHECK_BREAK) { - // Don't need to preserve edx and ecx, we don't return - xFastCall((void*)dynarecMemcheck); + xMOV(eax, i); + xFastCall((void*)dynarecMemcheck, eax); } next1.SetTarget();