macos/arm64: review fixes — NULL-safe portAddrIsInWindow, drop duplicate MSAA clamp and wrong terminator

- port_addr.h: portAddrIsInWindow() now rejects the low 64 KiB of the
  window. With PORT_ADDR_BASE == 0 that range contains NULL; the previous
  version let NULL through on Windows/Linux, regressing langGet()'s
  cleared-bank guard (the old numeric bound rejected < 0x10000). Matches
  the old bounds on every platform. Self-test covers NULL, the low guard,
  DRAM and above-window.
- gfx_opengl.cpp: revert to origin/macos. upstream already clamps MSAA to
  GL_MAX_SAMPLES (D405 / #106); the branch's second clamp and its comments
  were a duplicate. MACOS-MSAA-CLAMP finding removed.
- chr.c: drop the branch's crouched_rifle_firing_animation_group1 {0}
  terminator. GCC places groupA below group1, so it never resolved groupA
  and did not fix the crouched-rifle crash. Replaced by #120 (next commit).
- boss.c: guard the rspReplyMsg re-base with #ifdef PORT (the file has no
  PORT_N64PTR identity fallback for the N64 build).
- findings.md: correct the objDeform vtxslot write-up (host pointers, not
  u32 N64 addresses), give accurate numbers for unguarded src/ edits, and
  record why the terminator was replaced.

docs/dev/findings-index.csv

@@ -297,4 +297,3 @@ MACOS-CAST-CENSUS,FIXED,"macOS/arm64: six more pointer truncations, found by a s
 MACOS-TEXALIGN,FIXED,"macOS/arm64: texReadUncompressed() aligned its destination through u32 ((u32 *)(((u32)dst + 0xf) & ~0xf) x3), truncating a host pointer and then writing throug…","FIXED defensively (image.c), latent not live."
 MACOS-RENDER-CRASH,OPEN,"Intermittent renderer crash, level-agnostic, observed once on Dam:","OPEN — observed once, not investigated."
 MACOS-ADDR-STRICT,FIXED,PORT_ADDR_STRICT: a debug gate that catches the whole MACOS-SFX-BOUND through MACOS-RENDER-CRASH family at the conversion rather than at the fault.,FIXED (new CMake option + port_addr.h/port_addr.c).
-MACOS-MSAA-CLAMP,FIXED,MSAA=8 produced a completely BLACK screen —,FIXED (port/fast3d/gfx_opengl.cpp) —

docs/dev/findings.md

@@ -705,7 +705,6 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
 | MACOS-TEXALIGN | **macOS/arm64: `texReadUncompressed()` aligned its destination through `u32`** (`(u32 *)(((u32)dst + 0xf) & ~0xf)` x3), truncating a host pointer and then writing through it — but the path is **unreachable on PC** (a `GE_D303` probe over a full level boot recorded zero calls; textures come from the pre-converted `pccg` sidecars) | FIXED defensively (`image.c`), latent not live. Group-C census otherwise clean: `init()` is dead on PC, `rsp.c`'s XOR toggle already has a `PLATFORM_64BIT` branch, `objecthandler_2.c`'s `name` is an int-in-a-pointer, `animationtable_data.h`'s 25 warnings are placeholders rewritten by `initResolveAnimTable()` via `ANI_ADDR`. |
 | MACOS-RENDER-CRASH | **Intermittent renderer crash, level-agnostic, observed once on Dam: SIGSEGV in `gfx_sp_tri1` -> `gfx_texture_cache` with a garbage fault address (`0x16d5bbd636173b9e`).** Did not reproduce in 8 subsequent Dam runs, and is not attributable to the MACOS-CAST-CENSUS changes landed in the same session. Not a known signature (D233 was `gfx_sp_tri1` but a visibility bug, not a crash) | **OPEN — observed once, not investigated.** Garbage (non-address-shaped) fault value suggests a corrupted texture-cache node or a race rather than the MACOS-SFX-BOUND through MACOS-TEXALIGN truncation family. Next: run with `PORT_ADDR_STRICT=ON` to rule the address family out, then instrument the texture cache. |
 | MACOS-ADDR-STRICT | **`PORT_ADDR_STRICT`: a debug gate that catches the whole MACOS-SFX-BOUND through MACOS-RENDER-CRASH family at the conversion rather than at the fault.** Off by default, compiled out entirely; when on, `portN64ToHost()` validates its argument against the mapped regions and reports offenders with a backtrace | FIXED (new CMake option + `port_addr.h`/`port_addr.c`). Negative self-test (`GE_ADDRSTRICT_SELFTEST=1`) proves the detector fires. Zero reports across 6 levels, 5 under full-control input. |
