diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index 1fe0177a18020e..e1f8eacbf7957e 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -129,7 +129,9 @@ public abstract TConditionValue GetConditionValue( IOperation branchValueOperation, LocalDataFlowState state); - public abstract TValue GetFieldTargetValue(IFieldSymbol field, IFieldReferenceOperation fieldReferenceOperation, in TContext context); + public abstract TValue GetFieldTargetValue(IFieldReferenceOperation fieldReference, in TContext context); + + public abstract TValue GetBackingFieldTargetValue(IPropertyReferenceOperation propertyReference, in TContext context); public abstract TValue GetParameterTargetValue(IParameterSymbol parameter); @@ -247,7 +249,7 @@ private TValue ProcessSingleTargetAssignment(IOperation targetOperation, IAssign var current = state.Current; TValue targetValue = targetOperation switch { - IFieldReferenceOperation fieldRef => GetFieldTargetValue(fieldRef.Field, fieldRef, in current.Context), + IFieldReferenceOperation fieldRef => GetFieldTargetValue(fieldRef, in current.Context), IParameterReferenceOperation parameterRef => GetParameterTargetValue(parameterRef.Parameter), _ => throw new InvalidOperationException() }; @@ -266,19 +268,28 @@ private TValue ProcessSingleTargetAssignment(IOperation targetOperation, IAssign TValue instanceValue = Visit(propertyRef.Instance, state); TValue value = Visit(operation.Value, state); IMethodSymbol? setMethod = propertyRef.Property.GetSetMethod(); - if (setMethod == null) + + if (setMethod == null || + (OwningSymbol is IPropertySymbol && (ControlFlowGraph.OriginalOperation is not IAttributeOperation))) { + // This can be a write to a byref return of a property getter. + // Skip for now: https://github.com/dotnet/linker/issues/2158 + if (propertyRef.Property.RefKind is not RefKind.None) + break; + // This can happen in a constructor - there it is possible to assign to a property // without a setter. This turns into an assignment to the compiler-generated backing field. - // To match the linker, this should warn about the compiler-generated backing field. - // For now, just don't warn. https://github.com/dotnet/runtime/issues/93277 - break; + // Handle this similarly to field assignments. + + // Even if the property has a set method, if the assignment takes place in a property initializer, + // the write becomes a direct write to the underlying field. This should be treated the same as + // the case where there is no set method. + + var current = state.Current; + TValue targetValue = GetBackingFieldTargetValue(propertyRef, in current.Context); + HandleAssignment(value, targetValue, operation, in current.Context); + return value; } - // Even if the property has a set method, if the assignment takes place in a property initializer, - // the write becomes a direct write to the underlying field. This should be treated the same as - // the case where there is no set method. - if (OwningSymbol is IPropertySymbol && (ControlFlowGraph.OriginalOperation is not IAttributeOperation)) - break; // Property may be an indexer, in which case there will be one or more index arguments followed by a value argument ImmutableArray.Builder arguments = ImmutableArray.CreateBuilder(); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FieldValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FieldValue.cs index dce9fb248936c7..35bf5214c0fce5 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FieldValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FieldValue.cs @@ -12,13 +12,23 @@ namespace ILLink.Shared.TrimAnalysis internal partial record FieldValue { public FieldValue(IFieldSymbol fieldSymbol) + : this(fieldSymbol, fieldSymbol.Type, FlowAnnotations.GetFieldAnnotation(fieldSymbol)) + { + } + + public FieldValue(IPropertySymbol propertySymbol) + : this(propertySymbol, propertySymbol.Type, FlowAnnotations.GetBackingFieldAnnotation(propertySymbol)) + { + } + + private FieldValue(ISymbol fieldSymbol, ITypeSymbol fieldType, DynamicallyAccessedMemberTypes annotations) { FieldSymbol = fieldSymbol; - StaticType = new(fieldSymbol.Type); - DynamicallyAccessedMemberTypes = FlowAnnotations.GetFieldAnnotation(fieldSymbol); + StaticType = new(fieldType); + DynamicallyAccessedMemberTypes = annotations; } - public readonly IFieldSymbol FieldSymbol; + public readonly ISymbol FieldSymbol; public override DynamicallyAccessedMemberTypes DynamicallyAccessedMemberTypes { get; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs index 32d604217f5ad7..78368423563f27 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs @@ -120,6 +120,14 @@ internal static DynamicallyAccessedMemberTypes GetFieldAnnotation(IFieldSymbol f return field.GetDynamicallyAccessedMemberTypes(); } + internal static DynamicallyAccessedMemberTypes GetBackingFieldAnnotation(IPropertySymbol property) + { + if (!property.OriginalDefinition.Type.IsTypeInterestingForDataflow(isByRef: false)) + return DynamicallyAccessedMemberTypes.None; + + return property.GetDynamicallyAccessedMemberTypes(); + } + internal static DynamicallyAccessedMemberTypes GetTypeAnnotations(INamedTypeSymbol type) { DynamicallyAccessedMemberTypes typeAnnotation = type.GetDynamicallyAccessedMemberTypes(); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/GenericArgumentDataFlow.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/GenericArgumentDataFlow.cs index 679760c8528470..280d81b876e08f 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/GenericArgumentDataFlow.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/GenericArgumentDataFlow.cs @@ -33,6 +33,11 @@ public static void ProcessGenericArgumentDataFlow(Location location, IFieldSymbo ProcessGenericArgumentDataFlow(location, field.ContainingType, reportDiagnostic); } + public static void ProcessGenericArgumentDataFlow(Location location, IPropertySymbol property, Action reportDiagnostic) + { + ProcessGenericArgumentDataFlow(location, property.ContainingType, reportDiagnostic); + } + private static void ProcessGenericArgumentDataFlow( Location location, ImmutableArray typeArguments, @@ -102,6 +107,11 @@ public static bool RequiresGenericArgumentDataFlow(IFieldSymbol field) return RequiresGenericArgumentDataFlow(field.ContainingType); } + public static bool RequiresGenericArgumentDataFlow(IPropertySymbol property) + { + return RequiresGenericArgumentDataFlow(property.ContainingType); + } + private static bool RequiresGenericArgumentDataFlow(ImmutableArray typeParameters) { foreach (var typeParameter in typeParameters) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisBackingFieldAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisBackingFieldAccessPattern.cs new file mode 100644 index 00000000000000..7ad0d048f50125 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisBackingFieldAccessPattern.cs @@ -0,0 +1,55 @@ +// 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.Diagnostics; +using ILLink.Shared.TrimAnalysis; +using ILLink.Shared.DataFlow; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; +using ILLink.RoslynAnalyzer.DataFlow; + +namespace ILLink.RoslynAnalyzer.TrimAnalysis +{ + internal readonly record struct TrimAnalysisBackingFieldAccessPattern + { + public IPropertySymbol Property { get; init; } + public IPropertyReferenceOperation Operation { get; init; } + public ISymbol OwningSymbol { get; init; } + public FeatureContext FeatureContext { get; init; } + + public TrimAnalysisBackingFieldAccessPattern( + IPropertySymbol property, + IPropertyReferenceOperation operation, + ISymbol owningSymbol, + FeatureContext featureContext) + { + Property = property; + Operation = operation; + OwningSymbol = owningSymbol; + FeatureContext = featureContext; + } + + public TrimAnalysisBackingFieldAccessPattern Merge( + FeatureContextLattice featureContextLattice, + TrimAnalysisBackingFieldAccessPattern other) + { + Debug.Assert(SymbolEqualityComparer.Default.Equals(Property, other.Property)); + Debug.Assert(Operation == other.Operation); + Debug.Assert(SymbolEqualityComparer.Default.Equals(OwningSymbol, other.OwningSymbol)); + + return new TrimAnalysisBackingFieldAccessPattern( + Property, + Operation, + OwningSymbol, + featureContextLattice.Meet(FeatureContext, other.FeatureContext)); + } + + public void ReportDiagnostics(DataFlowAnalyzerContext context, Action reportDiagnostic) + { + DiagnosticContext diagnosticContext = new(Operation.Syntax.GetLocation(), reportDiagnostic); + foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) + requiresAnalyzer.CheckAndCreateRequiresDiagnostic(Operation, Property, OwningSymbol, context, FeatureContext, in diagnosticContext); + } + } +} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index 34e5226b31d949..96c1505a6cf9c9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -14,6 +14,7 @@ internal readonly struct TrimAnalysisPatternStore { private readonly Dictionary AssignmentPatterns; private readonly Dictionary FieldAccessPatterns; + private readonly Dictionary BackingFieldAccessPatterns; private readonly Dictionary GenericInstantiationPatterns; private readonly Dictionary MethodCallPatterns; private readonly Dictionary ReflectionAccessPatterns; @@ -27,6 +28,7 @@ public TrimAnalysisPatternStore( { AssignmentPatterns = new Dictionary(); FieldAccessPatterns = new Dictionary(); + BackingFieldAccessPatterns = new Dictionary(); GenericInstantiationPatterns = new Dictionary(); MethodCallPatterns = new Dictionary(); ReflectionAccessPatterns = new Dictionary(); @@ -35,6 +37,17 @@ public TrimAnalysisPatternStore( FeatureContextLattice = featureContextLattice; } + public void Add(TrimAnalysisBackingFieldAccessPattern pattern) + { + if (!BackingFieldAccessPatterns.TryGetValue(pattern.Operation, out var existingPattern)) + { + BackingFieldAccessPatterns.Add(pattern.Operation, pattern); + return; + } + + BackingFieldAccessPatterns[pattern.Operation] = pattern.Merge(FeatureContextLattice, existingPattern); + } + public void Add(TrimAnalysisAssignmentPattern trimAnalysisPattern) { // Finally blocks will be analyzed multiple times, once for normal control flow and once @@ -117,6 +130,9 @@ public void ReportDiagnostics(DataFlowAnalyzerContext context, Action new MethodParameterValue(parameter); @@ -453,6 +469,21 @@ private void ProcessGenericArgumentDataFlow(IFieldSymbol field, IOperation opera } } + private void ProcessGenericArgumentDataFlow(IPropertySymbol property, IOperation operation, in FeatureContext featureContext) + { + if (!property.IsStatic) + return; + + if (GenericArgumentDataFlow.RequiresGenericArgumentDataFlow(property)) + { + TrimAnalysisPatterns.Add(new TrimAnalysisGenericInstantiationPattern( + property, + operation, + OwningSymbol, + featureContext)); + } + } + private static bool TryGetConstantValue(IOperation operation, out MultiValue constValue) { if (operation.ConstantValue.HasValue) 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 index 462c8579605557..60309e3acdc96f 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructorDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructorDataFlow.cs @@ -39,11 +39,11 @@ public DataFlowInConstructor() [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type annotatedField = GetUnknown(); - [ExpectedWarning("IL2074", nameof(GetUnknown), nameof(AnnotatedProperty), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/93277", CompilerGeneratedCode = true)] + [ExpectedWarning("IL2074", nameof(GetUnknown), nameof(AnnotatedProperty), CompilerGeneratedCode = true)] [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type AnnotatedProperty { get; } = GetUnknown(); - [ExpectedWarning("IL2074", nameof(GetUnknown), nameof(AnnotatedPropertyWithSetter), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/93277", CompilerGeneratedCode = true)] + [ExpectedWarning("IL2074", nameof(GetUnknown), nameof(AnnotatedPropertyWithSetter), CompilerGeneratedCode = true)] [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type AnnotatedPropertyWithSetter { get; set; } = GetUnknown(); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs index 76ead1e85bfafc..8adf581c6c5031 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs @@ -489,8 +489,7 @@ class WriteToGetOnlyProperty [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] public Type GetOnlyProperty { get; } - // Analyzer doesn't warn about compiler-generated backing field of property: https://github.com/dotnet/runtime/issues/93277 - [ExpectedWarning("IL2074", nameof(WriteToGetOnlyProperty), nameof(GetUnknownType), Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2074", nameof(WriteToGetOnlyProperty), nameof(GetUnknownType))] public WriteToGetOnlyProperty() { GetOnlyProperty = GetUnknownType(); @@ -568,9 +567,8 @@ class WriteCapturedGetOnlyProperty [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type GetOnlyProperty { get; } - // Analyzer doesn't warn about compiler-generated backing field of property: https://github.com/dotnet/runtime/issues/93277 - [ExpectedWarning("IL2074", nameof(WriteCapturedGetOnlyProperty), nameof(GetUnknownType), Tool.Trimmer | Tool.NativeAot, "")] - [ExpectedWarning("IL2074", nameof(WriteCapturedGetOnlyProperty), nameof(GetTypeWithPublicConstructors), Tool.Trimmer | Tool.NativeAot, "")] + [ExpectedWarning("IL2074", nameof(WriteCapturedGetOnlyProperty), nameof(GetUnknownType))] + [ExpectedWarning("IL2074", nameof(WriteCapturedGetOnlyProperty), nameof(GetTypeWithPublicConstructors))] public WriteCapturedGetOnlyProperty() { GetOnlyProperty = GetUnknownType() ?? GetTypeWithPublicConstructors();