Repository navigation
feat(relinker): support short scalar guest GS instructions - #11661
Felipebmaitan wants to merge 27 commits into
Conversation
…ower of two ElfReader accepted any p_align on an executable's PT_LOAD, and on the uncovered PT_SCE_RELRO it turns into one, although the gABI says p_align should be 0, 1 or a power of two. When such a segment starts on a page that an earlier segment maps with other bytes, the Linux patcher pads the output one byte at a time to the next offset congruent to its address modulo that alignment: a 3221-byte executable with an alignment of 0xfffffffffffffffb ran for 46 s, reached 12.9 GiB and failed with std::bad_alloc. Both segments are now refused, as the reader already refuses a PT_TLS alignment that is not a power of two. The guest module reader takes its headers from the same function, so the power-of-two half of its own alignment check can no longer fail and is dropped; its offset congruence check stays. A power-of-two alignment still reaches the padding, which the relinker technical debt now records. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
The code and test review of the incremental #11628..#11661 changes did not find a confirmed defect. I cannot approve the performance claim/evaluation without the required paired captures, though.
Please provide all of the following from the same title, scene, and machine:
- The
aps5-summary.jsonlline formainand the line for this change. - Both trace files attached to the PR.
- The output of
python3 tools/trace_digest.py <trace>for each trace.
The current PR description reports synthetic relinker/runtime correctness tests, but contains no performance captures or digest output. Until this evidence is available, the performance impact is unverified. At the time of review, conventions, intake, eligibility, Ubuntu relinker, ASan, register-context and macOS write-watch checks passed; Windows relinker and several macOS jobs were still pending. This review covers the incremental changes after #11628; the prerequisite remains independently reviewable.
|
Thanks, borrowing the following instructions to fit the jump is a neat way to cover the short forms, and keeping the own-length |
|
I checked the base #11628 builder before assessing this. One nuance for the requested fallback and regression tests: a relative branch target inside the base's five-byte replacement jump already fails with I agree the fallback policy should be made explicit for cases this change newly rejects, especially the independently decoded GS/state stream covered by |
|
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.
|
# Conflicts: # core/relinker/relinker/tests/test_windows_guest_gs.py
|
@oneandonlydean @MarcZX10 Addressed the borrowed-span request and policy clarification in bd67c1b, retained by current head 199f953. Unsafe borrowed entry roots/branch targets, relocated follower operands and independently decoded GS/state streams now keep the original short GS instruction's own-length UD2/NOP fallback. Followers are untouched apart from normal ELF relocations, and the candidate's .ggs/.ggm records are discarded. Branch/root decisions happen after the complete scan. Overlapping GS streams outside short borrowing retain #11628's prior scanner behavior. The tests distinguish this from existing protections: entries inside the original GS instruction still refuse conversion, and long replay jumps/fallback traps retain their existing entry guards. Borrowed-span cases convert successfully. Relocated follower bytes are compared against an ordinary conversion with the source GS replaced by NOPs. Regressions also check empty discarded metadata/runtime requirements, direct follower entry without libc, and executable/module overlap paths. A runtime fixture executes valid replay records before and after a discarded candidate. I reran production libc/libkernel component builds and focused Windows/Linux runtime/relinker tests after current-main and prerequisite integration; all 70 normalized Windows/native Linux execution dumps match. Conventions against origin/main passed. The full graphics/media configuration remains blocked by missing SDL2/ffmpeg sources; no console/title verification is claimed. |
|
@MarcZX10 Regarding the performance review: this PR makes no speed or performance claim. The description now states explicitly that no performance measurements or paired title captures were performed. Its scope is instruction correctness and additional supported GS forms; the evidence is synthetic native Linux/rewritten Windows differential execution and emitted-program/runtime tests. Any performance impact remains unmeasured. There are no main/change title trace files or trace_digest summaries to provide, and I have not presented the correctness-test timings as performance evidence. |
What
Replay supported three- and four-byte scalar GS memory instructions on Windows by borrowing complete neighboring instructions until the replacement jump fits. Scalar memory followers use the existing protected replay frame without adding GS; bounded register operations, RIP-relative LEA and direct Jcc/JMP use relocated native copies. A libc continue handler resumes borrowed instructions at their replay locations while faults retain their original guest positions. Retrying a repaired follower fault does not repeat the completed GS operation. Normal executable startup and guest-module initialization require the continuation-capable runtime before guest code runs.
Depends on #11628, including its maintainer-requested UD2 fallback changes at 87ef2af. Unsupported GS forms and short instructions with unsupported neighbors retain main's UD2/NOP fallback; unused forms do not block conversion. Entries or branches inside the original GS instruction and relocations touching its own bytes retain the existing conversion refusals. Borrowed-span conflicts use the own-length UD2 fallback described below. No SSE/VEX feature is included.
Both requested adversarial reviews completed against the preserved initial PR 3 commit ef76113. This final branch fixes their two confirmed findings: C6/C7 followers require MOV /0, preventing XBEGIN/XABORT or reserved groups from being copied as ordinary scalar code; short borrowing is disabled for independently decoded overlapping GS/state streams, including a GS instruction entered beyond the overwritten bytes inside a borrowed MOVABS immediate. Regressions cover executable and guest-module paths and preserve safe suffix-entry controls. The RTM finding was proven from the emitted abort displacement; no RTM execution is claimed on this AMD host.
Main remains the intended target. Incremental review starts at the published #11628 head: Felipebmaitan/AnyPS5@914c450...feat/windows-guest-gs-short . The main-target diff includes that unmerged prerequisite stack. Merge order is #11624, #11090, #11276 and #10518 before #11628, then this PR. Their exact heads remain ancestors with original authorship; this PR requests no changes to those contributors' work and does not claim their agreement. Current main abed035 is also merged; #11276 is now merged upstream. No comments or messages were posted to other contributors.
Review follow-up at
199f953a4f26fe08b3a5bfb6267e76e72ad8396e: #11628's address-size execution regressions now belong to the scalar prerequisite itself and have been propagated here. Current main b986c11 is merged. This merge preserves the reviewed guest-GS feature source and tests at bd67c1b; no feature-source changes were needed. Fresh independent component validation passed on Windows and Linux, including the own-length fallback, ordinary relocation preservation, empty discarded metadata/dependency, existing original-instruction entry refusals and valid replay records before/after a pruned candidate. All 70 normalized Windows/native Linux execution dumps match. No performance claim or paired title captures are available or claimed.Collaborator integration follow-up at
339b7e0fd9bafe4bccbebb27777bb05752534b46: current main 20cfe35 is now an ancestor, including the merged #11090 entry-root protection (33b48ba). The immediate prerequisite is now00cfc900d6c4feeefc6ac109ea674ee5ca54f939, carrying #11624's removal of the two Windows FS refusal assertions. The production FS refusal and guest-GS behavior remain unchanged. The existing pre-patch GS entry-root checks and regression registration are retained. Fresh production libc/libkernel component builds passed on Windows and Linux/WSL, followed by 9 Windows and 8 Linux selected runtime/relinker checks; each completed within 30 seconds. #11090 and #11276 are merged upstream; #10518 and the remaining declared prerequisites still apply. No console/title or performance verification is claimed.Tested
Windows x86-64 WinLibs GCC 15.2.0 UCRT and native Linux x86-64 WSL GCC 13.3.0; synthetic ISA and emitted-program fixtures, no PS5 or title execution.
Maintenance validation (2026-10-29): exact head
1358bef8f5c8e28a2af940250b8d7e63a68e911b, including main1189ec3003672bb364dd9f3119ead7b3976e3c83. Complete production libc/libkernel component builds and 11 selected Windows / 8 Linux/WSL runtime/relinker checks passed independently for this layer, each under 30 seconds. Conventions passed against origin/main. Linux follow-up layers used immutable commit snapshots and separate native WSL build outputs. Full graphics/media configuration remains blocked by missing SDL2/FFmpeg sources; no PS5, title or speed verification. Maintainer changes are preserved.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)Live overlap scan and comparison limits
No overlaps were reported in compared regions. For otherwise-uncomparable branches, file-level three-way comparisons checked 38 shared incremental files across 27 PRs against this head and the #11628 prerequisite baseline. None became newly conflicting from PR 3. Existing stale documentation/build conflicts remain; #10861's source/API changes merge at file level, while its old GS debt wording conflicts with the prerequisite replay debt. These checks do not claim full combined runtime validation of stale branches.
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.
core/libs/prx/libkernel/Pthread/src/Thread.cppcore/libs/prx/libkernel/Pthread/src/Thread.cppcore/libs/prx/libkernel/System/src/Exception.cppdocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtcore/libs/prx/libkernel/Pthread/src/Thread.cppcore/libs/prx/libkernel/System/src/Exception.cppcore/libs/prx/libkernel/System/src/Process.cppcore/relinker/CMakeLists.txtdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/relinker.mddocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtcore/relinker/CMakeLists.txtcore/relinker/elfpatcher/src/windows/WindowsPePatcher.cppdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mdcore/libs/prx/libkernel/Pthread/src/Thread.cppdocs/dev/debt/libkernel.mdcore/libs/prx/libc/src/specifics/windows/CrashReport.cppcore/libs/prx/libkernel/System/src/Exception.cppdocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtcore/libs/prx/libkernel/Pthread/src/Thread.cppcore/libs/prx/libkernel/System/src/Exception.cppcore/libs/prx/libkernel/System/src/Process.cppdocs/dev/debt/libc.mdcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libc.mddocs/dev/debt/libc.mdcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mdcore/libs/prx/libkernel/Pthread/src/Thread.cppcore/libs/prx/libkernel/System/src/Process.cppdocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mdcore/libs/prx/libkernel/Pthread/src/Thread.cppcore/libs/prx/libkernel/System/src/Exception.cppdocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mdcore/libs/prx/libkernel/System/src/Process.cppdocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mdcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mdcore/libs/prx/libc/CMakeLists.txtcore/libs/prx/libkernel/System/src/Process.cppdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mdcore/libs/prx/libc/CMakeLists.txtdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mddocs/dev/debt/relinker.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libc.mddocs/dev/debt/libkernel.mdcore/relinker/CMakeLists.txtcore/relinker/elfpatcher/src/windows/WindowsGuestModuleWriter.cppcore/relinker/elfpatcher/src/windows/WindowsPePatcher.cppcore/relinker/elfpatcher/src/windows/WindowsTlsBuilder.cppdocs/dev/debt/relinker.mdcore/relinker/CMakeLists.txtdocs/dev/debt/relinker.mdcore/relinker/CMakeLists.txtdocs/dev/debt/relinker.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/relinker.mddocs/dev/debt/libkernel.mdcore/relinker/CMakeLists.txtdocs/dev/debt/relinker.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libkernel.mddocs/dev/debt/libc.mdcore/libs/prx/libkernel/System/src/Process.cppdocs/dev/debt/libc.mddocs/dev/debt/libkernel.mdcore/relinker/CMakeLists.txt