Skip to content

[release/10.0] Propagate DynamicallyAccessedMembers annotations to auto-properties only #119407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeffschwMSFT merged 1 commit into from
Sep 11, 2025
Merged

[release/10.0] Propagate DynamicallyAccessedMembers annotations to auto-properties only #119407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
119 changes: 71 additions & 48 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Reflection.Metadata;
Expand Down Expand Up @@ -523,7 +524,7 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key)
// Look for the compiler generated backing field. If it doesn't work out simply move on. In such case we would still
// propagate the annotation to the setter/getter and later on when analyzing the setter/getter we will warn
// that the field (which ever it is) must be annotated as well.
ScanMethodBodyForFieldAccess(methodBody, write: true, out backingFieldFromSetter);
backingFieldFromSetter = GetAutoPropertyCompilerGeneratedField(methodBody, isWriteAccessor: true);
}

MethodAnnotations? setterAnnotation = null;
Expand Down Expand Up @@ -560,16 +561,14 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key)
MethodDesc getMethod = property.GetMethod;
if (getMethod != null)
{

// Abstract property backing field propagation doesn't make sense, and any derived property will be validated
// to have the exact same annotations on getter/setter, and thus if it has a detectable backing field that will be validated as well.
// Look only at compiler-generated accessors
MethodIL methodBody = _ilProvider.GetMethodIL(getMethod);
if (methodBody != null)
{
// Look for the compiler generated backing field. If it doesn't work out simply move on. In such case we would still
// propagate the annotation to the setter/getter and later on when analyzing the setter/getter we will warn
// that the field (which ever it is) must be annotated as well.
ScanMethodBodyForFieldAccess(methodBody, write: false, out backingFieldFromGetter);
backingFieldFromGetter = GetAutoPropertyCompilerGeneratedField(methodBody, isWriteAccessor: false);
}

MethodAnnotations? getterAnnotation = null;
Expand All @@ -593,27 +592,41 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key)
}
}

FieldDesc? backingField;
if (backingFieldFromGetter != null && backingFieldFromSetter != null &&
backingFieldFromGetter != backingFieldFromSetter)
{
_logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName());
backingField = null;
}
else
{
backingField = backingFieldFromGetter ?? backingFieldFromSetter;
}

if (backingField != null)
if (IsAutoProperty(property))
{
if (annotatedFields.Any(a => a.Field == backingField))
FieldDesc? backingField = null;
if ((property.SetMethod is not null
&& property.SetMethod.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute")
&& backingFieldFromSetter is null)
|| (property.GetMethod is not null
&& property.GetMethod.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute")
&& backingFieldFromGetter is null))
{
_logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName());
// We failed to find the backing field of an auto-property accessor
_logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName());
}
else if (backingFieldFromGetter is not null && backingFieldFromSetter is not null
&& backingFieldFromSetter != backingFieldFromGetter)
{
// We found two different backing fields for the getter and the setter
_logger.LogWarning(property, DiagnosticId.DynamicallyAccessedMembersCouldNotFindBackingField, property.GetDisplayName());
}
else
{
annotatedFields.Add(new FieldAnnotation(backingField, annotation));
// We either have a single auto-property accessor or both accessors point to the same backing field
backingField = backingFieldFromSetter ?? backingFieldFromGetter;
}

if (backingField != null)
{
if (annotatedFields.Any(a => a.Field == backingField))
{
_logger.LogWarning(backingField, DiagnosticId.DynamicallyAccessedMembersOnPropertyConflictsWithBackingField, property.GetDisplayName(), backingField.GetDisplayName());
}
else
{
annotatedFields.Add(new FieldAnnotation(backingField, annotation));
}
}
}
}
Expand Down Expand Up @@ -651,59 +664,69 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key)
return attrs;
}

private static bool ScanMethodBodyForFieldAccess(MethodIL body, bool write, out FieldDesc? found)
/// <summary>
/// Returns true if the property has a single accessor which is compiler generated,
/// indicating that it is an auto-property.
/// </summary>
/// <remarks>
/// Ideally this would be tightened to only return true if both accessors are auto-property accessors,
/// but it allows for either for back compatibility with existing behavior.
/// </remarks>
private static bool IsAutoProperty(PropertyPseudoDesc property)
{
return property.SetMethod?.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute") == true
|| property.GetMethod?.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute") == true;
}

private static FieldDesc? GetAutoPropertyCompilerGeneratedField(MethodIL body, bool isWriteAccessor)
{
// Tries to find the backing field for a property getter/setter.
// Returns true if this is a method body that we can unambiguously analyze.
// The found field could still be null if there's no backing store.
found = null;

// Only analyze compiler-generated accessors
if (!body.OwningMethod.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute"))
{
return null;
}

FieldDesc? found = null;
ILReader ilReader = new ILReader(body.GetILBytes());

while (ilReader.HasNext)
{
ILOpcode opcode = ilReader.ReadILOpcode();
switch (opcode)
{
case ILOpcode.ldsfld when !write:
case ILOpcode.ldfld when !write:
case ILOpcode.stsfld when write:
case ILOpcode.stfld when write:
case ILOpcode.ldsfld when !isWriteAccessor:
case ILOpcode.ldfld when !isWriteAccessor:
case ILOpcode.stsfld when isWriteAccessor:
case ILOpcode.stfld when isWriteAccessor:
{
// Multiple field accesses - ambiguous, fail
if (found != null)
{
// This writes/reads multiple fields - can't guess which one is the backing store.
// Return failure.
if (found != null)
{
found = null;
return false;
}
found = (FieldDesc)body.GetObject(ilReader.ReadILToken());
return null;
}
found = (FieldDesc)body.GetObject(ilReader.ReadILToken());
break;
}
default:
ilReader.Skip(opcode);
break;
}
}

if (found == null)
{
// Doesn't access any fields. Could be e.g. "Type Foo => typeof(Bar);"
// Return success.
return true;
}

if (found.OwningType != body.OwningMethod.OwningType ||
if (found is null ||
found.OwningType != body.OwningMethod.OwningType ||
found.IsStatic != body.OwningMethod.Signature.IsStatic ||
!found.HasCustomAttribute("System.Runtime.CompilerServices", "CompilerGeneratedAttribute"))
{
// A couple heuristics to make sure we got the right field.
// Return failure.
found = null;
return false;
// Heuristics failed - not a backing field
return null;
}

return true;
return found;
}
}

Expand Down
33 changes: 33 additions & 0 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/IPropertySymbolExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace ILLink.RoslynAnalyzer
{
Expand Down Expand Up @@ -30,5 +31,37 @@ public static class IPropertySymbolExtensions
}
return setMethod;
}

public static bool IsAutoProperty(this IPropertySymbol property)
{
if (property.IsAbstract)
return false;

return (property.GetMethod?.IsAutoAccessor() ?? false) || (property.SetMethod?.IsAutoAccessor() ?? false);
}

private static bool IsAutoAccessor(this IMethodSymbol method)
{
if (method == null || method.IsAbstract)
return false;

foreach (var decl in method.DeclaringSyntaxReferences)
{
var syntax = decl.GetSyntax();
// Auto property accessors have no body in their syntax
switch (syntax)
{
case AccessorDeclarationSyntax a:
if (a.Body is not null)
return false;
if (a.ExpressionBody is not null)
return false;
return true;
default:
break;
}
}
return false;
}
}
}
7 changes: 7 additions & 0 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ internal static DynamicallyAccessedMemberTypes GetFieldAnnotation(IFieldSymbol f
if (!field.OriginalDefinition.Type.IsTypeInterestingForDataflow(isByRef: field.RefKind is not RefKind.None))
return DynamicallyAccessedMemberTypes.None;

if (field.AssociatedSymbol is IPropertySymbol property)
{
// If this is an auto property, we get the property annotation
if (property.IsAutoProperty())
return property.GetDynamicallyAccessedMemberTypes();
}

return field.GetDynamicallyAccessedMemberTypes();
}

Expand Down
Loading
Loading