From 3bb48d6680410ce4d635dcd6e10e880215db0790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 2 Apr 2025 14:50:45 +0200 Subject: [PATCH 1/4] Propagate `Type.GetInterfaces` through dataflow analysis 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. Opening as a draft because I'm having trouble making `foreach` of arrays work with the Roslyn analyzer. Roslyn models it as `IEnumerable` instead of array indexing (which is how foreach is actually expanded for arrays). I hope we can somehow force Roslyn to surface this as array indexing. In the worst case we'll need to live with the wart that `foreach` doesn't work, only `for`. --- .../Compiler/Dataflow/MethodBodyScanner.cs | 26 +++++- .../VisualBasic/CompilerServices/Symbols.vb | 2 +- .../Serialization/CollectionDataContract.cs | 27 +++--- .../Reflection/DispatchProxyGenerator.cs | 3 - .../System/Reflection/Emit/TypeBuilderImpl.cs | 4 +- .../TrimAnalysis/TrimAnalysisVisitor.cs | 8 ++ .../ArrayOfAnnotatedSystemTypeValue.cs | 42 +++++++++ .../TrimAnalysis/HandleCallAction.cs | 28 ++++++ .../ILLink.Shared/TrimAnalysis/IntrinsicId.cs | 4 + .../ILLink.Shared/TrimAnalysis/Intrinsics.cs | 5 ++ .../Linker.Dataflow/MethodBodyScanner.cs | 18 +++- .../DataFlow/ArrayDataFlow.cs | 89 +++++++++++++++++++ 12 files changed, 235 insertions(+), 21 deletions(-) create mode 100644 src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayOfAnnotatedSystemTypeValue.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs index c7df8e767d0899..eef3878c24461b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs +++ b/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) diff --git a/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb b/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb index 44c694595b35bb..c82343386ac0ca 100644 --- a/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb +++ b/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb @@ -931,7 +931,7 @@ 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. - Friend Function LookupWinRTCollectionInterfaceMembers(ByVal memberName As String) As List(Of MemberInfo) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/CollectionDataContract.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/CollectionDataContract.cs index 8d20b6d7a28979..27fbd868fe816b 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/CollectionDataContract.cs +++ b/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,9 +1282,6 @@ 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, @@ -1290,11 +1289,15 @@ private static void FindCollectionMethodsOnInterface( 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; + } } } diff --git a/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs b/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs index 5181722c578c4e..40eefbfa04c2e8 100644 --- a/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs +++ b/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, diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs index 1f485bc062f81f..9ed68f39c90c4b 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs +++ b/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) } } +// TODO: Huh? +#pragma warning disable IL2063 return match; +#pragma warning restore IL2063 } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)] diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 95ab7f31e08cdd..e2fc5fbee8f0bf 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/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 (); } } diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayOfAnnotatedSystemTypeValue.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/ArrayOfAnnotatedSystemTypeValue.cs new file mode 100644 index 00000000000000..b99c232e98cd65 --- /dev/null +++ b/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 +{ + /// + /// Represents an array of where initially each element of the array has the same DynamicallyAccessedMembers annotation. + /// + 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); + } +} diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index c6b8e73a93ba8c..ab9a956e901300 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -250,6 +250,34 @@ 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; + + AddReturnValue (new ArrayOfAnnotatedSystemTypeValue(_annotations.GetMethodReturnValue (calledMethod, isNewObj: false, returnMemberTypes))); + } + } + break; + + // // AssemblyQualifiedName // diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs index 9a8e2e343d79c9..8056970932704e 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs @@ -172,6 +172,10 @@ internal enum IntrinsicId /// Type_GetInterface, /// + /// + /// + Type_GetInterfaces, + /// /// /// Type_get_AssemblyQualifiedName, diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs index bdc94c11195c95..2d672707ac64fc 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs @@ -245,6 +245,11 @@ public static IntrinsicId GetIntrinsicIdForMethod (MethodProxy calledMethod) (calledMethod.HasMetadataParametersCount (2) && calledMethod.HasParameterOfType ((ParameterIndex) 2, "System.Boolean"))) => IntrinsicId.Type_GetInterface, + "GetInterfaces" when calledMethod.IsDeclaredOnType ("System.Type") + && calledMethod.HasImplicitThis () + && calledMethod.HasMetadataParametersCount (0) + => IntrinsicId.Type_GetInterfaces, + // System.Type.AssemblyQualifiedName "get_AssemblyQualifiedName" when calledMethod.IsDeclaredOnType ("System.Type") && calledMethod.HasImplicitThis () diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs index 849342bbe395f3..5d64386040dfe6 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -1109,6 +1109,8 @@ private void HandleCall ( foreach (var v in param.AsEnumerable ()) { if (v is ArrayValue arr) { MarkArrayValuesAsUnknown (arr, curBasicBlock); + } else if (v is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated) { + arrayOfAnnotated.MarkModified (); } } } @@ -1153,6 +1155,8 @@ private void ScanStelem ( // When we know the index, we can record the value at that index. StoreMethodLocalValue (arrValue.IndexValues, ArrayValue.SanitizeArrayElementValue (valueToStore.Value), indexToStoreAtInt.Value, curBasicBlock, MaxTrackedArrayValues); } + } else if (array is ArrayOfAnnotatedSystemTypeValue arrayOfAnnotated) { + arrayOfAnnotated.MarkModified (); } } } @@ -1165,12 +1169,22 @@ private void ScanLdelem ( { StackSlot indexToLoadFrom = PopUnknown (currentStack, 1, methodBody, operation.Offset); StackSlot arrayToLoadFrom = PopUnknown (currentStack, 1, methodBody, operation.Offset); + + bool isByRef = operation.OpCode.Code == Code.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 = operation.OpCode.Code == Code.Ldelema; int? index = indexToLoadFrom.Value.AsConstInt (); if (index == null) { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs index 8da3ce7d97ad5b..f7d6acf041d1de 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs @@ -55,6 +55,8 @@ public static void Main () ConstantFieldValuesAsIndex.Test (); HoistedArrayMutation.Test (); + + TestGetInterfacesDataflow.Test (); } [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresPublicMethods))] @@ -769,6 +771,93 @@ public static void Test () } } + class TestGetInterfacesDataflow + { + static void GetInterfacesOnInterfaces ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) + { + t.GetInterfaces ()[0].RequiresInterfaces (); + + Type[] interfaces = t.GetInterfaces (); + for (int i = 0; i < interfaces.Length; i++) { + interfaces[i].RequiresInterfaces (); + } + + foreach (var i in t.GetInterfaces ()) + i.RequiresInterfaces (); + } + + static void GetAllOnAll ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) + { + t.GetInterfaces ()[0].RequiresAll (); + + Type[] interfaces = t.GetInterfaces (); + for (int i = 0; i < interfaces.Length; i++) { + interfaces[i].RequiresAll (); + } + + foreach (var i in t.GetInterfaces ()) + i.RequiresAll (); + } + + + [ExpectedWarning ("IL2072", nameof(Type.GetInterfaces), nameof (DataFlowTypeExtensions.RequiresPublicMethods))] + static void GetMethodsOnInterfaces ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) + { + t.GetInterfaces ()[0].RequiresPublicMethods (); + } + + static void GetMethodsOnAll ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) + { + t.GetInterfaces ()[0].RequiresPublicMethods (); + + Type[] interfaces = t.GetInterfaces (); + for (int i = 0; i < interfaces.Length; i++) { + interfaces[i].RequiresPublicMethods (); + } + + foreach (var i in t.GetInterfaces ()) + i.RequiresPublicMethods (); + } + + [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresInterfaces))] + static void GetInterfacesOnModified ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) + { + Type[] interfaces = t.GetInterfaces (); + interfaces[0] = typeof (object); + interfaces[0].RequiresInterfaces (); + } + + [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresInterfaces), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/linker/issues/2680")] + static void GetInterfacesOnPassedByRef ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) + { + Type[] interfaces = t.GetInterfaces (); + Use (ref interfaces[0]); + interfaces[0].RequiresInterfaces (); + + static void Use (ref Type t) => t = null; + } + + [ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresInterfaces), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/linker/issues/2680")] + static void GetInterfacesOnSetValued ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) + { + Type[] interfaces = t.GetInterfaces (); + interfaces.SetValue (typeof (object), 0); + interfaces[0].RequiresInterfaces (); + } + + public static void Test() + { + GetInterfacesOnInterfaces (null); + GetAllOnAll (null); + GetMethodsOnAll (null); + GetMethodsOnInterfaces (null); + + GetInterfacesOnModified (null); + GetInterfacesOnPassedByRef (null); + GetInterfacesOnSetValued (null); + } + } + [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors)] private static Type GetTypeWithPublicConstructors () { From 26c56522a3ea5ee402b6ae0d83d471da0d73ee76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 9 Apr 2025 11:43:19 +0200 Subject: [PATCH 2/4] Fixes --- .../VisualBasic/CompilerServices/Symbols.vb | 7 ++----- ...tTypeDescriptionProvider.ReflectedTypeData.cs | 2 +- .../System/Reflection/DispatchProxyGenerator.cs | 8 +++----- .../DataFlow/ArrayDataFlow.cs | 16 +++++++++++++++- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb b/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb index c82343386ac0ca..608bd72399ab51 100644 --- a/src/libraries/Microsoft.VisualBasic.Core/src/Microsoft/VisualBasic/CompilerServices/Symbols.vb +++ b/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( ByVal genericParameter As Type) As Type 'Returns the class constraint for the type parameter, Nothing if it has 'no class constraint. Debug.Assert(IsGenericParameter(genericParameter), "expected type parameter") @@ -933,7 +933,7 @@ Namespace Microsoft.VisualBasic.CompilerServices ' implements. + 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 - 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. diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.ReflectedTypeData.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.ReflectedTypeData.cs index 35f7611854fd28..a8477215e4afd9 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.ReflectedTypeData.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.ReflectedTypeData.cs @@ -45,7 +45,7 @@ internal ReflectedTypeData(Type type, bool isRegisteredType) /// Retrieves custom attributes. /// [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 diff --git a/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs b/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs index 40eefbfa04c2e8..ac765e8be34ffd 100644 --- a/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs +++ b/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs @@ -199,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); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs index f7d6acf041d1de..1acdd900ad4186 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ArrayDataFlow.cs @@ -781,7 +781,11 @@ static void GetInterfacesOnInterfaces ([DynamicallyAccessedMembers (DynamicallyA for (int i = 0; i < interfaces.Length; i++) { interfaces[i].RequiresInterfaces (); } + } + [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/114425")] + static void GetInterfacesOnInterfaces_Foreach ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) + { foreach (var i in t.GetInterfaces ()) i.RequiresInterfaces (); } @@ -794,12 +798,15 @@ static void GetAllOnAll ([DynamicallyAccessedMembers (DynamicallyAccessedMemberT for (int i = 0; i < interfaces.Length; i++) { interfaces[i].RequiresAll (); } + } + [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/114425")] + static void GetAllOnAll_Foreach ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) + { foreach (var i in t.GetInterfaces ()) i.RequiresAll (); } - [ExpectedWarning ("IL2072", nameof(Type.GetInterfaces), nameof (DataFlowTypeExtensions.RequiresPublicMethods))] static void GetMethodsOnInterfaces ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.Interfaces)] Type t) { @@ -814,7 +821,11 @@ static void GetMethodsOnAll ([DynamicallyAccessedMembers (DynamicallyAccessedMem for (int i = 0; i < interfaces.Length; i++) { interfaces[i].RequiresPublicMethods (); } + } + [UnexpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/114425")] + static void GetMethodsOnAll_Foreach ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) + { foreach (var i in t.GetInterfaces ()) i.RequiresPublicMethods (); } @@ -848,8 +859,11 @@ static void GetInterfacesOnSetValued ([DynamicallyAccessedMembers (DynamicallyAc public static void Test() { GetInterfacesOnInterfaces (null); + GetInterfacesOnInterfaces_Foreach (null); GetAllOnAll (null); + GetAllOnAll_Foreach (null); GetMethodsOnAll (null); + GetMethodsOnAll_Foreach (null); GetMethodsOnInterfaces (null); GetInterfacesOnModified (null); From eedb94c0c94f0e4c966fd7c909ffe3f37788b0e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 9 Apr 2025 11:48:29 +0200 Subject: [PATCH 3/4] Update TypeBuilderImpl.cs --- .../src/System/Reflection/Emit/TypeBuilderImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs index 9ed68f39c90c4b..3e187d080f5d92 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs @@ -966,7 +966,7 @@ public override FieldInfo[] GetFields(BindingFlags bindingAttr) } } -// TODO: Huh? +// Analyzer is not able to propagate `.Interfaces` on `this`. #pragma warning disable IL2063 return match; #pragma warning restore IL2063 From a32aeb0cbdb82c6c7b848089b07b24130c9b319c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Mon, 14 Apr 2025 23:21:14 +0200 Subject: [PATCH 4/4] Update src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs --- .../illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index ab9a956e901300..700f68cb22b9f1 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -272,6 +272,8 @@ 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))); } }