Issue 66496

[madelson]

Restore the reverted PR (https://github.com/dotnet/runtime/pull/65237/files) but this time handle DynamicMethod
Fixes 66496

[ghost]

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Restore the reverted PR (https://github.com/dotnet/runtime/pull/65237/files) but this time handle DynamicMethod

Author:	madelson
Assignees:	-
Labels:	area-System.Reflection
Milestone:	-

[jkotas]

The new test is failing on Mono:

[02:46:22] fail: [FAIL] System.Tests.AttributeGetCustomAttributes.GetCustomAttributes_DynamicMethod
[02:46:22] info: Assert.Equal() Failure
[02:46:22] info: Expected: 1
[02:46:22] info: Actual:   0
[02:46:22] info:    at System.Tests.AttributeGetCustomAttributes.GetCustomAttributes_DynamicMethod()
[02:46:22] info:    at System.Reflection.RuntimeMethodInfo.InvokeWorker(Object obj, BindingFlags invokeAttr, Span`1 parameters)
[02:48:28] info: Finished:    System.Runtime.Tests.dll

[madelson]

The new test is failing on Mono:

@jkotas I spent some time looking into this and it seems to me that there is a bit of a web to untangle. TL;DR I set the test to skip on Mono.

First off, unlike with coreclr Mono's attempt to support MethodImplAttribute in DynamicMethod does not extend to the compiled method. That means you get a different answer when you call GetCustomAttributes() on the original DynamicMethod instance vs. the Method property of the created Delegate. On CoreCLR these call through to the same code in RTDynamicMethod. To solve for this I think we'd need Mono start using something like RTDynamicMethod.

If we restrict the problem to just looking at calling Attribute.GetCustomAttributes() on the DynamicMethod instance directly, there is a separate issue. Because CustomAttribute.IsUserCattrProvider thinks DynamicMethod should be treated as "native", it won't actually call through to DynamicMethod.GetCustomAttributes(). This is easy enough to change with a tweak to CustomAttribute.IsUserCattrProvider, but then we run into the issue that CustomAttribute.GetCustomAttributesBase() ends up calling the underlying GetCustomAttributes() passing null for the attribute type when the type starts out as typeof(Attribute) or is not provided. There are multiple FIXME comments in the file warning about this. I don't think this would be a huge deal to fix, but the cleanest way I can see to do it (using typeof(object) as the sole means to request unfiltered attributes) would result in a good number of changes to CustomAttribute.cs and Attribute.Mono.cs. Without more context, it's hard for me to say whether those changes might have other unforseen consequences.

In short, fixing either of these felt out of scope for the original problem; from what I can tell these are gaps/oddities in the Mono attribute functionality that are preexisting and not related to generic attributes at all.

Thoughts?

[jkotas]

I agree with your assessment.

[deeprobin]

LGTM

[buyaa-n]

LGTM, thanks!

[buyaa-n]

Build failures unrelated and known issue, merging