Repository navigation
fix(macos): read application process parameters from Mach-O images - #10876
krishalivaibhav wants to merge 2 commits into
Conversation
ApplicationProcessParameters used the dl_iterate_phdr search only on Linux, so any guest using the application heap threw "unsupported executable format" on macOS, where GuestImages.cpp already provides dl_iterate_phdr. ProcessParametersFixture::Install also had no macOS branch and returned false; it now fills a __APS5DATA,__meta section with version-2 relinker metadata. Co-authored-by: Himansh Raj <iamthehimansh@gmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
|
I compared the uncomparable patches for the exact changed function. Both open #5021 and #7201 already add the same two |
imellag
left a comment
There was a problem hiding this comment.
Tested head cb2416f8c2 on Apple M4 Pro, macOS 27.0.1, Apple clang 21, x86-64 under Rosetta 2. The tree is main 61fc572cee plus the open macOS build PRs and the parts of mugurc's series that #7201 also carries. Only this change was toggled, with main's versions of ApplicationProcessParameters.cpp, ProcessParametersFixture.hpp and ApplicationHeap.cpp as the base.
ctest -R '^guest_' -j6 (672 tests): 39 fail without this PR, 17 with it, and none fails only with it. The 22 that now pass are:
guest_appcontent,guest_coredump_attach,guest_coredump_user_data,guest_hmd,guest_hmd2,guest_http,guest_kernel_sys_sdk_version;guest_np_commerce_dialog,guest_rtc,guest_rtc_ps5_title,guest_screenshot,guest_screenshot_large_overlay,guest_ssl,guest_usb_storage_dialog,guest_vr_tracker2;- the eight
guest_convert_keycode*tests.
Requested change. application_heap_scripting and application_heap_lazy still abort with this PR, because ApplicationHeap.cpp installs the process parameters itself and has no macOS branch. mugurc's a5a9184e5c adds one with the fixture this PR now fills:
#elif defined(__APPLE__)
#include "ProcessParametersFixture.hpp"
#endif
...
#elif defined(__APPLE__)
require(ProcessParametersFixture::Install(process.data(), process.size()));
#endifWith those four lines added to this PR, all 9 application_heap_* tests pass. Without them, 7 of 9 pass, the same as on main. Since this PR is what makes the fixture work on macOS, could you add them here, so the application heap tests get the same fix as the others?
AI-assisted: yes
ApplicationHeap installed process parameters only on Windows and Linux, so application_heap_lazy had no way to do it on macOS. Use the shared fixture, as a5a9184 does. Co-authored-by: mugurc <mceker@gmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@imellag thanks for the A/B. I added the @MarcZX10 agreed: the two AI-assisted: yes (Claude Code) |
maximilianfv
left a comment
There was a problem hiding this comment.
Approving: on macOS the two __APPLE__ conditions and the fixture fix all 24 tests that abort for lack of process parameters, in 3 of 3 runs, and none of the 40 tests I ran fails with it. The three items below are small and none of them blocks merging.
Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)
Applied at d1f5a3410a to mugurc's macOS series (27e968974) merged with main at 2742004ed2, with other open macOS fixes. The series already carries near-identical macOS branches in all three files this PR touches, so for both arms those three files were reset to main's content. The rest of the series stayed in place, and only this PR's diff was toggled. 40 tests (application_heap_* plus every guest_* test whose source includes ProcessParametersFixture.hpp, plus 7 neighbouring tests the name prefixes matched), 3 runs per arm:
application_heap_* (9) |
the 22 fixture guest_* tests |
guest_ssl_transport_kernel, _net |
other 7 | |
|---|---|---|---|---|
| Without this PR | 7 pass. scripting and lazy abort with application heap: unsupported executable format |
all abort or fail. Install returns false (process parameters were not installed) |
fail | pass |
| With this PR | 9 pass | 22 pass | fail, same check: a server certificate without a loaded CA passed the initial verify options |
pass |
The same results came up in all 3 runs of each arm. The SSL transport failure is the separate libSceSsl gap that the description already names.
Findings (non-blocking)
core/libs/tests/ProcessParametersFixture.hpp:96:headers[0].p_flagsis left at 0. In a fixture test that registers the main image (GuestAllocationsRegisterMainImage, reached fromsceKernelMapDirectMemory),collectImagePagesincore/libs/prx/libc/src/GuestAllocations.cpp:286-287then marks the page ofprocessas neither readable nor writable, although the test writes to it. None of the 40 tests fails from it. Fix:headers[0].p_flags = PF_W;(PF_Wis inmain'sElfTypes.hpp,PF_Ris not, andcollectImagePagestreatsPF_Was readable and writable).GuestFlexibleMemoryModule.cppsetsPF_R | PF_Won its header.- Credit: closed #7332 (ycarogabriel-lgtm, opened 2026-10-18) has the same
ApplicationProcessParameters.cppchange (same blob pair,5efa1f00d31..8a556bd0e01) and the first macOS__APS5DATA,__metafixture. mugurc'sa5a9184e5cnames it ("#7332 makes the same change to the fixture"). Please link #7332 underWhatand add aCo-authored-by:for its author tocb2416f8c2, next to the #8658 credit. - Description:
- The fixture's layout is the one the macOS relinker writes (
MacOsImage.cpp) andGuestImages.cppreads. The Linux branch builds no such layout: it rewrites a spare program header in place. "the same layout the Linux branch builds" should say so. ctest -R '^(guest_|libc_|libkernel_)': no test names start withlibc_orlibkernel_, so that run covered onlyguest_*.
- The fixture's layout is the one the macOS relinker writes (
AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this review).
What
On macOS, any guest that uses the application heap threw
unsupported executable format. The reason is thatApplicationProcessParameters_nid_no_patchonly searched for the process parameters withdl_iterate_phdron Linux. macOS already providesdl_iterate_phdrthroughGuestImages.cpp, so this PR widens the two#elif defined(__linux__)checks inApplicationProcessParameters.cppto|| defined(__APPLE__).ProcessParametersFixture::Installhad no macOS branch and returned false. It now fills a__APS5DATA,__metasection with version-2 relinker metadata, which is the same layout the Linux branch builds.The Linux and Windows branches are unchanged. The fixture change follows iamthehimansh's
feat/macos-port(#8658), and iamthehimansh is credited as co-author.Tested
macOS: Apple Silicon M2, macOS 27.2, AppleClang, x86_64 Release, on a local integration of
main+ #10419 + #10366 + #10369 + #7631 + #7804 + #4630 + #10552 + #10853 + this commit:guest_http,guest_rtc,guest_rtc_ps5_titleandguest_sslpass (they failed before);guest_ssl_transport_kernelandguest_ssl_transport_netalso need the separate libSceSsl symbol PR;application_heap_*tests pass (with theApplicationHeap.cppcommit, from mugurc'sa5a9184e5c);ctest -R '^(guest_|libc_|libkernel_)': no new failures.Building on macOS needs #10552 and the PRs listed above.
Linux/Windows: not built locally. Only
__APPLE__branches are added.Checklist
main. Overlap: feat(aot): consolidate native ARM64 compiler, runtime and consumers #5021 and Macos port #7201 also add the two__APPLE__conditions inApplicationProcessParameters.cpp; the test fixture and theApplicationHeap.cppchange are only hereNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository🤖 Generated with Claude Code