Skip to content

[release/7.0-preview2] Class types with layout that inherit from System.Object should be ManagedSequential #65578

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

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Feb 18, 2022

Backport of #65447 to release/7.0-preview2

/cc @jkoritzinsky

Layout-classes marked as LayoutKind.Sequential with a System.Object base class should use a sequential layout for managed code.

Customer Impact

Users who use crossgen2 to Ready-to-Run their code before execution may experience unexpected failures when using [StructLayout(LayoutKind.Sequential)] on class types when interacting with non-R2R code.

This is a regression reported in #65412

Testing

This PR includes tests to validate layout expectations.

Risk

Low risk.

IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@ghost
Copy link

ghost commented Feb 18, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@jkoritzinsky
Copy link
Member

cc: @mangod9 I don't remember who needs to approve for servicing preview releases. Can you approve or do we need @jeffschwMSFT approval?

@mangod9
Copy link
Member

mangod9 commented Feb 18, 2022

I dont have access to merge though

@jeffschwMSFT
Copy link
Member

@jkoritzinsky the window to make Preview 2 has basically passed. How impactful is this?

@jkoritzinsky
Copy link
Member

This can cause issues anywhere where LayoutKind.Sequential is used with ReadyToRun code, so not everywhere but at least somewhat common. I don't think this is release blocking though as the same bug was in Preview 1 and the fix is already in main.

@jeffschwMSFT
Copy link
Member

I think in this case, let' wait for main to flow into Preview 3.

@jkoritzinsky
Copy link
Member

Sounds good. I’ll close this then.

@jkoritzinsky jkoritzinsky deleted the backport/pr-65447-to-release/7.0-preview2 branch February 21, 2022 18:22
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants