Fix generic parameter data flow validation in NativeAOT

src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs

@@ -89,7 +89,10 @@ public static string GetDisplayName(this MethodDesc method)
             if (method.Signature.Length > 0)
             {
                 for (int i = 0; i < method.Signature.Length - 1; i++)
-                    sb.Append(method.Signature[i].GetDisplayNameWithoutNamespace()).Append(',');
+                {
+                    TypeDesc instantiatedType = method.Signature[i].InstantiateSignature(method.OwningType.Instantiation, method.Instantiation);
+                    sb.Append(instantiatedType.GetDisplayNameWithoutNamespace()).Append(',');
+                }
 
                 sb.Append(method.Signature[method.Signature.Length - 1].GetDisplayNameWithoutNamespace());
             }

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

@@ -46,8 +46,8 @@ public bool RequiresDataflowAnalysis(MethodDesc method)
             try
             {
                 method = method.GetTypicalMethodDefinition();
-                return GetAnnotations(method.OwningType).TryGetAnnotation(method, out var methodAnnotations)
-                    && (methodAnnotations.ReturnParameterAnnotation != DynamicallyAccessedMemberTypes.None || methodAnnotations.ParameterAnnotations != null);
+                TypeAnnotations typeAnnotations = GetAnnotations(method.OwningType);
+                return typeAnnotations.HasGenericParameterAnnotation() || typeAnnotations.TryGetAnnotation(method, out _);
             }
             catch (TypeSystemException)
             {
@@ -73,7 +73,8 @@ public bool RequiresDataflowAnalysis(FieldDesc field)
             try
             {
                 field = field.GetTypicalFieldDefinition();
-                return GetAnnotations(field.OwningType).TryGetAnnotation(field, out _);
+                TypeAnnotations typeAnnotations = GetAnnotations(field.OwningType);
+                return typeAnnotations.HasGenericParameterAnnotation() || typeAnnotations.TryGetAnnotation(field, out _);
             }
             catch (TypeSystemException)
             {
@@ -105,6 +106,31 @@ public bool HasAnyAnnotations(TypeDesc type)
             }
         }
 
+        public bool HasGenericParameterAnnotation(TypeDesc type)
+        {
+            try
+            {
+                return GetAnnotations(type.GetTypeDefinition()).HasGenericParameterAnnotation();
+            }
+            catch (TypeSystemException)
+            {
+                return false;
+            }
+        }
+
+        public bool HasGenericParameterAnnotation(MethodDesc method)
+        {
+            try
+            {
+                method = method.GetTypicalMethodDefinition();
+                return GetAnnotations(method.OwningType).TryGetAnnotation(method, out var annotation) && annotation.GenericParameterAnnotations != null;
+            }
+            catch (TypeSystemException)
+            {
+                return false;
+            }
+        }
+
         internal DynamicallyAccessedMemberTypes GetParameterAnnotation(ParameterProxy param)
         {
             MethodDesc method = param.Method.Method.GetTypicalMethodDefinition();
@@ -884,6 +910,8 @@ public bool TryGetAnnotation(GenericParameterDesc genericParameter, out Dynamica
 
                 return false;
             }
+
+            public bool HasGenericParameterAnnotation() => _genericParameterAnnotations != null;
         }
 
         private readonly struct MethodAnnotations

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

