Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Jun 23, 2020

Port of dotnet/runtime#38242

In a change to enable Mojave hardened runtime support that was made last
year, a bug was introduced into the JIT_CheckedWriteBarrier. A
conditional relative jump before an updated piece of code that was jumping
after that piece of code was not updated and ended up jumping into the
middle of an instruction. Since that condition occurs only with specific
memory layout and it is very rare, that problem was not discovered until
now.

Customer impact

The bug is causing consistent crashes on some configurations of OSX machines while it works fine on others. Unity was hit by this problem. It was observed on the new Apple silicon devices when running under Rosetta 2 emulator, but it can occur on any x64 macOS device.

Regression?

Yes, introduced in 3.1.0

Testing

The original fix was supplied and tested by Unity

Risk

Low, this fix fixes an obvious bug in assembler helper code where a conditional jump jumps into the middle of an instruction and the app crashes.

In a change to enable Mojave hardened runtime support that was made last
year, a bug was introduced into the JIT_CheckedWriteBarrier. A
conditional relative jump before an updated piece of code that was jumping
after that piece of code was not updated and ended up jumping into the
middle of an instruction. Since that condition occurs only with specific
memory layout and it is very rare, that problem was not discovered until
now.
@janvorli janvorli added area-VM Servicing-consider Issue for next servicing release review labels Jun 23, 2020
@janvorli janvorli added this to the 3.1.x milestone Jun 23, 2020
@janvorli janvorli requested a review from jkotas June 23, 2020 10:26
@janvorli janvorli self-assigned this Jun 23, 2020
#ifdef FEATURE_WRITEBARRIER_COPY
// jb NotInHeap
.byte 0x72, 0x12
#else
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious, why can this not use a mnemonic?

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason is that the clang assembler generates the long version of the relative jump instruction instead of one with byte offset for some reason. We've tried various things to make it generate this short version, but never found a way. So we had to resort to this ugly way.

@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jun 25, 2020
@leecow leecow modified the milestones: 3.1.x, 3.1.7 Jun 25, 2020
@Anipik Anipik merged commit 68ec8a2 into dotnet:release/3.1 Jul 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants