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/5.0] Backport AVX context fix #66120 #66356

Merged
merged 4 commits into from
Mar 15, 2022

Conversation

agocke
Copy link
Member

@agocke agocke commented Mar 8, 2022

Backport of #66120

Port of a fix for #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.

VSadov and others added 4 commits March 8, 2022 11:36
* Use CopyContext to restore saved context on X86

* PR feedback

* more PR feedback
…net#65825)

* Do not copy XState other than AVX

* #if defined(TARGET_X86) || defined(TARGET_AMD64)

* mask XState unconditionally

* Ensure XSTATE_MASK_AVX is set before calling EEGetThreadContext

* redundant supportsAVX, more clear comment

* PR feedback
* null-check the redirect context before using.

* tweak the comment

* do not allocate context if InitializeContext has unexpected results.
…net#65878)

* RestoreContextSimulated

* probe for RtlRestoreContext

* ntdll.dll

* restore self-trap sequence

* PR feedback

* Clarify CopyContext in RedirectedHandledJITCaseExceptionFilter

* simpler indentation.

* restore last error on the legacy path.

* Update src/coreclr/vm/threads.h

Co-authored-by: Dan Moseley <[email protected]>
agocke added a commit to dotnet/coreclr that referenced this pull request Mar 9, 2022
@carlossanlop
Copy link
Member

@agocke this change did not have the servicing-consider label so it did not get triaged in Tactics this week. Please add it and don't forget to send the email. The deadline for servicing fixes is Monday 03/14.

@agocke agocke added the Servicing-approved Approved for servicing release label Mar 14, 2022
@agocke
Copy link
Member Author

agocke commented Mar 14, 2022

This was approved for backport in the original schedule

@agocke
Copy link
Member Author

agocke commented Mar 14, 2022

Test failures look like #66406. I think we should merge that one ASAP and then this

@carlossanlop carlossanlop merged commit c7a22f1 into dotnet:release/5.0 Mar 15, 2022
@agocke agocke deleted the backport-66120 branch March 15, 2022 19:51
VSadov added a commit to dotnet/coreclr that referenced this pull request Mar 17, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 15, 2022
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