Repository navigation
fix(tools): preserve weak guest module imports in audits - #8718
Conversation
|
Question about the weak-import semantics: |
|
Independent Linux test result on the published head |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
The exit policy is intentional and now explained in the description: exit 1 reports unresolved audit coverage, while The current tree-only overlap check also retains the #6171 conflict. I propose these narrow audit corrections land before #6171, whose integration should retain its declared aliases and physical host-module set together with the weak-binding and Windows-provider changes; no merge order has been agreed yet. |
harisibrar1
left a comment
harisibrar1
left a comment
There was a problem hiding this comment.
Verified on Windows 11, Python 3.14:
python tools/tests/test_import_audit.pyon 700be18: 122 tests, OK (3 skipped).- With
tools/import_audit.pyreverted to main,test_module_import_bindings_survive_the_auditfails in all three subtests (sectioned, sectionless SysV hash, sectionless GNU hash) onrecords["optional"]["weak"], so the new test catches the lost STB_WEAK binding. The updated existing tests fail only on the set-to-dict shape change. - Checked the surrounding code: only undefined (
st_shndx == 0) symbols are treated as imports, so weak definitions still go to exports. GLOBAL and GNU_UNIQUE undefined symbols stay required. Duplicate spellings merge with AND, so the import is weak only if every spelling is weak. Reference counts and conservation are unchanged.read_host_importsandelf_exportsonly use the needed/export parts of the tuples and are not affected. python tools/check_conventions.py --base main: passed.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
Linux data point for
|
|
@mehmetbayoglu I checked #9372 at The changes address different behavior. Integration needs to retain the weak-import label and neutral absent-import heading here, and #8719's Windows-provider normalization, while adding your partial classification to the audit signature, provider checks and CLI call. I propose landing #8718 and #8719 before #9372, then integrating those behaviors into #9372. Does that order work for you? This is a proposal, not an agreed dependency. No code changed or tests reran for this merge-only check. |
|
@ArjenS-LA Thanks for checking. That order works for me: #8718 and #8719 first, then I rebase #9372 on them, keeping the weak-import label, the neutral absent-import heading and #8719's Windows provider normalization, and adding the partial class to the audit signature, the provider checks and the CLI call. I'll mark #9372 |
|
@MarrasFederico I checked the new #10311 against this PR. The normalized trees merge cleanly against main With #10311 at Changing only that expected imports value to Would landing #8718 first, then adapting that assertion when integrating #10311, work for you? This is a proposed order, not an agreed dependency; maintainers retain the final choice. Validation: Windows x64, Python 3.13.7; isolated file extractions from exact Git trees, with |
What
Mari0's shipped libOSMesa imports optional Zstandard tracing symbols, but the audit loses ELF STB_WEAK and labels them as required. Preserve weak bindings through sectioned and sectionless ELF module audits, combine duplicate spellings conservatively, and label weak imports in text output. Classifications and exit policy are unchanged.
Weak-import exit policy: exit 1 means the audit found unresolved imports or missing dependencies; it is not a prediction that the loader will stop. This PR intentionally preserves that existing policy for both weak registry imports and weak module imports.
current_stop()excludes weak imports because they can resolve to null. A weak-only module fixture on this exact head returns 1, reportsweak: trueandabsent: 1, and reports no loader-stop NID. Windows x64 / Python 3.13.7: the focused policy check passes. No exit-policy behavior change is included.Tested
Commit
700be1869477a876cadc09bed94bac2239cfe209; branchfix/audit-module-weak-imports. Full audit suite: 122 tests run in 5.209 s, 3 platform skips, passed. Command:python -m unittest discover -s tools/tests -p test_import_audit.py.Exactly 12 weak flags change from false to true (10 libOSMesa imports and 2 love imports); all other import fields, missing-library reports, and counts remain identical. Baseline fails the sectioned, sectionless SysV, and sectionless GNU binding cases; fixed passes.
Windows 11 x64; Python 3.13.7. Fresh baseline
d97361bd0df0a54b11570602391aa003bff31047. The same focused regression fails on baseline for the intended behavior and passes with this commit. Full audit suites ran after rebasing; no tracked source edits were needed during validation.Homebrew/title evidence — indirect diagnostic workflow: Mari0 1.6.2 from websrv v0.34. Archive SHA-256
e0331ed5a22e7fe72843d268cd2c8b297d761a75b7c83d828f5cc58a82a72ce3; original love.elf54bcc91670fd759bbd3488284dce351b6421cb83016e9ad8da6b5186da067a39; shipped libOSMesa.so.8128d2ab20b980b96decb1f20a9ed57d10cc922605259666b63efa52963920f0a.Fresh baseline/fixed audits use the unchanged registry generated earlier from that original package, its original shipped modules, and the same 126 hashed system-library artifacts from
af691b717ad068bb93ac50bfc428af63ab259613. Historical conversion used relinker baseline11ee778434dacf31f2ce0ecff8229e0839e36b34with--windows --trap-sdk-syscall-stubs --registry --sce-module-path <original-Mari0> --module-dir . <original-Mari0>/love.elf <evidence>/mari0/love.exe. The trap flag, explicit module directory, external output directory, and older frozen system libraries are disclosed accommodations. This publication check does not claim reconversion with new main, game startup, guest execution, gameplay, GPU testing, or physical-PS5 behavior.The fresh command on each tool is
python <tool>/import_audit.py <pinned>/love.registry.json --libs <frozen-libraries> --source <rebased-source>/core/libs/prx --modules <original-Mari0> --json <report>. All audit runs retain exit 1 for unrelated unresolved imports. Package, registry, and library hashes remain unchanged. Every run retains 882 references and class counts{'implemented': 848, 'stub': 12, 'absent': 7, 'module': 0}; assertions check the report improvement directly.The tree-only merge of both exact rebased commits (
f92844fb00837d6d4ac66d5e7942358a0ffd782e) also passes 125 tests, with 4 skips (3 platform skips and 1 Git-fixture skip outside a checkout). Its Mari0 audit jointly preserves all 12 corrected weak flags and removes exactly 10 false missing-library reports.Historical native validation only: before rebasing, the relinker-only build passed 303 targets under MinGW-w64 GCC 15.2.0. Broad CTest reported 211 passed, 1 failed, 1 skipped of 213. The unrelated shader_cache_runtime_sources fixture could not find Ninja; isolated runs with explicit toolchain PATH passed on baseline (65.67 s) and fixed (61.76 s). That existing fixture exceeds 30 s locally; no timeout or unrelated source was changed. long_paths remained skipped. The previous suffix branch's registered import_audit CTest passed in 18.64 s. These results are not fresh native builds of the rebased commits; this change affects Python audit tooling only.
Fresh overlap: complete 799-PR inventory against
339217ebc90f6fe91556525425aa18292d5f563b, exact peer heads, implementation review, and tree-only merge comparisons. Tested base is d97361b; the subsequent main changes affect unrelated audio libraries. Both topics merge cleanly with checked main and with each other. #8572 (missing-directory hints) and #7460 (PE optional-header bounds) merge cleanly and implement different behavior. Both topics have a text conflict with #6171 (35bf15c510cb50ee8152d911c686e5eccdb95c15); the resolution must retain its declared aliases and physical host-module set together with this change. #7454 (11840ad87c4065873cb2923b2367232d33bbe5ee) already conflicts with main in audit_host_dependencies, so its pairwise result remains inconclusive. No semantic duplicate identified. #8124's guest ModuleNames support is already on main and is preserved.Recorded report assertions: 882 references; implemented 848, stub 12, absent 7, module 0. Weak-only: exactly 12 binding flags corrected and every other import field unchanged. Suffix-only: exactly 10 false missing-library reports removed and complete import records unchanged. Combined: both improvements preserved. Registry SHA-256
d9e9cf8cf2639221e61ba5600298130c452f1e53f8572e8b363ba43c40cbb053. All input hashes were checked before and after.The #6171 conflict is in read_modules: keep elf_imports(file, data, names), sorted(symbols.items()), and its exported/SONAME/identity handling together.
Checked overlapping heads: #8572
6736aa663d3234cf024af69162f40db455586375, #746070986ca71f0d44aec7d9e06b3ae7e47c7c08bcc2, #745411840ad87c4065873cb2923b2367232d33bbe5ee, #617135bf15c510cb50ee8152d911c686e5eccdb95c15. No merge ordering has been agreed with #6171.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)