Skip to content

fix(relinker): treat address-taken code as reachable in unused-filter=1 - #2332

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
simonbbby:fix/relinker-address-taken-roots
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
simonbbby:fix/relinker-address-taken-roots

Conversation

@simonbbby

@simonbbby simonbbby commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

At unused-filter=1 the CFG followed only direct branches and call/jmp [rip+slot] through RELATIVE-relocated slots. Code reached through a register (lea reg, [rip+f]; call reg, vtables and callback tables, switch jump tables) was unreachable, and imports used only there were removed. GOT accesses were counted only for mov loads and call/jmp [rip+slot].

The CFG now also walks RELATIVE relocation targets in text, RIP-relative lea targets, and int32 jump tables at a lea'd data address, using the strict filter's walk moved to ReadRelativeTableTargets. Any RIP-relative operand that names a GOT slot counts as an access. Address-taken targets are walked after the direct control flow. A target that does not decode is not code and ends that path, so a stray table entry cannot abort the conversion. Strict filter behaviour is unchanged.

Absolute immediates (mov r, imm, push imm) and raw data words pointing into text are still not roots at level 1.

Changes the same files as #2238 (LEA roots for the Windows TLS collector) and #2304 (CFG decoding performance), with a different scope.

Tested

macOS arm64, relinker-only Release build: ctest 39/39, strict_nid_filter unchanged. test_unused_filter_indirect.py covers lea + call reg, a RELATIVE pointer called through a register, a jump-table case block, GOT access by lea/push/cmp, two unreachable controls, and two address-taken targets on undecodable bytes. On main the six indirect cases report CFG/GOT filtering: 1 -> 0. No title was converted from a retail dump.

Linux (Ubuntu 24.04, GCC 13.3), relinker-only Release build: ctest 44/44.

Also converted 43 open-source titles from the PS5 homebrew catalog (eboot.bin and bundled libc.prx, unwrapped from fSELF) with main and with this branch, for Linux and Windows at unused-filter 0, 1 and 2. No conversion changed outcome. At unused-filter=1, all 38 titles that convert keep more imports, 1 to 283 each (5,812 in total), on both outputs; none keeps fewer.

Runtime check on Linux (Ryzen 9 9950X, RTX 3090, NVIDIA 595.99.02) with DOOM PPSA99666 v1.1.1 from the catalog. Both trees relink it with --registry unused-filter=1 and run the title's own test/console-play.cfg without its serve line: it starts doom1.wad, plays and exits at frame 900. Both trees carry #1529, without which the bundled libc.prx shim takes DOOM's libc imports on Linux (#1497); this PR does not depend on it.

Relinker Imports kept Result
main d70b899 + #1529 110 of 171 SIGSEGV after HU_Init, before the first frame (2 of 2 runs)
this branch + #1529 171 of 171 Plays to frame 900 and exits 0 (2 of 2 runs)

On main the crash is a call to address 0 in the audio thread. Its start routine is referenced only by the lea that passes it to pthread_create, so level 1 drops sceAudioOutOutput, which the thread loads with mov <GOT>, %r14 and calls through %r14. The other 59 dropped imports include ones used by the download worker and upload server threads (sceHttp*, accept), the on-screen keyboard (sceImeDialog*) and music (log10). At levels 0 and 2 both trees produce the same executable. #2238 and #2304, built the same way, produce a level-1 executable byte-identical to main's.

ScreenTester5 PPSA69420 on main and Yamagi Quake II PPSA99007 with #2330, both at level 1, stop on a call through the removed GOT slot of pthread_key_create. This branch keeps that slot. Both titles then keep imports that AnyPS5 does not implement yet and stop at load instead: ScreenTester5 with undefined symbol: MhC53TKmjVA (sysctlbyname); for Quake II the import audit lists 20.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@Kikoarl

Kikoarl commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Tested on Linux (CachyOS x86_64, Intel RPL-S iGPU / Mesa 26.3.0-devel / Vulkan 1.4.363, GCC 16.2.1) at 4ea9f974:

  • Build: Built cleanly in RAM (/tmp/anyps5_build) with zero warnings/errors.
  • Conventions: python3 tools/check_conventions.py --base origin/main --head HEAD -> PASS (0 errors).
  • Verification: Verified address-taken code reachability in unused-filter=1. Essential for indirect jump tables and function pointers in stripped guest modules.

LGTM!

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please rebase onto main: #2238 landed and conflicts in ControlFlowGraph.cpp and core/relinker/CMakeLists.txt.

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the DOOM measurement is convincing. This needs a rebase onto main after #2238, and the conflict in ControlFlowGraph.cpp is not mechanical, so it should be resolved by you:

  • BuildControlFlowGraph on main now takes bool followCodeAddresses (REX.W lea rip-relative → enqueue the target). This PR replaces that same parameter with const std::vector<StrictDataRegion>* data and handles the same lea with takeAddress plus jump-table targets. Please unify them: either keep one parameter that covers both cases or keep both explicitly, and make sure the lea target is not enqueued twice with different rules (#2238's callers must keep their behaviour; its tests must still pass).
  • core/relinker/CMakeLists.txt: keep both entry_point_bounds and unused_filter_indirect add_test entries.
  • Heads-up: since #2238, LEA-rooted targets can reach CPU-dispatched XOP code (see #2559); worth checking the new roots don't hit that on the catalog titles.

After the rebase please rerun unused_filter_indirect and tls_function_coverage.

@Mouhand-Kaddo

Copy link
Copy Markdown
Contributor

Additional platform validation on standalone PR head 4ea9f9748fd948396923c29ff76d2578e99af93e:

Windows: passed — Windows 11, supported WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7; CMake 4.3.1 / Ninja.

Release / BUILD_TESTING=ON: built the selected test targets; 41 CTest cases, 0 failures, 0 CTest skips.

All 41 registered relinker tests passed (ANYPS5_RELINKER_ONLY=ON).

Integration limitation: a local merge against main 948a526c2 has source conflicts; these results validate the standalone PR head, not a resolved merge.

@simonbbby
simonbbby force-pushed the fix/relinker-address-taken-roots branch from 4ea9f97 to 814fcc4 Compare October 10, 2026 13:41
@simonbbby

Copy link
Copy Markdown
Contributor Author

Rebased onto main 78aedb4, after #2238, #2330, #2304 and #2708.

  • followCodeAddresses and the data regions: I kept both, explicitly, as two arguments of one internal buildGraph. It walks perf(relinker): single-pass CFG instruction decoding and zero-copy text span #2304's std::span text into the caller-owned reachable set.
  • CMakeLists.txt: entry_point_bounds, init_array_roots and unused_filter_indirect are all registered.
  • XOP (relinker: Windows conversion fails since #2238 on titles with XOP code (decoder has no XOP support) #2559): with main's XOP decoding (fix(relinker): decode XOP instructions #2568), I converted the 43 catalog titles for Linux and for Windows at filter levels 0, 1 and 2, with main and with this PR. No conversion result changes, on either platform, at any level. With this PR, Stremio Plus (PPSA74126), OpenNOW (PPSA99082) and PPSA99420 convert for Windows at level 1. They keep 433, 316 and 507 imports, against 196, 143 and 210 on main.
  • Tests: relinker ctest passes 99/99, including unused_filter_indirect and every tls_function_coverage_* case. At level 1, 38 titles keep more imports than on main, the same set and counts as before the rebase. DOOM goes from 110 to 171 kept imports, and Quake II from 133 to 355. Levels 0 and 2 are unchanged.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@orishamir

orishamir commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Checked 814fcc47 merged onto current main (57f96fd2; clean merge) on Linux (Ubuntu 26.04 in WSL2, GCC 15.2.0, relinker-only Release build). The relinker ctest run is already covered by this PR's CI job; the checks below are the ones it does not do:

  • unused_filter_indirect pins the change: with the nine non-test sources reverted to main (the test and its add_test kept), it fails on exactly the six indirect cases the description lists: lea-call-register, relative-relocation-pointer, jump-table-case-block, got-slot-lea, got-slot-push, got-slot-compare. The tls_function_coverage_* tests pass with and without the change, as expected.
  • On the first review point: in the merged ControlFlowGraph.cpp the two lea rules are behind different gates. fix(relinker): root code addresses taken by RIP-relative LEA #2238's (followCodeAddresses, REX.W only, enqueued directly) is the only one reachable from the three BuildControlFlowGraph overloads, which pass data = nullptr; this PR's (data != nullptr, any RIP-relative lea plus the relative table at its target, deferred to the speculative pass) is reachable only from BuildAddressTakenControlFlowGraph, which passes followCodeAddresses = false. No walk applies both.
  • The overlap bot's hits with feat(relinker): convert for Rosetta with --to-rosetta #2119 and fix(relinker): report the syscall number of a forbidden syscall instruction #2206 are the add_test block only: all three append after init_array_roots (CMakeLists.txt:317) and share no source file (the USAGE.md lines differ and merge cleanly). Merging the three in sequence conflicts only in that block each time.

Not checked here: the catalog conversions and the XOP question from the review (no titles on this machine), and nothing ran in a game.

AI-assisted: yes (Claude Code).

Edited: removed the ctest pass count, which this PR's relinker CI job already reports.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@adriadam10
adriadam10 force-pushed the fix/relinker-address-taken-roots branch from 814fcc4 to a781718 Compare October 10, 2026 22:32

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto current main (one conflict in core/relinker/CMakeLists.txt after #2119: kept both to_rosetta and unused_filter_indirect). Relinker-scoped build and ctest: 91/91 pass. unused_filter_indirect fails with the non-test changes reverted. Merging on green CI.

The CFG used by unused-filter=1 followed only direct branches, so code reached
through lea, relative-relocated pointers or jump tables was unreachable and its
imports were dropped. It now roots RIP-relative lea targets, relative relocation
targets and relative jump tables, and counts any RIP-relative reference to a GOT
slot as an access. Address-taken targets are walked after the direct control
flow, and a target that does not decode as an instruction is not code.
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (one conflict in core/relinker/CMakeLists.txt after #2119 and the new syscall_number: kept both it and unused_filter_indirect). Relinker-scoped build and ctest: 93/93 pass. unused_filter_indirect fails with the non-test changes reverted. Merging on green CI.

@adriadam10
adriadam10 force-pushed the fix/relinker-address-taken-roots branch from a781718 to cfd61a5 Compare October 10, 2026 22:37
@adriadam10
adriadam10 merged commit 06b489d into boykopovar:main Oct 10, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build docs needs rebase Conflicts with main, rebase needed relinker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants