Skip to content

fix(libc): reject memset_s violations as titles' libc.prx does - #12594

Open
JJenkx wants to merge 3 commits into
boykopovar:mainfrom
JJenkx:windows/libc-memset-s-constraints
Open

JJenkx wants to merge 3 commits into
boykopovar:mainfrom
JJenkx:windows/libc-memset-s-constraints

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What

memset_s now accepts and rejects calls the way the libc.prx that titles ship does. Measured natively on PPSA07923 and PPSA05916, both of which agree:

  • Fills only when dest is not null, destsz <= SIZE_MAX >> 3, and count <= destsz. A 0/0 call on a valid dest succeeds.
  • Every other call returns ERANGE (34), writes nothing, and calls the constraint handler with "memset_s: bad destination argument", ERANGE and a null pointer. That includes a null dest with destsz = count = 0.

On main it returned EINVAL (22) without calling the handler, used SIZE_MAX >> 1 as the limit, and filled destsz bytes when count was larger.

The old checks in guest_strings asserted main's behaviour, which the titles contradict, so they move to guest_bounds_checking as CheckMemset, which installs a recording handler.

Fulfilled dependency: #9935 reached main through merged #10270, so the default constraint handler now writes and returns. orezraey’s request to keep this memset_s change separate from #10270’s copy functions is retained.

Fulfilled #10270 merge note (historical qualification): core/libs/tests/GuestStrings.cpp conflicts textually, because #10270's hunk at line 130 sits next to the removed memset_s lines. The resolution is #10270's merged file with those seven lines removed (six Require(memset_s_nid_postfix(...)) lines and the sentinel declaration). Merged that way on main 319621ba2e with #10270 0bc7597aed:

  • guest_bounds_checking, guest_strings, guest_safe_errors, guest_crt and guest_wide_strings pass 3/3;
  • all nine functions of the native comparison below match the titles.

Tested

Current-main integration — 2026-11-04. Head a2f6e23c8731896479fee84eccfa75396d6050c8 includes main b11c7bf84f5d2f636ff60166cc3371bd071e24d5 and preserves previous public commits and attribution/licence notices. Retains the measured memset_s violation behavior and write-preservation fixtures while preserving main’s updated copy-function size bounds and returning default handler.

Linux x86-64, GCC 16.2.1, Ninja Release, BUILD_TESTING=ON: rebuilt actual libc/Ampr and selected executables. guest_bounds_checking, guest_strings, guest_safe_errors, guest_crt each pass 3/3 (12 runtime passes, zero skips); public_headers, conventions and whitespace checks pass. Pinned main passes all 17 selected baseline groups 3/3 (51 passes) plus headers. Before/after dependency checks agree: 34 recursive trees and 31,551 tracked blobs, zero errors; the update=none QuickJS test262 corpus is explicitly excluded from these library inputs.

Source/parent/gitlink proofs, library/test hashes, raw logs/JUnit and head-leased push receipts: qualification archive, SHA-256 7c347f620951ad6f39068740750949212c6412815da094a46a96eca1d15c8bbf (download verified). Older platform, stress, negative-control and reference reports retain their original pins; no new Windows/macOS, full-suite, SDK-module, console, title/GPU or performance claim. AI-assisted: yes (Codex).

Earlier dated qualifications below retain their original source/platform pins.

Windows 11, WinLibs GCC 15.2.0 UCRT r7, Release, main 319621ba2e.

  • Native comparison: a scratch loader calls the titles' memset_s by NID next to this build's, with the same recording handler on both sides. It makes 20,000 random calls:

    • destination sizes including 0, SIZE_MAX >> 1 and one above it;
    • counts below and above the size;
    • null destinations.

    Each call compares the return value, whether the handler ran and with which error, and the destination bytes.

    • main: 11,242 of 20,000 differ, on both titles.
    • This PR: 0 of 20,000, on both titles.
    • The rules above come from 14 chosen cases run on the titles (each limit, each null combination, SIZE_MAX), which also gave the message and the null pointer. CheckMemset checks 10 of those cases against this build.
  • Regression: with the previous Strings.cpp, the new CheckMemset fails (memset_s accepted a violation).

  • Test set: guest_bounds_checking, guest_strings, guest_safe_errors, guest_crt and guest_wide_strings pass 3/3. Conventions pass.

Independent Windows evidence from bunbeans2-arch: Windows 11 Pro, MSYS2 UCRT64 GCC 16.1.0, Release; head 37d0c191b5a04732daaca697ca5678ac9c670aaf integrated with main 98ddd9c179. guest_bounds_checking and guest_strings: change passed 5/5; reverting only production changes while retaining the regression tests failed 5/5 at memset_s accepted a violation; plain main passed 3/3. This is the reporter's execution, not a new local Windows run.

Checklist

🤖 Generated with Claude Code

The libc.prx that titles ship (PPSA07923, PPSA05916) fills only when
dest is not null, destsz is at most SIZE_MAX >> 3 and count is at most
destsz. Any other call returns ERANGE, writes nothing and calls the
constraint handler with "memset_s: bad destination argument", ERANGE
and a null pointer. main returned EINVAL without the handler, used
SIZE_MAX >> 1, and filled destsz bytes when count was larger.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 28, 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 5021 core/libs/prx/libc/src/Strings.cpp
docs/dev/debt/libc.md
PR 5303 docs/dev/debt/libc.md
PR 6819 docs/dev/debt/libc.md
PR 7081 docs/dev/debt/libc.md
PR 7386 docs/dev/debt/libc.md
PR 7447 docs/dev/debt/libc.md
PR 7715 docs/dev/debt/libc.md
PR 7851 docs/dev/debt/libc.md
PR 8150 docs/dev/debt/libc.md
PR 8965 core/libs/prx/libc/src/Strings.cpp
core/libs/tests/GuestStrings.cpp
docs/dev/debt/libc.md
PR 9340 docs/dev/debt/libc.md
PR 9371 docs/dev/debt/libc.md
PR 9447 docs/dev/debt/libc.md
PR 9526 docs/dev/debt/libc.md
PR 10397 docs/dev/debt/libc.md
PR 10405 docs/dev/debt/libc.md
PR 10780 docs/dev/debt/libc.md
PR 11172 docs/dev/debt/libc.md
PR 11260 docs/dev/debt/libc.md
PR 11262 docs/dev/debt/libc.md
PR 11266 docs/dev/debt/libc.md
PR 11332 docs/dev/debt/libc.md
PR 11661 docs/dev/debt/libc.md
PR 11689 docs/dev/debt/libc.md
PR 11698 docs/dev/debt/libc.md
PR 11724 docs/dev/debt/libc.md
PR 11726 docs/dev/debt/libc.md
PR 12682 docs/dev/debt/libc.md
PR 12688 docs/dev/debt/libc.md
PR 12700 docs/dev/debt/libc.md
PR 12849 docs/dev/debt/libc.md
PR 12965 docs/dev/debt/libc.md
PR 13449 docs/dev/debt/libc.md
PR 13486 core/libs/prx/libc/src/Strings.cpp
docs/dev/debt/libc.md
PR 13794 docs/dev/debt/libc.md
PR 13804 docs/dev/debt/libc.md
PR 13809 docs/dev/debt/libc.md
PR 14212 docs/dev/debt/libc.md

The debt notes now state the titles' memset_s rule and that a
violation writes nothing where C11 K.3.7.4.1 fills min(count, destsz)
bytes. Suggested by oneandonlydean in review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@JJenkx

JJenkx commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@oneandonlydean following your note on #12593: 37d0c191b5 (on cc3a5b1f81) adds a memset_s debt entry with the measured rule from the libc.prx of PPSA07923 and PPSA05916:

  • a violation writes nothing, where C11 K.3.7.4.1 fills min(count, destsz) bytes of a valid destination;
  • the system libSceLibcInternal is not measured for this.

It's a docs-only change. Conventions pass, and the debt file still merges cleanly with #10270 (the only conflict remains GuestStrings.cpp, as described in the body).

AI-assisted: yes (Claude Code).

@rooklz rooklz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this on macOS, which the body doesn't list (head 37d0c191b5, based on main 319621ba2e).

Standalone ASan+UBSan build (macOS 26, Apple clang, -arch x86_64): the PR's Strings.cpp and WideConversion.cpp (for set_constraint_handler_s), linked to CheckMemset and the recording handler taken verbatim from GuestBoundsChecking.cpp. I added three abort-only symbols for unrelated Strings.cpp imports; none of them is called.

  • head: passes, with no sanitizer reports.
  • main's Strings.cpp with the new test: fails with memset_s accepted a violation, as the body says.

Mutants of memset_s_nid_postfix: the test kills all 16.

  • Limit changed to >> 2, to >> 4, or applied to count instead of destsz.
  • count > destsz changed to >=.
  • Each of the three guards dropped one at a time. Dropping the null check is caught as an ASan SEGV.
  • Handler not called, called twice, given dest as its pointer, given error 22, or given the message bad argument.
  • Returning 22.
  • Filling destsz bytes, or count - 1 bytes.
  • Zeroing destsz bytes on a violation.

Other checks:

  • No internal caller of memset_s_nid_postfix exists outside the export, so the stricter checks can't change another libc path.
  • After the removal, GuestStrings.cpp and GuestBoundsChecking.cpp compile without new warnings (-Wall -Wextra).
  • check_conventions passes. git merge-tree against main is clean.
  • pr_overlap lists only the shared docs/dev/debt/libc.md, plus #5021 and #11892 in Strings.cpp. #11892 merges cleanly with this PR. #5021 already conflicts with main itself.

LGTM once #9935 lands, as stated.

@bunbeans2-arch

Copy link
Copy Markdown
Contributor

A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release, this PR (37d0c191b5) merged into main 98ddd9c.

  • With this PR: guest_bounds_checking and guest_strings passed 5 of 5.
  • Same tree with only Strings.cpp put back to main (this PR's tests kept): guest_bounds_checking aborted at the new check memset_s accepted a violation, in 5 of 5 runs.
  • main 98ddd9c alone (its own tests and source): the same tests passed 3 of 3.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 4, 2026
@JJenkx

JJenkx commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Current-main integration — 2026-11-04. Head a2f6e23c8731896479fee84eccfa75396d6050c8 includes main b11c7bf84f5d2f636ff60166cc3371bd071e24d5 and preserves previous public commits and attribution/licence notices. Retains the measured memset_s violation behavior and write-preservation fixtures while preserving main’s updated copy-function size bounds and returning default handler.

Linux x86-64, GCC 16.2.1, Ninja Release, BUILD_TESTING=ON: rebuilt actual libc/Ampr and selected executables. guest_bounds_checking, guest_strings, guest_safe_errors, guest_crt each pass 3/3 (12 runtime passes, zero skips); public_headers, conventions and whitespace checks pass. Pinned main passes all 17 selected baseline groups 3/3 (51 passes) plus headers. Before/after dependency checks agree: 34 recursive trees and 31,551 tracked blobs, zero errors; the update=none QuickJS test262 corpus is explicitly excluded from these library inputs.

Source/parent/gitlink proofs, library/test hashes, raw logs/JUnit and head-leased push receipts: qualification archive, SHA-256 7c347f620951ad6f39068740750949212c6412815da094a46a96eca1d15c8bbf (download verified). Older platform, stress, negative-control and reference reports retain their original pins; no new Windows/macOS, full-suite, SDK-module, console, title/GPU or performance claim. AI-assisted: yes (Codex).

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 4, 2026
@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ AMD GPU test: passed

PR head a2f6e23 merged onto main b611e63 · tested 2026-11-05 01:52 UTC · 24 min

GPU AMD Radeon RX 6300 (RADV NAVI24), driver Mesa 25.2.8-0ubuntu0.24.04.4, Vulkan 1.4.318
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-42-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 3,817 passed · 19 failed (0 new, 14 also fail on main, 5 unstable on main) · 61 skipped (ANYPS5_REQUIRE_VULKAN=1)
Test setup CPU-only tests without a GPU; GPU tests (and any CPU-only test that failed or skipped without one) once on the GPU with VK_LAYER_KHRONOS_validation and ANYPS5_NO_SHADER_CACHE=1; main ran the same way
Test selection 3,897 of 3,946 tests run: the ones this change can reach, chosen by tools/select_tests.py from main's build graph
Vulkan validation ✅ no new validation IDs vs main (1390 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.7 s, ⚠️ agc_driver_graphics 120.3 s, ⚠️ agc_driver_graphics_maps_fallback 120.2 s, ⚠️ public_headers 67.9 s, ⚠️ agc_driver_bda_device 57.4 s
Unstable on main (5, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_array_layer_target_cpu: failed in 3 of the last 10 main runs
agc_driver_graphics: failed in 3 of the last 10 main runs
agc_driver_graphics_maps_fallback: failed in 2 of the last 10 main runs
agc_driver_pipeline_journal: failed in 9 of the last 10 main runs
agc_driver_two_sample_alpha_to_coverage_sample_mask_export: failed in 4 of the last 10 main runs
Failing on main too (14, not counted against this PR)
agc_driver_alpha_to_coverage_sample_mask_export
agc_driver_guest_memory_lifetime_direct
agc_driver_image_gather_volume
agc_driver_image_table
agc_driver_job_manager_compute_capacity
agc_driver_native_depth_bda_clear
agc_driver_native_depth_buffer_clear
agc_driver_native_depth_clears
agc_driver_native_multisample_depth
agc_driver_submit_progress
agc_submit_progress
agc_submit_progress_no_timeline
guest_mmap_executable
guest_socket_options_multicast
Passed on retry (2, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)
agc_driver_emulated_color_compare_srgb_decode: Timeout in the parallel run; also passed only on retry in 33 other runs in the last 7 days
agc_driver_recorder: Timeout in the parallel run; also passed only on retry in 24 other runs in the last 7 days
Validation IDs already present on main (4 IDs in 807 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (807 tests): agc_compute_pipeline_replay, agc_driver, agc_driver_address_draw_space_reuse, agc_driver_address_draw_space_reuse_held, agc_driver_address_draw_space_reuse_off, agc_driver_address_draw_space_reuse_transitions, agc_driver_address_draw_space_reuse_verify, agc_driver_address_passes, and 799 more
VUID-vkCmdCopyImage-srcImage-01548 (11 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_cpu_write_after_memo, agc_driver_depth_plane_cpu_write_after_page_unwatch, agc_driver_depth_plane_cpu_write_after_partial_unwatch, agc_driver_depth_plane_partial_store_image_copy, agc_driver_depth_plane_partial_store_unimported, agc_driver_depth_plane_partially_unmapped_plane, agc_driver_depth_plane_unmapped_plane, and 3 more
VUID-vkCmdCopyImage-srcImage-01551 (11 tests): agc_driver_depth_clear_hle, agc_driver_depth_plane_cpu_write_after_memo, agc_driver_depth_plane_cpu_write_after_page_unwatch, agc_driver_depth_plane_cpu_write_after_partial_unwatch, agc_driver_depth_plane_partial_store_image_copy, agc_driver_depth_plane_partial_store_unimported, agc_driver_depth_plane_partially_unmapped_plane, agc_driver_depth_plane_unmapped_plane, and 3 more
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (2 tests): agc_driver_prepared_exports, agc_driver_srgb8_color_target
Skipped (61)
agc_driver_bda_reads_shadowed_results: skipped, the device reports watched host memory written when it imports or reads it
agc_driver_bda_reads_shadowed_results_per_build: skipped, the device reports watched host memory written when it imports or reads it
agc_driver_copied_writer_label_order: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_copied_writer_label_order_drain: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_float16_unused_nan_payloads: skipped, the device has no native f16 conversions with RTE rounding and kept f16 denormals
agc_driver_gcn_display_present: skipped, no display: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
agc_driver_image_table_vouched_lookups: skipped, the device reports 4 of 4 host-imported pages written after a GPU read, so a watched imported table cannot be vouched for
agc_driver_linear_target_direct_write_completion: skipped, the device does not keep host imports write-watched, so no completion stamp over the target is tracked
agc_driver_ngg_vertex_wave_reduction: skipped, s3 describes a 32-lane wave only on 32-lane subgroups, and the device has 64 lanes
agc_driver_scattered_runs: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_sparse_dma_buf_import: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_driver_unit_shadow: skipped, the device does not import watched host memory for unit shadows
agc_driver_unit_shadow_budget: skipped, the device does not import watched host memory for unit shadows
agc_driver_vertex_elected_atomic_max: skipped, the device runs vertex programs in subgroups, so they keep their cross-lane reductions
agc_host_import_sparse: skipped, host imports are unavailable: /dev/udmabuf is not accessible
agc_idle_publish_wait: skipped, the device reports 4 of 4 host-imported pages written after a GPU read, so a forced watch of the import reads its submissions as CPU stores
agc_import_settle_initial_dcc: skipped, the device has no deferred settle of host imports: imports under 1 MiB wait for one only on Windows
agc_shadowed_key_stores: skipped, the device imports no watched host memory under a unit shadow
agc_small_tiled_shadow_label: skipped, host imports are unavailable: the test block is not imported and watched
agc_storage_clear_hand_over: skipped, the device imports no watched host memory for the clear-key hand-over
agc_unit_shadow: skipped, the device does not import watched host memory for unit shadows
agc_unit_shadow_completion_store: skipped, the device reports watched host memory written when it imports or reads it
guest_http_uri_locale: 
guest_net_ctl: skipped, the host has no IPv4 address
guest_video_out_color_space: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_hdr: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_high_frame_rate: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_output_mode: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_resolution_status: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
guest_video_out_wqhd_detection: skipped, no display or Vulkan device: SDL_InitSubSystem(VIDEO | GAMECONTROLLER) failed: wayland,x11 not available
video_out_flip_aftergpu: skipped, no display or Vulkan device: DisplayWindow: Vulkan support is either not configured in 
Reproduce
git fetch origin a2f6e23c8731896479fee84eccfa75396d6050c8 b611e634f7fc024f0c13e342d96de744448ef1d9
git checkout b611e634f7fc024f0c13e342d96de744448ef1d9 && git merge --no-ff a2f6e23c8731896479fee84eccfa75396d6050c8
git submodule update --init --recursive
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_ENABLE_SPIRV_TOOLS=ON
cmake --build build && cmake --build build --target libs
VK_DRIVER_FILES=/nonexistent/icd.json ctest --test-dir build -E '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 12 --output-on-failure
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \
  ctest --test-dir build -R '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 16 --output-on-failure
# failures are then re-run alone: --timeout 360 --parallel 2; new validation IDs: the test alone, twice, --parallel 1

Automated 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.

This branch has not been deployed

No deployments
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.

5 participants