fix(lift): a function with code below its entry starts at its entry #131

snaildev99 · 2026-11-13T05:43:37Z

snaildev99 commented

2026-11-13 05:43:37 +00:00

Why

The translator starts each of the game's functions at its lowest address. For 26 functions of the game that is not their first instruction, so they were started in the wrong place. A mod's code patch that is built in with the game runs into the same thing: the game crashed as soon as the mod's menu was used.

What changed

lift: nx2/lift_fixes.py, fix 4, wraps Lifter.emit_function. a64-kit's emit_function writes a function's instructions in address order, and the C function began at the first of them. A function is its entry and everything reachable from it without crossing another entry, so code it branches to at a lower address, which is no entry of its own, came first. When the lowest address of a function's body is not its entry, the entry now becomes a label and the C function starts with goto L_<entry>;. Every other function goes through a64-kit's code alone.
lift: nx2/lift.py prints how many functions were started that way.
tests: tests/test_lift_entry_below.py, game-free, without the translator or a compiler: a real Lifter over a made-up image.

The 26 functions on build e0f3ccfa:

25 are inside main+0x24a21e0..main+0x24dc600, which has a single unwind entry for its 238 KB (libcurl, by the strings it uses). Its functions are entries only as bl targets, data pointers and jump table targets, so one that branches to a lower function which nothing calls has that function below its entry.
1 is main+0x165ab04, the return address of a setjmp call (an entry since find_setjmp_owners); the code after the call loops back above it.

I do not know that any of the 26 runs in the port today, and I have not seen the unmodified game misbehave because of this. For the unmodified game this is a correctness fix without a known symptom.

The 26 entries

main+0x165ab04 main+0x24a4704 main+0x24a686c main+0x24a75a8 main+0x24a7a9c main+0x24a7d34 main+0x24b3ff0 main+0x24b5de0 main+0x24b7b58 main+0x24bd330 main+0x24c2324 main+0x24c66b4 main+0x24c8130 main+0x24c89c0 main+0x24ca5d8 main+0x24cc674 main+0x24ceb6c main+0x24cf200 main+0x24d06e0 main+0x24d07f8 main+0x24d0830 main+0x24d0868 main+0x24d08c8 main+0x24d0900 main+0x24d29a4 main+0x24d4454

Flags and settings

None.

Measurements

main: 341d63a4 (2026-11-13)
this PR: fix/lift-entry-below @ 3e0e891f (rebased onto main @ 341d63a4; the files of this PR and the lift's are unchanged between the two mains, so the numbers hold)
Hardware: RTX 3090, Windows 11
Scene: none (a fix in the lift: the proof is a test, the lifted C, and a crash gone)
Method: the new game-free test with main's nx2/lift_fixes.py and with this PR's; the lifted C of the unmodified game with and without the fix; one build played with a code mod
How to reproduce: python -I -B tests/test_lift_entry_below.py; for the lifted C, python -I nx2/lift.py --standalone --write into two work folders and compare sa/gen

	main @ `341d63a4`	this PR @ `3e0e891f`
`tests/test_lift_entry_below.py` (the test file of this PR)	2 of 5 fail	5 of 5 pass
Functions of the game started at an instruction that is not their entry	26	0
Lifted C of the unmodified game, 73 code files		2 files differ: 26 `goto L_<entry>;` lines and 20 labels added, no line removed or changed
A build with ExeFS code patches lifted in, a mod routine continuing below its entry	crash when a character is picked in the mod's menu	the menu, the pick and the swap in the field work

The lifted C and the played build are from v2.2.0-rc.1's lift, not from main @ 341d63a4: its nx2/lift_fixes.py and third_party/a64-kit/lift/a64_lift.py are the same files as main's (compared), and the fix was there in this PR's form (the lifted C of the played build is the same, file for file, with it).

main @ 341d63a4, tests/test_lift_entry_below.py:
FAIL: test_polls_see_the_same_loops (__main__.EntryBelow.test_polls_see_the_same_loops)
AssertionError: 'L_80001000:;' != 'goto L_80002000;'
FAIL: test_starts_at_its_entry (__main__.EntryBelow.test_starts_at_its_entry)
AssertionError: 'L_80001000:;' != 'goto L_80002000;'
Ran 5 tests in 0.002s
FAILED (failures=2)

this PR @ 3e0e891f:
Ran 5 tests in 0.002s
OK

the lift, the unmodified game, with the fix:
functions with code below their entry, started at it by a goto: 26

the played build before the fix (hookshot_log.txt), when a character was picked:
CRASH: exception 0xc0000005 (read 0xffffffffffffffff, guest 0xffffefffffffffff) at hookshot.exe+0xe29bcf5
CRASH: thread BaseProcExecutor5, sp 0x118a0fbe30
CRASH: guest backtrace: lr 0x83c66ff8 0x83c66ff8 0x800e998c 0x800e9a58 ...

lr 0x83c66ff8 is in the mod's code cave after main's text: the lifted routine began at its lower part (0x83bd5390), without its first instructions, and called a game function with the wrong argument.

Testing

Tested:

python -I -B tests/run_all.py on Windows 11: 457 passed, 70 skipped, 10 failed. The same 10 fail on main @ 341d63a4 on this PC: test_builder_details_hitbox, test_builder_dlss_update, test_portable_archive, test_portable_builder_home, test_portable_builder_startup (no .NET SDK here), test_fpx_sticky, test_spawn_emit (they build the translator; the builder's llvm-mingw copy has no C++ standard headers), test_check_third_party, test_release, test_setup_runtime.
The lifted C of the unmodified game, v2.2.0-rc.1's lift with and without the fix, the same dump: the table above.
In the game: Windows 11, RTX 3090, v2.2.0-rc.1 with this fix and the lift side of the mods draft (Hookshot PR 2478, https://github.com/NotRexed/Hookshot/pull/2478) ported to it, with a mod that has six ExeFS code patches (https://github.com/snaildev99/OoT-Character-Select). An adult save: the mod's menu, picking characters and the swap in the field.

Not tested:

The unmodified game in play with this change: its lifted C differs only in the 26 functions above, which I do not know to run.
A build of main @ 341d63a4 itself (the lift files are the same as the ones tested).
Linux.
Performance: 26 functions gain one goto.

Needs other testers: None

Screenshots

Nothing on screen.

Risk and rollback

The 26 functions now run from their entry, as the game's code does; if one of them is in use and something relied on the old start, it would show there (libcurl, and one setjmp return). Reverting the commit is enough.

Reviewer notes

The wrapper is installed by lift_fixes.install(), before poll.install() wraps the emit (nx2/lift.py's order). The goto to the entry is a forward one, so the polls are the same (the test checks it).
It adds the entry to the function's label set in place, so a64-kit's code writes L_<entry>:; itself.
Also up as https://github.com/NotRexed/Hookshot/pull/3038, opened before I knew of the move.
This is one of the things a code mod needs from the lift: see the mod interface idea, issue 42.

Issues

See #42

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main.
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A game-free test added for the fix, failing on main and passing here.

🤖 Generated with Claude Code

## Why The translator starts each of the game's functions at its lowest address. For 26 functions of the game that is not their first instruction, so they were started in the wrong place. A mod's code patch that is built in with the game runs into the same thing: the game crashed as soon as the mod's menu was used. ## What changed - lift: `nx2/lift_fixes.py`, fix 4, wraps `Lifter.emit_function`. a64-kit's `emit_function` writes a function's instructions in address order, and the C function began at the first of them. A function is its entry and everything reachable from it without crossing another entry, so code it branches to at a lower address, which is no entry of its own, came first. When the lowest address of a function's body is not its entry, the entry now becomes a label and the C function starts with `goto L_<entry>;`. Every other function goes through a64-kit's code alone. - lift: `nx2/lift.py` prints how many functions were started that way. - tests: `tests/test_lift_entry_below.py`, game-free, without the translator or a compiler: a real `Lifter` over a made-up image. The 26 functions on build `e0f3ccfa`: - 25 are inside main+0x24a21e0..main+0x24dc600, which has a single unwind entry for its 238 KB (libcurl, by the strings it uses). Its functions are entries only as `bl` targets, data pointers and jump table targets, so one that branches to a lower function which nothing calls has that function below its entry. - 1 is main+0x165ab04, the return address of a `setjmp` call (an entry since `find_setjmp_owners`); the code after the call loops back above it. I do not know that any of the 26 runs in the port today, and I have not seen the unmodified game misbehave because of this. For the unmodified game this is a correctness fix without a known symptom. <details> <summary>The 26 entries</summary> main+0x165ab04 main+0x24a4704 main+0x24a686c main+0x24a75a8 main+0x24a7a9c main+0x24a7d34 main+0x24b3ff0 main+0x24b5de0 main+0x24b7b58 main+0x24bd330 main+0x24c2324 main+0x24c66b4 main+0x24c8130 main+0x24c89c0 main+0x24ca5d8 main+0x24cc674 main+0x24ceb6c main+0x24cf200 main+0x24d06e0 main+0x24d07f8 main+0x24d0830 main+0x24d0868 main+0x24d08c8 main+0x24d0900 main+0x24d29a4 main+0x24d4454 </details> ## Flags and settings None. ## Measurements main: 341d63a4 (2026-11-13) this PR: fix/lift-entry-below @ 3e0e891f (rebased onto main @ 341d63a4; the files of this PR and the lift's are unchanged between the two mains, so the numbers hold) Hardware: RTX 3090, Windows 11 Scene: none (a fix in the lift: the proof is a test, the lifted C, and a crash gone) Method: the new game-free test with main's `nx2/lift_fixes.py` and with this PR's; the lifted C of the unmodified game with and without the fix; one build played with a code mod How to reproduce: `python -I -B tests/test_lift_entry_below.py`; for the lifted C, `python -I nx2/lift.py --standalone --write` into two work folders and compare `sa/gen` | | main @ 341d63a4 | this PR @ 3e0e891f | |---|---|---| | `tests/test_lift_entry_below.py` (the test file of this PR) | 2 of 5 fail | 5 of 5 pass | | Functions of the game started at an instruction that is not their entry | 26 | 0 | | Lifted C of the unmodified game, 73 code files | | 2 files differ: 26 `goto L_<entry>;` lines and 20 labels added, no line removed or changed | | A build with ExeFS code patches lifted in, a mod routine continuing below its entry | crash when a character is picked in the mod's menu | the menu, the pick and the swap in the field work | The lifted C and the played build are from v2.2.0-rc.1's lift, not from main @ 341d63a4: its `nx2/lift_fixes.py` and `third_party/a64-kit/lift/a64_lift.py` are the same files as main's (compared), and the fix was there in this PR's form (the lifted C of the played build is the same, file for file, with it). ``` main @ 341d63a4, tests/test_lift_entry_below.py: FAIL: test_polls_see_the_same_loops (__main__.EntryBelow.test_polls_see_the_same_loops) AssertionError: 'L_80001000:;' != 'goto L_80002000;' FAIL: test_starts_at_its_entry (__main__.EntryBelow.test_starts_at_its_entry) AssertionError: 'L_80001000:;' != 'goto L_80002000;' Ran 5 tests in 0.002s FAILED (failures=2) this PR @ 3e0e891f: Ran 5 tests in 0.002s OK the lift, the unmodified game, with the fix: functions with code below their entry, started at it by a goto: 26 the played build before the fix (hookshot_log.txt), when a character was picked: CRASH: exception 0xc0000005 (read 0xffffffffffffffff, guest 0xffffefffffffffff) at hookshot.exe+0xe29bcf5 CRASH: thread BaseProcExecutor5, sp 0x118a0fbe30 CRASH: guest backtrace: lr 0x83c66ff8 0x83c66ff8 0x800e998c 0x800e9a58 ... ``` `lr 0x83c66ff8` is in the mod's code cave after main's text: the lifted routine began at its lower part (0x83bd5390), without its first instructions, and called a game function with the wrong argument. ## Testing **Tested:** - `python -I -B tests/run_all.py` on Windows 11: 457 passed, 70 skipped, 10 failed. The same 10 fail on main @ 341d63a4 on this PC: `test_builder_details_hitbox`, `test_builder_dlss_update`, `test_portable_archive`, `test_portable_builder_home`, `test_portable_builder_startup` (no .NET SDK here), `test_fpx_sticky`, `test_spawn_emit` (they build the translator; the builder's llvm-mingw copy has no C++ standard headers), `test_check_third_party`, `test_release`, `test_setup_runtime`. - The lifted C of the unmodified game, v2.2.0-rc.1's lift with and without the fix, the same dump: the table above. - In the game: Windows 11, RTX 3090, v2.2.0-rc.1 with this fix and the lift side of the mods draft (Hookshot PR 2478, https://github.com/NotRexed/Hookshot/pull/2478) ported to it, with a mod that has six ExeFS code patches (https://github.com/snaildev99/OoT-Character-Select). An adult save: the mod's menu, picking characters and the swap in the field. **Not tested:** - The unmodified game in play with this change: its lifted C differs only in the 26 functions above, which I do not know to run. - A build of main @ 341d63a4 itself (the lift files are the same as the ones tested). - Linux. - Performance: 26 functions gain one `goto`. **Needs other testers:** None ## Screenshots Nothing on screen. ## Risk and rollback The 26 functions now run from their entry, as the game's code does; if one of them is in use and something relied on the old start, it would show there (libcurl, and one `setjmp` return). Reverting the commit is enough. ## Reviewer notes - The wrapper is installed by `lift_fixes.install()`, before `poll.install()` wraps the emit (nx2/lift.py's order). The goto to the entry is a forward one, so the polls are the same (the test checks it). - It adds the entry to the function's label set in place, so a64-kit's code writes `L_<entry>:;` itself. - Also up as https://github.com/NotRexed/Hookshot/pull/3038, opened before I knew of the move. - This is one of the things a code mod needs from the lift: see the mod interface idea, issue 42. ## Issues See #42 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

👀 1

snaildev99 added 1 commit

2026-11-13 05:43:38 +00:00

fix(lift): a function with code below its entry starts at its entry

PR description / Description follows the template (pull_request_target) Successful in 4s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

3e0e891f3e

a64-kit's emit_function writes a function's instructions in address order, so
the C function began at its lowest instruction. A function is its entry and
everything reachable from it without crossing another entry: code it branches
to at a lower address, which is no entry of its own, came first. 26 functions
of build e0f3ccfa are like that (25 in libcurl, which has one unwind entry for
all of its code, and the return address of a setjmp call), and so is a mod's
patch routine that goes on in spare code below its entry.

nx2/lift_fixes.py wraps emit_function: such a function starts with a goto to
its entry. tests/test_lift_entry_below.py checks it without the game.

Co-Authored-By: Claude Fable 5.2 <[email protected]>

snaildev99 commented

2026-11-13 05:44:18 +00:00

The description's Testing section fixed (no other testers needed). The branch is on main @ 341d63a4.

The description's Testing section fixed (no other testers needed). The branch is on main @ 341d63a4.

snaildev99 changed title from ~~WIP: fix(lift): a function with code below its entry starts at its entry~~ to fix(lift): a function with code below its entry starts at its entry

2026-11-13 05:44:19 +00:00

HumanBean added 1 commit

2026-11-13 05:58:03 +00:00

Merge branch 'main' into fix/lift-entry-below

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 10s

Details

CI / Conventional Commit title (pull_request) Successful in 7s

Details

CI / Workflow lint (actionlint) (pull_request) Failing after 10s

Details

Third-party parts / As their notes say (pull_request) Successful in 14s

Details

9b3fc83ce8

HumanBean approved these changes

2026-11-13 06:00:42 +00:00

HumanBean left a comment

Reviewed the diff and the code around it: a64-kit's walk / emit_function / transfer, the dead_flags and dirty_regs passes, localize_regs, nx2/poll.py and the spawned emit in nx2/chunks.py. I didn't run anything.

The fix is right for the lift as it is built by default. A function's body can reach below its entry: walk follows every branch that doesn't land on another entry. emit_function writes it in address order and enters at pcs[0]. Making the entry a label and jumping to it is the smallest change that fixes this. Some things I checked:

Flow analyses. dead_flags is a backward liveness pass, and dirty_regs starts every instruction clean and joins over predecessors. Neither assumes index 0 is the entry, so the leading goto changes neither result.
Branch targets. transfer already writes a goto for a branch to e from inside its own body (t == self.cur). With e in labels, the base now writes the label those gotos need.
Polls. The new goto is forward, so add_polls still finds the same back-edges.
Spawned emit (Windows). The workers get the patch because they run lift.py's module level again, and _identities() checks they did. entry_gotos comes back through the lifter.stats deltas, so the count lift.py prints is right there too.
PLT stubs are skipped before min(body). install() is guarded, so the wrapper is never added twice.
Project rules. third_party/ is untouched, the fix is in nx2/ as CONTRIBUTING asks, and the new test runs without the game. It has teeth: the unwrapped emit is captured before install().

One inline comment on where the goto goes when A64_LOCAL_REGS / A64_LOCAL_FLAGS is set. Both are off by default ("experiment" in a64-kit), so the default build isn't affected, but under them the goto would skip the locals' initializers. A follow-up is fine, as long as it doesn't get lost.

Approving.

Reviewed the diff and the code around it: a64-kit's `walk` / `emit_function` / `transfer`, the `dead_flags` and `dirty_regs` passes, `localize_regs`, `nx2/poll.py` and the spawned emit in `nx2/chunks.py`. I didn't run anything. The fix is right for the lift as it is built by default. A function's body can reach below its entry: `walk` follows every branch that doesn't land on another entry. `emit_function` writes it in address order and enters at `pcs[0]`. Making the entry a label and jumping to it is the smallest change that fixes this. Some things I checked: - **Flow analyses.** `dead_flags` is a backward liveness pass, and `dirty_regs` starts every instruction clean and joins over predecessors. Neither assumes index 0 is the entry, so the leading goto changes neither result. - **Branch targets.** `transfer` already writes a `goto` for a branch to `e` from inside its own body (`t == self.cur`). With `e` in `labels`, the base now writes the label those gotos need. - **Polls.** The new goto is forward, so `add_polls` still finds the same back-edges. - **Spawned emit (Windows).** The workers get the patch because they run `lift.py`'s module level again, and `_identities()` checks they did. `entry_gotos` comes back through the `lifter.stats` deltas, so the count `lift.py` prints is right there too. - **PLT stubs** are skipped before `min(body)`. `install()` is guarded, so the wrapper is never added twice. - **Project rules.** `third_party/` is untouched, the fix is in `nx2/` as CONTRIBUTING asks, and the new test runs without the game. It has teeth: the unwrapped emit is captured before `install()`. One inline comment on where the goto goes when `A64_LOCAL_REGS` / `A64_LOCAL_FLAGS` is set. Both are off by default ("experiment" in a64-kit), so the default build isn't affected, but under them the goto would skip the locals' initializers. A follow-up is fine, as long as it doesn't get lost. Approving.

nx2/lift_fixes.py

					
				@ -115,0 +132,4 @@

				        labels.add(e)   # (the base writes "L_<entry>:;" at an entry that is a label)

				        text, refs = base_emit_function(self, e, texts)

				        head = "void f_%08x(Ctx *__restrict A64_NOESC c) {\n" % e

				        at = text.index(head) + len(head)

With A64_LOCAL_REGS (or A64_LOCAL_FLAGS) set, localize_regs has already put the locals' declarations on the line right after the signature (uint64_t x0_ = c->x[0], ...; / uint8_t fn_ = c->n, ...;). Inserting the goto right after head therefore jumps over their initializers. C allows that jump: no error, and no warning from clang by default. Every local register would then start indeterminate in these 26 functions. Both switches are off by default, so today's build is fine, but this would be a silent miscompile for whoever tries the experiment.

A cheap fix: put the goto after the declarations instead, e.g. skip the lines after head that start with uint64_t x / uint8_t fn_. Or put it right before the first line of the body. A case in test_starts_at_its_entry under mock.patch.dict(os.environ, {"A64_LOCAL_REGS": "4"}) would cover it, since the test's texts already use c->x[0].

With `A64_LOCAL_REGS` (or `A64_LOCAL_FLAGS`) set, `localize_regs` has already put the locals' declarations on the line right after the signature (`uint64_t x0_ = c->x[0], ...;` / `uint8_t fn_ = c->n, ...;`). Inserting the goto right after `head` therefore jumps over their initializers. C allows that jump: no error, and no warning from clang by default. Every local register would then start indeterminate in these 26 functions. Both switches are off by default, so today's build is fine, but this would be a silent miscompile for whoever tries the experiment. A cheap fix: put the goto after the declarations instead, e.g. skip the lines after `head` that start with ` uint64_t x` / ` uint8_t fn_`. Or put it right before the first line of the body. A case in `test_starts_at_its_entry` under `mock.patch.dict(os.environ, {"A64_LOCAL_REGS": "4"})` would cover it, since the test's texts already use `c->x[0]`.