Add BreakpointManager::ChangeBreakPointAddress, use it from the ImDebugger

ChangeBreakPointAddress() moves the breakpoint keeping its action, condition and
log format, invalidates both ends, refuses to land on an existing breakpoint,
and resets the hit count since it belonged to the old address. The edit form now
works on a copy of the address and commits on deactivation rather than per
keystroke, so typing one address doesn't churn through every prefix of it.

The breakpoint edit form assigned straight to bp.addr and then invalidated the
icache at "bp.addr - 4, 8" - which by then is the *new* address - need both.

Also clear the selection after Delete in both edit forms - the reference into
the vector is dangling from that point on. Harmless today, but only because
nothing happens to touch it below.

Covered by a new Breakpoints unit test (verified to fail without the duplicate
check and the hit reset).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
This commit is contained in:
Henrik Rydgård
2026-08-17 00:28:39 +02:00
co-authored by Claude Opus 5
parent d4500d9353
commit 0c510ca62e
4 changed files with 112 additions and 16 deletions
+35
View File
@@ -210,6 +210,41 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, BreakAction action) {
}
}
// Relocates a breakpoint the user already set, rather than making them delete and re-add it.
// Returns false and changes nothing if there's no (non-temporary) breakpoint at oldAddr, or if
// newAddr already has one of its own.
//
// Refusing the duplicate matters: ExecBreakPoint() goes through FindBreakpoint(), which returns
// only one entry per address, so a second breakpoint at the same address is invisible - and if the
// one that gets found has a condition that evaluates false, the other silently never fires either.
bool BreakpointManager::ChangeBreakPointAddress(u32 oldAddr, u32 newAddr) {
if (oldAddr == newAddr)
return true;
// Match non-temporary only - a temp breakpoint belongs to an in-flight step, not to the user.
size_t bp = FindBreakpoint(oldAddr, true, false);
if (bp == INVALID_BREAKPOINT)
return false;
// ...but collide against any breakpoint at all, temporary ones included.
if (FindBreakpoint(newAddr) != INVALID_BREAKPOINT)
return false;
if (newAddr & 3) {
WARN_LOG(Log::Debugger, "Breakpoint moved to %08x will not be effective - unaligned address.", newAddr);
}
breakPoints_[bp].addr = newAddr;
// The count belonged to the old address, so carrying it over would just be misleading.
breakPoints_[bp].numHits = 0;
// Both ends need invalidating, not just the new one: under a JIT the old address still has a
// compiled-in check that now matches no breakpoint, and the new address has none at all.
currentMIPS->InvalidateICacheRangeDeferred(oldAddr - 4, 8);
currentMIPS->InvalidateICacheRangeDeferred(newAddr - 4, 8);
System_Notify(SystemNotification::DISASSEMBLY);
return true;
}
// This is not actually called, currently.
void BreakpointManager::ClearAllBreakPoints() {
if (!anyBreakPoints_)