Skip to content

[release/6.0] Fix incorrect SIMD temp allocation for Vector256 with AVX2 disabled - #58850

Merged
jeffschwMSFT merged 2 commits into
release/6.0from
backport/pr-58820-to-release/6.0
Sep 9, 2021
Merged

[release/6.0] Fix incorrect SIMD temp allocation for Vector256 with AVX2 disabled#58850
jeffschwMSFT merged 2 commits into
release/6.0from
backport/pr-58820-to-release/6.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Sep 9, 2021

Copy link
Copy Markdown
Contributor

Backport of #58820 to release/6.0

/cc @BruceForstall

Customer Impact

Using the Vector256<T> GetElement API on a machine with AVX but without AVX2 could, in some instances, lead to a corrupt stack frame and data corruption.

Testing

Manual testing of the fix on the repro case, as well as SuperPMI asm diffs to determine other impact of the change, as well as standard PR CI testing.

Risk

Low.

The NI_Vector256_GetElement intrinsic, in some situations, requires
a stack temporary. With AVX2 disabled, this temporary was getting
allocated as a TYP_SIMD16 instead of a TYP_SIMD32, leading to overwriting
the local variable.
Add a type argument to the temp variable allocation, and allocate the
temp as the largest sized type required by any use.
Fixes#58295
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 9, 2021
@ghost

ghost commented Sep 9, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #58820 to release/6.0

/cc @BruceForstall

Customer Impact

Testing

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@BruceForstall

Copy link
Copy Markdown
Contributor

@tannergooding @dotnet/jit-contrib PTAL -- port of fix to release/6.0

@JulieLeeMSFT

Copy link
Copy Markdown
Member

CC @jeffschwMSFT for 6.0 backport.

@jeffschwMSFT

Copy link
Copy Markdown
Member

Please fill in the customer impact and request a code review.

@tannergoodingtannergooding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@BruceForstall

Copy link
Copy Markdown
Contributor

@jeffschwMSFT This is ready now

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved.

@jeffschwMSFT
jeffschwMSFT merged commit 614c782 into release/6.0Sep 9, 2021
@akoeplinger
akoeplinger deleted the backport/pr-58820-to-release/6.0 branch September 10, 2021 16:02
@ghostghost locked as resolved and limited conversation to collaborators Oct 10, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@BruceForstall@JulieLeeMSFT@jeffschwMSFT@tannergooding