func_override tier: replace or wrap any guest function with hand-written C

[rickguedes]

Draft for discussion. The tier is running in production on a game repo today; opening as a draft so we can align on the design before it's final.

What this is

A recomp turns the game into C, but that C is a build artifact — regenerated from the player's own disc, never hand-edited. So there has been no supported way to say "this function should do something else", which is the single most important thing a port needs to grow beyond a faithful replay: progressive decompilation, native mods, QoL features.

This PR adds that mechanism: register hand-written C against a guest address and the dispatcher calls yours instead of the recompiled original.

int my_impl(CPUState *cpu) {
    uint32_t a0 = cpu->gpr[4], a1 = cpu->gpr[5];   /* guest args      */
    ...
    cpu->gpr[2] = result;                           /* $v0             */
    return 1;                                       /* handled         */
}
/* constructor, or a mod-package registration: */
func_override_add("game.my_function", 0x8001DCB0, my_impl);

Return 1 and the guest resumes at $ra exactly as if the original ran jr $ra. Return 0 and the original recompiled/interpreted code runs untouched — an override can decline case by case (conditional pre-hook: mutate state, return 0, original runs).

No size limit. This is not a ROM patch; there is no slot to fit inside, no code cave. The replacement is native code in a native binary — five instructions or five thousand lines. That freedom is the point of recompiling rather than hacking bytes.

Where it hooks — and why that placement is load-bearing

• In psx_dispatch_impl after the BIOS tiers (an override can never shadow a kernel service vector) and before every game backend — one address-keyed hook covers the static EXE, runtime-loaded overlays, and dirty RAM alike. Overlay code that the recompiler classified observed-PC-only (never became a standalone C function) is overridable; content-hash shard revocation/rebuild cannot orphan an override because the hook sits upstream of the shard cache.
• At the interpreter's JAL/JALR call-resolution tiers (the reserved CRES_OVERRIDE slot). This one is scar tissue: in the tier's first game-side deployment, the dirty-RAM interpreter resolved same-region calls itself (native shard or local pc-chain) and silently bypassed every registered hook — registration is not coverage. Both interpreter call sites consult the hook ahead of both resolution paths; handled calls resume via the same call contract as a native callee.
• Byte-identical when unused: the hook pointer stays NULL with nothing registered, so a build without overrides dispatches exactly as before. Games that never adopt the tier are unaffected (after their next regen — see below).

API surface (runtime/include/func_override.h has the full contract)

Call	What it does
func_override_add(id, addr, fn)	Register. Duplicate address = error, never silent last-wins
func_override_add_guarded(..., words, n)	+ prologue-word residency guard: if the first N guest words at addr do not match, the override declines instead of corrupting — for overlay/dirty-RAM addresses other code may occupy
func_override_call_original(cpu)	The wrap primitive: run the original from inside an override (one-shot bypass; recursion re-consults, matching a guest-level wrap)
func_override_guest_call(cpu, target, site_ra)	Call guest code from native, with an authentic $ra so callees, fntrace, and crash forensics see real values
psx_mod_register_function_override(id, addr, fn, guard, n)	Package-gated registration (below)
func_override TCP command	Live inventory: id, addr, calls, guard_misses. calls counts consults (declines included) — a decline-only probe proves an address crosses a hooked path; calls: 0 means never reached

Security model: overrides compile with the recomp — .psxmod files carry no code

This deliberately follows the existing format-5 trusted-plugin rule (MOD_PACKAGES.md, "Trusted adapters and archive safety"): a package archive supplies no native code. Overrides are C source compiled into the runtime binary by the game repo's own build (EXTRAS_SOURCES / the game's globbed mod sources). A .psxmod can only select statically-registered plugin ids:

• psx_mod_register_function_override("pkg.feature", addr, fn, ...) queues the override at constructor time; it is armed only when the resolved package plan selects that plugin id — the same gating as activation/VBlank callbacks. Not selected = the address is never hooked, dispatch identical to a build without the mod.
• The plugin id is a registry key, not a library path or symbol name. A malicious or corrupted .psxmod cannot inject code, hook new addresses, or reach beyond the implementations the game shipped — resolution fails before launch if an enabled plugin has no registered implementation.
• Armed override identities participate in the committed plan, and netplay's clear-mods path drops them like every other plugin kind.
• Direct func_override_add (no package) is for the game repo's own faithful decomp reimplementations — always on, meant to be indistinguishable from the code they replace, not player-toggleable.

How a game uses it

1. Progressive decompilation (direct, always-on — the faithful-reimplementation idiom):

/* src/decomp/game_damage_calc.c — compiled into the runtime by the game repo */
#include "func_override.h"
#include "cpu_state.h"

static int damage_calc(CPUState *cpu) {
    /* faithful C reimplementation of the routine at 0x800224E0 ... */
    cpu->gpr[2] = result;
    return 1;
}

__attribute__((constructor))
static void register_damage_calc(void) {
    /* prologue guard: decline (do not corrupt) if other overlay code is resident */
    static const uint32_t prologue[2] = { 0x27BDFFE0u, 0xAFB20018u };
    func_override_add_guarded("decomp.damage_calc", 0x800224E0,
                              damage_calc, prologue, 2);
}

2. A player-toggleable mod, gated by a .psxmod — the C side registers under the plugin id; several overrides can share one manifest plugin using the optional :label suffix (gating matches the part before the :, the full id names each row in the TCP inventory):

psx_mod_register_function_override("pkg.gunfire:aim",  0x80023F08, aim_impl,  guard, 2);
psx_mod_register_function_override("pkg.gunfire:fire", 0x8002AB08, fire_impl, guard, 2);

# manifest.toml — the .psxmod carries only this selection, no code
format_version = 5

[[feature]]
id = "gunfire"
name = "Sub-weapon gunfire"

[[plugin]]
feature = "gunfire"
id = "pkg.gunfire"

3. Wrapping instead of replacing (post-hook; also the right shape on timing-sensitive paths):

static int battle_assemble_wrap(CPUState *cpu) {
    func_override_call_original(cpu);          /* original runs fully    */
    if (player_wears_custom_body(cpu))         /* then adjust the result */
        func_override_guest_call(cpu, LOAD_VOICE_PACK, cpu->gpr[31]);
    return 1;
}

Rules an override must obey (documented in the header): pointers handed back to the guest must be guest addresses (the caller performs guest loads — a host pointer reads garbage; write into guest RAM and return that address). Keep all mutable state in guest RAM — rollback/rewind/netplay snapshot guest state only, host statics desync replays.

Verification workflow (from live deployments)

func_override over TCP is the ground truth: an override whose calls stays 0 was never reached (wrong address, or that path never ran); guard_misses counts guard declines. Because calls counts consults, registering a decline-only probe (always return 0) is a safe way to prove an address crosses a hooked path before writing the real implementation.

What was tested

• Bushido Blade 2 (SLUS-00663), MinGW RelWithDebInfo, GL, on this code: boots to gameplay; 10 overrides armed live — 3 always-on decomp reimplementations + 7 package-gated mod overrides (including a 4-override plugin), all prologue-guarded; package gating verified both ways (disabled feature = count drops, address unhooked). Battle-path overrides observed firing in play (the wrap + guest_call path drives a real voice-pack swap).
• The tier's earlier, more primitive revision has been shipping in a second game repo (Azure Dreams) for weeks — overlay-resident overrides at 500+ calls, user-confirmed on screen. This PR is that lineage with the interpreter-tier fix designed in, plus the wrap/guest-call/guard/gating/label API.
• recompiler ctest on this branch's own fresh tree: 45/50 pass. The 5 failures are pre-existing on master and unrelated (mod_load_acceleration asserts a gi.has_turbo_loads line removed by the turbo_loads retirement; launcher_vulkan_option is a cp1252 decode error in the test harness on Windows; aot_overlay_discovery / release_zip / vk_present_wait_stage fail identically without this change).
• tools/gen_tcp_commands.py --check passes; note the regenerated index also picks up rows for commands that had drifted on master since its last regen (the checked-in index says 292, the servers register 304 before this PR).
• Honest gaps: no netplay session was run with a package-gated override armed (the clear-mods path is exercised, an actual rollback session is not); no second maintainer-side game has regenerated with this emitter change yet.

