From b84dcb67c8d0e4a9e45fbd71dc10fce515b16a8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 28 Mar 2024 10:32:21 +0100 Subject: [PATCH 1/2] Make it possible to trim custom attribute artifacts Pretty much any app uses custom attribute metadata due to `FlagsAttribute`. But not every app needs to activate custom attributes. We currently root unreachable code when custom attributes are not actually reflected on. This adds a check to make sure we only do this if there's a chance the attributes are looked at. --- ...CustomAttributeBasedDependencyAlgorithm.cs | 58 +++++++++++++------ .../CustomAttributeMetadataNode.cs | 18 +++++- .../ReflectedCustomAttributeNode.cs | 41 +++++++++++++ .../DependencyAnalysis/TypeMetadataNode.cs | 4 -- .../ILCompiler.Compiler.csproj | 1 + .../AttributeTrimming/AttributeKept.csproj | 9 +++ .../AttributeTrimming/AttributeTrimmed.csproj | 10 ++++ .../AttributeTrimming/AttributeTrimming.cs | 41 +++++++++++++ 8 files changed, 159 insertions(+), 23 deletions(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedCustomAttributeNode.cs create mode 100644 src/tests/nativeaot/AttributeTrimming/AttributeKept.csproj create mode 100644 src/tests/nativeaot/AttributeTrimming/AttributeTrimmed.csproj create mode 100644 src/tests/nativeaot/AttributeTrimming/AttributeTrimming.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs index 68e8b5f6f0c39..ee12a3fbbc120 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs @@ -128,10 +128,9 @@ private static void AddDependenciesDueToCustomAttributes(ref DependencyList depe // Make a new list in case we need to abort. var caDependencies = factory.MetadataManager.GetDependenciesForCustomAttribute(factory, constructor, decodedValue, parent) ?? new DependencyList(); - caDependencies.Add(factory.ReflectedMethod(constructor.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Attribute constructor"); - caDependencies.Add(factory.ReflectedType(constructor.OwningType), "Attribute type"); + caDependencies.Add(factory.MethodMetadata(constructor.GetTypicalMethodDefinition()), "Attribute constructor"); - if (AddDependenciesFromCustomAttributeBlob(caDependencies, factory, constructor.OwningType, decodedValue)) + if (AddDependenciesFromCustomAttributeBlob(caDependencies, factory, constructor.OwningType, decodedValue, metadataOnly: true)) { dependencies ??= new DependencyList(); dependencies.AddRange(caDependencies); @@ -154,11 +153,27 @@ private static void AddDependenciesDueToCustomAttributes(ref DependencyList depe } } - private static bool AddDependenciesFromCustomAttributeBlob(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, CustomAttributeValue value) + public static void AddDependenciesDueToCustomAttributeActivation(ref DependencyList dependencies, NodeFactory factory, EcmaModule module, CustomAttributeHandle attributeHandle) + { + MetadataReader reader = module.MetadataReader; + CustomAttribute attribute = reader.GetCustomAttribute(attributeHandle); + + dependencies ??= new DependencyList(); + + MethodDesc constructor = module.GetMethod(attribute.Constructor); + dependencies.Add(factory.ReflectedMethod(constructor.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Attribute constructor"); + + var attributeTypeProvider = new CustomAttributeTypeProvider(module); + + CustomAttributeValue decodedValue = attribute.DecodeValue(attributeTypeProvider); + AddDependenciesFromCustomAttributeBlob(dependencies, factory, constructor.OwningType, decodedValue, metadataOnly: false); + } + + private static bool AddDependenciesFromCustomAttributeBlob(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, CustomAttributeValue value, bool metadataOnly) { foreach (CustomAttributeTypedArgument decodedArgument in value.FixedArguments) { - if (!AddDependenciesFromCustomAttributeArgument(dependencies, factory, decodedArgument.Type, decodedArgument.Value)) + if (!AddDependenciesFromCustomAttributeArgument(dependencies, factory, decodedArgument.Type, decodedArgument.Value, metadataOnly)) return false; } @@ -166,7 +181,7 @@ private static bool AddDependenciesFromCustomAttributeBlob(DependencyList depend { if (decodedArgument.Kind == CustomAttributeNamedArgumentKind.Field) { - if (!AddDependenciesFromField(dependencies, factory, attributeType, decodedArgument.Name)) + if (!AddDependenciesFromField(dependencies, factory, attributeType, decodedArgument.Name, metadataOnly)) return false; } else @@ -174,18 +189,18 @@ private static bool AddDependenciesFromCustomAttributeBlob(DependencyList depend Debug.Assert(decodedArgument.Kind == CustomAttributeNamedArgumentKind.Property); // Reflection will need to reflection-invoke the setter at runtime. - if (!AddDependenciesFromPropertySetter(dependencies, factory, attributeType, decodedArgument.Name)) + if (!AddDependenciesFromPropertySetter(dependencies, factory, attributeType, decodedArgument.Name, metadataOnly)) return false; } - if (!AddDependenciesFromCustomAttributeArgument(dependencies, factory, decodedArgument.Type, decodedArgument.Value)) + if (!AddDependenciesFromCustomAttributeArgument(dependencies, factory, decodedArgument.Type, decodedArgument.Value, metadataOnly)) return false; } return true; } - private static bool AddDependenciesFromField(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string fieldName) + private static bool AddDependenciesFromField(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string fieldName, bool metadataOnly) { FieldDesc field = attributeType.GetField(fieldName); if (field is not null) @@ -193,7 +208,10 @@ private static bool AddDependenciesFromField(DependencyList dependencies, NodeFa if (factory.MetadataManager.IsReflectionBlocked(field)) return false; - dependencies.Add(factory.ReflectedField(field), "Custom attribute blob"); + if (metadataOnly) + dependencies.Add(factory.FieldMetadata(field), "Custom attribute blob"); + else + dependencies.Add(factory.ReflectedField(field), "Custom attribute blob"); return true; } @@ -202,13 +220,13 @@ private static bool AddDependenciesFromField(DependencyList dependencies, NodeFa TypeDesc baseType = attributeType.BaseType; if (baseType != null) - return AddDependenciesFromField(dependencies, factory, baseType, fieldName); + return AddDependenciesFromField(dependencies, factory, baseType, fieldName, metadataOnly); // Not found. This is bad metadata that will result in a runtime failure, but we shouldn't fail the compilation. return true; } - private static bool AddDependenciesFromPropertySetter(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string propertyName) + private static bool AddDependenciesFromPropertySetter(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string propertyName, bool metadataOnly) { EcmaType attributeTypeDefinition = (EcmaType)attributeType.GetTypeDefinition(); @@ -234,7 +252,10 @@ private static bool AddDependenciesFromPropertySetter(DependencyList dependencie setterMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(setterMethod, (InstantiatedType)attributeType); } - dependencies.Add(factory.ReflectedMethod(setterMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Custom attribute blob"); + if (metadataOnly) + dependencies.Add(factory.MethodMetadata(setterMethod.GetTypicalMethodDefinition()), "Custom attribute blob"); + else + dependencies.Add(factory.ReflectedMethod(setterMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Custom attribute blob"); } return true; @@ -245,13 +266,13 @@ private static bool AddDependenciesFromPropertySetter(DependencyList dependencie TypeDesc baseType = attributeType.BaseType; if (baseType != null) - return AddDependenciesFromPropertySetter(dependencies, factory, baseType, propertyName); + return AddDependenciesFromPropertySetter(dependencies, factory, baseType, propertyName, metadataOnly); // Not found. This is bad metadata that will result in a runtime failure, but we shouldn't fail the compilation. return true; } - private static bool AddDependenciesFromCustomAttributeArgument(DependencyList dependencies, NodeFactory factory, TypeDesc type, object value) + private static bool AddDependenciesFromCustomAttributeArgument(DependencyList dependencies, NodeFactory factory, TypeDesc type, object value, bool metadataOnly) { // If this is an initializer that refers to e.g. a blocked enum, we can't encode this attribute. if (factory.MetadataManager.IsReflectionBlocked(type)) @@ -259,7 +280,10 @@ private static bool AddDependenciesFromCustomAttributeArgument(DependencyList de // Reflection will need to be able to allocate this type at runtime // (e.g. this could be an array that needs to be allocated, or an enum that needs to be boxed). - dependencies.Add(factory.ReflectedType(type), "Custom attribute blob"); + if (metadataOnly) + TypeMetadataNode.GetMetadataDependencies(ref dependencies, factory, type, "Custom attribute blob"); + else + dependencies.Add(factory.ReflectedType(type), "Custom attribute blob"); if (type.UnderlyingType.IsPrimitive || type.IsString || value == null) return true; @@ -272,7 +296,7 @@ private static bool AddDependenciesFromCustomAttributeArgument(DependencyList de foreach (CustomAttributeTypedArgument arrayElement in (ImmutableArray>)value) { - if (!AddDependenciesFromCustomAttributeArgument(dependencies, factory, arrayElement.Type, arrayElement.Value)) + if (!AddDependenciesFromCustomAttributeArgument(dependencies, factory, arrayElement.Type, arrayElement.Value, metadataOnly)) return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs index 7b4706cde57ba..e8897d5c399ca 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; +using Internal.TypeSystem; + using ILCompiler.DependencyAnalysisFramework; namespace ILCompiler.DependencyAnalysis @@ -22,8 +24,22 @@ public CustomAttributeMetadataNode(ReflectableCustomAttribute customAttribute) _customAttribute = customAttribute; } + public override bool HasConditionalStaticDependencies => true; + public ReflectableCustomAttribute CustomAttribute => _customAttribute; + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) + { + // Presence of this type indicates that more than just the attribute metadata is needed: + // we also need runtime artifacts, such as the method body of the attribute constructor. + MetadataType nativeFormatType = factory.TypeSystemContext.SystemModule.GetType("System.Reflection.Runtime.CustomAttributes.NativeFormat", "NativeFormatCustomAttributeData"); + return [new CombinedDependencyListEntry( + new ReflectedCustomAttributeNode(CustomAttribute), + factory.ConstructedTypeSymbol(nativeFormatType), + "Attributes are activated" + )]; + } + // The metadata that the attribute depends on gets injected into the entity that owns the attribute. // This makes the dependency graph less "nice", but it avoids either having to walk the attribute // blob twice, or wasting memory holding on to dependencies here. @@ -39,9 +55,7 @@ protected override string GetName(NodeFactory factory) public override bool InterestingForDynamicDependencyAnalysis => false; public override bool HasDynamicDependencies => false; - public override bool HasConditionalStaticDependencies => false; public override bool StaticDependenciesAreComputed => true; - public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => null; public override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory factory) => null; } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedCustomAttributeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedCustomAttributeNode.cs new file mode 100644 index 0000000000000..f7d9d20504039 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedCustomAttributeNode.cs @@ -0,0 +1,41 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +using ILCompiler.DependencyAnalysisFramework; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Represents dependencies necessary to activate a custom attribute at runtime. + /// + internal sealed class ReflectedCustomAttributeNode : DependencyNodeCore + { + private readonly ReflectableCustomAttribute _customAttribute; + + public ReflectedCustomAttributeNode(ReflectableCustomAttribute customAttribute) + { + _customAttribute = customAttribute; + } + + public override IEnumerable GetStaticDependencies(NodeFactory factory) + { + DependencyList dependencies = null; + CustomAttributeBasedDependencyAlgorithm.AddDependenciesDueToCustomAttributeActivation(ref dependencies, factory, _customAttribute.Module, _customAttribute.CustomAttributeHandle); + return dependencies; + } + + protected override string GetName(NodeFactory factory) + { + return $"Custom attribute activation {_customAttribute.CustomAttributeHandle} in {_customAttribute.Module}"; + } + + public override bool InterestingForDynamicDependencyAnalysis => false; + public override bool HasDynamicDependencies => false; + public override bool HasConditionalStaticDependencies => false; + public override bool StaticDependenciesAreComputed => true; + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => null; + public override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory factory) => null; + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs index 87a2c829c0269..7fbcd5af7b0f3 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs @@ -53,10 +53,6 @@ public override IEnumerable GetStaticDependencies(NodeFacto if (_type.IsEnum) { - // A lot of the enum reflection actually happens on top of the respective MethodTable (e.g. getting the underlying type), - // so for enums also include their MethodTable. - dependencies.Add(factory.ReflectedType(_type), "Reflectable enum"); - // Enums are not useful without their literal fields. The literal fields are not referenced // from anywhere (source code reference to enums compiles to the underlying numerical constants in IL). foreach (FieldDesc enumField in _type.GetFields()) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 37c8c2108d917..0cf32334fd837 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -429,6 +429,7 @@ + diff --git a/src/tests/nativeaot/AttributeTrimming/AttributeKept.csproj b/src/tests/nativeaot/AttributeTrimming/AttributeKept.csproj new file mode 100644 index 0000000000000..17cb6448ee4fb --- /dev/null +++ b/src/tests/nativeaot/AttributeTrimming/AttributeKept.csproj @@ -0,0 +1,9 @@ + + + Exe + 0 + + + + + diff --git a/src/tests/nativeaot/AttributeTrimming/AttributeTrimmed.csproj b/src/tests/nativeaot/AttributeTrimming/AttributeTrimmed.csproj new file mode 100644 index 0000000000000..99ca5bad8c24e --- /dev/null +++ b/src/tests/nativeaot/AttributeTrimming/AttributeTrimmed.csproj @@ -0,0 +1,10 @@ + + + Exe + 0 + $(DefineConstants);TRIMMED + + + + + diff --git a/src/tests/nativeaot/AttributeTrimming/AttributeTrimming.cs b/src/tests/nativeaot/AttributeTrimming/AttributeTrimming.cs new file mode 100644 index 0000000000000..6eb95c7ae988c --- /dev/null +++ b/src/tests/nativeaot/AttributeTrimming/AttributeTrimming.cs @@ -0,0 +1,41 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics.CodeAnalysis; + +[My] +class Program +{ + static int Main() + { +#if !TRIMMED + typeof(Program).GetCustomAttributes(inherit: false); +#endif + + Type t = GetTypeSecretly(nameof(Canary)); + +#if TRIMMED + return t == null ? 100 : 101; +#else + return t != null ? 100 : 101; +#endif + } + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", + Justification = "That's the point")] + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2057:UnrecognizedReflectionPattern", + Justification = "That's the point")] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + static Type GetTypeSecretly(string name) => Type.GetType(name); +} + +class MyAttribute : Attribute +{ + public MyAttribute() + { + Type.GetType(nameof(Canary)).ToString(); + } +} + +class Canary { } From 03fd18b71908bb36b5263250412bf2a4a3b8a6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 28 Mar 2024 13:01:54 +0100 Subject: [PATCH 2/2] Fix multifile --- .../CustomAttributeBasedDependencyAlgorithm.cs | 10 ++++++++-- .../DependencyAnalysis/CustomAttributeMetadataNode.cs | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs index ee12a3fbbc120..2196c78582881 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs @@ -19,6 +19,8 @@ namespace ILCompiler.DependencyAnalysis /// internal static class CustomAttributeBasedDependencyAlgorithm { + public static bool CanOptimizeAttributeArtifacts(NodeFactory factory) => factory.CompilationModuleGroup.IsSingleFileCompilation; + public static void AddDependenciesDueToCustomAttributes(ref DependencyList dependencies, NodeFactory factory, EcmaMethod method) { MetadataReader reader = method.MetadataReader; @@ -108,6 +110,7 @@ private static void AddDependenciesDueToCustomAttributes(ref DependencyList depe var mdManager = (UsageBasedMetadataManager)factory.MetadataManager; var attributeTypeProvider = new CustomAttributeTypeProvider(module); + bool metadataOnlyDependencies = CanOptimizeAttributeArtifacts(factory); foreach (CustomAttributeHandle caHandle in attributeHandles) { @@ -128,9 +131,12 @@ private static void AddDependenciesDueToCustomAttributes(ref DependencyList depe // Make a new list in case we need to abort. var caDependencies = factory.MetadataManager.GetDependenciesForCustomAttribute(factory, constructor, decodedValue, parent) ?? new DependencyList(); - caDependencies.Add(factory.MethodMetadata(constructor.GetTypicalMethodDefinition()), "Attribute constructor"); + if (metadataOnlyDependencies) + caDependencies.Add(factory.MethodMetadata(constructor.GetTypicalMethodDefinition()), "Attribute constructor"); + else + caDependencies.Add(factory.ReflectedMethod(constructor.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Attribute constructor"); - if (AddDependenciesFromCustomAttributeBlob(caDependencies, factory, constructor.OwningType, decodedValue, metadataOnly: true)) + if (AddDependenciesFromCustomAttributeBlob(caDependencies, factory, constructor.OwningType, decodedValue, metadataOnlyDependencies)) { dependencies ??= new DependencyList(); dependencies.AddRange(caDependencies); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs index e8897d5c399ca..99f64b8b63b46 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeMetadataNode.cs @@ -6,6 +6,7 @@ using Internal.TypeSystem; using ILCompiler.DependencyAnalysisFramework; +using System; namespace ILCompiler.DependencyAnalysis { @@ -30,6 +31,9 @@ public CustomAttributeMetadataNode(ReflectableCustomAttribute customAttribute) public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) { + if (!CustomAttributeBasedDependencyAlgorithm.CanOptimizeAttributeArtifacts(factory)) + return Array.Empty(); + // Presence of this type indicates that more than just the attribute metadata is needed: // we also need runtime artifacts, such as the method body of the attribute constructor. MetadataType nativeFormatType = factory.TypeSystemContext.SystemModule.GetType("System.Reflection.Runtime.CustomAttributes.NativeFormat", "NativeFormatCustomAttributeData");