mirror of
https://github.com/dolphin-emu/dolphin.git
synced 2025-07-21 05:09:34 -06:00
Debugger: Rework temporary breakpoints
Before: 1. In theory there could be multiple, but in practice they were (manually) cleared before creating one 2. (Some of) the conditions to clear one were either to reach it, to create a new one (due to the point above), or to step. This created weird behavior: let's say you Step Over a `bl` (thus creating a temporary breakpoint on `pc+4`), and you reached a regular breakpoint inside the `bl`. The temporary one would still be there: if you resumed, the emulation would still stop there, as a sort of Step Out. But, if before resuming, you made a Step, then it wouldn't do that. 3. The breakpoint widget had no idea concept of them, and will treat them as regular breakpoints. Also, they'll be shown only when the widget is updated in some other way, leading to more confusion. 4. Because only one breakpoint could exist per address, the creation of a temporary breakpoint on a top of a regular one would delete it and inherit its properties (e.g. being log-only). This could happen, for instance, if you Stepped Over a `bl` specifically, and pc+4 had a regular breakpoint. Now there can only be one temporary breakpoint, which is automatically cleared whenever emulation is paused. So, removing some manual clearing from 1., and removing the weird behavior of 2. As it is stored in a separate variable, it won't be seen at all depending on the function used (fixing 3., and removing some checks in other places), and it won't replace a regular breakpoint, instead simply having priority (fixing 4.).
This commit is contained in:
@ -1021,7 +1021,7 @@ void BranchWatchDialog::SetBreakpoints(bool break_on_hit, bool log_on_hit) const
|
||||
for (const QModelIndex& index : m_index_list_temp)
|
||||
{
|
||||
const u32 address = m_table_proxy->data(index, UserRole::ClickRole).value<u32>();
|
||||
breakpoints.Add(address, false, break_on_hit, log_on_hit, {});
|
||||
breakpoints.Add(address, break_on_hit, log_on_hit, {});
|
||||
}
|
||||
emit m_code_widget->BreakpointsChanged();
|
||||
m_code_widget->Update();
|
||||
@ -1111,11 +1111,9 @@ QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index)
|
||||
for (auto& breakpoints = m_system.GetPowerPC().GetBreakPoints();
|
||||
const QModelIndex& idx : m_index_list_temp)
|
||||
{
|
||||
if (const TBreakPoint* bp =
|
||||
breakpoints.GetBreakpoint(m_table_proxy->data(idx, UserRole::ClickRole).value<u32>()))
|
||||
if (const TBreakPoint* bp = breakpoints.GetRegularBreakpoint(
|
||||
m_table_proxy->data(idx, UserRole::ClickRole).value<u32>()))
|
||||
{
|
||||
if (bp->is_temporary)
|
||||
continue;
|
||||
if (bp->break_on_hit && bp->log_on_hit)
|
||||
{
|
||||
bp_both_count += 1;
|
||||
|
@ -289,7 +289,7 @@ void BreakpointDialog::accept()
|
||||
return;
|
||||
}
|
||||
|
||||
m_parent->AddBP(address, false, do_break, do_log, condition);
|
||||
m_parent->AddBP(address, do_break, do_log, condition);
|
||||
}
|
||||
else
|
||||
{
|
||||
|
@ -443,8 +443,8 @@ void BreakpointWidget::OnEditBreakpoint(u32 address, bool is_instruction_bp)
|
||||
{
|
||||
if (is_instruction_bp)
|
||||
{
|
||||
auto* dialog =
|
||||
new BreakpointDialog(this, m_system.GetPowerPC().GetBreakPoints().GetBreakpoint(address));
|
||||
auto* dialog = new BreakpointDialog(
|
||||
this, m_system.GetPowerPC().GetBreakPoints().GetRegularBreakpoint(address));
|
||||
dialog->setAttribute(Qt::WA_DeleteOnClose, true);
|
||||
SetQWidgetWindowDecorations(dialog);
|
||||
dialog->exec();
|
||||
@ -602,14 +602,13 @@ void BreakpointWidget::OnItemChanged(QTableWidgetItem* item)
|
||||
|
||||
void BreakpointWidget::AddBP(u32 addr)
|
||||
{
|
||||
AddBP(addr, false, true, true, {});
|
||||
AddBP(addr, true, true, {});
|
||||
}
|
||||
|
||||
void BreakpointWidget::AddBP(u32 addr, bool temp, bool break_on_hit, bool log_on_hit,
|
||||
const QString& condition)
|
||||
void BreakpointWidget::AddBP(u32 addr, bool break_on_hit, bool log_on_hit, const QString& condition)
|
||||
{
|
||||
m_system.GetPowerPC().GetBreakPoints().Add(
|
||||
addr, temp, break_on_hit, log_on_hit,
|
||||
addr, break_on_hit, log_on_hit,
|
||||
!condition.isEmpty() ? Expression::TryParse(condition.toUtf8().constData()) : std::nullopt);
|
||||
|
||||
emit BreakpointsChanged();
|
||||
@ -619,7 +618,7 @@ void BreakpointWidget::AddBP(u32 addr, bool temp, bool break_on_hit, bool log_on
|
||||
void BreakpointWidget::EditBreakpoint(u32 address, int edit, std::optional<QString> string)
|
||||
{
|
||||
TBreakPoint bp;
|
||||
const TBreakPoint* old_bp = m_system.GetPowerPC().GetBreakPoints().GetBreakpoint(address);
|
||||
const TBreakPoint* old_bp = m_system.GetPowerPC().GetBreakPoints().GetRegularBreakpoint(address);
|
||||
bp.is_enabled = edit == ENABLED_COLUMN ? !old_bp->is_enabled : old_bp->is_enabled;
|
||||
bp.log_on_hit = edit == LOG_COLUMN ? !old_bp->log_on_hit : old_bp->log_on_hit;
|
||||
bp.break_on_hit = edit == BREAK_COLUMN ? !old_bp->break_on_hit : old_bp->break_on_hit;
|
||||
|
@ -34,7 +34,7 @@ public:
|
||||
~BreakpointWidget();
|
||||
|
||||
void AddBP(u32 addr);
|
||||
void AddBP(u32 addr, bool temp, bool break_on_hit, bool log_on_hit, const QString& condition);
|
||||
void AddBP(u32 addr, 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, const QString& condition = {});
|
||||
void AddRangedMBP(u32 from, u32 to, bool do_read = true, bool do_write = true, bool do_log = true,
|
||||
|
@ -382,10 +382,11 @@ void CodeViewWidget::Update(const Core::CPUThreadGuard* guard)
|
||||
if (ins == "blr")
|
||||
ins_item->setForeground(dark_theme ? QColor(0xa0FFa0) : Qt::darkGreen);
|
||||
|
||||
if (debug_interface.IsBreakpoint(addr))
|
||||
const TBreakPoint* bp = power_pc.GetBreakPoints().GetRegularBreakpoint(addr);
|
||||
if (bp != nullptr)
|
||||
{
|
||||
auto icon = Resources::GetThemeIcon("debugger_breakpoint").pixmap(QSize(rowh - 2, rowh - 2));
|
||||
if (!power_pc.GetBreakPoints().IsBreakPointEnable(addr))
|
||||
if (!bp->is_enabled)
|
||||
{
|
||||
QPixmap disabled_icon(icon.size());
|
||||
disabled_icon.fill(Qt::transparent);
|
||||
|
@ -455,7 +455,6 @@ void CodeWidget::Step()
|
||||
auto& power_pc = m_system.GetPowerPC();
|
||||
PowerPC::CoreMode old_mode = power_pc.GetMode();
|
||||
power_pc.SetMode(PowerPC::CoreMode::Interpreter);
|
||||
power_pc.GetBreakPoints().ClearAllTemporary();
|
||||
cpu.StepOpcode(&sync_event);
|
||||
sync_event.WaitFor(std::chrono::milliseconds(20));
|
||||
power_pc.SetMode(old_mode);
|
||||
@ -482,8 +481,7 @@ void CodeWidget::StepOver()
|
||||
if (inst.LK)
|
||||
{
|
||||
auto& breakpoints = m_system.GetPowerPC().GetBreakPoints();
|
||||
breakpoints.ClearAllTemporary();
|
||||
breakpoints.Add(m_system.GetPPCState().pc + 4, true);
|
||||
breakpoints.SetTemporary(m_system.GetPPCState().pc + 4);
|
||||
cpu.SetStepping(false);
|
||||
Core::DisplayMessage(tr("Step over in progress...").toStdString(), 2000);
|
||||
}
|
||||
@ -519,12 +517,9 @@ void CodeWidget::StepOut()
|
||||
|
||||
auto& power_pc = m_system.GetPowerPC();
|
||||
auto& ppc_state = power_pc.GetPPCState();
|
||||
auto& breakpoints = power_pc.GetBreakPoints();
|
||||
{
|
||||
Core::CPUThreadGuard guard(m_system);
|
||||
|
||||
breakpoints.ClearAllTemporary();
|
||||
|
||||
PowerPC::CoreMode old_mode = power_pc.GetMode();
|
||||
power_pc.SetMode(PowerPC::CoreMode::Interpreter);
|
||||
|
||||
|
Reference in New Issue
Block a user