Repository navigation
Conversation
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
cf361f9 to
8abc394
Compare
|
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.
One point in the argument-position description needs tightening. The code and PR text define what happens when a format mixes numbered assignments (%n$) with unnumbered assignments, and say that this matches glibc. POSIX says those forms cannot be mixed in one format (except %% and assignment-suppressed %* conversions); otherwise the result is undefined: https://pubs.opengroup.org/onlinepubs/000095399/functions/fscanf.html. The added tests use numbered assignments throughout (the unnumbered %*ls case is suppressed), so they do not establish the mixed behavior claimed in the description. Please describe mixed assignment formats as unverified/unsupported, or add a test and document the chosen extension without attributing it to glibc. This is not a correctness blocker for formats that use numbered arguments consistently.
b80b1a9 to
d838ff6
Compare
Thanks, good catch. I went with the stricter option: a format that mixes numbered and unnumbered assignments now throws |
|
The latest update addresses my earlier POSIX concern: mixed numbered and unnumbered assignments now throw, the description is explicit about the exception for One merge-order detail remains upstream in the dependency chain: #11724 now depends on #9087, #9340, and #10927. I found explicit agreement from #9087's author, but no confirmation from #9340's author yet. Since this PR depends on #11724, please make sure that order is agreed with all overlapping PR authors before merging. |
|
Linux run of head
This adds a second Linux toolchain (GCC 16 on Arch) to the Ubuntu GCC 13 run in the description. AI-assisted: yes (Claude Code) |
boykopovar#12530 reads source.Remaining() in the contiguous branch of ScanSource, so dropping the accessor here would break the build whichever of the two lands second, without a textual conflict. AI-assisted: yes
sscanf, vsscanf and the fscanf family threw when a format mixed n$ positions with a 16-bit wide conversion. The in-process scanner now reads the n$ prefix and takes the nth pointer. Formats that mix numbered and unnumbered assignments, which POSIX leaves undefined, and positional arguments in the _s forms still throw. AI-assisted: yes
d838ff6 to
8dadc38
Compare
…-fields # Conflicts: # docs/dev/debt/libc.md
…oa55/anyps5 into feat/libc-sscanf-positional-wide # Conflicts: # core/libs/prx/libc/src/Formatting.cpp # docs/dev/debt/libc.md
|
Re-ran on the rebased head
The control fails with the messages this PR removes:
AI-assisted: yes (Claude Code) |
# Conflicts: # docs/dev/debt/libc.md
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (13, not counted against this PR)Validation IDs already present on main (5 IDs in 711 tests)Skipped (57)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
…-fields # Conflicts: # docs/dev/debt/libc.md
… feat/libc-sscanf-positional-wide
✅ AMD GPU test: passedPR head
Unstable on main (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (9, not counted against this PR)Validation IDs already present on main (5 IDs in 711 tests)Skipped (58)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
…/libc-sscanf-positional-wide # Conflicts: # core/libs/prx/libc/include/ScanWideField.hpp # core/libs/prx/libc/src/Formatting.cpp # core/libs/prx/libc/src/ScanWideField.cpp # core/libs/tests/Formatting.cpp # core/libs/tests/GuestWideStreamScan.cpp # docs/dev/debt/libc.md
✅ AMD GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (12, not counted against this PR)Validation IDs already present on main (8 IDs in 794 tests)Skipped (61)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
On Linux, a scanf-family format that mixes
%n$positions with a 16-bit wide conversion (%ls,%lc,%l[,%S,%C) used to throwNotImplemented("sscanf positional arguments with a 16-bit wide conversion on Linux"). Only wide formats hit this: narrow-only formats go straight to glibc, which already handlesn$.The in-process scanner (
ScanSource) now reads positions itself:NumberedScanArgumentsmakes one pass over the format and returns the highestn$position, or 0 when no conversion is numbered.ScanSourcereads that many pointers up front, since ava_listcan only be read forwards.n$with the existingLibcDetail::ArgumentPosition(the printf side uses the same helper), then takes the nth pointer.scanf positional arguments mixed with unnumbered assignments). POSIX leaves that case undefined, so it is rejected rather than given a meaning.%%and assignment-suppressed%*conversions may still appear next to numbered ones, as POSIX allows.RequireNoPositionalWideScanandScansWideInProcessare removed. The callers now testHasGuestWideScandirectly._sforms still throw for positional arguments (scanf_s positional arguments), because their size arguments are interleaved with the pointers.The debt entry is updated. FreeBSD's
vfscanfhas no$support (it fails to match at the$), so what the console does here is unverified.This builds on the fscanf PR (#11724), so until that one merges this PR also shows its commit. Only the last commit belongs to this PR.
Overlap: #9340 and #9087 also edit the wide-field block in
ScanSource. Inside that block, this PR changes only the line that fetches the destination pointer (pointerAt(position)). #9340 also edits the scanf entry indocs/dev/debt/libc.md, and #10927 edits the lines around it, but the positional text here is a separate entry. #9340 also changes the wide-scan rejection tests incore/libs/tests/Formatting.cppnext to the%2$lscase removed here, and the#includelines ofScanWideField.cpp. All of these are textual conflicts only, and I'll rebase whichever PR merges second.Tested
Run by Claude (AI) on Linux (Ubuntu, GCC 13.3), Release build with
-DBUILD_TESTING=ON. ryanc2112 also ran it on Arch Linux (GCC 16.2.1), negative control included (see the comments):guest_formatting_tests: newsscanf/vsscanfcases for reversed positions, a numbered narrow%3$dalongside wide fields, a suppressed%*lswith a numbered%1$n, a short input, mixed numbered/unnumbered formats (%d %1$ls,%1$ls %n) throwing, andvsscanf_swithn$throwing. The old "%2$lsthrows" case is removed.guest_wide_stream_scan_tests:fscanfwith%3$d %1$ls %2$lc.srcchanges reverted,guest_formatting_testsaborts with the old "not implemented" error.guest_scan_count,guest_wide_formatting,guest_stdio_variadicpass.sscanf_s/vsscanf_sformats that containn$, which now throw. Before, the scanner passed the$to the host as if it were a conversion.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)