Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup delegate-related code #107966

Merged
merged 5 commits into from
Sep 18, 2024
Merged

Cleanup delegate-related code #107966

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e07fa49
Cleanup delegate-related code
jkotas Sep 18, 2024
d42fb6f
Update src/coreclr/vm/stubmgr.cpp
jkotas Sep 18, 2024
eec905a
Update src/coreclr/vm/stubmgr.cpp
jkotas Sep 18, 2024
456df87
FB
jkotas Sep 18, 2024
04c4666
Build break
jkotas Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/coreclr/vm/class.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,6 @@ void EEClass::Destruct(MethodTable * pOwningMT)
ExecutableWriterHolder<Stub> stubWriterHolder(pDelegateEEClass->m_pInstRetBuffCallStub, sizeof(Stub));
stubWriterHolder.GetRW()->DecRef();
}
// While m_pMultiCastInvokeStub is also a member,
// it is owned by the m_pMulticastStubCache, not by the class
// - it is shared across classes. So we don't decrement
// its ref count here
}

#ifdef FEATURE_COMINTEROP
Expand Down
16 changes: 2 additions & 14 deletions src/coreclr/vm/class.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,11 +1895,9 @@ class DelegateEEClass : public EEClass
PTR_Stub m_pStaticCallStub;
PTR_Stub m_pInstRetBuffCallStub;
PTR_MethodDesc m_pInvokeMethod;
PTR_Stub m_pMultiCastInvokeStub;
PTR_Stub m_pWrapperDelegateInvokeStub;
PCODE m_pMultiCastInvokeStub;
PCODE m_pWrapperDelegateInvokeStub;
UMThunkMarshInfo* m_pUMThunkMarshInfo;
PTR_MethodDesc m_pBeginInvokeMethod;
jkotas marked this conversation as resolved.
Show resolved Hide resolved
PTR_MethodDesc m_pEndInvokeMethod;
Volatile<PCODE> m_pMarshalStub;

#ifdef FEATURE_COMINTEROP
Expand All @@ -1911,16 +1909,6 @@ class DelegateEEClass : public EEClass
return m_pInvokeMethod;
}

PTR_MethodDesc GetBeginInvokeMethod()
{
return m_pBeginInvokeMethod;
}

PTR_MethodDesc GetEndInvokeMethod()
{
return m_pEndInvokeMethod;
}

#ifndef DACCESS_COMPILE
DelegateEEClass() : EEClass()
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/clrtocomcall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,13 @@ static CallsiteDetails CreateCallsiteDetails(_In_ FramedMethodFrame *pFrame)
DelegateEEClass* delegateCls = (DelegateEEClass*)pMD->GetMethodTable()->GetClass();
_ASSERTE(pFrame->GetThis()->GetMethodTable()->IsDelegate());

if (pMD == delegateCls->m_pBeginInvokeMethod)
if (strcmp(pMD->GetName(), "BeginInvoke") == 0)
{
callsiteFlags |= CallsiteDetails::BeginInvoke;
}
else
{
_ASSERTE(pMD == delegateCls->m_pEndInvokeMethod);
_ASSERTE(strcmp(pMD->GetName(), "EndInvoke") == 0);
callsiteFlags |= CallsiteDetails::EndInvoke;
}

Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/vm/codeman.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ enum StubCodeBlockKind : int
STUB_CODE_BLOCK_NOCODE,
STUB_CODE_BLOCK_MANAGED,
STUB_CODE_BLOCK_STUBLINK,
// Placeholdes used by NGen images
STUB_CODE_BLOCK_VIRTUAL_METHOD_THUNK,
jkotas marked this conversation as resolved.
Show resolved Hide resolved
STUB_CODE_BLOCK_EXTERNAL_METHOD_THUNK,
// Placeholdes used by ReadyToRun images
STUB_CODE_BLOCK_METHOD_CALL_THUNK,
};
Expand Down
41 changes: 13 additions & 28 deletions src/coreclr/vm/comdelegate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1945,19 +1945,8 @@ PCODE COMDelegate::GetInvokeMethodStub(EEImplMethodDesc* pMD)
}
else
{

// Since we do not support asynchronous delegates in CoreCLR, we much ensure that it was indeed a async delegate call
// and not an invalid-delegate-layout condition.
//
// If the call was indeed for async delegate invocation, we will just throw an exception.
if ((pMD == pClass->GetBeginInvokeMethod()) || (pMD == pClass->GetEndInvokeMethod()))
{
COMPlusThrow(kPlatformNotSupportedException);
}


_ASSERTE(!"Bad Delegate layout");
COMPlusThrow(kInvalidProgramException);
// We do not support asynchronous delegates in CoreCLR
COMPlusThrow(kPlatformNotSupportedException);
}

RETURN ret;
Expand Down Expand Up @@ -2137,8 +2126,7 @@ FCIMPL1(PCODE, COMDelegate::GetMulticastInvoke, MethodTable* pDelegateMT)
_ASSERTE(pDelegateMT != NULL);

DelegateEEClass* delegateEEClass = (DelegateEEClass*)pDelegateMT->GetClass();
Stub *pStub = delegateEEClass->m_pMultiCastInvokeStub;
return (pStub != NULL) ? pStub->GetEntryPoint() : (PCODE)NULL;
return delegateEEClass->m_pMultiCastInvokeStub;
}
FCIMPLEND

Expand All @@ -2147,12 +2135,12 @@ extern "C" PCODE QCALLTYPE Delegate_GetMulticastInvokeSlow(MethodTable* pDelegat
QCALL_CONTRACT;
_ASSERTE(pDelegateMT != NULL);

PCODE fptr = (PCODE)NULL;
PCODE pStub = (PCODE)NULL;

BEGIN_QCALL;

DelegateEEClass *delegateEEClass = (DelegateEEClass*)pDelegateMT->GetClass();
Stub *pStub = delegateEEClass->m_pMultiCastInvokeStub;
PCODE pStub = delegateEEClass->m_pMultiCastInvokeStub;
jkotas marked this conversation as resolved.
Show resolved Hide resolved
if (pStub == NULL)
{
MethodDesc* pMD = delegateEEClass->GetInvokeMethod();
Expand Down Expand Up @@ -2263,17 +2251,15 @@ extern "C" PCODE QCALLTYPE Delegate_GetMulticastInvokeSlow(MethodTable* pDelegat
pSig, cbSig,
NULL,
&sl);
pStub = Stub::NewStub(JitILStub(pStubMD));
pStub = JitILStub(pStubMD);
jkotas marked this conversation as resolved.
Show resolved Hide resolved

InterlockedCompareExchangeT<PTR_Stub>(&delegateEEClass->m_pMultiCastInvokeStub, pStub, NULL);
InterlockedCompareExchangeT<PCODE>(&delegateEEClass->m_pMultiCastInvokeStub, pStub, NULL);
pStub = delegateEEClass->m_pMultiCastInvokeStub;
}

fptr = pStub->GetEntryPoint();

END_QCALL;

return fptr;
return pStub;
}

PCODE COMDelegate::GetWrapperInvoke(MethodDesc* pMD)
Expand All @@ -2288,11 +2274,10 @@ PCODE COMDelegate::GetWrapperInvoke(MethodDesc* pMD)

MethodTable * pDelegateMT = pMD->GetMethodTable();
DelegateEEClass* delegateEEClass = (DelegateEEClass*) pDelegateMT->GetClass();
Stub *pStub = delegateEEClass->m_pWrapperDelegateInvokeStub;
PCODE pStub = delegateEEClass->m_pWrapperDelegateInvokeStub;

if (pStub == NULL)
{

GCX_PREEMP();

MetaSig sig(pMD);
Expand Down Expand Up @@ -2333,12 +2318,12 @@ PCODE COMDelegate::GetWrapperInvoke(MethodDesc* pMD)
NULL,
&sl);

pStub = Stub::NewStub(JitILStub(pStubMD));

InterlockedCompareExchangeT<PTR_Stub>(&delegateEEClass->m_pWrapperDelegateInvokeStub, pStub, NULL);
pStub = JitILStub(pStubMD);

InterlockedCompareExchangeT<PCODE>(&delegateEEClass->m_pWrapperDelegateInvokeStub, pStub, NULL);
pStub = delegateEEClass->m_pWrapperDelegateInvokeStub;
}
return pStub->GetEntryPoint();
return pStub;
}


Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/comdelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class ShuffleThunkCache : public StubCacheBase
STANDARD_VM_CONTRACT;

((CPUSTUBLINKER*)pstublinker)->EmitShuffleThunk((ShuffleEntry*)pRawStub);
return NEWSTUB_FL_THUNK;
return NEWSTUB_FL_SHUFFLE_THUNK;
}

//---------------------------------------------------------
Expand Down
10 changes: 2 additions & 8 deletions src/coreclr/vm/methodtablebuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6138,15 +6138,9 @@ MethodTableBuilder::InitMethodDesc(
BAD_FORMAT_NOTHROW_ASSERT(((DelegateEEClass*)GetHalfBakedClass())->m_pInvokeMethod == NULL);
((DelegateEEClass*)GetHalfBakedClass())->m_pInvokeMethod = pNewMD;
}
else if (strcmp(pMethodName, "BeginInvoke") == 0)
else if (strcmp(pMethodName, "BeginInvoke") == 0 || strcmp(pMethodName, "EndInvoke") == 0)
{
BAD_FORMAT_NOTHROW_ASSERT(((DelegateEEClass*)GetHalfBakedClass())->m_pBeginInvokeMethod == NULL);
((DelegateEEClass*)GetHalfBakedClass())->m_pBeginInvokeMethod = pNewMD;
}
else if (strcmp(pMethodName, "EndInvoke") == 0)
{
BAD_FORMAT_NOTHROW_ASSERT(((DelegateEEClass*)GetHalfBakedClass())->m_pEndInvokeMethod == NULL);
((DelegateEEClass*)GetHalfBakedClass())->m_pEndInvokeMethod = pNewMD;
// Obsolete async methods
}
else
{
Expand Down
43 changes: 3 additions & 40 deletions src/coreclr/vm/stublink.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ StubLinker::StubLinker()
m_pCodeElements = NULL;
m_pFirstCodeLabel = NULL;
m_pFirstLabelRef = NULL;
m_pPatchLabel = NULL;
jkotas marked this conversation as resolved.
Show resolved Hide resolved
m_pTargetMethod = NULL;
m_stackSize = 0;
m_fDataOnly = FALSE;
Expand Down Expand Up @@ -622,26 +621,6 @@ CodeLabel* StubLinker::EmitNewCodeLabel()
}


//---------------------------------------------------------------
// Creates & emits the patch offset label for the stub
//---------------------------------------------------------------
VOID StubLinker::EmitPatchLabel()
{
CONTRACTL
{
THROWS;
GC_NOTRIGGER;
}
CONTRACTL_END;

//
// Note that it's OK to have re-emit the patch label,
// just use the later one.
//

m_pPatchLabel = EmitNewCodeLabel();
}

//---------------------------------------------------------------
// Returns final location of label as an offset from the start
// of the stub. Can only be called after linkage.
Expand Down Expand Up @@ -1099,31 +1078,15 @@ bool StubLinker::EmitStub(Stub* pStub, int globalsize, int totalSize, LoaderHeap
ZeroMemory(pCodeRW + lastCodeOffset, globalsize - lastCodeOffset);
}

// Set additional stub data.
// - Fill in the target method for the Instantiating stub.
//
// - Fill in patch offset, if we have one
// Note that these offsets are relative to the start of the stub,
// not the code, so you'll have to add sizeof(Stub) to get to the
// right spot.
// Fill in the target method for the Instantiating stub.
if (pStubRW->IsInstantiatingStub())
{
_ASSERTE(m_pTargetMethod != NULL);
_ASSERTE(m_pPatchLabel == NULL);
pStubRW->SetInstantiatedMethodDesc(m_pTargetMethod);

LOG((LF_CORDB, LL_INFO100, "SL::ES: InstantiatedMethod fd:0x%x\n",
pStub->GetInstantiatedMethodDesc()));
}
else if (m_pPatchLabel != NULL)
{
UINT32 uLabelOffset = GetLabelOffset(m_pPatchLabel);
_ASSERTE(FitsIn<USHORT>(uLabelOffset));
pStubRW->SetPatchOffset(static_cast<USHORT>(uLabelOffset));

LOG((LF_CORDB, LL_INFO100, "SL::ES: patch offset:0x%x\n",
pStub->GetPatchOffset()));
}

#ifdef STUBLINKER_GENERATES_UNWIND_INFO
if (pStub->HasUnwindInfo())
Expand Down Expand Up @@ -2260,8 +2223,8 @@ void Stub::SetupStub(int numCodeBytes, DWORD flags
m_numCodeBytesAndFlags |= EXTERNAL_ENTRY_BIT;
if ((flags & NEWSTUB_FL_INSTANTIATING_METHOD) != 0)
m_numCodeBytesAndFlags |= INSTANTIATING_STUB_BIT;
if ((flags & NEWSTUB_FL_THUNK) != 0)
m_numCodeBytesAndFlags |= THUNK_BIT;
if ((flags & NEWSTUB_FL_SHUFFLE_THUNK) != 0)
m_numCodeBytesAndFlags |= SHUFFLE_THUNK_BIT;
}

