diff --git a/Source/Core/Core/Debugger/CodeTrace.cpp b/Source/Core/Core/Debugger/CodeTrace.cpp index a6411acc14..9036213324 100644 --- a/Source/Core/Core/Debugger/CodeTrace.cpp +++ b/Source/Core/Core/Debugger/CodeTrace.cpp @@ -189,7 +189,6 @@ AutoStepResults CodeTrace::AutoStepping(const Core::CPUThreadGuard& guard, bool stop_condition = HitType::ACTIVE; auto& power_pc = guard.GetSystem().GetPowerPC(); - power_pc.GetBreakPoints().ClearAllTemporary(); using clock = std::chrono::steady_clock; clock::time_point timeout = clock::now() + std::chrono::seconds(4); diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.cpp b/Source/Core/Core/Debugger/PPCDebugInterface.cpp index 72cfd05b2c..786f95bfa7 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.cpp +++ b/Source/Core/Core/Debugger/PPCDebugInterface.cpp @@ -506,7 +506,7 @@ void PPCDebugInterface::SetPC(u32 address) void PPCDebugInterface::RunTo(u32 address) { auto& breakpoints = m_system.GetPowerPC().GetBreakPoints(); - breakpoints.Add(address, true); + breakpoints.SetTemporary(address); m_system.GetCPU().SetStepping(false); } diff --git a/Source/Core/Core/HW/CPU.cpp b/Source/Core/Core/HW/CPU.cpp index 583693c21a..7a161b0bf7 100644 --- a/Source/Core/Core/HW/CPU.cpp +++ b/Source/Core/Core/HW/CPU.cpp @@ -244,6 +244,8 @@ bool CPUManager::SetStateLocked(State s) { if (m_state == State::PowerDown) return false; + if (s == State::Stepping) + m_system.GetPowerPC().GetBreakPoints().ClearTemporary(); m_state = s; return true; } diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index 24b15750e0..565dc961e5 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -36,14 +37,17 @@ bool BreakPoints::IsBreakPointEnable(u32 address) const return bp != nullptr && bp->is_enabled; } -bool BreakPoints::IsTempBreakPoint(u32 address) const +const TBreakPoint* BreakPoints::GetBreakpoint(u32 address) const { - return std::any_of(m_breakpoints.begin(), m_breakpoints.end(), [address](const auto& bp) { - return bp.address == address && bp.is_temporary; - }); + // Give priority to the temporary breakpoint (it could be in the same address of a regular + // breakpoint that doesn't break) + if (m_temp_breakpoint && m_temp_breakpoint->address == address) + return &*m_temp_breakpoint; + + return GetRegularBreakpoint(address); } -const TBreakPoint* BreakPoints::GetBreakpoint(u32 address) const +const TBreakPoint* BreakPoints::GetRegularBreakpoint(u32 address) const { auto bp = std::find_if(m_breakpoints.begin(), m_breakpoints.end(), [address](const auto& bp_) { return bp_.address == address; }); @@ -59,21 +63,18 @@ BreakPoints::TBreakPointsStr BreakPoints::GetStrings() const TBreakPointsStr bp_strings; for (const TBreakPoint& bp : m_breakpoints) { - if (!bp.is_temporary) - { - std::ostringstream ss; - ss.imbue(std::locale::classic()); - ss << fmt::format("${:08x} ", bp.address); - if (bp.is_enabled) - ss << "n"; - if (bp.log_on_hit) - ss << "l"; - if (bp.break_on_hit) - ss << "b"; - if (bp.condition) - ss << "c " << bp.condition->GetText(); - bp_strings.emplace_back(ss.str()); - } + std::ostringstream ss; + ss.imbue(std::locale::classic()); + ss << fmt::format("${:08x} ", bp.address); + if (bp.is_enabled) + ss << "n"; + if (bp.log_on_hit) + ss << "l"; + if (bp.break_on_hit) + ss << "b"; + if (bp.condition) + ss << "c " << bp.condition->GetText(); + bp_strings.emplace_back(ss.str()); } return bp_strings; @@ -102,7 +103,6 @@ void BreakPoints::AddFromStrings(const TBreakPointsStr& bp_strings) std::getline(iss, condition); bp.condition = Expression::TryParse(condition); } - bp.is_temporary = false; Add(std::move(bp)); } } @@ -117,12 +117,12 @@ void BreakPoints::Add(TBreakPoint bp) m_breakpoints.emplace_back(std::move(bp)); } -void BreakPoints::Add(u32 address, bool temp) +void BreakPoints::Add(u32 address) { - BreakPoints::Add(address, temp, true, false, std::nullopt); + BreakPoints::Add(address, true, false, std::nullopt); } -void BreakPoints::Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit, +void BreakPoints::Add(u32 address, bool break_on_hit, bool log_on_hit, std::optional condition) { // Check for existing breakpoint, and overwrite with new info. @@ -132,7 +132,6 @@ void BreakPoints::Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit TBreakPoint bp; // breakpoint settings bp.is_enabled = true; - bp.is_temporary = temp; bp.break_on_hit = break_on_hit; bp.log_on_hit = log_on_hit; bp.address = address; @@ -151,6 +150,20 @@ void BreakPoints::Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit m_system.GetJitInterface().InvalidateICache(address, 4, true); } +void BreakPoints::SetTemporary(u32 address) +{ + TBreakPoint bp; // breakpoint settings + bp.is_enabled = true; + bp.break_on_hit = true; + bp.log_on_hit = false; + bp.address = address; + bp.condition = std::nullopt; + + m_temp_breakpoint.emplace(std::move(bp)); + + m_system.GetJitInterface().InvalidateICache(address, 4, true); +} + bool BreakPoints::ToggleBreakPoint(u32 address) { if (!Remove(address)) @@ -195,22 +208,15 @@ void BreakPoints::Clear() } m_breakpoints.clear(); + ClearTemporary(); } -void BreakPoints::ClearAllTemporary() +void BreakPoints::ClearTemporary() { - auto bp = m_breakpoints.begin(); - while (bp != m_breakpoints.end()) + if (m_temp_breakpoint) { - if (bp->is_temporary) - { - m_system.GetJitInterface().InvalidateICache(bp->address, 4, true); - bp = m_breakpoints.erase(bp); - } - else - { - ++bp; - } + m_system.GetJitInterface().InvalidateICache(m_temp_breakpoint->address, 4, true); + m_temp_breakpoint.reset(); } } diff --git a/Source/Core/Core/PowerPC/BreakPoints.h b/Source/Core/Core/PowerPC/BreakPoints.h index 6c60b8a24e..9b07fb53c0 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.h +++ b/Source/Core/Core/PowerPC/BreakPoints.h @@ -20,7 +20,6 @@ struct TBreakPoint { u32 address = 0; bool is_enabled = false; - bool is_temporary = false; bool log_on_hit = false; bool break_on_hit = false; std::optional condition; @@ -68,14 +67,21 @@ public: bool IsAddressBreakPoint(u32 address) const; bool IsBreakPointEnable(u32 adresss) const; - bool IsTempBreakPoint(u32 address) const; + // Get the breakpoint in this address (for most purposes) const TBreakPoint* GetBreakpoint(u32 address) const; + // Get the breakpoint in this address (ignore temporary breakpoint, e.g. for editing purposes) + const TBreakPoint* GetRegularBreakpoint(u32 address) const; // Add BreakPoint. If one already exists on the same address, replace it. - void Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit, - std::optional condition); - void Add(u32 address, bool temp = false); + void Add(u32 address, bool break_on_hit, bool log_on_hit, std::optional condition); + void Add(u32 address); void Add(TBreakPoint bp); + // Add temporary breakpoint (e.g., Step Over, Run to Here) + // It can be on the same address of a regular breakpoint (it will have priority in this case) + // It's cleared whenever the emulation is paused for any reason + // (CPUManager::SetStateLocked(State::Paused)) + // TODO: Should it somehow force to resume emulation when called? + void SetTemporary(u32 address); bool ToggleBreakPoint(u32 address); bool ToggleEnable(u32 address); @@ -83,10 +89,11 @@ public: // Remove Breakpoint. Returns whether it was removed. bool Remove(u32 address); void Clear(); - void ClearAllTemporary(); + void ClearTemporary(); private: TBreakPoints m_breakpoints; + std::optional m_temp_breakpoint; Core::System& m_system; }; diff --git a/Source/Core/Core/PowerPC/PowerPC.cpp b/Source/Core/Core/PowerPC/PowerPC.cpp index e3f66d8895..e97db77fc7 100644 --- a/Source/Core/Core/PowerPC/PowerPC.cpp +++ b/Source/Core/Core/PowerPC/PowerPC.cpp @@ -270,9 +270,6 @@ void PowerPCManager::Init(CPUCore cpu_core) auto& memory = m_system.GetMemory(); m_ppc_state.iCache.Init(memory); m_ppc_state.dCache.Init(memory); - - if (Config::Get(Config::MAIN_ENABLE_DEBUGGING)) - m_breakpoints.ClearAllTemporary(); } void PowerPCManager::Reset() diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index f16afbf51f..4b6233678c 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -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(); - 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())) + if (const TBreakPoint* bp = breakpoints.GetRegularBreakpoint( + m_table_proxy->data(idx, UserRole::ClickRole).value())) { - if (bp->is_temporary) - continue; if (bp->break_on_hit && bp->log_on_hit) { bp_both_count += 1; diff --git a/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp b/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp index 1899b42168..ba7fe1feb5 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BreakpointDialog.cpp @@ -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 { diff --git a/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp b/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp index 3bed35c576..0bf6c5b29c 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp @@ -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 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; diff --git a/Source/Core/DolphinQt/Debugger/BreakpointWidget.h b/Source/Core/DolphinQt/Debugger/BreakpointWidget.h index 3e204b41c1..1689270d44 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointWidget.h +++ b/Source/Core/DolphinQt/Debugger/BreakpointWidget.h @@ -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, diff --git a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp index 36e7ade321..6e7e12dcd2 100644 --- a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp @@ -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); diff --git a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp index 361f142c7c..36d3d92284 100644 --- a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp @@ -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);