Repository navigation
Conversation
The console's libSceLibcInternal mergesort checks its arguments as FreeBSD's merge.c does (size under 4 is EINVAL, a scratch block of count times size plus 8 bytes), but the guest export was a plain bottom-up merge, so it called the comparator in a different order and number of times. It now runs merge.c's natural/pairwise setup with insertion sort of the last four or five elements, the linked runs kept inside both buffers, and the switch to exponential search after six linear wins. guest_qsort checks the comparator call sequence for four inputs against libbsd's copy of merge.c, and 12-byte elements on an unaligned base. 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.
|
|
verdict: OK
Freshness envelope: head c84e076; merge-base 0cd07dc; behind main: 0; git merge-tree --write-tree origin/main pull/12271: exit 0, clean. Cross-PR ledger: open non-draft PR #11621 conflicts with this head in FileAndHeap.cpp; #11833 conflicts in GuestQsort.cpp; #12054 conflicts in docs/dev/debt/libc.md. Pairwise git merge-tree exits 1 for each, so those heads need conflict resolution if combined. |
|
Correction to the ownership statement: #11621 and #11833 are mine; #12054 belongs to @Aspct3434. I can set the order of my own contributions, not that contributor's work. #11621 and #11833 remain the declared predecessors for this PR, and I will refresh/retest the combined branch after they land. The reported shared-file conflict with #12054 is in AI-assisted: Claude Code; ownership/coordination clarification by Codex. |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The algorithm is explicitly adapted from FreeBSD 11.4 merge.c, and the new implementation closely follows that file, but core/libs/prx/libc/src/Mergesort.cpp has no source attribution/license notice. FreeBSD’s source license requires source redistributions to retain its copyright notice, conditions and disclaimer. Please add the applicable notice (including the Regents of the University of California / Peter McIlroy attribution) to the new source file, or document the permitted provenance if this is an independent clean-room implementation. I checked the exact 11.4 source header; this is the only blocking finding from this pass. The algorithm comparison and reported Linux/Windows tests are otherwise detailed, and current hosted build, relinker, sanitizer, conventions and registration checks pass.
…gesort Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Added in 6c42e74. AI-assisted (Claude Code). |
|
@B-Howell one more thing on this port, now that the maintainers have decided it for all borrowed code: please put FreeBSD's original copyright notice and licence text from |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean Done in e80f703. The top of AI-assisted (Claude Code). |
|
Manual follow-up to the skipped comparisons in the overlap report (comment6462559173). Reviewed target head: I inspected the listed shared-file patches for all 28 candidates. No duplicate target export/implementation was found in this scoped review. The table records the specific shared-file changes and integration requirements; shared debt/header changes still need to preserve both sides. This is not a claim that every pair merges cleanly or that a combined branch has been built/tested. This follow-up addresses the skipped-comparison request only. Existing CI limitations, dependency ordering and outstanding re-review requirements remain in force; approvals or tests from older heads do not become current-head evidence through this audit. Delta review: added #10002 ( Candidate heads, shared files and manual dispositions
AI-assisted: yes (Codex). Current skipped-scope supplement at published target
This is a review of the actual shared debt patches, not independent reproduction of those measurements or a review of either candidate's implementation. No clean combined merge/build/test or author-order agreement is established. The original 26 rows, the earlier #10002/#12397 supplement and their historical pins remain unchanged; #11850/#11870 remain recorded historically although absent from the current skipped list. The bot still lists #12054 as the sole compared pair. Its displayed debt locus moved from L153-154 to L154-155 after base movement; that offset-only report delta adds no pair and resolves no coordination obligation. Manual-source follow-up for current #12271 head
At #12271 head For these pinned patches, preserve the qsort/qsort_s implementations and tests together with #12271’s separate Mergesort.cpp registration, comparator tests/calls, and debt entry. This is a source-scope comparison only: it does not claim a clean pairwise merge, combined build/test, or clearance of other rows in the bot table. It does not set a dependency order or author agreement. The latest #12271 human approval remains pinned to its exact current head; CI and declared prerequisite status are reported separately. October 30 — two additional libc debt scopesFrozen target #12271: #12605 updates eight existing signed/unsigned min/max vector-helper debt bullets to name std::invalid_argument versus the documented firmware13.00 invalid-range faults; nonempty bundled-libc/host evidence remains distinct. #12659 adds one scaled-floating NaN debt bullet retaining non-C-locale, other-SDK and console-execution unknowns. These are the notes’ recorded provenance, not new measurements from this review. The same two immutable candidate patches are reused for the two targets, with separate target-specific preservation dispositions.
These are fresh pinned debt-composition notes only. They establish no same-export, implementation/dependency, author-order, composed runtime or console-verification conclusion; the shared path alone settles none of those. No new source edit, build, test, cache/ref change or public write was performed. This does not refresh every retained row or clear the whole current table; existing readiness/integration gates and old test pins remain unchanged. AI-assisted: yes (Codex). October 30 - pinned #12659 debt follow-upAt Our target remains AI-assisted: yes (Codex). Source-preservation follow-through for #12271 at
Owned contract retained: Selected owned input is CMake, so the reviewed contract is guest_qsort_tests registration, not unprovided GuestQsort.cpp assertion bodies: source GuestQsort.cpp, LIBS_INCLUDE_DIR, link libc, configure_windows_unwind, test guest_qsort, TIMEOUT 20, WIN32 libc directory PATH prepend. Keep BUILD_TESTING nesting and existing source list. No configure or inventory was run; no claim about future discovered test counts. Proof classes: 5 actual-merge-base raw reconstructions; 0 cached complete-primary patches. Existing published ABI/runtime evidence remains pinned to its original head/control/environment; no new build, test, merged composition, author ordering or whole-table approval is claimed. AI-assisted (Claude Code). Bounded libc debt-source supplement for #12271Target source Keep mergesort's own null/overflow exceptions versus the inspected SDK binary, size-under-four EINVAL/failure behavior, FreeBSD source and libbsd comparator-order check explicitly distinguished from a console run. The selected added/deleted debt paragraphs concern the following separate subjects; none replaces this target's own debt paragraph. During composition preserve both, and resolve overlapping foreign replacement paragraphs against the actual implementation rather than restoring stale text from hunk context.
Specific source distinctions:
This is AI-assisted source/documentation review. Shared prose supplies no new prerequisite, author consent or runtime qualification. Existing source/interface, CI and explicit dependency obligations remain separate. New captured source intersections, source onlyAgainst owned
No foreign implementation, pairwise runtime, author order, new mergesort dependency or whole-table clearance is claimed. Historical tests keep their original pins. AI-assisted (Codex). Bounded added-family supplement for #12271 at Reviewed #12688
These source findings add no native-I/O cancellation/runtime qualification or dependency agreement. They do not prove textual mergeability, combined execution, console behavior or clearance of the rest of the overlap tables. Historical tests keep their original pins. Frozen shared-file source follow-up for #12271 (2026-10-30 snapshot)Source target At the publication guard on 2026-10-31, the latest bot update (10:35:33 UTC) listed24 PRs /42 selected path tuples. This frozen30-PR/54-tuple review does not clear that newer selection; its newly listed #10780 debt-file tuple remains unreviewed here. Removed rows do not establish merged status or no overlap. The selected files were checked at the pins below against their actual common bases and current target text.34 private per-file compositions are clean;20 need explicit textual resolution, including adjacent insertions and replacement-paragraph conflicts. These are source dispositions, not whole-PR merges, builds or runtime results. Keep
The published #12688/#12700/#12965 findings retain their original source scope: their current selected target files are byte-identical to reviewed3483366. #12682 has the same selected base/head bytes as#12688/#12700; #12965 retains its distinct base and offset-only patch difference. In particular, #10405’s stream-state reset requires its matching header/state implementation, absent from this target’s current header; this isolated hunk is not qualified for transplantation. #5021’s native-graphics guard intentionally suppresses later test registration in that configuration. Neither establishes a new mergesort dependency. This update does not approve foreign implementations, combined runtime, console behavior, author order, current-main integration or all checks. Existing #11621/#11833 qualification and all historical test/review pins remain unchanged. AI-assisted (Claude Code). #10780 skipped debt-file comparison (source-only)At #12271 head Sources: #12271, #10780. Exact target and counterpart file heads/bases are pinned in the private source-disposition receipt. Scope is only these two exact file deltas. This does not verify #10780 implementation or console behavior, and does not claim any other overlap row, clean merge, combined qualification, approval or landing. |
|
Follow-up to the actual compared conflicts in overlap comment6462559173, for this PR at I inspected the listed shared-file patches for 1 compared candidate at the heads below. This is separate from the earlier skipped-comparison review.
These scoped findings do not establish clean pairwise merges or a successful combined build/test. They do not clear other review, CI, dependency or current-head verification obligations. @Aspct3434 For the shared debt edit I propose #12054 first, then I will preserve your environment-lifetime paragraph when refreshing this PR after its existing #11621/#11833 predecessors. Please confirm or propose another order. I will add the confirmed ordering dependency to the checklist; no agreement is claimed yet. AI-assisted: yes (Codex). |
|
@MarcZX10 Please re-review the blocking license finding in review 5680044826 against current head The original FreeBSD 11.4 copyright, Peter McIlroy attribution, conditions and disclaimer were added verbatim in The integrated Windows MSYS2 UCRT64 GCC16.2 build passed, AI-assisted: yes (Codex). |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — changes requested
License finding resolved
I compared the notice in exact head fa5b07684d1be2a632b7e60fed67744dc1080bf2 with the 11.4.0 merge.c source. The 1,680-byte copyright, Peter McIlroy attribution, three redistribution conditions, and disclaimer are byte-for-byte identical to FreeBSD's source. The SPDX identifier and source link are also present. My earlier missing-attribution finding is resolved.
The author reports the exact-head Windows guest_qsort run passing three times; hosted Windows, relinker, conventions, and eligibility checks pass. I did not rerun tests locally. The reported runtime integration used main 319621ba2e; current fetched main is 2960e9a6e7, 232 commits ahead. The current-main merge preview is clean, but the focused runtime check has not been reported on that current integration.
Remaining repository blockers
- The shared
docs/dev/debt/libc.mdedit with open #12054 still has no author agreement on order. Your proposal is #12054 first, but #12054's author has not confirmed it and its checklist still saysDepends on: none. Please get agreement and declare the dependency on the second PR; the declared #11621 and #11833 prerequisites remain open as stated in this PR. - Please refresh onto current
mainand rerunguest_qsorton the integration that preserves the ordered debt edits and open prerequisites.
I am requesting changes only for the unresolved order and current integration validation; the license notice is accepted.
|
@MarcZX10 Updated and normally pushed #12271 to The committed tree The focused build uses Please re-review this exact head. #11621 and #11833 remain open prerequisites, and new-head CI, human approval and upstream landing are still pending. AI-assisted: yes (Codex). |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — new head, looks good
The license notice is now present and matches the FreeBSD 11.4 source; the remaining order and current-integration points are addressed or correctly tracked as dependencies.
What I ran
Reviewed exact head 71e256be573e11d1e59d5ba8b4b0d6cefbcd4eff, PR base 4c8cff191609bc5f28bf9ee2f93672b531285aa4, and current main fdd1753204a4732e1011539cc9d3bb69f70b27a6. The merge preview is clean. On Linux, without building, I ran:
git diff --check upstream/main...refs/review/pr-12271
python3 tools/check_conventions.py --base upstream/main --head refs/review/pr-12271
git merge-tree --write-tree upstream/main refs/review/pr-12271
cmp -s /tmp/anyps5-merge-license.txt /tmp/freebsd-merge-license.txtThe cmp compares the 31-line BSD notice from the exact PR head with the corresponding notice from FreeBSD merge.c at commit 8c639945a44588f6cfdddfb057bb5aad88735da0. I also confirmed Mergesort.cpp is byte-identical to the previously reviewed source. I did not build or run tests locally.
Results
| Check | Result |
|---|---|
| Diff whitespace check | Passed. |
| Repository conventions | Passed. |
| Current-main merge preview | Clean; tree eb3ff54249f8134c5aab963bde0a05ba3d2a2a45. The only PR-touched path changed on main since the PR's base is the debt document; the merged preview preserves both the mergesort and existing debt entries. |
| BSD notice comparison | Passed; both 31-line files have SHA-256 ea45247eefad9ad1bc05d4c8fc96a80fcda304a01ed54e570ffeceb7bb75ec36. |
| Author-reported tests | The author reports the tested tree f215ab2e48acec003def2a008340fa54d041bec6 matches this head and guest_qsort passed 3/3 on Windows 11/MSYS2 GCC 16.2.0. |
| Hosted CI | Windows, Ubuntu register-context, relinker, ASan, conventions, eligibility and report passed; Linux product tests were skipped; macOS jobs were pending. |
Raw output from local checks
diff_check_exit:0
All conventions checks passed.
eb3ff54249f8134c5aab963bde0a05ba3d2a2a45
license_cmp_exit:0
ea45247eefad9ad1bc05d4c8fc96a80fcda304a01ed54e570ffeceb7bb75ec36 /tmp/anyps5-merge-license.txt
ea45247eefad9ad1bc05d4c8fc96a80fcda304a01ed54e570ffeceb7bb75ec36 /tmp/freebsd-merge-license.txt
Sources
Not verified
I did not independently run the Windows guest_qsort test or verify console behavior. The Windows results are author-reported; macOS jobs were still pending when checked. #11621 and #11833 remain open and are declared prerequisites; this review does not approve their changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Qualification update for #12271: published-candidate source Frozen-main refresh qualification: Windows/MSYS2 UCRT64 GCC 16.2.0 Rev6 and Linux/WSL GCC 11.4, Release, separate new build directories, at most two jobs, serial arms/platforms. The isolated PR candidate passed the existing
Eight native executions total, zero failures/skips; one registered test per arm, 20-second test timeout. Source groups differ as the PRs add their regressions. Every arm built production libc and its own fixture; Windows PE/CTest search paths and Linux ldd resolve the corresponding module. Recorded conventions checks passed on all eight arms. Independent source and result reviews passed. The Mergesort implementation blob and FreeBSD notice remain identical to the previously qualified topic; main qsort_r ordering and validation coverage is retained. The private combined check preserves both helper groups and calls; its result is separate from either public PR and does not establish landing order. Historical algorithm, corpus, negative-control and hosted results below retain their original pins and were not repeated by this refresh. Dependency agreements, current-tip follow-through, human/hosted-CI review, remaining overlap obligations and verified main landing remain open. AI-assisted: yes (Codex). Hosted CI follow-through on
|
What
mergesort(libSceLibcInternal, implemented in libc) now sorts with FreeBSD 11.4'smerge.c. Before this, it was a plain bottom-up merge, so it called the comparator in a different order and a different number of times than the FreeBSD/libbsd reference.The existing debt entry records argument checks from
libSceLibcInternal.sprx(SDK 4.5x build ofJ03950001, 0x762D0) that match merge.c's checks:PSIZE / 2(4) setsEINVALand returns -1;count * size + PSIZEbytes.Those argument checks motivate using the FreeBSD reference; they do not establish the console algorithm or its comparator-call sequence. Comparator ordering here is verified against FreeBSD/libbsd and remains unmeasured on a console.
What the implementation does, all from merge.c:
setup, natural and pairwise. The last 4 or 5 elements (all of them when there are 5 or fewer) go through its insertion sort. A run ofTHRESHOLD(16) same-sense pairs becomes a natural run, and a descending one is reversed in place.EVAL). The buffers swap roles each pass.bigstate across merges and passes, as merge.c does.base, the frees and the return values are merge.c's.It moves out of
FileAndHeap.cppintolibc/src/Mergesort.cpp. A null base or comparator, and a size product that overflows, still throw: the console does not check them. The debt entry is cut to those cases.Tested
Frozen f776 refresh pair - Windows: the ordinary merge candidate
25d9d7b9f71ec609d98d7e766029336b2fb5de19(treef4a5a7733bae8777f2657025dbc9aed3921ef72a) passed the existingguest_qsortCTest once (1/1) after the matching frozen-mainf776fddcffd1cfece220ecdd894371988b164f6acontrol also passed once (1/1). Both arms used Windows native Release/Ninja, WinLibs GCC 15.2.0 r7,BUILD_TESTING=ON, at most two build jobs, and the sameImageOneDimensionalArrayCMake selection; each builtguest_qsort_testsand libc before running CTest^guest_qsort$. Conventions passed againstorigin/mainwith 0 errors and two pre-existing debt-length warnings. This is a focused Windows libc test pair only; it does not claim Linux, GPU, console, full-suite, or hosted-CI results. Historical results below remain pinned to their stated commits and were not retargeted.Frozen-main refresh qualification:
3483366e8300fd3c06c711026e7f7c15ae76e6a9on main3d19f3fed31be38d7196ea1bd74e29ab6cc3c9a4.Windows/MSYS2 UCRT64 GCC 16.2.0 Rev6 and Linux/WSL GCC 11.4, Release, separate new build directories, at most two jobs, serial arms/platforms. The isolated PR candidate passed the existing
guest_qsortCTest once on each OS. All four frozen arms passed:3d19f3fed31be38d7196ea1bd74e29ab6cc3c9a466b4ecb13df784ba4a49c777d054bed39c2606503483366e8300fd3c06c711026e7f7c15ae76e6a99741133de15e810cc06b4cc69fec378bdd053b04Eight native executions total, zero failures/skips; one registered test per arm, 20-second test timeout. Source groups differ as the PRs add their regressions. Every arm built production libc and its own fixture; Windows PE/CTest search paths and Linux ldd resolve the corresponding module. Recorded conventions checks passed on all eight arms. Independent source and result reviews passed.
The Mergesort implementation blob and FreeBSD notice remain identical to the previously qualified topic; main qsort_r ordering and validation coverage is retained. The private combined check preserves both helper groups and calls; its result is separate from either public PR and does not establish landing order. Historical algorithm, corpus, negative-control and hosted results below retain their original pins and were not repeated by this refresh. Dependency agreements, current-tip follow-through, human/hosted-CI review, remaining overlap obligations and verified main landing remain open.
Earlier verification — original pins retained:
Latest integration verification:
71e256be573e11d1e59d5ba8b4b0d6cefbcd4eff, preserving published history and integrating main snapshot4c8cff191609bc5f28bf9ee2f93672b531285aa4.f215ab2e48acec003def2a008340fa54d041bec6exactly matches the isolated comparison that was rebuilt and tested on Windows with MSYS2 UCRT64 GCC 16.2.0, Ninja Release,BUILD_TESTING=ON.cmake --build build-compare --target guest_qsort_tests --parallel 2succeeds;ctest --test-dir build-compare -R '^guest_qsort$' --repeat until-fail:3 -Vpasses all three executions, with no skips. CTest uses the rebuilt unpatchedlibc.prx. The five-file PR implementation/test diff is preserved; current-main environment and other debt entries are retained, with only the mergesort entry replaced.CMAKE_SKIP_INSTALL_RULES=ONbecause the normal configuration encounters an nghttp2 installation/export-set dependency error. Installation/export generation is unverified. No fresh Linux/macOS, title, GPU or performance result is claimed.Earlier verification retained below:
Earlier integration commit
fa5b07684d1be2a632b7e60fed67744dc1080bf2(verified tree478ba5f7e2c2c45375f627a8e08b8d29d3d42dff), merging frozen main319621ba2e578bd053b0baba2877fe70520e0737without rewriting the published history:cmake --build build-focused --target guest_qsort_tests --parallel 2passed;ctest --test-dir build-focused -R '^guest_qsort$' --repeat until-fail:3 -Vpassed all three executions (test491: 0.03/0.01/0.01sec).Earlier published-source reference and native evidence follows:
merge.c. In WSL Ubuntu 22.04, I loaded itsmergesortwithdlopenand compared it with this implementation, compiled with ASan and UBSan.guest_qsort,CheckMergesortComparisons: it compares the comparator call sequence against libbsd's for four inputs:CheckMergesortUnalignedElements: 1,000 12-byte elements on a base 4 bytes off alignment.guest_qsortpasses 3 of 3 runs.libsbuilt):guest_qsortpasses 3 of 3 runs.main'sFileAndHeap.cppandCMakeLists.txtrestored, new tests kept.guest_qsortfails on both OSes: the four inputs make 9, 78, 73 and 107 comparisons, where merge.c makes 6, 84, 42 and 60, in a different order.merge.c, not measured on a PS5.python tools/check_conventions.py --base origin/mainpasses.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)FileAndHeap.cppandGuestQsort.cppand will be rebased after they land)🤖 Generated with Claude Code
CI configuration prerequisite — 2026-10-30
CI-only integration
4980f2197d7db42783f599b5776b25105ad48405(treec1d2e4c2f70bb8b867a0d505deed4bebd1c206b8) retains original #13220 commite9f09f54fabb6408d3e2711b8df10c10112ab00eas its second merge parent, after3483366e8300fd3c06c711026e7f7c15ae76e6a9. Whole-tree and source-pin verification confirms onlycore/libs/tests/write_watch/CMakeLists.txtchanges: five lines deleting the repeated execute-fault/read-fault registration. No libc implementation or fixture source changes.Configuration-only comparison used the exact exported four-file graphs: the old graphs match across #11831/#12271, the fixed graphs match across both integrations, and all three companion C++ files are unchanged. Two shared arms per OS were configured serially: Windows CMake 4.4.4 / WinLibs GCC 15.2.0 r7 / Ninja 1.13.2 and WSL CMake 3.22.1 / GCC 11.4 / Ninja 1.10.1. Both old arms failed on both duplicate names after genuine native compiler/Threads probes; both fixed arms configured/generated successfully. Each fixed generated inventory contains exactly 18 unique name/executable/mode mappings with 30-second timeouts. These are four configure checks (two expected RED, two GREEN), not 18 runtime passes. No fixture build or CTest execution occurred.
The isolated two native sort passes remain pinned to
3483366e8300fd3c06c711026e7f7c15ae76e6a9; the earlier four-arm/eight-execution group retains all four original pins on frozen3d19f3fed31be38d7196ea1bd74e29ab6cc3c9a4. Those executions were not repeated or retargeted. Existing #11621/#11833 dependencies, landing order, current-tip follow-through, human/bot review and verified main landing remain open.New-head hosted CI qualification remains pending after publication. No macOS tests were run locally; the earlier hosted failure and all historical results retain their original heads. This configuration result does not establish platform/runtime, whole-product or dependency-order clearance.