preserve-emulation/ppsspp
1
0 Fork 0
mirror of https://github.com/hrydgard/ppsspp.git synced 2026-10-21 02:09:05 +02:00
Code Issues Packages Projects Releases Wiki Activity

Debugger: Fix breakpoints swallowed or reported twice when resuming

Browse Source 
The skip-first marker suppressed everything at the address you resume or
step from. That's needed for the pause, or you could never get off a
breakpoint, but it also dropped the log and hit count. So a log breakpoint
you'd stepped onto, or were parked on for some other reason, never logged
when execution moved on.

Now the skip only drops the pause, unless the breakpoint at that address is
the one that stopped us, which has already logged and counted this
execution. The check moves into the Exec* functions instead of being
repeated in the interpreter, IR interpreter and three JITs, where the
register breakpoint path only applied it to the pause and so logged twice
when stepping off a log+pause one. The marker gets an explicit valid flag,
since address 0 meant "none", and Core_Break() clears it.

Based on the earlier cleanup-skip-first branch, reduced to one marker plus
a flag. Checked over the WebSocket debugger on the interpreter, IR, x86 JIT
and JIT-IR: log breakpoints on a parked and a stepped-onto address each log
once, and stepping off a log+pause one doesn't report it again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Henrik RydgårdandClaude Opus 5.5 committed 2026-10-16 18:16:33 +02:00
1 parent ab0fa898ae
commit 6a458bf148
7 files changed
+142 -41
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

Core/Debugger/Breakpoints.cpp
+67 -22
View File
@@ -61,9 +61,10 @@ BreakAction MemCheck::Apply(u32 addr, bool write, int size, u32 pc) {
return BREAK_ACTION_NONE;
}
BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc, const char *reason) {
BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc, const char *reason, bool skipPause) {
// Conditions have always already been checked if we get here.
Log(addr, write, size, pc, reason);
const BreakAction action = skipPause ? BreakAction(this->action & ~BREAK_ACTION_PAUSE) : this->action;
BreakpointHit hit;
if (WebSocketDebuggerHasClients() || (action & BREAK_ACTION_PAUSE)) {
@@ -358,11 +359,15 @@ BreakAction BreakpointManager::ExecBreakPoint(u32 addr) {
BreakAction result = BREAK_ACTION_NONE;
BreakpointHit hit;
const bool skipPause = SkipPauseAt(addr);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT) {
if (bp != INVALID_BREAKPOINT && !SkipReportAt(addr)) {
BreakPoint &info = breakPoints_[bp];
const BreakAction action = info.action;
BreakAction action = info.action;
if (skipPause) {
action = BreakAction(action & ~BREAK_ACTION_PAUSE);
}
bool condPassed = true;
if (info.hasCond)
@@ -404,7 +409,7 @@ BreakAction BreakpointManager::ExecBreakPoint(u32 addr) {
}
}
if (tempBreakPoint_.valid && tempBreakPoint_.addr == addr) {
if (tempBreakPoint_.valid && tempBreakPoint_.addr == addr && !skipPause) {
// The condition, when set, narrows down which hit counts - to one thread for a step
// ("threadid == ..."), or to a later frame for run-to-cursor ("flipcount > ..."). A hit that
// fails it leaves the breakpoint armed, so the next one gets a chance.
@@ -418,6 +423,9 @@ BreakAction BreakpointManager::ExecBreakPoint(u32 addr) {
// hit stays kind None when only the temporary breakpoint fired - there's no user
// breakpoint to describe in that case, just a step completing.
Core_Break(BreakReason::CpuBreakpoint, addr, hit.kind != BreakpointKind::None ? &hit : nullptr);
if (hit.kind != BreakpointKind::None) {
NoteStoppedOnReport(addr);
}
System_Notify(SystemNotification::DISASSEMBLY);
}
@@ -577,7 +585,7 @@ MemCheck *BreakpointManager::FindMemCheckInRange(u32 address, int size) {
BreakAction BreakpointManager::ExecMemCheck(u32 address, bool write, int size, u32 pc, const char *reason)
{
if (!anyMemChecks_)
if (!anyMemChecks_ || SkipReportAt(pc))
return BREAK_ACTION_NONE;
MemCheck *check = FindMemCheckInRange(address, size);
if (check) {
@@ -586,7 +594,11 @@ BreakAction BreakpointManager::ExecMemCheck(u32 address, bool write, int size, u
return applyAction;
MemCheck copy = *check;
return copy.Action(address, write, size, pc, reason);
const BreakAction result = copy.Action(address, write, size, pc, reason, SkipPauseAt(pc));
if (result & BREAK_ACTION_PAUSE) {
NoteStoppedOnReport(pc);
}
return result;
}
return BREAK_ACTION_NONE;
}
@@ -594,6 +606,11 @@ BreakAction BreakpointManager::ExecMemCheck(u32 address, bool write, int size, u
BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc) {
// Note: currently, we don't check "on changed" for HLE (ExecMemCheck.)
// We'd need to more carefully specify memory changes in HLE for that.
// pc may be the delay slot, but the skip is keyed on where execution resumes, which is the branch.
const u32 execPc = currentMIPS->pc;
if (SkipReportAt(execPc)) {
return BREAK_ACTION_NONE;
}
int size = MIPSAnalyst::OpMemoryAccessSize(pc);
if (size == 0 && MIPSAnalyst::OpHasDelaySlot(pc)) {
// This means that the delay slot is what tripped us.
@@ -619,7 +636,11 @@ BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc) {
return applyAction;
MemCheck copy = *check;
return copy.Action(address, write, size, pc, "CPU");
const BreakAction result = copy.Action(address, write, size, pc, "CPU", SkipPauseAt(execPc));
if (result & BREAK_ACTION_PAUSE) {
NoteStoppedOnReport(execPc);
}
return result;
}
}
return BREAK_ACTION_NONE;
@@ -752,23 +773,28 @@ BreakAction BreakpointManager::ExecRegBreakpoint(int reg, u32 pc) {
if (info.hasCond && !info.cond.Evaluate())
return BREAK_ACTION_NONE;
if (SkipReportAt(pc)) {
return BREAK_ACTION_NONE;
}
const BreakAction result = SkipPauseAt(pc) ? BreakAction(info.result & ~BREAK_ACTION_PAUSE) : info.result;
++info.numHits;
BreakpointHit hit;
if (WebSocketDebuggerHasClients() || (info.result & BREAK_ACTION_PAUSE)) {
if (WebSocketDebuggerHasClients() || (result & BREAK_ACTION_PAUSE)) {
hit.kind = BreakpointKind::Register;
hit.pc = pc;
hit.address = pc;
hit.reg = reg;
hit.numHits = info.numHits;
hit.logged = (info.result & BREAK_ACTION_LOG) != 0;
hit.paused = (info.result & BREAK_ACTION_PAUSE) != 0;
hit.logged = (result & BREAK_ACTION_LOG) != 0;
hit.paused = (result & BREAK_ACTION_PAUSE) != 0;
if (info.hasCond)
hit.condition = info.cond.expressionString;
WebSocketNotifyBreakpointHit(hit);
}
if (info.result & BREAK_ACTION_LOG) {
if (result & BREAK_ACTION_LOG) {
if (info.logFormat.empty()) {
NOTICE_LOG(Log::JIT, "BKP reg write r%d, PC=%08x (%s)", reg, pc, g_symbolMap->GetDescription(pc).c_str());
} else {
@@ -777,28 +803,47 @@ BreakAction BreakpointManager::ExecRegBreakpoint(int reg, u32 pc) {
NOTICE_LOG(Log::JIT, "BKP reg write r%d, PC=%08x: %s", reg, pc, formatted.c_str());
}
}
if ((info.result & BREAK_ACTION_PAUSE) && g_breakpoints.CheckSkipFirst() != pc) {
if (result & BREAK_ACTION_PAUSE) {
Core_Break(BreakReason::RegBreakpoint, pc, &hit);
NoteStoppedOnReport(pc);
}
return info.result;
return result;
}
void BreakpointManager::ClearSkipFirst() {
breakSkipFirstAt_ = 0;
breakSkipFirstTicks_ = 0;
skipFirstValid_ = false;
stoppedOnReport_ = false;
}
void BreakpointManager::SetSkipFirst(u32 pc) {
breakSkipFirstAt_ = pc;
breakSkipFirstTicks_ = CoreTiming::GetTicks(currentMIPS);
const u64 ticks = CoreTiming::GetTicks(currentMIPS);
// A step-over arms its temporary breakpoint and then resumes, which comes through here again.
if (skipFirstValid_ && skipFirstAt_ == pc && skipFirstTicks_ == ticks) {
return;
}
skipFirstValid_ = true;
skipFirstAt_ = pc;
skipFirstTicks_ = ticks;
// Consumed here, since a plain step stops again without going through Core_Break().
skipFirstReport_ = stoppedOnReport_ && stoppedOnReportAt_ == pc;
if (skipFirstReport_) {
stoppedOnReport_ = false;
}
}
u32 BreakpointManager::CheckSkipFirst() const {
u32 pc = breakSkipFirstAt_;
if (breakSkipFirstTicks_ == CoreTiming::GetTicks(currentMIPS))
return pc;
return 0;
bool BreakpointManager::SkipPauseAt(u32 pc) const {
return skipFirstValid_ && skipFirstAt_ == pc && skipFirstTicks_ == CoreTiming::GetTicks(currentMIPS);
}
bool BreakpointManager::SkipReportAt(u32 pc) const {
return skipFirstReport_ && SkipPauseAt(pc);
}
// Call after Core_Break(), which clears it.
void BreakpointManager::NoteStoppedOnReport(u32 pc) {
stoppedOnReport_ = true;
stoppedOnReportAt_ = pc;
}
static MemCheck NotCached(MemCheck mc) {