nvn: answer memory-pool calls made with no pool #510

Merged

nicholasgower merged 1 commit from fix/nvn-null-pool into main

2026-11-08 08:19:57 +00:00

nicholasgower commented

2026-11-08 07:25:02 +00:00

(Migrated from github.com)

Fixes #509. Related tracking issue: #287.

What the crash was

While Kakariko Village changed from day to night, the OptimizeChangeBanc thread called nvnMemoryPoolGetFlags(NULL). The HLE getter read flags at A64_MEM + 4 (guest 0x4) and took an access violation:

[hos OptimizeChangeBanc] CRASH: exception 0xc0000005 (read 0x100000000004, guest 0x4) at hookshot.exe+0x1a53b5dd

hookshot.exe+0x1a53b5dd is n_nvnMemoryPoolGetFlags+0xd. The guest caller is the texture-building helper f_80905130: it passes the pool pointer [x21] to the storage setter and to nvnTextureInitialize (which already handles a null pool), then asks the same [x21] = 0 for its flags. This is a texture with no storage pool being prepared during the banc change.

P(0) returns NULL for a zero argument, but ((nvn_pool *)NULL)->flags is undefined behaviour, so the compiler is free to fold the check away — and did, reading A64_MEM + 4. A guard has to come before the dereference.

Changes

Add no_pool(), following no_builder() from #284/#287.
Guard the memory-pool accessors before their first dereference:
- nvnMemoryPoolGetFlags / GetSize / GetBufferAddress return 0
- nvnMemoryPoolMap returns 0
- nvnMemoryPoolInvalidateMappedRange, Finalize, MapVirtual do nothing
Add a one-shot log line naming the call and the guest LR, like no_builder.
Add tests/test_nvn_null_pool.py, a game-free regression test that extracts the actual getter bodies from host/nvn_native.c, compiles them at -O2, and calls them with a null pool and with a real pool.

Verification

Against the pre-fix source, the fixture crashes with 0xC0000005:

test_nvn_null_pool: the null-pool getter crashed (exit 3221225477); the source dereferences the null pool

Against this branch:

PASS: a null memory pool answers 0; a real pool keeps its flags and size

Also added the test to the Linux source-regressions workflow. A full game build was not run; the change is host-side and game-free tested.

Note

This is the same family as #284/#287: a null NVN object was dereferenced during a scene/banc change. The no_builder fix covered the texture-builder queries; this covers the memory-pool calls.

Fixes #509. Related tracking issue: #287. ## What the crash was While Kakariko Village changed from day to night, the `OptimizeChangeBanc` thread called `nvnMemoryPoolGetFlags(NULL)`. The HLE getter read `flags` at `A64_MEM + 4` (guest `0x4`) and took an access violation: ``` [hos OptimizeChangeBanc] CRASH: exception 0xc0000005 (read 0x100000000004, guest 0x4) at hookshot.exe+0x1a53b5dd ``` `hookshot.exe+0x1a53b5dd` is `n_nvnMemoryPoolGetFlags+0xd`. The guest caller is the texture-building helper `f_80905130`: it passes the pool pointer `[x21]` to the storage setter and to `nvnTextureInitialize` (which already handles a null pool), then asks the same `[x21] = 0` for its flags. This is a texture with no storage pool being prepared during the banc change. `P(0)` returns `NULL` for a zero argument, but `((nvn_pool *)NULL)->flags` is undefined behaviour, so the compiler is free to fold the check away — and did, reading `A64_MEM + 4`. A guard has to come before the dereference. ## Changes - Add `no_pool()`, following `no_builder()` from #284/#287. - Guard the memory-pool accessors before their first dereference: - `nvnMemoryPoolGetFlags` / `GetSize` / `GetBufferAddress` return `0` - `nvnMemoryPoolMap` returns `0` - `nvnMemoryPoolInvalidateMappedRange`, `Finalize`, `MapVirtual` do nothing - Add a one-shot log line naming the call and the guest LR, like `no_builder`. - Add `tests/test_nvn_null_pool.py`, a game-free regression test that extracts the actual getter bodies from `host/nvn_native.c`, compiles them at `-O2`, and calls them with a null pool and with a real pool. ## Verification Against the pre-fix source, the fixture crashes with `0xC0000005`: ``` test_nvn_null_pool: the null-pool getter crashed (exit 3221225477); the source dereferences the null pool ``` Against this branch: ``` PASS: a null memory pool answers 0; a real pool keeps its flags and size ``` Also added the test to the Linux source-regressions workflow. A full game build was not run; the change is host-side and game-free tested. ## Note This is the same family as #284/#287: a null NVN object was dereferenced during a scene/banc change. The `no_builder` fix covered the texture-builder queries; this covers the memory-pool calls.

nicholasgower commented

2026-11-08 07:26:30 +00:00

(Migrated from github.com)

CI note: the Ubuntu job's existing "Check import windows" step fails on prefetch_chunks / Pool::prefetched. That is pre-existing on main — nx2/test_import_windows.py has not been updated for the AMD prefetch code in gpu/vk/vk_memory.cpp — and is unrelated to this PR. The new "Check a null NVN memory pool is answered, not dereferenced" step passed, as did every step before the import-windows one.

