Skip to content

[release/8.0-staging] Fix problem in ReadyToRun_TypeGenericInfoMap::HasConstraints#105728

Closed
ivdiazsa wants to merge 1 commit intodotnet:release/8.0-stagingfrom
ivdiazsa:pr-96358-backport
Closed

[release/8.0-staging] Fix problem in ReadyToRun_TypeGenericInfoMap::HasConstraints#105728
ivdiazsa wants to merge 1 commit intodotnet:release/8.0-stagingfrom
ivdiazsa:pr-96358-backport

Conversation

@ivdiazsa
Copy link
Contributor

@ivdiazsa ivdiazsa commented Jul 30, 2024

Customer Impact

  • Customer reported
  • Found internally

Backport of PR #96358 to release/8.0. This bug came to light as reported by a customer in issue #96225. Since it reproduces on .NET 8 but was gone by .NET 9 preview 1, some further bisecting was done to figure out the fix.

The problem was that the code in the VM that is supposed to check for whether a generic type had (a) constraining clause(s), actually checked for whether said type was either covariant or contravariant. This completely different and therefore erroneous behavior broke some scenarios with generics and made them crash, like the one hindering our customer in this case.

Regression

  • Yes
  • No

This is a regression as the same scenario works properly in .NET 7 and .NET 6 versions. For context, it was accidentally introduced in this PR: #85743

Testing

This was validated using the customer's failing repro scenarios.

Risk

This PR poses little to no risk to the product. Since it's a small and simple one, we decided it would be a convenient and easy backport. The failure is well understood and has a well defined scope.

@ghost ghost added the area-VM-coreclr label Jul 30, 2024
@ivdiazsa ivdiazsa added this to the 8.0.x milestone Jul 30, 2024
@ivdiazsa ivdiazsa linked an issue Jul 30, 2024 that may be closed by this pull request
@ivdiazsa ivdiazsa added the Servicing-consider Issue for next servicing release review label Jul 30, 2024
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 8.0.x

@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.9 Aug 1, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 1, 2024
@ivdiazsa ivdiazsa closed this by deleting the head repository Aug 1, 2024
@jkotas
Copy link
Member

jkotas commented Aug 1, 2024

@ivdiazsa Did you mean to close this?

@ivdiazsa
Copy link
Contributor Author

ivdiazsa commented Aug 1, 2024

Oh no certainly not! This was a big mistake!

@ivdiazsa
Copy link
Contributor Author

ivdiazsa commented Aug 1, 2024

@jeffschwMSFT I was housekeeping and forgot I had this pending. Would it be possible to open a new PR and get the approval right away?

@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 2024
@rbhanda rbhanda modified the milestones: 8.0.9, 8.0.10 Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-coreclr Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.NET 8 ReadyToRun regression bug: TypeLoadException

5 participants