diff --git a/src/agentdecompile_recovery/source_parity_synthesize.py b/src/agentdecompile_recovery/source_parity_synthesize.py index e43dbda..a8a73ab 100755 --- a/src/agentdecompile_recovery/source_parity_synthesize.py +++ b/src/agentdecompile_recovery/source_parity_synthesize.py @@ -573,6 +573,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) {{", @@ -598,7 +607,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", @@ -608,7 +620,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 0000000..d6b9bec --- /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"] diff --git a/tests/test_rewrite_queue.py b/tests/test_rewrite_queue.py index bc7d530..5cbe6e1 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: