Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0-staging] Fix hardware exception context extraction on Rosetta #107199

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 30, 2024

Backport of #107188 to release/8.0-staging

/cc @janvorli

Customer Impact

  • Customer reported
  • Found internally

The recently added AVX support in hardware exception handling path on macOS x64 that was also backported to .NET 8 has introduced a problem when running under Rosetta. Hardware exception handling always crashes when running as x64 under Rosetta emulation on arm64 macOS. The reason is that we try to set YMM registers and AVX instructions are not emulated by Rosetta.

Regression

  • Yes
  • No

Introduced in 8.0.8 by #104818

Testing

Directed test of a testing application where the issue got revealed and also running all coreclr tests under Rosetta.

Risk

Low, the change just removes CONTEXT_XSTATE flag from the captured context of the hardware exception location when the OS returns context without AVX.

The recently added AVX support in hardware exception handling path on macOS x64
has introduced a problem when running under Rosetta.
When we extract the floating point part of the context of the failing thread,
the thread can have AVX or AVX512 active, or none of these. The code accidentally
leaves CONTEXT_XSTATE set on the context even when no AVX was enabled on the thread.

Rosetta doesn't support AVX, so having CONTEXT_XSTATE set in the context flags
can lead to later call to RtlRestoreContext attempting to set ymm registers using
instructions that Rosetta cannot emulate and the app crashes due to that.

This doesn't happen in .NET 9, since we always clear the CONTEXT_XSTATE before
exception handling stack unwinding. But in .NET 8, this causes stack overflow
under Rosetta, since the attemt to execute the ymm instruction triggers the
hardware exception handling again and again.
@janvorli janvorli added the Servicing-consider Issue for next servicing release review label Aug 30, 2024
@janvorli janvorli added this to the 8.0.x milestone Aug 30, 2024
@janvorli janvorli self-assigned this Aug 30, 2024
@janvorli janvorli requested a review from jkotas August 30, 2024 22:26
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 8.0.x

@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.10 Sep 3, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 3, 2024
@carlossanlop
Copy link
Member

Friendly reminder that Code Complete for the October Release is September 9. If we want this fix to be included in that release, please merge this PR before that date.

@carlossanlop
Copy link
Member

Code complete is today. If this is ready, please merge it to include in the October Release.

@janvorli janvorli merged commit bcb0c09 into release/8.0-staging Sep 9, 2024
112 of 115 checks passed
@janvorli janvorli deleted the backport/pr-107188-to-release/8.0-staging branch September 9, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-PAL-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants