From 1f309408aa712bf9d12b60fad16e92dc619b242b Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 30 Apr 2024 15:38:49 -0700 Subject: [PATCH] Preserve static type info for return value of ctor (#101212) Instead of tracking the return value as "TopValue" or "unknown", this models the constructor as returning a value with a static type when called with newobj, letting us undo the workaround from https://github.com/dotnet/runtime/pull/101031. --- .../Common/Compiler/Dataflow/MethodProxy.cs | 2 + .../Compiler/Dataflow/FlowAnnotations.cs | 8 ++-- .../Compiler/Dataflow/HandleCallAction.cs | 11 ++--- .../Compiler/Dataflow/MethodReturnValue.cs | 6 ++- .../Dataflow/ReflectionMethodBodyScanner.cs | 5 ++- .../TrimAnalysis/FlowAnnotations.cs | 8 ++-- .../TrimAnalysis/HandleCallAction.cs | 5 ++- .../TrimAnalysis/MethodProxy.cs | 2 + .../TrimAnalysis/MethodReturnValue.cs | 10 +++-- .../TrimAnalysis/TrimAnalysisVisitor.cs | 4 +- .../TrimAnalysis/FlowAnnotations.cs | 4 +- .../TrimAnalysis/HandleCallAction.cs | 24 +++++----- .../TypeSystemProxy/MethodProxy.cs | 1 + .../linker/Linker.Dataflow/FlowAnnotations.cs | 8 ++-- .../Linker.Dataflow/HandleCallAction.cs | 5 ++- .../src/linker/Linker.Dataflow/MethodProxy.cs | 2 + .../Linker.Dataflow/MethodReturnValue.cs | 11 ++++- .../ReflectionMethodBodyScanner.cs | 5 ++- .../DataFlowTests.cs | 6 +++ .../DataFlow/IReflectDataflow.cs | 3 ++ .../DataFlow/MethodReturnParameterDataFlow.cs | 8 ++-- .../DataFlow/MethodThisDataFlow.cs | 2 +- .../DataFlow/ObjectGetTypeDataflow.cs | 44 +++++++++++++++++++ 23 files changed, 129 insertions(+), 55 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs diff --git a/src/coreclr/tools/Common/Compiler/Dataflow/MethodProxy.cs b/src/coreclr/tools/Common/Compiler/Dataflow/MethodProxy.cs index 3e282dfc76dcf9..cfb4bb12902958 100644 --- a/src/coreclr/tools/Common/Compiler/Dataflow/MethodProxy.cs +++ b/src/coreclr/tools/Common/Compiler/Dataflow/MethodProxy.cs @@ -66,6 +66,8 @@ internal partial ImmutableArray GetGenericParameters() return builder.ToImmutableArray(); } + internal partial bool IsConstructor() => Method.IsConstructor; + internal partial bool IsStatic() => Method.Signature.IsStatic; internal partial bool HasImplicitThis() => !Method.Signature.IsStatic; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index 730330a31a9bc6..fd8094bc702831 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -951,12 +951,12 @@ internal partial bool MethodRequiresDataFlowAnalysis(MethodProxy method) => RequiresDataflowAnalysisDueToSignature(method.Method); #pragma warning disable CA1822 // Other partial implementations are not in the ilc project - internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) #pragma warning restore CA1822 // Mark members as static - => new MethodReturnValue(method.Method, dynamicallyAccessedMemberTypes); + => new MethodReturnValue(method.Method, isNewObj, dynamicallyAccessedMemberTypes); - internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method) - => GetMethodReturnValue(method, GetReturnParameterAnnotation(method.Method)); + internal partial MethodReturnValue GetMethodReturnValue(MethodProxy method, bool isNewObj) + => GetMethodReturnValue(method, isNewObj, GetReturnParameterAnnotation(method.Method)); internal partial GenericParameterValue GetGenericParameterValue(GenericParameterProxy genericParameter) => new GenericParameterValue(genericParameter.GenericParameter, GetGenericParameterAnnotation(genericParameter.GenericParameter)); 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 c637185d15b539..7207efb6d03f59 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -39,6 +39,7 @@ public HandleCallAction( { _reflectionMarker = reflectionMarker; _operation = operation; + _isNewObj = operation == ILOpcode.newobj; _diagnosticContext = diagnosticContext; _callingMethod = callingMethod; _annotations = annotations; @@ -337,12 +338,6 @@ private partial bool TryHandleIntrinsic ( 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; @@ -390,7 +385,7 @@ private partial bool TryHandleIntrinsic ( 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)); + AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj)); } else if (staticType.IsSealed() || staticType.IsTypeOf("System", "Delegate")) { @@ -425,7 +420,7 @@ private partial bool TryHandleIntrinsic ( // 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)); + AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj, annotation)); } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs index 7bb244d8d2bf07..7b90ffe6404599 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodReturnValue.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using ILCompiler.Dataflow; using ILLink.Shared.DataFlow; @@ -16,9 +17,10 @@ namespace ILLink.Shared.TrimAnalysis /// internal partial record MethodReturnValue { - public MethodReturnValue(MethodDesc method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + public MethodReturnValue(MethodDesc method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) { - StaticType = method.Signature.ReturnType; + Debug.Assert(!isNewObj || method.IsConstructor, "isNewObj can only be true for constructors"); + StaticType = isNewObj ? method.OwningType : method.Signature.ReturnType; Method = method; DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes; } 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 3d9cfd1daa7c81..6a00b2df965762 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -118,7 +118,7 @@ protected override void Scan(MethodIL methodBody, ref InterproceduralState inter if (!methodBody.OwningMethod.Signature.ReturnType.IsVoid) { var method = methodBody.OwningMethod; - var methodReturnValue = _annotations.GetMethodReturnValue(method); + var methodReturnValue = _annotations.GetMethodReturnValue(method, isNewObj: false); if (methodReturnValue.DynamicallyAccessedMemberTypes != 0) HandleAssignmentPattern(_origin, ReturnValue, methodReturnValue, method.GetDisplayName()); } @@ -315,7 +315,8 @@ public static MultiValue HandleCall( var callingMethodDefinition = callingMethodBody.OwningMethod; Debug.Assert(callingMethodDefinition == diagnosticContext.Origin.MemberDefinition); - var annotatedMethodReturnValue = reflectionMarker.Annotations.GetMethodReturnValue(calledMethod); + bool isNewObj = operation == ILOpcode.newobj; + var annotatedMethodReturnValue = reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, isNewObj); Debug.Assert( RequiresReflectionMethodBodyScannerForCallSite(reflectionMarker.Annotations, calledMethod) || annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs index db0eb0af579ede..2fa967fdc9405d 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs @@ -103,11 +103,11 @@ public static DynamicallyAccessedMemberTypes GetTypeAnnotation(ITypeSymbol type) internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method) => RequiresDataFlowAnalysis (method.Method); - internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) - => new MethodReturnValue (method.Method, dynamicallyAccessedMemberTypes); + internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + => new MethodReturnValue (method.Method, isNewObj, dynamicallyAccessedMemberTypes); - internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method) - => GetMethodReturnValue (method, GetMethodReturnValueAnnotation (method.Method)); + internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj) + => GetMethodReturnValue (method, isNewObj, GetMethodReturnValueAnnotation (method.Method)); internal partial GenericParameterValue GetGenericParameterValue (GenericParameterProxy genericParameter) => new GenericParameterValue (genericParameter.TypeParameterSymbol); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs index 9212cdc298fefb..d41ccee2388a58 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs @@ -33,6 +33,7 @@ public HandleCallAction ( { _owningSymbol = owningSymbol; _operation = operation; + _isNewObj = operation.Kind == OperationKind.ObjectCreation; _diagnosticContext = diagnosticContext; _annotations = FlowAnnotations.Instance; _reflectionAccessAnalyzer = default; @@ -86,7 +87,7 @@ private partial bool TryHandleIntrinsic ( 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)); + AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj)); } 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 @@ -106,7 +107,7 @@ private partial bool TryHandleIntrinsic ( AddReturnValue (new SystemTypeValue (new (staticType))); } else { var annotation = FlowAnnotations.GetTypeAnnotation (staticType); - AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, annotation)); + AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj, annotation)); } } break; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs index 5001e5836eaccb..0b274879440f51 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodProxy.cs @@ -46,6 +46,8 @@ internal partial ImmutableArray GetGenericParameters () return builder.ToImmutableArray (); } + internal partial bool IsConstructor () => Method.IsConstructor (); + internal partial bool IsStatic () => Method.IsStatic; internal partial bool HasImplicitThis () => Method.HasImplicitThis (); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs index 6a84896e577a86..d11181ad96d250 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs @@ -2,6 +2,7 @@ // 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 ILLink.RoslynAnalyzer; using ILLink.Shared.DataFlow; @@ -11,16 +12,17 @@ namespace ILLink.Shared.TrimAnalysis { internal partial record MethodReturnValue { - public MethodReturnValue (IMethodSymbol methodSymbol) - : this (methodSymbol, FlowAnnotations.GetMethodReturnValueAnnotation (methodSymbol)) + public MethodReturnValue (IMethodSymbol methodSymbol, bool isNewObj) + : this (methodSymbol, isNewObj, FlowAnnotations.GetMethodReturnValueAnnotation (methodSymbol)) { } - public MethodReturnValue (IMethodSymbol methodSymbol, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + public MethodReturnValue (IMethodSymbol methodSymbol, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) { + Debug.Assert (!isNewObj || methodSymbol.MethodKind == MethodKind.Constructor, "isNewObj can only be true for constructors"); MethodSymbol = methodSymbol; DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes; - StaticType = new (methodSymbol.ReturnType); + StaticType = new (isNewObj ? methodSymbol.ContainingType : methodSymbol.ReturnType); } public readonly IMethodSymbol MethodSymbol; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 4ff9333c55904b..c3aaa62d99ba97 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -116,7 +116,7 @@ public override MultiValue VisitConversion (IConversionOperation operation, Stat var value = base.VisitConversion (operation, state); if (operation.OperatorMethod != null) - return operation.OperatorMethod.ReturnType.IsTypeInterestingForDataflow () ? new MethodReturnValue (operation.OperatorMethod) : value; + return operation.OperatorMethod.ReturnType.IsTypeInterestingForDataflow () ? new MethodReturnValue (operation.OperatorMethod, isNewObj: false) : value; // TODO - is it possible to have annotation on the operator method parameters? // if so, will these be checked here? @@ -341,7 +341,7 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera return; if (method.ReturnType.IsTypeInterestingForDataflow ()) { - var returnParameter = new MethodReturnValue (method); + var returnParameter = new MethodReturnValue (method, isNewObj: false); TrimAnalysisPatterns.Add ( new TrimAnalysisAssignmentPattern (returnValue, returnParameter, operation, OwningSymbol, featureContext), diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/FlowAnnotations.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/FlowAnnotations.cs index 7ca5df5a6b8fe4..4ef9a9ce61a41f 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/FlowAnnotations.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/FlowAnnotations.cs @@ -19,9 +19,9 @@ partial class FlowAnnotations { internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method); - internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes); + internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes); - internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method); + internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj); internal partial GenericParameterValue GetGenericParameterValue (GenericParameterProxy genericParameter); diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs index 1ba45a7668bff0..ff2a393928deaf 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/HandleCallAction.cs @@ -26,6 +26,7 @@ internal partial struct HandleCallAction private readonly DiagnosticContext _diagnosticContext; private readonly FlowAnnotations _annotations; private readonly RequireDynamicallyAccessedMembersAction _requireDynamicallyAccessedMembersAction; + private readonly bool _isNewObj; public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnlyList argumentValues, IntrinsicId intrinsicId, out MultiValue methodReturnValue) { @@ -37,11 +38,12 @@ public bool Invoke (MethodProxy calledMethod, MultiValue instanceValue, IReadOnl // 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); + var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod, _isNewObj); bool returnsVoid = calledMethod.ReturnsVoid (); - methodReturnValue = maybeMethodReturnValue ?? (returnsVoid ? - MultiValueLattice.Top : - annotatedMethodReturnValue); + methodReturnValue = maybeMethodReturnValue ?? + ((returnsVoid && !_isNewObj) + ? MultiValueLattice.Top + : annotatedMethodReturnValue); // Validate that the return value has the correct annotations as per the method return value annotations if (annotatedMethodReturnValue.DynamicallyAccessedMemberTypes != 0) { @@ -80,7 +82,7 @@ bool TryHandleSharedIntrinsic ( MultiValue? returnValue = methodReturnValue = null; bool requiresDataFlowAnalysis = _annotations.MethodRequiresDataFlowAnalysis (calledMethod); - var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod); + var annotatedMethodReturnValue = _annotations.GetMethodReturnValue (calledMethod, _isNewObj); Debug.Assert (requiresDataFlowAnalysis || annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None); switch (intrinsicId) { @@ -237,7 +239,7 @@ GenericParameterValue genericParam && valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.All) returnMemberTypes = DynamicallyAccessedMemberTypes.All; - AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, returnMemberTypes)); + AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, returnMemberTypes)); } } } @@ -544,7 +546,7 @@ GenericParameterValue genericParam // We only applied the annotation based on binding flags, so we will keep the necessary types // but we will not keep anything on them. So the return value has no known annotations on it - AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, DynamicallyAccessedMemberTypes.None)); + AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.None)); } } } else if (value is NullValue) { @@ -558,9 +560,9 @@ GenericParameterValue genericParam // since All applies recursively to all nested type (see MarkStep.MarkEntireType). // Otherwise we only mark the nested type itself, nothing on it, so the return value has no annotation on it. if (value is ValueWithDynamicallyAccessedMembers { DynamicallyAccessedMemberTypes: DynamicallyAccessedMemberTypes.All }) - AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, DynamicallyAccessedMemberTypes.All)); + AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.All)); else - AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, DynamicallyAccessedMemberTypes.None)); + AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.None)); } } } @@ -832,7 +834,7 @@ GenericParameterValue genericParam } else if (typeNameValue is ValueWithDynamicallyAccessedMembers valueWithDynamicallyAccessedMembers && valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes != 0) { // Propagate the annotation from the type name to the return value. Annotation on a string value will be fulfilled whenever a value is assigned to the string with annotation. // So while we don't know which type it is, we can guarantee that it will fulfill the annotation. - AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes)); + AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, valueWithDynamicallyAccessedMembers.DynamicallyAccessedMemberTypes)); } else { _diagnosticContext.AddDiagnostic (DiagnosticId.UnrecognizedTypeNameInTypeGetType, calledMethod.GetDisplayName ()); AddReturnValue (MultiValueLattice.Top); @@ -956,7 +958,7 @@ GenericParameterValue genericParam propagatedMemberTypes |= DynamicallyAccessedMemberTypes.Interfaces; } - AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, propagatedMemberTypes)); + AddReturnValue (_annotations.GetMethodReturnValue (calledMethod, _isNewObj, propagatedMemberTypes)); } else if (value is SystemTypeValue systemTypeValue) { if (TryGetBaseType (systemTypeValue.RepresentedType, out var baseType)) AddReturnValue (new SystemTypeValue (baseType.Value)); diff --git a/src/tools/illink/src/ILLink.Shared/TypeSystemProxy/MethodProxy.cs b/src/tools/illink/src/ILLink.Shared/TypeSystemProxy/MethodProxy.cs index 92ba44e630babf..008d1c1536ef77 100644 --- a/src/tools/illink/src/ILLink.Shared/TypeSystemProxy/MethodProxy.cs +++ b/src/tools/illink/src/ILLink.Shared/TypeSystemProxy/MethodProxy.cs @@ -56,6 +56,7 @@ internal bool HasParameterOfType (ParameterIndex parameterIndex, string fullType internal partial bool HasGenericParameters (); internal partial bool HasGenericParametersCount (int genericParameterCount); internal partial ImmutableArray GetGenericParameters (); + internal partial bool IsConstructor (); internal partial bool IsStatic (); internal partial bool HasImplicitThis (); internal partial bool ReturnsVoid (); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 0c257fa068d898..3b148cf3a1ac59 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -682,11 +682,11 @@ public FieldAnnotation (FieldDefinition field, DynamicallyAccessedMemberTypes an internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method) => RequiresDataFlowAnalysis (method.Method); - internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) - => new MethodReturnValue (method.Method.ReturnType.ResolveToTypeDefinition (_context), method.Method, dynamicallyAccessedMemberTypes); + internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + => MethodReturnValue.Create (method.Method, isNewObj, dynamicallyAccessedMemberTypes, _context); - internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method) - => GetMethodReturnValue (method, GetReturnParameterAnnotation (method.Method)); + internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj) + => GetMethodReturnValue (method, isNewObj, GetReturnParameterAnnotation (method.Method)); internal partial GenericParameterValue GetGenericParameterValue (GenericParameterProxy genericParameter) => new GenericParameterValue (genericParameter.GenericParameter, GetGenericParameterAnnotation (genericParameter.GenericParameter)); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 6721ec63478fac..998d9a06fdaeb6 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -37,6 +37,7 @@ public HandleCallAction ( { _context = context; _operation = operation; + _isNewObj = operation.OpCode == OpCodes.Newobj; _markStep = markStep; _reflectionMarker = reflectionMarker; _diagnosticContext = diagnosticContext; @@ -122,7 +123,7 @@ private partial bool TryHandleIntrinsic ( 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)); + AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj)); } 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 @@ -151,7 +152,7 @@ private partial bool TryHandleIntrinsic ( // 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)); + AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj, annotation)); } } } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodProxy.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodProxy.cs index a4ca0634a81f8d..6e6fd7a1bf2159 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodProxy.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodProxy.cs @@ -59,6 +59,8 @@ internal partial ImmutableArray GetGenericParameters () return builder.ToImmutableArray (); } + internal partial bool IsConstructor () => Method.IsConstructor; + internal partial bool IsStatic () => Method.IsStatic; internal partial bool HasImplicitThis () => Method.HasImplicitThis (); diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs index 9197a7bda6be10..bec6951b17e4d7 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs @@ -2,9 +2,11 @@ // 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 ILLink.Shared.DataFlow; using Mono.Cecil; +using Mono.Linker; using Mono.Linker.Dataflow; using TypeDefinition = Mono.Cecil.TypeDefinition; @@ -16,7 +18,14 @@ namespace ILLink.Shared.TrimAnalysis /// internal partial record MethodReturnValue { - public MethodReturnValue (TypeDefinition? staticType, MethodDefinition method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + public static MethodReturnValue Create (MethodDefinition method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, LinkContext context) + { + Debug.Assert (!isNewObj || method.IsConstructor, "isNewObj can only be true for constructors"); + var staticType = isNewObj ? method.DeclaringType : method.ReturnType.ResolveToTypeDefinition (context); + return new MethodReturnValue (staticType, method, dynamicallyAccessedMemberTypes); + } + + private MethodReturnValue (TypeDefinition? staticType, MethodDefinition method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) { StaticType = staticType == null ? null : new (staticType); Method = method; diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index c33649404f6b14..3f5109bfeff9bd 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -75,7 +75,7 @@ protected override void Scan (MethodIL methodIL, ref InterproceduralState interp if (!methodIL.Method.ReturnsVoid ()) { var method = methodIL.Method; - var methodReturnValue = _annotations.GetMethodReturnValue (method); + var methodReturnValue = _annotations.GetMethodReturnValue (method, isNewObj: false); if (methodReturnValue.DynamicallyAccessedMemberTypes != 0) HandleAssignmentPattern (_origin, ReturnValue, methodReturnValue); } @@ -179,7 +179,8 @@ public static MultiValue HandleCall ( Debug.Assert (callingMethodDefinition != null); bool requiresDataFlowAnalysis = context.Annotations.FlowAnnotations.RequiresDataFlowAnalysis (calledMethodDefinition); - var annotatedMethodReturnValue = context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethodDefinition); + bool isNewObj = operation.OpCode.Code == Code.Newobj; + var annotatedMethodReturnValue = context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethodDefinition, isNewObj); Debug.Assert (requiresDataFlowAnalysis || annotatedMethodReturnValue.DynamicallyAccessedMemberTypes == DynamicallyAccessedMemberTypes.None); var handleCallAction = new HandleCallAction (context, operation, markStep, reflectionMarker, diagnosticContext, callingMethodDefinition, calledMethod); diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index c3ed28034906ad..3a95e6ea046cbd 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -313,6 +313,12 @@ public Task NullableAnnotations () return RunTest (); } + [Fact] + public Task ObjectGetTypeDataflow () + { + return RunTest (); + } + [Fact] public Task PropertyDataFlow () { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/IReflectDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/IReflectDataflow.cs index ea14e8b4f2867f..db2bc09e13e59b 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/IReflectDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/IReflectDataflow.cs @@ -164,6 +164,9 @@ class TestType } [Kept] + // MyReflectOverType is not intrinsically understood by the analysis, so + // it doesn't satisfy the PublicFields | NonPublicFields requirement. + [ExpectedWarning ("IL2075")] public static void Test () { new MyReflectOverType (typeof (TestType)).GetFields (BindingFlags.Instance | BindingFlags.Public); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodReturnParameterDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodReturnParameterDataFlow.cs index 8f3adcbdaa0889..14c881bf2e4bdd 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodReturnParameterDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodReturnParameterDataFlow.cs @@ -190,7 +190,7 @@ class AnnotationOnUnsupportedReturnType { class UnsupportedType { - [UnexpectedWarning ("IL2082", Tool.Analyzer, "")] // https://github.com/dotnet/runtime/issues/101211 + [UnexpectedWarning ("IL2082", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")] public UnsupportedType () { RequirePublicFields (this); } @@ -201,8 +201,7 @@ public UnsupportedType () { [ExpectedWarning ("IL2106")] // Linker and NativeAot should not produce IL2073 // They produce dataflow warnings despite the invalid annotations. - // https://github.com/dotnet/runtime/issues/101211 - [ExpectedWarning ("IL2073", Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning ("IL2073", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/101211")] [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] static UnsupportedType GetWithPublicMethods () { return GetUnsupportedTypeInstance (); @@ -215,12 +214,13 @@ static void RequirePublicFields ( { } - [ExpectedWarning ("IL2072", Tool.Analyzer, "")] // https://github.com/dotnet/runtime/issues/101211 + [ExpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")] static void TestMethodReturnValue () { var t = GetWithPublicMethods (); RequirePublicFields (t); } + [ExpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")] static void TestCtorReturnValue () { var t = new UnsupportedType (); RequirePublicFields (t); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodThisDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodThisDataFlow.cs index 50ae6925bdb4ef..c57d784bce24db 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodThisDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodThisDataFlow.cs @@ -122,7 +122,7 @@ static void RequirePublicFields ( [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] UnsupportedType unsupportedTypeInstance) { } - [ExpectedWarning ("IL2075", nameof (UnsupportedType), nameof (UnsupportedType.GetMethod), Tool.Analyzer, "")] // BUG + [ExpectedWarning ("IL2075", nameof (UnsupportedType), nameof (UnsupportedType.GetMethod), Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101211")] static void TestMethodThisParameter () { var t = GetUnsupportedTypeInstance (); t.GetMethod ("foo"); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs new file mode 100644 index 00000000000000..e089abd68955b3 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs @@ -0,0 +1,44 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + public class ObjectGetTypeDataflow + { + public static void Main () + { + SealedConstructorAsSource.Test (); + } + + class SealedConstructorAsSource + { + [KeptMember (".ctor()")] + public class Base + { + } + + [KeptMember (".ctor()")] + [KeptBaseType (typeof (Base))] + public sealed class Derived : Base + { + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (Method))] + public void Method () { } + } + + [ExpectedWarning ("IL2026", nameof (Derived.Method))] + public static void Test () + { + new Derived ().GetType ().GetMethod ("Method"); + } + } + } +}