fix(oot): try a full heap's OptimizeChangeBank allocations again #2451

Merged
hckrman101 merged 2 commits from fix/heap-retry-1697 into main 2026-11-11 21:00:51 +00:00
hckrman101 commented 2026-11-11 19:41:23 +00:00 (Migrated from github.com)

Title: fix(oot): try a full heap's OptimizeChangeBank allocations again


The game can crash a few frames after loading a save when one of its small model heaps runs out of room (#1697, still
crashing on 2.1.0). The game uses the failed allocation anyway and writes through a null pointer.

What changes

The game's OptimizeChangeBank thread rebuilds a model's material block (f_804566e0) with memory from heaps its caller
names: small heaps of their own per model resource ("Model/....bfres", tens of KB), children of a large one. It checks
none of these allocations:

  • CPU buffers. f_804566e0 makes two tryAllocs of CPU buffers, sized for the model (lr 0x80456910 for the MatBlock's
    +200, lr 0x80456da4 for +168). A null result makes the bump allocator built on it hand out guest address 0. That is
    the crash in #1697's logs: thread OptimizeChangeBanc, a write to guest 0x8 in f_8044cb30, lr 0x80456d48, with the
    allocator's base word null in the register dump.
  • GPU memory. f_804566e0's two calls of f_80522050 (lr 0x80456f1c, 0x80457098) leave the block with no memory pool,
    and the block is written to guest 0 two calls later. This is the crash TomShafford found at backflip + attack.

Each of these is now tried again in the heap's parents up to the root, the first with room; the GPU memory then also in
the game's default heap (host/oot_heap_retry.c). The parents have gigabytes free, so the retry has room.

  • Scope. Only these call sites are changed (nx2/oot_patches.py, the supported build only). f_80522050 itself and
    its ~30 other callers are as shipped. Nothing waits or sleeps.
  • Freeing. The MatBlock's destructor frees the two CPU buffers through the game's free by address (f_8021dea0). That
    function walks the heap tree for the heap whose range holds the block, so a block from a parent goes back to that
    parent. This was read from the lifted code, not traced in a run.
  • Leak watch. Every block moved to a parent is counted per site, and the running totals stay in the log after the
    first 60 lines, so a leak over a session would show.
  • Logs. Each failure is logged with the heap's name, size and free space.

New switches:

  • HOS_GPU_ALLOC_RETRY=0 turns the retry off (as shipped).
  • HOS_GPU_ALLOC_TEST_FAIL=<n> or <gpu>,<cpu> makes the heap refuse the first allocations at these sites, for testing.

Credit

The GPU part is ported from TomShafford's Hookshot fork:
55d68e8cd6 and
65c1998222 (GPL-3.0), credited in the commits. Two things
changed from it after review: it is narrowed to these two call sites, and its waits are dropped (its own log showed they
did not help). The CPU part, the place #1697's log points to, is new in this PR.

Open question for #1697

Why the reporter's heap fills, and why only since 2.0.12, is not known. 2.0.12 is when the render target pool's 512 MB
minimum arrived (hos_rt_pool_size in host/nn_applet.c), and the Graphics heap grows by the same amount. Asking the
reporter to try HOS_RT_POOL_MB=305 (2.0.11's size) on 2.1.0 would show whether that alone stops the crash.

Verified

The game runs below used the first version: the first CPU buffer, and f_80522050 wrapped for every caller with waits.
The review changes (the second buffer, call-site-only GPU retry, no waits, per-site totals) are checked game-free only.

On Linux, RTX 3090, headless, on a copy of a Kokiri Forest save:

  • Refused 50 of each. HOS_GPU_ALLOC_TEST_FAIL=50 made the heap refuse the first 50 GPU and 50 CPU model
    allocations. All of them were made in the parent heap ("ModuleSystem", "NinVirtualAddressHeap"), and the game reached
    Kokiri Forest and played normally (screenshot).
  • One buffer refused, retry off. HOS_GPU_ALLOC_RETRY=0 HOS_GPU_ALLOC_TEST_FAIL=0,1 refuses one model buffer and
    kills the game at boot. The null buffer is written through, and the game later fails with "indirect call to an
    address with no function at 0x698".
  • One buffer refused, retry on. The same refusal is made in "NinVirtualAddressHeap" and the game plays.
  • Game-free test. tests/test_oot_heap_retry.py runs the retries on mock sead heaps:
    • a success is left alone;
    • a failure goes to a parent with room, then the default heap, then fails;
    • nothing ever sleeps, and only x0 changes, flags included;
    • both CPU sites, the off switch and the test switch;
    • nx2/oot_patches.py edits the four call sites exactly once and leaves f_80522050's other callers and f_80522050
      itself alone; a second run changes nothing; it runs on the supported build only.
  • Full test suite. python -I -B tests/run_all.py passes on this branch alone, apart from 3 tests this PC cannot
    run (its Python lacks zstandard and tkinter).
  • Compiles. host/oot_heap_retry.c compiles without warnings for Linux (clang) and Windows (llvm-mingw).

Not tested

  • #1697's own crash. It needs the reporter's save, and no save here filled a heap. The retry was only exercised
    through allocations refused on purpose. A full heap whose parents are full too would still fail, now with log lines
    that name the heaps.
  • TomShafford's backflip + attack crash. Not reproduced here.
  • The review version in the game. Not built or run. The patched call is checked by compiling it against a64.h.
  • The freeing path. Read from the lifted code, not traced in a run.
  • The third allocation. The temporary work-heap buffer at lr 0x804567a8 (freed in the same function) is not covered.
  • Heap accounting. The model heap's own accounting (MatBlock +680) does not count blocks moved to a parent.
  • Other platforms. Windows, and AMD or Intel cards.
Title: fix(oot): try a full heap's OptimizeChangeBank allocations again --- The game can crash a few frames after loading a save when one of its small model heaps runs out of room (#1697, still crashing on 2.1.0). The game uses the failed allocation anyway and writes through a null pointer. ### What changes The game's OptimizeChangeBank thread rebuilds a model's material block (f_804566e0) with memory from heaps its caller names: small heaps of their own per model resource ("Model/....bfres", tens of KB), children of a large one. It checks none of these allocations: - **CPU buffers.** f_804566e0 makes two tryAllocs of CPU buffers, sized for the model (lr 0x80456910 for the MatBlock's +200, lr 0x80456da4 for +168). A null result makes the bump allocator built on it hand out guest address 0. That is the crash in #1697's logs: thread OptimizeChangeBanc, a write to guest 0x8 in f_8044cb30, lr 0x80456d48, with the allocator's base word null in the register dump. - **GPU memory.** f_804566e0's two calls of f_80522050 (lr 0x80456f1c, 0x80457098) leave the block with no memory pool, and the block is written to guest 0 two calls later. This is the crash TomShafford found at backflip + attack. Each of these is now tried again in the heap's parents up to the root, the first with room; the GPU memory then also in the game's default heap (`host/oot_heap_retry.c`). The parents have gigabytes free, so the retry has room. - **Scope.** Only these call sites are changed (`nx2/oot_patches.py`, the supported build only). f_80522050 itself and its ~30 other callers are as shipped. Nothing waits or sleeps. - **Freeing.** The MatBlock's destructor frees the two CPU buffers through the game's free by address (f_8021dea0). That function walks the heap tree for the heap whose range holds the block, so a block from a parent goes back to that parent. This was read from the lifted code, not traced in a run. - **Leak watch.** Every block moved to a parent is counted per site, and the running totals stay in the log after the first 60 lines, so a leak over a session would show. - **Logs.** Each failure is logged with the heap's name, size and free space. New switches: - `HOS_GPU_ALLOC_RETRY=0` turns the retry off (as shipped). - `HOS_GPU_ALLOC_TEST_FAIL=<n>` or `<gpu>,<cpu>` makes the heap refuse the first allocations at these sites, for testing. ### Credit The GPU part is ported from TomShafford's Hookshot fork: [55d68e8cd6](https://github.com/TomShafford/Hookshot/commit/55d68e8cd6) and [65c1998222](https://github.com/TomShafford/Hookshot/commit/65c1998222) (GPL-3.0), credited in the commits. Two things changed from it after review: it is narrowed to these two call sites, and its waits are dropped (its own log showed they did not help). The CPU part, the place #1697's log points to, is new in this PR. ### Open question for #1697 Why the reporter's heap fills, and why only since 2.0.12, is not known. 2.0.12 is when the render target pool's 512 MB minimum arrived (`hos_rt_pool_size` in `host/nn_applet.c`), and the Graphics heap grows by the same amount. Asking the reporter to try `HOS_RT_POOL_MB=305` (2.0.11's size) on 2.1.0 would show whether that alone stops the crash. ### Verified The game runs below used the first version: the first CPU buffer, and f_80522050 wrapped for every caller with waits. The review changes (the second buffer, call-site-only GPU retry, no waits, per-site totals) are checked game-free only. On Linux, RTX 3090, headless, on a copy of a Kokiri Forest save: - **Refused 50 of each.** `HOS_GPU_ALLOC_TEST_FAIL=50` made the heap refuse the first 50 GPU and 50 CPU model allocations. All of them were made in the parent heap ("ModuleSystem", "NinVirtualAddressHeap"), and the game reached Kokiri Forest and played normally (screenshot). - **One buffer refused, retry off.** `HOS_GPU_ALLOC_RETRY=0 HOS_GPU_ALLOC_TEST_FAIL=0,1` refuses one model buffer and kills the game at boot. The null buffer is written through, and the game later fails with "indirect call to an address with no function at 0x698". - **One buffer refused, retry on.** The same refusal is made in "NinVirtualAddressHeap" and the game plays. - **Game-free test.** `tests/test_oot_heap_retry.py` runs the retries on mock sead heaps: - a success is left alone; - a failure goes to a parent with room, then the default heap, then fails; - nothing ever sleeps, and only x0 changes, flags included; - both CPU sites, the off switch and the test switch; - `nx2/oot_patches.py` edits the four call sites exactly once and leaves f_80522050's other callers and f_80522050 itself alone; a second run changes nothing; it runs on the supported build only. - **Full test suite.** `python -I -B tests/run_all.py` passes on this branch alone, apart from 3 tests this PC cannot run (its Python lacks zstandard and tkinter). - **Compiles.** `host/oot_heap_retry.c` compiles without warnings for Linux (clang) and Windows (llvm-mingw). ### Not tested - **#1697's own crash.** It needs the reporter's save, and no save here filled a heap. The retry was only exercised through allocations refused on purpose. A full heap whose parents are full too would still fail, now with log lines that name the heaps. - **TomShafford's backflip + attack crash.** Not reproduced here. - **The review version in the game.** Not built or run. The patched call is checked by compiling it against `a64.h`. - **The freeing path.** Read from the lifted code, not traced in a run. - **The third allocation.** The temporary work-heap buffer at lr 0x804567a8 (freed in the same function) is not covered. - **Heap accounting.** The model heap's own accounting (MatBlock +680) does not count blocks moved to a parent. - **Other platforms.** Windows, and AMD or Intel cards.
Commenting is not possible because the repository is archived.
No description provided.