fix: packaged-source stdcall symbols silently fell back to cdecl naming

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
@@ -19119,6 +19118,74 @@ def generate(row: dict[str, Any], max_variants: int) -> list[GeneratedCandidate]
     return candidates
 
 
+_CALLEE_CALL_RE = re.compile(r"\b(sub_[0-9a-fA-F]+|FUN_[0-9a-fA-F]+)\s*\(")
+
+
+def _infer_callee_return_type(callee_source: str, callee_name: str) -> str:
+    # Falls back to "int" (not "void") when unparseable: an implicit
+    # pre-C99 declaration defaults to int, and a caller that ignores the
+    # return value still compiles fine against an "int" prototype -- unlike
+    # "void", which is a hard compile error for any caller that consumes it
+    # (e.g. the common Ghidra pattern `iVar1 = calleeName(...)`).
+    match = re.search(rf"(^|\n)([A-Za-z_][A-Za-z0-9_ \*]*?)\s+{re.escape(callee_name)}\s*\(", callee_source)
+    if not match:
+        return "int"
+    # The captured group spans everything before the name, which includes
+    # any calling-convention keyword (e.g. "undefined4 __stdcall") since the
+    # regex must expand past it to reach the required whitespace+name
+    # boundary -- strip those keywords back out, since the prototype already
+    # adds the calling-convention keyword explicitly.
+    rettype = re.sub(r"\b(__stdcall|__fastcall|__cdecl)\b", "", match.group(2)).strip()
+    return rettype or "int"
+
+
+def infer_callee_prototype(callee_name: str, source_generation_root: Path) -> str | None:
+    """Infer a calling-convention-correct extern prototype for a callee by
+    reusing its own packaged-source candidate (a sibling directory under the
+    same source-generation root) and running it through the same
+    infer_packaged_callconv/packaged_stack_bytes inference already used for
+    the caller itself.
+
+    Without this, a call to another sub_XXXX/FUN_XXXX function compiles with
+    an implicit (cdecl) declaration regardless of the callee's real calling
+    convention, so a stdcall/fastcall callee causes the caller to emit a
+    spurious `add esp, N` stack-cleanup instruction that the real compiled
+    binary never has (the real callee already cleaned the stack itself).
+    """
+    matches = sorted(source_generation_root.glob(f"{callee_name}_*/candidate.c"))
+    if not matches:
+        return None
+    callee_source = matches[0].read_text(encoding="utf-8", errors="replace")
+    callconv = infer_packaged_callconv(callee_source, ".c")
+    if callconv not in {"stdcall", "fastcall"}:
+        return None
+    stack_bytes = packaged_stack_bytes({"name": callee_name}, callee_name, source=callee_source)
+    if stack_bytes is None or stack_bytes % 4 != 0:
+        return None
+    sig_match = re.search(rf"\b{re.escape(callee_name)}\s*\(([^)]*)\)", callee_source)
+    if sig_match and _EIGHT_BYTE_PARAM_TYPE_RE.search(sig_match.group(1)):
+        # packaged_stack_bytes only tracks total byte count, not individual
+        # param widths -- an 8-byte param would make stack_bytes // 4 emit
+        # the wrong number of (all 4-byte) params, an arity mismatch against
+        # the real call site. Bail rather than emit a wrong-arity prototype.
+        return None
+    param_count = stack_bytes // 4
+    params = ", ".join(["unsigned int"] * param_count) if param_count else "void"
+    keyword = "__stdcall" if callconv == "stdcall" else "__fastcall"
+    return_type = _infer_callee_return_type(callee_source, callee_name)
+    return f"extern {return_type} {keyword} {callee_name}({params});"
+
+
+def infer_callee_prototypes(source: str, self_name: str, source_generation_root: Path) -> str:
+    names = sorted({name for name in _CALLEE_CALL_RE.findall(source) if name != self_name})
+    prototypes = []
+    for name in names:
+        prototype = infer_callee_prototype(name, source_generation_root)
+        if prototype:
+            prototypes.append(prototype)
+    return "\n".join(prototypes)
+
+
 def packaged_source_candidate(row: dict[str, Any]) -> GeneratedCandidate | None:
     if not row.get("sourceTask"):
         return None
@@ -19134,7 +19201,12 @@ def packaged_source_candidate(row: dict[str, Any]) -> GeneratedCandidate | None:
     # Packaged .c sources are raw decompiler output (e.g. Ghidra's undefined4/code/byte
     # pseudo-types) with no typedefs of their own; without the shim MSVC/clang fail to
     # even parse the file, so nothing downstream ever reaches objdiff.
-    compile_source = build_shim(source) + "\n\n" + source if suffix == ".c" else source
+    callee_prototypes = ""
+    if suffix == ".c":
+        callee_prototypes = infer_callee_prototypes(source, c_name, source_path.parent.parent)
+        compile_source = build_shim(source) + (f"\n{callee_prototypes}\n" if callee_prototypes else "") + "\n" + source
+    else:
+        compile_source = source
     return GeneratedCandidate(
         rule=str(automatic_generator.get("rule") or "packaged-source"),
         variant="packaged-source",
@@ -19151,6 +19223,7 @@ def packaged_source_candidate(row: dict[str, Any]) -> GeneratedCandidate | None:
             "packagedSource": str(source_path),
             "packagedSourceSha256": hashlib.sha256(source.encode("utf-8")).hexdigest(),
             "sourceOrigin": row.get("sourceOrigin"),
+            "calleePrototypesInferred": callee_prototypes.splitlines(),
         },
         source_suffix=suffix,
         semantic_source=row.get("semanticSource") is not False,
@@ -19195,15 +19268,37 @@ def infer_packaged_symbol(row: dict[str, Any], source: str, c_name: str, suffix:
             return match.group(1)
         return c_name
     callconv = infer_packaged_callconv(source, suffix)
-    stack_bytes = packaged_stack_bytes(row, c_name)
+    stack_bytes = packaged_stack_bytes(row, c_name, source=source)
     if callconv == "stdcall" and stack_bytes is not None:
         return f"_{c_name}@{stack_bytes}"
     if callconv == "fastcall" and stack_bytes is not None:
         return fastcall_symbol(c_name, stack_bytes)
     return cdecl_symbol(c_name)
 
 
-def packaged_stack_bytes(row: dict[str, Any], c_name: str) -> int | None:
+# Pseudo-types (and their real-type equivalents) that occupy 8 bytes on the
+# stack rather than the 4-byte default -- matched against a parameter's type
+# tokens (everything before the parameter name).
+_EIGHT_BYTE_PARAM_TYPE_RE = re.compile(r"\b(undefined8|double|long\s+long|__int64)\b")
+
+
+def _count_stack_bytes_from_parameter_list(params_text: str) -> int:
+    params_text = params_text.strip()
+    if not params_text or params_text == "void":
+        return 0
+    total = 0
+    for param in params_text.split(","):
+        param = param.strip()
+        if not param:
+            continue
+        total += 8 if _EIGHT_BYTE_PARAM_TYPE_RE.search(param) else 4
+    return total
+
+
+_AUTO_DECOMPILER_NAME_RE = re.compile(r"^(?:sub|FUN)_[0-9a-fA-F]+$")
+
+
+def packaged_stack_bytes(row: dict[str, Any], c_name: str, *, source: str | None = None) -> int | None:
     generator = row.get("automaticGenerator") if isinstance(row.get("automaticGenerator"), dict) else {}
     stack_bytes = optional_int(generator.get("stackBytes"))
     if stack_bytes is not None:
@@ -19212,9 +19307,25 @@ def packaged_stack_bytes(row: dict[str, Any], c_name: str) -> int | None:
     match = re.search(r"@(\d+)$", name)
     if match:
         return int(match.group(1))
-    match = re.search(r"_(\d+)$", c_name)
-    if match:
-        return int(match.group(1))
+    # Skip the "_<digits>" suffix heuristic for Ghidra's default auto-named
+    # functions (sub_<hex>/FUN_<hex>) -- when the address happens to be
+    # composed entirely of decimal digits (no a-f), this would misread the
+    # function's own address as a stack-byte annotation (e.g. sub_11240 ->
+    # wrongly inferred as 11240 stack bytes). The suffix is only meaningful
+    # for deliberately-named helpers (e.g. "helper_16" meaning 16 bytes).
+    if not _AUTO_DECOMPILER_NAME_RE.match(c_name):
+        match = re.search(r"_(\d+)$", c_name)
+        if match:
+            return int(match.group(1))
+    if source:
+        # No external metadata carries the stack-byte count (common for a
+        # plain decompiler-named function like sub_1234 with no @N suffix
+        # anywhere) -- count it directly from the parsed function's own
+        # parameter list rather than silently falling back to cdecl naming
+        # for what may genuinely be a stdcall/fastcall function.
+        sig_match = re.search(rf"\b{re.escape(c_name)}\s*\(([^)]*)\)", source)
+        if sig_match:
+            return _count_stack_bytes_from_parameter_list(sig_match.group(1))
     return None
 
 

tests/test_packaged_callee_calling_convention.py

@@ -0,0 +1,143 @@
+"""Regression test: packaged-source candidates that call other sub_XXXX/
+FUN_XXXX functions must declare the callee's real calling convention,
+not silently let it default to cdecl.
+
+Found during the swkotor.exe cache sweep: sub_88c0's real target calls
+sub_7830 (2 args) with no stack cleanup after the call (`call sub_7830 / ret
+0x4`), because sub_7830 is itself __stdcall and cleans its own 8 bytes. The
+packaged-source candidate for sub_88c0 has no prototype for sub_7830 in
+scope, so the implicit C declaration defaults to cdecl and the caller wrongly
+emits `add esp, 0x8` after the call -- a real instruction-stream mismatch,
+not just a symbol-naming issue.
+
+infer_callee_prototype()/infer_callee_prototypes() fix this by reusing the
+callee's own packaged-source candidate.c (a sibling directory under the same
+source-generation root) and running it back through the same
+infer_packaged_callconv()/packaged_stack_bytes() inference already used for
+the caller itself, to emit a correctly-decorated extern prototype before the
+caller's body.
+
+Two follow-up fixes from code review are also covered here:
+- The emitted prototype's return type is inferred from the callee's own
+  source (falling back to "int", never "void") -- a hardcoded "void" broke
+  compilation for any caller that consumes the callee's return value (the
+  common Ghidra pattern `iVar1 = calleeName(...)`).
+- A callee whose parameter list contains an 8-byte type (undefined8/double/
+  long long/__int64) is skipped (returns None) rather than emitting a
+  wrong-arity prototype: packaged_stack_bytes only tracks total byte count,
+  and stack_bytes // 4 assumes every param is 4 bytes.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+from agentdecompile_recovery.source_parity_synthesize import (
+    _infer_callee_return_type,
+    infer_callee_prototype,
+    infer_callee_prototypes,
+)
+
+pytestmark = pytest.mark.unit
+
+
+def _write_callee_candidate(root: Path, name: str, entry: str, source: str) -> None:
+    directory = root / f"{name}_{entry}"
+    directory.mkdir(parents=True, exist_ok=True)
+    (directory / "candidate.c").write_text(source, encoding="utf-8")
+
+
+def test_infer_callee_prototype_declares_stdcall_from_sibling_candidate(tmp_path: Path) -> None:
+    _write_callee_candidate(
+        tmp_path,
+        "sub_7830",
+        "407830",
+        "undefined4 __stdcall sub_7830(undefined4 param_1,int param_2)\n\n{\n  return 1;\n}\n",
+    )
+
+    prototype = infer_callee_prototype("sub_7830", tmp_path)
+
+    assert prototype == "extern undefined4 __stdcall sub_7830(unsigned int, unsigned int);"
+
+
+def test_infer_callee_prototype_returns_none_for_cdecl_callee(tmp_path: Path) -> None:
+    _write_callee_candidate(
+        tmp_path,
+        "sub_1e5670",
+        "41e5670",
+        "void sub_1e5670(int param_1)\n\n{\n  return;\n}\n",
+    )
+
+    assert infer_callee_prototype("sub_1e5670", tmp_path) is None
+
+
+def test_infer_callee_prototype_returns_none_when_sibling_missing(tmp_path: Path) -> None:
+    assert infer_callee_prototype("sub_deadbeef", tmp_path) is None
+
+
+def test_infer_callee_prototypes_skips_self_recursive_calls(tmp_path: Path) -> None:
+    _write_callee_candidate(
+        tmp_path,
+        "sub_1060",
+        "401060",
+        "undefined4 __stdcall sub_1060(undefined4 param_1)\n\n{\n  return sub_1060(param_1);\n}\n",
+    )
+    source = "undefined4 __stdcall sub_1060(undefined4 param_1)\n\n{\n  return sub_1060(param_1);\n}\n"
+
+    assert infer_callee_prototypes(source, "sub_1060", tmp_path) == ""
+
+
+def test_infer_callee_prototypes_declares_multiple_callees(tmp_path: Path) -> None:
+    _write_callee_candidate(
+        tmp_path,
+        "sub_7830",
+        "407830",
+        "undefined4 __stdcall sub_7830(undefined4 param_1,int param_2)\n\n{\n  return 1;\n}\n",
+    )
+    _write_callee_candidate(
+        tmp_path,
+        "sub_7fe0",
+        "407fe0",
+        "undefined4 __stdcall sub_7fe0(undefined4 param_1,int param_2)\n\n{\n  return 1;\n}\n",
+    )
+    source = (
+        "void __stdcall sub_88e0(undefined4 param_1)\n\n"
+        "{\n  sub_7830(param_1, 2);\n  sub_7fe0(param_1, 2);\n}\n"
+    )
+
+    prototypes = infer_callee_prototypes(source, "sub_88e0", tmp_path)
+
+    assert "extern undefined4 __stdcall sub_7830(unsigned int, unsigned int);" in prototypes
+    assert "extern undefined4 __stdcall sub_7fe0(unsigned int, unsigned int);" in prototypes
+
+
+def test_infer_callee_prototype_infers_non_default_return_type(tmp_path: Path) -> None:
+    _write_callee_candidate(
+        tmp_path,
+        "sub_9000",
+        "409000",
+        "uint __stdcall sub_9000(undefined4 param_1)\n\n{\n  return 1;\n}\n",
+    )
+
+    assert infer_callee_prototype("sub_9000", tmp_path) == "extern uint __stdcall sub_9000(unsigned int);"
+
+
+def test_infer_callee_return_type_falls_back_to_int_when_unparseable() -> None:
+    assert _infer_callee_return_type("// no function signature here\n", "sub_9010") == "int"
+
+
+def test_infer_callee_prototype_skips_callee_with_eight_byte_param(tmp_path: Path) -> None:
+    # sub_9020's real stack cleanup would be 12 bytes (one 4-byte + one
+    # 8-byte param), but stack_bytes // 4 has no way to know that one slot
+    # is 8 bytes -- it would emit 3 plain `unsigned int` params, an arity
+    # mismatch against the real 2-argument call site. Bail instead.
+    _write_callee_candidate(
+        tmp_path,
+        "sub_9020",
+        "409020",
+        "undefined4 __stdcall sub_9020(undefined4 param_1,double param_2)\n\n{\n  return 1;\n}\n",
+    )
+
+    assert infer_callee_prototype("sub_9020", tmp_path) is None