Skip to content

enforce FIELD_OFFSET_LAST_REAL_OFFSET size limit for InlineArrays#97403

Merged
VSadov merged 1 commit intodotnet:mainfrom
VSadov:inlArrLim
Jan 24, 2024
Merged

enforce FIELD_OFFSET_LAST_REAL_OFFSET size limit for InlineArrays#97403
VSadov merged 1 commit intodotnet:mainfrom
VSadov:inlArrLim

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 previous release. 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
@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

@github-actions
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 previous release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InlineArray size validation does not handle all cases

4 participants