Consider existence of EETypes and metadata for typeof checks (dotnet#…

…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.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs

@@ -23,12 +23,14 @@ public class SubstitutedILProvider : ILProvider
         private readonly ILProvider _nestedILProvider;
         private readonly SubstitutionProvider _substitutionProvider;
         private readonly DevirtualizationManager _devirtualizationManager;
+        private readonly MetadataManager _metadataManager;
 
-        public SubstitutedILProvider(ILProvider nestedILProvider, SubstitutionProvider substitutionProvider, DevirtualizationManager devirtualizationManager)
+        public SubstitutedILProvider(ILProvider nestedILProvider, SubstitutionProvider substitutionProvider, DevirtualizationManager devirtualizationManager, MetadataManager metadataManager = null)
         {
             _nestedILProvider = nestedILProvider;
             _substitutionProvider = substitutionProvider;
             _devirtualizationManager = devirtualizationManager;
+            _metadataManager = metadataManager;
         }
 
         public override MethodIL GetMethodIL(MethodDesc method)
@@ -989,9 +991,15 @@ private bool TryExpandTypeEquality(in TypeEqualityPatternAnalyzer analyzer, Meth
                 if (!ConstructedEETypeNode.CreationAllowed(knownType))
                     return false;
 
+                // If a constructed MethodTable for this type exists, the comparison could succeed.
                 if (_devirtualizationManager.CanReferenceConstructedTypeOrCanonicalFormOfType(knownType.NormalizeInstantiation()))
                     return false;
 
+                // If we can have metadata for the type the comparison could succeed even if no MethodTable present.
+                // (This is the case of metadata-only types, where we were able to optimize the MethodTable away.)
+                if (_metadataManager != null && knownType.GetTypeDefinition() is MetadataType mdType && _metadataManager.CanGenerateMetadata(mdType))
+                    return false;
+
                 constant = 0;
             }
 

src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs

@@ -836,7 +836,16 @@ private void ImportBranch(ILOpcode opcode, BasicBlock target, BasicBlock fallthr
                     && ConstructedEETypeNode.CreationAllowed(typeEqualityCheckType)
                     && !typeEqualityCheckType.ConvertToCanonForm(CanonicalFormKind.Specific).IsCanonicalSubtype(CanonicalFormKind.Any))
                 {
-                    condition = _factory.MaximallyConstructableType(typeEqualityCheckType);
+                    // If the type could generate metadata, we set the condition to the presence of the metadata.
+                    // This covers situations where the typeof is compared against metadata-only types.
+                    // Note this assumes a constructed MethodTable always implies having metadata.
+                    // 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)
+                        condition = _factory.TypeMetadata(typeEqualityCheckMetadataType);
+                    else
+                        condition = _factory.MaximallyConstructableType(typeEqualityCheckType);
                 }
             }
 

src/coreclr/tools/aot/ILCompiler/Program.cs

@@ -499,7 +499,7 @@ void RunScanner()
 
                 interopStubManager = scanResults.GetInteropStubManager(interopStateManager, pinvokePolicy);
 
-                ilProvider = new SubstitutedILProvider(unsubstitutedILProvider, substitutionProvider, devirtualizationManager);
+                ilProvider = new SubstitutedILProvider(unsubstitutedILProvider, substitutionProvider, devirtualizationManager, metadataManager);
 
                 // Use a more precise IL provider that uses whole program analysis for dead branch elimination
                 builder.UseILProvider(ilProvider);

src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs

@@ -529,13 +529,27 @@ class TestTypeEqualityDeadBranchScanRemoval
     {
         class NeverAllocated1 { }
         class NeverAllocated2 { }
+        class NeverAllocated3 { }
 
         class PossiblyAllocated1 { }
         class PossiblyAllocated2 { }
 
+        class MyAttribute : Attribute
+        {
+            public Type TheType;
+
+            public MyAttribute(Type t) => TheType = t;
+        }
+
+        [My(typeof(NeverAllocated3))]
+        class AttributeHolder { }
+
         [MethodImpl(MethodImplOptions.NoInlining)]
         static Type GetNeverObject() => null;
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static Type GetNeverAllocated3Type() => typeof(AttributeHolder).GetCustomAttribute<MyAttribute>().TheType;
+
         static volatile Type s_sink;
 
         public static void Run()
@@ -557,6 +571,15 @@ public static void Run()
             }
             if (Environment.GetEnvironmentVariable("SURETHING") != null)
                 s_sink = typeof(PossiblyAllocated1);
+
+            if (GetNeverAllocated3Type() == typeof(NeverAllocated3))
+            {
+                Console.WriteLine($"{nameof(NeverAllocated3)} check succeeded");
+            }
+            else
+            {
+                throw new Exception();
+            }
         }
     }