Skip to content

System.Reflection.CustomAttributeFormatException thrown on a retrieval of derived attribute with overridden property getter #110945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fd853a3
System.Reflection.CustomAttributeFormatException thrown on a retrieva…
pedrobsaila Dec 25, 2024
e7eab06
fix remarks
pedrobsaila Dec 25, 2024
6ba02b9
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Dec 25, 2024
662ff24
fix remarks 1
pedrobsaila Dec 26, 2024
f907659
fix remark 2
pedrobsaila Dec 28, 2024
2ee4af2
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Dec 28, 2024
4be79cf
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Dec 29, 2024
f97cd57
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Dec 30, 2024
8fb59c2
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Dec 31, 2024
6ccc4cd
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Jan 3, 2025
12c5997
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Jan 12, 2025
961c626
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Feb 2, 2025
5465a7a
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Feb 9, 2025
51d2f19
port changes to mono
pedrobsaila Feb 9, 2025
6b11170
fix remark
pedrobsaila Feb 9, 2025
d96f4fe
fix remark
pedrobsaila Feb 9, 2025
0cd6b9e
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Mar 16, 2025
3a676b3
fix failing tests in nativeaot
pedrobsaila Mar 23, 2025
928c8b2
Merge remote-tracking branch 'upstream/main' into 110412
pedrobsaila Mar 23, 2025
78ef41d
fix StackOverflowException
pedrobsaila Mar 23, 2025
dc55844
add diagnostics
pedrobsaila Mar 23, 2025
8331b12
rethrow exception
pedrobsaila Mar 23, 2025
7634736
use context to log messages
pedrobsaila Mar 23, 2025
621a011
log directly in exception
pedrobsaila Mar 23, 2025
9b78063
read setter from base class in ilink
pedrobsaila Mar 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.IsPublic)
if (setMethod is null || !setMethod.IsPublic)
{
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,63 @@ 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 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;

while ((m_getterMethod is null || m_setterMethod is 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 is null)
{
Associates.AssignAssociates(baseScope, baseTkProperty, baseDeclaredRuntimeType, reflectedRuntimeType,
out _, out _, out _,
out _, out m_setterMethod, out _,
out _, out _);

if (m_setterMethod is not null)
{
break;
}
}
else
{
Associates.AssignAssociates(baseScope, baseTkProperty, baseDeclaredRuntimeType, reflectedRuntimeType,
out _, out _, out _,
out m_getterMethod, out _, out _,
out _, out _);

if (m_getterMethod is not null)
{
break;
}
}
}
}

baseDeclaredType = baseDeclaredType.BaseType;
}
}

GC.KeepAlive(module);
}
#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -240,9 +241,42 @@ private RuntimeNamedMethodInfo Getter
{
getter = GetPropertyMethod(PropertyMethodSemantics.Getter);

if (getter == null)
getter = RuntimeDummyMethodInfo.Instance;
if (getter is null)
{
RuntimeNamedMethodInfo setter = GetPropertyMethod(PropertyMethodSemantics.Setter);

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;
}

Expand All @@ -255,13 +289,46 @@ 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)
{
RuntimeNamedMethodInfo getter = GetPropertyMethod(PropertyMethodSemantics.Getter);

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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{ }
}
}
35 changes: 35 additions & 0 deletions src/mono/mono/metadata/class-init.c
Original file line number Diff line number Diff line change
Expand Up @@ -3737,6 +3737,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);
}
}
}
}

Expand Down
26 changes: 19 additions & 7 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,15 +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)
MarkMethod (property.SetMethod, reason, origin);
TypeDefinition? type = attribute;
MethodDefinition? method = null;
while (type is not null) {
PropertyDefinition? property = type.Properties.FirstOrDefault (p => p.Name == namedArgument.Name);

MarkCustomAttributeArgument (namedArgument.Argument, ca, origin);
if (property?.SetMethod is not null) {
method = property.SetMethod;
break;
}

if (property != null && Annotations.FlowAnnotations.RequiresDataFlowAnalysis (property.SetMethod)) {
var scanner = new AttributeDataFlow (Context, this, origin);
scanner.ProcessAttributeDataflow (property.SetMethod, new List<CustomAttributeArgument> { namedArgument.Argument });
type = Context.TryResolve (type.BaseType);
}

if (method is not null) {
MarkMethod (method, reason, origin);
MarkCustomAttributeArgument (namedArgument.Argument, ca, origin);

if (Annotations.FlowAnnotations.RequiresDataFlowAnalysis (method)) {
var scanner = new AttributeDataFlow (Context, this, origin);
scanner.ProcessAttributeDataflow (method, new List<CustomAttributeArgument> { namedArgument.Argument });
}
}
}

Expand Down
Loading