From 6207af40b80444a0773f777f301d52c69e2c7b0a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 29 Aug 2025 22:46:46 -0700 Subject: [PATCH 01/25] Initial P/Invokes on WASM Separate call invokers between JIT compatible interpreter and interpreter with Portable EntryPoints. --- src/coreclr/vm/callstubgenerator.cpp | 119 ++++++++++++ src/coreclr/vm/interpexec.cpp | 190 ++++++-------------- src/coreclr/vm/jitinterface.cpp | 32 ++-- src/coreclr/vm/precode_portable.cpp | 11 ++ src/coreclr/vm/precode_portable.hpp | 12 ++ src/coreclr/vm/prestub.cpp | 12 +- src/coreclr/vm/wasm/calldescrworkerwasm.cpp | 2 +- src/coreclr/vm/wasm/cgencpu.h | 4 +- src/coreclr/vm/wasm/helpers.cpp | 60 ++++++- 9 files changed, 278 insertions(+), 164 deletions(-) diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index a8342e19c09496..746425d004b775 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -1902,4 +1902,123 @@ CallStubGenerator::ReturnType CallStubGenerator::GetReturnType(ArgIterator *pArg return ReturnTypeVoid; } +static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMD)); + } + CONTRACTL_END + + GCX_PREEMP(); + + CallStubGenerator callStubGenerator; + + AllocMemTracker amTracker; + CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); + + if (pMD->SetCallStub(header)) + { + amTracker.SuppressRelease(); + } + else + { + // We have lost the race for generating the header, use the one that was generated by another thread + // and let the amTracker release the memory of the one we generated. + header = pMD->GetCallStub(); + } + + return header; +} + +void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMD)); + PRECONDITION(CheckPointer(pArgs)); + PRECONDITION(CheckPointer(pRet)); + } + CONTRACTL_END + + CallStubHeader *pHeader = pMD->GetCallStub(); + if (pHeader == NULL) + { + pHeader = UpdateCallStubForMethod(pMD); + } + + // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. + pHeader->SetTarget(target); // The method to call + + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + +void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget) +{ + InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); +} + +void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMDDelegateInvoke)); + PRECONDITION(CheckPointer(pArgs)); + PRECONDITION(CheckPointer(pRet)); + } + CONTRACTL_END + + CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); + if (stubHeaderTemplate == NULL) + { + stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke); + } + + // CallStubHeaders encode their destination addresses in the Routines array, so they need to be + // copied to a local buffer before we can actually set their target address. + size_t templateSize = stubHeaderTemplate->GetSize(); + uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); + memcpy(actualCallStub, stubHeaderTemplate, templateSize); + CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; + pHeader->SetTarget(target); // The method to call + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + +void InvokeCalliStub(PCODE ftn, void *cookie, int8_t *pArgs, int8_t *pRet) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer((void*)ftn)); + PRECONDITION(CheckPointer(cookie)); + } + CONTRACTL_END + + // CallStubHeaders encode their destination addresses in the Routines array, so they need to be + // copied to a local buffer before we can actually set their target address. + CallStubHeader* stubHeaderTemplate = (CallStubHeader*)cookie; + size_t templateSize = stubHeaderTemplate->GetSize(); + uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); + memcpy(actualCallStub, stubHeaderTemplate, templateSize); + CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; + pHeader->SetTarget(ftn); // The method to call + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + +LPVOID GetCookieForCalliSig(MetaSig* pMetaSig) +{ + STANDARD_VM_CONTRACT; + _ASSERTE(CheckPointer(pMetaSig)); + + CallStubGenerator callStubGenerator; + return callStubGenerator.GenerateCallStubForSig(*pMetaSig); +} + #endif // FEATURE_INTERPRETER && !TARGET_WASM diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index bc504bab76272e..bb3c2b7e9b1cab 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -11,116 +11,34 @@ // for numeric_limits #include -#ifdef TARGET_WASM -void InvokeCalliStub(PCODE ftn, CallStubHeader *stubHeaderTemplate, int8_t *pArgs, int8_t *pRet); -void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target); +// Call invoker helpers provided by platform. +void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target); +void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget); +void InvokeCalliStub(PCODE ftn, void* stubHeaderTemplate, int8_t *pArgs, int8_t *pRet); void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target); -#else -#include "callstubgenerator.h" - -CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) -{ - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer(pMD)); - } - CONTRACTL_END - - GCX_PREEMP(); - - CallStubGenerator callStubGenerator; - AllocMemTracker amTracker; - CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); - - if (pMD->SetCallStub(header)) - { - amTracker.SuppressRelease(); - } - else - { - // We have lost the race for generating the header, use the one that was generated by another thread - // and let the amTracker release the memory of the one we generated. - header = pMD->GetCallStub(); - } - - return header; -} - -void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) -{ - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer(pMD)); - PRECONDITION(CheckPointer(pArgs)); - PRECONDITION(CheckPointer(pRet)); - } - CONTRACTL_END - - CallStubHeader *pHeader = pMD->GetCallStub(); - if (pHeader == NULL) - { - pHeader = UpdateCallStubForMethod(pMD); - } - - // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. - pHeader->SetTarget(target); // The method to call - - pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); -} - -void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) +// Use the NOINLINE to ensure that the InlinedCallFrame in this method is a lower stack address than any InterpMethodContextFrame values. +NOINLINE +void InvokeUnmanagedMethodWithTransition(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget) { - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer(pMDDelegateInvoke)); - PRECONDITION(CheckPointer(pArgs)); - PRECONDITION(CheckPointer(pRet)); - } - CONTRACTL_END + InlinedCallFrame inlinedCallFrame; + inlinedCallFrame.m_pCallerReturnAddress = (TADDR)pFrame->ip; + inlinedCallFrame.m_pCallSiteSP = pFrame; + inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; + inlinedCallFrame.m_pThread = GetThread(); + inlinedCallFrame.m_Datum = NULL; + inlinedCallFrame.Push(); - CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); - if (stubHeaderTemplate == NULL) { - stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke); + GCX_PREEMP(); + InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); } - // CallStubHeaders encode their destination addresses in the Routines array, so they need to be - // copied to a local buffer before we can actually set their target address. - size_t templateSize = stubHeaderTemplate->GetSize(); - uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); - memcpy(actualCallStub, stubHeaderTemplate, templateSize); - CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; - pHeader->SetTarget(target); // The method to call - pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); + inlinedCallFrame.Pop(); } -void InvokeCalliStub(PCODE ftn, CallStubHeader *stubHeaderTemplate, int8_t *pArgs, int8_t *pRet) -{ - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer((void*)ftn)); - PRECONDITION(CheckPointer(stubHeaderTemplate)); - } - CONTRACTL_END - - // CallStubHeaders encode their destination addresses in the Routines array, so they need to be - // copied to a local buffer before we can actually set their target address. - size_t templateSize = stubHeaderTemplate->GetSize(); - uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); - memcpy(actualCallStub, stubHeaderTemplate, templateSize); - CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; - pHeader->SetTarget(ftn); // The method to call - pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); -} +#ifndef TARGET_WASM +#include "callstubgenerator.h" // Create call stub for calling interpreted methods from JITted/AOTed code. CallStubHeader *CreateNativeToInterpreterCallStub(InterpMethod* pInterpMethod) @@ -155,35 +73,14 @@ CallStubHeader *CreateNativeToInterpreterCallStub(InterpMethod* pInterpMethod) return pHeader; } - #endif // !TARGET_WASM -// Use the NOINLINE to ensure that the InlinedCallFrame in this method is a lower stack address than any InterpMethodContextFrame values. -NOINLINE void InvokePInvokeMethod(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget) -{ - InlinedCallFrame inlinedCallFrame; - inlinedCallFrame.m_pCallerReturnAddress = (TADDR)pFrame->ip; - inlinedCallFrame.m_pCallSiteSP = pFrame; - inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; - inlinedCallFrame.m_pThread = GetThread(); - inlinedCallFrame.m_Datum = NULL; - inlinedCallFrame.Push(); - - { - GCX_PREEMP(); - InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); - } - - inlinedCallFrame.Pop(); -} - typedef void* (*HELPER_FTN_P_P)(void*); typedef void* (*HELPER_FTN_BOX_UNBOX)(MethodTable*, void*); typedef Object* (*HELPER_FTN_NEWARR)(MethodTable*, intptr_t); typedef void* (*HELPER_FTN_P_PP)(void*, void*); typedef void (*HELPER_FTN_V_PPP)(void*, void*, void*); - InterpThreadContext::InterpThreadContext() { // FIXME VirtualAlloc/mmap with INTERP_STACK_ALIGNMENT alignment @@ -428,8 +325,8 @@ void* DoGenericLookup(void* genericVarAsPtr, InterpGenericLookup* pLookup) // TODO! If this becomes a performance bottleneck, we could expand out the various permutations of this // so that we have 24 versions of lookup (or 48 is we allow for avoiding the null check), do the only // if check to figure out which one to use, and then have the rest of the logic be straight-line code. - MethodTable *pMT = nullptr; - MethodDesc* pMD = nullptr; + MethodTable *pMT = NULL; + MethodDesc* pMD = NULL; uint8_t* lookup; if (pLookup->lookupType == InterpGenericLookupType::This) { @@ -500,20 +397,22 @@ LONG IgnoreCppExceptionFilter(PEXCEPTION_POINTERS pExceptionInfo, PVOID pv) : EXCEPTION_EXECUTE_HANDLER; } -// Wrapper around MethodDesc::PrepareInitialCode to handle possible managed exceptions thrown by it. -void PrepareInitialCode(MethodDesc *pMD) +// Wrapper around MethodDesc::DoPrestub to handle possible managed exceptions thrown by it. +static void CallPreStub(MethodDesc *pMD, CallerGCMode callerGCMode = CallerGCMode::Preemptive) { STATIC_STANDARD_VM_CONTRACT; + _ASSERTE(pMD != NULL); struct Param { MethodDesc *pMethodDesc; + CallerGCMode callerGCMode; } - param = { pMD }; + param = { pMD, callerGCMode }; PAL_TRY(Param *, pParam, ¶m) { - pParam->pMethodDesc->PrepareInitialCode(CallerGCMode::Coop); + (void)pParam->pMethodDesc->DoPrestub(NULL /* MethodTable */, pParam->callerGCMode); } PAL_EXCEPT_FILTER(IgnoreCppExceptionFilter) { @@ -1878,8 +1777,22 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr InterpGenericLookup *pLookup = (InterpGenericLookup*)&pMethod->pDataItems[ip[4]]; void* helperArg = DoGenericLookup(LOCAL_VAR(ip[2], void*), pLookup); - HELPER_FTN_P_P helperFtn = GetPossiblyIndirectHelper(pMethod, ip[3]); + MethodDesc *pILTargetMethod = NULL; + HELPER_FTN_P_P helperFtn = GetPossiblyIndirectHelper(pMethod, ip[3], &pILTargetMethod); + if (pILTargetMethod != NULL) + { + returnOffset = ip[1]; + int stackOffset = pMethod->allocaSize; + callArgsOffset = stackOffset; + + LOCAL_VAR(stackOffset, void*) = helperArg; + targetMethod = pILTargetMethod; + ip += 5; + goto CALL_INTERP_METHOD; + } + + _ASSERTE(helperFtn != NULL); LOCAL_VAR(ip[1], void*) = helperFtn(helperArg); ip += 5; break; @@ -1967,14 +1880,14 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr int32_t calliFunctionPointerVar = ip[3]; int32_t calliCookie = ip[4]; - CallStubHeader *pCallStub = (CallStubHeader*)pMethod->pDataItems[calliCookie]; + void* cookie = pMethod->pDataItems[calliCookie]; ip += 5; // Save current execution state for when we return from called method pFrame->ip = ip; // Interpreter-FIXME: isTailcall - InvokeCalliStub(LOCAL_VAR(calliFunctionPointerVar, PCODE), pCallStub, stack + callArgsOffset, stack + returnOffset); + InvokeCalliStub(LOCAL_VAR(calliFunctionPointerVar, PCODE), cookie, stack + callArgsOffset, stack + returnOffset); break; } @@ -2001,11 +1914,11 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr if (flags & (int32_t)PInvokeCallFlags::SuppressGCTransition) { - InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); + InvokeUnmanagedMethod(targetMethod, stack, pFrame, callArgsOffset, returnOffset, callTarget); } else { - InvokePInvokeMethod(targetMethod, stack, pFrame, callArgsOffset, returnOffset, callTarget); + InvokeUnmanagedMethodWithTransition(targetMethod, stack, pFrame, callArgsOffset, returnOffset, callTarget); } break; @@ -2067,18 +1980,15 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // small subset of frames high. pInterpreterFrame->SetTopInterpMethodContextFrame(pFrame); GCX_PREEMP(); - // Attempt to setup the interpreter code for the target method. - if ((targetMethod->IsIL() || targetMethod->IsNoMetadata()) && !targetMethod->IsUnboxingStub()) - { - PrepareInitialCode(targetMethod); - } + CallPreStub(targetMethod); + targetIp = targetMethod->GetInterpreterCode(); } if (targetIp == NULL) { // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. // Interpreter-FIXME: Implement tailcall via helpers, see https://github.com/dotnet/runtime/blob/main/docs/design/features/tailcalls-with-helpers.md - InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetMethod->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); + InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetMethod->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); break; } } @@ -2244,7 +2154,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr case INTOP_THROW: { OBJECTREF throwable; - if (LOCAL_VAR(ip[1], OBJECTREF) == nullptr) + if (LOCAL_VAR(ip[1], OBJECTREF) == NULL) { EEException ex(kNullReferenceException); throwable = ex.CreateThrowable(); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index b2d2b11543c2f9..77ac8e2d3e2e1b 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -59,10 +59,6 @@ #include "pgo.h" #endif -#ifdef FEATURE_INTERPRETER -#include "callstubgenerator.h" -#endif - #include "tailcallhelp.h" #include "patchpointinfo.h" @@ -10871,12 +10867,22 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN if (IndirectionAllowedForJitHelper(ftnNum)) { + InfoAccessType accessType; + LPVOID targetAddr; +#ifdef FEATURE_PORTABLE_ENTRYPOINTS + accessType = IAT_VALUE; + targetAddr = (LPVOID)helperMD->GetPortableEntryPoint(); +#else // !FEATURE_PORTABLE_ENTRYPOINTS Precode* pPrecode = helperMD->GetPrecode(); _ASSERTE(pPrecode->GetType() == PRECODE_FIXUP); + accessType = IAT_PVALUE; + targetAddr = ((FixupPrecode*)pPrecode)->GetTargetSlot(); +#endif // FEATURE_PORTABLE_ENTRYPOINTS + if (pNativeEntrypoint != NULL) { - pNativeEntrypoint->accessType = IAT_PVALUE; - pNativeEntrypoint->addr = ((FixupPrecode*)pPrecode)->GetTargetSlot(); + pNativeEntrypoint->accessType = accessType; + pNativeEntrypoint->addr = targetAddr; } goto exit; } @@ -11196,27 +11202,23 @@ LPVOID CEEInfo::GetCookieForInterpreterCalliSig(CORINFO_SIG_INFO* szMetaSig) #ifdef FEATURE_INTERPRETER +// Forward declare the function for mapping MetaSig to a cookie. +LPVOID GetCookieForCalliSig(MetaSig* pMetaSig); LPVOID CInterpreterJitInfo::GetCookieForInterpreterCalliSig(CORINFO_SIG_INFO* szMetaSig) { void* result = NULL; -#ifndef TARGET_WASM JIT_TO_EE_TRANSITION(); - Module* module = GetModule(szMetaSig->scope); - Instantiation classInst = Instantiation((TypeHandle*) szMetaSig->sigInst.classInst, szMetaSig->sigInst.classInstCount); Instantiation methodInst = Instantiation((TypeHandle*) szMetaSig->sigInst.methInst, szMetaSig->sigInst.methInstCount); SigTypeContext typeContext = SigTypeContext(classInst, methodInst); + Module* mod = GetModule(szMetaSig->scope); - MetaSig sig(szMetaSig->pSig, szMetaSig->cbSig, module, &typeContext); - CallStubGenerator callStubGenerator; - result = callStubGenerator.GenerateCallStubForSig(sig); + MetaSig sig(szMetaSig->pSig, szMetaSig->cbSig, mod, &typeContext); + result = GetCookieForCalliSig(&sig); EE_TO_JIT_TRANSITION(); -#else - PORTABILITY_ASSERT("GetCookieForInterpreterCalliSig is not supported on wasm yet"); -#endif // !TARGET_WASM return result; } diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 317add07b551a5..8541ac0992b645 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -29,6 +29,16 @@ void* PortableEntryPoint::GetActualCode(TADDR addr) return portableEntryPoint->_pActualCode; } +void PortableEntryPoint::SetActualCode(TADDR addr, void* actualCode) +{ + STANDARD_VM_CONTRACT; + _ASSERTE(actualCode != NULL); + + PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); + _ASSERTE(portableEntryPoint->_pActualCode == NULL); + portableEntryPoint->_pActualCode = actualCode; +} + MethodDesc* PortableEntryPoint::GetMethodDesc(TADDR addr) { STANDARD_VM_CONTRACT; @@ -53,6 +63,7 @@ void PortableEntryPoint::SetInterpreterData(TADDR addr, PCODE interpreterData) PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); _ASSERTE(portableEntryPoint->_pInterpreterData == NULL); + _ASSERTE(interpreterData != (PCODE)NULL); portableEntryPoint->_pInterpreterData = (void*)PCODEToPINSTR(interpreterData); } diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 07aec0eff4f5c6..73d891f6bae0d5 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -14,6 +14,7 @@ class PortableEntryPoint final public: // static static bool IsNativeEntryPoint(TADDR addr); static void* GetActualCode(TADDR addr); + static void SetActualCode(TADDR addr, void* actualCode); static MethodDesc* GetMethodDesc(TADDR addr); static void* GetInterpreterData(TADDR addr); static void SetInterpreterData(TADDR addr, PCODE interpreterData); @@ -31,6 +32,17 @@ class PortableEntryPoint final public: void Init(MethodDesc* pMD); + + // Query methods for entry point state. + bool IsInitialized() const { return _pMD != nullptr; } + bool IsByteCodeCompiled() const { return IsInitialized() && _pInterpreterData != nullptr; } + bool HasNativeCode() const { return IsInitialized() && _pActualCode != nullptr; } + bool IsReadyForR2R() const + { + // State when interpreted method was prepared to be called from R2R compiled code. + // pActualCode is a managed calling convention -> interpreter executor call stub in this case. + return IsInitialized() && _pInterpreterData != nullptr && _pActualCode != nullptr; + } }; extern InterleavedLoaderHeapConfig s_stubPrecodeHeapConfig; diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 0ef2ed8ff2fb29..03e47191e21789 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2297,7 +2297,17 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo pCode = GetStubForInteropMethod(this); } - GetOrCreatePrecode(); +#ifdef FEATURE_PORTABLE_ENTRYPOINTS + // Store the IL Stub interpreter data on the actual + // P/Invoke MethodDesc. + void* ilStubInterpData = PortableEntryPoint::GetInterpreterData(pCode); + _ASSERTE(ilStubInterpData != NULL); + SetInterpreterCode((InterpByteCodeStart*)ilStubInterpData); +#else // !FEATURE_PORTABLE_ENTRYPOINTS + // Only vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. + if (IsVarArg()) + GetOrCreatePrecode(); +#endif // FEATURE_PORTABLE_ENTRYPOINTS } else if (IsFCall()) { diff --git a/src/coreclr/vm/wasm/calldescrworkerwasm.cpp b/src/coreclr/vm/wasm/calldescrworkerwasm.cpp index afbf3e07ff7913..234f371de77482 100644 --- a/src/coreclr/vm/wasm/calldescrworkerwasm.cpp +++ b/src/coreclr/vm/wasm/calldescrworkerwasm.cpp @@ -13,7 +13,7 @@ extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData * pCallDescrData) if (targetIp == NULL) { GCX_PREEMP(); - pMethod->PrepareInitialCode(CallerGCMode::Coop); + (void)pMethod->DoPrestub(NULL /* MethodTable */, CallerGCMode::Preemptive); targetIp = pMethod->GetInterpreterCode(); } diff --git a/src/coreclr/vm/wasm/cgencpu.h b/src/coreclr/vm/wasm/cgencpu.h index e36a95297127b6..5bb9a5479c0f02 100644 --- a/src/coreclr/vm/wasm/cgencpu.h +++ b/src/coreclr/vm/wasm/cgencpu.h @@ -22,8 +22,8 @@ inline unsigned StackElemSize(unsigned parmSize, bool isValueType = false /* unused */, bool isFloatHfa = false /* unused */) { - _ASSERTE("The function is not implemented on wasm"); - return 0; + const unsigned stackSlotSize = sizeof(void*); + return ALIGN_UP(parmSize, stackSlotSize); } inline TADDR GetSP(const T_CONTEXT * context) diff --git a/src/coreclr/vm/wasm/helpers.cpp b/src/coreclr/vm/wasm/helpers.cpp index ac2298f34837a1..24bfcc8616fe0e 100644 --- a/src/coreclr/vm/wasm/helpers.cpp +++ b/src/coreclr/vm/wasm/helpers.cpp @@ -470,6 +470,7 @@ void _DacGlobals::Initialize() /* no-op on wasm */ } +// Incorrectly typed temporary symbol to satisfy the linker. int g_pDebugger; extern "C" int32_t mono_wasm_browser_entropy(uint8_t* buffer, int32_t bufferLength) @@ -478,17 +479,66 @@ extern "C" int32_t mono_wasm_browser_entropy(uint8_t* buffer, int32_t bufferLeng return -1; } -void InvokeCalliStub(PCODE ftn, CallStubHeader *stubHeaderTemplate, int8_t *pArgs, int8_t *pRet) +void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) { - PORTABILITY_ASSERT("InvokeCalliStub is not implemented on wasm"); + PORTABILITY_ASSERT("Attempted to execute non-interpreter code from interpreter on wasm, this is not yet implemented"); } -void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) +void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget) { - PORTABILITY_ASSERT("Attempted to execute non-interpreter code from interpreter on wasm, this is not yet implemented"); + PORTABILITY_ASSERT("Attempted to execute unmanaged code from interpreter on wasm, this is not yet implemented"); +} + +void InvokeCalliStub(PCODE ftn, void* cookie, int8_t *pArgs, int8_t *pRet) +{ + _ASSERTE(ftn != (PCODE)NULL); + _ASSERTE(cookie != NULL); + + ((void(*)(PCODE, int8_t*, int8_t*))cookie)(ftn, pArgs, pRet); } void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) { PORTABILITY_ASSERT("Attempted to execute non-interpreter code from interpreter on wasm, this is not yet implemented"); -} \ No newline at end of file +} + +namespace +{ + void CallFuncVoidI32(PCODE ftn, int8_t *pArgs, int8_t *pRet) + { + void (*fn)(int32_t) = (void (*)(int32_t))ftn; + (*fn)(((int32_t*)pArgs)[0]); + } + + void CallFuncVoidI32I32(PCODE ftn, int8_t *pArgs, int8_t *pRet) + { + void (*fn)(int32_t, int32_t) = (void (*)(int32_t, int32_t))ftn; + (*fn)(((int32_t*)pArgs)[0], ((int32_t*)pArgs)[2]); + } + + void CallFuncVoidI32I32I32(PCODE ftn, int8_t *pArgs, int8_t *pRet) + { + void (*fn)(int32_t, int32_t, int32_t) = (void (*)(int32_t, int32_t, int32_t))ftn; + (*fn)(((int32_t*)pArgs)[0], ((int32_t*)pArgs)[2], ((int32_t*)pArgs)[4]); + } +} + +LPVOID GetCookieForCalliSig(MetaSig* pMetaSig) +{ + STANDARD_VM_CONTRACT; + _ASSERTE(CheckPointer(pMetaSig)); + _ASSERTE(pMetaSig->IsReturnTypeVoid()); + + int32_t numArgs = pMetaSig->NumFixedArgs(); + switch (numArgs) + { + case 1: return (LPVOID)&CallFuncVoidI32; + case 2: return (LPVOID)&CallFuncVoidI32I32; + case 3: return (LPVOID)&CallFuncVoidI32I32I32; + default: + PORTABILITY_ASSERT("GetCookieForCalliSig: more than 3 arguments needs to be implemented"); + break; + } + + return NULL; +} From d8150793fcd60a2b0c08bef29c969c3d39f1cadd Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 17:07:02 -0700 Subject: [PATCH 02/25] Apply feedback. Add simple interpreter managed frame stack walker when native assert fires. This will help debugging asserts in interpreter scenarios. --- src/coreclr/inc/regdisp.h | 4 +- src/coreclr/pal/inc/pal_assert.h | 8 + src/coreclr/vm/callhelpers.cpp | 10 +- src/coreclr/vm/callstubgenerator.cpp | 2 +- src/coreclr/vm/dllimport.cpp | 3 + src/coreclr/vm/interpexec.cpp | 94 ++++++++++- src/coreclr/vm/precode_portable.cpp | 15 +- src/coreclr/vm/precode_portable.hpp | 25 ++- src/coreclr/vm/prestub.cpp | 4 - src/coreclr/vm/wasm/calldescrworkerwasm.cpp | 2 +- src/coreclr/vm/wasm/helpers.cpp | 169 +++++++++++++++++--- 11 files changed, 296 insertions(+), 40 deletions(-) diff --git a/src/coreclr/inc/regdisp.h b/src/coreclr/inc/regdisp.h index 7a5cf9d9d0cec4..78340a4268431b 100644 --- a/src/coreclr/inc/regdisp.h +++ b/src/coreclr/inc/regdisp.h @@ -471,7 +471,9 @@ inline void FillContextPointers(PT_KNONVOLATILE_CONTEXT_POINTERS pCtxPtrs, PT_CO *(&pCtxPtrs->Tp) = &pCtx->Tp; *(&pCtxPtrs->Fp) = &pCtx->Fp; *(&pCtxPtrs->Ra) = &pCtx->Ra; -#else // TARGET_RISCV64 +#elif defined(TARGET_WASM) + // Wasm doesn't have registers +#else // TARGET_WASM PORTABILITY_ASSERT("FillContextPointers"); #endif // _TARGET_???_ (ELSE) } diff --git a/src/coreclr/pal/inc/pal_assert.h b/src/coreclr/pal/inc/pal_assert.h index a4b49c1ddafc5f..a25d95e3f2e7a3 100644 --- a/src/coreclr/pal/inc/pal_assert.h +++ b/src/coreclr/pal/inc/pal_assert.h @@ -33,6 +33,13 @@ extern "C" { #ifndef _ASSERTE #if defined(_DEBUG) + +#ifdef FEATURE_INTERPRETER +extern void DBG_PrintInterpreterStack(); +#else // !FEATURE_INTERPRETER +#define DBG_PrintInterpreterStack() ((void)0) +#endif // FEATURE_INTERPRETER + #define _ASSERTE(e) do { \ if (!(e)) { \ fprintf (stderr, \ @@ -43,6 +50,7 @@ extern "C" { "\tProcess: %d\n", \ #e, __LINE__, __FILE__, __FUNCTION__, \ GetCurrentProcessId()); \ + DBG_PrintInterpreterStack(); \ DebugBreak(); \ } \ }while (0) diff --git a/src/coreclr/vm/callhelpers.cpp b/src/coreclr/vm/callhelpers.cpp index daf5df18ee75fc..5ddd815899d356 100644 --- a/src/coreclr/vm/callhelpers.cpp +++ b/src/coreclr/vm/callhelpers.cpp @@ -224,9 +224,13 @@ void* DispatchCallSimple( callDescrData.fpReturnSize = 0; callDescrData.pTarget = pTargetAddress; -#ifdef TARGET_WASM - PORTABILITY_ASSERT("wasm need to fill call description data"); -#endif +#ifdef FEATURE_PORTABLE_ENTRYPOINTS + callDescrData.pMD = PortableEntryPoint::GetMethodDesc(pTargetAddress); + callDescrData.nArgsSize = numStackSlotsToCopy * sizeof(ARGHOLDER_TYPE); + + TransitionBlock block{}; + callDescrData.pTransitionBlock = █ +#endif // FEATURE_PORTABLE_ENTRYPOINTS if ((dwDispatchCallSimpleFlags & DispatchCallSimple_CatchHandlerFoundNotification) != 0) { diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index 746425d004b775..695ee63756ad0f 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -1967,7 +1967,7 @@ void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, in CONTRACTL { THROWS; - MODE_ANY; + MODE_COOPERATIVE; PRECONDITION(CheckPointer(pMDDelegateInvoke)); PRECONDITION(CheckPointer(pArgs)); PRECONDITION(CheckPointer(pRet)); diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 00badfce6afc1f..d5a97e58a97afc 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5654,6 +5654,9 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, // varargs goes through vararg PInvoke stub // pStub = TheVarargPInvokeStub(pNMD->HasRetBuffArg()); + + // Only vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. + (void)pNMD->GetOrCreatePrecode(); } if (pNMD->IsEarlyBound()) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index bb3c2b7e9b1cab..6e6a815226d0d4 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -75,6 +75,42 @@ CallStubHeader *CreateNativeToInterpreterCallStub(InterpMethod* pInterpMethod) } #endif // !TARGET_WASM +#ifdef _DEBUG +void DBG_PrintInterpreterStack() +{ + Thread* pThread = GetThread(); + + int32_t frameCount = 0; + + // Get the "capital F" frame and start walking. + for (Frame* pFrame = pThread->GetFrame(); pFrame != FRAME_TOP; pFrame = pFrame->PtrNextFrame()) + { + fprintf(stderr, "Frame (%s): %p\n", Frame::GetFrameTypeName(pFrame->GetFrameIdentifier()), pFrame); + if (pFrame->GetFrameIdentifier() != FrameIdentifier::InterpreterFrame) + { + fprintf(stderr, " Skipping %p\n", pFrame); + continue; + } + + // Walk the current block of managed frames. + InterpreterFrame* pInterpFrame = (InterpreterFrame*)pFrame; + InterpMethodContextFrame* cxtFrame = pInterpFrame->GetTopInterpMethodContextFrame(); + while (cxtFrame != NULL) + { + MethodDesc* currentMD = ((MethodDesc*)cxtFrame->startIp->Method->methodHnd); + + size_t irOffset = ((size_t)cxtFrame->ip - (size_t)(&cxtFrame->startIp[1])) / sizeof(size_t); + fprintf(stderr, "%4d) %s::%s, IR_%04zx\n", + frameCount++, + currentMD->GetMethodTable()->GetDebugClassName(), + currentMD->GetName(), + irOffset); + cxtFrame = cxtFrame->pParent; + } + } +} +#endif // _DEBUG + typedef void* (*HELPER_FTN_P_P)(void*); typedef void* (*HELPER_FTN_BOX_UNBOX)(MethodTable*, void*); typedef Object* (*HELPER_FTN_NEWARR)(MethodTable*, intptr_t); @@ -403,6 +439,9 @@ static void CallPreStub(MethodDesc *pMD, CallerGCMode callerGCMode = CallerGCMod STATIC_STANDARD_VM_CONTRACT; _ASSERTE(pMD != NULL); + if (!pMD->IsPointingToPrestub()) + return; + struct Param { MethodDesc *pMethodDesc; @@ -1734,9 +1773,25 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr case INTOP_CALL_HELPER_P_P: { - HELPER_FTN_P_P helperFtn = GetPossiblyIndirectHelper(pMethod, ip[2]); void* helperArg = pMethod->pDataItems[ip[3]]; + MethodDesc *pILTargetMethod = NULL; + HELPER_FTN_P_P helperFtn = GetPossiblyIndirectHelper(pMethod, ip[2], &pILTargetMethod); + if (pILTargetMethod != NULL) + { + returnOffset = ip[1]; + int stackOffset = pMethod->allocaSize; + callArgsOffset = stackOffset; + + // Pass argument to the target method + LOCAL_VAR(stackOffset, void*) = helperArg; + + targetMethod = pILTargetMethod; + ip += 4; + goto CALL_INTERP_METHOD; + } + + _ASSERTE(helperFtn != NULL); LOCAL_VAR(ip[1], void*) = helperFtn(helperArg); ip += 4; break; @@ -1754,9 +1809,26 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr case INTOP_CALL_HELPER_P_PS: { - HELPER_FTN_P_PP helperFtn = GetPossiblyIndirectHelper(pMethod, ip[3]); void* helperArg = pMethod->pDataItems[ip[4]]; + MethodDesc *pILTargetMethod = NULL; + HELPER_FTN_P_PP helperFtn = GetPossiblyIndirectHelper(pMethod, ip[3], &pILTargetMethod); + if (pILTargetMethod != NULL) + { + returnOffset = ip[1]; + int stackOffset = pMethod->allocaSize; + callArgsOffset = stackOffset; + + // Pass arguments to the target method + LOCAL_VAR(stackOffset, void*) = helperArg; + LOCAL_VAR(stackOffset + INTERP_STACK_SLOT_SIZE, void*) = LOCAL_VAR(ip[2], void*); + + targetMethod = pILTargetMethod; + ip += 5; + goto CALL_INTERP_METHOD; + } + + _ASSERTE(helperFtn != NULL); LOCAL_VAR(ip[1], void*) = helperFtn(helperArg, LOCAL_VAR(ip[2], void*)); ip += 5; break; @@ -1803,8 +1875,24 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr InterpGenericLookup *pLookup = (InterpGenericLookup*)&pMethod->pDataItems[ip[5]]; void* helperArg = DoGenericLookup(LOCAL_VAR(ip[2], void*), pLookup); - HELPER_FTN_P_PP helperFtn = GetPossiblyIndirectHelper(pMethod, ip[4]); + MethodDesc *pILTargetMethod = NULL; + HELPER_FTN_P_PP helperFtn = GetPossiblyIndirectHelper(pMethod, ip[4], &pILTargetMethod); + if (pILTargetMethod != NULL) + { + returnOffset = ip[1]; + int stackOffset = pMethod->allocaSize; + callArgsOffset = stackOffset; + // Pass arguments to the target method + LOCAL_VAR(stackOffset, void*) = helperArg; + LOCAL_VAR(stackOffset + INTERP_STACK_SLOT_SIZE, void*) = LOCAL_VAR(ip[3], void*); + + targetMethod = pILTargetMethod; + ip += 6; + goto CALL_INTERP_METHOD; + } + + _ASSERTE(helperFtn != NULL); LOCAL_VAR(ip[1], void*) = helperFtn(helperArg, LOCAL_VAR(ip[3], void*)); ip += 6; break; diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 8541ac0992b645..44518db0740d49 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -25,7 +25,7 @@ void* PortableEntryPoint::GetActualCode(TADDR addr) STANDARD_VM_CONTRACT; PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode != NULL); + _ASSERTE(portableEntryPoint->_pActualCode != (BYTE*)NULL); return portableEntryPoint->_pActualCode; } @@ -35,8 +35,8 @@ void PortableEntryPoint::SetActualCode(TADDR addr, void* actualCode) _ASSERTE(actualCode != NULL); PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode == NULL); - portableEntryPoint->_pActualCode = actualCode; + _ASSERTE(portableEntryPoint->_pActualCode == (BYTE*)NULL || portableEntryPoint->_pActualCode == (BYTE*)actualCode); + portableEntryPoint->_pActualCode = (BYTE*)actualCode; } MethodDesc* PortableEntryPoint::GetMethodDesc(TADDR addr) @@ -73,7 +73,7 @@ PortableEntryPoint* PortableEntryPoint::ToPortableEntryPoint(TADDR addr) _ASSERTE(addr != NULL); PortableEntryPoint* portableEntryPoint = (PortableEntryPoint*)addr; - _ASSERTE(portableEntryPoint->_canary == CANARY_VALUE); + _ASSERTE(portableEntryPoint->IsValid()); return portableEntryPoint; } @@ -86,6 +86,13 @@ void PortableEntryPoint::Init(MethodDesc* pMD) INDEBUG(_canary = CANARY_VALUE); } +bool PortableEntryPoint::IsValid() const +{ + LIMITED_METHOD_CONTRACT; + _ASSERTE(_canary == CANARY_VALUE); + return _pMD != nullptr; +} + InterleavedLoaderHeapConfig s_stubPrecodeHeapConfig; void StubPrecode::Init(StubPrecode* pPrecodeRX, TADDR secretParam, LoaderAllocator *pLoaderAllocator, TADDR type, TADDR target) diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 73d891f6bae0d5..4e0e074dec52a2 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -23,7 +23,7 @@ class PortableEntryPoint final static PortableEntryPoint* ToPortableEntryPoint(TADDR addr); private: - void* _pActualCode; + VolatilePtr _pActualCode; MethodDesc* _pMD; void* _pInterpreterData; @@ -34,14 +34,29 @@ class PortableEntryPoint final void Init(MethodDesc* pMD); // Query methods for entry point state. - bool IsInitialized() const { return _pMD != nullptr; } - bool IsByteCodeCompiled() const { return IsInitialized() && _pInterpreterData != nullptr; } - bool HasNativeCode() const { return IsInitialized() && _pActualCode != nullptr; } + bool IsValid() const; + + bool IsByteCodeCompiled() const + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsValid()); + return _pInterpreterData != nullptr; + } + + bool HasNativeCode() const + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsValid()); + return _pActualCode != nullptr; + } + bool IsReadyForR2R() const { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsValid()); // State when interpreted method was prepared to be called from R2R compiled code. // pActualCode is a managed calling convention -> interpreter executor call stub in this case. - return IsInitialized() && _pInterpreterData != nullptr && _pActualCode != nullptr; + return _pInterpreterData != nullptr && _pActualCode != nullptr; } }; diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 03e47191e21789..4ea44bfe7eb5a2 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2303,10 +2303,6 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo void* ilStubInterpData = PortableEntryPoint::GetInterpreterData(pCode); _ASSERTE(ilStubInterpData != NULL); SetInterpreterCode((InterpByteCodeStart*)ilStubInterpData); -#else // !FEATURE_PORTABLE_ENTRYPOINTS - // Only vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. - if (IsVarArg()) - GetOrCreatePrecode(); #endif // FEATURE_PORTABLE_ENTRYPOINTS } else if (IsFCall()) diff --git a/src/coreclr/vm/wasm/calldescrworkerwasm.cpp b/src/coreclr/vm/wasm/calldescrworkerwasm.cpp index 234f371de77482..363acc1cdad425 100644 --- a/src/coreclr/vm/wasm/calldescrworkerwasm.cpp +++ b/src/coreclr/vm/wasm/calldescrworkerwasm.cpp @@ -13,7 +13,7 @@ extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData * pCallDescrData) if (targetIp == NULL) { GCX_PREEMP(); - (void)pMethod->DoPrestub(NULL /* MethodTable */, CallerGCMode::Preemptive); + (void)pMethod->DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop); targetIp = pMethod->GetInterpreterCode(); } diff --git a/src/coreclr/vm/wasm/helpers.cpp b/src/coreclr/vm/wasm/helpers.cpp index 24bfcc8616fe0e..57115cbf80c40d 100644 --- a/src/coreclr/vm/wasm/helpers.cpp +++ b/src/coreclr/vm/wasm/helpers.cpp @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // +#include "../../interpreter/interpretershared.h" + extern "C" void STDCALL CallCountingStubCode() { PORTABILITY_ASSERT("CallCountingStubCode is not implemented on wasm"); @@ -504,22 +506,150 @@ void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, in namespace { - void CallFuncVoidI32(PCODE ftn, int8_t *pArgs, int8_t *pRet) + // Arguments are passed on the stack with each argument aligned to INTERP_STACK_SLOT_SIZE. +#define ARG(i) *((int32_t*)(pArgs + (i * INTERP_STACK_SLOT_SIZE))) + + void CallFunc_Void_RetVoid(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + void (*fptr)(void) = (void (*)(void))pcode; + (*fptr)(); + } + + void CallFunc_I32_RetVoid(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + void (*fptr)(int32_t) = (void (*)(int32_t))pcode; + (*fptr)(ARG(0)); + } + + void CallFunc_I32_I32_RetVoid(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + void (*fptr)(int32_t, int32_t) = (void (*)(int32_t, int32_t))pcode; + (*fptr)(ARG(0), ARG(1)); + } + + void CallFunc_I32_I32_I32_RetVoid(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + void (*fptr)(int32_t, int32_t, int32_t) = (void (*)(int32_t, int32_t, int32_t))pcode; + (*fptr)(ARG(0), ARG(1), ARG(2)); + } + + void CallFunc_Void_RetI32(PCODE pcode, int8_t *pArgs, int8_t *pRet) { - void (*fn)(int32_t) = (void (*)(int32_t))ftn; - (*fn)(((int32_t*)pArgs)[0]); + int32_t (*fptr)(void) = (int32_t (*)(void))pcode; + *(int32_t*)pRet = (*fptr)(); } - void CallFuncVoidI32I32(PCODE ftn, int8_t *pArgs, int8_t *pRet) + void CallFunc_I32_RetI32(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + int32_t (*fptr)(int32_t) = (int32_t (*)(int32_t))pcode; + *(int32_t*)pRet = (*fptr)(ARG(0)); + } + + void CallFunc_I32_I32_RetI32(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + int32_t (*fptr)(int32_t, int32_t) = (int32_t (*)(int32_t, int32_t))pcode; + *(int32_t*)pRet = (*fptr)(ARG(0), ARG(1)); + } + + void CallFunc_I32_I32_I32_RetI32(PCODE pcode, int8_t *pArgs, int8_t *pRet) + { + int32_t (*fptr)(int32_t, int32_t, int32_t) = (int32_t (*)(int32_t, int32_t, int32_t))pcode; + *(int32_t*)pRet = (*fptr)(ARG(0), ARG(1), ARG(2)); + } + +#undef ARG +} + +namespace +{ + enum class CalliSigThunk + { + Unknown, + Void_RetVoid, + I32_RetVoid, + I32_I32_RetVoid, + I32_I32_I32_RetVoid, + Void_RetI32, + I32_RetI32, + I32_I32_RetI32, + I32_I32_I32_RetI32, + }; + + bool ConvertibleToI32(CorElementType argType) { - void (*fn)(int32_t, int32_t) = (void (*)(int32_t, int32_t))ftn; - (*fn)(((int32_t*)pArgs)[0], ((int32_t*)pArgs)[2]); + // See https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md + switch (argType) + { + case ELEMENT_TYPE_BOOLEAN: + case ELEMENT_TYPE_CHAR: + case ELEMENT_TYPE_I1: + case ELEMENT_TYPE_U1: + case ELEMENT_TYPE_I2: + case ELEMENT_TYPE_U2: + case ELEMENT_TYPE_I4: + case ELEMENT_TYPE_U4: + case ELEMENT_TYPE_STRING: + case ELEMENT_TYPE_PTR: + case ELEMENT_TYPE_BYREF: + case ELEMENT_TYPE_VALUETYPE: + case ELEMENT_TYPE_CLASS: + case ELEMENT_TYPE_ARRAY: + case ELEMENT_TYPE_TYPEDBYREF: + case ELEMENT_TYPE_I: + case ELEMENT_TYPE_U: + case ELEMENT_TYPE_FNPTR: + case ELEMENT_TYPE_SZARRAY: + return true; + default: + return false; + } } - void CallFuncVoidI32I32I32(PCODE ftn, int8_t *pArgs, int8_t *pRet) + // This is a simple signature computation routine for signatures currently supported in the wasm environment. + // Note: Currently only validates void return type and i32 wasm convertible arguments. + CalliSigThunk ComputeCalliSigThunk(MetaSig sig) { - void (*fn)(int32_t, int32_t, int32_t) = (void (*)(int32_t, int32_t, int32_t))ftn; - (*fn)(((int32_t*)pArgs)[0], ((int32_t*)pArgs)[2], ((int32_t*)pArgs)[4]); + STANDARD_VM_CONTRACT; + _ASSERTE(sizeof(int32_t) == sizeof(void*)); + + // Ensure an unmanaged calling convention. + BYTE callConv = sig.GetCallingConvention(); + switch (callConv) + { + case IMAGE_CEE_CS_CALLCONV_C: + case IMAGE_CEE_CS_CALLCONV_STDCALL: + case IMAGE_CEE_CS_CALLCONV_FASTCALL: + case IMAGE_CEE_CS_CALLCONV_UNMANAGED: + break; + default: + return CalliSigThunk::Unknown; + } + + // Check return value + bool returnsVoid = sig.IsReturnTypeVoid(); + if (!returnsVoid && !ConvertibleToI32(sig.GetReturnType())) + return CalliSigThunk::Unknown; + + CalliSigThunk baseType = returnsVoid ? CalliSigThunk::Void_RetVoid : CalliSigThunk::Void_RetI32; + + // Ensure all arguments are wasm i32 compatible types. + for (CorElementType argType = sig.NextArg(); + argType != ELEMENT_TYPE_END; + argType = sig.NextArg()) + { + if (!ConvertibleToI32(argType)) + return CalliSigThunk::Unknown; + } + + int32_t numArgs = sig.NumFixedArgs(); + switch (numArgs) + { + case 0: return baseType; + case 1: return (CalliSigThunk)((int)baseType + 1); + case 2: return (CalliSigThunk)((int)baseType + 2); + case 3: return (CalliSigThunk)((int)baseType + 3); + default: return CalliSigThunk::Unknown; + }; } } @@ -527,18 +657,21 @@ LPVOID GetCookieForCalliSig(MetaSig* pMetaSig) { STANDARD_VM_CONTRACT; _ASSERTE(CheckPointer(pMetaSig)); - _ASSERTE(pMetaSig->IsReturnTypeVoid()); - int32_t numArgs = pMetaSig->NumFixedArgs(); - switch (numArgs) + CalliSigThunk thunkType = ComputeCalliSigThunk(*pMetaSig); + switch (thunkType) { - case 1: return (LPVOID)&CallFuncVoidI32; - case 2: return (LPVOID)&CallFuncVoidI32I32; - case 3: return (LPVOID)&CallFuncVoidI32I32I32; - default: - PORTABILITY_ASSERT("GetCookieForCalliSig: more than 3 arguments needs to be implemented"); - break; + case CalliSigThunk::Void_RetVoid: return (LPVOID)&CallFunc_Void_RetVoid; + case CalliSigThunk::I32_RetVoid: return (LPVOID)&CallFunc_I32_RetVoid; + case CalliSigThunk::I32_I32_RetVoid: return (LPVOID)&CallFunc_I32_I32_RetVoid; + case CalliSigThunk::I32_I32_I32_RetVoid: return (LPVOID)&CallFunc_I32_I32_I32_RetVoid; + case CalliSigThunk::Void_RetI32: return (LPVOID)&CallFunc_Void_RetI32; + case CalliSigThunk::I32_RetI32: return (LPVOID)&CallFunc_I32_RetI32; + case CalliSigThunk::I32_I32_RetI32: return (LPVOID)&CallFunc_I32_I32_RetI32; + case CalliSigThunk::I32_I32_I32_RetI32: return (LPVOID)&CallFunc_I32_I32_I32_RetI32; + default: break; } + PORTABILITY_ASSERT("GetCookieForCalliSig: unknown thunk signature"); return NULL; } From 187ff1bd149b7c3b8b7abe789b9a70dc392d83e2 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 17:40:18 -0700 Subject: [PATCH 03/25] Narrow use of printing interpreter stack on assert to WASM. --- src/coreclr/pal/inc/pal_assert.h | 7 ------- src/coreclr/pal/src/arch/wasm/stubs.cpp | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/pal/inc/pal_assert.h b/src/coreclr/pal/inc/pal_assert.h index a25d95e3f2e7a3..d75c80ed3e19f8 100644 --- a/src/coreclr/pal/inc/pal_assert.h +++ b/src/coreclr/pal/inc/pal_assert.h @@ -34,12 +34,6 @@ extern "C" { #ifndef _ASSERTE #if defined(_DEBUG) -#ifdef FEATURE_INTERPRETER -extern void DBG_PrintInterpreterStack(); -#else // !FEATURE_INTERPRETER -#define DBG_PrintInterpreterStack() ((void)0) -#endif // FEATURE_INTERPRETER - #define _ASSERTE(e) do { \ if (!(e)) { \ fprintf (stderr, \ @@ -50,7 +44,6 @@ extern void DBG_PrintInterpreterStack(); "\tProcess: %d\n", \ #e, __LINE__, __FILE__, __FUNCTION__, \ GetCurrentProcessId()); \ - DBG_PrintInterpreterStack(); \ DebugBreak(); \ } \ }while (0) diff --git a/src/coreclr/pal/src/arch/wasm/stubs.cpp b/src/coreclr/pal/src/arch/wasm/stubs.cpp index 66b73f8badf17b..5f7c99e408474f 100644 --- a/src/coreclr/pal/src/arch/wasm/stubs.cpp +++ b/src/coreclr/pal/src/arch/wasm/stubs.cpp @@ -8,9 +8,16 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do /* debugbreak */ +#ifdef _DEBUG +extern void DBG_PrintInterpreterStack(); +#endif // _DEBUG + extern "C" void DBG_DebugBreak() { +#ifdef _DEBUG + DBG_PrintInterpreterStack(); +#endif // _DEBUG asm volatile ("unreachable"); } From 58c9ee42b4e2e1ac3b9c92286c41abd33cfd2a40 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 17:40:56 -0700 Subject: [PATCH 04/25] Remvove white space edit. --- src/coreclr/pal/inc/pal_assert.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/pal/inc/pal_assert.h b/src/coreclr/pal/inc/pal_assert.h index d75c80ed3e19f8..a4b49c1ddafc5f 100644 --- a/src/coreclr/pal/inc/pal_assert.h +++ b/src/coreclr/pal/inc/pal_assert.h @@ -33,7 +33,6 @@ extern "C" { #ifndef _ASSERTE #if defined(_DEBUG) - #define _ASSERTE(e) do { \ if (!(e)) { \ fprintf (stderr, \ From 9f6858ae0570d71471aa16106fb8d3f5bdc05964 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 19:02:17 -0700 Subject: [PATCH 05/25] Handle native helpers with portable entry points --- src/coreclr/vm/jitinterface.cpp | 8 +++++++- src/coreclr/vm/precode_portable.cpp | 22 ++++++++++++++++++---- src/coreclr/vm/precode_portable.hpp | 2 ++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 77ac8e2d3e2e1b..acda3749be6326 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10896,7 +10896,13 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN if (pNativeEntrypoint != NULL) { pNativeEntrypoint->accessType = IAT_VALUE; - pNativeEntrypoint->addr = (LPVOID)GetEEFuncEntryPoint(pfnHelper); + TADDR entryPoint = GetEEFuncEntryPoint(pfnHelper); + +#ifdef FEATURE_PORTABLE_ENTRYPOINTS + entryPoint = PortableEntryPoint::MarkNativeEntryPoint(entryPoint); +#endif // FEATURE_PORTABLE_ENTRYPOINTS + + pNativeEntrypoint->addr = (LPVOID)entryPoint; } exit: ; diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 44518db0740d49..577354d10fc129 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -8,22 +8,35 @@ #include "precode_portable.hpp" #ifdef HOST_64BIT - #define CANARY_VALUE 0x1234567812345678 +#define CANARY_VALUE 0x1234567812345678 #else // HOST_64BIT - #define CANARY_VALUE 0x12345678 +#define CANARY_VALUE 0x12345678 #endif // HOST_64BIT +#define NATIVE_ENTRYPOINT_HELPER_BIT 0x1 + bool PortableEntryPoint::IsNativeEntryPoint(TADDR addr) { - STANDARD_VM_CONTRACT; + LIMITED_METHOD_CONTRACT; + return (addr & NATIVE_ENTRYPOINT_HELPER_BIT) == NATIVE_ENTRYPOINT_HELPER_BIT; +} - return false; +TADDR PortableEntryPoint::MarkNativeEntryPoint(TADDR entryPoint) +{ + LIMITED_METHOD_CONTRACT; + return entryPoint | NATIVE_ENTRYPOINT_HELPER_BIT; } void* PortableEntryPoint::GetActualCode(TADDR addr) { STANDARD_VM_CONTRACT; + if (IsNativeEntryPoint(addr)) + { + const TADDR mask = ~NATIVE_ENTRYPOINT_HELPER_BIT; + return (void*)(addr & mask); + } + PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); _ASSERTE(portableEntryPoint->_pActualCode != (BYTE*)NULL); return portableEntryPoint->_pActualCode; @@ -71,6 +84,7 @@ PortableEntryPoint* PortableEntryPoint::ToPortableEntryPoint(TADDR addr) { LIMITED_METHOD_CONTRACT; _ASSERTE(addr != NULL); + _ASSERTE(!IsNativeEntryPoint(addr)); PortableEntryPoint* portableEntryPoint = (PortableEntryPoint*)addr; _ASSERTE(portableEntryPoint->IsValid()); diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 4e0e074dec52a2..077a578e8f99bc 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -13,6 +13,8 @@ class PortableEntryPoint final { public: // static static bool IsNativeEntryPoint(TADDR addr); + static TADDR MarkNativeEntryPoint(TADDR entryPoint); + static void* GetActualCode(TADDR addr); static void SetActualCode(TADDR addr, void* actualCode); static MethodDesc* GetMethodDesc(TADDR addr); From f385dacd18be3e1e19a9e873cf8ec2e5e06f44f9 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 19:23:51 -0700 Subject: [PATCH 06/25] Switch to high order bits for native entry points --- src/coreclr/vm/precode_portable.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 577354d10fc129..6a46e241291353 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -7,14 +7,16 @@ #include "common.h" #include "precode_portable.hpp" +// WASM uses function indices so we should avoid using +// low order bits for masking. #ifdef HOST_64BIT #define CANARY_VALUE 0x1234567812345678 +#define NATIVE_ENTRYPOINT_HELPER_BIT 0x8000000000000000 #else // HOST_64BIT #define CANARY_VALUE 0x12345678 +#define NATIVE_ENTRYPOINT_HELPER_BIT 0x80000000 #endif // HOST_64BIT -#define NATIVE_ENTRYPOINT_HELPER_BIT 0x1 - bool PortableEntryPoint::IsNativeEntryPoint(TADDR addr) { LIMITED_METHOD_CONTRACT; From f931b09cff8193885e6746f48e2eaaa35aa7bf76 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 20:57:59 -0700 Subject: [PATCH 07/25] Rework WASM thunk look-up --- src/coreclr/vm/callstubgenerator.cpp | 5 +- src/coreclr/vm/jitinterface.cpp | 4 +- src/coreclr/vm/wasm/helpers.cpp | 79 +++++++++++++--------------- 3 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index 695ee63756ad0f..5486ff01562bcc 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -2012,13 +2012,12 @@ void InvokeCalliStub(PCODE ftn, void *cookie, int8_t *pArgs, int8_t *pRet) pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); } -LPVOID GetCookieForCalliSig(MetaSig* pMetaSig) +LPVOID GetCookieForCalliSig(MetaSig metaSig) { STANDARD_VM_CONTRACT; - _ASSERTE(CheckPointer(pMetaSig)); CallStubGenerator callStubGenerator; - return callStubGenerator.GenerateCallStubForSig(*pMetaSig); + return callStubGenerator.GenerateCallStubForSig(metaSig); } #endif // FEATURE_INTERPRETER && !TARGET_WASM diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index acda3749be6326..90334cf3eb2ebe 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11209,7 +11209,7 @@ LPVOID CEEInfo::GetCookieForInterpreterCalliSig(CORINFO_SIG_INFO* szMetaSig) #ifdef FEATURE_INTERPRETER // Forward declare the function for mapping MetaSig to a cookie. -LPVOID GetCookieForCalliSig(MetaSig* pMetaSig); +LPVOID GetCookieForCalliSig(MetaSig metaSig); LPVOID CInterpreterJitInfo::GetCookieForInterpreterCalliSig(CORINFO_SIG_INFO* szMetaSig) { @@ -11222,7 +11222,7 @@ LPVOID CInterpreterJitInfo::GetCookieForInterpreterCalliSig(CORINFO_SIG_INFO* sz Module* mod = GetModule(szMetaSig->scope); MetaSig sig(szMetaSig->pSig, szMetaSig->cbSig, mod, &typeContext); - result = GetCookieForCalliSig(&sig); + result = GetCookieForCalliSig(sig); EE_TO_JIT_TRANSITION(); return result; diff --git a/src/coreclr/vm/wasm/helpers.cpp b/src/coreclr/vm/wasm/helpers.cpp index 57115cbf80c40d..c675dab549b9fd 100644 --- a/src/coreclr/vm/wasm/helpers.cpp +++ b/src/coreclr/vm/wasm/helpers.cpp @@ -558,21 +558,21 @@ namespace } #undef ARG -} -namespace -{ - enum class CalliSigThunk + void* RetVoidThunks[] = + { + (void*)&CallFunc_Void_RetVoid, + (void*)&CallFunc_I32_RetVoid, + (void*)&CallFunc_I32_I32_RetVoid, + (void*)&CallFunc_I32_I32_I32_RetVoid, + }; + + void* RetI32Thunks[] = { - Unknown, - Void_RetVoid, - I32_RetVoid, - I32_I32_RetVoid, - I32_I32_I32_RetVoid, - Void_RetI32, - I32_RetI32, - I32_I32_RetI32, - I32_I32_I32_RetI32, + (void*)&CallFunc_Void_RetI32, + (void*)&CallFunc_I32_RetI32, + (void*)&CallFunc_I32_I32_RetI32, + (void*)&CallFunc_I32_I32_I32_RetI32, }; bool ConvertibleToI32(CorElementType argType) @@ -607,7 +607,7 @@ namespace // This is a simple signature computation routine for signatures currently supported in the wasm environment. // Note: Currently only validates void return type and i32 wasm convertible arguments. - CalliSigThunk ComputeCalliSigThunk(MetaSig sig) + void* ComputeCalliSigThunk(MetaSig& sig) { STANDARD_VM_CONTRACT; _ASSERTE(sizeof(int32_t) == sizeof(void*)); @@ -622,15 +622,13 @@ namespace case IMAGE_CEE_CS_CALLCONV_UNMANAGED: break; default: - return CalliSigThunk::Unknown; + return NULL; } // Check return value bool returnsVoid = sig.IsReturnTypeVoid(); if (!returnsVoid && !ConvertibleToI32(sig.GetReturnType())) - return CalliSigThunk::Unknown; - - CalliSigThunk baseType = returnsVoid ? CalliSigThunk::Void_RetVoid : CalliSigThunk::Void_RetI32; + return NULL; // Ensure all arguments are wasm i32 compatible types. for (CorElementType argType = sig.NextArg(); @@ -638,40 +636,37 @@ namespace argType = sig.NextArg()) { if (!ConvertibleToI32(argType)) - return CalliSigThunk::Unknown; + return NULL; } - int32_t numArgs = sig.NumFixedArgs(); - switch (numArgs) + UINT numArgs = sig.NumFixedArgs(); + void** thunks; + if (returnsVoid) + { + thunks = RetVoidThunks; + if (numArgs >= ARRAY_SIZE(RetVoidThunks)) + return NULL; + } + else { - case 0: return baseType; - case 1: return (CalliSigThunk)((int)baseType + 1); - case 2: return (CalliSigThunk)((int)baseType + 2); - case 3: return (CalliSigThunk)((int)baseType + 3); - default: return CalliSigThunk::Unknown; - }; + thunks = RetI32Thunks; + if (numArgs >= ARRAY_SIZE(RetI32Thunks)) + return NULL; + } + + return thunks[numArgs]; } } -LPVOID GetCookieForCalliSig(MetaSig* pMetaSig) +LPVOID GetCookieForCalliSig(MetaSig metaSig) { STANDARD_VM_CONTRACT; - _ASSERTE(CheckPointer(pMetaSig)); - CalliSigThunk thunkType = ComputeCalliSigThunk(*pMetaSig); - switch (thunkType) + void* thunk = ComputeCalliSigThunk(metaSig); + if (thunk == NULL) { - case CalliSigThunk::Void_RetVoid: return (LPVOID)&CallFunc_Void_RetVoid; - case CalliSigThunk::I32_RetVoid: return (LPVOID)&CallFunc_I32_RetVoid; - case CalliSigThunk::I32_I32_RetVoid: return (LPVOID)&CallFunc_I32_I32_RetVoid; - case CalliSigThunk::I32_I32_I32_RetVoid: return (LPVOID)&CallFunc_I32_I32_I32_RetVoid; - case CalliSigThunk::Void_RetI32: return (LPVOID)&CallFunc_Void_RetI32; - case CalliSigThunk::I32_RetI32: return (LPVOID)&CallFunc_I32_RetI32; - case CalliSigThunk::I32_I32_RetI32: return (LPVOID)&CallFunc_I32_I32_RetI32; - case CalliSigThunk::I32_I32_I32_RetI32: return (LPVOID)&CallFunc_I32_I32_I32_RetI32; - default: break; + PORTABILITY_ASSERT("GetCookieForCalliSig: unknown thunk signature"); } - PORTABILITY_ASSERT("GetCookieForCalliSig: unknown thunk signature"); - return NULL; + return thunk; } From 998c61d35d8f60593a75b0b41fbdf0ddd949cf64 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 31 Aug 2025 21:43:29 -0700 Subject: [PATCH 08/25] Fix Release build. --- src/coreclr/vm/threads.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 92344beb931331..61c339dbb8272e 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1062,10 +1062,12 @@ void InitThreadManager() } CONTRACTL_END; +#ifndef TARGET_WASM // All patched helpers should fit into one page. // If you hit this assert on retail build, there is most likely problem with BBT script. _ASSERTE_ALL_BUILDS((BYTE*)JIT_PatchedCodeLast - (BYTE*)JIT_PatchedCodeStart > (ptrdiff_t)0); _ASSERTE_ALL_BUILDS((BYTE*)JIT_PatchedCodeLast - (BYTE*)JIT_PatchedCodeStart < (ptrdiff_t)GetOsPageSize()); +#endif // !TARGET_WASM if (IsWriteBarrierCopyEnabled()) { From 0aa701f46d46c9bd478e0699b4b879c9b0623afe Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Sep 2025 11:02:38 -0700 Subject: [PATCH 09/25] Feedback. --- src/coreclr/vm/callhelpers.cpp | 8 -- src/coreclr/vm/callhelpers.h | 3 - src/coreclr/vm/callingconvention.h | 33 +++--- src/coreclr/vm/callstubgenerator.cpp | 118 ------------------ src/coreclr/vm/interpexec.cpp | 125 +++++++++++++++++++- src/coreclr/vm/precode_portable.hpp | 4 +- src/coreclr/vm/wasm/calldescrworkerwasm.cpp | 7 +- 7 files changed, 145 insertions(+), 153 deletions(-) diff --git a/src/coreclr/vm/callhelpers.cpp b/src/coreclr/vm/callhelpers.cpp index 5ddd815899d356..79101882c7e028 100644 --- a/src/coreclr/vm/callhelpers.cpp +++ b/src/coreclr/vm/callhelpers.cpp @@ -225,11 +225,7 @@ void* DispatchCallSimple( callDescrData.pTarget = pTargetAddress; #ifdef FEATURE_PORTABLE_ENTRYPOINTS - callDescrData.pMD = PortableEntryPoint::GetMethodDesc(pTargetAddress); callDescrData.nArgsSize = numStackSlotsToCopy * sizeof(ARGHOLDER_TYPE); - - TransitionBlock block{}; - callDescrData.pTransitionBlock = █ #endif // FEATURE_PORTABLE_ENTRYPOINTS if ((dwDispatchCallSimpleFlags & DispatchCallSimple_CatchHandlerFoundNotification) != 0) @@ -523,9 +519,6 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT * CallDescrData callDescrData; callDescrData.pSrc = pTransitionBlock + sizeof(TransitionBlock); -#ifdef TARGET_WASM - callDescrData.pTransitionBlock = (TransitionBlock*)pTransitionBlock; -#endif _ASSERTE((nStackBytes % TARGET_POINTER_SIZE) == 0); callDescrData.numStackSlots = nStackBytes / TARGET_POINTER_SIZE; #ifdef CALLDESCR_ARGREGS @@ -543,7 +536,6 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT * callDescrData.fpReturnSize = fpReturnSize; callDescrData.pTarget = m_pCallTarget; #ifdef TARGET_WASM - callDescrData.pMD = m_pMD; callDescrData.nArgsSize = m_argIt.GetArgSize(); #endif diff --git a/src/coreclr/vm/callhelpers.h b/src/coreclr/vm/callhelpers.h index 91a249cbc7660b..0527dbe0d8076c 100644 --- a/src/coreclr/vm/callhelpers.h +++ b/src/coreclr/vm/callhelpers.h @@ -29,11 +29,8 @@ struct CallDescrData UINT32 fpReturnSize; PCODE pTarget; #ifdef TARGET_WASM - // method description is used to compile the method with the interpreter - MethodDesc* pMD; // size of the arguments and the transition block are used to execute the method with the interpreter size_t nArgsSize; - TransitionBlock* pTransitionBlock; #endif #ifdef CALLDESCR_RETBUFFARGREG diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index 3d4abbee5358d4..8485dc98a37d62 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -491,31 +491,31 @@ class ArgIteratorTemplate : public ARGITERATOR_BASE while (typ == ELEMENT_TYPE_VALUETYPE && pMT->GetNumInstanceFields() == 1 && (!pMT->HasLayout() || - pMT->GetNumInstanceFieldBytes() == 4 - )) // Don't do the optimization if we're getting specified anything but the trivial layout. - { - FieldDesc * pFD = pMT->GetApproxFieldDescListRaw(); + pMT->GetNumInstanceFieldBytes() == 4 + )) // Don't do the optimization if we're getting specified anything but the trivial layout. + { + FieldDesc * pFD = pMT->GetApproxFieldDescListRaw(); CorElementType type = pFD->GetFieldType(); bool exitLoop = false; - switch (type) + switch (type) { case ELEMENT_TYPE_VALUETYPE: { - //@todo: Is it more apropos to call LookupApproxFieldTypeHandle() here? - TypeHandle fldHnd = pFD->GetApproxFieldTypeHandleThrowing(); + //@todo: Is it more apropos to call LookupApproxFieldTypeHandle() here? + TypeHandle fldHnd = pFD->GetApproxFieldTypeHandleThrowing(); CONSISTENCY_CHECK(!fldHnd.IsNull()); pMT = fldHnd.GetMethodTable(); FALLTHROUGH; - } + } case ELEMENT_TYPE_PTR: case ELEMENT_TYPE_I: case ELEMENT_TYPE_U: - case ELEMENT_TYPE_I4: + case ELEMENT_TYPE_I4: case ELEMENT_TYPE_U4: - { + { typ = type; - break; + break; } default: exitLoop = true; @@ -853,7 +853,7 @@ class ArgIteratorTemplate : public ARGITERATOR_BASE pLoc->m_idxFloatReg = floatRegOfsInBytes / FLOAT_REGISTER_SIZE; pLoc->m_cFloatReg = 1; } - else + else #endif // UNIX_AMD64_ABI if (!TransitionBlock::IsStackArgumentOffset(argOffset)) { @@ -1874,11 +1874,14 @@ int ArgIteratorTemplate::GetNextOffset() return argOfs; #elif defined(TARGET_WASM) - int cbArg = ALIGN_UP(StackElemSize(argSize), TARGET_POINTER_SIZE); + // See duplicate definition in interpreter code. +#define INTERP_STACK_SLOT_SIZE 8 + int cbArg = ALIGN_UP(StackElemSize(argSize), INTERP_STACK_SLOT_SIZE); +#undef INTERP_STACK_SLOT_SIZE int argOfs = TransitionBlock::GetOffsetOfArgs() + m_ofsStack; - + m_ofsStack += cbArg; - + return argOfs; #else PORTABILITY_ASSERT("ArgIteratorTemplate::GetNextOffset"); diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index 5486ff01562bcc..a8342e19c09496 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -1902,122 +1902,4 @@ CallStubGenerator::ReturnType CallStubGenerator::GetReturnType(ArgIterator *pArg return ReturnTypeVoid; } -static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) -{ - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer(pMD)); - } - CONTRACTL_END - - GCX_PREEMP(); - - CallStubGenerator callStubGenerator; - - AllocMemTracker amTracker; - CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); - - if (pMD->SetCallStub(header)) - { - amTracker.SuppressRelease(); - } - else - { - // We have lost the race for generating the header, use the one that was generated by another thread - // and let the amTracker release the memory of the one we generated. - header = pMD->GetCallStub(); - } - - return header; -} - -void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) -{ - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer(pMD)); - PRECONDITION(CheckPointer(pArgs)); - PRECONDITION(CheckPointer(pRet)); - } - CONTRACTL_END - - CallStubHeader *pHeader = pMD->GetCallStub(); - if (pHeader == NULL) - { - pHeader = UpdateCallStubForMethod(pMD); - } - - // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. - pHeader->SetTarget(target); // The method to call - - pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); -} - -void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget) -{ - InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); -} - -void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) -{ - CONTRACTL - { - THROWS; - MODE_COOPERATIVE; - PRECONDITION(CheckPointer(pMDDelegateInvoke)); - PRECONDITION(CheckPointer(pArgs)); - PRECONDITION(CheckPointer(pRet)); - } - CONTRACTL_END - - CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); - if (stubHeaderTemplate == NULL) - { - stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke); - } - - // CallStubHeaders encode their destination addresses in the Routines array, so they need to be - // copied to a local buffer before we can actually set their target address. - size_t templateSize = stubHeaderTemplate->GetSize(); - uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); - memcpy(actualCallStub, stubHeaderTemplate, templateSize); - CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; - pHeader->SetTarget(target); // The method to call - pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); -} - -void InvokeCalliStub(PCODE ftn, void *cookie, int8_t *pArgs, int8_t *pRet) -{ - CONTRACTL - { - THROWS; - MODE_ANY; - PRECONDITION(CheckPointer((void*)ftn)); - PRECONDITION(CheckPointer(cookie)); - } - CONTRACTL_END - - // CallStubHeaders encode their destination addresses in the Routines array, so they need to be - // copied to a local buffer before we can actually set their target address. - CallStubHeader* stubHeaderTemplate = (CallStubHeader*)cookie; - size_t templateSize = stubHeaderTemplate->GetSize(); - uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); - memcpy(actualCallStub, stubHeaderTemplate, templateSize); - CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; - pHeader->SetTarget(ftn); // The method to call - pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); -} - -LPVOID GetCookieForCalliSig(MetaSig metaSig) -{ - STANDARD_VM_CONTRACT; - - CallStubGenerator callStubGenerator; - return callStubGenerator.GenerateCallStubForSig(metaSig); -} - #endif // FEATURE_INTERPRETER && !TARGET_WASM diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 6e6a815226d0d4..b03dfecaa84111 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -40,6 +40,124 @@ void InvokeUnmanagedMethodWithTransition(MethodDesc *targetMethod, int8_t *stack #ifndef TARGET_WASM #include "callstubgenerator.h" +static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMD)); + } + CONTRACTL_END + + GCX_PREEMP(); + + CallStubGenerator callStubGenerator; + + AllocMemTracker amTracker; + CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); + + if (pMD->SetCallStub(header)) + { + amTracker.SuppressRelease(); + } + else + { + // We have lost the race for generating the header, use the one that was generated by another thread + // and let the amTracker release the memory of the one we generated. + header = pMD->GetCallStub(); + } + + return header; +} + +void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMD)); + PRECONDITION(CheckPointer(pArgs)); + PRECONDITION(CheckPointer(pRet)); + } + CONTRACTL_END + + CallStubHeader *pHeader = pMD->GetCallStub(); + if (pHeader == NULL) + { + pHeader = UpdateCallStubForMethod(pMD); + } + + // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. + pHeader->SetTarget(target); // The method to call + + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + +void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *stack, InterpMethodContextFrame *pFrame, int32_t callArgsOffset, int32_t returnOffset, PCODE callTarget) +{ + InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); +} + +void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) +{ + CONTRACTL + { + THROWS; + MODE_COOPERATIVE; + PRECONDITION(CheckPointer(pMDDelegateInvoke)); + PRECONDITION(CheckPointer(pArgs)); + PRECONDITION(CheckPointer(pRet)); + } + CONTRACTL_END + + CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); + if (stubHeaderTemplate == NULL) + { + stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke); + } + + // CallStubHeaders encode their destination addresses in the Routines array, so they need to be + // copied to a local buffer before we can actually set their target address. + size_t templateSize = stubHeaderTemplate->GetSize(); + uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); + memcpy(actualCallStub, stubHeaderTemplate, templateSize); + CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; + pHeader->SetTarget(target); // The method to call + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + +void InvokeCalliStub(PCODE ftn, void *cookie, int8_t *pArgs, int8_t *pRet) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer((void*)ftn)); + PRECONDITION(CheckPointer(cookie)); + } + CONTRACTL_END + + // CallStubHeaders encode their destination addresses in the Routines array, so they need to be + // copied to a local buffer before we can actually set their target address. + CallStubHeader* stubHeaderTemplate = (CallStubHeader*)cookie; + size_t templateSize = stubHeaderTemplate->GetSize(); + uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); + memcpy(actualCallStub, stubHeaderTemplate, templateSize); + CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; + pHeader->SetTarget(ftn); // The method to call + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + +LPVOID GetCookieForCalliSig(MetaSig metaSig) +{ + STANDARD_VM_CONTRACT; + + CallStubGenerator callStubGenerator; + return callStubGenerator.GenerateCallStubForSig(metaSig); +} + // Create call stub for calling interpreted methods from JITted/AOTed code. CallStubHeader *CreateNativeToInterpreterCallStub(InterpMethod* pInterpMethod) { @@ -434,7 +552,7 @@ LONG IgnoreCppExceptionFilter(PEXCEPTION_POINTERS pExceptionInfo, PVOID pv) } // Wrapper around MethodDesc::DoPrestub to handle possible managed exceptions thrown by it. -static void CallPreStub(MethodDesc *pMD, CallerGCMode callerGCMode = CallerGCMode::Preemptive) +static void CallPreStub(MethodDesc *pMD) { STATIC_STANDARD_VM_CONTRACT; _ASSERTE(pMD != NULL); @@ -445,13 +563,12 @@ static void CallPreStub(MethodDesc *pMD, CallerGCMode callerGCMode = CallerGCMod struct Param { MethodDesc *pMethodDesc; - CallerGCMode callerGCMode; } - param = { pMD, callerGCMode }; + param = { pMD }; PAL_TRY(Param *, pParam, ¶m) { - (void)pParam->pMethodDesc->DoPrestub(NULL /* MethodTable */, pParam->callerGCMode); + (void)pParam->pMethodDesc->DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop); } PAL_EXCEPT_FILTER(IgnoreCppExceptionFilter) { diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 077a578e8f99bc..6f7dac49c044cd 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -38,7 +38,7 @@ class PortableEntryPoint final // Query methods for entry point state. bool IsValid() const; - bool IsByteCodeCompiled() const + bool HasInterpreterCode() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsValid()); @@ -52,7 +52,7 @@ class PortableEntryPoint final return _pActualCode != nullptr; } - bool IsReadyForR2R() const + bool IsPreparedForNativeCall() const { LIMITED_METHOD_CONTRACT; _ASSERTE(IsValid()); diff --git a/src/coreclr/vm/wasm/calldescrworkerwasm.cpp b/src/coreclr/vm/wasm/calldescrworkerwasm.cpp index 363acc1cdad425..c8e7e101c36080 100644 --- a/src/coreclr/vm/wasm/calldescrworkerwasm.cpp +++ b/src/coreclr/vm/wasm/calldescrworkerwasm.cpp @@ -6,9 +6,9 @@ extern "C" void* STDCALL ExecuteInterpretedMethodWithArgs(TransitionBlock* pTransitionBlock, TADDR byteCodeAddr, int8_t* pArgs, size_t size, void* retBuff); -extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData * pCallDescrData) +extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData* pCallDescrData) { - MethodDesc* pMethod = pCallDescrData->pMD; + MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(pCallDescrData->pTarget); InterpByteCodeStart* targetIp = pMethod->GetInterpreterCode(); if (targetIp == NULL) { @@ -17,5 +17,6 @@ extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData * pCallDescrData) targetIp = pMethod->GetInterpreterCode(); } - ExecuteInterpretedMethodWithArgs(pCallDescrData->pTransitionBlock, (TADDR)targetIp, (int8_t*)pCallDescrData->pSrc, pCallDescrData->nArgsSize, pCallDescrData->returnValue); + TransitionBlock dummy{}; + ExecuteInterpretedMethodWithArgs(&dummy, (TADDR)targetIp, (int8_t*)pCallDescrData->pSrc, pCallDescrData->nArgsSize, pCallDescrData->returnValue); } From ed8288890649751f102f185ba79442a040cfd613 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Sep 2025 11:26:48 -0700 Subject: [PATCH 10/25] Use Volatile. Update ifdef targets. --- src/coreclr/vm/callhelpers.cpp | 6 +++--- src/coreclr/vm/precode_portable.cpp | 6 +++--- src/coreclr/vm/precode_portable.hpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/callhelpers.cpp b/src/coreclr/vm/callhelpers.cpp index 79101882c7e028..da41b01f9c3310 100644 --- a/src/coreclr/vm/callhelpers.cpp +++ b/src/coreclr/vm/callhelpers.cpp @@ -224,9 +224,9 @@ void* DispatchCallSimple( callDescrData.fpReturnSize = 0; callDescrData.pTarget = pTargetAddress; -#ifdef FEATURE_PORTABLE_ENTRYPOINTS +#ifdef TARGET_WASM callDescrData.nArgsSize = numStackSlotsToCopy * sizeof(ARGHOLDER_TYPE); -#endif // FEATURE_PORTABLE_ENTRYPOINTS +#endif // TARGET_WASM if ((dwDispatchCallSimpleFlags & DispatchCallSimple_CatchHandlerFoundNotification) != 0) { @@ -537,7 +537,7 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT * callDescrData.pTarget = m_pCallTarget; #ifdef TARGET_WASM callDescrData.nArgsSize = m_argIt.GetArgSize(); -#endif +#endif // TARGET_WASM CallDescrWorkerWithHandler(&callDescrData); diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 6a46e241291353..d38302d415653e 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -40,7 +40,7 @@ void* PortableEntryPoint::GetActualCode(TADDR addr) } PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode != (BYTE*)NULL); + _ASSERTE(portableEntryPoint->_pActualCode != (void*)NULL); return portableEntryPoint->_pActualCode; } @@ -50,8 +50,8 @@ void PortableEntryPoint::SetActualCode(TADDR addr, void* actualCode) _ASSERTE(actualCode != NULL); PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode == (BYTE*)NULL || portableEntryPoint->_pActualCode == (BYTE*)actualCode); - portableEntryPoint->_pActualCode = (BYTE*)actualCode; + _ASSERTE(portableEntryPoint->_pActualCode == (void*)NULL || portableEntryPoint->_pActualCode == actualCode); + portableEntryPoint->_pActualCode = actualCode; } MethodDesc* PortableEntryPoint::GetMethodDesc(TADDR addr) diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 6f7dac49c044cd..3b56c2f0e005d2 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -25,7 +25,7 @@ class PortableEntryPoint final static PortableEntryPoint* ToPortableEntryPoint(TADDR addr); private: - VolatilePtr _pActualCode; + Volatile _pActualCode; MethodDesc* _pMD; void* _pInterpreterData; From e7755adab0a0fd5b037d33d102be70d3d5008b81 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Sep 2025 16:15:33 -0700 Subject: [PATCH 11/25] Use nullptr when possible. Mark static arrays as const. --- src/coreclr/vm/precode_portable.cpp | 4 ++-- src/coreclr/vm/wasm/helpers.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index d38302d415653e..94266ae14eb9b8 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -40,7 +40,7 @@ void* PortableEntryPoint::GetActualCode(TADDR addr) } PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode != (void*)NULL); + _ASSERTE(portableEntryPoint->_pActualCode != nullptr); return portableEntryPoint->_pActualCode; } @@ -50,7 +50,7 @@ void PortableEntryPoint::SetActualCode(TADDR addr, void* actualCode) _ASSERTE(actualCode != NULL); PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode == (void*)NULL || portableEntryPoint->_pActualCode == actualCode); + _ASSERTE(portableEntryPoint->_pActualCode == nullptr || portableEntryPoint->_pActualCode == actualCode); portableEntryPoint->_pActualCode = actualCode; } diff --git a/src/coreclr/vm/wasm/helpers.cpp b/src/coreclr/vm/wasm/helpers.cpp index c675dab549b9fd..1122842a0b97fe 100644 --- a/src/coreclr/vm/wasm/helpers.cpp +++ b/src/coreclr/vm/wasm/helpers.cpp @@ -559,7 +559,7 @@ namespace #undef ARG - void* RetVoidThunks[] = + void* const RetVoidThunks[] = { (void*)&CallFunc_Void_RetVoid, (void*)&CallFunc_I32_RetVoid, @@ -567,7 +567,7 @@ namespace (void*)&CallFunc_I32_I32_I32_RetVoid, }; - void* RetI32Thunks[] = + void* const RetI32Thunks[] = { (void*)&CallFunc_Void_RetI32, (void*)&CallFunc_I32_RetI32, @@ -640,7 +640,7 @@ namespace } UINT numArgs = sig.NumFixedArgs(); - void** thunks; + void* const * thunks; if (returnsVoid) { thunks = RetVoidThunks; From 03cf9087af83d41ece6edbdcde773f418e762358 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Sep 2025 17:10:20 -0700 Subject: [PATCH 12/25] Protect against null thread. Proactively record locals to avoid them being lost. --- src/coreclr/vm/interpexec.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index b03dfecaa84111..5511532e5d76f5 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -197,6 +197,8 @@ CallStubHeader *CreateNativeToInterpreterCallStub(InterpMethod* pInterpMethod) void DBG_PrintInterpreterStack() { Thread* pThread = GetThread(); + if (pThread == NULL) + return; int32_t frameCount = 0; @@ -1926,7 +1928,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr case INTOP_CALL_HELPER_P_PS: { - void* helperArg = pMethod->pDataItems[ip[4]]; + void* helperArg1 = pMethod->pDataItems[ip[4]]; + void* helperArg2 = LOCAL_VAR(ip[2], void*); MethodDesc *pILTargetMethod = NULL; HELPER_FTN_P_PP helperFtn = GetPossiblyIndirectHelper(pMethod, ip[3], &pILTargetMethod); @@ -1937,8 +1940,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr callArgsOffset = stackOffset; // Pass arguments to the target method - LOCAL_VAR(stackOffset, void*) = helperArg; - LOCAL_VAR(stackOffset + INTERP_STACK_SLOT_SIZE, void*) = LOCAL_VAR(ip[2], void*); + LOCAL_VAR(stackOffset, void*) = helperArg1; + LOCAL_VAR(stackOffset + INTERP_STACK_SLOT_SIZE, void*) = helperArg2; targetMethod = pILTargetMethod; ip += 5; @@ -1946,7 +1949,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr } _ASSERTE(helperFtn != NULL); - LOCAL_VAR(ip[1], void*) = helperFtn(helperArg, LOCAL_VAR(ip[2], void*)); + LOCAL_VAR(ip[1], void*) = helperFtn(helperArg1, helperArg2); ip += 5; break; } @@ -1974,6 +1977,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr int stackOffset = pMethod->allocaSize; callArgsOffset = stackOffset; + // Pass argument to the target method LOCAL_VAR(stackOffset, void*) = helperArg; targetMethod = pILTargetMethod; @@ -1990,7 +1994,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr case INTOP_CALL_HELPER_P_GS: { InterpGenericLookup *pLookup = (InterpGenericLookup*)&pMethod->pDataItems[ip[5]]; - void* helperArg = DoGenericLookup(LOCAL_VAR(ip[2], void*), pLookup); + void* helperArg1 = DoGenericLookup(LOCAL_VAR(ip[2], void*), pLookup); + void* helperArg2 = LOCAL_VAR(ip[3], void*); MethodDesc *pILTargetMethod = NULL; HELPER_FTN_P_PP helperFtn = GetPossiblyIndirectHelper(pMethod, ip[4], &pILTargetMethod); @@ -2001,8 +2006,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr callArgsOffset = stackOffset; // Pass arguments to the target method - LOCAL_VAR(stackOffset, void*) = helperArg; - LOCAL_VAR(stackOffset + INTERP_STACK_SLOT_SIZE, void*) = LOCAL_VAR(ip[3], void*); + LOCAL_VAR(stackOffset, void*) = helperArg1; + LOCAL_VAR(stackOffset + INTERP_STACK_SLOT_SIZE, void*) = helperArg2; targetMethod = pILTargetMethod; ip += 6; @@ -2010,7 +2015,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr } _ASSERTE(helperFtn != NULL); - LOCAL_VAR(ip[1], void*) = helperFtn(helperArg, LOCAL_VAR(ip[3], void*)); + LOCAL_VAR(ip[1], void*) = helperFtn(helperArg1, helperArg2); ip += 6; break; } From a86e411f45c26e2c87f7fd858b59027672d79c6c Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Sep 2025 18:15:00 -0700 Subject: [PATCH 13/25] Feedback on alignment define values. --- src/coreclr/vm/callingconvention.h | 3 --- src/coreclr/vm/wasm/cgencpu.h | 3 ++- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index 8485dc98a37d62..5f52cd27e449f8 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -1874,10 +1874,7 @@ int ArgIteratorTemplate::GetNextOffset() return argOfs; #elif defined(TARGET_WASM) - // See duplicate definition in interpreter code. -#define INTERP_STACK_SLOT_SIZE 8 int cbArg = ALIGN_UP(StackElemSize(argSize), INTERP_STACK_SLOT_SIZE); -#undef INTERP_STACK_SLOT_SIZE int argOfs = TransitionBlock::GetOffsetOfArgs() + m_ofsStack; m_ofsStack += cbArg; diff --git a/src/coreclr/vm/wasm/cgencpu.h b/src/coreclr/vm/wasm/cgencpu.h index 5bb9a5479c0f02..cab2c8d617082c 100644 --- a/src/coreclr/vm/wasm/cgencpu.h +++ b/src/coreclr/vm/wasm/cgencpu.h @@ -7,6 +7,7 @@ #include "stublink.h" #include "utilcode.h" +#include "../../interpreter/interpretershared.h" // preferred alignment for data #define DATA_ALIGNMENT 4 @@ -22,7 +23,7 @@ inline unsigned StackElemSize(unsigned parmSize, bool isValueType = false /* unused */, bool isFloatHfa = false /* unused */) { - const unsigned stackSlotSize = sizeof(void*); + const unsigned stackSlotSize = INTERP_STACK_SLOT_SIZE; return ALIGN_UP(parmSize, stackSlotSize); } From 6a7858fb814c58f2b762edaa854fa369b7f5ccc6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Sep 2025 18:36:34 -0700 Subject: [PATCH 14/25] Fix up assert for marshalling checks on stack slot size. --- src/coreclr/vm/mlinfo.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/coreclr/vm/mlinfo.cpp b/src/coreclr/vm/mlinfo.cpp index fd695c46741bc1..db9bcabae46019 100644 --- a/src/coreclr/vm/mlinfo.cpp +++ b/src/coreclr/vm/mlinfo.cpp @@ -2544,7 +2544,13 @@ void MarshalInfo::SetupArgumentSizes() const unsigned targetPointerSize = TARGET_POINTER_SIZE; const bool pointerIsValueType = false; const bool pointerIsFloatHfa = false; +#ifdef TARGET_WASM + // Wasm uses the interpreter which uses a pointer agnostic stack slot size. + // Therefore, we only need assert it is sufficiently large. + _ASSERTE(targetPointerSize <= StackElemSize(TARGET_POINTER_SIZE, pointerIsValueType, pointerIsFloatHfa)); +#else _ASSERTE(targetPointerSize == StackElemSize(TARGET_POINTER_SIZE, pointerIsValueType, pointerIsFloatHfa)); +#endif // TARGET_WASM if (m_byref) { From 9f1e3767e3716ba1a6396191f6a3bd88540dd57c Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 10:22:59 -0700 Subject: [PATCH 15/25] Remove bit marking for native implemented helpers. Add PCODE storage array for allocated helper wrappers. --- src/coreclr/debug/daccess/daccess.cpp | 6 +- src/coreclr/debug/ee/controller.cpp | 22 ++--- src/coreclr/vm/interpexec.cpp | 1 + src/coreclr/vm/jithelpers.cpp | 51 +++++++++--- src/coreclr/vm/jitinterface.cpp | 114 +++++++++++++------------- src/coreclr/vm/jitinterface.h | 22 +++-- src/coreclr/vm/precode_portable.cpp | 56 +++++-------- src/coreclr/vm/precode_portable.hpp | 9 +- 8 files changed, 157 insertions(+), 124 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index da319aed42c5b1..801f0fcaf82552 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -5635,7 +5635,7 @@ ClrDataAccess::GetJitHelperName( for (int i = 0; i < CORINFO_HELP_COUNT; i++) { - if (address == (TADDR)(pTable[i].pfnHelper)) + if (address == pTable[i].pfnHelper) return s_rgHelperNames[i]; } } @@ -5652,7 +5652,7 @@ ClrDataAccess::GetJitHelperName( PTR_READ(dac_cast(&hlpDynamicFuncTable), DYNAMIC_CORINFO_HELP_COUNT * sizeof(VMHELPDEF))); for (unsigned d = 0; d < DYNAMIC_CORINFO_HELP_COUNT; d++) { - if (address == (TADDR)(pDynamicTable[d].pfnHelper)) + if (address == pDynamicTable[d].pfnHelper) { return s_rgHelperNames[s_rgDynamicHCallIds[d]]; } @@ -5991,7 +5991,7 @@ ClrDataAccess::GetMethodVarInfo(MethodDesc* methodDesc, BOOL success = DebugInfoManager::GetBoundariesAndVars( request, DebugInfoStoreNew, NULL, // allocator - BoundsType::Instrumented, + BoundsType::Instrumented, NULL, NULL, &countNativeVarInfo, &nativeVars); diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 794f0a693bfeb8..eb7ea2f3205fcd 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -210,13 +210,13 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu // BYTE* bufferBypassRW = pSharedPatchBypassBufferRW->BypassBuffer; - + // Overwrite the *signed* displacement. int dwOldDisp = *(int*)(&patchBypassRX[instrAttrib.m_dwOffsetToDisp]); int dwNewDisp = offsetof(SharedPatchBypassBuffer, BypassBuffer) - (offsetof(SharedPatchBypassBuffer, PatchBypass) + instrAttrib.m_cbInstr); *(int*)(&patchBypassRW[instrAttrib.m_dwOffsetToDisp]) = dwNewDisp; - + // This could be an LEA, which we'll just have to change into a MOV and copy the original address. if (((patchBypassRX[0] == 0x4C) || (patchBypassRX[0] == 0x48)) && (patchBypassRX[1] == 0x8d)) { @@ -229,7 +229,7 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu _ASSERTE(instrAttrib.m_cOperandSize <= SharedPatchBypassBuffer::cbBufferBypass); // Copy the data into our buffer. memcpy(bufferBypassRW, this->address + instrAttrib.m_cbInstr + dwOldDisp, instrAttrib.m_cOperandSize); - + if (instrAttrib.m_fIsWrite) { // save the actual destination address and size so when we TriggerSingleStep() we can update the value @@ -238,17 +238,17 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu } } } - + #endif // TARGET_AMD64 m_pSharedPatchBypassBuffer->SetInstructionAttrib(instrAttrib); - + // Since we just created a new buffer of code, but the CPU caches code and may // not be aware of our changes. This should force the CPU to dump any cached // instructions it has in this region and load the new ones from memory FlushInstructionCache(GetCurrentProcess(), patchBypassRW + CORDbg_BREAK_INSTRUCTION_SIZE, MAX_INSTRUCTION_LENGTH - CORDbg_BREAK_INSTRUCTION_SIZE); - + return m_pSharedPatchBypassBuffer; } #endif // !FEATURE_EMULATE_SINGLESTEP @@ -2405,10 +2405,10 @@ static bool _AddrIsJITHelper(PCODE addr) { for (int i = 0; i < CORINFO_HELP_COUNT; i++) { - if (hlpFuncTable[i].pfnHelper == (void*)addr) + if (hlpFuncTable[i].pfnHelper == addr) { LOG((LF_CORDB, LL_INFO10000, - "_ANIM: address of helper function found: 0x%08x\n", + "_ANIM: address of helper function found: 0x%zx\n", addr)); return true; } @@ -2416,10 +2416,10 @@ static bool _AddrIsJITHelper(PCODE addr) for (unsigned d = 0; d < DYNAMIC_CORINFO_HELP_COUNT; d++) { - if (hlpDynamicFuncTable[d].pfnHelper == (void*)addr) + if (hlpDynamicFuncTable[d].pfnHelper == addr) { LOG((LF_CORDB, LL_INFO10000, - "_ANIM: address of helper function found: 0x%08x\n", + "_ANIM: address of helper function found: 0x%zx\n", addr)); return true; } @@ -2427,7 +2427,7 @@ static bool _AddrIsJITHelper(PCODE addr) LOG((LF_CORDB, LL_INFO10000, "_ANIM: address within runtime dll, but not a helper function " - "0x%08x\n", addr)); + "0x%zx\n", addr)); } #else // !defined(HOST_64BIT) && !defined(TARGET_UNIX) // TODO: Figure out what we want to do here diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 5511532e5d76f5..a0adac515dc91c 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -31,6 +31,7 @@ void InvokeUnmanagedMethodWithTransition(MethodDesc *targetMethod, int8_t *stack { GCX_PREEMP(); + // WASM-TODO: Handle unmanaged calling conventions InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); } diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index b93e15ad331e78..5cbb27dd272d32 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2515,24 +2515,31 @@ enum __CorInfoHelpFunc { #include "jithelpers.h" #ifdef _DEBUG -#define HELPERDEF(code, lpv, sig) { (LPVOID)(lpv), #code }, +#define HELPERDEF(code, lpv, isDynamicHelper) { (TADDR)(lpv), #code, isDynamicHelper }, +#elif defined(TARGET_WASM) +#define HELPERDEF(code, lpv, isDynamicHelper) { (TADDR)(lpv), isDynamicHelper }, #else // !_DEBUG -#define HELPERDEF(code, lpv, sig) { (LPVOID)(lpv) }, +#define HELPERDEF(code, lpv, isDynamicHelper) { (TADDR)(lpv) }, #endif // !_DEBUG // static helpers - constant array const VMHELPDEF hlpFuncTable[CORINFO_HELP_COUNT] = { -#define JITHELPER(code, pfnHelper, binderId) HELPERDEF(code, pfnHelper, binderId) -#define DYNAMICJITHELPER(code, pfnHelper, binderId) HELPERDEF(code, 1 + DYNAMIC_##code, binderId) +#define JITHELPER(code, pfnHelper, binderId) HELPERDEF(code, pfnHelper, false) +#define DYNAMICJITHELPER(code, pfnHelper, binderId) HELPERDEF(code, 1 + DYNAMIC_##code, true) #include "jithelpers.h" }; +#ifdef FEATURE_PORTABLE_ENTRYPOINTS +// Collection of entry points for JIT helpers +TADDR hlpFuncEntryPoints[CORINFO_HELP_COUNT] = {}; +#endif // FEATURE_PORTABLE_ENTRYPOINTS + // dynamic helpers - filled in at runtime - See definition of DynamicCorInfoHelpFunc. VMHELPDEF hlpDynamicFuncTable[DYNAMIC_CORINFO_HELP_COUNT] = { #define JITHELPER(code, pfnHelper, binderId) -#define DYNAMICJITHELPER(code, pfnHelper, binderId) HELPERDEF(DYNAMIC_ ## code, pfnHelper, binderId) +#define DYNAMICJITHELPER(code, pfnHelper, binderId) HELPERDEF(DYNAMIC_##code, pfnHelper, true) #include "jithelpers.h" }; @@ -2544,6 +2551,30 @@ static const BinderMethodID hlpDynamicToBinderMap[DYNAMIC_CORINFO_HELP_COUNT] = #include "jithelpers.h" }; +bool VMHELPDEF::IsDynamicHelper(size_t& dynamicFtnNum) const +{ + LIMITED_METHOD_CONTRACT; + + bool isDynamic; + dynamicFtnNum = (size_t)(pfnHelper - 1); + +#ifdef TARGET_WASM + // Functions on Wasm are ordinal values, not memory addresses. + // On Wasm, we need some metadata to indicate whether the helper is a dynamic helper. + isDynamic = isDynamicHelper; +#else // !TARGET_WASM + // If pfnHelper is an index into the dynamic helper table, it should be less + // than DYNAMIC_CORINFO_HELP_COUNT. + isDynamic = (dynamicFtnNum < DYNAMIC_CORINFO_HELP_COUNT); +#endif // TARGET_WASM + +#if defined(_DEBUG) || defined(TARGET_WASM) + _ASSERTE(isDynamic == isDynamicHelper); +#endif // _DEBUG || TARGET_WASM + + return isDynamic; +} + // Set the JIT helper function in the helper table // Handles the case where the function does not reside in mscorwks.dll @@ -2561,17 +2592,17 @@ void _SetJitHelperFunction(DynamicCorInfoHelpFunc ftnNum, void * pFunc) LOG((LF_JIT, LL_INFO1000000, "Setting JIT dynamic helper %3d (%s) to %p\n", ftnNum, hlpDynamicFuncTable[ftnNum].name, pFunc)); - hlpDynamicFuncTable[ftnNum].pfnHelper = (void*)pFunc; + hlpDynamicFuncTable[ftnNum].pfnHelper = (TADDR)pFunc; } -VMHELPDEF LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc) +TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc) { STANDARD_VM_CONTRACT; _ASSERTE(ftnNum < DYNAMIC_CORINFO_HELP_COUNT); MethodDesc* pMD = NULL; - void* helper = VolatileLoad(&hlpDynamicFuncTable[ftnNum].pfnHelper); + TADDR helper = VolatileLoad(&hlpDynamicFuncTable[ftnNum].pfnHelper); if (helper == NULL) { BinderMethodID binderId = hlpDynamicToBinderMap[ftnNum]; @@ -2584,7 +2615,7 @@ VMHELPDEF LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** metho pMD = CoreLibBinder::GetMethod(binderId); PCODE pFunc = pMD->GetMultiCallableAddrOfCode(); - InterlockedCompareExchangeT(&hlpDynamicFuncTable[ftnNum].pfnHelper, (void*)pFunc, nullptr); + InterlockedCompareExchangeT(&hlpDynamicFuncTable[ftnNum].pfnHelper, (TADDR)pFunc, NULL); } // If the caller wants the MethodDesc, we may need to try and load it. @@ -2600,7 +2631,7 @@ VMHELPDEF LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** metho *methodDesc = pMD; } - return hlpDynamicFuncTable[ftnNum]; + return hlpDynamicFuncTable[ftnNum].pfnHelper; } bool HasILBasedDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 90334cf3eb2ebe..2c0005a555fe59 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10751,25 +10751,28 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN CORINFO_CONST_LOOKUP* pNativeEntrypoint, /* OUT */ CORINFO_METHOD_HANDLE* pMethod) /* OUT */ { - CONTRACTL { + CONTRACTL + { THROWS; GC_TRIGGERS; MODE_PREEMPTIVE; - } CONTRACTL_END; + } + CONTRACTL_END; JIT_TO_EE_TRANSITION(); - if (pMethod != NULL) - { - *pMethod = NULL; - } - _ASSERTE(ftnNum < CORINFO_HELP_COUNT); - void* pfnHelper = hlpFuncTable[ftnNum].pfnHelper; + InfoAccessType accessType = IAT_PVALUE; + LPVOID targetAddr = nullptr; - size_t dynamicFtnNum = ((size_t)pfnHelper - 1); - if (dynamicFtnNum < DYNAMIC_CORINFO_HELP_COUNT) + MethodDesc* helperMD = NULL; + VMHELPDEF const& helperDef = hlpFuncTable[ftnNum]; + TADDR pfnHelper = helperDef.pfnHelper; + + size_t dynamicFtnNum; + bool isDynamicMethod = helperDef.IsDynamicHelper(dynamicFtnNum); + if (isDynamicMethod) { #if defined(TARGET_AMD64) // To avoid using a jump stub we always call certain helpers using an indirect call. @@ -10787,46 +10790,34 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN dynamicFtnNum == DYNAMIC_CORINFO_HELP_PROF_FCN_TAILCALL || dynamicFtnNum == DYNAMIC_CORINFO_HELP_DISPATCH_INDIRECT_CALL) { - if (pNativeEntrypoint != NULL) - { - pNativeEntrypoint->accessType = IAT_PVALUE; - _ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != NULL); // Confirm the helper is non-null and doesn't require lazy loading. - pNativeEntrypoint->addr = &hlpDynamicFuncTable[dynamicFtnNum].pfnHelper; - _ASSERTE(IndirectionAllowedForJitHelper(ftnNum)); - } + accessType = IAT_PVALUE; + _ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != NULL); // Confirm the helper is non-null and doesn't require lazy loading. + targetAddr = &hlpDynamicFuncTable[dynamicFtnNum].pfnHelper; + _ASSERTE(IndirectionAllowedForJitHelper(ftnNum)); goto exit; } -#endif +#endif // TARGET_AMD64 // Check if we already have a cached address of the final target static LPVOID hlpFinalTierAddrTable[DYNAMIC_CORINFO_HELP_COUNT] = {}; LPVOID finalTierAddr = hlpFinalTierAddrTable[dynamicFtnNum]; if (finalTierAddr != NULL) { - if (pNativeEntrypoint != NULL) - { - pNativeEntrypoint->accessType = IAT_VALUE; - pNativeEntrypoint->addr = finalTierAddr; - } + accessType = IAT_VALUE; + targetAddr = finalTierAddr; if (pMethod != nullptr && HasILBasedDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum)) { - (void)LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum, (MethodDesc**)pMethod); - _ASSERT(*pMethod != NULL); + (void)LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum, &helperMD); + _ASSERT(helperMD != NULL); } goto exit; } if (HasILBasedDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum)) { - MethodDesc* helperMD = NULL; (void)LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum, &helperMD); _ASSERT(helperMD != NULL); - if (pMethod != NULL) - { - *pMethod = (CORINFO_METHOD_HANDLE)helperMD; - } - // Check if the target MethodDesc is already jitted to its final Tier // so we no longer need to use indirections and can emit a direct call instead. // @@ -10855,11 +10846,8 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN { // Cache it for future uses to avoid taking the lock again. hlpFinalTierAddrTable[dynamicFtnNum] = finalTierAddr; - if (pNativeEntrypoint != NULL) - { - pNativeEntrypoint->accessType = IAT_VALUE; - pNativeEntrypoint->addr = finalTierAddr; - } + accessType = IAT_VALUE; + targetAddr = finalTierAddr; goto exit; } } @@ -10867,8 +10855,6 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN if (IndirectionAllowedForJitHelper(ftnNum)) { - InfoAccessType accessType; - LPVOID targetAddr; #ifdef FEATURE_PORTABLE_ENTRYPOINTS accessType = IAT_VALUE; targetAddr = (LPVOID)helperMD->GetPortableEntryPoint(); @@ -10878,34 +10864,50 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN accessType = IAT_PVALUE; targetAddr = ((FixupPrecode*)pPrecode)->GetTargetSlot(); #endif // FEATURE_PORTABLE_ENTRYPOINTS - - if (pNativeEntrypoint != NULL) - { - pNativeEntrypoint->accessType = accessType; - pNativeEntrypoint->addr = targetAddr; - } goto exit; } } - pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum).pfnHelper; + pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum); } + // + // Static helpers + // + _ASSERTE(pfnHelper != NULL); - if (pNativeEntrypoint != NULL) + accessType = IAT_VALUE; + +#ifdef FEATURE_PORTABLE_ENTRYPOINTS + targetAddr = (LPVOID)hlpFuncEntryPoints[ftnNum]; + if (targetAddr == NULL) +#endif // FEATURE_PORTABLE_ENTRYPOINTS { - pNativeEntrypoint->accessType = IAT_VALUE; TADDR entryPoint = GetEEFuncEntryPoint(pfnHelper); #ifdef FEATURE_PORTABLE_ENTRYPOINTS - entryPoint = PortableEntryPoint::MarkNativeEntryPoint(entryPoint); + NewHolder portableEntryPoint = new PortableEntryPoint(); + portableEntryPoint->Init((void*)entryPoint); + if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (TADDR)(PortableEntryPoint*)portableEntryPoint, NULL) == NULL) + portableEntryPoint.SuppressRelease(); + + entryPoint = hlpFuncEntryPoints[ftnNum]; #endif // FEATURE_PORTABLE_ENTRYPOINTS - pNativeEntrypoint->addr = (LPVOID)entryPoint; + targetAddr = (LPVOID)entryPoint; } exit: ; + if (pNativeEntrypoint != NULL) + { + pNativeEntrypoint->accessType = accessType; + pNativeEntrypoint->addr = targetAddr; + } + + if (pMethod != NULL) + *pMethod = (CORINFO_METHOD_HANDLE)helperMD; + EE_TO_JIT_TRANSITION(); } @@ -10917,15 +10919,15 @@ PCODE CEECodeGenInfo::getHelperFtnStatic(CorInfoHelpFunc ftnNum) MODE_PREEMPTIVE; } CONTRACTL_END; - void* pfnHelper = hlpFuncTable[ftnNum].pfnHelper; + VMHELPDEF const& helperDef = hlpFuncTable[ftnNum]; + TADDR pfnHelper = helperDef.pfnHelper; - // If pfnHelper is an index into the dynamic helper table, it should be less - // than DYNAMIC_CORINFO_HELP_COUNT. In this case we need to find the actual pfnHelper - // using an extra indirection. Note the special case - // where pfnHelper==0 where pfnHelper-1 will underflow and we will avoid the indirection. - if (((size_t)pfnHelper - 1) < DYNAMIC_CORINFO_HELP_COUNT) + // In this case we need to find the actual pfnHelper + // using an extra indirection. + size_t dynamicFtnNum; + if (helperDef.IsDynamicHelper(dynamicFtnNum)) { - pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)((size_t)pfnHelper - 1)).pfnHelper; + pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum); } _ASSERTE(pfnHelper != NULL); diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 8cc6c6773ed43b..5e34dc9b172b02 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -973,12 +973,20 @@ class CInterpreterJitInfo final : public CEECodeGenInfo /*********************************************************************/ /*********************************************************************/ -typedef struct { - void * pfnHelper; +struct VMHELPDEF +{ + TADDR pfnHelper; + #ifdef _DEBUG const char* name; -#endif -} VMHELPDEF; +#endif // _DEBUG + +#if defined(_DEBUG) || defined(TARGET_WASM) + bool isDynamicHelper; +#endif // _DEBUG || TARGET_WASM + + bool IsDynamicHelper(size_t& dynamicFtnNum) const; +}; #if defined(DACCESS_COMPILE) @@ -988,6 +996,10 @@ GARY_DECL(VMHELPDEF, hlpFuncTable, CORINFO_HELP_COUNT); extern "C" const VMHELPDEF hlpFuncTable[CORINFO_HELP_COUNT]; +#ifdef FEATURE_PORTABLE_ENTRYPOINTS +extern "C" TADDR hlpFuncEntryPoints[CORINFO_HELP_COUNT]; +#endif // FEATURE_PORTABLE_ENTRYPOINTS + #endif // enum for dynamically assigned helper calls @@ -1007,7 +1019,7 @@ GARY_DECL(VMHELPDEF, hlpDynamicFuncTable, DYNAMIC_CORINFO_HELP_COUNT); #define SetJitHelperFunction(ftnNum, pFunc) _SetJitHelperFunction(DYNAMIC_##ftnNum, (void*)(pFunc)) void _SetJitHelperFunction(DynamicCorInfoHelpFunc ftnNum, void * pFunc); -VMHELPDEF LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc = NULL); +TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc = NULL); bool HasILBasedDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum); bool IndirectionAllowedForJitHelper(CorInfoHelpFunc ftnNum); diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 94266ae14eb9b8..178abde6f2b088 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -7,59 +7,34 @@ #include "common.h" #include "precode_portable.hpp" -// WASM uses function indices so we should avoid using -// low order bits for masking. #ifdef HOST_64BIT #define CANARY_VALUE 0x1234567812345678 -#define NATIVE_ENTRYPOINT_HELPER_BIT 0x8000000000000000 #else // HOST_64BIT #define CANARY_VALUE 0x12345678 -#define NATIVE_ENTRYPOINT_HELPER_BIT 0x80000000 #endif // HOST_64BIT bool PortableEntryPoint::IsNativeEntryPoint(TADDR addr) { LIMITED_METHOD_CONTRACT; - return (addr & NATIVE_ENTRYPOINT_HELPER_BIT) == NATIVE_ENTRYPOINT_HELPER_BIT; -} - -TADDR PortableEntryPoint::MarkNativeEntryPoint(TADDR entryPoint) -{ - LIMITED_METHOD_CONTRACT; - return entryPoint | NATIVE_ENTRYPOINT_HELPER_BIT; + PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); + return portableEntryPoint->HasNativeCode(); } void* PortableEntryPoint::GetActualCode(TADDR addr) { STANDARD_VM_CONTRACT; - if (IsNativeEntryPoint(addr)) - { - const TADDR mask = ~NATIVE_ENTRYPOINT_HELPER_BIT; - return (void*)(addr & mask); - } - PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode != nullptr); + _ASSERTE(portableEntryPoint->HasNativeCode()); return portableEntryPoint->_pActualCode; } -void PortableEntryPoint::SetActualCode(TADDR addr, void* actualCode) -{ - STANDARD_VM_CONTRACT; - _ASSERTE(actualCode != NULL); - - PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pActualCode == nullptr || portableEntryPoint->_pActualCode == actualCode); - portableEntryPoint->_pActualCode = actualCode; -} - MethodDesc* PortableEntryPoint::GetMethodDesc(TADDR addr) { STANDARD_VM_CONTRACT; PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pMD != NULL); + _ASSERTE(portableEntryPoint->_pMD != nullptr); return portableEntryPoint->_pMD; } @@ -68,7 +43,7 @@ void* PortableEntryPoint::GetInterpreterData(TADDR addr) STANDARD_VM_CONTRACT; PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pInterpreterData != NULL); + _ASSERTE(portableEntryPoint->HasInterpreterCode()); return portableEntryPoint->_pInterpreterData; } @@ -77,7 +52,7 @@ void PortableEntryPoint::SetInterpreterData(TADDR addr, PCODE interpreterData) STANDARD_VM_CONTRACT; PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); - _ASSERTE(portableEntryPoint->_pInterpreterData == NULL); + _ASSERTE(!portableEntryPoint->HasInterpreterCode()); _ASSERTE(interpreterData != (PCODE)NULL); portableEntryPoint->_pInterpreterData = (void*)PCODEToPINSTR(interpreterData); } @@ -86,27 +61,38 @@ PortableEntryPoint* PortableEntryPoint::ToPortableEntryPoint(TADDR addr) { LIMITED_METHOD_CONTRACT; _ASSERTE(addr != NULL); - _ASSERTE(!IsNativeEntryPoint(addr)); PortableEntryPoint* portableEntryPoint = (PortableEntryPoint*)addr; _ASSERTE(portableEntryPoint->IsValid()); return portableEntryPoint; } +#ifdef _DEBUG +bool PortableEntryPoint::IsValid() const +{ + LIMITED_METHOD_CONTRACT; + return _canary == CANARY_VALUE; +} +#endif // _DEBUG + void PortableEntryPoint::Init(MethodDesc* pMD) { LIMITED_METHOD_CONTRACT; + _ASSERTE(pMD != NULL); _pActualCode = NULL; _pMD = pMD; _pInterpreterData = NULL; INDEBUG(_canary = CANARY_VALUE); } -bool PortableEntryPoint::IsValid() const +void PortableEntryPoint::Init(void* nativeEntryPoint) { LIMITED_METHOD_CONTRACT; - _ASSERTE(_canary == CANARY_VALUE); - return _pMD != nullptr; + _ASSERTE(nativeEntryPoint != NULL); + _pActualCode = nativeEntryPoint; + _pMD = NULL; + _pInterpreterData = NULL; + INDEBUG(_canary = CANARY_VALUE); } InterleavedLoaderHeapConfig s_stubPrecodeHeapConfig; diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 3b56c2f0e005d2..2df0f5d6c82803 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -13,10 +13,8 @@ class PortableEntryPoint final { public: // static static bool IsNativeEntryPoint(TADDR addr); - static TADDR MarkNativeEntryPoint(TADDR entryPoint); static void* GetActualCode(TADDR addr); - static void SetActualCode(TADDR addr, void* actualCode); static MethodDesc* GetMethodDesc(TADDR addr); static void* GetInterpreterData(TADDR addr); static void SetInterpreterData(TADDR addr, PCODE interpreterData); @@ -32,12 +30,15 @@ class PortableEntryPoint final // We keep the canary value last to ensure a stable ABI across build flavors INDEBUG(size_t _canary); +#ifdef _DEBUG + bool IsValid() const; +#endif // _DEBUG + public: void Init(MethodDesc* pMD); + void Init(void* nativeEntryPoint); // Query methods for entry point state. - bool IsValid() const; - bool HasInterpreterCode() const { LIMITED_METHOD_CONTRACT; From fc8660edd717c2e4e592f150071329e0080bed99 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 11:24:05 -0700 Subject: [PATCH 16/25] Add casts to avoid build breaks. --- src/coreclr/vm/jithelpers.cpp | 4 ++-- src/coreclr/vm/jitinterface.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 5cbb27dd272d32..9f41764f125234 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2603,7 +2603,7 @@ TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDes MethodDesc* pMD = NULL; TADDR helper = VolatileLoad(&hlpDynamicFuncTable[ftnNum].pfnHelper); - if (helper == NULL) + if (helper == (TADDR)NULL) { BinderMethodID binderId = hlpDynamicToBinderMap[ftnNum]; @@ -2615,7 +2615,7 @@ TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDes pMD = CoreLibBinder::GetMethod(binderId); PCODE pFunc = pMD->GetMultiCallableAddrOfCode(); - InterlockedCompareExchangeT(&hlpDynamicFuncTable[ftnNum].pfnHelper, (TADDR)pFunc, NULL); + InterlockedCompareExchangeT(&hlpDynamicFuncTable[ftnNum].pfnHelper, (TADDR)pFunc, (TADDR)NULL); } // If the caller wants the MethodDesc, we may need to try and load it. diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2c0005a555fe59..60a00c87c88e3c 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10889,7 +10889,7 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN #ifdef FEATURE_PORTABLE_ENTRYPOINTS NewHolder portableEntryPoint = new PortableEntryPoint(); portableEntryPoint->Init((void*)entryPoint); - if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (TADDR)(PortableEntryPoint*)portableEntryPoint, NULL) == NULL) + if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (TADDR)(PortableEntryPoint*)portableEntryPoint, (TADDR)NULL) == (TADDR)NULL) portableEntryPoint.SuppressRelease(); entryPoint = hlpFuncEntryPoints[ftnNum]; From 00e8b2603e1daa6c55fba991285612ed9a89e5d0 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 11:52:15 -0700 Subject: [PATCH 17/25] Add TADDR cast. --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 60a00c87c88e3c..1ddc22dfbf1e7b 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10791,7 +10791,7 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN dynamicFtnNum == DYNAMIC_CORINFO_HELP_DISPATCH_INDIRECT_CALL) { accessType = IAT_PVALUE; - _ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != NULL); // Confirm the helper is non-null and doesn't require lazy loading. + _ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != (TADDR)NULL); // Confirm the helper is non-null and doesn't require lazy loading. targetAddr = &hlpDynamicFuncTable[dynamicFtnNum].pfnHelper; _ASSERTE(IndirectionAllowedForJitHelper(ftnNum)); goto exit; From 9dd7738a4e3a0579f4c23769f5d10f71b2004f25 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 13:06:05 -0700 Subject: [PATCH 18/25] Add more casts --- src/coreclr/vm/jithelpers.cpp | 2 +- src/coreclr/vm/jitinterface.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 9f41764f125234..4e7ecb8d7263f1 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2547,7 +2547,7 @@ VMHELPDEF hlpDynamicFuncTable[DYNAMIC_CORINFO_HELP_COUNT] = static const BinderMethodID hlpDynamicToBinderMap[DYNAMIC_CORINFO_HELP_COUNT] = { #define JITHELPER(code, pfnHelper, binderId) -#define DYNAMICJITHELPER(code, pfnHelper, binderId) (pfnHelper != NULL) ? (BinderMethodID)METHOD__NIL : (BinderMethodID)binderId, // If pre-compiled code is provided for a jit helper, prefer that over the IL implementation +#define DYNAMICJITHELPER(code, pfnHelper, binderId) (pfnHelper != (TADDR)NULL) ? (BinderMethodID)METHOD__NIL : (BinderMethodID)binderId, // If pre-compiled code is provided for a jit helper, prefer that over the IL implementation #include "jithelpers.h" }; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 1ddc22dfbf1e7b..2d55eb661af5a3 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10875,7 +10875,7 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN // Static helpers // - _ASSERTE(pfnHelper != NULL); + _ASSERTE(pfnHelper != (TADDR)NULL); accessType = IAT_VALUE; @@ -10930,7 +10930,7 @@ PCODE CEECodeGenInfo::getHelperFtnStatic(CorInfoHelpFunc ftnNum) pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum); } - _ASSERTE(pfnHelper != NULL); + _ASSERTE(pfnHelper != (TADDR)NULL); return GetEEFuncEntryPoint(pfnHelper); } From 41d730c863d5a5e7b3054247535e2ed1ba85b819 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 13:37:00 -0700 Subject: [PATCH 19/25] Remove invalid cast. --- src/coreclr/vm/jithelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 4e7ecb8d7263f1..9f41764f125234 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2547,7 +2547,7 @@ VMHELPDEF hlpDynamicFuncTable[DYNAMIC_CORINFO_HELP_COUNT] = static const BinderMethodID hlpDynamicToBinderMap[DYNAMIC_CORINFO_HELP_COUNT] = { #define JITHELPER(code, pfnHelper, binderId) -#define DYNAMICJITHELPER(code, pfnHelper, binderId) (pfnHelper != (TADDR)NULL) ? (BinderMethodID)METHOD__NIL : (BinderMethodID)binderId, // If pre-compiled code is provided for a jit helper, prefer that over the IL implementation +#define DYNAMICJITHELPER(code, pfnHelper, binderId) (pfnHelper != NULL) ? (BinderMethodID)METHOD__NIL : (BinderMethodID)binderId, // If pre-compiled code is provided for a jit helper, prefer that over the IL implementation #include "jithelpers.h" }; From eb72bbada47781ab11a4c90abe586fcf78ca7f1b Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 14:28:16 -0700 Subject: [PATCH 20/25] Feedback --- src/coreclr/vm/callingconvention.h | 6 +++++- src/coreclr/vm/mlinfo.cpp | 15 ++------------- src/coreclr/vm/wasm/cgencpu.h | 5 ++--- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index 5f52cd27e449f8..6fa48d725577a6 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -12,6 +12,10 @@ #ifndef __CALLING_CONVENTION_INCLUDED #define __CALLING_CONVENTION_INCLUDED +#ifdef FEATURE_INTERPRETER +#include "../../interpreter/interpretershared.h" +#endif // FEATURE_INTERPRETER + BOOL IsRetBuffPassedAsFirstArg(); // Describes how a single argument is laid out in registers and/or stack locations when given as an input to a @@ -1874,7 +1878,7 @@ int ArgIteratorTemplate::GetNextOffset() return argOfs; #elif defined(TARGET_WASM) - int cbArg = ALIGN_UP(StackElemSize(argSize), INTERP_STACK_SLOT_SIZE); + int cbArg = ALIGN_UP(argSize, INTERP_STACK_SLOT_SIZE); int argOfs = TransitionBlock::GetOffsetOfArgs() + m_ofsStack; m_ofsStack += cbArg; diff --git a/src/coreclr/vm/mlinfo.cpp b/src/coreclr/vm/mlinfo.cpp index db9bcabae46019..780125df3d257f 100644 --- a/src/coreclr/vm/mlinfo.cpp +++ b/src/coreclr/vm/mlinfo.cpp @@ -2541,20 +2541,9 @@ void MarshalInfo::SetupArgumentSizes() } CONTRACTL_END; - const unsigned targetPointerSize = TARGET_POINTER_SIZE; - const bool pointerIsValueType = false; - const bool pointerIsFloatHfa = false; -#ifdef TARGET_WASM - // Wasm uses the interpreter which uses a pointer agnostic stack slot size. - // Therefore, we only need assert it is sufficiently large. - _ASSERTE(targetPointerSize <= StackElemSize(TARGET_POINTER_SIZE, pointerIsValueType, pointerIsFloatHfa)); -#else - _ASSERTE(targetPointerSize == StackElemSize(TARGET_POINTER_SIZE, pointerIsValueType, pointerIsFloatHfa)); -#endif // TARGET_WASM - if (m_byref) { - m_nativeArgSize = targetPointerSize; + m_nativeArgSize = TARGET_POINTER_SIZE; } else { @@ -2568,7 +2557,7 @@ void MarshalInfo::SetupArgumentSizes() #ifdef ENREGISTERED_PARAMTYPE_MAXSIZE if (m_nativeArgSize > ENREGISTERED_PARAMTYPE_MAXSIZE) { - m_nativeArgSize = targetPointerSize; + m_nativeArgSize = TARGET_POINTER_SIZE; } #endif // ENREGISTERED_PARAMTYPE_MAXSIZE } diff --git a/src/coreclr/vm/wasm/cgencpu.h b/src/coreclr/vm/wasm/cgencpu.h index cab2c8d617082c..e36a95297127b6 100644 --- a/src/coreclr/vm/wasm/cgencpu.h +++ b/src/coreclr/vm/wasm/cgencpu.h @@ -7,7 +7,6 @@ #include "stublink.h" #include "utilcode.h" -#include "../../interpreter/interpretershared.h" // preferred alignment for data #define DATA_ALIGNMENT 4 @@ -23,8 +22,8 @@ inline unsigned StackElemSize(unsigned parmSize, bool isValueType = false /* unused */, bool isFloatHfa = false /* unused */) { - const unsigned stackSlotSize = INTERP_STACK_SLOT_SIZE; - return ALIGN_UP(parmSize, stackSlotSize); + _ASSERTE("The function is not implemented on wasm"); + return 0; } inline TADDR GetSP(const T_CONTEXT * context) From 1ac630298fbf3dcf99257c9b3e841f1757bd8f84 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 2 Sep 2025 20:06:58 -0700 Subject: [PATCH 21/25] Feedback. --- src/coreclr/vm/interpexec.cpp | 6 +-- src/coreclr/vm/jithelpers.cpp | 25 ++++++----- src/coreclr/vm/jitinterface.cpp | 69 +++++++++++++++-------------- src/coreclr/vm/jitinterface.h | 24 +++++----- src/coreclr/vm/method.cpp | 6 +-- src/coreclr/vm/precode_portable.cpp | 14 +++--- src/coreclr/vm/precode_portable.hpp | 12 ++--- src/coreclr/vm/prestub.cpp | 3 +- 8 files changed, 80 insertions(+), 79 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index a0adac515dc91c..9b245c393b5524 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -308,13 +308,13 @@ template static THelper GetPossiblyIndirectHelper(const Inter } #ifdef FEATURE_PORTABLE_ENTRYPOINTS - if (!PortableEntryPoint::IsNativeEntryPoint((TADDR)addr)) + if (!PortableEntryPoint::HasNativeEntryPoint((PCODE)addr)) { _ASSERTE(pILTargetMethod != NULL); - *pILTargetMethod = PortableEntryPoint::GetMethodDesc((TADDR)addr); + *pILTargetMethod = PortableEntryPoint::GetMethodDesc((PCODE)addr); return NULL; // Return null to interpret this entrypoint } - addr = PortableEntryPoint::GetActualCode((TADDR)addr); + addr = PortableEntryPoint::GetActualCode((PCODE)addr); #endif // FEATURE_PORTABLE_ENTRYPOINTS return (THelper)addr; diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 9f41764f125234..a3212180345c42 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2515,11 +2515,11 @@ enum __CorInfoHelpFunc { #include "jithelpers.h" #ifdef _DEBUG -#define HELPERDEF(code, lpv, isDynamicHelper) { (TADDR)(lpv), #code, isDynamicHelper }, +#define HELPERDEF(code, lpv, isDynamicHelper) { (PCODE)(lpv), #code, isDynamicHelper }, #elif defined(TARGET_WASM) -#define HELPERDEF(code, lpv, isDynamicHelper) { (TADDR)(lpv), isDynamicHelper }, +#define HELPERDEF(code, lpv, isDynamicHelper) { (PCODE)(lpv), isDynamicHelper }, #else // !_DEBUG -#define HELPERDEF(code, lpv, isDynamicHelper) { (TADDR)(lpv) }, +#define HELPERDEF(code, lpv, isDynamicHelper) { (PCODE)(lpv) }, #endif // !_DEBUG // static helpers - constant array @@ -2532,7 +2532,7 @@ const VMHELPDEF hlpFuncTable[CORINFO_HELP_COUNT] = #ifdef FEATURE_PORTABLE_ENTRYPOINTS // Collection of entry points for JIT helpers -TADDR hlpFuncEntryPoints[CORINFO_HELP_COUNT] = {}; +PCODE hlpFuncEntryPoints[CORINFO_HELP_COUNT] = {}; #endif // FEATURE_PORTABLE_ENTRYPOINTS // dynamic helpers - filled in at runtime - See definition of DynamicCorInfoHelpFunc. @@ -2551,12 +2551,13 @@ static const BinderMethodID hlpDynamicToBinderMap[DYNAMIC_CORINFO_HELP_COUNT] = #include "jithelpers.h" }; -bool VMHELPDEF::IsDynamicHelper(size_t& dynamicFtnNum) const +bool VMHELPDEF::IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const { LIMITED_METHOD_CONTRACT; + _ASSERTE(dynamicFtnNum != nullptr); bool isDynamic; - dynamicFtnNum = (size_t)(pfnHelper - 1); + *dynamicFtnNum = (DynamicCorInfoHelpFunc)((size_t)pfnHelper - 1); #ifdef TARGET_WASM // Functions on Wasm are ordinal values, not memory addresses. @@ -2565,7 +2566,7 @@ bool VMHELPDEF::IsDynamicHelper(size_t& dynamicFtnNum) const #else // !TARGET_WASM // If pfnHelper is an index into the dynamic helper table, it should be less // than DYNAMIC_CORINFO_HELP_COUNT. - isDynamic = (dynamicFtnNum < DYNAMIC_CORINFO_HELP_COUNT); + isDynamic = (*dynamicFtnNum < DYNAMIC_CORINFO_HELP_COUNT); #endif // TARGET_WASM #if defined(_DEBUG) || defined(TARGET_WASM) @@ -2592,18 +2593,18 @@ void _SetJitHelperFunction(DynamicCorInfoHelpFunc ftnNum, void * pFunc) LOG((LF_JIT, LL_INFO1000000, "Setting JIT dynamic helper %3d (%s) to %p\n", ftnNum, hlpDynamicFuncTable[ftnNum].name, pFunc)); - hlpDynamicFuncTable[ftnNum].pfnHelper = (TADDR)pFunc; + hlpDynamicFuncTable[ftnNum].pfnHelper = (PCODE)pFunc; } -TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc) +PCODE LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc) { STANDARD_VM_CONTRACT; _ASSERTE(ftnNum < DYNAMIC_CORINFO_HELP_COUNT); MethodDesc* pMD = NULL; - TADDR helper = VolatileLoad(&hlpDynamicFuncTable[ftnNum].pfnHelper); - if (helper == (TADDR)NULL) + PCODE helper = VolatileLoad(&hlpDynamicFuncTable[ftnNum].pfnHelper); + if (helper == (PCODE)NULL) { BinderMethodID binderId = hlpDynamicToBinderMap[ftnNum]; @@ -2615,7 +2616,7 @@ TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDes pMD = CoreLibBinder::GetMethod(binderId); PCODE pFunc = pMD->GetMultiCallableAddrOfCode(); - InterlockedCompareExchangeT(&hlpDynamicFuncTable[ftnNum].pfnHelper, (TADDR)pFunc, (TADDR)NULL); + InterlockedCompareExchangeT(&hlpDynamicFuncTable[ftnNum].pfnHelper, (PCODE)pFunc, (PCODE)NULL); } // If the caller wants the MethodDesc, we may need to try and load it. diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2d55eb661af5a3..6f2e83a0daa6dc 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10768,11 +10768,10 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN MethodDesc* helperMD = NULL; VMHELPDEF const& helperDef = hlpFuncTable[ftnNum]; - TADDR pfnHelper = helperDef.pfnHelper; + PCODE pfnHelper = helperDef.pfnHelper; - size_t dynamicFtnNum; - bool isDynamicMethod = helperDef.IsDynamicHelper(dynamicFtnNum); - if (isDynamicMethod) + DynamicCorInfoHelpFunc dynamicFtnNum; + if (helperDef.IsDynamicHelper(&dynamicFtnNum)) { #if defined(TARGET_AMD64) // To avoid using a jump stub we always call certain helpers using an indirect call. @@ -10791,7 +10790,7 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN dynamicFtnNum == DYNAMIC_CORINFO_HELP_DISPATCH_INDIRECT_CALL) { accessType = IAT_PVALUE; - _ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != (TADDR)NULL); // Confirm the helper is non-null and doesn't require lazy loading. + _ASSERTE(hlpDynamicFuncTable[dynamicFtnNum].pfnHelper != (PCODE)NULL); // Confirm the helper is non-null and doesn't require lazy loading. targetAddr = &hlpDynamicFuncTable[dynamicFtnNum].pfnHelper; _ASSERTE(IndirectionAllowedForJitHelper(ftnNum)); goto exit; @@ -10805,17 +10804,17 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN { accessType = IAT_VALUE; targetAddr = finalTierAddr; - if (pMethod != nullptr && HasILBasedDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum)) + if (pMethod != nullptr && HasILBasedDynamicJitHelper(dynamicFtnNum)) { - (void)LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum, &helperMD); + (void)LoadDynamicJitHelper(dynamicFtnNum, &helperMD); _ASSERT(helperMD != NULL); } goto exit; } - if (HasILBasedDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum)) + if (HasILBasedDynamicJitHelper(dynamicFtnNum)) { - (void)LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum, &helperMD); + (void)LoadDynamicJitHelper(dynamicFtnNum, &helperMD); _ASSERT(helperMD != NULL); // Check if the target MethodDesc is already jitted to its final Tier @@ -10868,34 +10867,36 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN } } - pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum); - } + pfnHelper = LoadDynamicJitHelper(dynamicFtnNum); - // - // Static helpers - // - - _ASSERTE(pfnHelper != (TADDR)NULL); + accessType = IAT_VALUE; + targetAddr = (LPVOID)pfnHelper; + } + else + { + // Static helper + _ASSERTE(pfnHelper != (PCODE)NULL); - accessType = IAT_VALUE; + accessType = IAT_VALUE; #ifdef FEATURE_PORTABLE_ENTRYPOINTS - targetAddr = (LPVOID)hlpFuncEntryPoints[ftnNum]; - if (targetAddr == NULL) + targetAddr = (LPVOID)VolatileLoad(&hlpFuncEntryPoints[ftnNum]); + if (targetAddr == NULL) #endif // FEATURE_PORTABLE_ENTRYPOINTS - { - TADDR entryPoint = GetEEFuncEntryPoint(pfnHelper); + { + PCODE entryPoint = pfnHelper; #ifdef FEATURE_PORTABLE_ENTRYPOINTS - NewHolder portableEntryPoint = new PortableEntryPoint(); - portableEntryPoint->Init((void*)entryPoint); - if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (TADDR)(PortableEntryPoint*)portableEntryPoint, (TADDR)NULL) == (TADDR)NULL) - portableEntryPoint.SuppressRelease(); + AllocMemHolder portableEntryPoint{ SystemDomain::GetGlobalLoaderAllocator()->GetHighFrequencyHeap()->AllocMem(S_SIZE_T{ sizeof(PortableEntryPoint) }) }; + portableEntryPoint->Init((void*)entryPoint); + if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (PCODE)(PortableEntryPoint*)portableEntryPoint, (PCODE)NULL) == (PCODE)NULL) + portableEntryPoint.SuppressRelease(); - entryPoint = hlpFuncEntryPoints[ftnNum]; + entryPoint = hlpFuncEntryPoints[ftnNum]; #endif // FEATURE_PORTABLE_ENTRYPOINTS - targetAddr = (LPVOID)entryPoint; + targetAddr = (LPVOID)entryPoint; + } } exit: ; @@ -10920,19 +10921,19 @@ PCODE CEECodeGenInfo::getHelperFtnStatic(CorInfoHelpFunc ftnNum) } CONTRACTL_END; VMHELPDEF const& helperDef = hlpFuncTable[ftnNum]; - TADDR pfnHelper = helperDef.pfnHelper; + PCODE pfnHelper = helperDef.pfnHelper; // In this case we need to find the actual pfnHelper // using an extra indirection. - size_t dynamicFtnNum; - if (helperDef.IsDynamicHelper(dynamicFtnNum)) + DynamicCorInfoHelpFunc dynamicFtnNum; + if (helperDef.IsDynamicHelper(&dynamicFtnNum)) { - pfnHelper = LoadDynamicJitHelper((DynamicCorInfoHelpFunc)dynamicFtnNum); + pfnHelper = LoadDynamicJitHelper(dynamicFtnNum); } - _ASSERTE(pfnHelper != (TADDR)NULL); + _ASSERTE(pfnHelper != (PCODE)NULL); - return GetEEFuncEntryPoint(pfnHelper); + return pfnHelper; } // Wrapper around CEEInfo::GetProfilingHandle. The first time this is called for a @@ -13366,7 +13367,7 @@ PCODE UnsafeJitFunction(PrepareCodeConfig* config, #ifdef FEATURE_PORTABLE_ENTRYPOINTS PCODE portableEntryPoint = ftn->GetPortableEntryPoint(); _ASSERTE(portableEntryPoint != NULL); - PortableEntryPoint::SetInterpreterData(PCODEToPINSTR(portableEntryPoint), ret); + PortableEntryPoint::SetInterpreterData(portableEntryPoint, ret); ret = portableEntryPoint; #else // !FEATURE_PORTABLE_ENTRYPOINTS diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 5e34dc9b172b02..eab35b6ea0dd8b 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -973,9 +973,17 @@ class CInterpreterJitInfo final : public CEECodeGenInfo /*********************************************************************/ /*********************************************************************/ +// enum for dynamically assigned helper calls +enum DynamicCorInfoHelpFunc { +#define JITHELPER(code, pfnHelper, binderId) +#define DYNAMICJITHELPER(code, pfnHelper, binderId) DYNAMIC_##code, +#include "jithelpers.h" + DYNAMIC_CORINFO_HELP_COUNT +}; + struct VMHELPDEF { - TADDR pfnHelper; + PCODE pfnHelper; #ifdef _DEBUG const char* name; @@ -985,7 +993,7 @@ struct VMHELPDEF bool isDynamicHelper; #endif // _DEBUG || TARGET_WASM - bool IsDynamicHelper(size_t& dynamicFtnNum) const; + bool IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const; }; #if defined(DACCESS_COMPILE) @@ -997,19 +1005,11 @@ GARY_DECL(VMHELPDEF, hlpFuncTable, CORINFO_HELP_COUNT); extern "C" const VMHELPDEF hlpFuncTable[CORINFO_HELP_COUNT]; #ifdef FEATURE_PORTABLE_ENTRYPOINTS -extern "C" TADDR hlpFuncEntryPoints[CORINFO_HELP_COUNT]; +extern "C" PCODE hlpFuncEntryPoints[CORINFO_HELP_COUNT]; #endif // FEATURE_PORTABLE_ENTRYPOINTS #endif -// enum for dynamically assigned helper calls -enum DynamicCorInfoHelpFunc { -#define JITHELPER(code, pfnHelper, binderId) -#define DYNAMICJITHELPER(code, pfnHelper, binderId) DYNAMIC_##code, -#include "jithelpers.h" - DYNAMIC_CORINFO_HELP_COUNT -}; - #ifdef _MSC_VER // GCC complains about duplicate "extern". And it is not needed for the GCC build extern "C" @@ -1019,7 +1019,7 @@ GARY_DECL(VMHELPDEF, hlpDynamicFuncTable, DYNAMIC_CORINFO_HELP_COUNT); #define SetJitHelperFunction(ftnNum, pFunc) _SetJitHelperFunction(DYNAMIC_##ftnNum, (void*)(pFunc)) void _SetJitHelperFunction(DynamicCorInfoHelpFunc ftnNum, void * pFunc); -TADDR LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc = NULL); +PCODE LoadDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum, MethodDesc** methodDesc = NULL); bool HasILBasedDynamicJitHelper(DynamicCorInfoHelpFunc ftnNum); bool IndirectionAllowedForJitHelper(CorInfoHelpFunc ftnNum); diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 32bd8222e8577f..f07746a01e084b 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2216,7 +2216,7 @@ MethodDesc* NonVirtualEntry2MethodDesc(PCODE entryPoint) CONTRACTL_END #ifdef FEATURE_PORTABLE_ENTRYPOINTS - return PortableEntryPoint::GetMethodDesc(PCODEToPINSTR(entryPoint)); + return PortableEntryPoint::GetMethodDesc(entryPoint); #else // FEATURE_PORTABLE_ENTRYPOINTS RangeSection* pRS = ExecutionManager::FindCodeRange(entryPoint, ExecutionManager::GetScanFlags()); @@ -2648,7 +2648,7 @@ MethodDesc* MethodDesc::GetMethodDescFromPrecode(PCODE addr, BOOL fSpeculative / MethodDesc* pMD = NULL; #ifdef FEATURE_PORTABLE_ENTRYPOINTS - pMD = PortableEntryPoint::GetMethodDesc(PCODEToPINSTR(addr)); + pMD = PortableEntryPoint::GetMethodDesc(addr); #else // !FEATURE_PORTABLE_ENTRYPOINTS PTR_Precode pPrecode = Precode::GetPrecodeFromEntryPoint(addr, fSpeculative); @@ -2897,7 +2897,7 @@ void MethodDesc::MarkPrecodeAsStableEntrypoint() PCODE tempEntry = GetTemporaryEntryPointIfExists(); _ASSERTE(tempEntry != (PCODE)NULL); #ifdef FEATURE_PORTABLE_ENTRYPOINTS - _ASSERTE(PortableEntryPoint::GetMethodDesc(PCODEToPINSTR(tempEntry)) == this); + _ASSERTE(PortableEntryPoint::GetMethodDesc(tempEntry) == this); #else // !FEATURE_PORTABLE_ENTRYPOINTS PrecodeType requiredType = GetPrecodeType(); PrecodeType availableType = Precode::GetPrecodeFromEntryPoint(tempEntry)->GetType(); diff --git a/src/coreclr/vm/precode_portable.cpp b/src/coreclr/vm/precode_portable.cpp index 178abde6f2b088..e9687978ea9769 100644 --- a/src/coreclr/vm/precode_portable.cpp +++ b/src/coreclr/vm/precode_portable.cpp @@ -13,14 +13,14 @@ #define CANARY_VALUE 0x12345678 #endif // HOST_64BIT -bool PortableEntryPoint::IsNativeEntryPoint(TADDR addr) +bool PortableEntryPoint::HasNativeEntryPoint(PCODE addr) { LIMITED_METHOD_CONTRACT; PortableEntryPoint* portableEntryPoint = ToPortableEntryPoint(addr); return portableEntryPoint->HasNativeCode(); } -void* PortableEntryPoint::GetActualCode(TADDR addr) +void* PortableEntryPoint::GetActualCode(PCODE addr) { STANDARD_VM_CONTRACT; @@ -29,7 +29,7 @@ void* PortableEntryPoint::GetActualCode(TADDR addr) return portableEntryPoint->_pActualCode; } -MethodDesc* PortableEntryPoint::GetMethodDesc(TADDR addr) +MethodDesc* PortableEntryPoint::GetMethodDesc(PCODE addr) { STANDARD_VM_CONTRACT; @@ -38,7 +38,7 @@ MethodDesc* PortableEntryPoint::GetMethodDesc(TADDR addr) return portableEntryPoint->_pMD; } -void* PortableEntryPoint::GetInterpreterData(TADDR addr) +void* PortableEntryPoint::GetInterpreterData(PCODE addr) { STANDARD_VM_CONTRACT; @@ -47,7 +47,7 @@ void* PortableEntryPoint::GetInterpreterData(TADDR addr) return portableEntryPoint->_pInterpreterData; } -void PortableEntryPoint::SetInterpreterData(TADDR addr, PCODE interpreterData) +void PortableEntryPoint::SetInterpreterData(PCODE addr, PCODE interpreterData) { STANDARD_VM_CONTRACT; @@ -57,12 +57,12 @@ void PortableEntryPoint::SetInterpreterData(TADDR addr, PCODE interpreterData) portableEntryPoint->_pInterpreterData = (void*)PCODEToPINSTR(interpreterData); } -PortableEntryPoint* PortableEntryPoint::ToPortableEntryPoint(TADDR addr) +PortableEntryPoint* PortableEntryPoint::ToPortableEntryPoint(PCODE addr) { LIMITED_METHOD_CONTRACT; _ASSERTE(addr != NULL); - PortableEntryPoint* portableEntryPoint = (PortableEntryPoint*)addr; + PortableEntryPoint* portableEntryPoint = (PortableEntryPoint*)PCODEToPINSTR(addr); _ASSERTE(portableEntryPoint->IsValid()); return portableEntryPoint; } diff --git a/src/coreclr/vm/precode_portable.hpp b/src/coreclr/vm/precode_portable.hpp index 2df0f5d6c82803..eec9c17b59283c 100644 --- a/src/coreclr/vm/precode_portable.hpp +++ b/src/coreclr/vm/precode_portable.hpp @@ -12,15 +12,15 @@ class PortableEntryPoint final { public: // static - static bool IsNativeEntryPoint(TADDR addr); + static bool HasNativeEntryPoint(PCODE addr); - static void* GetActualCode(TADDR addr); - static MethodDesc* GetMethodDesc(TADDR addr); - static void* GetInterpreterData(TADDR addr); - static void SetInterpreterData(TADDR addr, PCODE interpreterData); + static void* GetActualCode(PCODE addr); + static MethodDesc* GetMethodDesc(PCODE addr); + static void* GetInterpreterData(PCODE addr); + static void SetInterpreterData(PCODE addr, PCODE interpreterData); private: // static - static PortableEntryPoint* ToPortableEntryPoint(TADDR addr); + static PortableEntryPoint* ToPortableEntryPoint(PCODE addr); private: Volatile _pActualCode; diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 4ea44bfe7eb5a2..a7edce009c1f09 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -996,8 +996,7 @@ PCODE MethodDesc::JitCompileCodeLocked(PrepareCodeConfig* pConfig, COR_ILMETHOD_ { InterpByteCodeStart* interpreterCode; #ifdef FEATURE_PORTABLE_ENTRYPOINTS - interpreterCode = (InterpByteCodeStart*)PortableEntryPoint::GetInterpreterData(PCODEToPINSTR(pCode)); - + interpreterCode = (InterpByteCodeStart*)PortableEntryPoint::GetInterpreterData(pCode); #else // !FEATURE_PORTABLE_ENTRYPOINTS InterpreterPrecode* pPrecode = InterpreterPrecode::FromEntryPoint(pCode); interpreterCode = dac_cast(pPrecode->GetData()->ByteCodeAddr); From 7317a7db93f5170dc912152aa6d99e1d5e7f05e2 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 2 Sep 2025 21:51:55 -0700 Subject: [PATCH 22/25] Fix getHelperFtn for portable entrypoints --- src/coreclr/vm/jitinterface.cpp | 65 +++++++++++++++++---------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 6f2e83a0daa6dc..e146c49eae4dee 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10771,6 +10771,37 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN PCODE pfnHelper = helperDef.pfnHelper; DynamicCorInfoHelpFunc dynamicFtnNum; + +#ifdef FEATURE_PORTABLE_ENTRYPOINTS + + accessType = IAT_VALUE; + targetAddr = (LPVOID)VolatileLoad(&hlpFuncEntryPoints[ftnNum]); + if (targetAddr == NULL) + { + if (helperDef.IsDynamicHelper(&dynamicFtnNum)) + { + pfnHelper = LoadDynamicJitHelper(dynamicFtnNum, &helperMD); + } + + // LoadDynamicJitHelper returns PortableEntryPoint for helpers backed by managed methods. We need to wrap + // the code address by PortableEntryPoint in all other cases. + if (helperMD == NULL) + { + AllocMemHolder portableEntryPoint{ SystemDomain::GetGlobalLoaderAllocator()->GetHighFrequencyHeap()->AllocMem(S_SIZE_T{ sizeof(PortableEntryPoint) }) }; + portableEntryPoint->Init((void*)pfnHelper); + if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (PCODE)(PortableEntryPoint*)portableEntryPoint, (PCODE)NULL) == (PCODE)NULL) + portableEntryPoint.SuppressRelease(); + pfnHelper = hlpFuncEntryPoints[ftnNum]; + } + else + { + hlpFuncEntryPoints[ftnNum] = pfnHelper; + } + targetAddr = pfnHelper; + } + +#else // FEATURE_PORTABLE_ENTRYPOINTS + if (helperDef.IsDynamicHelper(&dynamicFtnNum)) { #if defined(TARGET_AMD64) @@ -10854,51 +10885,23 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN if (IndirectionAllowedForJitHelper(ftnNum)) { -#ifdef FEATURE_PORTABLE_ENTRYPOINTS - accessType = IAT_VALUE; - targetAddr = (LPVOID)helperMD->GetPortableEntryPoint(); -#else // !FEATURE_PORTABLE_ENTRYPOINTS Precode* pPrecode = helperMD->GetPrecode(); _ASSERTE(pPrecode->GetType() == PRECODE_FIXUP); accessType = IAT_PVALUE; targetAddr = ((FixupPrecode*)pPrecode)->GetTargetSlot(); -#endif // FEATURE_PORTABLE_ENTRYPOINTS goto exit; } } pfnHelper = LoadDynamicJitHelper(dynamicFtnNum); - - accessType = IAT_VALUE; - targetAddr = (LPVOID)pfnHelper; } - else - { - // Static helper - _ASSERTE(pfnHelper != (PCODE)NULL); - - accessType = IAT_VALUE; - -#ifdef FEATURE_PORTABLE_ENTRYPOINTS - targetAddr = (LPVOID)VolatileLoad(&hlpFuncEntryPoints[ftnNum]); - if (targetAddr == NULL) -#endif // FEATURE_PORTABLE_ENTRYPOINTS - { - PCODE entryPoint = pfnHelper; -#ifdef FEATURE_PORTABLE_ENTRYPOINTS - AllocMemHolder portableEntryPoint{ SystemDomain::GetGlobalLoaderAllocator()->GetHighFrequencyHeap()->AllocMem(S_SIZE_T{ sizeof(PortableEntryPoint) }) }; - portableEntryPoint->Init((void*)entryPoint); - if (InterlockedCompareExchangeT(&hlpFuncEntryPoints[ftnNum], (PCODE)(PortableEntryPoint*)portableEntryPoint, (PCODE)NULL) == (PCODE)NULL) - portableEntryPoint.SuppressRelease(); + _ASSERTE(pfnHelper != (PCODE)NULL); + accessType = IAT_VALUE; + targetAddr = (LPVOID)pfnHelper; - entryPoint = hlpFuncEntryPoints[ftnNum]; #endif // FEATURE_PORTABLE_ENTRYPOINTS - targetAddr = (LPVOID)entryPoint; - } - } - exit: ; if (pNativeEntrypoint != NULL) { From 8aea5760af30aae47702a71d1d992fcc6fca4cdd Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 3 Sep 2025 07:12:36 -0700 Subject: [PATCH 23/25] Nits --- src/coreclr/vm/jitinterface.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e146c49eae4dee..1c782837c80948 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10763,8 +10763,8 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN _ASSERTE(ftnNum < CORINFO_HELP_COUNT); - InfoAccessType accessType = IAT_PVALUE; - LPVOID targetAddr = nullptr; + InfoAccessType accessType; + LPVOID targetAddr; MethodDesc* helperMD = NULL; VMHELPDEF const& helperDef = hlpFuncTable[ftnNum]; @@ -10795,12 +10795,13 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN } else { - hlpFuncEntryPoints[ftnNum] = pfnHelper; + VolatileStore(&hlpFuncEntryPoints[ftnNum], pfnHelper); } - targetAddr = pfnHelper; + + targetAddr = (LPVOID)pfnHelper; } -#else // FEATURE_PORTABLE_ENTRYPOINTS +#else // !FEATURE_PORTABLE_ENTRYPOINTS if (helperDef.IsDynamicHelper(&dynamicFtnNum)) { @@ -10900,9 +10901,9 @@ void CEECodeGenInfo::getHelperFtn(CorInfoHelpFunc ftnNum, /* IN accessType = IAT_VALUE; targetAddr = (LPVOID)pfnHelper; +exit: ; #endif // FEATURE_PORTABLE_ENTRYPOINTS -exit: ; if (pNativeEntrypoint != NULL) { pNativeEntrypoint->accessType = accessType; From 4d67d11f695b67b6aedfb779f91292ad65cdc04b Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 3 Sep 2025 07:17:51 -0700 Subject: [PATCH 24/25] Make field more obvious. --- src/coreclr/vm/jithelpers.cpp | 4 ++-- src/coreclr/vm/jitinterface.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index a3212180345c42..7a2ce91a522246 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2562,7 +2562,7 @@ bool VMHELPDEF::IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const #ifdef TARGET_WASM // Functions on Wasm are ordinal values, not memory addresses. // On Wasm, we need some metadata to indicate whether the helper is a dynamic helper. - isDynamic = isDynamicHelper; + isDynamic = _isDynamicHelper; #else // !TARGET_WASM // If pfnHelper is an index into the dynamic helper table, it should be less // than DYNAMIC_CORINFO_HELP_COUNT. @@ -2570,7 +2570,7 @@ bool VMHELPDEF::IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const #endif // TARGET_WASM #if defined(_DEBUG) || defined(TARGET_WASM) - _ASSERTE(isDynamic == isDynamicHelper); + _ASSERTE(isDynamic == _isDynamicHelper); #endif // _DEBUG || TARGET_WASM return isDynamic; diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index eab35b6ea0dd8b..ed9203c364205c 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -990,7 +990,7 @@ struct VMHELPDEF #endif // _DEBUG #if defined(_DEBUG) || defined(TARGET_WASM) - bool isDynamicHelper; + bool _isDynamicHelper; #endif // _DEBUG || TARGET_WASM bool IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const; From c2edbb2d333ba65a54c737374d7691488dc3a0d9 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 3 Sep 2025 10:59:50 -0700 Subject: [PATCH 25/25] Fix bug in casting. --- src/coreclr/vm/jithelpers.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 7a2ce91a522246..7f4a274c5fae30 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2556,9 +2556,9 @@ bool VMHELPDEF::IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const LIMITED_METHOD_CONTRACT; _ASSERTE(dynamicFtnNum != nullptr); - bool isDynamic; - *dynamicFtnNum = (DynamicCorInfoHelpFunc)((size_t)pfnHelper - 1); + size_t dynamicFtnNumMaybe = (size_t)pfnHelper - 1; + bool isDynamic; #ifdef TARGET_WASM // Functions on Wasm are ordinal values, not memory addresses. // On Wasm, we need some metadata to indicate whether the helper is a dynamic helper. @@ -2566,13 +2566,16 @@ bool VMHELPDEF::IsDynamicHelper(DynamicCorInfoHelpFunc* dynamicFtnNum) const #else // !TARGET_WASM // If pfnHelper is an index into the dynamic helper table, it should be less // than DYNAMIC_CORINFO_HELP_COUNT. - isDynamic = (*dynamicFtnNum < DYNAMIC_CORINFO_HELP_COUNT); + isDynamic = (dynamicFtnNumMaybe < DYNAMIC_CORINFO_HELP_COUNT); #endif // TARGET_WASM #if defined(_DEBUG) || defined(TARGET_WASM) _ASSERTE(isDynamic == _isDynamicHelper); #endif // _DEBUG || TARGET_WASM + if (isDynamic) + *dynamicFtnNum = (DynamicCorInfoHelpFunc)dynamicFtnNumMaybe; + return isDynamic; }