Skip to content

Commit ea1faed

Browse files
matouskozakmatouskozak
committed
workaround interpreter stack walking issues
1 parent ffa9862 commit ea1faed

File tree

3 files changed

+88
-15
lines changed

3 files changed

+88
-15
lines changed

‎src/coreclr/debug/ee/controller.cpp‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,58 @@ void ControllerStackInfo::GetStackInfo(
494494
_ASSERTE(!ISREDIRECTEDTHREAD(thread));
495495
}
496496

497+
#ifdef FEATURE_INTERPRETER
498+
// When hitting interpreter breakpoints, InterpBreakpoint creates a synthetic context
499+
// with SP pointing to InterpMethodContextFrame instead of native stack.
500+
//
501+
// The interpreter context has:
502+
// - IP = bytecode address (interpreter IR)
503+
// - SP = InterpMethodContextFrame pointer
504+
// - First arg register = InterpreterFrame pointer
505+
//
506+
// The stack walker's interpreter frame handling (stackwalk.cpp:2413-2468) expects to
507+
// DISCOVER the InterpreterFrame during the walk, saving the native context before
508+
// switching to interpreter frame enumeration. When we start with a synthetic context,
509+
// the "native context" being saved is actually the synthetic interpreter context,
510+
// which has invalid values for native stack restoration.
511+
//
512+
// Fix: When we detect an interpreter synthetic context, temporarily clear the filter
513+
// context and mark context as invalid so the stack walker starts from the thread's
514+
// frame chain. The InterpreterFrame will be discovered naturally and handled correctly.
515+
CONTEXT *pSavedFilterContext = NULL;
516+
bool fRestoredFilterContext = false;
517+
if (contextValid)
518+
{
519+
PCODE ip = GetIP(pContext);
520+
EECodeInfo codeInfo(ip);
521+
if (codeInfo.IsValid() && codeInfo.IsInterpretedCode())
522+
{
523+
TADDR interpreterFrameAddr = GetFirstArgReg(pContext);
524+
if (interpreterFrameAddr != 0)
525+
{
526+
Frame *pFrame = (Frame*)interpreterFrameAddr;
527+
if (pFrame != FRAME_TOP && pFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame)
528+
{
529+
LOG((LF_CORDB, LL_INFO10000, "CSI::GSI: Interpreter synthetic context detected - IP=%p, SP=%p, InterpreterFrame=%p. Using frame chain instead.\n",
530+
(void*)ip, (void*)GetSP(pContext), (void*)interpreterFrameAddr));
531+
532+
// Temporarily clear the filter context so the stack walker uses the thread's
533+
// frame chain. The InterpreterFrame is on the frame chain and will be found
534+
// and processed correctly, with proper native context saving/restoration.
535+
pSavedFilterContext = g_pEEInterface->GetThreadFilterContext(thread);
536+
if (pSavedFilterContext != NULL)
537+
{
538+
g_pEEInterface->SetThreadFilterContext(thread, NULL);
539+
fRestoredFilterContext = true;
540+
}
541+
contextValid = FALSE;
542+
pContext = &this->m_tempContext;
543+
}
544+
}
545+
}
546+
}
547+
#endif // FEATURE_INTERPRETER
548+
497549
// Mark this stackwalk as valid so that it can in turn be used to grandfather
498550
// in other stackwalks.
499551
INDEBUG(m_dbgExecuted = true);
@@ -514,6 +566,14 @@ void ControllerStackInfo::GetStackInfo(
514566
(void *) this,
515567
FALSE);
516568

569+
#ifdef FEATURE_INTERPRETER
570+
// Restore the filter context if we temporarily cleared it for interpreter stack walk
571+
if (fRestoredFilterContext)
572+
{
573+
g_pEEInterface->SetThreadFilterContext(thread, pSavedFilterContext);
574+
}
575+
#endif // FEATURE_INTERPRETER
576+
517577
_ASSERTE(m_activeFound); // All threads have at least one unmanaged frame
518578

519579
if (result == SWA_DONE)

‎src/coreclr/vm/stackwalk.cpp‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,17 @@ void StackFrameIterator::CommonCtor(Thread * pThread, PTR_Frame pFrame, ULONG32
990990
m_isRuntimeWrappedExceptions = false;
991991
m_fFoundFirstFunclet = false;
992992
m_pvResumableFrameTargetSP = NULL;
993+
#ifdef FEATURE_INTERPRETER
994+
// Initialize to sentinel value to detect if we ever saved context during interpreter frame walking.
995+
// When stackwalk starts with filter context inside interpreter code, the save path may be skipped.
996+
m_interpExecMethodIP = (TADDR)-1;
997+
m_interpExecMethodSP = (TADDR)-1;
998+
m_interpExecMethodFP = (TADDR)-1;
999+
m_interpExecMethodFirstArgReg = (TADDR)-1;
1000+
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
1001+
m_interpExecMethodSSP = (TADDR)-1;
1002+
#endif
1003+
#endif // FEATURE_INTERPRETER
9931004
} // StackFrameIterator::CommonCtor()
9941005

