From 8c0db1ccc4550ea0f378b314cda80c5ea725c873 Mon Sep 17 00:00:00 2001 From: Copilot Date: Thu, 30 Jul 2026 03:15:30 -0500 Subject: [PATCH 1/3] fix: inc_abs_global never wires target-side relocation evidence render_target_coff_for_candidate() can reconstruct the objdiff target side with a matching symbol relocation for absolute-address references (absolute_address_relocations(), already used by bink_buffer_set_direct_draw_forwarder), but inc_abs_global() set evidence={"absoluteAddress": ...} instead of the absoluteAddressRelocations shape the consumer actually reads. The target side was always rendered as a raw byte blob with the address baked in literally, so any correct candidate referencing the global through a compiler-visible symbol relocation could never byte-match -- confirmed against the real MSVC8/wine toolchain on FUN_004a23b0 in swkotor-parity-inv, where an instruction-for-instruction identical inline-asm rewrite still reported DIFF_ARG_MISMATCH. With the fix, the same candidate reaches objdiff differences: 0. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ros3797gzvmswnJudQ1Znk --- .../source_parity_synthesize.py | 19 +++++++- ...test_inc_abs_global_relocation_evidence.py | 48 +++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/test_inc_abs_global_relocation_evidence.py diff --git a/src/agentdecompile_recovery/source_parity_synthesize.py b/src/agentdecompile_recovery/source_parity_synthesize.py index 482ce25e..ab4541a4 100755 --- a/src/agentdecompile_recovery/source_parity_synthesize.py +++ b/src/agentdecompile_recovery/source_parity_synthesize.py @@ -574,6 +574,15 @@ def inc_abs_global(row: dict[str, Any], c_name: str, data: bytes) -> list[Genera if len(data) != 7 or data[0] != 0xFF or data[1] != 0x05 or data[-1] != 0xC3: return [] addr = u32(data[2:6]) + global_symbol = f"_DAT_{addr:08x}" + absolute_address_relocations = [ + { + "offset": 2, + "type": "IMAGE_REL_I386_DIR32", + "symbol": global_symbol, + "decodedAddress": f"0x{addr:08x}", + } + ] plain_source = header("inc-absolute-global", row) + "\n".join( [ f"void {c_name}(void) {{", @@ -599,7 +608,10 @@ def inc_abs_global(row: dict[str, Any], c_name: str, data: bytes) -> list[Genera source=plain_source, callconv="cdecl", return_type="void", - evidence={"absoluteAddress": f"0x{addr:08x}"}, + evidence={ + "absoluteAddress": f"0x{addr:08x}", + "absoluteAddressRelocations": absolute_address_relocations, + }, ), GeneratedCandidate( rule="inc-absolute-global", @@ -609,7 +621,10 @@ def inc_abs_global(row: dict[str, Any], c_name: str, data: bytes) -> list[Genera source=volatile_source, callconv="cdecl", return_type="void", - evidence={"absoluteAddress": f"0x{addr:08x}"}, + evidence={ + "absoluteAddress": f"0x{addr:08x}", + "absoluteAddressRelocations": absolute_address_relocations, + }, ) ] diff --git a/tests/test_inc_abs_global_relocation_evidence.py b/tests/test_inc_abs_global_relocation_evidence.py new file mode 100644 index 00000000..d6b9bec1 --- /dev/null +++ b/tests/test_inc_abs_global_relocation_evidence.py @@ -0,0 +1,48 @@ +"""Regression tests for inc_abs_global's target-side relocation evidence. + +Bug: inc_abs_global() populated evidence={"absoluteAddress": ...} instead of +the absoluteAddressRelocations shape that render_target_coff_for_candidate() +actually reads. Without it, the target side of every objdiff comparison for +this rule is rendered as a raw byte blob with the address baked in literally, +while any correct compiled candidate necessarily references the global via a +real relocation -- an unclosable structural mismatch regardless of candidate +source quality (confirmed via a real MSVC8/objdiff run on FUN_004a23b0: an +instruction-for-instruction identical inline-asm rewrite still reported +DIFF_ARG_MISMATCH because the two sides were never rendered comparably). +""" + +from __future__ import annotations + +from agentdecompile_recovery.source_parity_synthesize import ( + inc_abs_global, + render_target_coff_for_candidate, +) + +TARGET_BYTES = bytes.fromhex("ff0540058300c3") # inc dword ptr [0x00830540]; ret + + +def test_inc_abs_global_populates_absolute_address_relocations() -> None: + candidates = inc_abs_global({}, "FUN_004a23b0", TARGET_BYTES) + assert len(candidates) == 2 + + for candidate in candidates: + relocations = candidate.evidence.get("absoluteAddressRelocations") + assert isinstance(relocations, list) and len(relocations) == 1 + relocation = relocations[0] + assert relocation["offset"] == 2 + assert relocation["type"] == "IMAGE_REL_I386_DIR32" + assert relocation["symbol"] == "_DAT_00830540" + assert relocation["decodedAddress"] == "0x00830540" + + +def test_render_target_coff_reconstructs_relocation_for_inc_abs_global() -> None: + candidates = inc_abs_global({}, "FUN_004a23b0", TARGET_BYTES) + rendered = render_target_coff_for_candidate(candidates[0], TARGET_BYTES) + + # The address bytes (offset 2..6) must be emitted as a symbol relocation, + # not as raw .byte literals -- otherwise the target side can never match + # a candidate object that references the same global through a symbol. + assert ".long _DAT_00830540" in rendered["asm"] + assert rendered["relocations"] + assert rendered["relocations"][0]["symbol"] == "_DAT_00830540" + assert "reconstructed relocations" in rendered["origin"] From 7e0756b20cc3c79dd4a52b092d622d3b02fd0722 Mon Sep 17 00:00:00 2001 From: Copilot Date: Thu, 30 Jul 2026 05:21:59 -0500 Subject: [PATCH 2/3] fix: remove unused .state.now import (pre-existing ruff F401) Pre-existing on master; blocks CI's ruff lint step. Unrelated to the relocation-evidence fix itself; fixing since it's a one-line, zero-risk removal blocking merge readiness. --- src/agentdecompile_recovery/source_parity_synthesize.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/agentdecompile_recovery/source_parity_synthesize.py b/src/agentdecompile_recovery/source_parity_synthesize.py index ab4541a4..6f82abab 100755 --- a/src/agentdecompile_recovery/source_parity_synthesize.py +++ b/src/agentdecompile_recovery/source_parity_synthesize.py @@ -24,7 +24,6 @@ from typing import Any, Iterable from .package_verify import build_shim, compile_with_msvc -from .state import now ROOT = Path.cwd() DEFAULT_VC_ROOT: Path | None = None From 14f9ee1c0528030741e8b886d98c5508fe169cc9 Mon Sep 17 00:00:00 2001 From: Boden Crouch Date: Thu, 30 Jul 2026 13:18:58 -0500 Subject: [PATCH 3/3] fix: macOS multiprocessing failures + unused import in CI (#152) Two pre-existing CI failures on master, discovered while verifying the swkotor.exe autonomous recovery loop end-to-end: - tests/test_rewrite_queue.py's two concurrency tests create multiprocessing.Process with the default start method, which is "spawn" on macOS (vs "fork" on Linux). Spawn re-imports the target function in a fresh interpreter rather than reusing the parent's loaded image -- this fails on macOS CI with ModuleNotFoundError ("tests" package not reliably importable by the fresh interpreter) and, for a locally-nested function, AttributeError (can't pickle a function at all). Reproduced locally on Linux via an explicit spawn context to confirm the exact failure class, then fixed by forcing multiprocessing.get_context("fork") explicitly (available on both Linux and macOS, the only two CI platforms) and moving the previously-nested _writer to module level as defense in depth. - source_parity_synthesize.py: removed an unused `.state.now` import (ruff F401), unrelated pre-existing dead import blocking the lint step. 601 unit tests pass; ruff clean. Co-authored-by: Copilot --- tests/test_rewrite_queue.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/test_rewrite_queue.py b/tests/test_rewrite_queue.py index bc7d530d..5cbe6e16 100644 --- a/tests/test_rewrite_queue.py +++ b/tests/test_rewrite_queue.py @@ -23,6 +23,25 @@ def _claim_worker(work_dir: str, request_id: str, claimant: str, result_path: st Path(result_path).write_text("1" if ok else "0", encoding="utf-8") +def _write_request_worker(work_dir: str, name: str) -> None: + rewrite_queue.write_rewrite_request( + Path(work_dir), function_name=name, entry="0x1", candidate_source=f"src-{name}", mismatch_class=None, mismatch_histogram=None + ) + + +# macOS defaults multiprocessing to the "spawn" start method, which re-imports +# the target function in a fresh interpreter rather than fork()ing the +# already-loaded parent. Under pytest, the test module isn't reliably +# importable by that fresh interpreter (no guaranteed `tests` package on +# sys.path), so spawn-based Process creation fails here with +# ModuleNotFoundError/AttributeError on macOS CI even though the exact same +# code passes on Linux (which defaults to fork). Force fork explicitly -- +# available on both Linux and macOS (the only two CI platforms) -- since +# these tests only need process-level isolation, not spawn's clean-slate +# import behavior. +_FORK_CONTEXT = multiprocessing.get_context("fork") + + def test_write_rewrite_request_creates_pending_entry(tmp_path: Path) -> None: request_id = rewrite_queue.write_rewrite_request( tmp_path, @@ -241,8 +260,8 @@ def test_concurrent_claims_from_separate_processes_only_one_wins(tmp_path: Path) ) result_a = tmp_path / "result_a.txt" result_b = tmp_path / "result_b.txt" - proc_a = multiprocessing.Process(target=_claim_worker, args=(str(tmp_path), request_id, "proc-a", str(result_a))) - proc_b = multiprocessing.Process(target=_claim_worker, args=(str(tmp_path), request_id, "proc-b", str(result_b))) + proc_a = _FORK_CONTEXT.Process(target=_claim_worker, args=(str(tmp_path), request_id, "proc-a", str(result_a))) + proc_b = _FORK_CONTEXT.Process(target=_claim_worker, args=(str(tmp_path), request_id, "proc-b", str(result_b))) proc_a.start() proc_b.start() proc_a.join(timeout=10) @@ -264,13 +283,8 @@ def test_write_rewrite_request_survives_concurrent_writes_to_different_entries(t not lose each other's entries (the lock serializes the whole file, not just same-entry races).""" - def _writer(work_dir: str, name: str) -> None: - rewrite_queue.write_rewrite_request( - Path(work_dir), function_name=name, entry="0x1", candidate_source=f"src-{name}", mismatch_class=None, mismatch_histogram=None - ) - procs = [ - multiprocessing.Process(target=_writer, args=(str(tmp_path), f"sub_{i}")) + _FORK_CONTEXT.Process(target=_write_request_worker, args=(str(tmp_path), f"sub_{i}")) for i in range(6) ] for proc in procs: