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/6.0] Fix exception propagation over HW exception frame on macOS arm64 #64262

Merged
merged 3 commits into from
Feb 7, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 25, 2022

Backport of #63596 to release/6.0

/cc @janvorli

There is a bug in hardware exception handling on macOS arm64 in setting up
the fake stack frame for the PAL_DispatchExceptionWrapper. Unwinding
while propagating an exception through the frame that caused the hardware
exception restores incorrect value of the LR register.

Customer Impact

Attempt to throw a new exception or rethrow from a catch handler of a NullReferenceException that stemmed from a virtual call on a null reference results in a SIGSEGV crash of the process on macOS arm64.

Testing

coreclr and libraries tests, targeted regression test.

Risk

Low. With the change, the unwind gets an exact copy of the frame of the hardware exception that was reported by the OS. So it is correct by definition.

There is a problem unwinding over the PAL_DispatchExceptionWrapper
to the actual hardware exception location. The unwinder is unable
to get distinct LR and PC in that frame and sets both of them to
the same value. This is caused by the fact that the
PAL_DispatchExceptionWrapper is just an injected fake frame and
there was no real call. Calls always return with LR and PC set
to the same value.

The fix unifies the hardware exception frame unwinding with Linux
where we had problems unwinding over signal handler trampoline, so
PAL_VirtualUnwind skips the trampoline and now also the
PAL_DispatchExceptionWrapper frame by copying the context of
the exception as the unwound context.
The context that is restored in the PAL_DispatchException needs to be
the one from the exception, not the original saved one. That ensures
that the registers updated by the GC in GC stress C in the context
are properly restored after the execution is resumed.
The PAL_SEHException had the records allocated on stack, so the
direct context restoration after the EH for GC stress C completed
leaked those.
@janvorli janvorli requested a review from jkotas January 25, 2022 10:27
@janvorli janvorli self-assigned this Jan 25, 2022
@janvorli janvorli added arch-arm64 os-mac-os-x macOS aka OSX Servicing-consider Issue for next servicing release review labels Jan 25, 2022
@janvorli janvorli added this to the 6.0.x milestone Jan 25, 2022
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.

Approved. We should take for consideration in 6.0.x

@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 25, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.3 Jan 25, 2022
@safern safern merged commit 2caf6fb into release/6.0 Feb 7, 2022
@safern safern deleted the backport/pr-63596-to-release/6.0 branch February 7, 2022 19:15
@ghost ghost locked as resolved and limited conversation to collaborators Mar 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm64 area-PAL-coreclr os-mac-os-x macOS aka OSX Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants