Dusk/WeirdUtils
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix release-mode crashes: inline asm register clobbers and fastcall ABI

Browse Source 
Root cause: hook.fastcall used "r" constraints + explicit MOV to set
ECX/EDX. LLVM can allocate "r" inputs to clobbered registers, causing
cross-assignment (ecx_in→EDX, edx_in→ECX) or function address stomping
when func lands in ECX/EDX. Debug works by luck (trivial regalloc);
Release optimizes aggressively and hits the conflicts.

Fix: explicit "{ecx}", "{edx}", "{eax}" register constraints in zhook
fastcall — compiler places values directly, no MOV needed, no ambiguity.

Also fix 9 inline asm blocks across main.zig, interact.zig,
screenshot.zig, markers.zig missing ECX/EDX clobbers after CALL
instructions. Without clobbers the optimizer assumes registers retain
input values after the call — stale reuse in release builds.

Other changes in this commit:
- Rename markers→worldmarkers (build flag, DLL, Lua table)
- Rename assetfix→looseassets
- lua.zig: add .never_tail to pushcclosure, pcall, openlib, pushnumber
- Move internal marker functions into WorldMarkers Lua table via openlib
- Remove unused GetCurrentAreaId function
- Fix cleanup_file_handle_hook.original() → .callOriginal()
This commit is contained in:
MarcelineVQ
2026-03-02 23:38:04 -08:00
parent b69c3a55a1
commit c04680788d
11 changed files with 126 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/markers/MPQ_FILESYSTEM_RESEARCH.md
+9 -9
View File
@@ -408,9 +408,9 @@ A minimal V1 MPQ for ~20 uncompressed files requires:
The hash table uses a specific encryption algorithm (documented in wowdev.wiki and StormLib source). Could implement in Zig comptime or use a build-time tool.
### Assetfix Synergy
### Loose Assets Synergy
The [assetfix project](/media/storage/projects/zig/assetfix/) already implements the glob pattern patch (`0x82edc2: '?' → '*'`) for multi-character patch names. If combined with assetfix, the temp file could use any name like `patch-weirdutils.MPQ`.
The [looseassets project](/media/storage/projects/zig/looseassets/) already implements the glob pattern patch (`0x82edc2: '?' → '*'`) for multi-character patch names. If combined with looseassets, the temp file could use any name like `patch-weirdutils.MPQ`.
---
@@ -532,11 +532,11 @@ If we could insert entries into this hash table mapping our asset paths to disk
---
## Approach D: CheckFileExistence Hook (assetfix Pattern)
## Approach D: CheckFileExistence Hook (looseassets Pattern)
### Concept
The [assetfix project](/media/storage/projects/zig/assetfix/) takes a different approach entirely:
The [looseassets project](/media/storage/projects/zig/looseassets/) takes a different approach entirely:
1. **NOP two gates** in `File_FindInArchive` (0x654b5c, 0x654b6a) that restrict `CheckFileExistence` to only "Interface/AddOns" paths
2. **Hook `CheckFileExistence`** (0x654DD0) to check a hash map of loose disk files
@@ -544,7 +544,7 @@ The [assetfix project](/media/storage/projects/zig/assetfix/) takes a different
This works for **disk-based** loose files but not for in-memory embedded data. However, combined with writing temp files to disk, it provides a clean single-hook solution.
### Key Addresses (from assetfix)
### Key Addresses (from looseassets)
| Target | Address | Patch |
|--------|---------|-------|
@@ -564,7 +564,7 @@ This works for **disk-based** loose files but not for in-memory embedded data. H
| Fake contexts | ~20 | **0** | 0 | 1 | 0 | 0 |
| Async handling | Manual | **Native** | Native | Uncertain | Native | Native |
| Build complexity | Low | **Low** | Medium (MPQ builder) | Medium (MPQ builder) | High (struct RE) | Low |
| RE work needed | Done | **Minimal** (verify path redirect) | Archive_OpenUnified conv | Same as A + async verify | Full struct layout | Already done (assetfix) |
| RE work needed | Done | **Minimal** (verify path redirect) | Archive_OpenUnified conv | Same as A + async verify | Full struct layout | Already done (looseassets) |
| Risk | Proven but fragile | **Low** (real OS handles) | Low (native I/O) | Medium (async path?) | High (struct mismatch) | Low |
| Cleanup needed | None | **Auto** (DELETE_ON_CLOSE) | Delete temp file | None | Remove from array | Delete temp files |
| Purely in-memory | Yes | Mostly (cache-backed) | No | Yes | Yes | No |
@@ -595,9 +595,9 @@ This works for **disk-based** loose files but not for in-memory embedded data. H
If a single-file solution is preferred over ~20 temp files, building a real MPQ and registering it via `Archive_OpenUnified` eliminates all hooks entirely. The cost is implementing an MPQ V1 builder (hash table encryption, block table, header). Could be combined with Approach E: use temp files now, migrate to MPQ later.
### Worth Combining With: Assetfix Integration
### Worth Combining With: Loose Assets Integration
The assetfix project's `CheckFileExistence` hook and glob pattern patch could complement either approach, especially for supporting user-provided loose asset files alongside our embedded ones.
The looseassets project's `CheckFileExistence` hook and glob pattern patch could complement either approach, especially for supporting user-provided loose asset files alongside our embedded ones.
---
@@ -687,6 +687,6 @@ The assetfix project's `CheckFileExistence` hook and glob pattern patch could co
| Archive search critsec | 0xc54008 | Critical section for archive ops |
| Archive search state | 0xc53ff0 | Used by File_FindInArchive |
| Patch glob "patch-?.MPQ" | 0x82edbc | Glob pattern for patch discovery |
| Patch glob char | 0x82edc2 | The '?' byte (assetfix patches to '*') |
| Patch glob char | 0x82edc2 | The '?' byte (looseassets patches to '*') |
| Data path format | 0x82edc8 | `"Data\%s"` format string |
| SArchive RTTI | 0x82e248 | `".PAVSArchive@@"` |
src/markers/addon/Markers.lua
+10 -10
View File
@@ -20,7 +20,7 @@ end
-- Permission model: ALL permission checks are enforced DLL-side.
-- WorldMarker/ClearWorldMarker: DLL checks local player is leader/assist.
-- SetMarkerDef/ClearMarkerDef: DLL checks sender name against roster.
-- CanSetMarkers(): DLL returns 1 if local player has permission.
-- CanSetWorldMarkers(): DLL returns 1 if local player has permission.
-- =============================================================================
local MSG_PREFIX = "WMark"
@@ -63,7 +63,7 @@ local function broadcastAllDefs()
if not ch then return end
local count = 0
for i = 1, NUM_MARKERS do
local x, y, z, areaId = GetMarkerDef(i)
local x, y, z, areaId = WorldMarkers.GetMarkerDef(i)
if x then
local msg = "SF:" .. i .. ":" .. x .. ":" .. y .. ":" .. z .. ":" .. areaId
log("SEND [" .. ch .. "] " .. msg)
@@ -110,7 +110,7 @@ function WorldMarker(index, ...)
denyCount = 0
local ch = getChannel()
if ch then
local x, y, z, areaId = GetMarkerDef(index)
local x, y, z, areaId = WorldMarkers.GetMarkerDef(index)
if x then
broadcastPlace(index, x, y, z, areaId)
end
@@ -185,7 +185,7 @@ local function onAddonMessage(prefix, message, channel, sender)
if cmd == "SR" then
-- Normal sync request: only leader/assist responds
log(" sync request from " .. tostring(sender))
if CanSetMarkers() then
if CanSetWorldMarkers() then
broadcastAllDefs()
end
return
@@ -205,7 +205,7 @@ local function onAddonMessage(prefix, message, channel, sender)
end
local idx, x, y, z, areaId = parseMarkerFields(parts)
if idx then
SetMarkerDefSync(idx, x, y, z, areaId, sender)
WorldMarkers.SetMarkerDefSync(idx, x, y, z, areaId, sender)
else
log(" PARSE FAIL")
end
@@ -216,17 +216,17 @@ local function onAddonMessage(prefix, message, channel, sender)
if cmd == "P" then
local idx, x, y, z, areaId = parseMarkerFields(parts)
if idx then
SetMarkerDef(idx, x, y, z, areaId, sender)
WorldMarkers.SetMarkerDef(idx, x, y, z, areaId, sender)
else
log(" PARSE FAIL")
end
elseif cmd == "C" then
local idx = tonumber(parts[2])
if idx then
ClearMarkerDef(idx, sender)
WorldMarkers.ClearMarkerDef(idx, sender)
end
elseif cmd == "CA" then
ClearMarkerDef(sender)
WorldMarkers.ClearMarkerDef(sender)
else
log(" unknown: " .. tostring(cmd))
end
@@ -244,7 +244,7 @@ local function broadcastSyncRequest()
end
-- Reset first-responder lock before requesting
syncSender = nil
local cmd = CanSetMarkers() and "LSR" or "SR"
local cmd = CanSetWorldMarkers() and "LSR" or "SR"
log("SEND [" .. ch .. "] " .. cmd)
SendAddonMessage(MSG_PREFIX, cmd, ch)
end
@@ -302,7 +302,7 @@ rosterTimer:SetScript("OnUpdate", function()
rosterTimer.pending = false
rosterTimer.extensions = 0
log("roster timer fired, broadcasting")
if CanSetMarkers() then
if CanSetWorldMarkers() then
broadcastAllDefs()
end
end
src/markers/markers.zig
+14 -16
View File
@@ -5,15 +5,19 @@
//! Markers persist across zone transitions via MarkerDef definitions.
//! Entities are respawned automatically when the player approaches within 200y.
//!
//! Lua API:
//! Lua API (globals):
//! WorldMarker(index, x, y, z) — place marker at coordinates
//! WorldMarker(index, "unit") — place marker at unit's position
//! WorldMarker(index) — place marker at cursor terrain position
//! ClearWorldMarker(index) — remove specific marker (1-5)
//! ClearWorldMarker() — remove all markers
//! SetMarkerDef(i, x, y, z, area) — store definition (no immediate spawn)
//! ClearMarkerDef([index]) — clear definition (and entity)
//! GetMarkerDef(index) — returns x, y, z, areaId or nil
//! CanSetWorldMarkers() — returns 1 if leader/assist, nil otherwise
//!
//! Lua API (WorldMarkers table — internal, used by addon):
//! WorldMarkers.SetMarkerDef(i, x, y, z, area, sender)
//! WorldMarkers.SetMarkerDefSync(i, x, y, z, area, sender)
//! WorldMarkers.ClearMarkerDef([index,] sender)
//! WorldMarkers.GetMarkerDef(index) — returns x, y, z, areaId or nil
const std = @import("std");
const hook = @import("zhook");
@@ -151,7 +155,7 @@ fn getNameFromGUID(guid_lo: u32, guid_hi: u32) ?[*:0]const u8 {
: [_] "{ecx}" (@as(u32, o.NAME_CACHE_OBJ)),
[a] "r" (&stack_args),
[func] "r" (@as(u32, o.FN_NAME_CACHE_LOOKUP)),
: .{ .edx = true, .memory = true, .cc = true });
: .{ .ecx = true, .edx = true, .memory = true, .cc = true });
return if (result != 0) @ptrFromInt(result) else null;
}
@@ -340,7 +344,7 @@ fn createEntityInstance(path: [*:0]const u8, pos: *[3]f32, facing: f32, flags: u
[_] "{edx}" (@intFromPtr(pos)),
[a] "r" (&stack_args),
[func] "r" (o.FN_CREATE_ENTITY_INSTANCE),
: .{ .memory = true, .cc = true });
: .{ .ecx = true, .edx = true, .memory = true, .cc = true });
return if (result != 0) @ptrFromInt(result) else null;
}
@@ -351,7 +355,7 @@ fn cleanupEntity(obj: *anyopaque) void {
:
: [_] "{ecx}" (@intFromPtr(obj)),
[func] "r" (o.FN_CLEANUP_ENTITY),
: .{ .eax = true, .edx = true, .memory = true, .cc = true });
: .{ .eax = true, .ecx = true, .edx = true, .memory = true, .cc = true });
}
// =============================================================================
@@ -389,7 +393,7 @@ fn playAnimation(entity: *anyopaque, anim_id: u32, queue: bool) void {
: [_] "{ecx}" (model),
[a] "r" (&stack_args),
[func] "r" (o.FN_PLAY_BONE_ANIMATION),
: .{ .eax = true, .edx = true, .memory = true, .cc = true });
: .{ .eax = true, .ecx = true, .edx = true, .memory = true, .cc = true });
}
/// Clean up despawning entities whose Decay animation has finished.
@@ -800,7 +804,7 @@ pub fn luaGetMarkerDef(L: lua.State) callconv(.c) u32 {
return 4;
}
/// Lua: local ok = CanSetMarkers()
/// Lua: local ok = CanSetWorldMarkers()
/// Returns 1 if the local player has permission (leader/assist), nil otherwise.
/// Used by the addon for broadcast/sync decisions.
pub fn luaCanSetMarkers(L: lua.State) callconv(.c) u32 {
@@ -811,13 +815,7 @@ pub fn luaCanSetMarkers(L: lua.State) callconv(.c) u32 {
return 0;
}
/// Lua: local areaId = GetCurrentAreaId()
/// Returns the current zone area ID from the game global.
pub fn luaGetCurrentAreaId(L: lua.State) callconv(.c) u32 {
const area_id = hook.readMem(u32, o.ZONE_AREA_ID);
lua.pushnumber(L, @floatCast(@as(f64, @floatFromInt(area_id))));
return 1;
}
// =============================================================================
// World teardown hook