ci: validate untouched ECX fastcall path

.github/workflows/wp24b2-inplace-gc.yml

@@ -0,0 +1,148 @@
+name: Build WeirdPerformance 2.4-B2 in-place GC test
+
+on:
+  push:
+    branches:
+      - test/wp24b2-gc-inplace-sweep
+    paths:
+      - 'experiments/wp24b-gc/**'
+      - '.github/workflows/wp24b2-inplace-gc.yml'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: mlugg/setup-zig@v2
+        with:
+          version: '0.17.0-dev.1970+67f39b551'
+      - name: Build and ABI-validate x86 DLL
+        working-directory: experiments/wp24b-gc
+        run: |
+          set -euo pipefail
+          zig build --fetch
+          python3 - <<'PY'
+          from pathlib import Path
+          matches = list(Path("zig-pkg").glob("zhook-*/src/zhook.zig"))
+          if len(matches) != 1:
+              raise SystemExit(f"expected one pinned zhook source, found {len(matches)}")
+          p = matches[0]
+          text = p.read_text(encoding="utf-8")
+          old = "var patch: [MAX_STOLEN]u8 = .{0x90} ** MAX_STOLEN;"
+          new = "var patch: [MAX_STOLEN]u8 = @splat(0x90);"
+          if old not in text:
+              raise SystemExit("expected pinned zhook repeat initializer not found")
+          p.write_text(text.replace(old, new), encoding="utf-8")
+          PY
+
+          zig build -Doptimize=small
+          DLL=zig-out/bin/weirdperformance_gc24b2.dll
+          test -s "$DLL"
+          file "$DLL" | tee BINARY_INFO.txt
+          file "$DLL" | grep -Eq 'PE32.*Intel (80386|i386)'
+          strings "$DLL" | grep -q '2.4-B2-gc-inplace-safe-sweep'
+
+          if command -v llvm-objdump >/dev/null 2>&1; then
+            llvm-objdump -d --x86-asm-syntax=intel "$DLL" > ABI_DISASM.txt
+          else
+            objdump -d -M intel "$DLL" > ABI_DISASM.txt
+          fi
+
+          python3 - <<'PY'
+          import re
+          from pathlib import Path
+
+          asm = Path("ABI_DISASM.txt").read_text(encoding="utf-8", errors="replace").lower()
+
+          def require(pattern, label):
+              if not re.search(pattern, asm, re.S):
+                  raise SystemExit(f"ABI validation failed: {label}")
+
+          def callback_snippet(target, window):
+              m = re.search(rf"mov\s+edx,\s*0x{target}.{{0,240}}?push\s+0x([0-9a-f]+)", asm, re.S)
+              if not m:
+                  raise SystemExit(f"ABI validation failed: callback for 0x{target} not found")
+              cb = m.group(1).lstrip("0") or "0"
+              pos = asm.find(cb + ":")
+              if pos < 0:
+                  raise SystemExit(f"ABI validation failed: callback body {cb} not found")
+              return asm[pos:pos+window]
+
+          # Collector callback: same proven B1 code shape, L copied from ECX.
+          collector = callback_snippet("6f7340", 900)
+          if not re.search(r"mov\s+esi,\s*ecx", collector):
+              raise SystemExit("ABI validation failed: collector callback does not consume L from ECX")
+
+          # B2 lua_close has no helper call before callOriginal: Zig can legally
+          # leave the incoming fastcall ECX untouched and call the trampoline
+          # directly. Accept either explicit preservation or verified no write
+          # to ECX before the first call instruction.
+          close = callback_snippet("6f6ef0", 800)
+          first_call = re.search(r"\bcall\b", close)
+          if not first_call:
+              raise SystemExit("ABI validation failed: lua_close trampoline call missing")
+          before_call = close[:first_call.start()]
+          ecx_saved = re.search(
+              r"(?:mov\s+(?:esi|edi|ebx|ebp|eax|edx),\s*ecx|push\s+ecx|mov\s+(?:dword ptr\s*)?\[[^\]]+\],\s*ecx)",
+              before_call,
+          )
+          ecx_written = re.search(
+              r"\b(?:mov|lea|xor|and|or|add|sub|imul|inc|dec|pop)\s+ecx\b",
+              before_call,
+          )
+          if not ecx_saved and ecx_written:
+              print("lua_close callback snippet:")
+              print(close)
+              raise SystemExit("ABI validation failed: lua_close clobbers incoming ECX before trampoline")
+
+          # Native helper ABI observed directly in WoW 5875:
+          # lua_gc_remove_objects: ECX=L, EDX=&list, stack arg all=0.
+          require(
+              r"lea\s+edx,\s*\[edi\s*\+\s*0x14\].{0,200}?"
+              r"mov\s+eax,\s*0x6f7210.{0,160}?"
+              r"mov\s+ecx,\s*esi.{0,160}?"
+              r"push\s+0x0.{0,100}?call\s+eax",
+              "rootudata lua_gc_remove_objects ABI",
+          )
+
+          # At least one dynamic-list remove call must load L into ECX.
+          require(
+              r"mov\s+eax,\s*0x6f7210.{0,240}?mov\s+ecx,\s*e[a-z]{2}.{0,200}?call\s+eax",
+              "chunk lua_gc_remove_objects fastcall callsite",
+          )
+
+          require(
+              r"mov\s+eax,\s*0x6f72f0.{0,160}?"
+              r"mov\s+ecx,\s*esi.{0,120}?"
+              r"xor\s+edx,\s*edx.{0,100}?call\s+eax",
+              "lua_gc_sweep_all_lists fastcall ABI",
+          )
+
+          for addr, label in (
+              ("6f73e0", "lua_gc_full_collection"),
+              ("6f7370", "lua_gc_shrink_memory"),
+              ("6f7080", "luaCallUserDataGC"),
+          ):
+              require(
+                  rf"mov\s+eax,\s*0x{addr}.{{0,180}}?mov\s+ecx,\s*e[a-z]{{2}}.{{0,120}}?call\s+eax",
+                  f"{label} ECX ABI",
+              )
+
+          print("ABI validation: PASS")
+          PY
+
+          sha256sum "$DLL" | tee SHA256SUMS.txt
+      - uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: WeirdPerformance-2.4-B2-GC-InPlace-Safe-Sweep
+          path: |
+            experiments/wp24b-gc/zig-out/bin/weirdperformance_gc24b2.dll
+            experiments/wp24b-gc/SHA256SUMS.txt
+            experiments/wp24b-gc/BINARY_INFO.txt
+            experiments/wp24b-gc/ABI_DISASM.txt
+          if-no-files-found: warn

experiments/wp24b-gc/build.zig

@@ -15,7 +15,7 @@ pub fn build(b: *std.Build) void {
     });
 
     const lib = b.addLibrary(.{
-        .name = "weirdperformance_gc24b1",
+        .name = "weirdperformance_gc24b2",
         .linkage = .dynamic,
         .root_module = b.createModule(.{
             .root_source_file = b.path("main.zig"),

experiments/wp24b-gc/main.zig

@@ -1,11 +1,17 @@
-//! WeirdPerformance 2.4-B GC safe-sweep companion.
+//! WeirdPerformance 2.4-B2 GC in-place safe-sweep companion.
 //!
 //! A/B contract:
-//!   A = validated WeirdPerformance 2.4-A binary, unchanged.
-//!   B = the exact same 2.4-A binary + this companion DLL.
+//!   A  = validated WeirdPerformance 2.4-A binary, unchanged.
+//!   B2 = the exact same 2.4-A binary + this companion DLL.
 //!
-//! This companion only changes Lua GC behavior: WoW's native mark/udata/string
-//! work is kept, while the rootgc sweep is split into bounded chunks.
+//! B1 split rootgc by detaching swept survivors from the live rootgc chain
+//! between GC calls. B2 keeps the complete rootgc chain linked whenever
+//! control returns to WoW. Each chunk is isolated only for the duration of
+//! lua_gc_remove_objects(), then reconnected before the detour returns.
+//!
+//! This preserves B1's bounded 50,000-object rootgc sweep while removing the
+//! long-lived fragmented-list state suspected in delayed lua_table_set_value
+//! corruption (0x006FA8E2).
 //!
 //! Target: WoW 1.12.1 build 5875, x86 only.
 
@@ -16,7 +22,6 @@ const X86_FASTCALL: std.builtin.CallingConvention = .{ .x86_fastcall = .{} };
 
 const LUA_COLLECT_GARBAGE_ADDR: usize = 0x6F7340;
 const LUA_CLOSE_ADDR: usize = 0x6F6EF0;
-const BIRTH_MARK_ADDR: usize = 0x6F7B37;
 const IS_IN_WORLD_ADDR: u32 = 0x00B4B424;
 
 const GS_ROOTGC: u32 = 0x10;
@@ -50,19 +55,14 @@ var installed = false;
 var in_gc = false;
 var closing_lua = false;
 
+// B2 state. Unlike B1 there are no detached rootgc fragments.
+// sweep_next is the first object not yet swept in the current native mark cycle.
+// New luaC_link objects are prepended before it and are intentionally left for
+// the next cycle rather than being exposed to the current sweep without a mark.
 var sweeping = false;
-var swept_head: u32 = 0;
-var swept_tail: u32 = 0;
-var unswept_rest: u32 = 0;
+var sweep_next: u32 = 0;
 var saved_g: u32 = 0;
 
-var birth_mark_owned = false;
-var birth_mark_original: u8 = 0;
-
-inline fn readU8(addr: usize) u8 {
-    return @as(*volatile const u8, @ptrFromInt(addr)).*;
-}
-
 inline fn readU16(addr: usize) u16 {
     return @as(*volatile const u16, @ptrFromInt(addr)).*;
 }
@@ -96,15 +96,40 @@ fn validateClient() bool {
     return image_size == EXPECTED_IMAGE_SIZE;
 }
 
-fn findNth(head: u32, n: u32) struct { obj: u32, count: u32 } {
+fn resetSweepState() void {
+    sweeping = false;
+    sweep_next = 0;
+    saved_g = 0;
+}
+
+// Locate the pointer field which currently references target. This is required
+// because luaC_link prepends newly allocated objects to g->rootgc while a split
+// sweep is in progress. We must skip those new objects instead of accidentally
+// sweeping them with marks from the previous atomic phase.
+fn findLinkTo(g: u32, target: u32) ?u32 {
+    if (target == 0) return null;
+
+    var link_addr = g + GS_ROOTGC;
+    var obj = readU32(link_addr);
+    while (obj != 0) {
+        if (obj == target) return link_addr;
+        link_addr = obj + OBJ_NEXT;
+        obj = readU32(link_addr);
+    }
+    return null;
+}
+
+fn findChunkTail(head: u32, limit: u32) struct { tail: u32, count: u32 } {
+    if (head == 0 or limit == 0) return .{ .tail = 0, .count = 0 };
+
     var obj = head;
-    var i: u32 = 0;
-    while (obj != 0 and i < n) : (i += 1) {
+    var count: u32 = 1;
+    while (count < limit) : (count += 1) {
         const next = readU32(obj + OBJ_NEXT);
-        if (next == 0) return .{ .obj = 0, .count = i + 1 };
+        if (next == 0) return .{ .tail = obj, .count = count };
         obj = next;
     }
-    return .{ .obj = obj, .count = i };
+    return .{ .tail = obj, .count = count };
 }
 
 fn findTail(head: u32) u32 {
@@ -117,89 +142,83 @@ fn findTail(head: u32) u32 {
     }
 }
 
-fn acquireBirthMark() bool {
-    if (birth_mark_owned) return readU8(BIRTH_MARK_ADDR) == 0x01;
+const ChunkResult = enum {
+    more,
+    done,
+    invalid,
+};
 
-    const current = readU8(BIRTH_MARK_ADDR);
-    if (current != 0x00) return false;
+// Sweep exactly one bounded sub-list using WoW's native sweep routine.
+// The rootgc chain may be temporarily truncated while the native routine runs,
+// but it is always fully reconnected before this function returns.
+fn sweepOneChunk(L: u32, g: u32) ChunkResult {
+    if (!sweeping or sweep_next == 0) return .done;
+    if (g != saved_g) return .invalid;
 
-    birth_mark_original = current;
-    const patch = [1]u8{0x01};
-    hook.writeProtected(BIRTH_MARK_ADDR, &patch);
+    const link_addr = findLinkTo(g, sweep_next) orelse return .invalid;
+    const head = readU32(link_addr);
+    if (head != sweep_next) return .invalid;
 
-    if (readU8(BIRTH_MARK_ADDR) != 0x01) return false;
-    birth_mark_owned = true;
+    const chunk = findChunkTail(head, CHUNK_SIZE);
+    if (chunk.tail == 0) return .done;
+
+    const rest = readU32(chunk.tail + OBJ_NEXT);
+
+    // Final chunk: no temporary split is necessary. Let the native routine
+    // update the real list link directly and complete the cycle.
+    if (rest == 0) {
+        _ = lua_gc_remove_objects(L, link_addr, 0);
+        sweep_next = 0;
+        return .done;
+    }
+
+    // Isolate only the current chunk for the native sweep call.
+    writeU32(chunk.tail + OBJ_NEXT, 0);
+    _ = lua_gc_remove_objects(L, link_addr, 0);
+
+    // Reconnect the untouched remainder before returning to gameplay.
+    // If every object in the chunk died, link_addr itself becomes the bridge.
+    const survivors = readU32(link_addr);
+    if (survivors == 0) {
+        writeU32(link_addr, rest);
+    } else {
+        const survivor_tail = findTail(survivors);
+        if (survivor_tail == 0) return .invalid;
+        writeU32(survivor_tail + OBJ_NEXT, rest);
+    }
+
+    sweep_next = rest;
+    return .more;
+}
+
+fn finishCycle(L: u32) void {
+    resetSweepState();
+    lua_gc_shrink_memory(L);
+    luaCallUserDataGC(L);
+}
+
+// Used only for transitions such as leaving the world while a B2 cycle is
+// active. The list is fully linked, so we can finish remaining chunks without
+// any B1-style fragment reconstruction.
+fn finishSweepNow(L: u32, g: u32) bool {
+    while (sweeping) {
+        switch (sweepOneChunk(L, g)) {
+            .more => {},
+            .done => {
+                finishCycle(L);
+                return true;
+            },
+            .invalid => {
+                resetSweepState();
+                return false;
+            },
+        }
+    }
     return true;
 }
 
-fn releaseBirthMark() void {
-    if (!birth_mark_owned) return;
-
-    // Restore only while we still own exactly the byte we installed.
-    if (readU8(BIRTH_MARK_ADDR) == 0x01) {
-        const patch = [1]u8{birth_mark_original};
-        hook.writeProtected(BIRTH_MARK_ADDR, &patch);
-    }
-    birth_mark_owned = false;
-}
-
-fn resetSweepState() void {
-    sweeping = false;
-    swept_head = 0;
-    swept_tail = 0;
-    unswept_rest = 0;
-    saved_g = 0;
-    releaseBirthMark();
-}
-
-fn reconnectSweep() void {
-    if (!sweeping or saved_g == 0) {
-        resetSweepState();
-        return;
-    }
-
-    const g = saved_g;
-    var tail = findTail(readU32(g + GS_ROOTGC));
-
-    if (unswept_rest != 0) {
-        if (tail != 0) {
-            writeU32(tail + OBJ_NEXT, unswept_rest);
-        } else {
-            writeU32(g + GS_ROOTGC, unswept_rest);
-        }
-        tail = findTail(unswept_rest);
-    }
-
-    if (swept_head != 0) {
-        if (tail != 0) {
-            writeU32(tail + OBJ_NEXT, swept_head);
-        } else {
-            writeU32(g + GS_ROOTGC, swept_head);
-        }
-    }
-
-    resetSweepState();
-}
-
-fn detachSweptAndRestore(g: u32) void {
-    const current_head = readU32(g + GS_ROOTGC);
-    if (current_head != 0) {
-        const tail = findTail(current_head);
-        if (swept_head == 0) {
-            swept_head = current_head;
-            swept_tail = tail;
-        } else {
-            writeU32(swept_tail + OBJ_NEXT, current_head);
-            swept_tail = tail;
-        }
-    }
-
-    writeU32(g + GS_ROOTGC, unswept_rest);
-    unswept_rest = 0;
-}
-
 fn nativeFallback(L: u32) void {
-    reconnectSweep();
+    resetSweepState();
     collect_hook.callOriginal(.{L});
 }
 
@@ -209,12 +228,6 @@ fn collectGarbageDetour(L: u32) callconv(X86_FASTCALL) void {
         return;
     }
 
-    // Keep GlueXML/login/character-select on WoW's native GC.
-    // The experiment is intentionally gameplay-only.
-    if (readU32(IS_IN_WORLD_ADDR) == 0) {
-        collect_hook.callOriginal(.{L});
-        return;
-    }
     if (in_gc) return;
     if (L == 0) return;
     if (readU32(L + 0x60) == 0) return;
@@ -228,96 +241,67 @@ fn collectGarbageDetour(L: u32) callconv(X86_FASTCALL) void {
         return;
     }
 
-    // Never carry private list state into another Lua global_State.
-    if (sweeping and g != saved_g) {
-        resetSweepState();
+    // If gameplay ends mid-cycle, finish the already-marked sweep while the
+    // current Lua state is still valid, then return control to native GC.
+    if (readU32(IS_IN_WORLD_ADDR) == 0) {
+        if (sweeping and g == saved_g) {
+            if (!finishSweepNow(L, g)) {
+                collect_hook.callOriginal(.{L});
+                return;
+            }
+        } else if (sweeping) {
+            resetSweepState();
+        }
         collect_hook.callOriginal(.{L});
         return;
     }
 
-    // If another module changed the birth byte during our split cycle,
-    // reconnect immediately and hand this collection back to WoW.
-    if (sweeping and (!birth_mark_owned or readU8(BIRTH_MARK_ADDR) != 0x01)) {
+    // Never carry a cursor into a different Lua global_State.
+    if (sweeping and g != saved_g) {
         nativeFallback(L);
         return;
     }
 
     if (!sweeping) {
-        // Keep WoW's native mark, userdata sweep, and string sweep.
+        // Keep WoW's native atomic mark, userdata sweep, and string sweep.
         lua_gc_full_collection(L);
         _ = lua_gc_remove_objects(L, g + GS_ROOTUDATA, 0);
         lua_gc_sweep_all_lists(L, 0);
 
-        swept_head = 0;
-        swept_tail = 0;
-
-        const rootgc_head = readU32(g + GS_ROOTGC);
-        const result = findNth(rootgc_head, CHUNK_SIZE);
-
-        // Small rootgc lists remain one-shot, matching native behavior closely.
-        if (result.obj == 0) {
-            _ = lua_gc_remove_objects(L, g + GS_ROOTGC, 0);
-            lua_gc_shrink_memory(L);
-            luaCallUserDataGC(L);
-            return;
-        }
-
-        // If the birth marker is unavailable, do not split this collection.
-        if (!acquireBirthMark()) {
-            _ = lua_gc_remove_objects(L, g + GS_ROOTGC, 0);
-            lua_gc_shrink_memory(L);
-            luaCallUserDataGC(L);
-            return;
-        }
-
-        unswept_rest = readU32(result.obj + OBJ_NEXT);
-        writeU32(result.obj + OBJ_NEXT, 0);
-        sweeping = true;
+        sweep_next = readU32(g + GS_ROOTGC);
         saved_g = g;
+        sweeping = sweep_next != 0;
 
-        _ = lua_gc_remove_objects(L, g + GS_ROOTGC, 0);
-        detachSweptAndRestore(g);
-
-        const totalbytes = readU32(g + GS_TOTALBYTES);
-        writeU32(g + GS_GCTHRESHOLD, totalbytes + BATCH_HEADROOM);
-        return;
-    }
-
-    const rootgc_head = readU32(g + GS_ROOTGC);
-    const result = findNth(rootgc_head, CHUNK_SIZE);
-
-    if (result.obj == 0) {
-        _ = lua_gc_remove_objects(L, g + GS_ROOTGC, 0);
-
-        const current_head = readU32(g + GS_ROOTGC);
-        if (swept_head != 0) {
-            if (current_head != 0) {
-                writeU32(swept_tail + OBJ_NEXT, current_head);
-            }
-            writeU32(g + GS_ROOTGC, swept_head);
+        if (!sweeping) {
+            finishCycle(L);
+            return;
         }
-
-        resetSweepState();
-        lua_gc_shrink_memory(L);
-        luaCallUserDataGC(L);
-        return;
     }
 
-    unswept_rest = readU32(result.obj + OBJ_NEXT);
-    writeU32(result.obj + OBJ_NEXT, 0);
-
-    _ = lua_gc_remove_objects(L, g + GS_ROOTGC, 0);
-    detachSweptAndRestore(g);
-
-    const totalbytes = readU32(g + GS_TOTALBYTES);
-    writeU32(g + GS_GCTHRESHOLD, totalbytes + BATCH_HEADROOM);
+    switch (sweepOneChunk(L, g)) {
+        .done => {
+            finishCycle(L);
+            return;
+        },
+        .invalid => {
+            // The live rootgc chain itself was never fragmented across calls,
+            // so falling back to WoW's complete collector is safe here.
+            nativeFallback(L);
+            return;
+        },
+        .more => {
+            const totalbytes = readU32(g + GS_TOTALBYTES);
+            writeU32(g + GS_GCTHRESHOLD, totalbytes + BATCH_HEADROOM);
+            return;
+        },
+    }
 }
 
 fn luaCloseDetour(L: u32) callconv(X86_FASTCALL) void {
-    // lua_close may run native sweep paths that bypass luaC_collectgarbage.
-    // Reconnect every private fragment while the old Lua state is still valid.
+    // B2 never leaves detached rootgc fragments. lua_close can therefore own
+    // teardown normally; just discard the cursor before the state is destroyed.
     closing_lua = true;
-    reconnectSweep();
+    resetSweepState();
     in_gc = false;
 
     lua_close_hook.callOriginal(.{L});
@@ -330,7 +314,6 @@ fn install() void {
     if (!validateClient()) return;
 
     // Transactional install: lua_close guard first, collector second.
-    // If the collector cannot attach, roll the close hook back immediately.
     if (lua_close_hook.attach(LUA_CLOSE_ADDR, &luaCloseDetour) != .ok) return;
 
     if (collect_hook.attach(LUA_COLLECT_GARBAGE_ADDR, &collectGarbageDetour) != .ok) {
@@ -341,13 +324,13 @@ fn install() void {
     installed = true;
 }
 
-const version: [*:0]const u8 = "2.4-B1-gc-safe-sweep-abi";
+const version: [*:0]const u8 = "2.4-B2-gc-inplace-safe-sweep";
 
-pub export fn WeirdPerformanceGC24B_GetVersion() callconv(.c) [*:0]const u8 {
+pub export fn WeirdPerformanceGC24B2_GetVersion() callconv(.c) [*:0]const u8 {
     return version;
 }
 
-pub export fn WeirdPerformanceGC24B_IsActive() callconv(.c) i32 {
+pub export fn WeirdPerformanceGC24B2_IsActive() callconv(.c) i32 {
     return if (installed) 1 else 0;
 }