From 2b64adce192b445605f3a3d02c0ff020d2127d86 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 22 Apr 2024 11:15:05 -0700 Subject: [PATCH 1/6] Share logic for handling return values --- .../Compiler/Dataflow/HandleCallAction.cs | 559 ++++++++++++++++ .../Dataflow/ReflectionMethodBodyScanner.cs | 624 +----------------- .../TrimAnalysis/HandleCallAction.cs | 106 ++- .../TrimAnalysis/TrimAnalysisVisitor.cs | 86 +-- .../TrimAnalysis/HandleCallAction.cs | 79 ++- .../Linker.Dataflow/HandleCallAction.cs | 149 ++++- .../ReflectionMethodBodyScanner.cs | 203 +----- .../Reflection/ObjectGetType.cs | 6 +- 8 files changed, 878 insertions(+), 934 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index f09a76221ef37b..cd87469d48685f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -2,12 +2,18 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Reflection; using ILCompiler; using ILCompiler.Dataflow; +using ILCompiler.DependencyAnalysis; +using ILCompiler.DependencyAnalysisFramework; using ILLink.Shared.TypeSystemProxy; using Internal.TypeSystem; +using Internal.IL; +using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore.DependencyList; +using MultiValue = ILLink.Shared.DataFlow.ValueSet; using WellKnownType = ILLink.Shared.TypeSystemProxy.WellKnownType; #nullable enable @@ -19,17 +25,20 @@ internal partial struct HandleCallAction #pragma warning disable CA1822 // Mark members as static - the other partial implementations might need to be instance methods private readonly ReflectionMarker _reflectionMarker; + private ILOpcode _operation; private readonly MethodDesc _callingMethod; private readonly string _reason; public HandleCallAction( FlowAnnotations annotations, + ILOpcode operation, ReflectionMarker reflectionMarker, in DiagnosticContext diagnosticContext, MethodDesc callingMethod, string reason) { _reflectionMarker = reflectionMarker; + _operation = operation; _diagnosticContext = diagnosticContext; _callingMethod = callingMethod; _annotations = annotations; @@ -37,6 +46,527 @@ public HandleCallAction( _requireDynamicallyAccessedMembersAction = new(reflectionMarker, diagnosticContext, reason); } + private partial bool TryHandleIntrinsic ( + MethodProxy calledMethod, + MultiValue instanceValue, + IReadOnlyList argumentValues, + IntrinsicId intrinsicId, + out MultiValue? methodReturnValue) + { + MultiValue? maybeMethodReturnValue = methodReturnValue = null; + + switch (intrinsicId) + { + case IntrinsicId.Type_MakeGenericType: + { + bool triggersWarning = false; + + if (!instanceValue.IsEmpty() && !argumentValues[0].IsEmpty()) + { + foreach (var value in instanceValue.AsEnumerable()) + { + if (value is SystemTypeValue typeValue) + { + TypeDesc typeInstantiated = typeValue.RepresentedType.Type; + if (!typeInstantiated.IsGenericDefinition) + { + // Nothing to do, will fail at runtime + } + else if (TryGetMakeGenericInstantiation(_callingMethod, argumentValues[0], out Instantiation inst, out bool isExact)) + { + if (inst.Length == typeInstantiated.Instantiation.Length) + { + typeInstantiated = ((MetadataType)typeInstantiated).MakeInstantiatedType(inst); + + if (isExact) + { + _reflectionMarker.MarkType(_diagnosticContext.Origin, typeInstantiated, "MakeGenericType"); + } + else + { + _reflectionMarker.RuntimeDeterminedDependencies.Add(new MakeGenericTypeSite(typeInstantiated)); + } + } + } + else + { + triggersWarning = true; + } + + } + else if (value == NullValue.Instance) + { + // Nothing to do + } + else + { + // We don't know what type the `MakeGenericType` was called on + triggersWarning = true; + } + } + } + + if (triggersWarning) + { + ReflectionMethodBodyScanner.CheckAndReportRequires(_diagnosticContext, calledMethod.Method, DiagnosticUtilities.RequiresDynamicCodeAttribute); + } + + // This intrinsic is relevant to both trimming and AOT - call into trimming logic as well. + HandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + } + break; + + case IntrinsicId.MethodInfo_MakeGenericMethod: + { + bool triggersWarning = false; + + if (!instanceValue.IsEmpty()) + { + foreach (var methodValue in instanceValue.AsEnumerable()) + { + if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) + { + MethodDesc methodInstantiated = methodBaseValue.RepresentedMethod.Method; + if (!methodInstantiated.IsGenericMethodDefinition) + { + // Nothing to do, will fail at runtime + } + else if (!methodInstantiated.OwningType.IsGenericDefinition + && TryGetMakeGenericInstantiation(_callingMethod, argumentValues[0], out Instantiation inst, out bool isExact)) + { + if (inst.Length == methodInstantiated.Instantiation.Length) + { + methodInstantiated = methodInstantiated.MakeInstantiatedMethod(inst); + + if (isExact) + { + _reflectionMarker.MarkMethod(_diagnosticContext.Origin, methodInstantiated, "MakeGenericMethod"); + } + else + { + _reflectionMarker.RuntimeDeterminedDependencies.Add(new MakeGenericMethodSite(methodInstantiated)); + } + } + } + else + { + // If the owning type is a generic definition, we can't help much. + triggersWarning = true; + } + } + else if (methodValue == NullValue.Instance) + { + // Nothing to do + } + else + { + // We don't know what method the `MakeGenericMethod` was called on + triggersWarning = true; + } + } + } + + if (triggersWarning) + { + ReflectionMethodBodyScanner.CheckAndReportRequires(_diagnosticContext, calledMethod.Method, DiagnosticUtilities.RequiresDynamicCodeAttribute); + } + + // This intrinsic is relevant to both trimming and AOT - call into trimming logic as well. + HandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + } + break; + + case IntrinsicId.None: + { + if (ReflectionMethodBodyScanner.IsPInvokeDangerous(calledMethod.Method, out bool comDangerousMethod, out bool aotUnsafeDelegate)) + { + if (aotUnsafeDelegate) + { + _diagnosticContext.AddDiagnostic(DiagnosticId.CorrectnessOfAbstractDelegatesCannotBeGuaranteed, calledMethod.GetDisplayName()); + } + + if (comDangerousMethod) + { + _diagnosticContext.AddDiagnostic(DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethod.GetDisplayName()); + } + } + + ReflectionMethodBodyScanner.CheckAndReportAllRequires(_diagnosticContext, calledMethod.Method); + + HandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + } + break; + + case IntrinsicId.TypeDelegator_Ctor: + { + // This is an identity function for analysis purposes + if (_operation == ILOpcode.newobj) + AddReturnValue(argumentValues[0]); + } + break; + + case IntrinsicId.Array_Empty: + { + AddReturnValue(ArrayValue.Create(0, calledMethod.Method.Instantiation[0])); + } + break; + + // + // System.Array + // + // CreateInstance (Type, Int32) + // + case IntrinsicId.Array_CreateInstance: + { + // We could try to analyze if the type is known, but for now making sure this works for canonical arrays is enough. + TypeDesc canonArrayType = _reflectionMarker.Factory.TypeSystemContext.CanonType.MakeArrayType(); + _reflectionMarker.MarkType(_diagnosticContext.Origin, canonArrayType, "Array.CreateInstance was called"); + goto case IntrinsicId.None; + } + + // + // System.Enum + // + // static GetValues (Type) + // + case IntrinsicId.Enum_GetValues: + { + // Enum.GetValues returns System.Array, but it's the array of the enum type under the hood + // and people depend on this undocumented detail (could have returned enum of the underlying + // type instead). + // + // At least until we have shared enum code, this needs extra handling to get it right. + foreach (var value in argumentValues[0].AsEnumerable ()) + { + if (value is SystemTypeValue systemTypeValue + && !systemTypeValue.RepresentedType.Type.IsGenericDefinition + && !systemTypeValue.RepresentedType.Type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) + { + if (systemTypeValue.RepresentedType.Type.IsEnum) + { + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedType(systemTypeValue.RepresentedType.Type.MakeArrayType()), "Enum.GetValues"); + } + } + else + ReflectionMethodBodyScanner.CheckAndReportRequires(_diagnosticContext, calledMethod.Method, DiagnosticUtilities.RequiresDynamicCodeAttribute); + } + } + break; + + // + // System.Runtime.InteropServices.Marshal + // + // static SizeOf (Type) + // static PtrToStructure (IntPtr, Type) + // static DestroyStructure (IntPtr, Type) + // static OffsetOf (Type, string) + // + case IntrinsicId.Marshal_SizeOf: + case IntrinsicId.Marshal_PtrToStructure: + case IntrinsicId.Marshal_DestroyStructure: + case IntrinsicId.Marshal_OffsetOf: + { + int paramIndex = intrinsicId == IntrinsicId.Marshal_SizeOf + || intrinsicId == IntrinsicId.Marshal_OffsetOf + ? 0 : 1; + + // We need the data to do struct marshalling. + foreach (var value in argumentValues[paramIndex].AsEnumerable ()) + { + if (value is SystemTypeValue systemTypeValue + && !systemTypeValue.RepresentedType.Type.IsGenericDefinition + && !systemTypeValue.RepresentedType.Type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) + { + if (systemTypeValue.RepresentedType.Type.IsDefType) + { + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.StructMarshallingData((DefType)systemTypeValue.RepresentedType.Type), "Marshal API"); + if (intrinsicId == IntrinsicId.Marshal_PtrToStructure + && systemTypeValue.RepresentedType.Type.GetParameterlessConstructor() is MethodDesc ctorMethod + && !_reflectionMarker.Factory.MetadataManager.IsReflectionBlocked(ctorMethod)) + { + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedMethod(ctorMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Marshal API"); + } + } + } + else + ReflectionMethodBodyScanner.CheckAndReportRequires(_diagnosticContext, calledMethod.Method, DiagnosticUtilities.RequiresDynamicCodeAttribute); + } + } + break; + + // + // System.Runtime.InteropServices.Marshal + // + // static GetDelegateForFunctionPointer (IntPtr, Type) + // + case IntrinsicId.Marshal_GetDelegateForFunctionPointer: + { + // We need the data to do delegate marshalling. + foreach (var value in argumentValues[1].AsEnumerable ()) + { + if (value is SystemTypeValue systemTypeValue + && !systemTypeValue.RepresentedType.Type.IsGenericDefinition + && !systemTypeValue.RepresentedType.Type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) + { + if (systemTypeValue.RepresentedType.Type.IsDelegate) + { + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.DelegateMarshallingData((DefType)systemTypeValue.RepresentedType.Type), "Marshal API"); + } + } + else + ReflectionMethodBodyScanner.CheckAndReportRequires(_diagnosticContext, calledMethod.Method, DiagnosticUtilities.RequiresDynamicCodeAttribute); + } + } + break; + + // + // System.Delegate + // + // get_Method () + // + // System.Reflection.RuntimeReflectionExtensions + // + // GetMethodInfo (System.Delegate) + // + case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: + case IntrinsicId.Delegate_get_Method: + { + // Find the parameter: first is an instance method, second is an extension method. + MultiValue param = intrinsicId == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo + ? argumentValues[0] : instanceValue; + + // If this is Delegate.Method accessed from RuntimeReflectionExtensions.GetMethodInfo, ignore + // because we handle the callsites to that one here as well. + if (Intrinsics.GetIntrinsicIdForMethod(_callingMethod) == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo) + break; + + if (param.IsEmpty()) + { + // The static value is unknown and the below `foreach` won't execute + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedDelegate(null), "Delegate.Method access on unknown delegate type"); + } + + foreach (var valueNode in param.AsEnumerable()) + { + TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; + if (staticType is null || !staticType.IsDelegate) + { + // The static type is unknown or something useless like Delegate or MulticastDelegate. + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedDelegate(null), "Delegate.Method access on unknown delegate type"); + } + else + { + if (staticType.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedDelegate(staticType.GetTypeDefinition()), "Delegate.Method access (on inexact type)"); + else + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ReflectedDelegate(staticType.ConvertToCanonForm(CanonicalFormKind.Specific)), "Delegate.Method access"); + } + } + } + break; + + // + // System.Object + // + // GetType() + // + case IntrinsicId.Object_GetType: + { + foreach (var valueNode in instanceValue.AsEnumerable ()) + { + // Note that valueNode can be statically typed in IL as some generic argument type. + // For example: + // void Method(T instance) { instance.GetType().... } + // Currently this case will end up with null StaticType - since there's no typedef for the generic argument type. + // But it could be that T is annotated with for example PublicMethods: + // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } + // In this case it's in theory possible to handle it, by treating the T basically as a base class + // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking + // has to happen on the callsite, which doesn't know that GetType() will be used...). + // For now we're intentionally ignoring this case - it will produce a warning. + // The counter example is: + // Method(new Derived); + // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which + // currently it won't do. + + TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; + if (staticType is null || (!staticType.IsDefType && !staticType.IsArray)) + { + // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" + AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod)); + } + else if (staticType.IsSealed() || staticType.IsTypeOf("System", "Delegate")) + { + // We can treat this one the same as if it was a typeof() expression + + // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods + // on delegates anyway so reflection on something this approximation would miss is actually safe. + + // We ignore the fact that the type can be annotated (see below for handling of annotated types) + // This means the annotations (if any) won't be applied - instead we rely on the exact knowledge + // of the type. So for example even if the type is annotated with PublicMethods + // but the code calls GetProperties on it - it will work - mark properties, don't mark methods + // since we ignored the fact that it's annotated. + // This can be seen a little bit as a violation of the annotation, but we already have similar cases + // where a parameter is annotated and if something in the method sets a specific known type to it + // we will also make it just work, even if the annotation doesn't match the usage. + AddReturnValue(new SystemTypeValue(staticType)); + } + else + { + Debug.Assert(staticType is MetadataType || staticType.IsArray); + MetadataType closestMetadataType = staticType is MetadataType mdType ? + mdType : (MetadataType)_reflectionMarker.Factory.TypeSystemContext.GetWellKnownType(Internal.TypeSystem.WellKnownType.Array); + + var annotation = _reflectionMarker.Annotations.GetTypeAnnotation(staticType); + + if (annotation != default) + { + _reflectionMarker.Dependencies.Add(_reflectionMarker.Factory.ObjectGetTypeFlowDependencies(closestMetadataType), "GetType called on this type"); + } + + // Return a value which is "unknown type" with annotation. For now we'll use the return value node + // for the method, which means we're loosing the information about which staticType this + // started with. For now we don't need it, but we can add it later on. + AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, annotation)); + } + } + } + break; + + // + // string System.Reflection.Assembly.Location getter + // string System.Reflection.AssemblyName.CodeBase getter + // string System.Reflection.AssemblyName.EscapedCodeBase getter + // + case IntrinsicId.Assembly_get_Location: + case IntrinsicId.AssemblyName_get_CodeBase: + case IntrinsicId.AssemblyName_get_EscapedCodeBase: + _diagnosticContext.AddDiagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile, calledMethod.GetDisplayName()); + break; + + // + // string System.Reflection.Assembly.GetFile(string) + // string System.Reflection.Assembly.GetFiles() + // string System.Reflection.Assembly.GetFiles(bool) + // + case IntrinsicId.Assembly_GetFile: + case IntrinsicId.Assembly_GetFiles: + _diagnosticContext.AddDiagnostic(DiagnosticId.AvoidAssemblyGetFilesInSingleFile, calledMethod.GetDisplayName()); + break; + + default: + return false; + } + + methodReturnValue = maybeMethodReturnValue; + return true; + + void AddReturnValue(MultiValue value) + { + maybeMethodReturnValue = (maybeMethodReturnValue is null) ? value : MultiValueLattice.Meet((MultiValue)maybeMethodReturnValue, value); + } + } + + private static bool TryGetMakeGenericInstantiation( + MethodDesc contextMethod, + in MultiValue genericParametersArray, + out Instantiation inst, + out bool isExact) + { + // We support calling MakeGeneric APIs with a very concrete instantiation array. + // Only the form of `new Type[] { typeof(Foo), typeof(T), typeof(Foo) }` is supported. + + inst = default; + isExact = true; + Debug.Assert(contextMethod.GetTypicalMethodDefinition() == contextMethod); + + var typesValue = genericParametersArray.AsSingleValue(); + if (typesValue is NullValue) + { + // This will fail at runtime but no warning needed + inst = Instantiation.Empty; + return true; + } + + // Is this an array we model? + if (typesValue is not ArrayValue array) + { + return false; + } + + int? size = array.Size.AsConstInt(); + if (size == null) + { + return false; + } + + TypeDesc[]? sigInst = null; + TypeDesc[]? defInst = null; + + ArrayBuilder result = default; + for (int i = 0; i < size.Value; i++) + { + // Go over each element of the array. If the value is unknown, bail. + if (!array.TryGetValueByIndex(i, out MultiValue value)) + { + return false; + } + + var singleValue = value.AsSingleValue(); + + TypeDesc? type = singleValue switch + { + SystemTypeValue systemType => systemType.RepresentedType.Type, + GenericParameterValue genericParamType => genericParamType.GenericParameter.GenericParameter, + NullableSystemTypeValue nullableSystemType => nullableSystemType.NullableType.Type, + _ => null + }; + + if (type is null) + { + return false; + } + + // type is now some type. + // Because dataflow analysis oddly operates on method bodies instantiated over + // generic parameters (as opposed to instantiated over signature variables) + // We need to swap generic parameters (T, U,...) for signature variables (!0, !!1,...). + // We need to do this for both generic parameters of the owning type, and generic + // parameters of the owning method. + if (type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) + { + if (sigInst == null) + { + TypeDesc contextType = contextMethod.OwningType; + sigInst = new TypeDesc[contextType.Instantiation.Length + contextMethod.Instantiation.Length]; + defInst = new TypeDesc[contextType.Instantiation.Length + contextMethod.Instantiation.Length]; + TypeSystemContext context = type.Context; + for (int j = 0; j < contextType.Instantiation.Length; j++) + { + sigInst[j] = context.GetSignatureVariable(j, method: false); + defInst[j] = contextType.Instantiation[j]; + } + for (int j = 0; j < contextMethod.Instantiation.Length; j++) + { + sigInst[j + contextType.Instantiation.Length] = context.GetSignatureVariable(j, method: true); + defInst[j + contextType.Instantiation.Length] = contextMethod.Instantiation[j]; + } + } + + isExact = false; + + // defInst is [T, U, V], sigInst is `[!0, !!0, !!1]`. + type = type.ReplaceTypesInConstructionOfType(defInst, sigInst); + } + + result.Add(type); + } + + inst = new Instantiation(result.ToArray()); + return true; + } + private partial bool MethodIsTypeConstructor(MethodProxy method) { if (!method.Method.IsConstructor) @@ -124,5 +654,34 @@ private partial bool MarkAssociatedProperty(MethodProxy method) } private partial string GetContainingSymbolDisplayName() => _callingMethod.GetDisplayName(); + + private sealed class MakeGenericMethodSite : INodeWithRuntimeDeterminedDependencies + { + private readonly MethodDesc _method; + + public MakeGenericMethodSite(MethodDesc method) => _method = method; + + public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) + { + var list = new DependencyList(); + RootingHelpers.TryGetDependenciesForReflectedMethod(ref list, factory, _method.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericMethod"); + return list; + } + } + + private sealed class MakeGenericTypeSite : INodeWithRuntimeDeterminedDependencies + { + private readonly TypeDesc _type; + + public MakeGenericTypeSite(TypeDesc type) => _type = type; + + public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) + { + var list = new DependencyList(); + RootingHelpers.TryGetDependenciesForReflectedType(ref list, factory, _type.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericType"); + return list; + } + } + } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index ad84c511393d69..0e5e077ad984a9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -293,7 +293,7 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet ProcessGenericArgumentDataFlow(calledMethod); var diagnosticContext = new DiagnosticContext(_origin, diagnosticsEnabled: false, _logger); - return HandleCall( + HandleCall( callingMethodBody, calledMethod, operation, @@ -302,9 +302,10 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet diagnosticContext, _reflectionMarker, out methodReturnValue); + return true; } - public static bool HandleCall( + public static void HandleCall( MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, @@ -322,594 +323,9 @@ public static bool HandleCall( RequiresReflectionMethodBodyScannerForCallSite(reflectionMarker.Annotations, calledMethod) || annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None); - MultiValue? maybeMethodReturnValue = null; - - var handleCallAction = new HandleCallAction(reflectionMarker.Annotations, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod.GetDisplayName()); - + var handleCallAction = new HandleCallAction(reflectionMarker.Annotations, operation, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod.GetDisplayName()); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod(calledMethod); - switch (intrinsicId) - { - case IntrinsicId.IntrospectionExtensions_GetTypeInfo: - case IntrinsicId.TypeInfo_AsType: - case IntrinsicId.Type_get_UnderlyingSystemType: - case IntrinsicId.Type_GetTypeFromHandle: - case IntrinsicId.Type_get_TypeHandle: - case IntrinsicId.Type_GetInterface: - case IntrinsicId.Type_get_AssemblyQualifiedName: - case IntrinsicId.RuntimeHelpers_RunClassConstructor: - case IntrinsicId.Type_GetConstructors__BindingFlags: - case IntrinsicId.Type_GetMethods__BindingFlags: - case IntrinsicId.Type_GetFields__BindingFlags: - case IntrinsicId.Type_GetProperties__BindingFlags: - case IntrinsicId.Type_GetEvents__BindingFlags: - case IntrinsicId.Type_GetNestedTypes__BindingFlags: - case IntrinsicId.Type_GetMembers__BindingFlags: - case IntrinsicId.Type_GetField: - case IntrinsicId.Type_GetProperty: - case IntrinsicId.Type_GetEvent: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeEvent: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeField: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeMethod: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeProperty: - case IntrinsicId.Type_GetMember: - case IntrinsicId.Type_GetMethod: - case IntrinsicId.Type_GetNestedType: - case IntrinsicId.Nullable_GetUnderlyingType: - case IntrinsicId.Expression_Property: - case IntrinsicId.Expression_Field: - case IntrinsicId.Type_get_BaseType: - case IntrinsicId.Type_GetConstructor: - case IntrinsicId.MethodBase_GetMethodFromHandle: - case IntrinsicId.MethodBase_get_MethodHandle: - case IntrinsicId.Expression_Call: - case IntrinsicId.Expression_New: - case IntrinsicId.Type_GetType: - case IntrinsicId.Activator_CreateInstance__Type: - case IntrinsicId.Activator_CreateInstance__AssemblyName_TypeName: - case IntrinsicId.Activator_CreateInstanceFrom: - case IntrinsicId.AppDomain_CreateInstance: - case IntrinsicId.AppDomain_CreateInstanceAndUnwrap: - case IntrinsicId.AppDomain_CreateInstanceFrom: - case IntrinsicId.AppDomain_CreateInstanceFromAndUnwrap: - case IntrinsicId.Assembly_CreateInstance: - { - return handleCallAction.Invoke(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); - } - - case IntrinsicId.Type_MakeGenericType: - { - bool triggersWarning = false; - - if (!instanceValue.IsEmpty() && !argumentValues[0].IsEmpty()) - { - foreach (var value in instanceValue.AsEnumerable()) - { - if (value is SystemTypeValue typeValue) - { - TypeDesc typeInstantiated = typeValue.RepresentedType.Type; - if (!typeInstantiated.IsGenericDefinition) - { - // Nothing to do, will fail at runtime - } - else if (TryGetMakeGenericInstantiation(callingMethodDefinition, argumentValues[0], out Instantiation inst, out bool isExact)) - { - if (inst.Length == typeInstantiated.Instantiation.Length) - { - typeInstantiated = ((MetadataType)typeInstantiated).MakeInstantiatedType(inst); - - if (isExact) - { - reflectionMarker.MarkType(diagnosticContext.Origin, typeInstantiated, "MakeGenericType"); - } - else - { - reflectionMarker.RuntimeDeterminedDependencies.Add(new MakeGenericTypeSite(typeInstantiated)); - } - } - } - else - { - triggersWarning = true; - } - - } - else if (value == NullValue.Instance) - { - // Nothing to do - } - else - { - // We don't know what type the `MakeGenericType` was called on - triggersWarning = true; - } - } - } - - if (triggersWarning) - { - CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute); - } - - // This intrinsic is relevant to both trimming and AOT - call into trimming logic as well. - return handleCallAction.Invoke(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); - } - - case IntrinsicId.MethodInfo_MakeGenericMethod: - { - bool triggersWarning = false; - - if (!instanceValue.IsEmpty()) - { - foreach (var methodValue in instanceValue.AsEnumerable()) - { - if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) - { - MethodDesc methodInstantiated = methodBaseValue.RepresentedMethod.Method; - if (!methodInstantiated.IsGenericMethodDefinition) - { - // Nothing to do, will fail at runtime - } - else if (!methodInstantiated.OwningType.IsGenericDefinition - && TryGetMakeGenericInstantiation(callingMethodDefinition, argumentValues[0], out Instantiation inst, out bool isExact)) - { - if (inst.Length == methodInstantiated.Instantiation.Length) - { - methodInstantiated = methodInstantiated.MakeInstantiatedMethod(inst); - - if (isExact) - { - reflectionMarker.MarkMethod(diagnosticContext.Origin, methodInstantiated, "MakeGenericMethod"); - } - else - { - reflectionMarker.RuntimeDeterminedDependencies.Add(new MakeGenericMethodSite(methodInstantiated)); - } - } - } - else - { - // If the owning type is a generic definition, we can't help much. - triggersWarning = true; - } - } - else if (methodValue == NullValue.Instance) - { - // Nothing to do - } - else - { - // We don't know what method the `MakeGenericMethod` was called on - triggersWarning = true; - } - } - } - - if (triggersWarning) - { - CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute); - } - - // This intrinsic is relevant to both trimming and AOT - call into trimming logic as well. - return handleCallAction.Invoke(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); - } - - case IntrinsicId.None: - { - if (IsPInvokeDangerous(calledMethod, out bool comDangerousMethod, out bool aotUnsafeDelegate)) - { - if (aotUnsafeDelegate) - { - diagnosticContext.AddDiagnostic(DiagnosticId.CorrectnessOfAbstractDelegatesCannotBeGuaranteed, calledMethod.GetDisplayName()); - } - - if (comDangerousMethod) - { - diagnosticContext.AddDiagnostic(DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethod.GetDisplayName()); - } - } - - CheckAndReportAllRequires(diagnosticContext, calledMethod); - - return handleCallAction.Invoke(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); - } - - case IntrinsicId.TypeDelegator_Ctor: - { - // This is an identity function for analysis purposes - if (operation == ILOpcode.newobj) - AddReturnValue(argumentValues[0]); - } - break; - - case IntrinsicId.Array_Empty: - { - AddReturnValue(ArrayValue.Create(0, calledMethod.Instantiation[0])); - } - break; - - // - // System.Array - // - // CreateInstance (Type, Int32) - // - case IntrinsicId.Array_CreateInstance: - { - // We could try to analyze if the type is known, but for now making sure this works for canonical arrays is enough. - TypeDesc canonArrayType = reflectionMarker.Factory.TypeSystemContext.CanonType.MakeArrayType(); - reflectionMarker.MarkType(diagnosticContext.Origin, canonArrayType, "Array.CreateInstance was called"); - goto case IntrinsicId.None; - } - - // - // System.Enum - // - // static GetValues (Type) - // - case IntrinsicId.Enum_GetValues: - { - // Enum.GetValues returns System.Array, but it's the array of the enum type under the hood - // and people depend on this undocumented detail (could have returned enum of the underlying - // type instead). - // - // At least until we have shared enum code, this needs extra handling to get it right. - foreach (var value in argumentValues[0].AsEnumerable ()) - { - if (value is SystemTypeValue systemTypeValue - && !systemTypeValue.RepresentedType.Type.IsGenericDefinition - && !systemTypeValue.RepresentedType.Type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) - { - if (systemTypeValue.RepresentedType.Type.IsEnum) - { - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedType(systemTypeValue.RepresentedType.Type.MakeArrayType()), "Enum.GetValues"); - } - } - else - CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute); - } - } - break; - - // - // System.Runtime.InteropServices.Marshal - // - // static SizeOf (Type) - // static PtrToStructure (IntPtr, Type) - // static DestroyStructure (IntPtr, Type) - // static OffsetOf (Type, string) - // - case IntrinsicId.Marshal_SizeOf: - case IntrinsicId.Marshal_PtrToStructure: - case IntrinsicId.Marshal_DestroyStructure: - case IntrinsicId.Marshal_OffsetOf: - { - int paramIndex = intrinsicId == IntrinsicId.Marshal_SizeOf - || intrinsicId == IntrinsicId.Marshal_OffsetOf - ? 0 : 1; - - // We need the data to do struct marshalling. - foreach (var value in argumentValues[paramIndex].AsEnumerable ()) - { - if (value is SystemTypeValue systemTypeValue - && !systemTypeValue.RepresentedType.Type.IsGenericDefinition - && !systemTypeValue.RepresentedType.Type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) - { - if (systemTypeValue.RepresentedType.Type.IsDefType) - { - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.StructMarshallingData((DefType)systemTypeValue.RepresentedType.Type), "Marshal API"); - if (intrinsicId == IntrinsicId.Marshal_PtrToStructure - && systemTypeValue.RepresentedType.Type.GetParameterlessConstructor() is MethodDesc ctorMethod - && !reflectionMarker.Factory.MetadataManager.IsReflectionBlocked(ctorMethod)) - { - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedMethod(ctorMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Marshal API"); - } - } - } - else - CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute); - } - } - break; - - // - // System.Runtime.InteropServices.Marshal - // - // static GetDelegateForFunctionPointer (IntPtr, Type) - // - case IntrinsicId.Marshal_GetDelegateForFunctionPointer: - { - // We need the data to do delegate marshalling. - foreach (var value in argumentValues[1].AsEnumerable ()) - { - if (value is SystemTypeValue systemTypeValue - && !systemTypeValue.RepresentedType.Type.IsGenericDefinition - && !systemTypeValue.RepresentedType.Type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) - { - if (systemTypeValue.RepresentedType.Type.IsDelegate) - { - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.DelegateMarshallingData((DefType)systemTypeValue.RepresentedType.Type), "Marshal API"); - } - } - else - CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute); - } - } - break; - - // - // System.Delegate - // - // get_Method () - // - // System.Reflection.RuntimeReflectionExtensions - // - // GetMethodInfo (System.Delegate) - // - case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: - case IntrinsicId.Delegate_get_Method: - { - // Find the parameter: first is an instance method, second is an extension method. - MultiValue param = intrinsicId == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo - ? argumentValues[0] : instanceValue; - - // If this is Delegate.Method accessed from RuntimeReflectionExtensions.GetMethodInfo, ignore - // because we handle the callsites to that one here as well. - if (Intrinsics.GetIntrinsicIdForMethod(callingMethodDefinition) == IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo) - break; - - if (param.IsEmpty()) - { - // The static value is unknown and the below `foreach` won't execute - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(null), "Delegate.Method access on unknown delegate type"); - } - - foreach (var valueNode in param.AsEnumerable()) - { - TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType is null || !staticType.IsDelegate) - { - // The static type is unknown or something useless like Delegate or MulticastDelegate. - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(null), "Delegate.Method access on unknown delegate type"); - } - else - { - if (staticType.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(staticType.GetTypeDefinition()), "Delegate.Method access (on inexact type)"); - else - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedDelegate(staticType.ConvertToCanonForm(CanonicalFormKind.Specific)), "Delegate.Method access"); - } - } - } - break; - - // - // System.Object - // - // GetType() - // - case IntrinsicId.Object_GetType: - { - foreach (var valueNode in instanceValue.AsEnumerable ()) - { - // Note that valueNode can be statically typed in IL as some generic argument type. - // For example: - // void Method(T instance) { instance.GetType().... } - // Currently this case will end up with null StaticType - since there's no typedef for the generic argument type. - // But it could be that T is annotated with for example PublicMethods: - // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } - // In this case it's in theory possible to handle it, by treating the T basically as a base class - // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking - // has to happen on the callsite, which doesn't know that GetType() will be used...). - // For now we're intentionally ignoring this case - it will produce a warning. - // The counter example is: - // Method(new Derived); - // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which - // currently it won't do. - - TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType is null || (!staticType.IsDefType && !staticType.IsArray)) - { - // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" - AddReturnValue(reflectionMarker.Annotations.GetMethodReturnValue(calledMethod)); - } - else if (staticType.IsSealed() || staticType.IsTypeOf("System", "Delegate")) - { - // We can treat this one the same as if it was a typeof() expression - - // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods - // on delegates anyway so reflection on something this approximation would miss is actually safe. - - // We ignore the fact that the type can be annotated (see below for handling of annotated types) - // This means the annotations (if any) won't be applied - instead we rely on the exact knowledge - // of the type. So for example even if the type is annotated with PublicMethods - // but the code calls GetProperties on it - it will work - mark properties, don't mark methods - // since we ignored the fact that it's annotated. - // This can be seen a little bit as a violation of the annotation, but we already have similar cases - // where a parameter is annotated and if something in the method sets a specific known type to it - // we will also make it just work, even if the annotation doesn't match the usage. - AddReturnValue(new SystemTypeValue(staticType)); - } - else - { - Debug.Assert(staticType is MetadataType || staticType.IsArray); - MetadataType closestMetadataType = staticType is MetadataType mdType ? - mdType : (MetadataType)reflectionMarker.Factory.TypeSystemContext.GetWellKnownType(Internal.TypeSystem.WellKnownType.Array); - - var annotation = reflectionMarker.Annotations.GetTypeAnnotation(staticType); - - if (annotation != default) - { - reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ObjectGetTypeFlowDependencies(closestMetadataType), "GetType called on this type"); - } - - // Return a value which is "unknown type" with annotation. For now we'll use the return value node - // for the method, which means we're loosing the information about which staticType this - // started with. For now we don't need it, but we can add it later on. - AddReturnValue(reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, annotation)); - } - } - } - break; - - // - // string System.Reflection.Assembly.Location getter - // string System.Reflection.AssemblyName.CodeBase getter - // string System.Reflection.AssemblyName.EscapedCodeBase getter - // - case IntrinsicId.Assembly_get_Location: - case IntrinsicId.AssemblyName_get_CodeBase: - case IntrinsicId.AssemblyName_get_EscapedCodeBase: - diagnosticContext.AddDiagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile, calledMethod.GetDisplayName()); - break; - - // - // string System.Reflection.Assembly.GetFile(string) - // string System.Reflection.Assembly.GetFiles() - // string System.Reflection.Assembly.GetFiles(bool) - // - case IntrinsicId.Assembly_GetFile: - case IntrinsicId.Assembly_GetFiles: - diagnosticContext.AddDiagnostic(DiagnosticId.AvoidAssemblyGetFilesInSingleFile, calledMethod.GetDisplayName()); - break; - - default: - throw new NotImplementedException("Unhandled intrinsic"); - } - - // If we get here, we handled this as an intrinsic. As a convenience, if the code above - // didn't set the return value (and the method has a return value), we will set it to be an - // unknown value with the return type of the method. - bool returnsVoid = calledMethod.Signature.ReturnType.IsVoid; - methodReturnValue = maybeMethodReturnValue ?? (returnsVoid ? - MultiValueLattice.Top : - annotatedMethodReturnValue); - - // Validate that the return value has the correct annotations as per the method return value annotations - if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) - { - foreach (var uniqueValue in methodReturnValue.AsEnumerable ()) - { - if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) - { - if (!methodReturnValueWithMemberTypes.DynamicallyAccessedMemberTypes.HasFlag(annotatedMethodReturnValue.DynamicallyAccessedMemberTypes)) - throw new InvalidOperationException($"Internal trimming error: processing of call from {callingMethodDefinition.GetDisplayName()} to {calledMethod.GetDisplayName()} returned value which is not correctly annotated with the expected dynamic member access kinds."); - } - else if (uniqueValue is SystemTypeValue) - { - // SystemTypeValue can fulfill any requirement, so it's always valid - // The requirements will be applied at the point where it's consumed (passed as a method parameter, set as field value, returned from the method) - } - else - { - throw new InvalidOperationException($"Internal trimming error: processing of call from {callingMethodDefinition.GetDisplayName()} to {calledMethod.GetDisplayName()} returned value which is not correctly annotated with the expected dynamic member access kinds."); - } - } - } - - return true; - - void AddReturnValue(MultiValue value) - { - maybeMethodReturnValue = (maybeMethodReturnValue is null) ? value : MultiValueLattice.Meet((MultiValue)maybeMethodReturnValue, value); - } - } - - private static bool TryGetMakeGenericInstantiation( - MethodDesc contextMethod, - in MultiValue genericParametersArray, - out Instantiation inst, - out bool isExact) - { - // We support calling MakeGeneric APIs with a very concrete instantiation array. - // Only the form of `new Type[] { typeof(Foo), typeof(T), typeof(Foo) }` is supported. - - inst = default; - isExact = true; - Debug.Assert(contextMethod.GetTypicalMethodDefinition() == contextMethod); - - var typesValue = genericParametersArray.AsSingleValue(); - if (typesValue is NullValue) - { - // This will fail at runtime but no warning needed - inst = Instantiation.Empty; - return true; - } - - // Is this an array we model? - if (typesValue is not ArrayValue array) - { - return false; - } - - int? size = array.Size.AsConstInt(); - if (size == null) - { - return false; - } - - TypeDesc[]? sigInst = null; - TypeDesc[]? defInst = null; - - ArrayBuilder result = default; - for (int i = 0; i < size.Value; i++) - { - // Go over each element of the array. If the value is unknown, bail. - if (!array.TryGetValueByIndex(i, out MultiValue value)) - { - return false; - } - - var singleValue = value.AsSingleValue(); - - TypeDesc? type = singleValue switch - { - SystemTypeValue systemType => systemType.RepresentedType.Type, - GenericParameterValue genericParamType => genericParamType.GenericParameter.GenericParameter, - NullableSystemTypeValue nullableSystemType => nullableSystemType.NullableType.Type, - _ => null - }; - - if (type is null) - { - return false; - } - - // type is now some type. - // Because dataflow analysis oddly operates on method bodies instantiated over - // generic parameters (as opposed to instantiated over signature variables) - // We need to swap generic parameters (T, U,...) for signature variables (!0, !!1,...). - // We need to do this for both generic parameters of the owning type, and generic - // parameters of the owning method. - if (type.ContainsSignatureVariables(treatGenericParameterLikeSignatureVariable: true)) - { - if (sigInst == null) - { - TypeDesc contextType = contextMethod.OwningType; - sigInst = new TypeDesc[contextType.Instantiation.Length + contextMethod.Instantiation.Length]; - defInst = new TypeDesc[contextType.Instantiation.Length + contextMethod.Instantiation.Length]; - TypeSystemContext context = type.Context; - for (int j = 0; j < contextType.Instantiation.Length; j++) - { - sigInst[j] = context.GetSignatureVariable(j, method: false); - defInst[j] = contextType.Instantiation[j]; - } - for (int j = 0; j < contextMethod.Instantiation.Length; j++) - { - sigInst[j + contextType.Instantiation.Length] = context.GetSignatureVariable(j, method: true); - defInst[j + contextType.Instantiation.Length] = contextMethod.Instantiation[j]; - } - } - - isExact = false; - - // defInst is [T, U, V], sigInst is `[!0, !!0, !!1]`. - type = type.ReplaceTypesInConstructionOfType(defInst, sigInst); - } - - result.Add(type); - } - - inst = new Instantiation(result.ToArray()); - return true; + handleCallAction.Invoke (calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); } private static bool IsAotUnsafeDelegate(TypeDesc parameterType) @@ -1042,7 +458,7 @@ private void ProcessGenericArgumentDataFlow(TypeDesc type) } } - private static bool IsPInvokeDangerous(MethodDesc calledMethod, out bool comDangerousMethod, out bool aotUnsafeDelegate) + internal static bool IsPInvokeDangerous(MethodDesc calledMethod, out bool comDangerousMethod, out bool aotUnsafeDelegate) { if (!calledMethod.IsPInvoke) { @@ -1072,33 +488,5 @@ private static bool IsPInvokeDangerous(MethodDesc calledMethod, out bool comDang return aotUnsafeDelegate || comDangerousMethod; } - - private sealed class MakeGenericMethodSite : INodeWithRuntimeDeterminedDependencies - { - private readonly MethodDesc _method; - - public MakeGenericMethodSite(MethodDesc method) => _method = method; - - public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) - { - var list = new DependencyList(); - RootingHelpers.TryGetDependenciesForReflectedMethod(ref list, factory, _method.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericMethod"); - return list; - } - } - - private sealed class MakeGenericTypeSite : INodeWithRuntimeDeterminedDependencies - { - private readonly TypeDesc _type; - - public MakeGenericTypeSite(TypeDesc type) => _type = type; - - public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) - { - var list = new DependencyList(); - RootingHelpers.TryGetDependenciesForReflectedType(ref list, factory, _type.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericType"); - return list; - } - } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs index 96c4a07a92a79a..41bab03cbc2877 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs @@ -7,8 +7,11 @@ using ILLink.RoslynAnalyzer; using ILLink.RoslynAnalyzer.DataFlow; using ILLink.RoslynAnalyzer.TrimAnalysis; +using ILLink.Shared.DataFlow; using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; +using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.Shared.TrimAnalysis { @@ -20,8 +23,13 @@ internal partial struct HandleCallAction readonly ISymbol _owningSymbol; readonly IOperation _operation; readonly ReflectionAccessAnalyzer _reflectionAccessAnalyzer; + ValueSetLattice _multiValueLattice; - public HandleCallAction (in DiagnosticContext diagnosticContext, ISymbol owningSymbol, IOperation operation) + public HandleCallAction ( + in DiagnosticContext diagnosticContext, + ISymbol owningSymbol, + IOperation operation, + ValueSetLattice multiValueLattice) { _owningSymbol = owningSymbol; _operation = operation; @@ -29,6 +37,102 @@ public HandleCallAction (in DiagnosticContext diagnosticContext, ISymbol owningS _annotations = FlowAnnotations.Instance; _reflectionAccessAnalyzer = default; _requireDynamicallyAccessedMembersAction = new (diagnosticContext, _reflectionAccessAnalyzer); + _multiValueLattice = multiValueLattice; + } + + private partial bool TryHandleIntrinsic ( + MethodProxy calledMethod, + MultiValue instanceValue, + IReadOnlyList argumentValues, + IntrinsicId intrinsicId, + out MultiValue? methodReturnValue) + { + MultiValue? maybeMethodReturnValue = methodReturnValue = null; + ValueSetLattice multiValueLattice = _multiValueLattice; + + switch (intrinsicId) { + case IntrinsicId.Array_Empty: + AddReturnValue (ArrayValue.Create (0)); + break; + + case IntrinsicId.TypeDelegator_Ctor: + if (_operation is IObjectCreationOperation) + AddReturnValue (argumentValues[0]); + + break; + + case IntrinsicId.Object_GetType: { + foreach (var valueNode in instanceValue.AsEnumerable ()) { + // Note that valueNode can be statically typed as some generic argument type. + // For example: + // void Method(T instance) { instance.GetType().... } + // But it could be that T is annotated with for example PublicMethods: + // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } + // In this case it's in theory possible to handle it, by treating the T basically as a base class + // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking + // has to happen on the callsite, which doesn't know that GetType() will be used...). + // For now we're intentionally ignoring this case - it will produce a warning. + // The counter example is: + // Method(new Derived); + // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which + // currently it won't do. + + // To emulate IL tools behavior (trimmer, NativeAOT compiler), we're going to intentionally "forget" the static type + // if it is a generic argument type. + + ITypeSymbol? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; + if (staticType?.TypeKind == TypeKind.TypeParameter) + staticType = null; + + if (staticType is null) { + // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" + AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod)); + } else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.TypeKind == TypeKind.Array) { + // We can treat this one the same as if it was a typeof() expression + + // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods + // on delegates anyway so reflection on something this approximation would miss is actually safe. + + // We can also treat all arrays as "sealed" since it's not legal to derive from Array type (even though it is not sealed itself) + + // We ignore the fact that the type can be annotated (see below for handling of annotated types) + // This means the annotations (if any) won't be applied - instead we rely on the exact knowledge + // of the type. So for example even if the type is annotated with PublicMethods + // but the code calls GetProperties on it - it will work - mark properties, don't mark methods + // since we ignored the fact that it's annotated. + // This can be seen a little bit as a violation of the annotation, but we already have similar cases + // where a parameter is annotated and if something in the method sets a specific known type to it + // we will also make it just work, even if the annotation doesn't match the usage. + AddReturnValue (new SystemTypeValue (new (staticType))); + } else { + var annotation = FlowAnnotations.GetTypeAnnotation (staticType); + AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, annotation)); + } + } + break; + } + + // Some intrinsics are unimplemented by the analyzer. Analyzer should avoid crashing for these even though they are unimplemented. + case IntrinsicId.Assembly_GetFile: + case IntrinsicId.Assembly_GetFiles: + case IntrinsicId.AssemblyName_get_EscapedCodeBase: + case IntrinsicId.Assembly_get_Location: + case IntrinsicId.AssemblyName_get_CodeBase: + case IntrinsicId.Delegate_get_Method: + case IntrinsicId.Enum_GetValues: + break; + + default: + return false; + } + + methodReturnValue = maybeMethodReturnValue; + return true; + + void AddReturnValue (MultiValue value) + { + maybeMethodReturnValue = (maybeMethodReturnValue is null) ? value : multiValueLattice.Meet ((MultiValue) maybeMethodReturnValue, value); + } } private partial IEnumerable GetMethodsOnTypeHierarchy (TypeProxy type, string name, BindingFlags? bindingFlags) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 9db61498b28ceb..4f773ea109eb8a 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -321,92 +321,10 @@ internal static void HandleCall( ValueSetLattice multiValueLattice, out MultiValue methodReturnValue) { - var handleCallAction = new HandleCallAction (diagnosticContext, owningSymbol, operation); + var handleCallAction = new HandleCallAction (diagnosticContext, owningSymbol, operation, multiValueLattice); MethodProxy method = new (calledMethod); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method); - - if (handleCallAction.Invoke (method, instance, arguments, intrinsicId, out methodReturnValue)) { - return; - } - - MultiValue? maybeMethodReturnValue = default; - - switch (intrinsicId) { - case IntrinsicId.Array_Empty: - AddReturnValue (ArrayValue.Create (0)); - break; - - case IntrinsicId.TypeDelegator_Ctor: - if (operation is IObjectCreationOperation) - AddReturnValue (arguments[0]); - - break; - - case IntrinsicId.Object_GetType: { - foreach (var valueNode in instance.AsEnumerable ()) { - // Note that valueNode can be statically typed as some generic argument type. - // For example: - // void Method(T instance) { instance.GetType().... } - // But it could be that T is annotated with for example PublicMethods: - // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } - // In this case it's in theory possible to handle it, by treating the T basically as a base class - // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking - // has to happen on the callsite, which doesn't know that GetType() will be used...). - // For now we're intentionally ignoring this case - it will produce a warning. - // The counter example is: - // Method(new Derived); - // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which - // currently it won't do. - - // To emulate IL tools behavior (trimmer, NativeAOT compiler), we're going to intentionally "forget" the static type - // if it is a generic argument type. - - ITypeSymbol? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType?.TypeKind == TypeKind.TypeParameter) - staticType = null; - - if (staticType is null) { - // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" - AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (new (calledMethod))); - } else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.TypeKind == TypeKind.Array) { - // We can treat this one the same as if it was a typeof() expression - - // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods - // on delegates anyway so reflection on something this approximation would miss is actually safe. - - // We can also treat all arrays as "sealed" since it's not legal to derive from Array type (even though it is not sealed itself) - - // We ignore the fact that the type can be annotated (see below for handling of annotated types) - // This means the annotations (if any) won't be applied - instead we rely on the exact knowledge - // of the type. So for example even if the type is annotated with PublicMethods - // but the code calls GetProperties on it - it will work - mark properties, don't mark methods - // since we ignored the fact that it's annotated. - // This can be seen a little bit as a violation of the annotation, but we already have similar cases - // where a parameter is annotated and if something in the method sets a specific known type to it - // we will also make it just work, even if the annotation doesn't match the usage. - AddReturnValue (new SystemTypeValue (new (staticType))); - } else { - var annotation = FlowAnnotations.GetTypeAnnotation (staticType); - AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (new (calledMethod), annotation)); - } - } - } - - break; - - default: - Debug.Fail ($"Unexpected method {calledMethod.GetDisplayName ()} unhandled by HandleCallAction."); - - // Do nothing even if we reach a point which we didn't expect - the analyzer should never crash as it's a too disruptive experience for the user. - break; - } - - methodReturnValue = maybeMethodReturnValue ?? multiValueLattice.Top; - - void AddReturnValue (MultiValue value) - { - maybeMethodReturnValue = (maybeMethodReturnValue is null) ? value : multiValueLattice.Meet ((MultiValue) maybeMethodReturnValue, value); - } + handleCallAction.Invoke (method, instance, arguments, intrinsicId, out methodReturnValue); } public override void HandleReturnValue (MultiValue returnValue, IOperation operation, in FeatureContext featureContext) diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index 9063ac7368dbe4..5a0a4a86320cf2 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -27,7 +27,53 @@ internal partial struct HandleCallAction private readonly FlowAnnotations _annotations; private readonly RequireDynamicallyAccessedMembersAction _requireDynamicallyAccessedMembersAction; - public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList argumentValues, IntrinsicId intrinsicId, out MultiValue methodReturnValue) + public void Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList argumentValues, IntrinsicId intrinsicId, out MultiValue methodReturnValue) + { + MultiValue? maybeMethodReturnValue; + + if (!TryHandleIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue)) + HandleSharedIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + + // If we get here, we handled this as an intrinsic. As a convenience, if the code above + // didn't set the return value (and the method has a return value), we will set it to be an + // unknown value with the return type of the method. + var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod); + bool returnsVoid = calledMethod.ReturnsVoid (); + methodReturnValue = maybeMethodReturnValue ?? (returnsVoid ? + MultiValueLattice.Top : + annotatedMethodReturnValue); + + // Validate that the return value has the correct annotations as per the method return value annotations + if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) { + foreach (var uniqueValue in methodReturnValue.AsEnumerable ()) { + if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) { + if (!methodReturnValueWithMemberTypes.DynamicallyAccessedMemberTypes.HasFlag (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes)) + throw new InvalidOperationException ($"Internal trimming error: processing of call from {GetContainingSymbolDisplayName ()} to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); + } else if (uniqueValue is SystemTypeValue) { + // SystemTypeValue can fulfill any requirement, so it's always valid + // The requirements will be applied at the point where it's consumed (passed as a method parameter, set as field value, returned from the method) + } else if (uniqueValue == NullValue.Instance) { + // NullValue can fulfill any requirements because reflection access to it will typically throw. + } else { + throw new InvalidOperationException ($"Internal trimming error: processing of call from {GetContainingSymbolDisplayName ()} to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); + } + } + } + } + + private partial bool TryHandleIntrinsic ( + MethodProxy calledMethod, + MultiValue instanceValue, + IReadOnlyList argumentValues, + IntrinsicId intrinsicId, + out MultiValue? methodReturnValue); + + void HandleSharedIntrinsic ( + MethodProxy calledMethod, + MultiValue instanceValue, + IReadOnlyList argumentValues, + IntrinsicId intrinsicId, + out MultiValue? methodReturnValue) { MultiValue? returnValue = null; @@ -154,8 +200,7 @@ GenericParameterValue genericParam // Array.Empty must for now be handled by the specific implementation since it requires instantiated generic method handling case IntrinsicId.Object_GetType: // Object.GetType requires additional handling by the caller to implement type hierarchy marking and related diagnostics - methodReturnValue = MultiValueLattice.Top; - return false; + throw new NotImplementedException ("These intrinsics should be handled by the specific implementation: " + intrinsicId); // // GetInterface (String) @@ -1161,37 +1206,13 @@ GenericParameterValue genericParam // Ideally we would run everything through HandleCallAction and it would return "false" for intrinsics it doesn't handle // like it already does for Activator.CreateInstance for example. default: - methodReturnValue = MultiValueLattice.Top; - return true; + throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); } - // For now, if the intrinsic doesn't set a return value, fall back on the annotations. - // Note that this will be DynamicallyAccessedMembers.None for the intrinsics which don't return types. - returnValue ??= calledMethod.ReturnsVoid () ? MultiValueLattice.Top : annotatedMethodReturnValue; - if (MethodIsTypeConstructor (calledMethod)) returnValue = UnknownValue.Instance; - // Validate that the return value has the correct annotations as per the method return value annotations - if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None) { - foreach (var uniqueValue in returnValue.Value.AsEnumerable ()) { - if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) { - if (!methodReturnValueWithMemberTypes.DynamicallyAccessedMemberTypes.HasFlag (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes)) - throw new InvalidOperationException ($"Internal ILLink error: in {GetContainingSymbolDisplayName ()} processing call to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); - } else if (uniqueValue is SystemTypeValue) { - // SystemTypeValue can fulfill any requirement, so it's always valid - // The requirements will be applied at the point where it's consumed (passed as a method parameter, set as field value, returned from the method) - } else if (uniqueValue == NullValue.Instance) { - // NullValue can fulfill any requirements because reflection access to it will typically throw. - } else { - throw new InvalidOperationException ($"Internal ILLink error: in {GetContainingSymbolDisplayName ()} processing call to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); - } - } - } - - methodReturnValue = returnValue.Value; - - return true; + methodReturnValue = returnValue; void AddReturnValue (MultiValue value) { diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 108b79356a0228..675be591191c18 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -2,11 +2,16 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Reflection; using ILLink.Shared.TypeSystemProxy; using Mono.Cecil; +using Mono.Cecil.Cil; using Mono.Linker; using Mono.Linker.Dataflow; +using Mono.Linker.Steps; +using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.Shared.TrimAnalysis { @@ -15,21 +20,163 @@ internal partial struct HandleCallAction #pragma warning disable CA1822 // Mark members as static - the other partial implementations might need to be instance methods readonly LinkContext _context; + readonly Instruction _operation; + readonly MarkStep _markStep; readonly ReflectionMarker _reflectionMarker; readonly MethodDefinition _callingMethodDefinition; + readonly MethodReference _calledMethodReference; public HandleCallAction ( LinkContext context, + Instruction operation, + MarkStep markStep, ReflectionMarker reflectionMarker, in DiagnosticContext diagnosticContext, - MethodDefinition callingMethodDefinition) + MethodDefinition callingMethodDefinition, + MethodReference calledMethodReference) { _context = context; + _operation = operation; + _markStep = markStep; _reflectionMarker = reflectionMarker; _diagnosticContext = diagnosticContext; _callingMethodDefinition = callingMethodDefinition; _annotations = context.Annotations.FlowAnnotations; _requireDynamicallyAccessedMembersAction = new (reflectionMarker, diagnosticContext); + _calledMethodReference = calledMethodReference; + } + + private partial bool TryHandleIntrinsic ( + MethodProxy calledMethod, + MultiValue instanceValue, + IReadOnlyList argumentValues, + IntrinsicId intrinsicId, + out MultiValue? methodReturnValue) + { + MultiValue? maybeMethodReturnValue = methodReturnValue = null; + Debug.Assert (calledMethod.Method == _context.Resolve (_calledMethodReference)); + + switch (intrinsicId) { + case IntrinsicId.None: { + if (ReflectionMethodBodyScanner.IsPInvokeDangerous (calledMethod.Method, _context, out bool comDangerousMethod)) { + Debug.Assert (comDangerousMethod); // Currently COM dangerous is the only one we detect + _diagnosticContext.AddDiagnostic (DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethod.GetDisplayName ()); + } + if (_context.Annotations.DoesMethodRequireUnreferencedCode (calledMethod.Method, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCode)) + MarkStep.ReportRequiresUnreferencedCode (calledMethod.GetDisplayName (), requiresUnreferencedCode, _diagnosticContext); + + HandleSharedIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + } + break; + + case IntrinsicId.TypeDelegator_Ctor: { + // This is an identity function for analysis purposes + if (_operation.OpCode == OpCodes.Newobj) + AddReturnValue (argumentValues[0]); + } + break; + + case IntrinsicId.Array_Empty: { + AddReturnValue (ArrayValue.Create (0, ((GenericInstanceMethod) _calledMethodReference).GenericArguments[0])); + } + break; + + case IntrinsicId.Array_CreateInstance: + case IntrinsicId.Enum_GetValues: + case IntrinsicId.Marshal_SizeOf: + case IntrinsicId.Marshal_OffsetOf: + case IntrinsicId.Marshal_PtrToStructure: + case IntrinsicId.Marshal_DestroyStructure: + case IntrinsicId.Marshal_GetDelegateForFunctionPointer: + case IntrinsicId.Assembly_get_Location: + case IntrinsicId.Assembly_GetFile: + case IntrinsicId.Assembly_GetFiles: + case IntrinsicId.AssemblyName_get_CodeBase: + case IntrinsicId.AssemblyName_get_EscapedCodeBase: + case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: + case IntrinsicId.Delegate_get_Method: + // These intrinsics are not interesting for trimmer (they are interesting for AOT and that's why they are recognized) + break; + + // + // System.Object + // + // GetType() + // + case IntrinsicId.Object_GetType: { + foreach (var valueNode in instanceValue.AsEnumerable ()) { + // Note that valueNode can be statically typed in IL as some generic argument type. + // For example: + // void Method(T instance) { instance.GetType().... } + // Currently this case will end up with null StaticType - since there's no typedef for the generic argument type. + // But it could be that T is annotated with for example PublicMethods: + // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } + // In this case it's in theory possible to handle it, by treating the T basically as a base class + // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking + // has to happen on the callsite, which doesn't know that GetType() will be used...). + // For now we're intentionally ignoring this case - it will produce a warning. + // The counter example is: + // Method(new Derived); + // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which + // currently it won't do. + + TypeDefinition? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; + if (staticType is null) { + // We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations + AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod)); + } else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) { + // We can treat this one the same as if it was a typeof() expression + + // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods + // on delegates anyway so reflection on something this approximation would miss is actually safe. + + // We can also treat all arrays as "sealed" since it's not legal to derive from Array type (even though it is not sealed itself) + + // We ignore the fact that the type can be annotated (see below for handling of annotated types) + // This means the annotations (if any) won't be applied - instead we rely on the exact knowledge + // of the type. So for example even if the type is annotated with PublicMethods + // but the code calls GetProperties on it - it will work - mark properties, don't mark methods + // since we ignored the fact that it's annotated. + // This can be seen a little bit as a violation of the annotation, but we already have similar cases + // where a parameter is annotated and if something in the method sets a specific known type to it + // we will also make it just work, even if the annotation doesn't match the usage. + AddReturnValue (new SystemTypeValue (staticType)); + } else { + // Make sure the type is marked (this will mark it as used via reflection, which is sort of true) + // This should already be true for most cases (method params, fields, ...), but just in case + _reflectionMarker.MarkType (_diagnosticContext.Origin, staticType); + + var annotation = _markStep.DynamicallyAccessedMembersTypeHierarchy + .ApplyDynamicallyAccessedMembersToTypeHierarchy (staticType); + + // Return a value which is "unknown type" with annotation. For now we'll use the return value node + // for the method, which means we're loosing the information about which staticType this + // started with. For now we don't need it, but we can add it later on. + AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, annotation)); + } + } + } + break; + + // Note about Activator.CreateInstance + // There are 2 interesting cases: + // - The generic argument for T is either specific type or annotated - in that case generic instantiation will handle this + // since from .NET 6+ the T is annotated with PublicParameterlessConstructor annotation, so the trimming tools would apply this as for any other method. + // - The generic argument for T is unannotated type - the generic instantiantion handling has a special case for handling PublicParameterlessConstructor requirement + // in such that if the generic argument type has the "new" constraint it will not warn (as it is effectively the same thing semantically). + // For all other cases, the trimming tools would have already produced a warning. + + default: + return false; + } + + methodReturnValue = maybeMethodReturnValue; + return true; + + void AddReturnValue (MultiValue value) + { + maybeMethodReturnValue = (maybeMethodReturnValue is null) ? value : MultiValueLattice.Meet ((MultiValue) maybeMethodReturnValue, value); + } } private partial bool MethodIsTypeConstructor (MethodProxy method) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 72e9339bc3bc3f..d6762a633664da 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -6,7 +6,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; -using ILLink.Shared; using ILLink.Shared.TrimAnalysis; using ILLink.Shared.TypeSystemProxy; using Mono.Cecil; @@ -154,7 +153,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c )); var diagnosticContext = new DiagnosticContext (_origin, diagnosticsEnabled: false, _context); - return HandleCall ( + HandleCall ( operation, calledMethod, instanceValue, @@ -164,9 +163,10 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c _context, _markStep, out methodReturnValue); + return true; } - public static bool HandleCall ( + public static void HandleCall ( Instruction operation, MethodReference calledMethod, MultiValue instanceValue, @@ -187,200 +187,9 @@ public static bool HandleCall ( var annotatedMethodReturnValue = context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethodDefinition); Debug.Assert (requiresDataFlowAnalysis || annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None); - MultiValue? maybeMethodReturnValue = null; - - var handleCallAction = new HandleCallAction (context, reflectionMarker, diagnosticContext, callingMethodDefinition); + var handleCallAction = new HandleCallAction (context, operation, markStep, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (calledMethodDefinition); - switch (intrinsicId) { - case IntrinsicId.IntrospectionExtensions_GetTypeInfo: - case IntrinsicId.TypeInfo_AsType: - case IntrinsicId.Type_get_UnderlyingSystemType: - case IntrinsicId.Type_GetTypeFromHandle: - case IntrinsicId.Type_get_TypeHandle: - case IntrinsicId.Type_GetInterface: - case IntrinsicId.Type_get_AssemblyQualifiedName: - case IntrinsicId.RuntimeHelpers_RunClassConstructor: - case IntrinsicId.Type_GetConstructors__BindingFlags: - case IntrinsicId.Type_GetMethods__BindingFlags: - case IntrinsicId.Type_GetFields__BindingFlags: - case IntrinsicId.Type_GetProperties__BindingFlags: - case IntrinsicId.Type_GetEvents__BindingFlags: - case IntrinsicId.Type_GetNestedTypes__BindingFlags: - case IntrinsicId.Type_GetMembers__BindingFlags: - case IntrinsicId.Type_GetField: - case IntrinsicId.Type_GetProperty: - case IntrinsicId.Type_GetEvent: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeEvent: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeField: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeMethod: - case IntrinsicId.RuntimeReflectionExtensions_GetRuntimeProperty: - case IntrinsicId.Type_GetMember: - case IntrinsicId.Type_GetMethod: - case IntrinsicId.Type_GetNestedType: - case IntrinsicId.Nullable_GetUnderlyingType: - case IntrinsicId.Expression_Property: - case IntrinsicId.Expression_Field: - case IntrinsicId.Type_get_BaseType: - case IntrinsicId.Type_GetConstructor: - case IntrinsicId.MethodBase_GetMethodFromHandle: - case IntrinsicId.MethodBase_get_MethodHandle: - case IntrinsicId.Type_MakeGenericType: - case IntrinsicId.MethodInfo_MakeGenericMethod: - case IntrinsicId.Expression_Call: - case IntrinsicId.Expression_New: - case IntrinsicId.Type_GetType: - case IntrinsicId.Activator_CreateInstance__Type: - case IntrinsicId.Activator_CreateInstance__AssemblyName_TypeName: - case IntrinsicId.Activator_CreateInstanceFrom: - case IntrinsicId.AppDomain_CreateInstance: - case IntrinsicId.AppDomain_CreateInstanceAndUnwrap: - case IntrinsicId.AppDomain_CreateInstanceFrom: - case IntrinsicId.AppDomain_CreateInstanceFromAndUnwrap: - case IntrinsicId.Assembly_CreateInstance: { - return handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out methodReturnValue); - } - - case IntrinsicId.None: { - if (IsPInvokeDangerous (calledMethodDefinition, context, out bool comDangerousMethod)) { - Debug.Assert (comDangerousMethod); // Currently COM dangerous is the only one we detect - diagnosticContext.AddDiagnostic (DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethodDefinition.GetDisplayName ()); - } - if (context.Annotations.DoesMethodRequireUnreferencedCode (calledMethodDefinition, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCode)) - MarkStep.ReportRequiresUnreferencedCode (calledMethodDefinition.GetDisplayName (), requiresUnreferencedCode, diagnosticContext); - - return handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out methodReturnValue); - } - - case IntrinsicId.TypeDelegator_Ctor: { - // This is an identity function for analysis purposes - if (operation.OpCode == OpCodes.Newobj) - AddReturnValue (argumentValues[0]); - } - break; - - case IntrinsicId.Array_Empty: { - AddReturnValue (ArrayValue.Create (0, ((GenericInstanceMethod) calledMethod).GenericArguments[0])); - } - break; - - case IntrinsicId.Array_CreateInstance: - case IntrinsicId.Enum_GetValues: - case IntrinsicId.Marshal_SizeOf: - case IntrinsicId.Marshal_OffsetOf: - case IntrinsicId.Marshal_PtrToStructure: - case IntrinsicId.Marshal_DestroyStructure: - case IntrinsicId.Marshal_GetDelegateForFunctionPointer: - case IntrinsicId.Assembly_get_Location: - case IntrinsicId.Assembly_GetFile: - case IntrinsicId.Assembly_GetFiles: - case IntrinsicId.AssemblyName_get_CodeBase: - case IntrinsicId.AssemblyName_get_EscapedCodeBase: - case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: - case IntrinsicId.Delegate_get_Method: - // These intrinsics are not interesting for trimmer (they are interesting for AOT and that's why they are recognized) - break; - - // - // System.Object - // - // GetType() - // - case IntrinsicId.Object_GetType: { - foreach (var valueNode in instanceValue.AsEnumerable ()) { - // Note that valueNode can be statically typed in IL as some generic argument type. - // For example: - // void Method(T instance) { instance.GetType().... } - // Currently this case will end up with null StaticType - since there's no typedef for the generic argument type. - // But it could be that T is annotated with for example PublicMethods: - // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } - // In this case it's in theory possible to handle it, by treating the T basically as a base class - // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking - // has to happen on the callsite, which doesn't know that GetType() will be used...). - // For now we're intentionally ignoring this case - it will produce a warning. - // The counter example is: - // Method(new Derived); - // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which - // currently it won't do. - - TypeDefinition? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType is null) { - // We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations - AddReturnValue (context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethodDefinition)); - } else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) { - // We can treat this one the same as if it was a typeof() expression - - // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods - // on delegates anyway so reflection on something this approximation would miss is actually safe. - - // We can also treat all arrays as "sealed" since it's not legal to derive from Array type (even though it is not sealed itself) - - // We ignore the fact that the type can be annotated (see below for handling of annotated types) - // This means the annotations (if any) won't be applied - instead we rely on the exact knowledge - // of the type. So for example even if the type is annotated with PublicMethods - // but the code calls GetProperties on it - it will work - mark properties, don't mark methods - // since we ignored the fact that it's annotated. - // This can be seen a little bit as a violation of the annotation, but we already have similar cases - // where a parameter is annotated and if something in the method sets a specific known type to it - // we will also make it just work, even if the annotation doesn't match the usage. - AddReturnValue (new SystemTypeValue (staticType)); - } else { - // Make sure the type is marked (this will mark it as used via reflection, which is sort of true) - // This should already be true for most cases (method params, fields, ...), but just in case - reflectionMarker.MarkType (origin, staticType); - - var annotation = markStep.DynamicallyAccessedMembersTypeHierarchy - .ApplyDynamicallyAccessedMembersToTypeHierarchy (staticType); - - // Return a value which is "unknown type" with annotation. For now we'll use the return value node - // for the method, which means we're loosing the information about which staticType this - // started with. For now we don't need it, but we can add it later on. - AddReturnValue (context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethodDefinition, annotation)); - } - } - } - break; - - // Note about Activator.CreateInstance - // There are 2 interesting cases: - // - The generic argument for T is either specific type or annotated - in that case generic instantiation will handle this - // since from .NET 6+ the T is annotated with PublicParameterlessConstructor annotation, so the trimming tools would apply this as for any other method. - // - The generic argument for T is unannotated type - the generic instantiantion handling has a special case for handling PublicParameterlessConstructor requirement - // in such that if the generic argument type has the "new" constraint it will not warn (as it is effectively the same thing semantically). - // For all other cases, the trimming tools would have already produced a warning. - - default: - throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); - } - - // If we get here, we handled this as an intrinsic. As a convenience, if the code above - // didn't set the return value (and the method has a return value), we will set it to be an - // unknown value with the return type of the method. - bool returnsVoid = calledMethod.ReturnsVoid (); - methodReturnValue = maybeMethodReturnValue ?? (returnsVoid ? - MultiValueLattice.Top : - annotatedMethodReturnValue); - - // Validate that the return value has the correct annotations as per the method return value annotations - if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) { - foreach (var uniqueValue in methodReturnValue.AsEnumerable ()) { - if (uniqueValue is ValueWithDynamicallyAccessedMembers methodReturnValueWithMemberTypes) { - if (!methodReturnValueWithMemberTypes.DynamicallyAccessedMemberTypes.HasFlag (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes)) - throw new InvalidOperationException ($"Internal trimming error: processing of call from {callingMethodDefinition.GetDisplayName ()} to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); - } else if (uniqueValue is SystemTypeValue) { - // SystemTypeValue can fulfill any requirement, so it's always valid - // The requirements will be applied at the point where it's consumed (passed as a method parameter, set as field value, returned from the method) - } else { - throw new InvalidOperationException ($"Internal trimming error: processing of call from {callingMethodDefinition.GetDisplayName ()} to {calledMethod.GetDisplayName ()} returned value which is not correctly annotated with the expected dynamic member access kinds."); - } - } - } - - return true; - - void AddReturnValue (MultiValue value) - { - maybeMethodReturnValue = (maybeMethodReturnValue is null) ? value : MultiValueLattice.Meet ((MultiValue) maybeMethodReturnValue, value); - } + handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out methodReturnValue); } static bool IsComInterop (IMarshalInfoProvider marshalInfoProvider, TypeReference parameterType, LinkContext context) @@ -449,7 +258,7 @@ void HandleAssignmentPattern ( TrimAnalysisPatterns.Add (new TrimAnalysisAssignmentPattern (value, targetValue, origin)); } - private static bool IsPInvokeDangerous (MethodDefinition methodDefinition, LinkContext context, out bool comDangerousMethod) + 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 // so that the code looks very similar to AOT which has more than one condition. diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs index 9ec76e4de21ecb..5d21f678b1dbd5 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs @@ -682,8 +682,7 @@ public void Method () { } } [Kept] - // https://github.com/dotnet/linker/issues/2755 - [ExpectedWarning ("IL2075", "GetMethod", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL2075", "GetMethod")] public static void Test () { new Derived ().GetType ().GetMethod ("Method"); @@ -1586,8 +1585,7 @@ class Target [Kept] // https://github.com/dotnet/runtime/issues/93720 - // https://github.com/dotnet/linker/issues/2755 - [ExpectedWarning ("IL2072", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL2072")] static void TestIsInstOf (object o) { if (o is Target t) { From 56d1e243a5b3da199e40154888cb0984e485d5ee Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 22 Apr 2024 13:19:44 -0700 Subject: [PATCH 2/6] Remove MethodBodyScanner return value handling --- .../Compiler/Dataflow/MethodBodyScanner.cs | 35 +++---------------- .../Dataflow/ReflectionMethodBodyScanner.cs | 3 +- .../Linker.Dataflow/MethodBodyScanner.cs | 30 +++------------- .../ReflectionMethodBodyScanner.cs | 11 +++--- 4 files changed, 16 insertions(+), 63 deletions(-) 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 87764b54a3d738..5c498fa7214cb7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs @@ -1156,11 +1156,8 @@ private ValueNodeList PopCallArguments( Stack currentStack, MethodDesc methodCalled, MethodIL containingMethodBody, - bool isNewObj, int ilOffset, - out SingleValue? newObjValue) + bool isNewObj, int ilOffset) { - newObjValue = null; - int countToPop = 0; if (!isNewObj && !methodCalled.Signature.IsStatic) countToPop++; @@ -1175,8 +1172,7 @@ private ValueNodeList PopCallArguments( if (isNewObj) { - newObjValue = UnknownValue.Instance; - methodParams.Add(newObjValue); + methodParams.Add(UnknownValue.Instance); } methodParams.Reverse(); return methodParams; @@ -1275,9 +1271,7 @@ private void HandleCall( { bool isNewObj = opcode == ILOpcode.newobj; - SingleValue? newObjValue; - ValueNodeList methodArguments = PopCallArguments(currentStack, calledMethod, callingMethodBody, isNewObj, - offset, out newObjValue); + ValueNodeList methodArguments = PopCallArguments(currentStack, calledMethod, callingMethodBody, isNewObj, offset); // Multi-dimensional array access is represented as a call to a special Get method on the array (runtime provided method) // We don't track multi-dimensional arrays in any way, so return unknown value. @@ -1291,7 +1285,7 @@ private void HandleCall( foreach (var argument in methodArguments) dereferencedMethodParams.Add(DereferenceValue(callingMethodBody, offset, argument, locals, ref interproceduralState)); MultiValue methodReturnValue; - bool handledFunction = HandleCall( + HandleCall( callingMethodBody, calledMethod, opcode, @@ -1299,25 +1293,6 @@ private void HandleCall( new ValueNodeList(dereferencedMethodParams), out methodReturnValue); - // Handle the return value or newobj result - if (!handledFunction) - { - if (isNewObj) - { - if (newObjValue == null) - methodReturnValue = UnknownValue.Instance; - else - methodReturnValue = newObjValue; - } - else - { - if (!calledMethod.Signature.ReturnType.IsVoid) - { - methodReturnValue = UnknownValue.Instance; - } - } - } - if (isNewObj || !calledMethod.Signature.ReturnType.IsVoid) currentStack.Push(new StackSlot(methodReturnValue)); @@ -1335,7 +1310,7 @@ private void HandleCall( } } - public abstract bool HandleCall( + public abstract void HandleCall( MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index 0e5e077ad984a9..ac850e3fbc3a39 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -261,7 +261,7 @@ protected override void HandleFieldTokenAccess(MethodIL methodBody, int offset, ProcessGenericArgumentDataFlow(accessedField); } - public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, int offset, ValueNodeList methodParams, out MultiValue methodReturnValue) + public override void HandleCall(MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, int offset, ValueNodeList methodParams, out MultiValue methodReturnValue) { Debug.Assert(callingMethodBody.OwningMethod == _origin.MemberDefinition); @@ -302,7 +302,6 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet diagnosticContext, _reflectionMarker, out methodReturnValue); - return true; } public static void HandleCall( diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs index 4d4855dc7ce6d9..589ee454617dfc 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -986,11 +986,8 @@ private ValueNodeList PopCallArguments ( Stack currentStack, MethodReference methodCalled, MethodBody containingMethodBody, - bool isNewObj, int ilOffset, - out SingleValue? newObjValue) + bool isNewObj, int ilOffset) { - newObjValue = null; - int countToPop = 0; if (!isNewObj && methodCalled.HasThis && !methodCalled.ExplicitThis) countToPop++; @@ -1003,8 +1000,7 @@ private ValueNodeList PopCallArguments ( } if (isNewObj) { - newObjValue = UnknownValue.Instance; - methodParams.Add (newObjValue); + methodParams.Add (UnknownValue.Instance); } methodParams.Reverse (); return methodParams; @@ -1090,34 +1086,18 @@ private void HandleCall ( bool isNewObj = operation.OpCode.Code == Code.Newobj; - SingleValue? newObjValue; - ValueNodeList methodArguments = PopCallArguments (currentStack, calledMethod, callingMethodBody, isNewObj, - operation.Offset, out newObjValue); + ValueNodeList methodArguments = PopCallArguments (currentStack, calledMethod, callingMethodBody, isNewObj, operation.Offset); var dereferencedMethodParams = new List (); foreach (var argument in methodArguments) dereferencedMethodParams.Add (DereferenceValue (argument, locals, ref interproceduralState)); MultiValue methodReturnValue; - bool handledFunction = HandleCall ( + HandleCall ( callingMethodBody, calledMethod, operation, new ValueNodeList (dereferencedMethodParams), out methodReturnValue); - // Handle the return value or newobj result - if (!handledFunction) { - if (isNewObj) { - if (newObjValue == null) - methodReturnValue = new MultiValue (UnknownValue.Instance); - else - methodReturnValue = newObjValue; - } else { - if (!calledMethod.ReturnsVoid ()) { - methodReturnValue = UnknownValue.Instance; - } - } - } - if (isNewObj || !calledMethod.ReturnsVoid ()) currentStack.Push (new StackSlot (methodReturnValue)); @@ -1134,7 +1114,7 @@ private void HandleCall ( public TypeDefinition? ResolveToTypeDefinition (TypeReference typeReference) => typeReference.ResolveToTypeDefinition (_context); - public abstract bool HandleCall ( + public abstract void HandleCall ( MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index d6762a633664da..13340294a03081 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -117,19 +117,19 @@ protected override void HandleStoreParameter (MethodDefinition method, MethodPar protected override void HandleStoreMethodReturnValue (MethodDefinition method, MethodReturnValue returnValue, Instruction operation, MultiValue valueToStore) => HandleStoreValueWithDynamicallyAccessedMembers (returnValue, operation, valueToStore); - public override bool HandleCall (MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, ValueNodeList methodParams, out MultiValue methodReturnValue) + public override void HandleCall (MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, ValueNodeList methodParams, out MultiValue methodReturnValue) { var reflectionProcessed = _markStep.ProcessReflectionDependency (callingMethodBody, operation); if (reflectionProcessed) { - methodReturnValue = default; - return false; + methodReturnValue = UnknownValue.Instance; + return; } Debug.Assert (callingMethodBody.Method == _origin.Provider); var calledMethodDefinition = _context.TryResolve (calledMethod); if (calledMethodDefinition == null) { - methodReturnValue = default; - return false; + methodReturnValue = UnknownValue.Instance; + return; } _origin = _origin.WithInstructionOffset (operation.Offset); @@ -163,7 +163,6 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c _context, _markStep, out methodReturnValue); - return true; } public static void HandleCall ( From 3b8cc81596958f88b8a178f72c2a4719fbf96247 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 24 Apr 2024 13:26:20 -0700 Subject: [PATCH 3/6] Avoid throwing from analyzer in Release builds And add missing cases to the switch statement for the analyzer. --- .../Dataflow/ReflectionMethodBodyScanner.cs | 3 +- .../TrimAnalysis/HandleCallAction.cs | 10 ++++++- .../TrimAnalysis/TrimAnalysisVisitor.cs | 8 +++++- .../TrimAnalysis/HandleCallAction.cs | 28 ++++++++----------- .../Linker.Dataflow/HandleCallAction.cs | 3 +- .../ReflectionMethodBodyScanner.cs | 3 +- 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index ac850e3fbc3a39..8688046101cc77 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -324,7 +324,8 @@ public static void HandleCall( var handleCallAction = new HandleCallAction(reflectionMarker.Annotations, operation, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod.GetDisplayName()); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod(calledMethod); - handleCallAction.Invoke (calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); + if (!handleCallAction.Invoke (calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue)) + throw new NotImplementedException($"Unhandled intrinsic {intrinsicId}"); } private static bool IsAotUnsafeDelegate(TypeDesc parameterType) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs index 41bab03cbc2877..9212cdc298fefb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs @@ -112,7 +112,9 @@ private partial bool TryHandleIntrinsic ( break; } - // Some intrinsics are unimplemented by the analyzer. Analyzer should avoid crashing for these even though they are unimplemented. + // Some intrinsics are unimplemented by the analyzer. + // These will fall back to the usual return-value handling. + case IntrinsicId.Array_CreateInstance: case IntrinsicId.Assembly_GetFile: case IntrinsicId.Assembly_GetFiles: case IntrinsicId.AssemblyName_get_EscapedCodeBase: @@ -120,6 +122,12 @@ private partial bool TryHandleIntrinsic ( case IntrinsicId.AssemblyName_get_CodeBase: case IntrinsicId.Delegate_get_Method: case IntrinsicId.Enum_GetValues: + case IntrinsicId.Marshal_DestroyStructure: + case IntrinsicId.Marshal_GetDelegateForFunctionPointer: + case IntrinsicId.Marshal_OffsetOf: + case IntrinsicId.Marshal_PtrToStructure: + case IntrinsicId.Marshal_SizeOf: + case IntrinsicId.RuntimeReflectionExtensions_GetMethodInfo: break; default: diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 4f773ea109eb8a..4ff9333c55904b 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -324,7 +324,13 @@ internal static void HandleCall( var handleCallAction = new HandleCallAction (diagnosticContext, owningSymbol, operation, multiValueLattice); MethodProxy method = new (calledMethod); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method); - handleCallAction.Invoke (method, instance, arguments, intrinsicId, out methodReturnValue); + if (!handleCallAction.Invoke (method, instance, arguments, intrinsicId, out methodReturnValue)) + UnhandledIntrinsicHelper (intrinsicId); + + // Avoid crashing the analyzer in release builds + [Conditional ("DEBUG")] + static void UnhandledIntrinsicHelper (IntrinsicId intrinsicId) + => throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); } public override void HandleReturnValue (MultiValue returnValue, IOperation operation, in FeatureContext featureContext) diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index 5a0a4a86320cf2..1ba45a7668bff0 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -27,16 +27,16 @@ internal partial struct HandleCallAction private readonly FlowAnnotations _annotations; private readonly RequireDynamicallyAccessedMembersAction _requireDynamicallyAccessedMembersAction; - public void Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList argumentValues, IntrinsicId intrinsicId, out MultiValue methodReturnValue) + public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList argumentValues, IntrinsicId intrinsicId, out MultiValue methodReturnValue) { MultiValue? maybeMethodReturnValue; - if (!TryHandleIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue)) - HandleSharedIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + var handledIntrinsic = + TryHandleIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue) || + TryHandleSharedIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); - // If we get here, we handled this as an intrinsic. As a convenience, if the code above - // didn't set the return value (and the method has a return value), we will set it to be an - // unknown value with the return type of the method. + // As a convenience, if the code above didn't set the return value (and the method has a return value), + // we will set it to be an unknown value with the return type of the method. var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod); bool returnsVoid = calledMethod.ReturnsVoid (); methodReturnValue = maybeMethodReturnValue ?? (returnsVoid ? @@ -59,6 +59,8 @@ public void Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl } } } + + return handledIntrinsic; } private partial bool TryHandleIntrinsic ( @@ -68,14 +70,14 @@ private partial bool TryHandleIntrinsic ( IntrinsicId intrinsicId, out MultiValue? methodReturnValue); - void HandleSharedIntrinsic ( + bool TryHandleSharedIntrinsic ( MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList argumentValues, IntrinsicId intrinsicId, out MultiValue? methodReturnValue) { - MultiValue? returnValue = null; + MultiValue? returnValue = methodReturnValue = null; bool requiresDataFlowAnalysis = _annotations.MethodRequiresDataFlowAnalysis (calledMethod); var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod); @@ -1198,21 +1200,15 @@ GenericParameterValue genericParam } break; - // Disable warnings for all unimplemented intrinsics. Some intrinsic methods have annotations, but analyzing them - // would produce unnecessary warnings even for cases that are intrinsically handled. So we disable handling these calls - // until a proper intrinsic handling is made - // NOTE: Currently this is done "for the analyzer" and it relies on illink/NativeAOT to not call HandleCallAction - // for intrinsics which illink/NativeAOT need special handling for or those which are not implemented here and only there. - // Ideally we would run everything through HandleCallAction and it would return "false" for intrinsics it doesn't handle - // like it already does for Activator.CreateInstance for example. default: - throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); + return false; } if (MethodIsTypeConstructor (calledMethod)) returnValue = UnknownValue.Instance; methodReturnValue = returnValue; + return true; void AddReturnValue (MultiValue value) { diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 675be591191c18..6721ec63478fac 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -65,9 +65,8 @@ private partial bool TryHandleIntrinsic ( if (_context.Annotations.DoesMethodRequireUnreferencedCode (calledMethod.Method, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCode)) MarkStep.ReportRequiresUnreferencedCode (calledMethod.GetDisplayName (), requiresUnreferencedCode, _diagnosticContext); - HandleSharedIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + return TryHandleSharedIntrinsic (calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); } - break; case IntrinsicId.TypeDelegator_Ctor: { // This is an identity function for analysis purposes diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 13340294a03081..9a808d38a1f132 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -188,7 +188,8 @@ public static void HandleCall ( var handleCallAction = new HandleCallAction (context, operation, markStep, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (calledMethodDefinition); - handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out methodReturnValue); + if (!handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out methodReturnValue)) + throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); } static bool IsComInterop (IMarshalInfoProvider marshalInfoProvider, TypeReference parameterType, LinkContext context) From 69017ea55a42550981ec9ef995edba3fdefc1516 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 24 Apr 2024 14:07:56 -0700 Subject: [PATCH 4/6] Fix ILC callsites --- .../Compiler/Dataflow/HandleCallAction.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index cd87469d48685f..1ebef5d46b4b76 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -112,9 +112,8 @@ private partial bool TryHandleIntrinsic ( } // This intrinsic is relevant to both trimming and AOT - call into trimming logic as well. - HandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + return TryHandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); } - break; case IntrinsicId.MethodInfo_MakeGenericMethod: { @@ -172,9 +171,8 @@ private partial bool TryHandleIntrinsic ( } // This intrinsic is relevant to both trimming and AOT - call into trimming logic as well. - HandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + return TryHandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); } - break; case IntrinsicId.None: { @@ -193,9 +191,8 @@ private partial bool TryHandleIntrinsic ( ReflectionMethodBodyScanner.CheckAndReportAllRequires(_diagnosticContext, calledMethod.Method); - HandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out maybeMethodReturnValue); + return TryHandleSharedIntrinsic(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue); } - break; case IntrinsicId.TypeDelegator_Ctor: { From b898b27bace98b561c000f0fc0361c091770393c Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 24 Apr 2024 15:54:27 -0700 Subject: [PATCH 5/6] Fix Array_CreateInstance case The PInvoke logic and the CheckAndReportRequires logic should not be needed for this intrinsic, and the old shared HandleCallAction would go to the default case that sets the return value to Top and returns false. Now this instead returns true and lets the shared return value logic kick in. --- .../ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index 1ebef5d46b4b76..c637185d15b539 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -218,8 +218,8 @@ private partial bool TryHandleIntrinsic ( // We could try to analyze if the type is known, but for now making sure this works for canonical arrays is enough. TypeDesc canonArrayType = _reflectionMarker.Factory.TypeSystemContext.CanonType.MakeArrayType(); _reflectionMarker.MarkType(_diagnosticContext.Origin, canonArrayType, "Array.CreateInstance was called"); - goto case IntrinsicId.None; } + break; // // System.Enum From c450da6c39c60454d6e9d5d42e362361558a0106 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 25 Apr 2024 17:01:50 +0000 Subject: [PATCH 6/6] PR feedback - Return instead of out param in HandleCall - FIx formatting --- .../Compiler/Dataflow/MethodBodyScanner.cs | 11 ++++------ .../Dataflow/ReflectionMethodBodyScanner.cs | 15 +++++++------ .../Dataflow/TrimAnalysisMethodCallPattern.cs | 3 +-- .../Linker.Dataflow/MethodBodyScanner.cs | 11 ++++------ .../ReflectionMethodBodyScanner.cs | 21 ++++++++----------- .../TrimAnalysisMethodCallPattern.cs | 3 +-- 6 files changed, 26 insertions(+), 38 deletions(-) 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 5c498fa7214cb7..76da7a29ab8c98 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs @@ -1284,14 +1284,12 @@ private void HandleCall( var dereferencedMethodParams = new List(); foreach (var argument in methodArguments) dereferencedMethodParams.Add(DereferenceValue(callingMethodBody, offset, argument, locals, ref interproceduralState)); - MultiValue methodReturnValue; - HandleCall( + MultiValue methodReturnValue = HandleCall( callingMethodBody, calledMethod, opcode, offset, - new ValueNodeList(dereferencedMethodParams), - out methodReturnValue); + new ValueNodeList(dereferencedMethodParams)); if (isNewObj || !calledMethod.Signature.ReturnType.IsVoid) currentStack.Push(new StackSlot(methodReturnValue)); @@ -1310,13 +1308,12 @@ private void HandleCall( } } - public abstract void HandleCall( + public abstract MultiValue HandleCall( MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, int offset, - ValueNodeList methodParams, - out MultiValue methodReturnValue); + ValueNodeList methodParams); // Limit tracking array values to 32 values for performance reasons. There are many arrays much longer than 32 elements in .NET, but the interesting ones for trimming are nearly always less than 32 elements. private const int MaxTrackedArrayValues = 32; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index 8688046101cc77..3d9cfd1daa7c81 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -261,7 +261,7 @@ protected override void HandleFieldTokenAccess(MethodIL methodBody, int offset, ProcessGenericArgumentDataFlow(accessedField); } - public override void HandleCall(MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, int offset, ValueNodeList methodParams, out MultiValue methodReturnValue) + public override MultiValue HandleCall(MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, int offset, ValueNodeList methodParams) { Debug.Assert(callingMethodBody.OwningMethod == _origin.MemberDefinition); @@ -293,26 +293,24 @@ public override void HandleCall(MethodIL callingMethodBody, MethodDesc calledMet ProcessGenericArgumentDataFlow(calledMethod); var diagnosticContext = new DiagnosticContext(_origin, diagnosticsEnabled: false, _logger); - HandleCall( + return HandleCall( callingMethodBody, calledMethod, operation, instanceValue, arguments, diagnosticContext, - _reflectionMarker, - out methodReturnValue); + _reflectionMarker); } - public static void HandleCall( + public static MultiValue HandleCall( MethodIL callingMethodBody, MethodDesc calledMethod, ILOpcode operation, MultiValue instanceValue, ImmutableArray argumentValues, DiagnosticContext diagnosticContext, - ReflectionMarker reflectionMarker, - out MultiValue methodReturnValue) + ReflectionMarker reflectionMarker) { var callingMethodDefinition = callingMethodBody.OwningMethod; Debug.Assert(callingMethodDefinition == diagnosticContext.Origin.MemberDefinition); @@ -324,8 +322,9 @@ public static void HandleCall( var handleCallAction = new HandleCallAction(reflectionMarker.Annotations, operation, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod.GetDisplayName()); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod(calledMethod); - if (!handleCallAction.Invoke (calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue)) + if (!handleCallAction.Invoke(calledMethod, instanceValue, argumentValues, intrinsicId, out MultiValue methodReturnValue)) throw new NotImplementedException($"Unhandled intrinsic {intrinsicId}"); + return methodReturnValue; } private static bool IsAotUnsafeDelegate(TypeDesc parameterType) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisMethodCallPattern.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisMethodCallPattern.cs index 9f2caf292999b4..48b0bbf5aff643 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisMethodCallPattern.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisMethodCallPattern.cs @@ -86,8 +86,7 @@ public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, Logger logger); ReflectionMethodBodyScanner.HandleCall(MethodBody, CalledMethod, Operation, Instance, Arguments, diagnosticContext, - reflectionMarker, - out MultiValue _); + reflectionMarker); } } } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs index 589ee454617dfc..34e5797ffd03b9 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -1090,13 +1090,11 @@ private void HandleCall ( var dereferencedMethodParams = new List (); foreach (var argument in methodArguments) dereferencedMethodParams.Add (DereferenceValue (argument, locals, ref interproceduralState)); - MultiValue methodReturnValue; - HandleCall ( + MultiValue methodReturnValue = HandleCall ( callingMethodBody, calledMethod, operation, - new ValueNodeList (dereferencedMethodParams), - out methodReturnValue); + new ValueNodeList (dereferencedMethodParams)); if (isNewObj || !calledMethod.ReturnsVoid ()) currentStack.Push (new StackSlot (methodReturnValue)); @@ -1114,12 +1112,11 @@ private void HandleCall ( public TypeDefinition? ResolveToTypeDefinition (TypeReference typeReference) => typeReference.ResolveToTypeDefinition (_context); - public abstract void HandleCall ( + public abstract MultiValue HandleCall ( MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, - ValueNodeList methodParams, - out MultiValue methodReturnValue); + ValueNodeList methodParams); // Limit tracking array values to 32 values for performance reasons. There are many arrays much longer than 32 elements in .NET, but the interesting ones for trimming are nearly always less than 32 elements. private const int MaxTrackedArrayValues = 32; diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 9a808d38a1f132..c33649404f6b14 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -117,19 +117,17 @@ protected override void HandleStoreParameter (MethodDefinition method, MethodPar protected override void HandleStoreMethodReturnValue (MethodDefinition method, MethodReturnValue returnValue, Instruction operation, MultiValue valueToStore) => HandleStoreValueWithDynamicallyAccessedMembers (returnValue, operation, valueToStore); - public override void HandleCall (MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, ValueNodeList methodParams, out MultiValue methodReturnValue) + public override MultiValue HandleCall (MethodBody callingMethodBody, MethodReference calledMethod, Instruction operation, ValueNodeList methodParams) { var reflectionProcessed = _markStep.ProcessReflectionDependency (callingMethodBody, operation); if (reflectionProcessed) { - methodReturnValue = UnknownValue.Instance; - return; + return UnknownValue.Instance; } Debug.Assert (callingMethodBody.Method == _origin.Provider); var calledMethodDefinition = _context.TryResolve (calledMethod); if (calledMethodDefinition == null) { - methodReturnValue = UnknownValue.Instance; - return; + return UnknownValue.Instance; } _origin = _origin.WithInstructionOffset (operation.Offset); @@ -153,7 +151,7 @@ public override void HandleCall (MethodBody callingMethodBody, MethodReference c )); var diagnosticContext = new DiagnosticContext (_origin, diagnosticsEnabled: false, _context); - HandleCall ( + return HandleCall ( operation, calledMethod, instanceValue, @@ -161,11 +159,10 @@ public override void HandleCall (MethodBody callingMethodBody, MethodReference c diagnosticContext, _reflectionMarker, _context, - _markStep, - out methodReturnValue); + _markStep); } - public static void HandleCall ( + public static MultiValue HandleCall ( Instruction operation, MethodReference calledMethod, MultiValue instanceValue, @@ -173,8 +170,7 @@ public static void HandleCall ( DiagnosticContext diagnosticContext, ReflectionMarker reflectionMarker, LinkContext context, - MarkStep markStep, - out MultiValue methodReturnValue) + MarkStep markStep) { var origin = diagnosticContext.Origin; var calledMethodDefinition = context.TryResolve (calledMethod); @@ -188,8 +184,9 @@ public static void HandleCall ( var handleCallAction = new HandleCallAction (context, operation, markStep, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod); var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (calledMethodDefinition); - if (!handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out methodReturnValue)) + if (!handleCallAction.Invoke (calledMethodDefinition, instanceValue, argumentValues, intrinsicId, out MultiValue methodReturnValue)) throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); + return methodReturnValue; } static bool IsComInterop (IMarshalInfoProvider marshalInfoProvider, TypeReference parameterType, LinkContext context) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs index a7b42a048c7c04..417e0240a9c0a1 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/TrimAnalysisMethodCallPattern.cs @@ -69,8 +69,7 @@ public void MarkAndProduceDiagnostics (ReflectionMarker reflectionMarker, MarkSt diagnosticContext, reflectionMarker, context, - markStep, - out MultiValue _); + markStep); } } }