Skip to content

fix(libkernel): stabilize virtual-query layout across hosts - #5611

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
JJenkx:fix/kernel-virtual-query-layout
Oct 15, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
JJenkx:fix/kernel-virtual-query-layout

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

On MinGW, VirtualQueryInfo's 32-bit flag allocation unit puts name at byte 36 and gpu_mask_id at byte 68; SysV puts them at 33 and 65. Both structures are 72 bytes, so a size check misses the discrepancy. Use byte-sized flag units and assert size, alignment and trailing field offsets. A named-mapping regression checks fixed byte offsets, canaries and cleared padding.

This preserves the existing SysV contract. Astro's bundled AGC supports the 72-byte size and address/protection fields; console name and GPU-mask offsets remain unmeasured and are recorded in the library debt file.

Refreshed as one linear topic commit ad78fbcdf78d5a93f0ca3f9d79eff68adad4b38b on main 705b32d91986c02b2ba39ffcef3fff548a6dba68. The only merge conflict was additive debt text. Current main's complete debt file is retained, with this topic's entry appended. Source and test changes apply cleanly. No merge commits remain.

Tested

Current refresh, Linux x86-64, GCC, Ryzen 9 7950X:

  • Full libs, guest_memory_tests and patched_libraries_tests targets build successfully.
  • guest_memory and patched_libraries: 2/2 pass, no skips, 0.67 seconds. This runs the raw-byte layout and canary regression with the current memory implementation.
  • GCC and MinGW compile probes using the current complete SceTypes header pass all size/alignment/offset assertions. Linux probe executes successfully. Restoring only the original uint32 flag types makes MinGW fail with name offset 36 vs 33 and GPU-mask offset 68 vs 65. The MinGW probe is a compile check, not Windows execution.
  • Current conventions and whitespace checks pass.

The local runtime build uses a private SPIRV-Tools teardown repair b326807c0c9d8682b697fdad3153f77fc2642f5f; its only tree difference from the published topic is that gitlink, which is not part of this PR. The official dependency revision was not independently validated by this local build. Full Linux/Windows suite results for this refreshed head are pending maintainer CI.

Historical full CI, earlier topic/base only: topic cd28fc97e08b16e42d50f04a6d7ac4fcf3950dc1 against base 45d99dac82a9ad1ab8a7b1ff4ab8141e33cf920d passed Linux hosted validation (1105 passed /52 skipped) and Windows validation (757 passed /423 skipped). Skipped cases remain unvalidated. An earlier unchanged Windows guest_memory job needed a rerun after a resident-mapping assertion; the cause was not established. Those historical runs do not establish full-suite success for the new head.

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; silent stubs are listed in their library's technical debt file
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

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

The fix is right. uint32_t bitfields under MinGW's ms_struct layout take a 4-byte allocation unit, which moves name to 36 and gpu_mask_id to 68. Byte-sized units give the SysV offsets on both hosts. The offsetof asserts are the right guard, since the size alone hid this.

One convention point: the PR adds a code comment above the bitfields ("A byte-sized allocation unit also preserves the guest layout under MS bitfields."). CONVENTIONS.md only allows comments that mark technical debt, written by a human. The explanation is already in the debt entry, and the static_asserts document the constraint in code, so the comment can go.

@JJenkx

JJenkx commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Removed the redundant code comment in 2fd9f88023; the debt explanation and offsetof assertions remain. This head includes upstream main a5f493f90d. Current conventions pass. Fresh full Linux/Windows CI is pending; this is a comment-only correction to the layout change.

AI-assisted: yes.

This was referenced Oct 14, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 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 5993 docs/dev/debt/libkernel.md L199

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 3891 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4156 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4396 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4397 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4422 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4423 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4424 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4449 core/libs/tests/GuestMemory.cpp
PR 4520 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4521 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4522 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4523 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4566 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4599 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4718 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4740 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4749 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4760 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 4813 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 5021 core/libs/tests/GuestMemory.cpp
docs/dev/debt/libkernel.md
PR 5835 docs/dev/debt/libkernel.md
PR 5995 core/libs/SceTypes.hpp
PR 6044 core/libs/tests/GuestMemory.cpp
PR 6222 docs/dev/debt/libkernel.md

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on the merged tree: build and ctest for core/libs pass (32/32) and guest_memory passes. The byte-sized flag units keep the SysV offsets (name at 33, gpu_mask_id at 65), the static_asserts pin size, alignment and offsets so MinGW would fail to compile with the old layout, and CheckVirtualQueryWireLayout pins the raw bytes. The Windows run on the fork covers the MinGW side. Merging.

@github-actions github-actions Bot mentioned this pull request Oct 15, 2026
11 tasks
@JJenkx
JJenkx force-pushed the fix/kernel-virtual-query-layout branch from cd28fc9 to ad78fbc Compare October 15, 2026 12:18
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on the merged tree: build and ctest for core/libs pass (32/32) and guest_memory passes. The byte-sized flag units keep the SysV offsets (name at 33, gpu_mask_id at 65), the static_asserts pin size, alignment and offsets so MinGW would fail to compile with the old layout, and CheckVirtualQueryWireLayout pins the raw bytes. The Windows run on the fork covers the MinGW side. The only conflict with main was the adjacent debt line in docs/dev/debt/libkernel.md; it is resolved by keeping both. Merging.

@adriadam10
adriadam10 merged commit 51b1d4a into boykopovar:main Oct 15, 2026
7 of 8 checks passed
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.

3 participants