From b3a74c74c01b0cb6ecf8a14db39020a381ad94aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 10 Apr 2024 11:14:40 +0200 Subject: [PATCH 1/2] Don't emit analyzer warnings for MakeGeneric understood by ILC Follow up to #99037. Resolves #81204. When `MakeGenericXXX` API is used in the "bridging constraints" pattern and all types/members involved are statically known, don't emit warning from the Roslyn analyzer since ILC will do the right thing and ensure this works. --- .../RequiresAnalyzerBase.cs | 10 ++ .../RequiresDynamicCodeAnalyzer.cs | 74 +++++++++ .../TrimAnalysisMethodCallPattern.cs | 7 +- .../RequiresDynamicCodeAnalyzerTests.cs | 143 ++++++++++++++++++ 4 files changed, 232 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index c31a69a24d24b..28118233b656a 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; +using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer { @@ -342,5 +343,14 @@ internal bool CheckAndCreateRequiresDiagnostic ( incompatibleMembers, out diagnostic); } + + internal virtual bool IsIntrinsicallyHandled ( + IMethodSymbol calledMethod, + MultiValue instance, + ImmutableArray arguments + ) + { + return false; + } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs index 34bb7808d2031..4e5c5d26a253e 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs @@ -5,8 +5,11 @@ using System.Collections.Immutable; using System.Linq; using ILLink.Shared; +using ILLink.Shared.TrimAnalysis; +using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer { @@ -38,6 +41,77 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer); + internal override bool IsIntrinsicallyHandled (IMethodSymbol calledMethod, MultiValue instance, ImmutableArray arguments) { + MethodProxy method = new (calledMethod); + var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method); + + switch (intrinsicId) { + case IntrinsicId.Type_MakeGenericType: { + if (!instance.IsEmpty ()) { + foreach (var value in instance.AsEnumerable ()) { + if (value is SystemTypeValue) { + if (!IsKnownInstantiation (arguments[0])) { + return false; + } + } else { + return false; + } + } + } + return true; + } + case IntrinsicId.MethodInfo_MakeGenericMethod: { + if (!instance.IsEmpty ()) { + foreach (var methodValue in instance.AsEnumerable ()) { + if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) { + if (!IsKnownInstantiation (arguments[0])) { + return false; + } + } else { + return false; + } + } + } + return true; + } + } + + return false; + + static bool IsKnownInstantiation(MultiValue genericParametersArray) { + var typesValue = genericParametersArray.AsSingleValue (); + if (typesValue is NullValue) { + // This will fail at runtime but no warning needed + 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; + } + + 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 (); + + if (singleValue is not SystemTypeValue and not GenericParameterValue and not NullableSystemTypeValue) { + return false; + } + } + + return true; + } + } + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { var runtimeFeaturesType = compilation.GetTypeByMetadataName ("System.Runtime.CompilerServices.RuntimeFeature"); if (runtimeFeaturesType == null) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs index 3dfd7fa285521..845ac7e884a91 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs @@ -84,8 +84,11 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) { - if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic(Operation, CalledMethod, OwningSymbol, context, FeatureContext, out Diagnostic? diag)) - diagnosticContext.AddDiagnostic(diag); + if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, CalledMethod, OwningSymbol, context, FeatureContext, out Diagnostic? diag) + && !requiresAnalyzer.IsIntrinsicallyHandled (CalledMethod, Instance, Arguments)) + { + diagnosticContext.AddDiagnostic (diag); + } } return diagnosticContext.Diagnostics; diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs index ecc5e2c3e7c98..c2d32af6a4015 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs @@ -2,8 +2,10 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; using System.Threading.Tasks; using ILLink.Shared; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Text; using Xunit; @@ -34,6 +36,19 @@ public RequiresDynamicCodeAttribute (string message) } }"; + static async Task VerifyRequiresDynamicCodeAnalyzer ( + string source, + params DiagnosticResult[] expected) + { + + await VerifyCS.VerifyAnalyzerAsync ( + source, + consoleApplication: false, + TestCaseUtils.UseMSBuildProperties (MSBuildPropertyOptionNames.EnableAotAnalyzer), + Array.Empty (), + expected); + } + static Task VerifyRequiresDynamicCodeCodeFix ( string source, string fixedSource, @@ -332,5 +347,133 @@ private int M2 { }; return VerifyRequiresDynamicCodeCodeFix (src, fix, diag, Array.Empty ()); } + + [Fact] + public Task MakeGenericTypeWithAllKnownTypes () + { + const string src = $$""" + class C + { + public void M() => typeof(Gen<>).MakeGenericType(typeof(object)); + } + class Gen { } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src); + } + + [Fact] + public Task MakeGenericTypeWithAllKnownTypesInGenericContext () + { + const string src = $$""" + class C + { + public void M() => typeof(Gen<>).MakeGenericType(typeof(T)); + } + class Gen { } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src); + } + + [Fact] + public Task MakeGenericTypeWithUnknownDefinition () + { + const string src = $$""" + using System; + class C + { + public void M() => GetDefinition().MakeGenericType(typeof(object)); + static Type GetDefinition() => typeof(Gen<>); + } + class Gen { } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src, + // (4,21): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. + VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 68).WithArguments ("System.Type.MakeGenericType(params Type[])", " The native code for this instantiation might not be available at runtime.", "")); + } + + [Fact] + public Task MakeGenericTypeWithUnknownArgument () + { + const string src = $$""" + using System; + class C + { + public void M() => typeof(Gen<>).MakeGenericType(GetObject()); + static Type GetObject() => typeof(object); + } + class Gen { } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src, + // (4,21): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. + VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 63).WithArguments ("System.Type.MakeGenericType(params Type[])", " The native code for this instantiation might not be available at runtime.", "")); + } + + [Fact] + public Task MakeGenericMethodWithAllKnownTypes () + { + const string src = $$""" + class C + { + public void M() => typeof(C).GetMethod(nameof(N)).MakeGenericMethod(typeof(object)); + public void N() { } + } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src); + } + + [Fact] + public Task MakeGenericMethodWithAllKnownTypesInGenericContext () + { + const string src = $$""" + class C + { + public void M() => typeof(C).GetMethod(nameof(N)).MakeGenericMethod(typeof(T)); + public void N() { } + } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src); + } + + [Fact] + public Task MakeGenericMethodWithUnknownDefinition () + { + const string src = $$""" + using System.Reflection; + class C + { + public void M() => GetMethodInfo().MakeGenericMethod(typeof(object)); + public void N() { } + public MethodInfo GetMethodInfo() => typeof(C).GetMethod(nameof(N)); + } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src, + // (4,21): warning IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. + VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 70).WithArguments ("System.Reflection.MethodInfo.MakeGenericMethod(params Type[])", " The native code for this instantiation might not be available at runtime.", "")); + } + + [Fact] + public Task MakeGenericMethodWithUnknownArgument () + { + const string src = $$""" + using System; + class C + { + public void M() => typeof(C).GetMethod(nameof(N)).MakeGenericMethod(GetObject()); + public void N() { } + static Type GetObject() => typeof(object); + } + """; + + return VerifyRequiresDynamicCodeAnalyzer (src, + // (4,21): warning IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. + VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 82).WithArguments ("System.Reflection.MethodInfo.MakeGenericMethod(params Type[])", " The native code for this instantiation might not be available at runtime.", "")); + } } } From ddf26e553603d1eb34fd1ccbe998489cedaebebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 12 Apr 2024 07:36:56 +0200 Subject: [PATCH 2/2] More tests --- .../DataFlowTests.cs | 6 ++ .../DataFlowTests.g.cs | 6 ++ .../DataFlow/MakeGenericDataflowIntrinsics.cs | 72 +++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataflowIntrinsics.cs diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index 087c7afea46a5..c3ed28034906a 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -197,6 +197,12 @@ public Task MakeGenericDataFlow () return RunTest (); } + [Fact] + public Task MakeGenericDataflowIntrinsics () + { + return RunTest (); + } + [Fact] public Task MethodByRefReturnDataFlow () { diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/DataFlowTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/DataFlowTests.g.cs index 5c3bd5a9d6654..0a390275ea81c 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/DataFlowTests.g.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/DataFlowTests.g.cs @@ -19,6 +19,12 @@ public Task GenericParameterDataFlowMarking () return RunTest (allowMissingWarnings: true); } + [Fact] + public Task MakeGenericDataflowIntrinsics () + { + return RunTest (allowMissingWarnings: true); + } + [Fact] public Task MethodByRefParameterDataFlow () { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataflowIntrinsics.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataflowIntrinsics.cs new file mode 100644 index 0000000000000..4fe39ab6da1ca --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataflowIntrinsics.cs @@ -0,0 +1,72 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using Mono.Linker.Tests.Cases.Expectations.Assertions; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + class MakeGenericDataflowIntrinsics + { + public static void Main () + { + MakeGenericType.Test (); + MakeGenericMethod.Test (); + } + + class MakeGenericType + { + class Gen { } + + static Type GrabUnknownType () => null; + + public static void Test () + { + TestRecognizedIntrinsic (); + TestRecognizedGenericIntrinsic (); + TestUnknownOwningType (); + TestUnknownArgument (); + } + + public static void TestRecognizedIntrinsic () => typeof (Gen<>).MakeGenericType (typeof (object)); + + public static void TestRecognizedGenericIntrinsic () => typeof (Gen<>).MakeGenericType (typeof (T)); + + [ExpectedWarning ("IL2055", nameof (Type.MakeGenericType))] + [ExpectedWarning ("IL3050", nameof (Type.MakeGenericType), ProducedBy = Tool.Analyzer | Tool.NativeAot)] + public static void TestUnknownOwningType () => GrabUnknownType ().MakeGenericType (typeof (object)); + + [ExpectedWarning ("IL3050", nameof (Type.MakeGenericType), ProducedBy = Tool.Analyzer | Tool.NativeAot)] + public static void TestUnknownArgument () => typeof (Gen<>).MakeGenericType (GrabUnknownType ()); + } + + class MakeGenericMethod + { + public static void Gen () { } + + static MethodInfo GrabUnknownMethod () => null; + + static Type GrabUnknownType () => null; + + public static void Test () + { + TestRecognizedIntrinsic (); + TestRecognizedGenericIntrinsic (); + TestUnknownOwningMethod (); + TestUnknownArgument (); + } + + public static void TestRecognizedIntrinsic () => typeof (MakeGenericMethod).GetMethod (nameof (Gen)).MakeGenericMethod (typeof (object)); + + public static void TestRecognizedGenericIntrinsic () => typeof (MakeGenericMethod).GetMethod (nameof (Gen)).MakeGenericMethod (typeof (T)); + + [ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod))] + [ExpectedWarning ("IL3050", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Analyzer | Tool.NativeAot)] + public static void TestUnknownOwningMethod () => GrabUnknownMethod ().MakeGenericMethod (typeof (object)); + + [ExpectedWarning ("IL3050", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Analyzer | Tool.NativeAot)] + public static void TestUnknownArgument () => typeof (MakeGenericMethod).GetMethod (nameof (Gen)).MakeGenericMethod (GrabUnknownType()); + } + } +}