Skip to content

Commit cc17166

Browse files
jkotasjkotas
authored
[NativeAOT] Delegate bug fixes (#99185)
- Fix Delegate.Method and Delegate.Target for marshalled delegates - Add tests and fixes for various delegate corner case behaviors - Delete runtime test for GetInvocationList since there is a better test coverage for this API under libraries - Delete `m` prefix on System.Delegate member fields Fixes dotnet/runtimelab#164
1 parent 93e35a9 commit cc17166

File tree

26 files changed

+415
-620
lines changed

26 files changed

+415
-620
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,11 @@ public static Delegate CreateDelegate(RuntimeTypeHandle typeHandleForDelegate, I
138138
}
139139

140140
//
141-
// Helper to extract the artifact that uniquely identifies a method in the runtime mapping tables.
141+
// Helper to extract the artifact that identifies a reflectable delegate target in the runtime mapping tables.
142142
//
143-
public static IntPtr GetDelegateLdFtnResult(Delegate d, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver, out bool isInterpreterEntrypoint)
143+
public static IntPtr GetDelegateLdFtnResult(Delegate d, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver)
144144
{
145-
return d.GetFunctionPointer(out typeOfFirstParameterIfInstanceDelegate, out isOpenResolver, out isInterpreterEntrypoint);
145+
return d.GetDelegateLdFtnResult(out typeOfFirstParameterIfInstanceDelegate, out isOpenResolver);
146146
}
147147

148148
// Low level method that returns the loaded modules as array. ReadOnlySpan returning overload

‎src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,5 +149,16 @@ public static unsafe bool Compare(IntPtr functionPointerA, IntPtr functionPointe
149149

150150
return pointerDefA->MethodFunctionPointer == pointerDefB->MethodFunctionPointer;
151151
}
152+
153+
public static unsafe int GetHashCode(IntPtr functionPointer)
154+
{
155+
if (!IsGenericMethodPointer(functionPointer))
156+
{
157+
return functionPointer.GetHashCode();
158+
}
159+
160+
GenericMethodDescriptor* pointerDef = ConvertToGenericDescriptor(functionPointer);
161+
return pointerDef->GetHashCode();
162+
}
152163
}
153164
}

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs‎

Lines changed: 221 additions & 249 deletions
Large diffs are not rendered by default.

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeFunctionPointerWrapper.cs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,9 @@ internal abstract class NativeFunctionPointerWrapper
1212
{
1313
public NativeFunctionPointerWrapper(IntPtr nativeFunctionPointer)
1414
{
15-
m_nativeFunctionPointer = nativeFunctionPointer;
15+
NativeFunctionPointer = nativeFunctionPointer;
1616
}
1717

18-
private IntPtr m_nativeFunctionPointer;
19-
20-
public IntPtr NativeFunctionPointer
21-
{
22-
get { return m_nativeFunctionPointer; }
23-
}
18+
public IntPtr NativeFunctionPointer { get; }
2419
}
2520
}

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

Lines changed: 45 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public static unsafe IntPtr GetFunctionPointerForDelegate(Delegate del)
5656
throw new ArgumentException(SR.Argument_NeedNonGenericType, "delegate");
5757
#pragma warning restore CA2208
5858

59-
NativeFunctionPointerWrapper? fpWrapper = del.Target as NativeFunctionPointerWrapper;
59+
NativeFunctionPointerWrapper? fpWrapper = del.TryGetNativeFunctionPointerWrapper();
6060
if (fpWrapper != null)
6161
{
6262
//
@@ -104,64 +104,71 @@ internal unsafe struct ThunkContextData
104104
public IntPtr FunctionPtr; // Function pointer for open static delegates
105105
}
106106

107-
internal sealed class PInvokeDelegateThunk
107+
internal sealed unsafe class PInvokeDelegateThunk
108108
{
109-
public IntPtr Thunk; // Thunk pointer
110-
public IntPtr ContextData; // ThunkContextData pointer which will be stored in the context slot of the thunk
109+
public readonly IntPtr Thunk; // Thunk pointer
110+
public readonly IntPtr ContextData; // ThunkContextData pointer which will be stored in the context slot of the thunk
111111

112112
public PInvokeDelegateThunk(Delegate del)
113113
{
114-
115114
Thunk = RuntimeAugments.AllocateThunk(s_thunkPoolHeap);
116-
Debug.Assert(Thunk != IntPtr.Zero);
117-
118115
if (Thunk == IntPtr.Zero)
119116
{
120-
// We've either run out of memory, or failed to allocate a new thunk due to some other bug. Now we should fail fast
121-
Environment.FailFast("Insufficient number of thunks.");
117+
throw new OutOfMemoryException();
122118
}
123-
else
124-
{
125-
//
126-
// Allocate unmanaged memory for GCHandle of delegate and function pointer of open static delegate
127-
// We will store this pointer on the context slot of thunk data
128-
//
129-
unsafe
130-
{
131-
ContextData = (IntPtr)NativeMemory.Alloc((nuint)(2 * IntPtr.Size));
132119

133-
ThunkContextData* thunkData = (ThunkContextData*)ContextData;
120+
//
121+
// For open static delegates set target to ReverseOpenStaticDelegateStub which calls the static function pointer directly
122+
//
123+
IntPtr openStaticFunctionPointer = del.TryGetOpenStaticFunctionPointer();
124+
125+
//
126+
// Allocate unmanaged memory for GCHandle of delegate and function pointer of open static delegate
127+
// We will store this pointer on the context slot of thunk data
128+
//
129+
unsafe
130+
{
131+
ContextData = (IntPtr)NativeMemory.AllocZeroed((nuint)(2 * IntPtr.Size));
134132

135-
// allocate a weak GChandle for the delegate
136-
thunkData->Handle = GCHandle.Alloc(del, GCHandleType.Weak);
133+
ThunkContextData* thunkData = (ThunkContextData*)ContextData;
137134

138-
// if it is an open static delegate get the function pointer
139-
if (del.IsOpenStatic)
140-
thunkData->FunctionPtr = del.GetFunctionPointer(out RuntimeTypeHandle _, out bool _, out bool _);
141-
else
142-
thunkData->FunctionPtr = default;
143-
}
135+
// allocate a weak GChandle for the delegate
136+
thunkData->Handle = GCHandle.Alloc(del, GCHandleType.WeakTrackResurrection);
137+
thunkData->FunctionPtr = openStaticFunctionPointer;
144138
}
139+
140+
IntPtr pTarget = RuntimeInteropData.GetDelegateMarshallingStub(new RuntimeTypeHandle(del.GetMethodTable()), openStaticFunctionPointer != IntPtr.Zero);
141+
Debug.Assert(pTarget != IntPtr.Zero);
142+
143+
RuntimeAugments.SetThunkData(s_thunkPoolHeap, Thunk, ContextData, pTarget);
145144
}
146145

147146
~PInvokeDelegateThunk()
148147
{
149-
// Free the thunk
150-
RuntimeAugments.FreeThunk(s_thunkPoolHeap, Thunk);
151-
unsafe
148+
if (ContextData != IntPtr.Zero)
152149
{
153-
if (ContextData != IntPtr.Zero)
150+
// free the GCHandle
151+
GCHandle handle = ((ThunkContextData*)ContextData)->Handle;
152+
if (handle.IsAllocated)
154153
{
155-
// free the GCHandle
156-
GCHandle handle = ((ThunkContextData*)ContextData)->Handle;
157-
if (handle.IsAllocated)
154+
// If the delegate is still alive, defer finalization.
155+
if (handle.Target != null)
158156
{
159-
handle.Free();
157+
GC.ReRegisterForFinalize(this);
158+
return;
160159
}
161160

162-
// Free the allocated context data memory
163-
NativeMemory.Free((void*)ContextData);
161+
handle.Free();
164162
}
163+
164+
// Free the allocated context data memory
165+
NativeMemory.Free((void*)ContextData);
166+
}
167+
168+
// Free the thunk
169+
if (Thunk != IntPtr.Zero)
170+
{
171+
RuntimeAugments.FreeThunk(s_thunkPoolHeap, Thunk);
165172
}
166173
}
167174
}
@@ -179,19 +186,7 @@ private static unsafe PInvokeDelegateThunk AllocateThunk(Delegate del)
179186
Debug.Assert(s_thunkPoolHeap != null);
180187
}
181188

182-
var delegateThunk = new PInvokeDelegateThunk(del);
183-
184-
//
185-
// For open static delegates set target to ReverseOpenStaticDelegateStub which calls the static function pointer directly
186-
//
187-
bool openStaticDelegate = del.IsOpenStatic;
188-
189-
IntPtr pTarget = RuntimeInteropData.GetDelegateMarshallingStub(new RuntimeTypeHandle(del.GetMethodTable()), openStaticDelegate);
190-
Debug.Assert(pTarget != IntPtr.Zero);
191-
192-
RuntimeAugments.SetThunkData(s_thunkPoolHeap, delegateThunk.Thunk, delegateThunk.ContextData, pTarget);
193-
194-
return delegateThunk;
189+
return new PInvokeDelegateThunk(del);
195190
}
196191

