From a35fdeaba0c09acc6a464d9e840c80614ff4574c Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 13 Jul 2025 19:24:06 +0200 Subject: [PATCH 01/13] System.Reflection.CustomAttributeFormatException thrown on a retrieval of derived attribute with overridden property getter --- .../Reflection/RuntimeCustomAttributeData.cs | 37 ++++++++++++++++--- .../Reflection/TypeTests.Get.CornerCases.cs | 37 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index eab9a97e8ba8e4..cbaca4ea3e2e0d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1573,14 +1573,39 @@ private static void AddCustomAttributes( } } - RuntimePropertyInfo? property = (RuntimePropertyInfo?)(type is null ? - attributeType.GetProperty(name) : - attributeType.GetProperty(name, type, Type.EmptyTypes)) ?? + RuntimeMethodInfo? setMethod = null; + RuntimeMethodInfo? getMethod = null; + RuntimePropertyInfo? property = null; + Type? baseAttributeType = attributeType; + + while (setMethod is null && baseAttributeType is not null + && (property is null || getMethod is null || getMethod.IsVirtual)) + { + property = (RuntimePropertyInfo?)(type is null ? + baseAttributeType.GetProperty(name) : + baseAttributeType.GetProperty(name, type, Type.EmptyTypes)); + + if (property is not null) + { + // Public properties may have non-public setter methods + setMethod = property.GetSetMethod(true)!; + getMethod = property.GetGetMethod(true)!; + } + else + { + setMethod = null; + getMethod = null; + } + + baseAttributeType = attributeType.BaseType; + } + + if (property is null) + { throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)); - RuntimeMethodInfo setMethod = property.GetSetMethod(true)!; + } - // Public properties may have non-public setter methods - if (!setMethod.IsPublic) + if (setMethod is null || !setMethod.IsPublic) { continue; } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs index 5adab84fcc60e4..057285ab5b58ac 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs @@ -437,4 +437,41 @@ public void MyMethod3(int x, int y, double z) { } public void mymethod4(string x) { } } } + public static class InheritGetterAndSetterMethodInProperty + { + [Fact] + public static void GetterAndSetterAreAvailableOnSubTypeWhenOverridingOneButInheritingBothFromBaseClass() + { + object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); + object attribute = Assert.Single(attributes); + var derivedAttributeWithGetterAttr = Assert.IsType(attribute); + Assert.Equal(2, derivedAttributeWithGetterAttr.P); + } + + public class BaseAttributeWithGetterSetter : Attribute + { + protected int _p; + + public virtual int P + { + get => _p; + set + { + _p = value; + } + } + } + + public class DerivedAttributeWithGetter : BaseAttributeWithGetterSetter + { + public override int P + { + get => _p; + } + } + + [DerivedAttributeWithGetter(P = 2)] + public class ClassWithDerivedAttr + { } + } } From 93ed7389c5ec2b459ff97d12116bea4dd6100fb0 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Mon, 14 Jul 2025 23:41:53 +0200 Subject: [PATCH 02/13] fix build issues --- .../Reflection/RuntimeCustomAttributeData.cs | 2 +- src/mono/mono/metadata/custom-attrs.c | 28 ++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index cbaca4ea3e2e0d..137279e829fab2 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1597,7 +1597,7 @@ private static void AddCustomAttributes( getMethod = null; } - baseAttributeType = attributeType.BaseType; + baseAttributeType = baseAttributeType.BaseType; } if (property is null) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 8bb0fb414a235d..ef433975def858 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -1245,21 +1245,41 @@ create_custom_attr (MonoImage *image, MonoMethod *method, const guchar *data, gu mono_field_set_value_internal (MONO_HANDLE_RAW (attr), field, val); // FIXMEcoop } else if (named_type == CATTR_TYPE_PROPERTY) { - MonoProperty *prop; - prop = mono_class_get_property_from_name_internal (mono_handle_class (attr), name); + MonoProperty *prop = NULL; + MonoMethod *setMethod = NULL; + MonoMethod *getMethod = NULL; + MonoClass *attrBaseClass = mono_handle_class (attr); + while (!setMethod && attrBaseClass && (!prop || !getMethod || m_method_is_virtual(getMethod))) + { + prop = mono_class_get_property_from_name_internal (attrBaseClass, name); + + if (prop) + { + setMethod = prop->set; + getMethod = prop->get; + } + else + { + setMethod = NULL; + getMethod = NULL; + } + + attrBaseClass = m_class_get_parent(attrBaseClass); + } + if (!prop) { mono_error_set_generic_error (error, "System.Reflection", "CustomAttributeFormatException", "Could not find a property with name %s", name); goto fail; } - if (!prop->set) { + if (!setMethod) { mono_error_set_generic_error (error, "System.Reflection", "CustomAttributeFormatException", "Could not find the setter for %s", name); goto fail; } /* can we have more that 1 arg in a custom attr named property? */ prop_type = prop->get? mono_method_signature_internal (prop->get)->ret : - mono_method_signature_internal (prop->set)->params [mono_method_signature_internal (prop->set)->param_count - 1]; + mono_method_signature_internal (setMethod)->params [mono_method_signature_internal (setMethod)->param_count - 1]; MonoObject *param_obj; pparams [0] = load_cattr_value (image, prop_type, ¶m_obj, named, data_end, &named, error); From e6ae6c732e72aca3ce1f27c4388eaee7f9d284c9 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Wed, 16 Jul 2025 00:09:30 +0200 Subject: [PATCH 03/13] add nativeaot support --- .../Extensions/NonPortable/CustomAttributeInstantiator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs index 99b697d56c264a..4b9bec92631faf 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs @@ -107,7 +107,7 @@ public static Attribute Instantiate(this CustomAttributeData cad) for (; ; ) { PropertyInfo? propertyInfo = walk.GetProperty(name, BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly); - if (propertyInfo != null) + if (propertyInfo?.SetMethod is not null) { propertyInfo.SetValue(newAttribute, argumentValue); break; From 3e6ef1d067b346c3e8fbfe47079d35a456856a1b Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 14:57:09 +0200 Subject: [PATCH 04/13] normalize conditions and move and rename test --- .../CustomAttributeInstantiator.cs | 24 ++++++++++-- .../System.Runtime.Tests/System/Attributes.cs | 35 ++++++++++++++++++ .../Reflection/TypeTests.Get.CornerCases.cs | 37 ------------------- 3 files changed, 55 insertions(+), 41 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs index 4b9bec92631faf..ce17eb136b4ee4 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs @@ -104,16 +104,32 @@ public static Attribute Instantiate(this CustomAttributeData cad) else { // Property + MethodInfo? getMethod = null; + MethodInfo? setMethod = null; + for (; ; ) { PropertyInfo? propertyInfo = walk.GetProperty(name, BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly); - if (propertyInfo?.SetMethod is not null) + + if (propertyInfo is not null) { - propertyInfo.SetValue(newAttribute, argumentValue); - break; + getMethod = propertyInfo.GetGetMethod(true); + setMethod = propertyInfo.GetSetMethod(true); + + if (setMethod is not null) + { + propertyInfo.SetValue(newAttribute, argumentValue); + break; + } } + else + { + getMethod = null; + setMethod = null; + } + Type? baseType = walk.BaseType; - if (baseType == null) + if (baseType == null || (getMethod is not null && !getMethod.IsVirtual)) throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)); walk = baseType; } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs index 3784d52cfcaa0b..986e027e0e3e7b 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs @@ -316,6 +316,15 @@ public static void GetCustomAttributesGenericAttributeHasValues() () => Assert.Equal(4, attribute.OptionalValue)); } + [Fact] + public static void GetCustomAttributesWithSettersDefinedOnBaseClass() + { + object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); + object attribute = Assert.Single(attributes); + var derivedAttributeWithGetterAttr = Assert.IsType(attribute); + Assert.Equal(2, derivedAttributeWithGetterAttr.P); + } + private static void GenericAttributesTestHelper(Func getCustomAttributes) { Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>)); @@ -326,6 +335,32 @@ private static void GenericAttributesTestHelper(Func Assert.NotEmpty(closedGenericAttributes), () => Assert.All(closedGenericAttributes, a => Assert.IsType>(a))); } + + public class BaseAttributeWithGetterSetter : Attribute + { + protected int _p; + + public virtual int P + { + get => _p; + set + { + _p = value; + } + } + } + + public class DerivedAttributeWithGetter : BaseAttributeWithGetterSetter + { + public override int P + { + get => _p; + } + } + + [DerivedAttributeWithGetter(P = 2)] + public class ClassWithDerivedAttr + { } } public static class GetCustomAttribute diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs index 057285ab5b58ac..5adab84fcc60e4 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/TypeTests.Get.CornerCases.cs @@ -437,41 +437,4 @@ public void MyMethod3(int x, int y, double z) { } public void mymethod4(string x) { } } } - public static class InheritGetterAndSetterMethodInProperty - { - [Fact] - public static void GetterAndSetterAreAvailableOnSubTypeWhenOverridingOneButInheritingBothFromBaseClass() - { - object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); - object attribute = Assert.Single(attributes); - var derivedAttributeWithGetterAttr = Assert.IsType(attribute); - Assert.Equal(2, derivedAttributeWithGetterAttr.P); - } - - public class BaseAttributeWithGetterSetter : Attribute - { - protected int _p; - - public virtual int P - { - get => _p; - set - { - _p = value; - } - } - } - - public class DerivedAttributeWithGetter : BaseAttributeWithGetterSetter - { - public override int P - { - get => _p; - } - } - - [DerivedAttributeWithGetter(P = 2)] - public class ClassWithDerivedAttr - { } - } } From da46642253ba6c9c25c46eb891a2465b5829aa31 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 15:17:36 +0200 Subject: [PATCH 05/13] fix logic of null prop --- .../src/System/Reflection/RuntimeCustomAttributeData.cs | 4 +++- src/mono/mono/metadata/custom-attrs.c | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index 137279e829fab2..678dd3a81ada98 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1576,6 +1576,7 @@ private static void AddCustomAttributes( RuntimeMethodInfo? setMethod = null; RuntimeMethodInfo? getMethod = null; RuntimePropertyInfo? property = null; + RuntimePropertyInfo? firstNotNullPropInHierarchy = null; Type? baseAttributeType = attributeType; while (setMethod is null && baseAttributeType is not null @@ -1587,6 +1588,7 @@ private static void AddCustomAttributes( if (property is not null) { + firstNotNullPropInHierarchy ??= property; // Public properties may have non-public setter methods setMethod = property.GetSetMethod(true)!; getMethod = property.GetGetMethod(true)!; @@ -1600,7 +1602,7 @@ private static void AddCustomAttributes( baseAttributeType = baseAttributeType.BaseType; } - if (property is null) + if (firstNotNullPropInHierarchy is null) { throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)); } diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index ef433975def858..3d18d5f633ec6b 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -1246,6 +1246,7 @@ create_custom_attr (MonoImage *image, MonoMethod *method, const guchar *data, gu mono_field_set_value_internal (MONO_HANDLE_RAW (attr), field, val); // FIXMEcoop } else if (named_type == CATTR_TYPE_PROPERTY) { MonoProperty *prop = NULL; + MonoProperty *firstNotNullPropInHierarchy = NULL; MonoMethod *setMethod = NULL; MonoMethod *getMethod = NULL; MonoClass *attrBaseClass = mono_handle_class (attr); @@ -1255,6 +1256,11 @@ create_custom_attr (MonoImage *image, MonoMethod *method, const guchar *data, gu if (prop) { + if (!firstNotNullPropInHierarchy) + { + firstNotNullPropInHierarchy = prop; + } + setMethod = prop->set; getMethod = prop->get; } @@ -1267,7 +1273,7 @@ create_custom_attr (MonoImage *image, MonoMethod *method, const guchar *data, gu attrBaseClass = m_class_get_parent(attrBaseClass); } - if (!prop) { + if (!firstNotNullPropInHierarchy) { mono_error_set_generic_error (error, "System.Reflection", "CustomAttributeFormatException", "Could not find a property with name %s", name); goto fail; } From 615986a5e52b9c8d27b88037f25bb26fa282d101 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 15:59:17 +0200 Subject: [PATCH 06/13] fix linker issues --- .../tests/System.Runtime.Tests/System/Attributes.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs index 986e027e0e3e7b..dd80abca8ce01d 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs @@ -17,6 +17,7 @@ using System.Reflection; using System.Linq; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Reflection.Emit; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -316,6 +317,8 @@ public static void GetCustomAttributesGenericAttributeHasValues() () => Assert.Equal(4, attribute.OptionalValue)); } + [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(DerivedAttributeWithGetter))] + [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(ClassWithDerivedAttr))] [Fact] public static void GetCustomAttributesWithSettersDefinedOnBaseClass() { From 729ef60a4c09b8140af3c9de30657146266bc2d6 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 16:41:50 +0200 Subject: [PATCH 07/13] fix linker issues --- .../System.Runtime.Tests/System/Attributes.cs | 3 -- .../src/linker/Linker.Steps/MarkStep.cs | 43 ++++++++++++++++--- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs index dd80abca8ce01d..986e027e0e3e7b 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs @@ -17,7 +17,6 @@ using System.Reflection; using System.Linq; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Reflection.Emit; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -317,8 +316,6 @@ public static void GetCustomAttributesGenericAttributeHasValues() () => Assert.Equal(4, attribute.OptionalValue)); } - [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(DerivedAttributeWithGetter))] - [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(ClassWithDerivedAttr))] [Fact] public static void GetCustomAttributesWithSettersDefinedOnBaseClass() { diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 5feafeb12b3149..09ec63ae08b822 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1317,16 +1317,45 @@ protected void MarkCustomAttributeProperties(ICustomAttribute ca, TypeDefinition protected void MarkCustomAttributeProperty(CustomAttributeNamedArgument namedArgument, TypeDefinition attribute, ICustomAttribute ca, in DependencyInfo reason, MessageOrigin origin) { - PropertyDefinition? property = GetProperty(attribute, namedArgument.Name); - if (property != null) - MarkMethod(property.SetMethod, reason, origin); + TypeDefinition? type = attribute; + MethodDefinition? setMethod = null; + MethodDefinition? getMethod = null; + PropertyDefinition? property = null; - MarkCustomAttributeArgument(namedArgument.Argument, ca, origin); + while (type is not null && (getMethod is null || getMethod.IsVirtual)) + { + property = type.Properties.FirstOrDefault(p => p.Name == namedArgument.Name); + + if (property is not null) + { + setMethod = property.SetMethod; + getMethod = property.GetMethod; + + if (property.SetMethod is not null) + { + setMethod = property.SetMethod; + break; + } + } + else + { + setMethod = null; + getMethod = null; + } + + type = Context.TryResolve(type.BaseType); + } - if (property != null && Annotations.FlowAnnotations.RequiresDataFlowAnalysis(property.SetMethod)) + if (setMethod is not null) { - var scanner = new AttributeDataFlow(Context, this, origin); - scanner.ProcessAttributeDataflow(property.SetMethod, new List { namedArgument.Argument }); + MarkMethod(setMethod, reason, origin); + MarkCustomAttributeArgument(namedArgument.Argument, ca, origin); + + if (Annotations.FlowAnnotations.RequiresDataFlowAnalysis(setMethod)) + { + var scanner = new AttributeDataFlow(Context, this, origin); + scanner.ProcessAttributeDataflow(setMethod, new List { namedArgument.Argument }); + } } } From 51f01ad6ec60c5b2293388ce92f652a9080b8c82 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 16:43:27 +0200 Subject: [PATCH 08/13] refacto linker --- .../tools/aot/ILCompiler/repro/Program.cs | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs index 9e71394c3732a5..5fee4118551fbc 100644 --- a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs @@ -2,11 +2,51 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics.CodeAnalysis; class Program { + [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(DerivedAttributeWithGetter))] + [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(ClassWithDerivedAttr))] static void Main() { - Console.WriteLine("Hello world"); + DerivedAttributeWithGetter derivedAttributeWithGetter = new DerivedAttributeWithGetter(); + Console.WriteLine("Hello world " + derivedAttributeWithGetter.P); + object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); + + if (attributes.Length > 0 && attributes[0] is DerivedAttributeWithGetter attr) + { + Console.WriteLine($"attribute found with value {attr.P}"); + } + else + { + Console.WriteLine("No Attributes I guess" + derivedAttributeWithGetter.P); + } + } +} + +public class BaseAttributeWithGetterSetter : Attribute +{ + protected int _p; + + public virtual int P + { + get => _p; + set + { + _p = value; + } } } + +public class DerivedAttributeWithGetter : BaseAttributeWithGetterSetter +{ + public override int P + { + get => _p; + } +} + +[DerivedAttributeWithGetter(P = 2)] +public class ClassWithDerivedAttr +{ } From a6730f3ff216590a2304db368c1c8987a8d23f45 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 16:44:24 +0200 Subject: [PATCH 09/13] Revert "refacto linker" This reverts commit 51f01ad6ec60c5b2293388ce92f652a9080b8c82. --- .../tools/aot/ILCompiler/repro/Program.cs | 42 +------------------ 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs index 5fee4118551fbc..9e71394c3732a5 100644 --- a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs @@ -2,51 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Diagnostics.CodeAnalysis; class Program { - [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(DerivedAttributeWithGetter))] - [method: DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(ClassWithDerivedAttr))] static void Main() { - DerivedAttributeWithGetter derivedAttributeWithGetter = new DerivedAttributeWithGetter(); - Console.WriteLine("Hello world " + derivedAttributeWithGetter.P); - object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); - - if (attributes.Length > 0 && attributes[0] is DerivedAttributeWithGetter attr) - { - Console.WriteLine($"attribute found with value {attr.P}"); - } - else - { - Console.WriteLine("No Attributes I guess" + derivedAttributeWithGetter.P); - } - } -} - -public class BaseAttributeWithGetterSetter : Attribute -{ - protected int _p; - - public virtual int P - { - get => _p; - set - { - _p = value; - } + Console.WriteLine("Hello world"); } } - -public class DerivedAttributeWithGetter : BaseAttributeWithGetterSetter -{ - public override int P - { - get => _p; - } -} - -[DerivedAttributeWithGetter(P = 2)] -public class ClassWithDerivedAttr -{ } From 4b1c4904a5c28c7ad2ef390766154d4574847f42 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 16:44:48 +0200 Subject: [PATCH 10/13] little refacto linkler --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 09ec63ae08b822..4adba5f136ea60 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1331,9 +1331,8 @@ protected void MarkCustomAttributeProperty(CustomAttributeNamedArgument namedArg setMethod = property.SetMethod; getMethod = property.GetMethod; - if (property.SetMethod is not null) + if (setMethod is not null) { - setMethod = property.SetMethod; break; } } From 11c77ac55aa99ab7c68372c5184585d3c2324ccd Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 20 Jul 2025 16:48:26 +0200 Subject: [PATCH 11/13] second little refacto linker --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 4adba5f136ea60..baf2c74f64e0cd 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1322,7 +1322,7 @@ protected void MarkCustomAttributeProperty(CustomAttributeNamedArgument namedArg MethodDefinition? getMethod = null; PropertyDefinition? property = null; - while (type is not null && (getMethod is null || getMethod.IsVirtual)) + while (setMethod is null && type is not null && (property is null || getMethod is null || getMethod.IsVirtual)) { property = type.Properties.FirstOrDefault(p => p.Name == namedArgument.Name); @@ -1330,11 +1330,6 @@ protected void MarkCustomAttributeProperty(CustomAttributeNamedArgument namedArg { setMethod = property.SetMethod; getMethod = property.GetMethod; - - if (setMethod is not null) - { - break; - } } else { From b26b940b2f1846ca2eb9516e0cdc6ce8712e0c67 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Fri, 5 Sep 2025 19:37:40 +0200 Subject: [PATCH 12/13] fix remarks --- .../src/System/Reflection/RuntimeCustomAttributeData.cs | 7 ++++--- .../CustomAttributeBasedDependencyAlgorithm.cs | 8 +++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index 0b9e49c9a3762e..91f4f1c25e8c3f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1576,9 +1576,10 @@ private static void AddCustomAttributes( RuntimeMethodInfo? setMethod = null; RuntimeMethodInfo? getMethod = null; RuntimePropertyInfo? property = null; - RuntimePropertyInfo? firstNotNullPropInHierarchy = null; + RuntimePropertyInfo? firstPropInHierarchy = null; Type? baseAttributeType = attributeType; + // If the method getter is not virtual, the property of the current class (in hierarchy) has hidden its base-property instead of overriding it. So there is no need to continue traversal while (setMethod is null && baseAttributeType is not null && (property is null || getMethod is null || getMethod.IsVirtual)) { @@ -1588,7 +1589,7 @@ private static void AddCustomAttributes( if (property is not null) { - firstNotNullPropInHierarchy ??= property; + firstPropInHierarchy ??= property; // Public properties may have non-public setter methods setMethod = property.GetSetMethod(true)!; getMethod = property.GetGetMethod(true)!; @@ -1602,7 +1603,7 @@ private static void AddCustomAttributes( baseAttributeType = baseAttributeType.BaseType; } - if (firstNotNullPropInHierarchy is null) + if (firstPropInHierarchy is null) { throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs index a83ad7506418b8..40b2bdcc0b9bf7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CustomAttributeBasedDependencyAlgorithm.cs @@ -240,6 +240,7 @@ private static bool AddDependenciesFromPropertySetter(DependencyList dependencie MetadataReader reader = attributeTypeDefinition.MetadataReader; var typeDefinition = reader.GetTypeDefinition(attributeTypeDefinition.Handle); + MethodDesc getterMethod = null; foreach (PropertyDefinitionHandle propDefHandle in typeDefinition.GetProperties()) { @@ -263,6 +264,11 @@ private static bool AddDependenciesFromPropertySetter(DependencyList dependencie dependencies.Add(factory.ReflectedMethod(setterMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Custom attribute blob"); } + if (!accessors.Getter.IsNil) + { + getterMethod = (MethodDesc)attributeTypeDefinition.EcmaModule.GetObject(accessors.Getter); + } + return true; } } @@ -270,7 +276,7 @@ private static bool AddDependenciesFromPropertySetter(DependencyList dependencie // Haven't found it in current type. Check the base type. TypeDesc baseType = attributeType.BaseType; - if (baseType != null) + if (baseType != null && (getterMethod is null || getterMethod.IsVirtual)) return AddDependenciesFromPropertySetter(dependencies, factory, baseType, propertyName); // Not found. This is bad metadata that will result in a runtime failure, but we shouldn't fail the compilation. From ed7e7ddd188352d49260975d9b41e12501377ac4 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 6 Sep 2025 11:35:48 +0200 Subject: [PATCH 13/13] fix remarks --- .../Reflection/RuntimeCustomAttributeData.cs | 43 ++++++++----------- .../CustomAttributeInstantiator.cs | 7 ++- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index 91f4f1c25e8c3f..a75cea46857553 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1575,24 +1575,29 @@ private static void AddCustomAttributes( RuntimeMethodInfo? setMethod = null; RuntimeMethodInfo? getMethod = null; - RuntimePropertyInfo? property = null; - RuntimePropertyInfo? firstPropInHierarchy = null; - Type? baseAttributeType = attributeType; + Type baseAttributeType = attributeType; - // If the method getter is not virtual, the property of the current class (in hierarchy) has hidden its base-property instead of overriding it. So there is no need to continue traversal - while (setMethod is null && baseAttributeType is not null - && (property is null || getMethod is null || getMethod.IsVirtual)) + for (; ; ) { - property = (RuntimePropertyInfo?)(type is null ? + RuntimePropertyInfo? property = (RuntimePropertyInfo?)(type is null ? baseAttributeType.GetProperty(name) : baseAttributeType.GetProperty(name, type, [])); if (property is not null) { - firstPropInHierarchy ??= property; - // Public properties may have non-public setter methods - setMethod = property.GetSetMethod(true)!; - getMethod = property.GetGetMethod(true)!; + setMethod = property.GetSetMethod(true); + getMethod = property.GetGetMethod(true); + + if (setMethod is not null) + { + // Public properties may have non-public setter methods + if (setMethod.IsPublic) + { + setMethod.InvokePropertySetter(attribute, BindingFlags.Default, null, value, null); + } + + break; + } } else { @@ -1600,20 +1605,10 @@ private static void AddCustomAttributes( getMethod = null; } - baseAttributeType = baseAttributeType.BaseType; + baseAttributeType = baseAttributeType.BaseType is null || (getMethod is not null && !getMethod.IsVirtual) + ? throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)) + : baseAttributeType.BaseType; } - - if (firstPropInHierarchy is null) - { - throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)); - } - - if (setMethod is null || !setMethod.IsPublic) - { - continue; - } - - setMethod.InvokePropertySetter(attribute, BindingFlags.Default, null, value, null); } else { diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs index ce17eb136b4ee4..4dce71374eab94 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs @@ -118,7 +118,12 @@ public static Attribute Instantiate(this CustomAttributeData cad) if (setMethod is not null) { - propertyInfo.SetValue(newAttribute, argumentValue); + // Public properties may have non-public setter methods + if (setMethod.IsPublic) + { + propertyInfo.SetValue(newAttribute, argumentValue); + } + break; } }