ResourceManager contains code which ILLink can't analyze

[vitek-karas]

The implementation of ResourceManager has a code paths that uses Type.GetType and Activator.CreateInstance on types that are not known by the linker, effectively making all ResourceManager.GetString calls potentially linker-unsafe. This in turn means that applications using ILLink during publish may end up broken at runtime due to missing assemblies/types/members.

Creation of ResourceSet and IResourceReader

ManifestBasedResourceGroveler.CreateResourceSet reads strings from the resource stream to get the type names of a resource manager and resource set to create. If they are "System.Resources.ResourceReader" and "System.Resources.RuntimeResourceSet", which is likely the most common case by far, it simply creates a RuntimeResourceSet without reflection. If not, then it will use Type.GetType to get the types specified by name in the stream, and call Activator.CreateInstance on them. The resource reader instance gets cast to IResourceReader, and the resource set instance gets cast to ResourceSet. For the resource set, it may also instead use a user-specified System.Type that was passed to the ResourceManager constructor (userResourceSet), which takes precedence over the type specified in the resource stream and get instantiated via Activator.CreateInstance.

Without knowledge of the resource stream types, the linker can't tell if this is safe as the resource set or resource reader types may not be referenced anywhere else in the application's code. As such linker would tret them as dead code and remove them.

Creating instances of any type specified in the resource stream by its name

The default resource reading path will call Type.GetType on a type name retrieved from the resource stream, even for reading strings.

The default path for ResourceManager.GetString or ResourceManager.GetObject (which uses RuntimeResourceSet and ResourceReader) uses a shared helper, RuntimeResourceSet.GetObject. This fans out into three different implementations: one for reading strings (ResourceReader.LoadString), and two for objects (ResourceReader.LoadObject uses LoadObjectV1 or LoadObjectV2 depending on the version of the resource stream). All of these call into FindType, which does Type.GetType on a type name retrieved from the resource stream:

• LoadString calls FindType, and checks that the found type was typeof(string) before reading a string from the resource stream. This could possibly be factored so that the GetString path does not use Type.GetType, but just checks the string in the stream. Reading strings is probably the most common use case.
• LoadObjectV1 similarly calls FindType, and compares the found type to a few well-known types before falling back to the general case (DeserializeObject)
• LoadObjectV2 has an optimization that first reads a type code from the resource stream, and compares it to well-known type codes for built-in types. These paths do not rely on reflection. Only if the type code is not a known type does it call DeserializeObject, which in turn calls FindType.

Deserialization when reading resources

Reading objects from a resource stream relies on BinaryFormatter.Deserialize for all but a few special-cased types. BinaryFormatter is lazily initialized using unsafe reflection.

The ResourceReader constructor takes a parameter that can disable the deserialization path, but it looked to me like deserialization was enabled by default. I might be missing something here - I didn't check whether BinaryFormatter is somehow disabled in .NET Core, so maybe this is a light-up scenario.

The deserialization process itself is not linker safe as it reads types names from the stream and creates instances of those objects. Also it will set values on properties/fields which may not be accessed otherwise and thus removed by the linker.

Using reflection to create BinaryFormatter

Because the code is in CoreLib and the BinaryFormatter is implemented in System.Runtime.Serialization.Formatters there's no direct compile time dependency. Instead the BinaryFormatter is created via reflection in ResourceReader.InitializeBinaryFormatter. The way the code is written makes it not analyzable by linker as it won't see through storing values to static variables and closures. Overall this can probably be made linker-safe if property annotated with PreserveDependency attributes.

(Kudos to @sbomer for most of the analysis on this problem)

[eerhardt]

In order to fix this, I think we will need to teach ILLink how to read .resources streams. ILLink can then inspect which types are present in the resource, and mark those types as "necessary".

Using reflection here is a key to the design of System.Resources.Extensions, which is used by WinForms/WPF when putting Images and other types as resources. (Note, it can be used by any .NET Core app, it isn't limited to WinForms/WPF.) The System.Resources.Extensions.DeserializingResourceReader type name is written into the .resources file, and then at runtime, ResourceManager reads this type name to create the "extension" IResourceReader.

I'm not sure how much work it will be to teach ILLink how to read .resources streams. It shouldn't be too hard to do it. I think the hardest part may be engineering the code so we don't have a bunch of duplication.

/cc @ericstj @tarekgh

Using reflection to create BinaryFormatter

This is a pattern used throughout .NET Core in order to break dependency cycles. So we will probably need to come up with a generalized pattern here. Another example of this same pattern is in System.Security.Cryptography.Algorithms which needs to use Xml parsing.

runtime/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/XmlKeyHelper.cs

Line 268 in 7a362a0

private static readonly Type s_xDocument = Type.GetType("System.Xml.Linq.XDocument, System.Private.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51")!;

[ericstj]

It'd need to understand the formats used by ResourceReader as well as DeserializingResourceReader.

Both formats can represent types in resources.

Here's a sample I wrote a while back to demonstrate reading the types out of a ResourceReader stream: https://gist.github.com/ericstj/b0e8537f29dabdd73f726e8fee0f49d6

For ResourceReader & DeserializingResourceReader (binary format case) it would need to root the type, and whatever serialization constructor it had, as well as BinaryFormatter.

For DeserializingResourceReader there are also cases where we'd need to root the TypeConverter.

FWIW I expect this problem to be similar to Baml, though Baml has a much more complex format along with hundreds of special cases

[jkotas]

Note that the BinarySerializer path is only needed by legacy workloads (e.g. WinForms). It should not be needed by modern workloads (e.g. Blazor) unless they take dependency on a legacy component.

We should have a switch to disable use of BinarySerializer. This will solve the binary serializer problem for both ResourceReader and everything else going forward. This switch should be turned on by default (ie even without IL linker) for all modern workloads that are not expected to need BinarySerializer.

This switch is goodness for security hardening against binary serializer attacks (cc @GrabYourPitchforks). I expect that we would make it a best practice to turn this on for cloud services.

The work you are describing above is only needed for legacy workloads that need BinarySerializer. I believe that these workloads end up being under the cut line for "works great with IL linker" for .NET 5.

[eerhardt]

The work you are describing above is only needed for legacy workloads that need BinarySerializer. I believe that these workloads end up being under the cut line for "works great with IL linker" for .NET 5.

Which workloads would be above the cut line for "works great with IL linker" for .NET 5? I'm assuming Blazor and Xamarin are. What about web / micro services?

How about WPF on .NET 5? Are we enabling developers building single file Windows applications to trim their application?

For Blazor and Xamarin, my understanding is that those workloads don't use System.Resources.ResourceManager.

[jkotas]

Blazor and Xamarin are. What about web / micro services?

Yes, this sounds about right. We are still working through the exact web / micro services to target.

For Blazor and Xamarin, my understanding is that those workloads don't use System.Resources.ResourceManager.

They do use it for strings. Every other netcoreapp library that we ship uses ResourceManager for strings.