-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] [NativeAOT] Missing memory fence before bulk move of objects #90941
Conversation
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsBackport of #90890 to release/8.0 /cc @VSadov Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. once you have a code review and green ci we can merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1b2acee
to
84ef917
Compare
It asked me to rebase the changes (after all tests have passed). Now it is running tests again. |
Yes, unfortunately it's a protection that is causing confusion to a lot of people. If a rebase is not needed (for example, your change does not depend on something already merged), then the button should not be clicked, otherwise it restarts the CI. You can just ask me to merge it for you, I can bypass the protection. |
Makes sense.
I did not know about that. :-) Thanks!! |
Backport of #90890 to release/8.0
/cc @VSadov
Customer Impact
Per our memory model making objects available to other threads requires that all modifications to the object made prior to publishing are observable by other threads. This is normally guaranteed either by the strong memory ordering (i.e. x64) or via fences in GC barriers (ARM64).
The helper that performs bulk copying of objects is one of the scenarios that needs to ensure the invariant above and thus needs to have a fence on weak memory architectures. The NativeAOT version of the helper is missing the fence.
(similar helper on CoreCLR does have the fence).
The customer impact of the missing fence could be an occasional race or a GC hole on ARM64 in multithreaded apps. The frequency of the problem could depend on how far the memory weakness is exploited and thus hardware-dependent.
It is unclear if we have seen this kind of issues already and how often, since attributing such races to the cause could be challenging.
The issue was found by reviewing the code.
Testing
Regular tests indicate no regressions.
Risk
Very low. The added fence matches the pattern used in CoreCLR for many releases.