From fa3d1ec707ec67cb6cdf99a32f2c0564d87725ca Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 27 Feb 2024 11:11:17 -0800 Subject: [PATCH 01/14] [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 https://github.com/dotnet/runtimelab/issues/164 --- .../Runtime/Augments/RuntimeAugments.cs | 6 +- .../src/System/Delegate.cs | 310 ++++++++---------- .../Runtime/InteropServices/PInvokeMarshal.cs | 92 +++--- .../DelegateMethodInfoRetriever.cs | 21 +- .../src/Resources/Strings.resx | 3 - .../System/MulticastDelegateTests.cs | 25 +- .../delegate/delegate/DelegateCombine1.csproj | 4 +- .../delegate/DelegateCombineImpl.csproj | 4 +- .../delegate/delegate/DelegateEquals1.csproj | 3 - .../delegate/DelegateGetHashCode1.csproj | 3 - .../DelegateGetInvocationList1.csproj | 14 - .../delegate/delegate/DelegateRemove.csproj | 3 - .../delegate/delegateRemoveImpl.csproj | 3 - .../delegate/delegategetinvocationlist1.cs | 230 ------------- .../FunctionPointer/FunctionPointer.cs | 10 +- 15 files changed, 211 insertions(+), 520 deletions(-) delete mode 100644 src/tests/CoreMangLib/system/delegate/delegate/DelegateGetInvocationList1.csproj delete mode 100644 src/tests/CoreMangLib/system/delegate/delegate/delegategetinvocationlist1.cs diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs index 441036959931d9..8421eea29887ec 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs +++ b/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 diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs index 1c565a045b8265..9de1b4587081ab 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs @@ -7,6 +7,7 @@ using System.Reflection; using System.Runtime; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Runtime.Serialization; using Internal.Reflection.Augments; @@ -45,6 +46,15 @@ protected Delegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Al private nint m_extraFunctionPointerOrData; private IntPtr m_functionPointer; + // m_helperObject may point to an array of delegates if this is a multicast delegate. We use this wrapper to distinguish between + // our own array of delegates and user provided Wrapper[]. As a added benefit, this wrapper also eliminates array co-variance + // overhead for our own array of delegates. + private struct Wrapper + { + public Wrapper(Delegate value) => Value = value; + public Delegate Value; + } + // WARNING: These constants are also declared in System.Private.TypeLoader\Internal\Runtime\TypeLoader\CallConverterThunk.cs // Do not change their values without updating the values in the calling convention converter component private protected const int MulticastThunk = 0; @@ -63,14 +73,8 @@ private protected virtual IntPtr GetThunk(int whichThunk) } /// - /// Used by various parts of the runtime as a replacement for Delegate.Method - /// - /// The Interop layer uses this to distinguish between different methods on a - /// single type, and to get the function pointer for delegates to static functions - /// /// The reflection apis use this api to figure out what MethodInfo is related /// to a delegate. - /// /// /// /// This value indicates which type an delegate's function pointer is associated with @@ -79,27 +83,12 @@ private protected virtual IntPtr GetThunk(int whichThunk) /// /// This value indicates if the returned pointer is an open resolver structure. /// - /// - /// Delegate points to an object array thunk (the delegate wraps a Func<object[], object> delegate). This - /// is typically a delegate pointing to the LINQ expression interpreter. - /// - /// - internal unsafe IntPtr GetFunctionPointer(out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver, out bool isInterpreterEntrypoint) + internal unsafe IntPtr GetDelegateLdFtnResult(out RuntimeTypeHandle typeOfFirstParameterIfInstanceDelegate, out bool isOpenResolver) { typeOfFirstParameterIfInstanceDelegate = default(RuntimeTypeHandle); isOpenResolver = false; - isInterpreterEntrypoint = false; - if (GetThunk(MulticastThunk) == m_functionPointer) - { - return IntPtr.Zero; - } - else if (GetThunk(ObjectArrayThunk) == m_functionPointer) - { - isInterpreterEntrypoint = true; - return IntPtr.Zero; - } - else if (m_extraFunctionPointerOrData != 0) + if (m_extraFunctionPointerOrData != 0) { if (GetThunk(OpenInstanceThunk) == m_functionPointer) { @@ -113,13 +102,11 @@ internal unsafe IntPtr GetFunctionPointer(out RuntimeTypeHandle typeOfFirstParam if (m_firstParameter != null) typeOfFirstParameterIfInstanceDelegate = new RuntimeTypeHandle(m_firstParameter.GetMethodTable()); - // TODO! Implementation issue for generic invokes here ... we need another IntPtr for uniqueness. - return m_functionPointer; } } - // This function is known to the IL Transformer. + // This function is known to the compiler. private void InitializeClosedInstance(object firstParameter, IntPtr functionPointer) { if (firstParameter is null) @@ -129,7 +116,7 @@ private void InitializeClosedInstance(object firstParameter, IntPtr functionPoin m_firstParameter = firstParameter; } - // This function is known to the IL Transformer. + // This function is known to the compiler. private void InitializeClosedInstanceSlow(object firstParameter, IntPtr functionPointer) { // This method is like InitializeClosedInstance, but it handles ALL cases. In particular, it handles generic method with fun function pointers. @@ -180,6 +167,7 @@ private void InitializeClosedInstanceWithGVMResolution(object firstParameter, Ru return; } + // This function is known to the compiler. private void InitializeClosedInstanceToInterface(object firstParameter, IntPtr dispatchCell) { if (firstParameter is null) @@ -207,7 +195,7 @@ private void InitializeClosedInstanceWithoutNullCheck(object firstParameter, Int } } - // This function is known to the compiler backend. + // This function is known to the compiler. private void InitializeClosedStaticThunk(object firstParameter, IntPtr functionPointer, IntPtr functionPointerThunk) { m_extraFunctionPointerOrData = functionPointer; @@ -216,7 +204,7 @@ private void InitializeClosedStaticThunk(object firstParameter, IntPtr functionP m_firstParameter = this; } - // This function is known to the compiler backend. + // This function is known to the compiler. private void InitializeOpenStaticThunk(object _ /*firstParameter*/, IntPtr functionPointer, IntPtr functionPointerThunk) { // This sort of delegate is invoked by calling the thunk function pointer with the arguments to the delegate + a reference to the delegate object itself. @@ -241,15 +229,7 @@ private IntPtr GetActualTargetFunctionPointer(object thisObject) return OpenMethodResolver.ResolveMethod(m_extraFunctionPointerOrData, thisObject); } - internal bool IsDynamicDelegate() - { - if (this.GetThunk(MulticastThunk) == IntPtr.Zero) - { - return true; - } - - return false; - } + internal bool IsDynamicDelegate() => GetThunk(MulticastThunk) == IntPtr.Zero; [DebuggerGuidedStepThroughAttribute] protected virtual object? DynamicInvokeImpl(object?[]? args) @@ -274,19 +254,31 @@ internal bool IsDynamicDelegate() protected virtual MethodInfo GetMethodImpl() { + // Multi-cast delegates return the Method of the last delegate in the list + if (m_helperObject is Wrapper[] invocationList) + { + int invocationCount = (int)m_extraFunctionPointerOrData; + return invocationList[invocationCount - 1].Value.GetMethodImpl(); + } + + // Return the delegate Invoke method for marshalled function pointers and LINQ expressions + if ((m_firstParameter is NativeFunctionPointerWrapper) || (m_functionPointer == GetThunk(ObjectArrayThunk))) + { + return GetType().GetMethod("Invoke"); + } + return ReflectionAugments.ReflectionCoreCallbacks.GetDelegateMethod(this); } - public object Target + public object? Target { get { // Multi-cast delegates return the Target of the last delegate in the list - if (m_functionPointer == GetThunk(MulticastThunk)) + if (m_helperObject is Wrapper[] invocationList) { - Delegate[] invocationList = (Delegate[])m_helperObject; int invocationCount = (int)m_extraFunctionPointerOrData; - return invocationList[invocationCount - 1].Target; + return invocationList[invocationCount - 1].Value.Target; } // Closed static delegates place a value in m_helperObject that they pass to the target method. @@ -301,6 +293,12 @@ public object Target return null; } + // NativeFunctionPointerWrapper used by marshalled function pointers is not returned as a public target + if (m_firstParameter is NativeFunctionPointerWrapper) + { + return null; + } + // Closed instance delegates place a value in m_firstParameter, and we've ruled out all other types of delegates return m_firstParameter; } @@ -319,13 +317,9 @@ public object Target // V1 api: Creates open delegates to static methods only, relaxed signature checking disallowed. public static Delegate CreateDelegate(Type type, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type target, string method, bool ignoreCase, bool throwOnBindFailure) => ReflectionAugments.ReflectionCoreCallbacks.CreateDelegate(type, target, method, ignoreCase, throwOnBindFailure); - internal bool IsOpenStatic - { - get - { - return GetThunk(OpenStaticThunk) == m_functionPointer; - } - } + internal IntPtr TryGetOpenStaticFunctionPointer() => (GetThunk(OpenStaticThunk) == m_functionPointer) ? m_extraFunctionPointerOrData : 0; + + internal NativeFunctionPointerWrapper? TryGetNativeFunctionPointerWrapper() => m_firstParameter as NativeFunctionPointerWrapper; internal static unsafe bool InternalEqualTypes(object a, object b) { @@ -397,21 +391,14 @@ internal static unsafe Delegate CreateDelegate(MethodTable* delegateEEType, IntP return del; } - private unsafe MulticastDelegate NewMulticastDelegate(Delegate[] invocationList, int invocationCount, bool thisIsMultiCastAlready = false) + private unsafe Delegate NewMulticastDelegate(Wrapper[] invocationList, int invocationCount, bool thisIsMultiCastAlready = false) { - // First, allocate a new multicast delegate just like this one, i.e. same type as the this object - MulticastDelegate result = (MulticastDelegate)RuntimeImports.RhNewObject(this.GetMethodTable()); + // First, allocate a new delegate just like this one, i.e. same type as the this object + Delegate result = Unsafe.As(RuntimeImports.RhNewObject(this.GetMethodTable())); // Performance optimization - if this already points to a true multicast delegate, - // copy _methodPtr and _methodPtrAux fields rather than calling into the EE to get them - if (thisIsMultiCastAlready) - { - result.m_functionPointer = this.m_functionPointer; - } - else - { - result.m_functionPointer = GetThunk(MulticastThunk); - } + // copy m_functionPointer field rather than calling GetThunk to get it + result.m_functionPointer = thisIsMultiCastAlready ? m_functionPointer : GetThunk(MulticastThunk); result.m_firstParameter = result; result.m_helperObject = invocationList; result.m_extraFunctionPointerOrData = (IntPtr)invocationCount; @@ -419,22 +406,19 @@ private unsafe MulticastDelegate NewMulticastDelegate(Delegate[] invocationList, return result; } - private static bool TrySetSlot(Delegate[] a, int index, Delegate o) + private static bool TrySetSlot(Wrapper[] a, int index, Delegate o) { - if (a[index] == null && System.Threading.Interlocked.CompareExchange(ref a[index], o, null) == null) + if (a[index].Value == null && System.Threading.Interlocked.CompareExchange(ref a[index].Value, o, null) == null) return true; // The slot may be already set because we have added and removed the same method before. // Optimize this case, because it's cheaper than copying the array. - if (a[index] != null) + if (a[index].Value is Delegate dd) { - MulticastDelegate d = (MulticastDelegate)o; - MulticastDelegate dd = (MulticastDelegate)a[index]; - - if (object.ReferenceEquals(dd.m_firstParameter, d.m_firstParameter) && - object.ReferenceEquals(dd.m_helperObject, d.m_helperObject) && - dd.m_extraFunctionPointerOrData == d.m_extraFunctionPointerOrData && - dd.m_functionPointer == d.m_functionPointer) + if (object.ReferenceEquals(dd.m_firstParameter, o.m_firstParameter) && + object.ReferenceEquals(dd.m_helperObject, o.m_helperObject) && + dd.m_extraFunctionPointerOrData == o.m_extraFunctionPointerOrData && + dd.m_functionPointer == o.m_functionPointer) { return true; } @@ -446,35 +430,31 @@ private static bool TrySetSlot(Delegate[] a, int index, Delegate o) // to form a new delegate. protected virtual Delegate CombineImpl(Delegate? d) { - if (d is null) // cast to object for a more efficient test + if (d is null) return this; // Verify that the types are the same... if (!InternalEqualTypes(this, d)) throw new ArgumentException(SR.Arg_DlgtTypeMis); - if (IsDynamicDelegate() && d.IsDynamicDelegate()) - { + if (IsDynamicDelegate()) throw new InvalidOperationException(); - } - MulticastDelegate dFollow = (MulticastDelegate)d; - Delegate[]? resultList; int followCount = 1; - Delegate[]? followList = dFollow.m_helperObject as Delegate[]; + Wrapper[]? followList = d.m_helperObject as Wrapper[]; if (followList != null) - followCount = (int)dFollow.m_extraFunctionPointerOrData; + followCount = (int)d.m_extraFunctionPointerOrData; int resultCount; - Delegate[]? invocationList = m_helperObject as Delegate[]; - if (invocationList == null) + Wrapper[]? resultList; + if (m_helperObject is not Wrapper[] invocationList) { resultCount = 1 + followCount; - resultList = new Delegate[resultCount]; - resultList[0] = this; + resultList = new Wrapper[resultCount]; + resultList[0] = new Wrapper(this); if (followList == null) { - resultList[1] = dFollow; + resultList[1] = new Wrapper(d); } else { @@ -493,14 +473,14 @@ protected virtual Delegate CombineImpl(Delegate? d) resultList = invocationList; if (followList == null) { - if (!TrySetSlot(resultList, invocationCount, dFollow)) + if (!TrySetSlot(resultList, invocationCount, d)) resultList = null; } else { for (int i = 0; i < followCount; i++) { - if (!TrySetSlot(resultList, invocationCount + i, followList[i])) + if (!TrySetSlot(resultList, invocationCount + i, followList[i].Value)) { resultList = null; break; @@ -515,14 +495,14 @@ protected virtual Delegate CombineImpl(Delegate? d) while (allocCount < resultCount) allocCount *= 2; - resultList = new Delegate[allocCount]; + resultList = new Wrapper[allocCount]; for (int i = 0; i < invocationCount; i++) resultList[i] = invocationList[i]; if (followList == null) { - resultList[invocationCount] = dFollow; + resultList[invocationCount] = new Wrapper(d); } else { @@ -534,14 +514,13 @@ protected virtual Delegate CombineImpl(Delegate? d) } } - private Delegate[] DeleteFromInvocationList(Delegate[] invocationList, int invocationCount, int deleteIndex, int deleteCount) + private static Wrapper[] DeleteFromInvocationList(Wrapper[] invocationList, int invocationCount, int deleteIndex, int deleteCount) { - Delegate[] thisInvocationList = (Delegate[])m_helperObject; - int allocCount = thisInvocationList.Length; + int allocCount = invocationList.Length; while (allocCount / 2 >= invocationCount - deleteCount) allocCount /= 2; - Delegate[] newInvocationList = new Delegate[allocCount]; + Wrapper[] newInvocationList = new Wrapper[allocCount]; for (int i = 0; i < deleteIndex; i++) newInvocationList[i] = invocationList[i]; @@ -552,11 +531,11 @@ private Delegate[] DeleteFromInvocationList(Delegate[] invocationList, int invoc return newInvocationList; } - private static bool EqualInvocationLists(Delegate[] a, Delegate[] b, int start, int count) + private static bool EqualInvocationLists(Wrapper[] a, Wrapper[] b, int start, int count) { for (int i = 0; i < count; i++) { - if (!(a[start + i].Equals(b[i]))) + if (!(a[start + i].Value.Equals(b[i].Value))) return false; } return true; @@ -572,17 +551,14 @@ private static bool EqualInvocationLists(Delegate[] a, Delegate[] b, int start, // There is a special case were we are removing using a delegate as // the value we need to check for this case // - MulticastDelegate? v = d as MulticastDelegate; - - if (v is null) + if (d is null) return this; - if (v.m_helperObject as Delegate[] == null) + if (d.m_helperObject is not Wrapper[] dInvocationList) { - Delegate[]? invocationList = m_helperObject as Delegate[]; - if (invocationList == null) + if (m_helperObject is not Wrapper[] invocationList) { // they are both not real Multicast - if (this.Equals(v)) + if (this.Equals(d)) return null; } else @@ -590,16 +566,16 @@ private static bool EqualInvocationLists(Delegate[] a, Delegate[] b, int start, int invocationCount = (int)m_extraFunctionPointerOrData; for (int i = invocationCount; --i >= 0;) { - if (v.Equals(invocationList[i])) + if (d.Equals(invocationList[i])) { if (invocationCount == 2) { // Special case - only one value left, either at the beginning or the end - return invocationList[1 - i]; + return invocationList[1 - i].Value; } else { - Delegate[] list = DeleteFromInvocationList(invocationList, invocationCount, i, 1); + Wrapper[] list = DeleteFromInvocationList(invocationList, invocationCount, i, 1); return NewMulticastDelegate(list, invocationCount - 1, true); } } @@ -608,29 +584,28 @@ private static bool EqualInvocationLists(Delegate[] a, Delegate[] b, int start, } else { - Delegate[]? invocationList = m_helperObject as Delegate[]; - if (invocationList != null) + if (m_helperObject is Wrapper[] invocationList) { int invocationCount = (int)m_extraFunctionPointerOrData; - int vInvocationCount = (int)v.m_extraFunctionPointerOrData; - for (int i = invocationCount - vInvocationCount; i >= 0; i--) + int dInvocationCount = (int)d.m_extraFunctionPointerOrData; + for (int i = invocationCount - dInvocationCount; i >= 0; i--) { - if (EqualInvocationLists(invocationList, v.m_helperObject as Delegate[], i, vInvocationCount)) + if (EqualInvocationLists(invocationList, dInvocationList, i, dInvocationCount)) { - if (invocationCount - vInvocationCount == 0) + if (invocationCount - dInvocationCount == 0) { // Special case - no values left return null; } - else if (invocationCount - vInvocationCount == 1) + else if (invocationCount - dInvocationCount == 1) { // Special case - only one value left, either at the beginning or the end - return invocationList[i != 0 ? 0 : invocationCount - 1]; + return invocationList[i != 0 ? 0 : invocationCount - 1].Value; } else { - Delegate[] list = DeleteFromInvocationList(invocationList, invocationCount, i, vInvocationCount); - return NewMulticastDelegate(list, invocationCount - vInvocationCount, true); + Wrapper[] list = DeleteFromInvocationList(invocationList, invocationCount, i, dInvocationCount); + return NewMulticastDelegate(list, invocationCount - dInvocationCount, true); } } } @@ -642,41 +617,19 @@ private static bool EqualInvocationLists(Delegate[] a, Delegate[] b, int start, public virtual Delegate[] GetInvocationList() { - Delegate[] del; - Delegate[]? invocationList = m_helperObject as Delegate[]; - if (invocationList == null) - { - del = new Delegate[1]; - del[0] = this; - } - else + if (m_helperObject is Wrapper[] invocationList) { // Create an array of delegate copies and each // element into the array int invocationCount = (int)m_extraFunctionPointerOrData; - del = new Delegate[invocationCount]; + var del = new Delegate[invocationCount]; for (int i = 0; i < del.Length; i++) - del[i] = invocationList[i]; + del[i] = invocationList[i].Value; + return del; } - return del; - } - - private bool InvocationListEquals(MulticastDelegate d) - { - Delegate[] invocationList = (Delegate[])m_helperObject; - if (d.m_extraFunctionPointerOrData != m_extraFunctionPointerOrData) - return false; - int invocationCount = (int)m_extraFunctionPointerOrData; - for (int i = 0; i < invocationCount; i++) - { - Delegate dd = invocationList[i]; - Delegate[] dInvocationList = (Delegate[])d.m_helperObject; - if (!dd.Equals(dInvocationList[i])) - return false; - } - return true; + return new Delegate[] { this }; } public override bool Equals([NotNullWhen(true)] object? obj) @@ -688,73 +641,74 @@ public override bool Equals([NotNullWhen(true)] object? obj) if (!InternalEqualTypes(this, obj)) return false; - // Since this is a MulticastDelegate and we know - // the types are the same, obj should also be a - // MulticastDelegate - Debug.Assert(obj is MulticastDelegate, "Shouldn't have failed here since we already checked the types are the same!"); - var d = Unsafe.As(obj); + // Since this is a Delegate and we know the types are the same, obj should also be a Delegate + Debug.Assert(obj is Delegate, "Shouldn't have failed here since we already checked the types are the same!"); + var d = Unsafe.As(obj); // there are 2 kind of delegate kinds for comparison - // 1- Multicast (m_helperObject is Delegate[]) + // 1- Multicast (m_helperObject is Wrapper[]) // 2- Single-cast delegate, which can be compared with a structural comparison - IntPtr multicastThunk = GetThunk(MulticastThunk); - if (m_functionPointer == multicastThunk) + if (m_helperObject is Wrapper[] invocationList) { - return d.m_functionPointer == multicastThunk && InvocationListEquals(d); - } - else - { - if (!object.ReferenceEquals(m_helperObject, d.m_helperObject) || - (!FunctionPointerOps.Compare(m_extraFunctionPointerOrData, d.m_extraFunctionPointerOrData)) || - (!FunctionPointerOps.Compare(m_functionPointer, d.m_functionPointer))) - { + if (d.m_extraFunctionPointerOrData != m_extraFunctionPointerOrData) return false; - } - // Those delegate kinds with thunks put themselves into the m_firstParameter, so we can't - // blindly compare the m_firstParameter fields for equality. - if (object.ReferenceEquals(m_firstParameter, this)) + if (d.m_helperObject is not Wrapper[] dInvocationList) + return false; + + int invocationCount = (int)m_extraFunctionPointerOrData; + for (int i = 0; i < invocationCount; i++) { - return object.ReferenceEquals(d.m_firstParameter, d); + if (!invocationList[i].Value.Equals(dInvocationList[i].Value)) + return false; } + return true; + } - return object.ReferenceEquals(m_firstParameter, d.m_firstParameter); + if (!object.ReferenceEquals(m_helperObject, d.m_helperObject) || + (!FunctionPointerOps.Compare(m_extraFunctionPointerOrData, d.m_extraFunctionPointerOrData)) || + (!FunctionPointerOps.Compare(m_functionPointer, d.m_functionPointer))) + { + return false; + } + + // Those delegate kinds with thunks put themselves into the m_firstParameter, so we can't + // blindly compare the m_firstParameter fields for equality. + if (object.ReferenceEquals(m_firstParameter, this)) + { + return object.ReferenceEquals(d.m_firstParameter, d); } + + return object.ReferenceEquals(m_firstParameter, d.m_firstParameter); } public override int GetHashCode() { - Delegate[]? invocationList = m_helperObject as Delegate[]; - if (invocationList == null) - { - return base.GetHashCode(); - } - else + if (m_helperObject is Wrapper[] invocationList) { int hash = 0; for (int i = 0; i < (int)m_extraFunctionPointerOrData; i++) { - hash = hash * 33 + invocationList[i].GetHashCode(); + hash = hash * 33 + invocationList[i].Value.GetHashCode(); } - return hash; } + + return base.GetHashCode(); } - public bool HasSingleTarget => !(m_helperObject is Delegate[]); + public bool HasSingleTarget => m_helperObject is not Wrapper[]; // Used by delegate invocation list enumerator internal Delegate? TryGetAt(int index) { - if (!(m_helperObject is Delegate[] invocationList)) + if (m_helperObject is Wrapper[] invocationList) { - return (index == 0) ? this : null; - } - else - { - return ((uint)index < (uint)m_extraFunctionPointerOrData) ? invocationList[index] : null; + return ((uint)index < (uint)m_extraFunctionPointerOrData) ? invocationList[index].Value : null; } + + return (index == 0) ? this : null; } } } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs index bf7fa122af6c5d..87e3303b6f154b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs +++ b/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); } /// diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Extensions/NonPortable/DelegateMethodInfoRetriever.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Extensions/NonPortable/DelegateMethodInfoRetriever.cs index 5ae75d1d55eb90..5669203a6f39d1 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Extensions/NonPortable/DelegateMethodInfoRetriever.cs +++ b/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); } } } diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx index 80023f62d226a7..e5432845a5206b 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx @@ -204,9 +204,6 @@ Cannot retrieve a MethodInfo for this delegate because the necessary generic instantiation was not metadata-enabled. - - Cannot retrieve a MethodInfo for this delegate because the delegate target is an interpreted LINQ expression. - Could not retrieve the mapping of the interface '{0}' on type '{1}' because the type implements the interface abstractly. diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/MulticastDelegateTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/MulticastDelegateTests.cs index 354f997ed5c7b3..9994fa02f10fac 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/MulticastDelegateTests.cs +++ b/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 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) + { + } + } } diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj index 62c497a0992226..3ca1a050735ef8 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj @@ -1,9 +1,7 @@ - - true true - 1 + 0 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj index df8e80338fad09..7c4ca6e8d532b5 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj @@ -1,9 +1,7 @@ - - true true - 1 + 0 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj index 2d8fbf191cbb54..07251f373e3edc 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj @@ -1,9 +1,6 @@ - - true true - 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj index a597f02daeda12..6a33e215d0fc51 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj @@ -1,9 +1,6 @@ - - true true - 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetInvocationList1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetInvocationList1.csproj deleted file mode 100644 index e82724aaace7fe..00000000000000 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetInvocationList1.csproj +++ /dev/null @@ -1,14 +0,0 @@ - - - - true - true - 1 - - - - - - - - diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj index 0217c4ad8d879a..87fdce6e859941 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj @@ -1,9 +1,6 @@ - - true true - 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj b/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj index ab802b989cda73..6d021787920364 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj @@ -1,9 +1,6 @@ - - true true - 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/delegategetinvocationlist1.cs b/src/tests/CoreMangLib/system/delegate/delegate/delegategetinvocationlist1.cs deleted file mode 100644 index d0c8f343273623..00000000000000 --- a/src/tests/CoreMangLib/system/delegate/delegate/delegategetinvocationlist1.cs +++ /dev/null @@ -1,230 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Globalization; -using Xunit; -//test case for delegate GetInvocationList method. -namespace DelegateTest -{ - delegate bool booldelegate(); - public class DelegateGetInvocationList - { - - booldelegate starkWork; - - [Fact] - public static int TestEntryPoint() - { - DelegateGetInvocationList delegateGetInvocationList = new DelegateGetInvocationList(); - - TestLibrary.TestFramework.BeginTestCase("DelegateGetInvocationList"); - - if (delegateGetInvocationList.RunTests()) - { - TestLibrary.TestFramework.EndTestCase(); - TestLibrary.TestFramework.LogInformation("PASS"); - return 100; - - } - else - { - TestLibrary.TestFramework.EndTestCase(); - TestLibrary.TestFramework.LogInformation("FAIL"); - return 0; - } - } - - public bool RunTests() - { - bool retVal = true; - - TestLibrary.TestFramework.LogInformation("[Positive]"); - retVal = PosTest1() && retVal; - retVal = PosTest2() && retVal; - retVal = PosTest3() && retVal; - retVal = PosTest4() && retVal; - return retVal; - } - - // Returns true if the expected result is right - // Returns false if the expected result is wrong - public bool PosTest1() - { - bool retVal = true; - - TestLibrary.TestFramework.BeginScenario("PosTest1: Call GetInvocationList against a delegate with one function"); - try - { - DelegateGetInvocationList delctor = new DelegateGetInvocationList(); - booldelegate dStartWork_Bool = new booldelegate(new TestClass().StartWork_Bool); - delctor.starkWork = dStartWork_Bool; - Delegate[] invocationList = delctor.starkWork.GetInvocationList(); - if (invocationList.Length != 1) - { - TestLibrary.TestFramework.LogError("001", "Call GetInvocationList against a delegate with one function returns wrong result: " + invocationList.Length); - retVal = false; - } - if (!delctor.starkWork.GetInvocationList()[0].Equals(dStartWork_Bool)) - { - TestLibrary.TestFramework.LogError("002", " GetInvocationList return error method "); - retVal = false; - } - delctor.starkWork(); - } - catch (Exception e) - { - TestLibrary.TestFramework.LogError("003", "Unexpected exception: " + e); - retVal = false; - } - - return retVal; - } - // Returns true if the expected result is right - // Returns false if the expected result is wrong - public bool PosTest2() - { - bool retVal = true; - - TestLibrary.TestFramework.BeginScenario("PosTest2: Call GetInvocationList against a delegate with muti different functions "); - try - { - DelegateGetInvocationList delctor = new DelegateGetInvocationList(); - booldelegate bStartWork_Bool = new booldelegate(new TestClass().StartWork_Bool); - booldelegate bWorking_Bool = new booldelegate(new TestClass().Working_Bool); - booldelegate bCompleted_Bool = new booldelegate(new TestClass().Completed_Bool); - - delctor.starkWork += bStartWork_Bool; - delctor.starkWork += bWorking_Bool; - delctor.starkWork += bCompleted_Bool; - Delegate[] invocationList = delctor.starkWork.GetInvocationList(); - if (invocationList.Length != 3) - { - TestLibrary.TestFramework.LogError("004", "Call GetInvocationList against a delegate with one function returns wrong result: " + invocationList.Length); - retVal = false; - } - if (!delctor.starkWork.GetInvocationList()[0].Equals(bStartWork_Bool) - || !delctor.starkWork.GetInvocationList()[1].Equals(bWorking_Bool) - || !delctor.starkWork.GetInvocationList()[2].Equals(bCompleted_Bool)) - { - TestLibrary.TestFramework.LogError("005", " GetInvocationList return error method "); - retVal = false; - } - delctor.starkWork(); - } - catch (Exception e) - { - TestLibrary.TestFramework.LogError("006", "Unexpected exception: " + e); - retVal = false; - } - - return retVal; - } - // Returns true if the expected result is right - // Returns false if the expected result is wrong - public bool PosTest3() - { - bool retVal = true; - - TestLibrary.TestFramework.BeginScenario("PosTest3: Call GetInvocationList against a delegate with muti functions ,some is null"); - try - { - DelegateGetInvocationList delctor = new DelegateGetInvocationList(); - booldelegate bStartWork_Bool = new booldelegate(new TestClass().StartWork_Bool); - booldelegate bWorking_Bool = new booldelegate(new TestClass().Working_Bool); - booldelegate bCompleted_Bool = new booldelegate(new TestClass().Completed_Bool); - - delctor.starkWork += bStartWork_Bool; - delctor.starkWork += null; - delctor.starkWork += bWorking_Bool; - delctor.starkWork += bCompleted_Bool; - Delegate[] invocationList = delctor.starkWork.GetInvocationList(); - if (invocationList.Length != 3) - { - TestLibrary.TestFramework.LogError("007", "Call GetInvocationList against a delegate with one function returns wrong result: " + invocationList.Length); - retVal = false; - } - if (!delctor.starkWork.GetInvocationList()[0].Equals(bStartWork_Bool) - || !delctor.starkWork.GetInvocationList()[1].Equals(bWorking_Bool) - || !delctor.starkWork.GetInvocationList()[2].Equals(bCompleted_Bool)) - { - TestLibrary.TestFramework.LogError("008", " GetInvocationList return error method "); - retVal = false; - } - delctor.starkWork(); - } - catch (Exception e) - { - TestLibrary.TestFramework.LogError("009", "Unexpected exception: " + e); - retVal = false; - } - - return retVal; - } - - // Returns true if the expected result is right - // Returns false if the expected result is wrong - public bool PosTest4() - { - bool retVal = true; - - TestLibrary.TestFramework.BeginScenario("PosTest4: Call GetInvocationList against a delegate with muti functions ,some of these are the same"); - try - { - DelegateGetInvocationList delctor = new DelegateGetInvocationList(); - booldelegate bStartWork_Bool = new booldelegate(new TestClass().StartWork_Bool); - booldelegate bWorking_Bool = new booldelegate(new TestClass().Working_Bool); - booldelegate bCompleted_Bool = new booldelegate(new TestClass().Completed_Bool); - - delctor.starkWork += bStartWork_Bool; - delctor.starkWork += bStartWork_Bool; - delctor.starkWork += bWorking_Bool; - delctor.starkWork += bCompleted_Bool; - Delegate[] invocationList = delctor.starkWork.GetInvocationList(); - if (invocationList.Length != 4) - { - TestLibrary.TestFramework.LogError("010", "Call GetInvocationList against a delegate with one function returns wrong result: " + invocationList.Length); - retVal = false; - } - if (!delctor.starkWork.GetInvocationList()[0].Equals(bStartWork_Bool) - || !delctor.starkWork.GetInvocationList()[1].Equals(bStartWork_Bool) - || !delctor.starkWork.GetInvocationList()[2].Equals(bWorking_Bool) - || !delctor.starkWork.GetInvocationList()[3].Equals(bCompleted_Bool)) - { - TestLibrary.TestFramework.LogError("011", " GetInvocationList return error method "); - retVal = false; - } - delctor.starkWork(); - } - catch (Exception e) - { - TestLibrary.TestFramework.LogError("012", "Unexpected exception: " + e); - retVal = false; - } - - return retVal; - } - - } - //create testclass for providing test method and test target. - class TestClass - { - public bool StartWork_Bool() - { - TestLibrary.TestFramework.LogInformation("StartWork_Bool method is running ."); - return true; - } - public bool Working_Bool() - { - TestLibrary.TestFramework.LogInformation("Working_Bool method is running ."); - return true; - } - public bool Completed_Bool() - { - TestLibrary.TestFramework.LogInformation("Completed_Bool method is running ."); - return true; - } - } - - -} diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 3dbcb6aaf39807..7b0fd902a7bdfe 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -28,8 +28,6 @@ static class FunctionPointerNative delegate void VoidDelegate(); [Fact] - - [ActiveIssue("https://github.com/dotnet/runtimelab/issues/164", typeof(TestLibrary.Utilities), nameof(TestLibrary.Utilities.IsNativeAot))] public static void RunGetDelForFcnPtrTest() { Console.WriteLine($"Running {nameof(RunGetDelForFcnPtrTest)}..."); @@ -42,6 +40,10 @@ public static void RunGetDelForFcnPtrTest() VoidDelegate del = (VoidDelegate)Marshal.GetDelegateForFunctionPointer(fcnptr, typeof(VoidDelegate)); Assert.Equal(md.Target, del.Target); Assert.Equal(md.Method, del.Method); + + VoidDelegate del2 = (VoidDelegate)Marshal.GetDelegateForFunctionPointer(fcnptr, typeof(VoidDelegate)); + Assert.Equal(del, del2); + Assert.Equal(del.GetHashCode(), del2.GetHashCode()); } // Native FcnPtr -> Delegate @@ -51,6 +53,10 @@ public static void RunGetDelForFcnPtrTest() Assert.Null(del.Target); Assert.Equal("Invoke", del.Method.Name); + VoidDelegate del2 = (VoidDelegate)Marshal.GetDelegateForFunctionPointer(fcnptr, typeof(VoidDelegate));; + Assert.Equal(del, del2); + Assert.Equal(del.GetHashCode(), del2.GetHashCode()); + // Round trip of a native function pointer is never legal for a non-concrete Delegate type Assert.Throws(() => { From 745595b44eefe4ff75e134836494a063931bc4db Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Mar 2024 12:57:35 -0800 Subject: [PATCH 02/14] Fix MulticastThunk --- .../System.Private.CoreLib/src/System/Delegate.cs | 4 ---- .../Common/TypeSystem/IL/Stubs/DelegateThunks.cs | 15 +++++++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs index 9de1b4587081ab..64224c2b31603c 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs @@ -645,10 +645,6 @@ public override bool Equals([NotNullWhen(true)] object? obj) Debug.Assert(obj is Delegate, "Shouldn't have failed here since we already checked the types are the same!"); var d = Unsafe.As(obj); - // there are 2 kind of delegate kinds for comparison - // 1- Multicast (m_helperObject is Wrapper[]) - // 2- Single-cast delegate, which can be compared with a structural comparison - if (m_helperObject is Wrapper[] invocationList) { if (d.m_extraFunctionPointerOrData != m_extraFunctionPointerOrData) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs index 99834d7cb23632..66363197e7c18f 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs @@ -304,7 +304,8 @@ public override MethodIL EmitIL() ILEmitter emitter = new ILEmitter(); ILCodeStream codeStream = emitter.NewCodeStream(); - ArrayType invocationListArrayType = SystemDelegateType.MakeArrayType(); + TypeDesc delegateWrapperType = ((MetadataType)SystemDelegateType).GetKnownNestedType("Wrapper"); + ArrayType invocationListArrayType = delegateWrapperType.MakeArrayType(); ILLocalVariable delegateArrayLocal = emitter.NewLocal(invocationListArrayType); ILLocalVariable invocationCountLocal = emitter.NewLocal(Context.GetWellKnownType(WellKnownType.Int32)); @@ -318,11 +319,11 @@ public override MethodIL EmitIL() } // Fill in delegateArrayLocal - // Delegate[] delegateArrayLocal = (Delegate[])this.m_helperObject + // Wrapper[] delegateArrayLocal = (Wrapper[])this.m_helperObject // ldarg.0 (this pointer) // ldfld Delegate.HelperObjectField - // castclass Delegate[] + // castclass Wrapper[] // stloc delegateArrayLocal codeStream.EmitLdArg(0); codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(HelperObjectField)); @@ -352,15 +353,17 @@ public override MethodIL EmitIL() // Implement as do/while loop. We only have this stub in play if we're in the multicast situation // Find the delegate to call - // Delegate = delegateToCallLocal = delegateArrayLocal[iteratorLocal]; + // Delegate = delegateToCallLocal = delegateArrayLocal[iteratorLocal].Value; // ldloc delegateArrayLocal // ldloc iteratorLocal - // ldelem System.Delegate + // ldelema System.Delegate + // ldfld Wrapper.Value // stloc delegateToCallLocal codeStream.EmitLdLoc(delegateArrayLocal); codeStream.EmitLdLoc(iteratorLocal); - codeStream.Emit(ILOpcode.ldelem, emitter.NewToken(SystemDelegateType)); + codeStream.Emit(ILOpcode.ldelema, emitter.NewToken(delegateWrapperType)); + codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(delegateWrapperType.GetKnownField("Value"))); codeStream.EmitStLoc(delegateToCallLocal); // Call the delegate From ff396c5e4bca0d42ce57259c8d30625561de0f85 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Mar 2024 13:01:23 -0800 Subject: [PATCH 03/14] Delete `m` prefix --- .../src/System/Delegate.cs | 212 +++++++++--------- .../tools/Common/JitInterface/CorInfoImpl.cs | 2 +- .../IL/Stubs/DelegateMethodILEmitter.cs | 4 +- .../TypeSystem/IL/Stubs/DelegateThunks.cs | 20 +- .../Compiler/TypePreinit.cs | 16 +- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 2 +- .../JitInterface/CorInfoImpl.RyuJit.cs | 2 +- 7 files changed, 129 insertions(+), 129 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs index 64224c2b31603c..b3ba7a5e66f2ee 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs @@ -41,12 +41,12 @@ protected Delegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Al // New Delegate Implementation - private object m_firstParameter; - private object m_helperObject; - private nint m_extraFunctionPointerOrData; - private IntPtr m_functionPointer; + private object _firstParameter; + private object _helperObject; + private nint _extraFunctionPointerOrData; + private IntPtr _functionPointer; - // m_helperObject may point to an array of delegates if this is a multicast delegate. We use this wrapper to distinguish between + // _helperObject may point to an array of delegates if this is a multicast delegate. We use this wrapper to distinguish between // our own array of delegates and user provided Wrapper[]. As a added benefit, this wrapper also eliminates array co-variance // overhead for our own array of delegates. private struct Wrapper @@ -88,21 +88,21 @@ internal unsafe IntPtr GetDelegateLdFtnResult(out RuntimeTypeHandle typeOfFirstP typeOfFirstParameterIfInstanceDelegate = default(RuntimeTypeHandle); isOpenResolver = false; - if (m_extraFunctionPointerOrData != 0) + if (_extraFunctionPointerOrData != 0) { - if (GetThunk(OpenInstanceThunk) == m_functionPointer) + if (GetThunk(OpenInstanceThunk) == _functionPointer) { - typeOfFirstParameterIfInstanceDelegate = ((OpenMethodResolver*)m_extraFunctionPointerOrData)->DeclaringType; + typeOfFirstParameterIfInstanceDelegate = ((OpenMethodResolver*)_extraFunctionPointerOrData)->DeclaringType; isOpenResolver = true; } - return m_extraFunctionPointerOrData; + return _extraFunctionPointerOrData; } else { - if (m_firstParameter != null) - typeOfFirstParameterIfInstanceDelegate = new RuntimeTypeHandle(m_firstParameter.GetMethodTable()); + if (_firstParameter != null) + typeOfFirstParameterIfInstanceDelegate = new RuntimeTypeHandle(_firstParameter.GetMethodTable()); - return m_functionPointer; + return _functionPointer; } } @@ -112,8 +112,8 @@ private void InitializeClosedInstance(object firstParameter, IntPtr functionPoin if (firstParameter is null) throw new ArgumentException(SR.Arg_DlgtNullInst); - m_functionPointer = functionPointer; - m_firstParameter = firstParameter; + _functionPointer = functionPointer; + _firstParameter = firstParameter; } // This function is known to the compiler. @@ -126,15 +126,15 @@ private void InitializeClosedInstanceSlow(object firstParameter, IntPtr function if (!FunctionPointerOps.IsGenericMethodPointer(functionPointer)) { - m_functionPointer = functionPointer; - m_firstParameter = firstParameter; + _functionPointer = functionPointer; + _firstParameter = firstParameter; } else { - m_firstParameter = this; - m_functionPointer = GetThunk(ClosedInstanceThunkOverGenericMethod); - m_extraFunctionPointerOrData = functionPointer; - m_helperObject = firstParameter; + _firstParameter = this; + _functionPointer = GetThunk(ClosedInstanceThunkOverGenericMethod); + _extraFunctionPointerOrData = functionPointer; + _helperObject = firstParameter; } } @@ -153,15 +153,15 @@ private void InitializeClosedInstanceWithGVMResolution(object firstParameter, Ru } if (!FunctionPointerOps.IsGenericMethodPointer(functionResolution)) { - m_functionPointer = functionResolution; - m_firstParameter = firstParameter; + _functionPointer = functionResolution; + _firstParameter = firstParameter; } else { - m_firstParameter = this; - m_functionPointer = GetThunk(ClosedInstanceThunkOverGenericMethod); - m_extraFunctionPointerOrData = functionResolution; - m_helperObject = firstParameter; + _firstParameter = this; + _functionPointer = GetThunk(ClosedInstanceThunkOverGenericMethod); + _extraFunctionPointerOrData = functionResolution; + _helperObject = firstParameter; } return; @@ -173,8 +173,8 @@ private void InitializeClosedInstanceToInterface(object firstParameter, IntPtr d if (firstParameter is null) throw new NullReferenceException(); - m_functionPointer = RuntimeImports.RhpResolveInterfaceMethod(firstParameter, dispatchCell); - m_firstParameter = firstParameter; + _functionPointer = RuntimeImports.RhpResolveInterfaceMethod(firstParameter, dispatchCell); + _firstParameter = firstParameter; } // This is used to implement MethodInfo.CreateDelegate() in a desktop-compatible way. Yes, the desktop really @@ -183,50 +183,50 @@ private void InitializeClosedInstanceWithoutNullCheck(object firstParameter, Int { if (!FunctionPointerOps.IsGenericMethodPointer(functionPointer)) { - m_functionPointer = functionPointer; - m_firstParameter = firstParameter; + _functionPointer = functionPointer; + _firstParameter = firstParameter; } else { - m_firstParameter = this; - m_functionPointer = GetThunk(ClosedInstanceThunkOverGenericMethod); - m_extraFunctionPointerOrData = functionPointer; - m_helperObject = firstParameter; + _firstParameter = this; + _functionPointer = GetThunk(ClosedInstanceThunkOverGenericMethod); + _extraFunctionPointerOrData = functionPointer; + _helperObject = firstParameter; } } // This function is known to the compiler. private void InitializeClosedStaticThunk(object firstParameter, IntPtr functionPointer, IntPtr functionPointerThunk) { - m_extraFunctionPointerOrData = functionPointer; - m_helperObject = firstParameter; - m_functionPointer = functionPointerThunk; - m_firstParameter = this; + _extraFunctionPointerOrData = functionPointer; + _helperObject = firstParameter; + _functionPointer = functionPointerThunk; + _firstParameter = this; } // This function is known to the compiler. private void InitializeOpenStaticThunk(object _ /*firstParameter*/, IntPtr functionPointer, IntPtr functionPointerThunk) { // This sort of delegate is invoked by calling the thunk function pointer with the arguments to the delegate + a reference to the delegate object itself. - m_firstParameter = this; - m_functionPointer = functionPointerThunk; - m_extraFunctionPointerOrData = functionPointer; + _firstParameter = this; + _functionPointer = functionPointerThunk; + _extraFunctionPointerOrData = functionPointer; } private void InitializeOpenInstanceThunkDynamic(IntPtr functionPointer, IntPtr functionPointerThunk) { // This sort of delegate is invoked by calling the thunk function pointer with the arguments to the delegate + a reference to the delegate object itself. - m_firstParameter = this; - m_functionPointer = functionPointerThunk; - m_extraFunctionPointerOrData = functionPointer; + _firstParameter = this; + _functionPointer = functionPointerThunk; + _extraFunctionPointerOrData = functionPointer; } // This function is only ever called by the open instance method thunk, and in that case, - // m_extraFunctionPointerOrData always points to an OpenMethodResolver + // _extraFunctionPointerOrData always points to an OpenMethodResolver [MethodImpl(MethodImplOptions.NoInlining)] private IntPtr GetActualTargetFunctionPointer(object thisObject) { - return OpenMethodResolver.ResolveMethod(m_extraFunctionPointerOrData, thisObject); + return OpenMethodResolver.ResolveMethod(_extraFunctionPointerOrData, thisObject); } internal bool IsDynamicDelegate() => GetThunk(MulticastThunk) == IntPtr.Zero; @@ -237,7 +237,7 @@ private IntPtr GetActualTargetFunctionPointer(object thisObject) if (IsDynamicDelegate()) { // DynamicDelegate case - object? result = ((Func)m_helperObject)(args); + object? result = ((Func)_helperObject)(args); DebugAnnotations.PreviousCallContainsDebuggerStepInCode(); return result; } @@ -245,7 +245,7 @@ private IntPtr GetActualTargetFunctionPointer(object thisObject) { DynamicInvokeInfo dynamicInvokeInfo = ReflectionAugments.ReflectionCoreCallbacks.GetDelegateDynamicInvokeInfo(GetType()); - object? result = dynamicInvokeInfo.Invoke(m_firstParameter, m_functionPointer, + object? result = dynamicInvokeInfo.Invoke(_firstParameter, _functionPointer, args, binderBundle: null, wrapInTargetInvocationException: true); DebugAnnotations.PreviousCallContainsDebuggerStepInCode(); return result; @@ -255,14 +255,14 @@ private IntPtr GetActualTargetFunctionPointer(object thisObject) protected virtual MethodInfo GetMethodImpl() { // Multi-cast delegates return the Method of the last delegate in the list - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { - int invocationCount = (int)m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; return invocationList[invocationCount - 1].Value.GetMethodImpl(); } // Return the delegate Invoke method for marshalled function pointers and LINQ expressions - if ((m_firstParameter is NativeFunctionPointerWrapper) || (m_functionPointer == GetThunk(ObjectArrayThunk))) + if ((_firstParameter is NativeFunctionPointerWrapper) || (_functionPointer == GetThunk(ObjectArrayThunk))) { return GetType().GetMethod("Invoke"); } @@ -275,32 +275,32 @@ public object? Target get { // Multi-cast delegates return the Target of the last delegate in the list - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { - int invocationCount = (int)m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; return invocationList[invocationCount - 1].Value.Target; } - // Closed static delegates place a value in m_helperObject that they pass to the target method. - if (m_functionPointer == GetThunk(ClosedStaticThunk) || - m_functionPointer == GetThunk(ClosedInstanceThunkOverGenericMethod) || - m_functionPointer == GetThunk(ObjectArrayThunk)) - return m_helperObject; + // Closed static delegates place a value in _helperObject that they pass to the target method. + if (_functionPointer == GetThunk(ClosedStaticThunk) || + _functionPointer == GetThunk(ClosedInstanceThunkOverGenericMethod) || + _functionPointer == GetThunk(ObjectArrayThunk)) + return _helperObject; - // Other non-closed thunks can be identified as the m_firstParameter field points at this. - if (object.ReferenceEquals(m_firstParameter, this)) + // Other non-closed thunks can be identified as the _firstParameter field points at this. + if (object.ReferenceEquals(_firstParameter, this)) { return null; } // NativeFunctionPointerWrapper used by marshalled function pointers is not returned as a public target - if (m_firstParameter is NativeFunctionPointerWrapper) + if (_firstParameter is NativeFunctionPointerWrapper) { return null; } - // Closed instance delegates place a value in m_firstParameter, and we've ruled out all other types of delegates - return m_firstParameter; + // Closed instance delegates place a value in _firstParameter, and we've ruled out all other types of delegates + return _firstParameter; } } @@ -317,9 +317,9 @@ public object? Target // V1 api: Creates open delegates to static methods only, relaxed signature checking disallowed. public static Delegate CreateDelegate(Type type, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type target, string method, bool ignoreCase, bool throwOnBindFailure) => ReflectionAugments.ReflectionCoreCallbacks.CreateDelegate(type, target, method, ignoreCase, throwOnBindFailure); - internal IntPtr TryGetOpenStaticFunctionPointer() => (GetThunk(OpenStaticThunk) == m_functionPointer) ? m_extraFunctionPointerOrData : 0; + internal IntPtr TryGetOpenStaticFunctionPointer() => (GetThunk(OpenStaticThunk) == _functionPointer) ? _extraFunctionPointerOrData : 0; - internal NativeFunctionPointerWrapper? TryGetNativeFunctionPointerWrapper() => m_firstParameter as NativeFunctionPointerWrapper; + internal NativeFunctionPointerWrapper? TryGetNativeFunctionPointerWrapper() => _firstParameter as NativeFunctionPointerWrapper; internal static unsafe bool InternalEqualTypes(object a, object b) { @@ -344,9 +344,9 @@ internal static unsafe Delegate CreateObjectArrayDelegate(Type t, Func(RuntimeImports.RhNewObject(this.GetMethodTable())); // Performance optimization - if this already points to a true multicast delegate, - // copy m_functionPointer field rather than calling GetThunk to get it - result.m_functionPointer = thisIsMultiCastAlready ? m_functionPointer : GetThunk(MulticastThunk); - result.m_firstParameter = result; - result.m_helperObject = invocationList; - result.m_extraFunctionPointerOrData = (IntPtr)invocationCount; + // copy _functionPointer field rather than calling GetThunk to get it + result._functionPointer = thisIsMultiCastAlready ? _functionPointer : GetThunk(MulticastThunk); + result._firstParameter = result; + result._helperObject = invocationList; + result._extraFunctionPointerOrData = (IntPtr)invocationCount; return result; } @@ -415,10 +415,10 @@ private static bool TrySetSlot(Wrapper[] a, int index, Delegate o) // Optimize this case, because it's cheaper than copying the array. if (a[index].Value is Delegate dd) { - if (object.ReferenceEquals(dd.m_firstParameter, o.m_firstParameter) && - object.ReferenceEquals(dd.m_helperObject, o.m_helperObject) && - dd.m_extraFunctionPointerOrData == o.m_extraFunctionPointerOrData && - dd.m_functionPointer == o.m_functionPointer) + if (object.ReferenceEquals(dd._firstParameter, o._firstParameter) && + object.ReferenceEquals(dd._helperObject, o._helperObject) && + dd._extraFunctionPointerOrData == o._extraFunctionPointerOrData && + dd._functionPointer == o._functionPointer) { return true; } @@ -441,13 +441,13 @@ protected virtual Delegate CombineImpl(Delegate? d) throw new InvalidOperationException(); int followCount = 1; - Wrapper[]? followList = d.m_helperObject as Wrapper[]; + Wrapper[]? followList = d._helperObject as Wrapper[]; if (followList != null) - followCount = (int)d.m_extraFunctionPointerOrData; + followCount = (int)d._extraFunctionPointerOrData; int resultCount; Wrapper[]? resultList; - if (m_helperObject is not Wrapper[] invocationList) + if (_helperObject is not Wrapper[] invocationList) { resultCount = 1 + followCount; resultList = new Wrapper[resultCount]; @@ -465,7 +465,7 @@ protected virtual Delegate CombineImpl(Delegate? d) } else { - int invocationCount = (int)m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; resultCount = invocationCount + followCount; resultList = null; if (resultCount <= invocationList.Length) @@ -553,9 +553,9 @@ private static bool EqualInvocationLists(Wrapper[] a, Wrapper[] b, int start, in // if (d is null) return this; - if (d.m_helperObject is not Wrapper[] dInvocationList) + if (d._helperObject is not Wrapper[] dInvocationList) { - if (m_helperObject is not Wrapper[] invocationList) + if (_helperObject is not Wrapper[] invocationList) { // they are both not real Multicast if (this.Equals(d)) @@ -563,7 +563,7 @@ private static bool EqualInvocationLists(Wrapper[] a, Wrapper[] b, int start, in } else { - int invocationCount = (int)m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; for (int i = invocationCount; --i >= 0;) { if (d.Equals(invocationList[i])) @@ -584,10 +584,10 @@ private static bool EqualInvocationLists(Wrapper[] a, Wrapper[] b, int start, in } else { - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { - int invocationCount = (int)m_extraFunctionPointerOrData; - int dInvocationCount = (int)d.m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; + int dInvocationCount = (int)d._extraFunctionPointerOrData; for (int i = invocationCount - dInvocationCount; i >= 0; i--) { if (EqualInvocationLists(invocationList, dInvocationList, i, dInvocationCount)) @@ -617,11 +617,11 @@ private static bool EqualInvocationLists(Wrapper[] a, Wrapper[] b, int start, in public virtual Delegate[] GetInvocationList() { - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { // Create an array of delegate copies and each // element into the array - int invocationCount = (int)m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; var del = new Delegate[invocationCount]; for (int i = 0; i < del.Length; i++) @@ -645,15 +645,15 @@ public override bool Equals([NotNullWhen(true)] object? obj) Debug.Assert(obj is Delegate, "Shouldn't have failed here since we already checked the types are the same!"); var d = Unsafe.As(obj); - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { - if (d.m_extraFunctionPointerOrData != m_extraFunctionPointerOrData) + if (d._extraFunctionPointerOrData != _extraFunctionPointerOrData) return false; - if (d.m_helperObject is not Wrapper[] dInvocationList) + if (d._helperObject is not Wrapper[] dInvocationList) return false; - int invocationCount = (int)m_extraFunctionPointerOrData; + int invocationCount = (int)_extraFunctionPointerOrData; for (int i = 0; i < invocationCount; i++) { if (!invocationList[i].Value.Equals(dInvocationList[i].Value)) @@ -662,29 +662,29 @@ public override bool Equals([NotNullWhen(true)] object? obj) return true; } - if (!object.ReferenceEquals(m_helperObject, d.m_helperObject) || - (!FunctionPointerOps.Compare(m_extraFunctionPointerOrData, d.m_extraFunctionPointerOrData)) || - (!FunctionPointerOps.Compare(m_functionPointer, d.m_functionPointer))) + if (!object.ReferenceEquals(_helperObject, d._helperObject) || + (!FunctionPointerOps.Compare(_extraFunctionPointerOrData, d._extraFunctionPointerOrData)) || + (!FunctionPointerOps.Compare(_functionPointer, d._functionPointer))) { return false; } - // Those delegate kinds with thunks put themselves into the m_firstParameter, so we can't - // blindly compare the m_firstParameter fields for equality. - if (object.ReferenceEquals(m_firstParameter, this)) + // Those delegate kinds with thunks put themselves into the _firstParameter, so we can't + // blindly compare the _firstParameter fields for equality. + if (object.ReferenceEquals(_firstParameter, this)) { - return object.ReferenceEquals(d.m_firstParameter, d); + return object.ReferenceEquals(d._firstParameter, d); } - return object.ReferenceEquals(m_firstParameter, d.m_firstParameter); + return object.ReferenceEquals(_firstParameter, d._firstParameter); } public override int GetHashCode() { - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { int hash = 0; - for (int i = 0; i < (int)m_extraFunctionPointerOrData; i++) + for (int i = 0; i < (int)_extraFunctionPointerOrData; i++) { hash = hash * 33 + invocationList[i].Value.GetHashCode(); } @@ -694,14 +694,14 @@ public override int GetHashCode() return base.GetHashCode(); } - public bool HasSingleTarget => m_helperObject is not Wrapper[]; + public bool HasSingleTarget => _helperObject is not Wrapper[]; // Used by delegate invocation list enumerator internal Delegate? TryGetAt(int index) { - if (m_helperObject is Wrapper[] invocationList) + if (_helperObject is Wrapper[] invocationList) { - return ((uint)index < (uint)m_extraFunctionPointerOrData) ? invocationList[index].Value : null; + return ((uint)index < (uint)_extraFunctionPointerOrData) ? invocationList[index].Value : null; } return (index == 0) ? this : null; diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 5d10b4159feca5..999644d6a3630a 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3283,7 +3283,7 @@ private void getEEInfo(ref CORINFO_EE_INFO pEEInfoOut) pEEInfoOut.inlinedCallFrameInfo.size = (uint)SizeOfPInvokeTransitionFrame; - pEEInfoOut.offsetOfDelegateInstance = (uint)pointerSize; // Delegate::m_firstParameter + pEEInfoOut.offsetOfDelegateInstance = (uint)pointerSize; // Delegate::_firstParameter pEEInfoOut.offsetOfDelegateFirstTarget = OffsetOfDelegateFirstTarget; pEEInfoOut.sizeOfReversePInvokeFrame = (uint)SizeOfReversePInvokeTransitionFrame; diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateMethodILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateMethodILEmitter.cs index 905d40fd2828ea..baa2ff1f354b0b 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateMethodILEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateMethodILEmitter.cs @@ -46,8 +46,8 @@ public static MethodIL EmitIL(MethodDesc method) ILEmitter emit = new ILEmitter(); TypeDesc delegateType = context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType; - FieldDesc firstParameterField = delegateType.GetKnownField("m_firstParameter"); - FieldDesc functionPointerField = delegateType.GetKnownField("m_functionPointer"); + FieldDesc firstParameterField = delegateType.GetKnownField("_firstParameter"); + FieldDesc functionPointerField = delegateType.GetKnownField("_functionPointer"); ILCodeStream codeStream = emit.NewCodeStream(); codeStream.EmitLdArg(0); diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs index 66363197e7c18f..cf630ef56f8484 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs @@ -63,7 +63,7 @@ protected FieldDesc ExtraFunctionPointerOrDataField { get { - return SystemDelegateType.GetKnownField("m_extraFunctionPointerOrData"); + return SystemDelegateType.GetKnownField("_extraFunctionPointerOrData"); } } @@ -71,7 +71,7 @@ protected FieldDesc HelperObjectField { get { - return SystemDelegateType.GetKnownField("m_helperObject"); + return SystemDelegateType.GetKnownField("_helperObject"); } } @@ -79,7 +79,7 @@ protected FieldDesc FirstParameterField { get { - return SystemDelegateType.GetKnownField("m_firstParameter"); + return SystemDelegateType.GetKnownField("_firstParameter"); } } @@ -87,7 +87,7 @@ protected FieldDesc FunctionPointerField { get { - return SystemDelegateType.GetKnownField("m_functionPointer"); + return SystemDelegateType.GetKnownField("_functionPointer"); } } @@ -319,7 +319,7 @@ public override MethodIL EmitIL() } // Fill in delegateArrayLocal - // Wrapper[] delegateArrayLocal = (Wrapper[])this.m_helperObject + // Wrapper[] delegateArrayLocal = (Wrapper[])this._helperObject // ldarg.0 (this pointer) // ldfld Delegate.HelperObjectField @@ -331,9 +331,9 @@ public override MethodIL EmitIL() codeStream.EmitStLoc(delegateArrayLocal); // Fill in invocationCountLocal - // int invocationCountLocal = this.m_extraFunctionPointerOrData + // int invocationCountLocal = this._extraFunctionPointerOrData // ldarg.0 (this pointer) - // ldfld Delegate.m_extraFunctionPointerOrData + // ldfld Delegate._extraFunctionPointerOrData // stloc invocationCountLocal codeStream.EmitLdArg(0); codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(ExtraFunctionPointerOrDataField)); @@ -370,10 +370,10 @@ public override MethodIL EmitIL() // returnValueLocal = delegateToCallLocal(...); // ldloc delegateToCallLocal - // ldfld System.Delegate.m_firstParameter + // ldfld System.Delegate._firstParameter // ldarg 1, n // ldloc delegateToCallLocal - // ldfld System.Delegate.m_functionPointer + // ldfld System.Delegate._functionPointer // calli returnValueType thiscall (all the params) // IF there is a return value // stloc returnValueLocal @@ -504,7 +504,7 @@ public override MethodIL EmitIL() // args[1] = param1; // ... // try { - // ret = ((Func)dlg.m_helperObject)(args); + // ret = ((Func)dlg._helperObject)(args); // } finally { // param0 = (T0)args[0]; // only generated for each byref argument // } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs index e2e7a8b756e5ae..014f0815db6161 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs @@ -2784,16 +2784,16 @@ public void WriteContent(ref ObjectDataBuilder builder, ISymbolNode thisNode, No { Debug.Assert(creationInfo.Constructor.Method.Name == "InitializeOpenStaticThunk"); - // m_firstParameter + // _firstParameter builder.EmitPointerReloc(thisNode); - // m_helperObject + // _helperObject builder.EmitZeroPointer(); - // m_extraFunctionPointerOrData + // _extraFunctionPointerOrData builder.EmitPointerReloc(creationInfo.GetTargetNode(factory)); - // m_functionPointer + // _functionPointer Debug.Assert(creationInfo.Thunk != null); builder.EmitPointerReloc(creationInfo.Thunk); } @@ -2801,16 +2801,16 @@ public void WriteContent(ref ObjectDataBuilder builder, ISymbolNode thisNode, No { Debug.Assert(creationInfo.Constructor.Method.Name == "InitializeClosedInstance"); - // m_firstParameter + // _firstParameter _firstParameter.WriteFieldData(ref builder, factory); - // m_helperObject + // _helperObject builder.EmitZeroPointer(); - // m_extraFunctionPointerOrData + // _extraFunctionPointerOrData builder.EmitZeroPointer(); - // m_functionPointer + // _functionPointer builder.EmitPointerReloc(factory.CanonicalEntrypoint(_methodPointed)); } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index ad83b1eb42a5d6..6703771777d34e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -456,7 +456,7 @@ unsafe partial class CorInfoImpl { private const CORINFO_RUNTIME_ABI TargetABI = CORINFO_RUNTIME_ABI.CORINFO_CORECLR_ABI; - private uint OffsetOfDelegateFirstTarget => (uint)(3 * PointerSize); // Delegate::m_functionPointer + private uint OffsetOfDelegateFirstTarget => (uint)(3 * PointerSize); // Delegate::_functionPointer private readonly ReadyToRunCodegenCompilation _compilation; private MethodWithGCInfo _methodCodeNode; diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 8755580e3f2903..c3f89c5a86ffb9 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -28,7 +28,7 @@ internal unsafe partial class CorInfoImpl { private const CORINFO_RUNTIME_ABI TargetABI = CORINFO_RUNTIME_ABI.CORINFO_NATIVEAOT_ABI; - private uint OffsetOfDelegateFirstTarget => (uint)(4 * PointerSize); // Delegate::m_functionPointer + private uint OffsetOfDelegateFirstTarget => (uint)(4 * PointerSize); // Delegate::_functionPointer private int SizeOfReversePInvokeTransitionFrame => 2 * PointerSize; private RyuJitCompilation _compilation; From c56cb5fd127c2d1159faa62932fdb5c3b1586665 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Mar 2024 13:12:44 -0800 Subject: [PATCH 04/14] Fix Mono --- .../src/System/MulticastDelegate.Mono.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mono/System.Private.CoreLib/src/System/MulticastDelegate.Mono.cs b/src/mono/System.Private.CoreLib/src/System/MulticastDelegate.Mono.cs index e634afd23448d0..73aacd9ca0e0c6 100644 --- a/src/mono/System.Private.CoreLib/src/System/MulticastDelegate.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/MulticastDelegate.Mono.cs @@ -131,6 +131,10 @@ protected sealed override Delegate CombineImpl(Delegate? follow) if (follow == null) return this; + // Verify that the types are the same... + if (!InternalEqualTypes(this, follow)) + throw new ArgumentException(SR.Arg_DlgtTypeMis); + MulticastDelegate other = (MulticastDelegate)follow; MulticastDelegate ret = AllocDelegateLike_internal(this); From be9c7eb9088cfe4ef863acdc328120545dcebe8c Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 2 Mar 2024 13:45:01 -0800 Subject: [PATCH 05/14] Add delegate resurrection test --- .../GetDelegateForFunctionPointerTests.cs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs index abcf514822786d..c1a599b1cd9a36 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Reflection; using System.Reflection.Emit; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices.Tests.Common; using Xunit; @@ -153,6 +154,54 @@ public void GetDelegateForFunctionPointer_CantCast_ThrowsInvalidCastException() GC.KeepAlive(d); } + [Fact] + public void GetDelegateForFunctionPointer_Resurrection() + { + GCHandle handle = Alloc(); + + if (PlatformDetection.IsPreciseGcSupported) + { + while (handle.Target != null) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + } + + handle.Free(); + + [MethodImpl(MethodImplOptions.NoInlining)] + static GCHandle Alloc() + { + GCHandle gcHandle = default; + gcHandle = GCHandle.Alloc(new FreachableObject(), GCHandleType.WeakTrackResurrection); + return gcHandle; + } + } + + private class FreachableObject + { + private readonly Action _del; + private readonly IntPtr _fnptr; + private int _count; + + internal FreachableObject() + { + _del = new Action(() => { }); + _fnptr = Marshal.GetFunctionPointerForDelegate(_del); + } + + ~FreachableObject() + { + Assert.Same(Marshal.GetDelegateForFunctionPointer(_fnptr), _del); + + if (_count++ < 4) + { + GC.ReRegisterForFinalize(this); + } + } + } + public delegate void GenericDelegate(T t); public delegate void NonGenericDelegate(string t); public delegate void OtherNonGenericDelegate(string t); From acc7542d98753d1bf01675513e99df81d4ce2c0d Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sun, 3 Mar 2024 07:46:57 -0800 Subject: [PATCH 06/14] Fixes --- .../Marshal/GetDelegateForFunctionPointerTests.cs | 8 +++++++- .../System.Private.CoreLib/src/System/Delegate.Mono.cs | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs index c1a599b1cd9a36..8338618cc28813 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs @@ -161,7 +161,7 @@ public void GetDelegateForFunctionPointer_Resurrection() if (PlatformDetection.IsPreciseGcSupported) { - while (handle.Target != null) + while (IsNullTarget(handle)) { GC.Collect(); GC.WaitForPendingFinalizers(); @@ -177,6 +177,12 @@ static GCHandle Alloc() gcHandle = GCHandle.Alloc(new FreachableObject(), GCHandleType.WeakTrackResurrection); return gcHandle; } + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IsNullTarget(GCHandle handle) + { + return handle.Target is null; + } } private class FreachableObject diff --git a/src/mono/System.Private.CoreLib/src/System/Delegate.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Delegate.Mono.cs index 9f71ff470f3e4a..98ade01313be25 100644 --- a/src/mono/System.Private.CoreLib/src/System/Delegate.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Delegate.Mono.cs @@ -556,7 +556,7 @@ private DelegateData CreateDelegateData() return delegate_data; } - private static bool InternalEqualTypes(object source, object value) + internal static bool InternalEqualTypes(object source, object value) { return source.GetType() == value.GetType(); } From 55b3672ca9436eeb5907f4884c4ddcc5ac877eac Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Mon, 4 Mar 2024 21:27:04 -0800 Subject: [PATCH 07/14] Fix bug --- .../nativeaot/System.Private.CoreLib/src/System/Delegate.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs index b3ba7a5e66f2ee..9b33e9e6d53670 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs @@ -566,7 +566,7 @@ private static bool EqualInvocationLists(Wrapper[] a, Wrapper[] b, int start, in int invocationCount = (int)_extraFunctionPointerOrData; for (int i = invocationCount; --i >= 0;) { - if (d.Equals(invocationList[i])) + if (d.Equals(invocationList[i].Value)) { if (invocationCount == 2) { From 7a2df9b028726a82326c25c4e9efdbb467942b07 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Mon, 4 Mar 2024 21:40:31 -0800 Subject: [PATCH 08/14] Fix comments --- .../Common/TypeSystem/IL/Stubs/DelegateThunks.cs | 15 ++++++++------- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 2 +- .../JitInterface/CorInfoImpl.RyuJit.cs | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs index cf630ef56f8484..3845514eb4db0a 100644 --- a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs +++ b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateThunks.cs @@ -319,11 +319,11 @@ public override MethodIL EmitIL() } // Fill in delegateArrayLocal - // Wrapper[] delegateArrayLocal = (Wrapper[])this._helperObject + // Delegate.Wrapper[] delegateArrayLocal = (Delegate.Wrapper[])this._helperObject // ldarg.0 (this pointer) - // ldfld Delegate.HelperObjectField - // castclass Wrapper[] + // ldfld Delegate._helperObject + // castclass Delegate.Wrapper[] // stloc delegateArrayLocal codeStream.EmitLdArg(0); codeStream.Emit(ILOpcode.ldfld, emitter.NewToken(HelperObjectField)); @@ -332,6 +332,7 @@ public override MethodIL EmitIL() // Fill in invocationCountLocal // int invocationCountLocal = this._extraFunctionPointerOrData + // ldarg.0 (this pointer) // ldfld Delegate._extraFunctionPointerOrData // stloc invocationCountLocal @@ -357,8 +358,8 @@ public override MethodIL EmitIL() // ldloc delegateArrayLocal // ldloc iteratorLocal - // ldelema System.Delegate - // ldfld Wrapper.Value + // ldelema Delegate.Wrapper + // ldfld Delegate.Wrapper.Value // stloc delegateToCallLocal codeStream.EmitLdLoc(delegateArrayLocal); codeStream.EmitLdLoc(iteratorLocal); @@ -370,10 +371,10 @@ public override MethodIL EmitIL() // returnValueLocal = delegateToCallLocal(...); // ldloc delegateToCallLocal - // ldfld System.Delegate._firstParameter + // ldfld Delegate._firstParameter // ldarg 1, n // ldloc delegateToCallLocal - // ldfld System.Delegate._functionPointer + // ldfld Delegate._functionPointer // calli returnValueType thiscall (all the params) // IF there is a return value // stloc returnValueLocal diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 6703771777d34e..96911a9289804d 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -456,7 +456,7 @@ unsafe partial class CorInfoImpl { private const CORINFO_RUNTIME_ABI TargetABI = CORINFO_RUNTIME_ABI.CORINFO_CORECLR_ABI; - private uint OffsetOfDelegateFirstTarget => (uint)(3 * PointerSize); // Delegate::_functionPointer + private uint OffsetOfDelegateFirstTarget => (uint)(3 * PointerSize); // Delegate._methodPtr private readonly ReadyToRunCodegenCompilation _compilation; private MethodWithGCInfo _methodCodeNode; diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index c3f89c5a86ffb9..d6de83e8782697 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -28,7 +28,7 @@ internal unsafe partial class CorInfoImpl { private const CORINFO_RUNTIME_ABI TargetABI = CORINFO_RUNTIME_ABI.CORINFO_NATIVEAOT_ABI; - private uint OffsetOfDelegateFirstTarget => (uint)(4 * PointerSize); // Delegate::_functionPointer + private uint OffsetOfDelegateFirstTarget => (uint)(4 * PointerSize); // Delegate._functionPointer private int SizeOfReversePInvokeTransitionFrame => 2 * PointerSize; private RyuJitCompilation _compilation; From 7f2f8446e8ff70e74fba0773798091490b978ddd Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 5 Mar 2024 17:25:30 -0800 Subject: [PATCH 09/14] Bug fixes --- .../CompilerServices/FunctionPointerOps.cs | 11 +++++++ .../src/System/Delegate.cs | 30 ++++++++++++++++--- .../NativeFunctionPointerWrapper.cs | 9 ++---- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs index a0f5312c92304e..e77a106b252baf 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs @@ -149,5 +149,16 @@ public static unsafe bool Compare(IntPtr functionPointerA, IntPtr functionPointe return pointerDefA->MethodFunctionPointer == pointerDefB->MethodFunctionPointer; } + + public static unsafe int GetHashCode(IntPtr functionPointer) + { + if (!IsGenericMethodPointer(functionPointer)) + { + return functionPointer.GetHashCode(); + } + + GenericMethodDescriptor* pointerDef = ConvertToGenericDescriptor(functionPointer); + return pointerDef->GetHashCode(); + } } } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs index 9b33e9e6d53670..bc8c517acb8ea9 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Delegate.cs @@ -662,6 +662,14 @@ public override bool Equals([NotNullWhen(true)] object? obj) return true; } + if (_firstParameter is NativeFunctionPointerWrapper nativeFunctionPointerWrapper) + { + if (d._firstParameter is not NativeFunctionPointerWrapper dnativeFunctionPointerWrapper) + return false; + + return nativeFunctionPointerWrapper.NativeFunctionPointer == dnativeFunctionPointerWrapper.NativeFunctionPointer; + } + if (!object.ReferenceEquals(_helperObject, d._helperObject) || (!FunctionPointerOps.Compare(_extraFunctionPointerOrData, d._extraFunctionPointerOrData)) || (!FunctionPointerOps.Compare(_functionPointer, d._functionPointer))) @@ -683,15 +691,29 @@ public override int GetHashCode() { if (_helperObject is Wrapper[] invocationList) { - int hash = 0; + int multiCastHash = 0; for (int i = 0; i < (int)_extraFunctionPointerOrData; i++) { - hash = hash * 33 + invocationList[i].Value.GetHashCode(); + multiCastHash = multiCastHash * 33 + invocationList[i].Value.GetHashCode(); } - return hash; + return multiCastHash; + } + + if (_firstParameter is NativeFunctionPointerWrapper nativeFunctionPointerWrapper) + { + return nativeFunctionPointerWrapper.NativeFunctionPointer.GetHashCode(); + } + + int hash = RuntimeHelpers.GetHashCode(_helperObject) + + 7 * FunctionPointerOps.GetHashCode(_extraFunctionPointerOrData) + + 13 * FunctionPointerOps.GetHashCode(_functionPointer); + + if (!object.ReferenceEquals(_firstParameter, this)) + { + hash += 17 * RuntimeHelpers.GetHashCode(_firstParameter); } - return base.GetHashCode(); + return hash; } public bool HasSingleTarget => _helperObject is not Wrapper[]; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeFunctionPointerWrapper.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeFunctionPointerWrapper.cs index bb3ea2b5f10e27..4571165530abfe 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeFunctionPointerWrapper.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeFunctionPointerWrapper.cs @@ -12,14 +12,9 @@ internal abstract class NativeFunctionPointerWrapper { public NativeFunctionPointerWrapper(IntPtr nativeFunctionPointer) { - m_nativeFunctionPointer = nativeFunctionPointer; + NativeFunctionPointer = nativeFunctionPointer; } - private IntPtr m_nativeFunctionPointer; - - public IntPtr NativeFunctionPointer - { - get { return m_nativeFunctionPointer; } - } + public IntPtr NativeFunctionPointer { get; } } } From b6edd29423cc1e666a76bc6cf0fae48e55379f19 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 5 Mar 2024 23:08:54 -0800 Subject: [PATCH 10/14] Fix test --- .../Marshal/GetDelegateForFunctionPointerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs index 8338618cc28813..76d5e54ef2c4ea 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs @@ -161,7 +161,7 @@ public void GetDelegateForFunctionPointer_Resurrection() if (PlatformDetection.IsPreciseGcSupported) { - while (IsNullTarget(handle)) + while (!IsNullTarget(handle)) { GC.Collect(); GC.WaitForPendingFinalizers(); From e7d7550b2d92a689141dd1796361e49cc7ecece8 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 6 Mar 2024 21:01:37 -0800 Subject: [PATCH 11/14] Fix resurrection test to actually fail --- .../Marshal/GetDelegateForFunctionPointerTests.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs index 76d5e54ef2c4ea..8c03470429ee90 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs @@ -193,15 +193,20 @@ private class FreachableObject internal FreachableObject() { - _del = new Action(() => { }); + _del = new Action(SomeFunction); _fnptr = Marshal.GetFunctionPointerForDelegate(_del); } + // Note: This method cannot be replaced by a lambda for the test to trigger the delegate resurrection + private void SomeFunction() + { + } + ~FreachableObject() { Assert.Same(Marshal.GetDelegateForFunctionPointer(_fnptr), _del); - if (_count++ < 4) + if (_count++ < 3) { GC.ReRegisterForFinalize(this); } From de0c16a3749fe58f6169fcb1c93e1c14b175fd7c Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 6 Mar 2024 21:02:09 -0800 Subject: [PATCH 12/14] Fix --- .../Runtime/InteropServices/PInvokeMarshal.cs | 23 ++++++++++++------- .../delegate/delegate/DelegateCombine1.csproj | 2 +- .../delegate/DelegateCombineImpl.csproj | 2 +- .../delegate/delegate/DelegateEquals1.csproj | 1 + .../delegate/DelegateGetHashCode1.csproj | 1 + 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs index 87e3303b6f154b..0e1ad8d0486534 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs @@ -128,12 +128,12 @@ public PInvokeDelegateThunk(Delegate del) // unsafe { - ContextData = (IntPtr)NativeMemory.Alloc((nuint)(2 * IntPtr.Size)); + ContextData = (IntPtr)NativeMemory.AllocZeroed((nuint)(2 * IntPtr.Size)); ThunkContextData* thunkData = (ThunkContextData*)ContextData; // allocate a weak GChandle for the delegate - thunkData->Handle = GCHandle.Alloc(del, GCHandleType.Weak); + thunkData->Handle = GCHandle.Alloc(del, GCHandleType.WeakTrackResurrection); thunkData->FunctionPtr = openStaticFunctionPointer; } @@ -145,24 +145,31 @@ public PInvokeDelegateThunk(Delegate del) ~PInvokeDelegateThunk() { - // Free the thunk - if (Thunk != IntPtr.Zero) - { - RuntimeAugments.FreeThunk(s_thunkPoolHeap, Thunk); - } - if (ContextData != IntPtr.Zero) { // free the GCHandle GCHandle handle = ((ThunkContextData*)ContextData)->Handle; if (handle.IsAllocated) { + // If the delegate is still alive, defer finalization. + if (handle.Target != null) + { + GC.ReRegisterForFinalize(this); + return; + } + handle.Free(); } // Free the allocated context data memory NativeMemory.Free((void*)ContextData); } + + // Free the thunk + if (Thunk != IntPtr.Zero) + { + RuntimeAugments.FreeThunk(s_thunkPoolHeap, Thunk); + } } } diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj index 3ca1a050735ef8..9ba31e81100970 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombine1.csproj @@ -1,7 +1,7 @@ true - 0 + 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj index 7c4ca6e8d532b5..bcdc37ff1fd6b5 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateCombineImpl.csproj @@ -1,7 +1,7 @@ true - 0 + 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj index 07251f373e3edc..675f0977c04bf5 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateEquals1.csproj @@ -1,6 +1,7 @@ true + 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj index 6a33e215d0fc51..93d280cc34678b 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateGetHashCode1.csproj @@ -1,6 +1,7 @@ true + 1 From d662517b3dab224b6bdb259119c30327c12eed84 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 6 Mar 2024 21:12:38 -0800 Subject: [PATCH 13/14] Undo unnecessary change --- .../CoreMangLib/system/delegate/delegate/DelegateRemove.csproj | 1 + .../system/delegate/delegate/delegateRemoveImpl.csproj | 1 + 2 files changed, 2 insertions(+) diff --git a/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj b/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj index 87fdce6e859941..fccdb6634bb1b5 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/DelegateRemove.csproj @@ -1,6 +1,7 @@ true + 1 diff --git a/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj b/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj index 6d021787920364..8ef2b3c7819d32 100644 --- a/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj +++ b/src/tests/CoreMangLib/system/delegate/delegate/delegateRemoveImpl.csproj @@ -1,6 +1,7 @@ true + 1 From d3e40f6e034f9a0425cb5c572f3c9b6079daa846 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 8 Mar 2024 21:44:42 -0800 Subject: [PATCH 14/14] Disable test on Mono --- .../Marshal/GetDelegateForFunctionPointerTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs index 8c03470429ee90..f0745272af906c 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetDelegateForFunctionPointerTests.cs @@ -155,6 +155,7 @@ public void GetDelegateForFunctionPointer_CantCast_ThrowsInvalidCastException() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/99478", TestRuntimes.Mono)] public void GetDelegateForFunctionPointer_Resurrection() { GCHandle handle = Alloc();