Skip to content

Commit

Permalink
Remove HelperMethodFrame from Delegate construction (#108217)
Browse files Browse the repository at this point in the history 
* Remove HelperMethodFrame from Delegate construction
  • Loading branch information
@AaronRobinsonMSFT
AaronRobinsonMSFT authored Sep 27, 2024
1 parent 9b56e73 commit c20bdf6
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 114 deletions.
17 changes: 15 additions & 2 deletions src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/daccess/dacdbiimpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
141 changes: 60 additions & 81 deletions src/coreclr/vm/comdelegate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,12 +726,10 @@ VOID GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, SArray<S
#endif
}


ShuffleThunkCache *COMDelegate::m_pShuffleThunkCache = NULL;

CrstStatic COMDelegate::s_DelegateToFPtrHashCrst;
PtrHashMap* COMDelegate::s_pDelegateToFPtrHash = NULL;

static ShuffleThunkCache* s_pShuffleThunkCache = NULL;
static CrstStatic s_DelegateToFPtrHashCrst; // Lock for the following hash.
static PtrHashMap* s_pDelegateToFPtrHash = NULL; // Hash table containing the Delegate->FPtr pairs
// passed out to unmanaged code.

// One time init.
void COMDelegate::Init()
Expand All @@ -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
Expand Down Expand Up @@ -792,7 +790,7 @@ LoaderHeap *DelegateEEClass::GetStubHeap()
}


Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth)
static Stub* SetupShuffleThunk(MethodTable* pDelMT, MethodDesc* pTargetMeth)
{
CONTRACTL
{
Expand All @@ -812,7 +810,7 @@ Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMe
StackSArray<ShuffleEntry> rShuffleEntryArray;
GenerateShuffleArray(pMD, pTargetMeth, &rShuffleEntryArray);

ShuffleThunkCache* pShuffleThunkCache = m_pShuffleThunkCache;
ShuffleThunkCache* pShuffleThunkCache = s_pShuffleThunkCache;

LoaderAllocator* pLoaderAllocator = pDelMT->GetLoaderAllocator();
if (pLoaderAllocator->IsCollectible())
Expand Down Expand Up @@ -848,8 +846,6 @@ Stub* COMDelegate::SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMe
return pShuffleThunk;
}



static PCODE GetVirtualCallStub(MethodDesc *method, TypeHandle scopeType)
{
CONTRACTL
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
{
Expand Down
17 changes: 2 additions & 15 deletions src/coreclr/vm/comdelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
14 changes: 5 additions & 9 deletions src/coreclr/vm/ecall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<DWORD>& usedIDs)
Expand Down
Loading

0 comments on commit c20bdf6

Please sign in to comment.