Propagate DynamicallyAccessedMembers annotations to auto-properties only

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs

@@ -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;
@@ -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;
@@ -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;
@@ -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));
+                            }
                         }
                     }
                 }
@@ -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;
             }
         }
 

src/tools/illink/src/ILLink.RoslynAnalyzer/IPropertySymbolExtensions.cs

@@ -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
 {
@@ -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;
+        }
     }
 }

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs

@@ -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();
         }