@@ -18,45 +18,140 @@
 
 namespace ILCompiler.Dataflow
 {
-    internal readonly struct GenericArgumentDataFlow
+    internal static class GenericArgumentDataFlow
     {
-        private readonly Logger _logger;
-        private readonly NodeFactory _factory;
-        private readonly FlowAnnotations _annotations;
-        private readonly MessageOrigin _origin;
+        public static void ProcessGenericArgumentDataFlow(ref DependencyList dependencies, NodeFactory factory, in MessageOrigin origin, TypeDesc type, TypeDesc contextType)
+        {
+            ProcessGenericArgumentDataFlow(ref dependencies, factory, origin, type, contextType.Instantiation, Instantiation.Empty);
+        }
 
-        public GenericArgumentDataFlow(Logger logger, NodeFactory factory, FlowAnnotations annotations, in MessageOrigin origin)
+        public static void ProcessGenericArgumentDataFlow(ref DependencyList dependencies, NodeFactory factory, in MessageOrigin origin, TypeDesc type, MethodDesc contextMethod)
         {
-            _logger = logger;
-            _factory = factory;
-            _annotations = annotations;
-            _origin = origin;
+            ProcessGenericArgumentDataFlow(ref dependencies, factory, origin, type, contextMethod.OwningType.Instantiation, contextMethod.Instantiation);
         }
 
-        public DependencyList ProcessGenericArgumentDataFlow(GenericParameterDesc genericParameter, TypeDesc genericArgument)
+        private static void ProcessGenericArgumentDataFlow(ref DependencyList dependencies, NodeFactory factory, in MessageOrigin origin, TypeDesc type, Instantiation typeContext, Instantiation methodContext)
         {
-            var genericParameterValue = _annotations.GetGenericParameterValue(genericParameter);
-            Debug.Assert(genericParameterValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None);
+            if (!type.HasInstantiation)
+                return;
 
-            MultiValue genericArgumentValue = _annotations.GetTypeValueFromGenericArgument(genericArgument);
+            TypeDesc instantiatedType = type.InstantiateSignature(typeContext, methodContext);
+
+            var mdManager = (UsageBasedMetadataManager)factory.MetadataManager;
 
             var diagnosticContext = new DiagnosticContext(
-                _origin,
-                _logger.ShouldSuppressAnalysisWarningsForRequires(_origin.MemberDefinition, DiagnosticUtilities.RequiresUnreferencedCodeAttribute),
-                _logger);
-            return RequireDynamicallyAccessedMembers(diagnosticContext, genericArgumentValue, genericParameterValue, genericParameter.GetDisplayName());
-        }
-
-        private DependencyList RequireDynamicallyAccessedMembers(
-            in DiagnosticContext diagnosticContext,
-            in MultiValue value,
-            ValueWithDynamicallyAccessedMembers targetValue,
-            string reason)
-        {
-            var reflectionMarker = new ReflectionMarker(_logger, _factory, _annotations, typeHierarchyDataFlowOrigin: null, enabled: true);
-            var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction(reflectionMarker, diagnosticContext, reason);
-            requireDynamicallyAccessedMembersAction.Invoke(value, targetValue);
-            return reflectionMarker.Dependencies;
+                origin,
+                !mdManager.Logger.ShouldSuppressAnalysisWarningsForRequires(origin.MemberDefinition, DiagnosticUtilities.RequiresUnreferencedCodeAttribute),
+                mdManager.Logger);
+            var reflectionMarker = new ReflectionMarker(mdManager.Logger, factory, mdManager.FlowAnnotations, typeHierarchyDataFlowOrigin: null, enabled: true);
+
+            ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, instantiatedType);
+
+            if (reflectionMarker.Dependencies.Count > 0)
+            {
+                if (dependencies == null)
+                    dependencies = reflectionMarker.Dependencies;
+                else
+                    dependencies.AddRange(reflectionMarker.Dependencies);
+            }
+        }
+
+        public static void ProcessGenericArgumentDataFlow(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, TypeDesc type)
+        {
+            TypeDesc typeDefinition = type.GetTypeDefinition();
+            if (typeDefinition != type)
+            {
+                ProcessGenericInstantiation(diagnosticContext, reflectionMarker, type.Instantiation, typeDefinition.Instantiation);
+            }
+        }
+
+        public static void ProcessGenericArgumentDataFlow(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, MethodDesc method)
+        {
+            MethodDesc typicalMethod = method.GetTypicalMethodDefinition();
+            if (typicalMethod != method)
+            {
+                ProcessGenericInstantiation(diagnosticContext, reflectionMarker, method.Instantiation, typicalMethod.Instantiation);
+            }
+
+            ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, method.OwningType);
+        }
+
+        public static void ProcessGenericArgumentDataFlow(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, FieldDesc field)
+        {
+            ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, field.OwningType);
+        }
+
+        private static void ProcessGenericInstantiation(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, Instantiation instantiation, Instantiation typicalInstantiation)
+        {
+            for (int i = 0; i < instantiation.Length; i++)
+            {
+                // Apply annotations to the generic argument
+                var genericArgument = instantiation[i];
+                var genericParameter = (GenericParameterDesc)typicalInstantiation[i];
+                if (reflectionMarker.Annotations.GetGenericParameterAnnotation(genericParameter) != default)
+                {
+                    var genericParameterValue = reflectionMarker.Annotations.GetGenericParameterValue(genericParameter);
+                    Debug.Assert(genericParameterValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None);
+                    MultiValue genericArgumentValue = reflectionMarker.Annotations.GetTypeValueFromGenericArgument(genericArgument);
+                    var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction(reflectionMarker, diagnosticContext, genericParameter.GetDisplayName());
+                    requireDynamicallyAccessedMembersAction.Invoke(genericArgumentValue, genericParameterValue);
+                }
+
+                // Recursively process generic argument data flow on the generic argument if it itself is generic
+                if (genericArgument.HasInstantiation)
+                {
+                    ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, genericArgument);
+                }
+            }
+        }
+
+        public static bool RequiresGenericArgumentDataFlow(FlowAnnotations flowAnnotations, MethodDesc method)
+        {
+            // Method callsites can contain generic instantiations which may contain annotations inside nested generics
+            // so we have to check all of the instantiations for that case.
+            // For example:
+            //   OuterGeneric<InnerGeneric<Annotated>>.Method<InnerGeneric<AnotherAnnotated>>();
+
+            if (method.HasInstantiation)
+            {
+                if (flowAnnotations.HasGenericParameterAnnotation(method))
+                    return true;
+
+                foreach (TypeDesc typeParameter in method.Instantiation)
+                {
+                    if (RequiresGenericArgumentDataFlow(flowAnnotations, typeParameter))
+                        return true;
+                }
+            }
+
+            return RequiresGenericArgumentDataFlow(flowAnnotations, method.OwningType);
+        }
+
+        public static bool RequiresGenericArgumentDataFlow(FlowAnnotations flowAnnotations, FieldDesc field)
+            // Field access can contain generic instantiations which may contain annotations inside nested generics
+            // For example:
+            //  OuterGeneric<InnerGeneric<Annotated>>.Field
+            => RequiresGenericArgumentDataFlow(flowAnnotations, field.OwningType);
+
+        /// <summary>
+        /// For a given type determines if its usage means we need to run the callsite through data flow.
+        /// This is purely for type references alone, so for example for generic parameters.
+        /// </summary>
+        public static bool RequiresGenericArgumentDataFlow(FlowAnnotations flowAnnotations, TypeDesc type)
+        {
+            if (flowAnnotations.HasGenericParameterAnnotation(type))
+                return true;
+
+            if (type.HasInstantiation)
+            {
+                foreach (TypeDesc typeParameter in type.Instantiation)
+                {
+                    if (RequiresGenericArgumentDataFlow(flowAnnotations, typeParameter))
+                        return true;
+                }
+            }
+
+            return false;
         }
     }
 }