Skip to content

feat(libSceIcu): implement UTF-8 macro helpers - #12693

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
Ninot1Quyi:feat/icu-utf8-helpers
Oct 30, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
Ninot1Quyi:feat/icu-utf8-helpers

Conversation

@Ninot1Quyi

Copy link
Copy Markdown
Contributor

What

Add four missing PS5 inventory exports: utf8_nextCharSafeBody, utf8_prevCharSafeBody, utf8_appendCharSafeBody, and utf8_back1SafeBody. These NIDs are absent from current main rather than existing unknown stubs.

Implement forward/reverse decoding, append and backward positioning with ICU's macro-helper semantics: strictness modes, surrogate/noncharacter rules, malformed-prefix byte consumption, NUL-terminated forward input, length-dependent legacy errors, and append flag preservation. In particular, the append body encodes ASCII as two bytes because the calling macro handles ordinary ASCII inline; the helper must preserve that upstream behavior.

Behavior follows pinned ICU 78.1 utf_impl.cpp and utf8.h. All four names/NIDs appear in the PS5 3.20 inventory. The implementation uses independent generic parsing/encoding logic; no ICU source is vendored or linked.

Add canonical public declarations, a runtime fixture and technical debt describing unverified console ABI/behavior and guarded undefined inputs, including signed index arithmetic overflow.

Overlap: the repository checker scanned all 1,410 open PRs, followed by manual inspection of all 16 ICU candidates. None implements these four functions/NIDs. #12630 needs mechanical integration of CMake registrations and adjacent debt entries. #11088/#11115/#11118/#11122/#11132/#11136 already conflict with main and could not be compared automatically; manual inspection found distinct implementations, with a shared header insertion point requiring both includes to be retained. No implementation dependency is required.

Tested

Ubuntu 24.04 x86_64, GCC 13.3, Release, BUILD_TESTING=ON; base 3a2b91981651569163768854a1fcdc7193b6667b.

  • Independent full project CMake configuration succeeded; built real libSceIcu, libc/libkernel dependencies, nid_patcher and ICU test targets. Focused CTest: 69/69 passed (four ICU runtime tests, public-header audit, 64 public-header compile shards). Final header ordering was followed by another build and 5/5 runtime/audit checks. The entire repository test suite was not run.
  • Registered tests cover all 256 strictness values on malformed/boundary cases, every non-ASCII Unicode scalar for independent encoding and forward/reverse round trips, append flags/capacities, legacy errors, NUL termination, canaries and invalid/overflowing inputs.
  • A temporary oracle built from the pinned official helper bodies/macros passed 440,365,400 differential comparisons, including all one/two-byte strings at all 256 strictness values, all three-byte strings at representative strictness modes, randomized four-byte cases, and append boundaries. Oracle files are not part of the patch.
  • New production source plus the registered fixture passed ASan+UBSan. LeakSanitizer was disabled because process-task enumeration is unavailable in this environment; address and undefined-behavior checks remained enabled.
  • Patched real libc, libkernel and libSceIcu outputs using the CMake-prescribed nid_patcher arguments. All four inventory NIDs are absent on main; calls through the new patched NIDs pass encoding/decoding, index, malformed-prefix, error and flag checks. dladdr confirmed the isolated patched PRXs were loaded.
  • python3 tools/check_conventions.py --base origin/main, git diff --check, and tools/pr_overlap.py --branch codex/icu-utf8 passed (overlap findings described above).

No game/homebrew, console hardware, Windows or macOS execution was performed.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One 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 file
  • No notes, investigation .md files 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

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Progress report (bb04a4f → d5b612c)

📈 System libraries: 46.51% (+0.02%, +4 implemented functions)

✅ 4 implemented
Library Name
libSceIcu utf8_appendCharSafeBody_nid_postfix
libSceIcu utf8_back1SafeBody_nid_postfix
libSceIcu utf8_nextCharSafeBody_nid_postfix
libSceIcu utf8_prevCharSafeBody_nid_postfix

Changed libraries (implemented exports)

Library Before After Change
libSceIcu 17/120 (14.17%) 21/124 (16.94%) +2.77 pp

@adriadam10 adriadam10 self-assigned this Oct 29, 2026
@rooklz

rooklz commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

I checked this on macOS, since the body lists only Ubuntu (the linux lane was skipped and only windows ran).

Setup. macOS 26 arm64, Apple clang, ASan+UBSan. I built Utf8.cpp @bb143b3f5c on its own; the only shim is APS5_VABI and APS5_INVALID_ARG_EX, which throws std::invalid_argument as in General.hpp. GuestIcuUtf8.cpp passes unchanged in 0.25 s with no sanitizer reports. check_conventions --head passes, and git merge-tree against main 3a2b919 is clean.

Comparison with a separately built ICU. I compared against Homebrew's ICU 78.3 libicuuc, linked as a binary. Its exported utf8_*SafeBody functions are what the PR says it follows.

  • next/prev: every 1- and 2-byte string; every 3-byte string with a 0xC0–0xFF lead (next) or two trail bytes at the end (prev); 3M random 4–5-byte strings biased toward trail bytes. Strict values tried: −128, −4, −3, −2, −1, 0, 1, 2, 127. I tried both the bounded length and −1 (NUL-terminated) where there is a NUL, and every start.
  • back1: every case above that ends in a trail byte.
  • append: every value from −2 to 0x110002, plus INT32_MIN and INT32_MAX. Index 0 and 3, capacity from index−2 to index+5, with and without an error pointer. I compared return value, flag and all 16 buffer bytes.

Result: 300,510,996 calls, 0 differences. To make sure the harness can see a difference, I changed 0xfdef to 0xfdee; that one change gave 33 differences.

Mutants (27). Each mutant was checked two ways: does the PR test catch it, and does the ICU comparison see a difference.

  • 21 are caught by the PR test (lead range, the e0/ed/f0/f4 first-trail bounds, strict 0 vs >0, the −2/−3 modes, prev count limit and completeness, back1 surrogate handling, every append width/capacity/error-length/flag path, NUL termination).
  • 2 are equivalent: 0 differences in 300M calls, and the code paths can't be reached any other way.
    • count == 2 && needed == 2 → count == 2: with two trail bytes only an E0–EF lead passes.
    • The early position == length return removed: the loop then consumes nothing, and ErrorValue(0, …) is returned anyway.
  • 2 get past the PR test, but the ICU comparison finds them:
    • lead == 0xe0 lower bound >= 0xa0 → >= 0x9f (16,058 differences). No row has E0 9F or E0 A0.
    • noncharacter (value & 0xffff) >= 0xfffe → >= 0xffff (42 differences). The only noncharacter rows are U+FDD0 and U+10FFFF, and U+10FFFF ends in FFFF.

These four rows in ErrorModes catch both. They still pass on head and the test still runs in under 0.3 s:

--- a/core/libs/tests/GuestIcuUtf8.cpp
+++ b/core/libs/tests/GuestIcuUtf8.cpp
@@ -82,6 +82,10 @@
                  strict == -2 ? 0xd800 : e0, strict == -2 ? 0 : 2, 2);
         Sequence({0xed, 0xa0}, strict, strict == -2 ? e1 : e0, strict == -2 ? 2 : 1, e0, 1, 1);
         Sequence({0x80, 0x80, 0x80, 0x80}, strict, e0, 1, e0, 3, 3);
+        Sequence({0xe0, 0xa0, 0x80}, strict, 0x800, 3, 0x800, 0, 0);
+        Sequence({0xe0, 0x9f, 0xbf}, strict, e0, 1, e0, 2, 2);
+        Sequence({0xef, 0xbf, 0xbe}, strict, strict > 0 ? e2 : 0xfffe, 3, strict > 0 ? e2 : 0xfffe, 0, 0);
+        Sequence({0xf0, 0x9f, 0xbf, 0xbe}, strict, strict > 0 ? e3 : 0x1fffe, 4, strict > 0 ? e3 : 0x1fffe, 0, 0);
     }
 }

I only compared host behaviour with upstream ICU. Like the debt entry says, nothing here is checked against the console.

AI-assisted: yes

Cover the E0 9F/A0 boundary and the U+FFFE/U+1FFFE strictness rules across all int8 strict modes and both tested origins.

Addresses boykopovar#12693 (comment)
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 10962 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 10963 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 10964 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 10965 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 10966 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 10967 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 11088 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 11115 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 11118 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 11122 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 11132 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 11136 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 12320 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md
PR 12738 core/libs/prx/libSceIcu/CMakeLists.txt
core/libs/prx/libSceIcu/libSceIcu.hpp
docs/dev/debt/libSceIcu.md

@JJenkx

JJenkx commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Windows run of this head (2f5265a8f7), merged onto main 52c09d567d (clean merge, local only). Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, BUILD_TESTING=ON, no Vulkan driver visible.

  • This PR: guest_icu_utf8 passes 3 of 3 (ctest --repeat until-fail:3).
  • Negative control: not possible. With this PR's source files (core/libs/prx/libSceIcu/Utf8.cpp, core/libs/prx/libSceIcu/Utf8.hpp, core/libs/prx/libSceIcu/libSceIcu.hpp) put back to main's version, and the ones it adds removed, the build fails against the rest of the PR, so there is no source-only control.

AI-assisted: yes (Claude Code).

@github-actions github-actions Bot added build docs prx needs rebase Conflicts with main, rebase needed labels Oct 29, 2026
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 29, 2026
@adriadam10
adriadam10 merged commit de2ba4c into boykopovar:main Oct 30, 2026
10 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Re-reviewed on head d5b612c after main moved libSceIcu/CMakeLists.txt and libSceIcu.hpp (ConverterCallbacks added next to these lines). Merged onto current main f7f03da cleanly: the new UCNV_* callbacks share no symbol with the utf8_* helpers, the anonymous-namespace helpers in Utf8.cpp don't clash with the new header, and the CMake registrations stay separate. Conventions, Windows and eligibility are green; Linux runs in the merge queue.

Checked the merge with current main; merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants