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

Prevent placing ref fields on non-pointer boundaries #64520

Closed
MichalStrehovsky opened this issue Jan 31, 2022 · 3 comments · Fixed by #64643
Closed

Prevent placing ref fields on non-pointer boundaries #64520

MichalStrehovsky opened this issue Jan 31, 2022 · 3 comments · Fixed by #64643
Assignees
Milestone

Comments

@MichalStrehovsky
Copy link
Member

Similar to #12842 except for the following struct:

[StructLayout(LayoutKind.Explicit)]
ref struct RefStruct
{
    [FieldOffset(0)]
    public short X;
    [FieldOffset(2)]
    ref int Y;
}

We should not allow this to load.

@MichalStrehovsky MichalStrehovsky added this to the 7.0.0 milestone Jan 31, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 31, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Jan 31, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT self-assigned this Jan 31, 2022
@AaronRobinsonMSFT
Copy link
Member

I have the CoreCLR fix for this. The Mono changes are bit more complex and taking some time.

@MichalStrehovsky
Copy link
Member Author

I have the CoreCLR fix for this. The Mono changes are bit more complex and taking some time.

FWIW, Mono doesn't even detect #12842.

There's not even a bug for it because I don't think Mono ever ran a Pri-1 CoreCLR test pass (the regression test is Pri-1). If you make your regression test Pri-1, you can save yourself some work. It's not a Pri-0 scenario.

@AaronRobinsonMSFT
Copy link
Member

@MichalStrehovsky I've spent most of the day getting Mono in the right state. I am hitting issues now that are really confusing. I am close to doing what you suggest. Likely will make the call tomorrow.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 1, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants