Skip to content

Commit 98514de

Browse files
authored
Add star into link attributes (#81407)
One of the functionalities of the LinkAttributes files is to occurrences of custom attributes on the current assembly, on top of this functionality there is a special functionality that is only permitted if it's embedded in an XML inside the System.Private.CorLib module that allows the deletion of custom attributes from all the assemblies. Given that NativeAOT is multithreaded it cannot process all the assemblies to remove attributes as ILLink does, it also does not have a shared storage in which it can store the information about the removed attributes from System.Private.CoreLib. This PR adds logic to process the special functionality in the embedded System.Private.CoreLib XML every time an assembly is being processed. In order to make this more performant, an extra argument is passed to the XML reader to only look at the assembly="*" to get the set of attributes that need to be deleted from all assemblies. * Handle the star argument in LinkAttributes files for NativeAOT * Minor fixes to print more warnings * Add a way to only process star/nonstar assemblies in ProcessLinkerXmlBase * Add smoke test
1 parent f6b9578 commit 98514de

File tree

6 files changed

+209
-51
lines changed

6 files changed

+209
-51
lines changed

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

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public abstract class ProcessLinkerXmlBase
5555
private readonly XPathNavigator _document;
5656
private readonly Logger _logger;
5757
protected readonly ModuleDesc? _owningModule;
58+
protected readonly bool _globalAttributeRemoval;
5859
private readonly IReadOnlyDictionary<string, bool> _featureSwitchValues;
5960
protected readonly TypeSystemContext _context;
6061

@@ -70,10 +71,11 @@ protected ProcessLinkerXmlBase(Logger logger, TypeSystemContext context, Stream
7071
_featureSwitchValues = featureSwitchValues;
7172
}
7273

73-
protected ProcessLinkerXmlBase(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues)
74+
protected ProcessLinkerXmlBase(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval = false)
7475
: this(logger, context, documentStream, xmlDocumentLocation, featureSwitchValues)
7576
{
7677
_owningModule = resourceAssembly;
78+
_globalAttributeRemoval = globalAttributeRemoval;
7779
}
7880

7981
protected virtual bool ShouldProcessElement(XPathNavigator nav) => FeatureSettings.ShouldProcessElement(nav, _featureSwitchValues);
@@ -133,44 +135,46 @@ protected virtual void ProcessAssemblies(XPathNavigator nav)
133135
{
134136
foreach (XPathNavigator assemblyNav in nav.SelectChildren("assembly", ""))
135137
{
138+
if (!ShouldProcessElement(assemblyNav))
139+
continue;
140+
136141
// Errors for invalid assembly names should show up even if this element will be
137142
// skipped due to feature conditions.
138143
bool processAllAssemblies = ShouldProcessAllAssemblies(assemblyNav, out AssemblyName? name);
139-
if (processAllAssemblies && AllowedAssemblySelector != AllowedAssemblies.AllAssemblies)
144+
if (processAllAssemblies && !_globalAttributeRemoval)
140145
{
141-
// NativeAOT doesn't have a way to eliminate all the occurrences of an attribute yet
142-
// https://github.com/dotnet/runtime/issues/77753
143-
//LogWarning(assemblyNav, DiagnosticId.XmlUnsupportedWildcard);
146+
#if !READYTORUN
147+
if (AllowedAssemblySelector != AllowedAssemblies.AllAssemblies)
148+
LogWarning(assemblyNav, DiagnosticId.XmlUnsuportedWildcard);
149+
#endif
144150
continue;
145151
}
146-
147-
ModuleDesc? assemblyToProcess = null;
148-
if (!AllowedAssemblySelector.HasFlag(AllowedAssemblies.AnyAssembly))
152+
else if (!processAllAssemblies && _globalAttributeRemoval)
153+
{
154+
continue;
155+
}
156+
else if (processAllAssemblies && _globalAttributeRemoval)
149157
{
150-
Debug.Assert(!processAllAssemblies);
151158
Debug.Assert(_owningModule != null);
152-
if (_owningModule.Assembly.GetName().Name != name!.Name)
159+
ProcessAssembly(_owningModule, assemblyNav, warnOnUnresolvedTypes: false);
160+
}
161+
else
162+
{
163+
ModuleDesc? assemblyToProcess = null;
164+
if (!AllowedAssemblySelector.HasFlag(AllowedAssemblies.AnyAssembly))
153165
{
166+
Debug.Assert(!processAllAssemblies);
167+
Debug.Assert(_owningModule != null);
168+
if (_owningModule.Assembly.GetName().Name != name!.Name)
169+
{
154170
#if !READYTORUN
155-
LogWarning(assemblyNav, DiagnosticId.AssemblyWithEmbeddedXmlApplyToAnotherAssembly, _owningModule.Assembly.GetName().Name ?? "", name.ToString());
171+
LogWarning(assemblyNav, DiagnosticId.AssemblyWithEmbeddedXmlApplyToAnotherAssembly, _owningModule.Assembly.GetName().Name ?? "", name.ToString());
156172
#endif
157-
continue;
173+
continue;
174+
}
175+
assemblyToProcess = _owningModule;
158176
}
159-
assemblyToProcess = _owningModule;
160-
}
161-
162-
if (!ShouldProcessElement(assemblyNav))
163-
continue;
164177

165-
if (processAllAssemblies)
166-
{
167-
throw new NotImplementedException();
168-
// We could avoid loading all references in this case: https://github.com/dotnet/linker/issues/1708
169-
//foreach (ModuleDesc assembly in GetReferencedAssemblies())
170-
// ProcessAssembly(assembly, assemblyNav, warnOnUnresolvedTypes: false);
171-
}
172-
else
173-
{
174178
Debug.Assert(!processAllAssemblies);
175179
ModuleDesc? assembly = assemblyToProcess ?? _context.ResolveAssembly(name!, false);
176180

@@ -469,8 +473,8 @@ protected virtual void ProcessProperty(TypeDesc type, XPathNavigator nav, object
469473
bool foundMatch = false;
470474
foreach (PropertyPseudoDesc property in type.GetPropertiesOnTypeHierarchy(p => p.Name == name))
471475
{
472-
foundMatch = true;
473-
ProcessProperty(type, property, nav, customData, false);
476+
foundMatch = true;
477+
ProcessProperty(type, property, nav, customData, false);
474478
}
475479

476480
if (!foundMatch)

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

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public sealed class UsageBasedMetadataManager : GeneratingMetadataManager
3939

4040
internal readonly UsageBasedMetadataGenerationOptions _generationOptions;
4141

42-
private readonly FeatureSwitchHashtable _featureSwitchHashtable;
42+
private readonly LinkAttributesHashTable _linkAttributesHashTable;
4343

4444
private static (string AttributeName, DiagnosticId Id)[] _requiresAttributeMismatchNameAndId = new[]
4545
{
@@ -91,7 +91,7 @@ public UsageBasedMetadataManager(
9191
FlowAnnotations = flowAnnotations;
9292
Logger = logger;
9393

94-
_featureSwitchHashtable = new FeatureSwitchHashtable(Logger, new Dictionary<string, bool>(featureSwitchValues));
94+
_linkAttributesHashTable = new LinkAttributesHashTable(Logger, new Dictionary<string, bool>(featureSwitchValues));
9595
FeatureSwitches = new Dictionary<string, bool>(featureSwitchValues);
9696

9797
_rootEntireAssembliesModules = new HashSet<string>(rootEntireAssembliesModules);
@@ -821,7 +821,7 @@ public bool GeneratesAttributeMetadata(TypeDesc attributeType)
821821
var ecmaType = attributeType.GetTypeDefinition() as EcmaType;
822822
if (ecmaType != null)
823823
{
824-
var moduleInfo = _featureSwitchHashtable.GetOrCreateValue(ecmaType.EcmaModule);
824+
var moduleInfo = _linkAttributesHashTable.GetOrCreateValue(ecmaType.EcmaModule);
825825
return !moduleInfo.RemovedAttributes.Contains(ecmaType);
826826
}
827827

@@ -1070,12 +1070,12 @@ public bool IsBlocked(MethodDesc methodDef)
10701070
}
10711071
}
10721072

1073-
private sealed class FeatureSwitchHashtable : LockFreeReaderHashtable<EcmaModule, AssemblyFeatureInfo>
1073+
private sealed class LinkAttributesHashTable : LockFreeReaderHashtable<EcmaModule, AssemblyFeatureInfo>
10741074
{
10751075
private readonly Dictionary<string, bool> _switchValues;
10761076
private readonly Logger _logger;
10771077

1078-
public FeatureSwitchHashtable(Logger logger, Dictionary<string, bool> switchValues)
1078+
public LinkAttributesHashTable(Logger logger, Dictionary<string, bool> switchValues)
10791079
{
10801080
_logger = logger;
10811081
_switchValues = switchValues;
@@ -1103,19 +1103,30 @@ public AssemblyFeatureInfo(EcmaModule module, Logger logger, IReadOnlyDictionary
11031103
Module = module;
11041104
RemovedAttributes = new HashSet<TypeDesc>();
11051105

1106-
PEMemoryBlock resourceDirectory = module.PEReader.GetSectionData(module.PEReader.PEHeaders.CorHeader.ResourcesDirectory.RelativeVirtualAddress);
1106+
// System.Private.CorLib has a special functionality that could delete an attribute in all modules.
1107+
// In order to get the set of attributes that need to be removed the modules need collect both the
1108+
// set of attributes in it's embedded XML file and the set inside System.Private.CorLib embedded
1109+
// XML file
1110+
ParseLinkAttributesXml(module, logger, featureSwitchValues, globalAttributeRemoval: false);
1111+
ParseLinkAttributesXml(module, logger, featureSwitchValues, globalAttributeRemoval: true);
1112+
}
1113+
1114+
public void ParseLinkAttributesXml(EcmaModule module, Logger logger, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval)
1115+
{
1116+
EcmaModule xmlModule = globalAttributeRemoval ? (EcmaModule)module.Context.SystemModule : module;
1117+
PEMemoryBlock resourceDirectory = xmlModule.PEReader.GetSectionData(xmlModule.PEReader.PEHeaders.CorHeader.ResourcesDirectory.RelativeVirtualAddress);
11071118

1108-
foreach (var resourceHandle in module.MetadataReader.ManifestResources)
1119+
foreach (var resourceHandle in xmlModule.MetadataReader.ManifestResources)
11091120
{
1110-
ManifestResource resource = module.MetadataReader.GetManifestResource(resourceHandle);
1121+
ManifestResource resource = xmlModule.MetadataReader.GetManifestResource(resourceHandle);
11111122

11121123
// Don't try to process linked resources or resources in other assemblies
11131124
if (!resource.Implementation.IsNil)
11141125
{
11151126
continue;
11161127
}
11171128

1118-
string resourceName = module.MetadataReader.GetString(resource.Name);
1129+
string resourceName = xmlModule.MetadataReader.GetString(resource.Name);
11191130
if (resourceName == "ILLink.LinkAttributes.xml")
11201131
{
11211132
BlobReader reader = resourceDirectory.GetReader((int)resource.Offset, resourceDirectory.Length - (int)resource.Offset);
@@ -1127,37 +1138,61 @@ public AssemblyFeatureInfo(EcmaModule module, Logger logger, IReadOnlyDictionary
11271138
ms = new UnmanagedMemoryStream(reader.CurrentPointer, length);
11281139
}
11291140

1130-
RemovedAttributes = LinkAttributesReader.GetRemovedAttributes(logger, module.Context, ms, resource, module, "resource " + resourceName + " in " + module.ToString(), featureSwitchValues);
1141+
RemovedAttributes.UnionWith(LinkAttributesReader.GetRemovedAttributes(logger, xmlModule.Context, ms, resource, module, "resource " + resourceName + " in " + module.ToString(), featureSwitchValues, globalAttributeRemoval));
11311142
}
11321143
}
11331144
}
11341145
}
11351146

1136-
private sealed class LinkAttributesReader : ProcessLinkerXmlBase
1147+
internal sealed class LinkAttributesReader : ProcessLinkerXmlBase
11371148
{
11381149
private readonly HashSet<TypeDesc> _removedAttributes = new();
11391150

1140-
public LinkAttributesReader(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues)
1141-
: base(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues)
1151+
public LinkAttributesReader(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval)
1152+
: base(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues, globalAttributeRemoval)
11421153
{
11431154
}
11441155

1156+
private static bool IsRemoveAttributeInstances(string attributeName) => attributeName == "RemoveAttributeInstances" || attributeName == "RemoveAttributeInstancesAttribute";
1157+
11451158
private void ProcessAttribute(TypeDesc type, XPathNavigator nav)
11461159
{
11471160
string internalValue = GetAttribute(nav, "internal");
1148-
if (internalValue == "RemoveAttributeInstances" && nav.IsEmptyElement)
1161+
if (!string.IsNullOrEmpty(internalValue))
11491162
{
1150-
_removedAttributes.Add(type);
1163+
if (!IsRemoveAttributeInstances(internalValue) || !nav.IsEmptyElement)
1164+
{
1165+
LogWarning(nav, DiagnosticId.UnrecognizedInternalAttribute, internalValue);
1166+
}
1167+
if (IsRemoveAttributeInstances(internalValue) && nav.IsEmptyElement)
1168+
{
1169+
_removedAttributes.Add(type);
1170+
}
11511171
}
11521172
}
11531173

1154-
public static HashSet<TypeDesc> GetRemovedAttributes(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues)
1174+
public static HashSet<TypeDesc> GetRemovedAttributes(Logger logger, TypeSystemContext context, Stream documentStream, ManifestResource resource, ModuleDesc resourceAssembly, string xmlDocumentLocation, IReadOnlyDictionary<string, bool> featureSwitchValues, bool globalAttributeRemoval)
11551175
{
1156-
var rdr = new LinkAttributesReader(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues);
1176+
var rdr = new LinkAttributesReader(logger, context, documentStream, resource, resourceAssembly, xmlDocumentLocation, featureSwitchValues, globalAttributeRemoval);
11571177
rdr.ProcessXml(false);
11581178
return rdr._removedAttributes;
11591179
}
11601180

1181+
protected override AllowedAssemblies AllowedAssemblySelector
1182+
{
1183+
get
1184+
{
1185+
if (_owningModule?.Assembly == null)
1186+
return AllowedAssemblies.AllAssemblies;
1187+
1188+
// Corelib XML may contain assembly wildcard to support compiler-injected attribute types
1189+
if (_owningModule?.Assembly == _context.SystemModule)
1190+
return AllowedAssemblies.AllAssemblies;
1191+
1192+
return AllowedAssemblies.ContainingAssembly;
1193+
}
1194+
}
1195+
11611196
protected override void ProcessAssembly(ModuleDesc assembly, XPathNavigator nav, bool warnOnUnresolvedTypes)
11621197
{
11631198
ProcessTypes(assembly, nav, warnOnUnresolvedTypes);

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,6 @@ public ILScanResults Trim (ILCompilerOptions options, ILogWriter logWriter)
6666

6767
ILProvider ilProvider = new NativeAotILProvider ();
6868

69-
foreach (var descriptor in options.Descriptors) {
70-
if (!File.Exists (descriptor))
71-
throw new FileNotFoundException ($"'{descriptor}' doesn't exist");
72-
compilationRoots.Add (new ILCompiler.DependencyAnalysis.TrimmingDescriptorNode (descriptor));
73-
}
74-
7569
Logger logger = new Logger (
7670
logWriter,
7771
ilProvider,
@@ -82,6 +76,12 @@ public ILScanResults Trim (ILCompilerOptions options, ILogWriter logWriter)
8276
singleWarnDisabledModules: Enumerable.Empty<string> (),
8377
suppressedCategories: Enumerable.Empty<string> ());
8478

79+
foreach (var descriptor in options.Descriptors) {
80+
if (!File.Exists (descriptor))
81+
throw new FileNotFoundException ($"'{descriptor}' doesn't exist");
82+
compilationRoots.Add (new ILCompiler.DependencyAnalysis.TrimmingDescriptorNode (descriptor));
83+
}
84+
8585
ilProvider = new FeatureSwitchManager (ilProvider, logger, options.FeatureSwitches);
8686

8787
CompilerGeneratedState compilerGeneratedState = new CompilerGeneratedState (ilProvider, logger);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<linker>
2+
<!-- IL2049 -->
3+
<type fullname="ILLinkLinkAttributes/TestRemoveAttribute">
4+
<attribute internal="RemoveAttributeInstances"/>
5+
<attribute internal="InvalidInternal"/>
6+
</type>
7+
<!-- IL2048 -->
8+
<type fullname="ILLinkLinkAttributes">
9+
<method signature="_fieldWithCustomAttribute">
10+
<attribute internal="RemoveAttributeInstances"/>
11+
</method>
12+
</type>
13+
<type fullname="ILLinkLinkAttributes/TestMarkAllRemoveAttribute">
14+
<attribute internal="RemoveAttributeInstances"/>
15+
</type>
16+
</linker>
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Diagnostics.CodeAnalysis;
6+
7+
using BindingFlags = System.Reflection.BindingFlags;
8+
9+
class ILLinkLinkAttributes
10+
{
11+
public static int Run()
12+
{
13+
ThrowIfTypeNotPresent(typeof(ILLinkLinkAttributes), nameof(TestDontRemoveAttribute));
14+
ThrowIfTypePresent(typeof(ILLinkLinkAttributes), nameof(TestRemoveAttribute));
15+
ThrowIfTypePresent(typeof(ILLinkLinkAttributes), nameof(TestMarkAllRemoveAttribute));
16+
ThrowIfAttributePresent(typeof(ILLinkLinkAttributes), nameof(_fieldWithCustomAttribute), nameof(TestRemoveAttribute));
17+
ThrowIfAttributeNotPresent(typeof(ILLinkLinkAttributes), nameof(_fieldWithCustomAttribute), nameof(TestDontRemoveAttribute));
18+
ThrowIfAttributePresent(typeof(ILLinkLinkAttributes), nameof(_fieldWithCustomAttribute), nameof(AllowNullAttribute));
19+
return 100;
20+
}
21+
22+
[TestDontRemoveAttribute]
23+
[TestRemoveAttribute]
24+
[AllowNullAttribute]
25+
private string _fieldWithCustomAttribute = "Hello world";
26+
27+
class TestDontRemoveAttribute : Attribute
28+
{
29+
public TestDontRemoveAttribute()
30+
{
31+
}
32+
}
33+
34+
class TestRemoveAttribute : Attribute
35+
{
36+
public TestRemoveAttribute()
37+
{
38+
}
39+
}
40+
41+
class TestMarkAllRemoveAttribute : Attribute
42+
{
43+
}
44+
45+
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
46+
Justification = "That's the point")]
47+
private static bool IsTypePresent(Type testType, string typeName) => testType.GetNestedType(typeName, BindingFlags.NonPublic | BindingFlags.Public) != null;
48+
49+
private static void ThrowIfTypeNotPresent(Type testType, string typeName)
50+
{
51+
if (!IsTypePresent(testType, typeName))
52+
{
53+
throw new Exception(typeName);
54+
}
55+
}
56+
57+
private static void ThrowIfTypePresent(Type testType, string typeName)
58+
{
59+
if (IsTypePresent(testType, typeName))
60+
{
61+
throw new Exception(typeName);
62+
}
63+
}
64+
65+
private static bool IsAttributePresent(Type testType, string memberName, string attributeName)
66+
{
67+
foreach (var member in testType.GetMembers())
68+
{
69+
if (member.Name == memberName)
70+
{
71+
foreach (var attribute in member.GetCustomAttributes(false))
72+
{
73+
if (attribute.GetType().Name == attributeName)
74+
{
75+
return true;
76+
}
77+
}
78+
}
79+
}
80+
return false;
81+
}
82+
83+
private static void ThrowIfAttributeNotPresent(Type testType, string memberName, string attributeName)
84+
{
85+
if (!IsAttributePresent(testType, memberName, attributeName))
86+
{
87+
throw new Exception(attributeName);
88+
}
89+
}
90+
91+
private static void ThrowIfAttributePresent(Type testType, string memberName, string attributeName)
92+
{
93+
if (IsAttributePresent(testType, memberName, attributeName))
94+
{
95+
throw new Exception(attributeName);
96+
}
97+
}
98+
}

0 commit comments

Comments
 (0)