Skip to content

fix(relinker): retain declared import library identities - #1617

Closed
eliasbruno124-dev wants to merge 7 commits into
boykopovar:mainfrom
eliasbruno124-dev:fix/import-library-owner-followup
Closed

eliasbruno124-dev wants to merge 7 commits into
boykopovar:mainfrom
eliasbruno124-dev:fix/import-library-owner-followup

Conversation

@eliasbruno124-dev

@eliasbruno124-dev eliasbruno124-dev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Depends on #1503. This branch contains its corrected head 188dd9f5f814d2f258c5ac04775ba42ec80374c5 and main 4b005b229e24be9ec3f185def9e59cbc03aa9e4a. The library-owner change remains 98d9184b; merge #1503 first. The update preserves the generalized module/library resolver and includes the parent's .sprx handling, with additional library-owner tests.

When an import's module name does not match a declared dependency, the relinker drops its owner even if the library qualifier identifies that dependency. On Windows a colliding export can then bind to the first unrelated provider. The regression returns 11 from that provider instead of 22 from the declared one.

Read DT_SCE_IMPORT_LIB in both executable and guest readers. If module resolution finds no dependency, resolve the library qualifier against the existing dependency list. Exact and normalized module matches retain precedence. When module resolution succeeds, the library qualifier is not used. Otherwise, no library table or a declared library without a matching dependency retains the broad fallback requested in #684. A referenced ID missing from an existing library table is malformed and throws, as do invalid names and ambiguous matches. No libraries, export implementations or title-specific aliases are added.

Original input PPSA27836 / 01.000.001 demonstrates the mismatch: imports declare module libSceJson, library libSceJson2, and dependency libSceJson2.prx.

Tested

  • Windows, MinGW-w64 GCC 15.2.0, Release, ANYPS5_RELINKER_ONLY=ON, BUILD_TESTING=ON. Corrected parent 188dd9f5: 37/37 registered groups pass. Current commit 847feacc09bec211d8025002102e63e812137cb5: 38/38 pass, without failures or skips. Conventions and git diff --check pass. CI for this exact head passes the Linux and Windows full builds/tests and the relinker jobs on Linux, Windows and macOS.
  • The identical final fixture fails on the parent with the wrong-provider result and passes on the candidate. It executes generated Windows programs and a guest initializer; covers module precedence, .prx/.sprx/.suprx and native spellings, duplicate/invalid IDs, malformed metadata, ambiguous names, unresolved symbols and the existing fallback. The PR1503 identity regression also passes.
  • Original-title comparison on the previous heads: both relinkers converted the same preserved original PPSA27836 / 01.000.001 code with identical, explicitly older host libraries from 7d3b3930. All 1,815 emitted import occurrences / 896 unique queries were inspected: 84 previously unqualified owners now agree with the original library metadata and declared dependencies (62 Json2, 7 AppContent, 7 SaveDataDialog.native, 8 MsgDialog.native); the other 812 records are unchanged. Inputs and host binaries retain their hashes. Original filenames and HDD content were not changed.
  • Missing imports remain 35 on both sides, including the five JSON functions whose owner exposed this issue. This patch implements none of them, so no whole-title restart repeated that unchanged startup boundary.
  • In a separate bounded metadata check, all 23,736 qualified undefined symbol entries across 141 retained code files had declared import library and module IDs. This includes 1,525 entries across 13 Castlevania code files; no undeclared-library case was observed there. Twelve other inventoried code files are not covered by these caches. This is input evidence, not a guarantee about every PS5 toolchain or a current title run.
  • No Linux execution locally. No full runtime rebuild or gameplay claim.
  • Reviewed fix(relinker): bind Windows imports to their declared modules #684, refactor(relinker): read the executable's DT_NEEDED entries in one place #1403, feat(relinker): write a registry for each converted bundled module #1462 and fix(relinker): keep Linux imports declared from host modules off guest exports #1529. The last two implement registry output and Linux host/guest binding respectively; trial merges with their inspected heads are clean and neither is included. Those overlap checks are historical; the current branch includes main 4b005b22 and the corrected fix(relinker): discover required guest module identities #1503 parent. The shared resolver conflict was resolved locally and the result was tested above.

Overlap rechecked on October 9 against main f1451396: this branch and #1815 merge cleanly together. Both inherit #1503 (188dd9f5), including the identical test_guest_module_identity.py; neither follow-up depends on the other. This branch also merges cleanly with that main.

Checklist

@Dextroid17

Copy link
Copy Markdown
Contributor

Verified the same way as #1503 — by replaying the merge and running the fields, not by reading the diff.

Scope and replay. This PR's own change is the single commit 98d9184b (5 files, +171/-25: ImportModule.hpp, GuestImageReader.cpp, RelinkerPipeline.cpp, the CMake registration and the new test). I cherry-picked it onto today's main (4b005b22) with #1503 already merged, in the order your body asks for:

  cherry-pick 98d9184b onto (#1503 + 4b005b22)   no conflicts
  build                                          clean
  both new regressions                           guest_module_identity Passed, import_library_identity Passed
  all relinker groups, Linux                     38/38 passed in 5.3 s
  the run for this head                          success, event=pull_request, head 98d9184b, branch
                                                 fix/import-library-owner-followup

Negative control. Reverting only the three implementation files to 2eae8ac7 (its parent, i.e. #1503 without this change) and keeping the test:

  1 tests failed out of 1     AssertionError: ({'guest': False, 'symbol': 'shared#?#B'}, 0, '')
  restored (test and implementation)   ->  Passed

So the regression does gate this change: on the #1503-only tree the owner is dropped and the colliding provider answers 11 instead of 22, which is the failure your body describes.

Not a duplicate. I checked the symbol rather than the topic: 0x61000049, importLibraries and ImportLibrary appear nowhere else among the open PRs that touch this area (#1403, #1529, #1507, #1446, #1661, #1462, #846) — only in this PR and its stacked base. Nothing to reconcile.

The new failure modes are pinned by tests, which is the right call. The refactor widens the throwing surface: resolve is now reached through the library qualifier, so a symbol whose library qualifier is ?, empty, malformed, or an ID that is not declared in DT_SCE_IMPORT_LIB throws where #1503 (and main) returned an empty owner. Your test asserts all four (shared#?#B, shared##B, shared#QAA#B, shared#B#B) plus the invalid declared-name case, and separately asserts that a declared but unmatched library still yields the empty owner (library='unmatched', library=None) — so the boundary between "unowned import" and "hard error" is deliberate and covered rather than accidental.

One question, since that boundary moved. An empty owner means "treat this as a host-side import" downstream, and it used to be reachable for any symbol whose library qualifier was not a declared dependency. Now the same input throws if the qualifier is not declared at all. That is fine if PS5 toolchains always emit a DT_SCE_IMPORT_LIB entry for every library an import names — which the tag's purpose suggests — but it does mean a title that used to convert with an unowned import will now stop at the relinker. Worth one sentence in the body confirming you saw no such case in the Castlevania run, or a note that it should be impossible by construction; the review will otherwise ask.

Everything else checks out: the 0x61000049 reader sits before the catch-all that previously discarded those tags into nothing, both readers build the ID map with the same >> 48 / 0xffffffff convention as the module imports, and duplicate IDs still fail loudly on both sides.

Neither PR has a maintainer review: this one has no reviews and no comments at all, and #1503's single approval carries no review body. Both are green and mergeable and land in order without conflict, so the pair is a clean unit of work for whoever reviews next.

Written with AI assistance.

@eliasbruno124-dev

Copy link
Copy Markdown
Contributor Author

@Dextroid17 Confirmed against the retained original Castlevania metadata: all 1,525 qualified undefined symbol entries across its 13 code files reference declared import library and module IDs. No undeclared-library case was observed. The broader bounded check covered 23,736 entries across 141 files, with 12 other inventoried files not covered; this does not establish a universal toolchain guarantee.

The body now states that distinction and the fallback/error boundary explicitly. Updated to the corrected #1503 parent, preserving module precedence and adding .sprx library-owner coverage. Current head 847feac passes all 38 registered groups locally on Windows; the identical final fixture still fails on the parent with the wrong-provider result. New CI is running.

AI-assisted: yes

@Dextroid17

Copy link
Copy Markdown
Contributor

Checked the update at 847feacc:

  body       now says "input evidence, not a guarantee about every PS5 toolchain or a
             current title run" and names the 12 uncovered files — the distinction I asked for.
  .sprx       test_import_library_identity.py:94-96 covers library='b2', 'b2.sprx' and
             module='b2.sprx' all resolving owner 'b2.sprx'.
  parent     base is the corrected #1503 head (188dd9f5), so the shared resolver and the
             restored single ReadNeededNames carry through here too.
  checks     all green at this head, including the macos Relinker leg that is cancelled on
             #1503 — so that one is environmental, not this code.

The 23,736/141/12 scoping is the right way to state it: it answers my question about the undeclared-library boundary without turning one title's inventory into a universal claim. Nothing further from me on this one either.

Written with AI assistance.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#1815 core/relinker/relinker/tests/test_guest_module_identity.py

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: on top of #1503 this doesn't change a title run (PPSA27836 has the same missing imports either way), and the colliding-provider case only exists in the synthetic test. Please reopen with a title where main binds an import to the wrong provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants