observability: window traps that read as results, and a per-game watch list - #176
Merged
Conversation
…h list
Three related defects in the interp observability rings. All three share a
failure mode that is worse than an error: the tool reports an empty result
that is indistinguishable from a real finding. "total: 0" reads as proof the
events never happened.
1. callret_watch armed on `lo != 0`, making address 0 an off switch rather
than a legal window floor. `lo=0 hi=0x200000` — the obvious spelling of
"watch the whole address space" — replied ok and recorded nothing.
Measured on one build, identical but for the floor:
lo=0x00000000 hi=0x00200000 -> total 0
lo=0x00000004 hi=0x00200000 -> total 468732
Four bytes of window between an empty ring and half a million call
resolutions. This cost real time during beads-eio.3.59: an
interp->compiled call route was briefly believed absent when the ring was
simply never on.
Arm on a NON-EMPTY window (hi > lo) instead. `lo=0` is now an ordinary
floor. Note that `{"lo":"0"}` was the DOCUMENTED disarm spelling
(TCP_COMMANDS.md), so that contract is kept explicitly rather than broken:
lo=0 with no hi still disarms, and now says so in the reply. Added
`disarm=true` as the unambiguous spelling. A `lo` with no `hi` is refused
outright instead of silently inheriting the previous ceiling (usually 0,
i.e. an empty window) — that was the second way to arm nothing by accident.
2. s3_smear_watch had the same `lo`-nonzero gate, so lo=0 with a real hi
recorded nothing while answering ok. Same fix. It already cleared `hi`
whenever `hi` was omitted, so `{"lo":"0"}` naturally lands on an empty
window and still disarms — no special case needed there.
Both commands now report `armed`, and callret_watch's dump carries `armed`
plus the active window, so an empty read can always be told apart from an
unarmed one.
3. The xprobe JAL/JALR watched-target list was six guest addresses from two
specific titles COMPILED INTO the shared runtime (an MMX6 card-load flow
and a Tomba target). Every other game therefore watched nothing, silently:
the `watched` dump came back empty and gave no hint that the build was
incapable of watching anything. Same defect class as beads-eio.3.21, and
the direct reason the JAL call site was unusable while measuring
beads-eio.3.59 — only the JALR callret ring could be used there.
The list is now configured, never compiled in: `xprobe_watch` TCP command
to get/set, or PSX_XPROBE_WATCH in the environment so targets are watched
from instruction zero without a rebuild. Default EMPTY — no title is
privileged — and a read with nothing watched says so rather than looking
like a negative result. Matched on the normalised address so KSEG and
physical forms both work.
Verified against Ape Escape, headless, RelWithDebInfo:
- PSX_XPROBE_WATCH=0x8002C0E4 -> watching that target from boot
- xprobe_watch targets=<two addrs> -> count 2; the `watched` dump then
carries 41 JALR records for 0x8002C0E4 and 21 for 0x8002C14C, with full
frame/cycle/src/sp/ra context. This title could previously watch NOTHING.
- xprobe_watch targets= (empty) -> count 0 plus an explanatory note
- callret_watch lo=0 hi=0x200000 -> armed true, total 77484
- callret_watch lo=0 -> disarmed, legacy spelling named
- callret_watch lo=0x40000 -> refused, window left unchanged
- callret_watch disarm=1 -> disarmed
- s3_smear_watch lo=0 hi=0x200000 -> armed true (previously silent)
- pre-fix build, callret lo=0 hi=0x200000 -> plain ok, total 0 (the trap)
TCP_COMMANDS.md regenerated for the new command. Note the regen also picks
up rows that had drifted on master beforehand: `--check` already failed on
pristine master (index said 292, the servers register 304), which is why the
count line jumps 292 -> 305 rather than by one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related defects in the interpreter observability rings, all found while measuring #174. They share a failure mode that is worse than an error: the tool returns an empty result that is indistinguishable from a real finding.
total: 0reads as proof the events never happened.1.
callret_watch— address 0 was a silent off switchThe ring armed on
lo != 0, solo=0disabled recording rather than meaning "start at address 0".lo=0 hi=0x200000— the obvious way to say "watch everything" — repliedokand recorded nothing.Measured on one build, identical but for the floor:
lo=0x00000000 hi=0x00200000lo=0x00000004 hi=0x00200000Four bytes of window between an empty ring and half a million call resolutions. This cost real time: during the #174 measurement an interp→compiled call route was briefly believed absent when the ring was simply never on.
Now armed on a non-empty window (
hi > lo), solo=0is an ordinary floor.{"lo":"0"}was the documented disarm spelling, so that contract is kept rather than broken — it still disarms, and now says so in the reply. Addeddisarm=trueas the unambiguous spelling. Alowith nohiis now refused instead of silently inheriting the previous ceiling (usually0) — that was the second way to arm nothing by accident.2.
s3_smear_watch— same trapSame
lo-nonzero gate, same silent nothing. Same fix. It already clearedhiwhen omitted, so{"lo":"0"}naturally lands on an empty window and still disarms — no special case needed.Both commands now report
armed, andcallret_watch's dump carriesarmedplus the active window, so an empty read can always be told apart from an unarmed one.3.
xprobewatched-target list was one game's addresses, compiled inThe JAL/JALR watched-target filter was six guest addresses from two specific titles (an MMX6 card-load flow and a Tomba target) hardcoded into the shared runtime. Every other game therefore watched nothing, silently — the
watcheddump came back empty with no hint that the build was incapable of watching anything.Same defect class as
beads-eio.3.21, and the direct reason the JAL call site was unusable while measuring #174: only the JALRcallretring could be used.The list is now configured, never compiled in:
{"cmd":"xprobe_watch","targets":"0x...,0x..."}to set, no args to readPSX_XPROBE_WATCHin the environment, so targets are watched from instruction zero without a rebuildDefault is empty — no title is privileged — and a read with nothing watched says so rather than looking like a negative result. Matched on the normalised address so KSEG and physical forms both work.
Verification
Ape Escape, headless, RelWithDebInfo:
PSX_XPROBE_WATCH=0x8002C0E4→ watching that target from bootxprobe_watch targets=<two addrs>→count: 2, and thewatcheddump then carries 41 JALR records for0x8002C0E4and 21 for0x8002C14Cwith full frame/cycle/src/sp/ra context. This title could previously watch nothing at all.xprobe_watch targets=(empty) →count: 0plus an explanatory notecallret_watch lo=0 hi=0x200000→armed: true, total 77,484callret_watch lo=0→ disarmed, legacy spelling namedcallret_watch lo=0x40000→ refused, window left unchangedcallret_watch disarm=1→ disarmeds3_smear_watch lo=0 hi=0x200000→armed: true(previously silent)callret_watch lo=0 hi=0x200000→ plainok, total 0 — the trap reproducedNote on the doc diff
TCP_COMMANDS.mdis regenerated for the new command. The regen also picks up rows that had already drifted on master:tools/gen_tcp_commands.py --checkfailed on pristine master before this branch (index said 292, the servers register 304). That is why the count line jumps 292 → 305 rather than by one. No line-ending churn (verified).Tracked as
beads-eio.3.60.