Fix preinit of types placing the same value in two fields #73605
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We had a problem where types that put the same object instance in two different fields would see two different object instances at runtime due to two frozen objects being created for what should have been just one instance. (See the test.) Frozen objects were deriving their identity from the field to which they were assigned to so the problem fell out from this awkward design. The fix is actually a simplification - stop deriving object identity from field and use a "Allocation site ID" instead. The Allocation Site ID is a tuple of "Type whose cctor we were interpreting" + "instruction counter at the time of allocation". That way we can uniquely identify object instances and keep referring to objects allocated in different cctors. I've also lifted the limitation that instance delegates can only point to objects that were assigned to some fields in a different cctor because it's no longer required to limit it.
ghost
assigned 
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@dotnet/ilc-contrib could someone have a look? |
jkotas
reviewed
Aug 10, 2022
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FrozenObjectNode.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Aug 10, 2022
jkotas
approved these changes
Aug 10, 2022
…nalysis/FrozenObjectNode.cs Co-authored-by: Jan Kotas <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had a problem where types that put the same object instance in two different fields would see two different object instances at runtime due to two frozen objects being created for what should have been just one instance. (See the test.)
Frozen objects were deriving their identity from the field to which they were assigned to so the problem fell out from this awkward design.
The fix is actually a simplification - stop deriving object identity from field and use a "Allocation site ID" instead. The Allocation Site ID is a tuple of "Type whose cctor we were interpreting" + "instruction counter at the time of allocation". That way we can uniquely identify object instances and keep referring to objects allocated in different cctors.
I've also lifted the limitation that instance delegates can only point to objects that were assigned to some fields in a different cctor because it's no longer required to limit it.
Cc @dotnet/ilc-contrib
@LakshanF this will fix the issue you were seeing in ComponentModel tests.