Allow DynamicDependencyAttribute on Class and Struct

[eerhardt]

Background and Motivation

There are some "global" dynamic dependencies that can't be identified by managed IL methods, constructors, or fields. For example, some members on core types, like Task, ThreadPool, etc, are only necessary for debuggers to call. They are not called by any code in our libraries nor are they public. In order for the ILLinker to preserve these private members, we need to mark them with a [DynamicDependency], but the issue is there isn't a good place to put the attribute, since there are no callsites.

So instead, we need to place the [DynamicDependency] on the class/struct itself, meaning "if this type is preserved at all, then also preserve this member".

See the discussion here: #37288 (comment)

Proposed API

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(
-        AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Method,
+        AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Struct,
        AllowMultiple = true, Inherited = false)]
    public sealed class DynamicDependencyAttribute : Attribute

Usage Examples

namespace System.Threading.Tasks
{
    [DynamicDependency("get_ParentForDebugger")]
    [DynamicDependency("get_StateFlagsForDebugger")]
    [DynamicDependency("GetDelegateContinuationsForDebugger")]
    [DynamicDependency("SetNotificationForWaitCompletion")]
    [DynamicDependency("GetActiveTaskFromId")] // used by VS Tasks Window
    public class Task
    {
    ...
    }

Alternative Designs

Risks

We initially didn't want to allow [DynamicDependency] on a type itself because we were concerned that users could make a mistake, and too many members will be kept unnecessarily.

cc @vitek-karas @MichalStrehovsky @marek-safar @joperezr @layomia

[stephentoub]

I'd proposed this as well here:
dotnet/linker#797

[jkoritzinsky]

Is there any way we can also allow it on interfaces? That would help for interop scenarios when trying to project COM or WinRT interfaces to generated C#. C#/WinRT for example could use this to create more reliably linkable code even for interop scenarios without necessarily needing a tool to post-process the application to discover and hard-code the supported interop types.

[eerhardt]

Would adding support on interfaces help for more scenarios than COM?

See #36659 for more plans on how to support or trim COM.

[jkoritzinsky]

The COM support I mention would be more for out-of-band COM support. One example would be for an implementation of IDynamicInterfaceCastable (#36654) where we don't know all of the types we can cast to in advance, so the implementations have to be looked up via reflection. It's possible that the handling of CastableObjectImplementationAttribute (also described in the same issue) would also encapsulate this use case.

[MichalStrehovsky]

@jkoritzinsky Can you shed more light into the COM/WinRT support you are hoping to enable with this? CastableObjectImplementationAttribute is going to specifically mean "pretend this interface was implemented by an allocated object" - this will pretty much do the right thing for DynamicInterfaceCastable - e.g. if none of the interfaces the CastableObjectImplementation provides implementation for get used, none of the implementations get preserved - everything is still subject to normal treeshaking rules.

Replacing CastableObjectImplementation with DynamicDependency on the interface would mean the implementations get rooted even if the interface is not used - that's not desirable.

DynamicDependencyAttribute on a class or struct pretty much means that if I have a cast to the type, a local of that type, or field of that type, all the DynamicDependencies get preserved.

For the Task debugging support use case used as the example above, we would actually want to condition it on the Task being allocated - debugging support for tasks is not needed unless we have a task instance in the system. If Task didn't have a billon constructors, the fix would be to just put the DynamicDependency on the constructor.

[jkoritzinsky]

@MichalStrehovsky I think based on what you've said, the meaning of CastableObjectImplementationAttribute should cover the use case I'm thinking of.

[vitek-karas]

If Task didn't have a billon constructors, the fix would be to just put the DynamicDependency on the constructor.

Can we do this by adding a static method which gets called from all of the .ctors (nothing in it, so JIT will effectively remove it) and put the attribute on that one? From linkers point of view it should have the same effect as if it was on all the constructors.

[stephentoub]

Can we do this by adding a static method which gets called from all of the .ctors (nothing in it, so JIT will effectively remove it) and put the attribute on that one? From linkers point of view it should have the same effect as if it was on all the constructors.

I'd much rather we didn't. If we have to start playing such tricks to work around deficiencies in tools, we should instead fix the deficiencies in the tools. And these are sensitive code paths; the JIT should remove such a method when optimizations are enabled, but adding such additional code to such paths makes me nervous.

[eerhardt]

If Task didn't have a billon constructors, the fix would be to just put the DynamicDependency on the constructor.

I didn't list all the places that were needed, but another one is ThreadPool which is a static class that doesn't already have a static constructor.

runtime/src/coreclr/src/System.Private.CoreLib/ILLinkTrim.xml

Lines 19 to 23 in 13fbbee

	<type fullname="System.Threading.ThreadPool">
	<method name="GetQueuedWorkItemsForDebugger" />
	<method name="GetGloballyQueuedWorkItemsForDebugger" />
	<method name="GetLocallyQueuedWorkItemsForDebugger" />
	</type>

and Async*MethodBuilder* structs, where you can't explicitly define a parameterless constructor.

runtime/src/coreclr/src/System.Private.CoreLib/ILLinkTrim.xml

Lines 28 to 40 in 13fbbee

	<type fullname="System.Runtime.CompilerServices.AsyncMethodBuilderCore">
	<method name="TryGetStateMachineForDebugger" />
	</type>
	<type fullname="System.Runtime.CompilerServices.AsyncIteratorMethodBuilder">
	<property name="ObjectIdForDebugger" />
	</type>
	<type fullname="System.Runtime.CompilerServices.AsyncVoidMethodBuilder">
	<property name="ObjectIdForDebugger" />
	</type>
	<type fullname="System.Runtime.CompilerServices.AsyncTaskMethodBuilder*">
	<property name="ObjectIdForDebugger" />
	<method name="SetNotificationForWaitCompletion" />
	</type>

[marek-safar]

The struct case is the strongest motivation for me for this to be added

[MichalStrehovsky]

For the ThreadPool case - do we know under what conditions debugger accesses those members? If we condition this on the ThreadPool class being referenced, are we sure that the debugger is not going to crash when I e.g. open the Debug -> Windows -> Threads in an app that doesn't use threadpool at all?

[eerhardt]

Closing, as we are going to enable this scenario by using an XML descriptor + feature swtich. See dotnet/linker#1268 (comment) for discussion.