Repository navigation
fix(libc): return the calling thread from _Thrd_id as the console's libc.prx does - #9170
Conversation
|
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.
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.
|
|
Windows data point for
|
MarcZX10
left a comment
There was a problem hiding this comment.
Review
The one-line forwarding implementation matches the two cited console readings, and the added test checks it with the registered and test-provided thread forwards. I did not find a defect in that forwarding path. Two coordination items and a test-context update are needed before approval.
Same export in #9416
The current overlap report identifies #9416 as defining the same _Thrd_id_nid_postfix export. I confirmed its current diff defines that symbol in libc/Export.cpp returning 1, while this PR defines it in Threads.cpp by forwarding scePthreadSelf(). These are competing implementations of the same export and cannot both be integrated as written.
Please coordinate with the #9416 author and maintainers on the single implementation and landing order, then update the overlap notes and Depends on line if the PRs are intentionally stacked. I’m recording the conflict, not presuming which PR should land.
Title bring-up record
The linked Animal Well discussion currently says there is no blocker and does not list this PR. A newer report in #8636 says the title reaches _Thrd_id_nid_postfix not implemented after #8636’s fix. Please update the discussion with that current test context, add this PR to its open-PR list, and state whether the stop is still reproducible. If it is no longer a blocker on current main, change the checklist’s Unblocks entry accordingly.
Current base and checks
This head is 601 commits behind current main (merge base e072e8840a). Please update it and rerun guest_libc_threads on the rebased head.
git diff --check: passed.- Current conventions, eligibility, intake, labels, and report jobs passed.
- Register-context jobs passed on Ubuntu and macOS (Debug/Release); relinker jobs passed on macOS, Ubuntu, and Windows; relinker ASan and both write-watch jobs passed.
- Linux and Windows product-test jobs were skipped. The local and fork test results in the PR description are author-reported on the older base; the full Linux run included one intermittent AGC test failure on the PR side, which the report identifies.
The forwarding implementation is supported by strong source evidence. Please reconcile the competing export, title record, and current-base validation before merge.
|
Code review: |
ce50ab7 to
eca9862
Compare
|
Rebased onto current |
eca9862 to
64f3dc9
Compare
|
Rebased onto current |
|
The description already cites two readings of the export body; this adds the other side, how a title uses the value: a call-site data point for
This agrees with Not shown by the call: the value width beyond what the comparison needs, other titles' use of AI-assisted (Claude Code). |
|
@xXPBSXx thanks, that is the call-site side the description lacked: a title comparing all 64 bits of |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the implementation against the evidence summarized in the PR: the console export returns the result of scePthreadSelf unchanged, and _Thrd_current uses the same registered forward. The added test covers the registered kernel forward, a test double, and the missing-forward failure path. The author reports the focused Linux and Windows tests passing, with CI conventions, eligibility, labels, relinker matrix, register-context and write-watch jobs green. I found no blocking issue.
MarcZX10
left a comment
There was a problem hiding this comment.
The forwarding implementation and its focused test remain supported by the cited console evidence. I need to correct my earlier approval because the live overlap report still lists open #9416 as defining _Thrd_id_nid_postfix, and this PR still says Depends on: none. The #9416 author has not agreed to drop or sequence its competing export, so the repository’s overlap rule is not satisfied. Please coordinate the single implementation/landing order with that author and update the dependency line (or wait for #9416 to close/remove the duplicate). I’ll re-review once that is resolved.
|
@MarcZX10 the competing export in #9416 is a one-line placeholder, |
64f3dc9 to
ac34b9c
Compare
|
Title record updated as asked: the 2026-10-22 retest on #966 (unmodified |
|
The hosted |
adriadam10
left a comment
There was a problem hiding this comment.
The implementation and its test are fine: _Thrd_id forwards to scePthreadSelf like _Thrd_current, matching the cited readings, and guest_libc_threads fails on main. The merge queue refused it because of MarcZX10's change request, which is still unmet:
- #9416 is open and still defines
uint64_t _Thrd_id_nid_postfix() { return 1; }incore/libs/prx/libc/Export.cpp. Two definitions of the same export don't link, so whichever PR lands second breaks the build. The constant1isn't supported by either console reading or the PPSA23827 call site. - To unblock: #9416 drops that line (already asked there), or this PR lands after #9416 replaces it. If you want the second option, record it under
Depends onand say so here. Once #9416 no longer defines_Thrd_id, this is ready to merge.
|
@adriadam10 thanks. I'd rather not sequence after #9416: its |
|
Measured on a PS5: a homebrew payload called the console's |
ac34b9c to
0297073
Compare
|
Rebased onto current |
|
I rebased your commit onto current Branch if useful: https://github.com/edubox/AnyPS5/tree/verify/9170-thrd-id ( One direct data point from a title that imports the export: Cyberpunk 2077 ( |
|
@edubox thank you, that's the most direct evidence yet: a title's own |
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
|
Checked the diff, the test and the evidence: The Windows run's only failure was a 30 s timeout of |
What
_Thrd_id(np6xXcXEnXE) threw on main because its return value wasn't known. Two people have since read its body in the console'slibc.prx, and both found the same thing:np6xXcXEnXE#D#A, ELF vaddr0x4f20(11 bytes), calls the PLT ... and returns with RAX unchanged", with the PLT slot resolving toaI+OeCz8xrQ,scePthreadSelf. "It does not callscePthreadGetthreadid."0xcab10, whose relocation namesscePthreadSelf... and returns RAX unchanged."_Thrd_idwith no arguments and compares all 64 bits of the result with the id its_Thrd_createstored, a self-join check; this works only if_Thrd_idreturns the same pointer-wide value the creation stored.Earlier Animal Well (PPSA22520, #966) reports stopped at
_Thrd_id_nid_postfix not implemented; a retest on 2026-10-22 on unmodifiedmain7e2ba73e4creaches gameplay without this change, so this PR no longer claims to unblock that title. It stays a fix for any title that calls the export, as PPSA23827 does.MarcZX10 asked on #4164 for a focused follow-up once the libc export location was settled, which #8148 did. So
_Thrd_idnow returnsThreadForwards(__func__).self(), the samescePthreadSelfthat libkernel registers and that_Thrd_currentalready returns. It hands back the full pointer-width value, as the console body does (RAX unchanged), so a caller that only reads EAX gets the low half either way. With no thread forwards registered it throws, like_Thrd_current.The libc debt entry for
std::_Pad, _Thrd_join, _Thrd_detach, _Thrd_idnow says what_Thrd_idreturns and where that comes from (a reading of the body, not a measurement) instead of why it throws.Tested
libc.prxbuilds forward the export the same way: in Cyberpunk 2077 (PPSA04029),np6xXcXEnXEat0x4bb0jumps through the GOT entry relocated toscePthreadSelf(aI+OeCz8xrQ), PPSA02530 has the same shape at0x4e20, and the title'slibREDGalaxy64.prximportsnp6xXcXEnXE. They also rebased and verified the change onmainf26434afa3(Linux, GCC 16.2.1).Ubuntu 24.04 x86-64 container (GCC 13.3, Release) under Rosetta on an Apple M5 Mac.
guest_libc_threadsgets three checks:_Thrd_id()equalsscePthreadSelf()with libkernel's forwards, returns the fakeselfresult when the test registers its own forwards, and throws once they're unregistered.Threads.cpp,guest_libc_threadsaborts withwhat(): _Thrd_id_nid_postfix not implemented. With this change it passes.tools/progress.py --module libc: 1217/1225 before, 1218/1225 after.check_conventions.py --base origin/mainpasses.I don't have Animal Well, so I haven't run it. #8636 reports that on main "the title runs until
_Thrd_id_nid_postfix not implemented", which is the throw this removes; what it hits next is unknown to me.Hosted runs on my fork (GitHub runners), this branch
ce50ab75aand its basee072e8840:net_dontwait_blocking_*tests fail on both.agc_driver_sgpr_pattern_fill_keysfailed on one side only; it is intermittent under parallel ctest (agc_driver_sgpr_pattern_fill_keys times out under parallel ctest load #8708).guest_libc_threadspasses (1/1; 1/1 on the base).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)