diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 17be11e8a..7fa83f24e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -26,6 +26,27 @@ jobs: - name: Git config run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + # Determine which .cpp files changed (PR: vs base branch; push: vs prev commit) + - name: Changed .cpp files + run: | + if [ "${{ github.event_name }}" = "pull_request" ]; then + base="${{ github.event.pull_request.base.sha }}" + else + base="${{ github.event.before }}" + fi + if [ -z "$base" ] \ + || [ "$base" = "0000000000000000000000000000000000000000" ] \ + || ! git rev-parse -q --verify "$base^{commit}" >/dev/null 2>&1; then + base="$(git rev-parse -q --verify HEAD~1 || true)" + fi + if [ -n "$base" ]; then + git diff --name-only --diff-filter=d "$base" HEAD -- '*.cpp' \ + > changed_cpp.txt + else + : > changed_cpp.txt + fi + echo "Changed .cpp files:"; cat changed_cpp.txt + # Normalize file mod times - name: Restore timestamps run: | @@ -67,3 +88,22 @@ jobs: with: name: ${{ matrix.version }}_report path: build/${{ matrix.version }}/report.json + + # Validate map symbol presence/order/linkage for each changed .cpp + - name: Validate symbol order + run: | + if [ ! -s changed_cpp.txt ]; then + echo "No changed .cpp files; nothing to validate." + exit 0 + fi + NM="$(command -v powerpc-eabi-nm || true)" + if [ -z "$NM" ]; then + NM="$(find /binutils -name powerpc-eabi-nm -type f 2>/dev/null | head -1)" + fi + if [ -z "$NM" ] || [ ! -x "$NM" ]; then + echo "::error:: powerpc-eabi-nm not found; cannot run symbol-order check." + exit 1 + fi + export NM + echo "Using nm: $NM" + python tools/check-changed-symbol-order.py $(cat changed_cpp.txt) diff --git a/src/Enemy/riccohook.cpp b/src/Enemy/riccohook.cpp index efa6e7717..bb0d63dbc 100644 --- a/src/Enemy/riccohook.cpp +++ b/src/Enemy/riccohook.cpp @@ -4,6 +4,23 @@ #include #include +// @non-matching -- the issue seems to stem from the JDrama TNameRefGen +// search/push_back calls. +THookTake::THookTake(TRiccoHook* owner, const char* name) + : TTakeActor(name) + , mOwner(owner) +{ + initHitActor(0x400000BB, 1, -0x80000000, + mOwner->getSaveLoadParam()->mSLHitRadius.get(), + mOwner->getSaveLoadParam()->mSLHitHeight.get(), + mOwner->getSaveLoadParam()->mSLHitRadius.get(), + mOwner->getSaveLoadParam()->mSLHitHeight.get()); + + JDrama::TNameRefGen::search("オブジェクトグループ") + ->getChildren() + .push_back(this); +} + MtxPtr THookTake::getTakingMtx() { return nullptr; } f32 THookTake::getRadiusAtY(f32 y) const @@ -42,21 +59,14 @@ void THookTake::perform(u32 cue, JDrama::TGraphics* graphics) } } -// @non-matching -- the issue seems to stem from the JDrama TNameRefGen -// search/push_back calls. -THookTake::THookTake(TRiccoHook* owner, const char* name) - : TTakeActor(name) - , mOwner(owner) +THookParams::THookParams(const char* path) + : TSpineEnemyParams(path) + , PARAM_INIT(mSLHitHeight, 900.0f) + , PARAM_INIT(mSLHitRadius, 120.0f) + , PARAM_INIT(mSLHangRadius, 30.0f) + , PARAM_INIT(mSLMoveSpeed, 4.0f) { - initHitActor(0x400000BB, 1, -0x80000000, - mOwner->getSaveLoadParam()->mSLHitRadius.get(), - mOwner->getSaveLoadParam()->mSLHitHeight.get(), - mOwner->getSaveLoadParam()->mSLHitRadius.get(), - mOwner->getSaveLoadParam()->mSLHitHeight.get()); - - JDrama::TNameRefGen::search("オブジェクトグループ") - ->getChildren() - .push_back(this); + TParams::load(mPrmPath); } TRiccoHook::TRiccoHook(const char* name) @@ -96,16 +106,6 @@ void TRiccoHook::perform(u32 cue, JDrama::TGraphics* graphics) } } -THookParams::THookParams(const char* path) - : TSpineEnemyParams(path) - , PARAM_INIT(mSLHitHeight, 900.0f) - , PARAM_INIT(mSLHitRadius, 120.0f) - , PARAM_INIT(mSLHangRadius, 30.0f) - , PARAM_INIT(mSLMoveSpeed, 4.0f) -{ - TParams::load(mPrmPath); -} - TRiccoHookManager::TRiccoHookManager(const char* name) : TEnemyManager(name) { diff --git a/src/Enemy/spline.cpp b/src/Enemy/spline.cpp index 080d9dca2..e921d420d 100644 --- a/src/Enemy/spline.cpp +++ b/src/Enemy/spline.cpp @@ -20,6 +20,9 @@ TSplinePath::TSplinePath(int param_1) mXs[i] = mYs[i] = mZs[i] = i; } +// BUG: UNUSED symbol size tells us they even forgot to delete the arrays -_- +TSplinePath::~TSplinePath() { } + void TSplinePath::makeTable(f32* parametrization, f32* coords, f32* param_3) { param_3[0] = 0.0f; diff --git a/src/MSound/MSModBgm.cpp b/src/MSound/MSModBgm.cpp index 182942f8b..840306e77 100644 --- a/src/MSound/MSModBgm.cpp +++ b/src/MSound/MSModBgm.cpp @@ -48,6 +48,21 @@ JAISound* MSModBgm::modBgm(u8 param_1, u8 param_2) return sound; } +void MSModBgm::loop() +{ + switch (unk0) { + case true: + unk4 += 1; + break; + + case false: + default: + unk4 = 0; + return; + } + unk0 = 0; +} + void MSModBgm::changeTempo(u8 param_1, u8 param_2) { JAISound* sound = MSBgm::getHandle(param_2); @@ -77,21 +92,6 @@ void MSModBgm::changeTempo(u8 param_1, u8 param_2) } } -void MSModBgm::loop() -{ - switch (unk0) { - case true: - unk4 += 1; - break; - - case false: - default: - unk4 = 0; - return; - } - unk0 = 0; -} - f32 MSBgmXFade::scTiming[18] = { 0.052632f, 0.105263f, 0.157895f, 0.210526f, 0.26315799f, 0.315789f, 0.36842099f, 0.42105299f, 0.473684f, 0.52631599f, 0.578947f, 0.63157898f, @@ -126,15 +126,6 @@ void MSBgmXFade::xFadeBgmForce(f32 param_1) } } -u8 MSBgmXFade::getTimingForce(f32 param_1) -{ - for (u8 i = 0; i < 17; ++i) - if (param_1 >= scTiming[i] && param_1 < scTiming[i + 1]) - return i; - - return 0xff; -} - u8 MSBgmXFade::getTiming(f32 param_1, u32* param_2) { f32 f1 = unk0; @@ -147,3 +138,12 @@ u8 MSBgmXFade::getTiming(f32 param_1, u32* param_2) return 0xff; } + +u8 MSBgmXFade::getTimingForce(f32 param_1) +{ + for (u8 i = 0; i < 17; ++i) + if (param_1 >= scTiming[i] && param_1 < scTiming[i + 1]) + return i; + + return 0xff; +} diff --git a/src/MSound/MSoundDebug.cpp b/src/MSound/MSoundDebug.cpp index e280c29a3..e29094929 100644 --- a/src/MSound/MSoundDebug.cpp +++ b/src/MSound/MSoundDebug.cpp @@ -6,8 +6,6 @@ static void dummy() { JADPrm prm(0, ""); } -MSD* MSD::get() { return 0; } - MSPrintBase::MSPrintBase() { } MSPrintBase::MSPrintBase(u8 param1, u8 param2, u8 param3, u8 param4, u8 param5) @@ -25,3 +23,6 @@ void MSPrintEntry::print() { } MSPrint::MSPrint() { } void MSPrint::print() { } + +// UNUSED +MSD* MSD::get() { return 0; } diff --git a/src/MSound/MSoundScene.cpp b/src/MSound/MSoundScene.cpp index cf17ae2a1..98973f98f 100644 --- a/src/MSound/MSoundScene.cpp +++ b/src/MSound/MSoundScene.cpp @@ -108,8 +108,8 @@ void MSSceneSE::sortMaxTrans(Vec* param_1, u8 param_2, u8 param_3) } } -void MSSceneSE::calcPosPanSR(Vec* param_1, f32 param_2) { } +void MSSceneSE::calcPosVolume(Vec* param_1, f32* param_2, u8 param_3) { } void MSSceneSE::calcPosPanLR(Vec* param_1, f32 param_2) { } -void MSSceneSE::calcPosVolume(Vec* param_1, f32* param_2, u8 param_3) { } +void MSSceneSE::calcPosPanSR(Vec* param_1, f32 param_2) { } diff --git a/src/MarioUtil/GDUtil.cpp b/src/MarioUtil/GDUtil.cpp index efdde84b7..e0a239f2f 100644 --- a/src/MarioUtil/GDUtil.cpp +++ b/src/MarioUtil/GDUtil.cpp @@ -4,7 +4,7 @@ static TGDLStatic* currentTGDLStatic; -void TGDLStaticOverFlow() { currentTGDLStatic->unk11 = 1; } +static void TGDLStaticOverFlow() { currentTGDLStatic->unk11 = 1; } TGDLStatic::TGDLSentinel::~TGDLSentinel() { diff --git a/src/MarioUtil/RumbleType.cpp b/src/MarioUtil/RumbleType.cpp index 1b5800511..fa549a2fd 100644 --- a/src/MarioUtil/RumbleType.cpp +++ b/src/MarioUtil/RumbleType.cpp @@ -39,16 +39,7 @@ struct RumbleTypeInfo _info[23] = { const char* invalidStr = "Invalid RumbleType"; -int RumbleType::getIndex(char* strIn) -{ - for (int i = 0; i < channelNum; i++) { - if (!strcmp(strIn, _info[i].str)) { - return _info[i].unk0; - } - } - return -1; -} - +// UNUSED // Size needed: 0x54, current: 0x34 const char* RumbleType::getName(int index) { @@ -58,3 +49,13 @@ const char* RumbleType::getName(int index) return _info[index].str; } + +int RumbleType::getIndex(char* strIn) +{ + for (int i = 0; i < channelNum; i++) { + if (!strcmp(strIn, _info[i].str)) { + return _info[i].unk0; + } + } + return -1; +} diff --git a/src/Player/MarioCap.cpp b/src/Player/MarioCap.cpp index bac030a3f..2b2153779 100644 --- a/src/Player/MarioCap.cpp +++ b/src/Player/MarioCap.cpp @@ -118,6 +118,9 @@ void TMarioCap::createMirrorCap() } } +// UNUSED +void TMarioCap::addDirty() { } + void TMarioCap::perform(unsigned long param_1, JDrama::TGraphics* param_2) { // Unused stack space @@ -227,16 +230,14 @@ void TMarioCap::perform(unsigned long param_1, JDrama::TGraphics* param_2) } } -void TMarioCap::mtxEffectHide() -{ - unk20->flagOff(0x1); - unk24->flagOff(0x1); -} - void TMarioCap::mtxEffectShow() { unk20->flagOn(0x1); unk24->flagOn(0x1); } -void TMarioCap::addDirty() { } +void TMarioCap::mtxEffectHide() +{ + unk20->flagOff(0x1); + unk24->flagOff(0x1); +} diff --git a/src/System/ProcessMeter.cpp b/src/System/ProcessMeter.cpp index c19d7e90e..e932987ab 100644 --- a/src/System/ProcessMeter.cpp +++ b/src/System/ProcessMeter.cpp @@ -12,3 +12,6 @@ TProcessMeter::TProcessMeter(s32 r4) _01C = 0; _01E = 0; } + +// UNUSED +void TProcessMeter::draw(const JUTRect&) { } diff --git a/src/System/Resolution.cpp b/src/System/Resolution.cpp index 4f8b38af6..59abed7bd 100644 --- a/src/System/Resolution.cpp +++ b/src/System/Resolution.cpp @@ -21,6 +21,12 @@ u16 SMSGetGameVideoHeight() return ret; } +// UNUSED +s32 SMSGetDisplayWidthMax() { return 720; } + +// UNUSED +s32 SMSGetDisplayHeightMax() { return 574; } + u16 SMSGetTitleVideoWidth() { return 660; } u16 SMSGetTitleVideoHeight() { return SMSGetGameVideoHeight(); } diff --git a/src/System/StageUtil.cpp b/src/System/StageUtil.cpp index fa14f1142..a937ef628 100644 --- a/src/System/StageUtil.cpp +++ b/src/System/StageUtil.cpp @@ -23,6 +23,9 @@ static u8 exShineTable[] = { // clang-format on +// UNUSED +bool SMS_isExMap(u8 stage) { return stage > 0x14 && stage < 0x35; } + bool SMS_isExMap() { return (gpApplication.mCurrArea.unk0 > 0x14 diff --git a/tools/check-changed-symbol-order.py b/tools/check-changed-symbol-order.py new file mode 100644 index 000000000..88f09a527 --- /dev/null +++ b/tools/check-changed-symbol-order.py @@ -0,0 +1,90 @@ +#!/usr/bin/env python3 + +"""CI driver: run validate-symbol-order.py for each changed .cpp file. + +Given a list of changed paths (as arguments), map each .cpp to its decomp unit +via objdiff.json and run the symbol-order check on it. Files that aren't tracked +units -- new files, headers, SDK/JSystem sources without a built object, units +whose map TU can't be resolved -- are skipped, not failed. + +Exit status: + 0 every checked unit passed (or nothing to check) + 1 at least one checked unit failed its validation + +Usage: + python tools/check-changed-symbol-order.py src/Foo/Bar.cpp src/Baz/Qux.cpp + git diff --name-only BASE HEAD -- '*.cpp' | xargs python tools/check-changed-symbol-order.py +""" + +import json +import os +import subprocess +import sys + +script_dir = os.path.dirname(os.path.realpath(__file__)) +root_dir = os.path.abspath(os.path.join(script_dir, "..")) +VALIDATOR = os.path.join(script_dir, "validate-symbol-order.py") +OBJDIFF_JSON = os.path.join(root_dir, "objdiff.json") + + +def norm(p: str) -> str: + return p.replace("\\", "/").lstrip("./") + + +def source_to_unit() -> dict: + with open(OBJDIFF_JSON, encoding="utf-8") as f: + units = json.load(f).get("units", []) + out = {} + for u in units: + sp = u.get("metadata", {}).get("source_path") + if sp: + out[norm(sp)] = (u["name"], u.get("base_path", "")) + return out + + +def main(argv) -> int: + files = [norm(a) for a in argv if a.strip()] + cpp = [f for f in files if f.endswith(".cpp")] + if not cpp: + print("No changed .cpp files to check.") + return 0 + + mapping = source_to_unit() + passed, failed, skipped = [], [], [] + + for f in cpp: + entry = mapping.get(f) + if entry is None: + print(f"skip {f} (not a tracked decomp unit)") + skipped.append(f) + continue + unit, base = entry + if base and not os.path.exists(os.path.join(root_dir, base)): + print(f"skip {f} ({unit}: object not built)") + skipped.append(f) + continue + + print(f"\n{'=' * 72}\n{f} -> {unit}\n{'=' * 72}") + rc = subprocess.run([sys.executable, VALIDATOR, "-u", unit], + cwd=root_dir).returncode + if rc == 0: + passed.append(f) + elif rc == 2: + print(f"skip {f} ({unit}: check could not run -- see message above)") + skipped.append(f) + else: + failed.append((f, unit)) + + print(f"\n{'=' * 72}") + print(f"Symbol-order check: {len(passed)} passed, {len(failed)} failed, " + f"{len(skipped)} skipped.") + if failed: + print("\nFailed:") + for f, unit in failed: + print(f" - {f} ({unit})") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/tools/validate-symbol-order.py b/tools/validate-symbol-order.py new file mode 100644 index 000000000..cd14d796a --- /dev/null +++ b/tools/validate-symbol-order.py @@ -0,0 +1,447 @@ +#!/usr/bin/env python3 + +""" +Validate that a translation unit's function symbol order in our compiled object +file matches the order recorded in the original linker map (mario.MAP). + +The linker map's ".text section layout" is the ground truth: it lists every +function a TU contributed, in emission order, *including* deadstripped ones +(marked UNUSED). A correct decompilation must reproduce that exact order, must +not be missing any of those symbols, and must emit the UNUSED ones at the size +the map records for them (UNUSED symbols have no counterpart in the target +object, so objdiff can't check them -- the map size is the only signal we have). + +What is checked: + * MISSING (ERROR) -- a map symbol (used or UNUSED) absent from our object. + Nothing stops you from just defining the function. + * ORDER (ERROR) -- non-weak map symbols present in our object but in the + wrong relative order (extra object-only symbols + ignored). Reordering the definitions to match is + mechanical. A disorder involving ONLY weak symbols is a + WARNING instead (compiler-controlled, won't block a PR). + * BINDING (ERROR) -- a *placed* (linked) symbol whose weak/local/global + linkage disagrees with the map's symbol-closure section + (e.g. we defined an out-of-line global that should be a + weak header inline, or dropped a `static`). UNUSED + symbols are skipped: dead-stripping removes weak, local + and global symbols alike and records no binding, so the + linkage of an UNUSED symbol is unknowable from the map. + * SIZE (WARNING) -- an UNUSED map symbol whose size in our object differs + from the size the map records. Reproducing an inlined + body's exact size is genuinely hard, so this only warns. + +Exit status is non-zero only for ERRORs (MISSING/ORDER/BINDING). SIZE warnings +on their own still exit 0. + +Note on "-inline deferred" / reverse_fn_order: empirically the map and our +object are ALWAYS in the same forward order (verified across every unit), so +comparison is forward by default. `--reverse` is provided as a manual escape +hatch should a genuinely reversed unit ever turn up. + +Usage: + python tools/validate-symbol-order.py -u mario/MarioUtil/MathUtil + python tools/validate-symbol-order.py -u mario/Enemy/areacylinder --map orig/GMSE01/files/marioUS.MAP +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from typing import Dict, List, Optional, Tuple + +script_dir = os.path.dirname(os.path.realpath(__file__)) +root_dir = os.path.abspath(os.path.join(script_dir, "..")) + +DEFAULT_MAP = os.path.join(root_dir, "orig", "GMSJ01", "files", "mario.MAP") +NM = os.environ.get("NM", os.path.join(root_dir, "build", "binutils", "powerpc-eabi-nm.exe")) +OBJDIFF_JSON = os.path.join(root_dir, "objdiff.json") + + +def die(msg: str) -> None: + """Exit code 2 == cannot run the check (unresolved unit, missing object or + map, absent nm). Distinct from a genuine validation failure (exit 1), so a + CI driver can tell 'skip this file' apart from 'this file is wrong'.""" + print(msg, file=sys.stderr) + sys.exit(2) + + +class MapSym: + __slots__ = ("name", "size", "unused") + + def __init__(self, name: str, size: int, unused: bool): + self.name = name + self.size = size + self.unused = unused + + +def load_units() -> List[Dict]: + with open(OBJDIFF_JSON, encoding="utf-8") as f: + return json.load(f).get("units", []) + + +def resolve_unit(units: List[Dict], arg: str) -> Dict: + """Resolve a unit by exact name, then by suffix, matching decomp-diff -u.""" + for u in units: + if u.get("name") == arg: + return u + matches = [u for u in units if u.get("name", "").endswith(arg)] + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + names = "\n ".join(m["name"] for m in matches) + die(f"Ambiguous unit '{arg}', candidates:\n {names}") + die(f"Unit not found in objdiff.json: {arg}") + + +# --------------------------------------------------------------------------- +# Map parsing +# --------------------------------------------------------------------------- + +# UNUSED line: " UNUSED 00005c ........ GetAtanTable__Fff MarioUtil.a MathUtil.cpp" +_RE_UNUSED = re.compile(r"^\s*UNUSED\s+([0-9a-fA-F]+)\s+\.+\s+(\S+)\s+(.+?)\s*$") +# Placed line: " 000c01d0 000028 800c57d0 4 MsVECMag2__FP3Vec \t MarioUtil.a MathUtil.cpp" +_RE_PLACED = re.compile( + r"^\s*([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+(\d+)\s+(\S+)\s+(.+?)\s*$" +) + + +def parse_text_layout(map_path: str) -> Dict[str, List[MapSym]]: + """Return {tu_id: [MapSym,...]} for the .text section layout (in map order).""" + if not os.path.exists(map_path): + die(f"Map file not found: {map_path}") + with open(map_path, encoding="utf-8", errors="replace") as f: + lines = f.read().splitlines() + + start = None + for i, l in enumerate(lines): + if l.strip() == ".text section layout": + start = i + 1 + break + if start is None: + die(f"No '.text section layout' section in {map_path}") + end = len(lines) + for i in range(start, len(lines)): + if lines[i].strip().endswith("section layout"): + end = i + break + + tus: Dict[str, List[MapSym]] = {} + for l in lines[start:end]: + if not l.strip(): + continue + m = _RE_UNUSED.match(l) + if m: + tus.setdefault(m.group(3).strip(), []).append( + MapSym(m.group(2), int(m.group(1), 16), True) + ) + continue + m = _RE_PLACED.match(l) + if m: + name = m.group(5) + if name == ".text": # per-TU section header, not a symbol + continue + tus.setdefault(m.group(6).strip(), []).append( + MapSym(name, int(m.group(2), 16), False) + ) + return tus + + +def find_map_tu(tus: Dict[str, List[MapSym]], source_path: str, + override: Optional[str]) -> str: + if override: + if override not in tus: + die(f"--map-tu '{override}' not found in .text layout") + return override + base = os.path.basename(source_path) # e.g. MathUtil.cpp + stem = os.path.splitext(base)[0] # e.g. MathUtil + cands = [ + t for t in tus + if t.endswith(" " + base) or t == base or t == stem + ".o" + ] + if len(cands) == 1: + return cands[0] + if not cands: + die( + f"Could not find a .text-layout TU for source '{source_path}'.\n" + f"Pass --map-tu with the exact map identifier (e.g. 'MarioUtil.a {base}')." + ) + listed = "\n ".join(cands) + die( + f"Multiple map TUs match '{base}'. Disambiguate with --map-tu:\n {listed}" + ) + + +# --------------------------------------------------------------------------- +# Object file parsing (our compiled code) -- true emission order via nm +# --------------------------------------------------------------------------- + +# nm -S --numeric-sort: "00000000 00000028 T MsVECMag2__FP3Vec" +_RE_NM = re.compile(r"^([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+([a-zA-Z])\s+(\S.*)$") + + +# nm type letter -> linkage/binding, matching the map's (func,) tags. +_NM_BIND = {"T": "global", "t": "local", "W": "weak", "w": "weak"} + + +def obj_functions(obj_path: str) -> List[Tuple[str, int, str]]: + """[(name, size, binding), ...] for .text symbols in emission order. + + binding is 'global' | 'local' | 'weak', derived from the nm type letter. + """ + if not os.path.exists(obj_path): + die( + f"Object not found: {obj_path}\n" + f"Build it first (e.g. `ninja {obj_path}`)." + ) + if not os.path.exists(NM): + die(f"nm not found: {NM} (set $NM to override)") + out = subprocess.run( + [NM, "-S", "--numeric-sort", "--defined-only", obj_path], + capture_output=True, text=True, + ) + if out.returncode != 0: + die(f"nm failed: {out.stderr.strip()}") + syms: List[Tuple[str, int, str]] = [] + for line in out.stdout.splitlines(): + m = _RE_NM.match(line) + if not m: + continue + size, typ, name = int(m.group(2), 16), m.group(3), m.group(4) + # t/T = text (local/global); w/W = weak (functions & thunks live here). + # Weak *objects* are v/V, data is d/D/b/B/r/R -> excluded. + if typ in _NM_BIND: + syms.append((name, size, _NM_BIND[typ])) + return syms + + +# --------------------------------------------------------------------------- +# Linker-map symbol closure -- the authority on weak/global/local binding. +# Only *linked* symbols appear here; deadstripped (UNUSED) ones never do, which +# is exactly why UNUSED symbols are non-weak by construction. +# --------------------------------------------------------------------------- + +# " 3] __dt__26__partial_array_destructorFv (func,weak) found in Runtime..." +_RE_CLOSURE = re.compile( + r"^\s*(?:\d+\]\s*)?(\S+)\s+\((\w+),(\w+)\)\s+found in\s+(.+?)\s*$" +) + + +def parse_closure(map_path: str) -> Tuple[Dict[Tuple[str, str], str], Dict[str, str]]: + """Return (binding_by_tu_name, binding_by_name) for func symbols. + + binding_by_name maps a name to its binding only when unambiguous across + every TU that defines it (so weak/global resolve cleanly; a name that is + local in several TUs still collapses to 'local'). + """ + with open(map_path, encoding="utf-8", errors="replace") as f: + lines = f.read().splitlines() + + by_tu: Dict[Tuple[str, str], str] = {} + seen: Dict[str, set] = {} + for l in lines: + if l.strip().endswith("section layout"): + break # closure precedes all the section layouts + m = _RE_CLOSURE.match(l) + if not m or m.group(2) != "func": + continue + name, binding, tu = m.group(1), m.group(3), m.group(4).strip() + by_tu[(tu, name)] = binding + seen.setdefault(name, set()).add(binding) + + by_name = {n: next(iter(b)) for n, b in seen.items() if len(b) == 1} + return by_tu, by_name + + +# --------------------------------------------------------------------------- +# Comparison + reporting +# --------------------------------------------------------------------------- + +def order_diff(expected: List[str], actual: List[str]) -> List[str]: + """Readable unified-style diff of two orderings of the same symbol set.""" + import difflib + + sm = difflib.SequenceMatcher(a=expected, b=actual, autojunk=False) + out: List[str] = [] + for tag, i1, i2, j1, j2 in sm.get_opcodes(): + if tag == "equal": + for n in expected[i1:i2]: + out.append(f" {n}") + else: + for n in expected[i1:i2]: + out.append(f" - {n} (map position)") + for n in actual[j1:j2]: + out.append(f" + {n} (object position)") + return out + + +def main() -> None: + ap = argparse.ArgumentParser( + description="Validate TU function symbol order against the linker map." + ) + ap.add_argument("-u", "--unit", required=True, + help="objdiff unit name, e.g. mario/MarioUtil/MathUtil") + ap.add_argument("--map", default=DEFAULT_MAP, + help=f"path to the linker map (default: {DEFAULT_MAP})") + ap.add_argument("--map-tu", + help="override the map .text-layout TU identifier") + ap.add_argument("--reverse", action="store_true", + help="compare against the reversed object order (escape hatch; " + "no known unit needs this)") + ap.add_argument("-v", "--verbose", action="store_true", + help="list the object-only symbols not in the map " + "(by default only their count is shown)") + args = ap.parse_args() + + unit = resolve_unit(load_units(), args.unit) + meta = unit.get("metadata", {}) + source_path = meta.get("source_path", "") + base_path = os.path.join(root_dir, unit["base_path"]) + reverse_fn = bool(meta.get("reverse_fn_order")) + + tus = parse_text_layout(args.map) + tu_id = find_map_tu(tus, source_path, args.map_tu) + map_syms = tus[tu_id] + bind_by_tu, bind_by_name = parse_closure(args.map) + + def map_binding(name: str) -> Optional[str]: + return bind_by_tu.get((tu_id, name)) or bind_by_name.get(name) + + obj_syms = obj_functions(base_path) + if args.reverse: + obj_syms = list(reversed(obj_syms)) + + map_names = [s.name for s in map_syms] + map_set = set(map_names) + map_size = {s.name: s.size for s in map_syms} + unused_names = {s.name for s in map_syms if s.unused} + + obj_names = [n for n, _, _ in obj_syms] + obj_set = set(obj_names) + obj_size = {n: sz for n, sz, _ in obj_syms} + obj_bind = {n: b for n, _, b in obj_syms} + + print(f"Unit : {unit['name']}") + print(f"Source : {source_path}") + print(f"Object : {os.path.relpath(base_path, root_dir)}") + print(f"Map TU : {tu_id}") + print(f"Map symbols : {len(map_names)} ({len(unused_names)} UNUSED) " + f"Object .text symbols: {len(obj_names)}") + print(f"reverse_fn_order: {reverse_fn}" + + (" [comparing reversed]" if args.reverse else "")) + print("-" * 78) + + failures = 0 + + # --- MISSING ---------------------------------------------------------- + missing = [s for s in map_syms if s.name not in obj_set] + if missing: + failures += 1 + print(f"\n[FAIL] {len(missing)} map symbol(s) MISSING from the object:") + for s in missing: + tag = " (UNUSED)" if s.unused else "" + print(f" - {s.name}{tag}") + else: + print("\n[ OK ] All map symbols are present in the object.") + + # --- ORDER (over the intersection) ------------------------------------ + # Non-weak ordering is an error. Weak-symbol ordering is compiler-controlled + # and painful to steer, so a disorder that only involves weak symbols is a + # warning that does not fail the check (and won't block a PR). + present_map = [n for n in map_names if n in obj_set] + obj_in_map = [n for n in obj_names if n in map_set] + nonweak = lambda n: obj_bind.get(n) != "weak" + present_map_nw = [n for n in present_map if nonweak(n)] + obj_in_map_nw = [n for n in obj_in_map if nonweak(n)] + order_weak_warn = False + if present_map_nw != obj_in_map_nw: + failures += 1 + print("\n[FAIL] Non-weak symbol ORDER differs from the map " + "(weak & object-only symbols ignored):") + for line in order_diff(present_map_nw, obj_in_map_nw): + print(line) + elif present_map != obj_in_map: + order_weak_warn = True + print("\n[WARN] Only weak symbols are out of order " + "(compiler-controlled -- does not fail the check):") + for line in order_diff(present_map, obj_in_map): + print(line) + else: + print("[ OK ] Symbol order matches the map.") + + # --- SIZE of UNUSED symbols ------------------------------------------ + size_bad = [ + s for s in map_syms + if s.unused and s.name in obj_size and obj_size[s.name] != s.size + ] + if size_bad: + # Warning only: matching an inlined body's exact size is hard, and it + # doesn't affect correctness of what does get emitted. Does not fail. + print(f"\n[WARN] {len(size_bad)} UNUSED symbol(s) with wrong size " + f"(best-effort -- does not fail the check):") + for s in size_bad: + print(f" - {s.name}") + print(f" map: 0x{s.size:x} ({s.size})") + print(f" object: 0x{obj_size[s.name]:x} ({obj_size[s.name]})") + elif unused_names: + checked = sum(1 for n in unused_names if n in obj_size) + print(f"[ OK ] UNUSED symbol sizes match ({checked}/{len(unused_names)} checked).") + + # --- BINDING (weak / local / global) --------------------------------- + # Only *placed* (linked) symbols can be checked: the closure records their + # exact binding. UNUSED symbols are deadstripped, so they are absent from + # the closure and the .text layout records no binding for them -- their + # weak/local/global linkage is simply unknowable from the map (a weak symbol + # referenced only by dead code is dead-stripped and listed UNUSED just like + # a local or global one), so we cannot and do not check them. + bind_bad = [] # (name, expected, actual) + for s in map_syms: + if s.unused: + continue + actual = obj_bind.get(s.name) + if actual is None: + continue # missing -> already reported + expected = map_binding(s.name) + if expected is not None and expected != actual: + bind_bad.append((s.name, expected, actual)) + if bind_bad: + failures += 1 + print(f"\n[FAIL] {len(bind_bad)} symbol(s) with wrong linkage " + "(weak/local/global):") + for name, exp, act in bind_bad: + print(f" - {name}") + print(f" map: {exp}") + print(f" object: {act}") + else: + print("[ OK ] Symbol linkage (weak/local/global) matches the map.") + + # --- extras (informational) ------------------------------------------ + extras = [n for n in obj_names if n not in map_set] + if extras: + print(f"\n[INFO] {len(extras)} object-only symbol(s) not in the map " + "(usually deduplicated weak/inline helpers; not an error" + + ("; -v to list)." if not args.verbose else "):")) + if args.verbose: + for n in extras: + print(f" {n}") + + warns = [] + if order_weak_warn: + warns.append("weak-symbol order") + if size_bad: + warns.append(f"{len(size_bad)} UNUSED size mismatch(es)") + + print("-" * 78) + if failures: + tail = f", {'; '.join(warns)} warning(s)" if warns else "" + print(f"RESULT: FAIL ({failures} error category/categories{tail})") + sys.exit(1) + if warns: + print(f"RESULT: PASS with warnings ({'; '.join(warns)} -- see above)") + else: + print("RESULT: PASS") + + +if __name__ == "__main__": + main()