Skip to content

build(libkernel): compile Exception on macOS - #11249

Closed
iamthehimansh wants to merge 1 commit into
boykopovar:mainfrom
iamthehimansh:split/macos-exception
Closed

iamthehimansh wants to merge 1 commit into
boykopovar:mainfrom
iamthehimansh:split/macos-exception

Conversation

@iamthehimansh

@iamthehimansh iamthehimansh commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What

Next part of splitting #8658: libkernel's System/src/Exception.cpp does not compile on macOS. The change is macOS-only; Linux and Windows compile the same code as before.

It converts the host signal context to and from the guest machine context through Linux's gregs array and fpregs pointer. On macOS the general registers are named fields of mcontext_t->__ss, and the x87/SSE state is an FXSAVE-layout block inside mcontext_t->__fs. macOS now copies those fields directly. A static_assert checks that the Darwin float state has the FXSAVE layout (MXCSR at 24, XMM0 at 160) that the copy relies on, so a header change fails the build instead of corrupting registers.

Overlap

@marion909's #7201 ("Macos port", 100 files, needs a rebase) also converts the Darwin machine context in this file, and 46 of this PR's 56 added lines appear there too. This PR carries only Exception.cpp, so it can land on its own. It differs in one place: it copies the full 512-byte FXSAVE image from __fs into the guest state, checks the Darwin layout with a static_assert at MXCSR, XMM0 and the end of XMM15, and writes back only the 416 register bytes. #7201 copies 416 bytes in both directions. If #7201 lands first, I'll close this or rebase it onto that.

Tested

macOS 26 on an Apple M2 Pro, x86_64 under Rosetta 2, Release with Ninja, on main at b90d009d9: System/src/Exception.cpp.o compiles, where main fails it. The libkernel tests cannot run on macOS until the rest of libkernel builds (#10552, #7804, #10898 and follow-ups), so this is a compile check only. In #8658's full macOS run (1,960 tests) the same code passed guest_exception_handler, guest_raise_exception, guest_signals, guest_signal_sets and guest_signal_list. For Linux and Windows, I checked the file's view under unifdef -U__APPLE__, with -D__linux__ and with -D_WIN32: it equals main. python3 tools/check_conventions.py --base origin/main passes.

Checklist

  • Based on current main; Macos port #7201 also implements this conversion (see Overlap)
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Cana)

System/src/Exception.cpp converts the host signal context to and from the
guest machine context through Linux's gregs array and fpregs pointer. On
macOS the general registers are named fields of mcontext_t->__ss and the
x87/SSE state is an FXSAVE-layout block in mcontext_t->__fs, so macOS now
copies those fields directly. A static_assert checks that the Darwin float
state really has the FXSAVE layout the copy relies on.

Linux and Windows compile the same code as before.

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

I reviewed the full macOS conversion. The general registers map field-for-field between Darwin __ss and GuestMcontext, and the FXSAVE offsets are checked at MXCSR, XMM0, and the end of XMM15 before copying the state image. The 512-byte host read fits the guest state assertion; the reverse path copies the same 416 bytes as the Linux implementation. The Apple branch is isolated from the existing Linux conversion.

git diff --check and current-main conventions pass. Hosted eligibility, conventions, intake, labels and report pass; product/build jobs are skipped by policy. I did not run the macOS runtime test locally. The author reports this same code passed the named guest exception/signal tests in the previous full macOS run (1,960 tests), and the changed translation unit compiles on the current branch. No concrete defect found.

@github-actions

github-actions Bot commented Oct 25, 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 4630 core/libs/prx/libkernel/System/src/Exception.cpp
PR 5021 core/libs/prx/libkernel/System/src/Exception.cpp
PR 7192 core/libs/prx/libkernel/System/src/Exception.cpp
PR 8592 core/libs/prx/libkernel/System/src/Exception.cpp

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

I am updating my earlier approval after checking the exact overlap against current open #7201. #7201 is still open and contains this same Exception.cpp context conversion; this PR itself says 46 of its 56 added lines also appear there. CONTRIBUTING requires the authors to agree which overlapping change lands first and record any prerequisite on Depends on. The current description says this PR can land independently, but does not show coordination with #7201’s author. Please record the agreed merge order and list a dependency if #7201 must land first, or coordinate that this focused PR is the replacement for this portion of #7201. The technical review of this head remains sound; this request is specifically for the documented overlap/merge-order rule.

@imellag

imellag commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

macOS test report. Apple M4 Pro, macOS 27.0.1, Apple clang 21, x86_64 Release under Rosetta 2, MoltenVK 1.4.2.

I looked for the smallest set of open PRs that boots titles on macOS from current main (035ac96edd). The set is #10366, #10552, #7631, #7804, #11249, #10669, #10540, #11447, #11047, #11048, #10876 and #9263, plus a local VulkanDevice.cpp patch that no open PR covers yet (it loads libvulkan.1.dylib and enables portability enumeration). With it, Planet of Lana (PPSA19450) and God of War Sons of Sparta (PPSA28997), relinked with that tree (--macos --to-rosetta), start from New Game: Lana reaches gameplay and runs the full 120 s, GoW reaches the Cyclops fight and runs the full 145 s, and both exit cleanly.

This PR is needed: with it removed from that set (head 6da054a8d8), System/src/Exception.cpp doesn't build:

MacOSX.sdk/usr/include/ucontext.h:51:2: error: The deprecated ucontext routines require _XOPEN_SOURCE to be defined

@iamthehimansh

Copy link
Copy Markdown
Contributor Author

@imellag thanks, that's a very useful set to have written down.

On the VulkanDevice.cpp patch: I have the same change (libvulkan.1.dylib plus portability enumeration) ready and tested on an M2 Pro. Without it, the Vulkan loader fails on main; with it, Selected GPU name=Apple M2 Pro, 3 of 3 runs, no VUID messages. I'm holding it rather than opening a fifth PR, because #7201 also carries that file. I asked @marion909 about merge order there. I'll open it as soon as he answers or one of my open PRs lands.

AI-assisted: yes (Cana)

@iamthehimansh

Copy link
Copy Markdown
Contributor Author

Follow-up to my earlier note about a separate macOS Vulkan loader PR: @maximilianfv opened #11554 with the same change, plus VK_KHR_portability_subset. I tested it on an M2 Pro: Vulkan failed to load 1/1 without it, and the device came up 3/3 with it, with 0 validation messages (results on #11554). So I won't open my own PR for this.

AI-assisted: yes (Cana)

@iamthehimansh

iamthehimansh commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

Correction: the "0 validation messages" in my comment above was wrong, because the validation layer wasn't loading in my runs. With it loaded, #11554 shows 1 validation error per run, from the image-table test's own descriptor layout. My loader-only version shows 2, so #11554's extra portability subset removes one error. Details on #11554.

AI-assisted: yes (Cana)

@iamthehimansh

Copy link
Copy Markdown
Contributor Author

Closing this in favour of @mugurc's #4630, which was opened earlier and makes the same Exception.cpp change. All 56 lines this PR adds to that file are identical to his, because #8658 merged #4630 in and this PR was split out of #8658. #4630 also adds guest_raise_exception and a debt entry. Thanks @MarcZX10 and @imellag for the reviews here; anything still needed for the macOS register conversion belongs on #4630.

AI-assisted: yes (Cana)

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.

4 participants