Repository navigation
Conversation
Progress report (26fa121 → a7d9bb8)📉 System libraries: 26.22% (-0.01%, +10 declared functions) 🆕 10 declared as stubs
Changed libraries (implemented exports)
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The bulk-write guard is not checked before either write. On Windows, GuestArena::HostWrite::Open() can return false when the target overlaps a read-only guest mapping (WindowsMappings::BeginHostWrite rejects views whose protection is not writable). These functions then continue into std::memmove or the element stores, which can fault instead of following the guest-memory access path. Please check access.Open() and handle failure consistently with the library's error policy before writing, and add a Windows regression test using a read-only guest mapped view for both exports. The current tests only exercise ordinary host arrays, so they do not cover this guard failure path.
7f79600 to
92d7db3
Compare
|
Fixed in Rebased onto |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the exact updated head 92d7db3ea8fa81e387a9431a68e2cdbb8d09b478. Both bulk-write functions now stop before mutation when HostWrite::Open() rejects access. The added direct-memory regression checks a read-only page and a destination spanning read-only and writable pages for both u_memmove and u_memset, then verifies the memory contents remain unchanged. Existing overlap, boundary, Unicode search, and code-point comparison coverage remains in place.
I ran python3 tools/check_conventions.py --base upstream/main --head HEAD and git diff --check upstream/main...HEAD; both pass. The author reports the focused Linux runtime test and Windows-specific cases passing; hosted product build/test jobs are skipped, and I did not independently run the full CMake test here. I found the previously requested failure path addressed and no remaining blocking issue in this scoped change.
|
One clarification to the approval’s validation summary: the new Windows read-only regressions are added and source-reviewed, but not executed on my Linux host. My runtime result for this updated head is the Linux ICU build and 1/1 focused test; hosted product builds remain skipped. The PR body and fix reply retain that qualification. Thank you for the re-review. |
|
Tested head
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Thanks for testing Please re-review this head; integration can follow #10966. |
92d7db3 to
444f4df
Compare
|
Prepared a preserving rebase on Please re-review and merge after #10966 lands and the preserving integration rebase, applicable checks and maintainer review permit. |
Remove the 36 stubs whose NIDs open PRs boykopovar#10962-boykopovar#10967, boykopovar#11115, boykopovar#11118, boykopovar#11122, boykopovar#11132 and boykopovar#11136 add to libSceIcu, leaving 112.
444f4df to
7e26b76
Compare
|
Rebased to Please re-review this rebase; after #10966 lands and current-head integration CI passes, please merge in the agreed order. |
github-actions
Bot
added
needs rebase
7e26b76 to
cc73317
Compare
|
Rebased onto frozen main Exact-head Linux Release validation passed: Please re-review and merge after actual predecessor #10966 lands and the applicable full-CI/review gates pass. The agreed ICU landing order is unchanged. |
|
Native Windows validation now covers the published |
|
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.
|
|
Resolved the captured #12320 source/header and test-registration collisions in Fresh normal-root Linux Release: 3 ICU runtime tests +65 public-header tests passed, zero failures/skips, with real ICU/libc/kernel providers and independently checked fresh compilation. Native Windows run 40669914236/a1: candidate 3 runtime +65 header tests passed; separate baseline 2+65 passed (135 total, zero failures/errors/skips), with the unchanged native Windows fixture and its original aborting checks active. Each lane used a fresh normal-root Release build. Required conventions and a static merge check against pinned main |
|
2026-11-01 Windows validation: run 41295117065, Retested unchanged PR head Fresh Linux and native Windows Release builds with real ICU/libc/kernel each passed all 4 functional fixtures and 65 public-header CTests. On each platform, the exact bare-main control passed 3 functional fixtures and 65 header tests. All completed without failures, errors or skips. Native Windows run. Checked run 40855117318/a1, with this 2026-10-30 converter-callback integration update: #12692 actually merged as Exact published inputs: #10962 Fresh native Linux x86-64 Release, GCC 16.2.1: the pinned08a main baseline passed 18/18 selected runtime cases and 65/65 public-header checks; the exact private all-six union passed 26/26 runtime cases and 65/65 headers, with zero failures, skips or Not Run rows. The union executed all 13 ICU fixtures, including These results cover only that fixed baseline and private all-six union, not any bare published head, later main, Windows/native ABI, all-nine integration, full suite, title or GPU behavior. Earlier main750 and other historical results retain their original inputs and scope. The declared order remains #10962, #10963, #10964, #10965, #10966, #10967, then #11088; the already merged #12692 adds no new prerequisite. Actual #10966 merge must come first, followed by a preserving rebase, fresh integration checks and re-review before this PR lands. 2026-10-30 fresh Linux integration check on fixed main Fresh native Linux x86-64 Release builds passed 53/53 selected plain runtime CTests on that main baseline and 63/63 on the private candidate. With Production NID patching and These results apply only to the fixed baseline and private SIG-plus-six-ICU candidate. They add no bare-public-head, later-main, Windows, native-ABI, full-suite, GPU or unload qualification. Earlier results keep their original inputs and scope. The existing predecessor, preserving-rebase, maintainer CI and review requirements and agreed ICU landing order still apply. 2026-11-01 preserving rebase: These results cover this topic’s integration with pinned main. Full maintainer CI and the existing merge order still apply. Please re-review; merge after #10966 and the required maintainer checks. |
babf2bc to
a7d9bb8
Compare
github-actions
Bot
added
needs rebase
What
Implement
u_memmove,u_memset,u_memchr,u_memrchr,u_strchr,u_strrchr,u_strFindFirst,u_strFindLast,u_strcmpCodePointOrderandu_strncmpCodePointOrder. Preserve overlapping moves, full-width fills, code-point boundaries, embedded NULs, defined empty/null shortcuts, streaming early returns and exact comparison differences. Bulk writes holdGuestArena::HostWriteand reject a denied guard before modifying the destination.Head
babf2bc72012bd926f35408b3d602d162a5ecd1bresolves the captured #12320 integration collision by naming this topic's filesUtf16Search.cpp/.hppand its existing fixtureguest_icu_utf16_search_tests/guest_icu_utf16_search. Implementation, declaration and fixture bytes, all ten NIDs, and behavior are unchanged from publishedcc73317. The separate substring implementation can retainSearch.cpp/.hppand its test names. Shared source lists, umbrella includes and debt must retain both topics; #12320 adds no prerequisite for this PR.This standalone head retains frozen main
6e9b9134f6ed14e659d870a33118d164096551df, its merged ICU implementations/tests/debt and all 112 inherited throwing unknown exports. Its static merge with checked main3a40945e62b7e2543031d5da2b8e351ab603bc1ais clean; that check is not executed later-main integration. The actual predecessor #10966 remains required, with #11088 following #10967.Contracts follow pinned ICU 78.1 ustring.cpp and ustring.h; names/NIDs match the pinned PS5 3.20 inventory. Console verification, undefined-input and size differences remain in library debt.
Tested
2026-11-01 Windows CI: run 41295117065,
a7d9bb807d8b8361261dca649d01579c2d5d72b2+ main26fa1212b82f76dd49cf12d65e445d6ff3e0844c: 1,679 selected CTests passed, 904 skipped, zero failed. All 9 ICU cases passed, includingguest_icu_utf16_search. The production ICU module built, linked and was NID-patched. This is the fixed single-PR integration, not the six-topic Windows union. General full-suite, skip-audit and header CTests were not run; native ABI/context/unload, existing dependencies, CI and review gates remain.2026-11-01: preserving rebase
a7d9bb807d8b8361261dca649d01579c2d5d72b2onto fixed main26fa1212b82f76dd49cf12d65e445d6ff3e0844c. Linux GCC 16.2.1 Release: baseline 61 plain + 4 logging-enabled runtime CTests; private SIG-plus-six-ICU union4908848dab02fb1f0a9fad7e9525ddb8a628177a71 + 6. All passed, with 65 public-header tests per side. Relevant:guest_icu_utf16_search. Scope is this fixed baseline/private composition; bare heads, later main, Windows, native ABI and unloading remain unqualified. Earlier results retain their original scope.2026-10-30 fresh Linux integration check on fixed main
0362b1521c4ee08eeeccc2f98e9987e36fca9ea1: a separate private candidate8a5c8c499f0e0a157bd110430c1d99ee73bb0d47(tree620649fa8b0512d9c7e114c670342a691fd1433a) combines #10953 at581d45c282371af828efec2a8cec6f43beacc68awith the same six unchanged published ICU heads recorded in the earlier converter-callback integration update. No public branch was rebased by this check.Fresh native Linux x86-64 Release builds passed 53/53 selected plain runtime CTests on that main baseline and 63/63 on the private candidate. With
ANYPS5_LOG_FILEset before process startup, another 4/4 baseline and 6/6 candidate runtime executions passed. Together these are 126 runtime CTest executions; both lanes also passed 65/65 public-header checks, 130 header executions total, with zero failed, skipped or Not Run rows. The candidate ran all 13 registered ICU fixtures, including both guest-memory protection fixtures; converter callbacks ran in both lanes. The selected integration set also exercises logging, heap/allocator, signal, exception and thread behavior.Production NID patching and
readelf/nminspection found the baseline’s four converter-callback NIDs and all 39 targeted candidate NIDs; this is artifact evidence, not patched-loader functional coverage. Twelve separately observed controlledstd::abortchild runs returned the expected exit 3; they are not additional CTests or proof of native signal-context behavior. Source and dependency identities were checked before and after execution.These results apply only to the fixed baseline and private SIG-plus-six-ICU candidate. They add no bare-public-head, later-main, Windows, native-ABI, full-suite, GPU or unload qualification. Earlier results keep their original inputs and scope. The existing predecessor, preserving-rebase, maintainer CI and review requirements and agreed ICU landing order still apply.
2026-10-30 converter-callback integration update: #12692 actually merged as
91a69952c100(topic207ac4b5ecfe). Against pinned main08a10d225306ad102b57408e2ef7566bcfb829b2, all six published ICU heads remain independently clean to integrate; this source review found no duplicate implementation, new dependency or individually required rebase. A separate private all-six union,6e174798251592474dd94c61975655b409b9c2e9(tree446a6e346d3c6fac2e19e358d4376376778c7ae9), preserves 35 owned helpers, the four landed converter callbacks and 13 source-registered ICU fixtures through routine shared-file resolutions; the 98 unknown stubs remain unchanged.Exact published inputs: #10962
bbccec1140eeccb5adf61f7c497791b183c1bf93, #1096328cc96f3be1e16d2e3f098b0ae5b77b4fea85e09, #109644360e8f85b0bbd184199d9a3822dc30a8bd5f81e, #10965c2076fe30ee3442546af395930defd74b7bfac2a, #10966491e27444d83b53bc215a94edf132aee6a4707d7, #10967babf2bc72012bd926f35408b3d602d162a5ecd1b.Fresh native Linux x86-64 Release, GCC 16.2.1: the pinned08a main baseline passed 18/18 selected runtime cases and 65/65 public-header checks; the exact private all-six union passed 26/26 runtime cases and 65/65 headers, with zero failures, skips or Not Run rows. The union executed all 13 ICU fixtures, including
guest_icu_copy_guest_memoryandguest_icu_utf_memory, alongside 13 heap/allocator cases; the converter-callback fixture passed in both lanes. Production NID patching plusreadelf/nminspection verified the four callback NIDs in the baseline and all 39 targeted NIDs in the union (35 owned helpers plus four callbacks). This last check is artifact inspection, not patched-module loader runtime or native-ABI proof.These results cover only that fixed baseline and private all-six union, not any bare published head, later main, Windows/native ABI, all-nine integration, full suite, title or GPU behavior. Earlier main750 and other historical results retain their original inputs and scope. The declared order remains #10962, #10963, #10964, #10965, #10966, #10967, then #11088; the already merged #12692 adds no new prerequisite. Actual #10966 merge must come first, followed by a preserving rebase, fresh integration checks and re-review before this PR lands.
Exact candidate
babf2bc72012bd926f35408b3d602d162a5ecd1b, standalone base6e9b9134f6ed14e659d870a33118d164096551df:BUILD_TESTING=ON: built 197 objects and the real ICU/libc/kernel providers.guest_icu_utf16_search,guest_icu_versionandguest_icu_unescapepassed 3/3, plus 65/65 normal public-header CTests, with zero failures or skips. Renamed source compilation, actual fixture/provider artifacts, the recorded compiler-input manifest and critical actual inputs and unchanged source/dependency bindings were checked independently.10.0.26100.33650), verified WinLibs GCC 15.2.0 r7/UCRT, fresh normal-root Release,BUILD_TESTING=ON: run 40669914236, attempt 1 passed the candidate's 3/3 ICU runtime fixtures and 65/65 normal public-header CTests. The separate bare-main control passed 2/2 +65/65: 135 actual CTests total, zero failures, errors or skips. The original_WIN32aborting rejection checks remain active under Release; coverage of these subcases follows from the unchanged control flow, native macros and passing fixture rather than separate subcase telemetry. Both lanes compiled freshly without project-object/compiler-cache reuse; source/dependency identity, actual executable/provider payloads, native macros and raw test logs were verified.The Linux lane does not execute the Windows-only denied-write rejection cases. Current-head macOS, PS5, sanitizer, full-product/upstream CI and later-main/foreign-composition runtime are outside these results. Native provider resolution uses source/import/path evidence; no direct loaded-DLL trace is claimed. Required conventions against
origin/main, whitespace, source preservation and static merge checks passed.Earlier results remain historical at their named heads and bases, including the previous
cc73317Windows run. The review, applicable full maintainer CI, actual predecessor #10966 and agreed ICU landing order remain gates.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)