Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/agentdecompile_recovery/source_parity_synthesize.py
Original file line number Diff line number Diff line change
Expand Up @@ -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) {{",
Expand All @@ -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",
Expand All @@ -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,
},
)
]

Expand Down
48 changes: 48 additions & 0 deletions tests/test_inc_abs_global_relocation_evidence.py
Original file line number Diff line number Diff line change
@@ -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"]
30 changes: 22 additions & 8 deletions tests/test_rewrite_queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand Down