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 ee48dc77e94fe..664f218be6633 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs @@ -478,8 +478,21 @@ internal static unsafe bool InternalEqualTypes(object a, object b) // Used by the ctor. Do not call directly. // The name of this function will appear in managed stacktraces as delegate constructor. - [MethodImpl(MethodImplOptions.InternalCall)] - private extern void DelegateConstruct(object target, IntPtr slot); + private void DelegateConstruct(object target, IntPtr method) + { + // Via reflection you can pass in just about any value for the method. + // We can do some basic verification up front to prevent EE exceptions. + if (method == IntPtr.Zero) + { + throw new ArgumentNullException(nameof(method)); + } + + Delegate _this = this; + Construct(ObjectHandleOnStack.Create(ref _this), ObjectHandleOnStack.Create(ref target), method); + } + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Delegate_Construct")] + private static partial void Construct(ObjectHandleOnStack _this, ObjectHandleOnStack target, IntPtr method); [MethodImpl(MethodImplOptions.InternalCall)] private static extern unsafe void* GetMulticastInvoke(MethodTable* pMT); diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 0c9750c0282cb..770e81a9480e1 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -3391,7 +3391,7 @@ HRESULT DacDbiInterfaceImpl::GetDelegateType(VMPTR_Object delegateObject, Delega // - System.Private.CoreLib!System.Delegate.GetMethodImpl and System.Private.CoreLib!System.MulticastDelegate.GetMethodImpl // - System.Private.CoreLib!System.Delegate.GetTarget and System.Private.CoreLib!System.MulticastDelegate.GetTarget // - coreclr!COMDelegate::GetMethodDesc and coreclr!COMDelegate::FindMethodHandle - // - coreclr!COMDelegate::DelegateConstruct and the delegate type table in + // - coreclr!Delegate_Construct and the delegate type table in // - DELEGATE KINDS TABLE in comdelegate.cpp *delegateType = DelegateType::kUnknownDelegateType; diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index 59a55e2b1cd3d..b3dcb78a7e032 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -726,12 +726,10 @@ VOID GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, SArrayFPtr pairs + // passed out to unmanaged code. // One time init. void COMDelegate::Init() @@ -748,10 +746,10 @@ void COMDelegate::Init() s_pDelegateToFPtrHash = ::new PtrHashMap(); - LockOwner lock = {&COMDelegate::s_DelegateToFPtrHashCrst, IsOwnerOfCrst}; + LockOwner lock = {&s_DelegateToFPtrHashCrst, IsOwnerOfCrst}; s_pDelegateToFPtrHash->Init(TRUE, &lock); - m_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap()); + s_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap()); } #ifdef FEATURE_COMINTEROP @@ -792,7 +790,7 @@ LoaderHeap *DelegateEEClass::GetStubHeap() } -Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth) +static Stub* SetupShuffleThunk(MethodTable* pDelMT, MethodDesc* pTargetMeth) { CONTRACTL { @@ -812,7 +810,7 @@ Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMe StackSArray rShuffleEntryArray; GenerateShuffleArray(pMD, pTargetMeth, &rShuffleEntryArray); - ShuffleThunkCache* pShuffleThunkCache = m_pShuffleThunkCache; + ShuffleThunkCache* pShuffleThunkCache = s_pShuffleThunkCache; LoaderAllocator* pLoaderAllocator = pDelMT->GetLoaderAllocator(); if (pLoaderAllocator->IsCollectible()) @@ -848,8 +846,6 @@ Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMe return pShuffleThunk; } - - static PCODE GetVirtualCallStub(MethodDesc *method, TypeHandle scopeType) { CONTRACTL @@ -1299,11 +1295,11 @@ LPVOID COMDelegate::ConvertToCallback(OBJECTREF pDelegateObj) LPVOID key = (LPVOID)pUMEntryThunk; // Assert that the entry isn't already in the hash. - _ASSERTE((LPVOID)INVALIDENTRY == COMDelegate::s_pDelegateToFPtrHash->LookupValue((UPTR)key, 0)); + _ASSERTE((LPVOID)INVALIDENTRY == s_pDelegateToFPtrHash->LookupValue((UPTR)key, 0)); { - CrstHolder ch(&COMDelegate::s_DelegateToFPtrHashCrst); - COMDelegate::s_pDelegateToFPtrHash->InsertValue((UPTR)key, pUMEntryThunk->GetObjectHandle()); + CrstHolder ch(&s_DelegateToFPtrHashCrst); + s_pDelegateToFPtrHash->InsertValue((UPTR)key, pUMEntryThunk->GetObjectHandle()); } } @@ -1342,7 +1338,7 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT) // Otherwise, we'll treat this as an unmanaged callsite. // Make sure that the pointer doesn't have the value of 1 which is our hash table deleted item marker. LPVOID DelegateHnd = (pUMEntryThunk != NULL) && ((UPTR)pUMEntryThunk != (UPTR)1) - ? COMDelegate::s_pDelegateToFPtrHash->LookupValue((UPTR)pUMEntryThunk, 0) + ? s_pDelegateToFPtrHash->LookupValue((UPTR)pUMEntryThunk, 0) : (LPVOID)INVALIDENTRY; if (DelegateHnd != (LPVOID)INVALIDENTRY) @@ -1476,8 +1472,8 @@ void COMDelegate::RemoveEntryFromFPtrHash(UPTR key) WRAPPER_NO_CONTRACT; // Remove this entry from the lookup hash. - CrstHolder ch(&COMDelegate::s_DelegateToFPtrHashCrst); - COMDelegate::s_pDelegateToFPtrHash->DeleteValue(key, NULL); + CrstHolder ch(&s_DelegateToFPtrHashCrst); + s_pDelegateToFPtrHash->DeleteValue(key, NULL); } extern "C" void QCALLTYPE Delegate_InitializeVirtualCallStub(QCall::ObjectHandleOnStack d, PCODE method) @@ -1566,114 +1562,97 @@ uint32_t MethodDescToNumFixedArgs(MethodDesc *pMD) return data; } -// This is the single constructor for all Delegates. The compiler -// doesn't provide an implementation of the Delegate constructor. We -// provide that implementation through an ECall call to this method. -FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* targetUNSAFE, PCODE method) +// This is the single constructor for all Delegates. The compiler +// doesn't provide an implementation of the Delegate constructor. We +// provide that implementation through a QCall call to this method. +extern "C" void QCALLTYPE Delegate_Construct(QCall::ObjectHandleOnStack _this, QCall::ObjectHandleOnStack target, PCODE method) { - FCALL_CONTRACT; + QCALL_CONTRACT; + // If you modify this logic, please update DacDbiInterfaceImpl::GetDelegateType, DacDbiInterfaceImpl::GetDelegateType, // DacDbiInterfaceImpl::GetDelegateFunctionData, and DacDbiInterfaceImpl::GetDelegateTargetObject. + _ASSERTE(method != (PCODE)NULL); + BEGIN_QCALL; - struct _gc - { - DELEGATEREF refThis; - OBJECTREF target; - } gc; - - gc.refThis = (DELEGATEREF) ObjectToOBJECTREF(refThisUNSAFE); - gc.target = (OBJECTREF) targetUNSAFE; - - HELPER_METHOD_FRAME_BEGIN_PROTECT(gc); + GCX_COOP(); - // via reflection you can pass in just about any value for the method. - // we can do some basic verification up front to prevent EE exceptions. - if (method == (PCODE)NULL) - COMPlusThrowArgumentNull(W("method")); + DELEGATEREF refThis = (DELEGATEREF) ObjectToOBJECTREF(_this.Get()); + _ASSERTE(refThis != NULL); - _ASSERTE(gc.refThis); - _ASSERTE(method); + GCPROTECT_BEGIN(refThis); - // programmers could feed garbage data to DelegateConstruct(). + // Programmers could feed garbage data to DelegateConstruct(). // It's difficult to validate a method code pointer, but at least we'll // try to catch the easy garbage. _ASSERTE(isMemoryReadable(method, 1)); - MethodTable *pMTTarg = NULL; + MethodTable* pMTTarg = NULL; + if (target.Get() != NULL) + pMTTarg = target.Get()->GetMethodTable(); - if (gc.target != NULL) - { - pMTTarg = gc.target->GetMethodTable(); - } - - MethodDesc *pMethOrig = Entry2MethodDesc(method, pMTTarg); - MethodDesc *pMeth = pMethOrig; - - MethodTable* pDelMT = gc.refThis->GetMethodTable(); + MethodTable* pDelMT = refThis->GetMethodTable(); + MethodDesc* pMethOrig = Entry2MethodDesc(method, pMTTarg); + MethodDesc* pMeth = pMethOrig; + _ASSERTE(pMeth != NULL); LOG((LF_STUBS, LL_INFO1000, "In DelegateConstruct: for delegate type %s binding to method %s::%s%s, static = %d\n", pDelMT->GetDebugClassName(), pMeth->m_pszDebugClassName, pMeth->m_pszDebugMethodName, pMeth->m_pszDebugMethodSignature, pMeth->IsStatic())); - _ASSERTE(pMeth); - #ifdef _DEBUG // Assert that everything is OK...This is not some bogus // address...Very unlikely that the code below would work // for a random address in memory.... MethodTable* p = pMeth->GetMethodTable(); - _ASSERTE(p); + _ASSERTE(p != NULL); _ASSERTE(p->ValidateWithPossibleAV()); #endif // _DEBUG if (Nullable::IsNullableType(pMeth->GetMethodTable())) COMPlusThrow(kNotSupportedException); - DelegateEEClass *pDelCls = (DelegateEEClass*)pDelMT->GetClass(); - MethodDesc *pDelegateInvoke = COMDelegate::FindDelegateInvokeMethod(pDelMT); + DelegateEEClass* pDelCls = (DelegateEEClass*)pDelMT->GetClass(); + MethodDesc* pDelegateInvoke = COMDelegate::FindDelegateInvokeMethod(pDelMT); UINT invokeArgCount = MethodDescToNumFixedArgs(pDelegateInvoke); UINT methodArgCount = MethodDescToNumFixedArgs(pMeth); BOOL isStatic = pMeth->IsStatic(); if (!isStatic) - { methodArgCount++; // count 'this' - } - if (NeedsWrapperDelegate(pMeth)) - gc.refThis = CreateWrapperDelegate(gc.refThis, pMeth); + if (COMDelegate::NeedsWrapperDelegate(pMeth)) + refThis = COMDelegate::CreateWrapperDelegate(refThis, pMeth); if (pMeth->GetLoaderAllocator()->IsCollectible()) - gc.refThis->SetMethodBase(pMeth->GetLoaderAllocator()->GetExposedObject()); + refThis->SetMethodBase(pMeth->GetLoaderAllocator()->GetExposedObject()); // Open delegates. if (invokeArgCount == methodArgCount) { // set the target - gc.refThis->SetTarget(gc.refThis); + refThis->SetTarget(refThis); // set the shuffle thunk - Stub *pShuffleThunk = NULL; - if (!pMeth->IsStatic() && pMeth->HasRetBuffArg() && IsRetBuffPassedAsFirstArg()) - pShuffleThunk = pDelCls->m_pInstRetBuffCallStub; - else - pShuffleThunk = pDelCls->m_pStaticCallStub; - if (!pShuffleThunk) + Stub *pShuffleThunk = (!pMeth->IsStatic() && pMeth->HasRetBuffArg() && IsRetBuffPassedAsFirstArg()) + ? pDelCls->m_pInstRetBuffCallStub + : pDelCls->m_pStaticCallStub; + + if (pShuffleThunk == NULL) pShuffleThunk = SetupShuffleThunk(pDelMT, pMeth); - gc.refThis->SetMethodPtr(pShuffleThunk->GetEntryPoint()); + refThis->SetMethodPtr(pShuffleThunk->GetEntryPoint()); // set the ptr aux according to what is needed, if virtual need to call make virtual stub dispatch if (!pMeth->IsStatic() && pMeth->IsVirtual() && !pMeth->GetMethodTable()->IsValueType()) { PCODE pTargetCall = GetVirtualCallStub(pMeth, TypeHandle(pMeth->GetMethodTable())); - gc.refThis->SetMethodPtrAux(pTargetCall); - gc.refThis->SetInvocationCount((INT_PTR)(void *)pMeth); + refThis->SetMethodPtrAux(pTargetCall); + refThis->SetInvocationCount((INT_PTR)(void *)pMeth); } else { - gc.refThis->SetMethodPtrAux(method); + refThis->SetMethodPtrAux(method); } } else @@ -1682,7 +1661,10 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar if (!pMeth->IsStatic()) { - if (pMTTarg) + if (target.Get() == NULL) + COMPlusThrow(kArgumentException, W("Arg_DlgtNullInst")); + + if (pMTTarg != NULL) { // Use the Unboxing stub for value class methods, since the value // class is constructed using the boxed instance. @@ -1711,24 +1693,21 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar method = pMeth->GetMultiCallableAddrOfCode(); } } - - if (gc.target == NULL) - { - COMPlusThrow(kArgumentException, W("Arg_DlgtNullInst")); - } } #ifdef HAS_THISPTR_RETBUF_PRECODE else if (pMeth->HasRetBuffArg() && IsRetBuffPassedAsFirstArg()) + { method = pMeth->GetLoaderAllocator()->GetFuncPtrStubs()->GetFuncPtrStub(pMeth, PRECODE_THISPTR_RETBUF); + } #endif // HAS_THISPTR_RETBUF_PRECODE - gc.refThis->SetTarget(gc.target); - gc.refThis->SetMethodPtr((PCODE)(void *)method); + refThis->SetTarget(target.Get()); + refThis->SetMethodPtr((PCODE)(void *)method); } - HELPER_METHOD_FRAME_END(); + GCPROTECT_END(); + END_QCALL; } -FCIMPLEND MethodDesc *COMDelegate::GetMethodDesc(OBJECTREF orDelegate) { diff --git a/src/coreclr/vm/comdelegate.h b/src/coreclr/vm/comdelegate.h index 8b1750fc6579d..be68b5ef40dd4 100644 --- a/src/coreclr/vm/comdelegate.h +++ b/src/coreclr/vm/comdelegate.h @@ -29,21 +29,10 @@ BOOL GenerateShuffleArrayPortable(MethodDesc* pMethodSrc, MethodDesc *pMethodDst // This class represents the native methods for the Delegate class class COMDelegate { -private: - // friend VOID CPUSTUBLINKER::EmitShuffleThunk(...); - friend class CPUSTUBLINKER; - - static CrstStatic s_DelegateToFPtrHashCrst; // Lock for the following hash. - static PtrHashMap* s_pDelegateToFPtrHash; // Hash table containing the Delegate->FPtr pairs - // passed out to unmanaged code. public: - static ShuffleThunkCache *m_pShuffleThunkCache; - // One time init. static void Init(); - static FCDECL3(void, DelegateConstruct, Object* refThis, Object* target, PCODE method); - // Get the invoke method for the delegate. Used to transition delegates to multicast delegates. static FCDECL1(PCODE, GetMulticastInvoke, MethodTable* pDelegateMT); static FCDECL1(MethodDesc*, GetInvokeMethod, MethodTable* pDelegateMT); @@ -88,10 +77,6 @@ class COMDelegate // for UnmanagedCallersOnlyAttribute. static void ThrowIfInvalidUnmanagedCallersOnlyUsage(MethodDesc* pMD); -private: - static Stub* SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth); - -public: static MethodDesc* FindDelegateInvokeMethod(MethodTable *pMT); static BOOL IsDelegateInvokeMethod(MethodDesc *pMD); @@ -111,6 +96,8 @@ class COMDelegate BOOL fIsOpenDelegate); }; +extern "C" void QCALLTYPE Delegate_Construct(QCall::ObjectHandleOnStack _this, QCall::ObjectHandleOnStack target, PCODE method); + extern "C" PCODE QCALLTYPE Delegate_GetMulticastInvokeSlow(MethodTable* pDelegateMT); extern "C" PCODE QCALLTYPE Delegate_AdjustTarget(QCall::ObjectHandleOnStack target, PCODE method); diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index 99bbdeb397284..4e67d02a543ef 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -320,22 +320,20 @@ PCODE ECall::GetFCallImpl(MethodDesc * pMD, BOOL * pfSharedOrDynamicFCallImpl /* MethodTable * pMT = pMD->GetMethodTable(); // - // Delegate constructors are FCalls for which the entrypoint points to the target of the delegate - // We have to intercept these and set the call target to the helper COMDelegate::DelegateConstruct + // Delegate constructors are QCalls for which the entrypoint points to the target of the delegate + // We have to intercept these and set the call target to the managed helper Delegate.DelegateConstruct // if (pMT->IsDelegate()) { if (pfSharedOrDynamicFCallImpl) *pfSharedOrDynamicFCallImpl = TRUE; - // COMDelegate::DelegateConstruct is the only fcall used by user delegates. - // All the other gDelegateFuncs are only used by System.Delegate _ASSERTE(pMD->IsCtor()); // We need to set up the ECFunc properly. We don't want to use the pMD passed in, // since it may disappear. Instead, use the stable one on Delegate. Remember - // that this is 1:M between the FCall and the pMDs. - return GetFCallImpl(CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)); + // that this is 1:M between the method and the constructors. + return CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)->GetMultiCallableAddrOfCode(); } // COM imported classes have special constructors @@ -457,9 +455,7 @@ BOOL ECall::IsSharedFCallImpl(PCODE pImpl) PCODE pNativeCode = pImpl; - return - (pNativeCode == GetEEFuncEntryPoint(FCComCtor)) || - (pNativeCode == GetEEFuncEntryPoint(COMDelegate::DelegateConstruct)); + return (pNativeCode == GetEEFuncEntryPoint(FCComCtor)); } BOOL ECall::CheckUnusedECalls(SetSHash& usedIDs) diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 8996b3a16d811..7373c45b0aa29 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -234,11 +234,6 @@ FCFuncEnd() FCFuncStart(gDelegateFuncs) FCFuncElement("GetMulticastInvoke", COMDelegate::GetMulticastInvoke) FCFuncElement("GetInvokeMethod", COMDelegate::GetInvokeMethod) - - // The FCall mechanism knows how to wire multiple different constructor calls into a - // single entrypoint, without the following entry. But we need this entry to satisfy - // frame creation within the body: - FCFuncElement("DelegateConstruct", COMDelegate::DelegateConstruct) FCFuncEnd() FCFuncStart(gMathFuncs) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index f7447a60132f2..6528c0103e291 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -4017,7 +4017,7 @@ PCODE DynamicHelperFixup(TransitionBlock * pTransitionBlock, TADDR * pCell, DWOR } else { - target = ECall::GetFCallImpl(CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)); + target = CoreLibBinder::GetMethod(METHOD__DELEGATE__CONSTRUCT_DELEGATE)->GetMultiCallableAddrOfCode(); ctorData.pArg3 = NULL; } diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 18e080bbe12d2..f9a7753135a49 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -93,6 +93,7 @@ static const Entry s_QCall[] = DllImportEntry(Delegate_InitializeVirtualCallStub) DllImportEntry(Delegate_GetMulticastInvokeSlow) DllImportEntry(Delegate_AdjustTarget) + DllImportEntry(Delegate_Construct) DllImportEntry(Delegate_InternalAlloc) DllImportEntry(Delegate_InternalAllocLike) DllImportEntry(Delegate_FindMethodHandle)