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/6.0] Disable poisoning for large structs #61601

Merged
merged 5 commits into from
Dec 15, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 15, 2021

Backport of #61589 to release/6.0

/cc @jakobbotsch

Customer Impact

Code that uses large (> 64 KB) stack allocated structs without the .zeroinit bit may hit InvalidProgramException in debug mode in .NET 6. This is particularly the case for large stack allocated arrays in C++/CLI when the program is compiled without optimizations. Reported by a customer in #60852.

Regression

Yes, this is a .NET 6 regression introduced with the poisoning feature in #54685. The poisoning is a debug aid that puts a recognizable value into uninitialized variables so that developers will more easily notice uninitialized uses of these.

Testing

There is a regression test included.

Risk

Low, this disables the poisoning feature in cases that would hit the problem.

For very large structs (> 64K in size) poisoning could end up generating
instructions requiring larger local var offsets than we can handle. This
hits IMPL_LIMIT that throws InvalidProgramException. Turn off poisoning
for larger structs that require more than 16 movs to also avoid the
significant code bloat by the singular movs.

This is a less risky version of #61521 for backporting to .NET 6.

Fix #60852
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 15, 2021
@ghost
Copy link

ghost commented Nov 15, 2021

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

Issue Details

Backport of #61589 to release/6.0

/cc @jakobbotsch

Customer Impact

Testing

Risk

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

area-CodeGen-coreclr

Milestone: -

src/coreclr/jit/codegencommon.cpp Outdated Show resolved Hide resolved
@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.x milestone Nov 15, 2021
Since it's disabled, there is nothing to check.
@jakobbotsch jakobbotsch reopened this Nov 23, 2021
@JulieLeeMSFT
Copy link
Member

CC @jeffschwMSFT please consider this for .NET6 servicing.

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Nov 30, 2021
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. Let's take for consideration in 6.0.x

@Pilchie Pilchie added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 2, 2021
@Pilchie Pilchie modified the milestones: 6.0.x, 6.0.2 Dec 2, 2021
@safern safern merged commit 923ec4d into release/6.0 Dec 15, 2021
@safern safern deleted the backport/pr-61589-to-release/6.0 branch December 15, 2021 18:27
@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants