Add star into link attributes

src/coreclr/tools/Common/Compiler/ProcessLinkerXmlBase.cs

@@ -55,6 +55,7 @@ public abstract class ProcessLinkerXmlBase
         private readonly XPathNavigator _document;
         private readonly Logger _logger;
         protected readonly ModuleDesc? _owningModule;
+        protected readonly bool _globalAttributeRemoval;
         private readonly IReadOnlyDictionary<string, bool> _featureSwitchValues;
         protected readonly TypeSystemContext _context;
 
@@ -70,10 +71,11 @@ protected ProcessLinkerXmlBase(Logger logger, TypeSystemContext context, Stream
             _featureSwitchValues = featureSwitchValues;
         }
 
-        protected ProcessLinkerXmlBase(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues)
+        protected ProcessLinkerXmlBase(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval = false)
             : this(logger, context, documentStream, xmlDocumentLocation, featureSwitchValues)
         {
             _owningModule = resourceAssembly;
+            _globalAttributeRemoval = globalAttributeRemoval;
         }
 
         protected virtual bool ShouldProcessElement(XPathNavigator nav) => FeatureSettings.ShouldProcessElement(nav, _featureSwitchValues);
@@ -133,44 +135,46 @@ protected virtual void ProcessAssemblies(XPathNavigator nav)
         {
             foreach (XPathNavigator assemblyNav in nav.SelectChildren("assembly", ""))
             {
+                if (!ShouldProcessElement(assemblyNav))
+                    continue;
+
                 // Errors for invalid assembly names should show up even if this element will be
                 // skipped due to feature conditions.
                 bool processAllAssemblies = ShouldProcessAllAssemblies(assemblyNav, out AssemblyName? name);
-                if (processAllAssemblies && AllowedAssemblySelector != AllowedAssemblies.AllAssemblies)
+                if (processAllAssemblies && !_globalAttributeRemoval)
                 {
-                    // NativeAOT doesn't have a way to eliminate all the occurrences of an attribute yet
-                    // https://github.com/dotnet/runtime/issues/77753
-                    //LogWarning(assemblyNav, DiagnosticId.XmlUnsupportedWildcard);
+#if !READYTORUN
+                    if (AllowedAssemblySelector != AllowedAssemblies.AllAssemblies)
+                        LogWarning(assemblyNav, DiagnosticId.XmlUnsuportedWildcard);
+#endif
                     continue;
                 }
-
-                ModuleDesc? assemblyToProcess = null;
-                if (!AllowedAssemblySelector.HasFlag(AllowedAssemblies.AnyAssembly))
+                else if (!processAllAssemblies && _globalAttributeRemoval)
+                {
+                    continue;
+                }
+                else if (processAllAssemblies && _globalAttributeRemoval)
                 {
-                    Debug.Assert(!processAllAssemblies);
                     Debug.Assert(_owningModule != null);
-                    if (_owningModule.Assembly.GetName().Name != name!.Name)
+                    ProcessAssembly(_owningModule, assemblyNav, warnOnUnresolvedTypes: false);
+                }
+                else
+                {
+                    ModuleDesc? assemblyToProcess = null;
+                    if (!AllowedAssemblySelector.HasFlag(AllowedAssemblies.AnyAssembly))
                     {
+                        Debug.Assert(!processAllAssemblies);
+                        Debug.Assert(_owningModule != null);
+                        if (_owningModule.Assembly.GetName().Name != name!.Name)
+                        {
 #if !READYTORUN
-                        LogWarning(assemblyNav, DiagnosticId.AssemblyWithEmbeddedXmlApplyToAnotherAssembly, _owningModule.Assembly.GetName().Name ?? "", name.ToString());
+                            LogWarning(assemblyNav, DiagnosticId.AssemblyWithEmbeddedXmlApplyToAnotherAssembly, _owningModule.Assembly.GetName().Name ?? "", name.ToString());
 #endif
-                        continue;
+                            continue;
+                        }
+                        assemblyToProcess = _owningModule;
                     }
-                    assemblyToProcess = _owningModule;
-                }
-
-                if (!ShouldProcessElement(assemblyNav))
-                    continue;
 
-                if (processAllAssemblies)
-                {
-                    throw new NotImplementedException();
-                    // We could avoid loading all references in this case: https://github.com/dotnet/linker/issues/1708
-                    //foreach (ModuleDesc assembly in GetReferencedAssemblies())
-                    //    ProcessAssembly(assembly, assemblyNav, warnOnUnresolvedTypes: false);
-                }
-                else
-                {
                     Debug.Assert(!processAllAssemblies);
                     ModuleDesc? assembly = assemblyToProcess ?? _context.ResolveAssembly(name!, false);
 
@@ -469,8 +473,8 @@ protected virtual void ProcessProperty(TypeDesc type, XPathNavigator nav, object
                 bool foundMatch = false;
                 foreach (PropertyPseudoDesc property in type.GetPropertiesOnTypeHierarchy(p => p.Name == name))
                 {
-                   foundMatch = true;
-                   ProcessProperty(type, property, nav, customData, false);
+                    foundMatch = true;
+                    ProcessProperty(type, property, nav, customData, false);
                 }
 
                 if (!foundMatch)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

@@ -39,7 +39,7 @@ public sealed class UsageBasedMetadataManager : GeneratingMetadataManager
 
         internal readonly UsageBasedMetadataGenerationOptions _generationOptions;
 
-        private readonly FeatureSwitchHashtable _featureSwitchHashtable;
+        private readonly LinkAttributesHashTable _linkAttributesHashTable;
 
         private static (string AttributeName, DiagnosticId Id)[] _requiresAttributeMismatchNameAndId = new[]
             {
@@ -91,7 +91,7 @@ public UsageBasedMetadataManager(
             FlowAnnotations = flowAnnotations;
             Logger = logger;
 
-            _featureSwitchHashtable = new FeatureSwitchHashtable(Logger, new Dictionary<string, bool>(featureSwitchValues));
+            _linkAttributesHashTable = new LinkAttributesHashTable(Logger, new Dictionary<string, bool>(featureSwitchValues));
             FeatureSwitches = new Dictionary<string, bool>(featureSwitchValues);
 
             _rootEntireAssembliesModules = new HashSet<string>(rootEntireAssembliesModules);
@@ -821,7 +821,7 @@ public bool GeneratesAttributeMetadata(TypeDesc attributeType)
             var ecmaType = attributeType.GetTypeDefinition() as EcmaType;
             if (ecmaType != null)
             {
-                var moduleInfo = _featureSwitchHashtable.GetOrCreateValue(ecmaType.EcmaModule);
+                var moduleInfo = _linkAttributesHashTable.GetOrCreateValue(ecmaType.EcmaModule);
                 return !moduleInfo.RemovedAttributes.Contains(ecmaType);
             }
 
@@ -1070,12 +1070,12 @@ public bool IsBlocked(MethodDesc methodDef)
             }
         }
 
-        private sealed class FeatureSwitchHashtable : LockFreeReaderHashtable<EcmaModule, AssemblyFeatureInfo>
+        private sealed class LinkAttributesHashTable : LockFreeReaderHashtable<EcmaModule, AssemblyFeatureInfo>
         {
             private readonly Dictionary<string, bool> _switchValues;
             private readonly Logger _logger;
 
-            public FeatureSwitchHashtable(Logger logger, Dictionary<string, bool> switchValues)
+            public LinkAttributesHashTable(Logger logger, Dictionary<string, bool> switchValues)
             {
                 _logger = logger;
                 _switchValues = switchValues;
@@ -1103,19 +1103,30 @@ public AssemblyFeatureInfo(EcmaModule module, Logger logger, IReadOnlyDictionary
                 Module = module;
                 RemovedAttributes = new HashSet<TypeDesc>();
 
-                PEMemoryBlock resourceDirectory = module.PEReader.GetSectionData(module.PEReader.PEHeaders.CorHeader.ResourcesDirectory.RelativeVirtualAddress);
+                // System.Private.CorLib has a special functionality that could delete an attribute in all modules.
+                // In order to get the set of attributes that need to be removed the modules need collect both the
+                // set of attributes in it's embedded XML file and the set inside System.Private.CorLib embedded
+                // XML file
+                ParseLinkAttributesXml(module, logger, featureSwitchValues, globalAttributeRemoval: false);
+                ParseLinkAttributesXml(module, logger, featureSwitchValues, globalAttributeRemoval: true);
+            }
+
+            public void ParseLinkAttributesXml(EcmaModule module, Logger logger, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval)
+            {
+                EcmaModule xmlModule = globalAttributeRemoval ? (EcmaModule)module.Context.SystemModule : module;
+                PEMemoryBlock resourceDirectory = xmlModule.PEReader.GetSectionData(xmlModule.PEReader.PEHeaders.CorHeader.ResourcesDirectory.RelativeVirtualAddress);
 
-                foreach (var resourceHandle in module.MetadataReader.ManifestResources)
+                foreach (var resourceHandle in xmlModule.MetadataReader.ManifestResources)
                 {
-                    ManifestResource resource = module.MetadataReader.GetManifestResource(resourceHandle);
+                    ManifestResource resource = xmlModule.MetadataReader.GetManifestResource(resourceHandle);
 
                     // Don't try to process linked resources or resources in other assemblies
                     if (!resource.Implementation.IsNil)
                     {
                         continue;
                     }
 
-                    string resourceName = module.MetadataReader.GetString(resource.Name);
+                    string resourceName = xmlModule.MetadataReader.GetString(resource.Name);
                     if (resourceName == "ILLink.LinkAttributes.xml")
                     {
                         BlobReader reader = resourceDirectory.GetReader((int)resource.Offset, resourceDirectory.Length - (int)resource.Offset);
@@ -1127,37 +1138,61 @@ public AssemblyFeatureInfo(EcmaModule module, Logger logger, IReadOnlyDictionary
                             ms = new UnmanagedMemoryStream(reader.CurrentPointer, length);
                         }
 
-                        RemovedAttributes = LinkAttributesReader.GetRemovedAttributes(logger, module.Context, ms, resource, module, "resource " + resourceName + " in " + module.ToString(), featureSwitchValues);
+                        RemovedAttributes.UnionWith(LinkAttributesReader.GetRemovedAttributes(logger, xmlModule.Context, ms, resource, module, "resource " + resourceName + " in " + module.ToString(), featureSwitchValues, globalAttributeRemoval));
                     }
                 }
             }
         }
 
-        private sealed class LinkAttributesReader : ProcessLinkerXmlBase
+        internal sealed class LinkAttributesReader : ProcessLinkerXmlBase
         {
             private readonly HashSet<TypeDesc> _removedAttributes = new();
 
-            public LinkAttributesReader(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues)
-                : base(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues)
+            public LinkAttributesReader(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval)
+                : base(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues, globalAttributeRemoval)
             {
             }
 
+            private static bool IsRemoveAttributeInstances(string attributeName) => attributeName == "RemoveAttributeInstances" || attributeName == "RemoveAttributeInstancesAttribute";
+
             private void ProcessAttribute(TypeDesc type, XPathNavigator nav)
             {
                 string internalValue = GetAttribute(nav, "internal");
-                if (internalValue == "RemoveAttributeInstances" && nav.IsEmptyElement)
+                if (!string.IsNullOrEmpty(internalValue))
                 {
-                    _removedAttributes.Add(type);
+                    if (!IsRemoveAttributeInstances(internalValue) || !nav.IsEmptyElement)
+                    {
+                        LogWarning(nav, DiagnosticId.UnrecognizedInternalAttribute, internalValue);
+                    }
+                    if (IsRemoveAttributeInstances(internalValue) && nav.IsEmptyElement)
+                    {
+                        _removedAttributes.Add(type);
+                    }
                 }
             }
 
-            public static HashSet<TypeDesc> GetRemovedAttributes(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues)
+            public static HashSet<TypeDesc> GetRemovedAttributes(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval)
             {
-                var rdr = new LinkAttributesReader(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues);
+                var rdr = new LinkAttributesReader(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues, globalAttributeRemoval);
                 rdr.ProcessXml(false);
                 return rdr._removedAttributes;
             }
 
+            protected override AllowedAssemblies AllowedAssemblySelector
+            {
+                get
+                {
+                    if (_owningModule?.Assembly == null)
+                        return AllowedAssemblies.AllAssemblies;
+
+                    // Corelib XML may contain assembly wildcard to support compiler-injected attribute types
+                    if (_owningModule?.Assembly == _context.SystemModule)
+                        return AllowedAssemblies.AllAssemblies;
+
+                    return AllowedAssemblies.ContainingAssembly;
+                }
+            }
+
             protected override void ProcessAssembly(ModuleDesc assembly, XPathNavigator nav, bool warnOnUnresolvedTypes)
             {
                 ProcessTypes(assembly, nav, warnOnUnresolvedTypes);

src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/ILCompilerDriver.cs

@@ -66,12 +66,6 @@ public ILScanResults Trim (ILCompilerOptions options, ILogWriter logWriter)
 
 			ILProvider ilProvider = new NativeAotILProvider ();
 
-			foreach (var descriptor in options.Descriptors) {
-				if (!File.Exists (descriptor))
-					throw new FileNotFoundException ($"'{descriptor}' doesn't exist");
-				compilationRoots.Add (new ILCompiler.DependencyAnalysis.TrimmingDescriptorNode (descriptor));
-			}
-
 			Logger logger = new Logger (
 				logWriter,
 				ilProvider,
@@ -82,6 +76,12 @@ public ILScanResults Trim (ILCompilerOptions options, ILogWriter logWriter)
 				singleWarnDisabledModules: Enumerable.Empty<string> (),
 				suppressedCategories: Enumerable.Empty<string> ());
 
+			foreach (var descriptor in options.Descriptors) {
+				if (!File.Exists (descriptor))
+					throw new FileNotFoundException ($"'{descriptor}' doesn't exist");
+				compilationRoots.Add (new ILCompiler.DependencyAnalysis.TrimmingDescriptorNode (descriptor));
+			}
+
 			ilProvider = new FeatureSwitchManager (ilProvider, logger, options.FeatureSwitches);
 
 			CompilerGeneratedState compilerGeneratedState = new CompilerGeneratedState (ilProvider, logger);

src/tests/nativeaot/SmokeTests/TrimmingBehaviors/ILLink.LinkAttributes.xml

@@ -0,0 +1,16 @@
+<linker>
+  <!-- IL2049 -->
+  <type fullname="ILLinkLinkAttributes/TestRemoveAttribute">
+    <attribute internal="RemoveAttributeInstances"/>
+    <attribute internal="InvalidInternal"/>
+  </type>
+  <!-- IL2048 -->
+  <type fullname="ILLinkLinkAttributes">
+    <method signature="_fieldWithCustomAttribute">
+      <attribute internal="RemoveAttributeInstances"/>
+    </method>
+  </type>
+  <type fullname="ILLinkLinkAttributes/TestMarkAllRemoveAttribute">
+    <attribute internal="RemoveAttributeInstances"/>
+  </type>
+</linker>

src/tests/nativeaot/SmokeTests/TrimmingBehaviors/ILLinkLinkAttributes.cs

@@ -0,0 +1,98 @@
+// 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;
+
+using BindingFlags = System.Reflection.BindingFlags;
+
+class ILLinkLinkAttributes
+{
+    public static int Run()
+    {
+        ThrowIfTypeNotPresent(typeof(ILLinkLinkAttributes), nameof(TestDontRemoveAttribute));
+        ThrowIfTypePresent(typeof(ILLinkLinkAttributes), nameof(TestRemoveAttribute));
+        ThrowIfTypePresent(typeof(ILLinkLinkAttributes), nameof(TestMarkAllRemoveAttribute));
+        ThrowIfAttributePresent(typeof(ILLinkLinkAttributes), nameof(_fieldWithCustomAttribute), nameof(TestRemoveAttribute));
+        ThrowIfAttributeNotPresent(typeof(ILLinkLinkAttributes), nameof(_fieldWithCustomAttribute), nameof(TestDontRemoveAttribute));
+        ThrowIfAttributePresent(typeof(ILLinkLinkAttributes), nameof(_fieldWithCustomAttribute), nameof(AllowNullAttribute));
+        return 100;
+    }
+
+    [TestDontRemoveAttribute]
+    [TestRemoveAttribute]
+    [AllowNullAttribute]
+    private string _fieldWithCustomAttribute = "Hello world";
+
+    class TestDontRemoveAttribute : Attribute
+    {
+        public TestDontRemoveAttribute()
+        {
+        }
+    }
+
+    class TestRemoveAttribute : Attribute
+    {
+        public TestRemoveAttribute()
+        {
+        }
+    }
+
+    class TestMarkAllRemoveAttribute : Attribute
+    {
+    }
+
+    [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
+        Justification = "That's the point")]
+    private static bool IsTypePresent(Type testType, string typeName) => testType.GetNestedType(typeName, BindingFlags.NonPublic | BindingFlags.Public) != null;
+
+    private static void ThrowIfTypeNotPresent(Type testType, string typeName)
+    {
+        if (!IsTypePresent(testType, typeName))
+        {
+            throw new Exception(typeName);
+        }
+    }
+
+    private static void ThrowIfTypePresent(Type testType, string typeName)
+    {
+        if (IsTypePresent(testType, typeName))
+        {
+            throw new Exception(typeName);
+        }
+    }
+
+    private static bool IsAttributePresent(Type testType, string memberName, string attributeName)
+    {
+        foreach (var member in testType.GetMembers())
+        {
+            if (member.Name == memberName)
+            {
+                foreach (var attribute in member.GetCustomAttributes(false))
+                {
+                    if (attribute.GetType().Name == attributeName)
+                    {
+                        return true;
+                    }
+                }
+            }
+        }
+        return false;
+    }
+
+    private static void ThrowIfAttributeNotPresent(Type testType, string memberName, string attributeName)
+    {
+        if (!IsAttributePresent(testType, memberName, attributeName))
+        {
+            throw new Exception(attributeName);
+        }
+    }
+
+    private static void ThrowIfAttributePresent(Type testType, string memberName, string attributeName)
+    {
+        if (IsAttributePresent(testType, memberName, attributeName))
+        {
+            throw new Exception(attributeName);
+        }
+    }
+}