diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj index 9625b051d1fd0..1faf04ea61a65 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj @@ -158,6 +158,7 @@ + diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Attribute.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Attribute.NativeAot.cs new file mode 100644 index 0000000000000..0774e107f7d23 --- /dev/null +++ b/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(); + } + } +} diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ValueTypeGetFieldHelperMethodOverride.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ValueTypeGetFieldHelperMethodOverride.cs index 532087549e237..a66ce62ea00ef 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/ValueTypeGetFieldHelperMethodOverride.cs +++ b/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); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/BlockedInternalsBlockingPolicy.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/BlockedInternalsBlockingPolicy.cs index 7abb3b45155cb..32145754e9988 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/BlockedInternalsBlockingPolicy.cs +++ b/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; } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs index 97176e404cb82..a022eb556e4d6 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs +++ b/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 GetAllVirtualMethods(TypeDesc type) [MethodImpl(MethodImplOptions.AggressiveInlining)] private IEnumerable 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 GetAllMethods(TypeDesc type, bool virtualOnly) { return GetAllMethodsForValueType(type, virtualOnly); } + else if (type.CanCastTo(attributeType)) + { + return GetAllMethodsForAttribute(type, virtualOnly); + } return virtualOnly ? type.GetVirtualMethods() : type.GetMethods(); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs index f142691d68262..0e03ed4a6a843 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.ValueTypeMethods.cs @@ -35,7 +35,7 @@ protected virtual IEnumerable 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 GetAllMethodsForValueType(TypeDesc val yield return method; } - private bool RequiresGetFieldHelperMethod(MetadataType valueType) + protected virtual IEnumerable 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 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 diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index de0ecf7c45684..5403534d411ef 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/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) diff --git a/src/libraries/System.Private.CoreLib/src/System/Attribute.cs b/src/libraries/System.Private.CoreLib/src/System/Attribute.cs index 2bc115b9f686d..d9e2e0e43dfd0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Attribute.cs +++ b/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)