From fd853a39c4a27d8559bc9ffb1c4d373adc7d2d59 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Wed, 25 Dec 2024 18:03:22 +0100 Subject: [PATCH 01/14] System.Reflection.CustomAttributeFormatException thrown on a retrieval of derived attribute with overridden property getter --- .../Reflection/RuntimeCustomAttributeData.cs | 2 +- .../System/Reflection/RuntimePropertyInfo.cs | 56 ++++++++++++++++++- .../TypeBuilderSetCustomAttribute.cs | 53 ++++++++++++++++++ 3 files changed, 109 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 8a34a6996d14fe..4e170f99158296 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1580,7 +1580,7 @@ private static void AddCustomAttributes( RuntimeMethodInfo setMethod = property.GetSetMethod(true)!; // Public properties may have non-public setter methods - if (!setMethod.IsPublic) + if (setMethod == null || !setMethod.IsPublic) { continue; } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs index 51b1f84864b1a9..972458bc17900f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs @@ -44,10 +44,64 @@ internal RuntimePropertyInfo( scope.GetPropertyProps(tkProperty, out m_utf8name, out m_flags, out _); - Associates.AssignAssociates(scope, tkProperty, declaredType, reflectedTypeCache.GetRuntimeType(), + RuntimeType reflectedRuntimeType = reflectedTypeCache.GetRuntimeType(); + Associates.AssignAssociates(scope, tkProperty, declaredType, reflectedRuntimeType, out _, out _, out _, out m_getterMethod, out m_setterMethod, out m_otherMethod, out isPrivate, out m_bindingFlags); + + // lookup getter/setter when getter and setter are inherited from base class but just a setter/getter is overridden on a sub type + if ((m_getterMethod != null && m_getterMethod.IsVirtual && m_setterMethod == null) + || (m_setterMethod != null && m_setterMethod.IsVirtual && m_getterMethod == null)) + { + Type? baseDeclaredType = declaredType.BaseType; + + while ((m_getterMethod == null || m_setterMethod == null) + && baseDeclaredType != null && baseDeclaredType is RuntimeType baseDeclaredRuntimeType) + { + RuntimeModule baseModule = baseDeclaredRuntimeType.GetRuntimeModule(); + MetadataImport baseScope = baseModule.MetadataImport; + + baseScope.EnumProperties(RuntimeTypeHandle.GetToken(baseDeclaredRuntimeType), out MetadataEnumResult baseTkProperties); + + for (int i = 0; i < baseTkProperties.Length; i++) + { + int baseTkProperty = baseTkProperties[i]; + MdUtf8String baseTkPropertyName = baseScope.GetName(baseTkProperty); + + if (Name.Equals(baseTkPropertyName.ToString())) + { + if (m_setterMethod == null) + { + Associates.AssignAssociates(baseScope, baseTkProperty, baseDeclaredRuntimeType, reflectedRuntimeType, + out _, out _, out _, + out _, out m_setterMethod, out _, + out _, out _); + + if (m_setterMethod != null) + { + break; + } + } + else + { + Associates.AssignAssociates(baseScope, baseTkProperty, baseDeclaredRuntimeType, reflectedRuntimeType, + out _, out _, out _, + out m_getterMethod, out _, out _, + out _, out _); + + if (m_getterMethod != null) + { + break; + } + } + } + } + + baseDeclaredType = baseDeclaredType.BaseType; + } + } + GC.KeepAlive(module); } #endregion diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs index 1d905a51f41a45..28ce72998062fa 100644 --- a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs +++ b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs @@ -52,6 +52,26 @@ public void SetCustomAttribute() Assert.Equal("hello", ((TypeBuilderStringAttribute)attributes[0]).Creator); } + [Fact] + public void GetterAndSetterAreAvailableOnSubTypeWhenOverridingOneButInheritingBothFromBaseClass() + { + // checks that a setter in inherited + var pProperty = typeof(DerivedAttributeWithGetter).GetProperty("P", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy); + Assert.NotNull(pProperty); + Assert.NotNull(pProperty.SetMethod); + + // check that reflected base setter works as intended when invoked + object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); + Assert.Equal(2, attributes.Length); + Assert.Contains(attributes, + a => a is DerivedAttributeWithGetter derivedAttributeWithGetterAttr && derivedAttributeWithGetterAttr.P == 2); + + // checks that a getter in inherited + pProperty = typeof(DerivedAttributeWithSetter).GetProperty("P", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy); + Assert.NotNull(pProperty); + Assert.NotNull(pProperty.GetMethod); + } + public class TypeBuilderStringAttribute : Attribute { private string _creator; @@ -74,5 +94,38 @@ public TypeBuilderIntAttribute(int mc) public int m_ctorType2; } + 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; + } + } + + public class DerivedAttributeWithSetter : BaseAttributeWithGetterSetter + { + public override int P + { + set => _p = value; + } + } + + [DerivedAttributeWithGetter(P = 2), DerivedAttributeWithSetter(P = 2)] + public class ClassWithDerivedAttr + { } } } From e7eab062fc57d1237b0bf4485e780a838e67f38e Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Wed, 25 Dec 2024 21:54:11 +0100 Subject: [PATCH 02/14] fix remarks --- .../src/System/Reflection/RuntimeCustomAttributeData.cs | 4 ++-- 1 file changed, 2 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 4e170f99158296..b11bcee44d6c54 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1577,10 +1577,10 @@ private static void AddCustomAttributes( attributeType.GetProperty(name) : attributeType.GetProperty(name, type, Type.EmptyTypes)) ?? throw new CustomAttributeFormatException(SR.Format(SR.RFLCT_InvalidPropFail, name)); - RuntimeMethodInfo setMethod = property.GetSetMethod(true)!; + RuntimeMethodInfo? setMethod = property.GetSetMethod(true); // Public properties may have non-public setter methods - if (setMethod == null || !setMethod.IsPublic) + if (setMethod is null || !setMethod.IsPublic) { continue; } From 662ff245fe9ac6037bad09ecbea9df640e8d1591 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Thu, 26 Dec 2024 23:03:58 +0100 Subject: [PATCH 03/14] fix remarks 1 --- .../System/Reflection/RuntimePropertyInfo.cs | 3 +- .../TypeBuilderSetCustomAttribute.cs | 53 ----------------- .../Reflection/TypeTests.Get.CornerCases.cs | 57 +++++++++++++++++++ 3 files changed, 58 insertions(+), 55 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs index 972458bc17900f..b9af047667b8ca 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs @@ -56,8 +56,7 @@ internal RuntimePropertyInfo( { Type? baseDeclaredType = declaredType.BaseType; - while ((m_getterMethod == null || m_setterMethod == null) - && baseDeclaredType != null && baseDeclaredType is RuntimeType baseDeclaredRuntimeType) + while ((m_getterMethod == null || m_setterMethod == null) && baseDeclaredType is RuntimeType baseDeclaredRuntimeType) { RuntimeModule baseModule = baseDeclaredRuntimeType.GetRuntimeModule(); MetadataImport baseScope = baseModule.MetadataImport; diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs index 28ce72998062fa..1d905a51f41a45 100644 --- a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs +++ b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetCustomAttribute.cs @@ -52,26 +52,6 @@ public void SetCustomAttribute() Assert.Equal("hello", ((TypeBuilderStringAttribute)attributes[0]).Creator); } - [Fact] - public void GetterAndSetterAreAvailableOnSubTypeWhenOverridingOneButInheritingBothFromBaseClass() - { - // checks that a setter in inherited - var pProperty = typeof(DerivedAttributeWithGetter).GetProperty("P", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy); - Assert.NotNull(pProperty); - Assert.NotNull(pProperty.SetMethod); - - // check that reflected base setter works as intended when invoked - object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); - Assert.Equal(2, attributes.Length); - Assert.Contains(attributes, - a => a is DerivedAttributeWithGetter derivedAttributeWithGetterAttr && derivedAttributeWithGetterAttr.P == 2); - - // checks that a getter in inherited - pProperty = typeof(DerivedAttributeWithSetter).GetProperty("P", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy); - Assert.NotNull(pProperty); - Assert.NotNull(pProperty.GetMethod); - } - public class TypeBuilderStringAttribute : Attribute { private string _creator; @@ -94,38 +74,5 @@ public TypeBuilderIntAttribute(int mc) public int m_ctorType2; } - 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; - } - } - - public class DerivedAttributeWithSetter : BaseAttributeWithGetterSetter - { - public override int P - { - set => _p = value; - } - } - - [DerivedAttributeWithGetter(P = 2), DerivedAttributeWithSetter(P = 2)] - public class ClassWithDerivedAttr - { } } } 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..3f851b428e371d 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,61 @@ public void MyMethod3(int x, int y, double z) { } public void mymethod4(string x) { } } } + + public static class InheritGetterAndSetterMethodInProperty + { + [Fact] + public static void GetterAndSetterAreAvailableOnSubTypeWhenOverridingOneButInheritingBothFromBaseClass() + { + // checks that a setter in inherited + var pProperty = typeof(DerivedAttributeWithGetter).GetProperty("P", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy); + Assert.NotNull(pProperty); + Assert.NotNull(pProperty.SetMethod); + + // check that reflected base setter works as intended when invoked + object[] attributes = typeof(ClassWithDerivedAttr).GetCustomAttributes(true); + Assert.Equal(2, attributes.Length); + Assert.Contains(attributes, + a => a is DerivedAttributeWithGetter derivedAttributeWithGetterAttr && derivedAttributeWithGetterAttr.P == 2); + + // checks that a getter in inherited + pProperty = typeof(DerivedAttributeWithSetter).GetProperty("P", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy); + Assert.NotNull(pProperty); + Assert.NotNull(pProperty.GetMethod); + } + + 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; + } + } + + public class DerivedAttributeWithSetter : BaseAttributeWithGetterSetter + { + public override int P + { + set => _p = value; + } + } + + [DerivedAttributeWithGetter(P = 2), DerivedAttributeWithSetter(P = 2)] + public class ClassWithDerivedAttr + { } + } } From f9076592b2adde1b10b9fb9db03de397ae63479d Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sat, 28 Dec 2024 13:29:32 +0100 Subject: [PATCH 04/14] fix remark 2 --- .../src/System/Reflection/RuntimePropertyInfo.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs index b9af047667b8ca..dd461a644e5d16 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs @@ -50,9 +50,9 @@ internal RuntimePropertyInfo( out m_getterMethod, out m_setterMethod, out m_otherMethod, out isPrivate, out m_bindingFlags); - // lookup getter/setter when getter and setter are inherited from base class but just a setter/getter is overridden on a sub type - if ((m_getterMethod != null && m_getterMethod.IsVirtual && m_setterMethod == null) - || (m_setterMethod != null && m_setterMethod.IsVirtual && m_getterMethod == null)) + // Lookup getter/setter when getter and setter are inherited from base class but just a setter/getter is overridden on a sub type. + if ((m_getterMethod is not null && m_getterMethod.IsVirtual && m_setterMethod is null) + || (m_setterMethod is not null && m_setterMethod.IsVirtual && m_getterMethod is null)) { Type? baseDeclaredType = declaredType.BaseType; @@ -77,7 +77,7 @@ internal RuntimePropertyInfo( out _, out m_setterMethod, out _, out _, out _); - if (m_setterMethod != null) + if (m_setterMethod is not null) { break; } @@ -89,7 +89,7 @@ internal RuntimePropertyInfo( out m_getterMethod, out _, out _, out _, out _); - if (m_getterMethod != null) + if (m_getterMethod is not null) { break; } From 51d2f19625144a721de675e69885e9fff519b37c Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 9 Feb 2025 15:22:04 +0100 Subject: [PATCH 05/14] port changes to mono --- src/mono/mono/metadata/class-init.c | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 6a43869672e572..7c41faa305506c 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -3734,6 +3734,41 @@ mono_class_setup_properties (MonoClass *klass) break; } } + + // Lookup getter/setter when getter and setter are inherited from base class but just a setter/getter is overridden on a sub type. + if ((properties [i - first].get != NULL && properties [i - first].set == NULL && m_method_is_virtual(properties [i - first].get)) + || (properties [i - first].set != NULL && properties [i - first].get == NULL && m_method_is_virtual(properties [i - first].set))) + { + MonoClass *base_type = m_class_get_parent(klass); + + while ((properties [i - first].set == NULL || properties [i - first].get == NULL) && base_type != NULL) + { + mono_class_setup_properties(base_type); + + MonoClassPropertyInfo *base_properties_bag = mono_class_get_property_info (base_type); + + for (guint32 j = 0; j < base_properties_bag->count; j++) + { + MonoProperty base_property_candidate = base_properties_bag->properties[j]; + + if (strcmp(base_property_candidate.name, properties [i - first].name) == 0) + { + if (base_property_candidate.set != NULL && properties [i - first].set == NULL) + { + properties [i - first].set = base_property_candidate.set; + } + else if (base_property_candidate.get != NULL && properties [i - first].get == NULL) + { + properties [i - first].get = base_property_candidate.get; + } + + break; + } + } + + base_type = m_class_get_parent(base_type); + } + } } } From 6b1117016946e93f3ae19dca8c6bc37581eaec36 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 9 Feb 2025 15:23:59 +0100 Subject: [PATCH 06/14] fix remark --- .../src/System/Reflection/RuntimePropertyInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs index dd461a644e5d16..2d9ad1f413dd15 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs @@ -56,7 +56,7 @@ internal RuntimePropertyInfo( { Type? baseDeclaredType = declaredType.BaseType; - while ((m_getterMethod == null || m_setterMethod == null) && baseDeclaredType is RuntimeType baseDeclaredRuntimeType) + while ((m_getterMethod is null || m_setterMethod is null) && baseDeclaredType is RuntimeType baseDeclaredRuntimeType) { RuntimeModule baseModule = baseDeclaredRuntimeType.GetRuntimeModule(); MetadataImport baseScope = baseModule.MetadataImport; From d96f4fedab5fde5e97d2916eb03fa3c7eb806c90 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 9 Feb 2025 15:24:51 +0100 Subject: [PATCH 07/14] fix remark --- .../src/System/Reflection/RuntimePropertyInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs index 2d9ad1f413dd15..9834752c7d0a2e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs @@ -70,7 +70,7 @@ internal RuntimePropertyInfo( if (Name.Equals(baseTkPropertyName.ToString())) { - if (m_setterMethod == null) + if (m_setterMethod is null) { Associates.AssignAssociates(baseScope, baseTkProperty, baseDeclaredRuntimeType, reflectedRuntimeType, out _, out _, out _, From 3a676b3cecdf15635d999dce9e950c7370898c55 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 09:26:26 +0100 Subject: [PATCH 08/14] fix failing tests in nativeaot --- .../BindingFlagSupport/PropertyPolicies.cs | 4 +- .../PropertyInfos/RuntimePropertyInfo.cs | 71 +++++++++++++++++-- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs index 28eb0627c2fe6d..b870e2f97801cd 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs @@ -86,7 +86,9 @@ public sealed override bool IsSuppressedByMoreDerivedMember(PropertyInfo member, public sealed override bool OkToIgnoreAmbiguity(PropertyInfo m1, PropertyInfo m2) { - return false; + return m1.PropertyType == m2.PropertyType + && ((m1.GetMethod is null && m2.GetMethod is null) || ((m1.GetMethod is not null && m2.GetMethod is not null) && DefaultBinder.CompareMethodSig(m1.GetMethod, m2.GetMethod))) + && ((m1.SetMethod is null && m2.SetMethod is null) || ((m1.SetMethod is not null && m2.SetMethod is not null) && DefaultBinder.CompareMethodSig(m1.SetMethod, m2.SetMethod))); } private static MethodInfo? GetAccessorMethod(PropertyInfo property) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs index 1665d354582f73..d666b007798572 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Globalization; using System.Reflection; +using System.Reflection.Runtime.BindingFlagSupport; using System.Reflection.Runtime.CustomAttributes; using System.Reflection.Runtime.General; using System.Reflection.Runtime.MethodInfos; @@ -240,9 +241,39 @@ private RuntimeNamedMethodInfo Getter { getter = GetPropertyMethod(PropertyMethodSemantics.Getter); - if (getter == null) - getter = RuntimeDummyMethodInfo.Instance; + if (getter is null) + { + if (Setter is not null && Setter.IsVirtual) + { + Type? baseType = DeclaringType.BaseType; + + while (getter is null && baseType is not null) + { + foreach (PropertyInfo basePropertyInfo in PropertyPolicies.Instance.GetDeclaredMembers(baseType)) + { + if (basePropertyInfo.Name == Name) + { + if (basePropertyInfo is RuntimePropertyInfo baseRuntimePropertyInfo) + { + getter = baseRuntimePropertyInfo.GetPropertyMethod(PropertyMethodSemantics.Getter); + break; + } + } + } + + if (getter is not null) + { + break; + } + else + { + baseType = baseType.BaseType; + } + } + } + } + getter ??= RuntimeDummyMethodInfo.Instance; _lazyGetter = getter; } @@ -255,13 +286,43 @@ private RuntimeNamedMethodInfo Setter get { RuntimeNamedMethodInfo setter = _lazySetter; - if (setter == null) + if (setter is null) { setter = GetPropertyMethod(PropertyMethodSemantics.Setter); - if (setter == null) - setter = RuntimeDummyMethodInfo.Instance; + if (setter is null) + { + if (Getter is not null && Getter.IsVirtual) + { + Type? baseType = DeclaringType.BaseType; + + while (setter is null && baseType is not null) + { + foreach (PropertyInfo basePropertyInfo in PropertyPolicies.Instance.GetDeclaredMembers(baseType)) + { + if (basePropertyInfo.Name == Name) + { + if (basePropertyInfo is RuntimePropertyInfo baseRuntimePropertyInfo) + { + setter = baseRuntimePropertyInfo.GetPropertyMethod(PropertyMethodSemantics.Setter); + break; + } + } + } + + if (setter is not null) + { + break; + } + else + { + baseType = baseType.BaseType; + } + } + } + } + setter ??= RuntimeDummyMethodInfo.Instance; _lazySetter = setter; } From 78ef41df95f3e14d85fd0377b8b13ecd1938db17 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 11:27:42 +0100 Subject: [PATCH 09/14] fix StackOverflowException --- .../Runtime/BindingFlagSupport/PropertyPolicies.cs | 4 +--- .../Runtime/PropertyInfos/RuntimePropertyInfo.cs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs index b870e2f97801cd..28eb0627c2fe6d 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/BindingFlagSupport/PropertyPolicies.cs @@ -86,9 +86,7 @@ public sealed override bool IsSuppressedByMoreDerivedMember(PropertyInfo member, public sealed override bool OkToIgnoreAmbiguity(PropertyInfo m1, PropertyInfo m2) { - return m1.PropertyType == m2.PropertyType - && ((m1.GetMethod is null && m2.GetMethod is null) || ((m1.GetMethod is not null && m2.GetMethod is not null) && DefaultBinder.CompareMethodSig(m1.GetMethod, m2.GetMethod))) - && ((m1.SetMethod is null && m2.SetMethod is null) || ((m1.SetMethod is not null && m2.SetMethod is not null) && DefaultBinder.CompareMethodSig(m1.SetMethod, m2.SetMethod))); + return false; } private static MethodInfo? GetAccessorMethod(PropertyInfo property) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs index d666b007798572..1b60110f37737c 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Runtime/PropertyInfos/RuntimePropertyInfo.cs @@ -243,7 +243,9 @@ private RuntimeNamedMethodInfo Getter if (getter is null) { - if (Setter is not null && Setter.IsVirtual) + RuntimeNamedMethodInfo setter = GetPropertyMethod(PropertyMethodSemantics.Setter); + + if (setter is not null && setter.IsVirtual) { Type? baseType = DeclaringType.BaseType; @@ -256,8 +258,9 @@ private RuntimeNamedMethodInfo Getter if (basePropertyInfo is RuntimePropertyInfo baseRuntimePropertyInfo) { getter = baseRuntimePropertyInfo.GetPropertyMethod(PropertyMethodSemantics.Getter); - break; } + + break; } } @@ -292,7 +295,9 @@ private RuntimeNamedMethodInfo Setter if (setter is null) { - if (Getter is not null && Getter.IsVirtual) + RuntimeNamedMethodInfo getter = GetPropertyMethod(PropertyMethodSemantics.Getter); + + if (getter is not null && getter.IsVirtual) { Type? baseType = DeclaringType.BaseType; @@ -305,8 +310,9 @@ private RuntimeNamedMethodInfo Setter if (basePropertyInfo is RuntimePropertyInfo baseRuntimePropertyInfo) { setter = baseRuntimePropertyInfo.GetPropertyMethod(PropertyMethodSemantics.Setter); - break; } + + break; } } From dc55844304ace14625dff6f34f3a701007babee6 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 13:21:57 +0100 Subject: [PATCH 10/14] add diagnostics --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 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 885f0039928ec7..44930bfcbb8aac 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1197,8 +1197,13 @@ protected void MarkCustomAttributeProperties (ICustomAttribute ca, TypeDefinitio 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); + if (property != null) { + try { + MarkMethod (property.SetMethod, reason, origin); + } catch { + Console.WriteLine ($"Exception occured when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}"); + } + } MarkCustomAttributeArgument (namedArgument.Argument, ca, origin); From 8331b1284f3d0a34c4e491639d31116fb282920c Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 13:34:59 +0100 Subject: [PATCH 11/14] rethrow exception --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 44930bfcbb8aac..fb5062b7a4d8e2 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1202,6 +1202,7 @@ protected void MarkCustomAttributeProperty (CustomAttributeNamedArgument namedAr MarkMethod (property.SetMethod, reason, origin); } catch { Console.WriteLine ($"Exception occured when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}"); + throw; } } From 7634736ab941d8711609d76dd36840b961cdf8f2 Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 14:05:46 +0100 Subject: [PATCH 12/14] use context to log messages --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index fb5062b7a4d8e2..831c90e8320308 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1201,7 +1201,7 @@ protected void MarkCustomAttributeProperty (CustomAttributeNamedArgument namedAr try { MarkMethod (property.SetMethod, reason, origin); } catch { - Console.WriteLine ($"Exception occured when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}"); + Context.LogMessage ($"Exception occurred when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}"); throw; } } From 621a0117440bc78bf2f0d0f02c139e169b113a3e Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 14:35:56 +0100 Subject: [PATCH 13/14] log directly in exception --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 831c90e8320308..7665bde602219f 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1200,9 +1200,8 @@ protected void MarkCustomAttributeProperty (CustomAttributeNamedArgument namedAr if (property != null) { try { MarkMethod (property.SetMethod, reason, origin); - } catch { - Context.LogMessage ($"Exception occurred when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}"); - throw; + } catch (Exception ex) { + throw new Exception ($"Exception occurred when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}", ex); } } From 9b7806334d257a04aa1cbf04cae5977ca5d7eaba Mon Sep 17 00:00:00 2001 From: pedrobsaila Date: Sun, 23 Mar 2025 17:40:48 +0100 Subject: [PATCH 14/14] read setter from base class in ilink --- .../src/linker/Linker.Steps/MarkStep.cs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 7665bde602219f..3e980ad95e35b5 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1196,20 +1196,27 @@ protected void MarkCustomAttributeProperties (ICustomAttribute ca, TypeDefinitio protected void MarkCustomAttributeProperty (CustomAttributeNamedArgument namedArgument, TypeDefinition attribute, ICustomAttribute ca, in DependencyInfo reason, MessageOrigin origin) { - PropertyDefinition? property = GetProperty (attribute, namedArgument.Name); - if (property != null) { - try { - MarkMethod (property.SetMethod, reason, origin); - } catch (Exception ex) { - throw new Exception ($"Exception occurred when reading property {property.FullName} of type {property.DeclaringType.FullName} with SetMethod {property.SetMethod}", ex); + TypeDefinition? type = attribute; + MethodDefinition? method = null; + while (type is not null) { + PropertyDefinition? property = type.Properties.FirstOrDefault (p => p.Name == namedArgument.Name); + + if (property?.SetMethod is not null) { + method = property.SetMethod; + break; } + + type = Context.TryResolve (type.BaseType); } - MarkCustomAttributeArgument (namedArgument.Argument, ca, origin); + if (method is not null) { + MarkMethod (method, reason, origin); + MarkCustomAttributeArgument (namedArgument.Argument, ca, origin); - if (property != null && Annotations.FlowAnnotations.RequiresDataFlowAnalysis (property.SetMethod)) { - var scanner = new AttributeDataFlow (Context, this, origin); - scanner.ProcessAttributeDataflow (property.SetMethod, new List { namedArgument.Argument }); + if (Annotations.FlowAnnotations.RequiresDataFlowAnalysis (method)) { + var scanner = new AttributeDataFlow (Context, this, origin); + scanner.ProcessAttributeDataflow (method, new List { namedArgument.Argument }); + } } }