From a145b3ed019bbb547cf084de2d07e0e879a8fec6 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 26 Sep 2023 21:05:18 +0000 Subject: [PATCH 01/13] Report Requires diagnostics from dataflow analyzer This moves the logic for generating diagnostics about RequiresUnreferencedCode, RequiresAssemblyFiles, and RequiresDynamicCode attributes from the separate Requires* analyzers into the DynamicallyAccessedMembersAnalyzer. This includes logic for warning on access to attributed members, not the warnings about mismatching attributes on base/override methods. The override validation logic is still handled by the respective analyzers. The DynamicallyAccessedMembersAnalyzer is now turned on by any of the settings EnableTrimAnalyzer, EnableAotAnalyzer, or EnableSingleFileAnalyzer. A new type RequiresAnalyzerContext is used to cache per-compilation state of the analyzers, to avoid recomputing it for each trim analysis pattern that might potentially produce a warning. The context only stores state for enabled analyzers, based on the MSBuild property values. --- .../TestCasesRunner/ResultChecker.cs | 2 +- .../AnalyzerOptionsExtensions.cs | 15 +- .../src/ILLink.RoslynAnalyzer/COMAnalyzer.cs | 2 +- .../DataFlow/CapturedReferenceValue.cs | 2 +- .../DataFlow/LocalDataFlowAnalysis.cs | 14 +- .../DataFlow/LocalDataFlowVisitor.cs | 49 +++++- .../DataFlow/MethodBodyValue.cs | 13 +- .../DynamicallyAccessedMembersAnalyzer.cs | 21 ++- .../RequiresAnalyzerBase.cs | 150 +++++++++--------- .../RequiresAnalyzerContext.cs | 41 +++++ .../RequiresAssemblyFilesAnalyzer.cs | 22 ++- .../RequiresDynamicCodeAnalyzer.cs | 4 +- .../RequiresUnreferencedCodeAnalyzer.cs | 13 +- .../TrimAnalysis/DiagnosticContext.cs | 10 +- .../TrimAnalysisAssignmentPattern.cs | 34 ++-- .../TrimAnalysisMethodCallPattern.cs | 34 ++-- .../TrimAnalysis/TrimAnalysisPatternStore.cs | 7 +- .../TrimAnalysisReflectionAccessPattern.cs | 17 +- .../TrimAnalysis/TrimAnalysisVisitor.cs | 6 +- .../TrimAnalysis/TrimDataFlowAnalysis.cs | 7 + .../DataFlowTests.cs | 6 + .../RequiresAssemblyFilesAnalyzerTests.cs | 8 +- .../RequiresDynamicCodeAnalyzerTests.cs | 2 +- .../RequiresUnreferencedCodeAnalyzerTests.cs | 2 +- ...nconditionalSuppressMessageCodeFixTests.cs | 40 ++--- .../AnnotatedMembersAccessedViaReflection.cs | 9 ++ .../DataFlow/ConstructorDataFlow.cs | 80 ++++++++++ .../DataFlow/GenericParameterDataFlow.cs | 2 +- .../RequiresCapability/BasicRequires.cs | 2 + ...flectionAccessFromCompilerGeneratedCode.cs | 17 ++ .../RequiresInCompilerGeneratedCode.cs | 17 +- .../RequiresCapability/RequiresOnClass.cs | 12 +- .../TestCasesRunner/ResultChecker.cs | 2 +- 33 files changed, 447 insertions(+), 215 deletions(-) create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructorDataFlow.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs index ab4a0b94d76f6..76a0ced93c324 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs @@ -311,7 +311,7 @@ private void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogg break; } if (methodDesc.IsConstructor && - new AssemblyQualifiedToken (methodDesc.OwningType).Equals(new AssemblyQualifiedToken (expectedMember))) { + (expectedMember is FieldDefinition || expectedMember is PropertyDefinition || new AssemblyQualifiedToken (methodDesc.OwningType).Equals(new AssemblyQualifiedToken (expectedMember)))) { expectedWarningFound = true; loggedMessages.Remove (loggedMessage); break; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/AnalyzerOptionsExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/AnalyzerOptionsExtensions.cs index 4f18727426862..59f324332c9b3 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/AnalyzerOptionsExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/AnalyzerOptionsExtensions.cs @@ -11,16 +11,8 @@ internal static class AnalyzerOptionsExtensions { public static string? GetMSBuildPropertyValue ( this AnalyzerOptions options, - string optionName, - Compilation compilation) + string optionName) { - // MSBuild property values should be set at compilation level, and cannot have different values per-tree. - // So, we default to first syntax tree. - var tree = compilation.SyntaxTrees.FirstOrDefault (); - if (tree is null) { - return null; - } - return options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue ( $"build_property.{optionName}", out var value) ? value @@ -29,10 +21,9 @@ internal static class AnalyzerOptionsExtensions public static bool IsMSBuildPropertyValueTrue ( this AnalyzerOptions options, - string propertyName, - Compilation compilation) + string propertyName) { - var propertyValue = GetMSBuildPropertyValue (options, propertyName, compilation); + var propertyValue = GetMSBuildPropertyValue (options, propertyName); if (!string.Equals (propertyValue?.Trim (), "true", System.StringComparison.OrdinalIgnoreCase)) return false; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/COMAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/COMAnalyzer.cs index 2ffba6701ac3f..31d4955e329b9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/COMAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/COMAnalyzer.cs @@ -31,7 +31,7 @@ public override void Initialize (AnalysisContext context) context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics); context.RegisterCompilationStartAction (context => { var compilation = context.Compilation; - if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, compilation)) + if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer)) return; context.RegisterOperationAction (operationContext => { diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs index 37e22e2228175..cad45b03b462b 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs @@ -16,6 +16,7 @@ public CapturedReferenceValue (IOperation operation) { switch (operation.Kind) { case OperationKind.PropertyReference: + case OperationKind.EventReference: case OperationKind.LocalReference: case OperationKind.FieldReference: case OperationKind.ParameterReference: @@ -25,7 +26,6 @@ public CapturedReferenceValue (IOperation operation) case OperationKind.None: case OperationKind.InstanceReference: case OperationKind.Invocation: - case OperationKind.EventReference: case OperationKind.Invalid: // These will just be ignored when referenced later. break; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs index b6735f81a9e9a..cea5422834306 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs @@ -57,21 +57,15 @@ public void InterproceduralAnalyze () AnalyzeAttribute (Context.OwningSymbol, attribute); return; } - - if (Context.OwningSymbol is not IMethodSymbol owningMethod) - return; - - Debug.Assert (owningMethod.MethodKind is not (MethodKind.LambdaMethod or MethodKind.LocalFunction)); - var startMethod = new MethodBodyValue (owningMethod, Context.GetControlFlowGraph (OperationBlock)); + Debug.Assert (Context.OwningSymbol is not IMethodSymbol methodSymbol || + methodSymbol.MethodKind is not (MethodKind.LambdaMethod or MethodKind.LocalFunction)); + var startMethod = new MethodBodyValue (Context.OwningSymbol, Context.GetControlFlowGraph (OperationBlock)); interproceduralState.TrackMethod (startMethod); while (!interproceduralState.Equals (oldInterproceduralState)) { oldInterproceduralState = interproceduralState.Clone (); foreach (var method in oldInterproceduralState.Methods) { - if (method.Method.IsInRequiresUnreferencedCodeAttributeScope (out _)) - continue; - AnalyzeMethod (method, ref interproceduralState); } } @@ -89,7 +83,7 @@ void AnalyzeMethod (MethodBodyValue method, ref InterproceduralState state) + { + var eventReference = (IEventReferenceOperation) operation.EventReference; + TValue instanceValue = Visit (eventReference.Instance, state); + TValue value = Visit (operation.HandlerValue, state); + if (operation.Adds) { + IMethodSymbol? addMethod = eventReference.Event.AddMethod; + Debug.Assert (addMethod != null); + if (addMethod != null) + HandleMethodCall (addMethod, instanceValue, ImmutableArray.Create (value), operation); + return value; + } else { + IMethodSymbol? removeMethod = eventReference.Event.RemoveMethod; + Debug.Assert (removeMethod != null); + if (removeMethod != null) + HandleMethodCall (removeMethod, instanceValue, ImmutableArray.Create (value), operation); + return value; + } + } + TValue GetFlowCaptureValue (IFlowCaptureReferenceOperation operation, LocalDataFlowState state) { if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) { @@ -459,6 +483,8 @@ public override TValue VisitDelegateCreation (IDelegateCreationOperation operati public override TValue VisitPropertyReference (IPropertyReferenceOperation operation, LocalDataFlowState state) { + // Writing to a property should not go through this path. + Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)); if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) return TopValue; @@ -475,6 +501,19 @@ public override TValue VisitPropertyReference (IPropertyReferenceOperation opera return HandleMethodCall (getMethod!, instanceValue, arguments.ToImmutableArray (), operation); } + public override TValue VisitEventReference (IEventReferenceOperation operation, LocalDataFlowState state) + { + // Writing to an event should not go through this path. + Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)); + if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) + return TopValue; + + Visit (operation.Instance, state); + // Accessing event for reading retrieves the event delegate from the event's backing field, + // so there is no method call to handle. + return TopValue; + } + public override TValue VisitImplicitIndexerReference (IImplicitIndexerReferenceOperation operation, LocalDataFlowState state) { if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) @@ -540,7 +579,7 @@ public override TValue VisitObjectCreation (IObjectCreationOperation operation, public override TValue VisitFlowAnonymousFunction (IFlowAnonymousFunctionOperation operation, LocalDataFlowState state) { - Debug.Assert (operation.Symbol.ContainingSymbol is IMethodSymbol); + Debug.Assert (operation.Symbol.ContainingSymbol is IMethodSymbol or IFieldSymbol); var lambda = operation.Symbol; Debug.Assert (lambda.MethodKind == MethodKind.LambdaMethod); var lambdaCFG = ControlFlowGraph.GetAnonymousFunctionControlFlowGraphInScope (operation); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/MethodBodyValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/MethodBodyValue.cs index 99fe326248251..9e6b50f59824c 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/MethodBodyValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/MethodBodyValue.cs @@ -12,19 +12,22 @@ namespace ILLink.RoslynAnalyzer.DataFlow // along with its control flow graph. It implements IEquatable for the method. public readonly struct MethodBodyValue : IEquatable { - public IMethodSymbol Method { get; } + // Usually an IMethodSymbol, but may also be an IFieldSymbol or IPropertySymbol + // for field initializers. + public ISymbol OwningSymbol { get; } public ControlFlowGraph ControlFlowGraph { get; } - public MethodBodyValue (IMethodSymbol method, ControlFlowGraph cfg) + public MethodBodyValue (ISymbol owningSymbol, ControlFlowGraph cfg) { - Method = method; + Debug.Assert (owningSymbol is (IMethodSymbol or IFieldSymbol or IPropertySymbol)); + OwningSymbol = owningSymbol; ControlFlowGraph = cfg; } public bool Equals (MethodBodyValue other) { - if (!ReferenceEquals (Method, other.Method)) + if (!ReferenceEquals (OwningSymbol, other.OwningSymbol)) return false; Debug.Assert (ControlFlowGraph == other.ControlFlowGraph); @@ -34,6 +37,6 @@ public bool Equals (MethodBodyValue other) public override bool Equals (object obj) => obj is MethodBodyValue inst && Equals (inst); - public override int GetHashCode () => SymbolEqualityComparer.Default.GetHashCode (Method); + public override int GetHashCode () => SymbolEqualityComparer.Default.GetHashCode (OwningSymbol); } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index 39c8f8e71dad3..86b9e41ff1a64 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -25,6 +25,12 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute); public const string attributeArgument = "attributeArgument"; public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute; + public static Lazy> RequiresAnalyzers { get; } = new Lazy> (GetRequiresAnalyzers); + static ImmutableArray GetRequiresAnalyzers () => + ImmutableArray.Create ( + new RequiresAssemblyFilesAnalyzer (), + new RequiresUnreferencedCodeAnalyzer (), + new RequiresDynamicCodeAnalyzer ()); public static ImmutableArray GetSupportedDiagnostics () { @@ -49,6 +55,12 @@ public static ImmutableArray GetSupportedDiagnostics () diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedTypeNameInTypeGetType)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedParameterInMethodCreateInstance)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ParametersOfAssemblyCreateInstanceCannotBeAnalyzed)); + + foreach (var requiresAnalyzer in RequiresAnalyzers.Value) { + foreach (var diagnosticDescriptor in requiresAnalyzer.SupportedDiagnostics) + diagDescriptorsArrayBuilder.Add (diagnosticDescriptor); + } + return diagDescriptorsArrayBuilder.ToImmutable (); void AddRange (DiagnosticId first, DiagnosticId last) @@ -72,17 +84,16 @@ public override void Initialize (AnalysisContext context) context.EnableConcurrentExecution (); context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics); context.RegisterCompilationStartAction (context => { - if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, context.Compilation)) + if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer) && + !context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer) && + !context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer)) return; context.RegisterOperationBlockAction (context => { - if (context.OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) - return; - foreach (var operationBlock in context.OperationBlocks) { TrimDataFlowAnalysis trimDataFlowAnalysis = new (context, operationBlock); trimDataFlowAnalysis.InterproceduralAnalyze (); - foreach (var diagnostic in trimDataFlowAnalysis.TrimAnalysisPatterns.CollectDiagnostics ()) + foreach (var diagnostic in trimDataFlowAnalysis.CollectDiagnostics ()) context.ReportDiagnostic (diagnostic); } }); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index 8245f4c6a8ddd..fac9a29466354 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using ILLink.Shared; using Microsoft.CodeAnalysis; @@ -38,7 +39,7 @@ public override void Initialize (AnalysisContext context) context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics); context.RegisterCompilationStartAction (context => { var compilation = context.Compilation; - if (!IsAnalyzerEnabled (context.Options, compilation)) + if (!IsAnalyzerEnabled (context.Options)) return; var incompatibleMembers = GetSpecialIncompatibleMembers (compilation); @@ -54,63 +55,11 @@ public override void Initialize (AnalysisContext context) CheckMatchingAttributesInInterfaces (symbolAnalysisContext, typeSymbol); }, SymbolKind.NamedType); - context.RegisterOperationAction (operationContext => { - var methodInvocation = (IInvocationOperation) operationContext.Operation; - CheckCalledMember (operationContext, methodInvocation.TargetMethod, incompatibleMembers); - }, OperationKind.Invocation); - - context.RegisterOperationAction (operationContext => { - var objectCreation = (IObjectCreationOperation) operationContext.Operation; - var ctor = objectCreation.Constructor; - if (ctor is not null) { - CheckCalledMember (operationContext, ctor, incompatibleMembers); - } - }, OperationKind.ObjectCreation); - context.RegisterOperationAction (operationContext => { var fieldReference = (IFieldReferenceOperation) operationContext.Operation; CheckCalledMember (operationContext, fieldReference.Field, incompatibleMembers); }, OperationKind.FieldReference); - context.RegisterOperationAction (operationContext => { - var propAccess = (IPropertyReferenceOperation) operationContext.Operation; - var prop = propAccess.Property; - var usageInfo = propAccess.GetValueUsageInfo (operationContext.ContainingSymbol); - if (usageInfo.HasFlag (ValueUsageInfo.Read) && prop.GetMethod != null) - CheckCalledMember (operationContext, prop.GetMethod, incompatibleMembers); - - if (usageInfo.HasFlag (ValueUsageInfo.Write) && prop.SetMethod != null) - CheckCalledMember (operationContext, prop.SetMethod, incompatibleMembers); - }, OperationKind.PropertyReference); - - context.RegisterOperationAction (operationContext => { - var eventRef = (IEventReferenceOperation) operationContext.Operation; - var eventSymbol = (IEventSymbol) eventRef.Member; - var assignmentOperation = eventRef.Parent as IEventAssignmentOperation; - - if (assignmentOperation != null && assignmentOperation.Adds && eventSymbol.AddMethod is IMethodSymbol eventAddMethod) - CheckCalledMember (operationContext, eventAddMethod, incompatibleMembers); - - if (assignmentOperation != null && !assignmentOperation.Adds && eventSymbol.RemoveMethod is IMethodSymbol eventRemoveMethod) - CheckCalledMember (operationContext, eventRemoveMethod, incompatibleMembers); - - if (eventSymbol.RaiseMethod is IMethodSymbol eventRaiseMethod) - CheckCalledMember (operationContext, eventRaiseMethod, incompatibleMembers); - }, OperationKind.EventReference); - - context.RegisterOperationAction (operationContext => { - var delegateCreation = (IDelegateCreationOperation) operationContext.Operation; - IMethodSymbol methodSymbol; - if (delegateCreation.Target is IMethodReferenceOperation methodRef) - methodSymbol = methodRef.Method; - else if (delegateCreation.Target is IAnonymousFunctionOperation lambda) - methodSymbol = lambda.Symbol; - else - return; - - CheckCalledMember (operationContext, methodSymbol, incompatibleMembers); - }, OperationKind.DelegateCreation); - context.RegisterSyntaxNodeAction (syntaxNodeAnalysisContext => { var model = syntaxNodeAnalysisContext.SemanticModel; if (syntaxNodeAnalysisContext.ContainingSymbol is not ISymbol containingSymbol || containingSymbol.IsInRequiresScope (RequiresAttributeName, out _)) @@ -185,24 +134,13 @@ void CheckCalledMember ( ISymbol containingSymbol = FindContainingSymbol (operationContext, AnalyzerDiagnosticTargets); - // Do not emit any diagnostic if caller is annotated with the attribute too. - if (containingSymbol.IsInRequiresScope (RequiresAttributeName, out _)) - return; - - if (ReportSpecialIncompatibleMembersDiagnostic (operationContext, incompatibleMembers, member)) - return; - - // Warn on the most derived base method taking into account covariant returns - while (member is IMethodSymbol method && method.OverriddenMethod != null && SymbolEqualityComparer.Default.Equals (method.ReturnType, method.OverriddenMethod.ReturnType)) - member = method.OverriddenMethod; - - if (!member.DoesMemberRequire (RequiresAttributeName, out var requiresAttribute)) - return; - - if (!VerifyAttributeArguments (requiresAttribute)) - return; - - ReportRequiresDiagnostic (operationContext, member, requiresAttribute); + if (CheckAndCreateRequiresDiagnostic ( + operationContext.Operation, + containingSymbol, + member, + ImmutableArray.Empty, + out Diagnostic? diagnostic)) + operationContext.ReportDiagnostic (diagnostic); } void CheckMatchingAttributesInOverrides ( @@ -226,6 +164,35 @@ void CheckMatchingAttributesInInterfaces ( }); } + public bool CheckAndCreateRequiresDiagnostic ( + IOperation operation, + ISymbol containingSymbol, + ISymbol member, + ImmutableArray incompatibleMembers, + [NotNullWhen (true)] out Diagnostic? diagnostic) + { + diagnostic = null; + // Do not emit any diagnostic if caller is annotated with the attribute too. + if (containingSymbol.IsInRequiresScope (RequiresAttributeName, out _)) + return false; + + if (CreateSpecialIncompatibleMembersDiagnostic (operation, incompatibleMembers, member, out diagnostic)) + return diagnostic != null; + + // Warn on the most derived base method taking into account covariant returns + while (member is IMethodSymbol method && method.OverriddenMethod != null && SymbolEqualityComparer.Default.Equals (method.ReturnType, method.OverriddenMethod.ReturnType)) + member = method.OverriddenMethod; + + if (!member.DoesMemberRequire (RequiresAttributeName, out var requiresAttribute)) + return false; + + if (!VerifyAttributeArguments (requiresAttribute)) + return false; + + diagnostic = CreateRequiresDiagnostic (operation, member, requiresAttribute); + return true; + } + [Flags] protected enum DiagnosticTargets { @@ -276,16 +243,16 @@ protected static ISymbol FindContainingSymbol (OperationAnalysisContext operatio /// Analyzer operation context to be able to report the diagnostic. /// Information about the member that generated the diagnostic. /// Requires attribute data to print attribute arguments. - private void ReportRequiresDiagnostic (OperationAnalysisContext operationContext, ISymbol member, AttributeData requiresAttribute) + private Diagnostic CreateRequiresDiagnostic (IOperation operation, ISymbol member, AttributeData requiresAttribute) { var message = GetMessageFromAttribute (requiresAttribute); var url = GetUrlFromAttribute (requiresAttribute); - operationContext.ReportDiagnostic (Diagnostic.Create ( + return Diagnostic.Create ( RequiresDiagnosticRule, - operationContext.Operation.Syntax.GetLocation (), + operation.Syntax.GetLocation (), member.GetDisplayName (), message, - url)); + url); } private void ReportRequiresOnStaticCtorDiagnostic (SymbolAnalysisContext symbolAnalysisContext, IMethodSymbol ctor) @@ -336,21 +303,46 @@ public static string GetUrlFromAttribute (AttributeData? requiresAttribute) /// List of incompatible members. /// Member to compare. /// True if the function generated a diagnostic; otherwise, returns false - protected virtual bool ReportSpecialIncompatibleMembersDiagnostic (OperationAnalysisContext operationContext, ImmutableArray specialIncompatibleMembers, ISymbol member) => false; + protected virtual bool CreateSpecialIncompatibleMembersDiagnostic ( + IOperation operation, + ImmutableArray specialIncompatibleMembers, + ISymbol member, + out Diagnostic? incompatibleMembersDiagnostic) + { + incompatibleMembersDiagnostic = null; + return false; + } /// /// Creates a list of special incompatible members that can be used later on by the analyzer to generate diagnostics /// /// Compilation to search for members /// A list of special incomptaible members - protected virtual ImmutableArray GetSpecialIncompatibleMembers (Compilation compilation) => default; + internal virtual ImmutableArray GetSpecialIncompatibleMembers (Compilation compilation) => default; /// /// Verifies that the MSBuild requirements to run the analyzer are fulfilled /// /// Analyzer options - /// Analyzer compilation information /// True if the requirements to run the analyzer are met; otherwise, returns false - protected abstract bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation); + internal abstract bool IsAnalyzerEnabled (AnalyzerOptions options); + + internal bool CheckAndCreateRequiresDiagnostic ( + IOperation operation, + IMethodSymbol methodSymbol, + ISymbol owningSymbol, + RequiresAnalyzerContext context, + [NotNullWhen (true)] out Diagnostic? diagnostic) + { + ISymbol containingSymbol = operation.FindContainingSymbol (owningSymbol); + + var incompatibleMembers = context.GetSpecialIncompatibleMembers (this); + return CheckAndCreateRequiresDiagnostic ( + operation, + containingSymbol, + methodSymbol, + incompatibleMembers, + out diagnostic); + } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs new file mode 100644 index 0000000000000..50db1def90ab4 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs @@ -0,0 +1,41 @@ +// 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 System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace ILLink.RoslynAnalyzer +{ + public struct RequiresAnalyzerContext + { + private Dictionary> _enabledAnalyzers; + + public IEnumerable EnabledRequiresAnalyzers => _enabledAnalyzers.Keys; + + public ImmutableArray GetSpecialIncompatibleMembers (RequiresAnalyzerBase analyzer) + { + if (!_enabledAnalyzers.TryGetValue (analyzer, out var members)) + throw new System.ArgumentException ($"Analyzer {analyzer.GetType ().Name} is not in the cache"); + return members; + } + + RequiresAnalyzerContext (Dictionary> enabledAnalyzers) + { + _enabledAnalyzers = enabledAnalyzers; + } + + public static RequiresAnalyzerContext Create (OperationBlockAnalysisContext context, ImmutableArray requiresAnalyzers) + { + var enabledAnalyzers = new Dictionary> (); + foreach (var analyzer in requiresAnalyzers) { + if (analyzer.IsAnalyzerEnabled (context.Options)) { + var incompatibleMembers = analyzer.GetSpecialIncompatibleMembers (context.Compilation); + enabledAnalyzers.Add (analyzer, incompatibleMembers); + } + } + return new RequiresAnalyzerContext (enabledAnalyzers); + } + } +} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs index 507284afdad8c..aaa81970f36af 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs @@ -2,8 +2,11 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using ILLink.Shared; +using ILLink.Shared.TypeSystemProxy; +using ILLink.Shared.TrimAnalysis; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -42,20 +45,20 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresAssemblyFilesOnStaticCtor; - protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation) + internal override bool IsAnalyzerEnabled (AnalyzerOptions options) { - var isSingleFileAnalyzerEnabled = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer, compilation); + var isSingleFileAnalyzerEnabled = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer); if (!string.Equals (isSingleFileAnalyzerEnabled?.Trim (), "true", StringComparison.OrdinalIgnoreCase)) return false; - var includesAllContent = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.IncludeAllContentForSelfExtract, compilation); + var includesAllContent = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.IncludeAllContentForSelfExtract); if (string.Equals (includesAllContent?.Trim (), "true", StringComparison.OrdinalIgnoreCase)) return false; return true; } - protected override ImmutableArray GetSpecialIncompatibleMembers (Compilation compilation) + internal override ImmutableArray GetSpecialIncompatibleMembers (Compilation compilation) { var dangerousPatternsBuilder = ImmutableArray.CreateBuilder (); @@ -78,16 +81,21 @@ protected override ImmutableArray GetSpecialIncompatibleMembers (Compil return dangerousPatternsBuilder.ToImmutable (); } - protected override bool ReportSpecialIncompatibleMembersDiagnostic (OperationAnalysisContext operationContext, ImmutableArray dangerousPatterns, ISymbol member) + protected override bool CreateSpecialIncompatibleMembersDiagnostic ( + IOperation operation, + ImmutableArray dangerousPatterns, + ISymbol member, + out Diagnostic? diagnostic) { + diagnostic = null; if (member is IMethodSymbol method) { if (ImmutableArrayOperations.Contains (dangerousPatterns, member, SymbolEqualityComparer.Default)) { - operationContext.ReportDiagnostic (Diagnostic.Create (s_getFilesRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); + diagnostic = Diagnostic.Create (s_getFilesRule, operation.Syntax.GetLocation (), member.GetDisplayName ()); return true; } else if (method.AssociatedSymbol is ISymbol associatedSymbol && ImmutableArrayOperations.Contains (dangerousPatterns, associatedSymbol, SymbolEqualityComparer.Default)) { - operationContext.ReportDiagnostic (Diagnostic.Create (s_locationRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); + diagnostic = Diagnostic.Create (s_locationRule, operation.Syntax.GetLocation (), member.GetDisplayName ()); // The getters for CodeBase and EscapedCodeBase have RAF attribute on them // so our caller will produce the RAF warning (IL3002) by default. Since we handle these properties specifically // here and produce different warning (IL3000) we don't want the caller to produce IL3002. diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs index dcba6acfb1728..9ca2cfa52f7fe 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs @@ -33,8 +33,8 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresDynamicCodeOnStaticCtor; - protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation) => - options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer, compilation); + internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => + options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer); protected override bool VerifyAttributeArguments (AttributeData attribute) => attribute.ConstructorArguments.Length >= 1 && attribute.ConstructorArguments is [ { Type.SpecialType: SpecialType.System_String }, ..]; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs index c4ebfb516a30c..a6cae197839fc 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs @@ -70,11 +70,16 @@ private Action typeDerivesFromRucBase { private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresUnreferencedCodeOnStaticCtor; - protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation) => - options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, compilation); - - protected override bool ReportSpecialIncompatibleMembersDiagnostic (OperationAnalysisContext operationContext, ImmutableArray specialIncompatibleMembers, ISymbol member) + internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => + options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer); + + protected override bool CreateSpecialIncompatibleMembersDiagnostic ( + IOperation operation, + ImmutableArray specialIncompatibleMembers, + ISymbol member, + out Diagnostic? incompatibleMembersDiagnostic) { + incompatibleMembersDiagnostic = null; // Some RUC-annotated APIs are intrinsically handled by the trimmer if (member is IMethodSymbol method && Intrinsics.GetIntrinsicIdForMethod (new MethodProxy (method)) != IntrinsicId.None) { return true; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DiagnosticContext.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DiagnosticContext.cs index 760a719b3ea18..f292252c160b6 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DiagnosticContext.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DiagnosticContext.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using ILLink.RoslynAnalyzer; using Microsoft.CodeAnalysis; @@ -48,6 +49,13 @@ public partial void AddDiagnostic (DiagnosticId id, ValueWithDynamicallyAccessed if (Location == null) return; + Diagnostics.Add (CreateDiagnostic (id, actualValue, expectedAnnotationsValue, args)); + } + + private Diagnostic CreateDiagnostic (DiagnosticId id, ValueWithDynamicallyAccessedMembers actualValue, ValueWithDynamicallyAccessedMembers expectedAnnotationsValue, params string[] args) + { + Debug.Assert (Location != null); + if (actualValue is NullableValueWithDynamicallyAccessedMembers nv) actualValue = nv.UnderlyingTypeValue; @@ -79,7 +87,7 @@ public partial void AddDiagnostic (DiagnosticId id, ValueWithDynamicallyAccessed sourceLocation = new Location[] { symbolLocation }; } - Diagnostics.Add (Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (id), Location, sourceLocation, DAMArgument?.ToImmutableDictionary (), args)); + return Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (id), Location, sourceLocation, DAMArgument?.ToImmutableDictionary (), args); } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs index 9a41f47cd9a52..495e29c1c7012 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs @@ -4,9 +4,11 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.InteropServices.ComTypes; using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; using MultiValue = ILLink.Shared.DataFlow.ValueSet; @@ -17,36 +19,46 @@ public readonly record struct TrimAnalysisAssignmentPattern public MultiValue Source { init; get; } public MultiValue Target { init; get; } public IOperation Operation { init; get; } + public ISymbol OwningSymbol { init; get; } - public TrimAnalysisAssignmentPattern (MultiValue source, MultiValue target, IOperation operation) + public TrimAnalysisAssignmentPattern ( + MultiValue source, + MultiValue target, + IOperation operation, + ISymbol owningSymbol) { Source = source.DeepCopy (); Target = target.DeepCopy (); Operation = operation; + OwningSymbol = owningSymbol; } public TrimAnalysisAssignmentPattern Merge (ValueSetLattice lattice, TrimAnalysisAssignmentPattern other) { Debug.Assert (Operation == other.Operation); + Debug.Assert (SymbolEqualityComparer.Default.Equals (OwningSymbol, other.OwningSymbol)); return new TrimAnalysisAssignmentPattern ( lattice.Meet (Source, other.Source), lattice.Meet (Target, other.Target), - Operation); + Operation, + OwningSymbol); } public IEnumerable CollectDiagnostics () { var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); - foreach (var sourceValue in Source) { - foreach (var targetValue in Target) { - // The target should always be an annotated value, but the visitor design currently prevents - // declaring this in the type system. - if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers) - throw new NotImplementedException (); - - var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (diagnosticContext, default (ReflectionAccessAnalyzer)); - requireDynamicallyAccessedMembersAction.Invoke (sourceValue, targetWithDynamicallyAccessedMembers); + if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { + foreach (var sourceValue in Source) { + foreach (var targetValue in Target) { + // The target should always be an annotated value, but the visitor design currently prevents + // declaring this in the type system. + if (targetValue is not ValueWithDynamicallyAccessedMembers targetWithDynamicallyAccessedMembers) + throw new NotImplementedException (); + + var requireDynamicallyAccessedMembersAction = new RequireDynamicallyAccessedMembersAction (diagnosticContext, default (ReflectionAccessAnalyzer)); + requireDynamicallyAccessedMembersAction.Invoke (sourceValue, targetWithDynamicallyAccessedMembers); + } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs index c8e8d6d45dc8e..453e9934b7f13 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs @@ -1,14 +1,17 @@ // 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.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Runtime.InteropServices.ComTypes; +using ILLink.Shared; using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; - +using Microsoft.CodeAnalysis.Diagnostics; using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer.TrimAnalysis @@ -47,6 +50,7 @@ public TrimAnalysisMethodCallPattern Merge (ValueSetLattice lattice { Debug.Assert (Operation == other.Operation); Debug.Assert (SymbolEqualityComparer.Default.Equals (CalledMethod, other.CalledMethod)); + Debug.Assert (SymbolEqualityComparer.Default.Equals (OwningSymbol, other.OwningSymbol)); Debug.Assert (Arguments.Length == other.Arguments.Length); var argumentsBuilder = ImmutableArray.CreateBuilder (); @@ -62,18 +66,26 @@ public TrimAnalysisMethodCallPattern Merge (ValueSetLattice lattice OwningSymbol); } - public IEnumerable CollectDiagnostics () + public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) { DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); - HandleCallAction handleCallAction = new (diagnosticContext, OwningSymbol, Operation); - MethodProxy method = new (CalledMethod); - IntrinsicId intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method); - if (!handleCallAction.Invoke (method, Instance, Arguments, intrinsicId, out _)) { - // If this returns false it means the intrinsic needs special handling: - // case IntrinsicId.TypeDelegator_Ctor: - // No diagnostics to report - this is an "identity" operation for data flow, can't produce diagnostics on its own - // case IntrinsicId.Array_Empty: - // No diagnostics to report - constant value + + if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { + HandleCallAction handleCallAction = new (diagnosticContext, OwningSymbol, Operation); + MethodProxy method = new (CalledMethod); + IntrinsicId intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method); + if (!handleCallAction.Invoke (method, Instance, Arguments, intrinsicId, out _)) { + // If this returns false it means the intrinsic needs special handling: + // case IntrinsicId.TypeDelegator_Ctor: + // No diagnostics to report - this is an "identity" operation for data flow, can't produce diagnostics on its own + // case IntrinsicId.Array_Empty: + // No diagnostics to report - constant value + } + } + + foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) { + if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, CalledMethod, OwningSymbol, context, out Diagnostic? diag)) + diagnosticContext.AddDiagnostic (diag); } return diagnosticContext.Diagnostics; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index 0057ba8e8a836..cf1dc0bb87fe6 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using ILLink.Shared.DataFlow; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; namespace ILLink.RoslynAnalyzer.TrimAnalysis { @@ -60,7 +61,7 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) ReflectionAccessPatterns[pattern.Operation] = pattern.Merge (Lattice, existingPattern); } - public IEnumerable CollectDiagnostics () + public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) { foreach (var assignmentPattern in AssignmentPatterns.Values) { foreach (var diagnostic in assignmentPattern.CollectDiagnostics ()) @@ -68,12 +69,12 @@ public IEnumerable CollectDiagnostics () } foreach (var methodCallPattern in MethodCallPatterns.Values) { - foreach (var diagnostic in methodCallPattern.CollectDiagnostics ()) + foreach (var diagnostic in methodCallPattern.CollectDiagnostics (context)) yield return diagnostic; } foreach (var reflectionAccessPattern in ReflectionAccessPatterns.Values) { - foreach (var diagnostic in reflectionAccessPattern.CollectDiagnostics ()) + foreach (var diagnostic in reflectionAccessPattern.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 468d7bbf37756..2878edfa31b47 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -5,12 +5,13 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices.ComTypes; using ILLink.Shared; using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; - +using Microsoft.CodeAnalysis.Diagnostics; using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer.TrimAnalysis @@ -38,6 +39,7 @@ public TrimAnalysisReflectionAccessPattern Merge (ValueSetLattice l { Debug.Assert (Operation == other.Operation); Debug.Assert (SymbolEqualityComparer.Default.Equals (ReferencedMethod, other.ReferencedMethod)); + Debug.Assert (SymbolEqualityComparer.Default.Equals (OwningSymbol, other.OwningSymbol)); return new TrimAnalysisReflectionAccessPattern ( ReferencedMethod, @@ -46,11 +48,18 @@ public TrimAnalysisReflectionAccessPattern Merge (ValueSetLattice l OwningSymbol); } - public IEnumerable CollectDiagnostics () + public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) { DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); - foreach (var diagnostic in ReflectionAccessAnalyzer.GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, ReferencedMethod)) - yield return diagnostic; + if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { + foreach (var diagnostic in ReflectionAccessAnalyzer.GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, ReferencedMethod)) + yield return diagnostic; + } + + foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) { + if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, ReferencedMethod, OwningSymbol, context, out Diagnostic? diag)) + yield return diag; + } } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 86ea32c85481f..0c0e038ffcc5b 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -114,6 +114,7 @@ public override MultiValue VisitInstanceReference (IInstanceReferenceOperation i // The instance reference operation represents a 'this' or 'base' reference to the containing type, // so we get the annotation from the containing method. if (instanceRef.Type != null && instanceRef.Type.IsTypeInterestingForDataflow ()) { + // 'this' is not allowed in field/property initializers, so the owning symbol should be a method. Debug.Assert (OwningSymbol is IMethodSymbol); if (OwningSymbol is IMethodSymbol method) return new MethodParameterValue (method, (ParameterIndex) 0, method.GetDynamicallyAccessedMemberTypes ()); @@ -203,7 +204,7 @@ public override void HandleAssignment (MultiValue source, MultiValue target, IOp // annotated with DAMT. TrimAnalysisPatterns.Add ( // This will copy the values if necessary - new TrimAnalysisAssignmentPattern (source, target, operation), + new TrimAnalysisAssignmentPattern (source, target, operation, OwningSymbol), isReturnValue: false ); } @@ -297,6 +298,7 @@ public override MultiValue HandleMethodCall (IMethodSymbol calledMethod, MultiVa public override void HandleReturnValue (MultiValue returnValue, IOperation operation) { + // Return statements should only happen inside of method bodies. Debug.Assert (OwningSymbol is IMethodSymbol); if (OwningSymbol is not IMethodSymbol method) return; @@ -305,7 +307,7 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera var returnParameter = new MethodReturnValue (method); TrimAnalysisPatterns.Add ( - new TrimAnalysisAssignmentPattern (returnValue, returnParameter, operation), + new TrimAnalysisAssignmentPattern (returnValue, returnParameter, operation, OwningSymbol), isReturnValue: true ); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs index c7678b498066a..97414e4fb41bb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.IO; @@ -28,6 +29,12 @@ public TrimDataFlowAnalysis (OperationBlockAnalysisContext context, IOperation o TrimAnalysisPatterns = new TrimAnalysisPatternStore (Lattice.Lattice.ValueLattice); } + public IEnumerable CollectDiagnostics () + { + var requiresAnalyzerContext = RequiresAnalyzerContext.Create (Context, DynamicallyAccessedMembersAnalyzer.RequiresAnalyzers.Value); + return TrimAnalysisPatterns.CollectDiagnostics (requiresAnalyzerContext); + } + protected override TrimAnalysisVisitor GetVisitor ( ISymbol owningSymbol, ControlFlowGraph methodCFG, diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index 29622dc32365c..71690c473c1e3 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -112,6 +112,12 @@ public Task ConstructedTypesDataFlow () return RunTest (); } + [Fact] + public Task ConstructorDataFlow () + { + return RunTest (); + } + [Fact] public Task DynamicDependencyDataflow () { diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs index 0b2d44da17b55..0a83276ec30b7 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs @@ -10,7 +10,7 @@ using Microsoft.CodeAnalysis.Text; using Xunit; using VerifyCS = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.RequiresAssemblyFilesAnalyzer, + ILLink.RoslynAnalyzer.DynamicallyAccessedMembersAnalyzer, ILLink.CodeFix.RequiresAssemblyFilesCodeFixProvider>; namespace ILLink.RoslynAnalyzer.Tests @@ -82,7 +82,7 @@ void M() """; return VerifyRequiresAssemblyFilesAnalyzer (TestRequiresAssemblyFieldsOnEvent, // (11,17): warning IL3002: Using member 'C.E' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 4).WithArguments ("C.E.add", "", "")); + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 26).WithArguments ("C.E.add", "", "")); } [Fact] @@ -106,7 +106,7 @@ void M() """; return VerifyRequiresAssemblyFilesAnalyzer (TestRequiresAssemblyFilesOnProperty, // (11,3): warning IL3002: Using member 'C.P' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 4).WithArguments ("C.P.set", "", ""), + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 12).WithArguments ("C.P.set", "", ""), // (12,35): warning IL3002: Using member 'C.P' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (12, 35, 12, 36).WithArguments ("C.P.get", "", "")); } @@ -143,7 +143,7 @@ void M () """; return VerifyRequiresAssemblyFilesAnalyzer (TestRequiresAssemblyFilesOnMethodInsideProperty, // (23,3): warning IL3002: Using member 'C.P' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (23, 3, 23, 4).WithArguments ("C.P.set", "", "")); + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (23, 3, 23, 12).WithArguments ("C.P.set", "", "")); } [Fact] diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs index ef8203401a8b4..b43dcc134934a 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs @@ -8,7 +8,7 @@ using Microsoft.CodeAnalysis.Text; using Xunit; using VerifyCS = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.RequiresDynamicCodeAnalyzer, + ILLink.RoslynAnalyzer.DynamicallyAccessedMembersAnalyzer, ILLink.CodeFix.RequiresDynamicCodeCodeFixProvider>; namespace ILLink.RoslynAnalyzer.Tests diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs index 77497b9b398d5..7f4bc8ef624ce 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs @@ -10,7 +10,7 @@ using Microsoft.CodeAnalysis.Text; using Xunit; using VerifyCS = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.RequiresUnreferencedCodeAnalyzer, + ILLink.RoslynAnalyzer.DynamicallyAccessedMembersAnalyzer, ILLink.CodeFix.RequiresUnreferencedCodeCodeFixProvider>; namespace ILLink.RoslynAnalyzer.Tests diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs index ad9c1bebb6a18..fc899fbd1d356 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs @@ -7,14 +7,8 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Text; using Xunit; -using VerifyCSUSMwithRAF = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.RequiresAssemblyFilesAnalyzer, - ILLink.CodeFix.UnconditionalSuppressMessageCodeFixProvider>; -using VerifyCSUSMwithRDC = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.RequiresDynamicCodeAnalyzer, - ILLink.CodeFix.UnconditionalSuppressMessageCodeFixProvider>; -using VerifyCSUSMwithRUC = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.RequiresUnreferencedCodeAnalyzer, +using VerifyCSUSM = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< + ILLink.RoslynAnalyzer.DynamicallyAccessedMembersAnalyzer, ILLink.CodeFix.UnconditionalSuppressMessageCodeFixProvider>; namespace ILLink.RoslynAnalyzer.Tests @@ -46,7 +40,7 @@ static Task VerifyUnconditionalSuppressMessageCodeFixWithRUC ( DiagnosticResult[] baselineExpected, DiagnosticResult[] fixedExpected) { - var test = new VerifyCSUSMwithRUC.Test { + var test = new VerifyCSUSM.Test { TestCode = source, FixedCode = fixedSource, ReferenceAssemblies = TestCaseUtils.Net6PreviewAssemblies @@ -66,7 +60,7 @@ static Task VerifyUnconditionalSuppressMessageCodeFixWithRAF ( DiagnosticResult[] baselineExpected, DiagnosticResult[] fixedExpected) { - var test = new VerifyCSUSMwithRAF.Test { + var test = new VerifyCSUSM.Test { TestCode = source, FixedCode = fixedSource, ReferenceAssemblies = TestCaseUtils.Net6PreviewAssemblies @@ -86,7 +80,7 @@ static Task VerifyUnconditionalSuppressMessageCodeFixWithRDC ( DiagnosticResult[] baselineExpected, DiagnosticResult[] fixedExpected) { - var test = new VerifyCSUSMwithRDC.Test { + var test = new VerifyCSUSM.Test { TestCode = source + dynamicCodeAttribute, FixedCode = fixedSource + dynamicCodeAttribute, ReferenceAssemblies = TestCaseUtils.Net6PreviewAssemblies @@ -125,7 +119,7 @@ public class C fixtest, baselineExpected: new[] { // /0/Test0.cs(7,17): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", ""), + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", ""), }, fixedExpected: Array.Empty ()); } @@ -155,7 +149,7 @@ public class C fixtest, baselineExpected: new[] { // /0/Test0.cs(7,17): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRAF.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", "") }, fixedExpected: Array.Empty ()); } @@ -185,7 +179,7 @@ public class C fixtest, baselineExpected: new[] { // /0/Test0.cs(7,17): warning IL3050: Members annotated with 'RequiresDynamicCodeAttribute' require dynamic access otherwise can break functionality when trimming application code. message. - VerifyCSUSMwithRDC.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", "") }, fixedExpected: Array.Empty ()); } @@ -226,9 +220,9 @@ public void M2() { fixtest, baselineExpected: new[] { // /0/Test0.cs(7,27): warning IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCSUSMwithRAF.Diagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 27, 7, 44).WithArguments ("System.Reflection.Assembly.Location.get", "", ""), + VerifyCSUSM.Diagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 27, 7, 44).WithArguments ("System.Reflection.Assembly.Location.get", "", ""), // /0/Test0.cs(9,13): warning IL3001: 'System.Reflection.Assembly.GetFiles()' will throw for assemblies embedded in a single-file app - VerifyCSUSMwithRAF.Diagnostic(DiagnosticId.AvoidAssemblyGetFilesInSingleFile).WithSpan (9, 13, 9, 32).WithArguments("System.Reflection.Assembly.GetFiles()", "", ""), + VerifyCSUSM.Diagnostic(DiagnosticId.AvoidAssemblyGetFilesInSingleFile).WithSpan (9, 13, 9, 32).WithArguments("System.Reflection.Assembly.GetFiles()", "", ""), }, fixedExpected: Array.Empty ()); } @@ -264,7 +258,7 @@ public class C fix, baselineExpected: new[] { // /0/Test0.cs(10,15): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(10, 15, 10, 19).WithArguments("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(10, 15, 10, 19).WithArguments("C.M1()", " message.", "") }, fixedExpected: Array.Empty ()); } @@ -311,9 +305,9 @@ private int M2 { """; var diag = new[] { // /0/Test0.cs(12,16): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic(DiagnosticId.RequiresUnreferencedCode).WithSpan(12, 16, 12, 20).WithArguments("C.M1()", " message.", ""), + VerifyCSUSM.Diagnostic(DiagnosticId.RequiresUnreferencedCode).WithSpan(12, 16, 12, 20).WithArguments("C.M1()", " message.", ""), // /0/Test0.cs(13,17): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic(DiagnosticId.RequiresUnreferencedCode).WithSpan(13, 17, 13, 21).WithArguments("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic(DiagnosticId.RequiresUnreferencedCode).WithSpan(13, 17, 13, 21).WithArguments("C.M1()", " message.", "") }; return VerifyUnconditionalSuppressMessageCodeFixWithRUC (src, fix, diag, Array.Empty ()); } @@ -355,7 +349,7 @@ public static C InitC() { fixtest, baselineExpected: new[] { // /0/Test0.cs(6,50): warning IL3002: Using member 'C.InitC()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCSUSMwithRAF.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (6, 50, 6, 55).WithArguments ("C.InitC()", "", ""), + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (6, 50, 6, 55).WithArguments ("C.InitC()", "", ""), }, fixedExpected: Array.Empty ()); } @@ -399,7 +393,7 @@ Action M2() fix, baselineExpected: new[] { // /0/Test0.cs(12,28): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(12, 28, 12, 32).WithArguments("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(12, 28, 12, 32).WithArguments("C.M1()", " message.", "") }, fixedExpected: Array.Empty ()); } @@ -435,7 +429,7 @@ public class C fix, baselineExpected: new[] { // /0/Test0.cs(10,15): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(10, 20, 10, 24).WithArguments("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(10, 20, 10, 24).WithArguments("C.M1()", " message.", "") }, fixedExpected: Array.Empty ()); } @@ -485,7 +479,7 @@ public event EventHandler E1 fix, baselineExpected: new[] { // /0/Test0.cs(14,21): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. - VerifyCSUSMwithRUC.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(14, 21, 14, 25).WithArguments("C.M1()", " message.", "") + VerifyCSUSM.Diagnostic (DiagnosticId.RequiresUnreferencedCode).WithSpan(14, 21, 14, 25).WithArguments("C.M1()", " message.", "") }, fixedExpected: Array.Empty ()); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs index 170c0c6aeab49..fd637557a36a5 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs @@ -270,6 +270,12 @@ static void Ldftn () var _ = new Action (AnnotatedMethodParameters.MethodWithSingleAnnotatedParameter); } + [RequiresUnreferencedCode ("test")] + static void LdftnSuppressedByRequiresUnreferencedCode () + { + var _ = new Action (AnnotatedMethodParameters.MethodWithSingleAnnotatedParameter); + } + [ExpectedWarning ("IL2111")] static void LdftnOnLambda () @@ -321,6 +327,7 @@ static void Ldvirtftn () [ExpectedWarning ("IL2026", "ReflectionSuppressedByRUC", "test")] [ExpectedWarning ("IL2026", "DynamicDependencySuppressedByRUC", "test")] [ExpectedWarning ("IL2026", "DynamicallyAccessedMembersSuppressedByRUC", "test")] + [ExpectedWarning ("IL2026", "LdftnSuppressedByRequiresUnreferencedCode", "test")] [ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter))] [ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))] [ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))] @@ -334,6 +341,7 @@ static void DynamicallyAccessedMembersAll1 () [ExpectedWarning ("IL2026", "ReflectionSuppressedByRUC", "test")] [ExpectedWarning ("IL2026", "DynamicDependencySuppressedByRUC", "test")] [ExpectedWarning ("IL2026", "DynamicallyAccessedMembersSuppressedByRUC", "test")] + [ExpectedWarning ("IL2026", "LdftnSuppressedByRequiresUnreferencedCode", "test")] [ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter))] [ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))] [ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))] @@ -363,6 +371,7 @@ public static void Test () DynamicallyAccessedMembers (); DynamicallyAccessedMembersSuppressedByRUC (); Ldftn (); + LdftnSuppressedByRequiresUnreferencedCode (); LdftnOnLambda (); LdftnOnLocalMethod (); LdftnOnLambdaTriggersLamdaAnalysis (); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructorDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructorDataFlow.cs new file mode 100644 index 0000000000000..7b68e1d676925 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructorDataFlow.cs @@ -0,0 +1,80 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +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] + public class ConstructorDataFlow + { + public static void Main () + { + DataFlowInConstructor.Test (); + DataFlowInStaticConstructor.Test (); + } + + class DataFlowInConstructor + { + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll))] + public DataFlowInConstructor () + { + RequireAll (GetUnknown ()); + } + + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll), CompilerGeneratedCode = true)] + int field = RequireAll (GetUnknown ()); + + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll), CompilerGeneratedCode = true)] + int Property { get; } = RequireAll (GetUnknown ()); + + // The analyzer dataflow visitor asserts that we only see a return value + // inside of an IMethodSymbol. This testcase checks that we don't hit asserts + // in case the return statement is in a lambda owned by a field. + // When the lambda is analyzed, the OwningSymbol is still an IMethodSymbol + // (the symbol representing the lambda, not the field). + int fieldWithReturnStatementInInitializer = Execute( + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll))] + () => { + return RequireAll (GetUnknown ()); + }); + + static int Execute(Func f) => f(); + + public static void Test () + { + new DataFlowInConstructor (); + } + } + + class DataFlowInStaticConstructor + { + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll))] + static DataFlowInStaticConstructor () + { + RequireAll (GetUnknown ()); + } + + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll), CompilerGeneratedCode = true)] + static int field = RequireAll (GetUnknown ()); + + [ExpectedWarning ("IL2072", nameof (GetUnknown), nameof (RequireAll), CompilerGeneratedCode = true)] + static int Property { get; } = RequireAll (GetUnknown ()); + + public static void Test () + { + } + } + + static Type GetUnknown () => null; + + static int RequireAll ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type type) => 0; + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs index 0b9e865016899..a030aa31f6564 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs @@ -217,7 +217,7 @@ static void TestDerivedTypeWithOpenGenericOnBaseWithRUCOnBase () class DerivedTypeWithOpenGenericOnBaseWithRUCOnBase : BaseTypeWithOpenGenericDAMTAndRUC, IGenericInterfaceTypeWithRequirements { [ExpectedWarning ("IL2091", nameof (DerivedTypeWithOpenGenericOnBaseWithRUCOnBase), ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL2026", nameof (BaseTypeWithOpenGenericDAMTAndRUC), ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", nameof (BaseTypeWithOpenGenericDAMTAndRUC))] public DerivedTypeWithOpenGenericOnBaseWithRUCOnBase () { } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs index 40c4f33ed27b8..4668cdf23aedf 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs @@ -227,6 +227,8 @@ static void TestEvent () { EventRequires += (object sender, EventArgs e) => throw new NotImplementedException (); EventRequires -= (object sender, EventArgs e) => throw new NotImplementedException (); + EventRequires = (object sender, EventArgs e) => throw new NotImplementedException (); // no warning on field assignment + EventRequires (null, null); // no warning on invocation } [ExpectedWarning("IL3002", "--Requires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs index 6aa4bc0e78370..2b2c0438c1d01 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/ReflectionAccessFromCompilerGeneratedCode.cs @@ -167,6 +167,22 @@ static void TestLambdaWithRUC () lambda (); } + [ExpectedWarning ("IL2026", "--TestLambdaWithRUCLdftn--")] + [ExpectedWarning ("IL3002", "--TestLambdaWithRUCLdftn--", ProducedBy = Tool.NativeAot | Tool.Analyzer)] + [ExpectedWarning ("IL3050", "--TestLambdaWithRUCLdftn--", ProducedBy = Tool.NativeAot | Tool.Analyzer)] + static void TestLambdaWithRUCLdftn () + { + var lambda = + [RequiresUnreferencedCode ("--TestLambdaWithRUCLdftn--")] + [RequiresAssemblyFiles ("--TestLambdaWithRUCLdftn--")] + [RequiresDynamicCode ("--TestLambdaWithRUCLdftn--")] + () => { + var _ = new Action (TypeWithMethodWithRequires.MethodWithRequires); + var _2 = new Action (TypeWithMethodWithRequires.MethodWithAnnotations); + }; + lambda (); + } + [RequiresUnreferencedCode ("--TestLambdaInMethodWithRUC--")] [RequiresAssemblyFiles ("--TestLambdaInMethodWithRUC--")] [RequiresDynamicCode ("--TestLambdaInMethodWithRUC--")] @@ -186,6 +202,7 @@ public static void Test () { TestLambda (); TestLambdaWithRUC (); + TestLambdaWithRUCLdftn (); TestLambdaInMethodWithRUC (); } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs index ebb7b6639843c..3def1ee762bea 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs @@ -624,10 +624,7 @@ static void TestLocalFunctionWithRequires () static void TestCallUnused () { - // Analyzer emits warnings for code in unused local functions. - [ExpectedWarning ("IL2026", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + // No warning for code in unused local functions. void LocalFunction () => MethodWithRequires (); } @@ -647,10 +644,7 @@ void LocalFunction () static void TestCallWithClosureUnused (int p = 0) { - // Analyzer emits warnings for code in unused local functions. - [ExpectedWarning ("IL2026", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", ProducedBy = Tool.Analyzer)] + // No warning for code in unused local functions. void LocalFunction () { p++; @@ -933,12 +927,7 @@ public static void TestCallMethodWithRequiresInDynamicallyAccessedLocalFunction // ILLink isn't able to figure out which user method this local function // belongs to, but it doesn't warn because it is only accessed via reflection. // Instead this warns on the reflection access. - [ExpectedWarning ("IL2026", "--MethodWithRequires--", - ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3002", "--MethodWithRequires--", - ProducedBy = Tool.Analyzer)] - [ExpectedWarning ("IL3050", "--MethodWithRequires--", - ProducedBy = Tool.Analyzer)] + // No warning for code in unused local functions. void LocalFunction () => MethodWithRequires (); } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs index abc594d177739..633e1bed4195d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs @@ -100,8 +100,8 @@ public static void MethodWithRequires () { } private class DerivedWithoutRequires : ClassWithRequires { // This method contains implicit call to ClassWithRequires.ctor() - [ExpectedWarning ("IL2026", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3050", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL3050", ProducedBy = Tool.Analyzer | Tool.NativeAot)] public DerivedWithoutRequires () { } public static void StaticMethodInInheritedClass () { } @@ -309,8 +309,8 @@ public static void StaticMethodInInheritedClass () { } public class DerivedNestedClass : ClassWithRequires { // This method contains implicit call to ClassWithRequires.ctor() - [ExpectedWarning ("IL2026", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - [ExpectedWarning ("IL3050", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL3050", ProducedBy = Tool.Analyzer | Tool.NativeAot)] public DerivedNestedClass () { } public static void NestedStaticMethod () { } @@ -611,8 +611,8 @@ public BaseWithRequires () { } [ExpectedWarning ("IL2109", "ReflectionAccessOnCtor.DerivedWithoutRequires", "ReflectionAccessOnCtor.BaseWithRequires")] class DerivedWithoutRequires : BaseWithRequires { - [ExpectedWarning ("IL2026", "--BaseWithRequires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] // The body has direct call to the base.ctor() - [ExpectedWarning ("IL3050", "--BaseWithRequires--", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL2026", "--BaseWithRequires--")] // The body has direct call to the base.ctor() + [ExpectedWarning ("IL3050", "--BaseWithRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] public DerivedWithoutRequires () { } } diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs index 88bfcd26a519d..a13a73dee1c5e 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs @@ -880,7 +880,7 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge break; } if (memberDefinition.Name == ".ctor" && - memberDefinition.DeclaringType.FullName == expectedMember.FullName) { + (expectedMember is FieldDefinition || expectedMember is PropertyDefinition || memberDefinition.DeclaringType.FullName == expectedMember.FullName)) { expectedWarningFound = true; loggedMessages.Remove (loggedMessage); break; From b84bf33d20e75314c6bd00955af490f2cbedccce Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 17 Oct 2023 18:40:47 +0000 Subject: [PATCH 02/13] Add test for annotated value assigned to event --- .../DataFlow/EventDataFlow.cs | 31 +++++++++++++++++++ .../RequiresCapability/BasicRequires.cs | 8 +++++ 2 files changed, 39 insertions(+) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/EventDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/EventDataFlow.cs index 0c01a5a060e72..4cbd34c589535 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/EventDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/EventDataFlow.cs @@ -2,6 +2,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using Mono.Linker.Tests.Cases.Expectations.Assertions; namespace Mono.Linker.Tests.Cases.DataFlow @@ -33,12 +35,41 @@ public static void TestAssignCapturedEvent (bool b = false) MyEvent = b ? HandleMyEvent : HandleMyEvent2; } + delegate void TypeEventHandler(Type t); + + static event TypeEventHandler TypeEvent; + + [ExpectedWarning ("IL2111", nameof (DynamicallyAccessedMembersAttribute))] + public static void TestAssignEventMismatchingAnnotations () + { + TypeEvent = + ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type t) + => throw new Exception (); + } + + delegate void AnnotatedTypeEventHandler([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t); + + static event AnnotatedTypeEventHandler AnnotatedTypeEvent; + + // It doesn't matter whether the assigned lambda has annotations that match the + // delegate type - we treat the delegate creation as reflection access to the + // lambda method, and warn that it has annotations. + [ExpectedWarning ("IL2111", nameof (DynamicallyAccessedMembersAttribute))] + public static void TestAssignEventMatchingAnnotations () + { + AnnotatedTypeEvent = + ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) + => throw new Exception (); + } + // No dataflow warnings are involved, but the event assignment should not // crash the analyzer. public static void Test () { TestAssignEvent (); TestAssignCapturedEvent (); + TestAssignEventMismatchingAnnotations (); + TestAssignEventMatchingAnnotations (); } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs index 4668cdf23aedf..31ab1b0a65ca9 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs @@ -223,11 +223,19 @@ static void TestProperty () [ExpectedWarning ("IL3002", "--EventRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL3002", "--EventRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL2026", "--RequiresOnEventLambda--")] + [ExpectedWarning ("IL3002", "--RequiresOnEventLambda--")] + [ExpectedWarning ("IL3050", "--RequiresOnEventLambda--")] static void TestEvent () { EventRequires += (object sender, EventArgs e) => throw new NotImplementedException (); EventRequires -= (object sender, EventArgs e) => throw new NotImplementedException (); EventRequires = (object sender, EventArgs e) => throw new NotImplementedException (); // no warning on field assignment + EventRequires = + [RequiresUnreferencedCode ("--RequiresOnEventLambda--")] + [RequiresAssemblyFiles ("--RequiresOnEventLambda--")] + [RequiresDynamicCode ("--RequiresOnEventLambda--")] + (object sender, EventArgs e) => throw new NotImplementedException (); EventRequires (null, null); // no warning on invocation } From 9c286cf352949b71c8307013fad4e47f8e072aa7 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 19 Oct 2023 19:09:02 +0000 Subject: [PATCH 03/13] Use DiagnosticContext for consistency --- .../TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs index 2878edfa31b47..25e0bfff577bc 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -53,13 +53,15 @@ public IEnumerable CollectDiagnostics (RequiresAnalyzerContext conte DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { foreach (var diagnostic in ReflectionAccessAnalyzer.GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, ReferencedMethod)) - yield return diagnostic; + diagnosticContext.AddDiagnostic (diagnostic); } foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) { if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, ReferencedMethod, OwningSymbol, context, out Diagnostic? diag)) - yield return diag; + diagnosticContext.AddDiagnostic (diag); } + + return diagnosticContext.Diagnostics; } } } From 3923a309ff3c55d81b063308392c2f6485c6d109 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 19 Oct 2023 21:58:54 +0000 Subject: [PATCH 04/13] Move field access handling into dataflow analyzer --- .../DataFlow/LocalDataFlowVisitor.cs | 4 +- .../RequiresAnalyzerBase.cs | 38 ++------------ .../TrimAnalysisFieldAccessPattern.cs | 50 +++++++++++++++++++ .../TrimAnalysis/TrimAnalysisPatternStore.cs | 20 ++++++++ .../TrimAnalysis/TrimAnalysisVisitor.cs | 8 ++- 5 files changed, 81 insertions(+), 39 deletions(-) create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index b0752871faafb..bce994df6901d 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -92,7 +92,7 @@ public void Transfer (BlockProxy block, LocalDataFlowState GetFieldTargetValue (fieldRef.Field), + IFieldReferenceOperation fieldRef => GetFieldTargetValue (fieldRef.Field, fieldRef), IParameterReferenceOperation parameterRef => GetParameterTargetValue (parameterRef.Parameter), _ => throw new InvalidOperationException () }; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index fac9a29466354..daa0276a70788 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -55,11 +55,6 @@ public override void Initialize (AnalysisContext context) CheckMatchingAttributesInInterfaces (symbolAnalysisContext, typeSymbol); }, SymbolKind.NamedType); - context.RegisterOperationAction (operationContext => { - var fieldReference = (IFieldReferenceOperation) operationContext.Operation; - CheckCalledMember (operationContext, fieldReference.Field, incompatibleMembers); - }, OperationKind.FieldReference); - context.RegisterSyntaxNodeAction (syntaxNodeAnalysisContext => { var model = syntaxNodeAnalysisContext.SemanticModel; if (syntaxNodeAnalysisContext.ContainingSymbol is not ISymbol containingSymbol || containingSymbol.IsInRequiresScope (RequiresAttributeName, out _)) @@ -116,33 +111,6 @@ public override void Initialize (AnalysisContext context) foreach (var extraSymbolAction in ExtraSymbolActions) context.RegisterSymbolAction (extraSymbolAction.Action, extraSymbolAction.SymbolKind); - void CheckCalledMember ( - OperationAnalysisContext operationContext, - ISymbol member, - ImmutableArray incompatibleMembers) - { - // Do not emit diagnostics if the operation is nameof() - if (operationContext.Operation.Parent is IOperation operation && operation.Kind == OperationKind.NameOf) - return; - - // Do not emit any diagnostics for constant fields - they can only have Requires attributes applied to them - // via the type, and in that case the attribute is guarding the access to the static ctor. - // But constant fields are never accessed at runtime, and thus they don't cause static ctor to run. - // Constant fields are always inlined by the compiler (required by the ECMA spec). - if (member is IFieldSymbol field && field.HasConstantValue) - return; - - ISymbol containingSymbol = FindContainingSymbol (operationContext, AnalyzerDiagnosticTargets); - - if (CheckAndCreateRequiresDiagnostic ( - operationContext.Operation, - containingSymbol, - member, - ImmutableArray.Empty, - out Diagnostic? diagnostic)) - operationContext.ReportDiagnostic (diagnostic); - } - void CheckMatchingAttributesInOverrides ( SymbolAnalysisContext symbolAnalysisContext, ISymbol member) @@ -166,8 +134,8 @@ void CheckMatchingAttributesInInterfaces ( public bool CheckAndCreateRequiresDiagnostic ( IOperation operation, - ISymbol containingSymbol, ISymbol member, + ISymbol containingSymbol, ImmutableArray incompatibleMembers, [NotNullWhen (true)] out Diagnostic? diagnostic) { @@ -329,7 +297,7 @@ protected virtual bool CreateSpecialIncompatibleMembersDiagnostic ( internal bool CheckAndCreateRequiresDiagnostic ( IOperation operation, - IMethodSymbol methodSymbol, + ISymbol member, ISymbol owningSymbol, RequiresAnalyzerContext context, [NotNullWhen (true)] out Diagnostic? diagnostic) @@ -339,8 +307,8 @@ internal bool CheckAndCreateRequiresDiagnostic ( var incompatibleMembers = context.GetSpecialIncompatibleMembers (this); return CheckAndCreateRequiresDiagnostic ( operation, + member, containingSymbol, - methodSymbol, incompatibleMembers, out diagnostic); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs new file mode 100644 index 0000000000000..a69c63abbad1c --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs @@ -0,0 +1,50 @@ +// 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 System.Collections.Immutable; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices.ComTypes; +using ILLink.Shared; +using ILLink.Shared.DataFlow; +using ILLink.Shared.TrimAnalysis; +using ILLink.Shared.TypeSystemProxy; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Diagnostics; +using MultiValue = ILLink.Shared.DataFlow.ValueSet; + +namespace ILLink.RoslynAnalyzer.TrimAnalysis +{ + public readonly record struct TrimAnalysisFieldAccessPattern + { + public IFieldSymbol Field { init; get; } + public IFieldReferenceOperation Operation { init; get; } + public ISymbol OwningSymbol { init; get; } + + public TrimAnalysisFieldAccessPattern ( + IFieldSymbol field, + IFieldReferenceOperation operation, + ISymbol owningSymbol) + { + Field = field; + Operation = operation; + OwningSymbol = owningSymbol; + } + + // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the entity + // and there's only one way to "access" a field. + + public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) + { + DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); + foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) { + if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, Field, OwningSymbol, context, out Diagnostic? diag)) + diagnosticContext.AddDiagnostic (diag); + } + + return diagnosticContext.Diagnostics; + } + } +} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index cf1dc0bb87fe6..cee5d2d5602c7 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.Diagnostics; using ILLink.Shared.DataFlow; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -11,6 +12,7 @@ namespace ILLink.RoslynAnalyzer.TrimAnalysis public readonly struct TrimAnalysisPatternStore { readonly Dictionary<(IOperation, bool), TrimAnalysisAssignmentPattern> AssignmentPatterns; + readonly Dictionary FieldAccessPatterns; readonly Dictionary MethodCallPatterns; readonly Dictionary ReflectionAccessPatterns; readonly ValueSetLattice Lattice; @@ -18,6 +20,7 @@ public readonly struct TrimAnalysisPatternStore public TrimAnalysisPatternStore (ValueSetLattice lattice) { AssignmentPatterns = new Dictionary<(IOperation, bool), TrimAnalysisAssignmentPattern> (); + FieldAccessPatterns = new Dictionary (); MethodCallPatterns = new Dictionary (); ReflectionAccessPatterns = new Dictionary (); Lattice = lattice; @@ -41,6 +44,18 @@ public void Add (TrimAnalysisAssignmentPattern trimAnalysisPattern, bool isRetur AssignmentPatterns[(trimAnalysisPattern.Operation, isReturnValue)] = trimAnalysisPattern.Merge (Lattice, existingPattern); } + public void Add (TrimAnalysisFieldAccessPattern pattern) + { + if (!FieldAccessPatterns.TryGetValue (pattern.Operation, out var existingPattern)) { + FieldAccessPatterns.Add (pattern.Operation, pattern); + return; + } + + // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the entity + // and there's only one way to "access" a field. + Debug.Assert (existingPattern == pattern, "Field access patterns should be identical"); + } + public void Add (TrimAnalysisMethodCallPattern pattern) { if (!MethodCallPatterns.TryGetValue (pattern.Operation, out var existingPattern)) { @@ -68,6 +83,11 @@ public IEnumerable CollectDiagnostics (RequiresAnalyzerContext conte yield return diagnostic; } + foreach (var fieldAccessPattern in FieldAccessPatterns.Values) { + foreach (var diagnostic in fieldAccessPattern.CollectDiagnostics (context)) + yield return diagnostic; + } + foreach (var methodCallPattern in MethodCallPatterns.Values) { foreach (var diagnostic in methodCallPattern.CollectDiagnostics (context)) yield return diagnostic; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 0c0e038ffcc5b..7c5ba24321e2d 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -142,7 +142,7 @@ public override MultiValue VisitFieldReference (IFieldReferenceOperation fieldRe if (TryGetConstantValue (fieldRef, out var constValue)) return constValue; - return GetFieldTargetValue (fieldRef.Field); + return GetFieldTargetValue (fieldRef.Field, fieldRef); } public override MultiValue VisitTypeOf (ITypeOfOperation typeOfOperation, StateValue state) @@ -185,8 +185,12 @@ operation.OperatorMethod is null && // - method calls // - value returned from a method - public override MultiValue GetFieldTargetValue (IFieldSymbol field) + public override MultiValue GetFieldTargetValue (IFieldSymbol field, IFieldReferenceOperation fieldReferenceOperation) { + TrimAnalysisPatterns.Add ( + new TrimAnalysisFieldAccessPattern (field, fieldReferenceOperation, OwningSymbol) + ); + return field.Type.IsTypeInterestingForDataflow () ? new FieldValue (field) : TopValue; } From ee376697bbe2fbfc0f22fd812db50ccfdfb0469e Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 19 Oct 2023 22:05:19 +0000 Subject: [PATCH 05/13] Remove unnecessary merge logic for reflection access pattern --- .../TrimAnalysis/TrimAnalysisPatternStore.cs | 4 +++- .../TrimAnalysisReflectionAccessPattern.cs | 14 ++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index cee5d2d5602c7..a498ac1ebea51 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -73,7 +73,9 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) return; } - ReflectionAccessPatterns[pattern.Operation] = pattern.Merge (Lattice, existingPattern); + // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the entity + // and there's only one way to access the referenced method. + Debug.Assert (existingPattern == pattern, "Reflection access patterns should be identical"); } public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs index 25e0bfff577bc..5082060899956 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -35,18 +35,8 @@ public TrimAnalysisReflectionAccessPattern ( OwningSymbol = owningSymbol; } - public TrimAnalysisReflectionAccessPattern Merge (ValueSetLattice lattice, TrimAnalysisReflectionAccessPattern other) - { - Debug.Assert (Operation == other.Operation); - Debug.Assert (SymbolEqualityComparer.Default.Equals (ReferencedMethod, other.ReferencedMethod)); - Debug.Assert (SymbolEqualityComparer.Default.Equals (OwningSymbol, other.OwningSymbol)); - - return new TrimAnalysisReflectionAccessPattern ( - ReferencedMethod, - lattice.Meet (Instance, other.Instance), - Operation, - OwningSymbol); - } + // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the entity + // and there's only one way to access the referenced method. public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) { From 326ef2386673ab9e5f96c41e3636fb3c3890f447 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 20 Oct 2023 16:47:10 +0000 Subject: [PATCH 06/13] PR feedback - Clean up a few unused usings - Remove unused Instance field --- .../TrimAnalysis/TrimAnalysisFieldAccessPattern.cs | 9 --------- .../TrimAnalysis/TrimAnalysisMethodCallPattern.cs | 4 ---- .../TrimAnalysisReflectionAccessPattern.cs | 12 ------------ .../TrimAnalysis/TrimAnalysisVisitor.cs | 1 - 4 files changed, 26 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs index a69c63abbad1c..76db34849d5ba 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs @@ -2,18 +2,9 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Runtime.InteropServices.ComTypes; -using ILLink.Shared; -using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; -using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis.Diagnostics; -using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer.TrimAnalysis { diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs index 453e9934b7f13..8159d0480bfb5 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs @@ -1,17 +1,13 @@ // 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.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; -using System.Runtime.InteropServices.ComTypes; -using ILLink.Shared; using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer.TrimAnalysis diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs index 5082060899956..d9e6f23dc6e81 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -2,35 +2,23 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Runtime.InteropServices.ComTypes; -using ILLink.Shared; -using ILLink.Shared.DataFlow; using ILLink.Shared.TrimAnalysis; -using ILLink.Shared.TypeSystemProxy; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; -using MultiValue = ILLink.Shared.DataFlow.ValueSet; namespace ILLink.RoslynAnalyzer.TrimAnalysis { public readonly record struct TrimAnalysisReflectionAccessPattern { public IMethodSymbol ReferencedMethod { init; get; } - public MultiValue Instance { init; get; } public IOperation Operation { init; get; } public ISymbol OwningSymbol { init; get; } public TrimAnalysisReflectionAccessPattern ( IMethodSymbol referencedMethod, - MultiValue instance, IOperation operation, ISymbol owningSymbol) { ReferencedMethod = referencedMethod; - Instance = instance.DeepCopy (); Operation = operation; OwningSymbol = owningSymbol; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 7c5ba24321e2d..b82175435e718 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -321,7 +321,6 @@ public override MultiValue HandleDelegateCreation (IMethodSymbol method, MultiVa { TrimAnalysisPatterns.Add (new TrimAnalysisReflectionAccessPattern ( method, - instance, operation, OwningSymbol )); From f898cf12e8b412db17ea6b480a702cad45a99137 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 20 Oct 2023 16:57:46 +0000 Subject: [PATCH 07/13] Fix ExpectedWarnings for ILLink --- .../RequiresCapability/BasicRequires.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs index 31ab1b0a65ca9..d7643f8abe1cd 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs @@ -224,8 +224,8 @@ static void TestProperty () [ExpectedWarning ("IL3002", "--EventRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL3002", "--EventRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL2026", "--RequiresOnEventLambda--")] - [ExpectedWarning ("IL3002", "--RequiresOnEventLambda--")] - [ExpectedWarning ("IL3050", "--RequiresOnEventLambda--")] + [ExpectedWarning ("IL3002", "--RequiresOnEventLambda--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3050", "--RequiresOnEventLambda--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] static void TestEvent () { EventRequires += (object sender, EventArgs e) => throw new NotImplementedException (); From 5e684d33a7d6daa4c3c70eecb3d0a599f813a6a3 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 20 Oct 2023 18:31:50 +0000 Subject: [PATCH 08/13] Fix assert for ref property as out param And related cases involving implicit indexer references. --- .../DataFlow/LocalDataFlowVisitor.cs | 12 +- .../DataFlow/MethodByRefParameterDataFlow.cs | 115 ++++++++++++++++++ .../DataFlow/MethodByRefReturnDataFlow.cs | 27 ++++ 3 files changed, 150 insertions(+), 4 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index 5a522c9d3c709..ebf6aef982266 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -523,10 +523,11 @@ public override TValue VisitDelegateCreation (IDelegateCreationOperation operati public override TValue VisitPropertyReference (IPropertyReferenceOperation operation, LocalDataFlowState state) { - // Writing to a property should not go through this path. - Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)); - if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) + if (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Write)) { + // Property references may be passed as ref/out parameters. + Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference)); return TopValue; + } // Accessing property for reading is really a call to the getter // The setter case is handled in assignment operation since here we don't have access to the value to pass to the setter @@ -556,8 +557,11 @@ public override TValue VisitEventReference (IEventReferenceOperation operation, public override TValue VisitImplicitIndexerReference (IImplicitIndexerReferenceOperation operation, LocalDataFlowState state) { - if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read)) + if (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Write)) { + // Implicit indexer references may be passed as ref/out parameters. + Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference)); return TopValue; + } TValue instanceValue = Visit (operation.Instance, state); TValue indexArgumentValue = Visit (operation.Argument, state); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs index dc9cf29d73c6e..ccdbb4060c0d1 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Reflection; +using System.Runtime.CompilerServices; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Helpers; @@ -27,6 +28,9 @@ public static void Main () TestReadFromRefParameter_MismatchOnOutput_PassedTwice (); TestReadFromRefParameter_MismatchOnInput (); TestReadFromRefParameter_MismatchOnInput_PassedTwice (); + TestReadFromOutParameter (); + TestReadFromOutParameter_DeclaredBefore (); + TestReadFromOutParameter_Ovewrite (); Type nullType1 = null; TestPassingRefParameter (ref nullType1); Type nullType2 = null; @@ -37,6 +41,14 @@ public static void Main () TestAssigningToRefParameter_Mismatch (nullType4, ref nullType4); TestPassingRefsWithImplicitThis (); TestPassingCapturedOutParameter (); + TestPassingRefProperty (); + TestPassingRefProperty_OutParameter (); + TestPassingRefProperty_Mismatch (); + TestPassingRefProperty_OutParameter_Mismatch (); + TestPassingRefIndexer (); + TestPassingRefIndexer_OutParameter (); + TestPassingRefIndexer_Mismatch (); + TestPassingRefIndexer_OutParameter_Mismatch (); LocalMethodsAndLambdas.Test (); } @@ -132,6 +144,26 @@ static void TestReadFromRefParameter_MismatchOnInput_PassedTwice () typeWithMethods.RequiresPublicMethods (); } + static void TestReadFromOutParameter () + { + TryGetAnnotatedValueOut (out Type typeWithMethods); + typeWithMethods.RequiresPublicMethods (); + } + + static void TestReadFromOutParameter_DeclaredBefore () + { + Type typeWithMethods; + TryGetAnnotatedValueOut (out typeWithMethods); + typeWithMethods.GetMethods (); // Should not warn + } + + static void TestReadFromOutParameter_Ovewrite () + { + Type typeWithMethods = typeof (int); + TryGetAnnotatedValueOut (out typeWithMethods); + typeWithMethods.RequiresPublicMethods (); + } + static void TestPassingRefParameter ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] ref Type typeWithMethods) { TryGetAnnotatedValue (ref typeWithMethods); @@ -165,6 +197,12 @@ static bool TryGetAnnotatedValue ([DynamicallyAccessedMembers (DynamicallyAccess return false; } + static bool TryGetAnnotatedValueOut ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] out Type typeWithMethods) + { + typeWithMethods = null; + return false; + } + static bool TryGetAnnotatedValueFromValue ( [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type inValue, [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] ref Type typeWithMethods) @@ -210,6 +248,83 @@ class TestType { } + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static Type typeWithMethodsField; + + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static ref Type TypeWithMethodsProperty => ref typeWithMethodsField; + + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + static Type typeWithFieldsField; + + static ref Type TypeWithFieldsProperty => ref typeWithFieldsField; + + static void TestPassingRefProperty () + { + TryGetAnnotatedValue (ref TypeWithMethodsProperty); + } + + static void TestPassingRefProperty_OutParameter () + { + TryGetAnnotatedValueOut (out TypeWithMethodsProperty); + } + + static void TestPassingRefProperty_Mismatch () + { + TryGetAnnotatedValue (ref TypeWithFieldsProperty); + } + + static void TestPassingRefProperty_OutParameter_Mismatch () + { + TryGetAnnotatedValueOut (out TypeWithFieldsProperty); + } + + class RefIndexer_PublicMethods + { + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + Type typeWithMethodsField; + + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + public ref Type this[int index] => ref typeWithMethodsField; + + public int Length => 1; + } + + class RefIndexer_PublicFields + { + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + Type typeWithFieldsField; + + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + public ref Type this[int index] => ref typeWithFieldsField; + + public int Length => 1; + } + + static void TestPassingRefIndexer () + { + var indexer = new RefIndexer_PublicMethods (); + TryGetAnnotatedValue (ref indexer[new Index(0)]); + } + + static void TestPassingRefIndexer_OutParameter () + { + var indexer = new RefIndexer_PublicMethods (); + TryGetAnnotatedValueOut (out indexer[new Index(0)]); + } + + static void TestPassingRefIndexer_Mismatch () + { + var indexer = new RefIndexer_PublicFields (); + TryGetAnnotatedValue (ref indexer[0]); + } + + static void TestPassingRefIndexer_OutParameter_Mismatch () + { + var indexer = new RefIndexer_PublicFields (); + TryGetAnnotatedValueOut (out indexer[0]); + } + static class LocalMethodsAndLambdas { static ref Type GetTypeRefWithoutAnnotations () { throw null; } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs index eb8d6de677ae7..5985337d8f22d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs @@ -18,6 +18,10 @@ public static void Main () AssignDirectlyToAnnotatedTypeReference (); AssignToCapturedAnnotatedTypeReference (); AssignToAnnotatedTypeReferenceWithRequirements (); + var _ = AnnotatedTypeReferenceAsUnannotatedProperty; + AssignToAnnotatedTypeReferenceProperty (); + AssignDirectlyToAnnotatedTypeReferenceProperty (); + AssignToCapturedAnnotatedTypeReferenceProperty (); TestCompoundAssignment (typeof (int)); TestCompoundAssignmentCapture (typeof (int)); TestCompoundAssignmentMultipleCaptures (typeof (int), typeof (int)); @@ -71,6 +75,29 @@ static void AssignToAnnotatedTypeReferenceWithRequirements () ReturnAnnotatedTypeWithRequirements (GetWithPublicMethods ()) = GetWithPublicFields (); } + static ref Type AnnotatedTypeReferenceAsUnannotatedProperty => ref _annotatedField; + + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static ref Type AnnotatedTypeReferenceAsAnnotatedProperty => ref _annotatedField; + + static void AssignToAnnotatedTypeReferenceProperty () + { + ref Type typeShouldHaveAllMethods = ref AnnotatedTypeReferenceAsAnnotatedProperty; + typeShouldHaveAllMethods = typeof (TestTypeWithRequires); + _annotatedField.GetMethods (); + } + + static void AssignDirectlyToAnnotatedTypeReferenceProperty () + { + AnnotatedTypeReferenceAsAnnotatedProperty = typeof (TestTypeWithRequires); + _annotatedField.GetMethods (); + } + + static void AssignToCapturedAnnotatedTypeReferenceProperty () + { + AnnotatedTypeReferenceAsAnnotatedProperty = GetWithPublicMethods () ?? GetWithPublicFields (); + } + static int intField; static ref int GetRefReturnInt ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) => ref intField; From bc3c075b0e153fdfb4e2f134dd25aff628384291 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 20 Oct 2023 18:45:53 +0000 Subject: [PATCH 09/13] Add test for deconstruction assignment to ref property --- .../DataFlow/LocalDataFlowVisitor.cs | 5 ++++- .../DataFlow/ConstructedTypesDataFlow.cs | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index ebf6aef982266..c52f2e0cea66f 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -525,7 +525,10 @@ public override TValue VisitPropertyReference (IPropertyReferenceOperation opera { if (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Write)) { // Property references may be passed as ref/out parameters. - Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference)); + // Enable this assert once we have support for deconstruction assignments. + // https://github.com/dotnet/linker/issues/3158 + // Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference), + // $"{operation.Syntax.GetLocation ().GetLineSpan ()}"); return TopValue; } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs index 089537053303b..8c152f61dfd64 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs @@ -44,6 +44,19 @@ static void DeconstructVariableFlowCapture (bool b = true) type.RequiresPublicMethods (); } + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static Type annotatedfield; + + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + static ref Type AnnotatedProperty => ref annotatedfield; + + static void DeconstructVariablePropertyReference ((Type type, object instance) input) + { + object instance; + (AnnotatedProperty, instance) = input; + AnnotatedProperty.RequiresPublicMethods (); + } + record TypeAndInstance ( [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] [property: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] @@ -114,6 +127,7 @@ public static void Test () { DeconstructVariableNoAnnotation ((typeof (string), null)); DeconstructVariableFlowCapture (); + DeconstructVariablePropertyReference ((typeof (string), null)); DeconstructRecordWithAnnotation (new (typeof (string), null)); DeconstructClassWithAnnotation (new (typeof (string), null)); DeconstructRecordManualWithAnnotation (new (typeof (string), null)); From a03a6fe97b8ec89190b3f140a941b6e70b41d66b Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 20 Oct 2023 19:13:02 +0000 Subject: [PATCH 10/13] Fix delegate creation over invocation --- .../DataFlow/LocalDataFlowVisitor.cs | 61 +++++++++++-------- .../TrimAnalysis/TrimAnalysisVisitor.cs | 2 +- .../AnnotatedMembersAccessedViaReflection.cs | 57 ++++++++++++++--- 3 files changed, 83 insertions(+), 37 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index c52f2e0cea66f..4a3df554fe87d 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -488,38 +488,47 @@ public override TValue VisitInvocation (IInvocationOperation operation, LocalDat public override TValue VisitDelegateCreation (IDelegateCreationOperation operation, LocalDataFlowState state) { - if (operation.Target is IFlowAnonymousFunctionOperation lambda) { - VisitFlowAnonymousFunction (lambda, state); - - // Instance of a lambda or local function should be the instance of the containing method. - // Don't need to track a dataflow value, since the delegate creation will warn if the - // lambda or local function has an annotated this parameter. - var instance = TopValue; - return HandleDelegateCreation (lambda.Symbol, instance, operation); - } + Visit (operation.Target, state); - Debug.Assert (operation.Target is IMemberReferenceOperation, - $"{operation.Target.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}"); - if (operation.Target is not IMemberReferenceOperation memberReference) - return TopValue; + IMethodSymbol? targetMethodSymbol = null; + switch (operation.Target) { + case IFlowAnonymousFunctionOperation lambda: + // Tracking lambdas is handled by normal visiting logic for IFlowAnonymousFunctionOperation. - TValue instanceValue = Visit (memberReference.Instance, state); + // Instance of a lambda or local function should be the instance of the containing method. + // Don't need to track a dataflow value, since the delegate creation will warn if the + // lambda or local function has an annotated this parameter. + targetMethodSymbol = lambda.Symbol; + break; + case IMethodReferenceOperation methodReference: + IMethodSymbol method = methodReference.Method.OriginalDefinition; + if (method.ContainingSymbol is IMethodSymbol) { + // Track references to local functions + var localFunction = method; + Debug.Assert (localFunction.MethodKind == MethodKind.LocalFunction);; + var localFunctionCFG = ControlFlowGraph.GetLocalFunctionControlFlowGraphInScope (localFunction); + InterproceduralState.TrackMethod (new MethodBodyValue (localFunction, localFunctionCFG)); + } + targetMethodSymbol = method; + break; + case IMemberReferenceOperation: + case IInvocationOperation: + // No method symbol. + break; + default: + // Unimplemented case that might need special handling. + // Fail in debug mode only. + Debug.Fail ($"{operation.Target.GetType ()}: {operation.Target.Syntax.GetLocation ().GetLineSpan ()}"); + break; + } - if (memberReference.Member is not IMethodSymbol method) + if (targetMethodSymbol == null) return TopValue; - // Track references to local functions - if (method.OriginalDefinition.ContainingSymbol is IMethodSymbol) { - var localFunction = method.OriginalDefinition; - Debug.Assert (localFunction.MethodKind == MethodKind.LocalFunction);; - var localFunctionCFG = ControlFlowGraph.GetLocalFunctionControlFlowGraphInScope (localFunction); - InterproceduralState.TrackMethod (new MethodBodyValue (localFunction, localFunctionCFG)); - } - - return HandleDelegateCreation (method, instanceValue, operation); + return HandleDelegateCreation (targetMethodSymbol, operation); } - public abstract TValue HandleDelegateCreation (IMethodSymbol methodReference, TValue instance, IOperation operation); + public abstract TValue HandleDelegateCreation (IMethodSymbol methodReference, IOperation operation); public override TValue VisitPropertyReference (IPropertyReferenceOperation operation, LocalDataFlowState state) { @@ -528,7 +537,7 @@ public override TValue VisitPropertyReference (IPropertyReferenceOperation opera // Enable this assert once we have support for deconstruction assignments. // https://github.com/dotnet/linker/issues/3158 // Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference), - // $"{operation.Syntax.GetLocation ().GetLineSpan ()}"); + // $"{operation.Syntax.GetLocation ().GetLineSpan ()}"); return TopValue; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index b82175435e718..5dab06de44a4d 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -317,7 +317,7 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera } } - public override MultiValue HandleDelegateCreation (IMethodSymbol method, MultiValue instance, IOperation operation) + public override MultiValue HandleDelegateCreation (IMethodSymbol method, IOperation operation) { TrimAnalysisPatterns.Add (new TrimAnalysisReflectionAccessPattern ( method, diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs index aa1dfa72202c5..d3fcbf32df47e 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs @@ -892,29 +892,66 @@ class DelegateCreation static Action Property { get; set; } - [ExpectedWarning ("IL2111", "LocalMethod")] - [ExpectedWarning ("IL2111")] - public static void Test () + static Action MethodReturnValue () => null; + + static event Action Event; + + static void TestField () { - // Check that the analyzer is able to analyze delegate creation - // with various targets, without hitting an assert. - UnannotatedDelegate d; - d = new UnannotatedDelegate (field); + var d = new UnannotatedDelegate (field); d(typeof(int)); - d = new UnannotatedDelegate (Property); + } + + static void TestProperty () + { + var d = new UnannotatedDelegate (Property); d(typeof(int)); + } - d = new UnannotatedDelegate ( + [ExpectedWarning ("IL2111")] + static void TestLambda () + { + var d = new UnannotatedDelegate ( ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type t) => { }); d(typeof(int)); - d = new UnannotatedDelegate (LocalMethod); + } + + [ExpectedWarning ("IL2111", "LocalMethod")] + static void TestLocalMethod () + { + var d = new UnannotatedDelegate (LocalMethod); d(typeof(int)); void LocalMethod ( [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) { } } + + static void TestMethodReturnValue () + { + var d = new UnannotatedDelegate (MethodReturnValue ()); + d(typeof(int)); + } + + + static void TestEvent () + { + var d = new UnannotatedDelegate (Event); + d(typeof(int)); + } + + public static void Test () + { + // Check that the analyzer is able to analyze delegate creation + // with various targets, without hitting an assert. + TestField (); + TestProperty (); + TestLambda (); + TestLocalMethod (); + TestMethodReturnValue (); + TestEvent (); + } } class TestType { } From 08c5d0c3cef83bbff2b264a3d4f0625762f8ff6c Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 20 Oct 2023 20:02:40 +0000 Subject: [PATCH 11/13] Fix ExpectedWarnings for illink --- .../DataFlow/ConstructedTypesDataFlow.cs | 3 +++ .../DataFlow/MethodByRefParameterDataFlow.cs | 7 +++++++ .../DataFlow/MethodByRefReturnDataFlow.cs | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs index 8c152f61dfd64..abeed77e1bc0e 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs @@ -50,6 +50,9 @@ static void DeconstructVariableFlowCapture (bool b = true) [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] static ref Type AnnotatedProperty => ref annotatedfield; + // https://github.com/dotnet/linker/issues/3158 + [ExpectedWarning ("IL2062", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL2078", ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void DeconstructVariablePropertyReference ((Type type, object instance) input) { object instance; diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs index ccdbb4060c0d1..7ea036c5c39bb 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefParameterDataFlow.cs @@ -269,11 +269,13 @@ static void TestPassingRefProperty_OutParameter () TryGetAnnotatedValueOut (out TypeWithMethodsProperty); } + [ExpectedWarning ("IL2072", nameof (TryGetAnnotatedValue), ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void TestPassingRefProperty_Mismatch () { TryGetAnnotatedValue (ref TypeWithFieldsProperty); } + // TODO: Missing warning. static void TestPassingRefProperty_OutParameter_Mismatch () { TryGetAnnotatedValueOut (out TypeWithFieldsProperty); @@ -313,12 +315,17 @@ static void TestPassingRefIndexer_OutParameter () TryGetAnnotatedValueOut (out indexer[new Index(0)]); } + // https://github.com/dotnet/linker/issues/2158 + [ExpectedWarning ("IL2068", nameof (TryGetAnnotatedValue), ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [ExpectedWarning ("IL2072", nameof (TryGetAnnotatedValue), ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void TestPassingRefIndexer_Mismatch () { var indexer = new RefIndexer_PublicFields (); TryGetAnnotatedValue (ref indexer[0]); } + // https://github.com/dotnet/linker/issues/2158 + [ExpectedWarning ("IL2068", nameof (TryGetAnnotatedValue), ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void TestPassingRefIndexer_OutParameter_Mismatch () { var indexer = new RefIndexer_PublicFields (); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs index 5985337d8f22d..7220a83d3ce2d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MethodByRefReturnDataFlow.cs @@ -80,6 +80,7 @@ static void AssignToAnnotatedTypeReferenceWithRequirements () [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] static ref Type AnnotatedTypeReferenceAsAnnotatedProperty => ref _annotatedField; + [ExpectedWarning ("IL2026", "Message for --TestType.Requires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void AssignToAnnotatedTypeReferenceProperty () { ref Type typeShouldHaveAllMethods = ref AnnotatedTypeReferenceAsAnnotatedProperty; @@ -87,12 +88,16 @@ static void AssignToAnnotatedTypeReferenceProperty () _annotatedField.GetMethods (); } + // https://github.com/dotnet/linker/issues/2158 + [ExpectedWarning ("IL2026", "Message for --TestType.Requires--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void AssignDirectlyToAnnotatedTypeReferenceProperty () { AnnotatedTypeReferenceAsAnnotatedProperty = typeof (TestTypeWithRequires); _annotatedField.GetMethods (); } + // https://github.com/dotnet/linker/issues/2158 + [ExpectedWarning ("IL2073", nameof (GetWithPublicFields), ProducedBy = Tool.Trimmer | Tool.NativeAot)] static void AssignToCapturedAnnotatedTypeReferenceProperty () { AnnotatedTypeReferenceAsAnnotatedProperty = GetWithPublicMethods () ?? GetWithPublicFields (); From eb8c185e72c0a97be5d710322b6b09604dee5c98 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Sat, 21 Oct 2023 00:35:42 +0000 Subject: [PATCH 12/13] Silence dataflow warnings if EnableTrimAnalyer isn't set Extra warnings were being produced on some projects where the single-file analyzer was enabled, causing the DAM analyzer to light up. The dataflow warnings were missing a check of EnableTrimAnalzer to prevent these warnings. This also lifts the checking of MSBuild property values out into the compilation start action, so it only needs to be done per compilation. The context type has been renamed to reflect that it holds state relevant to all of the analyses handled by the dataflow logic now. --- ...rContext.cs => DataflowAnalyzerContext.cs} | 21 ++++++++++++------- .../DynamicallyAccessedMembersAnalyzer.cs | 7 +++---- .../RequiresAnalyzerBase.cs | 2 +- .../TrimAnalysisFieldAccessPattern.cs | 2 +- .../TrimAnalysisMethodCallPattern.cs | 4 ++-- .../TrimAnalysis/TrimAnalysisPatternStore.cs | 2 +- .../TrimAnalysisReflectionAccessPattern.cs | 4 ++-- .../TrimAnalysis/TrimDataFlowAnalysis.cs | 5 ++--- 8 files changed, 26 insertions(+), 21 deletions(-) rename src/tools/illink/src/ILLink.RoslynAnalyzer/{RequiresAnalyzerContext.cs => DataflowAnalyzerContext.cs} (56%) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs similarity index 56% rename from src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs rename to src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs index 50db1def90ab4..f3a3274fe85a8 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerContext.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs @@ -8,9 +8,9 @@ namespace ILLink.RoslynAnalyzer { - public struct RequiresAnalyzerContext + public readonly struct DataFlowAnalyzerContext { - private Dictionary> _enabledAnalyzers; + private readonly Dictionary> _enabledAnalyzers; public IEnumerable EnabledRequiresAnalyzers => _enabledAnalyzers.Keys; @@ -21,21 +21,28 @@ public ImmutableArray GetSpecialIncompatibleMembers (RequiresAnalyzerBa return members; } - RequiresAnalyzerContext (Dictionary> enabledAnalyzers) + public readonly bool EnableTrimAnalyzer { get; } + + public readonly bool AnyAnalyzersEnabled => EnableTrimAnalyzer || _enabledAnalyzers.Count > 0; + + DataFlowAnalyzerContext (Dictionary> enabledAnalyzers, bool enableTrimAnalyzer) { _enabledAnalyzers = enabledAnalyzers; + EnableTrimAnalyzer = enableTrimAnalyzer; } - public static RequiresAnalyzerContext Create (OperationBlockAnalysisContext context, ImmutableArray requiresAnalyzers) + public static DataFlowAnalyzerContext Create (AnalyzerOptions options, Compilation compilation, ImmutableArray requiresAnalyzers) { var enabledAnalyzers = new Dictionary> (); foreach (var analyzer in requiresAnalyzers) { - if (analyzer.IsAnalyzerEnabled (context.Options)) { - var incompatibleMembers = analyzer.GetSpecialIncompatibleMembers (context.Compilation); + if (analyzer.IsAnalyzerEnabled (options)) { + var incompatibleMembers = analyzer.GetSpecialIncompatibleMembers (compilation); enabledAnalyzers.Add (analyzer, incompatibleMembers); } } - return new RequiresAnalyzerContext (enabledAnalyzers); + return new DataFlowAnalyzerContext ( + enabledAnalyzers, + options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer)); } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index 86b9e41ff1a64..7b93dad541b92 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -84,16 +84,15 @@ public override void Initialize (AnalysisContext context) context.EnableConcurrentExecution (); context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics); context.RegisterCompilationStartAction (context => { - if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer) && - !context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer) && - !context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer)) + var dataFlowAnalyzerContext = DataFlowAnalyzerContext.Create (context.Options, context.Compilation, RequiresAnalyzers.Value); + if (!dataFlowAnalyzerContext.AnyAnalyzersEnabled) return; context.RegisterOperationBlockAction (context => { foreach (var operationBlock in context.OperationBlocks) { TrimDataFlowAnalysis trimDataFlowAnalysis = new (context, operationBlock); trimDataFlowAnalysis.InterproceduralAnalyze (); - foreach (var diagnostic in trimDataFlowAnalysis.CollectDiagnostics ()) + foreach (var diagnostic in trimDataFlowAnalysis.CollectDiagnostics (dataFlowAnalyzerContext)) context.ReportDiagnostic (diagnostic); } }); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index daa0276a70788..3e86d98c6bd50 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -299,7 +299,7 @@ internal bool CheckAndCreateRequiresDiagnostic ( IOperation operation, ISymbol member, ISymbol owningSymbol, - RequiresAnalyzerContext context, + DataFlowAnalyzerContext context, [NotNullWhen (true)] out Diagnostic? diagnostic) { ISymbol containingSymbol = operation.FindContainingSymbol (owningSymbol); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs index 76db34849d5ba..1cebeab6903e1 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs @@ -27,7 +27,7 @@ public TrimAnalysisFieldAccessPattern ( // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the entity // and there's only one way to "access" a field. - public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) { diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs index 8159d0480bfb5..5807ce79045ff 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs @@ -62,11 +62,11 @@ public TrimAnalysisMethodCallPattern Merge (ValueSetLattice lattice OwningSymbol); } - public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); - if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { + if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { HandleCallAction handleCallAction = new (diagnosticContext, OwningSymbol, Operation); MethodProxy method = new (CalledMethod); IntrinsicId intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index a498ac1ebea51..46d41ab75d033 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -78,7 +78,7 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) Debug.Assert (existingPattern == pattern, "Reflection access patterns should be identical"); } - public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { foreach (var assignmentPattern in AssignmentPatterns.Values) { foreach (var diagnostic in assignmentPattern.CollectDiagnostics ()) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs index d9e6f23dc6e81..fbd70c7be1dac 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -26,10 +26,10 @@ public TrimAnalysisReflectionAccessPattern ( // No Merge - there's nothing to merge since this pattern is uniquely identified by both the origin and the entity // and there's only one way to access the referenced method. - public IEnumerable CollectDiagnostics (RequiresAnalyzerContext context) + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); - if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { + if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { foreach (var diagnostic in ReflectionAccessAnalyzer.GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, ReferencedMethod)) diagnosticContext.AddDiagnostic (diagnostic); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs index 3ec6b10c2481a..8961405071afb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs @@ -29,10 +29,9 @@ public TrimDataFlowAnalysis (OperationBlockAnalysisContext context, IOperation o TrimAnalysisPatterns = new TrimAnalysisPatternStore (Lattice.Lattice.ValueLattice); } - public IEnumerable CollectDiagnostics () + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext dataFlowAnalyzerContext) { - var requiresAnalyzerContext = RequiresAnalyzerContext.Create (Context, DynamicallyAccessedMembersAnalyzer.RequiresAnalyzers.Value); - return TrimAnalysisPatterns.CollectDiagnostics (requiresAnalyzerContext); + return TrimAnalysisPatterns.CollectDiagnostics (dataFlowAnalyzerContext); } protected override TrimAnalysisVisitor GetVisitor ( From 5ceaa31090d6f65e853678bfbfaa9fa8cfa55709 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 23 Oct 2023 17:11:46 +0000 Subject: [PATCH 13/13] Add test coverage for EnableTrimAnalyzer checks And add missing checks in DAM analyzer --- .../DynamicallyAccessedMembersAnalyzer.cs | 5 ++ .../TrimAnalysisAssignmentPattern.cs | 4 +- .../TrimAnalysis/TrimAnalysisPatternStore.cs | 2 +- .../RequiresAssemblyFilesAnalyzerTests.cs | 64 +++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index 7b93dad541b92..33a7651c907aa 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -96,6 +96,11 @@ public override void Initialize (AnalysisContext context) context.ReportDiagnostic (diagnostic); } }); + + // Remaining actions are only for DynamicallyAccessedMembers analysis. + if (!dataFlowAnalyzerContext.EnableTrimAnalyzer) + return; + // Examine generic instantiations in base types and interface list context.RegisterSymbolAction (context => { var type = (INamedTypeSymbol) context.Symbol; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs index 495e29c1c7012..8cc8b41c43d68 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs @@ -45,10 +45,10 @@ public TrimAnalysisAssignmentPattern Merge (ValueSetLattice lattice OwningSymbol); } - public IEnumerable CollectDiagnostics () + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); - if (!OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { + if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _)) { foreach (var sourceValue in Source) { foreach (var targetValue in Target) { // The target should always be an annotated value, but the visitor design currently prevents diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index 46d41ab75d033..5d194b438d394 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -81,7 +81,7 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { foreach (var assignmentPattern in AssignmentPatterns.Values) { - foreach (var diagnostic in assignmentPattern.CollectDiagnostics ()) + foreach (var diagnostic in assignmentPattern.CollectDiagnostics (context)) yield return diagnostic; } diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs index d2a47efccbc65..c00a692e94c91 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs @@ -61,6 +61,70 @@ static Task VerifyRequiresAssemblyFilesCodeFix ( return test.RunAsync (); } + [Fact] + public Task NoDynamicallyAccessedMembersWarningsIfOnlySingleFileAnalyzerIsEnabled () + { + var TargetParameterWithAnnotations = $$""" + using System; + using System.Diagnostics.CodeAnalysis; + + class C + { + public static void Main() + { + MethodCallPattern(typeof(int)); + AssignmentPattern(typeof(int)); + ReflectionAccessPattern(); + FieldAccessPattern(); + GenericRequirement(); + } + + private static void NeedsPublicMethodsOnParameter( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type parameter) + { + } + + private static void MethodCallPattern(Type type) + { + NeedsPublicMethodsOnParameter(type); + } + + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] + private static Type NeedsPublicMethosOnField; + + private static void AssignmentPattern(Type type) + { + NeedsPublicMethosOnField = type; + } + + private static void ReflectionAccessPattern() + { + Action action = NeedsPublicMethodsOnParameter; + } + + private static void FieldAccessPattern() + { + var i = BeforeFieldInit.StaticField; + } + + [RequiresUnreferencedCode("BeforeFieldInit")] + class BeforeFieldInit { + public static int StaticField = 0; + } + + private static void GenericRequirement() + { + new NeedsPublicMethodsOnTypeParameter(); + } + + class NeedsPublicMethodsOnTypeParameter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T> + { + } + } + """; + return VerifyRequiresAssemblyFilesAnalyzer (TargetParameterWithAnnotations); + } + [Fact] public Task SimpleDiagnosticOnEvent () {