Skip to content

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

Draft
pedrobsaila wants to merge 22 commits intodotnet:mainfrom
pedrobsaila:110412-bis
Draft

System.Reflection.CustomAttributeFormatException thrown on a retrieval of derived attribute with overridden property getter#117584
pedrobsaila wants to merge 22 commits intodotnet:mainfrom
pedrobsaila:110412-bis

Conversation

@pedrobsaila
Copy link
Copy Markdown
Contributor

@pedrobsaila pedrobsaila commented Jul 13, 2025

Fixes #110412
Fixes #119490

Sorry for the delay, I've been busy these last weeks

…l of derived attribute with overridden property getter
@pedrobsaila pedrobsaila requested a review from marek-safar as a code owner July 20, 2025 14:41
@dotnet-policy-service dotnet-policy-service Bot added the linkable-framework Issues associated with delivering a linker friendly framework label Jul 20, 2025
@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

@pedrobsaila I've made some edits on a fork of your branch here. @jkotas helped bring me up to speed on the concerns that @MichalStrehovsky was expressing. I hadn't fully appreciated the issues that this represented. My branch has the following changes:

Please let me know if you're willing to fold in the changes I've made or handle them yourself. Appreciate your work here and want to ensure you are acknowledged for your effort.

@AaronRobinsonMSFT AaronRobinsonMSFT added the needs-author-action An issue or pull request that requires more info or actions from the author. label Mar 2, 2026
@pedrobsaila
Copy link
Copy Markdown
Contributor Author

Hey @AaronRobinsonMSFT yes you can fold in your changes, no worries about that.

@dotnet-policy-service dotnet-policy-service Bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Mar 3, 2026
@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

Hey @AaronRobinsonMSFT yes you can fold in your changes, no worries about that.

@pedrobsaila I don't think I can push to your branch. I can create a PR from my branch or you can cherry-pick my changes, up to you.

Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs Outdated
Comment thread src/mono/mono/metadata/custom-attrs.c
@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

/ba-g Known issues.

Issues fixed in #125051

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

@MichalStrehovsky Please take another look at this. I've narrowed the scenario considerably.

Comment on lines +331 to +333
Type expectedExceptionType = PlatformDetection.IsNativeAot
? typeof(AmbiguousMatchException)
: typeof(CustomAttributeFormatException);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the difference in exception type cause by the:

                                RuntimePropertyInfo? property = (RuntimePropertyInfo?)(type is null ?
                                    baseAttributeType.GetProperty(name) :
                                    baseAttributeType.GetProperty(name, type, []));

on the JIT side and

                        PropertyInfo? propertyInfo = walk.GetProperty(name, BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);

on the AOT side?

Could this be reconciled by adding type information to the GetProperty on the native AOT side too?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying it, worst case we revert it and accept the difference. Best case we update the test because the behavior will now be same.

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

AaronRobinsonMSFT commented Mar 9, 2026

@MichalStrehovsky It looks like there is now a new failure for native AOT. Thoughts? I can also revert the changes and we can live with it for now.

[FAIL] System.Reflection.Tests.CustomAttributeTests.BoxedEnumAttributes
System.Reflection.CustomAttributeFormatException : 'B1' property specified was not found.
   at Internal.Reflection.Extensions.NonPortable.CustomAttributeInstantiator.Instantiate(CustomAttributeData) in /_/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs:line 131
   at System.Reflection.CustomAttributeExtensions.GetCustomAttribute[T](MemberInfo) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/CustomAttributeExtensions.cs:line 38
   at System.Reflection.Tests.CustomAttributeTests.BoxedEnumAttributes() in /_/src/libraries/System.Runtime/tests/System.Reflection.Tests/CustomAttributeTests.cs:line 261
   at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) in /_/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs:line 230
[SKIP] System.Reflection.Tests.ParameterInfoTests.GetCustomAttributesDataOnParameterWithNullMetadataTokenReturnsEmptyList

@MichalStrehovsky
Copy link
Copy Markdown
Member

@MichalStrehovsky It looks like there is now a new failure for native AOT. Thoughts? I can also revert the chances and we can live with it for now.

Do you have any thoughts on failure modes of these with CoreCLR:

using System;

RunWithLog(() => typeof(C1).GetCustomAttributes(false));
RunWithLog(() => typeof(C2).GetCustomAttributes(false));
RunWithLog(() => typeof(C3).GetCustomAttributes(false));

static void RunWithLog(Action a)
{
    try
    {
        a();
    }
    catch (Exception ex)
    {
        Console.WriteLine("*******************************************");
        Console.WriteLine(ex.Message);
        Console.WriteLine(ex.StackTrace);
    }
}


public class BaseAttributeWithTypeProp : Attribute
{
    public virtual object Prop
    {
        get;
        set;
    }
}

public class DerivedAttributeWithObjectProp : BaseAttributeWithTypeProp
{
    public override Type Prop
    {
        get => (Type)base.Prop;
    }
}

[DerivedAttributeWithObjectProp(Prop = typeof(object))]
class C1;