9951006
//---------------------------------------------------------------------------------------
@@ -2438,16 +2449,21 @@ void StackFrameIterator::ProcessCurrentFrame(void)
24382449
{
24392450
// We have finished walking the interpreted frames. Process the InterpreterFrame itself.
24402451
// Restore the registers to the values they had before we started walking the interpreter frames.
2441-
LOG((LF_GCROOTS, LL_INFO10000, "STACKWALK: Completed walking of interpreted frames for InterpreterFrame %p, restoring SP=%p, IP=%p\n", m_crawl.pFrame, m_interpExecMethodSP, m_interpExecMethodIP));
2442-
_ASSERTE(dac_cast<TADDR>(m_crawl.pFrame) == GetFirstArgReg(pRD->pCurrentContext));
2443-
SetIP(pRD->pCurrentContext, m_interpExecMethodIP);
2444-
SetSP(pRD->pCurrentContext, m_interpExecMethodSP);
2445-
SetFP(pRD->pCurrentContext, m_interpExecMethodFP);
2446-
SetFirstArgReg(pRD->pCurrentContext, m_interpExecMethodFirstArgReg);
2452+
// Only restore if we actually saved - when stackwalk starts with filter context inside
2453+
// interpreter code, the save path is skipped and m_interpExecMethodIP remains sentinel.
2454+
if (m_interpExecMethodIP != (TADDR)-1)
2455+
{
2456+
LOG((LF_GCROOTS, LL_INFO10000, "STACKWALK: Completed walking of interpreted frames for InterpreterFrame %p, restoring SP=%p, IP=%p\n", m_crawl.pFrame, m_interpExecMethodSP, m_interpExecMethodIP));
2457+
_ASSERTE(dac_cast<TADDR>(m_crawl.pFrame) == GetFirstArgReg(pRD->pCurrentContext));
2458+
SetIP(pRD->pCurrentContext, m_interpExecMethodIP);
2459+
SetSP(pRD->pCurrentContext, m_interpExecMethodSP);
2460+
SetFP(pRD->pCurrentContext, m_interpExecMethodFP);
2461+
SetFirstArgReg(pRD->pCurrentContext, m_interpExecMethodFirstArgReg);
24472462
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
2448-
pRD->SSP = m_interpExecMethodSSP;
2463+
pRD->SSP = m_interpExecMethodSSP;
24492464
#endif
2450-
SyncRegDisplayToCurrentContext(pRD);
2465+
SyncRegDisplayToCurrentContext(pRD);
2466+
}
24512467
}
24522468
}
24532469
else if (InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame) && ((PTR_InlinedCallFrame)m_crawl.pFrame)->IsInInterpreter())

‎src/coreclr/vm/threads.cpp‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5676,20 +5676,17 @@ void Thread::FillRegDisplay(const PREGDISPLAY pRD, PT_CONTEXT pctx, bool fLightU
56765676

56775677
void CheckRegDisplaySP (REGDISPLAY *pRD)
56785678
{
5679-
// on wasm the SP is address to interpreter stack, which is located on heap and not on C runtime stack
5680-
// WASM-TODO: update this when we will have codegen
5681-
// For FEATURE_INTERPRETER, the SP can also point to interpreter stack memory which is outside native stack bounds.
5682-
// Skip validation in these cases as the interpreter uses heap-allocated frame structures.
5683-
// TODO: Figure this out without disabling the check entirely.
5684-
#if !defined(TARGET_WASM) && !defined(FEATURE_INTERPRETER)
5679+
// on wasm the SP is address to interpreter stack, which is located on heap and not on C runtime stack
5680+
// WASM-TODO: update this when we will have codegen
5681+
#if !defined(TARGET_WASM)
56855682
if (pRD->SP && pRD->_pThread)
56865683
{
56875684
#ifndef NO_FIXED_STACK_LIMIT
56885685
_ASSERTE(pRD->_pThread->IsExecutingOnAltStack() || PTR_VOID(pRD->SP) >= pRD->_pThread->GetCachedStackLimit());
56895686
#endif // NO_FIXED_STACK_LIMIT
56905687
_ASSERTE(pRD->_pThread->IsExecutingOnAltStack() || PTR_VOID(pRD->SP) < pRD->_pThread->GetCachedStackBase());
56915688
}
5692-
#endif // !TARGET_WASM && !FEATURE_INTERPRETER
5689+
#endif // !TARGET_WASM
56935690
}
56945691

56955692
#endif // DEBUG_REGDISPLAY

0 commit comments

Comments
 (0)