Skip to content

Issue #128 follow-on: bonus/camera GObj NULL-clears + ASan diagnostics#148

Merged
JRickey merged 1 commit intomainfrom
agent/asan-max
May 6, 2026
Merged

Issue #128 follow-on: bonus/camera GObj NULL-clears + ASan diagnostics#148
JRickey merged 1 commit intomainfrom
agent/asan-max

Conversation

@JRickey
Copy link
Copy Markdown
Owner

@JRickey JRickey commented May 6, 2026

Summary

  • Two defensive NULL-clears for BSS-resident GObj* statics that survive scene transitions and get dereferenced as freed-arena memory on the next scene's first draw — same family as PR Issue #103: free prev scene arena, widen token generation, clear long-lived GObj pointers #119's mnplayersvs.c InitPlayer fix, paste-mirrored to the 1P siblings two days ago.
  • ASan diagnostic instrumentation in the GObj/DLLink path so future trips name the caller injecting the stale GObj, not just the first-frame deref.
  • Port watchdog SIGSEGV gate so ASan's signal handler wins the race and produces its full alloc/free report instead of our backtrace-only dump.

What changed

Decomp (submodule bump on port-patches)

decomp/src/sc/sc1pmode/sc1pbonusstage.c — clear gGRCommonStruct.bonus1.interface_gobj and bonus2.interface_gobj at the top of sc1PBonusStageInitVars. These are read by sc1PBonusStageUpdateTargetInterface / ...UpdatePlatformInterface via SObjGetStruct() and written only inside sc1PBonusStageMakeTargets / ...MakePlatforms. Any Update-before-Make path on a re-entry derefs the prior bonus stage's freed GObj.

decomp/src/gm/gmcamera.c — clear gGMCameraGObj and gGMCameraStruct.{pzoom,pfollow}_fighter_gobj at the top of gmCameraMakeDefaultCamera. The {pzoom,pfollow} fields are conditionally written (only by gmCameraSetZoomFighter / ...SetFollowFighter); on scenes that don't enable zoom/follow, the prior fight's freed fighter_gobj survives. Reads at gmcamera.c:711-848 deref via DObjGetStruct/ftGetStruct.

decomp/src/sys/objman.c__asan_region_is_poisoned check at the start of gcAppendGObjToDLLinkedList (the funnel for every write into gGCCommonDLLinks[]). When a caller passes a stale GObj*, the diagnostic logs it, then the natural deref one line down trips ASan with a use-after-free report whose stack frame names the caller.

decomp/src/sys/objdisplay.c — defensive poison-check at the gcCaptureTaggedGObjs iteration site, throttled to 8 reports per frame.

Both diagnostics gated on __SANITIZE_ADDRESS__ / __has_feature(address_sanitizer). Zero impact on release builds. Pattern mirrors libultraship/src/fast/interpreter.cpp:50-56.

Port

port/port_watchdog.cpp — skip installing the SIGSEGV / SIGBUS handlers when ASan is compiled in. ASan's preinit handler then wins, and we get the full alloc/free trace instead of our register dump. SIGILL / FPE / ABRT stay ours (SIGABRT chains after ASan has already printed its report under abort_on_error=1).

Decomp submodule bump

b47c28b59ffaa7e769 on port-patches. Two commits:

  1. Issue #128: NULL-clear bonus-stage and camera GObj BSS statics on scene init
  2. Issue #128: ASan diagnostics for stale GObj injection into gGCCommonDLLinks[]

Background

Failure mode is documented at docs/issue128_investigation_2026-05-03.md. Two reporter paths converge on sc1PBonusStageStartScene's first-draw gcCaptureTaggedGObjs walking a stale GObj on gGCCommonDLLinks[]:

  • Polygon Fighters as Captain Falcon (other reporter, Windows minidump matched gfx_step in libultraship Fast3D)
  • 1P loss → VS-mode entry (this branch's owner)

PR #119's Lead 3 fix already covered the obvious VS-side and 1P-side InitPlayer slot fields. PR #144's Lead 1 fix added portPackedDisplayListCacheDeleteRange. Verified those are in place but the bug still reproduces — the BSS leak is in non-mnplayers/ code. Static analysis (parallel sub-agent sweep over it/, sc/, gm/, gr/, mn/, mv/, ft/) flagged gGRCommonStruct.bonus*.interface_gobj and gGMCameraGObj/pzoom/pfollow fields as the most-likely remaining leakers matching the issue #128 failure shape.

Status

The two NULL-clears are defensive — they fix a real null-hygiene gap that matches the failure shape, but the active reproducer hasn't been re-run post-fix to confirm. The ASan diagnostic infrastructure ships alongside so the next ASan repro names the leaker definitively if these two fixes don't fully close the issue.

Test plan

  • Build clean on Linux Debug + ASan (verified locally)
  • Build clean on Windows Release (CI)
  • Build clean on macOS Release (CI)
  • Reproduce the failing path with the new build (1P run with loss → bonus stage entry, or Polygon Fighters as Falcon) and confirm no crash
  • If still crashes under ASan, the new diagnostic in gcAppendGObjToDLLinkedList will name the remaining leaker for a follow-up PR

🤖 Generated with Claude Code

Two more BSS-resident GObj* statics that survive scene transitions and
get dereferenced as freed-arena memory on the next scene's first draw —
same family as PR #119's mnplayersvs.c InitPlayer fix. Static analysis
of the post-PR-119 codebase identified these as the most-likely
remaining leakers in the issue #128 / sc1PBonusStageStartScene crash
trace. Both fixes land in decomp (submodule bump):

- gGRCommonStruct.bonus1/bonus2.interface_gobj — read by
  sc1PBonusStageUpdateTargetInterface / ...UpdatePlatformInterface
  via SObjGetStruct() before the next bonus stage's Make* functions
  write fresh values. NULL-cleared at the top of
  sc1PBonusStageInitVars.

- gGMCameraGObj + gGMCameraStruct.{pzoom,pfollow}_fighter_gobj —
  the {pzoom,pfollow} fields are conditionally written by
  gmCameraSetZoomFighter/...SetFollowFighter; on scenes that don't
  enable those features, the prior fight's freed fighter_gobj
  survives and is read at lines 711-848 via DObjGetStruct/ftGetStruct.
  NULL-cleared at the top of gmCameraMakeDefaultCamera.

Plus port-side ASan diagnostic infrastructure (also in decomp bump):

- objman.c gcAppendGObjToDLLinkedList — poison-check this_gobj at
  the funnel for every write into gGCCommonDLLinks[]. Catches the
  injection AT THE CALLER, not at first-frame discovery, so ASan's
  use-after-free report stack names the function holding the stale
  BSS handle.

- objdisplay.c gcCaptureTaggedGObjs — defensive poison-check at
  the iteration site, throttled.

And one outer change:

- port_watchdog.cpp — skip installing the SIGSEGV/SIGBUS handlers
  when __SANITIZE_ADDRESS__ is defined, so ASan's preinit handler
  wins the race and produces its full report (alloc/free traces,
  shadow map). SIGILL/FPE/ABRT remain ours; SIGABRT chains after
  ASan has already printed its report (abort_on_error=1 path).

Decomp submodule bump:
  b47c28b59 → ffaa7e769 on port-patches

Closes nothing definitive yet — keeps issue #128 open until the
ASan diagnostics confirm the fix on a live repro. The two NULL-clear
sites are defensive: they match the issue #128 failure shape and the
PR #119 fix template, but the active reproducer hasn't been re-run
post-fix to confirm.
@JRickey JRickey merged commit bbca2cb into main May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant