Skip to content

feat(libkernel_sys): implement profil argument checks - #11026

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Gruzzberry:feat/profil
Oct 27, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Gruzzberry:feat/profil

Conversation

@Gruzzberry

@Gruzzberry Gruzzberry commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What

profil was a throwing stub in libkernel_sys. FreeBSD 11.4's sys_profil fails with EINVAL when scale is above 1 << 16, stops profiling and returns 0 when scale is 0, and otherwise records the buffer and starts the profiling clock. It never reads the samples pointer at call time; the clock does that on each tick.

profil in libkernel_sys now returns EINVAL for a scale above 1 << 16 and 0 for a scale of 0 (profiling is never on, so there is nothing to stop). A non-zero scale still throws NotImplemented_nid_no_patch, because the profiling clock that fills the sample buffer is not modelled. The declaration in Api.hpp now carries the real four arguments.

Tested

Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release, -DBUILD_TESTING=ON.

  • guest_kernel_sys: scale of (1 << 16) + 1 and 0xFFFFFFFF give -1 with EINVAL; scale 0 returns 0 with a null buffer and with the unreadable address 0x10 (the pointer is not dereferenced). On main the test does not build against the stub, which takes no arguments; with this change it passes 3 runs out of 3. The non-zero scale throw is not asserted.
  • Full suite on this branch and on main: the same tests fail on both under load (pr_overlap_encoding, public_headers, agc_import_settle_initial_dcc, agc_host_import_cut_lifetime, agc_driver_depth_plane_partial_store, audio_out2_timing, guest_job_manager_failures, guest_pthread_start, guest_pthread_rwlock, guest_select, guest_mutex_contention, guest_named_shm_windows_short_temp); guest_ampr_threads fails intermittently on both (0 of 4 on main, 3 of 4 here). No build failures on either.
  • python tools/check_conventions.py --base upstream/main: passes.
  • Not tested: Linux, a console, a title run.

Overlaps

Checklist

AI-assisted (Claude Code).

🤖 Generated with Claude Code

@MarcZX10 MarcZX10 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.

Requesting changes for an unrelated implementation included in this profil PR. The diff also removes the swapon stub from Export.Discovery.cpp, adds a four-argument swapon declaration, implements path validation and host filesystem checks for swapon in Export.cpp, and adds GuestEnotblk. None of this is described in the PR's What/Test sections, has no tests here, and is a separate API topic. This conflicts with the repository's one-topic-per-PR rule.

Please remove the swapon changes from this branch; if that behavior is still wanted, submit it as a separate change after checking overlap and documenting/testing its contract. The profil portion itself follows FreeBSD 11.4's scale limit and scale-zero behavior (sys_profil): https://github.com/freebsd/freebsd-src/blob/release/11.4.0/sys/kern/subr_prof.c#L1988-L2025. The local and hosted product jobs are skipped, so I have not independently run the reported Windows test.

@MarcZX10 MarcZX10 mentioned this pull request Oct 25, 2026
12 tasks done
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 10421 core/libs/tests/GuestKernelSys.cpp L676

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/libkernel_sys/Export.cpp
PR 9097 core/libs/prx/libkernel_sys/Export.Discovery.cpp
core/libs/prx/libkernel_sys/Export.cpp
core/libs/prx/libkernel_sys/include/Api.hpp
core/libs/tests/GuestKernelSys.cpp
PR 9162 core/libs/prx/libkernel_sys/Export.Discovery.cpp
core/libs/prx/libkernel_sys/include/Api.hpp
PR 9242 core/libs/prx/libkernel_sys/Export.Discovery.cpp
core/libs/prx/libkernel_sys/include/Api.hpp
PR 9284 core/libs/prx/libkernel_sys/Export.Discovery.cpp
PR 9293 core/libs/prx/libkernel_sys/Export.Discovery.cpp
PR 9297 core/libs/prx/libkernel_sys/Export.Discovery.cpp
PR 9302 core/libs/prx/libkernel_sys/Export.Discovery.cpp
PR 9383 core/libs/prx/libkernel_sys/Export.Discovery.cpp
PR 9389 core/libs/prx/libkernel_sys/Export.Discovery.cpp
core/libs/prx/libkernel_sys/Export.cpp
PR 9713 core/libs/prx/libkernel_sys/Export.Discovery.cpp
core/libs/prx/libkernel_sys/Export.cpp
core/libs/prx/libkernel_sys/include/Api.hpp
core/libs/tests/GuestKernelSys.cpp
PR 10427 core/libs/prx/libkernel_sys/Export.cpp
core/libs/tests/GuestKernelSys.cpp
PR 10657 core/libs/prx/libkernel_sys/Export.Discovery.cpp
PR 11267 core/libs/prx/libkernel_sys/Export.Discovery.cpp
core/libs/prx/libkernel_sys/include/Api.hpp

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed prx labels Oct 25, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@Gruzzberry

Copy link
Copy Markdown
Contributor Author

Thanks, you're right: the swapon hunks were a sibling branch's commit that leaked into this one. The branch is now rebuilt on current main with only profil: the stub removal, the four-argument declaration, the function and the four test lines. GuestEnotblk and every swapon line are gone, and swapon stays in #11034. Focused guest_kernel_sys passes 3 of 3, check_conventions.py --base upstream/main passes, and the full suite matches the earlier run. I've also updated the overlap note.

@wachi35676

Copy link
Copy Markdown
Contributor

Tested head 69ead78b5e merged onto main 2b5225605b, Windows 11 (WinLibs GCC 15.2.0, Release) and WSL2 Ubuntu 24.04 (GCC 13.3), BUILD_TESTING=ON. This covers the Linux run listed as not tested.

  • guest_kernel_sys and guest_filesystem pass 3 runs out of 3 on both hosts. With the new body replaced by main's stub (same signature), guest_kernel_sys fails with what(): profil_nid_postfix not implemented.
  • The head now touches only profil. No swapon or GuestEnotblk lines are left.
  • It matches release/11.4.0 sys_profil: scale > (1 << 16) gives EINVAL, and scale == 0 only stops the clock and returns 0 without reading samples. A real profiling request throws here, which the conventions expect.

AI-assisted: yes (Claude Code); results above were run on this machine.

@adriadam10 adriadam10 self-assigned this Oct 27, 2026
@adriadam10
adriadam10 dismissed MarcZX10’s stale review October 27, 2026 00:52

Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.

@adriadam10
adriadam10 merged commit 1392606 into boykopovar:main Oct 27, 2026
14 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked against FreeBSD 11.4 sys_profil: scale > 1 << 16 gives EINVAL, scale == 0 returns 0 without reading the buffer, and a non-zero scale still throws since the profiling clock is not modelled. The guest_kernel_sys rows pin those cases and fail on main; the head only touches profil now, so the earlier swapon request is met. Confirmed by @wachi35676 on Windows 11 and WSL2; merged.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants