Propagate Type.GetInterfaces through dataflow analysis (#114149)

Fixes dotnet/linker#1731

We currently maintain two invariants:

1. The types returned by the API will have `.Interfaces` annotations at minimum
2. If the parent type was annotated `.All`, the types returned by the API will also be `.All`

We do this in the logic that keeps things, but we don't do this in terms of dataflow analysis warnings (the types retrieved from the array are not annotated as such, as far as the analysis is concerned). Because of the lacking annotation, we have warning suppressions in multiple places within the framework.

This fixes it.

Author: MichalStrehovsky

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

@@ -1305,6 +1305,10 @@ private void HandleCall(
                     {
                         MarkArrayValuesAsUnknown(arr, curBasicBlock);
                     }
+                    else if (v is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated)
+                    {
+                        arrayOfAnnotated.MarkModified();
+                    }
                 }
             }
         }
@@ -1354,6 +1358,10 @@ private void ScanStelem(
                         StoreMethodLocalValue(arrValue.IndexValues, ArrayValue.SanitizeArrayElementValue(valueToStore.Value), indexToStoreAtInt.Value, curBasicBlock, MaxTrackedArrayValues);
                     }
                 }
+                else if (array is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated)
+                {
+                    arrayOfAnnotated.MarkModified();
+                }
             }
         }
 
@@ -1366,13 +1374,27 @@ private void ScanLdelem(
         {
             StackSlot indexToLoadFrom = PopUnknown(currentStack, 1, methodBody, offset);
             StackSlot arrayToLoadFrom = PopUnknown(currentStack, 1, methodBody, offset);
+
+            bool isByRef = opcode == ILOpcode.ldelema;
+
+            if (arrayToLoadFrom.Value.AsSingleValue() is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated)
+            {
+                if (isByRef)
+                {
+                    arrayOfAnnotated.MarkModified();
+                }
+                else if (!arrayOfAnnotated.IsModified)
+                {
+                    currentStack.Push(new StackSlot(arrayOfAnnotated.GetAnyElementValue()));
+                    return;
+                }
+            }
+
             if (arrayToLoadFrom.Value.AsSingleValue() is not ArrayValue arr)
             {
                 PushUnknown(currentStack);
                 return;
             }
-            // We don't yet handle arrays of references or pointers
-            bool isByRef = opcode == ILOpcode.ldelema;
 
             int? index = indexToLoadFrom.Value.AsConstInt();
             if (index == null)

src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb

@@ -524,7 +524,7 @@ Namespace Microsoft.VisualBasic.CompilerServices
             Return System.Linq.Enumerable.ToArray(genericParameter.GetInterfaces)
         End Function
 
-        Friend Shared Function GetClassConstraint(ByVal genericParameter As Type) As Type
+        Friend Shared Function GetClassConstraint(<DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)> ByVal genericParameter As Type) As <DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)> Type
             'Returns the class constraint for the type parameter, Nothing if it has
             'no class constraint.
             Debug.Assert(IsGenericParameter(genericParameter), "expected type parameter")
@@ -931,9 +931,9 @@ Namespace Microsoft.VisualBasic.CompilerServices
             ' For a WinRT object, we want to treat members of it's collection interfaces as members of the object
             ' itself. So GetMembers calls here to find the member in all the collection interfaces that this object
             ' implements.
-            <UnconditionalSuppressMessage("ReflectionAnalysis", "IL2065:UnrecognizedReflectionPattern",
+            <SuppressMessage("ReflectionAnalysis", "IL2065:UnrecognizedReflectionPattern",
                 Justification:="_type is annotated with .All, so it's Interfaces will be annotated as well and it is safe to call GetMember on the Interfaces.
-                    We should be able to remove once https://github.com/mono/linker/issues/1731 is fixed.")>
+                    We should be able to remove once https://github.com/dotnet/runtime/issues/114425 is fixed.")>
             Friend Function LookupWinRTCollectionInterfaceMembers(ByVal memberName As String) As List(Of MemberInfo)
                 Debug.Assert(Me.IsWindowsRuntimeObject(), "Expected a Windows Runtime Object")
 
@@ -950,9 +950,6 @@ Namespace Microsoft.VisualBasic.CompilerServices
                 Return result
             End Function
 
-            <UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
-                Justification:="_type is annotated with .All, so it's BaseType will be annotated as well and it is safe to call GetMember on the BaseType.
-                    We should be able to remove once https://github.com/mono/linker/issues/1731 is fixed.")>
             Friend Function LookupNamedMembers(ByVal memberName As String) As MemberInfo()
                 'Returns an array of members matching MemberName sorted by inheritance (most derived first).
                 'If no members match MemberName, returns an empty array.

src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.ReflectedTypeData.cs

@@ -45,7 +45,7 @@ internal ReflectedTypeData(Type type, bool isRegisteredType)
             /// Retrieves custom attributes.
             /// </summary>
             [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2062:UnrecognizedReflectionPattern",
-                Justification = "_type is annotated as preserve All members, so any Types returned from GetInterfaces should be preserved as well once https://github.com/mono/linker/issues/1731 is fixed.")]
+                Justification = "_type is annotated as preserve All members, so any Types returned from GetInterfaces should be preserved as well.")]
             internal AttributeCollection GetAttributes()
             {
                 // Worst case collision scenario:  we don't want the perf hit

src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/CollectionDataContract.cs

@@ -984,17 +984,19 @@ internal static bool TryCreateGetOnlyCollectionDataContract(Type type, [NotNullW
             }
         }
 
-        // Once https://github.com/mono/linker/issues/1731 is fixed we can remove the suppression from here as it won't be needed any longer.
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:GetMethod",
-            Justification = "The DynamicallyAccessedMembers declarations will ensure the interface methods will be preserved.")]
         internal static MethodInfo? GetTargetMethodWithName(string name,
             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
             Type type,
             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
             Type interfaceType)
         {
-            Type? t = type.GetInterfaces().Where(it => it.Equals(interfaceType)).FirstOrDefault();
-            return t?.GetMethod(name);
+            Type[] interfaces = type.GetInterfaces();
+            for (int i = 0; i < interfaces.Length; i++)
+            {
+                if (interfaces[i].Equals(interfaceType))
+                    return interfaces[i].GetMethod(name);
+            }
+            return null;
         }
 
         private static bool IsArraySegment(Type t)
