[NativeAOT] Delegate bug fixes

- Fix Delegate.Method and Delegate.Target for marshalled delegates
- Add tests and fixes for corner various delegate corner case behaviors
- Delete runtime test for GetInvocationList since there is a better test coverage for this API under libraries

Fixes dotnet/runtimelab#164

src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs

@@ -138,11 +138,11 @@ public static Delegate CreateDelegate(RuntimeTypeHandle typeHandleForDelegate, I
  }
 
  //
- // Helper to extract the artifact that uniquely identifies a method in the runtime mapping tables.
+ // Helper to extract the artifact that identifies a reflectable delegate target in the runtime mapping tables.
  //
- public static IntPtr GetDelegateLdFtnResult(Delegate d, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver, out bool isInterpreterEntrypoint)
+ public static IntPtr GetDelegateLdFtnResult(Delegate d, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver)
  {
- return d.GetFunctionPointer(out typeOfFirstParameterIfInstanceDelegate, out isOpenResolver, out isInterpreterEntrypoint);
+ return d.GetDelegateLdFtnResult(out typeOfFirstParameterIfInstanceDelegate, out isOpenResolver);
  }
 
  // Low level method that returns the loaded modules as array. ReadOnlySpan returning overload

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs

@@ -56,7 +56,7 @@ public static unsafe IntPtr GetFunctionPointerForDelegate(Delegate del)
  throw new ArgumentException(SR.Argument_NeedNonGenericType, "delegate");
 #pragma warning restore CA2208
 
- NativeFunctionPointerWrapper? fpWrapper = del.Target as NativeFunctionPointerWrapper;
+ NativeFunctionPointerWrapper? fpWrapper = del.TryGetNativeFunctionPointerWrapper();
  if (fpWrapper != null)
  {
  //
@@ -104,64 +104,64 @@ internal unsafe struct ThunkContextData
  public IntPtr FunctionPtr; // Function pointer for open static delegates
  }
 
- internal sealed class PInvokeDelegateThunk
+ internal sealed unsafe class PInvokeDelegateThunk
  {
- public IntPtr Thunk; // Thunk pointer
- public IntPtr ContextData; // ThunkContextData pointer which will be stored in the context slot of the thunk
+ public readonly IntPtr Thunk; // Thunk pointer
+ public readonly IntPtr ContextData; // ThunkContextData pointer which will be stored in the context slot of the thunk
 
  public PInvokeDelegateThunk(Delegate del)
  {
-
  Thunk = RuntimeAugments.AllocateThunk(s_thunkPoolHeap);
- Debug.Assert(Thunk != IntPtr.Zero);
-
  if (Thunk == IntPtr.Zero)
  {
- // We've either run out of memory, or failed to allocate a new thunk due to some other bug. Now we should fail fast
- Environment.FailFast("Insufficient number of thunks.");
+ throw new OutOfMemoryException();
  }
- else
- {
- //
- // Allocate unmanaged memory for GCHandle of delegate and function pointer of open static delegate
- // We will store this pointer on the context slot of thunk data
- //
- unsafe
- {
- ContextData = (IntPtr)NativeMemory.Alloc((nuint)(2 * IntPtr.Size));
 
- ThunkContextData* thunkData = (ThunkContextData*)ContextData;
+ //
+ // For open static delegates set target to ReverseOpenStaticDelegateStub which calls the static function pointer directly
+ //
+ IntPtr openStaticFunctionPointer = del.TryGetOpenStaticFunctionPointer();
 
- // allocate a weak GChandle for the delegate
- thunkData->Handle = GCHandle.Alloc(del, GCHandleType.Weak);
+ //
+ // Allocate unmanaged memory for GCHandle of delegate and function pointer of open static delegate
+ // We will store this pointer on the context slot of thunk data
+ //
+ unsafe
+ {
+ ContextData = (IntPtr)NativeMemory.Alloc((nuint)(2 * IntPtr.Size));
 
- // if it is an open static delegate get the function pointer
- if (del.IsOpenStatic)
- thunkData->FunctionPtr = del.GetFunctionPointer(out RuntimeTypeHandle _, out bool _, out bool _);
- else
- thunkData->FunctionPtr = default;
- }
+ ThunkContextData* thunkData = (ThunkContextData*)ContextData;
+
+ // allocate a weak GChandle for the delegate
+ thunkData->Handle = GCHandle.Alloc(del, GCHandleType.Weak);
+ thunkData->FunctionPtr = openStaticFunctionPointer;
  }
+
+ IntPtr pTarget = RuntimeInteropData.GetDelegateMarshallingStub(new RuntimeTypeHandle(del.GetMethodTable()), openStaticFunctionPointer != IntPtr.Zero);
+ Debug.Assert(pTarget != IntPtr.Zero);
+
+ RuntimeAugments.SetThunkData(s_thunkPoolHeap, Thunk, ContextData, pTarget);
  }
 
  ~PInvokeDelegateThunk()
  {
  // Free the thunk
- RuntimeAugments.FreeThunk(s_thunkPoolHeap, Thunk);
- unsafe
+ if (Thunk != IntPtr.Zero)
+ {
+ RuntimeAugments.FreeThunk(s_thunkPoolHeap, Thunk);
+ }
+
+ if (ContextData != IntPtr.Zero)
  {
- if (ContextData != IntPtr.Zero)
+ // free the GCHandle
+ GCHandle handle = ((ThunkContextData*)ContextData)->Handle;
+ if (handle.IsAllocated)
  {
- // free the GCHandle
- GCHandle handle = ((ThunkContextData*)ContextData)->Handle;
- if (handle.IsAllocated)
- {
- handle.Free();
- }
-
- // Free the allocated context data memory
- NativeMemory.Free((void*)ContextData);
+ handle.Free();
  }
+
+ // Free the allocated context data memory
+ NativeMemory.Free((void*)ContextData);
  }
  }
  }
@@ -179,19 +179,7 @@ private static unsafe PInvokeDelegateThunk AllocateThunk(Delegate del)
  Debug.Assert(s_thunkPoolHeap != null);
  }
 
