Skip to content
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

Cannot reflect on generic attributes whose constructors original definition uses type parameters #56492

Closed
RikkiGibson opened this issue Jul 28, 2021 · 9 comments · Fixed by #56879
Assignees
Milestone

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Jul 28, 2021

Related to dotnet/roslyn#55190

The following program throws CustomAttributeFormatException (using the generic-attributes feature compiler):

using System;
using System.Reflection;

class Attr<T> : Attribute { public Attr(T param) { } }

[Attr<string>("a")]
class Program
{
    static void Main()
    {
        // below line throws 'System.Reflection.CustomAttributeFormatException : Binary format of the specified custom attribute was invalid.'
        var attrs = CustomAttributeData.GetCustomAttributes(typeof(Program));
        Console.Write(attrs);
    }
}

Importantly, if we change the constructor parameter type to string, we don't get an exception. It's also worth noting that if we declare a property of a type parameter type, then specify a value for it at the usage of the attribute, we decode it just fine.

The compiler produces this IL for the type Program:

.class private auto ansi beforefieldinit Program
	extends [netstandard]System.Object
{
	.custom instance void class Attr`1<string>::.ctor(!0) = (
		01 00 01 61 00 00
	)
	// Methods
	.method private hidebysig static 
		void Main () cil managed 
	{
		// Method begins at RVA 0x2058
		// Code size 21 (0x15)
		.maxstack 8
		IL_0000: ldtoken Program
		IL_0005: call class [netstandard]System.Type [netstandard]System.Type::GetTypeFromHandle(valuetype [netstandard]System.RuntimeTypeHandle)
		IL_000a: call class [netstandard]System.Collections.Generic.IList`1<class [netstandard]System.Reflection.CustomAttributeData> [netstandard]System.Reflection.CustomAttributeData::GetCustomAttributes(class [netstandard]System.Reflection.MemberInfo)
		IL_000f: call void [netstandard]System.Console::Write(object)
		IL_0014: ret
	} // end of method Program::Main
	.method public hidebysig specialname rtspecialname 
		instance void .ctor () cil managed 
	{
		// Method begins at RVA 0x206e
		// Code size 7 (0x7)
		.maxstack 8
		IL_0000: ldarg.0
		IL_0001: call instance void [netstandard]System.Object::.ctor()
		IL_0006: ret
	} // end of method Program::.ctor
} // end of class Program

§II.21 Custom attributes in ECMA 335 reads:

Custom attributes are declared using the directive .custom, followed by the method declaration for a type constructor, optionally followed by a Bytes in parentheses:

CustomDecl ::=
Ctor [ '=' '(' Bytes ')' ]

The Ctor item represents a method declaration §II.15.4, specific for the case where the method's name is .ctor.

It seems like for the feature "generic attributes" to work properly it is necessary for the Ctor item to represent a method reference rather than a method declaration, because the method in question could be contained in a constructed type. However, I am not sure I comprehend the finer points of the spec here.

Actually, I am not sure if what I am observing is a compiler bug, or a coreclr bug, or a spec issue. Poking around the compiler's emit of the attribute constructor, it does seem to be simply producing a reference to the constructed method symbol.

cc @jkotas @AviAvni

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Reflection untriaged New issue has not been triaged by the area owner labels Jul 28, 2021
@davidwrighton
Copy link
Member

This looks like a bug in coreclr (and possibly mono). Could you provide a test written in IL to cover the scenario? The snippet above doesn't include the Attr type. Also, please identify if this C# feature is expected to be enabled in the .NET 6 timeframe, or if this is a post-.NET 6 request.

@AviAvni
Copy link
Contributor

AviAvni commented Jul 29, 2021

This bug was fixed in dotnet/coreclr#25054 an there is test for it

@davidwrighton
Copy link
Member

@RikkiGibson please verify that your issue still reproduces on some version of .NET 5 or 6. The change @AviAvni references should have fixed it in those versions.

