From 475070e910d427a82309e8670e3d926514682e15 Mon Sep 17 00:00:00 2001 From: MarcelineVQ Date: Mon, 2 Mar 2026 07:34:18 -0800 Subject: [PATCH] Fix marker crash on logout and add module lifecycle table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marker cleanup hook on CleanupWorldAndEntities was never installed — markers.installHooks() was missing from install() in main.zig. Entities created via WorldMarker were never cleaned up before the game's atexit handler iterated the hash table over freed heap memory. Key changes: - Add markers.installHooks() call (the actual crash fix) - Replace manual install/uninstall/shutdown lists with a single modules table that drives all three phases — prevents this class of bug - Gate marker Lua functions, addon, and keybindings behind isActive() so they're skipped when another DLL owns the hooks - Add world_cleanup_hook.detach() to removeHooks() (was missing) - Migrate from vendored libs/hook to external zhook dependency - Unify installHooks return types to void across all modules - Add diagnostic logging to marker cleanup (temporary, for testing) --- .gitignore | 2 + README.md | 2 +- build.zig | 37 +- build.zig.zon | 4 +- libs/hook/build.zig | 35 - libs/hook/build.zig.zon | 10 - libs/hook/src/generic_hook.zig | 279 -------- libs/hook/src/hook.zig | 470 ------------- libs/hook/src/x86dis.zig | 402 ----------- src/assetfix/assetfix.zig | 54 +- src/framecrash/RESEARCH.md | 263 ++++++++ src/framecrash/framecrash.zig | 356 +++++----- src/interact/interact.zig | 30 +- src/main.zig | 105 +-- src/markers/RESEARCH.md | 1107 +++++++++++++++++++++++++++++++ src/markers/addon/Markers.lua | 7 + src/markers/markers.zig | 79 ++- src/markers/offsets.zig | 11 + src/outline/api.zig | 2 +- src/outline/d3d9_hook.zig | 2 +- src/outline/model_hook.zig | 122 +--- src/outline/tracker.zig | 2 +- src/outline/wow.zig | 2 +- src/screenshot/screenshot.zig | 36 +- src/transmogfix/transmogfix.zig | 123 ++-- 25 files changed, 1793 insertions(+), 1749 deletions(-) delete mode 100644 libs/hook/build.zig delete mode 100644 libs/hook/build.zig.zon delete mode 100644 libs/hook/src/generic_hook.zig delete mode 100644 libs/hook/src/hook.zig delete mode 100644 libs/hook/src/x86dis.zig diff --git a/.gitignore b/.gitignore index 70553e7..7ccea66 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ zig-out/ reference/ research/ +docs/ +ideas/ diff --git a/README.md b/README.md index 7389551..7a84761 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ cd /media/storage/projects/zig/weirdutils zig build ``` -Target: x86-windows-msvc (32-bit DLL), Zig 0.15. +Target: x86-windows-msvc (32-bit DLL), Zig 0.16 (patched: fastcall inreg fix). Host: Linux (Arch), game runs via Wine/DXVK. ## Hook Installation Order diff --git a/build.zig b/build.zig index 514e4c2..94d1584 100644 --- a/build.zig +++ b/build.zig @@ -13,11 +13,12 @@ pub fn build(b: *std.Build) void { const enable_interact = b.option(bool, "interact", "Enable interact module") orelse true; const enable_outline = b.option(bool, "outline", "Enable outline module") orelse true; const enable_markers = b.option(bool, "markers", "Enable markers module") orelse true; - const enable_framecrash = b.option(bool, "framecrash", "Enable framecrash fix") orelse true; + const enable_framecrash = b.option(bool, "framecrash", "Enable framecrash fix") orelse false; const enable_combatlog = b.option(bool, "combatlog", "Enable combat log freshness") orelse true; const enable_minimapicons = b.option(bool, "minimapicons", "Enable custom minimap icons") orelse true; const enable_transmogfix = b.option(bool, "transmogfix", "Enable transmog update coalescing") orelse true; const enable_assetfix = b.option(bool, "assetfix", "Enable loose file loading & permissive patch glob") orelse true; + const enable_healtextfix = b.option(bool, "healtextfix", "Enable SuperWoW heal text fix") orelse true; // Create build options module const build_options = b.addOptions(); @@ -30,12 +31,14 @@ pub fn build(b: *std.Build) void { build_options.addOption(bool, "enable_minimapicons", enable_minimapicons); build_options.addOption(bool, "enable_transmogfix", enable_transmogfix); build_options.addOption(bool, "enable_assetfix", enable_assetfix); + build_options.addOption(bool, "enable_healtextfix", enable_healtextfix); const build_options_module = build_options.createModule(); - const hook_mod = b.dependency("hook", .{ + const zhook_dep = b.dependency("zhook", .{ .target = target, .optimize = optimize, - }).module("hook"); + }); + const zhook_mod = zhook_dep.module("zhook"); const lib = b.addLibrary(.{ .name = "weirdutils", @@ -45,7 +48,7 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = optimize, .imports = &.{ - .{ .name = "hook", .module = hook_mod }, + .{ .name = "zhook", .module = zhook_mod }, .{ .name = "build_options", .module = build_options_module }, }, }), @@ -57,18 +60,19 @@ pub fn build(b: *std.Build) void { const build_all_step = b.step("all-variants", "Build all DLL variants"); // Helper to create a single-module build - const Variant = struct { name: []const u8, screenshot: bool, interact: bool, outline: bool, markers: bool, framecrash: bool, combatlog: bool, minimapicons: bool, transmogfix: bool, assetfix: bool }; + const Variant = struct { name: []const u8, screenshot: bool, interact: bool, outline: bool, markers: bool, framecrash: bool, combatlog: bool, minimapicons: bool, transmogfix: bool, assetfix: bool, healtextfix: bool }; inline for (&[_]Variant{ - .{ .name = "full", .screenshot = true, .interact = true, .outline = true, .markers = true, .framecrash = true, .combatlog = true, .minimapicons = true, .transmogfix = true, .assetfix = true }, - .{ .name = "screenshot", .screenshot = true, .interact = false, .outline = false, .markers = false, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false }, - .{ .name = "interact", .screenshot = false, .interact = true, .outline = false, .markers = false, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false }, - .{ .name = "outline", .screenshot = false, .interact = false, .outline = true, .markers = false, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false }, - .{ .name = "markers", .screenshot = false, .interact = false, .outline = false, .markers = true, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false }, - .{ .name = "framecrash", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = true, .combatlog = false, .minimapicons = false, .transmogfix = false, .assetfix = false }, - .{ .name = "combatlog", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false }, - .{ .name = "minimapicons", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = true, .combatlog = false, .minimapicons = true, .transmogfix = false, .assetfix = false }, - .{ .name = "transmogfix", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = false, .minimapicons = false, .transmogfix = true, .assetfix = false }, - .{ .name = "assetfix", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = false, .minimapicons = false, .transmogfix = false, .assetfix = true }, + .{ .name = "full", .screenshot = true, .interact = true, .outline = true, .markers = true, .framecrash = true, .combatlog = true, .minimapicons = true, .transmogfix = true, .assetfix = true, .healtextfix = true }, + .{ .name = "screenshot", .screenshot = true, .interact = false, .outline = false, .markers = false, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "interact", .screenshot = false, .interact = true, .outline = false, .markers = false, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "outline", .screenshot = false, .interact = false, .outline = true, .markers = false, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "markers", .screenshot = false, .interact = false, .outline = false, .markers = true, .framecrash = true, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "framecrash", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = true, .combatlog = false, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "combatlog", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = true, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "minimapicons", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = true, .combatlog = false, .minimapicons = true, .transmogfix = false, .assetfix = false, .healtextfix = false }, + .{ .name = "transmogfix", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = false, .minimapicons = false, .transmogfix = true, .assetfix = false, .healtextfix = false }, + .{ .name = "assetfix", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = false, .minimapicons = false, .transmogfix = false, .assetfix = true, .healtextfix = false }, + .{ .name = "healtextfix", .screenshot = false, .interact = false, .outline = false, .markers = false, .framecrash = false, .combatlog = false, .minimapicons = false, .transmogfix = false, .assetfix = false, .healtextfix = true }, }) |variant| { const opts = b.addOptions(); opts.addOption(bool, "enable_screenshot", variant.screenshot); @@ -80,6 +84,7 @@ pub fn build(b: *std.Build) void { opts.addOption(bool, "enable_minimapicons", variant.minimapicons); opts.addOption(bool, "enable_transmogfix", variant.transmogfix); opts.addOption(bool, "enable_assetfix", variant.assetfix); + opts.addOption(bool, "enable_healtextfix", variant.healtextfix); const variant_lib = b.addLibrary(.{ .name = variant.name, @@ -89,7 +94,7 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = optimize, .imports = &.{ - .{ .name = "hook", .module = hook_mod }, + .{ .name = "zhook", .module = zhook_mod }, .{ .name = "build_options", .module = opts.createModule() }, }, }), diff --git a/build.zig.zon b/build.zig.zon index 01ce0f3..101ba6a 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -3,8 +3,8 @@ .version = "0.1.0", .fingerprint = 0x54f0a9542562d318, .dependencies = .{ - .hook = .{ - .path = "libs/hook", + .zhook = .{ + .path = "../zhook", }, }, .paths = .{ diff --git a/libs/hook/build.zig b/libs/hook/build.zig deleted file mode 100644 index 622b131..0000000 --- a/libs/hook/build.zig +++ /dev/null @@ -1,35 +0,0 @@ -const std = @import("std"); - -pub fn build(b: *std.Build) void { - const target = b.standardTargetOptions(.{}); - const optimize = b.standardOptimizeOption(.{}); - - const hwbp = b.option(bool, "hwbp", "Use hardware breakpoint hooks instead of inline patching") orelse false; - - const options = b.addOptions(); - options.addOption(bool, "use_hwbp", hwbp); - - const hook_mod = b.addModule("hook", .{ - .root_source_file = b.path("src/hook.zig"), - .target = target, - .optimize = optimize, - }); - hook_mod.addOptions("config", options); - - // x86 length disassembler — standalone, no dependencies - const x86dis_mod = b.addModule("x86dis", .{ - .root_source_file = b.path("src/x86dis.zig"), - .target = target, - .optimize = optimize, - }); - - // Generic hook — uses x86dis + hook for auto-sizing trampolines - const generic_hook_mod = b.addModule("generic_hook", .{ - .root_source_file = b.path("src/generic_hook.zig"), - .target = target, - .optimize = optimize, - }); - generic_hook_mod.addImport("x86dis", x86dis_mod); - generic_hook_mod.addImport("hook.zig", hook_mod); - generic_hook_mod.addOptions("config", options); -} diff --git a/libs/hook/build.zig.zon b/libs/hook/build.zig.zon deleted file mode 100644 index dac90b2..0000000 --- a/libs/hook/build.zig.zon +++ /dev/null @@ -1,10 +0,0 @@ -.{ - .name = .hook, - .version = "0.1.0", - .fingerprint = 0xa4584355bc807307, - .paths = .{ - "build.zig", - "build.zig.zon", - "src", - }, -} diff --git a/libs/hook/src/generic_hook.zig b/libs/hook/src/generic_hook.zig deleted file mode 100644 index 231c5f1..0000000 --- a/libs/hook/src/generic_hook.zig +++ /dev/null @@ -1,279 +0,0 @@ -//! Generic x86 inline hook — no manual prologue size or fixup lists needed. -//! -//! Uses the x86 length disassembler (HDE32 port) to automatically determine -//! how many prologue bytes to steal, and relocates all relative instructions. -//! -//! Combines MinHook's compact disassembler with HadesMem's type-safe approach: -//! declare the function signature once at comptime, get a correctly-typed -//! trampoline and detour with zero manual casting. -//! -//! ## Low-level API (GenericHook) -//! -//! ```zig -//! var my_hook: GenericHook = .{}; -//! if (my_hook.install(0x401000, @intFromPtr(&myDetour)) == .ok) { -//! const orig = my_hook.getTrampoline(OrigFnType); -//! _ = orig(); -//! } -//! my_hook.remove(); -//! ``` -//! -//! ## Type-safe API (Detour) — HadesMem-inspired -//! -//! ```zig -//! const MyHook = Detour(fn (u32, u32) callconv(.{ .x86_stdcall = .{} }) u32); -//! var hook: MyHook = .{}; -//! hook.attach(0x401000, myDetour); -//! // Inside detour: hook.callOriginal(.{arg1, arg2}); -//! ``` - -const std = @import("std"); -const x86dis = @import("x86dis"); -const hook_base = @import("hook.zig"); - -const VirtualAlloc = hook_base.VirtualAlloc; -const VirtualFree = hook_base.VirtualFree; -const PAGE_EXECUTE_READWRITE = hook_base.PAGE_EXECUTE_READWRITE; -const MEM_COMMIT = hook_base.MEM_COMMIT; -const MEM_RELEASE = hook_base.MEM_RELEASE; -const writeProtected = hook_base.writeProtected; - -const JMP_SIZE: usize = 5; // E9 + rel32 -const MAX_STOLEN: usize = 32; -const TRAMPOLINE_BUF: usize = 64; - -// ═══════════════════════════════════════════════════════════════════════ -// GenericHook — low-level auto-sizing hook -// ═══════════════════════════════════════════════════════════════════════ - -pub const GenericHook = struct { - mem: ?[*]u8 = null, - trampoline: usize = 0, - target: usize = 0, - stolen_size: usize = 0, - saved_bytes: [MAX_STOLEN]u8 = undefined, - - pub const Error = enum { - ok, - alloc_failed, - disasm_error, - prologue_too_short, - unsupported_relocation, - }; - - /// Analyse target, build trampoline, patch target → detour. One call. - pub fn install(self: *GenericHook, target: usize, detour_addr: usize) Error { - const err = self.prepare(target); - if (err != .ok) return err; - self.activate(detour_addr); - return .ok; - } - - /// Phase 1: disassemble prologue, allocate trampoline, copy + relocate. - pub fn prepare(self: *GenericHook, target: usize) Error { - if (self.mem != null) return .ok; - - const src: [*]const u8 = @ptrFromInt(target); - - // ── determine how many bytes to steal ── - var stolen: usize = 0; - while (stolen < JMP_SIZE) { - const insn = x86dis.decode(src + stolen); - if (insn.flags & x86dis.F_ERROR != 0) return .disasm_error; - if (insn.len == 0) return .disasm_error; - stolen += insn.len; - if (stolen > MAX_STOLEN) return .prologue_too_short; - } - - // ── allocate ── - const mem = VirtualAlloc(null, TRAMPOLINE_BUF, MEM_COMMIT, PAGE_EXECUTE_READWRITE) orelse return .alloc_failed; - - self.mem = mem; - self.target = target; - self.stolen_size = stolen; - self.trampoline = @intFromPtr(mem); - - @memcpy(self.saved_bytes[0..stolen], src[0..stolen]); - - // ── check if already hooked (E9 at target) — chain through ── - if (src[0] == 0xE9) { - const other_detour = hook_base.rel32Target(target); - mem[0] = 0xE9; - hook_base.writeRel32(mem + 1, self.trampoline + 1, other_detour); - return .ok; - } - - // ── build trampoline: copy + relocate ── - var t_pos: usize = 0; - var s_pos: usize = 0; - - while (s_pos < stolen) { - const insn = x86dis.decode(src + s_pos); - const op = insn.opcode; - const src_addr = target + s_pos; - const dst_addr = self.trampoline + t_pos; - - if (insn.flags & x86dis.F_RELATIVE != 0) { - if (op == 0xE8 or op == 0xE9) { - // CALL/JMP rel32 - const abs = hook_base.rel32Target(src_addr); - mem[t_pos] = op; - hook_base.writeRel32(mem + t_pos + 1, dst_addr + 1, abs); - t_pos += 5; - } else if (op == 0x0F and insn.opcode2 >= 0x80 and insn.opcode2 <= 0x8F) { - // Jcc rel32 (0F 80-8F) - const abs = jcc32Target(src_addr); - mem[t_pos] = 0x0F; - mem[t_pos + 1] = insn.opcode2; - hook_base.writeRel32(mem + t_pos + 2, dst_addr + 2, abs); - t_pos += 6; - } else if (op >= 0x70 and op <= 0x7F) { - // Short Jcc → expand to near Jcc (0F 8x) - const offset = @as(i8, @bitCast(src[s_pos + 1])); - const abs: usize = @bitCast(@as(isize, @intCast(src_addr + 2)) + offset); - mem[t_pos] = 0x0F; - mem[t_pos + 1] = op + 0x10; - hook_base.writeRel32(mem + t_pos + 2, dst_addr + 2, abs); - t_pos += 6; - } else if (op == 0xEB) { - // Short JMP → expand to near JMP (E9) - const offset = @as(i8, @bitCast(src[s_pos + 1])); - const abs: usize = @bitCast(@as(isize, @intCast(src_addr + 2)) + offset); - mem[t_pos] = 0xE9; - hook_base.writeRel32(mem + t_pos + 1, dst_addr + 1, abs); - t_pos += 5; - } else { - // LOOP/JECXZ or unknown — cannot trivially expand - self.cleanup(); - return .unsupported_relocation; - } - } else { - // Non-relative — copy verbatim - @memcpy(mem[t_pos .. t_pos + insn.len], src[s_pos .. s_pos + insn.len]); - t_pos += insn.len; - } - s_pos += insn.len; - } - - // ── JMP back to original code after stolen bytes ── - mem[t_pos] = 0xE9; - hook_base.writeRel32( - mem + t_pos + 1, - self.trampoline + t_pos + 1, - target + stolen, - ); - - return .ok; - } - - /// Phase 2: write the E9 JMP patch at the target. - pub fn activate(self: *GenericHook, detour_addr: usize) void { - var patch: [MAX_STOLEN]u8 = .{0x90} ** MAX_STOLEN; - patch[0] = 0xE9; - hook_base.writeRel32(patch[1..5], self.target + 1, detour_addr); - writeProtected(self.target, patch[0..self.stolen_size]); - } - - /// Restore original bytes and free trampoline memory. - pub fn remove(self: *GenericHook) void { - if (self.mem == null) return; - writeProtected(self.target, self.saved_bytes[0..self.stolen_size]); - _ = VirtualFree(@ptrFromInt(@intFromPtr(self.mem.?)), 0, MEM_RELEASE); - self.mem = null; - } - - /// Get trampoline as a typed function pointer. - pub fn getTrampoline(self: *const GenericHook, comptime T: type) T { - return @ptrFromInt(self.trampoline); - } - - fn cleanup(self: *GenericHook) void { - if (self.mem) |m| { - _ = VirtualFree(@ptrFromInt(@intFromPtr(m)), 0, MEM_RELEASE); - self.mem = null; - } - } -}; - -// ═══════════════════════════════════════════════════════════════════════ -// Detour(FnType) — HadesMem-style type-safe generic hook -// ═══════════════════════════════════════════════════════════════════════ - -/// Comptime-generic typed detour. Declare the target function's type once; -/// get type-checked attach/callOriginal with no manual pointer casts. -/// -/// ```zig -/// const StdcallU32x2 = fn (u32, u32) callconv(.{ .x86_stdcall = .{} }) u32; -/// const MyHook = generic_hook.Detour(StdcallU32x2); -/// var hook: MyHook = .{}; -/// hook.attach(0x401000, &myDetour); -/// // in detour: hook.callOriginal(.{ a, b }); -/// hook.detach(); -/// ``` -pub fn Detour(comptime FnType: type) type { - const FnInfo = @typeInfo(FnType).@"fn"; - const FnPtr = *const FnType; - const ReturnType = FnInfo.return_type orelse void; - const ParamTypes = FnInfo.params; - - return struct { - inner: GenericHook = .{}, - - const Self = @This(); - - /// Hook the function at `target` to redirect to `detour`. - pub fn attach(self: *Self, target: usize, detour: FnPtr) GenericHook.Error { - return self.inner.install(target, @intFromPtr(detour)); - } - - /// Call the original (pre-hook) function through the trampoline. - pub fn callOriginal(self: *const Self, args: anytype) ReturnType { - const orig: FnPtr = @ptrFromInt(self.inner.trampoline); - return @call(.auto, orig, coerceArgs(ParamTypes, args)); - } - - /// Unhook: restore original bytes, free trampoline. - pub fn detach(self: *Self) void { - self.inner.remove(); - } - - /// Get the trampoline as the correctly-typed function pointer. - pub fn original(self: *const Self) FnPtr { - return @ptrFromInt(self.inner.trampoline); - } - }; -} - -/// Coerce a tuple of args into the exact parameter types expected. -fn coerceArgs(comptime params: anytype, args: anytype) CoercedTuple(params) { - var result: CoercedTuple(params) = undefined; - inline for (0..params.len) |idx| { - @field(result, std.fmt.comptimePrint("{d}", .{idx})) = args[idx]; - } - return result; -} - -fn CoercedTuple(comptime params: anytype) type { - var fields: [params.len]std.builtin.Type.StructField = undefined; - inline for (0..params.len) |idx| { - fields[idx] = .{ - .name = std.fmt.comptimePrint("{d}", .{idx}), - .type = params[idx].type.?, - .default_value_ptr = null, - .is_comptime = false, - .alignment = 0, - }; - } - return @Type(.{ .@"struct" = .{ - .layout = .auto, - .fields = &fields, - .decls = &.{}, - .is_tuple = true, - } }); -} - -/// Resolve absolute target of a Jcc rel32 (0F 8x xx xx xx xx) — 6 byte insn. -fn jcc32Target(addr: usize) usize { - const disp: u32 = @bitCast(@as(*align(1) const i32, @ptrFromInt(addr + 2)).*); - return (addr + 6) +% disp; -} diff --git a/libs/hook/src/hook.zig b/libs/hook/src/hook.zig deleted file mode 100644 index 20905cf..0000000 --- a/libs/hook/src/hook.zig +++ /dev/null @@ -1,470 +0,0 @@ -//! x86 inline hooking library for Windows DLL injection. -//! -//! Provides a `Hook` struct for patching function prologues with JMP detours, -//! building trampolines to call the original, and chaining with other hooks. -//! Also includes memory read/write helpers, rel32 arithmetic, a generic -//! `fastcall` caller, and a fastcall-to-cdecl thunk builder. -//! -//! ## Quick start -//! -//! ```zig -//! const hook = @import("hook.zig"); -//! -//! var my_hook = hook.Hook{}; -//! -//! // One-shot: prepare trampoline + patch in one call. -//! // The last arg is a slice of opcode offsets within the prologue that -//! // contain E8/E9 (CALL/JMP rel32) instructions needing fixup. -//! _ = my_hook.install(target_addr, prologue_size, @intFromPtr(&detour), &.{1}); -//! -//! // Two-phase (when you need the alloc block before patching, e.g. for a thunk): -//! _ = my_hook.prepare(target_addr, prologue_size, &.{}); -//! const thunk_buf = my_hook.mem.? + 32; -//! _ = hook.buildFastcallToCdeclThunk(thunk_buf, @intFromPtr(&hookImpl), 1); -//! my_hook.activate(@intFromPtr(thunk_buf)); -//! -//! // Call the original from inside the detour: -//! const orig = my_hook.getTrampoline(*const fn () callconv(.{ .x86_stdcall = .{} }) void); -//! orig(); -//! -//! // Unhook (restore original bytes, free memory): -//! my_hook.remove(); -//! ``` - -const std = @import("std"); -const use_hwbp = @import("config").use_hwbp; - -// ============================================================================= -// Windows API -// ============================================================================= - -const WINAPI = std.builtin.CallingConvention.winapi; - -pub const PAGE_EXECUTE_READWRITE: u32 = 0x40; -pub const MEM_COMMIT: u32 = 0x1000; -pub const MEM_RELEASE: u32 = 0x8000; - -extern "kernel32" fn VirtualProtect( - lpAddress: *anyopaque, - dwSize: usize, - flNewProtect: u32, - lpflOldProtect: *u32, -) callconv(WINAPI) i32; - -extern "kernel32" fn VirtualAlloc( - lpAddress: ?*anyopaque, - dwSize: usize, - flAllocationType: u32, - flProtect: u32, -) callconv(WINAPI) ?[*]u8; - -extern "kernel32" fn VirtualFree( - lpAddress: *anyopaque, - dwSize: usize, - dwFreeType: u32, -) callconv(WINAPI) i32; - -// ============================================================================= -// Hardware breakpoint support (DR0-DR3 + Vectored Exception Handler) -// ============================================================================= - -const CONTEXT_DEBUG_REGISTERS: u32 = 0x00010010; -const EXCEPTION_SINGLE_STEP: u32 = 0x80000004; -const EXCEPTION_CONTINUE_EXECUTION: i32 = -1; -const EXCEPTION_CONTINUE_SEARCH: i32 = 0; - -extern "kernel32" fn GetCurrentThread() callconv(WINAPI) *anyopaque; - -extern "kernel32" fn GetThreadContext( - hThread: *anyopaque, - lpContext: *CONTEXT, -) callconv(WINAPI) i32; - -extern "kernel32" fn SetThreadContext( - hThread: *anyopaque, - lpContext: *const CONTEXT, -) callconv(WINAPI) i32; - -extern "kernel32" fn AddVectoredExceptionHandler( - First: u32, - Handler: *const fn (*EXCEPTION_POINTERS) callconv(WINAPI) i32, -) callconv(WINAPI) ?*anyopaque; - -extern "kernel32" fn RemoveVectoredExceptionHandler( - Handle: *anyopaque, -) callconv(WINAPI) u32; - -const CONTEXT = extern struct { - ContextFlags: u32, - Dr0: u32, - Dr1: u32, - Dr2: u32, - Dr3: u32, - Dr6: u32, - Dr7: u32, - FloatSave: [112]u8, - SegGs: u32, - SegFs: u32, - SegEs: u32, - SegDs: u32, - Edi: u32, - Esi: u32, - Ebx: u32, - Edx: u32, - Ecx: u32, - Eax: u32, - Ebp: u32, - Eip: u32, - SegCs: u32, - EFlags: u32, - Esp: u32, - SegSs: u32, - ExtendedRegisters: [512]u8, -}; - -const EXCEPTION_RECORD = extern struct { - ExceptionCode: u32, - ExceptionFlags: u32, - ExceptionRecord: ?*EXCEPTION_RECORD, - ExceptionAddress: ?*anyopaque, - NumberParameters: u32, - ExceptionInformation: [15]usize, -}; - -const EXCEPTION_POINTERS = extern struct { - ExceptionRecord: *EXCEPTION_RECORD, - ContextRecord: *CONTEXT, -}; - -var hwbp_slots: [4]?*Hook = .{ null, null, null, null }; -var hwbp_detours: [4]usize = .{ 0, 0, 0, 0 }; -var veh_handle: ?*anyopaque = null; - -fn vehHandler(info: *EXCEPTION_POINTERS) callconv(WINAPI) i32 { - if (info.ExceptionRecord.ExceptionCode != EXCEPTION_SINGLE_STEP) - return EXCEPTION_CONTINUE_SEARCH; - - const eip = info.ContextRecord.Eip; - for (0..4) |i| { - if (hwbp_slots[i]) |h| { - if (h.target == eip) { - info.ContextRecord.Eip = @intCast(hwbp_detours[i]); - return EXCEPTION_CONTINUE_EXECUTION; - } - } - } - - return EXCEPTION_CONTINUE_SEARCH; -} - -// ============================================================================= -// Memory helpers -// ============================================================================= - -/// Read a value of type `T` from an arbitrary memory address (unaligned). -pub fn readMem(comptime T: type, addr: usize) T { - return @as(*align(1) const T, @ptrFromInt(addr)).*; -} - -/// Write raw bytes to an arbitrary memory address. No protection change — -/// caller must ensure the page is writable (or use `writeProtected`). -pub fn writeMem(addr: usize, bytes: []const u8) void { - const dest: [*]u8 = @ptrFromInt(addr); - for (bytes, 0..) |b, i| { - dest[i] = b; - } -} - -/// Write bytes to a potentially read-only/executable page. Temporarily sets -/// PAGE_EXECUTE_READWRITE, writes, then restores the original protection. -pub fn writeProtected(addr: usize, bytes: []const u8) void { - var old: u32 = 0; - _ = VirtualProtect(@ptrFromInt(addr), bytes.len, PAGE_EXECUTE_READWRITE, &old); - writeMem(addr, bytes); - _ = VirtualProtect(@ptrFromInt(addr), bytes.len, old, &old); -} - -// ============================================================================= -// Rel32 helpers -// ============================================================================= - -/// Resolve the absolute target of an E8 (CALL) or E9 (JMP) at `addr`. -/// Reads the signed rel32 operand at addr+1 and computes addr+5+offset. -pub fn rel32Target(addr: usize) usize { - const offset: u32 = @bitCast(@as(*align(1) const i32, @ptrFromInt(addr + 1)).*); - return (addr + 5) +% offset; -} - -/// Write a rel32 displacement into dest[0..4] such that a JMP/CALL -/// from address `from` reaches `to`. Displacement = to - (from + 4). -pub fn writeRel32(dest: [*]u8, from: usize, to: usize) void { - std.mem.writeInt(u32, dest[0..4], to -% (from + 4), .little); -} - -// ============================================================================= -// Calling convention helper -// ============================================================================= - -/// Call a __fastcall function at `addr` with ECX and EDX arguments. -/// Dispatches on return type: void, f64 (x87 ST0), or integer/pointer (EAX). -pub fn fastcall(comptime R: type, addr: usize, ecx: anytype, edx: anytype) R { - if (R == f64) { - return asm volatile ("call *%[func]" - : [ret] "={st}" (-> f64), - : [_] "{ecx}" (ecx), [_] "{edx}" (edx), [func] "r" (addr), - : .{ .eax = true, .memory = true, .cc = true } - ); - } else if (R == void) { - asm volatile ("call *%[func]" - : - : [_] "{ecx}" (ecx), [_] "{edx}" (edx), [func] "r" (addr), - : .{ .eax = true, .memory = true, .cc = true } - ); - } else { - return asm volatile ("call *%[func]" - : [ret] "={eax}" (-> R), - : [_] "{ecx}" (ecx), [_] "{edx}" (edx), [func] "r" (addr), - : .{ .memory = true, .cc = true } - ); - } -} - -// ============================================================================= -// Hook struct -// ============================================================================= - -const ALLOC_SIZE: usize = 64; -const TRAMPOLINE_RESERVE: usize = 32; -const MAX_PROLOGUE: usize = 16; - -pub const Hook = struct { - mem: ?[*]u8 = null, - trampoline: usize = 0, - target: usize = 0, - prologue_size: usize = 0, - saved_bytes: [MAX_PROLOGUE]u8 = undefined, - - /// Allocate executable memory, save current bytes at target, and build the - /// trampoline. Does NOT patch the target yet — call activate() after. - /// `rel32_fixups` is a slice of opcode offsets within the prologue that - /// contain E8/E9 instructions needing rel32 adjustment. - pub fn prepare( - self: *Hook, - target: usize, - prologue_size: usize, - rel32_fixups: []const usize, - ) bool { - if (self.mem != null) return true; - - const mem = VirtualAlloc(null, ALLOC_SIZE, MEM_COMMIT, PAGE_EXECUTE_READWRITE) orelse return false; - self.mem = mem; - self.target = target; - self.prologue_size = prologue_size; - - // Save current bytes for remove() - const src: [*]const u8 = @ptrFromInt(target); - @memcpy(self.saved_bytes[0..prologue_size], src[0..prologue_size]); - - // Build trampoline - self.trampoline = @intFromPtr(mem); - - if (src[0] == 0xE9) { - // Another DLL already hooked — resolve their JMP and chain through it - const other_detour = rel32Target(target); - mem[0] = 0xE9; - writeRel32(mem + 1, self.trampoline + 1, other_detour); - } else { - // Original prologue — copy bytes, fix up any relative instructions, JMP back - @memcpy(mem[0..prologue_size], src[0..prologue_size]); - - for (rel32_fixups) |opcode_offset| { - const abs_target = rel32Target(target + opcode_offset); - const tramp_operand = self.trampoline + opcode_offset + 1; - writeRel32(mem + opcode_offset + 1, tramp_operand, abs_target); - } - - mem[prologue_size] = 0xE9; - writeRel32( - mem + prologue_size + 1, - self.trampoline + prologue_size + 1, - target + prologue_size, - ); - } - - return true; - } - - /// Write the E9 JMP patch (inline mode) or set a hardware breakpoint - /// (HWBP mode) to redirect target → detour_addr. - pub fn activate(self: *Hook, detour_addr: usize) void { - if (use_hwbp) { - // Register VEH on first use - if (veh_handle == null) { - veh_handle = AddVectoredExceptionHandler(1, &vehHandler); - } - // Find free DR slot - const slot: u2 = for (0..4) |i| { - if (hwbp_slots[i] == null) break @as(u2, @intCast(i)); - } else return; // all 4 slots occupied - - hwbp_slots[slot] = self; - hwbp_detours[slot] = detour_addr; - - const thread = GetCurrentThread(); - var ctx: CONTEXT = std.mem.zeroes(CONTEXT); - ctx.ContextFlags = CONTEXT_DEBUG_REGISTERS; - _ = GetThreadContext(thread, &ctx); - - // Set DRn to target address - switch (slot) { - 0 => { ctx.Dr0 = @intCast(self.target); }, - 1 => { ctx.Dr1 = @intCast(self.target); }, - 2 => { ctx.Dr2 = @intCast(self.target); }, - 3 => { ctx.Dr3 = @intCast(self.target); }, - } - - // Enable local execute breakpoint in DR7 - const s: u5 = slot; - ctx.Dr7 |= @as(u32, 1) << (s * 2); - ctx.Dr7 &= ~(@as(u32, 0xF) << (16 + s * 4)); - - _ = SetThreadContext(thread, &ctx); - } else { - var patch: [MAX_PROLOGUE]u8 = .{0x90} ** MAX_PROLOGUE; - patch[0] = 0xE9; - writeRel32(patch[1..5], self.target + 1, detour_addr); - writeProtected(self.target, patch[0..self.prologue_size]); - } - } - - /// Convenience: prepare + activate in one call. - pub fn install( - self: *Hook, - target: usize, - prologue_size: usize, - detour_addr: usize, - rel32_fixups: []const usize, - ) bool { - if (!self.prepare(target, prologue_size, rel32_fixups)) return false; - self.activate(detour_addr); - return true; - } - - /// Restore original bytes (inline) or clear the DR slot (HWBP), then free - /// the trampoline memory. - pub fn remove(self: *Hook) void { - if (self.mem == null) return; - - if (use_hwbp) { - for (0..4) |i| { - if (hwbp_slots[i]) |h| { - if (h == self) { - const thread = GetCurrentThread(); - var ctx: CONTEXT = std.mem.zeroes(CONTEXT); - ctx.ContextFlags = CONTEXT_DEBUG_REGISTERS; - _ = GetThreadContext(thread, &ctx); - - switch (@as(u2, @intCast(i))) { - 0 => { ctx.Dr0 = 0; }, - 1 => { ctx.Dr1 = 0; }, - 2 => { ctx.Dr2 = 0; }, - 3 => { ctx.Dr3 = 0; }, - } - const s: u5 = @intCast(i); - ctx.Dr7 &= ~(@as(u32, 1) << (s * 2)); - ctx.Dr7 &= ~(@as(u32, 0xF) << (16 + s * 4)); - - _ = SetThreadContext(thread, &ctx); - - hwbp_slots[i] = null; - hwbp_detours[i] = 0; - break; - } - } - } - } else { - writeProtected(self.target, self.saved_bytes[0..self.prologue_size]); - } - - _ = VirtualFree(@ptrFromInt(@intFromPtr(self.mem.?)), 0, MEM_RELEASE); - self.mem = null; - } - - /// Cast trampoline address to a typed function pointer for calling the original. - pub fn getTrampoline(self: *const Hook, comptime T: type) T { - return @ptrFromInt(self.trampoline); - } -}; - -// ============================================================================= -// Thunk builder: fastcall(ECX, EDX, stack...) → cdecl(stack, stack, stack...) -// ============================================================================= - -/// Build a fastcall-to-cdecl bridge thunk in `buf`. -/// `cdecl_fn` is the address of the cdecl target function. -/// `stack_arg_count` is the number of extra stack arguments beyond ECX/EDX. -/// Returns the total thunk size in bytes. -/// -/// Generated code: -/// push dword [esp + 4*N] ; for each stack arg, right-to-left -/// ... -/// push edx ; arg 2 -/// push ecx ; arg 1 -/// mov eax, -/// call eax -/// add esp, (2 + stack_arg_count) * 4 -/// ret (stack_arg_count * 4) -pub fn buildFastcallToCdeclThunk(buf: [*]u8, cdecl_fn: usize, stack_arg_count: u8) usize { - var pos: usize = 0; - - // Push stack args right-to-left. At entry, [esp] = return addr, - // [esp+4] = first stack arg, [esp+8] = second, etc. - // But each push shifts esp, so we always read from [esp + 4 * stack_arg_count] - // (the offset stays constant because we push the same number of times as the depth grows). - var i: u8 = stack_arg_count; - while (i > 0) : (i -= 1) { - // push dword ptr [esp + 4 * stack_arg_count] - buf[pos] = 0xFF; - buf[pos + 1] = 0x74; - buf[pos + 2] = 0x24; - buf[pos + 3] = stack_arg_count * 4; - pos += 4; - } - - // push edx (arg 2) - buf[pos] = 0x52; - pos += 1; - - // push ecx (arg 1) - buf[pos] = 0x51; - pos += 1; - - // mov eax, - buf[pos] = 0xB8; - std.mem.writeInt(u32, buf[pos + 1 ..][0..4], @intCast(cdecl_fn), .little); - pos += 5; - - // call eax - buf[pos] = 0xFF; - buf[pos + 1] = 0xD0; - pos += 2; - - // add esp, (2 + stack_arg_count) * 4 (cdecl caller cleanup) - const cleanup: u8 = (2 + stack_arg_count) * 4; - buf[pos] = 0x83; - buf[pos + 1] = 0xC4; - buf[pos + 2] = cleanup; - pos += 3; - - // ret (stack_arg_count * 4) (fastcall callee cleans stack args) - if (stack_arg_count == 0) { - buf[pos] = 0xC3; // ret - pos += 1; - } else { - buf[pos] = 0xC2; // ret imm16 - std.mem.writeInt(u16, buf[pos + 1 ..][0..2], @as(u16, stack_arg_count) * 4, .little); - pos += 3; - } - - return pos; -} diff --git a/libs/hook/src/x86dis.zig b/libs/hook/src/x86dis.zig deleted file mode 100644 index bb57720..0000000 --- a/libs/hook/src/x86dis.zig +++ /dev/null @@ -1,402 +0,0 @@ -//! Minimal x86 (32-bit) length disassembler. -//! -//! Faithful port of Vyacheslav Patkov's Hacker Disassembler Engine 32 (HDE32), -//! used by MinHook. Only computes instruction length + flags needed for -//! relocation (F_RELATIVE). ~470 bytes of table data, compiles to ~1-2 KB. - -const std = @import("std"); - -// ── public flags ─────────────────────────────────────────────────────── -pub const F_MODRM: u32 = 0x00000001; -pub const F_SIB: u32 = 0x00000002; -pub const F_IMM8: u32 = 0x00000004; -pub const F_IMM16: u32 = 0x00000008; -pub const F_IMM32: u32 = 0x00000010; -pub const F_DISP8: u32 = 0x00000020; -pub const F_DISP16: u32 = 0x00000040; -pub const F_DISP32: u32 = 0x00000080; -pub const F_RELATIVE: u32 = 0x00000100; -pub const F_ERROR: u32 = 0x00001000; - -// ── internal cflags ──────────────────────────────────────────────────── -const C_MODRM: u8 = 0x01; -const C_IMM8: u8 = 0x02; -const C_IMM16: u8 = 0x04; -const C_IMM_P66: u8 = 0x10; -const C_REL8: u8 = 0x20; -const C_REL32: u8 = 0x40; -const C_GROUP: u8 = 0x80; -const C_ERROR: u8 = 0xff; - -const PRE_NONE: u8 = 0x01; -const PRE_66: u8 = 0x08; -const PRE_67: u8 = 0x10; - -const DELTA_OPCODES: usize = 0x4a; - -// HDE32 opcode table — verbatim from table32.h -const hde32_table = [_]u8{ - 0xa3, 0xa8, 0xa3, 0xa8, 0xa3, 0xa8, 0xa3, 0xa8, 0xa3, 0xa8, 0xa3, 0xa8, 0xa3, 0xa8, 0xa3, - 0xa8, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xac, 0xaa, 0xb2, 0xaa, 0x9f, 0x9f, - 0x9f, 0x9f, 0xb5, 0xa3, 0xa3, 0xa4, 0xaa, 0xaa, 0xba, 0xaa, 0x96, 0xaa, 0xa8, 0xaa, 0xc3, - 0xc3, 0x96, 0x96, 0xb7, 0xae, 0xd6, 0xbd, 0xa3, 0xc5, 0xa3, 0xa3, 0x9f, 0xc3, 0x9c, 0xaa, - 0xaa, 0xac, 0xaa, 0xbf, 0x03, 0x7f, 0x11, 0x7f, 0x01, 0x7f, 0x01, 0x3f, 0x01, 0x01, 0x90, - 0x82, 0x7d, 0x97, 0x59, 0x59, 0x59, 0x59, 0x59, 0x7f, 0x59, 0x59, 0x60, 0x7d, 0x7f, 0x7f, - 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x9a, 0x88, 0x7d, - 0x59, 0x50, 0x50, 0x50, 0x50, 0x59, 0x59, 0x59, 0x59, 0x61, 0x94, 0x61, 0x9e, 0x59, 0x59, - 0x85, 0x59, 0x92, 0xa3, 0x60, 0x60, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, 0x59, - 0x59, 0x59, 0x9f, 0x01, 0x03, 0x01, 0x04, 0x03, 0xd5, 0x03, 0xcc, 0x01, 0xbc, 0x03, 0xf0, - 0x10, 0x10, 0x10, 0x10, 0x50, 0x50, 0x50, 0x50, 0x14, 0x20, 0x20, 0x20, 0x20, 0x01, 0x01, - 0x01, 0x01, 0xc4, 0x02, 0x10, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0xc0, 0xc2, 0x10, 0x11, - 0x02, 0x03, 0x11, 0x03, 0x03, 0x04, 0x00, 0x00, 0x14, 0x00, 0x02, 0x00, 0x00, 0xc6, 0xc8, - 0x02, 0x02, 0x02, 0x02, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0xff, 0xca, - 0x01, 0x01, 0x01, 0x00, 0x06, 0x00, 0x04, 0x00, 0xc0, 0xc2, 0x01, 0x01, 0x03, 0x01, 0xff, - 0xff, 0x01, 0x00, 0x03, 0xc4, 0xc4, 0xc6, 0x03, 0x01, 0x01, 0x01, 0xff, 0x03, 0x03, 0x03, - 0xc8, 0x40, 0x00, 0x0a, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x7f, 0x00, 0x33, 0x01, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xbf, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x07, 0x00, - 0x00, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0xff, 0xff, 0x00, 0x00, 0x00, 0xbf, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x7f, 0x00, 0x00, 0xff, 0x4a, 0x4a, 0x4a, 0x4a, 0x4b, 0x52, 0x4a, 0x4a, 0x4a, 0x4a, 0x4f, - 0x4c, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x55, 0x45, 0x40, 0x4a, 0x4a, 0x4a, - 0x45, 0x59, 0x4d, 0x46, 0x4a, 0x5d, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, - 0x4a, 0x4a, 0x4a, 0x4a, 0x4a, 0x61, 0x63, 0x67, 0x4e, 0x4a, 0x4a, 0x6b, 0x6d, 0x4a, 0x4a, - 0x45, 0x6d, 0x4a, 0x4a, 0x44, 0x45, 0x4a, 0x4a, 0x00, 0x00, 0x00, 0x02, 0x0d, 0x06, 0x06, - 0x06, 0x06, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x06, 0x06, 0x06, 0x00, 0x06, 0x06, 0x02, 0x06, - 0x00, 0x0a, 0x0a, 0x07, 0x07, 0x06, 0x02, 0x05, 0x05, 0x02, 0x02, 0x00, 0x00, 0x04, 0x04, - 0x04, 0x04, 0x00, 0x00, 0x00, 0x0e, 0x05, 0x06, 0x06, 0x06, 0x01, 0x06, 0x00, 0x00, 0x08, - 0x00, 0x10, 0x00, 0x18, 0x00, 0x20, 0x00, 0x28, 0x00, 0x30, 0x00, 0x80, 0x01, 0x82, 0x01, - 0x86, 0x00, 0xf6, 0xcf, 0xfe, 0x3f, 0xab, 0x00, 0xb0, 0x00, 0xb1, 0x00, 0xb3, 0x00, 0xba, - 0xf8, 0xbb, 0x00, 0xc0, 0x00, 0xc1, 0x00, 0xc7, 0xbf, 0x62, 0xff, 0x00, 0x8d, 0xff, 0x00, - 0xc4, 0xff, 0x00, 0xc5, 0xff, 0x00, -}; - -pub const Insn = struct { - len: u8, - flags: u32, - opcode: u8, - opcode2: u8, -}; - -/// Decode the instruction at `code`, returning its length and flags. -pub fn decode(code: [*]const u8) Insn { - var result = Insn{ .len = 0, .flags = 0, .opcode = 0, .opcode2 = 0 }; - - var p: usize = 0; - var pref: u8 = 0; - var disp_size: u8 = 0; - - // ── prefixes ── - var prefix_count: u8 = 16; - prefix_loop: while (prefix_count > 0) : (prefix_count -= 1) { - switch (code[p]) { - 0xf3, 0xf2 => pref |= if (code[p] == 0xf3) 0x04 else 0x02, - 0xf0 => pref |= 0x20, // PRE_LOCK - 0x26, 0x2e, 0x36, 0x3e, 0x64, 0x65 => pref |= 0x40, // PRE_SEG - 0x66 => pref |= PRE_66, - 0x67 => pref |= PRE_67, - else => break :prefix_loop, - } - p += 1; - } - - result.flags = @as(u32, pref) << 23; - - if (pref == 0) pref |= PRE_NONE; - - // ── opcode ── - var ht_base: usize = 0; - var c = code[p]; - p += 1; - result.opcode = c; - - if (c == 0x0f) { - // two-byte opcode - result.opcode2 = code[p]; - c = code[p]; - p += 1; - ht_base = DELTA_OPCODES; - } else if (c >= 0xa0 and c <= 0xa3) { - // MOV moffs — address-size prefix swaps operand-size behavior - if (pref & PRE_67 != 0) - pref |= PRE_66 - else - pref &= ~PRE_66; - } - - const opcode = c; - - // ── two-level table lookup: ht[ht[opcode/4] + (opcode%4)] ── - var cflags: u8 = blk: { - const idx1 = ht_base + @as(usize, opcode / 4); - if (idx1 >= hde32_table.len) break :blk C_ERROR; - const idx2 = ht_base + @as(usize, hde32_table[idx1]) + @as(usize, opcode % 4); - if (idx2 >= hde32_table.len) break :blk C_ERROR; - break :blk hde32_table[idx2]; - }; - - if (cflags == C_ERROR) { - result.flags |= F_ERROR; - cflags = 0; - if ((opcode & 0xfd) == 0x24) // (opcode & -3) == 0x24 - cflags +%= 1; - } - - // ── group resolution ── - var x: u8 = 0; - if (cflags & C_GROUP != 0) { - const group_idx = ht_base + @as(usize, cflags & 0x7f); - if (group_idx + 1 < hde32_table.len) { - const t = std.mem.readInt(u16, hde32_table[group_idx..][0..2], .little); - cflags = @truncate(t); - x = @truncate(t >> 8); - } - } - - // ── modrm ── - if (cflags & C_MODRM != 0) { - result.flags |= F_MODRM; - const modrm = code[p]; - p += 1; - const m_mod = modrm >> 6; - const m_rm: u8 = modrm & 7; - const m_reg: u3 = @truncate((modrm & 0x3f) >> 3); - - // F6 TEST imm8 / F7 TEST imm16/32 - if (m_reg <= 1) { - if (opcode == 0xf6) - cflags |= C_IMM8; - if (opcode == 0xf7) - cflags |= C_IMM_P66; - } - - // displacement - switch (m_mod) { - 0 => { - if (pref & PRE_67 != 0) { - if (m_rm == 6) disp_size = 2; - } else { - if (m_rm == 5) disp_size = 4; - } - }, - 1 => disp_size = 1, - 2 => { - disp_size = 2; - if (pref & PRE_67 == 0) - disp_size = 4; - }, - else => {}, - } - - // SIB byte - if (m_mod != 3 and m_rm == 4 and (pref & PRE_67 == 0)) { - result.flags |= F_SIB; - const sib = code[p]; - p += 1; - if ((sib & 7) == 5 and (m_mod & 1) == 0) - disp_size = 4; - } - - // displacement bytes - switch (disp_size) { - 1 => { - result.flags |= F_DISP8; - p += 1; - }, - 2 => { - result.flags |= F_DISP16; - p += 2; - }, - 4 => { - result.flags |= F_DISP32; - p += 4; - }, - else => {}, - } - } - - // ── immediates ── - if (cflags & C_IMM_P66 != 0) { - if (cflags & C_REL32 != 0) { - if (pref & PRE_66 != 0) { - result.flags |= F_IMM16 | F_RELATIVE; - p += 2; - // disasm_done — skip remaining immediate checks - result.len = @intCast(p); - if (result.len > 15) { - result.flags |= F_ERROR; - result.len = 15; - } - return result; - } - // fall through to rel32_ok below - } else { - if (pref & PRE_66 != 0) { - result.flags |= F_IMM16; - p += 2; - } else { - result.flags |= F_IMM32; - p += 4; - } - } - } - - if (cflags & C_IMM16 != 0) { - if (result.flags & F_IMM32 != 0) { - result.flags |= F_IMM16; - } else if (result.flags & F_IMM16 != 0) { - // F_2IMM16 - } else { - result.flags |= F_IMM16; - } - p += 2; - } - if (cflags & C_IMM8 != 0) { - result.flags |= F_IMM8; - p += 1; - } - - if (cflags & C_REL32 != 0) { - result.flags |= F_IMM32 | F_RELATIVE; - p += 4; - } else if (cflags & C_REL8 != 0) { - result.flags |= F_IMM8 | F_RELATIVE; - p += 1; - } - - result.len = @intCast(p); - if (result.len > 15) { - result.flags |= F_ERROR; - result.len = 15; - } - return result; -} - -// ── tests ────────────────────────────────────────────────────────────── - -test "push ebp" { - const d = decode(&[_]u8{ 0x55, 0xCC }); - try std.testing.expectEqual(@as(u8, 1), d.len); -} - -test "mov ebp, esp" { - // 8B EC (or 89 E5) - const d = decode(&[_]u8{ 0x8B, 0xEC }); - try std.testing.expectEqual(@as(u8, 2), d.len); - try std.testing.expect(d.flags & F_MODRM != 0); -} - -test "call rel32" { - const d = decode(&[_]u8{ 0xE8, 0x78, 0x56, 0x34, 0x12 }); - try std.testing.expectEqual(@as(u8, 5), d.len); - try std.testing.expect(d.flags & F_RELATIVE != 0); - try std.testing.expect(d.flags & F_IMM32 != 0); -} - -test "jmp rel32" { - const d = decode(&[_]u8{ 0xE9, 0x00, 0x00, 0x00, 0x00 }); - try std.testing.expectEqual(@as(u8, 5), d.len); - try std.testing.expect(d.flags & F_RELATIVE != 0); -} - -test "sub esp, imm8" { - // 83 EC 10 - const d = decode(&[_]u8{ 0x83, 0xEC, 0x10 }); - try std.testing.expectEqual(@as(u8, 3), d.len); - try std.testing.expect(d.flags & F_MODRM != 0); - try std.testing.expect(d.flags & F_IMM8 != 0); -} - -test "mov eax, [ebp+8]" { - // 8B 45 08 - const d = decode(&[_]u8{ 0x8B, 0x45, 0x08 }); - try std.testing.expectEqual(@as(u8, 3), d.len); - try std.testing.expect(d.flags & F_MODRM != 0); - try std.testing.expect(d.flags & F_DISP8 != 0); -} - -test "jz rel32 (0F 84)" { - const d = decode(&[_]u8{ 0x0F, 0x84, 0x10, 0x00, 0x00, 0x00 }); - try std.testing.expectEqual(@as(u8, 6), d.len); - try std.testing.expect(d.flags & F_RELATIVE != 0); -} - -test "nop" { - const d = decode(&[_]u8{0x90}); - try std.testing.expectEqual(@as(u8, 1), d.len); -} - -test "ret" { - const d = decode(&[_]u8{0xC3}); - try std.testing.expectEqual(@as(u8, 1), d.len); -} - -test "short jmp EB" { - const d = decode(&[_]u8{ 0xEB, 0x05 }); - try std.testing.expectEqual(@as(u8, 2), d.len); - try std.testing.expect(d.flags & F_RELATIVE != 0); - try std.testing.expect(d.flags & F_IMM8 != 0); -} - -test "short jcc 74 (jz rel8)" { - const d = decode(&[_]u8{ 0x74, 0x0A }); - try std.testing.expectEqual(@as(u8, 2), d.len); - try std.testing.expect(d.flags & F_RELATIVE != 0); -} - -test "mov eax, imm32" { - const d = decode(&[_]u8{ 0xB8, 0x44, 0x33, 0x22, 0x11 }); - try std.testing.expectEqual(@as(u8, 5), d.len); -} - -test "push imm32" { - const d = decode(&[_]u8{ 0x68, 0x44, 0x33, 0x22, 0x11 }); - try std.testing.expectEqual(@as(u8, 5), d.len); -} - -test "push imm8" { - // 6A 01 - const d = decode(&[_]u8{ 0x6A, 0x01 }); - try std.testing.expectEqual(@as(u8, 2), d.len); -} - -test "mov [ebp-4], eax" { - // 89 45 FC - const d = decode(&[_]u8{ 0x89, 0x45, 0xFC }); - try std.testing.expectEqual(@as(u8, 3), d.len); - try std.testing.expect(d.flags & F_MODRM != 0); - try std.testing.expect(d.flags & F_DISP8 != 0); -} - -test "lea eax, [ecx+edx*4+8]" { - // 8D 44 91 08 - const d = decode(&[_]u8{ 0x8D, 0x44, 0x91, 0x08 }); - try std.testing.expectEqual(@as(u8, 4), d.len); - try std.testing.expect(d.flags & F_MODRM != 0); - try std.testing.expect(d.flags & F_SIB != 0); - try std.testing.expect(d.flags & F_DISP8 != 0); -} - -test "mov [disp32], eax" { - // A3 xx xx xx xx - const d = decode(&[_]u8{ 0xA3, 0x00, 0x10, 0x40, 0x00 }); - try std.testing.expectEqual(@as(u8, 5), d.len); -} - -test "sub esp, imm32" { - // 81 EC 00 01 00 00 - const d = decode(&[_]u8{ 0x81, 0xEC, 0x00, 0x01, 0x00, 0x00 }); - try std.testing.expectEqual(@as(u8, 6), d.len); - try std.testing.expect(d.flags & F_MODRM != 0); -} - -test "test eax, imm32 (F7 C0)" { - // F7 C0 FF 00 00 00 = test eax, 0xFF - const d = decode(&[_]u8{ 0xF7, 0xC0, 0xFF, 0x00, 0x00, 0x00 }); - try std.testing.expectEqual(@as(u8, 6), d.len); -} - -test "ret imm16" { - // C2 04 00 - const d = decode(&[_]u8{ 0xC2, 0x04, 0x00 }); - try std.testing.expectEqual(@as(u8, 3), d.len); -} diff --git a/src/assetfix/assetfix.zig b/src/assetfix/assetfix.zig index 4520f9f..bef9541 100644 --- a/src/assetfix/assetfix.zig +++ b/src/assetfix/assetfix.zig @@ -11,7 +11,7 @@ // ============================================================================= const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const con = @import("../console.zig"); // ============================================================================= @@ -182,25 +182,18 @@ fn looseFilesLookup(game_path_ptr: u32) ?[*]const u8 { // Hook: CheckFileExistence (0x654DD0) // ============================================================================= // __fastcall(ECX=filename, EDX=flags, stack=outputBuffer) → EAX -// Prologue: 9 bytes (push ebp; mov ebp, esp; sub esp, 0x104) — no rel32 fixups -const CHECK_FILE_EXISTENCE: usize = 0x654DD0; +const fc: std.builtin.CallingConvention = .{ .x86_fastcall = .{} }; +const CheckFileExistenceFn = fn (u32, u32, u32) callconv(fc) u32; -var cfe_hook = hook.Hook{}; +var cfe_hook: hook.Detour(CheckFileExistenceFn) = .{}; -fn hookImpl(filename_ptr: u32, flags: u32, output_buffer_ptr: u32) callconv(.c) u32 { +fn checkFileExistenceDetour(filename_ptr: u32, flags: u32, output_buffer_ptr: u32) callconv(fc) u32 { if (filename_ptr != 0) { if (looseFilesLookup(filename_ptr)) |disk_path| { const raw: [*]const u8 = @ptrFromInt(filename_ptr); con.fmt("[assetfix] loose hit: \"{s}\"\n", .{raw[0..cStrLen(raw)]}); - // Write disk path (e.g. "Data\Character\...") to output buffer. - // The caller (File_FindInArchive) uses this to open the file from disk. - // Game paths contain '\' which makes CheckFileExistence's bit-0 handler - // skip BuildFilePath and use the raw path as-is — failing because the - // game-relative path has no "Data\" prefix. By writing the correct - // disk-relative path to the output buffer ourselves, we bypass that - // bug without transforming the filename argument (preserving chaining). if (output_buffer_ptr != 0) { const disk_len = cStrLen(disk_path); const out: [*]u8 = @ptrFromInt(output_buffer_ptr); @@ -212,31 +205,11 @@ fn hookImpl(filename_ptr: u32, flags: u32, output_buffer_ptr: u32) callconv(.c) return 1; } } - return callOriginal(filename_ptr, flags, output_buffer_ptr); -} - -fn callOriginal(filename: u32, flags: u32, output_buffer: u32) u32 { - // __fastcall: ECX=filename, EDX=flags, push outputBuffer, callee cleans 4 - return asm volatile ( - \\push %[output] - \\call *%[func] - : [ret] "={eax}" (-> u32), - : [_] "{ecx}" (filename), - [_] "{edx}" (flags), - [output] "r" (output_buffer), - [func] "r" (cfe_hook.trampoline), - : .{ .memory = true, .cc = true }); + return cfe_hook.callOriginal(.{ filename_ptr, flags, output_buffer_ptr }); } fn installHook() bool { - if (!cfe_hook.prepare(CHECK_FILE_EXISTENCE, 9, &.{})) return false; - - // Build fastcall→cdecl thunk in the hook's alloc block (after trampoline) - const thunk_buf = cfe_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(thunk_buf, @intFromPtr(&hookImpl), 1); - - cfe_hook.activate(@intFromPtr(thunk_buf)); - return true; + return cfe_hook.attach(0x654DD0, &checkFileExistenceDetour) == .ok; } // ============================================================================= @@ -307,37 +280,36 @@ var installed: bool = false; var g_mutex: ?*anyopaque = null; var g_is_hook_owner: bool = false; -pub fn installHooks() bool { +pub fn installHooks() void { con.print("[assetfix] Module loaded\n"); // Multi-DLL safety: only one instance per process should hook var mutex_name_buf: [64]u8 = undefined; - const mutex_name = std.fmt.bufPrint(&mutex_name_buf, "Local\\AssetfixHook_{d}", .{GetCurrentProcessId()}) catch return false; + const mutex_name = std.fmt.bufPrint(&mutex_name_buf, "Local\\AssetfixHook_{d}", .{GetCurrentProcessId()}) catch return; mutex_name_buf[mutex_name.len] = 0; g_mutex = CreateMutexA(null, 1, @ptrCast(mutex_name_buf[0..mutex_name.len :0])); - if (g_mutex == null) return false; + if (g_mutex == null) return; if (GetLastError() == ERROR_ALREADY_EXISTS) { _ = CloseHandle(g_mutex.?); g_mutex = null; g_is_hook_owner = false; con.print("[assetfix] Another DLL owns hooks (mutex taken), skipping\n"); - return true; + return; } g_is_hook_owner = true; applyGlobPatch(); applyLooseFilePatches(); looseFilesInit(); - if (!installHook()) return false; + if (!installHook()) return; installed = true; - return true; } pub fn removeHooks() void { if (g_is_hook_owner and installed) { - cfe_hook.remove(); + cfe_hook.detach(); revertLooseFilePatches(); revertGlobPatch(); looseFilesCleanup(); diff --git a/src/framecrash/RESEARCH.md b/src/framecrash/RESEARCH.md index 49895d0..be10e76 100644 --- a/src/framecrash/RESEARCH.md +++ b/src/framecrash/RESEARCH.md @@ -573,3 +573,266 @@ The existing GetRelativeTo vtable hook can be kept as defense-in-depth. 0x76772b: E8 F0 16 00 00 CALL SetAnimationOrigin ; 5 bytes ``` First 5 bytes (53 56 8B F1 57) can be replaced with JMP rel32 for a detour. + +--- + +## Stale UIParent Pointer — The Persistent Unknown Destruction Path + +### Discovery + +One persistent stale pointer escapes ALL hooked destruction paths. Pattern: +- Address always ends in `X008` (e.g., `0x17f20008`, `0x17fb4008`, `0x03bc0008`) +- CFrame base = addr - 0x24 = `XXXXffe4` — crosses page boundary +- First page (containing CFrame base, vtable) is DECOMMITTED +- Second page (containing CLayoutFrame inner at +0x24) survives +- Frame name at CFrame+0x98 reads garbage (`"t%Ç"`) from residual second-page data +- NOT in destruction history ring buffer — never went through any hooked detour + +### Diagnostic Hooks Added + +**PauseAnimationGroup (0x767ee0)** — dependency registration tracker: +- `__thiscall(ECX=relativeTo_frame, owner_frame, bitmask)`, RET 0x8 +- Prologue: `55 8B EC 53 8B D9` (6 bytes) +- Silently records every registration to a 2048-entry ring buffer +- Queried by vtable hooks when stale pointer detected + +**SetAnimationOrder (0x767c70)** — anchor creation validator: +- `__thiscall(ECX=frame, point_enum, relativeTo, relPoint, xOfs, yOfs, param_6)`, RET 0x18 +- Prologue: `55 8B EC 8B 45 0C` (6 bytes) +- Validates relativeTo AND CFrame base (relativeTo - 0x24) with IsBadReadPtr +- Catches race condition: relativeTo already dead when anchor created + +### Key Finding: The Stale Frame is UIParent + +**Confirmed by SetAnimationOrder RACE detection.** The following frames all call +SetAnimOrder with the dead relativeTo address: + +| Owner Frame | Point | Context | +|------------|-------|---------| +| `ScriptErrors` | 4 | Blizzard UI | +| `GroupLootDropDown` | 0 | Blizzard UI | +| `GroupLootFrame1` | 7 | Blizzard UI | +| `PlayerFrame` | 0 | Blizzard UI | +| `TargetFrame` | 0 | Blizzard UI | +| `WorldMapFrame` | 4 | Blizzard UI | +| `GuildBankFrame` | 0 | Blizzard UI | +| `TransmogFrame` | 0 | Addon UI | +| `NewTransmogAlertFrame` | 0 | Addon UI | +| `TWTMain` | 0 | Addon UI | +| `TWTMainSettings` | 0 | Addon UI | +| `TWTMainTankModeWindow` | 0 | Addon UI | +| `TWTWithAddonList` | 0 | Addon UI | + +**Every top-level frame** anchors to this address → it's `UIParent`. + +### Race Condition Confirmed + +- `DEP REGISTERED` — PauseAnimationGroup WAS called for the address (dependency existed) +- But name at registration time was `"t%Ç"` (garbage) — the frame was ALREADY DEAD + when PauseAnimationGroup ran +- `SetAnimOrder` RACE check: `IsBadReadPtr(relativeTo - 0x24)` FAILS (first page + decommitted), but `IsBadReadPtr(relativeTo)` passes (second page survives) +- The original game code doesn't validate relativeTo at all — just stores the raw pointer + +### Symptom: Black Screen + +When vtable hooks NULL all stale relativeTo pointers, every top-level frame loses its +anchor to UIParent → nothing can lay out → full black screen. The crash is prevented +but the UI is broken. + +### Theory + +UIParent is destroyed through an unknown path during a UI reload/transition (character +select → world, or loading screen). The destruction does NOT go through: +- `cleanup_linked_list_structures` (0x767720) — hooked, not triggered +- `destroyUIElement` (0x7645a0) — hooked, not triggered +- `ProcessUIUpdateEvent` (0x772ec0) — hooked, not triggered + +A new UIParent is created at a different address, but addon/Blizzard initialization +code passes the OLD (now dead) address to SetPoint/SetAnimOrder. + +### Next Steps + +1. Find UIParent global pointer in WoW binary (Ghidra) +2. Determine when/how UIParent is destroyed and recreated +3. Consider: hook SetAnimOrder to substitute live UIParent address when dead one detected +4. Alternative: find the destruction path that frees UIParent without our hooks firing + +--- + +## Ghidra RE: UIParent Resolution Mechanism + +### UIParent String + +- Address: `0x00842f14` (DATA, type=string, value="UIParent") +- **Only 1 xref**: from `InitializeGameInterface` at `0x00490065` + +### GetFrameFromLua (0x76c760) + +Resolves a named frame from the Lua global table at runtime. Called from +`InitializeGameInterface` to populate `PTR_00b4b44c`. + +**Calling convention**: `__fastcall(ECX=name_string, EDX=typeID)`, returns `CFrame*`. +Bare `RET` (no stack cleanup -- 0 stack args). + +**Prologue**: `53 56 57 8B DA 8B F9` (7 bytes) + +**Internal call chain**: +``` +0x76c767: CALL 0x7040d0 -- lua.getContext() -> ESI = L +0x76c772: CALL 0x6f3890 -- lua_pushstring(L, name) +0x76c77e: CALL 0x6f3a40 -- lua_gettable(L, LUA_GLOBALSINDEX) +0x76c788: CALL 0x6f3400 -- lua_type(L, -1) + CMP EAX, ... -- type check (userdata? table?) + JZ ... -- branch on type +0x76c794: MOV EDX, ... -- extract frame pointer from Lua value +``` + +Epilogue: two RET paths at `0x76c7a3` and `0x76c7db` (both bare `RET`). + +**Usage in InitializeGameInterface (0x48fbf0)**: +```c +g_ParentFrameTypeID = g_NextTypeID + 1; // if not already set +PTR_00b4b44c = GetFrameFromLua("UIParent", g_ParentFrameTypeID); +PTR_00b4b3c4 = GetFrameFromLua("GameTooltip", another_type_id); +``` + +**Key insight**: This function does a live Lua global lookup every time it's called. +We can call it from our hooks to get the CURRENT UIParent, not a cached stale pointer. +Just need `g_ParentFrameTypeID` from `0x00cf0c10` (runtime .bss value). + +### g_ParentFrameTypeID (0xcf0c10) + +Runtime type ID for the parent frame type. Set once during initialization, stable +for the lifetime of the process. Read from `.bss` at runtime. + +### Other Relevant Lookup Functions (found but not yet decompiled) + +| Address | Name | Notes | +|---------|------|-------| +| 0x4b3250 | `FindUIElementByName` | Alternative name-based lookup | +| 0x4c3c50 | `FrameScript_GetListOffset` | Frame list traversal helper | +| 0x4c3c80 | `FrameScript_GetListNodeAt` | Frame list node access | + +### Lua API Functions (confirmed addresses) + +| Address | Function | Ghidra Name | Notes | +|---------|----------|-------------|-------| +| 0x6f36e0 | `lua_tolstring` | lua_tolstring | Confirmed | +| 0x6f3740 | `lua_touserdata` | lua_objlen (WRONG) | See below | +| 0x6f3770 | `lua_objlen` | lua_get_userdata_size | Actual objlen | + +### lua_touserdata (0x6f3740) -- CONFIRMED + +Ghidra mislabels this as `lua_objlen`. Disassembly confirms it's `lua_touserdata`: +```asm +0x6f374a: MOV ECX, [EAX] ; type tag +0x6f374c: SUB ECX, 0x2 ; LUA_TLIGHTUSERDATA = 2 +0x6f374f: JZ 0x6f3760 ; -> return value[2] directly +0x6f3751: SUB ECX, 0x5 ; LUA_TUSERDATA = 7 (2+5) +0x6f3754: JZ 0x6f3759 ; -> return value[2] + 0x10 (skip Udata header) +0x6f3756: XOR EAX, EAX ; else return NULL +0x6f3758: RET +0x6f3759: MOV EAX, [EAX+8] ; full userdata data ptr +0x6f375c: ADD EAX, 0x10 ; skip 16-byte Udata header +0x6f375f: RET +0x6f3760: MOV EAX, [EAX+8] ; lightuserdata ptr +0x6f3763: RET +``` + +### GetFrameFromLua Full Flow (CONFIRMED) + +From decompilation and byte-level verification: + +```c +CFrame* __fastcall GetFrameFromLua(ECX=name, EDX=typeID) { + L = getContext(); // 0x7040d0 + lua_pushstring(L, name); // push "UIParent" + lua_gettable(L, LUA_GLOBALSINDEX); // EDX=0xffffd8ef (-10001) + type = lua_type(L, -1); // 0x6f3400 + if (type != 5) { // 5 = LUA_TTABLE + lua_settop(L, -2); // pop, not a table + return NULL; + } + lua_rawgeti(L, -1, 0); // push table[0] (CFrame* as userdata) + ptr = lua_touserdata(L, -1); // extract C pointer (0x6f3740) + lua_settop(L, -3); // pop table + userdata + if (ptr == NULL) return NULL; + if (!ptr->vtable[4](typeID)) return NULL; // validate frame type + return ptr; // CFrame base pointer +} +``` + +Frame Lua representation: named frame globals are Lua **tables** with the CFrame +pointer stored as userdata at `table[0]`. `GetFrameFromLua` extracts this, validates +the type via vtable dispatch, and returns the raw CFrame pointer. + +### Heal Implementation -- STATUS: CRASHES + +Replaced the stale C++ global approach (`0x00B4B44C`) with a live Lua lookup via +`hook.fastcall(u32, 0x76c760, name_ptr, type_id)`. The heal log message ("HEALED") +appears in the console, confirming `GetFrameFromLua` returns a valid pointer. But +the game segfaults shortly after with NO WoW crash log (bypasses the exception handler). + +**Current code** (in framecrash.zig): +```zig +fn getLiveUIParent() u32 { + const type_id = readAligned(G_PARENT_FRAME_TYPE_ID); // 0xcf0c10 + if (type_id == 0) return 0; + const cframe_base = hook.fastcall(u32, GET_FRAME_FROM_LUA, + @intFromPtr(@as([*:0]const u8, "UIParent")), type_id); + if (cframe_base == 0) return 0; + // validate + name check, return CLayoutFrame inner (+ 0x24) +} +``` + +Called from: +- `setAnimOrderDetour` -- substitutes dead relativeTo BEFORE anchor creation +- `tryFixStaleRelativeTo` -- heals existing anchors in vtable hooks +- Vtable hooks (GetWidth/GetHeight/GetRelativeTo) -- defense-in-depth + +**Suspected crash causes** (not yet verified): + +1. **`hook.fastcall` clobber list incomplete** -- the inline asm for fastcall does + NOT declare ECX/EDX as clobbered after `call`. This could cause the Zig compiler + to assume those registers are preserved, leading to register corruption in the + calling detour function. Fix: rewrite getLiveUIParent to call Lua API functions + directly via typed function pointers (same pattern as main.zig's lua struct), + avoiding hook.fastcall entirely. + +2. **Reentrancy** -- vtable hooks (GetWidth/GetHeight) fire during layout calculation, + which can happen many times per frame. Each call to getLiveUIParent does a full + Lua stack push/pop cycle. If layout triggers a metamethod or callback that + reenters layout, the Lua stack could be corrupted. + +3. **Dependency list inconsistency** -- vtable hook HEAL path writes the new UIParent + address directly into `anchor+0x0C`, but the PauseAnimationGroup dependency was + registered on the OLD (dead) address. The dependency list on the new UIParent + doesn't know about these anchors. This could cause issues when the new UIParent + is later destroyed. + +### Next Steps for Heal Fix + +1. **Rewrite getLiveUIParent with direct Lua calls** -- use typed function pointers + for each Lua API function instead of hook.fastcall into GetFrameFromLua. This + eliminates the clobber list risk and allows per-step error checking. Key addresses: + - `getContext` (0x7040d0): `fn() callconv(fc) u32` + - `lua_pushstring` (0x6f3890): `fn(u32, [*:0]const u8) callconv(fc) void` + - `lua_gettable` (0x6f3a40): `fn(u32, i32) callconv(fc) void` + - `lua_type` (0x6f3400): `fn(u32, i32) callconv(fc) i32` + - `lua_settop` (0x6f3080): `fn(u32, i32) callconv(fc) void` + - `lua_rawgeti` (0x6f3bc0): `fn(u32, i32, i32) callconv(fc) void` + - `lua_touserdata` (0x6f3740): `fn(u32, i32) callconv(fc) u32` + +2. **Add reentrancy guard** -- static bool to prevent recursive getLiveUIParent calls. + +3. **Cache result** -- call GetFrameFromLua once per stale-pointer batch, reuse for + all heals in the same layout pass. Invalidate on next frame/event. + +4. **Fix dependency list** -- after healing an anchor's relativeTo, call + PauseAnimationGroup on the new UIParent to register the dependency. Without + this, the new UIParent's destruction won't clean up these anchors. + +### Module currently DISABLED by default + +Build flag changed to `orelse false` in build.zig. Enable with `-Dframecrash=true`. diff --git a/src/framecrash/framecrash.zig b/src/framecrash/framecrash.zig index 8dfa3dd..08bd57a 100644 --- a/src/framecrash/framecrash.zig +++ b/src/framecrash/framecrash.zig @@ -20,11 +20,12 @@ //! See RESEARCH.md for full reverse engineering notes. const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const con = @import("../console.zig"); const WINAPI = std.builtin.CallingConvention.winapi; -const THISCALL = std.builtin.CallingConvention{ .x86_thiscall = .{} }; +const tc: std.builtin.CallingConvention = .{ .x86_thiscall = .{} }; +const fc: std.builtin.CallingConvention = .{ .x86_fastcall = .{} }; extern "kernel32" fn IsBadReadPtr(lp: ?*const anyopaque, ucb: usize) callconv(WINAPI) i32; extern "kernel32" fn CreateMutexA(lpMutexAttributes: ?*anyopaque, bInitialOwner: i32, lpName: [*:0]const u8) callconv(WINAPI) ?*anyopaque; @@ -56,6 +57,14 @@ var g_is_hook_owner: bool = false; const ANCHOR_VTABLE_ADDR: usize = 0x0081c44c; const GET_RELATIVE_TO_SLOT: usize = ANCHOR_VTABLE_ADDR + 0x0C; // vtable[3] +// GetFrameFromLua (0x76c760): __fastcall(ECX=name_ptr, EDX=typeID) -> CFrame* or NULL. +// Does a live Lua global lookup: pushstring(name) -> gettable(GLOBALS) -> rawgeti(0) +// -> touserdata -> validate type -> return. Stack-neutral (pops what it pushes). +const GET_FRAME_FROM_LUA: usize = 0x76c760; + +// g_ParentFrameTypeID: runtime type ID for parent frame type, set once during init. +const G_PARENT_FRAME_TYPE_ID: usize = 0x00cf0c10; + // ============================================================================= // Root cause fix: hook frame destruction to clean up reverse anchor references // @@ -76,9 +85,9 @@ const GET_RELATIVE_TO_SLOT: usize = ANCHOR_VTABLE_ADDR + 0x0C; // vtable[3] // ============================================================================= const CLEANUP_TARGET: usize = 0x767720; -const CLEANUP_PROLOGUE_SIZE: usize = 5; -var cleanup_hook: hook.Hook = .{}; +const CleanupFn = fn (u32) callconv(tc) void; +var cleanup_hook: hook.Detour(CleanupFn) = .{}; // ============================================================================= // Second destruction path: destroyUIElement (0x7645a0) @@ -93,9 +102,9 @@ var cleanup_hook: hook.Hook = .{}; // ============================================================================= const DESTROY_UI_TARGET: usize = 0x7645a0; -const DESTROY_UI_PROLOGUE_SIZE: usize = 6; -var destroy_ui_hook: hook.Hook = .{}; +const DestroyUIFn = fn (u32, u32) callconv(tc) u32; +var destroy_ui_hook: hook.Detour(DestroyUIFn) = .{}; // ============================================================================= // Third destruction path: ProcessUIUpdateEvent (0x772ec0) @@ -107,9 +116,9 @@ var destroy_ui_hook: hook.Hook = .{}; // ============================================================================= const PROCESS_UI_TARGET: usize = 0x772ec0; -const PROCESS_UI_PROLOGUE_SIZE: usize = 6; -var process_ui_hook: hook.Hook = .{}; +const ProcessUIFn = fn (u32, u32) callconv(tc) u32; +var process_ui_hook: hook.Detour(ProcessUIFn) = .{}; // ============================================================================= // Priority 1: Hook PauseAnimationGroup (0x767ee0) — dependency registration @@ -125,9 +134,9 @@ var process_ui_hook: hook.Hook = .{}; // ============================================================================= const PAUSE_ANIM_TARGET: usize = 0x767ee0; -const PAUSE_ANIM_PROLOGUE_SIZE: usize = 6; -var pause_anim_hook: hook.Hook = .{}; +const PauseAnimFn = fn (u32, u32, u32) callconv(tc) void; +var pause_anim_hook: hook.Detour(PauseAnimFn) = .{}; // ============================================================================= // Priority 2: Hook SetAnimationOrder (0x767c70) — anchor creation validation @@ -144,9 +153,9 @@ var pause_anim_hook: hook.Hook = .{}; // ============================================================================= const SET_ANIM_TARGET: usize = 0x767c70; -const SET_ANIM_PROLOGUE_SIZE: usize = 6; -var set_anim_hook: hook.Hook = .{}; +const SetAnimFn = fn (u32, u32, u32, u32, u32, u32, u32) callconv(tc) void; +var set_anim_hook: hook.Detour(SetAnimFn) = .{}; // ============================================================================= // Dependency registration ring buffer — track PauseAnimationGroup calls @@ -162,26 +171,32 @@ const DepRegistration = struct { owner: u32 = 0, // the frame that owns the anchor bitmask: u32 = 0, // which anchor slots (OR of 1< 0) return span; + } + return "(unknown)"; +} + // ============================================================================= // Destruction history ring buffer — correlate stale pointers with frame names // ============================================================================= @@ -249,169 +273,85 @@ fn fmtStaleInfo(relativeTo: u32) struct { name: []const u8, saw_destroy: bool } return .{ .name = fmtFrameName(relativeTo), .saw_destroy = false }; } -/// Log registration status for a stale relativeTo address. -fn logRegistrationStatus(relativeTo: u32) void { - const reg_count = countRegistrations(relativeTo); - if (lookupRegistration(relativeTo)) |reg| { - con.fmt("[framecrash] DEP REGISTERED: PauseAnimGroup was called {d}x for 0x{x:0>8}, last owner=0x{x:0>8} mask=0x{x}\n", .{ - reg_count, relativeTo, reg.owner, reg.bitmask, - }); - } else { - con.fmt("[framecrash] DEP NEVER REGISTERED: PauseAnimGroup was NEVER called for 0x{x:0>8} (in {d}-entry buffer)\n", .{ - relativeTo, REG_HISTORY_SIZE, - }); - } -} - -/// Dump diagnostic info for a stale relativeTo pointer not seen in our detour. -fn dumpStaleContext(relativeTo: u32, anchor: u32) void { - // Anchor relPoint enum at +0x10 - if (IsBadReadPtr(@ptrFromInt(anchor + 0x10), 4) != 0) return; - const rel_point = readAligned(anchor + 0x10); - - // Derive owner frame: anchor lives at owner_frame + relPoint*4 + 4 - const owner_layout = anchor -% (rel_point * 4 + 4); - const owner_name = fmtFrameName(owner_layout); - con.fmt("[framecrash] owner=\"{s}\" (0x{x:0>8}), relPoint={d}, stale=0x{x:0>8}\n", .{ - owner_name, owner_layout, rel_point, relativeTo, - }); -} +// logRegistrationStatus and dumpStaleContext removed — verbose diagnostic logging +// superseded by HEAL/FIX/RACE messages. Ring buffers still used by tryFixStaleRelativeTo. /// Detour for cleanup_linked_list_structures. Runs before the original to /// walk the dying frame's dependency list and destroy referencing anchors. -fn cleanupDetour(frame: u32) callconv(THISCALL) void { - // Record this frame in the destruction history before anything changes +fn cleanupDetour(frame: u32) callconv(tc) void { recordDestruction(frame); - - // Count reverse dependencies for logging - const dep_count = countReverseDependencies(frame); - if (dep_count > 0) { - con.fmt("[framecrash] Destroying frame \"{s}\" (0x{x:0>8}), {d} reverse dependencies\n", .{ - fmtFrameName(frame), - frame, - dep_count, - }); - } - cleanupReverseDependencies(frame); - - // Call original cleanup_linked_list_structures via trampoline - const orig = cleanup_hook.getTrampoline(*const fn (u32) callconv(THISCALL) void); - orig(frame); + cleanup_hook.callOriginal(.{frame}); } /// Detour for destroyUIElement. This is the second frame destruction path, /// called from cleanupGraphicsResources during UI teardown/reload. The original /// frees frames without walking the dependency list, leaving stale anchors. /// Signature: void* __thiscall destroyUIElement(void* this, byte free_flag) -fn destroyUIDetour(frame: u32, free_flag: u32) callconv(THISCALL) u32 { - // Record both possible interpretations: frame as CLayoutFrame inner, - // and frame+0x24 in case frame is actually a CFrame base. - // Anchors store CLayoutFrame inner ptrs as relativeTo. +fn destroyUIDetour(frame: u32, free_flag: u32) callconv(tc) u32 { recordDestruction(frame); if (IsBadReadPtr(@ptrFromInt(frame + 0x24), 4) == 0) { recordDestruction(frame + 0x24); } - - // Try cleaning deps at both offsets. cleanupReverseDependencies is - // guarded by IsBadReadPtr so the wrong offset safely no-ops. - const dep_count_a = countReverseDependencies(frame); - const dep_count_b = countReverseDependencies(frame + 0x24); - - if (dep_count_a > 0) { - con.fmt("[framecrash] destroyUIElement frame \"{s}\" (0x{x:0>8}), {d} reverse deps (layout)\n", .{ - fmtFrameName(frame), frame, dep_count_a, - }); - cleanupReverseDependencies(frame); - } - if (dep_count_b > 0) { - con.fmt("[framecrash] destroyUIElement frame \"{s}\" (0x{x:0>8}), {d} reverse deps (inner+0x24)\n", .{ - fmtFrameName(frame + 0x24), frame + 0x24, dep_count_b, - }); - cleanupReverseDependencies(frame + 0x24); - } - - // Call original destroyUIElement via trampoline - const orig: *const fn (u32, u32) callconv(THISCALL) u32 = @ptrFromInt(destroy_ui_hook.trampoline); - return orig(frame, free_flag); + cleanupReverseDependencies(frame); + cleanupReverseDependencies(frame + 0x24); + return destroy_ui_hook.callOriginal(.{ frame, free_flag }); } /// Detour for ProcessUIUpdateEvent — third destruction path, called via vtable. -fn processUIDetour(frame: u32, free_flag: u32) callconv(THISCALL) u32 { +fn processUIDetour(frame: u32, free_flag: u32) callconv(tc) u32 { recordDestruction(frame); if (IsBadReadPtr(@ptrFromInt(frame + 0x24), 4) == 0) { recordDestruction(frame + 0x24); } - - const dep_count_a = countReverseDependencies(frame); - const dep_count_b = countReverseDependencies(frame + 0x24); - - if (dep_count_a > 0) { - con.fmt("[framecrash] processUI frame \"{s}\" (0x{x:0>8}), {d} reverse deps (layout)\n", .{ - fmtFrameName(frame), frame, dep_count_a, - }); - cleanupReverseDependencies(frame); - } - if (dep_count_b > 0) { - con.fmt("[framecrash] processUI frame \"{s}\" (0x{x:0>8}), {d} reverse deps (inner+0x24)\n", .{ - fmtFrameName(frame + 0x24), frame + 0x24, dep_count_b, - }); - cleanupReverseDependencies(frame + 0x24); - } - - const orig: *const fn (u32, u32) callconv(THISCALL) u32 = @ptrFromInt(process_ui_hook.trampoline); - return orig(frame, free_flag); + cleanupReverseDependencies(frame); + cleanupReverseDependencies(frame + 0x24); + return process_ui_hook.callOriginal(.{ frame, free_flag }); } /// Detour for PauseAnimationGroup — records every dependency registration. /// This tells us whether a stale relativeTo was ever registered through the /// normal dependency tracking system. /// Signature: void __thiscall PauseAnimationGroup(ECX=relativeTo_frame, owner_frame, bitmask) -fn pauseAnimDetour(relativeTo_frame: u32, owner_frame: u32, bitmask: u32) callconv(THISCALL) void { - // Record this registration +fn pauseAnimDetour(relativeTo_frame: u32, owner_frame: u32, bitmask: u32) callconv(tc) void { + // Silently record — queried later by vtable hooks via logRegistrationStatus() recordRegistration(relativeTo_frame, owner_frame, bitmask); - con.fmt("[framecrash] PauseAnimGroup: relativeTo=0x{x:0>8} \"{s}\", owner=0x{x:0>8} \"{s}\", mask=0x{x}\n", .{ - relativeTo_frame, - fmtFrameName(relativeTo_frame), - owner_frame, - fmtFrameName(owner_frame), - bitmask, - }); - - // Call original - const orig = pause_anim_hook.getTrampoline(*const fn (u32, u32, u32) callconv(THISCALL) void); - orig(relativeTo_frame, owner_frame, bitmask); + pause_anim_hook.callOriginal(.{ relativeTo_frame, owner_frame, bitmask }); } /// Detour for SetAnimationOrder — validates relativeTo param before anchor creation. -/// Uses cdecl thunk bridge because the function has float params. -/// cdecl args: (ecx=frame, edx=unused, point_enum, relativeTo, relPoint, xOfs_bits, yOfs_bits, param_6) -fn setAnimOrderDetour(frame: u32, _edx: u32, point_enum: u32, relativeTo: u32, rel_point: u32, x_ofs: u32, y_ofs: u32, param_6: u32) callconv(.c) void { - _ = _edx; +/// Float params (xOfs, yOfs) are passed as raw u32 bit patterns on the stack. +fn setAnimOrderDetour(frame: u32, point_enum: u32, relativeTo: u32, rel_point: u32, x_ofs: u32, y_ofs: u32, param_6: u32) callconv(tc) void { + var fixed_relativeTo = relativeTo; - // Validate relativeTo BEFORE the original creates the anchor - if (relativeTo != 0) { - if (IsBadReadPtr(@ptrFromInt(relativeTo), 0x10) != 0) { - con.fmt("[framecrash] RACE: SetAnimOrder creating anchor with INVALID relativeTo=0x{x:0>8}! frame=0x{x:0>8} \"{s}\", point={d}\n", .{ - relativeTo, - frame, - fmtFrameName(frame), - point_enum, - }); - } else if (relativeTo == frame) { - // Self-reference — the original function rejects this, but log it - con.fmt("[framecrash] SetAnimOrder: self-reference rejected, frame=0x{x:0>8}\n", .{frame}); + // Validate relativeTo BEFORE the original creates the anchor. + // Check BOTH the CLayoutFrame inner (relativeTo) AND the CFrame base (relativeTo - 0x24). + // The stale pointer pattern: CFrame base crosses a page boundary, first page is + // decommitted but second page (containing the CLayoutFrame inner) survives. + if (relativeTo != 0 and relativeTo != frame) { + const frame_base = relativeTo -% 0x24; + const inner_bad = IsBadReadPtr(@ptrFromInt(relativeTo), 0x10) != 0; + const base_bad = relativeTo >= 0x24 and IsBadReadPtr(@ptrFromInt(frame_base), 0x10) != 0; + + if (inner_bad or base_bad) { + // Dead relativeTo detected. Do a live Lua lookup for UIParent. + const live_uiparent = getLiveUIParent(); + + if (live_uiparent != 0 and live_uiparent != relativeTo) { + con.fmt("[framecrash] FIX: SetAnimOrder dead relativeTo=0x{x:0>8} -> UIParent=0x{x:0>8}, owner=\"{s}\" point={d}\n", .{ + relativeTo, live_uiparent, fmtFrameName(frame), point_enum, + }); + fixed_relativeTo = live_uiparent; + } else { + con.fmt("[framecrash] RACE: SetAnimOrder dead relativeTo=0x{x:0>8}, no live UIParent! owner=\"{s}\" point={d}\n", .{ + relativeTo, fmtFrameName(frame), point_enum, + }); + } } } - // Call original trampoline as __thiscall(ECX=frame, 6 stack args). - // All args are u32 — float params (xOfs, yOfs) are passed as raw bit patterns - // which the original function reads from the stack as floats. The bit layout - // is identical because __thiscall pushes all non-this args onto the stack. - const orig: *const fn (u32, u32, u32, u32, u32, u32, u32) callconv(THISCALL) void = - @ptrFromInt(set_anim_hook.trampoline); - orig(frame, point_enum, relativeTo, rel_point, x_ofs, y_ofs, param_6); + set_anim_hook.callOriginal(.{ frame, point_enum, fixed_relativeTo, rel_point, x_ofs, y_ofs, param_6 }); } /// Count how many nodes are in the PauseAnimationGroup dependency list. @@ -525,6 +465,45 @@ fn fmtFrameName(layout_frame: u32) []const u8 { return "(unnamed)"; } +/// Get the live UIParent CLayoutFrame inner pointer via Lua global lookup. +/// Calls GetFrameFromLua("UIParent", g_ParentFrameTypeID) which does a live +/// Lua table lookup, bypassing the stale C++ global at 0x00B4B44C. +/// Returns CLayoutFrame inner (CFrame base + 0x24), or 0 if unavailable. +fn getLiveUIParent() u32 { + const type_id = readAligned(G_PARENT_FRAME_TYPE_ID); + if (type_id == 0) return 0; + + const cframe_base = hook.fastcall(u32, GET_FRAME_FROM_LUA, @intFromPtr(@as([*:0]const u8, "UIParent")), type_id); + if (cframe_base == 0) return 0; + + // Validate the returned pointer + if (IsBadReadPtr(@ptrFromInt(cframe_base), 0xA0) != 0) return 0; + const inner = cframe_base + 0x24; + if (IsBadReadPtr(@ptrFromInt(inner), 0x40) != 0) return 0; + + // Verify the name is actually "UIParent" (paranoia check) + if (getFrameName(inner)) |name| { + if (!std.mem.eql(u8, std.mem.span(name), "UIParent")) return 0; + } else return 0; + + return inner; +} + +/// Attempt to fix a stale relativeTo in an anchor by substituting live UIParent. +/// Uses GetFrameFromLua for a live Lua lookup, not the stale C++ global. +/// Returns true if the fix was applied, false if no valid substitute found. +fn tryFixStaleRelativeTo(anchor: u32, stale: u32) bool { + const live = getLiveUIParent(); + if (live == 0 or live == stale) return false; + + const field: *align(1) u32 = @ptrFromInt(anchor + 0x0C); + field.* = live; + con.fmt("[framecrash] HEALED: anchor 0x{x:0>8} relativeTo 0x{x:0>8} -> UIParent 0x{x:0>8}\n", .{ + anchor, stale, live, + }); + return true; +} + // ============================================================================= // Defense-in-depth: anchor vtable hooks // @@ -562,25 +541,19 @@ fn isRelativeToValid(relativeTo: u32) bool { /// Hook for vtable[3] GetRelativeTo. Validates the stored pointer. /// If stale, NULLs anchor+0x0C and returns 0 (safe "no relativeTo" path). -fn getRelativeToHook(this: u32) callconv(THISCALL) u32 { - const orig: *const fn (u32) callconv(THISCALL) u32 = @ptrFromInt(orig_get_relative_to); +fn getRelativeToHook(this: u32) callconv(tc) u32 { + const orig: *const fn (u32) callconv(tc) u32 = @ptrFromInt(orig_get_relative_to); const result = orig(this); if (result == 0) return 0; if (!isRelativeToValid(result)) { - const info = fmtStaleInfo(result); - if (info.saw_destroy) { - con.fmt("[framecrash] STALE: frame \"{s}\" (0x{x:0>8}) went through detour but dep list missed anchor 0x{x:0>8}, detected in GetRelativeTo\n", .{ - info.name, result, this, - }); - } else { - con.fmt("[framecrash] STALE: frame 0x{x:0>8} NOT seen in detour, anchor 0x{x:0>8}, detected in GetRelativeTo\n", .{ - result, this, - }); - dumpStaleContext(result, this); + // Try to substitute live UIParent before NULLing + if (tryFixStaleRelativeTo(this, result)) { + // Re-call original — it now reads the fixed pointer + return orig(this); } - logRegistrationStatus(result); + // No substitute available — NULL it out const field: *align(1) u32 = @ptrFromInt(this + 0x0C); field.* = 0; return 0; @@ -592,58 +565,44 @@ fn getRelativeToHook(this: u32) callconv(THISCALL) u32 { /// Hook for vtable[1] GetWidth. Checks anchor+0x0C before calling original. /// Returns sentinel if relativeTo is NULL or dangling. /// Signature: f32 __thiscall GetWidth(this, u32 param) — callee cleans 1 stack arg. -fn getWidthHook(this: u32, param: u32) callconv(THISCALL) f32 { +fn getWidthHook(this: u32, param: u32) callconv(tc) f32 { const relativeTo: u32 = readAligned(this + 0x0C); if (!isRelativeToValid(relativeTo)) { - // Self-heal if dangling (not just NULL) if (relativeTo != 0) { - const info = fmtStaleInfo(relativeTo); - if (info.saw_destroy) { - con.fmt("[framecrash] STALE: frame \"{s}\" (0x{x:0>8}) went through detour but dep list missed anchor 0x{x:0>8}, detected in GetWidth\n", .{ - info.name, relativeTo, this, - }); - } else { - con.fmt("[framecrash] STALE: frame 0x{x:0>8} NOT seen in detour, anchor 0x{x:0>8}, detected in GetWidth\n", .{ - relativeTo, this, - }); - dumpStaleContext(relativeTo, this); + // Try to substitute live UIParent instead of NULLing + if (tryFixStaleRelativeTo(this, relativeTo)) { + // Fixed — call original with the healed pointer + const orig: *const fn (u32, u32) callconv(tc) f32 = @ptrFromInt(orig_get_width); + return orig(this, param); } - logRegistrationStatus(relativeTo); const field: *align(1) u32 = @ptrFromInt(this + 0x0C); field.* = 0; } return @as(*align(1) const f32, @ptrFromInt(SENTINEL_ADDR)).*; } - const orig: *const fn (u32, u32) callconv(THISCALL) f32 = @ptrFromInt(orig_get_width); + const orig: *const fn (u32, u32) callconv(tc) f32 = @ptrFromInt(orig_get_width); return orig(this, param); } /// Hook for vtable[2] GetHeight. Same pattern as GetWidth. /// Signature: f32 __thiscall GetHeight(this, u32 param) — callee cleans 1 stack arg. -fn getHeightHook(this: u32, param: u32) callconv(THISCALL) f32 { +fn getHeightHook(this: u32, param: u32) callconv(tc) f32 { const relativeTo: u32 = readAligned(this + 0x0C); if (!isRelativeToValid(relativeTo)) { if (relativeTo != 0) { - const info = fmtStaleInfo(relativeTo); - if (info.saw_destroy) { - con.fmt("[framecrash] STALE: frame \"{s}\" (0x{x:0>8}) went through detour but dep list missed anchor 0x{x:0>8}, detected in GetHeight\n", .{ - info.name, relativeTo, this, - }); - } else { - con.fmt("[framecrash] STALE: frame 0x{x:0>8} NOT seen in detour, anchor 0x{x:0>8}, detected in GetHeight\n", .{ - relativeTo, this, - }); - dumpStaleContext(relativeTo, this); + // Try to substitute live UIParent instead of NULLing + if (tryFixStaleRelativeTo(this, relativeTo)) { + const orig: *const fn (u32, u32) callconv(tc) f32 = @ptrFromInt(orig_get_height); + return orig(this, param); } - logRegistrationStatus(relativeTo); const field: *align(1) u32 = @ptrFromInt(this + 0x0C); field.* = 0; } return @as(*align(1) const f32, @ptrFromInt(SENTINEL_ADDR)).*; } - const orig: *const fn (u32, u32) callconv(THISCALL) f32 = @ptrFromInt(orig_get_height); + const orig: *const fn (u32, u32) callconv(tc) f32 = @ptrFromInt(orig_get_height); return orig(this, param); } @@ -685,8 +644,7 @@ pub fn installHooks() void { // Root cause fix #1: detour cleanup_linked_list_structures to clean up // reverse anchor references before the frame is destroyed. - // Prologue: 53 56 8B F1 57 (5 bytes, no rel32 fixups needed) - if (!cleanup_hook.install(CLEANUP_TARGET, CLEANUP_PROLOGUE_SIZE, @intFromPtr(&cleanupDetour), &.{})) { + if (cleanup_hook.attach(CLEANUP_TARGET, &cleanupDetour) != .ok) { con.print("[framecrash] ERROR: Failed to install frame cleanup detour\n"); } else { con.print("[framecrash] Frame cleanup detour installed\n"); @@ -695,8 +653,7 @@ pub fn installHooks() void { // Root cause fix #2: detour destroyUIElement — the second destruction path // used by cleanupGraphicsResources during UI teardown/reload. This path // frees frames without walking the dependency list. - // Prologue: 55 8B EC 56 8B F1 (6 bytes, no rel32 fixups needed) - if (!destroy_ui_hook.install(DESTROY_UI_TARGET, DESTROY_UI_PROLOGUE_SIZE, @intFromPtr(&destroyUIDetour), &.{})) { + if (destroy_ui_hook.attach(DESTROY_UI_TARGET, &destroyUIDetour) != .ok) { con.print("[framecrash] ERROR: Failed to install destroyUIElement detour\n"); } else { con.print("[framecrash] destroyUIElement detour installed\n"); @@ -704,8 +661,7 @@ pub fn installHooks() void { // Root cause fix #3: detour ProcessUIUpdateEvent — virtual function that // calls CleanupUIElement + FreeMemory without layout cleanup. - // Prologue: 55 8B EC 56 8B F1 (6 bytes, no rel32 fixups needed) - if (!process_ui_hook.install(PROCESS_UI_TARGET, PROCESS_UI_PROLOGUE_SIZE, @intFromPtr(&processUIDetour), &.{})) { + if (process_ui_hook.attach(PROCESS_UI_TARGET, &processUIDetour) != .ok) { con.print("[framecrash] ERROR: Failed to install ProcessUIUpdateEvent detour\n"); } else { con.print("[framecrash] ProcessUIUpdateEvent detour installed\n"); @@ -720,8 +676,7 @@ pub fn installHooks() void { // Diagnostic: hook PauseAnimationGroup to track dependency registrations. // Answers: "was a dependency ever registered for this stale address?" - // Prologue: 55 8B EC 53 8B D9 (6 bytes, no rel32) - if (!pause_anim_hook.install(PAUSE_ANIM_TARGET, PAUSE_ANIM_PROLOGUE_SIZE, @intFromPtr(&pauseAnimDetour), &.{})) { + if (pause_anim_hook.attach(PAUSE_ANIM_TARGET, &pauseAnimDetour) != .ok) { con.print("[framecrash] ERROR: Failed to install PauseAnimationGroup detour\n"); } else { con.print("[framecrash] PauseAnimationGroup detour installed\n"); @@ -729,25 +684,20 @@ pub fn installHooks() void { // Diagnostic: hook SetAnimationOrder to detect race conditions. // Validates relativeTo param BEFORE anchor creation. - // Prologue: 55 8B EC 8B 45 0C (6 bytes, no rel32) - // Uses fastcall-to-cdecl thunk because of float stack params. - if (set_anim_hook.prepare(SET_ANIM_TARGET, SET_ANIM_PROLOGUE_SIZE, &.{})) { - const thunk = set_anim_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(thunk, @intFromPtr(&setAnimOrderDetour), 6); - set_anim_hook.activate(@intFromPtr(thunk)); - con.print("[framecrash] SetAnimationOrder detour installed\n"); - } else { + if (set_anim_hook.attach(SET_ANIM_TARGET, &setAnimOrderDetour) != .ok) { con.print("[framecrash] ERROR: Failed to install SetAnimationOrder detour\n"); + } else { + con.print("[framecrash] SetAnimationOrder detour installed\n"); } } pub fn removeHooks() void { if (g_is_hook_owner) { // Remove diagnostic hooks first (reverse install order) - set_anim_hook.remove(); + set_anim_hook.detach(); con.print("[framecrash] SetAnimationOrder detour removed\n"); - pause_anim_hook.remove(); + pause_anim_hook.detach(); con.print("[framecrash] PauseAnimationGroup detour removed\n"); // Restore original vtable pointers (reverse order) @@ -756,13 +706,13 @@ pub fn removeHooks() void { restoreVtableSlot(GET_WIDTH_SLOT, &orig_get_width); con.print("[framecrash] Anchor vtable hooks removed\n"); - process_ui_hook.remove(); + process_ui_hook.detach(); con.print("[framecrash] ProcessUIUpdateEvent detour removed\n"); - destroy_ui_hook.remove(); + destroy_ui_hook.detach(); con.print("[framecrash] destroyUIElement detour removed\n"); - cleanup_hook.remove(); + cleanup_hook.detach(); con.print("[framecrash] Frame cleanup detour removed\n"); } diff --git a/src/interact/interact.zig b/src/interact/interact.zig index a9073c2..2e301a2 100644 --- a/src/interact/interact.zig +++ b/src/interact/interact.zig @@ -1,5 +1,5 @@ const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const con = @import("../console.zig"); const WINAPI = std.builtin.CallingConvention.winapi; @@ -385,25 +385,16 @@ pub fn lootAllCorpses(_: *anyopaque) callconv(.c) u32 { // Per-frame hook for processing the loot queue. // ============================================================================= -var scene_end_hook = hook.Hook{}; - -fn hookSceneEnd(device: u32, _edx: u32) callconv(.c) void { - _ = _edx; +const tc: std.builtin.CallingConvention = .{ .x86_thiscall = .{} }; +const SceneEndFn = fn (u32) callconv(tc) void; +var scene_end_hook: hook.Detour(SceneEndFn) = .{}; +fn hookSceneEnd(device: u32) callconv(tc) void { if (loot_active) { processLootQueue(); } - callOriginalSceneEnd(device); -} - -fn callOriginalSceneEnd(device: u32) void { - asm volatile ( - \\call *%[func] - : - : [_] "{ecx}" (device), - [func] "r" (scene_end_hook.trampoline), - : .{ .eax = true, .edx = true, .memory = true, .cc = true }); + scene_end_hook.callOriginal(.{device}); } // ============================================================================= @@ -431,17 +422,12 @@ pub fn installHooks() void { g_is_hook_owner = true; // SceneEnd — per-frame loot queue processing - // Uses thunk: __fastcall(ECX=device, EDX) → cdecl(device, edx) - if (scene_end_hook.prepare(Offsets.ADDR_SceneEnd, 9, &.{})) { - const thunk = scene_end_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(thunk, @intFromPtr(&hookSceneEnd), 0); - scene_end_hook.activate(@intFromPtr(thunk)); - } + _ = scene_end_hook.attach(Offsets.ADDR_SceneEnd, &hookSceneEnd); } pub fn removeHooks() void { if (g_is_hook_owner) { - scene_end_hook.remove(); + scene_end_hook.detach(); } if (g_is_hook_owner) { diff --git a/src/main.zig b/src/main.zig index 5deb4cc..862b5b8 100644 --- a/src/main.zig +++ b/src/main.zig @@ -13,6 +13,7 @@ const build_opts = struct { const minimapicons = @import("build_options").enable_minimapicons; const transmogfix = @import("build_options").enable_transmogfix; const assetfix = @import("build_options").enable_assetfix; + const healtextfix = @import("build_options").enable_healtextfix; }; // Conditional module imports @@ -25,6 +26,7 @@ const combatlog = if (build_opts.combatlog) @import("combatlog/combatlog.zig") e const minimapicons = if (build_opts.minimapicons) @import("minimapicons/minimapicons.zig") else struct {}; const transmogfix = if (build_opts.transmogfix) @import("transmogfix/transmogfix.zig") else struct {}; const assetfix = if (build_opts.assetfix) @import("assetfix/assetfix.zig") else struct {}; +const healtextfix = if (build_opts.healtextfix) @import("healtextfix/healtextfix.zig") else struct {}; const WINAPI = std.builtin.CallingConvention.winapi; const fc: std.builtin.CallingConvention = .{ .x86_fastcall = .{} }; @@ -256,10 +258,11 @@ fn registerLuaFunctions() void { if (build_opts.outline) { registerFunction("OutlineCommand", @intFromPtr(&outline.outlineCommand)); } - if (build_opts.markers) { + if (build_opts.markers and markers.isActive()) { registerFunction("WorldMarker", @intFromPtr(&markers.luaWorldMarker)); registerFunction("ClearWorldMarker", @intFromPtr(&markers.luaClearWorldMarker)); registerFunction("GetPlayerPosition", @intFromPtr(&markers.luaGetPlayerPosition)); + registerFunction("ProcessMarkerAnimations", @intFromPtr(&markers.luaProcessAnimations)); } } @@ -852,7 +855,7 @@ fn loadAddonsDetour(error_handler: u32) callconv(fc) void { error_handler, ); } - if (build_opts.markers) { + if (build_opts.markers and markers.isActive()) { callLoadFileListWithIncludes( "Interface\\AddOns\\Markers\\Markers.toc", &md5ctx, @@ -911,6 +914,9 @@ fn engineInitDetour() callconv(sc) void { if (build_opts.outline) { _ = outline.init(); } + if (build_opts.healtextfix) { + healtextfix.lateInit(); + } } // ============================================================================= @@ -919,11 +925,45 @@ fn engineInitDetour() callconv(sc) void { var shutdown_hook: hook.Detour(fn () callconv(sc) void) = .{}; +// ============================================================================= +// Module lifecycle — single table drives install, shutdown, and uninstall. +// Adding a module here guarantees all three phases are handled. +// ============================================================================= + +const ModuleHooks = struct { + install: ?*const fn () void = null, + remove: ?*const fn () void = null, + /// If true, remove is also called during CGGameUI_Shutdown (before game + /// teardown), not just during DLL unload. Use for modules that create + /// world objects which must be destroyed while game systems are alive. + remove_on_shutdown: bool = false, +}; + +/// Order matters: modules are installed top-to-bottom, removed bottom-to-top. +/// Modules with remove_on_shutdown run their remove during shutdownDetour too. +const modules = [_]ModuleHooks{ + if (build_opts.assetfix) .{ .install = assetfix.installHooks, .remove = assetfix.removeHooks } else .{}, + if (build_opts.framecrash) .{ .install = framecrash.installHooks, .remove = framecrash.removeHooks } else .{}, + if (build_opts.combatlog) .{ .install = combatlog.installHooks, .remove = combatlog.removeHooks } else .{}, + if (build_opts.transmogfix) .{ .install = transmogfix.installHooks, .remove = transmogfix.removeHooks } else .{}, + if (build_opts.minimapicons) .{ .install = minimapicons.installHooks, .remove = minimapicons.removeHooks } else .{}, + if (build_opts.healtextfix) .{ .install = healtextfix.installHooks, .remove = healtextfix.removeHooks } else .{}, + if (build_opts.markers) .{ .install = markers.installHooks, .remove = markers.removeHooks, .remove_on_shutdown = true } else .{}, + if (build_opts.interact) .{ .install = interact.installHooks, .remove = interact.removeHooks } else .{}, + if (build_opts.outline) .{ .remove = outline.cleanup } else .{}, + if (build_opts.screenshot) .{ .remove = screenshot.removeHook } else .{}, +}; + fn shutdownDetour() callconv(sc) void { - // Clean up world objects BEFORE game shutdown — atexit handlers run before DllMain - // so we must destroy markers here, not in uninstall(). - if (build_opts.markers) { - markers.removeHooks(); + // Clean up world objects BEFORE game shutdown — atexit handlers run before + // DllMain so modules with remove_on_shutdown must destroy here. + comptime var i = modules.len; + inline while (i > 0) { + i -= 1; + const m = modules[i]; + if (m.remove_on_shutdown) { + if (m.remove) |rm| rm(); + } } shutdown_hook.callOriginal(.{}); @@ -941,28 +981,11 @@ fn install() void { _ = file_hook.attach(0x648620, &loadFileDetour); _ = lsf_hook.attach(0x490250, &loadScriptFunctionsDetour); - if (build_opts.assetfix) { - _ = assetfix.installHooks(); - } - if (build_opts.framecrash) { - framecrash.installHooks(); - } - if (build_opts.combatlog) { - combatlog.installHooks(); - } - if (build_opts.transmogfix) { - _ = transmogfix.installHooks(); - } - if (build_opts.minimapicons) { - minimapicons.installHooks(); + inline for (modules) |m| { + if (m.install) |inst| inst(); } _ = load_addons_hook.attach(0x51F600, &loadAddonsDetour); - - if (build_opts.interact) { - interact.installHooks(); - } - _ = engine_init_hook.attach(0x46a400, &engineInitDetour); _ = shutdown_hook.attach(0x490BD0, &shutdownDetour); } @@ -971,33 +994,11 @@ fn uninstall() void { shutdown_hook.detach(); engine_init_hook.detach(); - // Markers must be cleaned up first — destroys world objects while game systems are still alive - if (build_opts.markers) { - markers.removeHooks(); - } - if (build_opts.outline) { - outline.cleanup(); - } - if (build_opts.screenshot) { - screenshot.removeHook(); - } - if (build_opts.interact) { - interact.removeHooks(); - } - if (build_opts.framecrash) { - framecrash.removeHooks(); - } - if (build_opts.combatlog) { - combatlog.removeHooks(); - } - if (build_opts.minimapicons) { - minimapicons.removeHooks(); - } - if (build_opts.transmogfix) { - transmogfix.removeHooks(); - } - if (build_opts.assetfix) { - assetfix.removeHooks(); + // Remove in reverse order + comptime var i = modules.len; + inline while (i > 0) { + i -= 1; + if (modules[i].remove) |rm| rm(); } load_addons_hook.detach(); diff --git a/src/markers/RESEARCH.md b/src/markers/RESEARCH.md index b008722..6086df0 100644 --- a/src/markers/RESEARCH.md +++ b/src/markers/RESEARCH.md @@ -255,3 +255,1110 @@ AsyncTask_QueueForExecution(task); - Crash appears to be inside our cleanupFileHandleDetour during BLP context cleanup - BLP data IS served correctly (AsyncTaskWorkerThread → ReadFileFromMultipleSources → hook 3) - Investigation ongoing: possible stack corruption in callCleanupFileContext or freeGameBuffer + +## Cursor Terrain Position + +No single continuously-updated global stores the cursor terrain intersection point. +The raycast runs every frame in `WorldFrameUpdate` (0x481790) but the result is stored +on the stack (local `HitTestResult`) and discarded after dispatch. + +### WorldFrameUpdate Flow (every frame) +``` +WorldFrameUpdate(this, deltaTime): + inputHandler = *(this + 0xA0) + mouseNDC_X = *(inputHandler + 0x1118) + mouseNDC_Y = *(inputHandler + 0x111C) + NDCToDDC(&screenX, &screenY, mouseNDC_X, mouseNDC_Y) // 0x4242F0 + hitType = HitTestPoint(this, screenX, screenY, &localResult) // 0x481190 + if hitType == 1: HandleGroundTargeting(this, &localResult) // AoE spell + if hitType == 2: HandleTargetSelection(this, &localResult) // object hover +``` + +### UpdateHitTest (0x481F00) — __fastcall(ECX=worldFrame) +Called on **click events** (not every frame). Performs the same raycast but stores +the result persistently at `worldFrame + 0x350`: +```c +void __fastcall UpdateHitTest(void *worldFrame) { + NDCToDDC(&screenX, &screenY, + *(float *)(*(worldFrame + 0xA0) + 0x1118), + *(float *)(*(worldFrame + 0xA0) + 0x111C)); + hitType = HitTestPoint(worldFrame, screenX, screenY, worldFrame + 0x358); + *(worldFrame + 0x350) = hitType; +} +``` + +### WorldFrame HitTestResult Layout (worldFrame + 0x350) +| Offset | Size | Type | Description | +|--------|------|------|-------------| +| +0x350 | 4 | u32 | Hit type (see below) | +| +0x358 | 8 | u64 | Hit object GUID (0 for terrain) | +| +0x360 | 4 | f32 | Terrain intersection X | +| +0x364 | 4 | f32 | Terrain intersection Y | +| +0x368 | 4 | f32 | Terrain intersection Z | +| +0x36C | 4 | f32 | Intersection distance | +| +0x370 | 12 | Vec3 | Ray origin (camera position) | +| +0x37C | 12 | Vec3 | Ray end point | +| +0x388 | 4 | f32 | Ray distance | + +### AoE Targeting Reticle Globals (only during spell targeting) +- `0x00B4B3A0` Vec3 — terrain position under cursor (written by HandleGroundTargeting) +- `0x00B4B3B0` f32 — spell targeting radius +- `0x0083DC2C` u32 — validity: 0=valid, 1=out-of-range, 3=updating +- `0x00CECAC0` u16 — spell targeting state flags (0x20=terrain, 0x40=secondary) + +### Click-to-Move Destination +- `0x00C4D890` Vec3 — destination (only when CTM initiated) +- `0x00C4D888` u32 — movement mode + +### Key Functions +| Address | Name | Convention | Params | +|---------|------|------------|--------| +| 0x481190 | CGWorldFrame::HitTestPoint | __thiscall | ECX=worldFrame, screenX, screenY, *HitTestResult → hitType | +| 0x481790 | WorldFrameUpdate | __thiscall | ECX=worldFrame, deltaTime | +| 0x481F00 | UpdateHitTest | __fastcall | ECX=worldFrame → void | +| 0x4813B0 | GetWorldPositionFromScreenCoords | __thiscall | ECX=worldFrame, screenX, screenY, *rayStart, *rayEnd → bool | +| 0x480DF0 | WorldIntersectionTest | ??? | rayStart, rayEnd, flags, *result → hitType | +| 0x672170 | CWorld_Intersect | __fastcall | rayStart, rayEnd, ignored(0), *hitPoint, *distance, flags → bool | +| 0x4242F0 | NDCToDDC | ??? | outX*, outY*, inNDC_X, inNDC_Y | +| 0x4820F0 | HandleGroundTargeting | __thiscall | ECX=worldFrame, hitResultPtr | +| 0x6E60F0 | Spell_C_HandleTerrainClick | __fastcall | ECX=terrainClickEvent(Vec3*) | + +### Approach for Markers +### HitTestPoint / WorldIntersectionTest Return Values +`WorldIntersectionTest(rayStart, rayEnd, gameStateFlags, result)` returns: +- `0` — no intersection (sky) — coords NOT written to result +- `gameStateFlags & 1` — terrain hit — coords written. Returns 1 only during + AoE targeting (bit 0 set), otherwise returns 0 even on valid terrain hit +- `2` — object hit (closer than terrain) + +So hitType=0 is ambiguous: either "terrain hit in normal mode" or "no hit at all". +To distinguish: zero the result coords before calling, then check if they were written. + +### Approach for Markers +Call `UpdateHitTest(worldFrame)` to perform the raycast and store result at +`worldFrame+0x350`. Zero intersection coords before the call, then check if +they were populated. This is safe from Lua callbacks — `HitTestPoint` +saves/restores view matrices. The persistent result at `worldFrame+0x358` is +normally only click-updated, but overwriting it is harmless. + +--- + +## Animation System -- Hold Loop Glitch Investigation + +### Problem +Marker models have 3 animations: Stand (grow-in), Hold (idle), Decay (shrink-out). +Hold plays successfully after Stand, but has a visual glitch every ~4s where the +model briefly "rotates and changes scale" before recovering. Re-queuing Hold has +zero effect. Extending Hold duration to 300000ms also had zero effect. + +### M2 File Analysis (Raid_UI_FX_Yellow.m2) + +**Header structure** (v256 with vanilla-only `playableAnimLookup` field): +``` +globalSequences: n=4, ofs=343 +animations: n=3, ofs=359 +animationLookup: n=160, ofs=563 +playableAnimLookup: n=226, ofs=883 (vanilla/BC only, 4 bytes each) +bones: n=10, ofs=1787 +keyBoneLookup: n=1, ofs=4431 +vertices: n=562, ofs=4433 +``` + +**Animation sequences** (68 bytes each at ofs 359): +| Seq | AnimID | Name | Time Range | Duration | Flags | NextAnim | Alias | +|-----|--------|-------|-------------------|----------|--------|----------|-------| +| 0 | 0 | Stand | 3333-7333 | 4000ms | 0x00A0 | 1 | 0 | +| 1 | 158 | Hold | 10666-14666 | 4000ms* | 0x0020 | 1 | 1 | +| 2 | 159 | Decay | 17999-18665 | 666ms | 0x00A1 | -1 | 2 | + +*Hold was temporarily extended to 300000ms for testing; had no effect on glitch. +The M2 file on disk may still have the extended duration -- needs revert. + +**Flag meanings** (from decompiled GetAnimationPathData): +- 0x01: SetBlendTransition +- 0x10: Alternating (ping-pong -- negates direction in path resolution) +- 0x20: Looping (resets direction to 0 in path resolution) +- 0x40: IsAlias +- 0x80: Blended + +**Animation lookup table** (160 entries of int16 at ofs 563): +- `animLookup[0] = 0` (Stand -> Seq 0) -- CONFIRMED working +- `animLookup[158] = 1` (Hold -> Seq 1) -- CONFIRMED working +- `animLookup[159] = 2` (Decay -> Seq 2) -- CONFIRMED working +- All other entries = -1 + +**Global sequences**: GS[0]=16033ms (rotation), GS[1]=3533ms, GS[2]=3867ms, GS[3]=2200ms + +**NOTE**: Stand has `nextAnim=1` pointing to Hold. This is the M2 file's built-in +transition from Stand to Hold. The engine may use this for automatic chaining. + +### Decompiled Animation Functions + +#### GetAnimationPathData (0x711bf0) -- Inner resolver, 392 bytes +Resolves an animation index to a valid animation path. Called per-frame during +bone transform and also during PlayBoneAnimation setup. + +**Logic flow**: +1. Check if `animationIndex < nGlobalSequences` (m2data+0x2c): if so, return + global sequence data from array at m2data+0x30. This is for bone tracks + driven by global sequences (indices 0-3 for our model). +2. Otherwise, look up in animation lookup table at m2data+0x28 (array of int16, + indexed by animation ID). If `animLookup[animId] != -1`, found. +3. If not in local table, walk the GLOBAL animation table at PTR_00c0e070. + Each entry has +0x14=flags and +0x18=nextAnimID. Follows chain with: + - Flag 0x10: alternating (negate direction) + - Flag 0x20: looping (stop advancing) +4. If chain dead-ends, falls back to default animation: + - If animLookup[0] != -1 -> fallback = Stand (anim 0) + - Else if animLookup[147] != -1 -> fallback = 147 + - Else -> first animation in sequence table + +**For our model**: animLookup[158]=1, so Hold is found directly in step 2. +No fallback occurs. **Eliminates fallback-to-Stand as glitch cause.** + +#### GetAnimationPathData wrapper (0x711a20) -- 392 bytes +Higher-level wrapper called by ApplyComplexTransformWithAnimation. +1. Calls inner resolver to get animation index +2. Calls `GetAnimationIndex` to convert to sequence index +3. Follows `nextAnimation` chain in M2 sequence data (param_2 times) +4. Returns sequence flags, duration, bounding box from 0x44-byte sequence record + +**Sequence record layout** (at m2data+0x20, each 0x44 bytes): +- +0x04/+0x08: startTimestamp/endTimestamp +- +0x0C: moveSpeed +- +0x10: flags +- +0x24..+0x3B: bounding box +- +0x3C: nextAnimation field +- +0x40: aliasNext + +#### SetBoneAnimationTiming (0x7127f0) -- 275 bytes, __thiscall +Sets timing offsets for a bone animation. When model+0x10==0, buffers command. +Otherwise: resolves bone index, checks bone+0xA4 != -1 (active anim), updates +timing offsets at bone+0xA8 and bone+0xAC via __ftol conversions. +**NOT the animation timer advance function.** + +#### SetBoneAnimationSpeed (0x712910) -- 373 bytes, __thiscall +Similar structure to SetBoneAnimationTiming. Sets speed-related values at +bone+0xB0 and bone+0xB4. **NOT the animation timer advance function.** + +#### PrepareModelForRender (0x710450) -- 193 bytes, __thiscall +Per-frame entry for texture readiness. When model+0x10==0, calls LoadPlayerModelData. +Iterates texture array at this+0xA4, calls GetTextureBuffer for each. +Recursively prepares attached models via linked list at this+0x1DC. +Returns 0 (not ready) or 1 (ready). **Textures only, no animation logic.** + +#### ApplyComplexTransformWithAnimation (0x7106c0) -- 951 bytes, __thiscall +Per-bone transform computation. Called during rendering. +1. Sets identity matrix at this+0xBC +2. Applies translation, rotation from params +3. Handles billboard types (model type flags & 3) +4. Calls `GetModelAnimationDataAtIndex(this, 0xFFFFFFFF, &local_2c)` to get + current bone animation state +5. Calls wrapper `GetAnimationPathData(this, animId, count, &outData)` to + resolve animation path +6. Extracts movement type from flags (bits 1-3) +7. Calculates blend factor based on movement type +8. Blends between rest matrix and animation target matrix +**Calls**: ApplyTranslationMatrix, rotateParticleMatrixByAxisAngle, +CreateOrthonormalBasis, GetModelAnimationDataAtIndex, GetAnimationPathData, +scaleMatrix4x4, addMatrix4x4, scaleMatrix3x3ByVector + +#### updateAnimationTransform (0x714000) -- 521 bytes, __fastcall +Parent transform propagation for scene objects. Checks transform_sync_value +against animation_context_ptr+0x10. Calls transformMatrix4x4 with various +matrix parameters. Handles parent-child bone attachment transforms. + +#### transformMatrix4x4 (0x714260) -- 17703 bytes!, __thiscall +The MAIN per-frame bone transform function. Iterates all bones (0x118 stride), +reads animation state, samples keyframes, computes final bone matrices. +Called by renderFrame (0x707680) twice per frame, and by updateAnimationTransform. +**Too large for full decompilation -- needs targeted analysis of the timer +advance and loop handling code within it.** + +### Key Functions in Animation Range (0x710000-0x715000) + +| Address | Name | Size | Role | +|------------|-------------------------------------|---------|------| +| 0x7106c0 | ApplyComplexTransformWithAnimation | 951 | Per-bone transform with animation blend | +| 0x710450 | PrepareModelForRender | 193 | Texture readiness check | +| 0x710b90 | CM2Model_ManageRenderListNode | 90 | Render list add/remove | +| 0x711a20 | GetAnimationPathData (wrapper) | 392 | Animation resolver + sequence data | +| 0x711bf0 | GetAnimationPathData (inner) | ~350 | Animation ID fallback chain | +| 0x7119a0 | GetAnimationSequenceLength | 122 | Duration query | +| 0x711fe0 | GetModelAnimationDataAtIndex | ? | Read bone animation state | +| 0x712090 | GetBoneCurrentAnimation | 78 | Current anim ID for bone | +| 0x7120e0 | GetBoneAnimationTime | 108 | Current timer for bone | +| 0x7121a0 | CM2Model__PlayBoneAnimation | ~600 | Set/queue animation | +| 0x7127f0 | SetBoneAnimationTiming | 275 | Timing offset adjustment | +| 0x712910 | SetBoneAnimationSpeed | 373 | Speed adjustment | +| 0x712f70 | InitializeAnimationNode | 168 | Initial bone setup | +| 0x713d50 | findInterpolationIndices | 334 | Keyframe index lookup | +| 0x713ea0 | interpolateAnimationKeyframes | 337 | Keyframe interpolation | +| 0x714000 | updateAnimationTransform | 521 | Parent transform propagation | +| 0x714260 | transformMatrix4x4 | 17703 | Main bone transform engine | + +### Bone Entry Structure (0x118 bytes per bone, array at model+0x90) + +From PlayBoneAnimation decompilation: +- +0x08..+0x28: active animation parameters +- +0xA4: animation ID or sequence index (checked against -1 for "none") +- +0xA8, +0xAC: timing offsets (set by SetBoneAnimationTiming) +- +0xB0, +0xB4: speed values (set by SetBoneAnimationSpeed) +- +0xD0..+0xE4: queued animation data (0xFFFFFFFF = empty) +- +0x100/+0x104: blend weight/timer (used by blendMode=1) +- +0x110: queue prev ptr (linked list) +- +0x114: queue next index + +### Xref Map + +**PlayBoneAnimation (0x7121a0) callers**: +- CM2Model_LoadInstanceData (0x70ebd0) -- during model init +- HandleUnitAnimationEvent (0x5fc3f0) -- twice +- PlayAnimationWithSpeed (0x76cf80) -- Lua/script wrapper +- RenderCharacterPortrait (0x524f60) -- character screen +- configureDebugOutput (0xd06060) -- debug + +**GetAnimationPathData (0x711bf0) callers**: +- GetAnimationPathData wrapper (0x711a20) -- main per-frame path +- CM2Model__PlayBoneAnimation (0x7121a0) -- animation setup +- CM2Model_LoadInstanceData (0x70ebd0) -- model init +- GetAnimationSequenceLength (0x7119a0) -- duration query + +**transformMatrix4x4 (0x714260) callers**: +- renderFrame (0x707680) -- twice per frame +- transformMatrix4x4 itself (recursive for children) +- renderSceneNode (0x718960) +- updateAnimationTransform (0x714000) + +### Remaining Investigation + +**The actual animation timer advance function has NOT been found yet.** +None of the decompiled functions contain the per-frame timer increment, +duration comparison, or loop-end handling logic. The timer advance is most +likely inside `transformMatrix4x4` (0x714260, 17703 bytes) which is too +large for a single decompilation pass. It needs targeted analysis: + +1. Search within transformMatrix4x4 for duration comparison patterns +2. Find where bone+0xA8/0xAC timing values are read and updated +3. Find where the loop flag (0x20) is checked during playback +4. Find where queued animation (+0xD0) is activated + +**Alternative approach**: Add runtime debug logging to dump bone entry fields +(active anim at +0x08, timer at bone+0xA8/+0xAC, queued at +0xD0) every frame +around the 4s mark to see what changes at the glitch point. + +**Global sequence hypothesis**: GS[1]=3533ms and GS[2]=3867ms are close to +4s. Particle system or bone track resets at global sequence boundaries could +cause the visual glitch. Need to identify which bone tracks use global sequences. + +--- + +## World Teardown — Entity Cleanup Crash Investigation + +### Crash Details +- **Crash function**: 0x687220 — generic linked-list unlink operation + - First crash at 0x687243: `mov [edx], esi` — write to freed memory + - Second crash at 0x687221: `mov esi, [ecx]` — read from freed memory +- **When**: Logout to character select, map transitions — NOT during normal gameplay +- **Thread**: Background/worker thread (very short stack: WoW.exe → kernel32 → ntdll) +- **Root cause**: WDOODADDEF heap teardown iterates linked list, hits freed or corrupt node + +### Decompiled Crash Function (0x687220) +```c +// Linked-list unlink — removes node from intrusive doubly-linked list +void __fastcall UnlinkFromList(int *param_1) { + int prev = *param_1; // param_1[0] = prev pointer + if (prev != 0) { + uint next = param_1[1]; // param_1[1] = next pointer + int *target; + if (((next & 1) == 0) && (next != 0)) { + target = (int *)((int)param_1 + (next - *(int *)(prev + 4))); + } else { + target = (int *)(next & 0xfffffffe); + } + *target = prev; // target->prev = param_1->prev + *(int *)(*param_1 + 4) = param_1[1]; // param_1->prev->next = param_1->next + *param_1 = 0; + param_1[1] = 0; + } +} +``` + +### Crash Callers (who calls 0x687220) +- `CleanupMapDoodadContainer` (0x6a1280) +- `MapDoodadDestructor` (0x6a14d0) +- `CleanupDoodadList` (0x6a1725) +- `cleanup_data_structures` (0x67f4b5) +- `CreateUnitModelObject` (0x694c00) +- `FindOrCreateWorldUnit` (0x694f00) +- `InsertIntoHashTable` (0x696060) +- `RehashContainer`, `ResizeContainer`, `ReallocateContainer` (hash table ops) +- Two unnamed functions at 0x69f784 and 0x69f7b5 (the actual crash callers from stack) + +### World Teardown Chain (Ghidra-verified) +``` +CleanupWorldAndEntities (0x66fc40) — void(), no params, __stdcall +├── CleanupEntityList_ProcessAll() ← iterates UNKNOWN list +└── CleanupWorldAndReleaseResources (0x697ac0) + ├── ClearWorldObjectsAndResetState (0x6a6710) ← iterates linked list at PTR_00c96088 + │ └── destroyPrimaryGameObject() on each + ├── ... iterate PTR_00c92078 array ... + ├── ComplexMemoryCleanupAndRelease on remaining + └── destroyWorldEnvironment / cleanupGameObject on remaining +``` + +### Callers of CleanupWorldAndEntities (0x66fc40) +- `InitializeWorldScene` (0x401bc0) — **map change** (cleans old world before loading new) +- `ShutdownClientSystems` (0x401ee0) — **full game exit** + +### Callers of ClearWorldObjectsAndResetState (0x6a6710) +- `LoadWorldMap` (0x6941f0) — map loading +- `UpdateWorldAndGameObjects` (0x698390) — periodic world update (chunk unloading?) +- `CleanupWorldAndReleaseResources` (0x697ac0) — full teardown +- `SimpleWorldUpdate` (0x694920) + +### Entity Type Dispatch in CleanupEntity_ProcessAttachments (0x670d50) +```c +void __fastcall CleanupEntity_ProcessAttachments(entity) { + // Walk and free attachment children via entity[8] linked list + while (memoryObject = entity[8], ...) { + ComplexMemoryCleanupAndRelease(memoryObject); + } + // Decrement refcount + *(short *)(entity + 0x0E) -= 1; + // Dispatch to type destructor + if (entity[2] & 0x8) { + destroyWorldEnvironment(entity); // → DestroyDataStructureAndRelease + } else if (entity[2] & 0x40) { + cleanupGameObject(entity); // → CleanupVisualEffectAndRelease + } +} +``` +- M2 entities (CreateWorldUnit → WDOODADDEF heap) have flag 0x40 +- WMO entities (CreateGameObject → WMAPOBJDEF heap) have flag 0x8 +- Ghidra names are misleading — `cleanupGameObject` handles M2/WDOODADDEF, `destroyWorldEnvironment` handles WMO/WMAPOBJDEF + +### CleanupEntity_ProcessAttachments Callers (ONLY 3 in entire binary) +- `processCinematicExit` (0x6e4940) +- `executeSpellOrItem` (0x6e54f0) +- `DestroyPathObjectIfPresent` (0x5f4950) +- **NOT called by CleanupEntityList_ProcessAll** or any teardown function + +### WDOODADDEF Heap (0xCA7E20) References +- `AllocateRenderableObject` (0x6a07f7) — allocates from heap +- `CleanupVisualEffectAndRelease` (0x6a0916) — frees to heap +- `InitializeWorldSystem` (0x691f4f) — initializes heap +- `CleanupWorldSystem` (0x692241) — tears down heap + - Called by `ShutdownAllGameSystems` (0x66fb00) + +### Other Key Addresses +- `ShutdownAllGameSystems` (0x66fb00) → calls CleanupWorldSystem +- `CleanupWorldSystem` (0x6920c0) → called by ShutdownAllGameSystems +- `cleanupSecondaryResources` (0x6a6c70) → called from CleanupWorldAndEntities + CleanupWorldSystem + - This calls `cleanupGameObject` (0x6a67a0) and `destroyWorldEnvironment` (0x6a6870) on entities +- `gameQuit` (0x41f9b0) — fires on disconnect/quit (ref: UnitXP_SP3) + +### World Unit Hash Table (0xCA7DC0) — The Crash Structure + +The crash occurs during teardown of a **hash table at 0xCA7DC0** that tracks all world units +(doodads created via `CreateWorldUnit`). Our entities ARE in this table. + +**Hash table structure** (globals at 0xCA7DC0-0xCA7DE4): +| Offset | Global | Purpose | +|----------|----------------|--------------------------------------------| +| 0xCA7DC0 | vtable ptr | Points to 0x0081089c (DestroyMapDoodadDef) | +| 0xCA7DC4 | global list | Head of "all entries" linked list | +| 0xCA7DC8 | sentinel | List sentinel/header | +| 0xCA7DCC | iteration ptr | **Iterated during teardown crash loop** | +| 0xCA7DD0 | count | Zeroed during cleanup | +| 0xCA7DD4 | (unknown) | | +| 0xCA7DD8 | bucket count | Array length | +| 0xCA7DDC | bucket array | Hash buckets (0xC bytes each) | +| 0xCA7DE4 | hash mask | 0xFFFFFFFF = not initialized | + +**References to these globals**: +- `CreateWorldUnit` (0x694980): writes 0xCA7DC0, 0xCA7DC4, 0xCA7DDC (registers entities) +- `CreateUnitModelObject` (0x694c00): writes 0xCA7DC0, 0xCA7DDC +- `FindOrCreateWorldUnit` (0x694e90): reads 0xCA7DC4, 0xCA7DDC +- `complexListInitializerWithCleanup` (0x69f670): reads/writes ALL (init + teardown) + +### CreateWorldUnit (0x694980) — Entity Registration (Decompiled) + +`CreateWorldUnit` inserts entities into **three** linked lists: +```c +int *CreateWorldUnit(char *modelPath, float *pos, float facing, int param4) { + // 1. Check if entity already exists in hash table (by path hash + param4) + if (PTR_00ca7de4 != 0xFFFFFFFF) { + // Search bucket: PTR_00ca7ddc[hash & mask].list + // Compare entity[0x2d] == modelPath and entity[0x32] == param4 + // If found, return existing entity (no duplicate creation) + } + + // 2. Allocate new entity from WDOODADDEF heap + unitObject = AllocateRenderableObject(); + + // 3. Initialize hash table if needed + if (PTR_00ca7de4 == 0xFFFFFFFF) InitializeHashTable(0xca7dc0); + + // 4. INSERT into hash bucket linked list + ManageLinkedList(PTR_00ca7ddc + bucket * 0xc, unitObject, 2, 0); + + // 5. INSERT into global "all entries" linked list (PTR_00ca7dc4) + ManageLinkedList(&PTR_00ca7dc4, unitObject, 2, 0); + + // 6. INSERT into THIRD linked list (PTR_00c89f0c) + piVar2 = PTR_00c89f08 + unitObject; // node at entity + offset + *piVar2 = PTR_00c89f0c; + piVar2[1] = *(PTR_00c89f0c + 4); + *(PTR_00c89f0c + 4) = unitObject; + PTR_00c89f0c = piVar2; + + // 7. Store hash key and setup fields + unitObject[0x2d] = modelPath; // path hash for lookup + unitObject[0x32] = param4; + // ... copy position, set up model, etc. +} +``` + +**Critical**: Entity is in 3 lists. All 3 must be unlinked during cleanup. + +### CleanupVisualEffectAndRelease (0x6a0840) — What It Unlinks + +`CleanupVisualEffectAndRelease` unlinks from up to **three** linked lists: +```c +void CleanupVisualEffectAndRelease(entity) { + // 1. Remove visual effect handle + if (entity[0x5a]) RemoveVisualEffectByHandle(entity[0x5a]); + + // 2. Unlink from list at entity[4]/entity[5] (PTR_00ca7dc4 global list?) + if (entity[4] != NULL) { /* unlink prev/next */ } + + // 3. Unlink from list at entity[0x2e]/entity[0x2f] (conditional) + if (entity[0x2f] != NULL) { + /* unlink entity[0x2e]/entity[0x2f] */ + /* unlink entity[0x30]/entity[0x31] */ + } + + // 4. Free entity back to WDOODADDEF heap + // (via ObjectPool_Free or similar) +} +``` + +**Question**: Does this cover all 3 lists that `CreateWorldUnit` inserts into? +- entity[4]/[5] → PTR_00ca7dc4 global list ✓ +- entity[0x2e]/[0x2f] and entity[0x30]/[0x31] → possibly hash bucket + PTR_00c89f0c lists +- But is the **hash bucket list** (inserted via `ManageLinkedList(PTR_00ca7ddc + bucket*0xc, ...)`) also unlinked? + +### Crash Caller Disassembly (0x69f760-0x69f7b0) + +The crash is in `complexListInitializerWithCleanup` (Ghidra: `registerInitializer5`, 0x69f730). +The specific loop that crashes: +```asm +; Loop: iterate PTR_00ca7dcc linked list, unlink each node +0x69f762: MOV [0xca7dd0], EBX ; zero the count +0x69f768: MOV ECX, [0xca7dcc] ; load list head +0x69f76e: TEST CL, 0x1 ; check sentinel bit +0x69f771: JNZ 0x69f78b ; done if sentinel +0x69f773: CMP ECX, EBX ; check NULL +0x69f775: JZ 0x69f78b ; done if NULL +0x69f777: PUSH ECX ; arg: node from list +0x69f778: MOV ECX, 0xca7dc4 ; arg: container base +0x69f77d: CALL 0x687960 ; ContainerLookup(container, node) +0x69f782: MOV ECX, EAX ; result = node in different list space +0x69f784: CALL 0x687220 ; UnlinkFromList(result) *** CRASH *** +0x69f789: JMP 0x69f768 ; loop + +; Second loop: iterate bucket array +0x69f78b: MOV EAX, [0xca7dd8] ; bucket count +0x69f797: MOV EAX, [0xca7ddc] ; bucket array base +; ... iterate each bucket, unlink nodes ... +``` + +**Key insight**: The teardown iterates PTR_00ca7dcc AND the bucket array (PTR_00ca7ddc). +It uses `ContainerLookup(0xca7dc4, node)` to convert from the iteration list to the +global list, then calls `UnlinkFromList` on the global list node. + +The crash: `UnlinkFromList` receives ECX pointing to freed heap memory. + +### Stack Context from Crash #2 (0x687221) +``` +0x40a3a6 in terminateProcessWithCleanup (0x40a34d) ← atexit handler +0x40a33a in exitNormally (0x40a32f) ← game exit path +0x64cc46 in ??? (file I/O area) +``` +This confirms the crash is during **game exit**, in the atexit cleanup chain. + +### CreateEntityInstance_WithAttachment M2 Path (0x6707c0) + +The M2 path (no ".wmo" in path) is straightforward: +```c +positionData = CreateWorldUnit(param_1, param_2, param_3, param_4); +positionData[0x61] = param_7; +positionData[0x60] = param_6; +positionData[0x24] |= 0x2000; // set flag +if (updateNow) { + UpdateWorldPosition(positionData, param_2, param_3); + SetUnitPositionAndOrientation(positionData, param_2, param_3); +} +``` +The WMO path additionally inserts into PTR_00c9e350/PTR_00c9e354 spatial grid list. + +Only 3 CALLs visible in the function body: `FindSubstringInString`, `CreateGameObject`, +`ModelAttachment_CreateNode`. The M2 path (`CreateWorldUnit`) must be via tail-call or +the decompiler inlined it. + +### CleanupWorldAndEntities (0x66fc40) — Full Chain (Decompiled) + +```c +void CleanupWorldAndEntities(void) { + CleanupEntityList_ProcessAll(); // 0x672c40 — iterates PTR_00c7b2dc (NOT hash table) + CleanupWorldAndReleaseResources(); // 0x697ac0 + PTR_00c7b748 = 0; +} +``` + +### CleanupEntityList_ProcessAll (0x672c40) — Decompiled + +Iterates the linked list at `PTR_00c7b2dc` (NOT the hash table at 0xCA7DC0). +For each entry, calls `CleanupObjectAttachments_FreeMemory` which ends with +`DestroyWorldObjectAndRelease`. Our entities are NOT in `PTR_00c7b2dc` — +they're only registered in the hash table. So this function doesn't touch them. + +```c +void CleanupEntityList_ProcessAll(void) { + puVar6 = PTR_00c7b2dc; // scene entity list head + while (puVar6 valid) { + puVar1 = next_from_linked_list; + CleanupObjectAttachments_FreeMemory(*puVar6); // -> DestroyWorldObjectAndRelease + // Inline unlink puVar6 from its list (puVar6[3]/[4]) + // Move puVar6 to free list at PTR_00c63188 + puVar6 = puVar1; + } +} +``` + +**PTR_00c7b2dc xrefs** (who manages this list): +- `CleanupEntityList_ProcessAll` (0x672c40) — reads/iterates +- `UpdateFadeEffects_ProcessTimers` (0x672efe) — reads +- `DestroyFileMapping` (0x66f460) — reads + writes (teardown) +- `SetFileAttributes` (0x66f440) — writes (initialization) +- `CreateFadeEffect_EntityManagement` (0x672e76) — reads (entity insertion?) + +Our entities created via `CreateEntityInstance_WithAttachment` are NOT added to this +list. The callers (`CastSpellByID_Extended`, `CreateGameObjectPathEffect`) probably add +the returned entity to PTR_00c7b2dc themselves. We don't. + +### Vtable at 0x0081089c — Hash Table Entry Destructors + +``` +[0] 0x006a1170 DestroyMapDoodadDefinition — WDOODADDEF destructor +[1] 0x006a11a0 LoadAndAddMapDoodadToList +[2] 0x006a14d0 MapDoodadDestructor +[3] 0x006a1260 CleanupMapDoodadContainer +[4] 0x006a1320 DestroyMapObjectDefinition — WMAPOBJDEF destructor +[5] 0x006a1350 LoadAndAddMapObjectToList +[6] 0x006a1590 MapObjectDestructor +[7] 0x006a1410 CleanupMapObjectContainer +``` + +`DestroyMapDoodadDefinition` (vtable[0], 0x6a1170): +```c +void DestroyMapDoodadDefinition(undefined **param_1) { + (**(code **)*param_1)(0); // call entity's own virtual destructor + FreeMemory(param_1, "?AVCMapDoodadDef@@", 0xfffffffe); +} +``` +This is a simple destructor: calls entity vtable[0](0) then frees via SMemFree. +NOT the same as CleanupVisualEffectAndRelease — doesn't unlink from lists. + +### hashTableTeardownLoop (0x69f740) — atexit Handler (Decompiled) + +Registered via `validateMemoryOperation` (atexit) at 0x69f730. Runs during process exit. +```c +void hashTableTeardownLoop(void) { + if ((DAT_00ca7cf0 & 1) == 0) { + DAT_00ca7cf0 |= 1; // mark as running + _DAT_00ca7dc0 = &vtable_0081089c; + _DAT_00ca7dd0 = 0; // zero count + + // PHASE 1: Unlink all entries from global list (PTR_00ca7dcc) + while (PTR_00ca7dcc valid) { + piVar2 = ValidateLinkedList(&PTR_00ca7dc4, PTR_00ca7dcc); + UnlinkFromList(piVar2); // *** CRASH HERE *** + } + + // PHASE 2: Unlink all entries from each hash bucket + for each bucket in PTR_00ca7ddc (0xC bytes each) { + while (bucket[8] valid) { + piVar2 = ValidateLinkedList(bucket, bucket[8]); + UnlinkFromList(piVar2); // *** OR CRASH HERE *** + } + } + + // PHASE 3: Clear and free bucket array + for each bucket: ClearLinkedList(bucket); + FreeMemory(PTR_00ca7ddc); + + // PHASE 4: Unlink remaining from global list + while (PTR_00ca7dcc valid) { unlink inline; } + + // PHASE 5: Reset sentinel + PTR_00ca7dc8 = NULL; PTR_00ca7dcc = NULL; + } +} +``` + +**Critical**: This iterates ALL entries in the hash table's global list AND bucket lists. +If an entity was freed (by our cleanup) but not unlinked from these lists, the teardown +follows dangling pointers into freed heap memory. + +### cleanupGameObject (0x6a67a0) — Decompiled + +```c +void __fastcall cleanupGameObject(undefined **param_1) { + if (*(short*)(param_1 + 0xe) != 0) return; // refcount check + + // Walk and cleanup child objects via param_1[0x21] list + while (child in param_1[0x21]) { + CleanupSpecializedObjectAndRelease(child); + } + + // Detach model render context + if (param_1[0x22] != NULL) { + SetModelScale(param_1[0x22], NULL, NULL, NULL); + SetCallbackFunctions(param_1[0x22], NULL, NULL, NULL); + SetRenderCallbacks(param_1[0x22], NULL, NULL); + DecrementReferenceCount(param_1[0x22]); + param_1[0x22] = NULL; + } + + // Unlink from ONE list: param_1[0x5b]/param_1[0x5c] + if (param_1[0x5b] != NULL) { /* unlink */ } + + CleanupVisualEffectAndRelease(param_1); // frees entity +} +``` + +### CleanupVisualEffectAndRelease (0x6a0840) — What It Actually Unlinks + +```c +void __fastcall CleanupVisualEffectAndRelease(entity) { + if (entity[0x5a]) RemoveVisualEffectByHandle(entity[0x5a]); + + // Unlink from list 1: entity[4]/entity[5] (global list at PTR_00ca7dc4) + if (entity[4] != NULL) { /* unlink */ } + + // Conditionally unlink from lists 2+3: entity[0x2e-0x31] + if (entity[0x2f] != NULL) { + // Unlink entity[0x2e]/entity[0x2f] + // Unlink entity[0x30]/entity[0x31] + } + + // Call virtual destructor and free to WDOODADDEF heap + (**(code**)*entity)(0); + ReleaseToHeap(WDOODADDEF, entity[1]); +} +``` + +**CONFIRMED**: `CleanupVisualEffectAndRelease` unlinks entity[4]/[5] (global list) +and conditionally entity[0x2e-0x31]. It does NOT unlink from the **hash bucket list**. + +### ManageLinkedList (0x695ef0) — Intrusive List Insertion + +```c +void __thiscall ManageLinkedList(void *this, int *entity, int mode, int insert_point) { + // Compute node address: base_offset + entity_address + // base_offset = *this (first dword of list header) + piVar2 = (mode==0) ? this+4 : *this + entity; + + // First: unlink piVar2 from its current list (if linked) + if (*piVar2 != 0) { /* unlink prev/next */ } + + // Compute insertion point + piVar3 = (insert_point==0) ? this+4 : *this + insert_point; + + // Insert (mode 2 = insert before head) + if (mode != 1) { + node->next = *piVar3; + node->prev = piVar3->prev; + piVar3->prev->next = entity; + *piVar3 = node; + } +} +``` + +The node address within the entity = `*list_header + entity_ptr`. Each list stores its +own base offset at `*list_header`. The hash bucket list and global list use DIFFERENT +base offsets, so the intrusive nodes are at different positions within the entity struct. + +### ROOT CAUSE ANALYSIS (2026-03-02) + +**Root cause: Our cleanup via `CleanupEntity_ProcessAttachments` frees entities but +leaves them linked in the hash table's bucket list.** + +**Full crash chain**: +1. We create entities via `CreateEntityInstance_WithAttachment` -> `CreateWorldUnit` +2. `CreateWorldUnit` registers entity in 3 intrusive linked lists: + - Hash bucket list at `PTR_00ca7ddc + bucket*0xc` (node offset from `*bucket`) + - Global list at `PTR_00ca7dc4` (node at entity[4]/[5]) + - Third list at `PTR_00c89f0c` (node offset from PTR_00c89f08) +3. When we clear a marker or on world cleanup, we call `CleanupEntity_ProcessAttachments` +4. This calls `cleanupGameObject` -> `CleanupVisualEffectAndRelease` which: + - Unlinks entity[4]/[5] from global list -- OK + - Frees entity memory back to WDOODADDEF heap + - Does NOT unlink from hash bucket list or third list +5. Hash bucket list now has dangling pointer to freed memory +6. On game exit: atexit `hashTableTeardownLoop` iterates bucket list -> follows dangling + pointer -> ACCESS_VIOLATION on freed heap memory + +**Why normal game entities don't crash**: Spell entities created via +`CreateEntityInstance_WithAttachment` ARE also added to `PTR_00c7b2dc` by their +callers. During map change, `CleanupEntityList_ProcessAll` iterates `PTR_00c7b2dc` +and calls `DestroyWorldObjectAndRelease` which frees entities. But the hash table +teardown (`hashTableTeardownLoop`) only runs during atexit -- by which point +`CleanupWorldAndReleaseResources` has already cleaned up the hash table structure +itself (zeroed buckets, freed bucket array). So the atexit handler finds an empty +hash table and doesn't iterate any entries. Our entities bypass `PTR_00c7b2dc` +and survive into the atexit handler with dangling bucket list pointers. + +**Alternative hypothesis**: Our cleanup during `worldCleanupDetour` (before the +original `CleanupWorldAndEntities`) frees entities. Then either +`CleanupWorldAndReleaseResources` or the atexit handler iterates the bucket list +and crashes on our freed entries. + +**Test to confirm**: Remove ALL our cleanup (no `CleanupEntity_ProcessAttachments`, +no `worldCleanupDetour`), spawn markers, close the game. If the game's own teardown +can handle our entities naturally (they're properly registered in the hash table), +no crash. If it still crashes, the problem is in entity creation/registration. + +### Callers of CreateEntityInstance_WithAttachment + +Only 3 callers in the entire binary: +- `CastSpellByID_Extended` (0x6e518f) -- spell visual effects +- `CreateGameObjectPathEffect` (0x5f8076) -- path effects +- Unknown (0x6e5a6e) -- probably another spell effect + +These callers likely add the returned entity to `PTR_00c7b2dc` (scene entity list) +so it gets cleaned up during `CleanupEntityList_ProcessAll`. We don't do this. + +### Hash Table Bucket Base Offset: 0xB8 (CONFIRMED) + +From `InitializeHashTable` (0x6962e0): +```c +*bucket = 0xb8; // base offset for intrusive list nodes +``` + +From `RehashTableIfNeeded` (0x6964f0): +```c +ResetContainerState(bucket, 0xb8); // all buckets use 0xB8 +ManageLinkedList(bucket + (entity[0x2D] & mask) * 0xc, entity, 2, 0); +``` + +So the 3 intrusive list node offsets within a WDOODADDEF entity are: +- **0xB8** (entity[0x2E]/[0x2F]) = hash bucket list node +- **0xC0** (entity[0x30]/[0x31]) = hash global list node +- **0x10** (entity[4]/[5]) = third list (PTR_00c89f0c, base PTR_00c89f08) + +`CleanupVisualEffectAndRelease` unlinks all 3 (conditionally on entity[0x2F] != 0). +After `ManageLinkedList` insertion, entity[0x2F] should always be non-zero +(sentinel has bit 0 set = odd address). + +### InitializeRenderableObject (0x6a7d00) — Entity Initialization + +Called from `AllocateRenderableObject`. Zeroes most fields including: +- entity[0x2E] = 0, entity[0x2F] = 0 (bucket list node — zeroed before insertion) +- entity[0x30] = 0, entity[0x31] = 0 (global list node — zeroed before insertion) +- entity[2] |= 0x40 (sets the M2/WDOODADDEF flag) +- entity[0] = vtable PTR_DestroyRenderableObject_00810a74 + +### Callers of CreateEntityInstance_WithAttachment — What They Do After + +Only 3 callers in entire binary: + +1. **`CreateGameObjectPathEffect` (0x5f8030)** — `__thiscall` on a game object + - Does NOT save the return value! Fire-and-forget. + - Passes `(path, pos, facing, 0, 0, param_1, param_2)` — param_6/7 are parent refs + +2. **`CastSpellByID_Extended` (0x6e4b60)** — spell casting + - Stores in global `PTR_00ceca8c` + - Calls `SetEntityFlag_ToggleBit(entity, 0)` = sets `entity[0xD] |= 1` + - Cleaned up by `processCinematicExit` → `CleanupEntity_ProcessAttachments(PTR_00ceca8c)` + - Passes `(path, pos, 0.0, 0, 0, 0, 0)` — update_now=0! + +3. **Unknown (0x6e5a6e)** — likely another spell effect + +**Key differences from our call**: +- Both native callers pass `update_now=0` (param_5). We pass `update_now=1`. +- Spell caller calls `SetEntityFlag_ToggleBit(entity, 0)`. We don't. +- Neither caller registers entity in any extra tracking list. + +### CreateWorldUnit Has Only ONE Caller + +`CreateWorldUnit` (0x694980) is ONLY called from `CreateEntityInstance_WithAttachment`. +Map doodads use `FindOrCreateWorldUnit` (0x694e90) called from `AttachDoodadObjects` (0x695b1e). +These are separate creation paths that both register in the hash table but through different code. + +### CleanupWorldAndReleaseResources (0x697ac0) — Full Chain + +```c +void CleanupWorldAndReleaseResources(void) { + ClearWorldObjectsAndResetState(); // iterates PTR_00c96088 + // iterate PTR_00c92078[0..0x1000] // world chunk cleanup + // iterate PTR_00c9e358 // WMO entities → destroyWorldEnvironment + // iterate PTR_00c962bc // some entities → cleanupGameObject + CleanupWorldObjectList(); + // ... graphics cleanup ... +} +``` + +PTR_00c962bc is NOT for WDOODADDEF entities from CreateWorldUnit. It has its own +atexit teardown at 0x691830 (separate from hash table teardown). Base offset is 0xC +(set by InitializeDataPointers2 at 0x6917f0). + +### TEST RESULT: No-cleanup build still crashes + +Disabled ALL our cleanup (no CleanupEntity_ProcessAttachments, no world_cleanup_hook, +no removeHooks cleanup). Created 5 markers, replaced with 5 more, closed game. +**Still crashed.** This means the crash is NOT caused by our cleanup — the game's +own atexit handler can't handle our entities even when they're fully intact. + +The WDOODADDEF heap is destroyed by `CleanupWorldSystem` (called from +`ShutdownAllGameSystems`) BEFORE the atexit `hashTableTeardownLoop` runs. +Our entities' memory becomes invalid while they're still in the hash table. + +Normal map doodads presumably get removed from the hash table during +`ClearWorldObjectsAndResetState` or earlier in `CleanupWorldAndReleaseResources`, +so the hash table is empty before heap destruction. + +### OPEN QUESTION: How do spell-spawned entities survive teardown? + +Spell entities (from CastSpellByID_Extended) also use CreateEntityInstance_WithAttachment +and are NOT registered in any cleanup tracking list. They're cleaned up explicitly by +`processCinematicExit` when the spell ends. If a spell entity is still active during +map change/exit, it would have the same crash problem as our entities. + +The game avoids this because spells always end before map transitions. But we don't +have that guarantee — our markers persist across frames until explicitly cleared. + +### TODO +- [x] Decompile 0x672c40 (CleanupEntityList_ProcessAll) — iterates PTR_00c7b2dc, not hash table +- [x] Decompile hashTableTeardownLoop (0x69f740) — atexit handler, iterates all hash entries +- [x] Check vtable at 0x0081089c — DestroyMapDoodadDefinition, simple free +- [x] Decompile ContainerLookup (0x687960) — converts between list spaces +- [x] Decompile cleanupGameObject + CleanupVisualEffectAndRelease — handles all 3 lists IF entity[0x2F]!=0 +- [x] Decompile ManageLinkedList (0x695ef0) — intrusive list with base offset +- [x] Confirm bucket base offset = 0xB8 from InitializeHashTable +- [x] TEST: no-cleanup build still crashes — crash is NOT from our cleanup +- [x] Decompile native callers — neither registers in extra lists +- [x] **Investigate spell-spawned game objects (e.g. mailbox summon) as reference** + - Spell entities are NOT "persistent game objects" — they're client-side visual effects only + - `processCinematicExit` (0x6e4940) explicitly cleans them: `CleanupEntity_ProcessAttachments(entity); entity = NULL;` + - Called before every new spell cast — spell entities NEVER survive to teardown + - If a spell entity survived to atexit, it would crash too (same bug as ours) +- [x] Check what `ClearWorldObjectsAndResetState` (PTR_00c96088) contains + - PTR_00c96088 is a **terrain chunk list**, NOT a WDOODADDEF entity list + - Initialized by `InitializeDataPointers` (0x6916e0): offset=0xC, sentinel pattern + - `LoadWorldTerrainChunk` writes to PTR_00c96084 (the list head) + - Map doodads are attached to parent chunks via `ModelAttachment_CreateNode` in `AttachDoodadObjects` + - `destroyPrimaryGameObject` (0x6a69f0) destroys the parent chunk, which walks attachment children + - We CANNOT participate in this list — it's for terrain chunks, not standalone entities +- [x] Determine proper entity lifecycle for persistent world objects + - **There is no native path for standalone persistent WDOODADDEF entities** + - All native callers either: (a) attach to parent chunks, or (b) explicitly clean up before teardown + - Correct approach: explicit cleanup via `CleanupEntity_ProcessAttachments` before teardown + - Hook point: `CleanupWorldAndEntities` (0x66fc40) PRE-hook — fires for exit, logout, AND map change + +## Entity Lifecycle Solution (Confirmed) + +### Root Cause (Fully Traced) +The WDOODADDEF hash table at 0xCA7DC0 has an atexit handler (`hashTableTeardownLoop` at 0x69f740) +that iterates ALL entries via the global list (PTR_00ca7dcc) and every hash bucket. By the time this +runs, `ShutdownAllGameSystems` has already destroyed the WDOODADDEF heap. Accessing any entity +still in the hash table hits freed memory → ACCESS_VIOLATION at 0x687220. + +### How Native Code Avoids This +1. **Map doodads** (`FindOrCreateWorldUnit` via `AttachDoodadObjects`): + - Attached to parent terrain chunks via `ModelAttachment_CreateNode` + - Parent chunks are in PTR_00c96088 (terrain chunk list) + - `ClearWorldObjectsAndResetState` iterates chunks → `destroyPrimaryGameObject` → walks attachments + - Each doodad's `CleanupVisualEffectAndRelease` unlinks from hash table + - Hash table is empty before heap destruction + +2. **Spell effects** (`CastSpellByID_Extended` → `CreateEntityInstance_WithAttachment`): + - Stored in PTR_00ceca8c (single global pointer) + - `processCinematicExit` (0x6e4940) called before every new spell cast + - Explicitly calls `CleanupEntity_ProcessAttachments(PTR_00ceca8c); PTR_00ceca8c = NULL;` + - Spell entities NEVER survive to teardown + +3. **Our markers** (standalone WDOODADDEF, no parent, no tracking): + - Created via `CreateEntityInstance_WithAttachment` → `CreateWorldUnit` + - Registered in hash table only (bucket + global list) + - NOT attached to any parent chunk, NOT tracked in any game-managed cleanup list + - Must be explicitly cleaned up before `CleanupWorldAndReleaseResources` runs + +### Correct Fix: Pre-hook on CleanupWorldAndEntities +Hook `CleanupWorldAndEntities` (0x66fc40). Before calling the original: +1. Call `CleanupEntity_ProcessAttachments` on all active marker entities +2. Call `CleanupEntity_ProcessAttachments` on all despawning entities +3. Null all entity pointers / reset state +4. Call original — hash table no longer contains our entries → no crash + +This handles ALL scenarios: game exit, logout, map change. + +### Key Decompilations + +#### destroyPrimaryGameObject (0x6a69f0) +```c +void __fastcall destroyPrimaryGameObject(undefined **param_1) { + cleanupPrimaryResources((int)param_1); + UnlinkAndFreeNode(param_1); +} +``` + +#### processCinematicExit (0x6e4940) — Spell Entity Cleanup +```c +// After handling cinematic/targeting state... +if (PTR_00ceca8c != NULL) { + CleanupEntity_ProcessAttachments(PTR_00ceca8c); + PTR_00ceca8c = NULL; +} +``` + +#### CreateEntityInstance_WithAttachment (0x6707c0) — M2 Path +```c +int * __fastcall CreateEntityInstance_WithAttachment( + char *modelPath, float *pos, float facing, int flags, int updateNow, int p6, int p7) { + // M2 path (no ".wmo" in path): + positionData = CreateWorldUnit(modelPath, pos, facing, flags); + positionData[0x61] = p7; + positionData[0x60] = p6; + positionData[0x24] |= 0x2000; + if (updateNow != 0) { + UpdateWorldPosition(positionData, pos, facing); + SetUnitPositionAndOrientation(positionData, pos, facing); + } + *(short *)(positionData + 0x0E) += 1; // refcount: 0 -> 1 + return positionData; +} +``` + +#### hashTableTeardownLoop (0x69f740) — The Crash Site +```c +void hashTableTeardownLoop(void) { + if ((DAT_00ca7cf0 & 1) == 0) { + DAT_00ca7cf0 |= 1; + _DAT_00ca7dc0 = &PTR_DestroyMapDoodadDefinition_0081089c; + // Walk global list — crashes here if entries point to freed heap + while (PTR_00ca7dcc is valid) { + piVar2 = ValidateLinkedList(&PTR_00ca7dc4, PTR_00ca7dcc); + CalculateDistance3D(piVar2); // reads from freed entity memory + } + // Walk each hash bucket — also crashes + for each bucket in PTR_00ca7ddc { + while (bucket entry is valid) { + piVar2 = ValidateLinkedList(bucket, entry); + CalculateDistance3D(piVar2); + } + } + // Clear buckets, free bucket array, unlink remaining global entries + } +} +``` + +#### FindOrCreateWorldUnit (0x694e90) vs CreateWorldUnit (0x694980) +Key difference: `FindOrCreateWorldUnit` additionally registers in the third list +(PTR_00c89f08/PTR_00c89f0c via entity[4]/[5]), while `CreateWorldUnit` only registers +in the hash bucket and global lists. Both are cleaned up through `CleanupVisualEffectAndRelease` +which handles all three list types. + +#### ClearWorldObjectsAndResetState (0x6a6710) +```c +void ClearWorldObjectsAndResetState(void) { + // Iterates PTR_00c96088 (terrain chunk list, NOT entity list) + for each chunk in list { + ppuVar1 = chunk[1]; // parent object + // Clear lookup tables indexed by ppuVar1[0x26] + ComplexMemoryCleanupAndRelease(chunk); // free the list node + destroyPrimaryGameObject(ppuVar1); // destroy parent + attachments + } +} +``` + +### Complete Entity Lifecycle Audit (All Native Callers) + +**Every native M2 entity created via `CreateEntityInstance_WithAttachment` is explicitly cleaned up +by a parent.** The atexit handler (hashTableTeardownLoop) is a safety net for the hash table data +structure — it should NEVER encounter live entities in normal operation. + +#### All 3 callers of CreateEntityInstance_WithAttachment: + +1. **CastSpellByID_Extended (0x6e4b60)** — spell targeting reticle + - Stores entity in global `PTR_00ceca8c` + - Cleaned up by `processCinematicExit` before every new spell cast + - Also cleaned up by `executeSpellOrItem` (0x6e54f0) + +2. **CreateGameObjectPathEffect (0x5f8030)** — game object visual effect + - Called by `CreatePathObjectByUnitType` (0x5f4970) + - Return value saved at parent+0x10 (despite Ghidra typing it as void) + - Cleaned up by `DestroyPathObjectIfPresent` (0x5f4950) → `CleanupEntity_ProcessAttachments` + - Parent is a spell effect object (SpellEffectWithPath class at ~0x5f4800) + - `SpellEffectWithPathDestructor` (0x5f48d0) destroys parent during spell teardown + +3. **Unknown caller (0x6e5a6e)** — likely in `executeSpellOrItem` (0x6e54f0) + - Same pattern as #1 + +#### All 3 callers of CleanupEntity_ProcessAttachments (0x670d50): +- `processCinematicExit` (0x6e4940) — spell cleanup +- `executeSpellOrItem` (0x6e54f0) — spell/item cleanup +- `DestroyPathObjectIfPresent` (0x5f4950) — path effect cleanup + +#### CleanupVisualEffectAndRelease has only ONE caller: +- `cleanupGameObject` (0x6a67a0) — the M2/WDOODADDEF cleanup function + +So the full cleanup chain is always: +``` +Parent destroyed + → CleanupEntity_ProcessAttachments (0x670d50) + → cleanupGameObject (0x6a67a0) [entity flags & 0x40] + → CleanupVisualEffectAndRelease (0x6a0840) + → unlink entity[4]/[5] from third list + → if entity[0x2F] != 0: unlink entity[0x2E-0x31] from hash bucket + global list + → call vtable destructor + → ReleaseToHeap(WDOODADDEF, entity) +``` + +### Cleanup Lists Summary (what gets iterated during CleanupWorldAndReleaseResources) +| List | Offset | Contains | Cleanup Function | Heap | +|------|--------|----------|-----------------|------| +| PTR_00c96088 | 0xC | Terrain chunk wrappers → doodad parents | destroyPrimaryGameObject | various | +| PTR_00c9e358 | via PTR_00c9e350 | WMO entity wrappers (from CreateEntityInstance_WithAttachment WMO path) | destroyWorldEnvironment | WMAPOBJDEF | +| PTR_00c962bc | 0xC | **Empty in 1.12.1** — no insertion code found, only init+atexit | cleanupGameObject (conditional) | WDOODADDEF | +| PTR_00ca8044 | via PTR_00ca803c | Map manager objects | CleanupAndReleaseMemoryBlock | PTR_00ca7e10 (4th heap) | +| PTR_00c7b2dc | 0xC | WENTITY fade effect wrappers | CleanupObjectAttachments_FreeMemory → DestroyWorldObjectAndRelease | WENTITY | + +**None of these lists iterate the WDOODADDEF hash table.** The hash table is ONLY iterated by the +atexit handler. All native entities are removed from the hash table via explicit +`CleanupEntity_ProcessAttachments` calls from their parents BEFORE heap destruction. + +### Refcount Verification +- `AllocateRenderableObject` zeroes the entire struct → entity+0x0E = 0 +- `CreateWorldUnit` sets entity[3] lower 2 bytes to 1 (byte offset 0x0C, NOT 0x0E) → entity+0x0E still = 0 +- `CreateEntityInstance_WithAttachment` increments: `*(short*)(entity+0x0E) += 1` → entity+0x0E = 1 +- `CleanupEntity_ProcessAttachments` decrements: `*(short*)(entity+0x0E) -= 1` → entity+0x0E = 0 → proceeds to destroy +- Single call to CleanupEntity_ProcessAttachments is sufficient (refcount goes 1→0) + +### Our Creation vs Native Spell Path +| Parameter | Our markers | CastSpellByID_Extended | +|-----------|------------|----------------------| +| path | "Spells\\Raid_UI_FX_*.m2" | game object model path | +| pos | world position | caster position | +| facing | 0.0 | caster facing | +| flags | 0 | 0 | +| updateNow | **1** | **0** | +| p6 | 0 | 0 (or GUID low) | +| p7 | 0 | 0 (or GUID high) | +| post-create | (none) | `SetEntityFlag_ToggleBit(entity, 0)` = entity[0xD] \|= 1 | + +Difference: we pass `updateNow=1` (which calls `UpdateWorldPosition` + `SetUnitPositionAndOrientation`), +spell caster passes `updateNow=0`. Spell caster also calls `SetEntityFlag_ToggleBit` which sets +`entity[0xD] |= 1` (byte offset 0x34, flag byte). Neither difference should affect cleanup. diff --git a/src/markers/addon/Markers.lua b/src/markers/addon/Markers.lua index 3a5649b..0abf8de 100644 --- a/src/markers/addon/Markers.lua +++ b/src/markers/addon/Markers.lua @@ -14,6 +14,13 @@ frame:SetScript("OnEvent", function() end end) +-- Per-frame animation driver: queues Hold after Stand finishes on new +-- markers and re-queues it periodically so it never falls back to Stand. +local animFrame = CreateFrame("Frame") +animFrame:SetScript("OnUpdate", function() + ProcessMarkerAnimations() +end) + SLASH_MARKERS1 = "/markers" SLASH_MARKERS2 = "/mark" SlashCmdList["MARKERS"] = function(msg) diff --git a/src/markers/markers.zig b/src/markers/markers.zig index 62596b9..d70f4f5 100644 --- a/src/markers/markers.zig +++ b/src/markers/markers.zig @@ -28,6 +28,11 @@ const ERROR_ALREADY_EXISTS: u32 = 183; var g_mutex: ?*anyopaque = null; var g_is_hook_owner: bool = false; +/// True if this DLL instance owns the markers hooks and Lua API is safe to use. +pub fn isActive() bool { + return g_is_hook_owner; +} + // ============================================================================= // Constants // ============================================================================= @@ -83,6 +88,7 @@ var despawning: [MAX_DESPAWNING]?DespawningEntity = .{null} ** MAX_DESPAWNING; // ============================================================================= const fc = std.builtin.CallingConvention{ .x86_fastcall = .{} }; +const sc = std.builtin.CallingConvention{ .x86_stdcall = .{} }; const lapi = struct { fn gettop(L: u32) i32 { @@ -461,6 +467,58 @@ pub fn luaGetPlayerPosition(L: u32) callconv(.c) u32 { return 3; } +// ============================================================================= +// World teardown hook +// ============================================================================= + +var world_cleanup_hook: hook.Detour(fn () callconv(sc) void) = .{}; + +/// Pre-hook on CleanupWorldAndEntities (0x66fc40). +/// Destroys all our entities via CleanupEntity_ProcessAttachments before the +/// game's teardown runs — the same pattern every native caller uses (e.g. +/// processCinematicExit, DestroyPathObjectIfPresent). This unlinks them from +/// the WDOODADDEF hash table so the atexit handler never touches freed memory. +fn worldCleanupDetour() callconv(sc) void { + con.print("[markers] >>> worldCleanupDetour FIRING <<<\n"); + destroyAllEntities(); + world_cleanup_hook.callOriginal(.{}); +} + +/// Destroy all tracked entities (active markers + despawning). +/// Idempotent — safe to call multiple times. +fn destroyAllEntities() void { + var count: u32 = 0; + for (&marker_entities, 0..) |*slot, i| { + if (slot.*) |existing| { + const addr = @intFromPtr(existing); + const flags = hook.readMem(u32, addr + 0x8); + const refcount = hook.readMem(u16, addr + 0x0E); + con.fmt("[markers] destroying marker[{d}] @0x{x} flags=0x{x} refcount={d}\n", .{ i, addr, flags, refcount }); + cleanupEntity(existing); + slot.* = null; + count += 1; + } + } + for (&hold_queued) |*h| h.* = false; + for (&marker_created_tick) |*t| t.* = 0; + + for (&despawning, 0..) |*slot, i| { + if (slot.*) |d| { + const addr = @intFromPtr(d.entity); + const flags = hook.readMem(u32, addr + 0x8); + const refcount = hook.readMem(u16, addr + 0x0E); + con.fmt("[markers] destroying despawn[{d}] @0x{x} flags=0x{x} refcount={d}\n", .{ i, addr, flags, refcount }); + cleanupEntity(d.entity); + slot.* = null; + count += 1; + } + } + + if (count > 0) { + con.fmt("[markers] world cleanup: destroyed {d} entities\n", .{count}); + } +} + // ============================================================================= // Install hooks // ============================================================================= @@ -483,18 +541,23 @@ pub fn installHooks() void { return; } g_is_hook_owner = true; + + // Hook CleanupWorldAndEntities to destroy our entities before world teardown. + // This fires on map change, logout, AND exit — before heaps are destroyed. + const hook_result = world_cleanup_hook.attach(o.FN_CLEANUP_WORLD_AND_ENTITIES, &worldCleanupDetour); + if (hook_result != .ok) { + con.print("[markers] FAILED to hook CleanupWorldAndEntities!\n"); + } else { + con.print("[markers] hooked CleanupWorldAndEntities OK\n"); + } } pub fn removeHooks() void { if (g_is_hook_owner) { - // Force-cleanup: no time for animations during shutdown - for (&marker_entities) |*slot| { - if (slot.*) |existing| { - cleanupEntity(existing); - slot.* = null; - } - } - forceCleanupDespawning(); + // destroyAllEntities is idempotent — if worldCleanupDetour already ran, + // all slots are null and this is a no-op. + destroyAllEntities(); + world_cleanup_hook.detach(); } if (g_is_hook_owner) { diff --git a/src/markers/offsets.zig b/src/markers/offsets.zig index 67fb6bd..c2107f3 100644 --- a/src/markers/offsets.zig +++ b/src/markers/offsets.zig @@ -33,6 +33,17 @@ pub const MOVEMENT_POS_Z: usize = 0x18; /// Increments refcount at entity+0x0E. pub const FN_CREATE_ENTITY_INSTANCE: usize = 0x006707c0; +// ============================================================================= +// World teardown (map unload / logout / exit) +// ============================================================================= + +/// CleanupWorldAndEntities — void(), no params, __stdcall. +/// Top-level world teardown: calls CleanupEntityList_ProcessAll, then +/// CleanupWorldAndReleaseResources (which iterates heaps and force-frees). +/// Called from InitializeWorldScene (map change) and ShutdownClientSystems (exit). +/// Hook this to destroy custom entities BEFORE the game's teardown begins. +pub const FN_CLEANUP_WORLD_AND_ENTITIES: usize = 0x0066fc40; + // ============================================================================= // World object lifecycle // ============================================================================= diff --git a/src/outline/api.zig b/src/outline/api.zig index ef6337c..7e69f63 100644 --- a/src/outline/api.zig +++ b/src/outline/api.zig @@ -4,7 +4,7 @@ //! and a Lua C callback for `/wu outline` commands. const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const con = @import("../console.zig"); const tracker = @import("tracker.zig"); const model_hook = @import("model_hook.zig"); diff --git a/src/outline/d3d9_hook.zig b/src/outline/d3d9_hook.zig index 2aa3c38..2d3759a 100644 --- a/src/outline/d3d9_hook.zig +++ b/src/outline/d3d9_hook.zig @@ -14,7 +14,7 @@ //! occluded by world/WMO/game objects but show through other players. const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const types = @import("types.zig"); const tracker = @import("tracker.zig"); const model_hook = @import("model_hook.zig"); diff --git a/src/outline/model_hook.zig b/src/outline/model_hook.zig index be1b209..704addb 100644 --- a/src/outline/model_hook.zig +++ b/src/outline/model_hook.zig @@ -13,7 +13,7 @@ //! implementation function. const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const api = @import("api.zig"); const o = @import("offsets.zig"); const types = @import("types.zig"); @@ -25,15 +25,19 @@ const wow = @import("wow.zig"); // Calling convention constants // ============================================================================= -const THISCALL = std.builtin.CallingConvention{ .x86_thiscall = .{} }; +const tc: std.builtin.CallingConvention = .{ .x86_thiscall = .{} }; // ============================================================================= // Hook state // ============================================================================= -var render_draw_hook: hook.Hook = .{}; -var manage_render_hook: hook.Hook = .{}; -var draw_batch_hook: hook.Hook = .{}; +const RenderDrawFn = fn (u32, u32, u32, u32, u32) callconv(tc) void; +const ManageRenderFn = fn (u32, u32) callconv(tc) void; +const DrawBatchFn = fn (u32) callconv(tc) void; + +var render_draw_hook: hook.Detour(RenderDrawFn) = .{}; +var manage_render_hook: hook.Detour(ManageRenderFn) = .{}; +var draw_batch_hook: hook.Detour(DrawBatchFn) = .{}; /// D3D9 hooks are deferred until the first model hook fires, because creating /// a dummy D3D9 device during engine init corrupts the proxy's state. @@ -71,7 +75,7 @@ var reordered_indices: [MAX_REORDER]i32 = undefined; // through other players and gear (since those aren't in depth when stencil // is written). The outline composites on top of everything in EndScene. -fn renderDrawDetour(this: u32, view_matrix: u32, batch_data: u32, batch_indices: u32, batch_count: u32) callconv(THISCALL) void { +fn renderDrawDetour(this: u32, view_matrix: u32, batch_data: u32, batch_indices: u32, batch_count: u32) callconv(tc) void { // One-time: install D3D9 hooks now that the game is actively rendering. if (d3d9_deferred_pending) { d3d9_deferred_pending = false; @@ -80,7 +84,7 @@ fn renderDrawDetour(this: u32, view_matrix: u32, batch_data: u32, batch_indices: // Skip reordering if nothing to outline or too many batches if (!tracker.enabled or !tracker.hasTargets() or batch_count == 0 or batch_count > MAX_REORDER) { - callOrigRenderDraw(this, view_matrix, batch_data, batch_indices, batch_count); + render_draw_hook.callOriginal(.{ this, view_matrix, batch_data, batch_indices, batch_count }); return; } @@ -102,7 +106,7 @@ fn renderDrawDetour(this: u32, view_matrix: u32, batch_data: u32, batch_indices: } if (outline_count == 0) { - callOrigRenderDraw(this, view_matrix, batch_data, batch_indices, batch_count); + render_draw_hook.callOriginal(.{ this, view_matrix, batch_data, batch_indices, batch_count }); return; } @@ -134,26 +138,7 @@ fn renderDrawDetour(this: u32, view_matrix: u32, batch_data: u32, batch_indices: indices[i] = reordered_indices[i]; } - callOrigRenderDraw(this, view_matrix, batch_data, batch_indices, batch_count); -} - -fn callOrigRenderDraw(this: u32, view_matrix: u32, batch_data: u32, batch_indices: u32, batch_count: u32) void { - // __thiscall: ECX = this, stack = viewMatrix, batchData, batchIndices, batchCount - // Callee cleans 16 bytes (4 stack params). - // Pack args into a struct so we only need one "r" register to address them. - const args = [4]u32{ view_matrix, batch_data, batch_indices, batch_count }; - asm volatile ( - \\push 12(%[a]) - \\push 8(%[a]) - \\push 4(%[a]) - \\push (%[a]) - \\call *%[func] - : - : [_] "{ecx}" (this), - [a] "r" (&args), - [func] "r" (render_draw_hook.trampoline), - : .{ .eax = true, .edx = true, .memory = true, .cc = true } - ); + render_draw_hook.callOriginal(.{ this, view_matrix, batch_data, batch_indices, batch_count }); } // ============================================================================= @@ -162,22 +147,13 @@ fn callOrigRenderDraw(this: u32, view_matrix: u32, batch_data: u32, batch_indice // __thiscall(model_ECX, addToList_stack) // Native thiscall detour — no thunk needed. -fn manageRenderDetour(model: u32, add_to_list: u32) callconv(THISCALL) void { +fn manageRenderDetour(model: u32, add_to_list: u32) callconv(tc) void { // Classify the model when it's being ADDED to the render list if (add_to_list == 1 and model != 0 and tracker.enabled and tracker.hasTargets()) { tracker.classifyModel(model); } - // Call original: __thiscall(model_ECX, addToList_stack) - asm volatile ( - \\push %[add] - \\call *%[func] - : - : [_] "{ecx}" (model), - [add] "r" (add_to_list), - [func] "r" (manage_render_hook.trampoline), - : .{ .eax = true, .edx = true, .memory = true, .cc = true } - ); + manage_render_hook.callOriginal(.{ model, add_to_list }); } // ============================================================================= @@ -189,26 +165,10 @@ fn manageRenderDetour(model: u32, add_to_list: u32) callconv(THISCALL) void { // wrong ret instructions for functions with ≤2 register params. The naked // function bridges fastcall → cdecl and calls the implementation function. -fn drawBatchProjEntry() callconv(.naked) void { - // __fastcall(ECX): ECX = render context, 0 stack args. - // Bridge to cdecl: push edx + ecx as args, call impl, cleanup, ret. - asm volatile ( - \\push %%edx - \\push %%ecx - \\call *%%eax - \\add $8, %%esp - \\ret - : - : [_] "{eax}" (@intFromPtr(&drawBatchProjImpl)) - ); -} - -fn drawBatchProjImpl(ctx: u32, _edx: u32) callconv(.c) void { - _ = _edx; - +fn drawBatchProjDetour(ctx: u32) callconv(tc) void { // Fast path: no tracking enabled or nothing tracked → just call original if (!tracker.enabled or !tracker.hasTargets()) { - callOrigDrawBatch(ctx); + draw_batch_hook.callOriginal(.{ctx}); return; } @@ -224,60 +184,34 @@ fn drawBatchProjImpl(ctx: u32, _edx: u32) callconv(.c) void { rendering_outline = true; current_model = model_ptr; - callOrigDrawBatch(ctx); + draw_batch_hook.callOriginal(.{ctx}); rendering_outline = false; current_model = 0; } else { - // Normal rendering — no special handling needed - callOrigDrawBatch(ctx); + draw_batch_hook.callOriginal(.{ctx}); } } -fn callOrigDrawBatch(ctx: u32) void { - asm volatile ("call *%[func]" - : - : [_] "{ecx}" (ctx), - [func] "r" (draw_batch_hook.trampoline), - : .{ .eax = true, .edx = true, .memory = true, .cc = true } - ); -} - // ============================================================================= // Install / Remove // ============================================================================= pub fn installHooks() bool { - // CM2SceneRenderDraw — native thiscall detour, no thunk needed. - // Prologue is 9 bytes: PUSH EBP (1) + MOV EBP,ESP (2) + SUB ESP,0x80 (6). - if (!render_draw_hook.install( - o.FN_CM2SCENE_RENDER_DRAW, - 9, - @intFromPtr(&renderDrawDetour), - &.{}, - )) return false; + if (render_draw_hook.attach(o.FN_CM2SCENE_RENDER_DRAW, &renderDrawDetour) != .ok) + return false; - // ManageRenderListNode — native thiscall detour, no thunk needed. - if (!manage_render_hook.install( - o.FN_CM2MODEL_MANAGE_RENDER_LIST, - 6, - @intFromPtr(&manageRenderDetour), - &.{}, - )) return false; + if (manage_render_hook.attach(o.FN_CM2MODEL_MANAGE_RENDER_LIST, &manageRenderDetour) != .ok) + return false; - // DrawBatchProj — naked entry bridges fastcall → cdecl, no thunk needed. - if (!draw_batch_hook.install( - o.FN_DRAW_BATCH_PROJ, - 6, - @intFromPtr(&drawBatchProjEntry), - &.{}, - )) return false; + if (draw_batch_hook.attach(o.FN_DRAW_BATCH_PROJ, &drawBatchProjDetour) != .ok) + return false; return true; } pub fn removeHooks() void { - draw_batch_hook.remove(); - manage_render_hook.remove(); - render_draw_hook.remove(); + draw_batch_hook.detach(); + manage_render_hook.detach(); + render_draw_hook.detach(); } diff --git a/src/outline/tracker.zig b/src/outline/tracker.zig index efa4171..a86a381 100644 --- a/src/outline/tracker.zig +++ b/src/outline/tracker.zig @@ -11,7 +11,7 @@ //! comparison against the object manager's validated set. const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const wow = @import("wow.zig"); const o = @import("offsets.zig"); const types = @import("types.zig"); diff --git a/src/outline/wow.zig b/src/outline/wow.zig index d7f6632..a4d8c1c 100644 --- a/src/outline/wow.zig +++ b/src/outline/wow.zig @@ -5,7 +5,7 @@ //! game functions (UnitGUID, GetObjectByGUID, UnitReaction). const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const o = @import("offsets.zig"); const types = @import("types.zig"); diff --git a/src/screenshot/screenshot.zig b/src/screenshot/screenshot.zig index 0fe3a9b..ae0f207 100644 --- a/src/screenshot/screenshot.zig +++ b/src/screenshot/screenshot.zig @@ -1,5 +1,5 @@ const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const con = @import("../console.zig"); const png = @import("png.zig"); @@ -55,7 +55,9 @@ const ERROR_ALREADY_EXISTS: u32 = 183; var enabled: bool = true; var compression_level: i32 = 6; // user-facing 0–9, kept for Lua interface -var tga_hook: hook.Hook = .{}; +const tc: std.builtin.CallingConvention = .{ .x86_thiscall = .{} }; +const TgaWriteFn = fn (u32, u32) callconv(tc) i32; +var tga_hook: hook.Detour(TgaWriteFn) = .{}; var screenshot_dir: [260]u8 = undefined; var screenshot_dir_len: usize = 0; var screenshot_counter: u8 = 0; @@ -79,7 +81,7 @@ var queue: [MAX_PENDING]PendingScreenshot = undefined; var queue_head: usize = 0; var queue_tail: usize = 0; var queue_count: usize = 0; -var mutex: std.Thread.Mutex = .{}; +var mutex: std.atomic.Mutex = .unlocked; var worker_running: bool = false; var g_mutex: ?HANDLE = null; var g_is_hook_owner: bool = false; @@ -122,15 +124,7 @@ fn extractDir(filename_ptr: u32) void { // ============================================================================= fn callOriginal(self: u32, filename: u32) i32 { - return asm volatile ( - \\push %[filename] - \\call *%[func] - : [ret] "={eax}" (-> i32), - : [_] "{ecx}" (self), - [filename] "r" (filename), - [func] "r" (tga_hook.trampoline), - : .{ .edx = true, .memory = true, .cc = true } - ); + return tga_hook.callOriginal(.{ self, filename }); } // ============================================================================= @@ -138,8 +132,7 @@ fn callOriginal(self: u32, filename: u32) i32 { // Thunked from __fastcall(self_ECX, _EDX, filename_stack) → cdecl // ============================================================================= -fn tgaWriteDetour(self: u32, _edx: u32, filename: u32) callconv(.c) i32 { - _ = _edx; +fn tgaWriteDetour(self: u32, filename: u32) callconv(tc) i32 { if (!enabled) return callOriginal(self, filename); @@ -167,7 +160,7 @@ fn tgaWriteDetour(self: u32, _edx: u32, filename: u32) callconv(.c) i32 { @memcpy(buffer, src[0..size]); // Enqueue for async processing - mutex.lock(); + while (!mutex.tryLock()) {} defer mutex.unlock(); if (!enqueue(.{ .buffer = buffer.ptr, .width = width, .height = height, .size = size, .level = png.mapLevel(compression_level) })) { @@ -196,7 +189,7 @@ fn workerThread() void { while (true) { var shot: PendingScreenshot = undefined; { - mutex.lock(); + while (!mutex.tryLock()) {} defer mutex.unlock(); if (dequeue()) |s| { shot = s; @@ -368,24 +361,19 @@ pub fn installHook() void { g_is_hook_owner = true; // CTgaFile::Write at 0x5a4810 - // __thiscall(self, filename) — prologue: 55 8B EC 83 EC 08 = 6 bytes, no fixups - // Thunk: fastcall(ECX=self, EDX, stack: filename) → cdecl(self, edx, filename) + // __thiscall(self, filename) ret 4 // // Another DLL (UnitXP_SP3) hooks this same address during DLL_PROCESS_ATTACH, // replacing the prologue with an E9 JMP. Restore the original prologue first // so prepare() builds a trampoline to the real function rather than chaining // through UnitXP's detour. hook.writeProtected(0x5a4810, &.{ 0x55, 0x8B, 0xEC, 0x83, 0xEC, 0x08 }); - if (tga_hook.prepare(0x5a4810, 6, &.{})) { - const thunk = tga_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(thunk, @intFromPtr(&tgaWriteDetour), 1); - tga_hook.activate(@intFromPtr(thunk)); - } + _ = tga_hook.attach(0x5a4810, &tgaWriteDetour); } pub fn removeHook() void { if (g_is_hook_owner) { - tga_hook.remove(); + tga_hook.detach(); } if (g_is_hook_owner) { diff --git a/src/transmogfix/transmogfix.zig b/src/transmogfix/transmogfix.zig index 3031215..c55003a 100644 --- a/src/transmogfix/transmogfix.zig +++ b/src/transmogfix/transmogfix.zig @@ -15,7 +15,7 @@ // ============================================================================= const std = @import("std"); -const hook = @import("hook"); +const hook = @import("zhook"); const con = @import("../console.zig"); const WINAPI = std.builtin.CallingConvention.winapi; @@ -165,9 +165,14 @@ var g_mutex: ?*anyopaque = null; // Hooks // ============================================================================= -var set_block_hook = hook.Hook{}; -var refresh_hook = hook.Hook{}; -var scene_end_hook = hook.Hook{}; +const tc: std.builtin.CallingConvention = .{ .x86_thiscall = .{} }; +const SetBlockFn = fn (u32, u32, u32) callconv(tc) u32; +const RefreshFn = fn (u32, u32, u32, u32) callconv(tc) void; +const SceneEndFn = fn (u32) callconv(tc) void; + +var set_block_hook: hook.Detour(SetBlockFn) = .{}; +var refresh_hook: hook.Detour(RefreshFn) = .{}; +var scene_end_hook: hook.Detour(SceneEndFn) = .{}; // ============================================================================= // Object manager helpers @@ -358,55 +363,15 @@ fn findOtherPendingEntry(guid: u64, slot: i32) i32 { // ============================================================================= fn callOriginalSetBlock(obj: u32, index: u32, value: u32) u32 { - // Save/restore ECX around the call: the callee overwrites ECX internally - // (SetBlock does `mov ecx, [ebp+0xC]`), but we can't list ECX as a clobber - // since it's already an input constraint. Without the save/restore, the - // compiler may reuse the now-stale ECX for `obj` on a subsequent call. - // The trampoline's `ret 8` cleans up the pushed index+value, leaving our - // saved ECX on top for the pop. - return asm volatile ( - \\push %%ecx - \\push %[value] - \\push %[index] - \\call *%[func] - \\pop %%ecx - : [ret] "={eax}" (-> u32), - : [_] "{ecx}" (obj), - [index] "r" (index), - [value] "r" (value), - [func] "r" (set_block_hook.trampoline), - : .{ .edx = true, .memory = true, .cc = true } - ); + return set_block_hook.callOriginal(.{ obj, index, value }); } fn callOriginalRefresh(unit: u32, event_data: u32, extra_data: u32, force_update: u32) void { - // __thiscall: ECX=this, stack args right-to-left. EDX is caller-saved scratch - // (confirmed via Ghidra: __thiscall, EDX not part of calling convention). - // Pin force_update to EDX to stay within 3 "r" registers (EBX/ESI/EDI) - // since EBP is the frame pointer on x86. - asm volatile ( - \\push %[force] - \\push %[extra] - \\push %[event] - \\call *%[func] - : - : [_] "{ecx}" (unit), - [force] "{edx}" (force_update), - [event] "r" (event_data), - [extra] "r" (extra_data), - [func] "r" (refresh_hook.trampoline), - : .{ .eax = true, .memory = true, .cc = true } - ); + refresh_hook.callOriginal(.{ unit, event_data, extra_data, force_update }); } fn callOriginalSceneEnd(device: u32) void { - asm volatile ( - \\call *%[func] - : - : [_] "{ecx}" (device), - [func] "r" (scene_end_hook.trampoline), - : .{ .eax = true, .edx = true, .memory = true, .cc = true } - ); + scene_end_hook.callOriginal(.{device}); } // ============================================================================= @@ -429,7 +394,7 @@ fn processTimeouts(now: u32) void { } // OTHER PLAYERS: Use timeout since we don't have their INV_SLOT info - if (g_other_pending_count > 0 and set_block_hook.trampoline != 0) { + if (g_other_pending_count > 0 and set_block_hook.inner.trampoline != 0) { const UnitSlots = struct { unit: u32 = 0, slots: [19]i32 = .{0} ** 19, @@ -507,7 +472,7 @@ fn processTimeouts(now: u32) void { // Fallback to RefreshVisualAppearance con.fmt("[other] REFRESH fallback unit=0x{X:0>8} table=0x{X:0>8}\n", .{ unit, display_table }); - if (refresh_hook.trampoline != 0) { + if (refresh_hook.inner.trampoline != 0) { callOriginalRefresh(unit, 0, 0, 1); } } @@ -518,8 +483,7 @@ fn processTimeouts(now: u32) void { // Hook 1: SetBlock (0x6142E0) // ============================================================================= -fn hookSetBlock(obj: u32, _edx: u32, index: u32, value: u32) callconv(.c) u32 { - _ = _edx; +fn hookSetBlock(obj: u32, index: u32, value: u32) callconv(tc) u32 { const val = value; // VISIBLE_ITEM writes @@ -694,11 +658,9 @@ fn hookSetBlock(obj: u32, _edx: u32, index: u32, value: u32) callconv(.c) u32 { // Hook 2: RefreshVisualAppearance (0x5fb880) // ============================================================================= -fn hookRefreshVisualAppearance(unit: u32, _edx: u32, event_data: u32, extra_data: u32, force_update: u32) callconv(.c) void { - _ = _edx; - - if (!g_enabled or refresh_hook.trampoline == 0) { - if (refresh_hook.trampoline != 0) { +fn hookRefreshVisualAppearance(unit: u32, event_data: u32, extra_data: u32, force_update: u32) callconv(tc) void { + if (!g_enabled or refresh_hook.inner.trampoline == 0) { + if (refresh_hook.inner.trampoline != 0) { callOriginalRefresh(unit, event_data, extra_data, force_update); } return; @@ -802,8 +764,7 @@ fn hookRefreshVisualAppearance(unit: u32, _edx: u32, event_data: u32, extra_data // Hook 3: SceneEnd (0x5a17a0) // ============================================================================= -fn hookSceneEnd(device: u32, _edx: u32) callconv(.c) void { - _ = _edx; +fn hookSceneEnd(device: u32) callconv(tc) void { if (g_enabled and (g_local_pending_count > 0 or g_other_pending_count > 0)) { processTimeouts(GetTickCount()); @@ -816,16 +777,16 @@ fn hookSceneEnd(device: u32, _edx: u32) callconv(.c) void { // Init / Cleanup // ============================================================================= -pub fn installHooks() bool { +pub fn installHooks() void { con.print("[transmogfix] Module loaded\n"); // Multi-DLL safety: only one instance per process should hook var mutex_name_buf: [64]u8 = undefined; - const mutex_name = std.fmt.bufPrint(&mutex_name_buf, "Local\\TransmogCoalesceHook_{d}", .{GetCurrentProcessId()}) catch return false; + const mutex_name = std.fmt.bufPrint(&mutex_name_buf, "Local\\TransmogCoalesceHook_{d}", .{GetCurrentProcessId()}) catch return; mutex_name_buf[mutex_name.len] = 0; g_mutex = CreateMutexA(null, 1, @ptrCast(mutex_name_buf[0..mutex_name.len :0])); - if (g_mutex == null) return false; + if (g_mutex == null) return; if (GetLastError() == ERROR_ALREADY_EXISTS) { _ = CloseHandle(g_mutex.?); @@ -833,7 +794,7 @@ pub fn installHooks() bool { g_is_hook_owner = false; g_initialized = true; con.print("[transmogfix] Another DLL owns hooks (mutex taken), skipping\n"); - return true; // Success but not owner — no hooks + return; } g_is_hook_owner = true; @@ -846,41 +807,31 @@ pub fn installHooks() bool { g_unit_cache = [1]UnitVisualState{.{}} ** UNIT_CACHE_SIZE; g_cached_visible_item = .{0} ** 19; - // Hook 1: SetBlock (6 bytes, no fixups) - if (!set_block_hook.prepare(ADDR_SetBlock, 6, &.{})) return false; - const sb_thunk = set_block_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(sb_thunk, @intFromPtr(&hookSetBlock), 2); - set_block_hook.activate(@intFromPtr(sb_thunk)); + // Hook 1: SetBlock + if (set_block_hook.attach(ADDR_SetBlock, &hookSetBlock) != .ok) return; - // Hook 2: RefreshVisualAppearance (6 bytes, no fixups) - if (!refresh_hook.prepare(ADDR_RefreshVisualAppearance, 6, &.{})) { - set_block_hook.remove(); - return false; + // Hook 2: RefreshVisualAppearance + if (refresh_hook.attach(ADDR_RefreshVisualAppearance, &hookRefreshVisualAppearance) != .ok) { + set_block_hook.detach(); + return; } - const rv_thunk = refresh_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(rv_thunk, @intFromPtr(&hookRefreshVisualAppearance), 3); - refresh_hook.activate(@intFromPtr(rv_thunk)); - // Hook 3: SceneEnd (9 bytes, no fixups) - if (!scene_end_hook.prepare(ADDR_SceneEnd, 9, &.{})) { - refresh_hook.remove(); - set_block_hook.remove(); - return false; + // Hook 3: SceneEnd + if (scene_end_hook.attach(ADDR_SceneEnd, &hookSceneEnd) != .ok) { + refresh_hook.detach(); + set_block_hook.detach(); + return; } - const se_thunk = scene_end_hook.mem.? + 32; - _ = hook.buildFastcallToCdeclThunk(se_thunk, @intFromPtr(&hookSceneEnd), 0); - scene_end_hook.activate(@intFromPtr(se_thunk)); g_initialized = true; con.print("[transmogfix] All 3 hooks installed\n"); - return true; } pub fn removeHooks() void { if (g_is_hook_owner) { - scene_end_hook.remove(); - refresh_hook.remove(); - set_block_hook.remove(); + scene_end_hook.detach(); + refresh_hook.detach(); + set_block_hook.detach(); } if (g_is_hook_owner) {