Use SRM's TypeName parser in ILLink

[sbomer]

This replaces the type name parser we copied from corert with the one in System.Reflection.Metadata.

Adds a test for resolving escaped type names, shared by ILLink and Native AOT. To support this testcase, the test infra also uses TypeNameResolver. The ILLink tests use it via IVT, and the Native AOT tests use it by including source files. The sources have been factored a little to make it possible to use just the TypeNameResolver from NativeAot without pulling in other components of ILLink.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

[jkotas]

cc @adamsitnik @GrabYourPitchforks So far every single real-world use of TypeName.Parse except the one in NRBF overrides the low MaxNodes default with int.MaxValue. It makes me wonder whether we made the right choice for the default.

[sbomer]

FWIW I just copied this default from the type name parser used in Native AOT.

[jkotas]

int.MaxValue is the right value to use for tools like ILLinker. The tools like ILLinker are not hardened against too complex input. It is fine for these tools to crash with StackOverflowException or take a long time to run when the input is too complex.

[adamsitnik]

So far every single real-world use of TypeName.Parse except the one in NRBF overrides the low MaxNodes default with int.MaxValue. It makes me wonder whether we made the right choice for the default.

From my perspective, we wanted to be secure by default and this was the right choice.

IMO int.MaxValue is never the right value, as it is impossible to create such complex type names (runtime is unable to load them). For Runtime and its tools we should rather find a value that is large enough to allow for the most complex real-life types, but not big enough to cause SO.

[jkotas]

For Runtime and its tools we should rather find a value that is large enough to allow for the most complex real-life types, but not big enough to cause SO.

There is no point in doing that unless we make the runtime and tools hardened against too complex types. There are 1000 different ways how too complex types can introduce stackoverflow in the runtime and tools. Reducing that by one does not really help.

[adamsitnik]

Reducing that by one does not really help.

It does not solve the problem, but on the other hand it's almost free (the logic is implemented, it's just a matter of changing one value in few places) and it would not hurt.

But, I don't have the perspective of long term thinking about the .NET Runtime you have, so I am most probably missing a lot of details (like giving the users a false impression that Type.GetType(string) is hardened against all kinds of unsafe inputs)

[jtschuster]

LGTM aside from a couple test nits, thank you!

[jtschuster]

nit: Append to the errs list rather than failing immediately.

Suggested change

	if (!originalsTypeNameResolver.TryResolveTypeName (originalTargetAssembly, expectedTypeName, out TypeReference? expectedTypeRef, out _))
	Assert.Fail($"Could not resolve original type `{expectedTypeName}' in assembly {assemblyName}");
	TypeDefinition expectedType = expectedTypeRef.Resolve ();
	if (!originalsTypeNameResolver.TryResolveTypeName (originalTargetAssembly, expectedTypeName, out TypeReference? expectedTypeRef, out _)) {
	errs.Add($"Could not resolve original type `{expectedTypeName}' in assembly {assemblyName}");
	continue;
	}
	TypeDefinition expectedType = expectedTypeRef.Resolve ();

[sbomer]

I'd rather keep this one as an assert because it indicates a test authoring issue, and this matches what we do in some other places that fail to resolve something from the input.