From 199cfe01bc8f81720ca566974204969652c087ef Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Sat, 26 Sep 2026 07:34:20 -0700 Subject: [PATCH] mods: flat function-entry hook table built at activation; interpreter skips it when empty Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/MOD_PACKAGES.md | 8 ++- runtime/include/mod_packages.h | 9 ++- runtime/include/mod_plugins.h | 8 ++- runtime/src/dirty_ram_interp.c | 10 ++- runtime/src/mod_packages.cpp | 12 ++-- runtime/src/mod_runtime.cpp | 66 ++++++++++++++++-- runtime/tests/test_interp_mod_entry_guards.py | 5 ++ runtime/tests/test_mod_packages.cpp | 21 +++--- runtime/tests/test_mod_runtime.cpp | 69 ++++++++++++++++++- 9 files changed, 180 insertions(+), 28 deletions(-) diff --git a/docs/MOD_PACKAGES.md b/docs/MOD_PACKAGES.md index 99b7c93e..d5411ec6 100644 --- a/docs/MOD_PACKAGES.md +++ b/docs/MOD_PACKAGES.md @@ -447,8 +447,12 @@ An implementation may register an activation callback, a deterministic guest-VBlank callback, function-entry hooks, or any combination under the same id. A function-entry hook (`psx_mod_register_function_entry_plugin`) runs at the top of a generated function the game config lists in -`[recompiler] mod_function_entry_funcs`; like the other kinds it satisfies a -manifest `[[plugin]]` and runs only while the resolved plan activates its id. Activation runs after the +`[recompiler] mod_function_entry_funcs`, and at every interpreted entry to the +same address (segment bits ignored), so the backend running the page does not +matter. Like the other kinds it satisfies a manifest `[[plugin]]` and runs only +while the resolved plan activates its id: the active hooks are flattened into an +address table at plugin activation, and a plan change drops them until the next +activation. Activation runs after the launcher's final mod-plan commit and before renderer/window initialization; it is appropriate for a game-owned mod that selects a fixed display aspect or another pre-boot host feature. VBlank callbacks run from the emulated GPU diff --git a/runtime/include/mod_packages.h b/runtime/include/mod_packages.h index 13d13c19..bbbe95b9 100644 --- a/runtime/include/mod_packages.h +++ b/runtime/include/mod_packages.h @@ -463,8 +463,13 @@ bool mod_register_function_entry_plugin(const std::string& id, uint32_t address, bool mod_plugin_registered(const std::string& id); void mod_invoke_activation_plugin(const std::string& id); void mod_invoke_vblank_plugin(const std::string& id); -void mod_invoke_function_entry_plugin(const std::string& id, ::CPUState* cpu, - uint32_t address); +struct ModFunctionEntryHook { + uint32_t address = 0; + PSXModFunctionEntryCallback callback = nullptr; +}; +/* Hooks one implementation registered, in registration order. mod_runtime + * flattens these into an address table when the plan's plugins activate. */ +std::vector mod_function_entry_hooks(const std::string& id); void mod_clear_plugins_for_tests(); } // namespace PSXRecompV4 diff --git a/runtime/include/mod_plugins.h b/runtime/include/mod_plugins.h index 9de40626..53a4ca57 100644 --- a/runtime/include/mod_plugins.h +++ b/runtime/include/mod_plugins.h @@ -23,8 +23,14 @@ int psx_mod_register_vblank_plugin(const char* id, PSXModVBlankCallback callback); int psx_mod_register_function_entry_plugin( const char* id, uint32_t address, PSXModFunctionEntryCallback callback); -/* Called only from generated functions explicitly listed by the game config. */ +/* Called from generated functions listed by the game config and from every + * interpreted entry, so the hook contract does not depend on the backend. + * Hooks match by code address (segment bits ignored) and run only for plugins + * the active plan resolved; the table is rebuilt at plugin activation. */ void psx_mod_function_entry(struct CPUState* cpu, uint32_t address); +/* Active function-entry hook count (0 = none). Hot callers test it before the + * call, so a run without an active hook pays one load per interpreted entry. */ +extern uint32_t g_psx_mod_function_entry_hooks; /* Narrow guest services available to trusted plugin callbacks. */ int psx_mod_game_started(void); diff --git a/runtime/src/dirty_ram_interp.c b/runtime/src/dirty_ram_interp.c index b6ba33fd..5ae87357 100644 --- a/runtime/src/dirty_ram_interp.c +++ b/runtime/src/dirty_ram_interp.c @@ -2896,10 +2896,12 @@ static int dirty_ram_dispatch_inner(CPUState* cpu, uint32_t addr, uint32_t stop_ * psx_mod_function_entry at listed function entries, but a mod-patched * page runs here instead and would silently skip them. Fire the same hook * on interp dispatch so the contract does not depend on which backend - * executes the page. */ + * executes the page. The active-hook count keeps a plan without hooks + * to one load per interpreted entry. */ { + extern uint32_t g_psx_mod_function_entry_hooks; extern void psx_mod_function_entry(CPUState *cpu, uint32_t address); - psx_mod_function_entry(cpu, addr); + if (g_psx_mod_function_entry_hooks) psx_mod_function_entry(cpu, addr); } /* Per-PC entry counter (visible via dirty_ram_stats). */ @@ -3288,8 +3290,10 @@ static int dirty_ram_dispatch_inner(CPUState* cpu, uint32_t addr, uint32_t stop_ /* Local transfers bypass dispatch, but must retain the same * function-entry hooks as a surfaced interpreter entry. */ { + extern uint32_t g_psx_mod_function_entry_hooks; extern void psx_mod_function_entry(CPUState *, uint32_t); - psx_mod_function_entry(cpu, target); + if (g_psx_mod_function_entry_hooks) + psx_mod_function_entry(cpu, target); } pc = target; current_page = target_phys >> 12; diff --git a/runtime/src/mod_packages.cpp b/runtime/src/mod_packages.cpp index 006aa815..c69112ca 100644 --- a/runtime/src/mod_packages.cpp +++ b/runtime/src/mod_packages.cpp @@ -1456,8 +1456,9 @@ bool mod_register_function_entry_plugin(const std::string& id, uint32_t address, PSXModFunctionEntryCallback callback) { if (!valid_id(id) || !address || !callback) return false; RegisteredPlugin& plugin = registered_plugins()[id]; + /* Code addresses alias across KUSEG/KSEG0/KSEG1; one function is one hook. */ for (const auto& hook : plugin.function_entries) - if (hook.first == address) return false; + if (((hook.first ^ address) & 0x1FFFFFFFu) == 0u) return false; plugin.function_entries.emplace_back(address, callback); return true; } @@ -1481,12 +1482,13 @@ void mod_invoke_vblank_plugin(const std::string& id) { found->second.vblank(); } -void mod_invoke_function_entry_plugin(const std::string& id, ::CPUState* cpu, - uint32_t address) { +std::vector mod_function_entry_hooks(const std::string& id) { + std::vector hooks; const auto found = registered_plugins().find(id); - if (found == registered_plugins().end()) return; + if (found == registered_plugins().end()) return hooks; for (const auto& hook : found->second.function_entries) - if (hook.first == address) hook.second(cpu, address); + hooks.push_back(ModFunctionEntryHook{hook.first, hook.second}); + return hooks; } void mod_clear_plugins_for_tests() { diff --git a/runtime/src/mod_runtime.cpp b/runtime/src/mod_runtime.cpp index fe974f02..dfa3cece 100644 --- a/runtime/src/mod_runtime.cpp +++ b/runtime/src/mod_runtime.cpp @@ -50,6 +50,9 @@ extern "C" void gpu_ws_set_adaptive_backdrop_preload(int enabled); extern "C" void dirty_ram_mark_executable_range(uint32_t phys, uint32_t len); extern "C" int fntrace_is_game_started(void); +/* Declared in mod_plugins.h; see active_function_entry_hooks() below. */ +uint32_t g_psx_mod_function_entry_hooks = 0; + namespace PSXRecompV4 { namespace { @@ -80,6 +83,47 @@ RuntimeMods& state() { return value; } +/* Function-entry hooks of the ACTIVE plan, flattened at plugin activation into + * one table sorted by code key (address with the segment bits stripped, so a + * KUSEG or KSEG1 PC reaches the same hook as KSEG0). The interpreter consults + * this on every entry it dispatches, so the lookup is a binary search over + * integers, never a per-plugin string map, and an empty table short-circuits + * in the caller via g_psx_mod_function_entry_hooks. Plugin pointers refer into + * RuntimeMods::plan; every plan replacement clears the table first. */ +struct ActiveFunctionEntryHook { + uint32_t key = 0; + PSXModFunctionEntryCallback callback = nullptr; + const ModResolution::Plugin* plugin = nullptr; +}; + +std::vector& active_function_entry_hooks() { + static std::vector value; + return value; +} + +inline uint32_t function_entry_key(uint32_t address) { + return address & 0x1FFFFFFFu; +} + +void clear_function_entry_hooks() { + active_function_entry_hooks().clear(); + g_psx_mod_function_entry_hooks = 0; +} + +void build_function_entry_hooks(const RuntimeMods& s) { + clear_function_entry_hooks(); + if (!s.initialized || !s.plan.ok) return; + auto& table = active_function_entry_hooks(); + for (const ModResolution::Plugin& plugin : s.plan.plugins) + for (const ModFunctionEntryHook& hook : mod_function_entry_hooks(plugin.id)) + table.push_back({function_entry_key(hook.address), hook.callback, &plugin}); + /* Stable: hooks sharing an address keep plan (plugin order) order. */ + std::stable_sort(table.begin(), table.end(), + [](const ActiveFunctionEntryHook& a, + const ActiveFunctionEntryHook& b) { return a.key < b.key; }); + g_psx_mod_function_entry_hooks = (uint32_t)table.size(); +} + const ModPackage* selected_package(const std::string& id) { return state().manager.selected_package(id); } @@ -1092,6 +1136,7 @@ bool mod_runtime_initialize(const std::filesystem::path& root, std::string* error) { RuntimeMods& s = state(); s.manager.set_root({}); + clear_function_entry_hooks(); s.plan = {}; s.validation = {}; s.raw_disc_index.clear(); @@ -1137,6 +1182,7 @@ bool mod_runtime_clear_for_netplay(std::string* error) { if (error) error->clear(); return true; } + clear_function_entry_hooks(); s.plan = {}; s.validation = {}; s.raw_disc_index.clear(); @@ -1198,6 +1244,9 @@ bool mod_runtime_commit(const std::filesystem::path& disc_path, std::string* err if (error) *error = s.error; return false; } + /* Hooks follow activation, never a bare commit: a new plan runs none of + * its function-entry hooks until mod_runtime_activate_plugins(). */ + clear_function_entry_hooks(); s.plan = std::move(plan); build_disc_index(s); s.effective_disc_path = std::move(effective_disc); @@ -1332,6 +1381,7 @@ extern "C" void mod_runtime_activate_plugins(void) { mod_invoke_activation_plugin(plugin.id); s.current_plugin = nullptr; } + build_function_entry_hooks(s); } extern "C" void mod_runtime_on_vblank(void) { @@ -1477,12 +1527,18 @@ extern "C" int psx_mod_register_function_entry_plugin( extern "C" void psx_mod_function_entry(CPUState* cpu, uint32_t address) { using namespace PSXRecompV4; + if (!g_psx_mod_function_entry_hooks || !cpu) return; RuntimeMods& s = state(); - if (!cpu || !s.initialized || !s.plan.ok) return; - for (const ModResolution::Plugin& plugin : s.plan.plugins) { - s.current_plugin = &plugin; - mod_invoke_function_entry_plugin(plugin.id, cpu, address); - s.current_plugin = nullptr; + const auto& table = active_function_entry_hooks(); + const uint32_t key = function_entry_key(address); + auto it = std::lower_bound( + table.begin(), table.end(), key, + [](const ActiveFunctionEntryHook& hook, uint32_t k) { return hook.key < k; }); + for (; it != table.end() && it->key == key; ++it) { + const ModResolution::Plugin* previous = s.current_plugin; + s.current_plugin = it->plugin; + it->callback(cpu, address); + s.current_plugin = previous; } } diff --git a/runtime/tests/test_interp_mod_entry_guards.py b/runtime/tests/test_interp_mod_entry_guards.py index 7562d5da..3a757245 100644 --- a/runtime/tests/test_interp_mod_entry_guards.py +++ b/runtime/tests/test_interp_mod_entry_guards.py @@ -14,6 +14,11 @@ def main(): 'psx_overlay_dispatch(cpu, addr);') < static_dispatch.index( 'g_exec_phase = previous_phase;') < static_dispatch.index('if (handled) return 1;') assert source.count('psx_mod_function_entry(cpu, addr);') == 1 + # Every interpreted entry reaches these calls: without an active hook they + # must cost one load, never a call into the plugin table. + assert 'if (g_psx_mod_function_entry_hooks) psx_mod_function_entry(cpu, addr);' in source + assert ('if (g_psx_mod_function_entry_hooks)\n' + ' psx_mod_function_entry(cpu, target);') in source local_start = source.index('if (allow_local_dirty_flow && target != 0 &&') local_end = source.index('current_page = target_phys >> 12;', local_start) local = source[local_start:local_end] diff --git a/runtime/tests/test_mod_packages.cpp b/runtime/tests/test_mod_packages.cpp index c7d90002..4d34325e 100644 --- a/runtime/tests/test_mod_packages.cpp +++ b/runtime/tests/test_mod_packages.cpp @@ -372,16 +372,15 @@ int main() { /* A function-entry hook is selected by id exactly like activation and * VBlank plugins; a manifest naming only such an implementation must * resolve, and must still fail closed once it is gone. */ - static int entry_hits = 0; - check(mod_register_function_entry_plugin( - "test.entry", 0x80010000u, - +[](CPUState*, uint32_t address) { - if (address == 0x80010000u) entry_hits++; - }), + const PSXModFunctionEntryCallback entry_hook = +[](CPUState*, uint32_t) {}; + check(mod_register_function_entry_plugin("test.entry", 0x80010000u, entry_hook), "function-entry plugin must register"); check(!mod_register_function_entry_plugin( "test.entry", 0x80010000u, +[](CPUState*, uint32_t) {}), "duplicate function-entry hook must be rejected"); + check(!mod_register_function_entry_plugin( + "test.entry", 0x00010000u, +[](CPUState*, uint32_t) {}), + "a KUSEG alias of a hooked function is the same hook"); write_text(root / "packages/entry.mod/1.0.0/manifest.toml", "format_version = 5\n" "id = \"entry.mod\"\n" @@ -403,9 +402,13 @@ int main() { check(entry_resolution.ok && entry_resolution.plugins.size() == 1 && entry_resolution.plugins[0].id == "test.entry", "enabled function-entry plugin must resolve"); - mod_invoke_function_entry_plugin("test.entry", nullptr, 0x80010004u); - mod_invoke_function_entry_plugin("test.entry", nullptr, 0x80010000u); - check(entry_hits == 1, "function-entry hook must fire only for its address"); + const std::vector entry_hooks = + mod_function_entry_hooks("test.entry"); + check(entry_hooks.size() == 1 && entry_hooks[0].address == 0x80010000u && + entry_hooks[0].callback == entry_hook, + "an implementation exposes exactly the hooks it registered"); + check(mod_function_entry_hooks("test.missing").empty(), + "an unregistered implementation exposes no hooks"); mod_clear_plugins_for_tests(); ModResolution entry_unavailable = reload.resolve("SLUS-TEST"); check(!entry_unavailable.ok, diff --git a/runtime/tests/test_mod_runtime.cpp b/runtime/tests/test_mod_runtime.cpp index 9b66052f..d16c1760 100644 --- a/runtime/tests/test_mod_runtime.cpp +++ b/runtime/tests/test_mod_runtime.cpp @@ -4,6 +4,7 @@ #include "psx_sha256.h" #include "gpu.h" +#include "cpu_state.h" #include #include @@ -73,11 +74,28 @@ extern "C" void gpu_get_display_info(GpuDisplayInfo* out) { extern "C" void dirty_ram_mark_executable_range(uint32_t, uint32_t) {} extern "C" int fntrace_is_game_started(void) { return 1; } +/* Widescreen tag passthroughs mod_runtime forwards to the GPU; unused here. */ +extern "C" void gpu_ws_tag_hud_primitive(uint32_t, int) {} +extern "C" void gpu_ws_tag_world_primitive(uint32_t, int) {} +extern "C" void gpu_ws_set_adaptive_backdrop_preload(int) {} static void test_vblank_plugin(void) { plugin_calls++; } +/* Function-entry hooks: one owned by the plan's active plugin, one by a + * plugin whose feature is disabled, one by an id no package selects. */ +static int active_entry_hits; +static int disabled_entry_hits; +static int unselected_entry_hits; +static uint32_t active_entry_last; +static void test_active_entry(CPUState*, uint32_t address) { + active_entry_hits++; + active_entry_last = address; +} +static void test_disabled_entry(CPUState*, uint32_t) { disabled_entry_hits++; } +static void test_unselected_entry(CPUState*, uint32_t) { unselected_entry_hits++; } + static void test_activation_plugin(void) { activation_calls++; } @@ -253,7 +271,13 @@ int main() { sha256_hex(std::vector(overlay.size(), 0)) + "\"\n" "[[plugin]]\n" "feature = \"vblank-plugin\"\n" - "id = \"runtime.test-vblank\"\n"); + "id = \"runtime.test-vblank\"\n" + "[[feature]]\n" + "id = \"entry-disabled\"\n" + "name = \"Disabled Entry\"\n" + "[[plugin]]\n" + "feature = \"entry-disabled\"\n" + "id = \"runtime.test-disabled-entry\"\n"); write_text(root / "state.toml", "format_version = 2\n" "[[package]]\n" @@ -308,17 +332,60 @@ int main() { check(PSXRecompV4::mod_register_vblank_plugin( "runtime.test-vblank", test_vblank_plugin), "runtime test plugin must register"); + check(psx_mod_register_function_entry_plugin( + "runtime.test-vblank", 0x80003000u, test_active_entry) == 1, + "active plugin's function-entry hook must register"); + check(psx_mod_register_function_entry_plugin( + "runtime.test-disabled-entry", 0x80003000u, test_disabled_entry) == 1, + "disabled feature's function-entry hook must register"); + check(psx_mod_register_function_entry_plugin( + "runtime.unselected-entry", 0x80003000u, test_unselected_entry) == 1, + "unselected function-entry hook must register"); + CPUState entry_cpu{}; check(PSXRecompV4::mod_runtime_initialize( root, "SLUS-RUNTIME", 0x80002000, {}, &error), error.c_str()); check(PSXRecompV4::mod_runtime_commit(cue_path, &error), "CUE and its data-track BIN must have the same mod target identity"); + /* Committed but not yet activated: no hook may run. */ + psx_mod_function_entry(&entry_cpu, 0x80003000u); + check(g_psx_mod_function_entry_hooks == 0 && active_entry_hits == 0, + "function-entry hooks must not run before plugin activation"); mod_runtime_activate_plugins(); check(activation_calls == 1, "resolved trusted plugin must activate before runtime startup"); mod_runtime_on_vblank(); check(plugin_calls == 1, "resolved trusted plugin must run on guest VBlank"); + check(g_psx_mod_function_entry_hooks == 1, + "activation must flatten exactly the active plan's entry hooks"); + psx_mod_function_entry(&entry_cpu, 0x80003000u); + check(active_entry_hits == 1 && active_entry_last == 0x80003000u, + "active plan's function-entry hook must run at its address"); + psx_mod_function_entry(&entry_cpu, 0x00003000u); + psx_mod_function_entry(&entry_cpu, 0xA0003000u); + check(active_entry_hits == 3 && active_entry_last == 0xA0003000u, + "function-entry hooks must match every segment alias of the code address"); + psx_mod_function_entry(&entry_cpu, 0x80003004u); + psx_mod_function_entry(&entry_cpu, 0x80203000u); + check(active_entry_hits == 3, + "function-entry hooks must not run for other addresses or RAM mirrors"); + check(disabled_entry_hits == 0 && unselected_entry_hits == 0, + "hooks of plugins the plan does not activate must never run"); + /* A replaced plan drops every hook until its own activation. */ + check(PSXRecompV4::mod_runtime_clear_for_netplay(&error), error.c_str()); + psx_mod_function_entry(&entry_cpu, 0x80003000u); + check(g_psx_mod_function_entry_hooks == 0 && active_entry_hits == 3, + "clearing the plan must drop its function-entry hooks"); + check(PSXRecompV4::mod_runtime_commit(cue_path, &error), error.c_str()); + psx_mod_function_entry(&entry_cpu, 0x80003000u); + check(active_entry_hits == 3, + "a re-committed plan must not run hooks before activation"); + mod_runtime_activate_plugins(); + psx_mod_function_entry(&entry_cpu, 0x80003000u); + check(active_entry_hits == 4 && disabled_entry_hits == 0 && + unselected_entry_hits == 0, + "re-activation restores exactly the active plan's hooks"); ram[0x1000] = 1; ram[0x1001] = 2; ram[0x1002] = 3; ram[0x1003] = 4; ram[0x1100] = 0; ram[0x1101] = 0;