#ifdef STUBLINKER_GENERATES_UNWIND_INFO
Expand Down
58 changes: 6 additions & 52 deletions src/coreclr/vm/stublink.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,6 @@ class StubLinker
//---------------------------------------------------------------
VOID EmitLabel(CodeLabel* pCodeLabel);

//---------------------------------------------------------------
// Emits the patch label for the stub.
// Throws exception on failure.
//---------------------------------------------------------------
void EmitPatchLabel();

//---------------------------------------------------------------
// Create a new label to an external address.
// Throws exception on failure.
Expand Down Expand Up @@ -280,9 +274,6 @@ class StubLinker
CodeElement *m_pCodeElements; // stored in *reverse* order
CodeLabel *m_pFirstCodeLabel; // linked list of CodeLabels
LabelRef *m_pFirstLabelRef; // linked list of references
CodeLabel *m_pPatchLabel; // label of stub patch offset
// currently just for multicast
// frames.
PTR_MethodDesc m_pTargetMethod; // Used for instantiating stubs.
SHORT m_stackSize; // count of pushes/pops
CQuickHeap m_quickHeap; // throwaway heap for
Expand Down Expand Up @@ -452,7 +443,7 @@ enum NewStubFlags
NEWSTUB_FL_MULTICAST = 0x00000002,
NEWSTUB_FL_EXTERNAL = 0x00000004,
NEWSTUB_FL_LOADERHEAP = 0x00000008,
NEWSTUB_FL_THUNK = 0x00000010
NEWSTUB_FL_SHUFFLE_THUNK = 0x00000010
};


Expand All @@ -477,12 +468,12 @@ class Stub
LOADER_HEAP_BIT = 0x20000000,
INSTANTIATING_STUB_BIT = 0x10000000,
UNWIND_INFO_BIT = 0x08000000,
THUNK_BIT = 0x04000000,
SHUFFLE_THUNK_BIT = 0x04000000,
jkotas marked this conversation as resolved.
Show resolved Hide resolved

CODEBYTES_MASK = THUNK_BIT - 1,
CODEBYTES_MASK = SHUFFLE_THUNK_BIT - 1,
MAX_CODEBYTES = CODEBYTES_MASK + 1,
};
static_assert_no_msg(CODEBYTES_MASK < THUNK_BIT);
static_assert_no_msg(CODEBYTES_MASK < SHUFFLE_THUNK_BIT);

public:
//-------------------------------------------------------------------
Expand Down Expand Up @@ -530,46 +521,10 @@ class Stub
//-------------------------------------------------------------------
// Used by the debugger to help step through stubs
//-------------------------------------------------------------------
BOOL IsManagedThunk()
{
LIMITED_METHOD_CONTRACT;
return (m_numCodeBytesAndFlags & THUNK_BIT) != 0;
}

//-------------------------------------------------------------------
// For stubs which execute user code, a patch offset needs to be set
// to tell the debugger how far into the stub code the debugger has
// to step until the frame is set up.
//-------------------------------------------------------------------
void SetPatchOffset(USHORT offset)
{
LIMITED_METHOD_CONTRACT;
_ASSERTE(!IsInstantiatingStub());
m_data.PatchOffset = offset;
}

//-------------------------------------------------------------------
// For stubs which execute user code, a patch offset needs to be set
// to tell the debugger how far into the stub code the debugger has
// to step until the frame is set up.
//-------------------------------------------------------------------
USHORT GetPatchOffset()
{
LIMITED_METHOD_CONTRACT;
_ASSERTE(!IsInstantiatingStub());
return m_data.PatchOffset;
}

//-------------------------------------------------------------------
// For stubs which execute user code, a patch offset needs to be set
// to tell the debugger how far into the stub code the debugger has
// to step until the frame is set up.
//-------------------------------------------------------------------
TADDR GetPatchAddress()
BOOL IsShuffleThunk()
{
LIMITED_METHOD_CONTRACT;
_ASSERTE(!IsInstantiatingStub());
return dac_cast<TADDR>(GetEntryPointInternal()) + GetPatchOffset();
return (m_numCodeBytesAndFlags & SHUFFLE_THUNK_BIT) != 0;
}

//-------------------------------------------------------------------
Expand Down Expand Up @@ -834,7 +789,6 @@ class Stub
UINT32 m_numCodeBytesAndFlags;
union
{
USHORT PatchOffset;
PTR_MethodDesc InstantiatedMethod;
} m_data;
jkotas marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
Loading
Loading