From 4b5b9712806316260b3fe6dda462e90d1a22f045 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 16 Sep 2025 15:37:05 -0700 Subject: [PATCH 1/3] Add support for the concept of all funclet's using a separate stack frame - Allow any instruction to directly reference these things using negative offsets. - Calls from funclets to nested finally blocks can share the stack frame with their containing funclet - Add logic to GC reporting to allow funclets reporting to work. --- src/coreclr/interpreter/compiler.cpp | 89 ++++++++++++----- src/coreclr/interpreter/compiler.h | 7 +- .../interpreter/inc/interpretershared.h | 2 + src/coreclr/vm/eetwain.cpp | 19 +++- src/coreclr/vm/gcinfodecoder.cpp | 40 ++++++-- src/coreclr/vm/interpexec.cpp | 98 ++++++++++++------- src/coreclr/vm/interpexec.h | 61 ++++++++++-- src/coreclr/vm/prestub.cpp | 8 +- 8 files changed, 236 insertions(+), 88 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index a6ca51b83b0984..7c65f94adc1628 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -735,7 +735,20 @@ uint32_t InterpCompiler::ConvertOffset(int32_t offset) return offset * sizeof(int32_t) + sizeof(void*); } -int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArray *relocs) +int32_t InterpCompiler::GetFuncletAdjustedVarOffset(InterpInst *ins, int varIndex, bool forFunclet) +{ + if (forFunclet && m_pVars[varIndex].global) + { + // In funclets, global vars are accessed relative to the main method frame pointer + return -(m_pVars[varIndex].offset + FUNCLET_STACK_ADJUSTMENT_OFFSET); + } + else + { + return m_pVars[varIndex].offset; + } +} + +int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArray *relocs, bool forFunclet) { ins->nativeOffset = (int32_t)(ip - m_pMethodCode); @@ -749,7 +762,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydata [0]; - *ip++ = m_pVars[ins->sVars[0]].offset; + *ip++ = GetFuncletAdjustedVarOffset(ins, ins->sVars[0], forFunclet); *ip++ = numLabels; // Add relocation for each label for (int32_t i = 0; i < numLabels; i++) @@ -764,7 +777,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraysVars[i]].offset; + *ip++ = GetFuncletAdjustedVarOffset(ins, ins->sVars[i], forFunclet); if (ins->info.pTargetBB->nativeOffset >= 0) { @@ -796,9 +809,13 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydVar].offset; - int srcOffset = m_pVars[ins->sVars[0]].offset; - srcOffset += fOffset; + int destOffset = GetFuncletAdjustedVarOffset(ins, ins->dVar, forFunclet); + int srcOffset = GetFuncletAdjustedVarOffset(ins, ins->sVars[0], forFunclet); + if (srcOffset >= 0) + srcOffset += fOffset; + else + srcOffset -= fOffset; + if (fSize) opcode = INTOP_MOV_VT; else @@ -819,8 +836,8 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydVar].offset; - *ip++ = m_pVars[ins->sVars[0]].offset; + *ip++ = GetFuncletAdjustedVarOffset(ins, ins->dVar, forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins, ins->sVars[0], forFunclet); } else { @@ -829,7 +846,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydVar].offset; + *ip++ = GetFuncletAdjustedVarOffset(ins, ins->dVar, forFunclet); if (g_interpOpSVars[opcode]) { @@ -841,7 +858,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraysVars[i]].offset; + *ip++ = GetFuncletAdjustedVarOffset(ins, ins->sVars[i], forFunclet); } } } @@ -927,7 +944,7 @@ int32_t *InterpCompiler::EmitBBCode(int32_t *ip, InterpBasicBlock *bb, TArrayclauseNonTryType != BBClauseNone); } m_pCBB->nativeEndOffset = (int32_t)(ip - m_pMethodCode); @@ -1162,7 +1179,7 @@ class InterpGcSlotAllocator if (allocateNewSlot) { // Important to pass GC_FRAMEREG_REL, the default is broken due to GET_CALLER_SP being unimplemented - *pSlot = m_encoder->GetStackSlotId(offsetBytes, flags, GC_FRAMEREG_REL); + *pSlot = m_encoder->GetStackSlotId(offsetBytes, flags, (flags & GC_SLOT_UNTRACKED) ? GC_FRAMEREG_REL : GC_SP_REL); } else { @@ -2084,17 +2101,29 @@ void InterpCompiler::InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodI for (uint32_t j = clause.HandlerOffset; j < (clause.HandlerOffset + clause.HandlerLength); j++) { InterpBasicBlock* pBB = m_ppOffsetToBB[j]; - if (pBB != NULL && pBB->clauseType == BBClauseNone) + + if (pBB == NULL) + continue; + + uint8_t clauseType; + if ((clause.Flags == CORINFO_EH_CLAUSE_NONE) || (clause.Flags == CORINFO_EH_CLAUSE_FILTER)) { - if ((clause.Flags == CORINFO_EH_CLAUSE_NONE) || (clause.Flags == CORINFO_EH_CLAUSE_FILTER)) - { - pBB->clauseType = BBClauseCatch; - } - else - { - assert((clause.Flags == CORINFO_EH_CLAUSE_FINALLY) || (clause.Flags == CORINFO_EH_CLAUSE_FAULT)); - pBB->clauseType = BBClauseFinally; - } + clauseType = BBClauseCatch; + } + else + { + assert((clause.Flags == CORINFO_EH_CLAUSE_FINALLY) || (clause.Flags == CORINFO_EH_CLAUSE_FAULT)); + clauseType = BBClauseFinally; + } + + if (pBB->clauseType == BBClauseNone) + { + pBB->clauseType = clauseType; + } + + if (pBB->clauseNonTryType == BBClauseNone) + { + pBB->clauseNonTryType = clauseType; } } @@ -2118,10 +2147,17 @@ void InterpCompiler::InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodI for (uint32_t j = clause.FilterOffset; j < clause.HandlerOffset; j++) { InterpBasicBlock* pBB = m_ppOffsetToBB[j]; - if (pBB != NULL && pBB->clauseType == BBClauseNone) + if (pBB == NULL) + continue; + + if (pBB->clauseType == BBClauseNone) { pBB->clauseType = BBClauseFilter; } + if (pBB->clauseNonTryType == BBClauseNone) + { + pBB->clauseNonTryType = BBClauseFilter; + } } } else if (clause.Flags == CORINFO_EH_CLAUSE_FINALLY|| clause.Flags == CORINFO_EH_CLAUSE_FAULT) @@ -2165,6 +2201,7 @@ void InterpCompiler::InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodI InterpBasicBlock* pAfterFinallyBB = m_ppOffsetToBB[clause.HandlerOffset + clause.HandlerLength]; assert(pAfterFinallyBB != NULL); pFinallyCallIslandBB->clauseType = pAfterFinallyBB->clauseType; + pFinallyCallIslandBB->clauseNonTryType = pAfterFinallyBB->clauseNonTryType; pFinallyCallIslandBB = pFinallyCallIslandBB->pNextBB; } } @@ -2266,7 +2303,7 @@ void InterpCompiler::EmitLoadVar(int32_t var) InterpType interpType = m_pVars[var].interpType; CORINFO_CLASS_HANDLE clsHnd = m_pVars[var].clsHnd; - if (m_pCBB->clauseType == BBClauseFilter) + if (m_pCBB->clauseNonTryType == BBClauseFilter) { assert(m_pVars[var].ILGlobal); AddIns(INTOP_LOAD_FRAMEVAR); @@ -2301,7 +2338,7 @@ void InterpCompiler::EmitStoreVar(int32_t var) InterpType interpType = m_pVars[var].interpType; CHECK_STACK(1); - if (m_pCBB->clauseType == BBClauseFilter) + if (m_pCBB->clauseNonTryType == BBClauseFilter) { AddIns(INTOP_LOAD_FRAMEVAR); PushInterpType(InterpTypeI, NULL); @@ -3969,7 +4006,7 @@ void InterpCompiler::EmitLdLocA(int32_t var) m_shadowCopyOfThisPointerActuallyNeeded = true; } - if (m_pCBB->clauseType == BBClauseFilter) + if (m_pCBB->clauseNonTryType == BBClauseFilter) { AddIns(INTOP_LOAD_FRAMEVAR); PushInterpType(InterpTypeI, NULL); diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index c2c2378c7ee51e..115c257eaa6b7b 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -327,6 +327,9 @@ struct InterpBasicBlock // Type of the innermost try block, catch, filter, or finally that contains this basic block. uint8_t clauseType; + // Type of the innermost catch, filter, or finally that contains this basic block. Try blocks within catch, filter or finally blocks are accounted as catch, filter or finally blocks. + uint8_t clauseNonTryType; + // True indicates that this basic block is the first block of a filter, catch or filtered handler funclet. bool isFilterOrCatchFuncletEntry; @@ -358,6 +361,7 @@ struct InterpBasicBlock emitState = BBStateNotEmitted; clauseType = BBClauseNone; + clauseNonTryType = BBClauseNone; isFilterOrCatchFuncletEntry = false; clauseVarIndex = -1; overlappingEHClauseCount = 0; @@ -775,7 +779,8 @@ class InterpCompiler uint32_t ConvertOffset(int32_t offset); void EmitCode(); int32_t* EmitBBCode(int32_t *ip, InterpBasicBlock *bb, TArray *relocs); - int32_t* EmitCodeIns(int32_t *ip, InterpInst *pIns, TArray *relocs); + int32_t* EmitCodeIns(int32_t *ip, InterpInst *pIns, TArray *relocs, bool forFunclet); + int32_t GetFuncletAdjustedVarOffset(InterpInst *ins, int varIndex, bool forFunclet); void PatchRelocations(TArray *relocs); InterpMethod* CreateInterpMethod(); void CreateBasicBlocks(CORINFO_METHOD_INFO* methodInfo); diff --git a/src/coreclr/interpreter/inc/interpretershared.h b/src/coreclr/interpreter/inc/interpretershared.h index d9e79f3bffea57..c29f3f41f3abd6 100644 --- a/src/coreclr/interpreter/inc/interpretershared.h +++ b/src/coreclr/interpreter/inc/interpretershared.h @@ -178,4 +178,6 @@ enum class CalliFlags : int32_t PInvoke = 1 << 2, // The call is a PInvoke call }; +#define FUNCLET_STACK_ADJUSTMENT_OFFSET 8 + #endif diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index 4d84204dee347b..b2f7c68696b696 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -2157,7 +2157,7 @@ DWORD_PTR InterpreterCodeManager::CallFunclet(OBJECTREF throwable, void* pHandle // InterpMethodContextFrame. This is important for the stack walking code. struct Frames { - InterpMethodContextFrame interpMethodContextFrame = {0}; + InterpMethodContextFrame interpMethodContextFrame; InterpreterFrame interpreterFrame; Frames(TransitionBlock* pTransitionBlock) @@ -2176,9 +2176,20 @@ DWORD_PTR InterpreterCodeManager::CallFunclet(OBJECTREF throwable, void* pHandle StackVal retVal; - frames.interpMethodContextFrame.startIp = pOriginalFrame->startIp; - frames.interpMethodContextFrame.pStack = isFilter ? sp : pOriginalFrame->pStack; - frames.interpMethodContextFrame.pRetVal = (int8_t*)&retVal; + + // Frame pointer for original method + int8_t* originalStack; + if (pOriginalFrame->IsFuncletFrame()) + { + originalStack = *(int8_t**)pOriginalFrame->pStack; + } + else + { + originalStack = pOriginalFrame->pStack - FUNCLET_STACK_ADJUSTMENT_OFFSET; + } + *(int8_t**)sp = originalStack; + InterpreterFrameReporting frameReporting = isFilter ? InterpreterFrameReporting::FuncletNoReportGlobals : InterpreterFrameReporting::FuncletReportGlobals; + frames.interpMethodContextFrame.ReInit(NULL, pOriginalFrame->startIp, (int8_t*)&retVal, frameReporting, sp); ExceptionClauseArgs exceptionClauseArgs; exceptionClauseArgs.ip = (const int32_t *)pHandler; diff --git a/src/coreclr/vm/gcinfodecoder.cpp b/src/coreclr/vm/gcinfodecoder.cpp index a99c8543d9a422..7fd2b76ad3c0c2 100644 --- a/src/coreclr/vm/gcinfodecoder.cpp +++ b/src/coreclr/vm/gcinfodecoder.cpp @@ -7,6 +7,10 @@ #include "gcinfodecoder.h" +#ifdef FEATURE_INTERPRETER +#include "interpexec.h" +#endif // FEATURE_INTERPRETER + #ifdef USE_GC_INFO_DECODER #ifndef CHECK_APP_DOMAIN @@ -2122,9 +2126,14 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( { OBJECTREF* pObjRef = NULL; + InterpMethodContextFrame* pFrame = (InterpMethodContextFrame*)GetSP(pRD->pCurrentContext); + _ASSERTE(pFrame); + + int8_t* baseReg = nullptr; + if( GC_SP_REL == spBase ) { - _ASSERTE(!"GC_SP_REL is invalid for interpreter frames"); + baseReg = pFrame->pStack; } else if( GC_CALLER_SP_REL == spBase ) { @@ -2132,14 +2141,26 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( } else { - // Interpreter-TODO: Enhance GcInfoEncoder/Decoder to allow omitting the stack slot base register for interpreted - // methods, since only one base (fp) is ever used for interpreter locals. See Interpreter-TODO in DecodeSlotTable. - _ASSERTE( GC_FRAMEREG_REL == spBase ); - uint8_t* fp = (uint8_t *)GetFP(pRD->pCurrentContext); - _ASSERTE(fp); - pObjRef = (OBJECTREF*)(fp + spOffset); + if (pFrame->ShouldReportGlobals()) + { + if (pFrame->IsFuncletFrame()) + { + baseReg = *(int8_t**)pFrame->pStack + FUNCLET_STACK_ADJUSTMENT_OFFSET; + } + else + { + baseReg = pFrame->pStack; + } + } + else + { + return NULL; + } } + _ASSERTE(baseReg); + pObjRef = (OBJECTREF*)(baseReg + spOffset); + return pObjRef; } #endif @@ -2220,6 +2241,11 @@ template void TGcInfoDecoder::ReportSt GCINFODECODER_CONTRACT; OBJECTREF* pObjRef = GetStackSlot(spOffset, spBase, pRD); + + // Provide a means for GetStackSlot to indicate that this slot shoult not actually be reported + if (pObjRef == NULL) + return; + _ASSERTE(IS_ALIGNED(pObjRef, sizeof(OBJECTREF*))); #ifdef _DEBUG diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 211dd41cb38a7e..631cb18deff885 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -321,21 +321,22 @@ static void InterpBreakpoint() } #endif +#define LOCAL_VAR_ADDR(offset,type) offset < 0 ? (((type*)((*(uintptr_t*)stack) + (-offset)))) : ((type*)(stack + (offset))) +#define LOCAL_VAR(offset,type) (*(LOCAL_VAR_ADDR(offset, type))) +#define NULL_CHECK(o) do { if ((o) == NULL) { COMPlusThrow(kNullReferenceException); } } while (0) + static OBJECTREF CreateMultiDimArray(MethodTable* arrayClass, int8_t* stack, int32_t dimsOffset, int numArgs) { int32_t* dims = (int32_t*)alloca(numArgs * sizeof(int32_t)); + int8_t* dimsBase = LOCAL_VAR_ADDR(dimsOffset, int8_t); for (int i = 0; i < numArgs; i++) { - dims[i] = *(int32_t*)(stack + dimsOffset + i * 8); + dims[i] = *(int32_t*)(dimsBase + i * 8); } return AllocateArrayEx(arrayClass, dims, numArgs); } -#define LOCAL_VAR_ADDR(offset,type) ((type*)(stack + (offset))) -#define LOCAL_VAR(offset,type) (*LOCAL_VAR_ADDR(offset, type)) -#define NULL_CHECK(o) do { if ((o) == NULL) { COMPlusThrow(kNullReferenceException); } } while (0) - template static THelper GetPossiblyIndirectHelper(const InterpMethod* pMethod, int32_t _data, MethodDesc** pILTargetMethod = NULL) { InterpHelperData data{}; @@ -687,14 +688,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // in the first pass of EH when the frames between the current frame and the // parent frame are still alive. All accesses to the locals and arguments // in this case use the pExceptionClauseArgs->pFrame->pStack as a frame pointer. - // * Catch and finally funclets are running in the parent frame directly - - if (pExceptionClauseArgs->isFilter) - { - // Since filters run in their own frame, we need to clear the global variables - // so that GC doesn't pick garbage in variables that were not yet written to. - memset(pFrame->pStack, 0, pMethod->allocaSize); - } + // * Catch and finally funclets are also executed in the current frame, because + // they are executed in the second pass of EH, when the DispatchEx function is still on + // the managed stack. // Start executing at the beginning of the exception clause ip = pExceptionClauseArgs->ip; @@ -730,6 +726,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr break; #endif case INTOP_INITLOCALS: + assert(ip[1] >= 0); memset(stack + ip[1], 0, ip[2]); ip += 3; break; @@ -779,16 +776,16 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr break; case INTOP_RET: // Return stack slot sized value - *(int64_t*)pFrame->pRetVal = LOCAL_VAR(ip[1], int64_t); + *(int64_t*)pFrame->GetRetValAddr_KnownNormalReporting() = LOCAL_VAR(ip[1], int64_t); goto EXIT_FRAME; case INTOP_RET_VT: - memmove(pFrame->pRetVal, stack + ip[1], ip[2]); + memmove(pFrame->GetRetValAddr_KnownNormalReporting(), LOCAL_VAR_ADDR(ip[1], void*), ip[2]); goto EXIT_FRAME; case INTOP_RET_VOID: goto EXIT_FRAME; case INTOP_LDLOCA: - LOCAL_VAR(ip[1], void*) = stack + ip[2]; + LOCAL_VAR(ip[1], void*) = LOCAL_VAR_ADDR(ip[2], void*); ip += 3; break; case INTOP_LOAD_FRAMEVAR: @@ -814,7 +811,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr #undef MOV case INTOP_MOV_VT: - memmove(stack + ip[1], stack + ip[2], ip[3]); + memmove(LOCAL_VAR_ADDR(ip[1], void*), LOCAL_VAR_ADDR(ip[2], void*), ip[3]); ip += 4; break; @@ -1899,7 +1896,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { char *src = LOCAL_VAR(ip[2], char*); NULL_CHECK(src); - memcpy(stack + ip[1], (char*)src + ip[3], ip[4]); + memcpy(LOCAL_VAR_ADDR(ip[1], void*), (char*)src + ip[3], ip[4]); ip += 5; break; } @@ -1951,7 +1948,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { char *dest = LOCAL_VAR(ip[1], char*); NULL_CHECK(dest); - memcpyNoGCRefs(dest + ip[3], stack + ip[2], ip[4]); + memcpyNoGCRefs(dest + ip[3], LOCAL_VAR_ADDR(ip[2], void*), ip[4]); ip += 5; break; } @@ -1960,7 +1957,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr MethodTable *pMT = (MethodTable*)pMethod->pDataItems[ip[4]]; char *dest = LOCAL_VAR(ip[1], char*); NULL_CHECK(dest); - CopyValueClassUnchecked(dest + ip[3], stack + ip[2], pMT); + CopyValueClassUnchecked(dest + ip[3], LOCAL_VAR_ADDR(ip[2], void*), pMT); ip += 5; break; } @@ -2269,7 +2266,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { isTailcall = (*ip == INTOP_CALLI_TAIL); returnOffset = ip[1]; + int8_t* returnValueAddress = LOCAL_VAR_ADDR(returnOffset, int8_t); callArgsOffset = ip[2]; + assert(callArgsOffset >= 0); int32_t calliFunctionPointerVar = ip[3]; int32_t calliCookie = ip[4]; int32_t flags = ip[5]; @@ -2285,11 +2284,11 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { if (flags & (int32_t)CalliFlags::SuppressGCTransition) { - InvokeUnmanagedCalli(LOCAL_VAR(calliFunctionPointerVar, PCODE), cookie, stack + callArgsOffset, stack + returnOffset); + InvokeUnmanagedCalli(LOCAL_VAR(calliFunctionPointerVar, PCODE), cookie, stack + callArgsOffset, returnValueAddress); } else { - InvokeUnmanagedCalliWithTransition(LOCAL_VAR(calliFunctionPointerVar, PCODE), cookie, stack, pFrame, stack + callArgsOffset, stack + returnOffset); + InvokeUnmanagedCalliWithTransition(LOCAL_VAR(calliFunctionPointerVar, PCODE), cookie, stack, pFrame, stack + callArgsOffset, returnValueAddress); } } else @@ -2306,7 +2305,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr goto CALL_INTERP_METHOD; } #endif // FEATURE_PORTABLE_ENTRYPOINTS - InvokeCalliStub(calliFunctionPointer, cookie, stack + callArgsOffset, stack + returnOffset); + InvokeCalliStub(calliFunctionPointer, cookie, stack + callArgsOffset, returnValueAddress); } break; @@ -2320,6 +2319,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr isTailcall = false; returnOffset = ip[1]; callArgsOffset = ip[2]; + assert(callArgsOffset >= 0); + int8_t* returnValueAddress = LOCAL_VAR_ADDR(returnOffset, int8_t); methodSlot = ip[3]; int32_t targetAddrSlot = ip[4]; int32_t flags = ip[5]; @@ -2335,11 +2336,11 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr if (flags & (int32_t)PInvokeCallFlags::SuppressGCTransition) { - InvokeUnmanagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); + InvokeUnmanagedMethod(targetMethod, stack + callArgsOffset, returnValueAddress, callTarget); } else { - InvokeUnmanagedMethodWithTransition(targetMethod, stack, pFrame, stack + callArgsOffset, stack + returnOffset, callTarget); + InvokeUnmanagedMethodWithTransition(targetMethod, stack, pFrame, stack + callArgsOffset, returnValueAddress, callTarget); } break; @@ -2352,6 +2353,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr callArgsOffset = ip[2]; methodSlot = ip[3]; + assert(callArgsOffset >= 0); + int8_t* returnValueAddress = LOCAL_VAR_ADDR(returnOffset, int8_t); + // targetMethod holds a pointer to the Invoke method of the delegate, not the final actual target. targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; @@ -2369,7 +2373,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // TODO! Once we are investigating performance here, we may want to optimize this so that // delegate calls to interpeted methods don't have to go through the native invoke here, but for // now this should work well. - InvokeDelegateInvokeMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetAddress); + InvokeDelegateInvokeMethod(targetMethod, stack + callArgsOffset, returnValueAddress, targetAddress); break; } @@ -2389,6 +2393,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr CALL_INTERP_SLOT: targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; CALL_INTERP_METHOD: + + assert(callArgsOffset >= 0); + int8_t* returnValueAddress = LOCAL_VAR_ADDR(returnOffset, int8_t); // Save current execution state for when we return from called method pFrame->ip = ip; @@ -2413,7 +2420,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { // 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 - InvokeManagedMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetMethod->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); + InvokeManagedMethod(targetMethod, stack + callArgsOffset, returnValueAddress, targetMethod->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); break; } } @@ -2429,7 +2436,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr memcpy(pFrame->pStack, stack + callArgsOffset, pTargetMethod->argsSize); // Reuse current stack frame. We discard the call insn's returnOffset because it's not important and tail calls are // required to be followed by a ret, so we know nothing is going to read from stack[returnOffset] after the call. - pFrame->ReInit(pFrame->pParent, targetIp, pFrame->pRetVal, pFrame->pStack); + pFrame->ReInit(pFrame->pParent, targetIp, pFrame->GetRetValAddr_KnownNormalReporting(), InterpreterFrameReporting::Normal, pFrame->pStack); } else { @@ -2447,7 +2454,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // Save the lowest SP in the current method so that we can identify it by that during stackwalk pInterpreterFrame->SetInterpExecMethodSP((TADDR)GetCurrentSP()); } - pChildFrame->ReInit(pFrame, targetIp, stack + returnOffset, stack + callArgsOffset); + pChildFrame->ReInit(pFrame, targetIp, returnValueAddress, InterpreterFrameReporting::Normal, stack + callArgsOffset); pFrame = pChildFrame; } assert (((size_t)pFrame->pStack % INTERP_STACK_ALIGNMENT) == 0); @@ -2520,7 +2527,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr methodSlot = ip[3]; int32_t vtSize = ip[4]; - void *vtThis = stack + returnOffset; + void *vtThis = LOCAL_VAR_ADDR(returnOffset, void*); // pass the address of the valuetype LOCAL_VAR(callArgsOffset, void*) = vtThis; @@ -2768,7 +2775,7 @@ do { \ size_t componentSize = arr->GetMethodTable()->GetComponentSize(); void* elemAddr = pData + idx * componentSize; MethodTable* pElemMT = arr->GetArrayElementTypeHandle().AsMethodTable(); - CopyValueClassUnchecked(stack + ip[1], elemAddr, pElemMT); + CopyValueClassUnchecked(LOCAL_VAR_ADDR(ip[1], void*), elemAddr, pElemMT); ip += 5; break; } @@ -2879,7 +2886,7 @@ do { \ void* elemAddr = pData + idx * elemSize; MethodTable* pMT = (MethodTable*)pMethod->pDataItems[ip[5]]; - CopyValueClassUnchecked(elemAddr, stack + ip[3], pMT); + CopyValueClassUnchecked(elemAddr, LOCAL_VAR_ADDR(ip[3], void*), pMT); ip += 6; break; } @@ -2899,7 +2906,7 @@ do { \ size_t elemSize = ip[4]; void* elemAddr = pData + idx * elemSize; - memcpyNoGCRefs(elemAddr, stack + ip[3], elemSize); + memcpyNoGCRefs(elemAddr, LOCAL_VAR_ADDR(ip[3], void*), elemSize); ip += 5; break; } @@ -3038,6 +3045,9 @@ do \ case INTOP_CALL_FINALLY: { const int32_t* targetIp = ip + ip[1]; + // TODO-Interp, this is more adjustment than we need, but it'll work for now + const int32_t stackAdjust = pFrame->startIp->Method->allocaSize; + assert(stackAdjust >= 0); // Save current execution state for when we return from called method pFrame->ip = ip + 2; @@ -3052,8 +3062,22 @@ do \ // Save the lowest SP in the current method so that we can identify it by that during stackwalk pInterpreterFrame->SetInterpExecMethodSP((TADDR)GetCurrentSP()); } - // Set the frame to the same values as the caller frame. - pChildFrame->ReInit(pFrame, pFrame->startIp, pFrame->pRetVal, pFrame->pStack); + // Store the address for the current frame's stack pointer into the new frame's stack space, if we are + // adjusting the stack. + if (pFrame->IsFuncletFrame()) + { + // If we're already in a funclet, the finally can re-use the stack space of the current funclet's frame + pChildFrame->ReInit(pFrame, pFrame->startIp, NULL, InterpreterFrameReporting::FuncletReportGlobals, pFrame->pStack); + } + else + { + // If we're calling into the finally from the main method body, then we need to create a separate frame, and + // fill in the funclet frame pointer link slot + *(int8_t**)(pFrame->pStack + stackAdjust) = pFrame->pStack - FUNCLET_STACK_ADJUSTMENT_OFFSET; + pChildFrame->ReInit(pFrame, pFrame->startIp, NULL, InterpreterFrameReporting::FuncletReportGlobals, pFrame->pStack + stackAdjust); + stack = pChildFrame->pStack; + pThreadContext->pStackPointer = stack + pMethod->allocaSize; + } pFrame = pChildFrame; } assert (((size_t)pFrame->pStack % INTERP_STACK_ALIGNMENT) == 0); @@ -3063,10 +3087,10 @@ do \ break; } case INTOP_LEAVE_FILTER: - *(int64_t*)pFrame->pRetVal = LOCAL_VAR(ip[1], int32_t); + *(int64_t*)pFrame->GetRetValAddr() = LOCAL_VAR(ip[1], int32_t); goto EXIT_FRAME; case INTOP_LEAVE_CATCH: - *(const int32_t**)pFrame->pRetVal = ip + ip[1]; + *(const int32_t**)(pFrame->GetRetValAddr()) = ip + ip[1]; goto EXIT_FRAME; case INTOP_THROW_PNSE: COMPlusThrow(kPlatformNotSupportedException); diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index d03f3b236943f7..87a0398dc4270c 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -27,21 +27,66 @@ struct StackVal typedef DPTR(struct InterpMethodContextFrame) PTR_InterpMethodContextFrame; class InterpreterFrame; +enum class InterpreterFrameReporting +{ + Normal = 0, + FuncletReportGlobals = 1, + FuncletNoReportGlobals = 2, + + Mask = 3 +}; + struct InterpMethodContextFrame { - PTR_InterpMethodContextFrame pParent; - PTR_InterpByteCodeStart startIp; // from startIp we can obtain InterpMethod and MethodDesc - int8_t *pStack; - int8_t *pRetVal; - const int32_t *ip; // This ip is updated only when execution can leave the frame - PTR_InterpMethodContextFrame pNext; + PTR_InterpMethodContextFrame pParent = 0; + PTR_InterpByteCodeStart startIp = 0; // from startIp we can obtain InterpMethod and MethodDesc + int8_t *pStack = 0; +private: + int8_t *pRetVal = 0; // If the low bit is set on pRetVal, then Frame represents a funclet +public: + const int32_t *ip = 0; // This ip is updated only when execution can leave the frame + PTR_InterpMethodContextFrame pNext = 0; + + bool IsFuncletFrame() + { + LIMITED_METHOD_CONTRACT; + return ((size_t)pRetVal & (size_t)InterpreterFrameReporting::Mask) != (size_t)InterpreterFrameReporting::Normal; + } + bool ShouldReportGlobals() + { + LIMITED_METHOD_CONTRACT; + switch ((InterpreterFrameReporting)((size_t)pRetVal & (size_t)InterpreterFrameReporting::Mask)) + { + case InterpreterFrameReporting::FuncletReportGlobals: + case InterpreterFrameReporting::Normal: + return true; + case InterpreterFrameReporting::FuncletNoReportGlobals: + return false; + default: + assert(false); + return false; + } + } + + int8_t* GetRetValAddr() + { + LIMITED_METHOD_CONTRACT; + return (int8_t*)((size_t)pRetVal & ~(size_t)InterpreterFrameReporting::Mask); + } + + int8_t* GetRetValAddr_KnownNormalReporting() + { + LIMITED_METHOD_CONTRACT; + assert(!IsFuncletFrame()); + return pRetVal; + } #ifndef DACCESS_COMPILE - void ReInit(InterpMethodContextFrame *pParent, InterpByteCodeStart* startIp, int8_t *pRetVal, int8_t *pStack) + void ReInit(InterpMethodContextFrame *pParent, InterpByteCodeStart* startIp, int8_t *pRetVal, InterpreterFrameReporting reporting, int8_t *pStack) { this->pParent = pParent; this->startIp = startIp; - this->pRetVal = pRetVal; + this->pRetVal = (int8_t*)((size_t)pRetVal | (size_t)reporting); this->pStack = pStack; this->ip = NULL; } diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index dcd94a0c3b7546..df2dd0f39baa69 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2048,7 +2048,7 @@ extern "C" void* STDCALL ExecuteInterpretedMethod(TransitionBlock* pTransitionBl // InterpMethodContextFrame. This is important for the stack walking code. struct Frames { - InterpMethodContextFrame interpMethodContextFrame = {0}; + InterpMethodContextFrame interpMethodContextFrame; InterpreterFrame interpreterFrame; Frames(TransitionBlock* pTransitionBlock) @@ -2058,15 +2058,13 @@ extern "C" void* STDCALL ExecuteInterpretedMethod(TransitionBlock* pTransitionBl } frames(pTransitionBlock); - frames.interpMethodContextFrame.startIp = dac_cast(byteCodeAddr); - frames.interpMethodContextFrame.pStack = sp; - frames.interpMethodContextFrame.pRetVal = (retBuff != NULL) ? (int8_t*)retBuff : sp; + frames.interpMethodContextFrame.ReInit(NULL, dac_cast(byteCodeAddr), (retBuff != NULL) ? (int8_t*)retBuff : sp, InterpreterFrameReporting::Normal, sp); InterpExecMethod(&frames.interpreterFrame, &frames.interpMethodContextFrame, threadContext); frames.interpreterFrame.Pop(); - return frames.interpMethodContextFrame.pRetVal; + return frames.interpMethodContextFrame.GetRetValAddr_KnownNormalReporting(); } extern "C" void* STDCALL ExecuteInterpretedMethodWithArgs(TransitionBlock* pTransitionBlock, TADDR byteCodeAddr, int8_t* pArgs, size_t size, void* retBuff) From 5d9a4fe58f2969518d31d04f209682a98b959034 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 16 Sep 2025 16:13:51 -0700 Subject: [PATCH 2/3] Add support for re-use stack of main frame in catch/finally/filter funclets FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS --- src/coreclr/interpreter/compiler.cpp | 26 ++++++++++------ src/coreclr/interpreter/compiler.h | 2 +- src/coreclr/interpreter/compileropt.cpp | 5 +++ .../interpreter/inc/interpretershared.h | 8 +++++ src/coreclr/vm/eetwain.cpp | 7 ++++- src/coreclr/vm/gcinfodecoder.cpp | 4 +++ src/coreclr/vm/interpexec.cpp | 31 +++++++++++++++++-- src/coreclr/vm/interpexec.h | 18 +++++++++++ 8 files changed, 88 insertions(+), 13 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 7c65f94adc1628..3ddc179d2db1e5 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -735,14 +735,16 @@ uint32_t InterpCompiler::ConvertOffset(int32_t offset) return offset * sizeof(int32_t) + sizeof(void*); } -int32_t InterpCompiler::GetFuncletAdjustedVarOffset(InterpInst *ins, int varIndex, bool forFunclet) +int32_t InterpCompiler::GetFuncletAdjustedVarOffset(int varIndex, bool forFunclet) { +#ifndef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS if (forFunclet && m_pVars[varIndex].global) { // In funclets, global vars are accessed relative to the main method frame pointer return -(m_pVars[varIndex].offset + FUNCLET_STACK_ADJUSTMENT_OFFSET); } else +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS { return m_pVars[varIndex].offset; } @@ -762,7 +764,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydata [0]; - *ip++ = GetFuncletAdjustedVarOffset(ins, ins->sVars[0], forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins->sVars[0], forFunclet); *ip++ = numLabels; // Add relocation for each label for (int32_t i = 0; i < numLabels; i++) @@ -777,7 +779,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraysVars[i], forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins->sVars[i], forFunclet); if (ins->info.pTargetBB->nativeOffset >= 0) { @@ -809,8 +811,8 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydVar, forFunclet); - int srcOffset = GetFuncletAdjustedVarOffset(ins, ins->sVars[0], forFunclet); + int destOffset = GetFuncletAdjustedVarOffset(ins->dVar, forFunclet); + int srcOffset = GetFuncletAdjustedVarOffset(ins->sVars[0], forFunclet); if (srcOffset >= 0) srcOffset += fOffset; else @@ -836,8 +838,8 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydVar, forFunclet); - *ip++ = GetFuncletAdjustedVarOffset(ins, ins->sVars[0], forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins->dVar, forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins->sVars[0], forFunclet); } else { @@ -846,7 +848,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraydVar, forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins->dVar, forFunclet); if (g_interpOpSVars[opcode]) { @@ -858,7 +860,7 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArraysVars[i], forFunclet); + *ip++ = GetFuncletAdjustedVarOffset(ins->sVars[i], forFunclet); } } } @@ -2303,6 +2305,7 @@ void InterpCompiler::EmitLoadVar(int32_t var) InterpType interpType = m_pVars[var].interpType; CORINFO_CLASS_HANDLE clsHnd = m_pVars[var].clsHnd; +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS if (m_pCBB->clauseNonTryType == BBClauseFilter) { assert(m_pVars[var].ILGlobal); @@ -2312,6 +2315,7 @@ void InterpCompiler::EmitLoadVar(int32_t var) EmitLdind(interpType, clsHnd, m_pVars[var].offset); return; } +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS int32_t size = m_pVars[var].size; @@ -2338,6 +2342,7 @@ void InterpCompiler::EmitStoreVar(int32_t var) InterpType interpType = m_pVars[var].interpType; CHECK_STACK(1); +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS if (m_pCBB->clauseNonTryType == BBClauseFilter) { AddIns(INTOP_LOAD_FRAMEVAR); @@ -2346,6 +2351,7 @@ void InterpCompiler::EmitStoreVar(int32_t var) EmitStind(interpType, m_pVars[var].clsHnd, m_pVars[var].offset, true /* reverseSVarOrder */); return; } +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS #ifdef TARGET_64BIT // nint and int32 can be used interchangeably. Add implicit conversions. @@ -4006,6 +4012,7 @@ void InterpCompiler::EmitLdLocA(int32_t var) m_shadowCopyOfThisPointerActuallyNeeded = true; } +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS if (m_pCBB->clauseNonTryType == BBClauseFilter) { AddIns(INTOP_LOAD_FRAMEVAR); @@ -4019,6 +4026,7 @@ void InterpCompiler::EmitLdLocA(int32_t var) m_pLastNewIns->SetDVar(m_pStackPointer[-1].var); return; } +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS AddIns(INTOP_LDLOCA); m_pLastNewIns->SetSVar(var); diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 115c257eaa6b7b..39c464d1e856ec 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -780,7 +780,7 @@ class InterpCompiler void EmitCode(); int32_t* EmitBBCode(int32_t *ip, InterpBasicBlock *bb, TArray *relocs); int32_t* EmitCodeIns(int32_t *ip, InterpInst *pIns, TArray *relocs, bool forFunclet); - int32_t GetFuncletAdjustedVarOffset(InterpInst *ins, int varIndex, bool forFunclet); + int32_t GetFuncletAdjustedVarOffset(int varIndex, bool forFunclet); void PatchRelocations(TArray *relocs); InterpMethod* CreateInterpMethod(); void CreateBasicBlocks(CORINFO_METHOD_INFO* methodInfo); diff --git a/src/coreclr/interpreter/compileropt.cpp b/src/coreclr/interpreter/compileropt.cpp index 66069842f770ca..d934a61509e25e 100644 --- a/src/coreclr/interpreter/compileropt.cpp +++ b/src/coreclr/interpreter/compileropt.cpp @@ -299,7 +299,12 @@ void InterpCompiler::AllocOffsets() ForEachInsVar(pIns, pIns, &InterpCompiler::SetVarLiveRangeCB); insIndex++; } +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS int32_t currentOffset = m_totalVarsStackSize; +#else + // The first interp stack slot in a funclet is reserved to hold the pointer to the parent frame's stack, the rest is available for local vars + int32_t currentOffset = pBB->clauseNonTryType == BBClauseNone ? m_totalVarsStackSize : INTERP_STACK_SLOT_SIZE; +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS insIndex = 0; for (pIns = pBB->pFirstIns; pIns != NULL; pIns = pIns->pNext) { diff --git a/src/coreclr/interpreter/inc/interpretershared.h b/src/coreclr/interpreter/inc/interpretershared.h index c29f3f41f3abd6..bbc3e1454f75b7 100644 --- a/src/coreclr/interpreter/inc/interpretershared.h +++ b/src/coreclr/interpreter/inc/interpretershared.h @@ -178,6 +178,14 @@ enum class CalliFlags : int32_t PInvoke = 1 << 2, // The call is a PInvoke call }; +// FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS can be enabled to reuse the stack frame of the outer function for executing catch, finally and fault funclets +// This feature is not compatible with fully interpreted code, however, as while we execute the funclet we may overwrite the stack space of the DispatchEx +// function which is part of the actual funclet dispatch path. We may wish to re-enable this feature in the future if we run on a platform where +// we can guarantee that the DispatchEx and other related functions that make of the EH subsystem will not be interpreted. +//#define FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + +#ifndef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS #define FUNCLET_STACK_ADJUSTMENT_OFFSET 8 +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS #endif diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index b2f7c68696b696..279f57cfe18def 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -2176,7 +2176,7 @@ DWORD_PTR InterpreterCodeManager::CallFunclet(OBJECTREF throwable, void* pHandle StackVal retVal; - +#ifndef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS // Frame pointer for original method int8_t* originalStack; if (pOriginalFrame->IsFuncletFrame()) @@ -2189,6 +2189,11 @@ DWORD_PTR InterpreterCodeManager::CallFunclet(OBJECTREF throwable, void* pHandle } *(int8_t**)sp = originalStack; InterpreterFrameReporting frameReporting = isFilter ? InterpreterFrameReporting::FuncletNoReportGlobals : InterpreterFrameReporting::FuncletReportGlobals; +#else + sp = isFilter ? sp : pOriginalFrame->pStack; + InterpreterFrameReporting frameReporting = InterpreterFrameReporting::Normal; +#endif + frames.interpMethodContextFrame.ReInit(NULL, pOriginalFrame->startIp, (int8_t*)&retVal, frameReporting, sp); ExceptionClauseArgs exceptionClauseArgs; diff --git a/src/coreclr/vm/gcinfodecoder.cpp b/src/coreclr/vm/gcinfodecoder.cpp index 7fd2b76ad3c0c2..736390d2480993 100644 --- a/src/coreclr/vm/gcinfodecoder.cpp +++ b/src/coreclr/vm/gcinfodecoder.cpp @@ -2141,6 +2141,9 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( } else { +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + baseReg = pFrame->pStack; +#else if (pFrame->ShouldReportGlobals()) { if (pFrame->IsFuncletFrame()) @@ -2156,6 +2159,7 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( { return NULL; } +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS } _ASSERTE(baseReg); diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 631cb18deff885..1425a893910b8c 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -321,10 +321,16 @@ static void InterpBreakpoint() } #endif -#define LOCAL_VAR_ADDR(offset,type) offset < 0 ? (((type*)((*(uintptr_t*)stack) + (-offset)))) : ((type*)(stack + (offset))) -#define LOCAL_VAR(offset,type) (*(LOCAL_VAR_ADDR(offset, type))) +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS +#define LOCAL_VAR_ADDR(offset,type) ((type*)(stack + (offset))) +#else +#define LOCAL_VAR_ADDR(offset,type) ((offset) < 0 ? (((type*)((*(uintptr_t*)stack) + (-offset)))) : ((type*)(stack + (offset)))) +#endif + +#define LOCAL_VAR(offset,type) (*LOCAL_VAR_ADDR(offset, type)) #define NULL_CHECK(o) do { if ((o) == NULL) { COMPlusThrow(kNullReferenceException); } } while (0) + static OBJECTREF CreateMultiDimArray(MethodTable* arrayClass, int8_t* stack, int32_t dimsOffset, int numArgs) { int32_t* dims = (int32_t*)alloca(numArgs * sizeof(int32_t)); @@ -684,6 +690,20 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr } else { +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + // * Filter funclets are executed in the current frame, because they are executed + // in the first pass of EH when the frames between the current frame and the + // parent frame are still alive. All accesses to the locals and arguments + // in this case use the pExceptionClauseArgs->pFrame->pStack as a frame pointer. + // * Catch and finally funclets are running in the parent frame directly + + if (pExceptionClauseArgs->isFilter) + { + // Since filters run in their own frame, we need to clear the global variables + // so that GC doesn't pick garbage in variables that were not yet written to. + memset(pFrame->pStack, 0, pMethod->allocaSize); + } +#else // * Filter funclets are executed in the current frame, because they are executed // in the first pass of EH when the frames between the current frame and the // parent frame are still alive. All accesses to the locals and arguments @@ -691,6 +711,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // * Catch and finally funclets are also executed in the current frame, because // they are executed in the second pass of EH, when the DispatchEx function is still on // the managed stack. +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS // Start executing at the beginning of the exception clause ip = pExceptionClauseArgs->ip; @@ -3062,6 +3083,11 @@ do \ // Save the lowest SP in the current method so that we can identify it by that during stackwalk pInterpreterFrame->SetInterpExecMethodSP((TADDR)GetCurrentSP()); } + +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + // Set the frame to the same values as the caller frame. + pChildFrame->ReInit(pFrame, pFrame->startIp, NULL, InterpreterFrameReporting::Normal, pFrame->pStack); +#else // Store the address for the current frame's stack pointer into the new frame's stack space, if we are // adjusting the stack. if (pFrame->IsFuncletFrame()) @@ -3078,6 +3104,7 @@ do \ stack = pChildFrame->pStack; pThreadContext->pStackPointer = stack + pMethod->allocaSize; } +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS pFrame = pChildFrame; } assert (((size_t)pFrame->pStack % INTERP_STACK_ALIGNMENT) == 0); diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index 87a0398dc4270c..27931f61b28213 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -30,10 +30,12 @@ class InterpreterFrame; enum class InterpreterFrameReporting { Normal = 0, +#ifndef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS FuncletReportGlobals = 1, FuncletNoReportGlobals = 2, Mask = 3 +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS }; struct InterpMethodContextFrame @@ -50,11 +52,18 @@ struct InterpMethodContextFrame bool IsFuncletFrame() { LIMITED_METHOD_CONTRACT; +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + return false; +#else return ((size_t)pRetVal & (size_t)InterpreterFrameReporting::Mask) != (size_t)InterpreterFrameReporting::Normal; +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS } bool ShouldReportGlobals() { LIMITED_METHOD_CONTRACT; +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + return true; +#else switch ((InterpreterFrameReporting)((size_t)pRetVal & (size_t)InterpreterFrameReporting::Mask)) { case InterpreterFrameReporting::FuncletReportGlobals: @@ -66,12 +75,17 @@ struct InterpMethodContextFrame assert(false); return false; } +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS } int8_t* GetRetValAddr() { LIMITED_METHOD_CONTRACT; +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + return pRetVal; +#else return (int8_t*)((size_t)pRetVal & ~(size_t)InterpreterFrameReporting::Mask); +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS } int8_t* GetRetValAddr_KnownNormalReporting() @@ -86,7 +100,11 @@ struct InterpMethodContextFrame { this->pParent = pParent; this->startIp = startIp; +#ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS + this->pRetVal = pRetVal; +#else this->pRetVal = (int8_t*)((size_t)pRetVal | (size_t)reporting); +#endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS this->pStack = pStack; this->ip = NULL; } From 23435e74568459bc8353df3f6249c2b579d79679 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Sep 2025 14:40:35 -0700 Subject: [PATCH 3/3] Fix issues found in CI run --- src/coreclr/interpreter/compiler.cpp | 27 +++++++++++----- src/coreclr/vm/eetwain.cpp | 6 ++-- src/coreclr/vm/gcinfodecoder.cpp | 17 +++------- src/coreclr/vm/interpexec.h | 17 ++++++++++ src/tests/JIT/interpreter/Interpreter.cs | 40 ++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 24 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 3ddc179d2db1e5..72b2f71204d33b 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1175,17 +1175,28 @@ class InterpGcSlotAllocator void AllocateOrReuseGcSlot(uint32_t offsetBytes, GcSlotFlags flags) { - GcSlotId *pSlot = LocateGcSlotTableEntry(offsetBytes, flags); - bool allocateNewSlot = *pSlot == ((GcSlotId)-1); - - if (allocateNewSlot) + GcSlotId slot; + bool allocateNewSlot; + if (flags & GC_SLOT_UNTRACKED) { - // Important to pass GC_FRAMEREG_REL, the default is broken due to GET_CALLER_SP being unimplemented - *pSlot = m_encoder->GetStackSlotId(offsetBytes, flags, (flags & GC_SLOT_UNTRACKED) ? GC_FRAMEREG_REL : GC_SP_REL); + allocateNewSlot = true; + slot = m_encoder->GetStackSlotId(offsetBytes, flags, (flags & GC_SLOT_UNTRACKED) ? GC_FRAMEREG_REL : GC_SP_REL);; } else { - assert((flags & GC_SLOT_UNTRACKED) == 0); + GcSlotId *pSlot = LocateGcSlotTableEntry(offsetBytes, flags); + + allocateNewSlot = *pSlot == ((GcSlotId)-1); + + if (allocateNewSlot) + { + // Important to pass GC_FRAMEREG_REL, the default is broken due to GET_CALLER_SP being unimplemented + slot = *pSlot = m_encoder->GetStackSlotId(offsetBytes, flags, (flags & GC_SLOT_UNTRACKED) ? GC_FRAMEREG_REL : GC_SP_REL); + } + else + { + slot = *pSlot; + } } INTERP_DUMP( @@ -1194,7 +1205,7 @@ class InterpGcSlotAllocator (flags & GC_SLOT_UNTRACKED) ? "global " : "", (flags & GC_SLOT_INTERIOR) ? "interior " : "", (flags & GC_SLOT_PINNED) ? "pinned " : "", - *pSlot, + slot, offsetBytes ); } diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index 279f57cfe18def..b10ca4ee53b010 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -2620,8 +2620,7 @@ OBJECTREF InterpreterCodeManager::GetInstance(PREGDISPLAY pContext, EECodeInfo * pCodeInfo) { PTR_InterpMethodContextFrame frame = dac_cast(GetSP(pContext->pCurrentContext)); - TADDR baseStackSlot = dac_cast((uintptr_t)frame->pStack); - return *dac_cast(baseStackSlot); + return *dac_cast(frame->GetFunctionFrameStack()); } PTR_VOID InterpreterCodeManager::GetParamTypeArg(PREGDISPLAY pContext, @@ -2640,8 +2639,7 @@ PTR_VOID InterpreterCodeManager::GetParamTypeArg(PREGDISPLAY pContext, if (spOffsetGenericsContext != NO_GENERICS_INST_CONTEXT) { PTR_InterpMethodContextFrame frame = dac_cast(GetSP(pContext->pCurrentContext)); - TADDR baseStackSlot = dac_cast((uintptr_t)frame->pStack); - TADDR taSlot = (TADDR)( spOffsetGenericsContext + baseStackSlot ); + TADDR taSlot = (TADDR)(spOffsetGenericsContext + frame->GetFunctionFrameStack()); TADDR taExactGenericsToken = *PTR_TADDR(taSlot); return PTR_VOID(taExactGenericsToken); } diff --git a/src/coreclr/vm/gcinfodecoder.cpp b/src/coreclr/vm/gcinfodecoder.cpp index 736390d2480993..29386252090cf2 100644 --- a/src/coreclr/vm/gcinfodecoder.cpp +++ b/src/coreclr/vm/gcinfodecoder.cpp @@ -2129,11 +2129,11 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( InterpMethodContextFrame* pFrame = (InterpMethodContextFrame*)GetSP(pRD->pCurrentContext); _ASSERTE(pFrame); - int8_t* baseReg = nullptr; + TADDR baseReg = 0; if( GC_SP_REL == spBase ) { - baseReg = pFrame->pStack; + baseReg = dac_cast(pFrame->pStack); } else if( GC_CALLER_SP_REL == spBase ) { @@ -2142,18 +2142,11 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( else { #ifdef FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS - baseReg = pFrame->pStack; + baseReg = pFrame->GetFunctionFrameStack(); #else if (pFrame->ShouldReportGlobals()) { - if (pFrame->IsFuncletFrame()) - { - baseReg = *(int8_t**)pFrame->pStack + FUNCLET_STACK_ADJUSTMENT_OFFSET; - } - else - { - baseReg = pFrame->pStack; - } + baseReg = pFrame->GetFunctionFrameStack(); } else { @@ -2163,7 +2156,7 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( } _ASSERTE(baseReg); - pObjRef = (OBJECTREF*)(baseReg + spOffset); + pObjRef = dac_cast(baseReg + spOffset); return pObjRef; } diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index 27931f61b28213..98ec64c7a5ffea 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -78,6 +78,23 @@ struct InterpMethodContextFrame #endif // FEATURE_REUSE_INTERPRETER_STACK_FOR_NORMAL_FUNCLETS } + TADDR GetFunctionFrameStack() + { + LIMITED_METHOD_CONTRACT; + + TADDR baseStackSlot; + if (IsFuncletFrame()) + { + baseStackSlot = (*dac_cast((uintptr_t)pStack)) + FUNCLET_STACK_ADJUSTMENT_OFFSET; + } + else + { + baseStackSlot = dac_cast((uintptr_t)pStack); + } + + return baseStackSlot; + } + int8_t* GetRetValAddr() { LIMITED_METHOD_CONTRACT; diff --git a/src/tests/JIT/interpreter/Interpreter.cs b/src/tests/JIT/interpreter/Interpreter.cs index 64cecb59b8ac0f..6d94c36388212d 100644 --- a/src/tests/JIT/interpreter/Interpreter.cs +++ b/src/tests/JIT/interpreter/Interpreter.cs @@ -1004,6 +1004,8 @@ public static void TestExceptionHandling() TestFilterCatchNested(); TestFilterFailedCatchNested(); TestFilterCatchFinallyNested(); + TestGenericsCatchFinallyNested(); + TestGenericsCatchFinallyNested(); TestFilterFailedCatchFinallyNested(); TestFinallyBeforeCatch(); TestModifyAlias(); @@ -1221,6 +1223,15 @@ public static void Throw() throw null; // Simulating the throw operation } + public class GenericException : Exception + { + + } + public static void ThrowException() + { + throw new GenericException(); + } + public static void TestCatchCurrent() { int x = 0; @@ -1532,6 +1543,35 @@ public static void TestFilterCatchFinallyNested() } } + public static void TestGenericsCatchFinallyNested() + { + int x = 0; + try + { + Console.WriteLine("In try"); + } + finally + { + try + { + x *= 10; + x += 1; + ThrowException(); + } + catch (GenericException e) + { + x *= 10; + x += 2; + } + x *= 10; + x += 3; + } + if (x != 123) + { + throw null; + } + } + public static void TestFilterFailedCatchFinallyNested() { int x = 0;