-| MACOS-MSAA-CLAMP | **MSAA=8 produced a completely BLACK screen** — `GL_MAX_SAMPLES` is 4 on this GPU, and nothing ever clamped the requested level, so `glRenderbufferStorageMultisample` left the renderbuffer unallocated -> incomplete FBO. `video.c`'s own comment claims MSAA "snaps down to the highest supported level"; it did not | FIXED (`port/fast3d/gfx_opengl.cpp`) — clamp to `GL_MAX_SAMPLES`, log the snap once. Also fixed two index-vs-GL-name bugs in the same file (`resolve_msaa_color_buffer`, `copy_framebuffer`). Verified MSAA 1/2/4/8 all render. |
 
 
 Phase 2 replaced the Phase-1 demo loop with the real `mainproc()` on real OS
@@ -15617,7 +15616,7 @@ if (newvertices != NULL) { ... newvertices[j] = ...; }       /* test passes; sto
 **Lesson.** Both defects were invisible to a `-Wint-to-pointer-cast` census: defect 1 has no cast at all, and defect 2 was written as `(T *)(uintptr_t)x`, which silences the warning. A warning sweep finds the *producer* side of this class; the consumer side needs a type-driven audit of which 32-bit fields hold addresses.
 **The two `vtxstore_allocate()` callers, and `objDeform()`, are the same class.** Defect 1's failure-signalling shape (`p = PORT_N64PTR(T, f()); if (p != NULL)`) appears at three more sites, all re-based through `PORT_N64PTR` so a returned `0` still becomes `NULL`:
 - `src/game/chr.c:3609` and `:3626` — the two `chrCreateBloodStain()` callers (this finding's own function). `vtxstore_allocate()` returns `0` when the store is exhausted.
-- `src/game/propobj.c` `objDeform()` — `newverts = PORT_N64PTR(Vertex, vtxstore_allocate(...))`, plus every read of the `*vtxslot` field (a `u32` N64 address) through `PORT_N64PTR(Vertex, *vtxslot)`, for the same reason the `CollisionRelatedNode` cast in defect 2 was wrong.
+- `src/game/propobj.c` `objDeform()` — `newverts = PORT_N64PTR(Vertex, vtxstore_allocate(...))`, plus every read of `*vtxslot` through `PORT_N64PTR(Vertex, *vtxslot)`. `*vtxslot` is a `Vertex *` slot in the model RW pool that holds host pointers (`newverts`, `rodata->Vertices`), so the re-base is a lossless round trip for any pointer inside the window (`PORT_ADDR_BASE` is 4 GiB-aligned); it does not change behaviour and could be dropped, but it is harmless.
 These are pointer-width/ABI only (semantics-preserving, identity at `PORT_ADDR_BASE == 0`); the `src/` hunks were carried under this finding's `#ifdef PORT` guard work and are called out here so they are not mistaken for behaviour changes.
 
 ## MACOS-CAST-CENSUS — macOS/arm64: six pointer truncations found by census, not by crashing
@@ -15632,8 +15631,8 @@ These are pointer-width/ABI only (semantics-preserving, identity at `PORT_ADDR_B
 So: **after landing any fix in this family, grep the repository for the distinctive fragment of the line you changed and confirm every other occurrence is already in the fixed form.** That one step would have found four of these six for free.
 **The most interesting one is silent.** `bgRoomCalcBB()` (`bg.c:5213`) truncated its loop bound, so on arm64 `vertices < bound` was false on the first iteration, the body never executed, `limits` kept its sentinel init, and lines 5242-5247 wrote `minbounds = pos + 0x7fff`, `maxbounds = pos - 0x7fff` — an inverted bounding box for **every room of every level**. No crash. Those bounds feed room centres (`bg.c:5155`), portal corners (`bg.c:1468`) and `chrprop.c:619`. Worth checking against open **D249** (distant geometry dropping out on Streets/Egyptian).
 **Not verified.** The two MP-only fixes (`mp_respawn_handler`'s stan pointer, and the intro-record walk) are unreachable from the solo sweeps and need a multiplayer respawn.
-**Guarding status (honest note).** Only the `bondview2.c:8762` (`StandTile *start_stan`, `MACOS-CAST-CENSUS`) and `bg.c:5213` (`bgRoomCalcBB`) hunks are inside an `#ifdef PORT` block. The `intro_record` strides in `bondview_r.c`/`bondview2.c` are **unguarded**: the change is `(s32)` → `(uintptr_t)` in place, which is the identity under N64's 32-bit pointers and therefore semantics-preserving, matching the existing house style for these files (`main` already has unguarded `(uintptr_t)` pointer arithmetic in `bondview_r.c:392`, `bondview2.c:1374-1399`, `image.c:2406,2538`). The owner's branch ground rule is stricter (`#ifdef PORT` on every `src/` edit); converting these to guarded `#ifdef PORT` / `#else` pairs is left as a reviewer-flagged follow-up rather than done unilaterally.
-**Terminator note (documented here because it landed in the same `chr.c` hunks).** While in `chr.c` this pass also added a `#ifdef PORT` `{0}` terminator to `crouched_rifle_firing_animation_group1`. Every other `*_firing_animation_groupN[]` is zero-terminated and `initResolveAnimGroupTable()` walks to that `0`; without one here the walk runs into the next global (`crouched_rifle_firing_animation_groupA`) and over-counts `len`. This is the same layout/linker-adjacency class as `main`'s `D420`/PR #120, which fixes the same table independently; the branch's copy is a `#ifdef PORT`-only guard and will be dropped in favour of the `main` fix when it merges.
+**Guarding status.** Not every `src/` hunk on this branch is inside `#ifdef PORT`. Against `origin/macos` there are about 190 unguarded lines that call `PORT_N64PTR`/`portN64ToHost` inline. Every file that does so defines `PORT_N64PTR(T, x)` as `((T *)(x))` for the N64 build, so the N64 text compiles to the original cast. There are also about 50 unguarded `(s32)`/`(u32)` → `(uintptr_t)` widenings (`prop.c` x20, `bondview_r.c`/`bondview2.c` `intro_record` x20, `bg.c` x4, `music.c` x3, `image.c`, `cseq.c`), which are the identity where pointers are 32-bit. A few cast-only edits are also unguarded (`image.c:2478`, `options.c:3493`, `bondview2.c:7998`, `model.c:6251-6273`). `main` already uses this unguarded style in these files (`bondview_r.c:392`, `bondview2.c:1374-1399`, `image.c:2406,2538`), but the branch rule asks for `#ifdef PORT` with the N64 text unchanged. The open question for review is whether inline `PORT_N64PTR` with an identity N64 fallback counts as meeting that rule, or whether each site should become an `#ifdef PORT` / `#else` pair.
+**Crouched-rifle table.** An earlier version of this branch added a `{0}` terminator to `crouched_rifle_firing_animation_group1`. It was wrong: GCC places `groupA` *below* `group1` (confirmed with `nm`), so the terminator never resolved `groupA` and the crouched-rifle `firing_animation_groups` crash remained. It has been replaced by a cherry-pick of `main`'s fix from PR #120 (`D420`, one array with `groupA` as an alias of `&group1[1]`).
 
 ## MACOS-TEXALIGN — macOS/arm64: latent truncation in texReadUncompressed (unreachable on PC)
 `texReadUncompressed()` (`image.c`) aligned its destination through `u32` — `(u32 *)(((u32)dst + 0xf) & ~0xf)` plus `u16`/`u8` siblings — truncating a host pointer and then writing through it. On arm64 `dst` is `~0x1000_70xx_xxxx`, so the aligned pointers would be unbased `0x70xx_xxxx`.

port/fast3d/gfx_opengl.cpp

@@ -1222,44 +1222,11 @@ static int gfx_opengl_create_framebuffer() {
     return i;
 }
 
-/* video.c documents MSAA as "snaps down to the highest supported level", but
- * nothing ever queried the limit: gfx_msaa_level went to
- * glRenderbufferStorageMultisample() verbatim, and a request above
- * GL_MAX_SAMPLES leaves the renderbuffer unallocated -> incomplete FBO ->
- * a completely BLACK screen (MSAA=8 on Apple Silicon, where GL_MAX_SAMPLES
- * is 4). Clamp here, where the GL limit actually matters. */
-static uint32_t gfx_opengl_clamp_msaa(uint32_t level)
-{
-    static GLint max_samples = 0;
-    static uint32_t warned_level = 0;
-
-    if (level <= 1) {
-        return 1;
-    }
-    if (max_samples == 0) {
-        glGetIntegerv(GL_MAX_SAMPLES, &max_samples);
-        if (max_samples < 1) {
-            max_samples = 1;
-        }
-    }
-    if ((GLint)level > max_samples) {
-        if (warned_level != level) {
-            sysLogPrintf(LOG_WARNING,
-                         "MSAA: %u samples requested but GL_MAX_SAMPLES is %d; using %d",
-                         level, (int)max_samples, (int)max_samples);
-            warned_level = level;
-        }
-        return (uint32_t)max_samples;
-    }
-    return level;
-}
-
 static void gfx_opengl_update_framebuffer_parameters(int fb_id, uint32_t width, uint32_t height, uint32_t msaa_level,
                                                      bool opengl_invert_y, bool render_target, bool has_depth_buffer,
                                                      bool can_extract_depth) {
     Framebuffer& fb = framebuffers[fb_id];
 
-    msaa_level = gfx_opengl_clamp_msaa(msaa_level);
     width = max(width, 1U);
     height = max(height, 1U);
 
@@ -1373,12 +1340,7 @@ void gfx_opengl_resolve_msaa_color_buffer(int fb_id_target, int fb_id_source) {
     glBindFramebuffer(GL_READ_FRAMEBUFFER, fb_src.fbo);
     glBlitFramebuffer(0, 0, fb_src.width, fb_src.height, 0, 0, fb_dst.width, fb_dst.height, GL_COLOR_BUFFER_BIT,
                       GL_NEAREST);
-    /* Restore whatever was bound before the resolve. `current_framebuffer` is
-     * an INDEX into framebuffers[], NOT a GL framebuffer name -- binding it
-     * directly named an unrelated GL object (or nothing) and left the wrong
-     * target bound for the rest of the frame. With MSAA on that produced a
-     * completely black screen. Use the same fb_id==0 -> GL name 0 rule as
-     * gfx_opengl_bind_framebuffer above. */
+    /* current_framebuffer is an INDEX into framebuffers[], not a GL name; fb 0 is the window's default framebuffer (name 0). */
     glBindFramebuffer(GL_FRAMEBUFFER, current_framebuffer == 0 ? 0 : framebuffers[current_framebuffer].fbo);
     glEnable(GL_SCISSOR_TEST);
 }
@@ -1443,10 +1405,6 @@ void gfx_opengl_copy_framebuffer(int fb_dst, int fb_src, int left, int top, bool
 
     glBlitFramebuffer(srcX0, srcY0, srcX1, srcY1, dstX0, dstY0, dstX1, dstY1, GL_COLOR_BUFFER_BIT, GL_NEAREST);
 
-    /* Same index-vs-name bug as the MSAA resolve above: for current_framebuffer
-     * == 0 this bound framebuffers[0].fbo, which is a generated but
-     * attachment-less FBO (incomplete on a strict GL implementation) -- the
-     * window's default framebuffer is GL name 0. */
     glBindFramebuffer(GL_FRAMEBUFFER, current_framebuffer == 0 ? 0 : framebuffers[current_framebuffer].fbo);
 
     glReadBuffer(GL_BACK);

port/include/port_addr.h

@@ -134,11 +134,18 @@ static inline uint32_t portHostToN64(const void *p)
 /* Is a host pointer inside the N64 address window (DRAM / cart / stacks)?
  * Use this instead of hardcoded numeric bounds: the window is shifted by
  * PORT_ADDR_BASE on arm64, so an absolute bound that is correct on x86_64
- * silently rejects every real pointer there. */
+ * silently rejects every real pointer there.
+ *
+ * The low PORT_ADDR_LOW_GUARD bytes of the window are rejected: nothing is
+ * mapped there, and with PORT_ADDR_BASE == 0 that range contains NULL, which
+ * is exactly what the callers' plausibility checks exist to catch (e.g.
+ * langGet() on a cleared bank). */
+#define PORT_ADDR_LOW_GUARD 0x10000u
 static inline int portAddrIsInWindow(const void *p)
 {
     uintptr_t v = (uintptr_t)p;
-    return v - (uintptr_t)PORT_ADDR_BASE < (uintptr_t)PORT_ADDR_WINDOW;
+    uintptr_t off = v - (uintptr_t)PORT_ADDR_BASE;
+    return off >= PORT_ADDR_LOW_GUARD && off < (uintptr_t)PORT_ADDR_WINDOW;
 }
 
 /* Cast-site helper for game code (the D3x ABI/layout class): reads a 32-bit

port/src/port_addr.c

@@ -58,6 +58,14 @@ static void portAddrSelfTest(void)
                         portN64ToHost(0x10456789u) == (void *)(B + 0x10456789u));
     ok &= portAddrCheck("cfb_16 at base+DRAM_V1",
                         (uintptr_t)cfb_16 == B + 0x70000000u);
+    ok &= portAddrCheck("in-window: NULL rejected",
+                        !portAddrIsInWindow(NULL));
+    ok &= portAddrCheck("in-window: low guard",
+                        !portAddrIsInWindow((void *)(B + PORT_ADDR_LOW_GUARD - 1u)));
+    ok &= portAddrCheck("in-window: DRAM V1",
+                        portAddrIsInWindow((void *)(B + 0x70001234u)));
+    ok &= portAddrCheck("in-window: above window",
+                        !portAddrIsInWindow((void *)(B + (uintptr_t)PORT_ADDR_WINDOW)));
     if (g_portUseImageRel) {
         ok &= portAddrCheck("host->n64 image",
                             portHostToN64((void *)(g_portImageBase + 0x1234u)) == 0x40001234u);

src/boss.c

@@ -833,7 +833,13 @@ void bossMainloop(void)
                             }
 
                             rspReplyMsg = (s32)(&localGfxDoneMsg);
+#ifdef PORT
+                            /* MACOS-ADDR-WINDOW: rspReplyMsg is an s32 holding a
+                             * truncated window pointer; re-base it. */
                             rspGfxTaskStart(firstGdl, gdl, 0, PORT_N64PTR(s32, rspReplyMsg));
+#else
+                            rspGfxTaskStart(firstGdl, gdl, 0, (s32*)rspReplyMsg);
+#endif
 
                             pendingGfx++;
                             memaSingleDefragPass();

src/game/chr.c

@@ -896,16 +896,6 @@ struct anim_group_info *ptr_doubles_firing_animation_groups[] = {
 
 struct weapon_firing_animation_table crouched_rifle_firing_animation_group1[] = {
     { PTR_ANIM_fire_kneel_right_leg, 27.0, 0, 0, 0, -1.0, 35.0, 75.0, -1.0, -1.0, 31.0, 75.0, 0.87266463, -0.69813174, 0.90757126, -0.69813174, 1.5, 1.5 },
-#if defined(PORT)
-    /* MACOS-ADDR-WINDOW: every other *_firing_animation_groupN[] is zero-terminated;
-     * initResolveAnimGroupTable() walks to that 0, so without one here it runs
-     * into the next global (crouched_rifle_firing_animation_groupA) and
-     * over-counts `len`. On N64 the 72-byte stride happens to stop it; on PC
-     * the 80-byte stride runs several globals deep, so `len` allows a
-     * `next_anim` past the array and the guard-attack path reads a bogus
-     * animation. N64 build unchanged. */
-    {0, 0.0, 0, 0, 0, -1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0}
-#endif
 };
 
 struct weapon_firing_animation_table crouched_rifle_firing_animation_groupA[] = {