Switch attribute equality to the valuetype equality plan

src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj

@@ -158,6 +158,7 @@
     <Compile Include="System\Activator.NativeAot.cs" />
     <Compile Include="System\AppContext.NativeAot.cs" />
     <Compile Include="System\ArgIterator.cs" />
+    <Compile Include="System\Attribute.NativeAot.cs" />
     <Compile Include="System\Buffer.NativeAot.cs" />
     <Compile Include="System\Collections\Generic\ArraySortHelper.NativeAot.cs" />
     <Compile Include="System\Collections\Generic\Comparer.NativeAot.cs" />

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Attribute.NativeAot.cs

@@ -0,0 +1,87 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+using System.Runtime;
+using System.Runtime.CompilerServices;
+
+using Internal.Runtime;
+
+namespace System
+{
+    public abstract partial class Attribute
+    {
+        // An override of this method will be injected by the compiler into all attributes that have fields.
+        // This API is a bit awkward because we want to avoid burning more than one vtable slot on this.
+        // When index is negative, this method is expected to return the number of fields of this
+        // valuetype. Otherwise, it returns the offset and type handle of the index-th field on this type.
+        internal virtual unsafe int __GetFieldHelper(int index, out MethodTable* mt)
+        {
+            Debug.Assert(index < 0);
+            mt = default;
+            return 0;
+        }
+
+        public override unsafe bool Equals([NotNullWhen(true)] object? obj)
+        {
+            if (obj == null)
+                return false;
+
+            if (this.GetType() != obj.GetType())
+                return false;
+
+            int numFields = __GetFieldHelper(-1, out _);
+
+            ref byte thisRawData = ref this.GetRawData();
+            ref byte thatRawData = ref obj.GetRawData();
+
+            for (int i = 0; i < numFields; i++)
+            {
+                int fieldOffset = __GetFieldHelper(i, out MethodTable* fieldType);
+
+                // Fetch the value of the field on both types
+                object thisResult = RuntimeImports.RhBoxAny(ref Unsafe.Add(ref thisRawData, fieldOffset), fieldType);
+                object thatResult = RuntimeImports.RhBoxAny(ref Unsafe.Add(ref thatRawData, fieldOffset), fieldType);
+
+                if (!AreFieldValuesEqual(thisResult, thatResult))
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        public override unsafe int GetHashCode()
+        {
+            int numFields = __GetFieldHelper(-1, out _);
+
+            ref byte thisRawData = ref this.GetRawData();
+
+            object? vThis = null;
+
+            for (int i = 0; i < numFields; i++)
+            {
+                int fieldOffset = __GetFieldHelper(i, out MethodTable* fieldType);
+                object? fieldValue = RuntimeImports.RhBoxAny(ref Unsafe.Add(ref thisRawData, fieldOffset), fieldType);
+
+                // The hashcode of an array ignores the contents of the array, so it can produce
+                // different hashcodes for arrays with the same contents.
+                // Since we do deep comparisons of arrays in Equals(), this means Equals and GetHashCode will
+                // be inconsistent for arrays. Therefore, we ignore hashes of arrays.
+                if (fieldValue != null && !fieldValue.GetType().IsArray)
+                    vThis = fieldValue;
+
+                if (vThis != null)
+                    break;
+            }
+
+            if (vThis != null)
+                return vThis.GetHashCode();
+
+            // Matches the reflection-based implementation in other runtimes
+            return typeof(Attribute).GetHashCode();
+        }
+    }
+}

src/coreclr/tools/Common/TypeSystem/IL/Stubs/ValueTypeGetFieldHelperMethodOverride.cs

@@ -67,6 +67,9 @@ public override MethodIL EmitIL()
             TypeDesc methodTableType = Context.SystemModule.GetKnownType("Internal.Runtime", "MethodTable");
             MethodDesc methodTableOfMethod = methodTableType.GetKnownMethod("Of", null);
 
+            ILToken rawDataToken = owningType.IsValueType ? default :
+                emitter.NewToken(Context.SystemModule.GetKnownType("System.Runtime.CompilerServices", "RawData").GetKnownField("Data"));
+
             var switchStream = emitter.NewCodeStream();
             var getFieldStream = emitter.NewCodeStream();
 
@@ -107,6 +110,10 @@ public override MethodIL EmitIL()
 
                 getFieldStream.EmitLdArg(0);
 
+                // If this is a reference type, we subtract from the first field. Otherwise subtract from `ref this`.
+                if (!owningType.IsValueType)
+                    getFieldStream.Emit(ILOpcode.ldflda, rawDataToken);
+
                 getFieldStream.Emit(ILOpcode.sub);
 
                 getFieldStream.Emit(ILOpcode.ret);
@@ -118,9 +125,38 @@ public override MethodIL EmitIL()
                 switchStream.EmitSwitch(fieldGetters.ToArray());
             }
 
-            switchStream.EmitLdc(fieldGetters.Count);
-
-            switchStream.Emit(ILOpcode.ret);
+            if (!owningType.IsValueType
+                && MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(this) is MethodDesc slotMethod
+                && owningType.BaseType.FindVirtualFunctionTargetMethodOnObjectType(slotMethod) is MethodDesc baseMethod
+                && slotMethod != baseMethod)
+            {
+                // On reference types, we recurse into base implementation too, handling both the case of asking
+                // for number of fields (add number of fields on the current class before returning), and
+                // handling field indices higher than what we handle (subtract number of fields handled here first).
+                switchStream.EmitLdArg(0);
+                switchStream.EmitLdArg(1);
+                switchStream.EmitLdc(fieldGetters.Count);
+                switchStream.Emit(ILOpcode.sub);
+                switchStream.EmitLdArg(2);
+                switchStream.Emit(ILOpcode.call, emitter.NewToken(baseMethod));
+                switchStream.EmitLdArg(1);
+                switchStream.EmitLdc(0);
+                ILCodeLabel lGotBaseFieldAddress = emitter.NewCodeLabel();
+                switchStream.Emit(ILOpcode.bge, lGotBaseFieldAddress);
+                switchStream.EmitLdc(fieldGetters.Count);
+                ILCodeLabel lGotNumFieldsInBase = emitter.NewCodeLabel();
+                switchStream.Emit(ILOpcode.br, lGotNumFieldsInBase);
+                switchStream.EmitLabel(lGotBaseFieldAddress);
+                switchStream.EmitLdc(0);
+                switchStream.EmitLabel(lGotNumFieldsInBase);
+                switchStream.Emit(ILOpcode.add);
+                switchStream.Emit(ILOpcode.ret);
+            }
+            else
+            {
+                switchStream.EmitLdc(fieldGetters.Count);
+                switchStream.Emit(ILOpcode.ret);
+            }
 
             return emitter.Link(this);
         }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/BlockedInternalsBlockingPolicy.cs

@@ -144,14 +144,12 @@ private bool ComputeIsBlocked(EcmaType type, ModuleBlockingMode blockingMode)
         private BlockedTypeHashtable _blockedTypes;
 
         private MetadataType ArrayOfTType { get; }
-        private MetadataType AttributeType { get; }
 
         public BlockedInternalsBlockingPolicy(TypeSystemContext context)
         {
             _blockedTypes = new BlockedTypeHashtable(_blockedModules);
 
             ArrayOfTType = context.SystemModule.GetType("System", "Array`1", throwIfNotFound: false);
-            AttributeType = context.SystemModule.GetType("System", "Attribute", throwIfNotFound: false);
         }
 
         public override bool IsBlocked(MetadataType type)
@@ -227,25 +225,9 @@ public override bool IsBlocked(FieldDesc field)
                 return true;
 
             FieldAttributes accessibility = ecmaField.Attributes & FieldAttributes.FieldAccessMask;
-            if (accessibility != FieldAttributes.Family
+            return accessibility != FieldAttributes.Family
                 && accessibility != FieldAttributes.FamORAssem
-                && accessibility != FieldAttributes.Public)
-            {
-                // Exempt fields on custom attributes from blocking.
-                // Attribute.Equals and Attribute.GetHashCode depends on being able to
-                // walk all fields on custom attributes using reflection.
-                // We're opening this hole in hopes that the fields won't have any "interesting"
-                // types (that would be themselves blocked). Doing that could be problematic.
-                if (AttributeType != null
-                    && owningType.CanCastTo(AttributeType))
-                {
-                    return false;
-                }
-
-                return true;
-            }
-
-            return false;
+                && accessibility != FieldAttributes.Public;
         }
     }
 }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs

@@ -38,6 +38,7 @@ public SharedGenericsConfiguration GenericsConfig
         private TypeDesc[] _arrayOfTInterfaces;
         private ArrayOfTRuntimeInterfacesAlgorithm _arrayOfTRuntimeInterfacesAlgorithm;
         private MetadataType _arrayOfTType;
+        private MetadataType _attributeType;
 
         public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode genericsMode, DelegateFeature delegateFeatures, int genericCycleCutoffPoint = DefaultGenericCycleCutoffPoint)
             : base(details)
@@ -132,6 +133,8 @@ protected override IEnumerable<MethodDesc> GetAllVirtualMethods(TypeDesc type)
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private IEnumerable<MethodDesc> GetAllMethods(TypeDesc type, bool virtualOnly)
         {
+            MetadataType attributeType = _attributeType ??= SystemModule.GetType("System", "Attribute");
+
             if (type.IsDelegate)
             {
                 return GetAllMethodsForDelegate(type, virtualOnly);
@@ -144,6 +147,10 @@ private IEnumerable<MethodDesc> GetAllMethods(TypeDesc type, bool virtualOnly)
             {
                 return GetAllMethodsForValueType(type, virtualOnly);
             }
+            else if (type.CanCastTo(attributeType))
+            {
+                return GetAllMethodsForAttribute(type, virtualOnly);
+            }
 
             return virtualOnly ? type.GetVirtualMethods() : type.GetMethods();
         }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs

@@ -35,7 +35,7 @@ protected virtual IEnumerable<MethodDesc> GetAllMethodsForValueType(TypeDesc val
         {
             TypeDesc valueTypeDefinition = valueType.GetTypeDefinition();
 
-            if (RequiresGetFieldHelperMethod((MetadataType)valueTypeDefinition))
+            if (RequiresValueTypeGetFieldHelperMethod((MetadataType)valueTypeDefinition))
             {
                 MethodDesc getFieldHelperMethod = _valueTypeMethodHashtable.GetOrCreateValue((DefType)valueTypeDefinition);
 
@@ -54,7 +54,30 @@ protected virtual IEnumerable<MethodDesc> GetAllMethodsForValueType(TypeDesc val
                 yield return method;
         }
 
-        private bool RequiresGetFieldHelperMethod(MetadataType valueType)
+        protected virtual IEnumerable<MethodDesc> GetAllMethodsForAttribute(TypeDesc attributeType, bool virtualOnly)
+        {
+            TypeDesc attributeTypeDefinition = attributeType.GetTypeDefinition();
+
+            if (RequiresAttributeGetFieldHelperMethod(attributeTypeDefinition))
+            {
+                MethodDesc getFieldHelperMethod = _valueTypeMethodHashtable.GetOrCreateValue((DefType)attributeTypeDefinition);
+
+                if (attributeType != attributeTypeDefinition)
+                {
+                    yield return GetMethodForInstantiatedType(getFieldHelperMethod, (InstantiatedType)attributeType);
+                }
+                else
+                {
+                    yield return getFieldHelperMethod;
+                }
+            }
+
+            IEnumerable<MethodDesc> metadataMethods = virtualOnly ? attributeType.GetVirtualMethods() : attributeType.GetMethods();
+            foreach (MethodDesc method in metadataMethods)
+                yield return method;
+        }
+
+        private bool RequiresValueTypeGetFieldHelperMethod(MetadataType valueType)
         {
             _objectEqualsMethod ??= GetWellKnownType(WellKnownType.Object).GetMethod("Equals", null);
 
@@ -90,6 +113,19 @@ public bool IsAsyncStateMachineType(MetadataType type)
                 && Array.IndexOf(type.RuntimeInterfaces, _iAsyncStateMachineType) >= 0;
         }
 
+        private static bool RequiresAttributeGetFieldHelperMethod(TypeDesc attributeTypeDef)
+        {
+            foreach (FieldDesc field in attributeTypeDef.GetFields())
+            {
+                if (field.IsStatic)
+                    continue;
+
+                return true;
+            }
+
+            return false;
+        }
+
         private sealed class TypeState
         {
             private enum Flags

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

@@ -663,40 +663,8 @@ public override void GetDependenciesDueToAccess(ref DependencyList dependencies,
                 AddDataflowDependency(ref dependencies, factory, methodIL, "Access to interesting field");
             }
 
-            string reason = "Use of a field";
-
-            bool generatesMetadata = false;
-            if (!IsReflectionBlocked(writtenField))
-            {
-                if ((_generationOptions & UsageBasedMetadataGenerationOptions.CreateReflectableArtifacts) != 0)
-                {
-                    // If access to the field should trigger metadata generation, we should generate the field
-                    generatesMetadata = true;
-                }
-                else
-                {
-                    // There's an invalid suppression in the CoreLib that assumes used fields on attributes will be kept.
-                    // It's used in the reflection-based implementation of Attribute.Equals and Attribute.GetHashCode.
-                    // .NET Native used to have a non-reflection based implementation of Equals/GetHashCode to get around
-                    // this problem. We could explore that as well, but for now, emulate the fact that accessed fields
-                    // on custom attributes will be visible in reflection metadata.
-                    MetadataType currentType = (MetadataType)writtenField.OwningType.BaseType;
-                    while (currentType != null)
-                    {
-                        if (currentType.Module == factory.TypeSystemContext.SystemModule
-                            && currentType.Name == "Attribute" && currentType.Namespace == "System")
-                        {
-                            generatesMetadata = true;
-                            reason = "Field of an attribute";
-                            break;
-                        }
-
-                        currentType = currentType.MetadataBaseType;
-                    }
-                }
-            }
-
-            if (generatesMetadata)
+            if ((_generationOptions & UsageBasedMetadataGenerationOptions.CreateReflectableArtifacts) != 0
+                && !IsReflectionBlocked(writtenField))
             {
                 FieldDesc fieldToReport = writtenField;
 
@@ -714,7 +682,7 @@ public override void GetDependenciesDueToAccess(ref DependencyList dependencies,
                 }
 
                 dependencies ??= new DependencyList();
-                dependencies.Add(factory.ReflectedField(fieldToReport), reason);
+                dependencies.Add(factory.ReflectedField(fieldToReport), "Use of a field");
             }
 
             if (writtenField.GetTypicalFieldDefinition() is EcmaField ecmaField)

src/libraries/System.Private.CoreLib/src/System/Attribute.cs

@@ -14,6 +14,7 @@ public abstract partial class Attribute
     {
         protected Attribute() { }
 
+#if !NATIVEAOT
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
             Justification = "Unused fields don't make a difference for equality")]
         public override bool Equals([NotNullWhen(true)] object? obj)
@@ -82,6 +83,7 @@ public override int GetHashCode()
 
             return type.GetHashCode();
         }
+#endif
 
         // Compares values of custom-attribute fields.
         private static bool AreFieldValuesEqual(object? thisValue, object? thatValue)