197192
/// <summary>

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

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,7 @@ public static class DelegateMethodInfoRetriever
1717
{
1818
public static MethodInfo GetDelegateMethodInfo(Delegate del)
1919
{
20-
Delegate[] invokeList = del.GetInvocationList();
21-
del = invokeList[invokeList.Length - 1];
22-
IntPtr originalLdFtnResult = RuntimeAugments.GetDelegateLdFtnResult(del, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver, out bool isInterpreterEntrypoint);
23-
24-
if (isInterpreterEntrypoint)
25-
{
26-
// This is a special kind of delegate where the invoke method is "ObjectArrayThunk". Typically,
27-
// this will be a delegate that points the LINQ Expression interpreter. We could manufacture
28-
// a MethodInfo based on the delegate's Invoke signature, but let's just throw for now.
29-
throw new NotSupportedException(SR.DelegateGetMethodInfo_ObjectArrayDelegate);
30-
}
31-
32-
if (originalLdFtnResult == (IntPtr)0)
33-
return null;
20+
IntPtr originalLdFtnResult = RuntimeAugments.GetDelegateLdFtnResult(del, out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver);
3421

3522
QMethodDefinition methodHandle = default(QMethodDefinition);
3623
RuntimeTypeHandle[] genericMethodTypeArgumentHandles = null;
@@ -79,11 +66,7 @@ public static MethodInfo GetDelegateMethodInfo(Delegate del)
7966
throw new NotSupportedException(SR.Format(SR.DelegateGetMethodInfo_NoDynamic_WithDisplayString, methodDisplayString));
8067
}
8168
}
82-
MethodBase methodBase = ExecutionDomain.GetMethod(typeOfFirstParameterIfInstanceDelegate, methodHandle, genericMethodTypeArgumentHandles);
83-
MethodInfo methodInfo = methodBase as MethodInfo;
84-
if (methodInfo != null)
85-
return methodInfo;
86-
return null; // GetMethod() returned a ConstructorInfo.
69+
return (MethodInfo)ExecutionDomain.GetMethod(typeOfFirstParameterIfInstanceDelegate, methodHandle, genericMethodTypeArgumentHandles);
8770
}
8871
}
8972
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,6 @@
204204
<data name="DelegateGetMethodInfo_NoInstantiation" xml:space="preserve">
205205
<value>Cannot retrieve a MethodInfo for this delegate because the necessary generic instantiation was not metadata-enabled.</value>
206206
</data>
207-
<data name="DelegateGetMethodInfo_ObjectArrayDelegate" xml:space="preserve">
208-
<value>Cannot retrieve a MethodInfo for this delegate because the delegate target is an interpreted LINQ expression.</value>
209-
</data>
210207
<data name="Arg_InterfaceMapMustNotBeAbstract" xml:space="preserve">
211208
<value>Could not retrieve the mapping of the interface '{0}' on type '{1}' because the type implements the interface abstractly.</value>
212209
</data>

‎src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3285,7 +3285,7 @@ private void getEEInfo(ref CORINFO_EE_INFO pEEInfoOut)
32853285

32863286
pEEInfoOut.inlinedCallFrameInfo.size = (uint)SizeOfPInvokeTransitionFrame;
32873287

3288-
pEEInfoOut.offsetOfDelegateInstance = (uint)pointerSize; // Delegate::m_firstParameter
3288+
pEEInfoOut.offsetOfDelegateInstance = (uint)pointerSize; // Delegate::_firstParameter
32893289
pEEInfoOut.offsetOfDelegateFirstTarget = OffsetOfDelegateFirstTarget;
32903290

32913291
pEEInfoOut.sizeOfReversePInvokeFrame = (uint)SizeOfReversePInvokeTransitionFrame;

‎src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateMethodILEmitter.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ public static MethodIL EmitIL(MethodDesc method)
4646

4747
ILEmitter emit = new ILEmitter();
4848
TypeDesc delegateType = context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType;
49-
FieldDesc firstParameterField = delegateType.GetKnownField("m_firstParameter");
50-
FieldDesc functionPointerField = delegateType.GetKnownField("m_functionPointer");
49+
FieldDesc firstParameterField = delegateType.GetKnownField("_firstParameter");
50+
FieldDesc functionPointerField = delegateType.GetKnownField("_functionPointer");
5151
ILCodeStream codeStream = emit.NewCodeStream();
5252

5353
codeStream.EmitLdArg(0);

‎src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,31 +63,31 @@ protected FieldDesc ExtraFunctionPointerOrDataField
6363
{
6464
get
6565
{
66-
return SystemDelegateType.GetKnownField("m_extraFunctionPointerOrData");
66+
return SystemDelegateType.GetKnownField("_extraFunctionPointerOrData");
6767
}
6868
}
6969

7070
protected FieldDesc HelperObjectField
7171
{
7272
get
7373
{
74-
return SystemDelegateType.GetKnownField("m_helperObject");
74+
return SystemDelegateType.GetKnownField("_helperObject");
7575
}
7676
}
7777

7878
protected FieldDesc FirstParameterField
7979
{
8080
get
8181
{
82-
return SystemDelegateType.GetKnownField("m_firstParameter");
82+
return SystemDelegateType.GetKnownField("_firstParameter");
8383
}
8484
}
8585

8686
protected FieldDesc FunctionPointerField
8787
{
8888
get
8989
{
90-
return SystemDelegateType.GetKnownField("m_functionPointer");
90+
return SystemDelegateType.GetKnownField("_functionPointer");
9191
}
9292
}
9393

@@ -304,7 +304,8 @@ public override MethodIL EmitIL()
304304
ILEmitter emitter = new ILEmitter();
305305
ILCodeStream codeStream = emitter.NewCodeStream();
306306

307-
ArrayType invocationListArrayType = SystemDelegateType.MakeArrayType();
307+
TypeDesc delegateWrapperType = ((MetadataType)SystemDelegateType).GetKnownNestedType("Wrapper");
308+
ArrayType invocationListArrayType = delegateWrapperType.MakeArrayType();
308309

309310
ILLocalVariable delegateArrayLocal = emitter.NewLocal(invocationListArrayType);
310311
ILLocalVariable invocationCountLocal = emitter.NewLocal(Context.GetWellKnownType(WellKnownType.Int32));
@@ -318,21 +319,22 @@ public override MethodIL EmitIL()
318319
}
319320

320321
// Fill in delegateArrayLocal
321-
// Delegate[] delegateArrayLocal = (Delegate[])this.m_helperObject
322+
// Delegate.Wrapper[] delegateArrayLocal = (Delegate.Wrapper[])this._helperObject
322323

323324
// ldarg.0 (this pointer)
324-
// ldfld Delegate.HelperObjectField
325-
// castclass Delegate[]
325+
// ldfld Delegate._helperObject
326+
// castclass Delegate.Wrapper[]
326327
// stloc delegateArrayLocal
327328
codeStream.EmitLdArg(0);
328329
codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(HelperObjectField));
329330
codeStream.Emit(ILOpcode.castclass, emitter.NewToken(invocationListArrayType));
330331
codeStream.EmitStLoc(delegateArrayLocal);
331332

332333
// Fill in invocationCountLocal
333-
// int invocationCountLocal = this.m_extraFunctionPointerOrData
334+
// int invocationCountLocal = this._extraFunctionPointerOrData
335+
334336
// ldarg.0 (this pointer)
335-
// ldfld Delegate.m_extraFunctionPointerOrData
337+
// ldfld Delegate._extraFunctionPointerOrData
336338
// stloc invocationCountLocal
337339
codeStream.EmitLdArg(0);
338340
codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(ExtraFunctionPointerOrDataField));
@@ -352,25 +354,27 @@ public override MethodIL EmitIL()
352354

353355
// Implement as do/while loop. We only have this stub in play if we're in the multicast situation
354356
// Find the delegate to call
355-
// Delegate = delegateToCallLocal = delegateArrayLocal[iteratorLocal];
357+
// Delegate = delegateToCallLocal = delegateArrayLocal[iteratorLocal].Value;
356358

357359
// ldloc delegateArrayLocal
358360
// ldloc iteratorLocal
359-
// ldelem System.Delegate
361+
// ldelema Delegate.Wrapper
362+
// ldfld Delegate.Wrapper.Value
360363
// stloc delegateToCallLocal
361364
codeStream.EmitLdLoc(delegateArrayLocal);
362365
codeStream.EmitLdLoc(iteratorLocal);
363-
codeStream.Emit(ILOpcode.ldelem, emitter.NewToken(SystemDelegateType));
366+
codeStream.Emit(ILOpcode.ldelema, emitter.NewToken(delegateWrapperType));
367+
codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(delegateWrapperType.GetKnownField("Value")));
364368
codeStream.EmitStLoc(delegateToCallLocal);
365369

366370
// Call the delegate
367371
// returnValueLocal = delegateToCallLocal(...);
368372

369373
// ldloc delegateToCallLocal
370-
// ldfld System.Delegate.m_firstParameter
374+
// ldfld Delegate._firstParameter
371375
// ldarg 1, n
372376
// ldloc delegateToCallLocal
373-
// ldfld System.Delegate.m_functionPointer
377+
// ldfld Delegate._functionPointer
374378
// calli returnValueType thiscall (all the params)
375379
// IF there is a return value
376380
// stloc returnValueLocal
@@ -501,7 +505,7 @@ public override MethodIL EmitIL()
501505
// args[1] = param1;
502506
// ...
503507
// try {
504-
// ret = ((Func<object[], object>)dlg.m_helperObject)(args);
508+
// ret = ((Func<object[], object>)dlg._helperObject)(args);
505509
// } finally {
506510
// param0 = (T0)args[0]; // only generated for each byref argument
507511
// }

0 commit comments

Comments
 (0)