[DerivedAttributeWithObjectProp(Prop = "Hello")]
class C2;

[DerivedAttributeWithObjectProp(Prop = (AttributeTargets)0)]
class C3;

On .NET 10 this will print:

*******************************************
'Prop' property specified was not found.
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at Program.<>c.<<Main>$>b__0_0() in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 3
   at Program.<<Main>$>g__RunWithLog|0_3(Action a) in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 11
*******************************************
Could not resolve type 'Hello' in assembly 'ConsoleApp1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.
   at System.Reflection.TypeNameResolver.GetTypeFromDefaultAssemblies(String typeName, ReadOnlySpan`1 nestedTypeNames, TypeName parsedName)
   at System.Reflection.TypeNameResolver.GetType(String escapedTypeName, ReadOnlySpan`1 nestedTypeNames, TypeName parsedName)
   at System.Reflection.TypeNameResolver.GetSimpleType(TypeName typeName)
   at System.Reflection.TypeNameResolver.Resolve(TypeName typeName)
   at System.Reflection.TypeNameResolver.GetTypeHelper(ReadOnlySpan`1 typeName, RuntimeAssembly requestingAssembly, Boolean throwOnError, Boolean requireAssemblyQualifiedName, IntPtr unsafeAccessorMethod)
   at System.Reflection.TypeNameResolver.GetTypeHelper(Char* pTypeName, RuntimeAssembly requestingAssembly, Boolean throwOnError, Boolean requireAssemblyQualifiedName, IntPtr unsafeAccessorMethod)
   at System.Reflection.CustomAttribute.GetPropertyOrFieldData(RuntimeModule module, IntPtr& blobStart, IntPtr blobEnd, String& name, Boolean& isProperty, RuntimeType& type, Object& value)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at Program.<>c.<<Main>$>b__0_1() in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 4
   at Program.<<Main>$>g__RunWithLog|0_3(Action a) in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 11
*******************************************
Binary format of the specified custom attribute was invalid.
   at System.Reflection.CustomAttribute.GetPropertyOrFieldData(RuntimeModule module, IntPtr& blobStart, IntPtr blobEnd, String& name, Boolean& isProperty, RuntimeType& type, Object& value)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at Program.<>c.<<Main>$>b__0_2() in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 5
   at Program.<<Main>$>g__RunWithLog|0_3(Action a) in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 11

I assume the first one might behave differently with this fix, but I don't believe the rest will be affected.

This might be a game that is better not to play. We should ask Roslyn to block this at the compiler level. Then whether we have a sensible failure mode or behaviors is a different question. We certainly won't be writing C# tests for this anymore.

@MichalStrehovsky
Copy link
Copy Markdown
Member

Another thing that occurred to me to test:

dynamic dd = new DerivedAttributeWithObjectProp();
dd.Prop = null;

public class BaseAttributeWithTypeProp : Attribute
{
    public virtual object Prop
    {
        get;
        set;
    }
}

public class DerivedAttributeWithObjectProp : BaseAttributeWithTypeProp
{
    public override Type Prop
    {
        get => (Type)base.Prop;
    }
}

This will throw Microsoft.CSharp.RuntimeBinder.RuntimeBinderException: 'Property or indexer 'DerivedAttributeWithObjectProp.Prop' cannot be assigned to -- it is read only'

Replacing the dynamic with var will make it work.

Then if you replace the null with "Hello" (one of the examples that do work with custom attributes in my previous comment) it will no longer compile because now C# will require this to be a Type.

I think we're running into a bunch of undefined corner cases on the C# language side. What the reflection side does is a separate question, but I don't think we even know what property the blob is supposed to refer to when C# makes it, and C# might not be sure either.

@AaronRobinsonMSFT AaronRobinsonMSFT self-requested a review April 28, 2026 17:36
@steveisok
Copy link
Copy Markdown
Member

@pedrobsaila I appreciate the contribution. My read from Aaron's and Michal's comments is that this may take a while. I'm going to move this to draft. Please feel free to move to ready when the time comes.

@steveisok steveisok marked this pull request as draft May 4, 2026 19:12
@MichalStrehovsky
Copy link
Copy Markdown
Member

I asked GPT-5.5 to explain what we're seeing. Roslyn has a very unique idea of what property the blob is setting, the algorithm is different from the default C# language overload resolution. Replicating it in reflection doesn't seem exactly trivial:

Copilot Chat Conversation Export: Roslyn attribute argument handling explained

User: @MichalStrehovsky
Thread URL: https://github.com/copilot/c/3fd3c235-0126-4ab8-a0ae-8e849d9e51ba

@MichalStrehovsky

using System;

var dd = new DerivedAttributeWithObjectProp();

// Below line doesn't compile (error CS0029: Cannot implicitly convert type 'string' to 'System.Type')
//dd.Prop = "Hello";

// Below line compiles just fine
[DerivedAttributeWithObjectProp(Prop = "Hello")]
public class BaseAttributeWithTypeProp : Attribute
{
    public virtual object Prop {  get; set; }
}

public class DerivedAttributeWithObjectProp : BaseAttributeWithTypeProp
{
    public override Type Prop => (Type)base.Prop;
}

Look at the two comments in this snippet. Why does one work and the other doesn't work. You have source code for the Roslyn C# compiler available here.

Copilot

The attribute form works because Roslyn gives named attribute arguments special handling for overridden properties: it walks back to the “least overridden” property and uses that property’s type/accessors for validation. In your example, that means it effectively validates Prop = "Hello" against the base object Prop { get; set; }, not against the overriding Type Prop => ....

The normal assignment does not use that special attribute rule. For dd.Prop = "Hello", the property visible on DerivedAttributeWithObjectProp has type System.Type, so Roslyn tries to convert "Hello" to System.Type and reports CS0029.

Key Roslyn code is in Binder_Attributes.cs:

// SPEC:    For each named-argument Arg in named-argument-list N:
// SPEC:        Let Name be the identifier of the named-argument Arg.
// SPEC:        Name must identify a non-static read-write public field or property on 
// SPEC:            attribute class T. If T has no such field or property, then a compile-time error occurs.

bool invalidNamedArgument = false;
TypeSymbol? namedArgumentType = null;
invalidNamedArgument |= (namedArgumentNameSymbol.DeclaredAccessibility != Accessibility.Public);
invalidNamedArgument |= namedArgumentNameSymbol.IsStatic;

if (!invalidNamedArgument)
{
    switch (namedArgumentNameSymbol.Kind)
    {
        case SymbolKind.Field:
            var fieldSymbol = (FieldSymbol)namedArgumentNameSymbol;
            namedArgumentType = fieldSymbol.Type;
            invalidNamedArgument |= fieldSymbol.IsReadOnly;
            invalidNamedArgument |= fieldSymbol.IsConst;
            break;

        case SymbolKind.Property:
            var propertySymbol = ((PropertySymbol)namedArgumentNameSymbol).GetLeastOverriddenProperty(this.ContainingType);
            namedArgumentType = propertySymbol.Type;
            invalidNamedArgument |= propertySymbol.IsReadOnly;
            var getMethod = propertySymbol.GetMethod;
            var setMethod = propertySymbol.SetMethod;
            invalidNamedArgument = invalidNamedArgument || (object)getMethod == null || (object)setMethod == null;
            if (!invalidNamedArgument)
            {
                invalidNamedArgument =
                    getMethod!.DeclaredAccessibility != Accessibility.Public ||
                    setMethod!.DeclaredAccessibility != Accessibility.Public;
            }
            break;

The important line is:

var propertySymbol = ((PropertySymbol)namedArgumentNameSymbol).GetLeastOverriddenProperty(this.ContainingType);
namedArgumentType = propertySymbol.Type;

GetLeastOverriddenProperty walks from an override down to the accessible base property:

internal PropertySymbol GetLeastOverriddenProperty(NamedTypeSymbol accessingTypeOpt)
{
    accessingTypeOpt = accessingTypeOpt?.OriginalDefinition;
    PropertySymbol p = this;
    while (p.IsOverride && !p.HidesBasePropertiesByName)
    {
        // We might not be able to access the overridden method. For example,
        // 
        //   .assembly A
        //   {
        //      InternalsVisibleTo("B")
        //      public class A { internal virtual int P { get; } }
        //   }
        // 
        //   .assembly B
        //   {
        //      InternalsVisibleTo("C")
        //      public class B : A { internal override int P { get; } }
        //   }
        // 
        //   .assembly C
        //   {
        //      public class C : B { ... new B().P ... }       // A.P is not accessible from here
        //   }
        //
        // See InternalsVisibleToAndStrongNameTests: IvtVirtualCall1, IvtVirtualCall2, IvtVirtual_ParamsAndDynamic.
        PropertySymbol overridden = p.OverriddenProperty;
        var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
        if ((object)overridden == null ||
            (accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo)))
        {
            break;
        }

        p = overridden;
    }

    return p;
}

So for the attribute:

[DerivedAttributeWithObjectProp(Prop = "Hello")]

Roslyn:

  1. Looks up Prop on DerivedAttributeWithObjectProp.
  2. Sees it is an override.
  3. Calls GetLeastOverriddenProperty(...).
  4. Gets BaseAttributeWithTypeProp.Prop.
  5. Uses its type: object.
  6. Converts "Hello" to object, which is valid.

For the normal assignment:

dd.Prop = "Hello";

there is no attribute-specific “least overridden property” rule. The member access is bound as the overriding property whose type is System.Type, so the compiler checks whether string converts to System.Type. It does not, hence:

CS0029: Cannot implicitly convert type 'string' to 'System.Type'

I used GitHub code search while looking this up; search results are limited and may be incomplete. You can view more matches in GitHub UI here: https://github.com/search?q=repo%3Adotnet%2Froslyn+GetLeastOverriddenProperty+Binder_Attributes.cs&type=code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Reflection community-contribution Indicates that the PR has been added by a community member linkable-framework Issues associated with delivering a linker friendly framework

Projects

Status: No status

5 participants