-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add runtime MVID checks for composite images #49668
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
Conversation
(*) Renamed 'ComponentAssemblyMvids' to 'ManifestAssemblyMvids' to better express the fact that the MVID table is parallel to the manifest metadata. (*) Added basic R2RDump support for dumping the manifest assembly MVID table. (*) Fixed R2R format version number in Crossgen2. Thanks Tomas
...Compiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestAssemblyMvidHeaderNode.cs
Outdated
Show resolved
Hide resolved
...ot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj
Outdated
Show resolved
Hide resolved
...ot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
Outdated
Show resolved
Hide resolved
davidwrighton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mangod9, @dotnet/crossgen-contrib Before we ship, we need to figure out what to do about hammer servicing, as a hammer service event will cause a failfast with this, but otherwise, this looks ok.
|
Thanks David and Anton for your feedback. Just to set the right expectations, this is supposed to be a mitigation of the large version bubble consistency issue, not its full resolution. In particular, while the proposed update to the R2R format also supports MVID tables for single-input compilations in large version bubble mode, I haven't implemented the consistency checks for this case as they would be more costly and my current understanding is that it's not a shipping scenario. |
|
We should discuss hammer servicing during our cg2 sync. |
This change introduces runtime consistency checks that make sure that for component assemblies of component images we're loading the same versions of these assemblies (with matching MVID). The change has the following aspects:
I'm still testing the change locally and I intend to author a negative unit test but from my perspective the change should be code complete so I'm looking forward to feedback.
Thanks
Tomas