Precisely track reflected on fields (dotnet#70546)

The AOT compiler used simple logic to decide what fields should be kept/generated: if a type is considered constructed, generate all fields. This works, but it's not very efficient.

With this change I'm introducing precise tracking of each field that should be accessible from reflection at runtime. The fields are represented by new nodes within the dependency graph.

We track fields on individual generic instantiations and ensure we end up in a consistent state where if e.g. we decided `Class<int>.Foo` should be reflection accessible and `Class<double>` is used elsewhere in the program, `Class<double>.Foo` is also reflection accessible. This matches how IL Linker thinks about reflectability where genericness doesn't matter. We could be more optimal here, but various suppressions in framework rely on this logic. Additional reflectable fields only cost tens of bytes.

I had to update various places within the compiler that didn't bother specifying field dependencies because they didn't matter in the past.

Added a couple new tests that test the various invariants.

This saves 2% in size on a `dotnet new webapi` template project with IlcTrimMetadata on. It is a small regression with `IlcTrimMetadata` off because we actually now track more things for the reflectable field: previously we would not make sure the field is reflection-accessible at runtime. We need a `TypeHandle` for the field type for it to be usable. As a potential future optimization, we could look into allowing reflection to see "unconstructed" `TypeHandles` (and use that one). Reflection is currently not allowed to see unconstructed `TypeHandles` to prevent people falling into a `RuntimeHelpers.AllocateUninitializedObject(someField.FieldType.TypeHandle)` trap that has a bad failure mode right now. Once we fix the failure mode, we could potentially allow it.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs

@@ -161,7 +161,9 @@ internal void MarkStaticConstructor(in MessageOrigin origin, TypeDesc type)
         {
             if (!type.IsGenericDefinition && !type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true) && type.HasStaticConstructor)
             {
-                _dependencies.Add(_factory.CanonicalEntrypoint(type.GetStaticConstructor()), "RunClassConstructor reference");
+                // Mark the GC static base - it contains a pointer to the class constructor, but also info
+                // about whether the class constructor already executed and it's what is looked at at runtime.
+                _dependencies.Add(_factory.TypeNonGCStaticsSymbol((MetadataType)type), "RunClassConstructor reference");
             }
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs

@@ -168,7 +168,8 @@ private static bool AddDependenciesFromCustomAttributeBlob(DependencyList depend
             {
                 if (decodedArgument.Kind == CustomAttributeNamedArgumentKind.Field)
                 {
-                    // This is an instance field. We don't track them right now.
+                    if (!AddDependenciesFromField(dependencies, factory, attributeType, decodedArgument.Name))
+                        return false;
                 }
                 else
                 {
@@ -186,6 +187,29 @@ private static bool AddDependenciesFromCustomAttributeBlob(DependencyList depend
             return true;
         }
 
+        private static bool AddDependenciesFromField(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string fieldName)
+        {
+            FieldDesc field = attributeType.GetField(fieldName);
+            if (field is not null)
+            {
+                if (factory.MetadataManager.IsReflectionBlocked(field))
+                    return false;
+
+                dependencies.Add(factory.ReflectableField(field), "Custom attribute blob");
+
+                return true;
+            }
+
+            // Haven't found it in current type. Check the base type.
+            TypeDesc baseType = attributeType.BaseType;
+
+            if (baseType != null)
+                return AddDependenciesFromField(dependencies, factory, baseType, fieldName);
+
+            // Not found. This is bad metadata that will result in a runtime failure, but we shouldn't fail the compilation.
+            return true;
+        }
+
         private static bool AddDependenciesFromPropertySetter(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string propertyName)
         {
             EcmaType attributeTypeDefinition = (EcmaType)attributeType.GetTypeDefinition();

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FieldMetadataNode.cs

@@ -41,7 +41,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
         }
         protected override string GetName(NodeFactory factory)
         {
-            return "Reflectable field: " + _field.ToString();
+            return "Field metadata: " + _field.ToString();
         }
 
         protected override void OnMarked(NodeFactory factory)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GCStaticsNode.cs

@@ -19,6 +19,7 @@ public class GCStaticsNode : ObjectNode, ISymbolDefinitionNode, ISortableSymbolN
         public GCStaticsNode(MetadataType type, PreinitializationManager preinitManager)
         {
             Debug.Assert(!type.IsCanonicalSubtype(CanonicalFormKind.Specific));
+            Debug.Assert(!type.IsGenericDefinition);
             _type = type;
 
             if (preinitManager.IsPreinitialized(type))

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/MethodMetadataNode.cs

@@ -49,7 +49,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
         }
         protected override string GetName(NodeFactory factory)
         {
-            return "Reflectable method: " + _method.ToString();
+            return "Method metadata: " + _method.ToString();
         }
 
         protected override void OnMarked(NodeFactory factory)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

@@ -294,6 +294,11 @@ private void CreateNodeCaches()
                 return new ReflectableMethodNode(method);
             });
 
+            _reflectableFields = new NodeCache<FieldDesc, ReflectableFieldNode>(field =>
+            {
+                return new ReflectableFieldNode(field);
+            });
+
             _objectGetTypeFlowDependencies = new NodeCache<MetadataType, ObjectGetTypeFlowDependenciesNode>(type =>
             {
                 return new ObjectGetTypeFlowDependenciesNode(type);
@@ -852,6 +857,12 @@ public ReflectableMethodNode ReflectableMethod(MethodDesc method)
             return _reflectableMethods.GetOrAdd(method);
         }
 
+        private NodeCache<FieldDesc, ReflectableFieldNode> _reflectableFields;
+        public ReflectableFieldNode ReflectableField(FieldDesc field)
+        {
+            return _reflectableFields.GetOrAdd(field);
+        }
+
         private NodeCache<MetadataType, ObjectGetTypeFlowDependenciesNode> _objectGetTypeFlowDependencies;
         internal ObjectGetTypeFlowDependenciesNode ObjectGetTypeFlowDependencies(MetadataType type)
         {

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NonGCStaticsNode.cs

@@ -25,6 +25,7 @@ public class NonGCStaticsNode : ObjectNode, ISymbolDefinitionNode, ISortableSymb
         public NonGCStaticsNode(MetadataType type, PreinitializationManager preinitializationManager)
         {
             Debug.Assert(!type.IsCanonicalSubtype(CanonicalFormKind.Specific));
+            Debug.Assert(!type.IsGenericDefinition);
             _type = type;
             _preinitializationManager = preinitializationManager;
         }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectableFieldNode.cs

@@ -0,0 +1,105 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+
+using ILCompiler.DependencyAnalysisFramework;
+
+using Internal.TypeSystem;
+
+using Debug = System.Diagnostics.Debug;
+
+namespace ILCompiler.DependencyAnalysis
+{
+    /// <summary>
+    /// Represents a field that is gettable/settable from reflection.
+    /// </summary>
+    public class ReflectableFieldNode : DependencyNodeCore<NodeFactory>
+    {
+        private readonly FieldDesc _field;
+
+        public ReflectableFieldNode(FieldDesc field)
+        {
+            Debug.Assert(!field.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)
+                || field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific) == field.OwningType);
+            _field = field;
+        }
+
+        public FieldDesc Field => _field;
+
+        public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
+        {
+            Debug.Assert(!factory.MetadataManager.IsReflectionBlocked(_field.GetTypicalFieldDefinition()));
+
+            DependencyList dependencies = new DependencyList();
+            factory.MetadataManager.GetDependenciesDueToReflectability(ref dependencies, factory, _field);
+
+            // No runtime artifacts needed if this is a generic definition or literal field
+            if (_field.OwningType.IsGenericDefinition || _field.IsLiteral)
+            {
+                return dependencies;
+            }
+
+            FieldDesc typicalField = _field.GetTypicalFieldDefinition();
+            if (typicalField != _field)
+            {
+                // Ensure we consistently apply reflectability to all fields sharing the same definition.
+                // Bases for different instantiations of the field have a conditional dependency on the definition node that
+                // brings a ReflectableField of the instantiated field if it's necessary for it to be reflectable.
+                dependencies.Add(factory.ReflectableField(typicalField), "Definition of the reflectable field");
+            }
+
+            // Runtime reflection stack needs to see the type handle of the owning type
+            dependencies.Add(factory.MaximallyConstructableType(_field.OwningType), "Instance base of a reflectable field");
+
+            // Root the static base of the type
+            if (_field.IsStatic && !_field.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any))
+            {
+                // Infrastructure around static constructors is stashed in the NonGC static base
+                bool needsNonGcStaticBase = factory.PreinitializationManager.HasLazyStaticConstructor(Field.OwningType);
+
+                if (_field.HasRva)
+                {
+                    // No reflection access right now
+                }
+                else if (_field.IsThreadStatic)
+                {
+                    dependencies.Add(factory.TypeThreadStaticIndex((MetadataType)_field.OwningType), "Threadstatic base of a reflectable field");
+                }
+                else if (_field.HasGCStaticBase)
+                {
+                    dependencies.Add(factory.TypeGCStaticsSymbol((MetadataType)_field.OwningType), "GC static base of a reflectable field");
+                }
+                else
+                {
+                    dependencies.Add(factory.TypeNonGCStaticsSymbol((MetadataType)_field.OwningType), "NonGC static base of a reflectable field");
+                    needsNonGcStaticBase = false;
+                }
+
+                if (needsNonGcStaticBase)
+                {
+                    dependencies.Add(factory.TypeNonGCStaticsSymbol((MetadataType)_field.OwningType), "CCtor context");
+                }
+            }
+
+            // Runtime reflection stack needs to obtain the type handle of the field
+            // (but there's no type handles for function pointers)
+            if (!_field.FieldType.IsFunctionPointer)
+                dependencies.Add(factory.MaximallyConstructableType(_field.FieldType.NormalizeInstantiation()), "Type of the field");
+
+            return dependencies;
+        }
+        protected override string GetName(NodeFactory factory)
+        {
+            return "Reflectable field: " + _field.ToString();
+        }
+
+        public override bool InterestingForDynamicDependencyAnalysis => false;
+        public override bool HasDynamicDependencies => false;
+        public override bool HasConditionalStaticDependencies => false;
+        public override bool StaticDependenciesAreComputed => true;
+        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
+        public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null;
+    }
+}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ThreadStaticsNode.cs

@@ -19,7 +19,8 @@ public class ThreadStaticsNode : EmbeddedObjectNode, ISymbolDefinitionNode
 
         public ThreadStaticsNode(MetadataType type, NodeFactory factory)
         {
-            Debug.Assert(factory.Target.Abi == TargetAbi.NativeAot || factory.Target.Abi == TargetAbi.CppCodegen);
+            Debug.Assert(!type.IsCanonicalSubtype(CanonicalFormKind.Specific));
+            Debug.Assert(!type.IsGenericDefinition);
             _type = type;
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs

@@ -58,6 +58,16 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
                 // A lot of the enum reflection actually happens on top of the respective MethodTable (e.g. getting the underlying type),
                 // so for enums also include their MethodTable.
                 dependencies.Add(factory.MaximallyConstructableType(_type), "Reflectable enum");
+
+                // Enums are not useful without their literal fields. The literal fields are not referenced
+                // from anywhere (source code reference to enums compiles to the underlying numerical constants in IL).
+                foreach (FieldDesc enumField in _type.GetFields())
+                {
+                    if (enumField.IsLiteral)
+                    {
+                        dependencies.Add(factory.FieldMetadata(enumField), "Value of a reflectable enum");
+                    }
+                }
             }
 
             // If the user asked for complete metadata to be generated for all types that are getting metadata, ensure that.

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

@@ -13,6 +13,7 @@ public interface IRootingServiceProvider
         void AddCompilationRoot(MethodDesc method, string reason, string exportName = null);
         void AddCompilationRoot(TypeDesc type, string reason);
         void AddReflectionRoot(MethodDesc method, string reason);
+        void AddReflectionRoot(FieldDesc field, string reason);
         void RootThreadStaticBaseForType(TypeDesc type, string reason);
         void RootGCStaticBaseForType(TypeDesc type, string reason);
         void RootNonGCStaticBaseForType(TypeDesc type, string reason);

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

@@ -345,6 +345,19 @@ public void GetDependenciesDueToReflectability(ref DependencyList dependencies,
             }
         }
 
+        /// <summary>
+        /// This method is an extension point that can provide additional metadata-based dependencies to generated fields.
+        /// </summary>
+        public void GetDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, FieldDesc field)
+        {
+            MetadataCategory category = GetMetadataCategory(field);
+
+            if ((category & MetadataCategory.Description) != 0)
+            {
+                GetMetadataDependenciesDueToReflectability(ref dependencies, factory, field);
+            }
+        }
+
         /// <summary>
         /// This method is an extension point that can provide additional metadata-based dependencies on a virtual method.
         /// </summary>
@@ -359,6 +372,13 @@ protected virtual void GetMetadataDependenciesDueToReflectability(ref Dependency
             // and property setters)
         }
 
+        protected virtual void GetMetadataDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, FieldDesc field)
+        {
+            // MetadataManagers can override this to provide additional dependencies caused by the emission of metadata
+            // (E.g. dependencies caused by the field having custom attributes applied to it: making sure we compile the attribute constructor
+            // and property setters)
+        }
+
         /// <summary>
         /// This method is an extension point that can provide additional metadata-based dependencies to generated EETypes.
         /// </summary>
@@ -371,15 +391,6 @@ public void GetDependenciesDueToReflectability(ref DependencyList dependencies,
                 GetMetadataDependenciesDueToReflectability(ref dependencies, factory, type);
             }
 
-            if ((category & MetadataCategory.RuntimeMapping) != 0)
-            {
-                // We're going to generate a mapping table entry for this. Collect dependencies.
-
-                // Nothing special is needed for the mapping table (we only emit the MethodTable and we already
-                // have one, since we got this callback). But check if a child wants to do something extra.
-                GetRuntimeMappingDependenciesDueToReflectability(ref dependencies, factory, type);
-            }
-
             GetDependenciesDueToEETypePresence(ref dependencies, factory, type);
         }
 
@@ -390,12 +401,6 @@ protected virtual void GetMetadataDependenciesDueToReflectability(ref Dependency
             // and property setters)
         }
 
-        protected virtual void GetRuntimeMappingDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
-        {
-            // MetadataManagers can override this to provide additional dependencies caused by the emission of a runtime
-            // mapping for a type.
-        }
-
         protected virtual void GetDependenciesDueToEETypePresence(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
         {
             // MetadataManagers can override this to provide additional dependencies caused by the emission of an MethodTable.