From 123c90a6c6878608a0c8f65efe52a158371f62f8 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Mon, 13 Apr 2026 21:30:38 +0200 Subject: [PATCH 1/5] Fix interpreter stack walking: double frames and line number off-by-one Fix two issues in the StackFrameIterator for FEATURE_INTERPRETER: 1. Double-frame bug (stackwalk.cpp): After the DummyCallerIP transition from the last interpreter frame to native code, advance m_crawl.pFrame past the InterpreterFrame. Without this, the DAC/DBI debugger path (which starts with context already pointing into the interpreter chain) would encounter the InterpreterFrame again and re-enter the chain, reporting each interpreter frame twice. The GC path is unaffected because it already advances pFrame when entering the chain via the explicit frame handler. 2. Line number off-by-one (eetwain.cpp): In VirtualUnwindInterpreterCallFrame, subtract 1 from pFrame->ip when unwinding to a parent frame. The interpreter stores ip after advancing past the call instruction (ip += N), so the raw IP points to the start of the next instruction. Subtracting 1 places the IP within the call instruction, ensuring the debug info maps to the correct IL offset and source line for non-leaf frames. This is analogous to how JIT return addresses are adjusted for native code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/eetwain.cpp | 5 ++++- src/coreclr/vm/stackwalk.cpp | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index 41185860179a64..698460077f5e4d 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -2106,6 +2106,9 @@ static void VirtualUnwindInterpreterCallFrame(TADDR sp, T_CONTEXT *pContext) SetIP(pContext, (TADDR)pFrame->ip); SetSP(pContext, dac_cast(pFrame)); SetFP(pContext, (TADDR)pFrame->pStack); + + // Interpreter ip points past the call instruction (like a return address). + pContext->ContextFlags = CONTEXT_FULL | CONTEXT_UNWOUND_TO_CALL; } else { @@ -2118,8 +2121,8 @@ static void VirtualUnwindInterpreterCallFrame(TADDR sp, T_CONTEXT *pContext) SetIP(pContext, InterpreterFrame::DummyCallerIP); TADDR interpreterFrameAddress = GetFirstArgReg(pContext); SetSP(pContext, interpreterFrameAddress); + pContext->ContextFlags = CONTEXT_FULL; } - pContext->ContextFlags = CONTEXT_FULL; } bool InterpreterCodeManager::UnwindStackFrame(PREGDISPLAY pRD, diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index 1aae7049d026b7..4283303358b7e3 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -2161,6 +2161,13 @@ StackWalkAction StackFrameIterator::NextRaw(void) PTR_InterpreterFrame pInterpreterFrame = dac_cast(GetSP(m_crawl.pRD->pCurrentContext)); pInterpreterFrame->UpdateRegDisplay(m_crawl.pRD, m_flags & UNWIND_FLOATS); LOG((LF_GCROOTS, LL_INFO10000, "STACKWALK: Transitioning from last interpreted frame under InterpreterFrame %p to native frame (IP=%p, SP=%p)\n", pInterpreterFrame, GetControlPC(m_crawl.pRD), GetRegdisplaySP(m_crawl.pRD))); + + // Advance past the InterpreterFrame to prevent re-entering the interpreter chain. + while (m_crawl.pFrame != FRAME_TOP && + dac_cast(m_crawl.pFrame) <= dac_cast(pInterpreterFrame)) + { + m_crawl.GotoNextFrame(); + } } #endif // FEATURE_INTERPRETER From beab3607735a3ba0c05f5ee9c7f8823a5e6f931b Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 16 Apr 2026 19:04:42 +0200 Subject: [PATCH 2/5] Proper fix of the doubled stack trace issue --- src/coreclr/vm/stackwalk.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index 4283303358b7e3..4373033af61f3e 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -2161,13 +2161,6 @@ StackWalkAction StackFrameIterator::NextRaw(void) PTR_InterpreterFrame pInterpreterFrame = dac_cast(GetSP(m_crawl.pRD->pCurrentContext)); pInterpreterFrame->UpdateRegDisplay(m_crawl.pRD, m_flags & UNWIND_FLOATS); LOG((LF_GCROOTS, LL_INFO10000, "STACKWALK: Transitioning from last interpreted frame under InterpreterFrame %p to native frame (IP=%p, SP=%p)\n", pInterpreterFrame, GetControlPC(m_crawl.pRD), GetRegdisplaySP(m_crawl.pRD))); - - // Advance past the InterpreterFrame to prevent re-entering the interpreter chain. - while (m_crawl.pFrame != FRAME_TOP && - dac_cast(m_crawl.pFrame) <= dac_cast(pInterpreterFrame)) - { - m_crawl.GotoNextFrame(); - } } #endif // FEATURE_INTERPRETER @@ -2431,6 +2424,19 @@ void StackFrameIterator::ProcessCurrentFrame(void) m_frameState = SFITER_INITIAL_NATIVE_CONTEXT; fDone = true; } + else + { +#ifdef FEATURE_INTERPRETER + if (m_crawl.pFrame != FRAME_TOP && m_crawl.pFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) + { + // When stack walk starts on an explicit context in interpreted code, we need to skip the related interpreter + // frame, because the stack frame iterator assumes that when it is walking interpreted frames, it has + // already processed the interpreter frame. Without this skip, the stack walk would end up walking the + // interpreted frames twice. + m_crawl.GotoNextFrame(); + } +#endif // FEATURE_INTERPRETER + } } else { From e59314112cb47444c253b15b4668c9616aa1042e Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Wed, 22 Apr 2026 14:16:58 +0200 Subject: [PATCH 3/5] Revert interpreter IP adjustment in VirtualUnwindInterpreterCallFrame The stack walker should return the real IP. IP adjustment for call sites belongs on the consumer side (EH already does it; debugger-side handling for the interpreter line-number off-by-one is tracked separately). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/eetwain.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index 698460077f5e4d..41185860179a64 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -2106,9 +2106,6 @@ static void VirtualUnwindInterpreterCallFrame(TADDR sp, T_CONTEXT *pContext) SetIP(pContext, (TADDR)pFrame->ip); SetSP(pContext, dac_cast(pFrame)); SetFP(pContext, (TADDR)pFrame->pStack); - - // Interpreter ip points past the call instruction (like a return address). - pContext->ContextFlags = CONTEXT_FULL | CONTEXT_UNWOUND_TO_CALL; } else { @@ -2121,8 +2118,8 @@ static void VirtualUnwindInterpreterCallFrame(TADDR sp, T_CONTEXT *pContext) SetIP(pContext, InterpreterFrame::DummyCallerIP); TADDR interpreterFrameAddress = GetFirstArgReg(pContext); SetSP(pContext, interpreterFrameAddress); - pContext->ContextFlags = CONTEXT_FULL; } + pContext->ContextFlags = CONTEXT_FULL; } bool InterpreterCodeManager::UnwindStackFrame(PREGDISPLAY pRD, From 9f219da7e3f5133da34c21ef2e5f113f49197ba5 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Tue, 28 Apr 2026 14:52:58 +0200 Subject: [PATCH 4/5] Address review: handle interpreted CONTEXT in ResetRegDisp Move the doubled-interpreter-frames fix from ProcessCurrentFrame to ResetRegDisp so the StackFrameIterator starts from a correct state when DAC/DBI seeds it with a CONTEXT pointing into interpreted code. - Init: assert the CONTEXT does not reference interpreted code; that path is for OS-thread-top contexts and only DAC/DBI seeds interpreter contexts (via ResetRegDisp). - ResetRegDisp: when CONTEXT references interpreted code, locate the owning InterpreterFrame whose InterpMethodContextFrame chain contains the SP, and advance m_crawl.pFrame past it. - ProcessCurrentFrame: drop the post-hoc workaround that skipped the re-encountered InterpreterFrame; no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/stackwalk.cpp | 57 +++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index 4373033af61f3e..2386935119c538 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -70,7 +70,7 @@ PTR_VOID ConvertStackMarkToPointerOnOSStack(PTR_Thread pThread, PTR_VOID stackMa } pCurrent = pCurrent->pParent; } while (pCurrent != NULL); - + } pFrame = pFrame->PtrNextFrame(); @@ -1087,6 +1087,9 @@ BOOL StackFrameIterator::Init(Thread * pThread, // process the REGDISPLAY and stop at the first frame ProcessIp(GetControlPC(m_crawl.pRD)); +#ifdef FEATURE_INTERPRETER + _ASSERTE(!m_crawl.codeInfo.IsInterpretedCode()); +#endif // FEATURE_INTERPRETER if (m_crawl.isFrameless && !!(m_crawl.pRD->pCurrentContext->ContextFlags & CONTEXT_EXCEPTION_ACTIVE)) { m_crawl.hasFaulted = true; @@ -1160,6 +1163,45 @@ BOOL StackFrameIterator::ResetRegDisp(PREGDISPLAY pRegDisp, PCODE curPc = GetControlPC(pRegDisp); ProcessIp(curPc); +#ifdef FEATURE_INTERPRETER + if (m_crawl.codeInfo.IsInterpretedCode()) + { + // SP points at an InterpMethodContextFrame on the interpreter stack. + // Locate the owning InterpreterFrame and advance past it so the iterator + // does not re-enter the same chain via the explicit frame link. + TADDR interpSP = GetRegdisplaySP(m_crawl.pRD); + PTR_Frame pSearch = m_crawl.pFrame; + PTR_Frame pOwningInterpFrame = (PTR_Frame)FRAME_TOP; + while (pSearch != FRAME_TOP) + { + if (pSearch->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) + { + PTR_InterpreterFrame pInterpFrame = dac_cast(pSearch); + for (PTR_InterpMethodContextFrame pCur = pInterpFrame->GetTopInterpMethodContextFrame(); + pCur != NULL; + pCur = pCur->pParent) + { + if (dac_cast(pCur) == interpSP) + { + pOwningInterpFrame = pSearch; + break; + } + } + if (pOwningInterpFrame != FRAME_TOP) + { + break; + } + } + pSearch = pSearch->PtrNextFrame(); + } + _ASSERTE(pOwningInterpFrame != FRAME_TOP); + if (pOwningInterpFrame != FRAME_TOP) + { + m_crawl.pFrame = pOwningInterpFrame->PtrNextFrame(); + } + } + else +#endif // FEATURE_INTERPRETER // loop the frame chain to find the closet explicit frame which is lower than the specified REGDISPLAY // (stack grows up towards lower address) if (m_crawl.pFrame != FRAME_TOP) @@ -2424,19 +2466,6 @@ void StackFrameIterator::ProcessCurrentFrame(void) m_frameState = SFITER_INITIAL_NATIVE_CONTEXT; fDone = true; } - else - { -#ifdef FEATURE_INTERPRETER - if (m_crawl.pFrame != FRAME_TOP && m_crawl.pFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) - { - // When stack walk starts on an explicit context in interpreted code, we need to skip the related interpreter - // frame, because the stack frame iterator assumes that when it is walking interpreted frames, it has - // already processed the interpreter frame. Without this skip, the stack walk would end up walking the - // interpreted frames twice. - m_crawl.GotoNextFrame(); - } -#endif // FEATURE_INTERPRETER - } } else { From e781d99b638112b0f418b78266f5484fec62ad45 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Tue, 28 Apr 2026 16:47:33 +0200 Subject: [PATCH 5/5] Use first-arg register to locate owning InterpreterFrame When the StackFrameIterator is seeded from a CONTEXT pointing into interpreted code, InterpreterFrame::SetContextToInterpMethodContextFrame already writes the owning InterpreterFrame pointer into the first-arg register. Read it from the CONTEXT instead of searching the explicit Frame chain for an InterpMethodContextFrame whose address matches SP. Drops the chain search and removes the implicit assumption that the seeded context always belongs to the topmost InterpreterFrame, which also generalizes correctly to mid-stack seeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/stackwalk.cpp | 42 ++++++++---------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index 2386935119c538..00f2123f005fe0 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -1166,39 +1166,15 @@ BOOL StackFrameIterator::ResetRegDisp(PREGDISPLAY pRegDisp, #ifdef FEATURE_INTERPRETER if (m_crawl.codeInfo.IsInterpretedCode()) { - // SP points at an InterpMethodContextFrame on the interpreter stack. - // Locate the owning InterpreterFrame and advance past it so the iterator - // does not re-enter the same chain via the explicit frame link. - TADDR interpSP = GetRegdisplaySP(m_crawl.pRD); - PTR_Frame pSearch = m_crawl.pFrame; - PTR_Frame pOwningInterpFrame = (PTR_Frame)FRAME_TOP; - while (pSearch != FRAME_TOP) - { - if (pSearch->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) - { - PTR_InterpreterFrame pInterpFrame = dac_cast(pSearch); - for (PTR_InterpMethodContextFrame pCur = pInterpFrame->GetTopInterpMethodContextFrame(); - pCur != NULL; - pCur = pCur->pParent) - { - if (dac_cast(pCur) == interpSP) - { - pOwningInterpFrame = pSearch; - break; - } - } - if (pOwningInterpFrame != FRAME_TOP) - { - break; - } - } - pSearch = pSearch->PtrNextFrame(); - } - _ASSERTE(pOwningInterpFrame != FRAME_TOP); - if (pOwningInterpFrame != FRAME_TOP) - { - m_crawl.pFrame = pOwningInterpFrame->PtrNextFrame(); - } + // The CONTEXT carries the owning InterpreterFrame in the first-arg register + // (set by InterpreterFrame::SetContextToInterpMethodContextFrame). Advance + // m_crawl.pFrame past it so the iterator does not re-enter the same + // InterpMethodContextFrame chain via the explicit frame link. + PTR_InterpreterFrame pOwningInterpFrame = + dac_cast((TADDR)GetFirstArgReg(m_crawl.pRD->pCurrentContext)); + _ASSERTE(pOwningInterpFrame != NULL); + _ASSERTE(pOwningInterpFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame); + m_crawl.pFrame = pOwningInterpFrame->PtrNextFrame(); } else #endif // FEATURE_INTERPRETER