IL2091 errors for all APIs using an annotated type as parameter, is this by design?

[Sergio0694]

I'm working on adding NativeAOT support to my swap chain Win32 sample for ComputeSharp (see here) and I'm having trouble understanding a lot of linker errors I'm getting. I've fixed them adding the necessary annotations, but I'm still quite confused as to why they're needed in the first place.

You can see the full code in this PR, but to summarize, I have this:

// I have some type like this
public class SomeType<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T>
{
    public SomeTime()
    {
        SomeMethodDoingReflectionOnTheTypeArgument<T>(); // This is why I need the annotation
    }
}

// And then somewhere else
public class SomeClass
{
    public static void SomeMethod<T>(SomeType<T> foo)
    {
        // Do stuff...
    }
}

Now, I'm getting a whole bunch of linker errors like this:

error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.NonPublicFields' in 'ComputeSharp.ReadWriteBuffer'. The generic parameter 'T' of 'ComputeSharp.ComputeContextExtensions.Barrier(in ComputeContext, ReadWriteBuffer)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

But like... Why? I do need the annotation for the type, because I'm doing reflection in the constructor, sure. But if I have a generic method that receives an instance of this method, why does it also need the trimming annotation? The input type would have already be constructed there, so this method will never reach any code paths doing reflection. Is this just because the linker can't know the constructor is the only place where this will happen, and it assumes every API on this type might also reflect on T? If so, is there a more narrow annotation I can use so that only callsites constructing the type require the annotation?

Right now otherwise I have to add a whole bunch of annotations everywhere that I'm not sure are actually needed 🤔

Side question: if I need to reflect on the fields of a type and all their fields, recursively, what annotation do I need? I'm using [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)] but then I still get a linker error when recursive on the method on the type of one of the explored fields. Is there some annotation to imply I want to retain all fields of a type, recursively so I could use it here? Thank you! 😄

[kant2002]

Maybe same as #2843

[MichalStrehovsky]

ILLink currently validates that all T's are annotated the same way. I agree that a lot of this validation doesn't translate to actual issues.

@vitek-karas @sbomer @tlakollo looks like this is what we were discussing on email last week. I've ran into this annotating BenchmarkDotNet too: dotnet/BenchmarkDotNet#2046

[vitek-karas]

This is a valid point - and as @MichalStrehovsky pointed out we've already ran into this recently. We're trying to figure out what is the smaller subset where linker will need to do the validation. Unfortunately right now, it requires the annotations to match everywhere.

As for the fields question - the annotations follow the reflection's BindingFlags semantics. Reflection doesn't have a single API to get private members on base types. It does work on public members though. Meaning GetFields will return all public fields, including those from all base types. But GetFields(BindingFlags.NonPublic) only returns private fields from the type itself, not from base types. Same goes for the annotations.

Currently the only annotation which preserves everything across base types is DynamicallyAccessedMemberTypes.All (that preserves everything on the type and on all of its base types and interfaces).

Basically it tries to hint you to not use private reflection 😉 , which is typically a good idea.

[Sergio0694]

Glad to hear this is being looked into! I'll just leave those annotations everywhere for now then 🙂

@vitek-karas just to clarify, in my scenario I'm only dealing with generic types constrained to unmanaged, and the only thing I care about is checking whether any field (recursively) is of type double. Given I'm only dealing with structs, there's no concern about base types. In this case, would just [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)] do the trick? As in, is that also preserving info for all types of fields of this type, recursively? For context: I need this because I want to detect and throw a nice exception if a user tries to allocate a buffer of a type that contains a double field, when using a GPU device that doesn't support double precision numbers. Otherwise the allocation would work (I'm just allocating some bytes anyway), but then dispatching a shader later on would throw with a harder to understand exception, which is worse UX for the developer. Essentially, all my reflection use is just the one you can see in this TypeInfo<T> type. Thank you! 😄

[vitek-karas]

Sorry - I didn't understand that "recursively" means recursing into the fields (I though recursing into base types).

Linker has a rule where types with explicit layout (structs are by default explicit layout) always keep all their fields (so that we don't change the size and field layout for interop reasons). So if your struct only stores other value types, then this would work. But if there's a reference type somewhere, that will break. This will not be warning free - linker doesn't incorporate this rules when making warning decisions (not sure if it would help, but in any case it's low priority), but if you can guarantee that everything you walk over are explicit layout types, then it should work. In a way you would not need any annotation - since just the fact that the type is instantiated (which is always assumed for value types) will preserve all fields on it. I would keep the annotation - more as a documentation really - and probably add a comment that this relies on the types to be explicit layout, or maybe add a check that it's all values types (non-explicit layout value types should be rare).

One other comment about the code - don't use pragma suppressions since they only work for analyzer. All pragmas are removed by the compiler and don't make it into IL, so the same warning should popup again from trimmer/NativeAOT. Instead use UnconditionalSuppressMessageAttribute (even SuppressMessageAttribute is removed by the compiler).

Currently there's no annotation which would make the trimmer preserve something "recursively into fields/members". We've been thinking about it for a while but there's no solution in sight (it usually leads into "keep almost everything", or "everybody needs slightly different things... how are we supposed to know what").

[Sergio0694]

"Linker has a rule where types with explicit layout (structs are by default explicit layout) always keep all their fields (so that we don't change the size and field layout for interop reasons). So if your struct only stores other value types, then this would work."
[...]
"I would keep the annotation - more as a documentation really - and probably add a comment that this relies on the types to be explicit layout, or maybe add a check that it's all values types (non-explicit layout value types should be rare)."

Ah, that's great to know, thank you for the additional info! I have where T : unmanaged everywhere so I do have the guarantee that all types involved will just be value types, so that's good to know. I was worried that even with the actual types being preserved, the linker could just trim the metadata out (like on .NET Native) so that while using the type would work, trying to do this kind of reflection would still crash. I'll keep the annotations just in case as you suggest, yeah makes sense 🙂

"One other comment about the code - don't use pragma suppressions since they only work for analyzer. All pragmas are removed by the compiler and don't make it into IL, so the same warning should popup again from trimmer/NativeAOT. Instead use UnconditionalSuppressMessageAttribute (even SuppressMessageAttribute is removed by the compiler)."

Ah, I didn't realize that, thank you! I've opened a PR with an hotfix for this in the MVVM Toolkit as well (CommunityToolkit/dotnet#359) and I'll do the same fix in ComputeSharp too now that I'm aware of this, thank you for the additional details here! 😄

I guess all my questions have been resolved, feel free to close this or keep it to track the more narrow annotations 👍

[vitek-karas]

Actually you're right that this won't necessarily work for NativeAOT. I'm not even sure if NativeAOT will keep the fields, but it is probably unlikely to keep the metadata, meaning your code would probably not work.
@MichalStrehovsky does NativeAOT have some special rules related to this? Ones we we're OK to take a dependency on?

[Sergio0694]

"meaning your code would probably not work"

Wait, can you elaborate? Do you mean it would still not work even with the annotations I've added?
Or was that to mean, it wouldn't work if I had no annotations and was just relying on those fields being implicitly kept?

[vitek-karas]

For NativeAOT it probably would not work even with the annotations - the annotations are NOT recursive, so the first level would work, but technically there's no guarantee that the second level would be able to enumerate the fields (the fields would probably still be there in the data structure, but runtime would have no metadata about them -> not able to enumerate them). @MichalStrehovsky would know for sure.

[MichalStrehovsky]

Yep, it won't work with NativeAOT. IL Linker needs to keep fields including their metadata because the program cannot access them without metadata. It's not the case for native code that accesses fields by doing mov (and doesn't care how things are named or what type they have).

Here's a quick test program:

using System.Reflection;
using System.Runtime.InteropServices;

unsafe
{
    Console.WriteLine($"sizeof S1: {sizeof(S1)}");
    Console.WriteLine($"sizeof S2: {sizeof(S2)}");
    Console.WriteLine($"sizeof S3: {sizeof(S3)}");
}

DumpFields(typeof(S1));

static void DumpFields(Type t, int pad = 0)
{
    if (t.IsPrimitive) return;

    Console.Write(new string(' ', pad));
    Console.WriteLine(t.FullName);
    foreach (var f in t.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic))
    {
        Console.Write(new string(' ', pad + 1));
        Console.WriteLine(f.Name);
        DumpFields(f.FieldType, pad + 2);
    }
}


struct S1
{
    public S2 Field1;
    public S3 Field2;
}

struct S2
{
    public int Field;
}

[StructLayout(LayoutKind.Auto)]
struct S3
{
    public int Field;
}

With dotnet run:

sizeof S1: 8
sizeof S2: 4
sizeof S3: 4
S1
 Field1
  S2
   Field
 Field2
  S3
   Field

With PublishTrimmed:

sizeof S1: 8
sizeof S2: 4
sizeof S3: 1
S1
 Field1
  S2
   Field
 Field2
  S3

With PublishAot:

sizeof S1: 8
sizeof S2: 4
sizeof S3: 4
S1

Notice PublishTrimmed also touched fields on the Auto layout type. NativeAOT doesn't currently do that because the cost is basically just padding.

[Sergio0694]

Mmh I'm confused, I tried this on my end too but while I do get warnings, the code still runs fine? 🤔

I get:

sizeof S1: 8
sizeof S2: 4
sizeof S3: 4
S1
 Field1
  S2
   Field
 Field2
  S3
   Field

(I updated your code to add an S3 Field2 into S1 as well, as it seems you forgot to include it in the script?).

This was with:

dotnet publish .\NativeAotTrimmingTest.csproj -c Release -r win10-x64 --self-contained
.\bin\Release\net7.0\win10-x64\publish\NativeAotTrimmingTest.exe

In a blank console app with just the code above, plus <PublishAot> set in the .csproj file.

Anyway, it would seem NativeAot is preserving all metadata here by default? I assume one shouldn't rely on this though? My question is: if I add the trimming annotations, would this still be technically not safe? I would like to not have to go as far as adding [RequiresUnreferencedCode] just for checking fields in a value type 🥲

[MichalStrehovsky]

Try with one if the rc1 builds.

[MichalStrehovsky]

I've updated the test program with what I actually ended up using.

NativeAOT will very aggressively trim non-reflected things. Fields have zero importance after native compilation. Code that accesses them just uses movs. illink needs to keep them if someone uses them (at least right now - it could also just access offsets if it knew the layout and ensured the layout stays the same with IL definition of the field removed).

[Sergio0694]

I see. And you can confirm that even with the annotation, only the top level would be preserved?
I assume if that's indeed the case it means in my scenario I'd have to use [RequiresUnreferencedCode]? 🥲

If that was the case, I guess I could live with it but then I would really like to not have to repeat these everywhere.
As in, to just be able to have the constructor be annotated, in some way.

[MichalStrehovsky]

Yes, we don't have a way to express "keep fields recursively" (it runs into the "what does it mean to keep fields recursively" question that everyone has different answers for - e.g. serializers would want to recursively keep fields of T if the field is IEnumerable<T>, do special things for Dictionaries, etc.). This is probably the first use case for "no, really, just the fields recursively".

[Sergio0694]

I see. Alright I guess [RequiresUnreferencedCode] it is then, and I'll look forward for a way to only annotate constructors in the future then, or possibly just for an update to the linker so that it automatically knows that only callsites creating an instance or accessing static methods should be marked, but not instance ones, that'd fix the main pain point I guess 🙂

"e.g. serializers would want to recursively keep fields of T if the field is IEnumerable, do special things for Dictionaries, etc.)."

Just out of curiosity, isn't that what Serialize="Requires All" does on .NET Native? I thought that also had special handling so that it'd be able to keep all types in the whole type graph of fields/properties of a type? Or does that also have limitations?

[MichalStrehovsky]

Just out of curiosity, isn't that what Serialize="Requires All" does on .NET Native? I thought that also had special handling so that it'd be able to keep all types in the whole type graph of fields/properties of a type? Or does that also have limitations?

That did a bunch of random heuristics. E.g. if a field was an ImmutableDictionary, it ensured the builder is kept.

.NET Native relied on heuristics and then crossed its fingers that it's all that's necessary to make code work at runtime. One wouldn't know unless they run the app and thoroughly test.

What we do now is we rely on statically analyzable code and generating warnings if it's not analyzable. The warnings tell if it will work. Not everything can be expressed that way.

[vitek-karas]

.NET Native has lot of code which understands XmlSerializer and DataContractSerializer and how to keep enough for these to work. It's basically a losing proposition trying to teach the tooling about every serializer out there. Also of note is that there are lot of libraries which are not called "serializer" but actually act like one, for example "DB -> OM mappers", some loggers, "config -> OM readers", "XAML -> OM readers", ...

[Sergio0694]

I see. Yeah I guess the fact .NET Native has so many heuristics can save you in some cases, but life now on NAOT with all the trimming annotations is indeed waaaay better, for sure. Enabling trimming in the Store was really painful and it took a ton of trial and error to try to spot whether anything had silently broken, because there's just no real way to know in advance 🥲

And then even after spotting that it's a whole other lot of work to fix that and then ensure it really fixed the issue. Things are really so much better on this front now, you folks did some incredible job. If only we had trimming annotations on UWP too... 😄

[vitek-karas]

Thanks a lot for the kind words - really happy that it helps.

I'm going to keep this issue to track the work to reduce the necessary annotations for generic parameters.