From 50d289cc2b945c8ee6b4cca5b71d8d137c09a762 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Mon, 6 Jun 2022 20:56:44 -0500 Subject: [PATCH] Fix linker ref param warnings (#70082) * Update dependencies from https://github.com/dotnet/linker build 20220523.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22270.1 -> To Version 7.0.100-1.22273.1 * Update dependencies from https://github.com/dotnet/linker build 20220524.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22270.1 -> To Version 7.0.100-1.22274.1 * Update dependencies from https://github.com/dotnet/linker build 20220525.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22270.1 -> To Version 7.0.100-1.22275.1 * Update dependencies from https://github.com/dotnet/linker build 20220530.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22270.1 -> To Version 7.0.100-1.22280.1 * Update dependencies from https://github.com/dotnet/linker build 20220531.1 Microsoft.NET.ILLink.Tasks From Version 7.0.100-1.22270.1 -> To Version 7.0.100-1.22281.1 * WIP * Supress warnings produced by ref param dataflow * Update comments and remove IL2118/9 suppressions * whitespace * Trailing whitespace * PR feedback * Fix wording Co-authored-by: dotnet-maestro[bot] Co-authored-by: Sven Boemer --- eng/Version.Details.xml | 4 ++-- eng/Versions.props | 2 +- .../src/System/Reflection/Emit/TypeBuilder.cs | 4 ++++ .../Reflection/RuntimeCustomAttributeData.cs | 2 +- .../src/System/RuntimeHandles.cs | 6 +++++ .../AssociatedMetadataTypeTypeDescriptor.cs | 23 +++++++++---------- .../src/ILLink/ILLink.Suppressions.xml | 2 +- 7 files changed, 26 insertions(+), 17 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index ed4ff82b1696f..8df000911ca19 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -238,9 +238,9 @@ https://github.com/dotnet/runtime 0864cc5539e0ddd109b443b0bee804878cd7ba76 - + https://github.com/dotnet/linker - b0479355a71752de103a2aac89af5cdcdb0455ad + 79b5653ccfd0663790b4dc4efe65c1285ede73fc https://github.com/dotnet/xharness diff --git a/eng/Versions.props b/eng/Versions.props index b35eea5354d8b..3dede7c2b7143 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -168,7 +168,7 @@ 7.0.0-preview-20220429.1 - 7.0.100-1.22270.1 + 7.0.100-1.22281.1 $(MicrosoftNETILLinkTasksVersion) 7.0.0-preview.5.22269.3 diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs index d2e84804adbe6..cc0c86e70a763 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs @@ -1699,6 +1699,10 @@ private EventBuilder DefineEventNoLock(string name, EventAttributes attributes, [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2083:UnrecognizedReflectionPattern", Justification = "Reflection.Emit is not subject to trimming")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2068:UnrecognizedReflectionPattern", + Justification = "Reflection.Emit is not subject to trimming")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2069:UnrecognizedReflectionPattern", + Justification = "Reflection.Emit is not subject to trimming")] [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] private TypeInfo? CreateTypeNoLock() { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index b3fdf8b21d259..ccd73948609d1 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1137,7 +1137,7 @@ private static object[] GetCustomAttributes( return result; } - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2065:UnrecognizedReflectionPattern", + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:MethodParameterDoesntMeetThisParameterRequirements", Justification = "Linker guarantees presence of all the constructor parameters, property setters and fields which are accessed by any " + "attribute instantiation which is present in the code linker has analyzed." + "As such the reflection usage in this method will never fail as those methods/fields will be present.")] diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 8bb7896559563..2639b5e62df87 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -237,6 +237,9 @@ internal static ReadOnlySpan CopyRuntimeTypeHandles(RuntimeTypeHandle[]? return outHandles; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:ParameterDoesntMeetParameterRequirements", + Justification = "The parameter 'type' is passed by ref to QCallTypeHandle which only instantiates" + + "the type using the public parameterless constructor and doesn't modify it")] internal static object CreateInstanceForAnotherGenericParameter( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] RuntimeType type, RuntimeType genericParameter) @@ -257,6 +260,9 @@ internal static object CreateInstanceForAnotherGenericParameter( return instantiatedObject!; } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:ParameterDoesntMeetParameterRequirements", + Justification = "The parameter 'type' is passed by ref to QCallTypeHandle which only instantiates" + + "the type using the public parameterless constructor and doesn't modify it")] internal static object CreateInstanceForAnotherGenericParameter( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] RuntimeType type, RuntimeType genericParameter1, diff --git a/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/AssociatedMetadataTypeTypeDescriptor.cs b/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/AssociatedMetadataTypeTypeDescriptor.cs index 252674bd53a62..619c47dc14d8a 100644 --- a/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/AssociatedMetadataTypeTypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/AssociatedMetadataTypeTypeDescriptor.cs @@ -113,21 +113,10 @@ public static void ValidateMetadataType( } } - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2063:UnrecognizedReflectionPattern", - Justification = "The cache is a dictionary which is hard to annotate. All values in the cache" + - "have annotation All (since we only ever add attribute.MetadataClassType which has All)." + - "But the call to TryGetValue doesn't carry the annotation so this warns when trying" + - "to return value.")] - // Better "fix" for the missing annotation would be a local funcion - // `bool TryGetAssociatedMetadataTypeFromCache(Type type, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] out Type? associatedMetadataType)` - // With the suppression on it - since that would be much more localized. - // Unfortunately this currently doesn't work due to an issue in the trimmer - // https://github.com/dotnet/linker/issues/2632 [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] public static Type? GetAssociatedMetadataType(Type type) { - Type? associatedMetadataType; - if (s_metadataTypeCache.TryGetValue(type, out associatedMetadataType)) + if (TryGetAssociatedMetadataTypeFromCache(type, out Type? associatedMetadataType)) { return associatedMetadataType; } @@ -140,6 +129,16 @@ public static void ValidateMetadataType( } s_metadataTypeCache.TryAdd(type, associatedMetadataType); return associatedMetadataType; + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:ParameterDoesntMeetParameterRequirements", + Justification = "The cache is a dictionary which is hard to annotate. All values in the cache" + + "have annotation All (since we only ever add attribute.MetadataClassType which has All)." + + "But the call to TryGetValue doesn't carry the annotation so this warns when trying" + + "to assign to the out parameter.")] + static bool TryGetAssociatedMetadataTypeFromCache(Type type, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] out Type? associatedMetadataType) + { + return s_metadataTypeCache.TryGetValue(type, out associatedMetadataType); + } } private static void CheckAssociatedMetadataType( diff --git a/src/libraries/System.Resources.Extensions/src/ILLink/ILLink.Suppressions.xml b/src/libraries/System.Resources.Extensions/src/ILLink/ILLink.Suppressions.xml index 226b691236890..42ebf43eee02d 100644 --- a/src/libraries/System.Resources.Extensions/src/ILLink/ILLink.Suppressions.xml +++ b/src/libraries/System.Resources.Extensions/src/ILLink/ILLink.Suppressions.xml @@ -27,7 +27,7 @@ ILLink - IL2062 + IL2067 member M:System.Resources.Extensions.PreserializedResourceWriter.AddResource(System.String,System.String,System.String)