Skip to content

Commit e9e33e1

Browse files
sbomersbomer
authored
Add ILLink/ILCompiler support for feature checks (#99267)
This will substitute static boolean properties with `false` when decorated with `[FeatureGuard(typeof(SomeFeature))]`, where `SomeFeature` is known to be disabled. The "feature" of unreferenced code, represented by `RequiresUnreferencedCodeAttribute`, is always considered disabled. For ILC, `RequiresDynamicCodeAttribute` and `RequiresAssemblyFilesAttribute` are also disabled. ILLink also respects the feature switch for `IsDynamicCodeSupported` to disable the `RequiresDynamicCodeAttribute` feature. We don't want this kicking in when trimming in library mode, so this adds an ILLink option to disable the optimization. Additionally, a property is substituted if it has `[FeatureSwitchDefinition("SomeFeatureName")]` and `"SomeFeatureName"` is set in the command-line arguments. XML substitutions take precedence over this behavior. Includes a few tweaks and cleanup to the analyzer logic.
1 parent 6aca07a commit e9e33e1

File tree

23 files changed

+794
-199
lines changed

23 files changed

+794
-199
lines changed

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutionProvider.cs

+74
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IO;
7+
using System.Reflection;
78
using System.Reflection.Metadata;
89
using System.Reflection.PortableExecutable;
910
using System.Resources;
@@ -33,11 +34,84 @@ public BodySubstitution GetSubstitution(MethodDesc method)
3334
AssemblyFeatureInfo info = _hashtable.GetOrCreateValue(ecmaMethod.Module);
3435
if (info.BodySubstitutions != null && info.BodySubstitutions.TryGetValue(ecmaMethod, out BodySubstitution result))
3536
return result;
37+
38+
if (TryGetFeatureCheckValue(ecmaMethod, out bool value))
39+
return BodySubstitution.Create(value ? 1 : 0);
3640
}
3741

3842
return null;
3943
}
4044

45+
private bool TryGetFeatureCheckValue(EcmaMethod method, out bool value)
46+
{
47+
value = false;
48+
49+
if (!method.Signature.IsStatic)
50+
return false;
51+
52+
if (!method.Signature.ReturnType.IsWellKnownType(WellKnownType.Boolean))
53+
return false;
54+
55+
if (FindProperty(method) is not PropertyPseudoDesc property)
56+
return false;
57+
58+
if (property.SetMethod != null)
59+
return false;
60+
61+
foreach (var featureSwitchDefinitionAttribute in property.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "FeatureSwitchDefinitionAttribute"))
62+
{
63+
if (featureSwitchDefinitionAttribute.FixedArguments is not [CustomAttributeTypedArgument<TypeDesc> { Value: string switchName }])
64+
continue;
65+
66+
// If there's a FeatureSwitchDefinition, don't continue looking for FeatureGuard.
67+
// We don't want to infer feature switch settings from FeatureGuard.
68+
return _hashtable._switchValues.TryGetValue(switchName, out value);
69+
}
70+
71+
foreach (var featureGuardAttribute in property.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "FeatureGuardAttribute"))
72+
{
73+
if (featureGuardAttribute.FixedArguments is not [CustomAttributeTypedArgument<TypeDesc> { Value: EcmaType featureType }])
74+
continue;
75+
76+
if (featureType.Namespace == "System.Diagnostics.CodeAnalysis") {
77+
switch (featureType.Name) {
78+
case "RequiresAssemblyFilesAttribute":
79+
case "RequiresUnreferencedCodeAttribute":
80+
return true;
81+
case "RequiresDynamicCodeAttribute":
82+
if (_hashtable._switchValues.TryGetValue(
83+
"System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported",
84+
out bool isDynamicCodeSupported)
85+
&& !isDynamicCodeSupported)
86+
return true;
87+
break;
88+
}
89+
}
90+
}
91+
92+
return false;
93+
94+
static PropertyPseudoDesc FindProperty(EcmaMethod method)
95+
{
96+
if ((method.Attributes & MethodAttributes.SpecialName) == 0)
97+
return null;
98+
99+
if (method.OwningType is not EcmaType declaringType)
100+
return null;
101+
102+
var reader = declaringType.MetadataReader;
103+
foreach (PropertyDefinitionHandle propertyHandle in reader.GetTypeDefinition(declaringType.Handle).GetProperties())
104+
{
105+
PropertyDefinition propertyDef = reader.GetPropertyDefinition(propertyHandle);
106+
var property = new PropertyPseudoDesc(declaringType, propertyHandle);
107+
if (property.GetMethod == method)
108+
return property;
109+
}
110+
111+
return null;
112+
}
113+
}
114+
41115
public object GetSubstitution(FieldDesc field)
42116
{
43117
if (field.GetTypicalFieldDefinition() is EcmaField ecmaField)

‎src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestDatabase.cs

+5
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ public static IEnumerable<object[]> SingleFile ()
6969
return TestNamesBySuiteName ();
7070
}
7171

72+
public static IEnumerable<object[]> Substitutions ()
73+
{
74+
return TestNamesBySuiteName ();
75+
}
76+
7277
public static IEnumerable<object[]> TopLevelStatements ()
7378
{
7479
return TestNamesBySuiteName ();

‎src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestSuites.cs

+14
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,20 @@ public void SingleFile (string t)
103103
Run (t);
104104
}
105105

106+
[Theory]
107+
[MemberData (nameof (TestDatabase.Substitutions), MemberType = typeof (TestDatabase))]
108+
public void Substitutions (string t)
109+
{
110+
switch (t) {
111+
case "FeatureGuardSubstitutions":
112+
Run (t);
113+
break;
114+
default:
115+
// Skip the rest for now
116+
break;
117+
}
118+
}
119+
106120
[Theory]
107121
[MemberData (nameof (TestDatabase.TopLevelStatements), MemberType = typeof (TestDatabase))]
108122
public void TopLevelStatements (string t)

‎src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes
7474
internal static ValueSet<string> GetFeatureGuardAnnotations (this IPropertySymbol propertySymbol)
7575
{
7676
HashSet<string> featureSet = new ();
77-
foreach (var attributeData in propertySymbol.GetAttributes (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureGuardAttribute)) {
78-
if (attributeData.ConstructorArguments is [TypedConstant { Value: INamedTypeSymbol featureType }])
77+
foreach (var featureGuardAttribute in propertySymbol.GetAttributes (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureGuardAttribute)) {
78+
if (featureGuardAttribute.ConstructorArguments is [TypedConstant { Value: INamedTypeSymbol featureType }])
7979
featureSet.Add (featureType.GetDisplayName ());
8080
}
8181
return featureSet.Count == 0 ? ValueSet<string>.Empty : new ValueSet<string> (featureSet);

‎src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ protected virtual bool CreateSpecialIncompatibleMembersDiagnostic (
305305
internal static bool IsAnnotatedFeatureGuard (IPropertySymbol propertySymbol, string featureName)
306306
{
307307
// Only respect FeatureGuardAttribute on static boolean properties.
308-
if (!propertySymbol.IsStatic || propertySymbol.Type.SpecialType != SpecialType.System_Boolean)
308+
if (!propertySymbol.IsStatic || propertySymbol.Type.SpecialType != SpecialType.System_Boolean || propertySymbol.SetMethod != null)
309309
return false;
310310

311311
ValueSet<string> featureCheckAnnotations = propertySymbol.GetFeatureGuardAnnotations ();

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public IEnumerable<Diagnostic> CollectDiagnostics (DataFlowAnalyzerContext conte
3636
if (!context.EnableTrimAnalyzer)
3737
return diagnosticContext.Diagnostics;
3838

39-
if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean) {
40-
// Warn about invalid feature checks (non-static or non-bool properties)
39+
if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean || OwningSymbol.SetMethod != null) {
40+
// Warn about invalid feature checks (non-static or non-bool properties or properties with setter)
4141
diagnosticContext.AddDiagnostic (
4242
DiagnosticId.InvalidFeatureGuard);
4343
return diagnosticContext.Diagnostics;

‎src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs

+2-4
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@ public class TrimAnalysisVisitor : LocalDataFlowVisitor<
4343

4444
FeatureChecksVisitor _featureChecksVisitor;
4545

46-
DataFlowAnalyzerContext _dataFlowAnalyzerContext;
47-
4846
public TrimAnalysisVisitor (
4947
Compilation compilation,
5048
LocalStateAndContextLattice<MultiValue, FeatureContext, ValueSetLattice<SingleValue>, FeatureContextLattice> lattice,
@@ -59,7 +57,6 @@ public TrimAnalysisVisitor (
5957
_multiValueLattice = lattice.LocalStateLattice.Lattice.ValueLattice;
6058
TrimAnalysisPatterns = trimAnalysisPatterns;
6159
_featureChecksVisitor = new FeatureChecksVisitor (dataFlowAnalyzerContext);
62-
_dataFlowAnalyzerContext = dataFlowAnalyzerContext;
6360
}
6461

6562
public override FeatureChecksValue GetConditionValue (IOperation branchValueOperation, StateValue state)
@@ -436,7 +433,8 @@ public override void HandleReturnConditionValue (FeatureChecksValue returnCondit
436433
if (OwningSymbol is not IMethodSymbol method)
437434
return;
438435

439-
// FeatureGuard validation needs to happen only for properties.
436+
// FeatureGuard validation needs to happen only for property getters.
437+
// Include properties with setters here because they will get validated later.
440438
if (method.MethodKind != MethodKind.PropertyGet)
441439
return;
442440

‎src/tools/illink/src/linker/Linker.Steps/RootAssemblyInputStep.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ protected override void Process ()
7878
CodeOptimizations.RemoveLinkAttributes |
7979
CodeOptimizations.RemoveSubstitutions |
8080
CodeOptimizations.RemoveDynamicDependencyAttribute |
81-
CodeOptimizations.OptimizeTypeHierarchyAnnotations, assembly.Name.Name);
81+
CodeOptimizations.OptimizeTypeHierarchyAnnotations |
82+
CodeOptimizations.SubstituteFeatureGuards, assembly.Name.Name);
8283

8384
// Enable EventSource special handling
8485
Context.DisableEventSourceSpecialHandling = false;

‎src/tools/illink/src/linker/Linker/CustomAttributeSource.cs

+8
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ public bool TryGetEmbeddedXmlInfo (ICustomAttributeProvider provider, [NotNullWh
5252
return xmlInfo != null;
5353
}
5454

55+
public IEnumerable<CustomAttribute> GetCustomAttributes (ICustomAttributeProvider provider, string attributeNamespace, string attributeName)
56+
{
57+
foreach (var attr in GetCustomAttributes (provider)) {
58+
if (attr.AttributeType.Namespace == attributeNamespace && attr.AttributeType.Name == attributeName)
59+
yield return attr;
60+
}
61+
}
62+
5563
public IEnumerable<CustomAttribute> GetCustomAttributes (ICustomAttributeProvider provider)
5664
{
5765
if (provider.HasCustomAttributes) {

‎src/tools/illink/src/linker/Linker/Driver.cs

+4
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,9 @@ protected bool GetOptimizationName (string text, out CodeOptimizations optimizat
11791179
case "sealer":
11801180
optimization = CodeOptimizations.Sealer;
11811181
return true;
1182+
case "substitutefeatureguards":
1183+
optimization = CodeOptimizations.SubstituteFeatureGuards;
1184+
return true;
11821185
}
11831186

11841187
Context.LogError (null, DiagnosticId.InvalidOptimizationValue, text);
@@ -1361,6 +1364,7 @@ static void Usage ()
13611364
Console.WriteLine (" unreachablebodies: Instance methods that are marked but not executed are converted to throws");
13621365
Console.WriteLine (" unusedinterfaces: Removes interface types from declaration when not used");
13631366
Console.WriteLine (" unusedtypechecks: Inlines never successful type checks");
1367+
Console.WriteLine (" substitutefeatureguards: Substitutes properties annotated as FeatureGuard(typeof(RequiresUnreferencedCodeAttribute)) to false");
13641368
Console.WriteLine (" --enable-opt NAME [ASM] Enable one of the additional optimizations globaly or for a specific assembly name");
13651369
Console.WriteLine (" sealer: Any method or type which does not have override is marked as sealed");
13661370
Console.WriteLine (" --explicit-reflection Adds to members never used through reflection DisablePrivateReflection attribute. Defaults to false");

‎src/tools/illink/src/linker/Linker/LinkContext.cs

+7-1
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ protected LinkContext (Pipeline pipeline, ILogger logger, string outputDirectory
246246
CodeOptimizations.RemoveLinkAttributes |
247247
CodeOptimizations.RemoveSubstitutions |
248248
CodeOptimizations.RemoveDynamicDependencyAttribute |
249-
CodeOptimizations.OptimizeTypeHierarchyAnnotations;
249+
CodeOptimizations.OptimizeTypeHierarchyAnnotations |
250+
CodeOptimizations.SubstituteFeatureGuards;
250251

251252
DisableEventSourceSpecialHandling = true;
252253

@@ -1144,5 +1145,10 @@ public enum CodeOptimizations
11441145
/// Otherwise, type annotation will only be applied with calls to object.GetType()
11451146
/// </summary>
11461147
OptimizeTypeHierarchyAnnotations = 1 << 24,
1148+
1149+
/// <summary>
1150+
/// Option to substitute properties annotated as FeatureGuard(typeof(RequiresUnreferencedCodeAttribute)) with false
1151+
/// </summary>
1152+
SubstituteFeatureGuards = 1 << 25,
11471153
}
11481154
}

‎src/tools/illink/src/linker/Linker/MemberActionStore.cs

+76-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.Diagnostics;
56
using System.Diagnostics.CodeAnalysis;
7+
using ILLink.Shared;
68
using Mono.Cecil;
79

810
namespace Mono.Linker
@@ -20,7 +22,7 @@ public MemberActionStore (LinkContext context)
2022
_context = context;
2123
}
2224

23-
public bool TryGetSubstitutionInfo (MemberReference member, [NotNullWhen (true)] out SubstitutionInfo? xmlInfo)
25+
private bool TryGetSubstitutionInfo (MemberReference member, [NotNullWhen (true)] out SubstitutionInfo? xmlInfo)
2426
{
2527
var assembly = member.Module.Assembly;
2628
if (!_embeddedXmlInfos.TryGetValue (assembly, out xmlInfo)) {
@@ -41,6 +43,9 @@ public MethodAction GetAction (MethodDefinition method)
4143
return action;
4244
}
4345

46+
if (TryGetFeatureCheckValue (method, out _))
47+
return MethodAction.ConvertToStub;
48+
4449
return MethodAction.Nothing;
4550
}
4651

@@ -49,10 +54,78 @@ public bool TryGetMethodStubValue (MethodDefinition method, out object? value)
4954
if (PrimarySubstitutionInfo.MethodStubValues.TryGetValue (method, out value))
5055
return true;
5156

52-
if (!TryGetSubstitutionInfo (method, out var embeddedXml))
57+
if (TryGetSubstitutionInfo (method, out var embeddedXml)
58+
&& embeddedXml.MethodStubValues.TryGetValue (method, out value))
59+
return true;
60+
61+
if (TryGetFeatureCheckValue (method, out bool bValue)) {
62+
value = bValue ? 1 : 0;
63+
return true;
64+
}
65+
66+
return false;
67+
}
68+
69+
internal bool TryGetFeatureCheckValue (MethodDefinition method, out bool value)
70+
{
71+
value = false;
72+
73+
if (!method.IsStatic)
74+
return false;
75+
76+
if (method.ReturnType.MetadataType != MetadataType.Boolean)
77+
return false;
78+
79+
if (FindProperty (method) is not PropertyDefinition property)
5380
return false;
5481

55-
return embeddedXml.MethodStubValues.TryGetValue (method, out value);
82+
if (property.SetMethod != null)
83+
return false;
84+
85+
foreach (var featureSwitchDefinitionAttribute in _context.CustomAttributes.GetCustomAttributes (property, "System.Diagnostics.CodeAnalysis", "FeatureSwitchDefinitionAttribute")) {
86+
if (featureSwitchDefinitionAttribute.ConstructorArguments is not [CustomAttributeArgument { Value: string switchName }])
87+
continue;
88+
89+
// If there's a FeatureSwitchDefinition, don't continue looking for FeatureGuard.
90+
// We don't want to infer feature switch settings from FeatureGuard.
91+
return _context.FeatureSettings.TryGetValue (switchName, out value);
92+
}
93+
94+
if (!_context.IsOptimizationEnabled (CodeOptimizations.SubstituteFeatureGuards, method))
95+
return false;
96+
97+
foreach (var featureGuardAttribute in _context.CustomAttributes.GetCustomAttributes (property, "System.Diagnostics.CodeAnalysis", "FeatureGuardAttribute")) {
98+
if (featureGuardAttribute.ConstructorArguments is not [CustomAttributeArgument { Value: TypeReference featureType }])
99+
continue;
100+
101+
if (featureType.Namespace == "System.Diagnostics.CodeAnalysis") {
102+
switch (featureType.Name) {
103+
case "RequiresUnreferencedCodeAttribute":
104+
return true;
105+
case "RequiresDynamicCodeAttribute":
106+
if (_context.FeatureSettings.TryGetValue (
107+
"System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported",
108+
out bool isDynamicCodeSupported)
109+
&& !isDynamicCodeSupported)
110+
return true;
111+
break;
112+
}
113+
}
114+
}
115+
116+
return false;
117+
118+
static PropertyDefinition? FindProperty (MethodDefinition method) {
119+
if (!method.IsGetter)
120+
return null;
121+
122+
foreach (var property in method.DeclaringType.Properties) {
123+
if (property.GetMethod == method)
124+
return property;
125+
}
126+
127+
return null;
128+
}
56129
}
57130

58131
public bool TryGetFieldUserValue (FieldDefinition field, out object? value)

‎src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/SubstitutionsTests.cs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using Xunit;
4+
5+
namespace ILLink.RoslynAnalyzer.Tests
6+
{
7+
public sealed partial class SubstitutionsTests : LinkerTestBase
8+
{
9+
protected override string TestSuiteName => "Substitutions";
10+
11+
[Fact]
12+
public Task FeatureGuardSubstitutions ()
13+
{
14+
return RunTest ();
15+
}
16+
}
17+
}

‎src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SubstitutionsTests.g.cs

+6-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ namespace ILLink.RoslynAnalyzer.Tests
77
public sealed partial class SubstitutionsTests : LinkerTestBase
88
{
99

10-
protected override string TestSuiteName => "Substitutions";
11-
1210
[Fact]
1311
public Task EmbeddedFieldSubstitutionsInReferencedAssembly ()
1412
{
@@ -45,6 +43,12 @@ public Task EmbeddedSubstitutionsNotProcessedWithIgnoreSubstitutionsAndRemoved (
4543
return RunTest (allowMissingWarnings: true);
4644
}
4745

46+
[Fact]
47+
public Task FeatureGuardSubstitutionsDisabled ()
48+
{
49+
return RunTest (allowMissingWarnings: true);
50+
}
51+
4852
[Fact]
4953
public Task InitField ()
5054
{

0 commit comments

Comments
 (0)