M-28: watch-menu text (D150/D151) + audio-lock deadlock mitigation (D152)

D150 (src/str.c) — NULL-tolerant strcpy/strncpy/strcat under #ifdef PORT.
The watch BRIEF/OBJECTIVES pages build text with strcat(buf, langGet(id));
langGet() returns NULL on PC for a string bank the in-level flow never
loaded, and str.c's primitives are __nonnull__ builtins so a plain param
NULL check is optimised away — the guard launders the ptr through an empty
asm (GE_IS_NULL). NULL src -> empty string instead of an AV crash.

D151 (src/bondtypes.h) — watch data (objectives / mission background / M /
Q / Moneypenny) all rendered blank. propDef types 35 (WatchMenuObjectiveText)
and 23 (ObjectiveStart) store a plain s32 lang slot id in their 3rd word;
the decomp structs decode it as [u16 reserved][u16 text] reading text at
offset 0xA, which only works because the id is in the low 16 bits of a
big-endian word. d88_propdefs.py bswap32's the whole word -> text @0xA
reads 0 -> langGet(0) -> NULL. Fix: text is a full u32 at the word offset
under #ifdef PORT (N64 layout kept under #else). No converter change, no
sidecar regen. This is the D143/D150 "briefing text renders blank" root cause.

D152 (port/src/libultra.c) — mission-failed / objective-failed permanent
black screen. The D147 recursive-mutex behind osSetIntMask can be leaked
(libaudio has unbalanced / early-return mask calls; a transient thread can
acquire OS_IM_NONE and exit) -> mainThread + amMain both block forever in
alEvtq* -> gfx pipeline stalls. Live gdb.txt from the user's session
confirmed the deadlock (mainThread in the mission-fail audio fade-out
sndSetScalerApplyVolumeAllSfxSlot -> alEvtqPostEvent). Mitigation: model
the mask as a self-healing logical lock (owner/depth under s_imMx + condvar)
— normal microsecond critical sections behave as before, but a waiter
blocked > 2s concludes the holder leaked it, logs the stale owner + the
stealing caller's return address, and steals the section. Worst case is a
~2s audio hiccup + a log line naming the leak, instead of a hung game.
Proper narrow fix (dedicated ALEventQueue lock) still owed.

D153 investigated and dismissed — the "-level_09 crashes at frame ~900"
seen while regression-testing was D117/D134 host-scheduling flakiness under
heavy concurrent build+run load; on a quiet machine -level_09 runs past
frame 2400 clean with all three changes applied.

Build: green (ntsc-final). Golden framediff not run clean (same machine-load
flakiness); one quiet -level_09 run = 2400+ frames, 0 heartbeats.
This commit is contained in:
James Dansereau committed 2026-08-30 20:25:02 -04:00
1 parent 81a0c0d253
commit 57e24a374b
7 files changed
+289 -20

No files matched your search

+39
View File
@@ -1,5 +1,44 @@
All of these items should be completed once the main original game is playable from start to finish on PC with no crashing or major issues. All of these items should be completed once the main original game is playable from start to finish on PC with no crashing or major issues.
---
## Playtest defects — logged M-28 (2026-08-30), not yet worked
User-reported during free playtest. Suggested priority order below (cheap
input fixes first, then the texture correctness bug, then the filtering
feature).
- **B1 — texture filtering / "interlaced" shimmer (feature).** Whole game
has an interlaced/shimmering look. Want N64 3-point-filter emulation +
bilinear/trilinear handling like the Nightdive / Quake-remaster ports.
MVP: one good default filtering mode in `port/fast3d/` (3-point emu
shader or clean bilinear). Long-term: full toggle set (see "PC graphics
settings" → Quality + stretch goals below). Biggest of the four.
- **B2 — glitchy textures / wrong colors (correctness bug).** Some
textures render with wrong colors / garbage — **very noticeable on
Depot (`-level_30`)**, present in many other spots too. Likely a
texture-decode/format bug (CI/TLUT, RZ decode `port/src/rzdecomp.c`, or
the `import_texture_*` path in `gfx_pc.cpp`). Distinct from B1. Needs
investigation — could be a real fidelity blocker, not cosmetic-parked.
- **B3 — RMB aim far too mouse-sensitive.** Manual-aim (RMB/LShift) mouse
sensitivity is way too high. `Input.MouseAimSpeed` (default 50) still
overshoots badly. Quick fix: lower default + recheck the aim-band curve
in `port/src/input.c` (the 61..80 stick push added M-24). Port-only.
- **B4 — menu crosshair-cursor screens: PC input is wrong.** Menu screens
that use the crosshair cursor don't behave like a PC pointer. The
original front-end was PC-style "select a file with the mouse" — menu
input should be a **separate mode from in-game input**, not the same
analog-stick emulation. Needs a dedicated menu-pointer path in
`port/src/input.c` (absolute cursor → menu hit-test) rather than
feeding synthetic stick deltas. Port-only.
- **B5 — no game save on level completion.** On the OG cartridge, finishing
a level writes progress to EEPROM (unlocks next mission, records
time/difficulty). On PC nothing is saved — progress is lost on quit.
This is **Phase 4** (file-backed EEPROM): `osEepromWrite/Read` are
currently no-op stubs; build on PD's `fs.c` (file-backed save dir). The
save *logic* already runs in the decomp — it just needs real backing
storage. Blocks any multi-session campaign playthrough.
### PC graphics settings ### PC graphics settings
Optional video/rendering options, modelled on the PD port (baseline) with Optional video/rendering options, modelled on the PD port (baseline) with
+68
View File
@@ -401,6 +401,74 @@ Start, exactly like the retail game.** No crashes, no hangs. 10 commits
- The D75 front-end / cutscene 3D-model family (D148/D149) is now the - The D75 front-end / cutscene 3D-model family (D148/D149) is now the
biggest *visible* gap but is cosmetic — below crashes. biggest *visible* gap but is cosmetic — below crashes.
## Done this session (M-28) — D150 (watch page crash) + D151 (watch text blank)
- **D150** (`src/str.c`, `#ifdef PORT`, uncommitted) — user found a crash
opening **level objectives on the watch**. AV in `strcat` (`str.c:25`,
NULL src). The watch BRIEF/OBJECTIVES pages (`options.c:3940-4068`) build
text with `strcat(buf, langGet(id))`; `langGet` returns NULL on PC for the
briefing/objective string banks the in-level watch flow never loads
(same missing-bank issue as the D143 "briefing text renders blank" note).
Fix: NULL-tolerant `strcpy/strncpy/strcat` under `#ifdef PORT`. Note these
are `__nonnull__` builtins to GCC so a plain `if (src==NULL)` is optimised
away — the guard laundras the ptr through an empty `__asm__` (`GE_IS_NULL`).
Build green; `-level_09` 600+ frames @ 91.67% unregressed. **Watch-page
repro is interactive — needs user re-verify.** §F/§H D150, PORT-LEARNINGS §C.
- **D151** (`src/bondtypes.h`, `#ifdef PORT`, uncommitted) — the D150
"objective/briefing text renders blank" consequence, ROOT-CAUSED and fixed.
NOT a missing bank: the per-level lang bank IS loaded in-level
(`langLoadToAddr`, `prop.c:1274`). The propDef records that carry the watch
text (types 35 `WatchMenuObjectiveText`, 23 `ObjectiveStart`) store a plain
`s32` slot id in their 3rd word; `struct watchMenuObjectiveText` /
`struct objective_entry` decode it as `u16 reserved; u16 text;` and read
`text` at offset `0xA` — works only because on BE the id is in the low 16
bits. `d88_propdefs.py` `_bswap32`'s the whole word → `text` @0xA reads 0 →
`langGet(0)` → NULL → blank. Fix: `text` is a full `u32` at the word offset
under `#ifdef PORT` (N64 `u16 reserved; u16 text;` kept under `#else`). No
converter change / no sidecar regen. Compiles clean (link needs the running
game closed). **Interactive re-verify pending** — open the watch, all of
OBJECTIVES / mission background / M / Q / Moneypenny should now show text.
§F **D151**, PORT-LEARNINGS §C.
### D152 (OPEN) — mission-failed → permanent black screen = `osSetIntMask` lock deadlock
User killed Trevelyan in Facility (`Ctrl` = **fire**, no crouch bind) → failed
the objective → fade to black, never returns; process alive. **Live `gdb.txt`
inspected** (game left running under `debug.ps1`): NOT the front-end
mission-failed *screen* — it's a **deadlock on `s_imLock`** (the D147
recursive-mutex behind `osSetIntMask`). Rendering froze at frame 12600.
`mainThread` in `sndSetScalerApplyVolumeAllSfxSlot`→`alEvtqPostEvent`→
`osSetIntMask(OS_IM_NONE)`→`pthread_mutex_lock` (waiting); `amMain` in
`sndPlayerVoiceHandler`→`alEvtqNextEvent`→same, also waiting; **neither owns the
lock** → a leaked unbalanced `OS_IM_NONE` (libaudio early-return paths) or a
transient thread that acquired-and-exited holds it. The mission-failed **audio
fade-out** (`sndSetScalerApplyVolumeAllSfxSlot` posts `AL_SNDP_RELEASE_EVT` per
`ALSoundState` per frame) is what exercises the window. D147 family; D147's
"cannot deadlock" claim is now falsified. §F **D152**, PORT-LEARNINGS §D4.
**MITIGATED (M-28, `port/src/libultra.c`, port-only, uncommitted).**
`osSetIntMask` is now a **self-healing** logical lock: same recursive
acquire/release as the D147 mutex on the normal (microsecond) path, but a
waiter blocked **> 2 s** (`OS_IM_STUCK_NS`) declares the holder leaked it,
logs `LOG_ERROR` with the stale owner + the stealing caller's return
address, and steals the section. A leaked/unbalanced `OS_IM_NONE` (or an
acquire-and-exit transient thread) can no longer wedge the game forever —
worst case is a ~2 s audio hiccup + a log line that names the leak. Builds
clean. Still OPEN: the exact leaking call site (read it from the next
`D152: ... stealing from owner=` log) + the proper narrow fix (dedicated
`ALEventQueue` lock). Next: replay the Facility mission-fail; if the steal
fires, `addr2line` the logged caller.
Separate minor gap: no crouch keybind (BACKLOG B3/B4).
### D153 — `-level_09` frame ~900 crash = load flakiness, NOT a bug (closed)
Chased for ~30 min in M-28: `-level_09` crashed `0xc0000005` at a
nondeterministic frame 900↔1400, 6/6 runs — **but every one was while the
machine was hammered with concurrent builds + multi-run loops.** On a quiet
machine `-level_09` ran past frame 2400, 0 heartbeats, no crash. It's the
D117/D134 host-scheduling flakiness the docs warn about every session (M-25
Silo ~frame 300 is the same). Not a regression, not D150/D151/D152.
`romdataFixupMusicSeqTable: seqCount 63 exceeds blob capacity 1` is a benign
expected warning (header-only first call). **Lesson: never regression-test
while builds/other game runs are in flight.** §F **D153** (closed).
<details><summary>M-27 earlier — front-end/pause bucket: D140 + D141 FIXED</summary> <details><summary>M-27 earlier — front-end/pause bucket: D140 + D141 FIXED</summary>
Started the parked "front-end / transitions / pause / watch" bucket. Started the parked "front-end / transitions / pause / watch" bucket.
+5
View File
@@ -815,6 +815,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
| D113–D116 | portal BFS ok · matrix chain ok · player raw-offset audit + `gunfire.c` THROW* · HUD text X-mirror | D116 OPEN (parked — DO NOT re-static-trace) | | D113–D116 | portal BFS ok · matrix chain ok · player raw-offset audit + `gunfire.c` THROW* · HUD text X-mirror | D116 OPEN (parked — DO NOT re-static-trace) |
| D117 | frame nondeterminism root-caused; `GE_DETERM` deferred; `framediff.py` structural | resolved (mode deferred) | | D117 | frame nondeterminism root-caused; `GE_DETERM` deferred; `framediff.py` structural | resolved (mode deferred) |
| D118 | SDL input layer (`port/src/input.c`); M-24 mouse-look rework: mode-aware map (aim mode = analog stick, hipfire = digital C-pitch), `config.c` INI now real. D118b/c FIXED; D118a residual (hipfire pitch only) | resolved (D118a residual) | | D118 | SDL input layer (`port/src/input.c`); M-24 mouse-look rework: mode-aware map (aim mode = analog stick, hipfire = digital C-pitch), `config.c` INI now real. D118b/c FIXED; D118a residual (hipfire pitch only) | resolved (D118a residual) |
| D150 | watch OBJECTIVES / BRIEF page crash: `strcat(buf, langGet(id))` with `langGet` → NULL (unloaded bank) → NULL deref; `str.c` str* builtins elide a plain param NULL check, so guard via asm-laundered `GE_IS_NULL` | FIXED (`src/str.c` `#ifdef PORT`); interactive re-verify pending |
| D137 | right-mouse (aim-sight) crash: `gunDrawSight` (`gunfire.c:6235`) `s32 sp54` holds a `Gfx*` the whole function (`sp54 = *gdl`; `texSelect(&sp54,…)`/`display_image_at_position(&sp54,…)` take `Gfx**`). As N64 `s32` it is 4 B, so `*(Gfx**)&sp54` reads 4 B of the adjacent stack float as the pointer high word → `_g = 0x03e4fca0_70081220` → wild DL write in `texSetRenderMode` (`othermodemicrocode.c:177`) the moment the crosshair raises. §A. | FIXED (`src/game/gunfire.c` `#ifdef PORT` — `sp54` is `Gfx *`) | | D137 | right-mouse (aim-sight) crash: `gunDrawSight` (`gunfire.c:6235`) `s32 sp54` holds a `Gfx*` the whole function (`sp54 = *gdl`; `texSelect(&sp54,…)`/`display_image_at_position(&sp54,…)` take `Gfx**`). As N64 `s32` it is 4 B, so `*(Gfx**)&sp54` reads 4 B of the adjacent stack float as the pointer high word → `_g = 0x03e4fca0_70081220` → wild DL write in `texSetRenderMode` (`othermodemicrocode.c:177`) the moment the crosshair raises. §A. | FIXED (`src/game/gunfire.c` `#ifdef PORT` — `sp54` is `Gfx *`) |
| D119 · D120 | guard-attack `weapons_held[]->chr` pun crash · blood-stain `PointUsage[]` chain hang (guarded, `d43_emit.py` opcode-0x18 gap) | D119 fixed · D120 guarded | | D119 · D120 | guard-attack `weapons_held[]->chr` pun crash · blood-stain `PointUsage[]` chain hang (guarded, `d43_emit.py` opcode-0x18 gap) | D119 fixed · D120 guarded |
| D121 | WS1 frictionless per-level boot: bare `-level_XX` injects its `memallocstringtable[]` `-m*` row (`#ifdef PORT` in `boss.c`) | resolved | | D121 | WS1 frictionless per-level boot: bare `-level_XX` injects its `memallocstringtable[]` `-m*` row (`#ifdef PORT` in `boss.c`) | resolved |
@@ -832,6 +833,10 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
| D140 | **Pause-menu crash (M-23 open → M-27 fixed).** `bondviewRenderWatch` (`bondview2.c:8604`) → `bondviewTransformManyPosToViewMatrix(g_CurrentPlayer->field_23C, objheader->numMatrices)` with `field_23C == NULL` → `matrix_4x4_copy(src=0x0)` AV. Root cause (NOT "cache never allocated"): `something_with_watch_object_instance` (N64 player +0x230) is a **`struct Model` (0xBC) + its RW-data pool (0xC8) punned into a 0x184-byte hole** (`field_234..field_3B0`); three of those "fields" are Model members the game reads by name — `watch_scale_destination`==`.scale`, `pause_watch_related_adjust`==`.animframe1`, **`field_23C`==`.render_pos`**. `render_pos` IS populated (by `instcalcmatrices` via `subcalcmatrices` at `:8524`), but on x86-64 `sizeof(struct Model)` grows (4→8B ptrs) so `field_23C` no longer overlaps `.render_pos` → reads Model padding (~0) → NULL. Same class as D100 (gait Model) / D102 (weapon Model) / D56 (this very watch Model's pool). Fix: `#ifdef PORT` makes `something_with_watch_object_instance` a real inline `struct Model` + `u32 watchRwPool[192]` in `struct player` (`bondview.h`); `bondview2.c` redirects the 3 named reads to the member via `GE_WATCH_{ANIMFRAME,SCALE,RENDERPOS}` macros (N64 `#else` = the verbatim field). The D56 branch of `sub_GAME_7F07E7CC` drops its `static u8 watchRwPool[0xC8]` for the inline pool. | FIXED (`src/game/bondview.h`, `src/game/bondview2.c`; `#ifdef PORT`). Exposed the next watch-render crash → **D141**. `-level_09`/`-level_20` unregressed (3/3). PORT-LEARNINGS §A. | | D140 | **Pause-menu crash (M-23 open → M-27 fixed).** `bondviewRenderWatch` (`bondview2.c:8604`) → `bondviewTransformManyPosToViewMatrix(g_CurrentPlayer->field_23C, objheader->numMatrices)` with `field_23C == NULL` → `matrix_4x4_copy(src=0x0)` AV. Root cause (NOT "cache never allocated"): `something_with_watch_object_instance` (N64 player +0x230) is a **`struct Model` (0xBC) + its RW-data pool (0xC8) punned into a 0x184-byte hole** (`field_234..field_3B0`); three of those "fields" are Model members the game reads by name — `watch_scale_destination`==`.scale`, `pause_watch_related_adjust`==`.animframe1`, **`field_23C`==`.render_pos`**. `render_pos` IS populated (by `instcalcmatrices` via `subcalcmatrices` at `:8524`), but on x86-64 `sizeof(struct Model)` grows (4→8B ptrs) so `field_23C` no longer overlaps `.render_pos` → reads Model padding (~0) → NULL. Same class as D100 (gait Model) / D102 (weapon Model) / D56 (this very watch Model's pool). Fix: `#ifdef PORT` makes `something_with_watch_object_instance` a real inline `struct Model` + `u32 watchRwPool[192]` in `struct player` (`bondview.h`); `bondview2.c` redirects the 3 named reads to the member via `GE_WATCH_{ANIMFRAME,SCALE,RENDERPOS}` macros (N64 `#else` = the verbatim field). The D56 branch of `sub_GAME_7F07E7CC` drops its `static u8 watchRwPool[0xC8]` for the inline pool. | FIXED (`src/game/bondview.h`, `src/game/bondview2.c`; `#ifdef PORT`). Exposed the next watch-render crash → **D141**. `-level_09`/`-level_20` unregressed (3/3). PORT-LEARNINGS §A. |
| D118d | **Watch-menu list over-scrolls with keyboard (M-27, user bug report).** `game_options_inventory_navigation` / `sub_GAME_7F0A611C` (`options.c`) have a "slam the stick" fast-scroll: a raw *level* check `joyGetStickY(PLAYER_1) < -0x46` / `>= 0x47` that steps `watch_inventory_cursor_pos` by 1 **every frame** it's held. An N64 stick rarely sustains past ~0x46 on-axis so a human taps it; keyboard W/S (and SDL pads pegged at 0x80) sit there every frame → one keypress skips several items (breaks item selection needed to finish the game). Fix: `#ifdef PORT` macro `GE_WATCH_STICK_FAST{UP,DOWN}` → `0` for the port — stick list-nav then goes only through the latched single-step path (`watch_stick_y_pressed_*`, one step per press, existing game mechanism) and the -0x1e..-0x45 smooth-scroll band; D-pad/C-buttons keep edge-repeat. N64 `#else` = verbatim. | FIXED (`src/game/options.c`, `#ifdef PORT`). Build green, `-level_09` unregressed. **Feel to be confirmed in playtest** (probe couldn't reach the INVENTORY watch page headlessly). D118 input family. Latent sibling: `front.c` menu nav uses `joyGetStick*InRange` level checks the same way. | | D118d | **Watch-menu list over-scrolls with keyboard (M-27, user bug report).** `game_options_inventory_navigation` / `sub_GAME_7F0A611C` (`options.c`) have a "slam the stick" fast-scroll: a raw *level* check `joyGetStickY(PLAYER_1) < -0x46` / `>= 0x47` that steps `watch_inventory_cursor_pos` by 1 **every frame** it's held. An N64 stick rarely sustains past ~0x46 on-axis so a human taps it; keyboard W/S (and SDL pads pegged at 0x80) sit there every frame → one keypress skips several items (breaks item selection needed to finish the game). Fix: `#ifdef PORT` macro `GE_WATCH_STICK_FAST{UP,DOWN}` → `0` for the port — stick list-nav then goes only through the latched single-step path (`watch_stick_y_pressed_*`, one step per press, existing game mechanism) and the -0x1e..-0x45 smooth-scroll band; D-pad/C-buttons keep edge-repeat. N64 `#else` = verbatim. | FIXED (`src/game/options.c`, `#ifdef PORT`). Build green, `-level_09` unregressed. **Feel to be confirmed in playtest** (probe couldn't reach the INVENTORY watch page headlessly). D118 input family. Latent sibling: `front.c` menu nav uses `joyGetStick*InRange` level checks the same way. |
| D141 | **Watch-menu item-model crash (M-27), exposed by D140.** Pressing Start → watch page renders → `set_enviro_fog_for_items_in_solo_watch_menu` (`gunfire.c:1620`) → `modelGetNodeRwData(&model, root)` with `root` garbage → AV reading `root->Opcode` (`model.c:478`). `gunfire.c:1720-1743` walks `bodymodel->Switches[]` (a `ModelNode*` array) with **raw byte offsets** `((u8*)Switches) + j + 0x48` / `+ 0x5c` and `j += 4` — assumes 4-byte N64 pointers. `0x48/4=18`, `0x5c/4=23`, `k=j>>2=0..4` → intent is `Switches[18+k]` and `Switches[23+k]`. PC's 8-byte stride makes the raw math read misaligned garbage → bogus non-NULL `ModelNode*`. PORT-LEARNINGS §B (raw hardcoded stride into a pointer array, cf. D128). Fix: `#ifdef PORT` uses `bodymodel->Switches[18 + (j>>2)]` / `[23 + (j>>2)]`. | FIXED (`src/game/gunfire.c`, `#ifdef PORT`). Watch page now renders (weapon page verified — mirrored text D114/D116, dark weapon model D75, both parked cosmetic). `-level_09`/`-level_20` unregressed. | | D141 | **Watch-menu item-model crash (M-27), exposed by D140.** Pressing Start → watch page renders → `set_enviro_fog_for_items_in_solo_watch_menu` (`gunfire.c:1620`) → `modelGetNodeRwData(&model, root)` with `root` garbage → AV reading `root->Opcode` (`model.c:478`). `gunfire.c:1720-1743` walks `bodymodel->Switches[]` (a `ModelNode*` array) with **raw byte offsets** `((u8*)Switches) + j + 0x48` / `+ 0x5c` and `j += 4` — assumes 4-byte N64 pointers. `0x48/4=18`, `0x5c/4=23`, `k=j>>2=0..4` → intent is `Switches[18+k]` and `Switches[23+k]`. PC's 8-byte stride makes the raw math read misaligned garbage → bogus non-NULL `ModelNode*`. PORT-LEARNINGS §B (raw hardcoded stride into a pointer array, cf. D128). Fix: `#ifdef PORT` uses `bodymodel->Switches[18 + (j>>2)]` / `[23 + (j>>2)]`. | FIXED (`src/game/gunfire.c`, `#ifdef PORT`). Watch page now renders (weapon page verified — mirrored text D114/D116, dark weapon model D75, both parked cosmetic). `-level_09`/`-level_20` unregressed. |
| D150 | **Watch objective/briefing page crash (M-28).** Opening the watch → BRIEF (any subpage) or OBJECTIVES page → AV in `strcat` (`str.c:25`, `Rdi/src = 0`). The pages assemble their display text with `strcpy`/`strcat(buf, langGet(id))` (`options.c:3940-4068`, `front.c:3538-3562`, `propobj.c`). `langGet()` returns **NULL** on PC for a string bank the menu flow never loaded (D129/D143 guards — briefing/objective banks in the in-level watch flow), so `strcat(buf, NULL)` derefs NULL. On N64 those ids always resolve so the decomp never guards. Twist: `str.c`'s `strcpy/strncpy/strcat` are `__nonnull__` **builtins** to GCC, so a plain `if (src == NULL)` on the parameter is optimised away as provably-dead (verified: `-Og` elided it, no `test` in the disassembly). Fix: `#ifdef PORT` NULL guards that launder the pointer through an empty `__asm__` (`GE_IS_NULL()` / `ge_launder_ptr()`) so the check survives — NULL src → treat as empty string (blank text, not a crash), NULL dst → return. Same philosophy as the D143 textRender/textMeasure NULL guards, one layer down. Inert on N64. | FIXED (`src/str.c`, `#ifdef PORT`). Build green (`ntsc-final`); `-level_09` 600+ frames @ 91.67% unregressed. Watch-page repro is interactive — user to re-verify. PORT-LEARNINGS §C. |
| D152 | **Mission-failed / objective-failed → permanent black screen (M-28, user bug report — LIVE process inspected, not yet fixed).** User killed Trevelyan in Facility (Ctrl = fire, no crouch bind) → failed the objective → screen faded to black and never came back; process stayed alive under `debug.ps1`. `gdb.txt` (kept running): rendering stops dead at frame 12600 (`frames=12657` frozen across 206 heartbeat dumps / ~7 real min), VI pacemaker still posts. **Deadlock on `s_imLock`** — the D147 recursive-mutex that backs `osSetIntMask`. Both consumer threads are blocked in `pthread_mutex_lock(&s_imLock)` and neither owns it: `mainThread` = `sndSetScalerApplyVolumeAllSfxSlot` → `sndApplyVolumeAllSfxSlot` → `sndSetSfxSlotVolume` → `alEvtqPostEvent` (`event.c:82`) → `osSetIntMask(OS_IM_NONE)` (`libultra.c:1232`); `amMain`/audioThread = `amHandleFrameMessage` → `alAudioFrame` → `sndPlayerVoiceHandler` → `alEvtqNextEvent` (`event.c:47`) → `osSetIntMask(OS_IM_NONE)`. `sndSetScalerApplyVolumeAllSfxSlot` is the **mission-failed audio fade-out** (ramps `g_sndSfxVolumeScale` → 0 each frame, `AL_SNDP_RELEASE_EVT` posted per active `ALSoundState` per frame) — so the fade-to-black *is* the intended fail transition, and it hammers the event queue hard enough that the `osSetIntMask` lock is left owned by a party not in the 3-thread dump (a leaked unbalanced `OS_IM_NONE` — libaudio has early-return mask paths — or a transient thread that acquired it and exited; `gdb.txt` shows `New Thread`/`exited` churn every few k frames). D147's "the decomp never blocks while holding OS_IM_NONE → cannot deadlock" assumption is **falsified** by this path. Same family as D147 / PORT-LEARNINGS §D4. | MITIGATED (`port/src/libultra.c`, M-28, `#ifdef PORT`/port-only). `osSetIntMask` is now a **self-healing** logical lock: `OS_IM_NONE`/`OS_IM_ALL` acquire/release a process-wide recursive critical section tracked by `s_imHeld`/`s_imOwner`/`s_imDepth` under a short bookkeeping mutex `s_imMx` + condvar. Normal (balanced, microsecond) audio critical sections behave exactly as the D147 recursive mutex did. A waiter that blocks **> 2 s** (`OS_IM_STUCK_NS`) concludes the holder leaked it, logs `LOG_ERROR` with the stale `s_imOwner` + the stealing caller's return address, and **steals** the section — so a leaked/unbalanced `OS_IM_NONE` (or an acquire-and-exit transient thread) can no longer wedge the game forever; worst case is a ~2 s audio hiccup + a log line naming the leak. Builds clean. Still OPEN: the exact leaking call site (find it from the `D152: ... stealing from owner=` log next time it fires) and the proper narrow fix (dedicated `ALEventQueue` lock, or restore-on-thread-exit). Regression check deferred — `-level_09`/`-level_20` both flaky under the current machine load (D117/D134), and clean `master` itself has a separate nondeterministic `-level_09` frame ~900-1400 `0xc0000005` (see below). |
| D153 | **`-level_09` frame ~900–1400 `0xc0000005` = D117/D134 load flakiness, NOT a bug (M-28).** Reproduced 6/6 while the machine was under heavy concurrent build+run load; on a **quiet** machine `-level_09` ran past frame 2400 with 0 heartbeats, no crash. Same false-positive class the docs warn about every session (M-13..M-22, M-25 Silo ~frame 300 freeze). The crash PC was in a system DLL (a `memcpy` off a buffer) and the frame varied run-to-run — both consistent with the port's host-scheduling jitter under load (`osGetCount` is wall-clock, variable-timestep sim), not a deterministic defect. `romdataFixupMusicSeqTable: seqCount 63 exceeds blob capacity 1` is an EXPECTED benign warning on the header-only first `romdataFixupMusicSeqTable` call (music.c:736; the full-size second call at :742 is silent) — not related. **Lesson: don't regression-test while builds/other runs are in flight.** | NOT A BUG — closed. Run anchors on a quiet machine. |
| D151 | **Watch data all blank — objectives / mission background / M / Q / Moneypenny (M-28, user bug report).** Not a crash (D150 fixed that); every line renders empty. The watch text lives in `propDefs` record types **35** (`WatchMenuObjectiveText`) and **23** (`ObjectiveStart`). In the setup stream each record's 3rd word is a **plain 32-bit language slot id** (see `assets/obseg/setup/Usetup*Z.c` `propDefs[]`: `_mkword(0,_mkshort(0,35)), menu, 11281, 0`). The decomp structs `struct watchMenuObjectiveText` / `struct objective_entry` decode that word as `u16 reserved; u16 text;` and read `text` at offset `0xA` — which only works on the N64 because the id sits in the **low 16 bits of a big-endian word**. On LE, `tools_pc/d88_propdefs.py` `_bswap32`'s the whole word (correct for a 32-bit field), moving the id to the low bytes, so `text` (u16 @0xA) reads `0x0000` → `langGet(0)` → NULL → `get_ptr_text_for_watch_breifing_page` / `get_text_for_objective` return NULL/blank. This is the "briefing/objective text renders blank" side-effect left open under D143 / D150. The per-level lang bank itself IS loaded in-level (`langLoadToAddr(langGetLangBankIndexFromStagenum(stageId))`, `prop.c:1274`). | FIXED (`src/bondtypes.h`, `#ifdef PORT` — `text` is a full `u32` covering the whole word in both structs; N64 `u16 reserved; u16 text;` kept under `#else`). No converter change, no sidecar regen. `nextentry`/`difficulty`/`unkD` unchanged. PORT-LEARNINGS §C. Build compiles clean; interactive re-verify pending (open the watch in any level). |
| D130 | crash class C2 (Facility `-level_34`, Runway `-level_35`): `import_texture_i8` AV on a wild texture pointer (`0x72181ee8`), baked into the HUD glyph DL by `gDPLoadTextureBlock(gdl, curchar->pixeldata, …)` when the level-title string ("Chemical Warfare Facility #2") renders `#`/`"`. Root cause: `romdataFixupFont` (`port/src/romdata.c`) converts the N64 24B fontchar array to the PC 32B array **in place**, forward-field, backward-glyph. PC−N64 stride = 8, so for glyphs 0/1/2 `d` overlaps `s` by less than the 24B read span and an early field write clobbers a later field's source mid-`for k<5` loop → glyph 1's `width` = `bswap(index)` = `0x01000000`, glyph 2's `pixeldata` = `pcPixOff+(index−pixStart)` ≈ `0x020002e8` → `+= font_base` → wild → fast3d AV. On N64 the array is read-only rodata, never re-laid-out, so no bug there. NOT the model-GDL relocation (`sub_GAME_7F0762E0` / `texLoadFromGdl`) the D124-Facility addendum suspected — `gdl` there is still a segmented `0x05xxxxxx` value so the `& 0x00ffffff` masks are correct, and `texLoadFromGdl` never copies a `G_SETTIMG` for these levels. | resolved — stage all 6 N64 fields in a local `f[6]` before writing any. Facility + Runway now PASS (16→18/21). Probes reverted. | | D130 | crash class C2 (Facility `-level_34`, Runway `-level_35`): `import_texture_i8` AV on a wild texture pointer (`0x72181ee8`), baked into the HUD glyph DL by `gDPLoadTextureBlock(gdl, curchar->pixeldata, …)` when the level-title string ("Chemical Warfare Facility #2") renders `#`/`"`. Root cause: `romdataFixupFont` (`port/src/romdata.c`) converts the N64 24B fontchar array to the PC 32B array **in place**, forward-field, backward-glyph. PC−N64 stride = 8, so for glyphs 0/1/2 `d` overlaps `s` by less than the 24B read span and an early field write clobbers a later field's source mid-`for k<5` loop → glyph 1's `width` = `bswap(index)` = `0x01000000`, glyph 2's `pixeldata` = `pcPixOff+(index−pixStart)` ≈ `0x020002e8` → `+= font_base` → wild → fast3d AV. On N64 the array is read-only rodata, never re-laid-out, so no bug there. NOT the model-GDL relocation (`sub_GAME_7F0762E0` / `texLoadFromGdl`) the D124-Facility addendum suspected — `gdl` there is still a segmented `0x05xxxxxx` value so the `& 0x00ffffff` masks are correct, and `texLoadFromGdl` never copies a `G_SETTIMG` for these levels. | resolved — stage all 6 N64 fields in a local `f[6]` before writing any. Facility + Runway now PASS (16→18/21). Probes reverted. |
Phase 2 replaced the Phase-1 demo loop with the real `mainproc()` on real OS Phase 2 replaced the Phase-1 demo loop with the real `mainproc()` on real OS
+39
View File
@@ -169,6 +169,30 @@ through a converter or a runtime bswap fixup reads scrambled.
Grep for `(u8)*`/`(s8)*`/`& 0xff` on a `u32`/`s32` that's really a Grep for `(u8)*`/`(s8)*`/`& 0xff` on a `u32`/`s32` that's really a
serialized multi-field word — especially GBI-word and header-word reads serialized multi-field word — especially GBI-word and header-word reads
that were never routed through a converter or `#ifdef PORT`. that were never routed through a converter or `#ifdef PORT`.
- **D150 — `langGet()` NULL flows into a `str*` primitive.** The watch
BRIEF/OBJECTIVES pages (and `front.c` mission text) build their strings with
`strcpy`/`strcat(buf, langGet(id))`. `langGet` returns NULL on PC for any
bank the current menu flow never loaded (D129/D143); N64 always resolves
these ids so the decomp never guards. `strcat(buf, NULL)` then derefs 0.
Fix = NULL-tolerant `strcpy/strncpy/strcat` under `#ifdef PORT` in `str.c`
(NULL src → empty string, NULL dst → return). **Gotcha:** these are
`__nonnull__` **builtins** to GCC — a plain `if (src == NULL)` on the
parameter is deleted as provably-dead (confirmed in `-Og` disassembly:
no `test` emitted). Launder the pointer through an empty `__asm__("":"+r"(p))`
(`GE_IS_NULL()`) so the guard survives. Same class as the D143 textRender
NULL guards. Audit any hand-rolled libc primitive in `src/` that a port
NULL can reach.
- **D151 — a ROM-serialized `s32` slot decoded by the struct as `[u16 hi][u16 lo]`
reads zero on LE.** N64 code frequently splits a 32-bit setup-stream word into
`u16 reserved; u16 realvalue;` where the useful value is always small and lands
in the **low 16 bits of the big-endian word**. The offline converter correctly
`_bswap32`'s the word (it IS a 32-bit field), which puts the value in the low
bytes — but the struct still reads `realvalue` from the *high* offset → 0.
Instances: `struct watchMenuObjectiveText.text` / `struct objective_entry.text`
(propDef types 35 / 23) — every watch briefing + objective line rendered blank
(`langGet(0)` → NULL). Fix: `#ifdef PORT` widens the field to a full `u32` at
the word offset; converter unchanged. Grep every ROM-serialized struct for a
`u16 reserved`/`u16 pad` immediately before a `u16` the runtime actually reads.
- **Python slice-assignment width is a silent corruptor** (D125): - **Python slice-assignment width is a silent corruptor** (D125):
`out[a:a+8] = b"\x00\x00\x00\x00"` on a `bytearray` does NOT raise — it `out[a:a+8] = b"\x00\x00\x00\x00"` on a `bytearray` does NOT raise — it
deletes `(8 - len(rhs))` bytes, shrinking `out` and shifting everything deletes `(8 - len(rhs))` bytes, shrinking `out` and shifting everything
@@ -310,6 +334,21 @@ a few others use it as their **only** mutual-exclusion primitive. On PC the
what every paired restore passes, so `OS_IM_ALL` releases; other specific what every paired restore passes, so `OS_IM_ALL` releases; other specific
masks (`OS_IM_VI`) pass through. Safe because the decomp never blocks masks (`OS_IM_VI`) pass through. Safe because the decomp never blocks
while holding `OS_IM_NONE` (N64 contract). while holding `OS_IM_NONE` (N64 contract).
- **D152 — the recursive-mutex model is fragile; it CAN deadlock.** The
"decomp never blocks while holding `OS_IM_NONE`" assumption fails on
heavy `ALEventQueue` paths: libaudio has unbalanced / early-`return`
`osSetIntMask` calls, and a transient thread can acquire `OS_IM_NONE`
and exit without the paired `OS_IM_ALL`, leaving `s_imLock` owned forever
→ every later `alEvtq*` on `mainThread` + `amMain` blocks in
`pthread_mutex_lock` → hang. Seen on the **mission-failed audio
fade-out** (`sndSetScalerApplyVolumeAllSfxSlot` → per-frame
`alEvtqPostEvent` storm) = permanent black screen. **Mitigation shipped
(M-28):** `osSetIntMask` now tracks owner/depth under a short bookkeeping
mutex + condvar; a waiter blocked > 2 s **steals** the section and logs
the stale owner + the stealing caller's return address. Self-heals any
leak (worst case ~2 s audio hiccup). Proper narrow fix (dedicated
`ALEventQueue` lock) still owed — do it once the steal-log names the
leaking call site. §F D152.
## E. Process / method notes ## E. Process / method notes
+94 -20
View File
@@ -26,6 +26,7 @@
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <stdint.h> #include <stdint.h>
#include <time.h>
#include <SDL.h> #include <SDL.h>
@@ -1194,45 +1195,118 @@ static uint64_t portNextTimerUs(void)
return earliest; return earliest;
} }
/* Interrupt mask -> global recursive lock (D147). /* Interrupt mask -> global recursive lock (D147), self-healing (D152).
* *
* On N64 `osSetIntMask(OS_IM_NONE)` disables interrupts so the following * On N64 `osSetIntMask(OS_IM_NONE)` disables interrupts so the following
* region cannot be preempted; the paired `osSetIntMask(saved)` restores it. * region cannot be preempted; the paired `osSetIntMask(saved)` restores it.
* libaudio relies on this to serialise the event queue (alEvtqPostEvent, * libaudio relies on this to serialise the event queue (alEvtqPostEvent,
* sndRemoveEvents, alEvtqNextEvent, ...) between the game thread and the * sndRemoveEvents, alEvtqNextEvent, ...) between the game thread and the
* audio-manager "interrupt". On PC that audio manager is a real preemptible * audio-manager "interrupt". On PC that audio manager is a real preemptible
* thread (amMain), and this shim was a no-op -> both threads mutate the * thread (amMain), and a no-op shim let both threads mutate the same
* same ALEventQueue linked list concurrently -> list corruption -> the game * ALEventQueue linked list concurrently -> list corruption -> the game
* thread spins forever in alEvtqPostEvent's insert walk (hang seen when a * thread spins forever in alEvtqPostEvent's insert walk (hang seen when a
* door finishes opening and posts its close SFX, propobj.c objTick). * door finishes opening and posts its close SFX, propobj.c objTick).
* *
* Model the mask as one process-wide recursive mutex: OS_IM_NONE acquires, * Model the mask as one process-wide recursive critical section: OS_IM_NONE
* OS_IM_ALL (the value we hand back, so every paired restore passes it) * acquires, OS_IM_ALL (the value we hand back, so every paired restore
* releases. Other specific masks (OS_IM_VI in sched.c) are not lock ops and * passes it) releases. Other specific masks (OS_IM_VI in sched.c) are not
* pass through. The decomp never blocks while holding OS_IM_NONE, so this * lock ops and pass through.
* cannot deadlock. */ *
static pthread_mutex_t s_imLock; * D152: the recursive-mutex form CAN wedge permanently. libaudio has
static pthread_once_t s_imOnce = PTHREAD_ONCE_INIT; * unbalanced / early-return mask calls, and a transient host thread can
* acquire OS_IM_NONE and exit without the paired OS_IM_ALL, leaving the
* lock owned forever -> every later alEvtq* on mainThread + amMain blocks
* -> black screen (repro: mission-failed audio fade-out,
* sndSetScalerApplyVolumeAllSfxSlot posts a release event per active sound
* per frame). Real audio critical sections are microseconds; anything
* holding for OS_IM_STUCK_NS is leaked, so a waiter that has blocked that
* long STEALS the section (logging the stale owner + the caller so the leak
* site can be found and fixed narrowly later). Bookkeeping lives under
* s_imMx (only ever held briefly); s_imHeld/s_imOwner/s_imDepth are the
* logical lock. */
#define OS_IM_STUCK_NS (2000ll * 1000 * 1000) /* 2 s */
static void imLockInit(void) static pthread_mutex_t s_imMx = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t s_imCv = PTHREAD_COND_INITIALIZER;
static int s_imHeld;
static unsigned long s_imOwner;
static int s_imDepth;
static unsigned long imSelf(void)
{ {
pthread_mutexattr_t a; return (unsigned long)(uintptr_t)pthread_self();
pthread_mutexattr_init(&a); }
pthread_mutexattr_settype(&a, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&s_imLock, &a); static void imAcquire(void *caller)
pthread_mutexattr_destroy(&a); {
unsigned long self = imSelf();
struct timespec now;
pthread_mutex_lock(&s_imMx);
if (s_imHeld && s_imOwner == self) { /* recursive re-entry */
s_imDepth++;
pthread_mutex_unlock(&s_imMx);
return;
}
long long startNs = 0; /* set on first contended iteration */
while (s_imHeld) {
struct timespec dl;
if (startNs == 0) {
clock_gettime(CLOCK_REALTIME, &now);
startNs = (long long)now.tv_sec * 1000000000ll + now.tv_nsec;
}
clock_gettime(CLOCK_REALTIME, &now);
long long nowNs = (long long)now.tv_sec * 1000000000ll + now.tv_nsec;
if (nowNs - startNs >= OS_IM_STUCK_NS) {
sysLogPrintf(LOG_ERROR,
"D152: osSetIntMask lock stuck >2s -- stealing from owner=%lu "
"depth=%d (this caller=%p). A prior OS_IM_NONE was never "
"restored; find that call site.",
s_imOwner, s_imDepth, caller);
break; /* fall through, steal it */
}
/* wake ~10x/s to re-check the steal deadline */
long long wakeNs = nowNs + 100ll * 1000 * 1000;
dl.tv_sec = (time_t)(wakeNs / 1000000000ll);
dl.tv_nsec = (long)(wakeNs % 1000000000ll);
pthread_cond_timedwait(&s_imCv, &s_imMx, &dl);
}
s_imHeld = 1;
s_imOwner = self;
s_imDepth = 1;
pthread_mutex_unlock(&s_imMx);
}
static void imRelease(void)
{
unsigned long self = imSelf();
pthread_mutex_lock(&s_imMx);
if (!s_imHeld || s_imOwner != self) { /* spurious / already stolen: no-op */
pthread_mutex_unlock(&s_imMx);
return;
}
if (--s_imDepth <= 0) {
s_imHeld = 0;
s_imOwner = 0;
s_imDepth = 0;
pthread_cond_signal(&s_imCv);
}
pthread_mutex_unlock(&s_imMx);
} }
OSIntMask osSetIntMask(OSIntMask mask) OSIntMask osSetIntMask(OSIntMask mask)
{ {
pthread_once(&s_imOnce, imLockInit);
if (mask == OS_IM_NONE) { /* enter critical section */ if (mask == OS_IM_NONE) { /* enter critical section */
pthread_mutex_lock(&s_imLock); imAcquire(__builtin_return_address(0));
return OS_IM_ALL; /* paired restore will pass this back */ return OS_IM_ALL; /* paired restore will pass this back */
} }
if (mask == OS_IM_ALL) { /* leave critical section */ if (mask == OS_IM_ALL) { /* leave critical section */
pthread_mutex_unlock(&s_imLock); /* recursive: EPERM no-op if unheld */ imRelease();
return OS_IM_NONE; return OS_IM_NONE;
} }
return mask; /* OS_IM_VI etc: not a lock operation */ return mask; /* OS_IM_VI etc: not a lock operation */
+15
View File
@@ -4032,8 +4032,19 @@ struct watchMenuObjectiveText
{ {
u32 id; u32 id;
enum WATCH_BRIEFING_PAGE menu; enum WATCH_BRIEFING_PAGE menu;
#ifdef PORT
/* D151: the serialized propDefs word here is a plain s32 slot id (see
* assets/obseg/setup/Usetup*Z.c `propDefs[]`: `_mkword(0,_mkshort(0,35)),
* menu, textid, 0`). The N64 decode `[u16 reserved][u16 text]` only reads
* the id because it lives in the low 16 bits of a big-endian word. On LE
* the offline d88 converter bswap32's the whole word, so the id lands in
* `text`'s low bytes only when `text` covers the full word. Was: watch
* BRIEF/OBJECTIVES + M/Q/Moneypenny pages all rendered blank (langGet(0)). */
u32 text;
#else
u16 reserved; u16 reserved;
u16 text; u16 text;
#endif
struct watchMenuObjectiveText *nextentry; struct watchMenuObjectiveText *nextentry;
}; };
// PROPDEF_OBJECTIVE_x (23,24 ) - maybe not a propdef // PROPDEF_OBJECTIVE_x (23,24 ) - maybe not a propdef
@@ -4042,8 +4053,12 @@ struct objective_entry
{ {
u32 id; // 0 u32 id; // 0
enum WATCH_BRIEFING_PAGE menu; // 4 enum WATCH_BRIEFING_PAGE menu; // 4
#ifdef PORT
u32 text; // 8 (D151: full-word slot id, see above)
#else
u16 reserved; // 8 u16 reserved; // 8
u16 text; // a u16 text; // a
#endif
u16 unkC; // c u16 unkC; // c
u8 unkD; // d u8 unkD; // d
s8 difficulty; // f s8 difficulty; // f
+29
View File
@@ -1,13 +1,38 @@
#include "str.h" #include "str.h"
#ifdef PORT
/* D150: strcpy/strncpy/strcat are __nonnull__ builtins to GCC, so a plain
* `if (src == NULL)` on the parameter is optimised away as provably-dead.
* Launder the pointer through an empty asm so the compiler cannot prove it
* non-NULL and the guard survives (-Og elided it otherwise — verified in
* the disassembly). */
static inline const void *ge_launder_ptr(const void *p) {
__asm__("" : "+r"(p));
return p;
}
#define GE_IS_NULL(p) (ge_launder_ptr(p) == NULL)
#endif
char *strcpy(char *dst, const char *src) { char *strcpy(char *dst, const char *src) {
unsigned char *ptr = dst; unsigned char *ptr = dst;
#ifdef PORT
/* D150: the watch briefing/objective pages assemble their text with
* strcpy/strcat(buf, langGet(id)); langGet() returns NULL for a string
* bank the PC menu flow never loaded (D129/D143), so a NULL src here
* faults. Treat NULL as the empty string (blank text, not a crash) —
* same philosophy as the D143 textRender/textMeasure NULL guards.
* N64 langGet never returns NULL for these ids, so this is inert there. */
if (GE_IS_NULL(src)) { if (!GE_IS_NULL(dst)) *ptr = '\0'; return dst; }
#endif
while(*ptr++ = *src++); while(*ptr++ = *src++);
return dst; return dst;
} }
char *strncpy(char *dst, const char *src, size_t n) { char *strncpy(char *dst, const char *src, size_t n) {
unsigned char *ptr = dst; unsigned char *ptr = dst;
#ifdef PORT
if (GE_IS_NULL(src)) { while (n--) *ptr++ = '\0'; return dst; } /* D150 */
#endif
while((*ptr++ = *src++)) { while((*ptr++ = *src++)) {
if (--n == 0) { if (--n == 0) {
break; break;
@@ -21,6 +46,10 @@ char *strncpy(char *dst, const char *src, size_t n) {
char *strcat(char *dst, const char *src) { char *strcat(char *dst, const char *src) {
unsigned char *ptr = dst; unsigned char *ptr = dst;
#ifdef PORT
if (GE_IS_NULL(dst)) { return dst; } /* D150 */
if (GE_IS_NULL(src)) { return dst; } /* D150 — see strcpy note */
#endif
while (*ptr) { ptr++; }; while (*ptr) { ptr++; };
while(*ptr++ = *src++); while(*ptr++ = *src++);
return dst; return dst;