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] Fix JIT_ByRefWriteBarrier unwinding on macOS x64 #91076

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 24, 2023

Backport of #90967 to release/8.0

/cc @janvorli

Customer Impact

On macOS x64, processing null reference can sometimes result in an access violation in the runtime native code. The problem is caused by the libunwind not being able to unwind from the JIT_ByRefWriteBarrier due to labels inside of this asm function not being marked as local. On macOS, that means that the libunwind would look up the unwind info for the closest label before the failing instruction and that means there would be none.

Testing

Running the coreclr failing test (it was disabled before) and CI testing

Risk

Low, the change just marks labels internal to the JIT_ByRefWriteBarrier (and other functions in the same file for consistency) as local. That essentially just renames them by adding a L prefix that the compiler / linker understands.

Local labels in the JIT_ByRefWriteBarrier were not wrapped in the
LOCAL_LABEL macro. That causes incorrect unwinding in case an
access violation occurs inside of this function. The closest label
before the failure point is considered to be the actual function,
but it obviously doesn't have the right unwind info.

Close #89585
@janvorli janvorli self-assigned this Aug 24, 2023
@janvorli janvorli requested a review from jkotas August 24, 2023 17:38
@janvorli janvorli added the Servicing-consider Issue for next servicing release review label Aug 24, 2023
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. once ready this can be merged

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 24, 2023
@jeffschwMSFT jeffschwMSFT added this to the 8.0.0 milestone Aug 24, 2023
@carlossanlop carlossanlop merged commit 06c76d6 into release/8.0 Aug 24, 2023
@carlossanlop carlossanlop deleted the backport/pr-90967-to-release/8.0 branch August 24, 2023 23:24
@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants