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

Conversation

agocke
Copy link
Member

@agocke agocke commented Mar 9, 2022

Port of dotnet/runtime#66356

Port of a fix for dotnet/runtime#65292 and issues found along the way related to AVX registers and EE suspension.

Customer Impact

It is relatively easy to end up running AVX-accelerated code nowdays. Copying or comparing arrays or spans, using buffered IO are just some examples.

Issues with storing/restoring AVX context during EE suspension could result in:

  • data corruption when running in x86 WOW on Windows11 and Server2022
  • crashes when performing GC stress (x64, Win10)

Testing

Regular test passes.

Risk

Medium.

The code involved is fairly old. We keep discovering assumptions that may no longer hold. We have seen cases where fixing one bug exposes another. To our knowledge these fixes are robust, but this is a delicate area.

@agocke agocke requested a review from VSadov March 9, 2022 19:51
@VSadov
Copy link
Member

VSadov commented Mar 9, 2022

Why so many whitespace changes? Is that just auto-format after pasting code or these are real old/new diffs?

@agocke
Copy link
Member Author

agocke commented Mar 9, 2022

Auto-whitespace changes, I think. If it's removing whitespace at the end of lines, VS code did it.

@agocke agocke changed the title Backport #66356 [release/3.1] Backport AVX context fix Mar 10, 2022
@VSadov
Copy link
Member

VSadov commented Mar 17, 2022

I will take a look at the failures

@VSadov
Copy link
Member

VSadov commented Mar 18, 2022

I have re-done porting manually.
3.1 is old enough that mechanical porting from 5.0+ breaks things.

@agocke agocke requested a review from jkotas March 18, 2022 17:50
@agocke
Copy link
Member Author

agocke commented Mar 31, 2022

@jkotas Could you take a look?

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM

@carlossanlop
Copy link

@agocke needs the servicing label.

@agocke agocke added the Servicing-consider Issue for next servicing release review label Apr 13, 2022
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 14, 2022
@leecow leecow added this to the 3.1.26 milestone Apr 14, 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 will take for consideration in 3.1.x

@carlossanlop carlossanlop merged commit 2a88fcf into release/3.1 May 3, 2022
@carlossanlop carlossanlop deleted the backport-66356 branch May 3, 2022 22:59
@VSadov
Copy link
Member

VSadov commented May 4, 2022

WooHoo!!! I can close dotnet/runtime#65292 now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants