diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs index 268833431274a0..028628f2dd5934 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs @@ -13,11 +13,15 @@ namespace ILLink.RoslynAnalyzer.DataFlow // For now, this is only designed to track the built-in "features"/"capabilities" // like RuntimeFeatures.IsDynamicCodeSupported, where a true return value // indicates that a feature/capability is available. - public record struct FeatureChecksValue : INegate + public record struct FeatureChecksValue : INegate, IDeepCopyValue { public ValueSet EnabledFeatures; public ValueSet DisabledFeatures; + public static readonly FeatureChecksValue All = new FeatureChecksValue (ValueSet.Unknown, ValueSet.Empty); + + public static readonly FeatureChecksValue None = new FeatureChecksValue (ValueSet.Empty, ValueSet.Empty); + public FeatureChecksValue (string enabledFeature) { EnabledFeatures = new ValueSet (enabledFeature); @@ -48,5 +52,10 @@ public FeatureChecksValue Negate () { return new FeatureChecksValue (DisabledFeatures.DeepCopy (), EnabledFeatures.DeepCopy ()); } + + public FeatureChecksValue DeepCopy () + { + return new FeatureChecksValue (EnabledFeatures.DeepCopy (), DisabledFeatures.DeepCopy ()); + } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureCheckVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs similarity index 60% rename from src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureCheckVisitor.cs rename to src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs index 707294718fda32..7c0935eb05ea48 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureCheckVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs @@ -24,7 +24,7 @@ namespace ILLink.RoslynAnalyzer.DataFlow // (a set features that are checked to be enabled or disabled). // The visitor takes a LocalDataFlowState as an argument, allowing for checks that // depend on the current dataflow state. - public class FeatureChecksVisitor : OperationVisitor + public class FeatureChecksVisitor : OperationVisitor { DataFlowAnalyzerContext _dataFlowAnalyzerContext; @@ -33,32 +33,48 @@ public FeatureChecksVisitor (DataFlowAnalyzerContext dataFlowAnalyzerContext) _dataFlowAnalyzerContext = dataFlowAnalyzerContext; } - public override FeatureChecksValue? VisitArgument (IArgumentOperation operation, StateValue state) + public override FeatureChecksValue DefaultVisit (IOperation operation, StateValue state) + { + // Visiting a non-understood pattern should return the empty set of features, which will + // prevent this check from acting as a guard for any feature. + return FeatureChecksValue.None; + } + + public override FeatureChecksValue VisitArgument (IArgumentOperation operation, StateValue state) { return Visit (operation.Value, state); } - public override FeatureChecksValue? VisitPropertyReference (IPropertyReferenceOperation operation, StateValue state) + public override FeatureChecksValue VisitPropertyReference (IPropertyReferenceOperation operation, StateValue state) { + // A single property may serve as a feature check for multiple features. + FeatureChecksValue featureChecks = FeatureChecksValue.None; foreach (var analyzer in _dataFlowAnalyzerContext.EnabledRequiresAnalyzers) { - if (analyzer.IsRequiresCheck (_dataFlowAnalyzerContext.Compilation, operation.Property)) { - return new FeatureChecksValue (analyzer.FeatureName); + if (analyzer.IsFeatureCheck (operation.Property, _dataFlowAnalyzerContext.Compilation)) { + var featureCheck = new FeatureChecksValue (analyzer.RequiresAttributeFullyQualifiedName); + featureChecks = featureChecks.And (featureCheck); } } - return null; + return featureChecks; } - public override FeatureChecksValue? VisitUnaryOperator (IUnaryOperation operation, StateValue state) + public override FeatureChecksValue VisitUnaryOperator (IUnaryOperation operation, StateValue state) { if (operation.OperatorKind is not UnaryOperatorKind.Not) - return null; + return FeatureChecksValue.None; - FeatureChecksValue? context = Visit (operation.Operand, state); - if (context == null) - return null; + FeatureChecksValue context = Visit (operation.Operand, state); + return context.Negate (); + } - return context.Value.Negate (); + public override FeatureChecksValue VisitLiteral (ILiteralOperation operation, StateValue state) + { + // 'false' can guard any feature + if (GetConstantBool (operation.ConstantValue) is false) + return FeatureChecksValue.All; + + return FeatureChecksValue.None; } public bool? GetLiteralBool (IOperation operation) @@ -77,7 +93,7 @@ public FeatureChecksVisitor (DataFlowAnalyzerContext dataFlowAnalyzerContext) return value; } - public override FeatureChecksValue? VisitBinaryOperator (IBinaryOperation operation, StateValue state) + public override FeatureChecksValue VisitBinaryOperator (IBinaryOperation operation, StateValue state) { bool expectEqual; switch (operation.OperatorKind) { @@ -88,36 +104,32 @@ public FeatureChecksVisitor (DataFlowAnalyzerContext dataFlowAnalyzerContext) expectEqual = false; break; default: - return null; + return FeatureChecksValue.None; } if (GetLiteralBool (operation.LeftOperand) is bool leftBool) { - if (Visit (operation.RightOperand, state) is not FeatureChecksValue rightValue) - return null; + FeatureChecksValue rightValue = Visit (operation.RightOperand, state); return leftBool == expectEqual ? rightValue : rightValue.Negate (); } if (GetLiteralBool (operation.RightOperand) is bool rightBool) { - if (Visit (operation.LeftOperand, state) is not FeatureChecksValue leftValue) - return null; + FeatureChecksValue leftValue = Visit (operation.LeftOperand, state); return rightBool == expectEqual ? leftValue : leftValue.Negate (); } - return null; + return FeatureChecksValue.None; } - public override FeatureChecksValue? VisitIsPattern (IIsPatternOperation operation, StateValue state) + public override FeatureChecksValue VisitIsPattern (IIsPatternOperation operation, StateValue state) { if (GetExpectedValueFromPattern (operation.Pattern) is not bool patternValue) - return null; - - if (Visit (operation.Value, state) is not FeatureChecksValue value) - return null; + return FeatureChecksValue.None; + FeatureChecksValue value = Visit (operation.Value, state); return patternValue ? value : value.Negate (); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index bbaeff53f0c591..dc2345b5646323 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -88,12 +88,12 @@ public LocalDataFlowVisitor ( return null; var branchValue = Visit (branchValueOperation, state); - + TConditionValue conditionValue = GetConditionValue (branchValueOperation, state); if (block.Block.ConditionKind != ControlFlowConditionKind.None) { // BranchValue may represent a value used in a conditional branch to the ConditionalSuccessor. // If so, give the analysis an opportunity to model the checked condition, and return the model // of the condition back to the generic analysis. It will be applied to the state of each outgoing branch. - return GetConditionValue (branchValueOperation, state); + return conditionValue; } // If not, the BranchValue represents a return or throw value associated with the FallThroughSuccessor of this block. @@ -118,10 +118,13 @@ public LocalDataFlowVisitor ( // We don't want the return operation because this might have multiple possible return values in general. var current = state.Current; HandleReturnValue (branchValue, branchValueOperation, in current.Context); + // Must be called for every return value even if it did not return an understood condition, + // because the non-understood conditions will produce warnings for FeatureCheck properties. + HandleReturnConditionValue (conditionValue, branchValueOperation); return null; } - public abstract TConditionValue? GetConditionValue ( + public abstract TConditionValue GetConditionValue ( IOperation branchValueOperation, LocalDataFlowState state); @@ -146,6 +149,10 @@ public abstract void HandleReturnValue ( IOperation operation, in TContext context); + public abstract void HandleReturnConditionValue ( + TConditionValue returnConditionValue, + IOperation branchValueOperation); + // This is called for any method call, which includes: // - Normal invocation operation // - Accessing property value - which is treated as a call to the getter @@ -776,9 +783,7 @@ TValue HandleMethodCallHelper ( // Get the condition value that is being asserted. If the attribute is DoesNotReturnIf(true), // the condition value needs to be negated so that we can assert the false condition. - if (GetConditionValue (argumentOperation, state) is not TConditionValue conditionValue) - continue; - + TConditionValue conditionValue = GetConditionValue (argumentOperation, state); var current = state.Current; ApplyCondition ( doesNotReturnIfConditionValue == false diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlowAnalyzerContext.cs similarity index 100% rename from src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs rename to src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlowAnalyzerContext.cs diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index 173bb667a4d178..8c506606c292da 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -21,6 +21,8 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute); public const string attributeArgument = "attributeArgument"; public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute; + public const string FullyQualifiedFeatureCheckAttribute = "System.Diagnostics.CodeAnalysis.FeatureCheckAttribute"; + public const string FullyQualifiedFeatureDependsOnAttribute = "System.Diagnostics.CodeAnalysis.FeatureDependsOnAttribute"; public static Lazy> RequiresAnalyzers { get; } = new Lazy> (GetRequiresAnalyzers); static ImmutableArray GetRequiresAnalyzers () => ImmutableArray.Create ( @@ -51,6 +53,8 @@ public static ImmutableArray GetSupportedDiagnostics () diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedTypeNameInTypeGetType)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedParameterInMethodCreateInstance)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ParametersOfAssemblyCreateInstanceCannotBeAnalyzed)); + diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ReturnValueDoesNotMatchFeatureChecks)); + diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.InvalidFeatureCheck)); foreach (var requiresAnalyzer in RequiresAnalyzers.Value) { foreach (var diagnosticDescriptor in requiresAnalyzer.SupportedDiagnostics) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs index 7e830f7c6ecd4c..42ad2f9c9ae177 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs @@ -1,9 +1,14 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Collections.Immutable; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Text; using Microsoft.CodeAnalysis; +using ILLink.RoslynAnalyzer.DataFlow; +using ILLink.Shared.DataFlow; namespace ILLink.RoslynAnalyzer { @@ -34,6 +39,14 @@ internal static bool TryGetAttribute (this ISymbol member, string attributeName, return false; } + internal static IEnumerable GetAttributes (this ISymbol member, string attributeName) + { + foreach (var attr in member.GetAttributes ()) { + if (attr.AttributeClass is { } attrClass && attrClass.HasName (attributeName)) + yield return attr; + } + } + internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes (this ISymbol symbol) { if (!TryGetAttribute (symbol, DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var dynamicallyAccessedMembers)) @@ -58,6 +71,28 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes return (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers.ConstructorArguments[0].Value!; } + internal static ValueSet GetFeatureCheckAnnotations (this IPropertySymbol propertySymbol) + { + HashSet featureSet = new (); + foreach (var attributeData in propertySymbol.GetAttributes (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureCheckAttribute)) { + if (attributeData.ConstructorArguments is [TypedConstant { Value: INamedTypeSymbol featureType }]) + AddFeatures (featureType); + } + return featureSet.Count == 0 ? ValueSet.Empty : new ValueSet (featureSet); + + void AddFeatures (INamedTypeSymbol featureType) { + var featureName = featureType.GetDisplayName (); + if (!featureSet.Add (featureName)) + return; + + // Look at FeatureDependsOn attributes on the feature type. + foreach (var featureTypeAttributeData in featureType.GetAttributes (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureDependsOnAttribute)) { + if (featureTypeAttributeData.ConstructorArguments is [TypedConstant { Value: INamedTypeSymbol featureTypeSymbol }]) + AddFeatures (featureTypeSymbol); + } + } + } + internal static bool TryGetReturnAttribute (this IMethodSymbol member, string attributeName, [NotNullWhen (returnValue: true)] out AttributeData? attribute) { attribute = null; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index d951404845cd41..5b4c820c4c6d55 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -5,8 +5,9 @@ using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Linq; -using ILLink.Shared; using ILLink.RoslynAnalyzer.DataFlow; +using ILLink.Shared; +using ILLink.Shared.DataFlow; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -19,9 +20,7 @@ public abstract class RequiresAnalyzerBase : DiagnosticAnalyzer { private protected abstract string RequiresAttributeName { get; } - internal abstract string FeatureName { get; } - - private protected abstract string RequiresAttributeFullyQualifiedName { get; } + internal abstract string RequiresAttributeFullyQualifiedName { get; } private protected abstract DiagnosticTargets AnalyzerDiagnosticTargets { get; } @@ -301,7 +300,23 @@ protected virtual bool CreateSpecialIncompatibleMembersDiagnostic ( // - false return value indicating that a feature is supported // - feature settings supplied by the project // - custom feature checks defined in library code - internal virtual bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) => false; + private protected virtual bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) => false; + + internal static bool IsAnnotatedFeatureCheck (IPropertySymbol propertySymbol, string featureName) + { + // Only respect FeatureCheckAttribute on static boolean properties. + if (!propertySymbol.IsStatic || propertySymbol.Type.SpecialType != SpecialType.System_Boolean) + return false; + + ValueSet featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (); + return featureCheckAnnotations.Contains (featureName); + } + + internal bool IsFeatureCheck (IPropertySymbol propertySymbol, Compilation compilation) + { + return IsAnnotatedFeatureCheck (propertySymbol, RequiresAttributeFullyQualifiedName) + || IsRequiresCheck (propertySymbol, compilation); + } internal bool CheckAndCreateRequiresDiagnostic ( IOperation operation, @@ -312,7 +327,7 @@ internal bool CheckAndCreateRequiresDiagnostic ( [NotNullWhen (true)] out Diagnostic? diagnostic) { // Warnings are not emitted if the featureContext says the feature is available. - if (featureContext.IsEnabled (FeatureName)) { + if (featureContext.IsEnabled (RequiresAttributeFullyQualifiedName)) { diagnostic = null; return false; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs index e8807896d93735..8949b249b35ecb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs @@ -36,9 +36,7 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase private protected override string RequiresAttributeName => RequiresAssemblyFilesAttribute; - internal override string FeatureName => "AssemblyFiles"; - - private protected override string RequiresAttributeFullyQualifiedName => RequiresAssemblyFilesAttributeFullyQualifiedName; + internal override string RequiresAttributeFullyQualifiedName => RequiresAssemblyFilesAttributeFullyQualifiedName; private protected override DiagnosticTargets AnalyzerDiagnosticTargets => DiagnosticTargets.MethodOrConstructor | DiagnosticTargets.Property | DiagnosticTargets.Event; @@ -61,7 +59,7 @@ internal override bool IsAnalyzerEnabled (AnalyzerOptions options) return true; } - internal override bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { // "IsAssemblyFilesSupported" is treated as a requires check for testing purposes only, and // is not officially-supported product behavior. diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs index 5232ca9a9854e3..34bb7808d20314 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs @@ -25,9 +25,7 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase private protected override string RequiresAttributeName => RequiresDynamicCodeAttribute; - internal override string FeatureName => "DynamicCode"; - - private protected override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresDynamicCodeAttribute; + internal override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresDynamicCodeAttribute; private protected override DiagnosticTargets AnalyzerDiagnosticTargets => DiagnosticTargets.MethodOrConstructor | DiagnosticTargets.Class; @@ -40,7 +38,7 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer); - internal override bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) { + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { var runtimeFeaturesType = compilation.GetTypeByMetadataName ("System.Runtime.CompilerServices.RuntimeFeature"); if (runtimeFeaturesType == null) return false; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs index 3623150b752057..69c38629c43e17 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs @@ -49,11 +49,7 @@ private Action typeDerivesFromRucBase { private protected override string RequiresAttributeName => RequiresUnreferencedCodeAttribute; - public const string UnreferencedCode = nameof (UnreferencedCode); - - internal override string FeatureName => UnreferencedCode; - - private protected override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresUnreferencedCodeAttribute; + internal override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresUnreferencedCodeAttribute; private protected override DiagnosticTargets AnalyzerDiagnosticTargets => DiagnosticTargets.MethodOrConstructor | DiagnosticTargets.Class; @@ -66,7 +62,7 @@ private Action typeDerivesFromRucBase { internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer); - internal override bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { // "IsUnreferencedCodeSupported" is treated as a requires check for testing purposes only, and // is not officially-supported product behavior. diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs new file mode 100644 index 00000000000000..b46ef52e1f3a62 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs @@ -0,0 +1,70 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using ILLink.Shared; +using ILLink.Shared.DataFlow; +using ILLink.Shared.TrimAnalysis; +using ILLink.RoslynAnalyzer.DataFlow; +using Microsoft.CodeAnalysis; + +namespace ILLink.RoslynAnalyzer.TrimAnalysis +{ + public readonly record struct FeatureCheckReturnValuePattern + { + public FeatureChecksValue ReturnValue { get; init; } + public ValueSet FeatureCheckAnnotations { get; init; } + public IOperation Operation { get; init; } + public IPropertySymbol OwningSymbol { get; init; } + + public FeatureCheckReturnValuePattern ( + FeatureChecksValue returnValue, + ValueSet featureCheckAnnotations, + IOperation operation, + IPropertySymbol owningSymbol) + { + ReturnValue = returnValue.DeepCopy (); + FeatureCheckAnnotations = featureCheckAnnotations.DeepCopy (); + Operation = operation; + OwningSymbol = owningSymbol; + } + + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) + { + var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); + // For now, feature check validation is enabled only when trim analysis is enabled. + if (!context.EnableTrimAnalyzer) + return diagnosticContext.Diagnostics; + + if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean) { + // Warn about invalid feature checks (non-static or non-bool properties) + diagnosticContext.AddDiagnostic ( + DiagnosticId.InvalidFeatureCheck); + return diagnosticContext.Diagnostics; + } + + if (ReturnValue == FeatureChecksValue.All) + return diagnosticContext.Diagnostics; + + ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; + // For any analyzer-supported feature that this property is declared to guard, + // the abstract return value must include that feature + // (indicating it is known to be enabled when the return value is true). + foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { + foreach (var analyzer in context.EnabledRequiresAnalyzers) { + if (feature != analyzer.RequiresAttributeFullyQualifiedName) + continue; + + if (!returnValueFeatures.Contains (feature)) { + diagnosticContext.AddDiagnostic ( + DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, + OwningSymbol.GetDisplayName (), + feature); + } + } + } + + return diagnosticContext.Diagnostics; + } + } +} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs index 5dfc31db3486f9..2ffcbc43ae8846 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs @@ -58,7 +58,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) { + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { foreach (var sourceValue in Source.AsEnumerable ()) { foreach (var targetValue in Target.AsEnumerable ()) { // The target should always be an annotated value, but the visitor design currently prevents diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisGenericInstantiationPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisGenericInstantiationPattern.cs index 8d484e66036b9a..26f275085fa8bf 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisGenericInstantiationPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisGenericInstantiationPattern.cs @@ -48,7 +48,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) { + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { switch (GenericInstantiation) { case INamedTypeSymbol type: GenericArgumentDataFlow.ProcessGenericArgumentDataFlow (diagnosticContext, type); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs index 8341afa2ea5f24..3dfd7fa285521a 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs @@ -77,7 +77,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope(out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { TrimAnalysisVisitor.HandleCall(Operation, OwningSymbol, CalledMethod, Instance, Arguments, diagnosticContext, default, out var _); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index 78de8fdf4235ce..dd66d802934be6 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -17,16 +17,20 @@ public readonly struct TrimAnalysisPatternStore readonly Dictionary GenericInstantiationPatterns; readonly Dictionary MethodCallPatterns; readonly Dictionary ReflectionAccessPatterns; + readonly Dictionary FeatureCheckReturnValuePatterns; readonly ValueSetLattice Lattice; readonly FeatureContextLattice FeatureContextLattice; - public TrimAnalysisPatternStore (ValueSetLattice lattice, FeatureContextLattice featureContextLattice) + public TrimAnalysisPatternStore ( + ValueSetLattice lattice, + FeatureContextLattice featureContextLattice) { AssignmentPatterns = new Dictionary<(IOperation, bool), TrimAnalysisAssignmentPattern> (); FieldAccessPatterns = new Dictionary (); GenericInstantiationPatterns = new Dictionary (); MethodCallPatterns = new Dictionary (); ReflectionAccessPatterns = new Dictionary (); + FeatureCheckReturnValuePatterns = new Dictionary (); Lattice = lattice; FeatureContextLattice = featureContextLattice; } @@ -89,6 +93,16 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) ReflectionAccessPatterns[pattern.Operation] = pattern.Merge (Lattice, FeatureContextLattice, existingPattern); } + public void Add (FeatureCheckReturnValuePattern pattern) + { + if (!FeatureCheckReturnValuePatterns.TryGetValue (pattern.Operation, out var existingPattern)) { + FeatureCheckReturnValuePatterns.Add (pattern.Operation, pattern); + return; + } + + Debug.Assert (existingPattern == pattern, "Return values should be identical"); + } + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { foreach (var assignmentPattern in AssignmentPatterns.Values) { @@ -115,6 +129,11 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte foreach (var diagnostic in reflectionAccessPattern.CollectDiagnostics (context)) yield return diagnostic; } + + foreach (var returnValuePattern in FeatureCheckReturnValuePatterns.Values) { + foreach (var diagnostic in returnValuePattern.CollectDiagnostics (context)) + yield return diagnostic; + } } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs index 0e4c45a9f011fc..85897420596f49 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -50,7 +50,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) { + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { foreach (var diagnostic in ReflectionAccessAnalyzer.GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, ReferencedMethod)) diagnosticContext.AddDiagnostic (diagnostic); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 6f118ac8c47922..04efbb63ff5bda 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -43,6 +43,8 @@ public class TrimAnalysisVisitor : LocalDataFlowVisitor< FeatureChecksVisitor _featureChecksVisitor; + DataFlowAnalyzerContext _dataFlowAnalyzerContext; + public TrimAnalysisVisitor ( Compilation compilation, LocalStateAndContextLattice, FeatureContextLattice> lattice, @@ -57,14 +59,15 @@ public TrimAnalysisVisitor ( _multiValueLattice = lattice.LocalStateLattice.Lattice.ValueLattice; TrimAnalysisPatterns = trimAnalysisPatterns; _featureChecksVisitor = new FeatureChecksVisitor (dataFlowAnalyzerContext); + _dataFlowAnalyzerContext = dataFlowAnalyzerContext; } - public override FeatureChecksValue? GetConditionValue (IOperation branchValueOperation, StateValue state) + public override FeatureChecksValue GetConditionValue (IOperation branchValueOperation, StateValue state) { return _featureChecksVisitor.Visit (branchValueOperation, state); } - public override void ApplyCondition (FeatureChecksValue featureChecksValue, ref LocalStateAndContext currentState) + public override void ApplyCondition (FeatureChecksValue featureChecksValue, ref LocalStateAndContext currentState) { currentState.Context = currentState.Context.Union (new FeatureContext (featureChecksValue.EnabledFeatures)); } @@ -426,6 +429,28 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera } } + public override void HandleReturnConditionValue (FeatureChecksValue returnConditionValue, IOperation operation) + { + // Return statements should only happen inside of method bodies. + Debug.Assert (OwningSymbol is IMethodSymbol); + if (OwningSymbol is not IMethodSymbol method) + return; + + // FeatureCheck validation needs to happen only for properties. + if (method.MethodKind != MethodKind.PropertyGet) + return; + + IPropertySymbol propertySymbol = (IPropertySymbol) method.AssociatedSymbol!; + var featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (); + + // If there are no feature checks, there is nothing to validate. + if (featureCheckAnnotations.IsEmpty()) + return; + + TrimAnalysisPatterns.Add ( + new FeatureCheckReturnValuePattern (returnConditionValue, featureCheckAnnotations, operation, propertySymbol)); + } + public override MultiValue HandleDelegateCreation (IMethodSymbol method, IOperation operation, in FeatureContext featureContext) { TrimAnalysisPatterns.Add (new TrimAnalysisReflectionAccessPattern ( diff --git a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs index 1c33bb084a04af..1a3a27abbbdc27 100644 --- a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs +++ b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs @@ -202,6 +202,10 @@ public enum DiagnosticId GenericRecursionCycle = 3054, CorrectnessOfAbstractDelegatesCannotBeGuaranteed = 3055, RequiresDynamicCodeOnStaticConstructor = 3056, + + // Feature guard diagnostic ids. + ReturnValueDoesNotMatchFeatureChecks = 4000, + InvalidFeatureCheck = 4001 } public static class DiagnosticIdExtensions diff --git a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx index 111c1c5877de7f..ae50b7dcd9fff6 100644 --- a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx +++ b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx @@ -1197,4 +1197,16 @@ Unused 'UnconditionalSuppressMessageAttribute' found. Consider removing the unused warning suppression. - \ No newline at end of file + + Return value does not match FeatureCheckAttribute '{1}'. + + + Return value does not match FeatureCheck annotations of the property. The check should return false whenever any of the features referenced in the FeatureCheck annotations is disabled. + + + Invalid FeatureCheckAttribute. The attribute must be placed on a static boolean property with only a 'get' accessor. + + + Invalid FeatureCheckAttribute. + + diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index 2b1cf973d8a234..090a11b8616714 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -149,6 +149,12 @@ public Task FeatureCheckDataFlow () return RunTest (); } + [Fact] + public Task FeatureCheckAttributeDataFlow () + { + return RunTest (); + } + [Fact] public Task FieldDataFlow () { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs new file mode 100644 index 00000000000000..2d284c2b3ea375 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + // Allow AttributeTargets.Method for testing invalid usages of a custom FeatureCheckAttribute + [AttributeUsage (AttributeTargets.Property | AttributeTargets.Method, Inherited=false)] + public sealed class FeatureCheckAttribute : Attribute + { + public Type FeatureType { get; } + + public FeatureCheckAttribute (Type featureType) + { + FeatureType = featureType; + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs new file mode 100644 index 00000000000000..da9a19bbfebc6b --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + [AttributeUsage(AttributeTargets.Class, Inherited=false, AllowMultiple=true)] + public sealed class FeatureDependsOnAttribute : Attribute + { + public Type FeatureType { get; } + + public FeatureDependsOnAttribute(Type featureType) + { + FeatureType = featureType; + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs new file mode 100644 index 00000000000000..942c9f3586dd34 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace ILLink.RoslynAnalyzer +{ + public class TestFeatures + { + public static bool IsUnreferencedCodeSupported => true; + + public static bool IsAssemblyFilesSupported => true; + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs new file mode 100644 index 00000000000000..d8aa258c377546 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs @@ -0,0 +1,615 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Runtime.CompilerServices; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using ILLink.RoslynAnalyzer; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + // Note: the XML must be passed as an embedded resource named ILLink.Substitutions.xml, + // not as a separate substitution file, for it to work with NativeAot. + // Related: https://github.com/dotnet/runtime/issues/88647 + [SetupCompileBefore ("TestFeatures.dll", new[] { "Dependencies/TestFeatures.cs" }, + resources: new object[] { new [] { "FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml" } })] + // FeatureCheckAttribute is currently only supported by the analyzer. + // The same guard behavior is achieved for ILLink/ILCompiler using substitutions. + [SetupCompileResource ("FeatureCheckAttributeDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml")] + [IgnoreSubstitutions (false)] + public class FeatureCheckAttributeDataFlow + { + public static void Main () + { + DefineFeatureCheck.Test (); + ValidGuardBodies.Test (); + InvalidGuardBodies.Test (); + InvalidFeatureChecks.Test (); + } + + class DefineFeatureCheck { + [FeatureCheck (typeof(RequiresDynamicCodeAttribute))] + static bool GuardDynamicCode => RuntimeFeature.IsDynamicCodeSupported; + + static void TestGuardDynamicCode () + { + if (GuardDynamicCode) + RequiresDynamicCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool GuardUnreferencedCode => TestFeatures.IsUnreferencedCodeSupported; + + static void TestGuardUnreferencedCode () + { + if (GuardUnreferencedCode) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresAssemblyFilesAttribute))] + static bool GuardAssemblyFiles => TestFeatures.IsAssemblyFilesSupported; + + static void TestGuardAssemblyFiles () + { + if (GuardAssemblyFiles) + RequiresAssemblyFiles (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresDynamicCodeAttribute), ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(DynamicCodeAndUnreferencedCode))] + static bool GuardDynamicCodeAndUnreferencedCode => RuntimeFeature.IsDynamicCodeSupported && TestFeatures.IsUnreferencedCodeSupported; + + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + [FeatureDependsOn (typeof (RequiresUnreferencedCodeAttribute))] + static class DynamicCodeAndUnreferencedCode {} + + static void TestMultipleGuards () + { + if (GuardDynamicCodeAndUnreferencedCode) { + RequiresDynamicCode (); + RequiresUnreferencedCode (); + } + } + + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + static class DynamicCode1 {} + + [FeatureDependsOn (typeof (DynamicCode1))] + static class DynamicCode2 {} + + [FeatureCheck (typeof (DynamicCode2))] + static bool GuardDynamicCodeIndirect => RuntimeFeature.IsDynamicCodeSupported; + + static void TestIndirectGuard () + { + if (GuardDynamicCodeIndirect) + RequiresDynamicCode (); + } + + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + [FeatureDependsOn (typeof (DynamicCodeCycle))] + static class DynamicCodeCycle {} + + [FeatureCheck (typeof (DynamicCodeCycle))] + static bool GuardDynamicCodeCycle => RuntimeFeature.IsDynamicCodeSupported; + + [FeatureCheck (typeof (DynamicCodeCycle))] + static void TestFeatureDependencyCycle1 () + { + if (GuardDynamicCodeCycle) + RequiresDynamicCode (); + } + + [FeatureDependsOn (typeof (DynamicCodeCycle2_B))] + static class DynamicCodeCycle2_A {} + + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + [FeatureDependsOn (typeof (DynamicCodeCycle2_A))] + static class DynamicCodeCycle2_B {} + + [FeatureDependsOn (typeof (DynamicCodeCycle2_A))] + static class DynamicCodeCycle2 {} + + [FeatureCheck (typeof (DynamicCodeCycle2))] + static bool GuardDynamicCodeCycle2 => RuntimeFeature.IsDynamicCodeSupported; + + static void TestFeatureDependencyCycle2 () + { + if (GuardDynamicCodeCycle2) + RequiresDynamicCode (); + } + + public static void Test () + { + TestGuardDynamicCode (); + TestGuardUnreferencedCode (); + TestGuardAssemblyFiles (); + TestMultipleGuards (); + TestIndirectGuard (); + TestFeatureDependencyCycle1 (); + TestFeatureDependencyCycle2 (); + } + } + + class ValidGuardBodies { + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool ReturnFalseGuard => false; + + static void TestReturnFalseGuard () + { + if (ReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool DirectGuard => TestFeatures.IsUnreferencedCodeSupported; + + static void TestDirectGuard () + { + if (DirectGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IndirectGuard => DirectGuard; + + static void TestIndirectGuard () + { + if (IndirectGuard) + RequiresUnreferencedCode (); + } + + // Analyzer doesn't understand this pattern because it compiles into a CFG that effectively + // looks like this: + // + // bool tmp; + // if (TestFeatures.IsUnreferencedCodeSupported) + // tmp = OtherCondition (); + // else + // tmp = false; + // return tmp; + // + // The analyzer doesn't do constant propagation of the boolean, so it doesn't know that + // the return value is always false when TestFeatures.IsUnreferencedCodeSupported is false. + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool AndGuard => TestFeatures.IsUnreferencedCodeSupported && OtherCondition (); + + static void TestAndGuard () + { + if (AndGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool NotNotGuard => !!TestFeatures.IsUnreferencedCodeSupported; + + static void TestNotNotGuard () + { + if (NotNotGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool EqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported == true; + + static void TestEqualsTrueGuard () + { + if (EqualsTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool TrueEqualsGuard => true == TestFeatures.IsUnreferencedCodeSupported; + + static void TestTrueEqualsGuard () + { + if (TrueEqualsGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool NotEqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported != false; + + static void TestNotEqualsFalseGuard () + { + if (NotEqualsFalseGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool FalseNotEqualsGuard => false != TestFeatures.IsUnreferencedCodeSupported; + + static void TestFalseNotEqualsGuard () + { + if (FalseNotEqualsGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsTrueGuard => TestFeatures.IsUnreferencedCodeSupported is true; + + static void TestIsTrueGuard () + { + if (IsTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsNotFalseGuard => TestFeatures.IsUnreferencedCodeSupported is not false; + + static void TestIsNotFalseGuard () + { + if (IsNotFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IfReturnTrueGuard { + get { + if (TestFeatures.IsUnreferencedCodeSupported) + return true; + return false; + } + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool ElseReturnTrueGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + return false; + else + return true; + } + } + + static void TestElseReturnTrueGuard () + { + if (ElseReturnTrueGuard) + RequiresUnreferencedCode (); + } + + static void TestIfReturnTrueGuard () + { + if (IfReturnTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertReturnFalseGuard { + get { + Debug.Assert (TestFeatures.IsUnreferencedCodeSupported); + return false; + } + } + + static void TestAssertReturnFalseGuard () + { + if (AssertReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertNotReturnFalseGuard { + get { + Debug.Assert (!TestFeatures.IsUnreferencedCodeSupported); + return false; + } + } + + static void TestAssertNotReturnFalseGuard () + { + if (AssertNotReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertReturnTrueGuard { + get { + Debug.Assert (TestFeatures.IsUnreferencedCodeSupported); + return true; + } + } + + static void TestAssertReturnTrueGuard () + { + if (AssertReturnTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool ThrowGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + throw new Exception (); + return false; + } + } + + static void TestThrowGuard () + { + if (ThrowGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool TernaryIfGuard => TestFeatures.IsUnreferencedCodeSupported ? true : false; + + static void TestTernaryIfGuard () + { + if (TernaryIfGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool TernaryElseGuard => !TestFeatures.IsUnreferencedCodeSupported ? false : true; + + static void TestTernaryElseGuard () + { + if (TernaryElseGuard) + RequiresUnreferencedCode (); + } + + public static void Test () + { + TestDirectGuard (); + TestIndirectGuard (); + + TestReturnFalseGuard (); + TestAndGuard (); + TestNotNotGuard (); + TestEqualsTrueGuard (); + TestTrueEqualsGuard (); + TestNotEqualsFalseGuard (); + TestFalseNotEqualsGuard (); + TestIsTrueGuard (); + TestIsNotFalseGuard (); + TestIfReturnTrueGuard (); + TestElseReturnTrueGuard (); + TestAssertReturnFalseGuard (); + TestAssertNotReturnFalseGuard (); + TestAssertReturnTrueGuard (); + TestThrowGuard (); + TestTernaryIfGuard (); + TestTernaryElseGuard (); + } + } + + class InvalidGuardBodies { + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool ReturnTrueGuard => true; + + static void TestReturnTrueGuard () + { + if (ReturnTrueGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool OtherConditionGuard => OtherCondition (); + + static void TestOtherConditionGuard () + { + if (OtherConditionGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool OrGuard => TestFeatures.IsUnreferencedCodeSupported || OtherCondition (); + + static void TestOrGuard () + { + if (OrGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool NotGuard => !TestFeatures.IsUnreferencedCodeSupported; + + static void TestNotGuard () + { + if (NotGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool EqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported == false; + + static void TestEqualsFalseGuard () + { + if (EqualsFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool FalseEqualsGuard => false == TestFeatures.IsUnreferencedCodeSupported; + + static void TestFalseEqualsGuard () + { + if (FalseEqualsGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool NotEqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported != true; + + static void TestNotEqualsTrueGuard () + { + if (NotEqualsTrueGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool TrueNotEqualsGuard => true != TestFeatures.IsUnreferencedCodeSupported; + + static void TestTrueNotEqualsGuard () + { + if (TrueNotEqualsGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsNotTrueGuard => TestFeatures.IsUnreferencedCodeSupported is not true; + + static void TestIsNotTrueGuard () + { + if (IsNotTrueGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsFalseGuard => TestFeatures.IsUnreferencedCodeSupported is false; + + static void TestIsFalseGuard () + { + if (IsFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IfReturnFalseGuard { + get { + if (TestFeatures.IsUnreferencedCodeSupported) + return false; + return true; + } + } + + static void TestIfReturnFalseGuard () + { + if (IfReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool ElseReturnFalseGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + return true; + else + return false; + } + } + + static void TestElseReturnFalseGuard () + { + if (ElseReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertNotReturnTrueGuard { + get { + Debug.Assert (!TestFeatures.IsUnreferencedCodeSupported); + return true; + } + } + + static void TestAssertNotReturnTrueGuard () + { + if (AssertNotReturnTrueGuard) + RequiresUnreferencedCode (); + } + + public static void Test () + { + TestOtherConditionGuard (); + + TestReturnTrueGuard (); + TestOrGuard (); + TestNotGuard (); + TestEqualsFalseGuard (); + TestFalseEqualsGuard (); + TestNotEqualsTrueGuard (); + TestTrueNotEqualsGuard (); + TestIsNotTrueGuard (); + TestIsFalseGuard (); + TestIfReturnFalseGuard (); + TestElseReturnFalseGuard (); + TestAssertNotReturnTrueGuard (); + } + } + + class InvalidFeatureChecks { + [ExpectedWarning ("IL4001", ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static int NonBooleanProperty => 0; + + [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] + static void TestNonBooleanProperty () + { + if (NonBooleanProperty == 0) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4001", ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + bool NonStaticProperty => true; + + [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] + static void TestNonStaticProperty () + { + var instance = new InvalidFeatureChecks (); + if (instance.NonStaticProperty) + RequiresUnreferencedCode (); + } + + // No warning for this case because we don't validate that the attribute usage matches + // the expected AttributeUsage.Property for assemblies that define their own version + // of FeatureCheckAttributes. + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool Method () => true; + + [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] + static void TestMethod () + { + if (Method ()) + RequiresUnreferencedCode (); + } + + public static void Test () + { + TestNonBooleanProperty (); + TestNonStaticProperty (); + TestMethod (); + } + } + + [RequiresDynamicCode (nameof (RequiresDynamicCode))] + static void RequiresDynamicCode () { } + + [RequiresUnreferencedCode (nameof (RequiresUnreferencedCode))] + static void RequiresUnreferencedCode () { } + + [RequiresAssemblyFiles (nameof (RequiresAssemblyFiles))] + static void RequiresAssemblyFiles () { } + + static bool OtherCondition () => true; + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml new file mode 100644 index 00000000000000..828ef795ae3721 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs index 29f18ea7063890..d0d236997445ed 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs @@ -20,7 +20,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow // Note: the XML must be passed as an embedded resource named ILLink.Substitutions.xml, // not as a separate substitution file, for it to work with NativeAot. // Related: https://github.com/dotnet/runtime/issues/88647 - [SetupCompileResource ("FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml")] + [SetupCompileBefore ("TestFeatures.dll", new[] { "Dependencies/TestFeatures.cs" }, + resources: new object[] { new [] { "FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml" } })] [IgnoreSubstitutions (false)] public class FeatureCheckDataFlow { @@ -525,14 +526,14 @@ static void CallTestUnreferencedCodeUnguarded () RequiresUnreferencedCode (); } - static void CallTestRequiresDynamicCodeGuarded () + static void CallTestDynamicCodeGuarded () { if (RuntimeFeature.IsDynamicCodeSupported) RequiresDynamicCode (); } [ExpectedWarning ("IL3050", nameof (RequiresDynamicCode), ProducedBy = Tool.Analyzer | Tool.NativeAot)] - static void CallTestRequiresDynamicCodeUnguarded () + static void CallTestDynamicCodeUnguarded () { RequiresDynamicCode (); } @@ -554,8 +555,8 @@ public static void Test () { CallTestUnreferencedCodeGuarded (); CallTestUnreferencedCodeUnguarded (); - CallTestRequiresDynamicCodeGuarded (); - CallTestRequiresDynamicCodeUnguarded (); + CallTestDynamicCodeGuarded (); + CallTestDynamicCodeUnguarded (); CallTestAssemblyFilesGuarded (); CallTestAssemblyFilesUnguarded (); } @@ -1220,12 +1221,3 @@ class ClassWithRequires class RequiresAllGeneric<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] T> {} } } - -namespace ILLink.RoslynAnalyzer -{ - class TestFeatures - { - public static bool IsUnreferencedCodeSupported => true; - public static bool IsAssemblyFilesSupported => true; - } -} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml index c096cf07d6e7c1..db0bf370336765 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml @@ -1,5 +1,5 @@ - +