Scope / downstream

• Fully game-agnostic: no title checks, no magic addresses; the tier is address-keyed by whatever the consuming game registers.
• Regen required downstream: full_function_emitter.cpp emits the dispatch consult, so consuming games pick the tier up on their next BIOS + game regen. Until then their builds are unchanged. The runtime side is inert without registrations either way.
• Cycle accounting is deliberately deferred and documented: a handled override credits no guest cycles for skipped code; the header steers timing-sensitive paths to wrap (call_original) instead of replace, until the shared cycle core exposes a public credit API. Happy to discuss whether a cycles parameter should land now instead.
• Caps are static and small (FO_MAX_OVERRIDES 128, FO_MAX_GUARD_WORDS 4) — sized for hand-written registries, trivially raisable if a game outgrows them.

[mstan]

Thanks for opening this as a draft — the design reads well, and the hook-placement notes made it much easier to review.

I read the change against master, and I have pushed one commit onto this branch (8105d937) with fixes for three things I found. To be clear about that: it is a fast-forward on top of your three commits, nothing of yours was rewritten. Please assess it as a proposal, not a decision. If you disagree with any part, say so and we roll it back or split it out — I pushed it rather than describing it because it is easier to judge code than prose.

There is also one thing I would like to decide together, and I have deliberately not touched it.

The thing I want to discuss: cycle accounting

The header says a cycle-credit parameter is deferred "until the shared cycle core exposes a public credit API." I do not think that blocker exists. psx_advance_cycles() is already public in runtime/include/psx_cycles.h, and the BIOS HLE tier right next to this one already charges per service with it (bios_hle.c:184, 193, 206, 218, 262) under its own TIMING NOTE.

So the mechanism is there. What is left is a policy choice, and that is what I would like your view on.

My concern: with zero credit, a handled override collapses the replaced function to 0 guest cycles and shifts IRQ phase. For a player-toggled mod that is fine. For the always-on faithful-decomp use case — the one the header describes as "indistinguishable from the code they replace" — a rewrite that takes zero time is not indistinguishable. Interrupt-phase drift is the class of bug this project treats as foundation work, so I am wary of a tier that defaults the whole thing to free.

My suggestion is that the credit becomes a required argument at registration, even when the honest answer is 0, so every call site has to state its timing intent instead of inheriting a silent default.

Two questions for you, since you have actually shipped this and I have only read it:

1. Does a required credit argument sound right, or does it get in the way in practice?
2. For a wrap via func_override_call_original, what should be charged? The original runs and accrues its own cycles, so I would expect the wrapper to charge only its own added work — but tell me if that is wrong.

In the commit I only corrected the header's factual claim about the missing API and relabelled this as an open policy question. The zero-credit behaviour itself is unchanged.

What the commit fixes

1. The override is skipped for interp → statically-compiled callees.

At both interpreter call sites (dirty_ram_interp.c:1586 JALR, :1797 JAL) the consult sat after interp_enter_compiled. That path goes interp_enter_compiled → psx_dispatch_game_compiled → entry->fn(cpu) (emitted at main_psx.cpp:1632) and never re-enters psx_dispatch_impl, so the hook is never reached. An override on a static-EXE function called from interpreted overlay/dirty-RAM code runs the original — while registration, the armed count, and the func_override inventory all still look healthy.

It is the same "registration is not coverage" shape you fixed for the overlay-native and pc-chain paths. The commit moves the consult above that block at both sites, which also restores the invariant the header states ("BEFORE every game code backend").

I also documented the tail-transfer sites (:3114, :3163) as deliberately not hooked — overrides are call-site keyed, so a j-style tail call into an overridden address will show calls: 0. If you would rather those fire too, that is a real design choice and I am happy either way; I only wanted the answer written down instead of implied.

2. There is no teardown, so clear-mods cannot disarm an override.

func_override.c only ever adds; s_count never shrinks. Clearing s.plan works for activation/vblank callbacks because those only run while something iterates the plan, but armed overrides live in the module's own table with the hook installed.

On the rematch path this is reachable: main.cpp:13824 clears, then goto session_reboot lands at main.cpp:12275 — past mod_runtime_activate_plugins and func_override_install. So a modded session entering netplay prints the vanilla-session banner and keeps running its overrides, diverging from a peer without the mod.

The commit adds func_override_add_package / func_override_reset_package_armed, called from mod_runtime_clear_for_netplay. It drops package-armed entries and keeps direct ones, on the reasoning that direct registrations are the game's own always-on reimplementations — not player-selectable, and identical in both peers' builds. Tell me if you would rather it dropped everything.

3. The residency guard read through a traced accessor.

psx_read_word is not an inert peek (memory.c:1436): under lockstep it feeds ls_read_hook, under lockstep replay it returns the replayed value instead of RAM, and under DuckStation recording it calls ds_note_read. So guard words showed up in the divergence streams as reads the guest never made, and under replay the guard compared replayed data rather than resident bytes.

The commit adds psx_peek_word_untraced (same address decode, no tracing) and uses it for the guard. It is also cheaper than the traced path.

Smaller items in the same commit: reject an address normalising to phys 0 (it collides with the "not inside an override" sentinel and would quietly break call_original); give func_override_get/_get_ex an id buffer size instead of an implicit FO_MAX_ID requirement; clamp the accumulated length in handle_func_override so raising FO_MAX_ID or FO_MAX_OVERRIDES cannot underflow the remaining-size argument.

I moved XRES_OVERRIDE to the end of its enum but deliberately did not renumber it into consult order — that enum is dense, unlike CRES, so inserting would shift the codes above it and invalidate previously captured xprobe traces.

Tests

The tier had no tests, and every failure mode here is silent, so I would rather pin the behaviour than rely on field reports. The commit adds runtime/tests/test_func_override.c (registered as func_override_test): install-is-NULL-when-empty, argument and duplicate refusal, declines still counting as consults, guard declining without running the body, call_original one-shot semantics, package reset keeping direct entries, and bounded id copy.

I checked the test is not vacuous by breaking the code on purpose three times. Two failures were caught immediately. The third — leaving the one-shot bypass armed — passed, which was a hole in my test, not in your code: call_original restores the saved bypass value on return, so a non-recursive wrap looks correct even when hook() never clears the flag. The clear only matters when the original recursively calls itself, which is exactly what the header promises. I added a self-recursive-original case and it now catches it.

Two things I checked that are fine

So you do not have to worry about them:

• full_function_emitter.cpp is in runtime/codegen_hash_sources.cmake:27, so the cg hash moves and overlay shard caches self-invalidate. No stale-shard risk from the emitter change.
• CRES_OVERRIDE = 6 really was pre-reserved in consult order on master (dirty_ram_interp.c:425), exactly as you described.

I also want to withdraw something before you read it anywhere else: I initially thought the TCP_COMMANDS.md regen had introduced CRLF damage. I measured it properly afterwards — master is 540 CRLF lines plus 27 stray LF lines, and your regen normalised those 27 to match. It cleaned up an existing inconsistency. My mistake.

What is NOT verified

I want to be straight about the limits of this review. Everything above came from reading the code, plus compiling the new test. Specifically:

• I have not run a game and watched an override fire from interpreted code, so fix 1 is confirmed by compile and reasoning only. Given that its failure mode is invisible by construction, that is the one I would most like to see measured before this leaves draft.
• I did not do a full runtime link or a cmake configure, only per-file syntax checks on the files I touched plus a standalone build of the new test.

If any of the three fixes conflicts with something you learned running this in Bushido Blade 2 or Azure Dreams, your field experience beats my reading — say so and we will change it.

[mstan]

Follow-up: I said the coverage fix (defect 1) was reasoned about but not measured. It is measured now, in a real game. Reproduced on the PR head, confirmed fixed by the commit.

Setup

Single-variable A/B, headless, Ape Escape (SCUS-94423):

	base arm	fix arm
PSXRECOMP_ROOT	_wt-pr174-base @ 6ae886e9 (your head)	_wt-pr174-fixes @ 8105d937
build	RelWithDebInfo / Ninja	identical
disc, game.toml, flags, probe string	identical	identical
build dir, debug port, exe name	separate (4621)	separate (4622)

git diff 6ae886e9..8105d937 touches zero recompiler/ files, so both arms share an identical cg hash and identical generated code. The one variable is where the interpreter consults the hook.

The instrument was a temporary decline-only probe mod driven by an env var (PSX_FO_PROBE=<addrs>), registered from a constructor. Every probe returns 0, so the original always runs and the only observable effect is the calls counter — the decline-only probe idiom your header documents. It has since been deleted from the title repo.

Result

Probes on the two statically-compiled targets, after ~290M / 231M interpreted instructions:

address	base	fix
0x8002C0E4	0	29,249
0x8002C14C	0	12,645

41,894 overridden calls that silently ran the original on the PR head.

Controls, same run — six probes on interp→overlay targets, which reach the consult in both arms because interp_enter_compiled declines for them:

address	base	fix
0x8013AE80	6,992,463	6,992,463
0x80136E00	2,220	1,676
0x8013ADA0	6	6
0x80136E68	4	4
0x80136454	2	2
0x801369E0	1	1

6/6 fire in both arms, one of them bit-identical at 6,992,463. So the base arm's probe machinery is fully functional and a zero on the compiled targets is a genuine miss, not a dead instrument. The base arm also executed more interpreted work than the fix arm (288M vs 231M instructions) and still recorded zero, so it is not a workload artefact.

No regression. Both arms render the title screen correctly, zero abort/fatal/wedge markers in either log. Worth calling out specifically: the fix arm processed 41,894 declining consults on compiled functions and every one fell through to the compiled original correctly — so moving the consult above interp_enter_compiled did not break the fall-through.

How to find these targets again

callret_watch with a nonzero lo (lo=0 silently disables the ring — callret_begin tests !g_callret_lo), sampled repeatedly, then histogram path & 0xFF against the CRES enum. Codes 2–5 are CRES_EC_*, i.e. the interpreter resolved into compiled code — exactly the bypassed route. Ape showed 186 of 1143 sampled JALR resolutions on EC paths.

A negative result worth recording

Tomba 2 cannot demonstrate this bug, and I want to save you the run. 2290 sampled JALR resolutions there produced zero EC paths — only NL_RET and PCCHAIN. Its whole text region is dirty, so psx_game_text_native_ok always fails, interp_enter_compiled always declines, and every call therefore reaches the hook in both arms. Probes on Tomba 2 targets read 10/10 and 1/1, identical across arms, for that reason and not because the fix is inert.

Practical consequence for the tier: whether this gap bites a given title depends on whether its static text stays clean. A game that self-modifies its text is accidentally immune; one that does not is exposed. That is not a property anyone would guess from the registration API, which is part of why I wanted it measured rather than argued.

Still not measured

Being straight about what this run does and does not cover:

• Defect 2 (netplay teardown) — unit test plus mutation testing only. No actual rollback session was run, the same gap you flagged in your own testing notes.
• Defect 3 (untraced guard read) — compile and reasoning only.
• The cycle-credit question is untouched and still yours to weigh in on.