diff --git a/docs/design/datacontracts/Object.md b/docs/design/datacontracts/Object.md index b1deda0504f581..f0f6f107748e96 100644 --- a/docs/design/datacontracts/Object.md +++ b/docs/design/datacontracts/Object.md @@ -221,6 +221,7 @@ DelegateInfo GetDelegateInfo(TargetPointer address) Data.Delegate del = new Data.Delegate(target, address); // Classify the delegate from its invocation count and auxiliary pointer. + // This does not handle open virtual delegates correctly. DelegateType delegateType = target.ReadNInt(address + /* Delegate::InvocationCount offset */) switch { 0 => del.MethodPtrAux == TargetCodePointer.Null diff --git a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs index 7539685cb0ad69..cc951806f2a4ea 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; +using System.Reflection.Emit; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Serialization; @@ -218,9 +219,6 @@ protected virtual MethodInfo GetMethodImpl() return (MethodInfo)_helperObject; } - public object? Target => GetTarget(); - - // V1 API. [RequiresUnreferencedCode("The target method might be removed")] public static Delegate? CreateDelegate(Type type, object target, string method, bool ignoreCase, bool throwOnBindFailure) { @@ -256,7 +254,6 @@ protected virtual MethodInfo GetMethodImpl() return d; } - // V1 API. public static Delegate? CreateDelegate(Type type, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.AllMethods)] Type target, string method, bool ignoreCase, bool throwOnBindFailure) { ArgumentNullException.ThrowIfNull(type); @@ -292,7 +289,6 @@ protected virtual MethodInfo GetMethodImpl() return d; } - // V1 API. public static Delegate? CreateDelegate(Type type, MethodInfo method, bool throwOnBindFailure) { ArgumentNullException.ThrowIfNull(type); @@ -327,7 +323,6 @@ protected virtual MethodInfo GetMethodImpl() return d; } - // V2 API. public static Delegate? CreateDelegate(Type type, object? firstArgument, MethodInfo method, bool throwOnBindFailure) { ArgumentNullException.ThrowIfNull(type); @@ -359,12 +354,8 @@ protected virtual MethodInfo GetMethodImpl() return d; } - // - // internal implementation details (FCALLS and utilities) - // - - // V2 internal API. - internal static Delegate CreateDelegateNoSecurityCheck(Type type, object? target, RuntimeMethodHandle method) + internal static Delegate CreateDelegateForDynamicMethod(Type type, object? target, RuntimeMethodHandle method, + DynamicMethod dynamicMethod) { ArgumentNullException.ThrowIfNull(type); @@ -377,10 +368,7 @@ internal static Delegate CreateDelegateNoSecurityCheck(Type type, object? target if (!rtType.IsDelegate()) throw new ArgumentException(SR.Arg_MustBeDelegate, nameof(type)); - // Initialize the method... Delegate d = InternalAlloc(rtType); - // This is a new internal API added in Whidbey. Currently it's only - // used by the dynamic method code to generate a wrapper delegate. // Allow flexible binding options since the target method is // unambiguously provided to us. @@ -389,6 +377,8 @@ internal static Delegate CreateDelegateNoSecurityCheck(Type type, object? target RuntimeMethodHandle.GetDeclaringType(method.GetMethodInfo()), DelegateBindingFlags.RelaxedSignature)) throw new ArgumentException(SR.Arg_DlgtTargMeth); + + d._helperObject = dynamicMethod; return d; } @@ -549,11 +539,6 @@ internal void InitializeVirtualCallStub(IntPtr methodPtr) [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Delegate_InitializeVirtualCallStub")] private static partial void InitializeVirtualCallStub(ObjectHandleOnStack d, IntPtr methodPtr); - - internal virtual object? GetTarget() - { - return (_methodPtrAux == IntPtr.Zero) ? _target : null; - } } // These flags effect the way BindToMethodInfo and BindToMethodName are allowed to bind a delegate to a target method. Their diff --git a/src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs index d5727eb2ce13b4..f9e22594e1a3ad 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs @@ -17,23 +17,19 @@ namespace System [ComVisible(true)] public abstract partial class MulticastDelegate : Delegate { + private object? _invocationList; + // This is set under 3 circumstances // 1. Multicast delegate // 2. Unmanaged function pointer // 3. Open virtual delegate - private object? _invocationList; // Initialized by VM as needed private nint _invocationCount; - internal bool IsUnmanagedFunctionPtr() + private bool IsUnmanagedFunctionPtr() { return _invocationCount == -1; } - internal bool InvocationListLogicallyNull() - { - return (_invocationList == null) || (_invocationList is LoaderAllocator) || (_invocationList is DynamicResolver); - } - [Obsolete(Obsoletions.LegacyFormatterImplMessage, DiagnosticId = Obsoletions.LegacyFormatterImplDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [EditorBrowsable(EditorBrowsableState.Never)] public override void GetObjectData(SerializationInfo info, StreamingContext context) @@ -63,9 +59,9 @@ public sealed override bool Equals([NotNullWhen(true)] object? obj) // there are 3 kind of delegate kinds that fall into this bucket // 1- Multicast (_invocationList is Object[]) // 2- Unmanaged FntPtr (_invocationList == null) - // 3- Open virtual (_invocationCount == MethodDesc of target, _invocationList == null, LoaderAllocator, or DynamicResolver) + // 3- Open virtual (_invocationCount == MethodDesc of target, _invocationList == null) - if (InvocationListLogicallyNull()) + if (HasSingleTarget) { if (IsUnmanagedFunctionPtr()) { @@ -164,12 +160,6 @@ internal MulticastDelegate NewMulticastDelegate(object[] invocationList, int inv return NewMulticastDelegate(invocationList, invocationCount, false); } - internal void StoreDynamicMethod(MethodInfo dynamicMethod) - { - Debug.Assert(_invocationCount == 0); - _helperObject = dynamicMethod; - } - // This method will combine this delegate with the passed delegate // to form a new delegate. internal new Delegate CombineImpl(Delegate? follow) @@ -382,7 +372,7 @@ private static bool EqualInvocationLists(object[] a, object[] b, int start, int return del; } - internal new bool HasSingleTarget => _invocationList is not object[]; + internal new bool HasSingleTarget => _invocationList is null; // Used by delegate invocation list enumerator internal object? /* Delegate? */ TryGetAt(int index) @@ -418,29 +408,19 @@ public sealed override int GetHashCode() } } - internal override object? GetTarget() + internal new object? Target { - if (_invocationCount != 0) + get { - // _invocationCount != 0 we are in one of these cases: - // - Multicast -> return the target of the last delegate in the list - // - unmanaged function pointer - return null - // - virtual open delegate - return null - if (InvocationListLogicallyNull()) - { - // both open virtual and ftn pointer return null for the target - return null; - } - else + Delegate instance = this; + if (_invocationList is object[] invocationList) { - if (_invocationList is object[] invocationList) - { - int invocationCount = (int)_invocationCount; - return ((Delegate)invocationList[invocationCount - 1]).GetTarget(); - } + // Multicast -> return the target of the last delegate in the list + int invocationCount = (int)_invocationCount; + instance = (Delegate)invocationList[invocationCount - 1]; } + return instance._methodPtrAux == 0 ? instance._target : null; } - return base.GetTarget(); } protected override MethodInfo GetMethodImpl() diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs index abdb8be14b27a3..77aa9fc4fc77b9 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs @@ -55,10 +55,7 @@ public sealed override Delegate CreateDelegate(Type delegateType, object? target GC.KeepAlive(methodHandle); } - MulticastDelegate d = (MulticastDelegate)Delegate.CreateDelegateNoSecurityCheck(delegateType, target, GetMethodDescriptor()); - // stash this MethodInfo by brute force. - d.StoreDynamicMethod(this); - return d; + return Delegate.CreateDelegateForDynamicMethod(delegateType, target, GetMethodDescriptor(), this); } // This is guaranteed to return a valid handle diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index d4b39f4024b909..a35d84da2adb12 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -3267,8 +3267,7 @@ DacDbiInterfaceImpl::DelegateType DacDbiInterfaceImpl::GetDelegateType(VMPTR_Obj if (invocationCount == 0) { // If this delegate points to a static function or this is a open virtual delegate, this should be non-null - // Special case: This might fail in a VSD delegate (instance open virtual)... - // TODO: There is the special signatures cases missing. + // This does not handle open virtual delegates correctly. TADDR targetMethodPtr = PCODEToPINSTR(pDelObj->GetMethodPtrAux()); if (targetMethodPtr == (TADDR)NULL) { diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index 527b63af44f77b..ec6d9f4277c91e 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -26,9 +26,6 @@ #include "comcallablewrapper.h" #endif // FEATURE_COMINTEROP -#define DELEGATE_MARKER_UNMANAGEDFPTR -1 - - #ifndef DACCESS_COMPILE #if defined(TARGET_X86) @@ -1812,61 +1809,33 @@ MethodDesc *COMDelegate::GetMethodDesc(OBJECTREF orDelegate) // If you modify this logic, please update cDAC IObject.GetDelegateInfo. - MethodDesc *pMethodHandle = NULL; - DELEGATEREF thisDel = (DELEGATEREF) orDelegate; - DELEGATEREF innerDel = NULL; INT_PTR count = thisDel->GetInvocationCount(); if (count != 0) { // this is one of the following: // - multicast - _invocationList is Array && _invocationCount != 0 - // - unamanaged ftn ptr - _invocationList == NULL && _invocationCount == -1 + // - unamanaged ftn ptr - _invocationList == null && _invocationCount == -1 // - virtual delegate - _invocationList == null && _invocationCount == (target MethodDesc) - // or _invocationList points to a LoaderAllocator/DynamicResolver - innerDel = (DELEGATEREF) thisDel->GetInvocationList(); - bool fOpenVirtualDelegate = false; - if (innerDel != NULL) - { - MethodTable *pMT = innerDel->GetMethodTable(); - if (pMT->IsDelegate()) - return GetMethodDesc(innerDel); - if (!pMT->IsArray()) - { - // must be a virtual one - fOpenVirtualDelegate = true; - } - } - else - { - if (count != DELEGATE_MARKER_UNMANAGEDFPTR) - { - // must be a virtual one - fOpenVirtualDelegate = true; - } - } + // we return the method desc for the invoke for the first two cases + OBJECTREF invocationList = thisDel->GetInvocationList(); + if (invocationList != NULL || count == DELEGATE_MARKER_UNMANAGEDFPTR) + return FindDelegateInvokeMethod(thisDel->GetMethodTable()); - if (fOpenVirtualDelegate) - pMethodHandle = GetMethodDescForOpenVirtualDelegate(thisDel); - else - pMethodHandle = FindDelegateInvokeMethod(thisDel->GetMethodTable()); + return GetMethodDescForOpenVirtualDelegate(thisDel); } - else - { - // Next, check for an open delegate - PCODE code = thisDel->GetMethodPtrAux(); - if (code == (PCODE)NULL) - { - // Must be a normal delegate - code = thisDel->GetMethodPtr(); - } - - pMethodHandle = NonVirtualEntry2MethodDesc(code); + // Next, check for an open delegate + PCODE code = thisDel->GetMethodPtrAux(); + if (code == (PCODE)NULL) + { + // Must be a closed delegate + code = thisDel->GetMethodPtr(); } + MethodDesc *pMethodHandle = NonVirtualEntry2MethodDesc(code); _ASSERTE(pMethodHandle); return pMethodHandle; } @@ -1889,8 +1858,7 @@ BOOL COMDelegate::IsTrueMulticastDelegate(OBJECTREF delegate) OBJECTREF invocationList = ((DELEGATEREF)delegate)->GetInvocationList(); if (invocationList != NULL) { - MethodTable *pMT = invocationList->GetMethodTable(); - isMulticast = pMT->IsArray(); + isMulticast = TRUE; } } diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index 524e3750ee81fa..c8da677080966e 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -1743,6 +1743,7 @@ typedef BStrWrapper* BSTRWRAPPEROBJECTREF; #endif // FEATURE_COMINTEROP +#define DELEGATE_MARKER_UNMANAGEDFPTR (-1) // This class corresponds to System.MulticastDelegate on the managed side. class DelegateObject : public Object diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index e8f55803ece74a..1e0811883df545 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -1242,39 +1242,6 @@ BOOL StubLinkStubManager::TraceDelegateObject(BYTE* pbDel, TraceDestination *tra return res; } - // invocationList is not null, so it can be one of the following: - // Multicast, Static closed (special sig), Secure - - // rule out the static with special sig - BYTE *pbCount = *(BYTE **)(pbDel + DelegateObject::GetOffsetOfInvocationCount()); - if (pbCount == NULL) - { - // it's a static closed, the target lives in _methodAuxPtr - ppbDest = (BYTE **)(pbDel + DelegateObject::GetOffsetOfMethodPtrAux()); - - if (*ppbDest == NULL) - { - // it's not looking good, bail out - LOG((LF_CORDB,LL_INFO10000, "SLSM::TDO: can't trace into it\n")); - return FALSE; - } - - LOG((LF_CORDB,LL_INFO10000, "SLSM::TDO: ppbDest: %p *ppbDest:%p\n", ppbDest, *ppbDest)); - - BOOL res = StubManager::TraceStub((PCODE) (*ppbDest), trace); - - LOG((LF_CORDB,LL_INFO10000, "SLSM::TDO: res: %d, result type: %d\n", (res ? "true" : "false"), trace->GetTraceType())); - - return res; - } - - MethodTable *pType = *(MethodTable**)pbDelInvocationList; - if (pType->IsDelegate()) - { - // this is a secure delegate. The target is hidden inside this field, so recurse. - return TraceDelegateObject(pbDelInvocationList, trace); - } - // Otherwise, we're going for the first invoke of the multi case. // In order to go to the correct spot, we have just have to fish out // slot 0 of the invocation list, and figure out where that's going to, diff --git a/src/libraries/System.Private.CoreLib/src/System/Delegate.cs b/src/libraries/System.Private.CoreLib/src/System/Delegate.cs index 0e4c4d28ec5122..1fddcf0357ba25 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Delegate.cs @@ -79,6 +79,8 @@ public abstract partial class Delegate : ICloneable, ISerializable /// /// true if the has a single invocation target. public bool HasSingleTarget => Unsafe.As(this).HasSingleTarget; + + public object? Target => Unsafe.As(this).Target; #endif /// 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 de3120dac128f2..fa749f1e9a6b64 100644 --- a/src/mono/System.Private.CoreLib/src/System/Delegate.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Delegate.Mono.cs @@ -105,8 +105,6 @@ protected Delegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Al }; } - public object? Target => GetTarget(); - internal virtual object? GetTarget() => _target; public static Delegate CreateDelegate(Type type, object? firstArgument, MethodInfo method, bool throwOnBindFailure) 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 c026cf46a88539..03f3716c908927 100644 --- a/src/mono/System.Private.CoreLib/src/System/MulticastDelegate.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/MulticastDelegate.Mono.cs @@ -268,6 +268,8 @@ private static int LastIndexOf(Delegate[] haystack, Delegate[] needle) } } + internal new object? Target => GetTarget(); + internal override object? GetTarget() { return delegates?.Length > 0 ? delegates[delegates.Length - 1].GetTarget() : base.GetTarget(); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs index 0d56541dbb049c..6607f78295f1d5 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs @@ -163,10 +163,8 @@ public DelegateInfo GetDelegateInfo(TargetPointer address) { Data.Delegate del = _target.ProcessedData.GetOrAdd(address); - // Classify by invocation count first: - // anything other than 0 indicates a multicast/wrapper/special-sig delegate - // that this API does not interpret further. Only when invocationCount==0 - // do MethodPtr/MethodPtrAux unambiguously identify a closed/open delegate. + // Classify by invocation count first to handle multicast and unmanaged. + // This does not handle open virtual delegates correctly. DelegateType delegateType = DelegateType.Unknown; if (del.InvocationCount.Value == 0) {