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 777db4bcf561c5..de156c59833c51 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs @@ -626,9 +626,9 @@ protected virtual void Scan(MethodIL methodIL, ref InterproceduralState interpro case ILOpcode.newarr: { StackSlot count = PopUnknown(currentStack, 1, methodIL, offset); - var arrayElement = (TypeDesc)methodIL.GetObject(reader.ReadILToken()); - HandleTypeTokenAccess(methodIL, offset, arrayElement); - currentStack.Push(new StackSlot(ArrayValue.Create(count.Value, arrayElement))); + var arrayElementType = (TypeDesc)methodIL.GetObject(reader.ReadILToken()); + HandleTypeTokenAccess(methodIL, offset, arrayElementType); + currentStack.Push(new StackSlot(ArrayValue.Create(count.Value, arrayElementType))); } break; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/RequireDynamicallyAccessedMembersAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/RequireDynamicallyAccessedMembersAction.cs index a1ae67004fc2af..47860a02eaf6f4 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/RequireDynamicallyAccessedMembersAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/RequireDynamicallyAccessedMembersAction.cs @@ -29,7 +29,7 @@ public partial bool TryResolveTypeNameAndMark(string typeName, bool needsAssembl { if (_reflectionMarker.TryResolveTypeNameAndMark(typeName, _diagnosticContext, needsAssemblyName, _reason, out TypeDesc? foundType)) { - if (foundType.HasInstantiation && _reflectionMarker.Annotations.HasGenericParameterAnnotation(foundType)) + if (GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(_reflectionMarker.Annotations, foundType)) { GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(_diagnosticContext, _reflectionMarker, foundType); } diff --git a/src/tools/illink/src/linker/CompatibilitySuppressions.xml b/src/tools/illink/src/linker/CompatibilitySuppressions.xml index 6740a9ba35ac59..eba23205563f29 100644 --- a/src/tools/illink/src/linker/CompatibilitySuppressions.xml +++ b/src/tools/illink/src/linker/CompatibilitySuppressions.xml @@ -561,6 +561,12 @@ CP0001 T:System.Reflection.RuntimeAssemblyName + + CP0001 + T:Mono.Linker.Dataflow.TrimAnalysisGenericInstantiationAccessPattern + ref/net10.0/illink.dll + lib/net10.0/illink.dll + CP0002 F:Mono.Linker.AnnotationStore.assembly_actions diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 1010982cf86d81..a49af0259ada3e 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -42,7 +42,23 @@ public bool RequiresVirtualMethodDataFlowAnalysis(MethodDefinition method) => public bool RequiresDataFlowAnalysis(FieldDefinition field) => GetAnnotations(field.DeclaringType).TryGetAnnotation(field, out _); - public bool RequiresGenericArgumentDataFlowAnalysis(GenericParameter genericParameter) => + public bool HasGenericParameterAnnotation(TypeReference type) + { + if (type.ResolveToTypeDefinition(_context) is not TypeDefinition typeDefinition) + return false; + + return GetAnnotations(typeDefinition).HasGenericParameterAnnotation(); + } + + public bool HasGenericParameterAnnotation(MethodReference method) + { + if (_context.TryResolve(method) is not MethodDefinition methodDefinition) + return false; + + return GetAnnotations(methodDefinition.DeclaringType).TryGetAnnotation(methodDefinition, out var annotation) && annotation.GenericParameterAnnotations != null; + } + + public bool RequiresGenericArgumentDataFlow(GenericParameter genericParameter) => GetGenericParameterAnnotation(genericParameter) != DynamicallyAccessedMemberTypes.None; internal DynamicallyAccessedMemberTypes GetParameterAnnotation(ParameterProxy param) @@ -731,6 +747,8 @@ public bool TryGetAnnotation(GenericParameter genericParameter, out DynamicallyA return false; } + + public bool HasGenericParameterAnnotation() => _genericParameterAnnotations != null; } readonly struct MethodAnnotations diff --git a/src/tools/illink/src/linker/Linker.Dataflow/GenericArgumentDataFlow.cs b/src/tools/illink/src/linker/Linker.Dataflow/GenericArgumentDataFlow.cs index 6aaa5677199deb..b7852fa800fb1e 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/GenericArgumentDataFlow.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/GenericArgumentDataFlow.cs @@ -1,6 +1,7 @@ // 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.Collections.ObjectModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using ILLink.Shared.TrimAnalysis; @@ -10,35 +11,111 @@ namespace Mono.Linker.Dataflow { - public readonly struct GenericArgumentDataFlow + internal static class GenericArgumentDataFlow { - readonly LinkContext _context; - readonly MarkStep _markStep; - readonly MessageOrigin _origin; + public static void ProcessGenericArgumentDataFlow(in MessageOrigin origin, MarkStep markStep, LinkContext context, TypeReference type) + { + var diagnosticContext = new DiagnosticContext(origin, !context.Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode(origin.Provider, out _), context); + var reflectionMarker = new ReflectionMarker(context, markStep, enabled: true); + ProcessGenericArgumentDataFlow(in diagnosticContext, reflectionMarker, context, type); + } + + public static void ProcessGenericArgumentDataFlow(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, LinkContext context, TypeReference type) + { + if (type is GenericInstanceType genericInstanceType && context.TryResolve(type) is TypeDefinition typeDefinition) + { + ProcessGenericInstantiation(diagnosticContext, reflectionMarker, context, genericInstanceType, typeDefinition); + } + } + + public static void ProcessGenericArgumentDataFlow(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, LinkContext context, MethodReference method) + { + if (method is GenericInstanceMethod genericInstanceMethod && context.TryResolve(method) is MethodDefinition methodDefinition) + { + ProcessGenericInstantiation(diagnosticContext, reflectionMarker, context, genericInstanceMethod, methodDefinition); + } + + ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, context, method.DeclaringType); + } + + public static void ProcessGenericArgumentDataFlow(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, LinkContext context, FieldReference field) + { + ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, context, field.DeclaringType); + } - public GenericArgumentDataFlow(LinkContext context, MarkStep markStep, in MessageOrigin origin) + private static void ProcessGenericInstantiation(in DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, LinkContext context, IGenericInstance genericInstance, IGenericParameterProvider genericParameterProvider) { - _context = context; - _markStep = markStep; - _origin = origin; + var arguments = genericInstance.GenericArguments; + var parameters = genericParameterProvider.GenericParameters; + + for (int i = 0; i < arguments.Count; i++) + { + var genericArgument = arguments[i]; + var genericParameter = parameters[i]; + + var genericParameterValue = context.Annotations.FlowAnnotations.GetGenericParameterValue(genericParameter); + if (genericParameterValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None) + { + MultiValue genericArgumentValue = context.Annotations.FlowAnnotations.GetTypeValueFromGenericArgument(genericArgument); + + var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction(context, reflectionMarker, diagnosticContext); + requireDynamicallyAccessedMembersAction.Invoke(genericArgumentValue, genericParameterValue); + } + + // Recursively process generic argument data flow on the generic argument if it itself is generic + if (genericArgument.IsGenericInstance) + { + ProcessGenericArgumentDataFlow(diagnosticContext, reflectionMarker, context, genericArgument); + } + } } - public void ProcessGenericArgumentDataFlow(GenericParameter genericParameter, TypeReference genericArgument) + internal static bool RequiresGenericArgumentDataFlow(FlowAnnotations flowAnnotations, MethodReference method) { - var genericParameterValue = _context.Annotations.FlowAnnotations.GetGenericParameterValue(genericParameter); - Debug.Assert(genericParameterValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None); + // 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>.Method>(); + + if (method is GenericInstanceMethod genericInstanceMethod) + { + if (flowAnnotations.HasGenericParameterAnnotation(method)) + return true; - MultiValue genericArgumentValue = _context.Annotations.FlowAnnotations.GetTypeValueFromGenericArgument(genericArgument); + foreach (var genericArgument in genericInstanceMethod.GenericArguments) + { + if (RequiresGenericArgumentDataFlow(flowAnnotations, genericArgument)) + return true; + } + } - var diagnosticContext = new DiagnosticContext(_origin, !_context.Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode(_origin.Provider, out _), _context); - RequireDynamicallyAccessedMembers(diagnosticContext, genericArgumentValue, genericParameterValue); + return RequiresGenericArgumentDataFlow(flowAnnotations, method.DeclaringType); } - void RequireDynamicallyAccessedMembers(in DiagnosticContext diagnosticContext, in MultiValue value, ValueWithDynamicallyAccessedMembers targetValue) + internal static bool RequiresGenericArgumentDataFlow(FlowAnnotations flowAnnotations, FieldReference field) { - var reflectionMarker = new ReflectionMarker(_context, _markStep, enabled: true); - var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction(_context, reflectionMarker, diagnosticContext); - requireDynamicallyAccessedMembersAction.Invoke(value, targetValue); + return RequiresGenericArgumentDataFlow(flowAnnotations, field.DeclaringType); + } + + internal static bool RequiresGenericArgumentDataFlow(FlowAnnotations flowAnnotations, TypeReference type) + { + if (flowAnnotations.HasGenericParameterAnnotation(type)) + { + return true; + } + + if (type is GenericInstanceType genericInstanceType) + { + foreach (var genericArgument in genericInstanceType.GenericArguments) + { + if (RequiresGenericArgumentDataFlow(flowAnnotations, genericArgument)) + { + return true; + } + } + } + + return false; } } } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs index e420eca03d4358..ae0d2de743cdd9 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -422,7 +422,9 @@ protected virtual void Scan(MethodIL methodIL, ref InterproceduralState interpro break; case Code.Ldftn: - TrackNestedFunctionReference((MethodReference)operation.Operand, ref interproceduralState); + MethodReference methodReference = (MethodReference)operation.Operand; + HandleMethodTokenAccess(methodIL, operation.Offset, methodReference); + TrackNestedFunctionReference(methodReference, ref interproceduralState); PushUnknown(currentStack); break; @@ -457,7 +459,7 @@ protected virtual void Scan(MethodIL methodIL, ref InterproceduralState interpro break; case Code.Ldtoken: - ScanLdtoken(operation, currentStack); + ScanLdtoken(methodIL, operation, currentStack); break; case Code.Ldind_I: @@ -510,16 +512,21 @@ protected virtual void Scan(MethodIL methodIL, ref InterproceduralState interpro case Code.Conv_R8: case Code.Ldind_Ref: case Code.Ldobj: - case Code.Mkrefany: case Code.Unbox: case Code.Unbox_Any: - case Code.Box: case Code.Neg: case Code.Not: PopUnknown(currentStack, 1, methodIL, operation.Offset); PushUnknown(currentStack); break; + case Code.Box: + case Code.Mkrefany: + HandleTypeTokenAccess(methodIL, operation.Offset, (TypeReference)operation.Operand); + PopUnknown(currentStack, 1, methodIL, operation.Offset); + PushUnknown(currentStack); + break; + case Code.Isinst: case Code.Castclass: // We can consider a NOP because the value doesn't change. @@ -537,7 +544,9 @@ protected virtual void Scan(MethodIL methodIL, ref InterproceduralState interpro case Code.Newarr: { StackSlot count = PopUnknown(currentStack, 1, methodIL, operation.Offset); - currentStack.Push(new StackSlot(ArrayValue.Create(count.Value, (TypeReference)operation.Operand))); + var arrayElementType = (TypeReference)operation.Operand; + HandleTypeTokenAccess(methodIL, operation.Offset, arrayElementType); + currentStack.Push(new StackSlot(ArrayValue.Create(count.Value, arrayElementType))); } break; @@ -828,7 +837,7 @@ private void ScanLdloc( currentStack.Push(newSlot); } - void ScanLdtoken(Instruction operation, Stack currentStack) + void ScanLdtoken(MethodIL methodIL, Instruction operation, Stack currentStack) { switch (operation.Operand) { @@ -846,25 +855,36 @@ void ScanLdtoken(Instruction operation, Stack currentStack) var nullableDam = new RuntimeTypeHandleForNullableValueWithDynamicallyAccessedMembers(new TypeProxy(resolvedDefinition, _context), new RuntimeTypeHandleForGenericParameterValue(genericParam)); currentStack.Push(new StackSlot(nullableDam)); - return; + break; case TypeReference underlyingTypeReference when ResolveToTypeDefinition(underlyingTypeReference) is TypeDefinition underlyingType: var nullableType = new RuntimeTypeHandleForNullableSystemTypeValue(new TypeProxy(resolvedDefinition, _context), new SystemTypeValue(new(underlyingType, _context))); currentStack.Push(new StackSlot(nullableType)); - return; + break; default: PushUnknown(currentStack); - return; + break; } } else { var typeHandle = new RuntimeTypeHandleValue(new TypeProxy(resolvedDefinition, _context)); currentStack.Push(new StackSlot(typeHandle)); - return; } + + HandleTypeTokenAccess(methodIL, operation.Offset, typeReference); + return; + case MethodReference methodReference when _context.TryResolve(methodReference) is MethodDefinition resolvedMethod: var method = new RuntimeMethodHandleValue(resolvedMethod); currentStack.Push(new StackSlot(method)); + + HandleMethodTokenAccess(methodIL, operation.Offset, methodReference); + return; + + case FieldReference fieldReference: + PushUnknown(currentStack); + + HandleFieldTokenAccess(methodIL, operation.Offset, fieldReference); return; default: PushUnknown(currentStack); @@ -1003,6 +1023,12 @@ protected virtual void HandleReturnValue(MethodIL method, MethodReturnValue this { } + protected abstract void HandleTypeTokenAccess(MethodIL methodIL, int offset, TypeReference accessedType); + + protected abstract void HandleMethodTokenAccess(MethodIL methodIL, int offset, MethodReference accessedMethod); + + protected abstract void HandleFieldTokenAccess(MethodIL methodIL, int offset, FieldReference accessedField); + private void ScanStfld( MethodIL methodIL, Instruction operation, @@ -1014,12 +1040,13 @@ private void ScanStfld( if (operation.OpCode.Code == Code.Stfld) PopUnknown(currentStack, 1, methodIL, operation.Offset); - FieldDefinition? field = _context.TryResolve((FieldReference)operation.Operand); - if (field != null) + var field = (FieldReference)operation.Operand; + FieldDefinition? fieldDefinition = _context.TryResolve(field); + if (fieldDefinition != null) { - if (CompilerGeneratedState.IsHoistedLocal(field)) + if (CompilerGeneratedState.IsHoistedLocal(fieldDefinition)) { - interproceduralState.SetHoistedLocal(new HoistedLocalKey(field), valueToStoreSlot.Value); + interproceduralState.SetHoistedLocal(new HoistedLocalKey(fieldDefinition), valueToStoreSlot.Value); return; } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs index 7605c109d926d7..6f6e634b2907c6 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs @@ -13,13 +13,13 @@ namespace Mono.Linker.Dataflow { public readonly struct ReflectionMarker { - readonly LinkContext _context; readonly MarkStep _markStep; readonly bool _enabled; + public LinkContext Context { get; } public ReflectionMarker(LinkContext context, MarkStep markStep, bool enabled) { - _context = context; + Context = context; _markStep = markStep; _enabled = enabled; } @@ -29,10 +29,10 @@ internal void MarkTypeForDynamicallyAccessedMembers(in MessageOrigin origin, Typ if (!_enabled) return; - if (type.ResolveToTypeDefinition(_context) is not TypeDefinition typeDefinition) + if (type.ResolveToTypeDefinition(Context) is not TypeDefinition typeDefinition) return; - foreach (var member in typeDefinition.GetDynamicallyAccessedMembers(_context, requiredMemberTypes, declaredOnly)) + foreach (var member in typeDefinition.GetDynamicallyAccessedMembers(Context, requiredMemberTypes, declaredOnly)) { switch (member) { @@ -62,7 +62,7 @@ internal void MarkTypeForDynamicallyAccessedMembers(in MessageOrigin origin, Typ // This method will probe the current context assembly and if that fails CoreLib for the specified type. Emulates behavior of Type.GetType. internal bool TryResolveTypeNameAndMark(string typeName, in DiagnosticContext diagnosticContext, bool needsAssemblyName, [NotNullWhen(true)] out TypeReference? type) { - if (!_context.TypeNameResolver.TryResolveTypeName(typeName, diagnosticContext, out type, out var typeResolutionRecords, needsAssemblyName)) + if (!Context.TypeNameResolver.TryResolveTypeName(typeName, diagnosticContext, out type, out var typeResolutionRecords, needsAssemblyName)) { type = default; return false; @@ -75,7 +75,7 @@ internal bool TryResolveTypeNameAndMark(string typeName, in DiagnosticContext di // Resolve a type from the specified assembly and mark it for reflection. internal bool TryResolveTypeNameAndMark(AssemblyDefinition assembly, string typeName, in DiagnosticContext diagnosticContext, [NotNullWhen(true)] out TypeReference? type) { - if (!_context.TypeNameResolver.TryResolveTypeName(assembly, typeName, out type, out var typeResolutionRecords)) + if (!Context.TypeNameResolver.TryResolveTypeName(assembly, typeName, out type, out var typeResolutionRecords)) { type = default; return false; @@ -101,7 +101,7 @@ void MarkType( _markStep.MarkTypeVisibleToReflection(typeReference, new DependencyInfo(DependencyKind.AccessedViaReflection, origin.Provider), origin); foreach (var typeResolutionRecord in typeResolutionRecords) { - _context.MarkingHelpers.MarkMatchingExportedType(typeResolutionRecord.ResolvedType, typeResolutionRecord.ReferringAssembly, new DependencyInfo(DependencyKind.DynamicallyAccessedMember, typeReference), origin); + Context.MarkingHelpers.MarkMatchingExportedType(typeResolutionRecord.ResolvedType, typeResolutionRecord.ReferringAssembly, new DependencyInfo(DependencyKind.DynamicallyAccessedMember, typeReference), origin); } } } @@ -111,7 +111,7 @@ internal void MarkType(in MessageOrigin origin, TypeReference typeRef, Dependenc if (!_enabled) return; - if (typeRef.ResolveToTypeDefinition(_context) is not TypeDefinition type) + if (typeRef.ResolveToTypeDefinition(Context) is not TypeDefinition type) return; _markStep.MarkTypeVisibleToReflection(type, new DependencyInfo(dependencyKind, origin.Provider), origin); @@ -122,7 +122,7 @@ internal void MarkMethod(in MessageOrigin origin, MethodReference methodRef, Dep if (!_enabled) return; - if (_context.TryResolve(methodRef) is not MethodDefinition method) + if (Context.TryResolve(methodRef) is not MethodDefinition method) return; _markStep.MarkMethodVisibleToReflection(method, new DependencyInfo(dependencyKind, origin.Provider), origin); @@ -165,7 +165,7 @@ internal void MarkConstructorsOnType(in MessageOrigin origin, TypeReference type if (!_enabled) return; - if (typeRef.ResolveToTypeDefinition(_context) is not TypeDefinition type) + if (typeRef.ResolveToTypeDefinition(Context) is not TypeDefinition type) return; foreach (var ctor in type.GetConstructorsOnType(filter, bindingFlags)) @@ -177,10 +177,10 @@ internal void MarkFieldsOnTypeHierarchy(in MessageOrigin origin, TypeReference t if (!_enabled) return; - if (typeRef.ResolveToTypeDefinition(_context) is not TypeDefinition type) + if (typeRef.ResolveToTypeDefinition(Context) is not TypeDefinition type) return; - foreach (var field in type.GetFieldsOnTypeHierarchy(_context, filter, bindingFlags)) + foreach (var field in type.GetFieldsOnTypeHierarchy(Context, filter, bindingFlags)) MarkField(origin, field); } @@ -189,10 +189,10 @@ internal void MarkPropertiesOnTypeHierarchy(in MessageOrigin origin, TypeReferen if (!_enabled) return; - if (typeRef.ResolveToTypeDefinition(_context) is not TypeDefinition type) + if (typeRef.ResolveToTypeDefinition(Context) is not TypeDefinition type) return; - foreach (var property in type.GetPropertiesOnTypeHierarchy(_context, filter, bindingFlags)) + foreach (var property in type.GetPropertiesOnTypeHierarchy(Context, filter, bindingFlags)) MarkProperty(origin, property); } @@ -201,10 +201,10 @@ internal void MarkEventsOnTypeHierarchy(in MessageOrigin origin, TypeReference t if (!_enabled) return; - if (typeRef.ResolveToTypeDefinition(_context) is not TypeDefinition type) + if (typeRef.ResolveToTypeDefinition(Context) is not TypeDefinition type) return; - foreach (var @event in type.GetEventsOnTypeHierarchy(_context, filter, bindingFlags)) + foreach (var @event in type.GetEventsOnTypeHierarchy(Context, filter, bindingFlags)) MarkEvent(origin, @event); } @@ -213,7 +213,7 @@ internal void MarkStaticConstructor(in MessageOrigin origin, TypeReference typeR if (!_enabled) return; - if (typeRef.ResolveToTypeDefinition(_context) is not TypeDefinition type) + if (typeRef.ResolveToTypeDefinition(Context) is not TypeDefinition type) return; _markStep.MarkStaticConstructorVisibleToReflection(type, new DependencyInfo(DependencyKind.AccessedViaReflection, origin.Provider), origin); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index ef2ac6fe837a00..a23514ca8d9619 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -29,9 +29,12 @@ public static bool RequiresReflectionMethodBodyScannerForCallSite(LinkContext co if (methodDefinition == null) return false; + var annotations = context.Annotations; + var flowAnnotations = annotations.FlowAnnotations; return Intrinsics.GetIntrinsicIdForMethod(methodDefinition) > IntrinsicId.RequiresReflectionBodyScanner_Sentinel || - context.Annotations.FlowAnnotations.RequiresDataFlowAnalysis(methodDefinition) || - context.Annotations.DoesMethodRequireUnreferencedCode(methodDefinition, out _) || + flowAnnotations.RequiresDataFlowAnalysis(methodDefinition) || + GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(flowAnnotations, calledMethod) || + annotations.DoesMethodRequireUnreferencedCode(methodDefinition, out _) || IsPInvokeDangerous(methodDefinition, context, out _); } @@ -47,7 +50,20 @@ public static bool RequiresReflectionMethodBodyScannerForAccess(LinkContext cont if (fieldDefinition == null) return false; - return context.Annotations.FlowAnnotations.RequiresDataFlowAnalysis(fieldDefinition); + var flowAnnotations = context.Annotations.FlowAnnotations; + return GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(flowAnnotations, field) || + flowAnnotations.RequiresDataFlowAnalysis(fieldDefinition); + } + + public static bool RequiresReflectionMethodBodyScannerForAccess(LinkContext context, TypeReference type) + { + TypeDefinition? typeDefinition = context.TryResolve(type); + if (typeDefinition == null) + return false; + + var annotations = context.Annotations; + return GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(annotations.FlowAnnotations, type) + || annotations.TryGetLinkerAttribute(typeDefinition, out _); } public ReflectionMethodBodyScanner(LinkContext context, MarkStep parent, MessageOrigin origin) @@ -91,7 +107,11 @@ protected override ValueWithDynamicallyAccessedMembers GetMethodParameterValue(P MethodParameterValue GetMethodParameterValue(ParameterProxy parameter, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) => _annotations.GetMethodParameterValue(parameter, dynamicallyAccessedMemberTypes); - protected override MultiValue GetFieldValue(FieldReference field) => _annotations.GetFieldValue(field); + protected override MultiValue GetFieldValue(FieldReference field) + { + ProcessGenericArgumentDataFlow(field); + return _annotations.GetFieldValue(field); + } protected override MethodReturnValue GetReturnValue(MethodIL methodIL) => _annotations.GetMethodReturnValue(methodIL.Method, isNewObj: false); @@ -113,6 +133,31 @@ protected override void HandleStoreParameter(MethodIL methodIL, MethodParameterV protected override void HandleReturnValue(MethodIL methodIL, MethodReturnValue returnValue, Instruction operation, MultiValue valueToStore) => HandleStoreValueWithDynamicallyAccessedMembers(returnValue, operation, valueToStore, null); + protected override void HandleTypeTokenAccess(MethodIL methodIL, int offset, TypeReference accessedType) + { + // Note that ldtoken alone is technically a reflection access to the type + // it doesn't lead to full reflection marking of the type + // since we implement full dataflow for type values and accesses to them. + _origin = _origin.WithInstructionOffset(offset); + + // Only check for generic instantiations. + ProcessGenericArgumentDataFlow(accessedType); + } + + protected override void HandleMethodTokenAccess(MethodIL methodIL, int offset, MethodReference accessedMethod) + { + _origin = _origin.WithInstructionOffset(offset); + + ProcessGenericArgumentDataFlow(accessedMethod); + } + + protected override void HandleFieldTokenAccess(MethodIL methodIL, int offset, FieldReference accessedField) + { + _origin = _origin.WithInstructionOffset(offset); + + ProcessGenericArgumentDataFlow(accessedField); + } + public override MultiValue HandleCall(MethodIL callingMethodIL, MethodReference calledMethod, Instruction operation, ValueNodeList methodParams) { var reflectionProcessed = _markStep.ProcessReflectionDependency(callingMethodIL.Body, operation); @@ -151,6 +196,8 @@ public override MultiValue HandleCall(MethodIL callingMethodIL, MethodReference _origin )); + ProcessGenericArgumentDataFlow(calledMethod); + var diagnosticContext = new DiagnosticContext(_origin, diagnosticsEnabled: false, _context); return HandleCall( operation, @@ -277,6 +324,47 @@ static bool IsComInterop(IMarshalInfoProvider marshalInfoProvider, TypeReference return false; } + private void ProcessGenericArgumentDataFlow(MethodReference method) + { + // We mostly need to validate static methods and generic methods + // Instance non-generic methods on reference types don't need validation + // because the creation of the instance is the place where the validation will happen. + if (_context.TryResolve(method) is not MethodDefinition methodDefinition) + return; + + if (!methodDefinition.IsStatic && !method.IsGenericInstance && !methodDefinition.IsConstructor && !methodDefinition.DeclaringType.IsValueType) + return; + + if (GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(_annotations, method)) + { + TrimAnalysisPatterns.Add(new TrimAnalysisGenericInstantiationAccessPattern(method, _origin)); + } + } + + private void ProcessGenericArgumentDataFlow(FieldReference field) + { + // We only need to validate static field accesses, instance field accesses don't need generic parameter validation + // because the create of the instance would do that instead. + if (_context.TryResolve(field) is not FieldDefinition fieldDefinition) + return; + + if (!fieldDefinition.IsStatic) + return; + + if (GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(_annotations, field)) + { + TrimAnalysisPatterns.Add(new TrimAnalysisGenericInstantiationAccessPattern(field, _origin)); + } + } + + private void ProcessGenericArgumentDataFlow(TypeReference type) + { + if (type.IsGenericInstance && _annotations.HasGenericParameterAnnotation(type)) + { + TrimAnalysisPatterns.Add(new TrimAnalysisGenericInstantiationAccessPattern(type, _origin)); + } + } + internal static bool IsPInvokeDangerous(MethodDefinition methodDefinition, LinkContext context, out bool comDangerousMethod) { // The method in ILLink only detects one condition - COM Dangerous, but it's structured like this diff --git a/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs index e38dbfc0caaa5e..cea129d153e45a 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs @@ -28,6 +28,10 @@ public partial bool TryResolveTypeNameAndMark(string typeName, bool needsAssembl { if (_reflectionMarker.TryResolveTypeNameAndMark(typeName, _diagnosticContext, needsAssemblyName, out TypeReference? foundType)) { + if (GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(_reflectionMarker.Context.Annotations.FlowAnnotations, foundType)) + { + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(_diagnosticContext, _reflectionMarker, _reflectionMarker.Context, foundType); + } type = new(foundType, _resolver); return true; } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisGenericInstantiationAccessPattern.cs b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisGenericInstantiationAccessPattern.cs new file mode 100644 index 00000000000000..61a57f3d9b3db0 --- /dev/null +++ b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisGenericInstantiationAccessPattern.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 ILLink.Shared.TrimAnalysis; +using Mono.Cecil; +using Mono.Linker.Steps; + +namespace Mono.Linker.Dataflow +{ + public readonly record struct TrimAnalysisGenericInstantiationAccessPattern + { + public readonly MemberReference MemberReference; + public readonly MessageOrigin Origin; + + internal TrimAnalysisGenericInstantiationAccessPattern(MemberReference memberReference, MessageOrigin origin) + { + MemberReference = memberReference; + Origin = origin; + } + + public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, MarkStep markStep, LinkContext context) + { + bool diagnosticsEnabled = !context.Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode(Origin.Provider, out _); + var diagnosticContext = new DiagnosticContext(Origin, diagnosticsEnabled, context); + + switch (MemberReference) + { + case TypeReference typeReference: + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in diagnosticContext, reflectionMarker, context, typeReference); + break; + + case MethodReference methodReference: + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in diagnosticContext, reflectionMarker, context, methodReference); + break; + + case FieldReference fieldReference: + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in diagnosticContext, reflectionMarker, context, fieldReference); + break; + } + } + } +} diff --git a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs index 3d0cdbe1064dce..4731c0b22967a0 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisPatternStore.cs @@ -3,8 +3,8 @@ using System.Collections.Generic; using ILLink.Shared.DataFlow; -using ILLink.Shared.TrimAnalysis; using Mono.Linker.Steps; +using Mono.Cecil; namespace Mono.Linker.Dataflow { @@ -12,6 +12,7 @@ public readonly struct TrimAnalysisPatternStore { readonly Dictionary<(MessageOrigin, int?), TrimAnalysisAssignmentPattern> AssignmentPatterns; readonly Dictionary MethodCallPatterns; + readonly Dictionary<(MessageOrigin, MemberReference), TrimAnalysisGenericInstantiationAccessPattern> GenericInstantiations; readonly ValueSetLattice Lattice; readonly LinkContext _context; @@ -19,6 +20,7 @@ public TrimAnalysisPatternStore(ValueSetLattice lattice, LinkContex { AssignmentPatterns = new Dictionary<(MessageOrigin, int?), TrimAnalysisAssignmentPattern>(); MethodCallPatterns = new Dictionary(); + GenericInstantiations = new Dictionary<(MessageOrigin, MemberReference), TrimAnalysisGenericInstantiationAccessPattern>(); Lattice = lattice; _context = context; } @@ -46,6 +48,14 @@ public void Add(TrimAnalysisMethodCallPattern pattern) MethodCallPatterns[pattern.Origin] = pattern.Merge(Lattice, existingPattern); } + public void Add(TrimAnalysisGenericInstantiationAccessPattern pattern) + { + GenericInstantiations.TryAdd((pattern.Origin, pattern.MemberReference), pattern); + + // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the member reference + // and there's only one way to "access" a generic instantiation. + } + public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, MarkStep markStep) { foreach (var pattern in AssignmentPatterns.Values) @@ -53,6 +63,9 @@ public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, MarkSte foreach (var pattern in MethodCallPatterns.Values) pattern.MarkAndProduceDiagnostics(reflectionMarker, markStep, _context); + + foreach (var pattern in GenericInstantiations.Values) + pattern.MarkAndProduceDiagnostics(reflectionMarker, markStep, _context); } } } diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 6eda4e165992f0..d9dcab78e104a5 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1995,6 +1995,16 @@ internal void MarkMethodVisibleToReflection(MethodReference method, in Dependenc { Annotations.MarkReflectionUsed(methodDefinition); Annotations.MarkIndirectlyCalledMethod(methodDefinition); + + // On a reflectable method, perform generic data flow for the return type and all the parameter types + // This is a compensation for the DI issue described in https://github.com/dotnet/runtime/issues/81358 + var methodOrigin = new MessageOrigin(methodDefinition); + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in methodOrigin, this, Context, methodDefinition.ReturnType); + + foreach (var parameter in methodDefinition.GetMetadataParameters()) + { + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in methodOrigin, this, Context, parameter.ParameterType); + } } } @@ -2015,6 +2025,13 @@ bool MarkMethodsVisibleToReflection(TypeDefinition type, in DependencyInfo reaso internal void MarkFieldVisibleToReflection(FieldReference field, in DependencyInfo reason, in MessageOrigin origin) { MarkField(field, reason, origin); + if (Context.Resolve(field) is FieldDefinition fieldDefinition) + { + // On a reflectable field, perform generic data flow for the field's type + // This is a compensation for the DI issue described in https://github.com/dotnet/runtime/issues/81358 + var fieldOrigin = new MessageOrigin(fieldDefinition); + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in fieldOrigin, this, Context, fieldDefinition.FieldType); + } } bool MarkFieldsVisibleToReflection(TypeDefinition type, in DependencyInfo reason, MessageOrigin origin, bool markBackingFieldsOnlyIfPropertyMarked = false) @@ -2141,6 +2158,7 @@ internal void MarkStaticConstructorVisibleToReflection(TypeDefinition type, in D handleMarkType(type); MarkType(type.BaseType, new DependencyInfo(DependencyKind.BaseType, type), typeOrigin); + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in typeOrigin, this, Context, type.BaseType); // The DynamicallyAccessedMembers hierarchy processing must be done after the base type was marked // (to avoid inconsistencies in the cache), but before anything else as work done below @@ -2913,17 +2931,6 @@ void MarkGenericArguments(IGenericInstance instance, MessageOrigin origin) var parameter = parameters?[i]; var argumentTypeDef = MarkType(argument, new DependencyInfo(DependencyKind.GenericArgumentType, instance), origin); - - if (parameter is not null && Annotations.FlowAnnotations.RequiresGenericArgumentDataFlowAnalysis(parameter)) - { - // The only two implementations of IGenericInstance both derive from MemberReference - Debug.Assert(instance is MemberReference); - - var instanceMemberOrigin = origin.Provider is not null ? origin : new MessageOrigin(((MemberReference)instance).Resolve()); - var scanner = new GenericArgumentDataFlow(Context, this, instanceMemberOrigin); - scanner.ProcessGenericArgumentDataFlow(parameter, argument); - } - if (argumentTypeDef == null) continue; @@ -3820,6 +3827,29 @@ bool CheckRequiresReflectionMethodBodyScanner(MethodIL methodIL) if (ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForCallSite(Context, (MethodReference)instruction.Operand)) return true; break; + + case OperandType.InlineType: + if (InstructionRequiresReflectionMethodBodyScannerForTypeAccess(instruction)) + return true; + break; + + case OperandType.InlineTok: + if (instruction.Operand is FieldReference) + { + if (InstructionRequiresReflectionMethodBodyScannerForFieldAccess(instruction)) + return true; + } + else if (instruction.Operand is TypeReference) + { + if (InstructionRequiresReflectionMethodBodyScannerForTypeAccess(instruction)) + return true; + } + else if (instruction.Operand is MethodReference methodReference) + { + if (ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForCallSite(Context, methodReference)) + return true; + } + break; } } return false; @@ -3868,7 +3898,7 @@ bool MarkAndCheckRequiresReflectionMethodBodyScanner(MethodIL methodIL, MessageO PostMarkMethodBody(methodIL.Body); - Debug.Assert(requiresReflectionMethodBodyScanner == CheckRequiresReflectionMethodBodyScanner(methodIL)); + Debug.Assert(requiresReflectionMethodBodyScanner == CheckRequiresReflectionMethodBodyScanner(methodIL), $"Inconsistent result for {nameof(requiresReflectionMethodBodyScanner)} for method {methodIL.Method.GetDisplayName()}"); return requiresReflectionMethodBodyScanner; } @@ -3903,15 +3933,28 @@ Code.Stfld or Code.Stsfld or // Field address loads (as those can be used to store values to annotated field and thus must be checked) Code.Ldflda or - Code.Ldsflda - => ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForAccess(Context, (FieldReference)instruction.Operand), - // For ref fields, ldfld loads an address which can be used to store values to annotated fields - Code.Ldfld or Code.Ldsfld when ((FieldReference)instruction.Operand).FieldType.IsByRefOrPointer() + Code.Ldsflda or + Code.Ldfld or + Code.Ldsfld => ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForAccess(Context, (FieldReference)instruction.Operand), // Other field operations are not interesting as they don't need to be checked _ => false }; + bool InstructionRequiresReflectionMethodBodyScannerForTypeAccess(Instruction instruction) + { + var ret = instruction.OpCode.Code switch + { + Code.Mkrefany or + Code.Box or + Code.Newarr or + Code.Ldtoken + => ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForAccess(Context, (TypeReference)instruction.Operand), + _ => false + }; + return ret; + } + protected virtual void MarkInstruction(Instruction instruction, MethodDefinition method, ref bool requiresReflectionMethodBodyScanner, ref MessageOrigin origin) { switch (instruction.OpCode.OperandType) @@ -3962,14 +4005,19 @@ protected virtual void MarkInstruction(Instruction instruction, MethodDefinition if (token is TypeReference typeReference) { + requiresReflectionMethodBodyScanner |= InstructionRequiresReflectionMethodBodyScannerForTypeAccess(instruction); MarkTypeVisibleToReflection(typeReference, reason, origin); } else if (token is MethodReference methodReference) { + requiresReflectionMethodBodyScanner |= + ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForCallSite(Context, methodReference); MarkMethodVisibleToReflection(methodReference, reason, origin); } else { + requiresReflectionMethodBodyScanner |= + InstructionRequiresReflectionMethodBodyScannerForFieldAccess(instruction); MarkFieldVisibleToReflection((FieldReference)token, reason, origin); } break; @@ -3977,6 +4025,7 @@ protected virtual void MarkInstruction(Instruction instruction, MethodDefinition case OperandType.InlineType: var operand = (TypeReference)instruction.Operand; + requiresReflectionMethodBodyScanner |= InstructionRequiresReflectionMethodBodyScannerForTypeAccess(instruction); switch (instruction.OpCode.Code) { case Code.Newarr: @@ -4029,6 +4078,7 @@ protected internal virtual void MarkInterfaceImplementation(InterfaceImplementat if (Annotations.IsMarked(iface)) return; Annotations.MarkProcessed(iface, reason ?? new DependencyInfo(DependencyKind.InterfaceImplementationOnType, origin.Provider)); + GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in origin, this, Context, iface.InterfaceType); // Blame the type that has the interfaceimpl, expecting the type itself to get marked for other reasons. MarkCustomAttributes(iface, new DependencyInfo(DependencyKind.CustomAttribute, iface), origin); diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index eb88652e62a252..c47d24181f00eb 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -142,6 +142,12 @@ public Task DataflowInLocalMethodGroupArgument() return RunTest(); } + [Fact] + public Task DependencyInjectionPattern() + { + return RunTest(); + } + [Fact] public Task DynamicDependencyDataflow() { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ApplyTypeAnnotations.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ApplyTypeAnnotations.cs index b9c18fa51968f7..0ac636e25d67a9 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ApplyTypeAnnotations.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ApplyTypeAnnotations.cs @@ -142,7 +142,7 @@ class FromStringConstantWithGeneric [Kept] class FromStringConstantWithGenericInnerInner { - [Kept(By = Tool.Trimmer)] + [Kept] public void Method() { } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs index 354c5d1ef9676f..f516a12ed016fb 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs @@ -25,8 +25,9 @@ public static void Main() // The compiler generated state will see the modified body, // and will not associate the local function with the user method. - // Generic argument warnings from the local function will not be suppressed - // by RUC on the user method. + // This is a repro for a bug where generic argument warnings from the local + // function was not suppressed by RUC on the user method. + // The bug has been fixed so this should produce no warnings class Inner { @@ -38,10 +39,8 @@ public static void WithLocalFunctionInner() LocalWithWarning(); } - [ExpectedWarning("IL2091", Tool.Trimmer, "https://github.com/dotnet/linker/issues/2937")] void LocalWithWarning() { - // Warning! RequiresAllOnT(); } } @@ -55,10 +54,8 @@ public static void WithLocalFunction() LocalWithWarning(); } - [ExpectedWarning("IL2091", Tool.Trimmer, "https://github.com/dotnet/linker/issues/2937")] void LocalWithWarning() { - // No warning RequiresAllOnT(); } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs index ec801d05682e3e..6bcf6892cc001c 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs @@ -173,8 +173,6 @@ private static void IteratorInsideClosure() #if INCLUDE_UNEXPECTED_LOWERING_WARNINGS [UnexpectedWarning("IL2091", "T1", "PublicMethods", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] [UnexpectedWarning("IL2091", "T1", "PublicMethods", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] - [UnexpectedWarning("IL2091", "T1", "PublicMethods", Tool.Trimmer, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] - [UnexpectedWarning("IL2091", "T1", "PublicMethods", Tool.Trimmer, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] #endif IEnumerable Outer<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T1>() { @@ -200,8 +198,6 @@ private static void IteratorInsideClosureMismatch() #if INCLUDE_UNEXPECTED_LOWERING_WARNINGS [UnexpectedWarning("IL2091", "T1", "PublicProperties", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] [UnexpectedWarning("IL2091", "T1", "PublicProperties", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] - [UnexpectedWarning("IL2091", "T1", "PublicProperties", Tool.Trimmer, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] - [UnexpectedWarning("IL2091", "T1", "PublicProperties", Tool.Trimmer, "https://github.com/dotnet/roslyn/issues/79333", CompilerGeneratedCode = true)] #endif IEnumerable Outer<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T1>() { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/DependencyInjectionPattern.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/DependencyInjectionPattern.cs new file mode 100644 index 00000000000000..d2e4dc87075404 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/DependencyInjectionPattern.cs @@ -0,0 +1,305 @@ +// 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 System.Diagnostics.CodeAnalysis; +using System.Diagnostics; +using System.Reflection; +using Mono.Linker.Tests.Cases.Expectations.Assertions; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [ExpectedNoWarnings] + public class DependencyInjectionPattern + { + public static int Main() + { + Services services = new(); + services.RegisterService(typeof(INameProvider<>), typeof(NameProviderService<>)); + services.RegisterService(typeof(IDataObjectPrinter), typeof(DataObjectPrinterService)); + + var printer = services.GetService(); + var actual = printer.GetNameLength(new DataObject() { Name = "0123456789" }); + Assert.Equal(10, actual); + + services.RegisterService(typeof(ICustomFactory<>), typeof(CustomFactoryImpl<>)); + + var customFactory = services.GetService>(); + var created = customFactory.Create(); + Assert.Equal(42, created.GetValue()); + + services.RegisterService(typeof(ICustomFactoryWithConstraint<>), typeof(CustomFactoryWithConstraintImpl<>)); + services.RegisterService(typeof(ICustomFactoryWithConstraintWrapper), typeof(CustomFactoryWithConstraintWrapperImpl)); + + var customFactoryWithConstraintWrapper = services.GetService(); + var createdWithConstraint = customFactoryWithConstraintWrapper.Create(); + Assert.Equal(42, createdWithConstraint.GetValue()); + + return 100; + } + + [Kept] + public class DataObject + { + [Kept] + public string Name { [Kept] get; [Kept] set; } + + [Kept] + public DataObject() { } + } + + // Simplistic implementation of DI which is comparable in behavior to our DI + [Kept] + class Services + { + [Kept] + private Dictionary _services = new Dictionary(); + + [Kept] + public void RegisterService(Type interfaceType, [KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))][DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType) + { + _services.Add(interfaceType, implementationType); + } + + [Kept] + public T GetService() + { + return (T)GetService(typeof(T)); + } + + [Kept] + public object GetService(Type interfaceType) + { + Type typeDef = interfaceType.IsGenericType ? interfaceType.GetGenericTypeDefinition() : interfaceType; + Type implementationType = GetImplementationType(typeDef); + + if (implementationType.IsGenericTypeDefinition) + { + for (int i = 0; i < implementationType.GetGenericArguments().Length; i++) + { + Type genericArgument = implementationType.GetGenericArguments()[i]; + Type genericParameter = interfaceType.GetGenericArguments()[i]; + + // Validate that DAM annotations match + if (!DamAnnotationsMatch(genericArgument, genericParameter)) + throw new InvalidOperationException(); + + if (genericParameter.IsValueType) + throw new InvalidOperationException(); + } + + implementationType = InstantiateServiceType(implementationType, interfaceType.GetGenericArguments()); + } + + ConstructorInfo constructor = implementationType.GetConstructors()[0]; // Simplification + if (constructor.GetParameters().Length > 0) + { + List instances = new(); + foreach (var parameter in constructor.GetParameters()) + { + instances.Add(GetService(parameter.ParameterType)); + } + + return Activator.CreateInstance(implementationType, instances.ToArray())!; + } + else + { + return Activator.CreateInstance(implementationType)!; + } + + [UnconditionalSuppressMessage("", "IL2068", Justification = "We only add types with the right annotation to the dictionary")] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + Type GetImplementationType(Type interfaceType) + { + if (!_services.TryGetValue(interfaceType, out Type? implementationType)) + throw new NotImplementedException(); + + return implementationType; + } + + [UnconditionalSuppressMessage("", "IL2055", Justification = "We validated that the type parameters match - THIS IS WRONG")] + [UnconditionalSuppressMessage("", "IL3050", Justification = "We validated there are no value types")] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + Type InstantiateServiceType([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type typeDef, Type[] typeParameters) + { + return typeDef.MakeGenericType(typeParameters); + } + } + + [Kept] + private bool DamAnnotationsMatch(Type argument, Type parameter) + { + // .... - not interesting for this test, it will be true in the cases we use in this test + return true; + } + + [Kept] + public Services() { } + } + + [Kept] + interface INameProvider<[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))][DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> + { + [Kept(By = Tool.Trimmer)] + [return: KeptAttributeAttribute(typeof(System.Runtime.CompilerServices.NullableAttribute))] + string? GetName(T instance); + } + + [Kept] + [KeptInterface(typeof(INameProvider<>))] + class NameProviderService<[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))][DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> + : INameProvider + { + [Kept] + [return: KeptAttributeAttribute(typeof(System.Runtime.CompilerServices.NullableAttribute))] + public string? GetName(T instance) + { + return (string?)typeof(T).GetProperty("Name")?.GetValue(instance); + } + + [Kept] + public NameProviderService() { } + } + + [Kept] + interface IDataObjectPrinter + { + [Kept(By = Tool.Trimmer)] + int GetNameLength(DataObject instance); + } + + [Kept] + [KeptInterface(typeof(IDataObjectPrinter))] + class DataObjectPrinterService : IDataObjectPrinter + { + // The data flow is not applied on the INameProvider here, or in the method parameter + // or in the call to the GetName below inside Print. + [Kept] + INameProvider _nameProvider; + + [Kept] + public DataObjectPrinterService(INameProvider nameProvider) + { + _nameProvider = nameProvider; + } + + [Kept] + public int GetNameLength(DataObject instance) + { + // This throws because DataObject.Name is not preserved + string? name = _nameProvider.GetName(instance); + return name == null ? 0 : name.Length; + } + } + + [Kept] + interface ICustomFactory<[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))][DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T> + { + [Kept(By = Tool.Trimmer)] + T Create(); + } + + [Kept] + [KeptInterface(typeof(ICustomFactory<>))] + class CustomFactoryImpl<[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))][DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T> : ICustomFactory + { + [Kept] + public T Create() + { + return Activator.CreateInstance(); + } + + [Kept] + public CustomFactoryImpl() { } + } + + [Kept] + class FactoryCreated + { + [Kept] + int _value; + + [Kept] + public FactoryCreated() + { + _value = 42; + } + + [Kept] + public int GetValue() => _value; + } + + [Kept] + interface ICustomFactoryWithConstraintWrapper + { + [Kept(By = Tool.Trimmer)] + FactoryWithConstraintCreated Create(); + } + + [Kept] + [KeptInterface(typeof(ICustomFactoryWithConstraintWrapper))] + class CustomFactoryWithConstraintWrapperImpl : ICustomFactoryWithConstraintWrapper + { + [Kept] + private FactoryWithConstraintCreated _value; + + [Kept] + public CustomFactoryWithConstraintWrapperImpl(ICustomFactoryWithConstraint factory) + { + _value = factory.Create(); + } + + [Kept] + public FactoryWithConstraintCreated Create() => _value; + } + + [Kept] + interface ICustomFactoryWithConstraint<[KeptGenericParamAttributes(GenericParameterAttributes.DefaultConstructorConstraint)] T> where T : new() + { + [Kept(By = Tool.Trimmer)] + T Create(); + } + + [Kept] + [KeptInterface(typeof(ICustomFactoryWithConstraint<>))] + class CustomFactoryWithConstraintImpl<[KeptGenericParamAttributes(GenericParameterAttributes.DefaultConstructorConstraint)] T> : ICustomFactoryWithConstraint where T : new() + { + [Kept] + public T Create() + { + return Activator.CreateInstance(); + } + + [Kept] + public CustomFactoryWithConstraintImpl() { } + } + + [Kept] + class FactoryWithConstraintCreated + { + [Kept] + int _value; + + [Kept] + public FactoryWithConstraintCreated() + { + _value = 42; + } + + [Kept] + public int GetValue() => _value; + } + + [Kept] + static class Assert + { + [Kept] + public static void Equal(int expected, int actual) + { + if (expected != actual) + throw new Exception($"{expected} != {actual}"); + } + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs index e18798ba61d412..ce27dd4a4965af 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs @@ -8,13 +8,9 @@ namespace Mono.Linker.Tests.Cases.DataFlow { - // NativeAOT/analyzer differences in behavior compared to ILLink: // - // See the description on top of GenericParameterWarningLocation for the expected differences in behavior - // for NativeAOT and the analyzer. - // The tests affected by this are marked with "NativeAOT_StorageSpaceType" + // See the description on top of GenericParameterWarningLocation for the expected behavior. // - [SkipKeptItemsValidation] [ExpectedNoWarnings] public class GenericParameterDataFlow @@ -413,7 +409,6 @@ class TypeGenericRequirementsOnMembers<[DynamicallyAccessedMembers(DynamicallyAc { public TypeRequiresPublicFields PublicFieldsField; - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType public TypeRequiresPublicMethods PublicMethodsField; public TypeRequiresPublicFields PublicFieldsProperty @@ -424,23 +419,17 @@ public TypeRequiresPublicFields PublicFieldsProperty public TypeRequiresPublicMethods PublicMethodsProperty { - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType get => null; - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType set { } } - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "", CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType public TypeRequiresPublicMethods PublicMethodsImplicitGetter => null; public void PublicFieldsMethodParameter(TypeRequiresPublicFields param) { } - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType public void PublicMethodsMethodParameter(TypeRequiresPublicMethods param) { } public TypeRequiresPublicFields PublicFieldsMethodReturnValue() { return null; } - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType public TypeRequiresPublicMethods PublicMethodsMethodReturnValue() { return null; } public void PublicFieldsMethodLocalVariable() @@ -448,8 +437,6 @@ public void PublicFieldsMethodLocalVariable() TypeRequiresPublicFields t = null; } - // The analyzer matches NativeAot behavior for local variables - it doesn't warn on generic types of local variables. - [ExpectedWarning("IL2091", nameof(TypeRequiresPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType public void PublicMethodsMethodLocalVariable() { TypeRequiresPublicMethods t = null; @@ -838,10 +825,7 @@ static void TestNoWarningsInRUCMethod() [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields")] // StaticMethodRequiresPublicMethods [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields")] // StaticMethodRequiresPublicMethods [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields")] // RUCTypeRequiresPublicFields ctor - [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields", Tool.Trimmer, "")] // RUCTypeRequiresPublicFields local, // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields", Tool.Trimmer, "")] // InstanceMethod, // NativeAOT_StorageSpaceType [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields")] // InstanceMethodRequiresPublicMethods - [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields", Tool.Trimmer, "")] // VirtualMethod, // NativeAOT_StorageSpaceType [ExpectedWarning("IL2091", "RUCTypeRequiresPublicFields")] // VirtualMethodRequiresPublicMethods static void TestNoWarningsInRUCType() { @@ -857,18 +841,12 @@ static void TestNoWarningsInRUCType() } [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] static void TestInstanceMethodOnValueType() { default(RequiresParameterlessCtor).Do(); } [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] - [ExpectedWarning("IL2091", "IRequireParameterlessCtor", Tool.Trimmer, "")] - [ExpectedWarning("IL2091", "IRequireParameterlessCtor", Tool.Trimmer, "")] static void TestValueTypeBox() { if (default(RequiresParameterlessCtor) is IRequireParameterlessCtor i) @@ -878,8 +856,6 @@ static void TestValueTypeBox() } [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] static void TestMkrefAny() { RequiresParameterlessCtor val = default; @@ -888,7 +864,6 @@ static void TestMkrefAny() } [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2091", "RequiresParameterlessCtor", Tool.Trimmer, "")] static void TestInArray() { var arr = new RequiresParameterlessCtor[1]; @@ -1026,10 +1001,6 @@ static void TestGenericParameterFlowsToDelegateMethodDeclaringType() } [ExpectedWarning("IL2091", nameof(DelegateMethodTypeRequiresFields))] - // NativeAOT_StorageSpaceType: illink warns about the type of 'instance' local variable - [ExpectedWarning("IL2091", nameof(DelegateMethodTypeRequiresFields), Tool.Trimmer, "")] - // NativeAOT_StorageSpaceType: illink warns about the declaring type of 'InstanceMethod' on ldftn - [ExpectedWarning("IL2091", nameof(DelegateMethodTypeRequiresFields), Tool.Trimmer, "")] static void TestGenericParameterFlowsToDelegateMethodDeclaringTypeInstance() { var instance = new DelegateMethodTypeRequiresFields(); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterWarningLocation.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterWarningLocation.cs index 7515952791d474..020d9252030b85 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterWarningLocation.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterWarningLocation.cs @@ -9,7 +9,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow { - // NativeAOT/analyzer differences in behavior compared to ILLink: // // Validation of generic parameters only matters if the instantiation can be used to run code with the substituted type. // So for generic methods the validation has to happen basically always (since any access to the method can lead to the code @@ -17,16 +16,11 @@ namespace Mono.Linker.Tests.Cases.DataFlow // For generic types though the situation is different. Code on the type can only run if the type is instantiated (new) // or if static members are accessed on it (method calls, or fields accesses both can lead to static .cctor execution). // Others usages of the type cannot themselves lead to code execution in the type, and thus don't need to be validated. - // Currently linker validates every time there's a type occurrence in the code. - // NativeAOT and the analyzer on the other hand only validate the cases which can lead to code execution (this is partially + // The tools only validate the cases which can lead to code execution (this is partially // because the compiler doesn't care about the type in other situations really). // So for example local variables of a given type, or method parameters of that type alone will not cause code execution - // inside that type and thus won't be validated by NativeAOT compiler or the analyzer. + // inside that type and thus won't be validated. // - // Below this explanation/fact is referred to as "NativeAOT_StorageSpaceType" - // Storage space - declaring a storage space as having a specific type doesn't in itself do anything with that type as per - // the above description. - [SkipKeptItemsValidation] [ExpectedNoWarnings] public class GenericParameterWarningLocation @@ -176,7 +170,7 @@ public static void Test() } } - //.NativeAOT: Method parameter types are not interesting until something creates an instance of them + // Method parameter types are not interesting until something creates an instance of them // so there's no need to validate generic arguments. See comment at the top of the file for more details. class MethodParametersAndReturn { @@ -190,11 +184,8 @@ interface IWithTwo< static void MethodWithSpecificType(TypeWithPublicMethods one, IWithTwo two) { } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MethodWithOneMismatch(TypeWithPublicMethods one) { } - [ExpectedWarning("IL2091", nameof(IWithTwo), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", nameof(TypeWithPublicMethods), Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MethodWithTwoMismatches< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -205,11 +196,8 @@ static void MethodWithTwoMismatches< static TypeWithPublicMethods MethodWithMatchingReturn<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods>() => null; - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static TypeWithPublicMethods MethodWithOneMismatchReturn() => null; - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static IWithTwo MethodWithTwoMismatchesInReturn< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -217,7 +205,6 @@ static IWithTwo MethodWithTwoMismatchesInReturn< class ConstructorWithOneMatchAndOneMismatch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TMethods> { - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType public ConstructorWithOneMatchAndOneMismatch(IWithTwo two) { } } @@ -238,8 +225,6 @@ public static void Test() // NativeAot warns for members accessed by reflection as a workaround for an incorrect suppression // in DI: https://github.com/dotnet/runtime/issues/81358 - // ILLink doesn't differentiate between reflection and non-reflection access, so it warns in both cases. - // Analyzer doesn't implement the workaround, so it warns in neither case. class MethodParametersAndReturnAccessedViaReflection { class TypeWithPublicMethods<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -291,7 +276,7 @@ public static void Test() } } - //.NativeAOT: Field types are not interesting until something creates an instance of them + // Field types are not interesting until something creates an instance of them // so there's no need to validate generic arguments. See comment at the top of the file for more details. class FieldDefinition { @@ -325,10 +310,8 @@ public static void Test() class MultipleReferencesToTheSameType<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown> { - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static TypeWithPublicMethods _field1; static TypeWithPublicMethods _field2; - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static TypeWithPublicMethods _field3; public static void Test() @@ -343,8 +326,6 @@ class TwoMismatchesInOne< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields> { - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static IWithTwo _field; public static void Test() @@ -364,8 +345,6 @@ public static void Test() // NativeAot warns for members accessed by reflection as a workaround for an incorrect suppression // in DI: https://github.com/dotnet/runtime/issues/81358 - // ILLink doesn't differentiate between reflection and non-reflection access, so it warns in both cases. - // Analyzer doesn't implement the workaround, so it warns in neither case. class FieldDefinitionViaReflection { class TypeWithPublicMethods<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -388,10 +367,10 @@ class OneMatchingAnnotation<[DynamicallyAccessedMembers(DynamicallyAccessedMembe class MultipleReferencesToTheSameType<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown> { - [ExpectedWarning("IL2091", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2091", Tool.NativeAot | Tool.Trimmer, "")] static TypeWithPublicMethods _field1; static TypeWithPublicMethods _field2; - [ExpectedWarning("IL2091", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2091", Tool.NativeAot | Tool.Trimmer, "")] static TypeWithPublicMethods _field3; } @@ -399,8 +378,8 @@ class TwoMismatchesInOne< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields> { - [ExpectedWarning("IL2091", Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2091", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2091", Tool.NativeAot | Tool.Trimmer, "")] + [ExpectedWarning("IL2091", Tool.NativeAot | Tool.Trimmer, "")] static IWithTwo _field; } @@ -413,7 +392,7 @@ public static void Test() } } - //.NativeAOT: Property types are not interesting until something creates an instance of them + // Property types are not interesting until something creates an instance of them // so there's no need to validate generic arguments. See comment at the top of the file for more details. // In case of trimmer/aot it's even less important because properties don't exist in IL really // and thus no code can manipulate them directly - only through reflection. @@ -449,14 +428,10 @@ public static void Test() class MultipleReferencesToTheSameType<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown> { - // The warning is generated on the backing field - [ExpectedWarning("IL2091", Tool.Trimmer, "", CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType static TypeWithPublicMethods Property1 { - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType get; - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType set; } @@ -466,14 +441,9 @@ static TypeWithPublicMethods Property2 set; } - // The warning is generated on the backing field - [ExpectedWarning("IL2091", Tool.Trimmer, "", CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType static TypeWithPublicMethods Property3 { - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType get; - - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType set; } @@ -489,16 +459,10 @@ class TwoMismatchesInOne< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields> { - // The warnings are generated on the backing field - [ExpectedWarning("IL2091", Tool.Trimmer, "", CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "", CompilerGeneratedCode = true)] // NativeAOT_StorageSpaceType static IWithTwo Property { // Getter is trimmed and doesn't produce any warning get; - - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType set; } @@ -558,7 +522,6 @@ static void MethodWithTwo< static MethodBody GetInstance() => null; - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // return type // NativeAOT_StorageSpaceType static TypeWithPublicMethods GetInstanceForTypeWithPublicMethods() => null; class TypeOf @@ -698,8 +661,6 @@ static void TwoMismatchesInOneStatement< IWithTwo.Method(); } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // local variable // NativeAOT_StorageSpaceType static void InstanceMethodMismatch() { TypeWithPublicMethods instance = GetInstanceForTypeWithPublicMethods(); @@ -750,8 +711,6 @@ static void TwoMismatchesInOneStatement< _ = IWithTwo.Field; } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // access to the field // NativeAOT_StorageSpaceType static void InstanceFieldMismatch() { TypeWithPublicMethods instance = GetInstanceForTypeWithPublicMethods(); @@ -768,7 +727,7 @@ public static void Test() } } - //.NativeAOT: Local variable types are not interesting until something creates an instance of them + // Local variable types are not interesting until something creates an instance of them // so there's no need to validate generic arguments. See comment at the top of the file for more details. class LocalVariable { @@ -783,8 +742,6 @@ static void SpecificType() TypeWithPublicMethods t = null; } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MultipleReferencesToTheSameType< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown>() @@ -794,8 +751,6 @@ static void MultipleReferencesToTheSameType< TypeWithPublicMethods t3 = null; // Warn } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void TwoMismatchesInOneStatement< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -1143,7 +1098,7 @@ public static void Test() } } - //.NativeAOT: Checking an instance for its type is not interesting until something creates an instance of that type + // Checking an instance for its type is not interesting until something creates an instance of that type // so there's no need to validate generic arguments. See comment at the top of the file for more details. class IsInstance { @@ -1159,8 +1114,6 @@ static void SpecificType() bool a = _value is TypeWithPublicMethods; } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MultipleReferencesToTheSameMethod< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown>() @@ -1170,8 +1123,6 @@ static void MultipleReferencesToTheSameMethod< bool a3 = _value is TypeWithPublicMethods; // Warn } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void TwoMismatchesInOneStatement< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -1204,8 +1155,6 @@ static void SpecificType() object a = _value as TypeWithPublicMethods; } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MultipleReferencesToTheSameMethod< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown>() @@ -1215,8 +1164,6 @@ static void MultipleReferencesToTheSameMethod< object a3 = _value as TypeWithPublicMethods; // Warn } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void TwoMismatchesInOneStatement< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -1234,7 +1181,7 @@ public static void Test() } } - //.NativeAOT: Exception types are effectively very similar to local variable or method parameters. + // Exception types are effectively very similar to local variable or method parameters. // and are not interesting until something creates an instance of them // so there's no need to validate generic arguments. See comment at the top of the file for more details. class ExceptionCatch @@ -1261,8 +1208,6 @@ static void SpecificType() } } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MultipleReferencesToTheSameType< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown>() @@ -1292,8 +1237,6 @@ static void MultipleReferencesToTheSameType< } } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void TwoMismatchesInOneStatement< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> @@ -1342,8 +1285,6 @@ static void SpecificType() } } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void MultipleReferencesToTheSameType< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods, TUnknown>() @@ -1373,8 +1314,6 @@ static void MultipleReferencesToTheSameType< } } - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType - [ExpectedWarning("IL2091", Tool.Trimmer, "")] // NativeAOT_StorageSpaceType static void TwoMismatchesInOneStatement< [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TPublicFields, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TPublicMethods> diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/UnresolvedMembers.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/UnresolvedMembers.cs index df982c1d58e7bb..5a6e995a483bd4 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/UnresolvedMembers.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/UnresolvedMembers.cs @@ -45,7 +45,6 @@ static void MethodWithUnresolvedGenericArgument< { } [Kept] - [ExpectedWarning("IL2066", "TypeWithUnresolvedGenericArgument", Tool.Trimmer | Tool.Analyzer, "")] // Local variable type [ExpectedWarning("IL2066", "TypeWithUnresolvedGenericArgument", Tool.Trimmer | Tool.Analyzer, "")] // Called method declaring type [ExpectedWarning("IL2066", nameof(MethodWithUnresolvedGenericArgument), Tool.Trimmer | Tool.Analyzer, "")] static void UnresolvedGenericArgument() diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresExcludeStatics.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresExcludeStatics.cs index 524a0c0f3a2866..5ab52c1dee9a2b 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresExcludeStatics.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresExcludeStatics.cs @@ -205,10 +205,8 @@ class Requires<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Public { } - [UnexpectedWarning("IL2091", Tool.Trimmer, "https://github.com/dotnet/runtime/issues/113249")] static Requires StaticField; - [UnexpectedWarning("IL2091", Tool.Trimmer, "https://github.com/dotnet/runtime/issues/113249")] Requires InstanceField; [ExpectedWarning("IL2091", "PublicFields", "Requires")] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs index 886743417bb549..95e922b2e26ace 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs @@ -1319,7 +1319,6 @@ public class ClassWithRequires { public static RequiresAll field; - [UnexpectedWarning("IL2091", Tool.Trimmer, "https://github.com/dotnet/runtime/issues/108523")] public RequiresAll instanceField; [RequiresOnCtor] @@ -1402,7 +1401,6 @@ public class GenericAnnotatedWithWarningWithRequires<[DynamicallyAccessedMembers [ExpectedWarning("IL2026", "--ClassWithWarningWithRequires--")] [ExpectedWarning("IL2026", "--ClassWithWarningOnGenericArgumentConstructorWithRequires--")] [ExpectedWarning("IL2026", "--GenericAnnotatedWithWarningWithRequires--")] - [ExpectedWarning("IL2091", Tool.Trimmer, "")] public static void Test(ClassWithRequires inst = null) { var f = ClassWithRequires.field;