@RikkiGibson
Copy link
Contributor Author

RikkiGibson commented Jul 29, 2021

I ran the test again and verified that the value of RuntimeInformation.FrameworkDescription is .NET 6.0.0-preview.6.21352.12. It still seems to have this behavior.

I also dumped the IL. Unfortunately the compiler I'm using is built from source, otherwise it would be a bit easier to give you a source reproducer:

IL dump
//  Microsoft (R) .NET Framework IL Disassembler.  Version 4.8.3928.0
//  Copyright (c) Microsoft Corporation.  All rights reserved.



// Metadata version: v4.0.30319
.assembly extern System.Runtime
{
  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )                         // .?_....:
  .ver 6:0:0:0
}
.assembly extern System.Console
{
  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )                         // .?_....:
  .ver 6:0:0:0
}
.assembly 'generic-attributes-test'
{
  .custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 ) 
  .custom instance void [System.Runtime]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78   // ....T..WrapNonEx
                                                                                                                   63 65 70 74 69 6F 6E 54 68 72 6F 77 73 01 )       // ceptionThrows.

  // --- The following custom attribute is added automatically, do not uncomment -------
  //  .custom instance void [System.Runtime]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.DebuggableAttribute/DebuggingModes) = ( 01 00 07 01 00 00 00 00 ) 

  .custom instance void [System.Runtime]System.Runtime.Versioning.TargetFrameworkAttribute::.ctor(string) = ( 01 00 18 2E 4E 45 54 43 6F 72 65 41 70 70 2C 56   // ....NETCoreApp,V
                                                                                                              65 72 73 69 6F 6E 3D 76 36 2E 30 01 00 54 0E 14   // ersion=v6.0..T..
                                                                                                              46 72 61 6D 65 77 6F 72 6B 44 69 73 70 6C 61 79   // FrameworkDisplay
                                                                                                              4E 61 6D 65 00 )                                  // Name.
  .custom instance void [System.Runtime]System.Reflection.AssemblyCompanyAttribute::.ctor(string) = ( 01 00 17 67 65 6E 65 72 69 63 2D 61 74 74 72 69   // ...generic-attri
                                                                                                      62 75 74 65 73 2D 74 65 73 74 00 00 )             // butes-test..
  .custom instance void [System.Runtime]System.Reflection.AssemblyConfigurationAttribute::.ctor(string) = ( 01 00 05 44 65 62 75 67 00 00 )                   // ...Debug..
  .custom instance void [System.Runtime]System.Reflection.AssemblyFileVersionAttribute::.ctor(string) = ( 01 00 07 31 2E 30 2E 30 2E 30 00 00 )             // ...1.0.0.0..
  .custom instance void [System.Runtime]System.Reflection.AssemblyInformationalVersionAttribute::.ctor(string) = ( 01 00 05 31 2E 30 2E 30 00 00 )                   // ...1.0.0..
  .custom instance void [System.Runtime]System.Reflection.AssemblyProductAttribute::.ctor(string) = ( 01 00 17 67 65 6E 65 72 69 63 2D 61 74 74 72 69   // ...generic-attri
                                                                                                      62 75 74 65 73 2D 74 65 73 74 00 00 )             // butes-test..
  .custom instance void [System.Runtime]System.Reflection.AssemblyTitleAttribute::.ctor(string) = ( 01 00 17 67 65 6E 65 72 69 63 2D 61 74 74 72 69   // ...generic-attri
                                                                                                    62 75 74 65 73 2D 74 65 73 74 00 00 )             // butes-test..
  .hash algorithm 0x00008004
  .ver 1:0:0:0
}
.module 'generic-attributes-test.dll'
// MVID: {A48B278B-DB74-42DA-8787-A1254356E44C}
.imagebase 0x00400000
.file alignment 0x00000200
.stackreserve 0x00100000
.subsystem 0x0003       // WINDOWS_CUI
.corflags 0x00000001    //  ILONLY
// Image base: 0x066C0000


