Linker hints for C++/CLI dependencies

[vatsan-madhavan]

Fixes #1385

When producing ReadyToRun images, the ILLinker is configured to skip C++/CLI images. See dotnet/linker#651 and dotnet/linker#658.

In turn, this results in a failure of dependencies of such assemblies (like System.Diagnostics.Debug.dll, which is required by DirectWriteForwarder.dll) from being identified and included in the ReadyToRun images.

These linker hints tell ILLinker to include certain dependencies, which we know to be required by DirectWriteForwarder and System.Printing respectively.

Also related: dotnet/linker#675

[vatsan-madhavan]

/cc @sbomer

[vatsan-madhavan]

/cc @fadimounir

I don't see this fixing the orignal problem @sbomer. The xml files get embedded into PresentationCore & ReachFramework respectively. The published apps continue to fail as before - FileNotFoundException missing System.Diagnostics.Debug

This is a screenshot of assemblies from within the temporary unzipped location, while running the published app under the debugger.

[sbomer]

I confirmed that the linker fix in dotnet/linker#675 keeps System.Diagnostics.Debug. @vatsan-madhavan do you think we should also root netstandard and mscorlib? I saw them referenced from the following (but haven't actually seen a failure related to these references):

from DirectWriteForwarder:
[mscorlib]System.Runtime.CompilerServices.AssemblyAttributesGoHere
[mscorlib]System.Runtime.CompilerServices.AssemblyAttributesGoHereM
[netstandard]System.URi

from System.Printing:
[mscorlib]System.Runtime.CompilerServices.AssemblyAttributesGoHere
[mscorlib]System.Runtime.CompilerServices.AssemblyAttributesGoHereM
[mscorlib]System.Security.SuppressUnmanagedCodeSecurityAttribute
[netstandard]System.IO.FileMode

Otherwise the change LGTM.

[vatsan-madhavan]

[mscorlib]System.Runtime.CompilerServices.AssemblyAttributesGoHere
[mscorlib]System.Runtime.CompilerServices.AssemblyAttributesGoHereM

I believe these are bad type-refs. There are no such types in mscorlib. In addition to these, there are a couple more bad type-refs (unrelated to this issue)

[System.Runtime]System.Runtime.CompilerServices.SuppressMergeCheckAttribute
[System.Runtime]System.Runtime.CompilerServices.DecoratedNameAttribute
[System.Runtime]System.Runtime.CompilerServices.AssemblyAttributesGoHere

[vatsan-madhavan]

/cc @ericstj, do you have thoughts on whether we should include mscorlib and netstandard as well?

[sbomer]

Are the netstandard refs also bad? I'm seeing the following in MS.Internal.TrueTypeSubsetter.ComputeSubset:

IL_0063:  newobj     instance void [System.IO.Packaging]System.IO.FileFormatException::.ctor(class [netstandard_152]System.Uri)

[vatsan-madhavan]

Are the netstandard refs also bad? I'm seeing the following in MS.Internal.TrueTypeSubsetter.ComputeSubset:

IL_0063:  newobj     instance void [System.IO.Packaging]System.IO.FileFormatException::.ctor(class [netstandard_152]System.Uri)

That looks fine to me. I see a TypeForwardedTo(typeof(Uri)) in netstandard, pointing to System.Runtime.

[sbomer]

LGTM. Thank you for taking care of this!

[ghost]

Hello @vatsan-madhavan!

Because this pull request has the auto_merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[ericstj]

I'm not a fan of this hack. What keeps this list up to date? The linker should be able to traverse the C++/CLI assembly without rewriting it.

[sbomer]

Not a fan either, and I definitely hope to analyze them in the future and remove the roots. The alternative that I see to adding these roots now, short of fixing dotnet/linker#639, is to document a requirement for pretty much every linked WPF app to include these roots. Thoughts?

Also, @vatsan-madhavan, based on discussion with @marek-safar it probably makes sense to make these embedded roots more specific, to only root the types that are needed by these assemblies. I need to check the behavior of type forwards, but I think we'll want to update them.

[sbomer]

Another alternative would be to include roots for System.Diagnostics.Debug but not netstandard/mscorlib. System.Diagnostics.Debug was being loaded for every app that renders text (see dotnet/linker#675 (comment)), but I wasn't seeing any failures for the mscorlib/netstandard dependencies, so they may be less common.