From 4904c6b4285f0f993e29eaad60d29dd921960c0a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 17 Oct 2024 16:24:38 -0700 Subject: [PATCH 1/2] Fix Delegate dispatch. --- src/coreclr/vm/comdelegate.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index b3dcb78a7e03..c19db7ad0a77 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -1879,6 +1879,10 @@ Stub* COMDelegate::GetInvokeMethodStub(EEImplMethodDesc* pMD) if (*pMD->GetSig() != (IMAGE_CEE_CS_CALLCONV_HASTHIS | IMAGE_CEE_CS_CALLCONV_DEFAULT)) COMPlusThrow(kInvalidProgramException); + PCCOR_SIGNATURE pSig; + DWORD cbSig; + pMD->GetSig(&pSig,&cbSig); + MetaSig sig(pMD); BOOL fReturnVal = !sig.IsReturnTypeVoid(); @@ -1901,16 +1905,21 @@ Stub* COMDelegate::GetInvokeMethodStub(EEImplMethodDesc* pMD) for (UINT paramCount = 0; paramCount < sig.NumFixedArgs(); paramCount++) pCode->EmitLDARG(paramCount); - // recursively call the delegate itself +#ifdef FEATURE_INTERPRETER + // Call the underlying method pointer. + pCode->EmitLoadThis(); + pCode->EmitLDFLD(FIELD__DELEGATE__METHOD_PTR); + + mdToken sigTok = pCode->GetSigToken(pSig, cbSig); + pCode->EmitCALLI(sigTok, sig.NumFixedArgs(), fReturnVal); +#else + // Recursively call the delegate itself, will be lower by the JIT. pCode->EmitCALL(pCode->GetToken(pMD), sig.NumFixedArgs(), fReturnVal); +#endif // !FEATURE_INTERPRETER // return pCode->EmitRET(); - PCCOR_SIGNATURE pSig; - DWORD cbSig; - pMD->GetSig(&pSig,&cbSig); - MethodDesc* pStubMD = ILStubCache::CreateAndLinkNewILStubMethodDesc(pMD->GetLoaderAllocator(), pMD->GetMethodTable(), ILSTUB_DELEGATE_INVOKE_METHOD, From 914019e7226f2a89edcf88c1041c647a2e136ff8 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 18 Oct 2024 14:08:20 -0700 Subject: [PATCH 2/2] Update Delegate impl for interpreter. Add GetMethodTable intrinsic. Fix issue with RW/RX confusion with unwinder headers. --- src/coreclr/vm/comdelegate.cpp | 17 +++++++++------- src/coreclr/vm/interpreter.cpp | 37 ++++++++++++++++++++++++++++++++++ src/coreclr/vm/interpreter.h | 2 ++ src/coreclr/vm/stublink.cpp | 4 ++-- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index c19db7ad0a77..0024b00d9691 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -1892,12 +1892,6 @@ Stub* COMDelegate::GetInvokeMethodStub(EEImplMethodDesc* pMD) ILCodeStream *pCode = sl.NewCodeStream(ILStubLinker::kDispatch); - // This stub is only used for rare indirect cases, for example - // when Delegate.Invoke method is wrapped into another delegate. - // Direct invocation of delegate is expanded by JIT. - // Emit a recursive call here to let JIT handle complex cases like - // virtual dispatch and GC safety. - // Load the delegate object pCode->EmitLoadThis(); @@ -1912,8 +1906,17 @@ Stub* COMDelegate::GetInvokeMethodStub(EEImplMethodDesc* pMD) mdToken sigTok = pCode->GetSigToken(pSig, cbSig); pCode->EmitCALLI(sigTok, sig.NumFixedArgs(), fReturnVal); + + pCode->EmitLoadThis(); + pCode->EmitCALL(METHOD__GC__KEEP_ALIVE, 1, 0); #else - // Recursively call the delegate itself, will be lower by the JIT. + // This stub is only used for rare indirect cases, for example + // when Delegate.Invoke method is wrapped into another delegate. + // Direct invocation of delegate is expanded by JIT. + // Emit a recursive call here to let JIT handle complex cases like + // virtual dispatch and GC safety. + + // Recursively call the delegate itself. pCode->EmitCALL(pCode->GetToken(pMD), sig.NumFixedArgs(), fReturnVal); #endif // !FEATURE_INTERPRETER diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index 0bd673efab44..35f01f8af787 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9289,6 +9289,11 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T didIntrinsic = true; break; + case NI_System_Runtime_CompilerServices_RuntimeHelpers_GetMethodTable: + DoGetMethodTable(); + didIntrinsic = true; + break; + case NI_System_Threading_Interlocked_CompareExchange: // Here and in other Interlocked.* intrinsics we use sigInfo.retType to be able // to detect small-integer overloads. @@ -10986,6 +10991,34 @@ void Interpreter::DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method m_curStackHt++; } +void Interpreter::DoGetMethodTable() +{ + CONTRACTL{ + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } CONTRACTL_END; + + _ASSERTE(m_curStackHt > 0); + unsigned ind = m_curStackHt - 1; + +#ifdef _DEBUG + _ASSERTE(OpStackTypeGet(ind).ToCorInfoType() == CORINFO_TYPE_CLASS); +#endif // _DEBUG + + Object* obj = OpStackGet(ind); + + if (obj == NULL) + { + ThrowNullPointerException(); + } + + MethodTable* pMT = obj->RawGetMethodTable(); + + OpStackSet(m_curStackHt, pMT); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_NATIVEINT)); +} + bool Interpreter::DoInterlockedCompareExchange(CorInfoType retType) { CONTRACTL{ @@ -11992,6 +12025,10 @@ Interpreter::InterpreterNamedIntrinsics Interpreter::getNamedIntrinsicID(CEEInfo { result = NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences; } + else if (strcmp(methodName, "GetMethodTable") == 0) + { + result = NI_System_Runtime_CompilerServices_RuntimeHelpers_GetMethodTable; + } } } } diff --git a/src/coreclr/vm/interpreter.h b/src/coreclr/vm/interpreter.h index 023f15e0e57f..db895147de8b 100644 --- a/src/coreclr/vm/interpreter.h +++ b/src/coreclr/vm/interpreter.h @@ -923,6 +923,7 @@ class Interpreter NI_System_StubHelpers_GetStubContext, NI_System_Runtime_InteropService_MemoryMarshal_GetArrayDataReference, NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences, + NI_System_Runtime_CompilerServices_RuntimeHelpers_GetMethodTable, NI_System_Threading_Interlocked_CompareExchange, NI_System_Threading_Interlocked_Exchange, NI_System_Threading_Interlocked_ExchangeAdd, @@ -1797,6 +1798,7 @@ class Interpreter void DoGetIsSupported(); void DoGetArrayDataReference(); void DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method); + void DoGetMethodTable(); bool DoInterlockedCompareExchange(CorInfoType retType); bool DoInterlockedExchange(CorInfoType retType); bool DoInterlockedExchangeAdd(CorInfoType retType); diff --git a/src/coreclr/vm/stublink.cpp b/src/coreclr/vm/stublink.cpp index b1f36d7d5d2b..7df6cd2d1b95 100644 --- a/src/coreclr/vm/stublink.cpp +++ b/src/coreclr/vm/stublink.cpp @@ -1811,7 +1811,7 @@ bool StubLinker::EmitUnwindInfo(Stub* pStubRX, Stub* pStubRW, int globalsize, Lo // pHeaderRW->pNext = pStubHeapSegment->pUnwindHeaderList; - pStubHeapSegment->pUnwindHeaderList = pHeaderRW; + pStubHeapSegment->pUnwindHeaderList = pHeaderRX; #ifdef TARGET_AMD64 // Publish Unwind info to ETW stack crawler @@ -1826,7 +1826,7 @@ bool StubLinker::EmitUnwindInfo(Stub* pStubRX, Stub* pStubRW, int globalsize, Lo #ifdef _DEBUG _ASSERTE(pHeaderRW->IsRegistered()); - _ASSERTE( &pHeaderRW->FunctionEntry + _ASSERTE( &pHeaderRX->FunctionEntry == FindStubFunctionEntry((ULONG64)pCode, EncodeDynamicFunctionTableContext(pStubHeapSegment, DYNFNTABLE_STUB))); #endif