From 9407c9cfa85cb7169cca47dfb350d5c2ca35be88 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 12 Jul 2024 14:07:32 -0700 Subject: [PATCH] Fix type parsing issues in ILLink and ILC (#104060) - Fixes https://github.com/dotnet/runtime/issues/98955 We will now produce a warning when a non-assembly-qualified type flows into a string location annotated with DynamicallyAccessedMembers, and we don't try to look up or mark the type (since we don't know which assemblies will be searched at runtime by the Type.GetType call). - Fixes https://github.com/dotnet/runtime/issues/103906 The ILLink intrinsic handling for `Type.GetType` will now look in corelib for generic arguments, matching native AOT. This replaces the existing warning IL2105. This uses a new warning instead of repurposing IL2105, because we already documented IL2105 and older versions of ILLink will produce it. Best to avoid any confusion about them. --- docs/tools/illink/error-codes.md | 16 +++ eng/testing/tests.mobile.targets | 2 +- eng/testing/tests.singlefile.targets | 2 +- .../CustomAttributeTypeNameParser.cs | 41 +++++- .../Compiler/Dataflow/ReflectionMarker.cs | 9 +- .../TestCasesRunner/AssemblyChecker.cs | 1 + ...ypeDescriptionProviderAttributeCtorTest.cs | 2 +- .../DebuggerTypeProxyAttributeTests.cs | 2 +- .../DebuggerVisualizerAttributeTests.cs | 4 +- .../apple/build/AppleBuild.LocalBuild.targets | 2 +- .../illink/src/ILLink.Shared/DiagnosticId.cs | 5 +- .../src/ILLink.Shared/SharedStrings.resx | 10 +- ...RequireDynamicallyAccessedMembersAction.cs | 2 +- .../src/linker/CompatibilitySuppressions.xml | 12 ++ .../Linker.Steps/LinkAttributesParser.cs | 34 ++--- .../src/linker/Linker.Steps/MarkStep.cs | 4 - .../illink/src/linker/Linker/LinkContext.cs | 10 +- .../TypeNameResolver.WithDiagnostics.cs | 61 ++++----- .../src/linker/Linker/TypeNameResolver.cs | 13 ++ .../ComponentModel/TypeConverterOnMembers.cs | 4 +- .../DataFlow/ApplyTypeAnnotations.cs | 23 +++- .../DataFlow/AssemblyQualifiedNameDataflow.cs | 2 +- .../DataFlow/AttributeConstructorDataflow.cs | 2 +- .../DataFlow/AttributeFieldDataflow.cs | 3 +- .../DataFlow/AttributePropertyDataflow.cs | 8 +- .../GenericParameterWarningLocation.cs | 28 ++-- .../Dependencies/EscapedTypeNames.il | 4 +- .../Reflection/Dependencies/RequireHelper.cs | 14 ++ .../Reflection/TypeUsedViaReflection.cs | 123 +++++++++++++++--- .../TestCasesRunner/ResultChecker.cs | 1 + 30 files changed, 308 insertions(+), 136 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/RequireHelper.cs diff --git a/docs/tools/illink/error-codes.md b/docs/tools/illink/error-codes.md index 4850f92152b..282ff575a94 100644 --- a/docs/tools/illink/error-codes.md +++ b/docs/tools/illink/error-codes.md @@ -1870,6 +1870,22 @@ void TestMethod() } ``` +#### `IL2122`: Type 'typeName' is not assembly qualified. Type name strings used for dynamically accessing a type should be assembly qualified. + +- The type name string passed to a location with `DynamicallyAccessedMembers` requirements was not assembly-qualified, so the trimmer cannot guarantee that the type is preserved. Consider using an assembly-qualified name instead. + + ```C# + // warning IL2122: Type 'MyClass' is not assembly qualified. Type name strings used for dynamically accessing a type should be assembly qualified. + GetTypeWrapper("MyClass"); + + class MyClass { } + + // May be defined in another assembly, so at runtime Type.GetType will look in that assembly for "MyClass". + void GetTypeWrapper([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string typeName) + { + var type = Type.GetType(typeName); + } + ``` ## Single-File Warning Codes diff --git a/eng/testing/tests.mobile.targets b/eng/testing/tests.mobile.targets index e87458abd06..b599da5d228 100644 --- a/eng/testing/tests.mobile.targets +++ b/eng/testing/tests.mobile.targets @@ -29,7 +29,7 @@ true true - $(NoWarn);IL2103;IL2105;IL2025;IL2111 + $(NoWarn);IL2103;IL2025;IL2111;IL2122 false diff --git a/eng/testing/tests.singlefile.targets b/eng/testing/tests.singlefile.targets index 9e6e4d0b358..f40703bac41 100644 --- a/eng/testing/tests.singlefile.targets +++ b/eng/testing/tests.singlefile.targets @@ -29,7 +29,7 @@ $(CoreCLRAotSdkDir) $(NetCoreAppCurrentTestHostSharedFrameworkPath) $(NetCoreAppCurrentTestHostSharedFrameworkPath) - $(NoWarn);IL1005;IL2105;IL3000;IL3001;IL3002;IL3003;IL3050;IL3051;IL3052;IL3053 + $(NoWarn);IL1005;IL2122;IL3000;IL3001;IL3002;IL3003;IL3050;IL3051;IL3052;IL3053 partial true false diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs index 7f19644ad10..83e74b02dd9 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs @@ -36,13 +36,18 @@ namespace Internal.TypeSystem } public static TypeDesc GetTypeByCustomAttributeTypeNameForDataFlow(string name, ModuleDesc callingModule, - TypeSystemContext context, List referencedModules, out bool typeWasNotFoundInAssemblyNorBaseLibrary) + TypeSystemContext context, List referencedModules, bool needsAssemblyName, out bool failedBecauseNotFullyQualified) { - typeWasNotFoundInAssemblyNorBaseLibrary = false; - + failedBecauseNotFullyQualified = false; if (!TypeName.TryParse(name.AsSpan(), out TypeName parsed, s_typeNameParseOptions)) return null; + if (needsAssemblyName && !IsFullyQualified(parsed)) + { + failedBecauseNotFullyQualified = true; + return null; + } + TypeNameResolver resolver = new() { _context = context, @@ -52,8 +57,33 @@ namespace Internal.TypeSystem TypeDesc type = resolver.Resolve(parsed); - typeWasNotFoundInAssemblyNorBaseLibrary = resolver._typeWasNotFoundInAssemblyNorBaseLibrary; return type; + + static bool IsFullyQualified(TypeName typeName) + { + if (typeName.AssemblyName is null) + { + return false; + } + + if (typeName.IsArray || typeName.IsPointer || typeName.IsByRef) + { + return IsFullyQualified(typeName.GetElementType()); + } + + if (typeName.IsConstructedGenericType) + { + foreach (var typeArgument in typeName.GetGenericArguments()) + { + if (!IsFullyQualified(typeArgument)) + { + return false; + } + } + } + + return true; + } } private struct TypeNameResolver @@ -64,7 +94,6 @@ namespace Internal.TypeSystem internal Func _canonResolver; internal List _referencedModules; - internal bool _typeWasNotFoundInAssemblyNorBaseLibrary; internal TypeDesc Resolve(TypeName typeName) { @@ -136,8 +165,6 @@ namespace Internal.TypeSystem return type; } } - - _typeWasNotFoundInAssemblyNorBaseLibrary = true; } if (_throwIfNotFound) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs index d4e81f23b0c..bf966393c0f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs @@ -89,12 +89,13 @@ namespace ILCompiler.Dataflow List referencedModules = new(); TypeDesc foundType = CustomAttributeTypeNameParser.GetTypeByCustomAttributeTypeNameForDataFlow(typeName, callingModule, diagnosticContext.Origin.MemberDefinition!.Context, - referencedModules, out bool typeWasNotFoundInAssemblyNorBaseLibrary); + referencedModules, needsAssemblyName, out bool failedBecauseNotFullyQualified); if (foundType == null) { - if (needsAssemblyName && typeWasNotFoundInAssemblyNorBaseLibrary) - diagnosticContext.AddDiagnostic(DiagnosticId.TypeWasNotFoundInAssemblyNorBaseLibrary, typeName); - + if (failedBecauseNotFullyQualified) + { + diagnosticContext.AddDiagnostic(DiagnosticId.TypeNameIsNotAssemblyQualified, typeName); + } type = default; return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs index b2a6ea4cee9..bbe9184c4d7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs @@ -12,6 +12,7 @@ using Internal.TypeSystem.Ecma; using Mono.Cecil; using Mono.Cecil.Cil; using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; using Mono.Linker.Tests.Extensions; using Xunit; using MetadataType = Internal.TypeSystem.MetadataType; diff --git a/src/libraries/System.ObjectModel/tests/TrimmingTests/TypeDescriptionProviderAttributeCtorTest.cs b/src/libraries/System.ObjectModel/tests/TrimmingTests/TypeDescriptionProviderAttributeCtorTest.cs index cfdb6635875..1d46eb8e401 100644 --- a/src/libraries/System.ObjectModel/tests/TrimmingTests/TypeDescriptionProviderAttributeCtorTest.cs +++ b/src/libraries/System.ObjectModel/tests/TrimmingTests/TypeDescriptionProviderAttributeCtorTest.cs @@ -14,7 +14,7 @@ class Program /// static int Main(string[] args) { - TypeDescriptionProviderAttribute attr = new TypeDescriptionProviderAttribute("Program+MyTypeDescriptionProvider"); + TypeDescriptionProviderAttribute attr = new TypeDescriptionProviderAttribute("Program+MyTypeDescriptionProvider, project"); if (!RunTest(attr)) { return -1; diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerTypeProxyAttributeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerTypeProxyAttributeTests.cs index b80f4a1d61b..382d8edbfd8 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerTypeProxyAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerTypeProxyAttributeTests.cs @@ -53,7 +53,7 @@ class Program } } - [DebuggerTypeProxy("Program+MyClassWithProxyStringProxy")] + [DebuggerTypeProxy("Program+MyClassWithProxyStringProxy, project")] public class MyClassWithProxyString { public string Name { get; set; } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerVisualizerAttributeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerVisualizerAttributeTests.cs index dccaf03b4dd..51ace3fa295 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerVisualizerAttributeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/TrimmingTests/DebuggerVisualizerAttributeTests.cs @@ -113,8 +113,8 @@ class Program } } - [DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer")] - [DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer2", "Program+MyClassWithVisualizerStringVisualizerObjectSource")] + [DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer, project")] + [DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer2, project", "Program+MyClassWithVisualizerStringVisualizerObjectSource, project")] public class MyClassWithVisualizerString { public string Name { get; set; } diff --git a/src/mono/msbuild/apple/build/AppleBuild.LocalBuild.targets b/src/mono/msbuild/apple/build/AppleBuild.LocalBuild.targets index a28bff240a6..14df939e12f 100644 --- a/src/mono/msbuild/apple/build/AppleBuild.LocalBuild.targets +++ b/src/mono/msbuild/apple/build/AppleBuild.LocalBuild.targets @@ -37,7 +37,7 @@ true - $(NoWarn);IL2103;IL2105;IL2025;IL2111 + $(NoWarn);IL2103;IL2025;IL2111;IL2122