Consider existence of EETypes and metadata for typeof checks#107347
Consider existence of EETypes and metadata for typeof checks#107347MichalStrehovsky merged 6 commits intodotnet:mainfrom
Conversation
ghost
added
the
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
sbomer
left a comment
sbomer
left a comment
There was a problem hiding this comment.
The PR title describes an older approach right?
| // This will likely remain true because anyone can call Object.GetType on a constructed type. | ||
| // If the type cannot generate metadata, we only condition on the MethodTable itself. | ||
| if (!_factory.MetadataManager.IsReflectionBlocked(typeEqualityCheckType) | ||
| && typeEqualityCheckType.GetTypeDefinition() is MetadataType typeEqualityCheckMetadataType) |
There was a problem hiding this comment.
Why is this condition different than the CanGenerateMetadata check above?
If having a constructed MethodTable implies having metadata, could the check in SubstitutedILProvider only look for metadata, instead of looking for both the constructed MethodTabla and metadata?
There was a problem hiding this comment.
The reflection-free mode wouldn't have any metadata because we never generate any there. In that case, the comparison will only succeed if there's a MethodTable.
When scanning, we're not sure what generates and what doesn't generate metadata yet, so we check IsReflectionBlocked (would never generate metadata). In the scanning phase, it's equivalent to CanGenerateMetadata. Could have used that too, but this is more explicit.
MichalStrehovsky
changed the title
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/backport to release/9.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10787848434 |
…107347) The problem was that when we optimize `typeof(Foo) == somevalue` checks, we were looking at the presence of a constructed `MethodTable` of `Foo` in the whole program view. If it didn't exist, we'd declare the equality comparison can't succeed. There is however an edge case where someone could obtain a `System.Type` representing a type by browsing the reflection metadata. Such `System.Type` might not have a workable constructed `MethodTable`, but should still compare equal to the `typeof`. An example of when this could happen is when the type is used in a custom attribute metadata blob - such type may even have no `MethodTable` at all. The fix is to look not just at `MethodTable` but also at the metadata for the type.
…107347) The problem was that when we optimize `typeof(Foo) == somevalue` checks, we were looking at the presence of a constructed `MethodTable` of `Foo` in the whole program view. If it didn't exist, we'd declare the equality comparison can't succeed. There is however an edge case where someone could obtain a `System.Type` representing a type by browsing the reflection metadata. Such `System.Type` might not have a workable constructed `MethodTable`, but should still compare equal to the `typeof`. An example of when this could happen is when the type is used in a custom attribute metadata blob - such type may even have no `MethodTable` at all. The fix is to look not just at `MethodTable` but also at the metadata for the type.
2 participants
Fixes #107300.
The problem was that when we optimize
typeof(Foo) == somevaluechecks, we were looking at the presence of a constructedMethodTableofFooin the whole program view. If it didn't exist, we'd declare the equality comparison can't succeed. There is however an edge case where someone could obtain aSystem.Typerepresenting a type by browsing the reflection metadata. SuchSystem.Typemight not have a workable constructedMethodTable, but should still compare equal to thetypeof.An example of when this could happen is when the type is used in a custom attribute metadata blob - such type may even have no
MethodTableat all.The fix is to look not just at
MethodTablebut also at the metadata for the type.Cc @dotnet/ilc-contrib