DynamicallyAccessedMembers with a string TypeName of a generic type with different assemblies doesn't appear to work

[eerhardt]

See dotnet/runtime#39709 for the tests that found this issue.

Using the following code:

using System;
using System.ComponentModel;
using System.Xml.Linq;

class Program
{
    static void Main(string[] args)
    {
        var xel = new XElement("someElement");
        var props = TypeDescriptor.GetProperties(xel);
        foreach (PropertyDescriptor prop in props)
        {
            Console.WriteLine(prop.Name);
        }
    }
}

Run the above program without trimming, and you get:

Attribute
Descendants
Element
Elements
Value
Xml
FirstAttribute
HasAttributes
HasElements
IsEmpty
LastAttribute
Name
NodeType
Value
FirstNode
LastNode
NextNode
PreviousNode
BaseUri
Document
Parent

Run the above program after it has been trimmed (TrimMode=link) and you get:

Name

The TypeDescriptionProvider attribute is attributed with DynamicallyAccessedMembers. However, the value being passed in is causing problems for the linker.

    [System.ComponentModel.TypeDescriptionProvider("MS.Internal.Xml.Linq.ComponentModel.XTypeDescriptionProvider`1[[System.Xml.Linq.XElement, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]],System.ComponentModel.TypeConverter")]
    public class XElement : XContainer, IXmlSerializable
    {

It appears this code is the problem:

https://github.com/mono/linker/blob/90fd1feac709a910ba2aa849657b0c04777fc67d/src/linker/Linker/TypeNameResolver.cs#L63-L66

This code assumes that the generic type argument is in the same assembly as the outer generic type. But this is not the case above - the outer generic type is in System.ComponentModel.TypeConverter. While the generic argument is in System.Xml.Linq.

I tried fixing this myself, but then I ran into that _context.GetLoadedAssembly("System.Xml.Linq") was returning null for some reason. We should investigate this complete scenario and ensure it works properly.

cc @mateoatr @vitek-karas @marek-safar

[vitek-karas]

@eerhardt What have you done 😄 - as I dig into this I find more and more "broken things".

I'm working on a PR which will fix the immediate problem of not looking in the right assembly. But that alone will probably not fix the original issue since then it hits the problem of not being able to lazy-load assemblies. To fix that we will need to add DynamicDependency attribute in the right place to "bring in" the necessary assembly. I'll work on this some more.

Only semi-related:
Along the way I found two other issues, both described here: #1537
As well as an SDK bug described here: dotnet/sdk#13999

[vitek-karas]

The failure to resolve the assembly is because of facades .. booo.
By the time we parse this string linker already fully loaded the XElement type, but it comes from System.Private.Xml.Linq - and that assembly is loaded. The type name specified uses the public facade System.Xml.Linq which the linker didn't really load. (I need to look into exactly why).
So one fix would be to change the attribute on XElement to use System.Private.Xml.Linq instead. It's similar to DynamicDependency which also doesn't handle facades well - specifying a DynamicDependency(".ctor()", "System.Xml.Linq.XElement", "System.Linq.Xml") also doesn't work - linker won't resolve the type.

[vitek-karas]

I filed #1542 to track the problem with not seeing through facades.

[vitek-karas]

As a side note: Why do we have System.Xml.Linq and also System.Xml.XDocument - they are both empty facades and both contain basically the same type forwarders...

For example in the test app, the XElement is actually directly referenced from main, but that reference goes through System.Xml.XDocument (how does the compiler pick this one?). And then in code we have attributes which use the System.Xml.Linq facade instead.

I can understand having two similar facades due to backward compatibility reasons, but if I start a new .NET 5 app, I would expect we always use only one of those facades.

[eerhardt]

FYI - I tested this scenario with the latest 6.0 build, and it still doesn't work because the System.Xml.Linq facade isn't getting copied to the published app folder. Is that issue still #1542? (which I thought should have been fixed by #1837)

[vitek-karas]

I think the linker works correctly now:

• TypeDescriptorProviderAttribute has PublicParameterlessConstructor annotation on its parameter
• The parameter is XTypeDescriptorProvider<XElement>
• After trimming XTypeDescriptorProvider has its public parameterless constructor (it even has virtual overrides, so linker correctly thinks it's instantiated).
• XTypeDescriptorProvider doesn't enforce any requirements on its generic parameter, so the XElement is only mark to the extent necessary by the test code. Which leaves it almost empty (just .ctor and WriteTo and the Name property).

I don't see anything which should force XElement to preserve more stuff.

The "normal" way the code gets the information via object.GetType on the XElement instance, which obviously doesn't work.
For this reason, the repro produces a warning:

IL2026: Using method 'System.ComponentModel.TypeDescriptor.GetProperties(object)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. PropertyDescriptor's PropertyType cannot be statically discovered. The Type of component cannot be statically discovered.

I don't know why XTypeDescriptorProvider is a generic, it only uses the generic parameter to create a matching XTypeDescriptor which in turn only uses that generic parameter to have some specific behavior around XElement and XAttribute, but it seems it will work on any type really.

[eerhardt]

I don't see anything which should force XElement to preserve more stuff.

Sorry, I should have said more above.

Change:

        var xel = new XElement("someElement");
        var props = TypeDescriptor.GetProperties(xel);

to

        var props = TypeDescriptor.GetProperties(typeof(XElement));

You'll still get an IL2026 warning, because GetProperties(Type) is RUC since drilling into the PropertyDescriptor instances will result in using unbounded reflection.

But now instead of 21 properties, you get back 15. The 6 that are missing are the ones injected by XTypeDescriptorProvider:

Attribute
Descendants
Element
Elements
Value
Xml

https://github.com/dotnet/runtime/blob/85fde0536b64c4901056b894f5184103f39cac63/src/libraries/System.ComponentModel.TypeConverter/src/MS/Internal/Xml/Linq/ComponentModel/XComponentModel.cs#L46-L51

The reason for this, as far as I can tell, is because the System.Xml.Linq facade doesn't get copied to the output directory, but it is needed in order to Type.GetType on this string:

[System.ComponentModel.TypeDescriptionProvider("MS.Internal.Xml.Linq.ComponentModel.XTypeDescriptionProvider`1[[System.Xml.Linq.XElement, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]],System.ComponentModel.TypeConverter")]

Since the type System.Xml.Linq.XElement, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 can't be found at runtime, the Type referenced by the TypeDescriptionProviderAttribute is skipped/ignored.

I don't know why XTypeDescriptorProvider is a generic,

It looks to be that way in .NET Framework as well: https://referencesource.microsoft.com/#System.Xml.Linq/System/Xml/Linq/XComponentModel.cs,13. I assume the reason they did this was so they didn't have to duplicate 3 sets of classes for XElement, XAttribute, and XDocument. You could accomplish the same thing that way, with more code.

[vitek-karas]

#2099 adds a simpler test case for the same issue.

[eerhardt]

I see this is still in the ".NET 6" milestone. Are we expecting to resolve this issue for 6?

I am moving the blocked issue in dotnet/runtime (dotnet/runtime#39709) to 7.0, because I don't believe the scenario that found this linker issue is necessary for 6.0. But I'm wondering if the underlying linker issue will be fixed for 6.

[lewing]

Is this targeting 7 now?

[vitek-karas]

Sorry we dropped the ball on this. I'll try to get to this (or find somebody to look into it).
The repro from #2099 still works (as in it is still a problem, some of the more recent forwarder changes didn't fix this).