// =============== CLASS MEMBERS DECLARATION ===================

.class private auto ansi beforefieldinit Attr`1<T>
       extends [System.Runtime]System.Attribute
{
  .method public hidebysig specialname rtspecialname 
          instance void  .ctor(!T param) cil managed
  {
    // Code size       9 (0x9)
    .maxstack  8
    IL_0000:  ldarg.0
    IL_0001:  call       instance void [System.Runtime]System.Attribute::.ctor()
    IL_0006:  nop
    IL_0007:  nop
    IL_0008:  ret
  } // end of method Attr`1::.ctor

} // end of class Attr`1

.class private auto ansi beforefieldinit Program
       extends [System.Runtime]System.Object
{
  .custom instance void class Attr`1<string>::.ctor(!0) = ( 01 00 01 61 00 00 )                               // ...a..
  .method private hidebysig static void  Main() cil managed
  {
    .entrypoint
    // Code size       25 (0x19)
    .maxstack  1
    .locals init (class [System.Runtime]System.Collections.Generic.IList`1<class [System.Runtime]System.Reflection.CustomAttributeData> V_0)
    IL_0000:  nop
    IL_0001:  ldtoken    Program
    IL_0006:  call       class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
    IL_000b:  call       class [System.Runtime]System.Collections.Generic.IList`1<class [System.Runtime]System.Reflection.CustomAttributeData> [System.Runtime]System.Reflection.CustomAttributeData::GetCustomAttributes(class [System.Runtime]System.Reflection.MemberInfo)
    IL_0010:  stloc.0
    IL_0011:  ldloc.0
    IL_0012:  call       void [System.Console]System.Console::Write(object)
    IL_0017:  nop
    IL_0018:  ret
  } // end of method Program::Main

  .method public hidebysig specialname rtspecialname 
          instance void  .ctor() cil managed
  {
    // Code size       8 (0x8)
    .maxstack  8
    IL_0000:  ldarg.0
    IL_0001:  call       instance void [System.Runtime]System.Object::.ctor()
    IL_0006:  nop
    IL_0007:  ret
  } // end of method Program::.ctor

} // end of class Program


// =============================================================

// *********** DISASSEMBLY COMPLETE ***********************
// WARNING: Created Win32 resource file C:\Users\rikki\Documents\generic-attributes.res

In case it helps I am also attaching a zip file with the project. You won't be able to build it because it depends on a local compiler, but you should be able to run it with e.g. dotnet run --no-build or just dotnet on the dll in the bin folder.

generic-attributes-test.zip

@RikkiGibson
Copy link
Contributor Author

please identify if this C# feature is expected to be enabled in the .NET 6 timeframe, or if this is a post-.NET 6 request.

We would like to ship the feature in the .NET 6 timeframe if possible.

@RikkiGibson
Copy link
Contributor Author

@davidwrighton Please let me know if there's anything I can do to help drive this.

@davidwrighton davidwrighton self-assigned this Aug 4, 2021
@davidwrighton
Copy link
Member

@RikkiGibson, this is on my radar, and I expect to get it fixed before RC1. I'm currently looking at some stress issues, and then I'll take a look at this one.

@davidwrighton davidwrighton added this to the 6.0.0 milestone Aug 4, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 5, 2021
@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Aug 5, 2021
davidwrighton added a commit that referenced this issue Aug 5, 2021
* Fix CustomAttributeData in the presence of generic attributes
- Generic attributes need to force the actual exact method to be loaded not just the canonical scenario
- GenericAttribute testing had been disabled due to dotnet/msbuild#6734
- Move GenericAttribute test project to Pri0 as its the only generic custom attribute runtime testing that will occur before .NET 6.0 ships
- Test disabled on Mono as Mono currently doesn't support this feature.

Fixes #56492
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 5, 2021
@RikkiGibson
Copy link
Contributor Author

Thank you so much for the fix @davidwrighton!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants