Fix SIGILL crash on ARM64 platforms with SME but no SVE #127398
Fix SIGILL crash on ARM64 platforms with SME but no SVE #127398AndyAyersMS wants to merge 3 commits intodotnet:mainfrom
Conversation
Replace CONTEXT_GetSveLengthFromOS() calls in signal context handling with direct reads of sve->vl from the kernel-provided signal frame. The CONTEXT_GetSveLengthFromOS function executes the SVE 'rdvl' instruction, which causes SIGILL on platforms that have SME (streaming SVE) but not standalone SVE — such as Apple M4 under macOS Virtualization.Framework with Podman/Colima. On these platforms, the Linux kernel includes an SVE_MAGIC record in signal frames (with vl=0 and minimal size) due to SME's streaming SVE mode, but the CPU does not support SVE instructions. When a signal fires (e.g. SIGUSR1 for activation injection), CONTEXTFromNativeContext sees the SVE record and calls rdvl, which triggers SIGILL. The SIGILL handler then tries to capture context again, hitting rdvl recursively. The fix uses sve->vl from the signal frame directly, which is always available when an SVE context record is present. On real SVE hardware, sve->vl equals what rdvl would return. On SME-only platforms, sve->vl is 0, so the SVE register save/restore is correctly skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@janvorli PTAL Pre-fix, |
There was a problem hiding this comment.
Pull request overview
This PR fixes an ARM64 SIGILL crash in CoreCLR PAL signal-context handling on platforms that provide an SVE signal-frame record but don’t support executing SVE instructions (e.g., SME-only environments). It does so by avoiding the SVE rdvl instruction and instead using the kernel-provided sve_context::vl value from the signal frame.
Changes:
- Replace
CONTEXT_GetSveLengthFromOS()(usesrdvl) with direct reads ofsve->vlwhen handling SVE state in signal contexts. - Add explanatory comments documenting the SME-without-SVE scenario and why
rdvlmust be avoided in this path.
| if (sve->vl == 16) | ||
| { | ||
| _ASSERT((lpContext->XStateFeaturesMask & XSTATE_MASK_ARM64_SVE) == XSTATE_MASK_ARM64_SVE); | ||
|
|
There was a problem hiding this comment.
In CONTEXTToNativeContext, the SVE block later derives vq from lpContext->Vl and relies only on an _ASSERTE to ensure lpContext->Vl == sve->vl. Since _ASSERTE is compiled out in non-debug builds, a mismatch in release could make the subsequent offset calculations write to the wrong locations in the signal frame. Consider computing vq from sve->vl (the actual frame layout) and using a non-assert runtime check for equality before writing, otherwise skip updating the SVE region.
| // (e.g. Apple M4 with SME streaming SVE under Virtualization.Framework). | ||
| if (sve->vl == 16) | ||
| { | ||
| _ASSERTE((sve->vl > 0) && (sve->vl % 16 == 0)); |
There was a problem hiding this comment.
Inside if (sve->vl == 16), the _ASSERTE((sve->vl > 0) && (sve->vl % 16 == 0)) becomes redundant (it can only ever be true). Consider removing it or changing it to assert the condition that actually matters here (e.g., that the context record size matches the expected minimum for VL=16).
| _ASSERTE((sve->vl > 0) && (sve->vl % 16 == 0)); | |
| _ASSERTE(sve->head.size >= SVE_SIG_CONTEXT_SIZE(sve_vq_from_vl(16))); |
| // (e.g. Apple M4 with SME streaming SVE under Virtualization.Framework). | ||
| if (sve->vl == 16) | ||
| { | ||
| _ASSERTE((sve->vl > 0) && (sve->vl % 16 == 0)); |
There was a problem hiding this comment.
A nit - this assert is now useless
There was a problem hiding this comment.
Do you want to replace this with the other assert suggested above?
- Remove the now-tautological _ASSERTE((sve->vl > 0) && (sve->vl % 16 == 0)) inside the 'if (sve->vl == 16)' block (janvorli). - In CONTEXTToNativeContext, derive vq from sve->vl (the signal frame's authoritative layout) instead of lpContext->Vl (copilot-reviewer). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
These are the only two usages, we can delete its definition: Also, the comment can be reworded to avoid mentions of |
|
Other places using rdvl: |
- Remove unused CONTEXT_GetSveLengthFromOS definition (context2.S) and declaration (context.h) since all callers now use sve->vl (am11). - Reword comments to not reference the deleted function (am11). - Replace redundant assert with meaningful size check (janvorli/AndyAyersMS): _ASSERTE(sve->head.size >= SVE_SIG_CONTEXT_SIZE(sve_vq_from_vl(16))) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JIT usage should be ok. Let me check the helpers. Looks like they're only invoked after we have verified SVE support. So they seem ok too. |
Replace CONTEXT_GetSveLengthFromOS() calls in signal context handling with direct reads of sve->vl from the kernel-provided signal frame. The CONTEXT_GetSveLengthFromOS function executes the SVE 'rdvl' instruction, which causes SIGILL on platforms that have SME (streaming SVE) but not standalone SVE — such as Apple M4 under macOS Virtualization.Framework with Podman/Colima.
On these platforms, the Linux kernel includes an SVE_MAGIC record in signal frames (with vl=0 and minimal size) due to SME's streaming SVE mode, but the CPU does not support SVE instructions. When a signal fires (e.g. SIGUSR1 for activation injection), CONTEXTFromNativeContext sees the SVE record and calls rdvl, which triggers SIGILL. The SIGILL handler then tries to capture context again, hitting rdvl recursively.
The fix uses sve->vl from the signal frame directly, which is always available when an SVE context record is present. On real SVE hardware, sve->vl equals what rdvl would return. On SME-only platforms, sve->vl is 0, so the SVE register save/restore is correctly skipped.
Fixes #122608