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

enforce FIELD_OFFSET_LAST_REAL_OFFSET size limit for InlineArrays #97403

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Jan 23, 2024

Fixes: #95193

  • Fixes a faulty test that was passing for a wrong reason
  • Enforces the limit in a case of sequential layout
  • Increases the limit to FIELD_OFFSET_LAST_REAL_OFFSET

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

(I'm still learning about the CoreCLR type system) CoreCLR bits lgtm

@jkotas
Copy link
Member

jkotas commented Jan 23, 2024

Is this fixing the silent overflow in sequential layout too? #95193 (comment)

@VSadov
Copy link
Member Author

VSadov commented Jan 23, 2024

Is this fixing the silent overflow in sequential layout too?

No. We can fix that, but it should probably be a separate change.
If it is an older issue, we may not need to port that to 8.0

@VSadov
Copy link
Member Author

VSadov commented Jan 23, 2024

I've logged an issue to follow up with other silent overflows in sequential layout: #97412

@MichalStrehovsky MichalStrehovsky added needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. labels Jan 23, 2024
@ghost
Copy link

ghost commented Jan 23, 2024

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@VSadov
Copy link
Member Author

VSadov commented Jan 24, 2024

@lambdageek - while not strictly necessary, it may make sense to increase the limit on Mono as well.

I am not sure if Mono has some other limits for the size of a struct. If such limit is lower, it might make sense to have a different limit.

@VSadov
Copy link
Member Author

VSadov commented Jan 24, 2024

Thanks!!

@VSadov VSadov merged commit b47fdea into dotnet:main Jan 24, 2024
121 checks passed
@VSadov VSadov deleted the inlArrLim branch January 24, 2024 23:36
@VSadov
Copy link
Member Author

VSadov commented Jan 26, 2024

/backport to release/8.0-staging

Copy link
Contributor

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/7663297851

@VSadov VSadov removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Jan 26, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-TypeSystem-coreclr breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InlineArray size validation does not handle all cases
4 participants