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

PersistedAssemblyBuilder: fix IL reference tokens to another generated assembly members #107661

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Sep 10, 2024

A generated assembly member tokens will not populated properly until its saved. Therefore we reserve Blob for such tokens and fill-in during assembly.Save() operation. As per this design current code reserves tokens in case the IL referencing TypeBuilder, FieldBuilder, MethodBuilder, ConstructorBuilder or generic type parameter references TypeBuilder. This caused a bug when members are refenced from another generated assembly, tokens are reserved as it was in the same assembly instead it should have reference tokens to that assembly

The fix basically involves checking if the referenced member is in a same module, that is pretty much adding another check && Equals(member.Module) and make the static methods instance in order to use the Equals(object) instance method of the module

Fixes #107658

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@buyaa-n buyaa-n merged commit ea269f9 into dotnet:main Sep 11, 2024
85 checks passed
@buyaa-n buyaa-n deleted the reference-gen-assembly branch September 11, 2024 15:54
@buyaa-n
Copy link
Contributor Author

buyaa-n commented Sep 11, 2024

/backport to release/9.0

Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10815345665

@leppie
Copy link
Contributor

leppie commented Sep 12, 2024

Thanks!

Will this be in the daily builds?

Edit: I see I have to look for the commits of the 'rolling' builds. Seems runtime is a day or 2 behind, but will look for when it comes.

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Sep 12, 2024

I don't see it made to daily build yet https://github.com/dotnet/sdk/blob/main/documentation/package-table.md

@leppie
Copy link
Contributor

leppie commented Sep 13, 2024

Arrived with 9.0.100-rc.2.24463.1.

Hoping there is no more issues!

Update: No other issues detected, all persisted dll's passes IL verify checks.

@leppie
Copy link
Contributor

leppie commented Sep 13, 2024

Good news! Successfully compiled all 234 libraries I have in IronScheme.

Thanks a lot for your work on this, my 5 users will be ecstatic!

Nice speed bump too compared to .NET 2 x86!

.NET 2 x86

Statistics for 'total compile time':
  Real Time:  65306ms
  CPU Time:   29219ms
  User Time:  32578ms
  GC's:       1167

.NET 9

Statistics for 'total compile time':
  Real Time:  48039ms
  CPU Time:   26656ms
  User Time:  28469ms
  GC's:       1326

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Sep 13, 2024

Great to hear @leppie, thank you for giving it try and reporting issues found!

@leppie
Copy link
Contributor

leppie commented Sep 14, 2024

Great to hear @leppie, thank you for giving it try and reporting issues found!

Pleasure. IronScheme tends to break things, so a pretty good coverage test.

It is a single assembly that run on any .NET 2 onwards basically (MS or Mono). There is a very small PAL that is loaded dynamically from embedded resources. If the platform can run dotnet, it can run IronScheme. I have even ran it on my TV.

Persisted compilation will just make it a lot faster for limited platforms, instead of relying on the incremental (runtime) compiler.

jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 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.

Tokens that reference generated assembly members not populated properly
3 participants