@@ -1280,21 +1282,22 @@ private static string GetInvalidCollectionMessage(string message, string nestedM
             return (param == null) ? SR.Format(message, nestedMessage) : SR.Format(message, nestedMessage, param);
         }
 
-        // Once https://github.com/mono/linker/issues/1731 is fixed we can remove the suppression from here as it won't be needed any longer.
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:GetMethod",
-            Justification = "The DynamicallyAccessedMembers declarations will ensure the interface methods will be preserved.")]
         private static void FindCollectionMethodsOnInterface(
             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
             Type type,
             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
             Type interfaceType,
             ref MethodInfo? addMethod, ref MethodInfo? getEnumeratorMethod)
         {
-            Type? t = type.GetInterfaces().Where(it => it.Equals(interfaceType)).FirstOrDefault();
-            if (t != null)
+            Type[] interfaces = type.GetInterfaces();
+            for (int i = 0; i < interfaces.Length; i++)
             {
-                addMethod = t.GetMethod(Globals.AddMethodName) ?? addMethod;
-                getEnumeratorMethod = t.GetMethod(Globals.GetEnumeratorMethodName) ?? getEnumeratorMethod;
+                if (interfaces[i].Equals(interfaceType))
+                {
+                    addMethod = interfaces[i].GetMethod(Globals.AddMethodName) ?? addMethod;
+                    getEnumeratorMethod = interfaces[i].GetMethod(Globals.GetEnumeratorMethodName) ?? getEnumeratorMethod;
+                    break;
+                }
             }
         }
 

src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs

@@ -164,9 +164,6 @@ public GeneratedTypeInfo GetProxyType(
                 }
             }
 
-            // Unconditionally generates a new proxy type derived from 'baseType' and implements 'interfaceType'
-            [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2062:UnrecognizedReflectionPattern",
-                Justification = "interfaceType is annotated as preserve All members, so any Types returned from GetInterfaces should be preserved as well once https://github.com/mono/linker/issues/1731 is fixed.")]
             private GeneratedTypeInfo GenerateProxyType(
                 [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] Type baseType,
                 [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type interfaceType,
@@ -202,11 +199,9 @@ private GeneratedTypeInfo GenerateProxyType(
                 // Create a type that derives from 'baseType' provided by caller
                 ProxyBuilder pb = CreateProxy("generatedProxy", baseType);
 
-                foreach (Type t in interfaceType.GetInterfaces())
-                    // interfaceType is annotated as preserve All members, so any Types returned from GetInterfaces should be preserved as well once https://github.com/mono/linker/issues/1731 is fixed.
-#pragma warning disable IL2072
-                    pb.AddInterfaceImpl(t);
-#pragma warning restore IL2072
+                Type[] interfacesOnInterfaceType = interfaceType.GetInterfaces();
+                for (int i = 0; i < interfacesOnInterfaceType.Length; i++)
+                    pb.AddInterfaceImpl(interfacesOnInterfaceType[i]);
 
                 pb.AddInterfaceImpl(interfaceType);
 

src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs

@@ -939,7 +939,6 @@ public override FieldInfo[] GetFields(BindingFlags bindingAttr)
             return fields.ToArray();
         }
 
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2063:UnrecognizedReflectionPattern")]
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)]
         [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)]
         public override Type? GetInterface(string name, bool ignoreCase)
@@ -967,7 +966,10 @@ public override FieldInfo[] GetFields(BindingFlags bindingAttr)
                 }
             }
 
+// Analyzer is not able to propagate `.Interfaces` on `this`.
+#pragma warning disable IL2063
             return match;
+#pragma warning restore IL2063
         }
 
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)]

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs

@@ -243,6 +243,10 @@ public override void HandleAssignment (MultiValue source, MultiValue target, IOp
 
 		public override MultiValue HandleArrayElementRead (MultiValue arrayValue, MultiValue indexValue, IOperation operation)
 		{
+			if (arrayValue.AsSingleValue() is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated && !arrayOfAnnotated.IsModified) {
+				return arrayOfAnnotated.GetAnyElementValue ();
+			}
+
 			if (indexValue.AsConstInt () is not int index)
 				return UnknownValue.Instance;
 
@@ -270,6 +274,8 @@ public override void HandleArrayElementWrite (MultiValue arrayValue, MultiValue
 							? _multiValueLattice.Meet (arr.IndexValues[index.Value], sanitizedValue)
 							: sanitizedValue;
 					}
+				} else if (arraySingleValue is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated) {
+					arrayOfAnnotated.MarkModified ();
 				}
 			}
 		}
@@ -308,6 +314,8 @@ public override MultiValue HandleMethodCall (
 				foreach (var argumentValue in argument.AsEnumerable ()) {
 					if (argumentValue is ArrayValue arrayValue)
 						arrayValue.IndexValues.Clear ();
+					else if (argumentValue is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated)
+						arrayOfAnnotated.MarkModified ();
 				}
 			}
 

src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayOfAnnotatedSystemTypeValue.cs

@@ -0,0 +1,42 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+
+using ILLink.Shared.DataFlow;
+
+// This is needed due to NativeAOT which doesn't enable nullable globally yet
+#nullable enable
+
+namespace ILLink.Shared.TrimAnalysis
+{
+	/// <summary>
+	/// Represents an array of <see cref="System.Type"/> where initially each element of the array has the same DynamicallyAccessedMembers annotation.
+	/// </summary>
+	internal sealed record ArrayOfAnnotatedSystemTypeValue : SingleValue
+	{
+		private readonly ValueWithDynamicallyAccessedMembers _initialValue;
+
+		public bool IsModified { get; private set; }
+
+		public ArrayOfAnnotatedSystemTypeValue (ValueWithDynamicallyAccessedMembers value) => _initialValue = value;
+
+		public override SingleValue DeepCopy ()
+		{
+			return new ArrayOfAnnotatedSystemTypeValue (this);
+		}
+
+		public SingleValue GetAnyElementValue()
+		{
+			Debug.Assert (!IsModified);
+			return _initialValue;
+		}
+
+		public void MarkModified () => IsModified = true;
+
+		public override string ToString () => this.ValueToString (_initialValue.DynamicallyAccessedMemberTypes);
+	}
+}

src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs

@@ -250,6 +250,36 @@ ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers
 				}
 				break;
 
+			//
+			// GetInterfaces
+			//
+			case IntrinsicId.Type_GetInterfaces: {
+					if (instanceValue.IsEmpty ()) {
+						returnValue = MultiValueLattice.Top;
+						break;
+					}
+
+					var targetValue = _annotations.GetMethodThisParameterValue (calledMethod, DynamicallyAccessedMemberTypes.Interfaces);
+					foreach (var value in instanceValue.AsEnumerable ()) {
+						// Require Interfaces annotation
+						_requireDynamicallyAccessedMembersAction.Invoke (value, targetValue);
+
+						// Interfaces is transitive, so the return values will always have at least Interfaces annotation
+						DynamicallyAccessedMemberTypes returnMemberTypes = DynamicallyAccessedMemberTypes.Interfaces;
+
+						// Propagate All annotation across the call - All is a superset of Interfaces
+						if (value is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers
+							&& valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.All)
+							returnMemberTypes = DynamicallyAccessedMemberTypes.All;
+
+						// We model this as if each individual element of the returned array was returned from the GetInterfaces method call.
+						// It makes diagnostics fall out nicer because we now know where the Type comes from and where to assign blame, should the requirements not match
+						AddReturnValue (new ArrayOfAnnotatedSystemTypeValue(_annotations.GetMethodReturnValue (calledMethod, isNewObj: false, returnMemberTypes)));
+					}
+				}
+				break;
+
+
 			//
 			// AssemblyQualifiedName
 			//

src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs

@@ -172,6 +172,10 @@ internal enum IntrinsicId
 		/// </summary>
 		Type_GetInterface,
 		/// <summary>
+		/// <see cref="System.Type.GetInterfaces"/>
+		/// </summary>
+		Type_GetInterfaces,
+		/// <summary>
 		/// <see cref="System.Type.AssemblyQualifiedName"/>
 		/// </summary>
 		Type_get_AssemblyQualifiedName,