Trim and AOT warnings in Delegates and PerTypeHelpers

[eerhardt]

In trying to use Microsoft.Extensions.Caching.StackExchangeRedis in a Native AOT app (see dotnet/aspnetcore#45910), I'm getting the following warnings from these two places:

Pipelines.Sockets.Unofficial/src/Pipelines.Sockets.Unofficial/Delegates.cs

Lines 37 to 58 in f044f67

	private static readonly Func<MulticastDelegate, object> s_getArr = GetGetter<object>("_invocationList");
	private static readonly Func<MulticastDelegate, IntPtr> s_getCount = GetGetter<IntPtr>("_invocationCount");
	private static readonly bool s_isAvailable = s_getArr != null & s_getCount != null;
	internal static bool IsAvailable => s_isAvailable;
	private static Func<MulticastDelegate, T> GetGetter<T>(string fieldName)
	{
	try
	{
	var field = typeof(MulticastDelegate).GetField(fieldName, BindingFlags.NonPublic | BindingFlags.Instance);
	if (field == null || field.FieldType != typeof(T)) return null;
	#if !NETSTANDARD2_0
	try // we can try use ref-emit
	{
	var dm = new DynamicMethod(fieldName, typeof(T), new[] { typeof(MulticastDelegate) }, typeof(MulticastDelegate), true);
	var il = dm.GetILGenerator();
	il.Emit(OpCodes.Ldarg_0);
	il.Emit(OpCodes.Ldfld, field);
	il.Emit(OpCodes.Ret);
	return (Func<MulticastDelegate, T>)dm.CreateDelegate(typeof(Func<MulticastDelegate, T>));
	}

/_/src/Pipelines.Sockets.Unofficial/Delegates.cs(52): AOT analysis warning IL3050: Pipelines.Sockets.Unofficial.Delegates.GetGetter<T>(String): Using member 'System.Reflection.Emit.DynamicMethod.DynamicMethod(String,Type,Type[],Type,Boolean)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Creating a DynamicMethod requires dynamic code. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]

This looks simply like we can just check for RuntimeFeature.IsDynamicCodeSupported and fallback to reflection when it isn't supported. But the bigger issue is that _invocationList and _invocationCount don't appear to be fields of MulticastDelegate in Native AOT: https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/System.Private.CoreLib/src/System/MulticastDelegate.cs. So I'm not sure how to fix this.

Pipelines.Sockets.Unofficial/src/Pipelines.Sockets.Unofficial/Arenas/PerTypeHelpers.cs

Lines 13 to 47 in f044f67

	static Allocator<T> Calculate()
	{
	if (IsBlittable)
	{
	try
	{
	typeof(UnmanagedAllocator<>).MakeGenericType(typeof(T))
	.GetProperty(nameof(UnmanagedAllocator<int>.Shared))
	.GetValue(null);
	}
	catch { }
	}
	return PreferPinned(); // safe fallback
	}
	}
	public static Allocator<T> PreferPinned()
	{
	return _preferPinned ??= Calculate();
	static Allocator<T> Calculate()
	{
	if (IsBlittable)
	{
	try
	{
	typeof(PinnedArrayPoolAllocator<>).MakeGenericType(typeof(T))
	.GetProperty(nameof(PinnedArrayPoolAllocator<int>.Shared))
	.GetValue(null);
	}
	catch { }
	}
	return ArrayPoolAllocator<T>.Shared; // safe fallback
	}
	}

/_/src/Pipelines.Sockets.Unofficial/Arenas/PerTypeHelpers.cs(39): Trim analysis warning IL2090: Pipelines.Sockets.Unofficial.Arenas.PerTypeHelpers`1.<PreferPinned>g__Calculate|3_0(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Type.MakeGenericType(Type[])'. The generic parameter 'T' of 'Pipelines.Sockets.Unofficial.Arenas.PerTypeHelpers`1' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/Pipelines.Sockets.Unofficial/Arenas/PerTypeHelpers.cs(39): AOT analysis warning IL3050: Pipelines.Sockets.Unofficial.Arenas.PerTypeHelpers`1.<PreferPinned>g__Calculate|3_0(): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/Pipelines.Sockets.Unofficial/Arenas/PerTypeHelpers.cs(19): Trim analysis warning IL2090: Pipelines.Sockets.Unofficial.Arenas.PerTypeHelpers`1.<PreferUnmanaged>g__Calculate|2_0(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Type.MakeGenericType(Type[])'. The generic parameter 'T' of 'Pipelines.Sockets.Unofficial.Arenas.PerTypeHelpers`1' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/Pipelines.Sockets.Unofficial/Arenas/PerTypeHelpers.cs(19): AOT analysis warning IL3050: Pipelines.Sockets.Unofficial.Arenas.PerTypeHelpers`1.<PreferUnmanaged>g__Calculate|2_0(): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]

Maybe these can be wrapped in RuntimeFeature.IsDynamicCodeSupported? I'm a bit confused by this code because it isn't actually returning the result of invoking those properties through reflection.

cc @mgravell

[mgravell]

So; you're absolutely right about the allocater thing; that was... just bad code, very embarrassing. I'm torn now; do I fix it, or do I deprecate the scenario on the basis that it has literally never worked, so... does it matter? but now I need to go check "are we relying on that pin", because if we are: GODAMMIT

on the delegates thing: no additional change should be required - it already handles the "no field" scenario

I've added a work-in-progress tidy up that also adds an AOT smoke test project: #74

I'm going to see whether SE.Redis relies on that flag before deciding how to proceed on the allocator thing

[mgravell]

reopening to continue discussion; initial fixes: look good and merged

[mgravell]

SE.Redis doesn't use the Delegates helper or the flags, so: no urgency on resolving those I'm going to merge my fixes "as is" for now, and see if we can get a step further (we seem to be actively discussing it, let's finish that conversation first)