fix(linux): thread a va_list* through _Printf (x86-64 SysV ABI) (#22)

* fix(linux): thread a va_list* through _Printf (x86-64 SysV ABI)

On the x86-64 System V ABI `va_list` is an array type, so a by-value
`va_list` parameter decays to a pointer. `_Printf()` was passing `&args`
(the address of that local pointer, not the argument list) to `_Putfld()`,
which does `va_arg(*args, ...)` — so `_Putfld` walked garbage and
segfaulted. Latent at `-Og`, fatal at `-O2` (PR #9), which is why the
Linux release build crashed at boot:

  bossInitMainthreadData (boss.c:247, the D121 per-level -m token sprintf)
    -> sprintf("-level_%c%c %s", ...) -> _Putfld -> SIGSEGV @ xprintf.c

Same UB class as D187. Windows x64's `va_list` is a scalar (`char*`), so
`&args` happened to work there — the Windows build is unaffected.

Fix: under `#ifdef PORT`, `_Printf` takes `va_list *` and passes it
straight to `_Putfld` (which already expects `va_list *`); `src/sprintf.c`
(the only PC caller — `rmon.c` is excluded from the build) passes `&args`
of a real `va_list` object. N64 build untouched (`#ifdef PORT`, and it
byte-matches the ROM).

Verified:
- Windows: build links, `-level_09` framediff 3/3 within golden threshold.
- Linux -O2: boot crash gone, now advances past bossInitMainthreadData
  into stage load (uncovers a separate pre-existing stan.c stack overflow
  during BUNKER1 prop setup — tracked separately).

* docs: D188 (xprintf va_list ABI) + D189 (stan.c stack-smash) findings + porting-notes D7

docs/dev/findings.md

@@ -388,6 +388,8 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
 | D180 · D181 | native-PC input pass (QoL run, `qol/native-pc-input-menu`): D180 = `Input.MouseCaptureMode` click-to-lock + absolute-cursor menu tracking + aim-speed default retune (`port/src/input.c`, `video.c`, port-only); D181 = `Game.ScreenShakeIntensity` route-(b) hook in `src/fr.c viShake` (`#ifdef PORT`, default 1.0 = no-op). | LANDED, config-gated, feel-checks owed |
 | D186 | **`Video.FpsCap` throttles the sim, not just presentation (M-39 diag, M-42 clamp).** The frame-pacing `sysSleep`+busy-wait in `sync_framerate_with_timer` (`port/fast3d/gfx_sdl2.cpp`) runs inline on the scheduler thread (`osSpTaskStartGo`→`gfx_run`→`swap_buffers_begin`), so a low cap blocks VI-retrace delivery and drags every game thread down to the cap rate (no N64 analogue — RSP/RDP are separate silicon). **Landed:** `0 < FpsCap < 30` → treated as `0` (uncapped) + warning, at both `videoInit` (normalises a bad `ge007.ini` for the next `configSave`) and `gfx_sdl_set_target_fps` (covers the F10-overlay live path); caps ≥ 30 unchanged. Verified: default `FpsCap=0` framediff 3/3 golden-identical; `FpsCap=10` now runs at the uncapped rate; `FpsCap=60` still paces. **Owed:** move pacing off the scheduler thread so any cap only drops presented frames. See §F "D186" + porting-notes.md §E. | CLAMP LANDED (`port/fast3d/gfx_sdl2.cpp`, `port/src/video.c`); real fix owed |
 | D187 | **HUD ammo-type/weapon icon vanished at `-O2` (M-46 root-cause, M-47 fix + verify).** `set_rgba_redirect_generate_microcode()` (`src/game/gunfire.c:5929`, sole emitter of the bottom-right magazine icon; call sites 6057/6123/6217/6219, all `gdl = f(...)`) is `Gfx *` with **no `return`** — the decomp banner above it flags exactly this. Harmless under `-Og` (GCC left the tail-call result in `$v0`); commit `34885535` (Sep 2) switched release to `-O2` → undefined return reg → callers resume the DL from the icon's start offset and overwrite its `texSelect`/`display_image` commands with the following `gunDrawHudInteger` output. Ammo *numbers* use a separate correctly-returning path — hence the exact symptom split. Fix = one line, `return` under `#ifdef AVOID_UB` (already `-DAVOID_UB=1`, `CMakeLists.txt:207`); N64 byte-match path unchanged under `#else`. **Verified M-47 (interactive console):** `-O2` build, `-level_09` `GE_PCDUMP` 200–440 — orange clip glyph absent pre-fix, present post-fix; `framediff.py` frame 320 worst-cell dmean **21.8 → 4.0**, frame 440 **21.7 → 4.6** (converges to the pre-regression golden); other levels unaffected (change is one HUD function). Do **not** revert `34885535`. | FIXED (`src/game/gunfire.c`, `#ifdef AVOID_UB`); PR #19 |
+| D188 | **Linux `-O2` boot crash: `_Printf` passes `&args` of a by-value `va_list` param (M-48).** On the x86-64 System V ABI `va_list` is an array type → a by-value `va_list` parameter decays to a pointer, so `_Printf`'s `_Putfld(&x, &args, …)` handed `_Putfld` the address of its own local pointer slot, not the arg list; `_Putfld`'s `va_arg(*args, …)` then walked garbage. Latent at `-Og`, SIGSEGV at `-O2` (PR #9) — crash path `bossInitMainthreadData` (`boss.c:247`, D121 per-level `-m` sprintf) → `sprintf("-level_%c%c %s", …)` → `_Putfld` @ `xprintf.c`. Same UB class as D187. Windows x64 `va_list` is a scalar so `&args` worked there (Windows build unaffected — `-level_09` framediff 3/3). Fix: `#ifdef PORT` — `_Printf` takes `va_list *`, threads it straight to `_Putfld`; `src/sprintf.c` (sole PC caller; `rmon.c` excluded) passes `&args` of a real object. N64 build untouched (`#ifdef PORT`; file byte-matches ROM). | FIXED (`src/libultrare/libc/xprintf.c`, `src/sprintf.c`, `#ifdef PORT`); PR #22 |
+| D189 | **Linux stage-load stack-smash: `sub_GAME_7F0B1DDC` `tileStack[39]` vs `>= 41` traversal guard (M-48, open).** `-level_09` BUNKER1 prop setup (`proplvreset2` → `domakedefaultobj` → `setupUpdateObjectRoomPosition` → `chrpropUpdateRoomList` → `sub_GAME_7F0B21B0` → `sub_GAME_7F0B1DDC`, `src/game/stan.c:2162`): local `StandTile *tileStack[39]` is appended `tileStack[cat]=…; cat++` *before* the bail check `if (((u32)cat) >= 41) return …` — so `cat` reaches 40, writing 1–2 pointers past the array; on x86-64 (8-byte ptrs + `-fstack-protector` canary) → `*** stack smashing detected ***` abort. N64 (4-byte ptrs, no canary) tolerated it. Open question: inherent latent decomp overflow that BUNKER1 data legitimately hits (→ port-layer guard needed, `stan.c` is byte-matched), or a PC-side `StandTile` layout / `link<<3` stride mismatch driving a runaway graph walk. Triage subagent dispatched M-48. Blocks the Linux half of v0.1.0. | OPEN — root-cause in progress (M-48) |
 | D152+ | D152 addendum (M-31): static audit of every compiled-audio `osSetIntMask(OS_IM_NONE)` — **all balanced**, no unbalanced early-return. Real fixes: `sndSetSfxSlotVolume` now holds the mask across its list walk (matches its twin `sndDeactivateAllSfxByFlag`) + `sndApplyVolumeAllSfxSlot` batches the slot loop under one recursive hold (kills the fade-out lock-acquire storm); `portThreadWrapper` calls `imThreadExitRelease()` on thread exit (kills the "transient thread acquired `OS_IM_NONE` and died" leak + the pthread-id-reuse re-wedge). Steal-lock kept as backstop. | FIXED (`src/snd.c`, `port/src/libultra.c`, `#ifdef PORT`) — fade-out repro playtest-gated, not headless-verified |
 | RC3 · D167 | **Non-power-of-two texture wrap period (`docs/dev/TEXTURE-GLITCH-ANALYSIS.md` §6 RC3 — "textures repeat oddly", residual Depot-ceiling noise after D161).** The N64 RDP masks the texel coordinate of a wrapping render tile at `1<<mask`, and GE sets `mask = texDimensionToMask(dim) = ceil(log2(dim))` (`src/game/tex.c:361`), so a non-PoT tile (Depot's 65×65 / 96×48 / 56×56 room surfaces) repeats at the **next power of two**, not at its image size the way GL `GL_REPEAT` does → the pattern is squashed/stretched and the seam lands in the wrong place. fast3d never stored `masks`/`maskt` at all (`gfx_dp_set_tile` dropped them) and wrapped purely at the uploaded image dimension. **Fix (`port/fast3d/gfx_pc.cpp`, behind the existing `Video.WrapFix` knob, default OFF):** store `masks`/`maskt` on the tile; in the hoisted per-texunit pre-wrap block in `gfx_sp_tri1` (D74 block — already lifted out of the vertex loop, indexed by texunit `t` not vertex `i`), when the tile is WRAP (no CLAMP/MIRROR bit) and `1<<mask != tex_width`, fold the UV at the N64 period `1<<mask` and clamp the `[dim, 1<<mask)` overflow band (which is a TMEM smear on console, no real texels) to the last texel so it reads as an edge streak instead of a bogus early image restart. `GE_WRAPFIX=0/1` env override added (env wins over the ini, matching `Debug.FrameDump`). **Per-level captures (`-level_09`/`-30`/`-34`/`-20`, WrapFix OFF vs ON, `GE_PCDUMP` 6-frame windows):** no crashes, 6/6 frames each; on settled/comparable frames Silo is ~pixel-identical (phash 0–11), Facility 180–260 pixel-identical, Depot shows small localized texel changes on ceiling/wall cells (dmean 5–9, no structural break); the large per-run deltas are all the D117 intro-camera-pan nondeterminism, not the fix. Default kept **OFF** — no regression, but a headless structural diff can't confirm the Depot ceiling actually looks *better*; needs a human eyeball with `Video.WrapFix=1`. Default-off is byte-identical to golden (all new behaviour is inside `if (g_wrap_fix)`). D74's dead in-vertex-loop wrap block was already reworked/hoisted at M-30; this only adds the mask-period trigger. Confidence: **medium** (mechanism correct; overflow-band handling is an approximation, not exact TMEM-smear emulation; visual win unconfirmed). porting-notes.md §D. | KNOB ADDED, default OFF (`port/fast3d/gfx_pc.cpp`). Needs user visual check on Depot. |
 | D168 | **`GE_PCDUMP` / F12 PPM captures were vertically flipped — the entire source of the bogus D114/D116 "HUD/text X-mirror" (M-33, developer-confirmed).** `gfx_opengl_dump_bound_fbo` (`port/fast3d/gfx_opengl.cpp`) wrote `glReadPixels` output straight to a P6 PPM. GL framebuffer origin is bottom-left; PPM P6 is top-row-first — so every capture (and the `tools_pc/golden/` set, and every screenshot pasted into the finding log since M-6) was upside-down. On real hardware / an actual screen the game renders correctly (developer confirmed). The successive D114→D116 "shared fast3d mirror" investigations — each of which found *every probed stage clean yet the output "mirrored"* — were reading an inverted capture and pattern-matching upside-down asymmetric content (text, guards, the Nintendo logo) as "mirrored". Fix: emit PPM rows bottom-to-top. `tools_pc/golden/*.png` flipped in place to match (see `tools_pc/golden/README.md`); regenerate from a real run when convenient. **Not runtime-verified this session** (no ROM / toolchain in the migrated tree — see the migration note) — needs a fresh capture to confirm text reads normally. Confidence: **high** (mechanism is unambiguous; developer has hardware confirmation the screen is correct). porting-notes.md §D2. | FIXED (`port/fast3d/gfx_opengl.cpp`); D114 / D116 reclassified as capture-orientation artifacts (below). |

docs/porting-notes.md

@@ -606,6 +606,31 @@ silently disappears.
   (the missing primitive is often small). Needs an `-O2` build + an eyeball
   or a targeted crop diff.
 
+## D7. `va_list` is an ARRAY type on x86-64 SysV — never take `&` of a by-value `va_list` param
+
+On Windows x64 `va_list` is a scalar (`char *`), so passing a `va_list`
+parameter by value and later taking its address (`&args`) to hand a helper a
+"pointer to the list" happens to work. On the **x86-64 System V ABI**
+(Linux/macOS) `va_list` is `__va_list_tag[1]` — an array — so a by-value
+`va_list` parameter has already decayed to `__va_list_tag *`. `&args` is then
+the address of the *local pointer slot*, not the argument list, and a helper
+doing `va_arg(*args, …)` walks garbage → segfault. Latent at `-Og`, fatal at
+`-O2`.
+
+- Instance: **D188** — the IDO printf engine `_Printf`
+  (`src/libultrare/libc/xprintf.c`) passed `&args` to `_Putfld`; crashed at
+  boot on Linux `-O2` via `bossInitMainthreadData` → `sprintf`. Fix: under
+  `#ifdef PORT`, thread a real `va_list *` from the top (`sprintf.c` passes
+  `&args` of an actual `va_list` object; `_Printf` takes `va_list *` and
+  passes it straight through). `#else` keeps the ROM-matching body.
+- Rule: to share one arg-walk between functions, pass `va_list *` explicitly
+  from the outermost `va_start` scope. Never `&` a `va_list` that arrived as
+  a parameter. `va_copy` into a fresh local is the other portable option when
+  the callee must not disturb the caller's position.
+- Same "works on MinGW, UB on SysV, exposed at `-O2`" shape as D6 — suspect
+  this class for any Linux-only crash whose backtrace runs through
+  `xprintf`/`_Putfld`/`vsnprintf`-alikes.
+
 ## E. Process / method notes
 
 - Investigation loop is: reproduce → env-gated capped probe → root-cause

src/libultrare/libc/xprintf.c

@@ -35,8 +35,23 @@ static char spaces[] = "                                ";
 static char zeroes[] = "00000000000000000000000000000000";
 
 static void _Putfld(printf_struct *pf, va_list *pap, char code, char *ac);
+#ifdef PORT
+/* x86-64 System V makes va_list an array type, so a by-value va_list
+ * parameter decays to a pointer: `&args` below would be the address of
+ * _Printf's local pointer slot, not the argument list, and _Putfld()'s
+ * `va_arg(*args, ...)` then walks garbage (crash seen at
+ * bossInitMainthreadData -> sprintf -> _Putfld; same UB class as D187,
+ * latent at -Og, fatal at -O2). Thread a real va_list* through instead.
+ * The only PC caller is src/sprintf.c. The N64 build is semantically
+ * unaffected (scalar va_list) but this file byte-matches the ROM, so the
+ * change is guarded. */
+int _Printf(outfun prout, char *arg, const char *fmt, va_list *argsp)
+{
+#define args (*argsp)
+#else
 int _Printf(outfun prout, char *arg, const char *fmt, va_list args)
 {
+#endif
     printf_struct x;
     const char *s;
     char c;
@@ -96,7 +111,11 @@ int _Printf(outfun prout, char *arg, const char *fmt, va_list args)
             x.length = 'L';
             ++s;
         }
+#ifdef PORT
+        _Putfld(&x, argsp, *s, ac);
+#else
         _Putfld(&x, &args, *s, ac);
+#endif
         x.width -= x.n0 + x.num_leading_zeros + x.part2_len + x.num_mid_zeros + x.part3_len + x.num_trailing_zeros;
         if (!(x.flags & FLAGS_MINUS))
             PAD(spaces, x.width);
@@ -112,6 +131,9 @@ int _Printf(outfun prout, char *arg, const char *fmt, va_list args)
     }
     return 0;
 }
+#ifdef PORT
+#undef args
+#endif
 
 static void _Putfld(printf_struct *x, va_list *args, char type, char *buff)
 {

src/sprintf.c

@@ -12,7 +12,13 @@ int sprintf(char *dst, const char *fmt, ...) {
     s32 written;
     va_list args;
     va_start(args, fmt);
+#ifdef PORT
+    /* xprintf.c's _Printf takes a va_list* under PORT — see the comment there
+     * (x86-64 SysV va_list-is-an-array ABI). */
+    written = _Printf(proutSprintf, dst, fmt, &args);
+#else
     written = _Printf(proutSprintf, dst, fmt, args);
+#endif
     if (written >= 0) {
         dst[written] = 0;
     }