- var delegateThunk = new PInvokeDelegateThunk(del);
-
- //
- // For open static delegates set target to ReverseOpenStaticDelegateStub which calls the static function pointer directly
- //
- bool openStaticDelegate = del.IsOpenStatic;
-
- IntPtr pTarget = RuntimeInteropData.GetDelegateMarshallingStub(new RuntimeTypeHandle(del.GetMethodTable()), openStaticDelegate);
- Debug.Assert(pTarget != IntPtr.Zero);
-
- RuntimeAugments.SetThunkData(s_thunkPoolHeap, delegateThunk.Thunk, delegateThunk.ContextData, pTarget);
-
- return delegateThunk;
+ return new PInvokeDelegateThunk(del);
  }
 
  /// <summary>

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Extensions/NonPortable/DelegateMethodInfoRetriever.cs

@@ -17,20 +17,7 @@ public static class DelegateMethodInfoRetriever
  {
  public static MethodInfo GetDelegateMethodInfo(Delegate del)
  {
- Delegate[] invokeList = del.GetInvocationList();
- del = invokeList[invokeList.Length - 1];
- IntPtr originalLdFtnResult = RuntimeAugments.GetDelegateLdFtnResult(del, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver, out bool isInterpreterEntrypoint);
-
- if (isInterpreterEntrypoint)
- {
- // This is a special kind of delegate where the invoke method is "ObjectArrayThunk". Typically,
- // this will be a delegate that points the LINQ Expression interpreter. We could manufacture
- // a MethodInfo based on the delegate's Invoke signature, but let's just throw for now.
- throw new NotSupportedException(SR.DelegateGetMethodInfo_ObjectArrayDelegate);
- }
-
- if (originalLdFtnResult == (IntPtr)0)
- return null;
+ IntPtr originalLdFtnResult = RuntimeAugments.GetDelegateLdFtnResult(del, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver);
 
  QMethodDefinition methodHandle = default(QMethodDefinition);
  RuntimeTypeHandle[] genericMethodTypeArgumentHandles = null;
@@ -79,11 +66,7 @@ public static MethodInfo GetDelegateMethodInfo(Delegate del)
  throw new NotSupportedException(SR.Format(SR.DelegateGetMethodInfo_NoDynamic_WithDisplayString, methodDisplayString));
  }
  }
- MethodBase methodBase = ExecutionDomain.GetMethod(typeOfFirstParameterIfInstanceDelegate, methodHandle, genericMethodTypeArgumentHandles);
- MethodInfo methodInfo = methodBase as MethodInfo;
- if (methodInfo != null)
- return methodInfo;
- return null; // GetMethod() returned a ConstructorInfo.
+ return (MethodInfo)ExecutionDomain.GetMethod(typeOfFirstParameterIfInstanceDelegate, methodHandle, genericMethodTypeArgumentHandles);
  }
  }
 }

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx

@@ -204,9 +204,6 @@
  <data name="DelegateGetMethodInfo_NoInstantiation" xml:space="preserve">
  <value>Cannot retrieve a MethodInfo for this delegate because the necessary generic instantiation was not metadata-enabled.</value>
  </data>
- <data name="DelegateGetMethodInfo_ObjectArrayDelegate" xml:space="preserve">
- <value>Cannot retrieve a MethodInfo for this delegate because the delegate target is an interpreted LINQ expression.</value>
- </data>
  <data name="Arg_InterfaceMapMustNotBeAbstract" xml:space="preserve">
  <value>Could not retrieve the mapping of the interface '{0}' on type '{1}' because the type implements the interface abstractly.</value>
  </data>

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/MulticastDelegateTests.cs

@@ -66,6 +66,21 @@ public static void EqualsTest()
  Assert.Equal(d1.GetHashCode(), d1.GetHashCode());
  }
 
+ [Fact]
+ public static void ArrayDelegates()
+ {
+ // Delegate implementation may use Delegate[] arrays as sentinels. Validate that
+ // the sentinels are not confused with user provided targets.
+
+ Action da = new Delegate[5].MyExtension;
+ Assert.True(da.HasSingleTarget);
+ Assert.Equal(1, da.GetInvocationList().Length);
+
+ Func<int, int> dd = new Delegate[10].GetLength;
+ Assert.True(dd.HasSingleTarget);
+ Assert.Equal(1, dd.GetInvocationList().Length);
+ }
+
  [Fact]
  public static void CombineReturn()
  {
@@ -199,7 +214,7 @@ private static void CheckInvokeList(D[] expected, D combo, Tracker target)
  {
  CheckIsSingletonDelegate((D)(expected[i]), (D)(invokeList[i]), target);
  }
- Assert.Same(combo.Target, expected[expected.Length - 1].Target);
+ Assert.Same(combo.Target, expected[^1].Target);
  Assert.Same(combo.Target, target);
  Assert.Equal(combo.HasSingleTarget, invokeList.Length == 1);
  int count = 0;
@@ -209,6 +224,7 @@ private static void CheckInvokeList(D[] expected, D combo, Tracker target)
  count++;
  }
  Assert.Equal(count, invokeList.Length);
+ Assert.Equal(combo.Method, invokeList[^1].Method);
  }
 
  private static void CheckIsSingletonDelegate(D expected, D actual, Tracker target)
@@ -283,4 +299,11 @@ private class C
  public string Goo(int x) => new string('A', x);
  }
  }
+
+ static class MulticastDelegateTestsExtensions
+ {
+ public static void MyExtension(this Delegate[] delegates)
+ {
+ }
+ }
 }

src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj

@@ -1,9 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
- <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
- <RequiresProcessIsolation>true</RequiresProcessIsolation>
  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <CLRTestPriority>1</CLRTestPriority>
+ <CLRTestPriority>0</CLRTestPriority>
  </PropertyGroup>
  <ItemGroup>
  <Compile Include="delegatecombine1.cs" />

src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj

@@ -1,9 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
- <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
- <RequiresProcessIsolation>true</RequiresProcessIsolation>
  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <CLRTestPriority>1</CLRTestPriority>
+ <CLRTestPriority>0</CLRTestPriority>
  </PropertyGroup>
  <ItemGroup>
  <Compile Include="delegatecombineimpl.cs" />

src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj

@@ -1,9 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
- <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
- <RequiresProcessIsolation>true</RequiresProcessIsolation>
  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <CLRTestPriority>1</CLRTestPriority>
  </PropertyGroup>
  <ItemGroup>
  <Compile Include="delegateequals1.cs" />

src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj

@@ -1,9 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
- <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
- <RequiresProcessIsolation>true</RequiresProcessIsolation>
  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <CLRTestPriority>1</CLRTestPriority>
  </PropertyGroup>
  <ItemGroup>
  <Compile Include="delegategethashcode1.cs" />

src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj

@@ -1,9 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
- <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
- <RequiresProcessIsolation>true</RequiresProcessIsolation>
  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <CLRTestPriority>1</CLRTestPriority>
  </PropertyGroup>
  <ItemGroup>
  <Compile Include="delegateremove.cs" />

src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj

@@ -1,9 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
- <!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
- <RequiresProcessIsolation>true</RequiresProcessIsolation>
  <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
- <CLRTestPriority>1</CLRTestPriority>
  </PropertyGroup>
  <ItemGroup>
  <Compile Include="delegateremoveimpl.cs" />