diff --git a/config/regions.tsv b/config/regions.tsv index 92bc9c3..1523225 100644 --- a/config/regions.tsv +++ b/config/regions.tsv @@ -22,3 +22,5 @@ 0x8002D2BC 0x8002D2D4 src/func_8002D2BC.c 0x80036308 0x80036328 src/func_80036308.c 0x800697A4 0x800697C4 src/func_800697A4.c +0x800F8F9C 0x800F8FC0 src/func_800F8F9C.c +0x80109314 0x80109338 src/func_800F8F9C.c diff --git a/docs/MATCHING_CONVENTIONS.md b/docs/MATCHING_CONVENTIONS.md index ec6c5dd..aedc533 100644 --- a/docs/MATCHING_CONVENTIONS.md +++ b/docs/MATCHING_CONVENTIONS.md @@ -94,6 +94,30 @@ An optional third column `gp` marks a symbol the original accessed `gp`-relative harness rewrites that symbol's macro accesses to explicit `%gp_rel(sym)($gp)`, and the linker resolves `R_MIPS_GPREL16` against the `_gp` row. See [PHASE6_SMALL_DATA.md](PHASE6_SMALL_DATA.md). +### Address-named symbols need no row + +A symbol whose **name is an address** resolves to that address automatically. The rule is +`func_XXXXXXXX`, `D_XXXXXXXX`, `g_XXXXXXXX` or `lbl_XXXXXXXX`, where `XXXXXXXX` is eight hex digits: +`func_800F79F0` is defined as `0x800F79F0`. This is the convention the registry already used +(`g_80122354`, `D_8012E2C8`), applied without the row. + +The harness resolves these from the **object's own undefined-symbol list** (`nm -u`), not from a guess +about the source, so a name the source defines but never references is never mistaken for one that +needs resolving. A registry row always wins over the implicit address, which is how a `gp` marker or a +real name is attached. A wrong address cannot pass unnoticed: the byte gate compares the whole binary. + +Anything that is neither in the registry nor address-shaped **fails loudly before the link**, naming +the symbol and saying what to do: + +``` +error: unresolved symbol(s) referenced by src/func_XXXXXXXX.c: v_strlen; add a row to the symbol +registry (NAMEaddress[gp]) or name the symbol func_XXXXXXXX / D_XXXXXXXX so it resolves to +that address +``` + +A name is still earned, not guessed: `func_XXXXXXXX` is a placeholder for an address and says nothing +about what the object means. + Symbols are resolved by the **linker**, not the assembler: the assembler leaves them undefined and emits `%hi`/`%lo` relocations, and the linker applies the HI16 carry adjustment. This is what makes a symbol **address** (`la`) reproduce the original's `addiu` form (cookbook finding 4). diff --git a/phase-ends/CURRENT_PHASE.md b/phase-ends/CURRENT_PHASE.md index fca72ef..776050d 100644 --- a/phase-ends/CURRENT_PHASE.md +++ b/phase-ends/CURRENT_PHASE.md @@ -13,8 +13,8 @@ function bodies, bringing the project past **thirty** distinct byte-identical fu - [x] **P7-T2 — Evidence-graded function extents** (complete) - [x] **P7-T3 — Duplicate-body census** (complete) - [x] **P7-T4 — Candidate triage worklist** (complete) -- [ ] **Rules check** -- [ ] **P7-T5 — Symbol rows at scale** +- [x] **Rules check** — re-read `AGENTS.md` mandatory behavior after P7-T4 and stated the required continuation notice. +- [x] **P7-T5 — Symbol rows at scale** (complete) - [ ] **P7-T6 — Scaled batch A: at least fifteen new bodies** - [ ] **P7-T7 — Scaled batch B: past thirty bodies total** - [ ] **P7-T8 — Cookbook, conventions, verification record, and phase gate** @@ -205,6 +205,40 @@ one task at a time with a checkpoint before ending a session. `Rules check — re-read complete. Continuing with P7-T5.` +## P7-T5 — Symbol rows at scale (2026-09-23) + +**Delivered:** + +- **Implicit address-symbol resolution** in `tools/sf3_match`: a symbol named `func_XXXXXXXX`, + `D_XXXXXXXX`, `g_XXXXXXXX` or `lbl_XXXXXXXX` is defined at that address with no registry row. The + names are read from the **object's own undefined-symbol list** (`nm -u`), not guessed from the source, + so a name the source defines but never references is never mistaken for one needing resolution. A + registry row always wins, which is how a `gp` marker or a real name is attached. +- **Loud failure** on anything else: the harness raises before the link, naming the symbol and saying + what to do, instead of leaving a bare `ld` message. +- `docs/MATCHING_CONVENTIONS.md` §Symbols updated with the rule and the failure text. + +**Verification:** + +| Check | Command | Result | +|---|---|---| +| A registered `la` function needs no registry | `sf3_match range --source src/func_8002D2BC.c` with no `--symbols` | 24 bytes, 0 differing, `MATCH` | +| Unresolvable symbol fails loudly | `sf3_match range` on a source referencing `not_an_address_name` | exit 2, message names the symbol and the fix | +| End-to-end with derived rows only | `sf3_match range --source src/func_800F8F9C.c` | 36 bytes, 0 differing, `MATCH` | +| Full-binary gate | `make gate` | `c_regions=14`, 0 differing bytes, SHA-1 `e173426c…` | +| Synthetic suite | `python3 -m unittest discover -s tools/tests` | **168 tests pass** (8 added) | + +**First match from the worklist:** `func_800F8F9C` (`0x800F8F9C..0x800F8FC0`, 36 bytes) — worklist rank +3, a duplicate-group representative (`g0008`) with a frame and a call. It matched byte-identically on the +first attempt and is registered twice (`0x800F8F9C`, `0x80109314`) against one source, so **two** +functions were matched for one body. Its callee `func_800F79F0` needed no registry row. Ghidra's +independent body `[800f8f9c, 800f8fbf]` agrees with the derived extent — the third independent +confirmation of the extents tool. + +**Limits:** an address-named symbol is a placeholder for an address and says nothing about meaning; a +`gp`-relative access still needs an explicit registry row with the `gp` marker, and without it the byte +gate fails loudly (it cannot fail silently). + ## Notes and limits - The four class items are the plan's P7-T2..T5 work; the single-function items are explicitly out of diff --git a/src/func_800F8F9C.c b/src/func_800F8F9C.c new file mode 100644 index 0000000..34ff470 --- /dev/null +++ b/src/func_800F8F9C.c @@ -0,0 +1,39 @@ +/* + * func_800F8F9C / func_80109314 — 36 bytes each + * + * Byte-identical reconstruction of a small framed wrapper that forwards its + * first argument to a two-argument callee with a fixed first argument. The + * identical 36-byte body occurs at both 0x800F8F9C..0x800F8FC0 and + * 0x80109314..0x80109338, so it is matched once and registered twice against + * this source (the documented N-rows-to-one-source mechanism). This is the + * project's first match taken from the Phase 7 worklist and the first whose + * cross-reference is resolved by the implicit address-symbol rule rather than + * by a hand-written `config/symbols.tsv` row. + * + * The observed instructions are: + * addiu sp,sp,-0x18 frame + * sw ra,0x10(sp) + * move a1,a0 the callee's second argument is this function's first + * jal 0x800F79F0 + * li a0,3 delay slot: the callee's first argument is the constant 3 + * lw ra,0x10(sp) + * addiu sp,sp,0x18 + * jr ra + * nop + * + * `func_800F79F0` is named by its address only: the implicit address-symbol rule + * resolves `func_XXXXXXXX` to that address, so no registry row is needed. The + * argument types and the callee's meaning are hypotheses. + * + * LIMITS: the function names are address placeholders and the parameter types + * are inferred from register usage alone. Only the compiled bytes are evidence. + * The two addresses are treated as two function entries because each is a + * distinct `jal` target with its own `jr ra`; this is an evidence judgement, not + * a proven original symbol table. + */ + +extern void func_800F79F0(int first, int second); + +void func_800F8F9C(int value) { + func_800F79F0(3, value); +} diff --git a/tools/sf3_match b/tools/sf3_match index d0749b8..e5f68d2 100755 --- a/tools/sf3_match +++ b/tools/sf3_match @@ -60,6 +60,14 @@ _BINUTILS = REPO_ROOT / "tools/mipsel-none-elf-binutils/prefix/usr/bin" DEFAULT_AS = _BINUTILS / "mipsel-none-elf-as" DEFAULT_LD = _BINUTILS / "mipsel-none-elf-ld" DEFAULT_OBJCOPY = _BINUTILS / "mipsel-none-elf-objcopy" +DEFAULT_NM = _BINUTILS / "mipsel-none-elf-nm" + +# A symbol whose name is an address is a placeholder for that address, so it +# needs no registry row: `func_80017AD4` resolves to 0x80017AD4. The convention +# is already used by the registry (`g_80122354`, `D_8012E2C8`). A wrong address +# cannot pass unnoticed -- the byte gate compares the whole binary -- so the only +# cost of an implicit symbol is a loud mismatch, never a silent one. +ADDRESS_SYMBOL = re.compile(r"^(?:func|D|g|lbl)_([0-9A-Fa-f]{8})$") DEFAULT_CPP_FLAGS = ["-E", "-P", "-undef"] DEFAULT_CC1_FLAGS = ["-quiet", "-O2", "-G0"] @@ -413,6 +421,7 @@ class Toolchain: gp_symbols: frozenset[str] = frozenset() maspsx: Path | None = None aspsx_version: str = DEFAULT_ASPSX_VERSION + nm: Path | None = None # A symbol macro access that can be forced gp-relative. @@ -568,6 +577,8 @@ def command_range(args: argparse.Namespace) -> int: object_path = work / "candidate.o" tools = args.toolchain compile_c(source, object_path, work, tools) + tools = replace(tools, defsyms=resolve_undefined_symbols( + tools.defsyms, undefined_symbols(object_path, tools), str(source))) candidate = link_object_bytes(object_path, args.start, work, tools) expected_length = args.end - args.start @@ -633,6 +644,10 @@ def _build(args: argparse.Namespace) -> tuple[Path, Path]: ) tools = args.toolchain + # Symbols an address-named placeholder can satisfy are added as the objects + # that reference them are built; anything else fails loudly, naming the + # symbol, instead of leaving a bare linker error. + link_defsyms = list(tools.defsyms) by_start = {region.start: region for region in regions} objects: list[str] = ["header.o"] assemble_asm(header, out / "header.o", tools) @@ -649,13 +664,15 @@ def _build(args: argparse.Namespace) -> tuple[Path, Path]: ) compile_c(item.source, object_path, work, region_tools) localize_symbols(object_path, tools) + link_defsyms = resolve_undefined_symbols( + link_defsyms, undefined_symbols(object_path, tools), item.source) objects.append(item.object_name) script = out / "link.ld" script.write_text(linker_script(items, exe.text_address), encoding="ascii") elf = out / "scus_946_40.elf" link_command = [str(tools.linker), "-T", script.name, "--no-check-sections"] - for symbol in tools.defsyms: + for symbol in link_defsyms: link_command += ["--defsym", symbol] link_command += ["-o", elf.name, *objects] run(link_command, cwd=out) @@ -720,6 +737,8 @@ def add_toolchain_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument("--assembler", type=Path, default=DEFAULT_AS) parser.add_argument("--linker", type=Path, default=DEFAULT_LD) parser.add_argument("--objcopy", type=Path, default=DEFAULT_OBJCOPY) + parser.add_argument("--nm", type=Path, default=DEFAULT_NM, + help="nm, used to resolve address-named symbols implicitly") parser.add_argument("--maspsx", type=Path, default=DEFAULT_MASPSX, help="ASPSX emulator run between cc1 and the assembler") parser.add_argument("--no-maspsx", action="store_true", @@ -738,6 +757,61 @@ def add_toolchain_arguments(parser: argparse.ArgumentParser) -> None: help="tracked symbol registry file (NAMEaddress rows)") +def undefined_symbols(object_path: Path, tools: Toolchain) -> set[str]: + """The symbols one assembled object still needs defined. + + Read from the object itself rather than guessed from its C source, so a + symbol the source defines but never references is not mistaken for one that + needs resolving. + """ + if tools.nm is None: + return set() + completed = subprocess.run( + [str(tools.nm), "-u", str(object_path)], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + ) + if completed.returncode: + text = completed.stderr.decode("utf-8", "replace").strip().splitlines() + message = text[0] if text else "no diagnostic" + raise ToolError(f"nm failed ({completed.returncode}): {message}") + names: set[str] = set() + for line in completed.stdout.decode("utf-8", "replace").splitlines(): + fields = line.split() + if fields: + names.add(fields[-1]) + return names + + +def resolve_undefined_symbols(defsyms: Sequence[str], undefined: set[str], + label: str) -> list[str]: + """Add an address-named placeholder for each undefined symbol; fail loudly otherwise. + + A name that is neither in the registry nor shaped like an address cannot be + resolved, and the link would fail with a bare `ld` message. Failing here + instead names the symbol and says what to do about it. + """ + known = {defsym.split("=", 1)[0] for defsym in defsyms} + resolved = list(defsyms) + unresolved: list[str] = [] + for name in sorted(undefined): + if name in known: + continue + match = ADDRESS_SYMBOL.match(name) + if match is not None: + resolved.append(f"{name}=0x{int(match.group(1), 16):X}") + known.add(name) + else: + unresolved.append(name) + if unresolved: + listed = ", ".join(unresolved) + raise ToolError( + f"unresolved symbol(s) referenced by {label}: {listed}; add a row to the symbol " + "registry (NAMEaddress[gp]) or name the symbol func_XXXXXXXX / " + "D_XXXXXXXX so it resolves to that address" + ) + return resolved + + def resolve_toolchain(args: argparse.Namespace) -> Toolchain: cpp = args.cpp if cpp is None: @@ -761,6 +835,7 @@ def resolve_toolchain(args: argparse.Namespace) -> Toolchain: assembler=require_file(args.assembler, "assembler"), linker=require_file(args.linker, "linker"), objcopy=require_file(args.objcopy, "objcopy"), + nm=require_file(args.nm, "nm") if args.nm is not None else None, cpp_flags=args.cpp_flag or DEFAULT_CPP_FLAGS, cc1_flags=args.cc1_flag or DEFAULT_CC1_FLAGS, as_flags=args.as_flag or DEFAULT_AS_FLAGS, diff --git a/tools/tests/test_sf3_match.py b/tools/tests/test_sf3_match.py index 46cd178..b1954a2 100644 --- a/tools/tests/test_sf3_match.py +++ b/tools/tests/test_sf3_match.py @@ -7,8 +7,10 @@ The end-to-end tests skip cleanly when the ignored local toolchain is absent. from __future__ import annotations +import contextlib import importlib.machinery import importlib.util +import io from pathlib import Path import struct import sys @@ -197,6 +199,43 @@ class RegionOptionTests(unittest.TestCase): sf3_match.parse_regions("0x80010000 0x80010010 src/a.c\tcc1=-O0 extra\n") +class AddressSymbolTests(unittest.TestCase): + """Implicit resolution of address-named symbols, and the loud failure otherwise.""" + + def test_an_address_named_symbol_resolves_to_its_address(self) -> None: + resolved = sf3_match.resolve_undefined_symbols([], {"func_80017AD4"}, "src/x.c") + self.assertEqual(resolved, ["func_80017AD4=0x80017AD4"]) + + def test_every_documented_prefix_resolves(self) -> None: + names = {"func_80010010", "D_8012E2C8", "g_80122354", "lbl_800FB368"} + resolved = sf3_match.resolve_undefined_symbols([], names, "src/x.c") + self.assertEqual(resolved, [ + "D_8012E2C8=0x8012E2C8", + "func_80010010=0x80010010", + "g_80122354=0x80122354", + "lbl_800FB368=0x800FB368", + ]) + + def test_a_registry_row_wins_over_the_implicit_address(self) -> None: + resolved = sf3_match.resolve_undefined_symbols( + ["g_80122354=0x80129999"], {"g_80122354"}, "src/x.c") + self.assertEqual(resolved, ["g_80122354=0x80129999"]) + + def test_a_name_that_is_not_an_address_fails_loudly(self) -> None: + with self.assertRaises(sf3_match.ToolError) as caught: + sf3_match.resolve_undefined_symbols([], {"v_strlen"}, "src/x.c") + self.assertIn("v_strlen", str(caught.exception)) + self.assertIn("symbol registry", str(caught.exception)) + + def test_a_misshapen_address_name_is_not_resolved(self) -> None: + with self.assertRaises(sf3_match.ToolError): + sf3_match.resolve_undefined_symbols([], {"func_80017AD"}, "src/x.c") + + def test_undefined_symbols_is_empty_without_nm(self) -> None: + tools = type("Tools", (), {"nm": None})() + self.assertEqual(sf3_match.undefined_symbols(Path("absent.o"), tools), set()) + + class SymbolTests(unittest.TestCase): def test_parses_names_comments_and_hex(self) -> None: table = sf3_match.parse_symbols( @@ -257,6 +296,7 @@ class EndToEndTests(unittest.TestCase): args.assembler = sf3_match.DEFAULT_AS args.linker = sf3_match.DEFAULT_LD args.objcopy = sf3_match.DEFAULT_OBJCOPY + args.nm = sf3_match.DEFAULT_NM args.cpp_flag = [] args.cc1_flag = list(cc1_flags) args.as_flag = list(as_flags) @@ -425,6 +465,36 @@ class EndToEndTests(unittest.TestCase): "--symbols", str(symbols), "--out", str(root / "out")]) self.assertEqual(rc, 2) + def test_an_address_named_symbol_needs_no_registry_row(self) -> None: + """A `D_XXXXXXXX` reference resolves to its own address, with no registry at all.""" + source = "extern int D_80123456;\nint synthetic_load(void) { return D_80123456; }\n" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + exe, code_length = self._compile_payload( + root, source_text=source, defsyms=["D_80123456=0x80123456"] + ) + regions = self._registry(root, code_length) + out = root / "out" + rc = sf3_match.main(["gate", "--exe", str(exe), "--regions", str(regions), + "--out", str(out)]) + self.assertEqual(rc, 0) + self.assertEqual((out / "scus_946_40.rebuilt").read_bytes(), exe.read_bytes()) + + def test_an_unresolvable_symbol_names_itself(self) -> None: + source = "extern int v_strlen_count;\nint synthetic_load(void) { return v_strlen_count; }\n" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + exe, code_length = self._compile_payload( + root, source_text=source, defsyms=["v_strlen_count=0x80123456"] + ) + regions = self._registry(root, code_length) + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + rc = sf3_match.main(["gate", "--exe", str(exe), "--regions", str(regions), + "--out", str(root / "out")]) + self.assertEqual(rc, 2) + self.assertIn("v_strlen_count", stderr.getvalue()) + def test_per_region_flag_override_changes_codegen(self) -> None: source = "int synthetic_mul(int a, int b) { return (a + 1) * (b - 2); }\n" with tempfile.TemporaryDirectory() as tmp: