step-over, step-out and run-until plant a one-shot breakpoint at the address
they want execution to return to. Keeping it in breakPoints_ alongside the
user's own meant the two kept colliding:
- Adding a log-only user breakpoint at the same address hijacked the temporary
one. AddBreakPoint() didn't match across temp-ness so both existed, and then
ChangeBreakPoint() looked up "the first enabled breakpoint at this address" -
a log-only breakpoint isn't enabled, so the temporary one won and had its
action overwritten to log-only. It lost PAUSE and the step never came back.
- RemoveBreakPoint() erased up to two entries per address to catch an
overlapping temporary one, so deleting either deleted both - including the
interpreter's cleanup path in CheckExecBreakpoints() taking the user's
breakpoint with it.
- ExecBreakPoint() handled one breakpoint per address, so with both at the same
address only one of them did anything: the step completed but the user's log
line never printed.
- Nothing dropped it when something *else* stopped us first, so an interrupted
step left a breakpoint armed at an address nobody was waiting for anymore,
which later fired as a phantom stop.
It's a single TempBreakPoint member now, invisible to the breakpoint lists and
untouched by user edits. One is enough: step over/out and cross-thread step into
all require the CPU to already be stepping and resume it immediately, so only
one can be in flight, and run-until now replaces rather than stacking (two
pending run-untils had no coherent meaning, and the loser stayed armed).
Behavior follows what other debuggers do. Both breakpoints at an address are
evaluated independently and their actions combine, so a log-only breakpoint
logs without stopping and still lets the step finish. Core_Break() drops the
temporary breakpoint on any stop, whatever the reason - the same way gdb deletes
its step-resume breakpoint and lldb discards the thread plan.
Two things to be careful of, both covered by the new TempBreakpoints test:
HasBreakPoints() has to account for it, or the interpreter's checked run loop
and the JIT skip breakpoint checking entirely and a step with no user
breakpoints set never returns; and IsAddressBreakPoint() (user-facing, for the
lists and disassembly markers) is now separate from NeedsBreakCheckAt() (what
the JIT frontends and interpreter ask), since only the latter should see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
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
AddLabel() won't overwrite an existing label. That's deliberate and right for
bulk import - a real ELF symbol name shouldn't lose to the analyzer's later
z_un_* - but wrong when someone is explicitly naming an address, so a second
hle.data.add at the same address silently kept the old name. The response echoed
the requested name back either way, so there was no sign anything had been
ignored.
Force the requested name in with SetLabelName() now, except when a function
starts at that address and owns the label - renaming that function isn't what
"label this data" should mean, and it would undo the care hle.data.remove takes
not to destroy it. Either way the response now reports the name the symbol
actually ended up with rather than the one that was asked for.
Also, in the ImDebugger memcheck edit form: the Enabled checkbox didn't mark the
memcheck as changed, and the condition combo marked it changed on every frame
the popup was open rather than when a condition was actually picked (Selectable
returns true only on click, BeginCombo stays true while open).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
DisassemblyManager used to fuse lui+addiu/load/store into single pseudo-
instructions ("li", fused loads/stores) for display. This only applied to a
handful of opcodes, complicated DisassemblyManager, and was the root cause of
a stepping bug: Core_PerformCPUStep's Into/Over cases treated stepSize as a
byte count, while the WebSocket cpu.stepInto handler computed it as an
instruction count (needed to step over a whole fused macro in one go) - so a
plain, non-fused stepInto silently executed zero instructions.
Removed the fusion logic entirely (DisassemblyMacro, DISTYPE_MACRO) - every
disassembly line is now exactly one 4-byte instruction. With that,
"how many instructions does this line span" is always 1, so the
getInstructionSizeAt() byte-size queries in the legacy Windows and ImGui
debuggers are gone too; step requests just pass 1. Core_RequestCPUStep's
stepSize is now consistently in instructions everywhere.
Also fixes the PPSSPPHeadless build, broken since 0ed1f3e added
OpenWebDebugger() (which calls System_LaunchUrl) without a headless stub.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
