Repository navigation
fix(libc): read wide scanf fields byte by byte, as titles' libc.prx does - #12468
Conversation
Progress report (ba62780 → dd88656)📈 System libraries: 26.25% (+0.01%, +3 implemented functions) ✅ 3 implemented
Changed libraries (implemented exports)
|
|
I syntax-compiled the exact-head |
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
I found one concrete regression in the exact head d17f1fbbae26 while tracing the new narrow-format wide conversion path. In ScanSource, IsWideScan includes uppercase %C, but the new branch immediately executes if (conversion == 'C') return finish(assigned);. That returns before reading input or consuming the destination argument. A format containing %C followed by another conversion therefore stops early and leaves the %C input unread; a standalone %C reports zero assignments. The previous scanner explicitly mapped %C to the wide-character (c) path, and the PR's updated debt text removes %C from the unsupported list. Please implement the %C conversion (including its correct field width and secure-scan behavior) or explicitly reject it consistently, and add a regression covering input consumption and the following directive. I did not independently run the focused test binary in this session; this finding follows directly from the current control flow and the prior implementation.
|
@MarcZX10 thanks for tracing it. The early return is intentional: it's what the titles'
So in the narrow scanners Pushed Tested on Windows 11, WinLibs GCC 15.2.0 UCRT r7, Release, this exact head: AI-assisted: yes (Claude Code). |
|
Linux run at this PR's exact head Arch Linux, kernel 7.2.9, Ryzen 9 7950X, g++ (GCC) 16.2.1 20260810, glibc 2.44, x86-64. Release,
|
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — approved
The %C concern from my earlier review is resolved. The title-library measurements you added show that narrow %C deliberately ends the scan without reading input or consuming an argument; the new cases cover the following conversion, %n, empty input, suppression, width/length modifiers, and vsscanf_s. Thanks for adding those exact-head regressions.
I also checked the follow-up Linux report: guest_formatting and guest_wide_stream_scan passed three runs each on the exact PR head. The PR checks report the Windows build, relinker on Ubuntu/Windows/macOS (including x86_64), ASan, register-context, write-watch, eligibility, conventions, labels, and report jobs passing; Linux hosted CI was skipped. The full library test matrix remains supported by the author-reported Windows/Linux test runs in the PR description and comments.
The PR is mergeable against current main, and the checklist records its dependency on #11724. I did not run local tests in this review.
|
Restored Linux Release: five focused formatting/scanning suites pass three repetitions on this public one-line fix (15 passes, zero skips). A private source tree exactly matching the merge of this fix with #12530 No new Windows/macOS, title/reference-library or GPU result is claimed. The external #11724 author coordination remains pending; current-main integration is described below. AI-assisted: yes. Current-main integration follow-up: For my own PRs, the landing order is #12507 → #12530 → #12468. The retained accessor also permits the two later #12530 calls to compile if branches are combined in the other order. This does not claim external #11724 author agreement. Evidence: immutable Linux qualification bundle. Its SHA-256 is |
|
Integrated upstream main Linux CPU: 15 passes (guest_formatting, guest_scan_count, guest_wide_stream_scan, guest_stdio_variadic, guest_crt); public_headers 1 passes; no skips. MinGW source syntax and conventions pass. The reviewed No new Windows/macOS runtime, native SDK, game, GPU or performance claim. The Windows user pause remains in force. Evidence: immutable source and qualification receipts, SHA-256 AI-assisted: yes. |
Re-checked — new head, dependency order pendingThe current-main integration is clean and the author reports focused checks passing, but the PR’s declared dependencies and landing order are still unresolved. What I ranChecked head git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr-12468
git diff --check 08a10d225306ad102b57408e2ef7566bcfb829b2...refs/remotes/upstream/pr-12468Results
Not verifiedI did not rerun the reported Linux tests or MinGW syntax check. To get this to a good stateThe PR lists #11724 and #12530 as dependencies and gives the landing order #12507 → #12530 → #12468; its description says coordination with #11724’s author is still pending. Please update the dependency/order status once confirmed. I’ll re-check this head and the same commands after that update. |
|
@MarcZX10 The external author agreement was already present but missing from our current description: cocoa55 explicitly confirmed keeping #11724 and #12468 separate, with this byte-by-byte follow-up stacked on #11724. I have corrected the stale “coordination pending” wording and linked that confirmation. The required predecessors remain #11724 and #12530, with the owned order #12507 → #12530 → #12468. This resolves the missing author-coordination statement; it does not claim those dependencies have landed. No source or branch head changed, and no new test result is claimed. AI-assisted: yes. |
|
@MarcZX10 Corrected the previously unmeasured stream-failure consumption in 96 direct/va-list ordinary/secure fixture cases fail the previous library and pass the change. Four focused CPU suites pass, zero skips; MinGW production/fixture syntax, conventions and whitespace pass. No new Windows runtime or title/FPS claim. The author-agreed dependency order from the prior reply is unchanged. Numeric/synthetic evidence, SHA-256 |
|
A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release,
AI-assisted: yes (Claude Code) |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — new head, looks good
I reviewed exact head 3a27a5b8159c4899f5a7cf9f097ae0b880eb1771; the author reports the focused Windows tests passing 5/5 on current main f7f03daf52f11191174b1be089a56e268390a707 with this diff, and both red controls failing as expected. Hosted Windows, conventions, eligibility, labels, report, Ubuntu relinker, relinker ASan, and Ubuntu register-context checks pass; Linux product tests are skipped, and the macOS jobs remain pending. I did not rerun the C++ tests locally or verify console behavior. No blocking issue remains in the reviewed change.
|
Both failed macOS write-watch jobs in run 41016170432 stop at CMake configure, before building or executing scanner/write-watch fixtures. The tested integration has two identical fault-mode registration loops; current main 51c31 reproduces the duplicate-name diagnostics, while this PR public 3a27a5 has only one loop. A separate main-only fix removes the redundant loop, preserving all 18 unique tests and both fault modes. Actual before/after configure/generate proof includes Darwin cross-configuration on Linux with APPLE=1. No macOS runtime pass or scanner branch fix is claimed. Fix PR: #13293. Evidence: immutable qualification receipts, SHA-256 AI-assisted: yes (Codex). |
fscanf, vfscanf, scanf, vscanf and the _s stream forms threw on %ls, %lc, %l[, %S and %C on Linux. Route those formats through the guest scanner sscanf already uses, reading the stream through StreamSource. The wide field reader now works on any source through Peek and Skip instead of a contiguous buffer: it decides whether to take a character from its lead byte, then consumes the whole UTF-8 sequence, so a rejected character stays in the stream without multi-byte pushback. AI-assisted: yes
Deciding from the lead byte only holds while scanset members are ASCII. Each source now decodes its next character with PeekWide without consuming it, and the field reader skips its bytes only once it takes the character. StreamSource peeks by reading the UTF-8 sequence and pushing it back with ungetc, as the fwscanf stream reader in ScanningWide.cpp already does. Keep TSource::Contiguous. AI-assisted: yes
Titles' libc.prx (PPSA07923, PPSA05916, PPSA13934) reads %ls, %lc, %l[ and %S by passing each byte to mbrtowc with the state kept for the field, so in the C locale any byte from 0x80 up ends the conversion: - a UTF-8 lead byte is held, and the next byte the field takes fails it with EILSEQ as a matching failure; - a field that ends while a byte is held is an input failure; - any other byte mbrtowc refuses is an input failure as the field's first byte and a matching failure after it; - the units read before a failure stay stored without a terminator. Widths count bytes, scanset members are compared as bytes, and %C is not a conversion there, so it ends the scan. The guest scanner decoded UTF-8 instead and threw for anything else, and Windows handed these conversions to the host, which stored each byte as one unit and threw for %S and %C. The scanner now does the above on every platform, and Windows sends formats with a wide conversion to it, as Linux does. Wide detection now follows the scanner's own rule, so %hS and %lS count. The wide tests run on Windows too and expect the titles' results; the UTF-8 cases they had are replaced by measured ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The narrow sscanf of titles' libc.prx (PPSA07923 and PPSA05916 agree) reads a %l[ set left to right: X-Y is a byte range when a - follows a member and is not last, a reversed range matches nothing, and a - first, last or after a range is a member. InWideScanset made a reversed range's first end a member and chained a following -, so %l[z-a] took z and %l[a-a-c] took b. It now follows the rule swscanf's InSet uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ater directives Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3a27a5b to
8a0ba60
Compare
|
Current-main qualification — 2026-10-30, repeat 3 Rebased to Fresh Linux Release libc and test builds pass Later main Immutable source, build and test receipts, SHA-256 The separate inherited macOS write-watch configuration failure is tracked by pending #13220. A maintainer closed #13293 as a duplicate of that earlier identical fix. This scanner refresh does not claim a macOS runtime pass. |
# Conflicts: # docs/dev/debt/libc.md
|
Linux run at exact head
CPU-only. No title, GPU or macOS run. |
|
Windows exact-head check of
🤖 Generated with Claude Code |
What
Titles' own
libc.prx(PPSA07923, PPSA05916 and PPSA13934 agree on every row) doesn't decode UTF-8 in the narrowscanffamily's 16-bit wide conversions. Each byte goes tombrtowc, whose state is kept for the field, and in the C localeMB_CUR_MAXis 1, so any byte from 0x80 up ends the conversion. The full measurements are on #11724 (comment). In short:mbrtowcreturns -2) is held.EILSEQ, as a matching failure.%ls, a byte outside the set for%l[, or end of input), it's an input failure.X-Yranges are bytes. A reversed range matches nothing, and a-first, last or right after a range is a member, as inswscanf(fix(libc): read swscanf scanset ranges and string lengths as titles do #12420) and Windows narrow%[(fix(libc): match bundled narrow scansets and scan directive parsing #12428).%Cisn't a conversion and ends the scan;%S,%hSand%lSread like%ls.Before this PR:
%Sand%C.The change, on top of #11724:
ScanSourceruns on every platform. Windowssscanf,vsscanf,fscanfandvfscanfsend formats with a wide conversion to it, as Linux does.mbrtowc(which matches the titles' on every byte value) with the state kept for the field, so the lead-byte and invalid-byte cases fall out of the same call.PeekWide, the UTF-8 decoder and its throws are gone.InWideScansetcompares bytes with the range rule above.IsWideScan, so%hSand%lScount too.Formatting.cppandGuestWideStreamScan.cppnow run on Windows as well. Their UTF-8 cases are replaced by measured title results.Unchanged and not measured:
sscanf_ssizes (that's #9087's), positional arguments next to a wide conversion (still thrown; #11726).Tested
Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release,
BUILD_TESTING=ON, on #11724's head6a95925815.Against titles' own
libc.prx: I loaded PPSA07923's, PPSA05916's and PPSA13934'slibc.prxnatively and called theirsscanfby NID (1Pk0qZQGeWo) side by side with this build. I compared the return value,errnoand every stored unit.318 hand-picked pairs (the feat(libc): read 16-bit wide fields in fscanf on Linux #11724 tables,
%S/%Cwith lengths and widths, and 16%l[range cases), excludingsscanf_ssizes: 0 differences.maindiffers on most of them and throws on%S/%C.200,000 random pairs each, built from
a b c, a space,5and the bytes 0x7F, 0x80, 0xA9, 0xBF, 0xC0, 0xC3, 0xE2, 0xEF, 0xF0 and 0xFF, over 17 wide formats:main(without%S/%C, which abort it)All three were run on the final head
d17f1fbbae.Tests:
guest_formatting,guest_wide_stream_scan,guest_stdio_variadic,guest_scan_countandguest_wide_formattingpass in each of 3 runs.Negative controls:
Formatting.cpp/ScanWideField.*,guest_formattingandguest_wide_stream_scanfail;InWideScanset,guest_formattingfails.Full CTest (2,612 tests, GPU drivers hidden; on
8c4c3f6376, before the range commit): the same failures asmain, plusguest_raise_exception, which fails some runs onmaintoo and passes 4 of 5 here.The initial results above are Windows measurements; later Linux qualification is recorded below. macOS runtime remains untested.
Overlap:
tools/pr_overlap.pyreports:sscanf_sconstraints;qlength;ScanSourcelines.I offered on #11724 to leave this to cocoa55 if they'd rather fold it in, and the offer stands. I'll rebase after #11724 and whichever of the others lands first.
Current Linux qualification — 2026-10-29
Restored
StringSource::Remaining()ina84100962ceb61a1646c04aa62bdcc254c8cec01, a one-line fast-forward ond9cf30a7a26ebd847a1aefb24d33037d3decd706. The existing prefix and range code from #12530 can now retain its two calls; no scanner behavior or feature is widened.Linux Release: five focused formatting/scanning suites pass three repetitions on this public one-line fix (15 passes, zero skips). A private source tree exactly matching the merge of this fix with #12530
3eca2be06731a86e6dc6a7cf07259dc1378eb149builds and passes the same15checks. Removing only the restored accessor from that combined production makes libc fail to compile at the Remaining calls. Restoring it passes all15checks again and reproduces the initial combined libc SHA256. Conventions against current main157b and whitespace pass. The private combined tree was not published; the public accessor fix was followed by the current-main integration described below.No new Windows/macOS, title/reference-library or GPU result is claimed. The #11724 author confirmed the separate stacked follow-up; current-main integration is described below. AI-assisted: yes.
Current-main integration follow-up:
068725777be07527eda51e086d8850500b7b8707includes main157b02c4f18, preserving the actual #11724 ancestry and restored accessor. The resolution retains the measured cross-platform wide routing and main’s quad-length guard before Linux string-scan dispatch, without restoring the obsolete wide-scan refusal. All five suites pass three times on this integrated source (15 further passes); both formatting translation units pass MinGW cross syntax checks. Conventions and whitespace pass. The private #12530 experiment remains unpublished.For my own PRs, the landing order is #12507 → #12530 → #12468. The retained accessor also permits the two later #12530 calls to compile if branches are combined in the other order. The #11724 author subsequently confirmed this separate stacked follow-up.
Evidence: immutable Linux qualification bundle. Its SHA-256 is
ca6b35926bcc6d3f21a9650517894c86b65594e2e2b653c5c32cd1c1de89c835. No fresh Windows runtime execution is claimed; older measurements retain their recorded source pins.AI-assisted: yes (Codex).
Current qualification — 2026-10-30
Integrated upstream main
08a10d225306ad102b57408e2ef7566bcfb829b2atabe5a9d8036e7174a7c7664cde8e4c2f5a5ac3c4, retaining the actual previous head and main as parents. Manual conflict resolution changes only debt documentation; production and test files equal the automatic merge output. Both parents' added test lines and main gitlinks are preserved.Linux CPU: 15 passes (guest_formatting, guest_scan_count, guest_wide_stream_scan, guest_stdio_variadic, guest_crt); public_headers 1 passes; no skips. MinGW source syntax and conventions pass.
The reviewed
StringSource::Remaining()accessor and actual parent history are retained. Sibling scanner ordering remains a separate integration limit.No new Windows/macOS runtime, native SDK, game, GPU or performance claim. The Windows user pause remains in force.
Evidence: immutable source and qualification receipts, SHA-256
61f5500e7526ea4e107159927de78c37db7e0ac1cb7ebd8625b22ab0ad5d9bd7(download verified).Linux stream-failure correction — 2026-10-30, head
3a27a5b8159c4899f5a7cf9f097ae0b880eb1771The previously unmeasured stream-position behavior is now measured against two bundled SDK libc modules (FAR and Tetris). For input
C3 A9,%lsleaves both bytes unread after the encoding failure; the previous head consumedC3. The correction restores the held lead byte and follows the SDK's signed-byte pushback behavior, includingFFbeing consumed as the EOF sentinel while any held lead byte is restored. Width/end-of-input stops keep their measured consumption. Return values, stored units and%nare preserved.z, across 14 formats: 10,752 ordinary stream rows and 10,752 secure stream rows. Both references agree on every field, including complete remaining input. Previous headabe5a9d8036e7174a7c7664cde8e4c2f5a5ac3c4differs in 3,056 rows per mode; the correction has zero differences.%n, logical stream position and all remaining bytes. The final fixture fails the previous library's stream-position assertion and passes the correction. The older test expecting onlyA9unread now checksC3 A9.guest_wide_stream_scan,guest_formatting,guest_scan_count,guest_stdio_variadic. MinGW production/fixture syntax, conventions and whitespace pass.CPU-only Linux qualification; no new Windows/macOS runtime, console hardware, GPU, title or FPS result. Source parent and dependency order are unchanged.
Synthetic probes, exact patch, numeric reference outputs and qualification logs, SHA-256
36a482b7b69ade13d5b9d091196c47338956e7f8000dba7c708c85e86d954f75, uploaded/downloaded and verified. No SDK binaries or game assets included. AI-assisted: yes.Independent Windows validation: bunbeans2-arch reports Windows 11 Pro 22631, i9-9900K, MSYS2 UCRT64 GCC16.1, Release, main
f7f03daf52plus this exact3a27a5b815diff. Bothguest_wide_stream_scanandguest_formattingpass5/5; keeping the tests while restoring main production makes both fail5/5; plain-main tests pass3/3. This is the contributor's reported run, not a Windows run performed in this Linux session. The exact head has a subsequent approved review with no blocking code issue.Current-main qualification — 2026-10-30, repeat 3
Rebased to
8a0ba602095aa5a280dc363263ae8f914e3b53beon tested maind2863131efb0e41c116e4197841b6a3086ce087a, using an exact lease from3a27a5b8159c4899f5a7cf9f097ae0b880eb1771. Seven remaining topic commits retain their authorship and order. The source union preserves main's measured q-directive cutoff before ordinarysscanf/vsscanfdispatch, together with this topic's bytewise 16-bit wide-field path on every host. Stream and secure-form routing is retained. The publishedScanWideField.cppis byte-identical, including range handling;Remainingand invalid-character pushback remain. Main's locking, EOF/read-error errno fixtures, portable temporary-file helper and adjacent debt notes are preserved.Fresh Linux Release libc and test builds pass
guest_formatting,guest_wide_formatting,guest_wide_stream_scan,guest_scan_count,guest_stdio_variadic,guest_crtandfile_streamsthree times each: 21 passes, pluspublic_headersonce, zero skips. Four MinGW production/fixture syntax checks pass. Conventions, whitespace, authored-series and submodule-pointer checks pass.Later main
6b48aee5d2ea9fce6155163d78f8ce8dc84f7827leaves every topic file unchanged and has a clean merge preview; it was not executed. Earlier Windows, title-library/reference and negative-control measurements keep their original pins. This refresh adds Linux execution and MinGW syntax qualification; it adds no new Windows/macOS runtime, GPU, title or performance claim. AI-assisted: yes (Codex).Immutable source, build and test receipts, SHA-256
784c7afd95828e9d3ec65614c80683f1ed09ce1874c0fa685b3f96c48b25dba0(download verified).The separate inherited macOS write-watch configuration failure is tracked by pending #13220. A maintainer closed #13293 as a duplicate of that earlier identical fix. This scanner refresh does not claim a macOS runtime pass.
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)🤖 Generated with Claude Code
Dependency coordination confirmed: cocoa55 explicitly agreed that #11724 retains the stream-wide conversion work and #12468 changes strings and streams to the measured byte-by-byte rule on top (author reply). Required predecessors remain #11724 and #12530; the owned-branch order remains #12507 → #12530 → #12468. These dependencies are not claimed merged.