Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ public abstract TConditionValue GetConditionValue(
IOperation branchValueOperation,
LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> 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);

Expand Down Expand Up @@ -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()
};
Expand All @@ -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<TValue>.Builder arguments = ImmutableArray.CreateBuilder<TValue>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Diagnostic> reportDiagnostic)
{
ProcessGenericArgumentDataFlow(location, property.ContainingType, reportDiagnostic);
}

private static void ProcessGenericArgumentDataFlow(
Location location,
ImmutableArray<ITypeSymbol> typeArguments,
Expand Down Expand Up @@ -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<ITypeParameterSymbol> typeParameters)
{
foreach (var typeParameter in typeParameters)
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Diagnostic> reportDiagnostic)
{
DiagnosticContext diagnosticContext = new(Operation.Syntax.GetLocation(), reportDiagnostic);
foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers)
requiresAnalyzer.CheckAndCreateRequiresDiagnostic(Operation, Property, OwningSymbol, context, FeatureContext, in diagnosticContext);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ internal readonly struct TrimAnalysisPatternStore
{
private readonly Dictionary<IOperation, TrimAnalysisAssignmentPattern> AssignmentPatterns;
private readonly Dictionary<IOperation, TrimAnalysisFieldAccessPattern> FieldAccessPatterns;
private readonly Dictionary<IOperation, TrimAnalysisBackingFieldAccessPattern> BackingFieldAccessPatterns;
private readonly Dictionary<IOperation, TrimAnalysisGenericInstantiationPattern> GenericInstantiationPatterns;
private readonly Dictionary<IOperation, TrimAnalysisMethodCallPattern> MethodCallPatterns;
private readonly Dictionary<IOperation, TrimAnalysisReflectionAccessPattern> ReflectionAccessPatterns;
Expand All @@ -27,6 +28,7 @@ public TrimAnalysisPatternStore(
{
AssignmentPatterns = new Dictionary<IOperation, TrimAnalysisAssignmentPattern>();
FieldAccessPatterns = new Dictionary<IOperation, TrimAnalysisFieldAccessPattern>();
BackingFieldAccessPatterns = new Dictionary<IOperation, TrimAnalysisBackingFieldAccessPattern>();
GenericInstantiationPatterns = new Dictionary<IOperation, TrimAnalysisGenericInstantiationPattern>();
MethodCallPatterns = new Dictionary<IOperation, TrimAnalysisMethodCallPattern>();
ReflectionAccessPatterns = new Dictionary<IOperation, TrimAnalysisReflectionAccessPattern>();
Expand All @@ -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
Expand Down Expand Up @@ -117,6 +130,9 @@ public void ReportDiagnostics(DataFlowAnalyzerContext context, Action<Diagnostic
foreach (var fieldAccessPattern in FieldAccessPatterns.Values)
fieldAccessPattern.ReportDiagnostics(context, reportDiagnostic);

foreach (var backingFieldAccessPattern in BackingFieldAccessPatterns.Values)
backingFieldAccessPattern.ReportDiagnostics(context, reportDiagnostic);

foreach (var genericInstantiationPattern in GenericInstantiationPatterns.Values)
genericInstantiationPattern.ReportDiagnostics(context, reportDiagnostic);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public override MultiValue VisitFieldReference(IFieldReferenceOperation fieldRef
return constValue;

var current = state.Current;
return GetFieldTargetValue(fieldRef.Field, fieldRef, in current.Context);
return GetFieldTargetValue(fieldRef, in current.Context);
}

public override MultiValue VisitTypeOf(ITypeOfOperation typeOfOperation, StateValue state)
Expand Down Expand Up @@ -224,17 +224,33 @@ operation.OperatorMethod is null &&
// - method calls
// - value returned from a method

public override MultiValue GetFieldTargetValue(IFieldSymbol field, IFieldReferenceOperation fieldReferenceOperation, in FeatureContext featureContext)
public override MultiValue GetFieldTargetValue(IFieldReferenceOperation fieldReference, in FeatureContext featureContext)
{
var field = fieldReference.Field;

TrimAnalysisPatterns.Add(
new TrimAnalysisFieldAccessPattern(field, fieldReferenceOperation, OwningSymbol, featureContext)
new TrimAnalysisFieldAccessPattern(field, fieldReference, OwningSymbol, featureContext)
);

ProcessGenericArgumentDataFlow(field, fieldReferenceOperation, featureContext);
ProcessGenericArgumentDataFlow(field, fieldReference, featureContext);

return new FieldValue(field);
}

public override MultiValue GetBackingFieldTargetValue(IPropertyReferenceOperation propertyReference, in FeatureContext featureContext)
{
var property = propertyReference.Property;

TrimAnalysisPatterns.Add(
new TrimAnalysisBackingFieldAccessPattern(propertyReference.Property, propertyReference, OwningSymbol, featureContext)
);

ProcessGenericArgumentDataFlow(property, propertyReference, featureContext);

return new FieldValue(property);
}


public override MultiValue GetParameterTargetValue(IParameterSymbol parameter)
=> new MethodParameterValue(parameter);

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Loading