From a17fbe7c6566682ef88119875b9ce0f719ebf257 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Fri, 2 Sep 2022 02:57:30 -0700 Subject: [PATCH] Expand conditional breakpoints to memory breakpoints --- .../Core/Core/Debugger/PPCDebugInterface.cpp | 2 +- Source/Core/Core/PowerPC/BreakPoints.cpp | 38 +++++++++--- Source/Core/Core/PowerPC/BreakPoints.h | 4 +- Source/Core/Core/PowerPC/GDBStub.cpp | 2 +- .../DolphinQt/Debugger/BreakpointDialog.cpp | 58 ++++++++++--------- .../DolphinQt/Debugger/BreakpointDialog.h | 6 +- .../DolphinQt/Debugger/BreakpointWidget.cpp | 21 +++++-- .../DolphinQt/Debugger/BreakpointWidget.h | 4 +- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 2 +- 9 files changed, 88 insertions(+), 49 deletions(-) diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.cpp b/Source/Core/Core/Debugger/PPCDebugInterface.cpp index 03d8331a11..2cbeae41fb 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.cpp +++ b/Source/Core/Core/Debugger/PPCDebugInterface.cpp @@ -335,7 +335,7 @@ void PPCDebugInterface::ToggleMemCheck(u32 address, bool read, bool write, bool MemCheck.log_on_hit = log; MemCheck.break_on_hit = true; - PowerPC::memchecks.Add(MemCheck); + PowerPC::memchecks.Add(std::move(MemCheck)); } else { diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index 9c12768493..be2ae192e1 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -83,7 +83,6 @@ void BreakPoints::AddFromStrings(const TBreakPointsStr& bp_strings) if (iss.peek() == '$') iss.ignore(); - iss >> std::hex >> bp.address; iss >> flags; bp.is_enabled = flags.find('n') != flags.npos; @@ -203,11 +202,21 @@ MemChecks::TMemChecksStr MemChecks::GetStrings() const { std::ostringstream ss; ss.imbue(std::locale::classic()); + ss << fmt::format("${:08x} {:08x} ", mc.start_address, mc.end_address); + if (mc.is_enabled) + ss << 'n'; + if (mc.is_break_on_read) + ss << 'r'; + if (mc.is_break_on_write) + ss << 'w'; + if (mc.log_on_hit) + ss << 'l'; + if (mc.break_on_hit) + ss << 'b'; + if (mc.condition) + ss << "c " << mc.condition->GetText(); - ss << std::hex << mc.start_address << " " << mc.end_address << " " << (mc.is_enabled ? "n" : "") - << (mc.is_break_on_read ? "r" : "") << (mc.is_break_on_write ? "w" : "") - << (mc.log_on_hit ? "l" : "") << (mc.break_on_hit ? "b" : ""); - mc_strings.push_back(ss.str()); + mc_strings.emplace_back(ss.str()); } return mc_strings; @@ -221,6 +230,9 @@ void MemChecks::AddFromStrings(const TMemChecksStr& mc_strings) std::istringstream iss(mc_string); iss.imbue(std::locale::classic()); + if (iss.peek() == '$') + iss.ignore(); + std::string flags; iss >> std::hex >> mc.start_address >> mc.end_address >> flags; @@ -230,12 +242,19 @@ void MemChecks::AddFromStrings(const TMemChecksStr& mc_strings) mc.is_break_on_write = flags.find('w') != flags.npos; mc.log_on_hit = flags.find('l') != flags.npos; mc.break_on_hit = flags.find('b') != flags.npos; + if (flags.find('c') != std::string::npos) + { + iss >> std::ws; + std::string condition; + std::getline(iss, condition); + mc.condition = Expression::TryParse(condition); + } - Add(mc); + Add(std::move(mc)); } } -void MemChecks::Add(const TMemCheck& memory_check) +void MemChecks::Add(TMemCheck memory_check) { bool had_any = HasAny(); Core::RunAsCPUThread([&] { @@ -254,7 +273,7 @@ void MemChecks::Add(const TMemCheck& memory_check) } else { - m_mem_checks.push_back(memory_check); + m_mem_checks.emplace_back(std::move(memory_check)); } // If this is the first one, clear the JIT cache so it can switch to // watchpoint-compatible code. @@ -338,7 +357,8 @@ bool TMemCheck::Action(Common::DebugInterface* debug_interface, u64 value, u32 a if (!is_enabled) return false; - if ((write && is_break_on_write) || (!write && is_break_on_read)) + if (((write && is_break_on_write) || (!write && is_break_on_read)) && + EvaluateCondition(this->condition)) { if (log_on_hit) { diff --git a/Source/Core/Core/PowerPC/BreakPoints.h b/Source/Core/Core/PowerPC/BreakPoints.h index 5a8a74b9f7..0be6981ebe 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.h +++ b/Source/Core/Core/PowerPC/BreakPoints.h @@ -42,6 +42,8 @@ struct TMemCheck u32 num_hits = 0; + std::optional condition; + // returns whether to break bool Action(Common::DebugInterface* debug_interface, u64 value, u32 addr, bool write, size_t size, u32 pc); @@ -93,7 +95,7 @@ public: TMemChecksStr GetStrings() const; void AddFromStrings(const TMemChecksStr& mc_strings); - void Add(const TMemCheck& memory_check); + void Add(TMemCheck memory_check); bool ToggleBreakPoint(u32 address); diff --git a/Source/Core/Core/PowerPC/GDBStub.cpp b/Source/Core/Core/PowerPC/GDBStub.cpp index 9a9d8f8649..a5cdc7dbb3 100644 --- a/Source/Core/Core/PowerPC/GDBStub.cpp +++ b/Source/Core/Core/PowerPC/GDBStub.cpp @@ -865,7 +865,7 @@ static bool AddBreakpoint(BreakpointType type, u32 addr, u32 len) new_memcheck.break_on_hit = true; new_memcheck.log_on_hit = false; new_memcheck.is_enabled = true; - PowerPC::memchecks.Add(new_memcheck); + PowerPC::memchecks.Add(std::move(new_memcheck)); INFO_LOG_FMT(GDB_STUB, "gdb: added {} memcheck: {:08x} bytes at {:08x}", static_cast(type), len, addr); } diff --git a/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp b/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp index 6c9edd54c4..29f879a482 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp @@ -43,7 +43,7 @@ BreakpointDialog::BreakpointDialog(BreakpointWidget* parent, const TBreakPoint* m_instruction_address->setText(QString::number(breakpoint->address, 16)); if (breakpoint->condition) - m_instruction_condition->setText(QString::fromStdString(breakpoint->condition->GetText())); + m_conditional->setText(QString::fromStdString(breakpoint->condition->GetText())); m_do_break->setChecked(breakpoint->break_on_hit && !breakpoint->log_on_hit); m_do_log->setChecked(!breakpoint->break_on_hit && breakpoint->log_on_hit); @@ -73,6 +73,8 @@ BreakpointDialog::BreakpointDialog(BreakpointWidget* parent, const TMemCheck* me { m_memory_address_to->setText(QString::number(memcheck->start_address + 1, 16)); } + if (memcheck->condition) + m_conditional->setText(QString::fromStdString(memcheck->condition->GetText())); m_memory_on_read->setChecked(memcheck->is_break_on_read && !memcheck->is_break_on_write); m_memory_on_write->setChecked(!memcheck->is_break_on_read && memcheck->is_break_on_write); @@ -88,7 +90,8 @@ BreakpointDialog::BreakpointDialog(BreakpointWidget* parent, const TMemCheck* me void BreakpointDialog::CreateWidgets() { - m_buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); + m_buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel | + QDialogButtonBox::Help); auto* type_group = new QButtonGroup(this); // Instruction BP @@ -99,18 +102,13 @@ void BreakpointDialog::CreateWidgets() type_group->addButton(m_instruction_bp); m_instruction_box = new QGroupBox; m_instruction_address = new QLineEdit; - m_instruction_condition = new QLineEdit; - m_cond_help_btn = new QPushButton(tr("Help")); - auto* instruction_data_layout = new QGridLayout; + auto* instruction_data_layout = new QHBoxLayout; m_instruction_box->setLayout(instruction_data_layout); - instruction_data_layout->addWidget(new QLabel(tr("Address:")), 0, 0); - instruction_data_layout->addWidget(m_instruction_address, 0, 1); - instruction_data_layout->addWidget(new QLabel(tr("Condition:")), 1, 0); - instruction_data_layout->addWidget(m_instruction_condition, 1, 1); + instruction_data_layout->addWidget(new QLabel(tr("Address:"))); + instruction_data_layout->addWidget(m_instruction_address); instruction_layout->addWidget(m_instruction_bp, 0, 0, 1, 1); - instruction_layout->addWidget(m_cond_help_btn, 0, 1, 1, 1); instruction_layout->addWidget(m_instruction_box, 1, 0, 1, 2); instruction_widget->setLayout(instruction_layout); @@ -155,6 +153,7 @@ void BreakpointDialog::CreateWidgets() memory_data_layout->addWidget(m_memory_address_from, 1, 1); memory_data_layout->addWidget(m_memory_address_to_label, 1, 2); memory_data_layout->addWidget(m_memory_address_to, 1, 3); + QGroupBox* condition_box = new QGroupBox(tr("Condition")); auto* condition_layout = new QHBoxLayout; condition_box->setLayout(condition_layout); @@ -169,12 +168,23 @@ void BreakpointDialog::CreateWidgets() memory_widget->setLayout(memory_layout); QGroupBox* action_box = new QGroupBox(tr("Action")); + + QHBoxLayout* conditional_layout = new QHBoxLayout; + m_conditional = new QLineEdit(); + conditional_layout->addWidget(new QLabel(tr("Condition:"))); + conditional_layout->addWidget(m_conditional); + auto* action_layout = new QHBoxLayout; - action_box->setLayout(action_layout); action_layout->addWidget(m_do_log); action_layout->addWidget(m_do_break); action_layout->addWidget(m_do_log_and_break); + auto* action_vlayout = new QVBoxLayout; + action_vlayout->addLayout(conditional_layout); + action_vlayout->addLayout(action_layout); + + action_box->setLayout(action_vlayout); + auto* layout = new QVBoxLayout; layout->addWidget(instruction_widget); layout->addWidget(memory_widget); @@ -195,7 +205,6 @@ void BreakpointDialog::CreateWidgets() break; case OpenMode::EditMemCheck: instruction_widget->setVisible(false); - m_cond_help_btn->setVisible(false); m_memory_bp->setChecked(true); m_memory_address_from->setEnabled(false); m_memory_address_to->setFocus(); @@ -209,8 +218,7 @@ void BreakpointDialog::ConnectWidgets() { connect(m_buttons, &QDialogButtonBox::accepted, this, &BreakpointDialog::accept); connect(m_buttons, &QDialogButtonBox::rejected, this, &BreakpointDialog::reject); - - connect(m_cond_help_btn, &QPushButton::clicked, this, &BreakpointDialog::ShowConditionHelp); + connect(m_buttons, &QDialogButtonBox::helpRequested, this, &BreakpointDialog::ShowConditionHelp); connect(m_instruction_bp, &QRadioButton::toggled, this, &BreakpointDialog::OnBPTypeChanged); connect(m_memory_bp, &QRadioButton::toggled, this, &BreakpointDialog::OnBPTypeChanged); @@ -257,6 +265,14 @@ void BreakpointDialog::accept() bool good; + const QString condition = m_conditional->text().trimmed(); + + if (!condition.isEmpty() && !Expression::TryParse(condition.toUtf8().constData())) + { + invalid_input(tr("Condition")); + return; + } + if (instruction) { u32 address = m_instruction_address->text().toUInt(&good, 16); @@ -267,14 +283,6 @@ void BreakpointDialog::accept() return; } - const QString condition = m_instruction_condition->text().trimmed(); - - if (!condition.isEmpty() && !Expression::TryParse(condition.toUtf8().constData())) - { - invalid_input(tr("Condition")); - return; - } - m_parent->AddBP(address, false, do_break, do_log, condition); } else @@ -296,11 +304,11 @@ void BreakpointDialog::accept() return; } - m_parent->AddRangedMBP(from, to, on_read, on_write, do_log, do_break); + m_parent->AddRangedMBP(from, to, on_read, on_write, do_log, do_break, condition); } else { - m_parent->AddAddressMBP(from, on_read, on_write, do_log, do_break); + m_parent->AddAddressMBP(from, on_read, on_write, do_log, do_break, condition); } } @@ -310,8 +318,6 @@ void BreakpointDialog::accept() void BreakpointDialog::ShowConditionHelp() { const auto message = QStringLiteral( - "Set a code breakpoint for when an instruction is executed. Use with the code widget.\n" - "\n" "Conditions:\n" "Sets an expression that is evaluated when a breakpoint is hit. If the expression is false " "or 0, the breakpoint is ignored until hit again. Statements should be separated by a comma. " diff --git a/Source/Core/DolphinQt/Debugger/BreakpointDialog.h b/Source/Core/DolphinQt/Debugger/BreakpointDialog.h index 120c48b289..5a09763e10 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointDialog.h +++ b/Source/Core/DolphinQt/Debugger/BreakpointDialog.h @@ -16,6 +16,7 @@ class QGroupBox; class QLabel; class QLineEdit; class QRadioButton; +class QPushButton; class BreakpointDialog : public QDialog { @@ -46,8 +47,6 @@ private: QRadioButton* m_instruction_bp; QGroupBox* m_instruction_box; QLineEdit* m_instruction_address; - QLineEdit* m_instruction_condition; - QPushButton* m_cond_help_btn; // Memory BPs QRadioButton* m_memory_bp; @@ -67,6 +66,9 @@ private: QRadioButton* m_do_break; QRadioButton* m_do_log_and_break; + QLineEdit* m_conditional; + QPushButton* m_cond_help_btn; + QDialogButtonBox* m_buttons; BreakpointWidget* m_parent; diff --git a/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp b/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp index fcc989f80a..019494491b 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp @@ -255,6 +255,13 @@ void BreakpointWidget::Update() m_table->setItem(i, 4, create_item(flags)); + QString condition; + + if (mbp.condition) + condition = QString::fromStdString(mbp.condition->GetText()); + + m_table->setItem(i, 5, create_item(condition)); + i++; } } @@ -430,7 +437,7 @@ void BreakpointWidget::AddBP(u32 addr, bool temp, bool break_on_hit, bool log_on } void BreakpointWidget::AddAddressMBP(u32 addr, bool on_read, bool on_write, bool do_log, - bool do_break) + bool do_break, const QString& condition) { TMemCheck check; @@ -441,10 +448,11 @@ void BreakpointWidget::AddAddressMBP(u32 addr, bool on_read, bool on_write, bool check.is_break_on_write = on_write; check.log_on_hit = do_log; check.break_on_hit = do_break; - + check.condition = + !condition.isEmpty() ? Expression::TryParse(condition.toUtf8().constData()) : std::nullopt; { const QSignalBlocker blocker(Settings::Instance()); - PowerPC::memchecks.Add(check); + PowerPC::memchecks.Add(std::move(check)); } emit BreakpointsChanged(); @@ -452,7 +460,7 @@ void BreakpointWidget::AddAddressMBP(u32 addr, bool on_read, bool on_write, bool } void BreakpointWidget::AddRangedMBP(u32 from, u32 to, bool on_read, bool on_write, bool do_log, - bool do_break) + bool do_break, const QString& condition) { TMemCheck check; @@ -463,10 +471,11 @@ void BreakpointWidget::AddRangedMBP(u32 from, u32 to, bool on_read, bool on_writ check.is_break_on_write = on_write; check.log_on_hit = do_log; check.break_on_hit = do_break; - + check.condition = + !condition.isEmpty() ? Expression::TryParse(condition.toUtf8().constData()) : std::nullopt; { const QSignalBlocker blocker(Settings::Instance()); - PowerPC::memchecks.Add(check); + PowerPC::memchecks.Add(std::move(check)); } emit BreakpointsChanged(); diff --git a/Source/Core/DolphinQt/Debugger/BreakpointWidget.h b/Source/Core/DolphinQt/Debugger/BreakpointWidget.h index dac89ac05c..bff6d2b883 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointWidget.h +++ b/Source/Core/DolphinQt/Debugger/BreakpointWidget.h @@ -23,9 +23,9 @@ public: void AddBP(u32 addr); void AddBP(u32 addr, bool temp, bool break_on_hit, bool log_on_hit, const QString& condition); void AddAddressMBP(u32 addr, bool on_read = true, bool on_write = true, bool do_log = true, - bool do_break = true); + bool do_break = true, const QString& condition = {}); void AddRangedMBP(u32 from, u32 to, bool do_read = true, bool do_write = true, bool do_log = true, - bool do_break = true); + bool do_break = true, const QString& condition = {}); void UpdateButtonsEnabled(); void Update(); diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index e83e77eb1b..1b544e3f48 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -797,7 +797,7 @@ void MemoryViewWidget::ToggleBreakpoint(u32 addr, bool row) check.log_on_hit = m_do_log; check.break_on_hit = true; - PowerPC::memchecks.Add(check); + PowerPC::memchecks.Add(std::move(check)); } else if (check_ptr != nullptr) {