PreserveDependencyAttribute on a type causes a fatal error

[stephentoub]

The PreserveDependencyAttribute defined in mono currently doesn't allow it to be specified on types, but it should. When I add that allowance in corefx and try to use it on a type, however, the linker blows up with a null ref:

  Fatal error in IL Linker

  Unhandled Exception: Mono.Linker.MarkException: Error processing method: 'System.Void System.Text.Json.JsonSerializerOptions::.ctor()' in assembly: 'System.Text.Json.dll' ---> System.NullReferenceException: Object reference not set to an instance of an object.
     at Mono.Linker.Steps.MarkStep.MarkCustomAttributes(ICustomAttributeProvider provider)
     at Mono.Linker.Steps.MarkStep.MarkType(TypeReference reference)
     at Mono.Linker.Steps.MarkStep.MarkGenericArguments(IGenericInstance instance)
     at Mono.Linker.Steps.MarkStep.GetOriginalType(TypeReference type)
     at Mono.Linker.Steps.MarkStep.MarkType(TypeReference reference)
     at Mono.Linker.Steps.MarkStep.MarkMethod(MethodReference reference)
     at Mono.Linker.Steps.MarkStep.MarkInstruction(Instruction instruction)
     at Mono.Linker.Steps.MarkStep.MarkMethodBody(MethodBody body)
     at Mono.Linker.Steps.MarkStep.ProcessMethod(MethodDefinition method)
     at Mono.Linker.Steps.MarkStep.ProcessQueue()
     --- End of inner exception stack trace ---
     at Mono.Linker.Steps.MarkStep.ProcessQueue()
     at Mono.Linker.Steps.MarkStep.ProcessPrimaryQueue()
     at Mono.Linker.Steps.MarkStep.Process()
     at Mono.Linker.Steps.MarkStep.Process(LinkContext context)
     at Mono.Linker.Pipeline.ProcessStep(LinkContext context, IStep step)
     at Mono.Linker.Pipeline.Process(LinkContext context)
     at Mono.Linker.Driver.Run(ILogger customLogger)
     at Mono.Linker.Driver.Execute(String[] args, ILogger customLogger)
     at Mono.Linker.Driver.Main(String[] args)

cc: @marek-safar

[marek-safar]

Could you share the use case why do you think it should apply to types? We very intentionally didn't allow it on types because it's ambiguous what it actually means.

[stephentoub]

Could you share the use case why do you think it should apply to types?

For example, a case where we need to keep XElement's internal ctor around because it's accessed by reflection from existing tools. It would seem the cleanest way to do this is to put a [PreserveDependency(".ctor")] attribute on XElement itself, so that if code is using XElement, its constructor will be kept. I can't depend on any other method having been accessed in the consuming code and so seemingly have nowhere else to put the attribute.

because it's ambiguous what it actually means

How so?

[stephentoub]

As another example, we have several places where Visual Studio depends on internal/private methods existing on a type. How can I use [PreserveDependency(...)] to keep those alive? I could do so by putting such an attribute on the type itself, such that as long as the type is around, the methods won't be trimmed. But today it seems my only option is the ILLinkTrim.xml file.

[stephentoub]

Another example: if PreserveDependency could be placed on types, then I'd have a workaround for #801.

[marek-safar]

It would seem the cleanest way to do this is to put a [PreserveDependency(".ctor")] attribute on XElement itself

That does not make much sense to me. There is no guarantee the XElement type will be kept. What reflection pattern does the tool use, perhaps we could fix that instead? What about making the ctor public when it's used so often we need to keep it all the time?

because it's ambiguous what it actually means

Just two examples, imagine the type which has PreserveDependency at type level is used only as cast. Should that bring the dependency in? What about case where the type is used at declaration level only, something like void Foo<T> () where T : TypeWithPreserveDependency should that bring it in and so on.

If we really need something like this I'd prefer to use a different attribute for this. For example, use reverse order and specify caller dependency at the constructor.

[stephentoub]

There is no guarantee the XElement type will be kept.

If XElement isn't kept, then there's no need to keep the ability to construct it.

What about making the ctor public when it's used so often we need to keep it all the time?

I don't think we should be deciding on the public shape of API (especially API like this that's been around for a long time) based on the needs of optimization tooling. If I'm understanding your proposal, it would also entail changing the consuming code; I'm not even sure what/where that code is.

the type which has PreserveDependency at type level is used only as cast. Should that bring the dependency in?

Sorry, I'm not seeing the ambiguity. The attribute shouldn't affect whether the type that's attributed is kept. It only says "if you keep the attributed type, then also keep this dependency". If the type is kept because of the cast, then so would its dependency.

[mrvoorhe]

I was speaking with @marek-safar the other day and he asked me to share my thoughts on this specific request as well as my thoughts on an attribute based api for controlling stripping in general. I've been thinking about this in the background for quite some time. Unity end-users, developers at Unity , and us subset of developers at Unity who are responsible for mono class libraries all have different goals, constraints, use cases, and as a result we all want slightly different things.

First let me recap the mechanisms we have today and the pros/cons of them.

link.xml

This is the xml file format that lets you explicitly preserve anything. This is a great backdoor to fix up code you do not own. It's not such a great api for managing the stripping of code you do have control over. It can easily break from renaming, getting full signatures specified correctly can be a huge pain, and there is no way to conditionally define a preservation.

Unity/mono used to depend exclusively on this mechanism for preserving things in the class libraries that were needed by the runtime, or reflection in the class libraries. This was imprecise and results in over preservation. Both unity & mono have been trying to move away from this mechanism.
[PreserveDependency] has been one alternative. Improved reflection detection has been another.

My experience with Unity end-users is that they rarely care enough to figure out exactly what is being stripped away that is needed and then adding just that to the link.xml file. What they do instead is just start preserving entire assemblies via link.xml until the problem goes away.

[Preserve]

This is an attribute unique to Unity. We've had it around for a long time. You can put it on anything and that thing will be preserved no matter what. This mechanism is easier to use than link.xml. You don't have to figure out the full signature of anything, but it still suffers from the same problem as link.xml in that it essentially causes the type/member to be a root.

Additionally, if the [Preserve] attribute is placed on a type, it will cause the types default ctor() to be marked. This behavior choice came before my time at Unity so I'm exactly sure why this choice was made. However, I can speculate based on my experience with other people who do not work on managed code stripping. People generally don't realize a types default ctor is a separate entity from the TypeDefintion itself. They think, "I preserved the type, I should be able to create an instance of that type now" (i.e. that Activator.CreateInstance(type) call somewhere in my code should work now). Once you explain that needing a TypeDefinition is separate from needing that types default ctor, they get it. It's just that this detail is often overlooked by developers who interact with stripping infrequently.

[PreserveDependency]

This was added by @marek-safar a year or so ago. It let's a method precisely say that it depends on something else. This is great for 1 off things where a dependency cannot be detected by the linker. Such as calls into native code that then callback into managed code. Or a reflection usage that cannot be detected.

However, this attribute suffers from the same figure-out-the-signature problem as link.xml. It can easily break from renaming. It's not great if a method has many dependencies. It simply doesn't work for reflection patterns where you do something like find all types that implement an interface. And it's not a practical tool for poking around trying to guess what was incorrectly stripped. Remember, in most scenarios to date, the developer doesn't know what was incorrectly stripped.

Add a usage hack

I've seen variations of this numerous times. It may even be more commonly used than link.xml and [Preserve] for preserving individual methods that are being stripped when they are needed.

public static class SomeType
{
     static void MethodThatIsNotCalledButDoesSurviveLinking()
    {
        AnotherType.Method1ThatWasBeingStripped();
        AnotherType.Method2ThatWasBeingStripped();
        AnotherType1.Method1ThatWasBeingStripped();
        AnotherType2.Method2ThatWasBeingStripped();
    }
}

I see the appeal of this. It's essentially the same behavior as link.xml without the headaches.

The conclusion I've reached

All of this has led me to believe that different types of users/code bases will want different types of attributes for controlling the linker.

• mono / .NET Core class library code will want to be very precise about it's annotations. Making an annotation that would lead to marking things that are not needed for all usages would be undesirable.

• Unity owned code. UnityEngine, and Unity packages will generally want to be as precise as we would for class library code. However, we do have some reflection and native code patterns that may lead us to over preserve a little bit.

• Middle ware developers. These would be developers of things on the Unity Asset store, or maybe even something like developers of Newtonsoft.Json.dll. I think at this level reflection patterns that are undetectable will be more common. Things like "Find all types that implement X interface", or "get all fields on a type and do X". Some developers will want sane ways of annotating their code such that there is not the blind over preservation that occurs with something like [Preserve]. There really is no solution for these scenarios today.

• End developers. This could be the developer of a Unity project, or developer of a .NET Core application. They have some project that depends on a set of assemblies they own, plus middle ware assemblies, and the class library assemblies. They want the size savings provided by the class libraries being linked and they may or may not want some level of linking the assemblies they own. If they hit problems in code they own, then often just want to cast some wide nets until the problem goes away.

What about allowing [PreserveDependcy] on a Type

Now onto the request in this issue.

For example, a case where we need to keep XElement's internal ctor around because it's accessed by reflection from existing tools

As another example, we have several places where Visual Studio depends on internal/private methods existing on a type.

Based on these comments my feeling is that these are dependencies of external tools / code. Not of XElement itself or of other class library code. In an ideal world then, I don't think it makes sense to define this dependency on XElement itself since class library code should strive to avoid adding annotations that do not apply in all cases. That said, I appreciate sometimes it's necessary to be practical. I don't know the details of these use cases enough to comment on it from that perspective.

I will say that if you do absolutely need a behavior like you are requesting, I'd lean away from using [PreserveDependency]. [PreserveDepdency] is precise. I think that it's probably impossible to fulfill all annotation scenarios with 1 attribute. And if that's the case then I think having an attribute that is precise and other attributes that are less precise seems like a sane way to organize it. That way developers could have rules such as "class library code can only use attributes A, B, and C" even if something like [Preserve] existed officially in .NET.

What could a more complete attribute api look like?

This post is getting pretty long. I'll go into thoughts I have on this in a follow on post.

[mrvoorhe]

Over the last year or so I've started to accumulate additional attribute ideas that would solve a particular use case that someone would come to me with. Sometimes the use case would come from a developer at Unity and other times it would come from one of our end users.

We have also begun linking the assemblies that are in our IL2CPP test suite. These assemblies are a great accumulation of coding patterns. Sometimes these tests can be contrived examples, but non the less we started adding attributes that our UnityLinker supports that would allow us to annotate these tests in a sane way. My thinking was that some day we may end up with a nice set of attributes for annotating code stripping that we could then polish up and expose to our users.

I'll go into details about the attributes we internally support as well as some other ideas I've only jotted down to give everyone an idea of what an ideal set of attributes could look like that would satisfy the many different types of users that are out there.

[RequiredMember]

When the DeclaringType is marked so are any members with this attribute. Pros of this attribute are that it avoids putting signatures into strings. Cons are that it can certainly lead to unnecessary preservations. For members that are part of some reflection design that the linker cannot detect the possibility of over preserving may not be a concern.

UnityLinker does support this attribute, however, it is not officially supported in Unity.

[RequiredInterface(Type)]

Allowed on a Type. When the type is marked, so will all InterfaceImplementations of the specified type. Generally an attribute like this shouldn't be necessary. However, there is at least one api doesn't play nice with the linkers interface stripping behavior.

        [Test]
        public void IsAssignableFromWithInterface()
        {
            var ibaseType = typeof(IBase);
            var baseType = typeof(Base);
            var derivedType = typeof(Derived);

            if (ibaseType.IsAssignableFrom(baseType))
                Trace("Success 1");
            if (ibaseType.IsAssignableFrom(derivedType))
                Trace("Success 2");
            if (!baseType.IsAssignableFrom(ibaseType))
                Trace("Success 3");
            if (!derivedType.IsAssignableFrom(ibaseType))
                Trace("Success 4");
        }

UnityLinker does support this attribute, however, it is not officially supported in Unity.

[RequireAttributeUsages]

Can be placed on classes, although it will only have an effect if placed on an attribute class. This attribute tells the linker to mark all CustomAttributes of this type no matter what. The use case for this one is commonly when code external to the assemblies being linked are going to inspect the assemblies and look for certain attributes. In this scenario, combine with the linkers --used-atts-only option, attributes like this can be stripped away leading to w/e is post processing the assemblies and looking for attributes not working.

UnityLinker does support this attribute, however, it is not officially supported in Unity.

[AlwaysLinkAssembly]

This is an officially supported attribute in Unity. See documentation here https://docs.unity3d.com/Manual/ManagedCodeStripping.html

This is likely Unity specific, but I thought I'd mention it in case someone encounters a need that feels similar.

Now onto the attributes that I've merely jotted down as ideas but have never implemented support for.

Disclaimer. These are all just ideas. Some of the names may not be great 😄

[InstanceCreated]

This attribute is similar to what @stephentoub is requesting in this issue.

    /// <summary>
    /// Used to indicate that an instance of the type will exist if the type is marked
    /// </summary>
    [AttributeUsage(AttributeTargets.Class)]
    public class InstanceCreatedAttribute : Attribute
    {
    }

I believe my original motivation was related to the uninstantiated type work. I don't know if this attribute should cause the default ctor to be marked or not. I think it could help with the "most people don't realize the TypeDefinition is separate from the default ctor issue".

[PreserveMembersIfTypeMarked]

Once a TypeDefinition is marked, mark all of it's members. This one is catered toward end users. Maybe they have a heavy reflection that applies to a class. It could also act as a wide'ish net to start casting in order to find a stripping problem. Recall that a lot of developers quickly resort to preserving the entire assembly via link.xml and then may never come back to optimize. This attribute is at least a little more granular.

I made a note about optionally having some member level granularity exposed. I.e. "Preserve all fields if the type is marked" or "Preserve all private methods if the type is marked". That might be overkill. I don't have an specific example where this was necessary.

[RequireImplementors]

Can be added to an interface type. All classes, structs, and interfaces implementing this interface will be marked if the interface type is marked. A request for something like this has come up numerous times as a way to handle coding patterns where all types implementing some interface are collected and something is done with them. Maybe create instance via reflection and then call some interface method.

[RequireNestedType]

Can be placed on a type to indicate that when the type is marked the specified nested type should also be marked. I never had a request where this was needed. I came up with the idea for the sake of completeness when I implemented support for [RequireMember].

RequireSubclasses

If the type is marked, all derived types will be marked as well. Useful during the same pattern as [RequireImplementors] only for base types.

[PreserveMembers]

All members of the type are preserved no matter what. An extension of [Preserve] that is easier to use. I've seen code where people have put [Preserve] on a majority of a types methods. This would streamline that users workflow and make their life easier. They probably just wanted stuff to survive stripping and then move on to w/e they actually care about.

[CreatesInstance(Type)]

Can be placed on a method. Signals that a method creates an instance of the specified type. Scenario where this might be desired is an icall that will create an instance of a type from native code. I think I also jotted this down when I was adding the instantiated type tracking to the linker. So I'm sure another motivation for the idea was as a way to by pass the optimizations the linker makes when it determines a type is never instantiated.

[SuppressReflectionWarning]

Could be placed on a method to tell the linker to not issue a warning about a reflection usage it could not figure out. I think it would be interesting if we ever got to a point where the linker would issue a warning on all undetectable reflection and calls into native code that did not have some sort of attribute on them telling the linker "I know you can't figure out something here, it's OK, I've added the proper annotations to ensure the dependencies of this method will be marked". We could then maybe get to a point where we could "know" (assuming you can trust the suppression's) that all dependencies of a linked program were marked.

There's a brain dump of the ideas I've accumulated. I've never sat down and thought about all the different scenarios developers may encounter. I've just collected ideas as I've encountered issues. I think it would be great if someone took some time to come up with a thought out set of attribute API's that would satisfy the entire .net ecosystem.

I've attached the project where I was collecting ideas and some of our linker tests for attributes we do support. I tried to summarize as much as I could here, but there might be a few more insights to find in these.

StrippingAttributePrototyping.zip
StrippingControls.Balanced.zip

[rynowak]

To add to this, here's some recent analysis I did on Blazor's usage scenarios: dotnet/aspnetcore#17022

TLDR - today we run the linker on the BCL only, we want to run it on all Blazor's assemblies as well, and possibly user-code (opt-in) in the future. The things that break Blazor today are mostly the removal of property accessors and constructors, which are not big contributors to size. Having the ability to configure the granularity of marking via command line switches or attributes would really help us solve this in a maintainable way.

[mrvoorhe]

For the sake of record keeping, another idea came up in #854 (comment)

[RequireMembersWithAttribute]

You would put this on the attribute type and the linker would mark all members that have this attribute. We could add some sort of optional enum for controlling what should trigger the marking. i.e. Always or ParentMarked.

This would be useful for things like [JSinterop], [JsonProperty] (Newtonsoft.Json), and [Serialized] (UnityEngine).

[marek-safar]

PreserveDependencyAttribute has been superseded with DynamicDependencyAttribute so closing this issue.