Repository navigation
fix(tools): recognize guest module identities in import audit #6171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| ROOT = Path(__file__).resolve().parent.parent | ||
| PRX = ROOT / "core" / "libs" / "prx" | ||
| MODULE_SUFFIX = "#" | ||
| MODULE_EXTENSIONS = (".prx", ".sprx", ".suprx", ".debug_prx") | ||
| CLASSES = ("implemented", "stub", "absent", "module") | ||
| PE_DIRECTORY_OFFSET = {0x20b: 112, 0x10b: 96} | ||
|
|
||
|
|
@@ -57,7 +58,18 @@ def elf_sections(path, data): | |
| return [unpack("<IIQQQQIIQQ", data, section_offset + index * 64, path, f"section header {index}") for index in range(section_count)] | ||
|
|
||
|
|
||
| def elf_program_dynamic_symbols(path, data): | ||
| def add_module_name(path, data, offset, size, tag, value, names): | ||
| if names is None or tag not in (14, 0x6100000d, 0x61000043): | ||
| return | ||
| position = value if tag == 14 else value & 0xffffffff | ||
| name = cstring(data, offset + position, offset + size, path, "module name") | ||
| forbidden = "\\:$\r\n" if tag == 14 else "/\\:$\r\n" | ||
| if not name or any(character in name for character in forbidden): | ||
| raise AuditError(f"{path}: invalid module name {name!r}") | ||
| names["sonames" if tag == 14 else "identities"].add(name) | ||
|
|
||
|
|
||
| def elf_program_dynamic_symbols(path, data, module_names=None): | ||
| header = unpack("<16sHHIQQQIHHHHHH", data, 0, path, "ELF header") | ||
| ident, program_offset, program_size, program_count = header[0], header[5], header[9], header[10] | ||
| if ident[6] != 1 or header[3] != 1: | ||
|
|
@@ -85,6 +97,7 @@ def file_offset(address, size, what): | |
| raise AuditError(f"{path}: dynamic segment size {dynamic_size} is not a multiple of 16") | ||
| dynamic = {} | ||
| needed_offsets = [] | ||
| named_entries = [] | ||
| for position in range(0, dynamic_size, 16): | ||
| tag, value = unpack("<qQ", data, dynamic_offset + position, path, "dynamic entry") | ||
| if tag == 0: | ||
|
|
@@ -93,13 +106,17 @@ def file_offset(address, size, what): | |
| needed_offsets.append(value) | ||
| else: | ||
| dynamic[tag] = value | ||
| if tag in (14, 0x6100000d, 0x61000043): | ||
| named_entries.append((tag, value)) | ||
| string_address = dynamic.get(5) | ||
| string_size = dynamic.get(10) | ||
| symbol_address = dynamic.get(6) | ||
| symbol_size = dynamic.get(11) | ||
| if None in (string_address, string_size, symbol_address, symbol_size) or symbol_size != 24 or string_size == 0: | ||
| raise AuditError(f"{path}: incomplete dynamic string or symbol table") | ||
| string_offset = file_offset(string_address, string_size, "dynamic string table") | ||
| for tag, value in named_entries: | ||
| add_module_name(path, data, string_offset, string_size, tag, value, module_names) | ||
|
|
||
| symbol_count = None | ||
| if 4 in dynamic: | ||
|
|
@@ -183,10 +200,10 @@ def elf_exports(path, data): | |
| return names | ||
|
|
||
|
|
||
| def elf_imports(path, data): | ||
| def elf_imports(path, data, module_names=None): | ||
| header = unpack("<16sHHIQQQIHHHHHH", data, 0, path, "ELF header") | ||
| if not header[6] or header[6] >= len(data): | ||
| imports, _, needed = elf_program_dynamic_symbols(path, data) | ||
| imports, _, needed = elf_program_dynamic_symbols(path, data, module_names) | ||
| return imports, needed | ||
| sections = elf_sections(path, data) | ||
| symbols = set() | ||
|
|
@@ -214,6 +231,7 @@ def elf_imports(path, data): | |
| break | ||
| if tag == 1: | ||
| needed.add(cstring(data, strings_offset + value, strings_offset + strings_size, path, "needed library name")) | ||
| add_module_name(path, data, strings_offset, strings_size, tag, value, module_names) | ||
| return symbols, needed | ||
|
|
||
|
|
||
|
|
@@ -429,10 +447,12 @@ def module_files(directory): | |
| raise AuditError(f"{file}: expected ELF module (first bytes {magic!r})") | ||
|
|
||
|
|
||
| def read_modules(directories): | ||
| def read_modules(directories, needed_names=()): | ||
| imports = {} | ||
| exports = defaultdict(set) | ||
| images = {} | ||
| aliases = {} | ||
| identities = defaultdict(set) | ||
| for directory in directories: | ||
| for file in module_files(directory): | ||
| data = file.read_bytes() | ||
|
|
@@ -443,16 +463,34 @@ def read_modules(directories): | |
| raise AuditError(f"ambiguous guest module {file.name}: {previous} and {file} have different contents") | ||
| continue | ||
| images[file.name] = (file, digest) | ||
| names = {"sonames": set(), "identities": set()} | ||
| if data.startswith(b"MZ"): | ||
| imports[file.name] = ([], []) | ||
| names = pe_exports(file, data) | ||
| exported = pe_exports(file, data) | ||
| else: | ||
| symbols, needed = elf_imports(file, data) | ||
| symbols, needed = elf_imports(file, data, names) | ||
| imports[file.name] = (sorted(symbols), sorted(needed)) | ||
| names = elf_exports(file, data) | ||
| for name in names: | ||
| exported = elf_exports(file, data) | ||
| for name in {file.name} | names["sonames"]: | ||
| if name in aliases and aliases[name] != file: | ||
| raise AuditError(f"ambiguous guest module name {name}: {aliases[name]} and {file}") | ||
| aliases[name] = file | ||
| for name in names["identities"]: | ||
| identities[name].add(file) | ||
| for name in exported: | ||
| exports[name.split(MODULE_SUFFIX, 1)[0]].add(file.name) | ||
| return imports, {name: sorted(files) for name, files in exports.items()} | ||
| requested = set(needed_names) | {name for _, needed in imports.values() for name in needed} | ||
| for name in sorted(requested): | ||
| if name in aliases: | ||
| continue | ||
| suffix = next((extension for extension in MODULE_EXTENSIONS if name.endswith(extension)), "") | ||
| stem = name[:-len(suffix)] if suffix else name | ||
| providers = identities.get(name, set()) | identities.get(stem, set()) | ||
| if len(providers) > 1: | ||
| raise AuditError(f"ambiguous guest module identity {name}: " + ", ".join(str(file) for file in sorted(providers))) | ||
| if providers: | ||
| aliases[name] = next(iter(providers)) | ||
| return imports, {name: sorted(files) for name, files in exports.items()}, set(aliases) | ||
|
|
||
|
|
||
| def nids_of(name, aliases): | ||
|
|
@@ -751,12 +789,13 @@ def main(argv=None): | |
| needed = sorted({library for path in dependency_registries for library in read_needed_registry(path)}) | ||
| exports, libraries = built_libraries(args.libs) | ||
| host_dependencies, windows = built_host_dependencies(args.libs) | ||
| module_imports, module_exports = read_modules(args.modules) | ||
| modules = set(module_imports) | ||
| modules |= {name.removesuffix(".guest.prx") for name in modules if name.endswith(".guest.prx")} | ||
| module_imports, module_exports, modules = read_modules(args.modules, set(needed) | {library for _, library, _ in imports if library}) | ||
| host_modules = set(module_imports) | ||
| host_modules |= {name.removesuffix(".guest.prx") for name in host_modules if name.endswith(".guest.prx")} | ||
| modules |= host_modules | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Keep declared aliases out of Windows filename case folding The returned alias set is later wrapped in |
||
| modules = ModuleNames(modules, fold=bool(windows)) | ||
| records, missing = audit(imports, exports, source_stubs(args.source), libraries, modules, module_imports, module_exports, needed_libraries=needed) | ||
| missing_hosts = audit_host_dependencies(host_dependencies, libraries, modules, windows) | ||
| missing_hosts = audit_host_dependencies(host_dependencies, libraries, host_modules, windows) | ||
| if args.names: | ||
| attach_names(records, load_names(args.names)) | ||
| summary = summarize(records, sum(record["references"] for record in records), missing, missing_hosts) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Resolve existing filename aliases before rejecting identity ambiguity
This exact-name guard bypasses the existing suffix matcher. With a physical
Declared.prx, two other modules declaring identityDeclared, and a registry plus.needed.jsonrequestingDeclared.sprx, current main exits 0 but this head exits 2 withambiguous guest module identity Declared.sprx..suprxand.debug_prxreproduce it too; the exact.prxcontrol passes. The relinker installs the filename suffix alias inneededAliasesbefore identity resolution (GuestModuleBuilder.cpp:85,146at this head). Please preserve that filename precedence before checking identity collisions and add a cross-suffix regression; unrelated identities should not invalidate an already-resolved physical module.