CI note: the Ubuntu job's existing "Check import windows" step fails on `prefetch_chunks` / `Pool::prefetched`. That is pre-existing on `main` — `nx2/test_import_windows.py` has not been updated for the AMD prefetch code in `gpu/vk/vk_memory.cpp` — and is unrelated to this PR. The new "Check a null NVN memory pool is answered, not dereferenced" step passed, as did every step before the import-windows one.

nicholasgower commented

2026-11-08 07:30:51 +00:00

(Migrated from github.com)

The failing step is the Ubuntu job's existing Check import windows step (nx2/test_import_windows.py), not the new NVN regression test. It fails while compiling the generated C++ fixture:

fixture.cpp:145:20: error: `prefetch_chunks' was not declared in this scope
fixture.cpp:155:21: error: `struct Pool' has no member named `prefetched'
fixture.cpp:157:9:  error: `st_prefetch_used' was not declared in this scope
fixture.cpp:158:9:  error: `prefetch_chunks' was not declared in this scope
fixture.cpp:162:107: error: `st_prefetch_sent' was not declared in this scope
fixture.cpp:163:58: error: `st_prefetch_dropped' was not declared in this scope

nx2/test_import_windows.py extracts pool_lookup/pool_of/import_pool from gpu/vk/vk_memory.cpp and compiles them against a small hand-written fixture. The fixture's struct Pool and stubs have not been updated for the import-prefetch code added in f174cfc:

gpu/vk/vk_memory.cpp:55 — bool prefetched = false; in Pool
gpu/vk/vk_memory.cpp:1246 — st_prefetch_sent, st_prefetch_used, st_prefetch_dropped
gpu/vk/vk_memory.cpp:1319 — prefetch_chunks(...)
gpu/vk/vk_memory.cpp:1469-1476 — pool_lookup uses all of them

This is pre-existing and unrelated to this PR:

A clean detached worktree at origin/main (cdb4f1a) fails the same test with the same errors.
git log -S"prefetch_chunks" -- nx2/test_import_windows.py is empty — the test was never updated for f174cfc.
main already has failing source-regressions runs at the same step: 42450422930 and 42454098719.

This PR only changes host/nvn_native.c, tests/test_nvn_null_pool.py and .github/workflows/source-regressions.yml; it does not touch gpu/vk/vk_memory.cpp or nx2/test_import_windows.py. The new Check a null NVN memory pool is answered, not dereferenced step passed, and the Windows job passed.

The fixture can be unblocked separately by adding bool prefetched=false; to its Pool, declaring the three counters, and stubbing prefetch_chunks. I have left that out of this PR to keep it to one topic; I can send it as a separate PR.

The failing step is the Ubuntu job's existing **`Check import windows`** step (`nx2/test_import_windows.py`), not the new NVN regression test. It fails while compiling the generated C++ fixture: ```text fixture.cpp:145:20: error: `prefetch_chunks' was not declared in this scope fixture.cpp:155:21: error: `struct Pool' has no member named `prefetched' fixture.cpp:157:9: error: `st_prefetch_used' was not declared in this scope fixture.cpp:158:9: error: `prefetch_chunks' was not declared in this scope fixture.cpp:162:107: error: `st_prefetch_sent' was not declared in this scope fixture.cpp:163:58: error: `st_prefetch_dropped' was not declared in this scope ``` `nx2/test_import_windows.py` extracts `pool_lookup`/`pool_of`/`import_pool` from `gpu/vk/vk_memory.cpp` and compiles them against a small hand-written fixture. The fixture's `struct Pool` and stubs have not been updated for the import-prefetch code added in `f174cfc`: - `gpu/vk/vk_memory.cpp:55` — `bool prefetched = false;` in `Pool` - `gpu/vk/vk_memory.cpp:1246` — `st_prefetch_sent`, `st_prefetch_used`, `st_prefetch_dropped` - `gpu/vk/vk_memory.cpp:1319` — `prefetch_chunks(...)` - `gpu/vk/vk_memory.cpp:1469-1476` — `pool_lookup` uses all of them This is pre-existing and unrelated to this PR: - A clean detached worktree at `origin/main` (`cdb4f1a`) fails the same test with the same errors. - `git log -S"prefetch_chunks" -- nx2/test_import_windows.py` is empty — the test was never updated for `f174cfc`. - `main` already has failing source-regressions runs at the same step: [42450422930](https://github.com/NotRexed/Hookshot/actions/runs/42450422930) and [42454098719](https://github.com/NotRexed/Hookshot/actions/runs/42454098719). This PR only changes `host/nvn_native.c`, `tests/test_nvn_null_pool.py` and `.github/workflows/source-regressions.yml`; it does not touch `gpu/vk/vk_memory.cpp` or `nx2/test_import_windows.py`. The new `Check a null NVN memory pool is answered, not dereferenced` step passed, and the Windows job passed. The fixture can be unblocked separately by adding `bool prefetched=false;` to its `Pool`, declaring the three counters, and stubbing `prefetch_chunks`. I have left that out of this PR to keep it to one topic; I can send it as a separate PR.

Commenting is not possible because the repository is archived.

No reviewers