Repository navigation
Conversation
|
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.
|
78c1e2d to
fe01208
Compare
|
Rebased onto 677354d (was e51d4e1) and reconciled with main without deleting main content. New HEAD fe01208, MERGEABLE, conventions pass. Per file:
Verified: Release/Ninja build OK; guest_crt/fiopen/locale/ampr_markers/wide_strings + qsort/wide_conversion/math/mktime/strftime/time_conversion/allocating_strings all pass; git diff --check clean; check_conventions --base origin/main clean. Fail-without/pass-with holds: disconnecting Ampr slot fails guest_crt (abort), restoring passes. Overlap audit: true coordination needed with #6133 (adds SonyModti3 inside same namespace{} - must rebase onto this) and #6707 (touches SonyCrt.hpp handler typedef/namespace + qsort_s per-TU handler docs, will need shared update). #7068 (fopen_s host errno) still open, reconciled here as handler+errno; everything else rebase-only disjoint. |
|
CI note: everything runnable passes (conventions, register-context x4, relinker on Linux/Windows/macOS, write-watch x2). The full Linux + Windows builds need collaborator dispatch (automatic full builds only run on main) - requesting dispatch when review allows. |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
|
Pre-review verification found a link failure — the handoff branch builds, but |
fe01208 to
5acfae9
Compare
|
Handoff picked up, thanks @blawesom - branch reset to ca94073 (authorship intact) plus one fix commit on top, and thanks for the Linux verification. fopen_s errno verdict, checked hunk by hunk: the constraint-handler invocations are kept as intended (null result/filename/mode invoke the handler and return EINVAL 22, per Annex K which returns the passed error - matches GuestFiopen/GuestCrt). One correction to the handoff: the open-failure path returns the guest-translated errno, not a hardcoded ENOENT - that is main's newer behavior (GuestErrnoFromHost in fopen_nid_postfix, debt note included) and GuestFopenErrors requires it for the ELOOP/ENAMETOOLONG/EISDIR cases, where a hardcoded 2 would fail. I also removed the now-unused GuestEnoent constexpr the merge left behind. Link failure fixed via extern C _nid_no_patch, the same mechanism as AddPathProvider_nid_no_patch: the four Sony CRT entry points Ampr calls (SonyLocaleconv, SonySetConstraintHandlerS, SonyIgnoreHandlerS, and SonyInvokeConstraintHandler via the inlined SonyWcscpyS) are now published under literal _nid_no_patch names, so the NID patcher leaves them verbatim and the patched libSceAmpr.prx resolves them from libc.prx. Root cause was that plain C++ names get NID-hashed in libc's exports while Ampr still imported the mangled names. Handler slot and lconv storage stay single-definition in libc SonyCrt.cpp - no inlined copy, shared state intact. Verified (Release, Ninja, BUILD_TESTING=ON, Windows): full patched-libs deploy green - patched_libraries passes (previously libSceAmpr.prx failed with error 127); unit gate green: guest_crt, guest_fiopen, guest_locale, guest_ampr_markers, guest_wide_strings, plus guest_safe_errors, guest_wide_conversion, guest_amm_placement, guest_ampr_threads, guest_ampr_cpp_error, guest_rune_locale, guest_collate, guest_errors, guest_posix_scalar_io. git diff --check and tools/check_conventions.py --base origin/main --head HEAD clean. |
5acfae9 to
347c538
Compare
github-actions
Bot
added
unblocks title
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
Thanks, the shared handler/lconv state across the libc/libSceAmpr boundary is a real gap and the _nid_no_patch exports are the right mechanism. Before this can merge:
- Conflicts with
main(SonyCrt.hpp,FileAndHeap.cpp,docs/dev/debt/libc.md).mainchangedSonyConstraintHandlertoAPS5_VABI, moved the slot/mutex/default handler out of the anonymous namespace, andqsort_s(#6707) andstrerror_snow callSonyInvokeConstraintHandler/ compare againstSonyDefaultConstraintHandler. The out-of-line definitions inSonyCrt.cpp(lines 7-44) need theAPS5_VABIsignature, andstrerror_smust keep seeing the same default-handler pointer. Please rebase and re-runguest_crt,guest_qsort,guest_fiopen. - Drop
SonyLconvVariant::Compact0/SonyLconvCompact(SceTypes.hpp:36,SonyCrt.cpp:76-106,GuestCrt.cpp:94-105). No export uses it; only the test does, and the debt note itself says the layout is unverified. An unreferenced, unverified layout is dead code. Re-add it in the PR that wires a title-backed export to it, with the evidence. GuestCrt.cpp:275: the abort check re-launchesargv[0]throughstd::systemwith a hand-quoted path; that is shell-dependent on Windows. PreferCreateProcess/fork-free alternatives already used by other guest tests, or assert the default handler pointer instead of spawning.
The CI on this head only covers Conventions/relinker; the full Linux + Windows Build will run in the merge queue after the rebase.
|
Hi Thank you for your patience with this one, and thanks to BunnyFl for the firmware 13.42 data on our fopen_s question. That data settles the question, and main has since caught up on that part. On current main, fopen_s already calls the constraint handler for a null So the rebase (the branch conflicts with main in
The core of the PR is still needed: on main, libSceAmpr includes |
What
lconvstorage betweenlibcandlibSceAmprthrough a single definition inlibc(core/libs/prx/libc/include/SonyCrt.hpp+src/SonyCrt.cpp). Each module previously owned its own handler/mutex/lconv static (internal linkage per TU), so a handler set via one module never fired in the other.SonyLconvVariant { Ampr48, Compact0 }:SonyLocaleconv()keeps the verified Ampr layout (decimal_point@0x48);Compact0(decimal_point@0x00, no prefix) covers catalog titles reusing the Dinkumware layout with a different offset (e.g. PPSA99902, per review on feat(libs): implement C runtime re-exports for libSceAmpr #545). Layouts pinned withstatic_assert(offsetof(...)).libSceAmprre-exports the shared state (0hlfW1O4Aa4localeconv,6f5f-qx4ucAwcscpy_s,ENLfKJEZTjE/ODGONXcSmz4handlers) and linkslibc.wcscpy_sstays 16-bit (guestwchar); the default handler still aborts. Both assumptions debt-noted inTechnicalDebt.md.Tested
guest_crttest:lconvoffset asserts (0x48 incl.sceAmprLocaleconv == localeconv, 0x00 compact), cross-TU handler visibility both directions, default-abort via child process. Passes; verified it fails without the fix (slot disconnected → abort0xc0000409) and passes with it.guest_fiopen,guest_locale,guest_ampr_markers,guest_wide_strings,file_position: 5/5 pass (WinLibs GCC 15.2.0, Release).python tools/check_conventions.py --base origin/main --head HEADclean,git diff --checkclean.libstarget has a pre-existing, unrelated failure (libSceVideoOut.prxNID patch:duplicate patched export name: LibwuIonIBw), reproduced on pristineorigin/main.Checklist
main; no other open PR implements the same changelocaleconvreuse (SymphonyStation5/Cloud Play same NID, PPSA99902 compact variant), feat(libc): implement qsort_s #6707qsort_srebase