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/9.0-staging] Allow nested types inside of the <Module> type #111479

Open
wants to merge 1 commit into
base: release/9.0-staging
Choose a base branch
from

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Jan 16, 2025

Backport of #111257 to release/9.0-staging

/cc @davidwrighton

Customer Impact

  • Customer reported
  • Found internally

This was reported by @4nonym0us as a regression in .NET 9 which causes some .NET obfuscators to generate faulty code with .NET 9. I do not know which obfuscators, or how prevalent this pattern is. The impact is that assemblies obfuscated by these tools will fail to load on .NET 9. An analysis of the ECMA specification indicates that this particular approach is permitted by ECMA 335 although it certainly wasn't anticipated by any of our tooling.

Regression

  • Yes
  • No

This was introduced with the work to improve the performance of assembly loading in scenarios where many nested types exist. See PR #94825

Testing

Manual testing of a repro scenario provided by @4nonym0us. It is not possible to use ilasm to add a regression test to our system, as it is unable to represent this form of metadata. To do so would require adding new functionality to ilasm, and probably ildasm to enable new syntax that would allow for types nested in the <module> class.

Risk

Low , fix is effectively a check that just skips doing the problematic operation.

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.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

  • This isn't disallowed by spec, although ilasm and ildasm cannot handle these cases
  • Simply skip adding the type to the available class hash.

Backport of bugfix portion of PR #111435
Fixes #111164 in .NET 9

- This isn't disallowed by spec, although ilasm and ildasm cannot handle these cases
- Simply skip adding the type to the available class hash.

Backport of bugfix portion of PR dotnet#111435
Fixes dotnet#111164 in .NET 9
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 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jan 16, 2025
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TypeSystem-coreclr Servicing-consider Issue for next servicing release review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants