Cleanup dead delegate code

docs/design/datacontracts/Object.md

@@ -220,14 +220,11 @@ DelegateInfo GetDelegateInfo(TargetPointer address)
 {
     Data.Delegate del = new Data.Delegate(target, address);
 
-    // Classify the delegate from its invocation count and auxiliary pointer.
-    DelegateType delegateType = target.ReadNInt(address + /* Delegate::InvocationCount offset */) switch
+    DelegateType delegateType = DelegateType.Unknown;
+    if (target.ReadPointer(address + /* Delegate::InvocationList offset */) == TargetPointer.Null && target.ReadNInt(address + /* Delegate::InvocationCount offset */) != -1)
     {
-        0  => del.MethodPtrAux == TargetCodePointer.Null
-                ? DelegateType.Closed
-                : DelegateType.Open,
-        _  => DelegateType.Unknown,
-    };
+        delegateType = del.MethodPtrAux == TargetCodePointer.Null ? DelegateType.Closed : DelegateType.Open;
+    }
 
     // Pick the bound object and primary entry point based on the classification.
     // For Closed delegates the target is the bound `this` and MethodPtr is invoked on it.

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,7 +219,7 @@ protected virtual MethodInfo GetMethodImpl()
             return (MethodInfo)_helperObject;
         }
 
-        public object? Target => GetTarget();
+        internal object? GetTarget() => Unsafe.As<MulticastDelegate>(this).GetTarget();
 
         // V1 API.
         [RequiresUnreferencedCode("The target method might be removed")]
@@ -363,8 +364,8 @@ protected virtual MethodInfo GetMethodImpl()
         // 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 +378,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 +387,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 +549,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

src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs

@@ -17,23 +17,19 @@ namespace System
     [ComVisible(true)]
     public abstract partial class MulticastDelegate : Delegate
     {
+        internal 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 nint _invocationCount;
 
         internal 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)
@@ -65,7 +61,7 @@ public sealed override bool Equals([NotNullWhen(true)] object? obj)
                 // 2- Unmanaged FntPtr (_invocationList == null)
                 // 3- Open virtual (_invocationCount == MethodDesc of target, _invocationList == null, LoaderAllocator, or DynamicResolver)
 
-                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,15 @@ public sealed override int GetHashCode()
             }
         }
 
-        internal override object? GetTarget()
+        internal new object? GetTarget()
         {
-            if (_invocationCount != 0)
+            if (_invocationList is object[] invocationList)
             {
-                // _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
-                {
-                    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;
+                return ((Delegate)invocationList[invocationCount - 1]).GetTarget();
             }
-            return base.GetTarget();
+            return _methodPtrAux == 0 ? _target : null;
         }
 
         protected override MethodInfo GetMethodImpl()

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

src/coreclr/debug/daccess/dacdbiimpl.cpp

@@ -3262,23 +3262,14 @@ DacDbiInterfaceImpl::DelegateType DacDbiInterfaceImpl::GetDelegateType(VMPTR_Obj
 
     DelegateType delegateType = DelegateType::kUnknownDelegateType;
     PTR_DelegateObject pDelObj = dac_cast<PTR_DelegateObject>(delegateObject.GetDacPtr());
-    INT_PTR invocationCount = pDelObj->GetInvocationCount();
 
-    if (invocationCount == 0)
+    INT_PTR invocationCount = pDelObj->GetInvocationCount();
+    PTR_Object invocationList = OBJECTREFToObject(pDelObj->GetInvocationList());
+    if (invocationList == NULL && invocationCount != DELEGATE_MARKER_UNMANAGEDFPTR)
     {
-        // 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.
+        // If this delegate points to an open delegate, this should be non-null
         TADDR targetMethodPtr = PCODEToPINSTR(pDelObj->GetMethodPtrAux());
-        if (targetMethodPtr == (TADDR)NULL)
-        {
-            // Static extension methods, other closed static delegates, and instance delegates fall into this category.
-            delegateType = DelegateType::kClosedDelegate;
-        }
-        else
-        {
-            delegateType = DelegateType::kOpenDelegate;
-        }
+        delegateType = targetMethodPtr == (TADDR)NULL ? DelegateType::kClosedDelegate : DelegateType::kOpenDelegate;
     }
     return delegateType;
 }

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;
         }
     }
 

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

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,

src/libraries/System.Private.CoreLib/src/System/Delegate.cs

@@ -79,6 +79,8 @@ public abstract partial class Delegate : ICloneable, ISerializable
         /// </summary>
         /// <value>true if the <see cref="Delegate"/> has a single invocation target.</value>
         public bool HasSingleTarget => Unsafe.As<MulticastDelegate>(this).HasSingleTarget;
+
+        public object? Target => Unsafe.As<MulticastDelegate>(this).GetTarget();
 #endif
 
         /// <summary>

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)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs

@@ -162,18 +162,12 @@ public TargetPointer GetSyncBlockAddress(TargetPointer address)
     public DelegateInfo GetDelegateInfo(TargetPointer address)
     {
         Data.Delegate del = _target.ProcessedData.GetOrAdd<Data.Delegate>(address);
+        Data.MulticastDelegate multicast = _target.ProcessedData.GetOrAdd<Data.MulticastDelegate>(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.
         DelegateType delegateType = DelegateType.Unknown;
-        if (del.InvocationCount.Value == 0)
+        if (multicast.InvocationList == TargetPointer.Null && multicast.InvocationCount.Value != -1)
         {
-            if (del.MethodPtrAux == TargetCodePointer.Null)
-                delegateType = DelegateType.Closed;
-            else
-                delegateType = DelegateType.Open;
+            delegateType = del.MethodPtrAux == TargetCodePointer.Null ? DelegateType.Closed : DelegateType.Open;
         }
 
         (TargetPointer targetObject, TargetCodePointer targetMethodPtr) = delegateType switch