Repository navigation
fix(libSceLibcInternal): preserve combined POSIX spawn flags - #11841
oneandonlydean merged 2 commits into
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the changed POSIX path: the supported guest bits are accumulated into one short host mask and applied with one posix_spawnattr_setflags call, preserving both flags when supplied together. The Linux fixture observes the actual native attribute flags and checks process-group behavior; the PR reports three repeated Linux runs plus existing Windows regressions. I did not independently build the full libc target in this checkout. Eligibility, intake and conventions pass; Windows product CI, labels and the progress report remain pending, so this is not an approval.
|
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.
|
|
Thanks for the review. No code changes were requested. At head I also checked the newer overlap-bot warning manually. With current main |
|
Additional integration validation is complete for PR head Both Windows 11 / WinLibs GCC 15.2.0 r7 and Ubuntu 24.04.1 under WSL2 / GCC 13.3.0 built the complete This adds full-library integration build coverage to the earlier focused-target evidence. It does not claim a full application or all-test-suite run. No contribution code changed for this check; the published PR head remains |
What
Spawning a POSIX child with guest flags
0x03(RESETIDS | SETPGROUP) currently callsposix_spawnattr_setflagstwice. The second call replaces the first, so the host receives onlySETPGROUPand loses the identity-reset request. Combine the translated bits into one host mask and set it once. Zero flags and each individual flag retain their existing behavior; the Windows implementation is unchanged.The bit definitions and combined application follow FreeBSD 11.4 spawn.h and process_spawnattr. No exports, public layouts, third-party dependencies or supported-flag ranges are added. Existing console/sandbox and unsupported-attribute gaps remain recorded in the library debt.
A dedicated Linux fixture observes the actual native attribute object with
posix_spawnattr_getflagsat the host call boundary, forwards the unchanged call to the real libc function, and checks each child's process group and exit status. It covers flags 0, RESETIDS, SETPGROUP and both together. The observer is test-only; production uses the native API directly. The existing spawn test remains byte-for-byte unchanged, and the new fixture has its own working directory. This keeps it independent of the macOS declaration change in #11358 and the stopped-child synchronization change in #11520.Tested
Candidate
d3d7ae0ae2946e9d539dfe85aa8bc43480254de2, based on upstream5d52b06ff3de0b2ce640dd296db465295c80e7fa.Local Ubuntu 24.04.1 x86-64/WSL2, GCC 13.3.0, and Windows 11 x64, WinLibs GCC 15.2.0 r7: Ninja Release, bundled dependencies, BUILD_TESTING=ON, ANYPS5_ENABLE_SPIRV_TOOLS=OFF. The real project
guest_posix_spawn_teststarget builds on both hosts; Linux also buildsguest_posix_spawn_flags_tests. The existing spawn/reaping test passes three consecutive runs on each host; the dedicated Linux flag fixture also passes three times, andpublic_headerspasses on each host. No selected test was skipped. Windows runs its existing regression cases; the native flag observer and process-group checks run on Linux.Regression-only commit
e8db1c1648ab40b33e6e5509461856a0fcf19f38reproducesSpawn flags 3: expected host flags 3, observed 2against the unchanged implementation. The corrected candidate passes the same committed fixture. Testing used unprivileged hosts: the observer verifies the RESETIDS request, while a privileged real/effective-ID transition was not exercised. No macOS, title or console-hardware execution is claimed.Conventions and whitespace pass. The live overlap scan reports no conflicting hunk, same export or same new file after isolating the fixture. The shared CMake file was checked manually against the two pre-conflicting peers: #5021 retains one existing conflict and #10953 has none in this file; the new registration is outside their conflict hunks.
Rechecked upstream
801d5b9bc11049e6274daca09d70690bd5b8d0a9immediately before publication: this branch merges cleanly, and none of the affected spawn implementation, attribute header, test or CMake files changed after the tested base. Results above are pinned to the submitted head.Additional integration validation is complete for PR head
d3d7ae0ae2946e9d539dfe85aa8bc43480254de2merged with main8ff4baf53489033dae79090640143e82d159ee4dat local merge commit75a02a10b8e2e11ef9da68db1cd99c511bc16a9d.Both Windows 11 / WinLibs GCC 15.2.0 r7 and Ubuntu 24.04.1 under WSL2 / GCC 13.3.0 built the complete
libstarget plus the selected spawn executables. Both public-header audits passed. The existing Windows spawn regression passed three runs; the existing and new Linux spawn fixtures each passed three runs, with zero skips. Linux checks include the observed native flag mask and actual child process groups.This adds full-library integration build coverage to the earlier focused-target evidence. It does not claim a full application or all-test-suite run. No contribution code changed for this check; the published PR head remains
d3d7ae0ae2946e9d539dfe85aa8bc43480254de2.Checklist
Based on current
main; no other open PR implements the same functionsOne 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 under Silent stubs in their library's technical debt fileNo notes, investigation
.mdfiles 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
Fixes: none
Unblocks: none
GPUs tested: n/a
AI-assisted: yes