Mono ignores non-public custom attributes in dynamic assemblies

[elinor-fung]

Description

When building a dynamic assembly via reflection with the various *Builder classes, non-public attributes are explicitly ignored in mono.

In mono_custom_attrs_from_builders_handle:

runtime/src/mono/mono/metadata/custom-attrs.c

Lines 738 to 741 in 6122bba

	for (int i = 0; i < count; ++i) {
	MONO_HANDLE_ARRAY_GETREF (cattr, cattrs, i);
	if (!custom_attr_visible (image, cattr, ctor_handle, &ctor_method))
	continue;

There's a comment about it matching .NET, but that is not what I see with coreclr or .NET Framework.

runtime/src/mono/mono/metadata/custom-attrs.c

Line 723 in 6122bba

/* Skip nonpublic attributes since MS.NET seems to do the same */

Not sure if there's another reason mono skips non-public attributes?

Reproduction Steps

Dynamic assembly with an assembly-level non-public attribute:

AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("DynamicAssembly"), AssemblyBuilderAccess.Run);
ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("DynamicModule");

// Create internal attribute
TypeBuilder typeBuilder = moduleBuilder.DefineType("InternalAttribute", TypeAttributes.NotPublic, typeof(Attribute));
ConstructorBuilder constructorBuilder = typeBuilder.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, new Type[0]);
constructorBuilder.GetILGenerator().Emit(OpCodes.Ret);

// Add assembly-level attribute
ConstructorInfo constructorInfo = typeBuilder.CreateTypeInfo().GetConstructor(Array.Empty<Type>());
CustomAttributeBuilder customAttributeBuilder = new CustomAttributeBuilder(constructorInfo, Array.Empty<object>());
assemblyBuilder.SetCustomAttribute(customAttributeBuilder);

IList<CustomAttributeData> attrs = assemblyBuilder.GetCustomAttributesData();
Console.WriteLine($"{assemblyBuilder.GetName().Name} - Attributes (count: {attrs.Count})");
foreach (CustomAttributeData attr in attrs)
{
    Console.WriteLine($"  {attr}");
}

Expected behavior

The internal attribute is listed. On coreclr (and also .NET Framework 4.6.2), the above prints:

DynamicAssembly - Attributes (count: 1)
  [InternalAttribute()]

Actual behavior

The internal attribute is not listed. On mono, the above prints:

DynamicAssembly - Attributes (count: 0)

Other information

This results in the test infrastructure for Roslyn analyzers (in dotnet/roslyn-sdk) being unable to run on mono.

https://github.com/dotnet/roslyn-sdk/blob/27fe3d6060150031b532cc7255259c40a37a5fda/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerInfo.cs

They rely on emitting a non-public IgnoresAccessChecksToAttribute to skip access checks. Since mono ends up ignoring the attribute, using their test infrastructure results in:

System.MethodAccessException : Method `Microsoft.CodeAnalysis.Testing.AnalyzerInfo+CustomAnalysisContext..ctor()' is inaccessible from method `CustomAnalysisContextImpl..ctor()'
    Stack Trace:
        at CustomAnalysisContextImpl..ctor()

cc @lambdageek

[ghost]

Tagging subscribers to this area:
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

When building a dynamic assembly via reflection with the various *Builder classes, non-public attributes are explicitly ignored in mono.

In mono_custom_attrs_from_builders_handle:

runtime/src/mono/mono/metadata/custom-attrs.c

Lines 738 to 741 in 6122bba

	for (int i = 0; i < count; ++i) {
	MONO_HANDLE_ARRAY_GETREF (cattr, cattrs, i);
	if (!custom_attr_visible (image, cattr, ctor_handle, &ctor_method))
	continue;

There's a comment about it matching .NET, but that is not what I see with coreclr or .NET Framework.

runtime/src/mono/mono/metadata/custom-attrs.c

Line 723 in 6122bba

/* Skip nonpublic attributes since MS.NET seems to do the same */

Reproduction Steps

Dynamic assembly with an assembly-level non-public attribute:

AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("DynamicAssembly"), AssemblyBuilderAccess.Run);
ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("DynamicModule");

// Create internal attribute
TypeBuilder typeBuilder = moduleBuilder.DefineType("InternalAttribute", TypeAttributes.NotPublic, typeof(Attribute));
ConstructorBuilder constructorBuilder = typeBuilder.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, new Type[0]);
constructorBuilder.GetILGenerator().Emit(OpCodes.Ret);

// Add assembly-level attribute
ConstructorInfo constructorInfo = typeBuilder.CreateTypeInfo().GetConstructor(Array.Empty<Type>());
CustomAttributeBuilder customAttributeBuilder = new CustomAttributeBuilder(constructorInfo, Array.Empty<object>());
assemblyBuilder.SetCustomAttribute(customAttributeBuilder);

IList<CustomAttributeData> attrs = assemblyBuilder.GetCustomAttributesData();
Console.WriteLine($"{assemblyBuilder.GetName().Name} - Attributes (count: {attrs.Count})");
foreach (CustomAttributeData attr in attrs)
{
    Console.WriteLine($"  {attr}");
}

Expected behavior

The internal attribute is listed. On coreclr (and also .NET Framework 4.6.2), the above prints:

DynamicAssembly - Attributes (count: 1)
  [InternalAttribute()]

Actual behavior

The internal attribute is not listed. On mono, the above prints:

DynamicAssembly - Attributes (count: 0)

Regression?

No response

Known Workarounds

No response

Configuration

This results in the test infrastructure for Roslyn analyzers (in dotnet/roslyn-sdk) being unable to run on mono.

https://github.com/dotnet/roslyn-sdk/blob/27fe3d6060150031b532cc7255259c40a37a5fda/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerInfo.cs

They rely on emitting a non-public IgnoresAccessChecksToAttribute to skip access checks. Since mono ends up ignoring the attribute, using their test infrastructure results in:

System.MethodAccessException : Method `Microsoft.CodeAnalysis.Testing.AnalyzerInfo+CustomAnalysisContext..ctor()' is inaccessible from method `CustomAnalysisContextImpl..ctor()'
    Stack Trace:
        at CustomAnalysisContextImpl..ctor()

Other information

No response

Author:	elinor-fung
Assignees:	-
Labels:	area-System.Reflection-mono
Milestone:	-

[lambdageek]

Not sure if there's another reason mono skips non-public attributes?

I doubt it.

That comment is from 2005 from mono/mono@6c9333e Perhaps it's matching a bug from some old version of .NET Framework.

[marek-safar]

Targetting net7 to enable tests to run to possibly discover other issues

[ivanpovazan]

@lambdageek this seems to be limited by a call to the local function custom_attr_visible here:

runtime/src/mono/mono/metadata/custom-attrs.c

Line 951 in 9f8f653

count_visible += custom_attr_visible (image, cattr, ctor_handle, &ctor_method);

as it covers only public attribute visibility:

runtime/src/mono/mono/metadata/custom-attrs.c

Lines 107 to 108 in 9f8f653

	if ((visibility != TYPE_ATTRIBUTE_PUBLIC) && (visibility != TYPE_ATTRIBUTE_NESTED_PUBLIC))
	return FALSE;

do you see any major implications if we would just try to loosen the limitation?

[lambdageek]

@ivanpovazan I think you're onto something with custom_attr_visible. I think what might be wrong is that we expect this check to be true in this case (the attributeInternalAttribute is defined in the assembly builder and is applied in the same assembly builder - so this should be true):

runtime/src/mono/mono/metadata/custom-attrs.c

Line 105 in 18c6495

if (m_class_get_image (klass) != image) {

But note that actually the attribute is defined in the moduleBuilder, but it's applied in the assemblyBuilder. Due to the way SRE works in .NET, those are actually two different MonoImages. so this comparison is actually false. For assmebly-level attributes we probably need to also compare m_class_get_image (klass)->assembly->image to image - I think that will end up pointing to the assemblyBuilder related to the moduleBuilder.

[tmds]

@lambdageek do you expect this to be fixed on the short term (or for .NET 8)? This issue also affects the aspnetcore tests, so we're looking into skipping the tests there too (dotnet/aspnetcore#48710).

cc @Sapana-Khemkar @omajid

[ivanpovazan]

@tmds we are currently looking into this and will try to get the fix upstreamed by preview 6 snap

[ivanpovazan]

Just an update: the fix will most probably not be ready for preview 6 snap, but we actively working on resolving the issue.

EDIT : Aiming to resolve the issue for .NET8