From bf33f979f55020cdb61b09cd26693205be81c496 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Thu, 2 Apr 2026 14:59:14 +0100 Subject: [PATCH 1/9] func eval support initial --- src/coreclr/debug/ee/debugger.cpp | 125 ++++++++++++++++++++++++------ src/coreclr/debug/ee/debugger.h | 5 ++ src/coreclr/vm/dbginterface.h | 7 ++ src/coreclr/vm/interpexec.cpp | 8 ++ src/coreclr/vm/interpexec.h | 6 ++ 5 files changed, 126 insertions(+), 25 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 575392b318425d..1046725545c4e9 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -28,6 +28,10 @@ #include "dbgtransportsession.h" #endif // FEATURE_DBGIPC_TRANSPORT_VM +#ifdef FEATURE_INTERPRETER +#include "../../vm/interpexec.h" +#endif // FEATURE_INTERPRETER + #ifdef TEST_DATA_CONSISTENCY #include "datatest.h" #endif // TEST_DATA_CONSISTENCY @@ -9848,6 +9852,39 @@ void Debugger::UnloadClass(mdTypeDef classMetadataToken, } +#ifdef FEATURE_INTERPRETER +/****************************************************************************** + * Execute any pending func evals queued on the interpreter thread context. + * Called from the interpreter's INTOP_BREAKPOINT handler after the debugger + * callback returns. This keeps FuncEvalHijackWorker/DebuggerEval out of the + * interpreter execution loop. + ******************************************************************************/ +void Debugger::ExecutePendingInterpreterFuncEval(Thread* pThread) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } + CONTRACTL_END; + + InterpThreadContext* pInterpCtx = pThread->GetInterpThreadContext(); + if (pInterpCtx == NULL) + return; + + while (pInterpCtx->m_pPendingFuncEval != NULL) + { + DebuggerEval* pDE = (DebuggerEval*)pInterpCtx->m_pPendingFuncEval; + pInterpCtx->m_pPendingFuncEval = NULL; + + LOG((LF_CORDB, LL_INFO1000, "D::EPIFE: Executing pending func eval pDE=%p on thread %p\n", pDE, pThread)); + ::FuncEvalHijackWorker(pDE); + LOG((LF_CORDB, LL_INFO1000, "D::EPIFE: Func eval completed for pDE=%p\n", pDE)); + } +} +#endif // FEATURE_INTERPRETER + /****************************************************************************** * ******************************************************************************/ @@ -14334,11 +14371,25 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, return CORDBG_E_ILLEGAL_AT_GC_UNSAFE_POINT; } - if (filterContext != NULL && ::GetSP(filterContext) != ALIGN_DOWN(::GetSP(filterContext), STACK_ALIGN_SIZE)) +#ifdef FEATURE_INTERPRETER + // For interpreter threads, the filter context contains synthetic values (IP = bytecode address, + // SP = InterpMethodContextFrame*, FP = stack pointer) — not real native register values. + // Skip the SP alignment check since it only applies to native stack pointers. + bool fIsInterpreterThread = false; + if (filterContext != NULL) { - // SP is not aligned, we cannot do a FuncEval here - LOG((LF_CORDB, LL_INFO1000, "D::FES SP is unaligned")); - return CORDBG_E_FUNC_EVAL_BAD_START_POINT; + EECodeInfo codeInfo((PCODE)GetIP(filterContext)); + fIsInterpreterThread = codeInfo.IsInterpretedCode(); + } + if (!fIsInterpreterThread) +#endif // FEATURE_INTERPRETER + { + if (filterContext != NULL && ::GetSP(filterContext) != ALIGN_DOWN(::GetSP(filterContext), STACK_ALIGN_SIZE)) + { + // SP is not aligned, we cannot do a FuncEval here + LOG((LF_CORDB, LL_INFO1000, "D::FES SP is unaligned")); + return CORDBG_E_FUNC_EVAL_BAD_START_POINT; + } } // Allocate the breakpoint instruction info for the debugger info in executable memory. @@ -14401,40 +14452,63 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, { _ASSERTE(filterContext != NULL); - ::SetIP(filterContext, (UINT_PTR)GetEEFuncEntryPoint(::FuncEvalHijack)); +#ifdef FEATURE_INTERPRETER + // For interpreter threads, we cannot hijack the native CPU context because the interpreter + // manages execution through its own bytecode dispatch loop. Instead, we store the DebuggerEval + // on the interpreter's thread context. The INTOP_BREAKPOINT handler will pick it up after + // the debugger callback returns. + if (fIsInterpreterThread) + { + InterpThreadContext* pInterpCtx = pThread->GetInterpThreadContext(); + _ASSERTE(pInterpCtx != NULL); + + pDE->m_evalDuringException = true; + _ASSERTE(pInterpCtx->m_pPendingFuncEval == NULL); + pInterpCtx->m_pPendingFuncEval = pDE; + + LOG((LF_CORDB, LL_INFO1000, "D::FES: Interpreter func eval setup for pDE:%p on thread %p\n", pDE, pThread)); + + // No context modification needed — interpreter checks the pending flag on resume. + // No IncThreadsAtUnsafePlaces — stack remains walkable (no context change). + } + else +#endif // FEATURE_INTERPRETER + { + ::SetIP(filterContext, (UINT_PTR)GetEEFuncEntryPoint(::FuncEvalHijack)); - // Don't be fooled into thinking you can push things onto the thread's stack now. If the thread is stopped at a - // breakpoint or from a single step, then its really suspended in the SEH filter. ESP in the thread's CONTEXT, - // therefore, points into the middle of the thread's current stack. So we pass things we need in the hijack in - // the thread's registers. + // Don't be fooled into thinking you can push things onto the thread's stack now. If the thread is stopped at a + // breakpoint or from a single step, then its really suspended in the SEH filter. ESP in the thread's CONTEXT, + // therefore, points into the middle of the thread's current stack. So we pass things we need in the hijack in + // the thread's registers. - // Set the first argument to point to the DebuggerEval. + // Set the first argument to point to the DebuggerEval. #if defined(TARGET_X86) - filterContext->Eax = (DWORD)pDE; + filterContext->Eax = (DWORD)pDE; #elif defined(TARGET_AMD64) #ifdef UNIX_AMD64_ABI - filterContext->Rdi = (SIZE_T)pDE; + filterContext->Rdi = (SIZE_T)pDE; #else // UNIX_AMD64_ABI - filterContext->Rcx = (SIZE_T)pDE; + filterContext->Rcx = (SIZE_T)pDE; #endif // !UNIX_AMD64_ABI #elif defined(TARGET_ARM) - filterContext->R0 = (DWORD)pDE; + filterContext->R0 = (DWORD)pDE; #elif defined(TARGET_ARM64) - filterContext->X0 = (SIZE_T)pDE; + filterContext->X0 = (SIZE_T)pDE; #elif defined(TARGET_RISCV64) - filterContext->A0 = (SIZE_T)pDE; + filterContext->A0 = (SIZE_T)pDE; #elif defined(TARGET_LOONGARCH64) - filterContext->A0 = (SIZE_T)pDE; + filterContext->A0 = (SIZE_T)pDE; #else - PORTABILITY_ASSERT("Debugger::FuncEvalSetup is not implemented on this platform."); + PORTABILITY_ASSERT("Debugger::FuncEvalSetup is not implemented on this platform."); #endif - // - // To prevent GCs until the func-eval gets a chance to run, we increment the counter here. - // We only need to do this if we have changed the filter CONTEXT, since the stack will be unwalkable - // in this case. - // - g_pDebugger->IncThreadsAtUnsafePlaces(); + // + // To prevent GCs until the func-eval gets a chance to run, we increment the counter here. + // We only need to do this if we have changed the filter CONTEXT, since the stack will be unwalkable + // in this case. + // + g_pDebugger->IncThreadsAtUnsafePlaces(); + } } else { @@ -16131,7 +16205,8 @@ void FuncEvalFrame::UpdateRegDisplay_Impl(const PREGDISPLAY pRD, bool updateFloa SUPPORTS_DAC; DebuggerEval * pDE = GetDebuggerEval(); - // No context to update if we're doing a func eval from within exception processing. + // No context to update if we're doing a func eval from within exception processing + // or from interpreter code (both use m_evalDuringException to share the direct-send path). if (pDE->m_evalDuringException) { return; diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index f1ed7e1ea70960..bc24d175adc27e 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -2575,6 +2575,11 @@ class Debugger : public DebugInterface #ifndef DACCESS_COMPILE void MulticastTraceNextStep(DELEGATEREF pbDel, INT32 count); void ExternalMethodFixupNextStep(PCODE address); + +#ifdef FEATURE_INTERPRETER + void ExecutePendingInterpreterFuncEval(Thread* pThread); +#endif // FEATURE_INTERPRETER + #endif #ifdef DACCESS_COMPILE diff --git a/src/coreclr/vm/dbginterface.h b/src/coreclr/vm/dbginterface.h index 75c3e387175c89..c7337dbde27fda 100644 --- a/src/coreclr/vm/dbginterface.h +++ b/src/coreclr/vm/dbginterface.h @@ -392,6 +392,13 @@ class DebugInterface virtual HRESULT IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BOOL *pResult) = 0; virtual void MulticastTraceNextStep(DELEGATEREF pbDel, INT32 count) = 0; virtual void ExternalMethodFixupNextStep(PCODE address) = 0; + +#ifdef FEATURE_INTERPRETER + // Execute any pending func evals queued on the interpreter thread context. + // Called from the interpreter's INTOP_BREAKPOINT handler after the debugger callback returns. + virtual void ExecutePendingInterpreterFuncEval(Thread* pThread) = 0; +#endif // FEATURE_INTERPRETER + #endif //DACCESS_COMPILE }; diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 3f4c2279c4834d..ce0f69239976df 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -639,6 +639,7 @@ InterpThreadContext::InterpThreadContext() #ifdef DEBUGGING_SUPPORTED m_bypassAddress = NULL; m_bypassOpcode = 0; + m_pPendingFuncEval = NULL; #endif // DEBUGGING_SUPPORTED } @@ -1259,6 +1260,13 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Hit breakpoint at IP %p\n", ip)); InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame); + // Execute any pending func evals queued by the debugger's FuncEvalSetup. + if (pThreadContext->m_pPendingFuncEval != NULL && g_pDebugInterface != NULL) + { + Thread *pThread = GetThread(); + g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); + } + int32_t bypassOpcode = 0; // After debugger callback, check if bypass was set on the thread context diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index 48c4c2c1a6f8ff..35ab8d2185a1cf 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -69,6 +69,12 @@ struct InterpThreadContext const int32_t *m_bypassAddress; // Address of breakpoint to bypass (NULL = no bypass) int32_t m_bypassOpcode; // Original opcode to execute instead of INTOP_BREAKPOINT + // Pending func eval. When the debugger requests a function evaluation on a thread + // stopped in interpreter code, FuncEvalSetup stores the DebuggerEval* here instead + // of hijacking the native CPU context (which doesn't work for interpreter threads). + // The INTOP_BREAKPOINT handler checks this after the debugger callback returns. + void *m_pPendingFuncEval; // DebuggerEval* (void* to avoid header dependency) + void SetBypass(const int32_t* address, int32_t opcode) { _ASSERTE(m_bypassAddress == NULL); From f38e985f264650028c387ab3706a0c2596e18eca Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Thu, 2 Apr 2026 16:30:52 +0100 Subject: [PATCH 2/9] set filter context for func eval --- src/coreclr/vm/interpexec.cpp | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index ce0f69239976df..1dc21af12077b2 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1261,10 +1261,34 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame); // Execute any pending func evals queued by the debugger's FuncEvalSetup. + // DispatchNativeException clears the filter context before returning, but + // FuncEvalHijackWorker needs it to identify the thread as stopped in managed + // code and at a GC-safe point. Set a synthetic filter context here so that + // nested/subsequent func evals can pass the safety checks in FuncEvalSetup. if (pThreadContext->m_pPendingFuncEval != NULL && g_pDebugInterface != NULL) { Thread *pThread = GetThread(); - g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); + + CONTEXT funcEvalCtx; + memset(&funcEvalCtx, 0, sizeof(CONTEXT)); + funcEvalCtx.ContextFlags = CONTEXT_FULL; + SetSP(&funcEvalCtx, (DWORD64)pFrame); + SetFP(&funcEvalCtx, (DWORD64)stack); + SetIP(&funcEvalCtx, (DWORD64)ip); + SetFirstArgReg(&funcEvalCtx, dac_cast(pInterpreterFrame)); + + pThread->SetFilterContext(&funcEvalCtx); + EX_TRY + { + g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); + } + EX_CATCH + { + pThread->SetFilterContext(NULL); + EX_RETHROW; + } + EX_END_CATCH + pThread->SetFilterContext(NULL); } int32_t bypassOpcode = 0; From 5bbd2abc6b967c9b3c882f787c3671addc7d9a6e Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Mon, 6 Apr 2026 12:58:58 +0100 Subject: [PATCH 3/9] Skip allocating bpInfoSegmentRX for interpreter --- src/coreclr/debug/ee/debugger.cpp | 66 ++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 1046725545c4e9..493386435339bd 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -1326,22 +1326,37 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval { WRAPPER_NO_CONTRACT; + // bpInfoSegmentRX is NULL only for interpreter func evals — the interpreter signals completion + // directly via FuncEvalComplete, not the native breakpoint trap mechanism. +#ifdef FEATURE_INTERPRETER + _ASSERTE(bpInfoSegmentRX != NULL || (pContext != NULL && EECodeInfo((PCODE)GetIP(pContext)).IsInterpretedCode())); +#else + _ASSERTE(bpInfoSegmentRX != NULL); +#endif + if (bpInfoSegmentRX != NULL) + { #if !defined(DBI_COMPILE) && !defined(DACCESS_COMPILE) && defined(HOST_OSX) && defined(HOST_ARM64) - ExecutableWriterHolder bpInfoSegmentWriterHolder(bpInfoSegmentRX, sizeof(DebuggerEvalBreakpointInfoSegment)); - DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRW = bpInfoSegmentWriterHolder.GetRW(); + ExecutableWriterHolder bpInfoSegmentWriterHolder(bpInfoSegmentRX, sizeof(DebuggerEvalBreakpointInfoSegment)); + DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRW = bpInfoSegmentWriterHolder.GetRW(); #else // !DBI_COMPILE && !DACCESS_COMPILE && HOST_OSX && HOST_ARM64 - DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRW = bpInfoSegmentRX; + DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRW = bpInfoSegmentRX; #endif // !DBI_COMPILE && !DACCESS_COMPILE && HOST_OSX && HOST_ARM64 - new (bpInfoSegmentRW) DebuggerEvalBreakpointInfoSegment(this); - m_bpInfoSegment = bpInfoSegmentRX; + new (bpInfoSegmentRW) DebuggerEvalBreakpointInfoSegment(this); + m_bpInfoSegment = bpInfoSegmentRX; - // This must be non-zero so that the saved opcode is non-zero, and on IA64 we want it to be 0x16 - // so that we can have a breakpoint instruction in any slot in the bundle. - bpInfoSegmentRW->m_breakpointInstruction[0] = 0x16; + // This must be non-zero so that the saved opcode is non-zero, and on IA64 we want it to be 0x16 + // so that we can have a breakpoint instruction in any slot in the bundle. + bpInfoSegmentRW->m_breakpointInstruction[0] = 0x16; #if defined(TARGET_ARM) - USHORT *bp = (USHORT*)&m_bpInfoSegment->m_breakpointInstruction; - *bp = CORDbg_BREAK_INSTRUCTION; + USHORT *bp = (USHORT*)&m_bpInfoSegment->m_breakpointInstruction; + *bp = CORDbg_BREAK_INSTRUCTION; #endif // TARGET_ARM + } + else + { + m_bpInfoSegment = NULL; + } + m_thread = pEvalInfo->vmThreadToken.GetRawPtr(); m_evalType = pEvalInfo->funcEvalType; m_methodToken = pEvalInfo->funcMetadataToken; @@ -14393,16 +14408,25 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, } // Allocate the breakpoint instruction info for the debugger info in executable memory. - DebuggerHeap *pHeap = g_pDebugger->GetInteropSafeExecutableHeap_NoThrow(); - if (pHeap == NULL) + // Interpreter func evals don't need this — completion is signaled directly via + // FuncEvalComplete, not a native breakpoint trap. Skip the allocation to avoid + // requiring executable memory on platforms where it's unavailable (e.g. iOS). + DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRX = NULL; +#ifdef FEATURE_INTERPRETER + if (!fIsInterpreterThread) +#endif // FEATURE_INTERPRETER { - return E_OUTOFMEMORY; - } + DebuggerHeap *pHeap = g_pDebugger->GetInteropSafeExecutableHeap_NoThrow(); + if (pHeap == NULL) + { + return E_OUTOFMEMORY; + } - DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRX = (DebuggerEvalBreakpointInfoSegment*)pHeap->Alloc(sizeof(DebuggerEvalBreakpointInfoSegment)); - if (bpInfoSegmentRX == NULL) - { - return E_OUTOFMEMORY; + bpInfoSegmentRX = (DebuggerEvalBreakpointInfoSegment*)pHeap->Alloc(sizeof(DebuggerEvalBreakpointInfoSegment)); + if (bpInfoSegmentRX == NULL) + { + return E_OUTOFMEMORY; + } } // Create a DebuggerEval to hold info about this eval while its in progress. Constructor copies the thread's @@ -14413,7 +14437,13 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, { return E_OUTOFMEMORY; } +#ifdef FEATURE_INTERPRETER + // Interpreter func evals skip bpInfoSegment — completion is signaled directly + // via FuncEvalComplete, not the native breakpoint trap. Only call Init() for JIT func evals. + else if (!fIsInterpreterThread && !pDE->Init()) +#else else if (!pDE->Init()) +#endif { // We fail to change the m_breakpointInstruction field to PAGE_EXECUTE_READWRITE permission. return E_FAIL; From ccae94315841fa0782f20721212df45373e99213 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Mon, 6 Apr 2026 13:23:26 +0100 Subject: [PATCH 4/9] clarify wording around pending func evals --- src/coreclr/debug/ee/debugger.cpp | 4 ++-- src/coreclr/vm/dbginterface.h | 2 +- src/coreclr/vm/interpexec.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 493386435339bd..f4c8a9133bb368 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -9869,7 +9869,7 @@ void Debugger::UnloadClass(mdTypeDef classMetadataToken, #ifdef FEATURE_INTERPRETER /****************************************************************************** - * Execute any pending func evals queued on the interpreter thread context. + * Execute the pending func eval on the interpreter thread context, if any. * Called from the interpreter's INTOP_BREAKPOINT handler after the debugger * callback returns. This keeps FuncEvalHijackWorker/DebuggerEval out of the * interpreter execution loop. @@ -9888,7 +9888,7 @@ void Debugger::ExecutePendingInterpreterFuncEval(Thread* pThread) if (pInterpCtx == NULL) return; - while (pInterpCtx->m_pPendingFuncEval != NULL) + if (pInterpCtx->m_pPendingFuncEval != NULL) { DebuggerEval* pDE = (DebuggerEval*)pInterpCtx->m_pPendingFuncEval; pInterpCtx->m_pPendingFuncEval = NULL; diff --git a/src/coreclr/vm/dbginterface.h b/src/coreclr/vm/dbginterface.h index c7337dbde27fda..47e40138437dbd 100644 --- a/src/coreclr/vm/dbginterface.h +++ b/src/coreclr/vm/dbginterface.h @@ -394,7 +394,7 @@ class DebugInterface virtual void ExternalMethodFixupNextStep(PCODE address) = 0; #ifdef FEATURE_INTERPRETER - // Execute any pending func evals queued on the interpreter thread context. + // Execute the pending func eval on the interpreter thread context, if any. // Called from the interpreter's INTOP_BREAKPOINT handler after the debugger callback returns. virtual void ExecutePendingInterpreterFuncEval(Thread* pThread) = 0; #endif // FEATURE_INTERPRETER diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 1dc21af12077b2..bb0a68f00caff7 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1260,7 +1260,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Hit breakpoint at IP %p\n", ip)); InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame); - // Execute any pending func evals queued by the debugger's FuncEvalSetup. + // Execute the pending func eval set by the debugger's FuncEvalSetup, if any. // DispatchNativeException clears the filter context before returning, but // FuncEvalHijackWorker needs it to identify the thread as stopped in managed // code and at a GC-safe point. Set a synthetic filter context here so that From 10d728c04094aefa68fdddf88838dd2b440dc8d0 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Tue, 7 Apr 2026 15:59:23 +0200 Subject: [PATCH 5/9] Move FuncEval execution to InterpBreakpoint --- src/coreclr/vm/interpexec.cpp | 52 ++++++++++++++--------------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index bb0a68f00caff7..bc1034e61e473f 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -703,6 +703,27 @@ static void InterpBreakpoint(const int32_t *ip, const InterpMethodContextFrame * &ctx, STATUS_BREAKPOINT, pThread); + + // Execute the pending func eval set by the debugger's FuncEvalSetup, if any. + // DispatchNativeException clears the filter context before returning. + // Re-set it as filter context so FuncEvalHijackWorker can pass the managed-code / GC-safe-point checks. + // This expects that the ctx was not modified by the debugger. + InterpThreadContext *pThreadContext = pThread->GetInterpThreadContext(); + while (pThreadContext != NULL && pThreadContext->m_pPendingFuncEval != NULL) + { + pThread->SetFilterContext(&ctx); + EX_TRY + { + g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); + } + EX_CATCH + { + pThread->SetFilterContext(NULL); + EX_RETHROW; + } + EX_END_CATCH + pThread->SetFilterContext(NULL); + } } } #endif // DEBUGGING_SUPPORTED @@ -1260,37 +1281,6 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Hit breakpoint at IP %p\n", ip)); InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame); - // Execute the pending func eval set by the debugger's FuncEvalSetup, if any. - // DispatchNativeException clears the filter context before returning, but - // FuncEvalHijackWorker needs it to identify the thread as stopped in managed - // code and at a GC-safe point. Set a synthetic filter context here so that - // nested/subsequent func evals can pass the safety checks in FuncEvalSetup. - if (pThreadContext->m_pPendingFuncEval != NULL && g_pDebugInterface != NULL) - { - Thread *pThread = GetThread(); - - CONTEXT funcEvalCtx; - memset(&funcEvalCtx, 0, sizeof(CONTEXT)); - funcEvalCtx.ContextFlags = CONTEXT_FULL; - SetSP(&funcEvalCtx, (DWORD64)pFrame); - SetFP(&funcEvalCtx, (DWORD64)stack); - SetIP(&funcEvalCtx, (DWORD64)ip); - SetFirstArgReg(&funcEvalCtx, dac_cast(pInterpreterFrame)); - - pThread->SetFilterContext(&funcEvalCtx); - EX_TRY - { - g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); - } - EX_CATCH - { - pThread->SetFilterContext(NULL); - EX_RETHROW; - } - EX_END_CATCH - pThread->SetFilterContext(NULL); - } - int32_t bypassOpcode = 0; // After debugger callback, check if bypass was set on the thread context From 03388a62f89f1a58cc9fd954e79056b2e6b84e94 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Wed, 15 Apr 2026 18:07:05 +0200 Subject: [PATCH 6/9] Rename m_evalDuringException to m_evalUsesHijack and update related logic for clarity in DebuggerEval handling --- src/coreclr/debug/ee/debugger.cpp | 68 ++++++++----------- src/coreclr/debug/ee/debugger.h | 6 +- src/coreclr/debug/ee/funceval.cpp | 8 +-- .../vm/datadescriptor/datadescriptor.inc | 2 +- src/coreclr/vm/interpexec.cpp | 57 +++++++++++----- src/coreclr/vm/interpexec.h | 6 -- .../FrameHandling/BaseFrameHandler.cs | 4 +- .../FrameHandling/X86FrameHandler.cs | 4 +- .../Data/Frames/DebuggerEval.cs | 4 +- 9 files changed, 86 insertions(+), 73 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index a1f14c0c5c61aa..2a8e1fc03aa555 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -1382,7 +1382,7 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval m_aborting = FE_ABORT_NONE; m_aborted = false; m_completed = false; - m_evalDuringException = fInException; + m_evalUsesHijack = !fInException; m_retValueBoxing = Debugger::NoValueTypeBoxing; m_vmObjectHandle = VMPTR_OBJECTHANDLE::NullPtr(); @@ -7709,7 +7709,7 @@ void Debugger::ProcessAnyPendingEvals(Thread *pThread) { DebuggerEval *pDE = pfe->pDE; - _ASSERTE(pDE->m_evalDuringException); + _ASSERTE(!pDE->m_evalUsesHijack); _ASSERTE(pDE->m_thread == GetThreadNULLOk()); // Remove the pending eval from the hash. This ensures that if we take a first chance exception during the eval @@ -9870,10 +9870,10 @@ void Debugger::UnloadClass(mdTypeDef classMetadataToken, #ifdef FEATURE_INTERPRETER /****************************************************************************** - * Execute the pending func eval on the interpreter thread context, if any. - * Called from the interpreter's INTOP_BREAKPOINT handler after the debugger - * callback returns. This keeps FuncEvalHijackWorker/DebuggerEval out of the - * interpreter execution loop. + * Execute pending func evals on the interpreter thread. Called from the + * interpreter's INTOP_BREAKPOINT handler after the debugger callback returns. + * Routes through ProcessAnyPendingEvals to share the dispatch logic with the + * exception-time func-eval path. ******************************************************************************/ void Debugger::ExecutePendingInterpreterFuncEval(Thread* pThread) { @@ -9885,19 +9885,7 @@ void Debugger::ExecutePendingInterpreterFuncEval(Thread* pThread) } CONTRACTL_END; - InterpThreadContext* pInterpCtx = pThread->GetInterpThreadContext(); - if (pInterpCtx == NULL) - return; - - if (pInterpCtx->m_pPendingFuncEval != NULL) - { - DebuggerEval* pDE = (DebuggerEval*)pInterpCtx->m_pPendingFuncEval; - pInterpCtx->m_pPendingFuncEval = NULL; - - LOG((LF_CORDB, LL_INFO1000, "D::EPIFE: Executing pending func eval pDE=%p on thread %p\n", pDE, pThread)); - ::FuncEvalHijackWorker(pDE); - LOG((LF_CORDB, LL_INFO1000, "D::EPIFE: Func eval completed for pDE=%p\n", pDE)); - } + ProcessAnyPendingEvals(pThread); } #endif // FEATURE_INTERPRETER @@ -14397,6 +14385,13 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, EECodeInfo codeInfo((PCODE)GetIP(filterContext)); fIsInterpreterThread = codeInfo.IsInterpretedCode(); } + else if (!fInException && pThread->GetInterpThreadContext() != NULL) + { + // The thread is an interpreter thread but not at a breakpoint (no filter context). + // Non-exception evals on interpreter threads require a breakpoint stop. + LOG((LF_CORDB, LL_INFO1000, "D::FES: Func eval requested on non-breakpoint interpreter thread\n")); + return CORDBG_E_FUNC_EVAL_BAD_START_POINT; + } if (!fIsInterpreterThread) #endif // FEATURE_INTERPRETER { @@ -14438,13 +14433,7 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, { return E_OUTOFMEMORY; } -#ifdef FEATURE_INTERPRETER - // Interpreter func evals skip bpInfoSegment — completion is signaled directly - // via FuncEvalComplete, not the native breakpoint trap. Only call Init() for JIT func evals. - else if (!fIsInterpreterThread && !pDE->Init()) -#else else if (!pDE->Init()) -#endif { // We fail to change the m_breakpointInstruction field to PAGE_EXECUTE_READWRITE permission. return E_FAIL; @@ -14485,21 +14474,24 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, #ifdef FEATURE_INTERPRETER // For interpreter threads, we cannot hijack the native CPU context because the interpreter - // manages execution through its own bytecode dispatch loop. Instead, we store the DebuggerEval - // on the interpreter's thread context. The INTOP_BREAKPOINT handler will pick it up after - // the debugger callback returns. + // manages execution through its own bytecode dispatch loop. Instead, we queue the DebuggerEval + // in the pending evals table. The INTOP_BREAKPOINT handler will call ProcessAnyPendingEvals + // after the debugger callback returns. if (fIsInterpreterThread) { - InterpThreadContext* pInterpCtx = pThread->GetInterpThreadContext(); - _ASSERTE(pInterpCtx != NULL); + pDE->m_evalUsesHijack = false; - pDE->m_evalDuringException = true; - _ASSERTE(pInterpCtx->m_pPendingFuncEval == NULL); - pInterpCtx->m_pPendingFuncEval = pDE; + HRESULT hr = CheckInitPendingFuncEvalTable(); + if (FAILED(hr)) + { + DeleteInteropSafeExecutable(pDE); + return hr; + } + GetPendingEvals()->AddPendingEval(pDE->m_thread, pDE); LOG((LF_CORDB, LL_INFO1000, "D::FES: Interpreter func eval setup for pDE:%p on thread %p\n", pDE, pThread)); - // No context modification needed — interpreter checks the pending flag on resume. + // No context modification needed — interpreter checks pending evals on resume. // No IncThreadsAtUnsafePlaces — stack remains walkable (no context change). } else @@ -16204,7 +16196,7 @@ unsigned FuncEvalFrame::GetFrameAttribs_Impl(void) { LIMITED_METHOD_DAC_CONTRACT; - if (GetDebuggerEval()->m_evalDuringException) + if (!GetDebuggerEval()->m_evalUsesHijack) { return FRAME_ATTR_NONE; } @@ -16218,7 +16210,7 @@ TADDR FuncEvalFrame::GetReturnAddressPtr_Impl() { LIMITED_METHOD_DAC_CONTRACT; - if (GetDebuggerEval()->m_evalDuringException) + if (!GetDebuggerEval()->m_evalUsesHijack) { return (TADDR)NULL; } @@ -16237,8 +16229,8 @@ void FuncEvalFrame::UpdateRegDisplay_Impl(const PREGDISPLAY pRD, bool updateFloa DebuggerEval * pDE = GetDebuggerEval(); // No context to update if we're doing a func eval from within exception processing - // or from interpreter code (both use m_evalDuringException to share the direct-send path). - if (pDE->m_evalDuringException) + // or from interpreter code (both skip the hijack path). + if (!pDE->m_evalUsesHijack) { return; } diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index bc24d175adc27e..8512a2329f458a 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -3487,7 +3487,7 @@ class DebuggerEval FUNC_EVAL_ABORT_TYPE m_aborting; // Has an abort been requested, and what type. bool m_aborted; // Was this eval aborted bool m_completed; // Is the eval complete - successfully or by aborting - bool m_evalDuringException; + bool m_evalUsesHijack; VMPTR_OBJECTHANDLE m_vmObjectHandle; TypeHandle m_ownerTypeHandle; DebuggerEvalBreakpointInfoSegment* m_bpInfoSegment; @@ -3496,6 +3496,10 @@ class DebuggerEval bool Init() { + // Interpreter func evals and exception-time evals don't use the breakpoint instruction segment, so skip the executability check. + if (!m_evalUsesHijack) + return true; + _ASSERTE(DbgIsExecutable(&m_bpInfoSegment->m_breakpointInstruction, sizeof(m_bpInfoSegment->m_breakpointInstruction))); return true; } diff --git a/src/coreclr/debug/ee/funceval.cpp b/src/coreclr/debug/ee/funceval.cpp index f251de3016ce34..d40c71c6d4daff 100644 --- a/src/coreclr/debug/ee/funceval.cpp +++ b/src/coreclr/debug/ee/funceval.cpp @@ -3822,7 +3822,7 @@ void * STDCALL FuncEvalHijackWorker(DebuggerEval *pDE) #endif #endif - if (!pDE->m_evalDuringException) + if (pDE->m_evalUsesHijack) { // // From this point forward we use FORBID regions to guard against GCs. @@ -3842,7 +3842,7 @@ void * STDCALL FuncEvalHijackWorker(DebuggerEval *pDE) if (filterContext) { - _ASSERTE(pDE->m_evalDuringException); + _ASSERTE(!pDE->m_evalUsesHijack); g_pEEInterface->SetThreadFilterContext(pDE->m_thread, NULL); } @@ -3901,7 +3901,7 @@ void * STDCALL FuncEvalHijackWorker(DebuggerEval *pDE) // Codepitching can hijack our frame's return address. That means that we'll need to update PC in our saved context // so that when its restored, its like we've returned to the codepitching hijack. At this point, the old value of // EIP is worthless anyway. - if (!pDE->m_evalDuringException) + if (pDE->m_evalUsesHijack) { SetIP(&pDE->m_context, (SIZE_T)FEFrame.GetReturnAddress()); } @@ -3913,7 +3913,7 @@ void * STDCALL FuncEvalHijackWorker(DebuggerEval *pDE) void *dest = NULL; - if (!pDE->m_evalDuringException) + if (pDE->m_evalUsesHijack) { // Signal to the helper thread that we're done with our func eval. Start by creating a DebuggerFuncEvalComplete // object. Give it an address at which to create the patch, which is a chunk of memory specified by our diff --git a/src/coreclr/vm/datadescriptor/datadescriptor.inc b/src/coreclr/vm/datadescriptor/datadescriptor.inc index b85488e42c2a88..04704f8a1d1e07 100644 --- a/src/coreclr/vm/datadescriptor/datadescriptor.inc +++ b/src/coreclr/vm/datadescriptor/datadescriptor.inc @@ -962,7 +962,7 @@ CDAC_TYPE_END(FuncEvalFrame) CDAC_TYPE_BEGIN(DebuggerEval) CDAC_TYPE_SIZE(sizeof(DebuggerEval)) CDAC_TYPE_FIELD(DebuggerEval, EXTERN_TYPE(Context), TargetContext, offsetof(DebuggerEval, m_context)) -CDAC_TYPE_FIELD(DebuggerEval, T_BOOL, EvalDuringException, offsetof(DebuggerEval, m_evalDuringException)) +CDAC_TYPE_FIELD(DebuggerEval, T_BOOL, EvalUsesHijack, offsetof(DebuggerEval, m_evalUsesHijack)) CDAC_TYPE_END(DebuggerEval) #endif // DEBUGGING_SUPPORTED diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index bc1034e61e473f..334694c511946a 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -639,7 +639,6 @@ InterpThreadContext::InterpThreadContext() #ifdef DEBUGGING_SUPPORTED m_bypassAddress = NULL; m_bypassOpcode = 0; - m_pPendingFuncEval = NULL; #endif // DEBUGGING_SUPPORTED } @@ -671,7 +670,7 @@ static void InterpHalt() #endif // DEBUG #ifdef DEBUGGING_SUPPORTED -static void InterpBreakpoint(const int32_t *ip, const InterpMethodContextFrame *pFrame, const int8_t *stack, InterpreterFrame *pInterpreterFrame) +static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodContextFrame *pFrame, const int8_t *stack, InterpreterFrame *pInterpreterFrame) { Thread *pThread = GetThread(); if (pThread != NULL && g_pDebugInterface != NULL) @@ -704,27 +703,40 @@ static void InterpBreakpoint(const int32_t *ip, const InterpMethodContextFrame * STATUS_BREAKPOINT, pThread); - // Execute the pending func eval set by the debugger's FuncEvalSetup, if any. + // Execute pending func evals set by the debugger's FuncEvalSetup, if any. // DispatchNativeException clears the filter context before returning. // Re-set it as filter context so FuncEvalHijackWorker can pass the managed-code / GC-safe-point checks. - // This expects that the ctx was not modified by the debugger. InterpThreadContext *pThreadContext = pThread->GetInterpThreadContext(); - while (pThreadContext != NULL && pThreadContext->m_pPendingFuncEval != NULL) + + // Save and restore bypass state around func eval execution. + // Func eval triggers its own INTOP_BREAKPOINT callbacks which would + // overwrite the bypass that was set for the original breakpoint. + const int32_t *savedBypassAddress = pThreadContext->m_bypassAddress; + int32_t savedBypassOpcode = pThreadContext->m_bypassOpcode; + + pThread->SetFilterContext(&ctx); + EX_TRY + { + g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); + } + EX_CATCH { - pThread->SetFilterContext(&ctx); - EX_TRY - { - g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); - } - EX_CATCH - { - pThread->SetFilterContext(NULL); - EX_RETHROW; - } - EX_END_CATCH pThread->SetFilterContext(NULL); + EX_RETHROW; } + EX_END_CATCH + pThread->SetFilterContext(NULL); + + // Restore bypass state that may have been overwritten during func eval. + pThreadContext->m_bypassAddress = savedBypassAddress; + pThreadContext->m_bypassOpcode = savedBypassOpcode; + + // The debugger may have modified the IP via SetIP (e.g. the setip command). + // Return the potentially updated IP so the interpreter can resume from the + // new position. + return (const int32_t*)(TADDR)GetIP(&ctx); } + return ip; } #endif // DEBUGGING_SUPPORTED @@ -1279,7 +1291,18 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr case INTOP_BREAKPOINT: { LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Hit breakpoint at IP %p\n", ip)); - InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame); + const int32_t *newIp = InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame); + + // The debugger may have changed the IP via setip. If so, update + // the interpreter's bytecode IP and resume from the new location. + if (newIp != ip) + { + LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: SetIP changed IP from %p to %p\n", ip, newIp)); + ip = newIp; + pFrame->ip = ip; + opcode = *ip; + goto SWITCH_OPCODE; + } int32_t bypassOpcode = 0; diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index 35ab8d2185a1cf..48c4c2c1a6f8ff 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -69,12 +69,6 @@ struct InterpThreadContext const int32_t *m_bypassAddress; // Address of breakpoint to bypass (NULL = no bypass) int32_t m_bypassOpcode; // Original opcode to execute instead of INTOP_BREAKPOINT - // Pending func eval. When the debugger requests a function evaluation on a thread - // stopped in interpreter code, FuncEvalSetup stores the DebuggerEval* here instead - // of hijacking the native CPU context (which doesn't work for interpreter threads). - // The INTOP_BREAKPOINT handler checks this after the debugger callback returns. - void *m_pPendingFuncEval; // DebuggerEval* (void* to avoid header dependency) - void SetBypass(const int32_t* address, int32_t opcode) { _ASSERTE(m_bypassAddress == NULL); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/BaseFrameHandler.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/BaseFrameHandler.cs index f27cd11ecc36b9..3d56a63532613e 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/BaseFrameHandler.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/BaseFrameHandler.cs @@ -59,8 +59,8 @@ public virtual void HandleFuncEvalFrame(FuncEvalFrame funcEvalFrame) { Data.DebuggerEval debuggerEval = _target.ProcessedData.GetOrAdd(funcEvalFrame.DebuggerEvalPtr); - // No context to update if we're doing a func eval from within exception processing. - if (debuggerEval.EvalDuringException) + // No context to update if the eval doesn't use a hijack (exception or interpreter path). + if (!debuggerEval.EvalUsesHijack) { return; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/X86FrameHandler.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/X86FrameHandler.cs index af2d60cd31c156..8b06193322f99f 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/X86FrameHandler.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/X86FrameHandler.cs @@ -50,8 +50,8 @@ public override void HandleFuncEvalFrame(FuncEvalFrame funcEvalFrame) { Data.DebuggerEval debuggerEval = _target.ProcessedData.GetOrAdd(funcEvalFrame.DebuggerEvalPtr); - // No context to update if we're doing a func eval from within exception processing. - if (debuggerEval.EvalDuringException) + // No context to update if the eval doesn't use a hijack (exception or interpreter path). + if (!debuggerEval.EvalUsesHijack) { return; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Frames/DebuggerEval.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Frames/DebuggerEval.cs index 31352b4cc6880a..f6039c5f43f3f1 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Frames/DebuggerEval.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Frames/DebuggerEval.cs @@ -12,11 +12,11 @@ public DebuggerEval(Target target, TargetPointer address) { Target.TypeInfo type = target.GetTypeInfo(DataType.DebuggerEval); TargetContext = address + (ulong)type.Fields[nameof(TargetContext)].Offset; - EvalDuringException = target.Read(address + (ulong)type.Fields[nameof(EvalDuringException)].Offset) != 0; + EvalUsesHijack = target.Read(address + (ulong)type.Fields[nameof(EvalUsesHijack)].Offset) != 0; Address = address; } public TargetPointer Address { get; } public TargetPointer TargetContext { get; } - public bool EvalDuringException { get; } + public bool EvalUsesHijack { get; } } From fc14fc812b3f067136bc70250105e7cde2dd3595 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Wed, 15 Apr 2026 18:37:01 +0200 Subject: [PATCH 7/9] Fix bypass state restore on exception path and remove unused interpexec.h include - Restore m_bypassAddress/m_bypassOpcode in EX_CATCH before EX_RETHROW so bypass state is not corrupted if func-eval throws - Remove unused interpexec.h include from debugger.cpp (GetInterpThreadContext is declared on Thread, not in interpexec.h) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 4 ---- src/coreclr/vm/interpexec.cpp | 2 ++ 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 50c1c2e9e09c1e..c54472ec399bda 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -28,10 +28,6 @@ #include "dbgtransportsession.h" #endif // FEATURE_DBGIPC_TRANSPORT_VM -#ifdef FEATURE_INTERPRETER -#include "../../vm/interpexec.h" -#endif // FEATURE_INTERPRETER - #ifdef TEST_DATA_CONSISTENCY #include "datatest.h" #endif // TEST_DATA_CONSISTENCY diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 505b290b76a886..d9d192e4563885 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -722,6 +722,8 @@ static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodCont EX_CATCH { pThread->SetFilterContext(NULL); + pThreadContext->m_bypassAddress = savedBypassAddress; + pThreadContext->m_bypassOpcode = savedBypassOpcode; EX_RETHROW; } EX_END_CATCH From 6ed593b35de6b4620ef0bef138447a9f1db21254 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Wed, 22 Apr 2026 11:32:28 +0200 Subject: [PATCH 8/9] Refactor DebuggerEval handling and remove unused ExecutePendingInterpreterFuncEval method --- src/coreclr/debug/ee/debugger.cpp | 35 ++++--------------------------- src/coreclr/debug/ee/debugger.h | 8 +------ src/coreclr/vm/dbginterface.h | 7 +------ src/coreclr/vm/interpexec.cpp | 10 +-------- 4 files changed, 7 insertions(+), 53 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index c54472ec399bda..e2f2e003d572ec 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -1378,7 +1378,10 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval m_aborting = FE_ABORT_NONE; m_aborted = false; m_completed = false; - m_evalUsesHijack = !fInException; + // Exception-time evals (fInException) and interpreter evals + // (bpInfoSegmentRX == NULL) both use alternate completion paths + // and must not be treated as hijacked frames. + m_evalUsesHijack = !fInException && (bpInfoSegmentRX != NULL); m_retValueBoxing = Debugger::NoValueTypeBoxing; m_vmObjectHandle = VMPTR_OBJECTHANDLE::NullPtr(); @@ -9864,27 +9867,6 @@ void Debugger::UnloadClass(mdTypeDef classMetadataToken, } -#ifdef FEATURE_INTERPRETER -/****************************************************************************** - * Execute pending func evals on the interpreter thread. Called from the - * interpreter's INTOP_BREAKPOINT handler after the debugger callback returns. - * Routes through ProcessAnyPendingEvals to share the dispatch logic with the - * exception-time func-eval path. - ******************************************************************************/ -void Debugger::ExecutePendingInterpreterFuncEval(Thread* pThread) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - ProcessAnyPendingEvals(pThread); -} -#endif // FEATURE_INTERPRETER - /****************************************************************************** * ******************************************************************************/ @@ -14381,13 +14363,6 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, EECodeInfo codeInfo((PCODE)GetIP(filterContext)); fIsInterpreterThread = codeInfo.IsInterpretedCode(); } - else if (!fInException && pThread->GetInterpThreadContext() != NULL) - { - // The thread is an interpreter thread but not at a breakpoint (no filter context). - // Non-exception evals on interpreter threads require a breakpoint stop. - LOG((LF_CORDB, LL_INFO1000, "D::FES: Func eval requested on non-breakpoint interpreter thread\n")); - return CORDBG_E_FUNC_EVAL_BAD_START_POINT; - } if (!fIsInterpreterThread) #endif // FEATURE_INTERPRETER { @@ -14475,8 +14450,6 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, // after the debugger callback returns. if (fIsInterpreterThread) { - pDE->m_evalUsesHijack = false; - HRESULT hr = CheckInitPendingFuncEvalTable(); if (FAILED(hr)) { diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 8512a2329f458a..35232c8190b80f 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -2575,11 +2575,6 @@ class Debugger : public DebugInterface #ifndef DACCESS_COMPILE void MulticastTraceNextStep(DELEGATEREF pbDel, INT32 count); void ExternalMethodFixupNextStep(PCODE address); - -#ifdef FEATURE_INTERPRETER - void ExecutePendingInterpreterFuncEval(Thread* pThread); -#endif // FEATURE_INTERPRETER - #endif #ifdef DACCESS_COMPILE @@ -3496,8 +3491,7 @@ class DebuggerEval bool Init() { - // Interpreter func evals and exception-time evals don't use the breakpoint instruction segment, so skip the executability check. - if (!m_evalUsesHijack) + if (m_bpInfoSegment == NULL) return true; _ASSERTE(DbgIsExecutable(&m_bpInfoSegment->m_breakpointInstruction, sizeof(m_bpInfoSegment->m_breakpointInstruction))); diff --git a/src/coreclr/vm/dbginterface.h b/src/coreclr/vm/dbginterface.h index 47e40138437dbd..52533fc5d5405c 100644 --- a/src/coreclr/vm/dbginterface.h +++ b/src/coreclr/vm/dbginterface.h @@ -392,12 +392,7 @@ class DebugInterface virtual HRESULT IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BOOL *pResult) = 0; virtual void MulticastTraceNextStep(DELEGATEREF pbDel, INT32 count) = 0; virtual void ExternalMethodFixupNextStep(PCODE address) = 0; - -#ifdef FEATURE_INTERPRETER - // Execute the pending func eval on the interpreter thread context, if any. - // Called from the interpreter's INTOP_BREAKPOINT handler after the debugger callback returns. - virtual void ExecutePendingInterpreterFuncEval(Thread* pThread) = 0; -#endif // FEATURE_INTERPRETER + virtual void ProcessAnyPendingEvals(Thread* pThread) = 0; #endif //DACCESS_COMPILE }; diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index d9d192e4563885..19df85cb7c7b11 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -680,7 +680,6 @@ static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodCont exceptionRecord.ExceptionCode = STATUS_BREAKPOINT; exceptionRecord.ExceptionAddress = (PVOID)ip; - // Construct a CONTEXT for the debugger CONTEXT ctx; memset(&ctx, 0, sizeof(CONTEXT)); @@ -703,21 +702,15 @@ static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodCont STATUS_BREAKPOINT, pThread); - // Execute pending func evals set by the debugger's FuncEvalSetup, if any. - // DispatchNativeException clears the filter context before returning. - // Re-set it as filter context so FuncEvalHijackWorker can pass the managed-code / GC-safe-point checks. InterpThreadContext *pThreadContext = pThread->GetInterpThreadContext(); - // Save and restore bypass state around func eval execution. - // Func eval triggers its own INTOP_BREAKPOINT callbacks which would - // overwrite the bypass that was set for the original breakpoint. const int32_t *savedBypassAddress = pThreadContext->m_bypassAddress; int32_t savedBypassOpcode = pThreadContext->m_bypassOpcode; pThread->SetFilterContext(&ctx); EX_TRY { - g_pDebugInterface->ExecutePendingInterpreterFuncEval(pThread); + g_pDebugInterface->ProcessAnyPendingEvals(pThread); } EX_CATCH { @@ -729,7 +722,6 @@ static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodCont EX_END_CATCH pThread->SetFilterContext(NULL); - // Restore bypass state that may have been overwritten during func eval. pThreadContext->m_bypassAddress = savedBypassAddress; pThreadContext->m_bypassOpcode = savedBypassOpcode; From 2a4b5fffbf8dd5e29b28ab820b1c0ff66856cf2a Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Wed, 22 Apr 2026 13:52:10 +0200 Subject: [PATCH 9/9] Refactor DebuggerEval constructor and improve bypass address handling in interpreter --- src/coreclr/debug/ee/debugger.cpp | 142 +++++++++++++----------------- src/coreclr/debug/ee/debugger.h | 2 +- src/coreclr/vm/interpexec.cpp | 22 ++++- 3 files changed, 80 insertions(+), 86 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index e2f2e003d572ec..916d5937f1d0c1 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -1313,22 +1313,15 @@ ULONG DebuggerMethodInfoTable::CheckDmiTable(void) // Arguments: // pContext - The context to return to when done with this eval. // pEvalInfo - Contains all the important information, such as parameters, type args, method. -// fInException - TRUE if the thread for the eval is currently in an exception notification. -// bpInfoSegmentRX - bpInfoSegmentRX is an InteropSafe allocation allocated by the caller. -// (Caller allocated as there is no way to fail the allocation without -// throwing, and this function is called in a NOTHROW region) +// bpInfoSegmentRX - Non-NULL only when the eval hijacks the native CPU context through +// FuncEvalHijack. NULL for non-hijack evals (exception-time or interpreter), +// which complete via the pending-eval queue instead of a native breakpoint +// trap. Caller-allocated because this function is NOTHROW. // -DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEvalInfo, bool fInException, DebuggerEvalBreakpointInfoSegment* bpInfoSegmentRX) +DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEvalInfo, DebuggerEvalBreakpointInfoSegment* bpInfoSegmentRX) { WRAPPER_NO_CONTRACT; - // bpInfoSegmentRX is NULL only for interpreter func evals — the interpreter signals completion - // directly via FuncEvalComplete, not the native breakpoint trap mechanism. -#ifdef FEATURE_INTERPRETER - _ASSERTE(bpInfoSegmentRX != NULL || (pContext != NULL && EECodeInfo((PCODE)GetIP(pContext)).IsInterpretedCode())); -#else - _ASSERTE(bpInfoSegmentRX != NULL); -#endif if (bpInfoSegmentRX != NULL) { #if !defined(DBI_COMPILE) && !defined(DACCESS_COMPILE) && defined(HOST_OSX) && defined(HOST_ARM64) @@ -1378,10 +1371,10 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval m_aborting = FE_ABORT_NONE; m_aborted = false; m_completed = false; - // Exception-time evals (fInException) and interpreter evals - // (bpInfoSegmentRX == NULL) both use alternate completion paths - // and must not be treated as hijacked frames. - m_evalUsesHijack = !fInException && (bpInfoSegmentRX != NULL); + // Hijacked evals redirect the native CPU context through FuncEvalHijack; non-hijack + // evals (exception-time and interpreter) complete via the pending-eval queue. The + // presence of the breakpoint info segment is the single source of truth. + m_evalUsesHijack = (bpInfoSegmentRX != NULL); m_retValueBoxing = Debugger::NoValueTypeBoxing; m_vmObjectHandle = VMPTR_OBJECTHANDLE::NullPtr(); @@ -14353,20 +14346,26 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, return CORDBG_E_ILLEGAL_AT_GC_UNSAFE_POINT; } + // A func eval uses a CONTEXT hijack (redirects the native CPU context through FuncEvalHijack) + // only when the thread is stopped at a breakpoint or single-step in JIT-compiled code. For + // exception-time evals and interpreter evals we cannot hijack the native context — those paths + // queue the DebuggerEval into the pending-eval table and let the suspend-resume logic dispatch + // it: for exceptions via Debugger::ProcessAnyPendingEvals on continue, for the interpreter via + // INTOP_BREAKPOINT after the debugger callback returns. + bool funcEvalUsesHijack = !fInException; #ifdef FEATURE_INTERPRETER - // For interpreter threads, the filter context contains synthetic values (IP = bytecode address, - // SP = InterpMethodContextFrame*, FP = stack pointer) — not real native register values. - // Skip the SP alignment check since it only applies to native stack pointers. - bool fIsInterpreterThread = false; - if (filterContext != NULL) + if (funcEvalUsesHijack && filterContext != NULL) { EECodeInfo codeInfo((PCODE)GetIP(filterContext)); - fIsInterpreterThread = codeInfo.IsInterpretedCode(); + if (codeInfo.IsInterpretedCode()) + funcEvalUsesHijack = false; } - if (!fIsInterpreterThread) #endif // FEATURE_INTERPRETER + + if (funcEvalUsesHijack) { - if (filterContext != NULL && ::GetSP(filterContext) != ALIGN_DOWN(::GetSP(filterContext), STACK_ALIGN_SIZE)) + _ASSERTE(filterContext != NULL); + if (::GetSP(filterContext) != ALIGN_DOWN(::GetSP(filterContext), STACK_ALIGN_SIZE)) { // SP is not aligned, we cannot do a FuncEval here LOG((LF_CORDB, LL_INFO1000, "D::FES SP is unaligned")); @@ -14374,14 +14373,13 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, } } - // Allocate the breakpoint instruction info for the debugger info in executable memory. - // Interpreter func evals don't need this — completion is signaled directly via - // FuncEvalComplete, not a native breakpoint trap. Skip the allocation to avoid - // requiring executable memory on platforms where it's unavailable (e.g. iOS). + // Allocate the breakpoint instruction info only for hijacked evals. Non-hijack paths + // (exception-time and interpreter) signal completion via FuncEvalComplete on the pending-eval + // queue, not via a native breakpoint trap, so the segment would never be used. Avoiding the + // allocation also means we don't require executable memory on platforms where it's unavailable + // (e.g. iOS). DebuggerEvalBreakpointInfoSegment *bpInfoSegmentRX = NULL; -#ifdef FEATURE_INTERPRETER - if (!fIsInterpreterThread) -#endif // FEATURE_INTERPRETER + if (funcEvalUsesHijack) { DebuggerHeap *pHeap = g_pDebugger->GetInteropSafeExecutableHeap_NoThrow(); if (pHeap == NULL) @@ -14398,7 +14396,7 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, // Create a DebuggerEval to hold info about this eval while its in progress. Constructor copies the thread's // CONTEXT. - DebuggerEval *pDE = new (interopsafe, nothrow) DebuggerEval(filterContext, pEvalInfo, fInException, bpInfoSegmentRX); + DebuggerEval *pDE = new (interopsafe, nothrow) DebuggerEval(filterContext, pEvalInfo, bpInfoSegmentRX); if (pDE == NULL) { @@ -14437,70 +14435,46 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, *argDataArea = pDE->m_argData; } - // Set the thread's IP (in the filter context) to our hijack function if we're stopped due to a breakpoint or single - // step. - if (!fInException) + // Hijacked evals rewrite the thread's native context to enter FuncEvalHijack when execution resumes. + // Non-hijack evals are queued in the pending-eval table and dispatched from the resume path. + if (funcEvalUsesHijack) { _ASSERTE(filterContext != NULL); -#ifdef FEATURE_INTERPRETER - // For interpreter threads, we cannot hijack the native CPU context because the interpreter - // manages execution through its own bytecode dispatch loop. Instead, we queue the DebuggerEval - // in the pending evals table. The INTOP_BREAKPOINT handler will call ProcessAnyPendingEvals - // after the debugger callback returns. - if (fIsInterpreterThread) - { - HRESULT hr = CheckInitPendingFuncEvalTable(); - if (FAILED(hr)) - { - DeleteInteropSafeExecutable(pDE); - return hr; - } - GetPendingEvals()->AddPendingEval(pDE->m_thread, pDE); - - LOG((LF_CORDB, LL_INFO1000, "D::FES: Interpreter func eval setup for pDE:%p on thread %p\n", pDE, pThread)); + ::SetIP(filterContext, (UINT_PTR)GetEEFuncEntryPoint(::FuncEvalHijack)); - // No context modification needed — interpreter checks pending evals on resume. - // No IncThreadsAtUnsafePlaces — stack remains walkable (no context change). - } - else -#endif // FEATURE_INTERPRETER - { - ::SetIP(filterContext, (UINT_PTR)GetEEFuncEntryPoint(::FuncEvalHijack)); + // Don't be fooled into thinking you can push things onto the thread's stack now. If the thread is stopped at a + // breakpoint or from a single step, then its really suspended in the SEH filter. ESP in the thread's CONTEXT, + // therefore, points into the middle of the thread's current stack. So we pass things we need in the hijack in + // the thread's registers. - // Don't be fooled into thinking you can push things onto the thread's stack now. If the thread is stopped at a - // breakpoint or from a single step, then its really suspended in the SEH filter. ESP in the thread's CONTEXT, - // therefore, points into the middle of the thread's current stack. So we pass things we need in the hijack in - // the thread's registers. - - // Set the first argument to point to the DebuggerEval. + // Set the first argument to point to the DebuggerEval. #if defined(TARGET_X86) - filterContext->Eax = (DWORD)pDE; + filterContext->Eax = (DWORD)pDE; #elif defined(TARGET_AMD64) #ifdef UNIX_AMD64_ABI - filterContext->Rdi = (SIZE_T)pDE; + filterContext->Rdi = (SIZE_T)pDE; #else // UNIX_AMD64_ABI - filterContext->Rcx = (SIZE_T)pDE; + filterContext->Rcx = (SIZE_T)pDE; #endif // !UNIX_AMD64_ABI #elif defined(TARGET_ARM) - filterContext->R0 = (DWORD)pDE; + filterContext->R0 = (DWORD)pDE; #elif defined(TARGET_ARM64) - filterContext->X0 = (SIZE_T)pDE; + filterContext->X0 = (SIZE_T)pDE; #elif defined(TARGET_RISCV64) - filterContext->A0 = (SIZE_T)pDE; + filterContext->A0 = (SIZE_T)pDE; #elif defined(TARGET_LOONGARCH64) - filterContext->A0 = (SIZE_T)pDE; + filterContext->A0 = (SIZE_T)pDE; #else - PORTABILITY_ASSERT("Debugger::FuncEvalSetup is not implemented on this platform."); + PORTABILITY_ASSERT("Debugger::FuncEvalSetup is not implemented on this platform."); #endif - // - // To prevent GCs until the func-eval gets a chance to run, we increment the counter here. - // We only need to do this if we have changed the filter CONTEXT, since the stack will be unwalkable - // in this case. - // - g_pDebugger->IncThreadsAtUnsafePlaces(); - } + // + // To prevent GCs until the func-eval gets a chance to run, we increment the counter here. + // We only need to do this if we have changed the filter CONTEXT, since the stack will be unwalkable + // in this case. + // + g_pDebugger->IncThreadsAtUnsafePlaces(); } else { @@ -14511,9 +14485,15 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, DeleteInteropSafeExecutable(pDE); // Note this runs the destructor for DebuggerEval, which releases its internal buffers return (hr); } - // If we're in an exception, then add a pending eval for this thread. This will cause us to perform the func - // eval when the user continues the process after the current exception event. + + // Queue the eval. Exception-time evals run from Debugger::ProcessAnyPendingEvals when + // the process continues. Interpreter evals run from the INTOP_BREAKPOINT handler after + // the debugger callback returns — no context modification and no IncThreadsAtUnsafePlaces + // needed because the stack remains walkable. GetPendingEvals()->AddPendingEval(pDE->m_thread, pDE); + + LOG((LF_CORDB, LL_INFO1000, "D::FES: Non-hijack func eval setup for pDE:%p on thread %p (fInException=%d)\n", + pDE, pThread, fInException)); } diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 35232c8190b80f..700cdf4ae82bc0 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -3487,7 +3487,7 @@ class DebuggerEval TypeHandle m_ownerTypeHandle; DebuggerEvalBreakpointInfoSegment* m_bpInfoSegment; - DebuggerEval(T_CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEvalInfo, bool fInException, DebuggerEvalBreakpointInfoSegment* bpInfoSegmentRX); + DebuggerEval(T_CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEvalInfo, DebuggerEvalBreakpointInfoSegment* bpInfoSegmentRX); bool Init() { diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 19df85cb7c7b11..369017de34f898 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -706,6 +706,11 @@ static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodCont const int32_t *savedBypassAddress = pThreadContext->m_bypassAddress; int32_t savedBypassOpcode = pThreadContext->m_bypassOpcode; + const int32_t *originalIp = ip; + + // Clear the bypass before dispatching pending evals + pThreadContext->m_bypassAddress = NULL; + pThreadContext->m_bypassOpcode = 0; pThread->SetFilterContext(&ctx); EX_TRY @@ -722,13 +727,22 @@ static const int32_t* InterpBreakpoint(const int32_t *ip, const InterpMethodCont EX_END_CATCH pThread->SetFilterContext(NULL); - pThreadContext->m_bypassAddress = savedBypassAddress; - pThreadContext->m_bypassOpcode = savedBypassOpcode; - // The debugger may have modified the IP via SetIP (e.g. the setip command). // Return the potentially updated IP so the interpreter can resume from the // new position. - return (const int32_t*)(TADDR)GetIP(&ctx); + const int32_t *newIp = (const int32_t*)(TADDR)GetIP(&ctx); + + // Only restore the bypass when the debugger is resuming to the original breakpoint IP. + // If SetIP moved execution elsewhere, the bypass (which was set up to skip the opcode + // at the original address exactly once) must be dropped; otherwise, if execution later + // returns to the original IP, the breakpoint would be incorrectly skipped. + if (newIp == originalIp) + { + pThreadContext->m_bypassAddress = savedBypassAddress; + pThreadContext->m_bypassOpcode = savedBypassOpcode; + } + + return newIp; } return ip; }