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

TrimmerRootDescriptor is not able to preserve generic instantiations #95058

Closed
hez2010 opened this issue Nov 21, 2023 · 8 comments
Closed

TrimmerRootDescriptor is not able to preserve generic instantiations #95058

hez2010 opened this issue Nov 21, 2023 · 8 comments

Comments

@hez2010
Copy link
Contributor

hez2010 commented Nov 21, 2023

Description

Assuming we have the below lines in root.xml:

<linker>
  <assembly fullname="MyAssembly">
    <type fullname="Foo`1[[System.Int32,System.Private.CoreLib]]" preserve="all" />
  </assembly>
</linker>

It will fail to resolve the type when compiling with NativeAOT because:

  1. In SplitFullName it splits the full name into namespace Foo`1[[System and type Int32,System.Private.CoreLib]]:
    public static void SplitFullName(string fullName, out string @namespace, out string name)
  2. Even if it was split correctly, it still failed to resolve the type because the type name Foo`1[[System.Int32,System.Private.CoreLib]] was not in the name cache (there was only Foo`1 in the name cache, so any instantiations will fail to resolve):
    if ((currentModule._nameLookupCache ?? currentModule.CreateNameLookupCache()).TryGetValue((name, nameSpace), out EntityHandle foundHandle))

This is preventing us to move from rd.xml to TrimmerRootDescriptor. TrimmerRootDescriptor should handle generic instantiations as well.

/cc: @MichalStrehovsky

Reproduction Steps

Create a console project MyAssembly with code:

var t = Console.ReadLine();
Console.WriteLine(typeof(Foo<>).MakeGenericType(Type.GetType(t!)!));

class Foo<T>
{
    public override string ToString()
    {
        return typeof(T).ToString();
    }
}

and TrimmerRootDescriptor:

<linker>
  <assembly fullname="MyAssembly">
    <type fullname="Foo`1[[System.Int32,System.Private.CoreLib]]" preserve="all" />
  </assembly>
</linker>

Run it and input System.Int32.

Expected behavior

Print Foo`1[System.Int32]

Actual behavior

warning IL2008: Could not resolve type 'Foo`1[[System.Int32,System.Private.CoreLib]]'
Unhandled Exception: System.NotSupportedException: 'Foo`1[System.Int32]' is missing native code or metadata. This can happen for code that is not compatible with trimming or AOT. Inspect and fix trimming and AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 21, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 21, 2023
@teo-tsirpanis teo-tsirpanis added area-NativeAOT-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 21, 2023
@ghost
Copy link

ghost commented Nov 21, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Assuming we have the below lines in root.xml:

<linker>
  <assembly fullname="MyAssembly">
    <type fullname="Foo`1[[System.Int32,System.Private.CoreLib]]" preserve="all" />
  </assembly>
</linker>

It will fail to resolve the type when compiling with NativeAOT because:

  1. In SplitFullName it splits the full name into namespace Foo`1[[System and type Int32,System.Private.CoreLib]]:
    public static void SplitFullName(string fullName, out string @namespace, out string name)
  2. Even if it was split correctly, it still failed to resolve the type because the type name Foo`1[[System.Int32,System.Private.CoreLib]] was not in the name cache (there was only Foo`1 in the name cache, so any instantiations will fail to resolve):
    if ((currentModule._nameLookupCache ?? currentModule.CreateNameLookupCache()).TryGetValue((name, nameSpace), out EntityHandle foundHandle))

This is preventing us to move from rd.xml to TrimmerRootDescriptor. TrimmerRootDescriptor should handle generic instantiations as well.

/cc: @MichalStrehovsky

Reproduction Steps

Create a console project MyAssembly with code:

var t = Console.ReadLine();
Console.WriteLine(typeof(Foo<>).MakeGenericType(Type.GetType(t!)!));

class Foo<T>
{
    public override string ToString()
    {
        return typeof(T).ToString();
    }
}

and TrimmerRootDescriptor:

<linker>
  <assembly fullname="MyAssembly">
    <type fullname="Foo`1[[System.Int32,System.Private.CoreLib]]" preserve="all" />
  </assembly>
</linker>

Run it and input System.Int32.

Expected behavior

Print Foo`1[System.Int32]

Actual behavior

warning IL2008: Could not resolve type 'Foo`1[[System.Int32,System.Private.CoreLib]]'
Unhandled Exception: System.NotSupportedException: 'Foo`1[System.Int32]' is missing native code or metadata. This can happen for code that is not compatible with trimming or AOT. Inspect and fix trimming and AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: hez2010
Assignees: -
Labels:

untriaged, area-NativeAOT-coreclr

Milestone: -

@MichalStrehovsky
Copy link
Member

This matches the behavior of IL linker. It's not legal in the descriptors file format to put instantiated generics as type name. The problem is more pronounced for generic methods. We'd need a file format change to do this.

Cc @dotnet/illink-contrib

@vitek-karas
Copy link
Member

Ideally you would not use the XML files at all, can you replace it with a DynamicDependency attribute instead? Or you can use Type.GetType("") in code somewhere, that should work for all kinds of types (and will use the Reflection syntax). Both should work "cross assembly", so if you add those somewhere reachable from your main, that should be enough.

@hez2010
Copy link
Contributor Author

hez2010 commented Nov 22, 2023

Ideally you would not use the XML files at all

Yeah that's the ideal case I want to see too. But in the real world you will have to workaround against some 3rd-party even 1st-party libraries using those XML files.
Now that we retired rd.xml, I think the recommended approach TrimmerRootDescriptor should at least on pair with rd.xml on functionality.

@MichalStrehovsky
Copy link
Member

Using the Type.GetType trick to ensure the type is constructed is probably the best. The syntax can be validated during F5 debug time on a JIT-based runtime instead of worrying one got the XML wrong.

The experience with any of the XML formats is extremely poor (first one can never be sure whether the file is getting picked up at all, and then there's no way to troubleshoot it if things don't work). We don't optimize for them, they're last resort option and the user is already on pretty much unsupported paths (we cannot help authoring these XML files).

@hez2010
Copy link
Contributor Author

hez2010 commented Nov 22, 2023

Using the Type.GetType trick to ensure the type is constructed is probably the best.

Does this also work for delegate marshalling metadata and generic methods?

I just tried

Type.GetType("Foo`1[[System.Int32,System.Private.CoreLib]]")!.GetMethod("Test")!.MakeGenericMethod(typeof(int))

against

class Foo<T>
{
    public void Test<U>()
    {
        Console.WriteLine(typeof(U).ToString());
    }
}

but got

System.NotSupportedException: 'Foo`1[System.Int32].Test[System.Int32]()' is missing native code. MethodInfo.MakeGenericMethod() is not compatible with AOT compilation. Inspect and fix AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility

Apparently it's not working without #81204

@MichalStrehovsky
Copy link
Member

Does this also work for delegate marshalling metadata and generic methods?

I thought we're only discussing accepting generics in the fullname. If the scope is "what is the replacement for RD.XML", we don't have any. The position is really that if something is generating warnings, it may not work, it needs to be written so that it doesn't warn, and that's it. RD.XML will work in .NET 8 for workarounds. We don't plan to build a long-term path for that experience. TrimmerRootDescriptors only exist because they survived from the time when no static analysis was done and unfortunately we documented them.

@agocke agocke added this to AppModel Nov 28, 2023
@MichalStrehovsky
Copy link
Member

I'm going to close this. I was looking at yet another customer-created RD.XML that had nonsensical things in it, like rooting all of CoreLib (that was then causing problems for the customer). Very few people understand how to use this. We don't want to build experiences around sidecar XML files that people cargo cult around.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Dec 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

No branches or pull requests

4 participants