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/8.0-staging] enforce FIELD_OFFSET_LAST_REAL_OFFSET size limit for InlineArrays #97533

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 26, 2024

Backport of #97403 to release/8.0-staging

/cc @VSadov

Customer Impact

  • Customer reported
  • Found internally

[Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.]

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

[How was the fix verified? How was the issue missed previously? What tests were added?]

Risk

[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.]

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@jkotas
Copy link
Member

jkotas commented Jan 26, 2024

I do not think that this meets servicing bar.

@VSadov
Copy link
Member

VSadov commented Jan 26, 2024

I thought we wanted to backport to minimize impact from fixing the limit enforcement in 9.0. Perhaps that reason alone is not enough for backporting as the issue by itself is not strictly blocking/breaking anybody.

Also since, in addition to fixing the limit enforcement, we are also increasing the limit to a fairly large value, I think the impact of fixing the enforcement will be fairly small.

Right. The backporting was on my TODO list and I was going through the list and started backport mechanically.
However it does not look like a candidate for backporting.

@VSadov VSadov closed this Jan 26, 2024
@MichalStrehovsky
Copy link
Member

If we wanted to backport something, it should be a fix that stops enforcing a 1 MB limit for Sequential structs on Native AOT. That one is a bug that meets servicing bar if we count me doing C# stuff in my spare time as "customer". It results in a broken app after publish. I wish we fixed that one first so that we can do an easy backport later if a more important customer hits it.

I agree the part of this fix that breaks scenarios that worked with CoreCLR would not pass shiproom. The code only kicks in when it actually breaks people. There's no risk in not having it (see #97412 that has been unnoticed for 20 years) but all risk in introducing it.

@MichalStrehovsky MichalStrehovsky deleted the backport/pr-97403-to-release/8.0-staging branch January 26, 2024 07:19
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants