Skip to content

Remove extra x86 unwind from DBI #130011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rcj1 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Remove extra x86 unwind from DBI #130011

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f40b8fd
remove extra unwind
rcj1 Jun 30, 2026
b76e848
bug fix
rcj1 Jun 30, 2026
047cb4d
CCR
rcj1 Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/coreclr/debug/daccess/dacdbiimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,9 +718,6 @@ class DacDbiInterfaceImpl :
// Return the stack parameter size of the given method.
ULONG32 GetStackParameterSize(EECodeInfo * pCodeInfo);

// Return the FramePointer of the current frame at which the stackwalker is stopped.
HRESULT STDMETHODCALLTYPE GetFramePointer(StackWalkHandle pSFIHandle, OUT FramePointer * pRetVal);

FramePointer GetFramePointerWorker(StackFrameIterator * pIter);

// Return TRUE if the specified CONTEXT is the CONTEXT of the leaf frame.
Expand Down
39 changes: 27 additions & 12 deletions src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,23 +637,24 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetStackParameterSize(CORDB_ADDRE
return hr;
}

// Return the FramePointer of the current frame at which the stackwalker is stopped.
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetFramePointer(StackWalkHandle pSFIHandle, OUT FramePointer * pRetVal)
#ifdef TARGET_X86
static FramePointer ComputeX86FramePointer(CrawlFrame * pCF, T_CONTEXT * pSourceContext)
{
DD_ENTER_MAY_THROW;
REGDISPLAY * pRD = pCF->GetRegisterSet();

HRESULT hr = S_OK;
EX_TRY
{
T_CONTEXT tempContext;
Comment thread
rcj1 marked this conversation as resolved.
Outdated
CopyOSContext(&tempContext, pSourceContext);
pCF->GetCodeManager()->UnwindStackFrame(&tempContext);

StackFrameIterator * pIter = GetIteratorFromHandle(pSFIHandle);
*pRetVal = GetFramePointerWorker(pIter);
}
EX_CATCH_HRESULT(hr);
return hr;
EECodeInfo codeInfo;
codeInfo.Init(pRD->pCurrentContext->Eip);

LPVOID PCTAddr = tempContext.Esp - codeInfo.GetCodeManager()->GetStackParameterSize(&codeInfo) - sizeof(DWORD);
return FramePointer::MakeFramePointer(PCTAddr);
}
#endif // TARGET_X86

// Internal helper for GetFramePointer.
// Internal helper for GetStackWalkCurrentFrameInfo.
FramePointer DacDbiInterfaceImpl::GetFramePointerWorker(StackFrameIterator * pIter)
{
CrawlFrame * pCF = &(pIter->m_crawl);
Expand All @@ -665,17 +666,31 @@ FramePointer DacDbiInterfaceImpl::GetFramePointerWorker(StackFrameIterator * pIt
// For managed methods, we have the full CONTEXT. Additionally, we also have the caller CONTEXT
// on WIN64.
case StackFrameIterator::SFITER_FRAMELESS_METHOD:
{
#ifdef TARGET_X86
fp = ComputeX86FramePointer(pCF, pRD->pCurrentContext);
#else
fp = FramePointer::MakeFramePointer(GetRegdisplayStackMark(pRD));
#endif
break;
}

// In these cases, we only have the full CONTEXT, not the caller CONTEXT.
case StackFrameIterator::SFITER_NATIVE_MARKER_FRAME:
//
// fall through
//
case StackFrameIterator::SFITER_INITIAL_NATIVE_CONTEXT:
{
#ifdef TARGET_X86
// We only get here if we are unwinding a runtime-unwindable stub.
// So first we need to unwind the stub to get to the caller frame, and then we can get the frame pointer from that.
fp = ComputeX86FramePointer(pCF, RetrieveHijackedContext(pRD));
#else
fp = FramePointer::MakeFramePointer(GetRegdisplayStackMark(pRD));
#endif
Comment thread
rcj1 marked this conversation as resolved.
break;
}

// In these cases, we use the address of the explicit frame as the frame marker.
case StackFrameIterator::SFITER_FRAME_FUNCTION:
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/debug/di/rspriv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6386,12 +6386,6 @@ class CordbStackWalk : public CordbBase, public ICorDebugStackWalk

// cached flag used for refreshing a CordbStackWalk
CorDebugSetContextFlag m_cachedSetContextFlag;

// We unwind one frame ahead of time to get the FramePointer on x86.
// These fields are used for the bookkeeping.
RSSmartPtr<CordbFrame> m_pCachedFrame;
HRESULT m_cachedHR;
bool m_fIsOneFrameAhead;
};


Expand Down
181 changes: 35 additions & 146 deletions src/coreclr/debug/di/rsstackwalk.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@ CordbStackWalk::CordbStackWalk(CordbThread * pCordbThread)
: CordbBase(pCordbThread->GetProcess(), 0, enumCordbStackWalk),
m_pCordbThread(pCordbThread),
m_pSFIHandle(NULL),
m_cachedSetContextFlag(SET_CONTEXT_FLAG_ACTIVE_FRAME),
m_cachedHR(S_OK),
m_fIsOneFrameAhead(false)
m_cachedSetContextFlag(SET_CONTEXT_FLAG_ACTIVE_FRAME)
{
m_pCachedFrame.Clear();
}

void CordbStackWalk::Init()
Expand Down Expand Up @@ -136,11 +133,6 @@ void CordbStackWalk::DeleteAll()
SIMPLIFYING_ASSUMPTION_SUCCEEDED(hr);
m_pSFIHandle = NULL;
}

// clear out the cached frame
m_pCachedFrame.Clear();
m_cachedHR = S_OK;
m_fIsOneFrameAhead = false;
}

//---------------------------------------------------------------------------------------
Expand Down Expand Up @@ -204,15 +196,7 @@ void CordbStackWalk::RefreshIfNeeded()
{
// Make a local copy of the CONTEXT here. DeleteAll() will delete the CONTEXT on the cached frame,
// and CreateStackWalk() actually uses the CONTEXT buffer we pass to it.
DT_CONTEXT ctx;
if (m_fIsOneFrameAhead)
{
ctx = *(m_pCachedFrame->GetContext());
}
else
{
ctx = m_context;
}
DT_CONTEXT ctx = m_context;

Comment thread
rcj1 marked this conversation as resolved.
Outdated
// clear all the state
DeleteAll();
Expand Down Expand Up @@ -282,48 +266,27 @@ HRESULT CordbStackWalk::GetContext(ULONG32 contextFlags,
ThrowWin32(ERROR_INSUFFICIENT_BUFFER);
}

// Check if we are one frame ahead. If so, returned the CONTEXT on the cached frame.
if (m_fIsOneFrameAhead)
// We have to call the DDI.
IDacDbiInterface * pDAC = GetProcess()->GetDAC();

IDacDbiInterface::FrameType ft;
IfFailThrow(pDAC->GetStackWalkCurrentFrameInfo(m_pSFIHandle, NULL, &ft));
if (ft == IDacDbiInterface::kInvalid)
{
if (m_pCachedFrame != NULL)
{
const DT_CONTEXT * pSrcContext = m_pCachedFrame->GetContext();
_ASSERTE(pSrcContext);
CORDbgCopyThreadContext(pContext, pSrcContext);
}
else
{
// We encountered a problem when we were trying to initialize the CordbNativeFrame.
// However, the problem occurred after we have unwound the current frame.
// What do we do here? We don't have the CONTEXT anymore.
_ASSERTE(FAILED(m_cachedHR));
ThrowHR(m_cachedHR);
}
ThrowHR(E_FAIL);
}
else if (ft == IDacDbiInterface::kAtEndOfStack)
{
ThrowHR(CORDBG_E_PAST_END_OF_STACK);
}
else if (ft == IDacDbiInterface::kExplicitFrame)
{
ThrowHR(CORDBG_E_NO_CONTEXT_FOR_INTERNAL_FRAME);
}
else
{
// No easy way out in this case. We have to call the DDI.
IDacDbiInterface * pDAC = GetProcess()->GetDAC();

IDacDbiInterface::FrameType ft;
IfFailThrow(pDAC->GetStackWalkCurrentFrameInfo(m_pSFIHandle, NULL, &ft));
if (ft == IDacDbiInterface::kInvalid)
{
ThrowHR(E_FAIL);
}
else if (ft == IDacDbiInterface::kAtEndOfStack)
{
ThrowHR(CORDBG_E_PAST_END_OF_STACK);
}
else if (ft == IDacDbiInterface::kExplicitFrame)
{
ThrowHR(CORDBG_E_NO_CONTEXT_FOR_INTERNAL_FRAME);
}
else
{
// We always store the current CONTEXT, so just copy it into the buffer.
CORDbgCopyThreadContext(pContext, &m_context);
}
// We always store the current CONTEXT, so just copy it into the buffer.
CORDbgCopyThreadContext(pContext, &m_context);
}
}
}
Expand Down Expand Up @@ -376,11 +339,6 @@ void CordbStackWalk::SetContextWorker(CorDebugSetContextFlag flag, ULONG32 conte
ThrowWin32(ERROR_INSUFFICIENT_BUFFER);
}

// invalidate the cache
m_pCachedFrame.Clear();
m_cachedHR = S_OK;
m_fIsOneFrameAhead = false;

DT_CONTEXT * pSrcContext = reinterpret_cast<DT_CONTEXT *>(context);

// Check the incoming CONTEXT using a temporary CONTEXT buffer before updating our real CONTEXT buffer.
Expand Down Expand Up @@ -450,40 +408,26 @@ HRESULT CordbStackWalk::Next()
PUBLIC_REENTRANT_API_BEGIN(this)
{
RefreshIfNeeded();
if (m_fIsOneFrameAhead)

IDacDbiInterface * pDAC = GetProcess()->GetDAC();
IDacDbiInterface::FrameType ft = IDacDbiInterface::kInvalid;

IfFailThrow(pDAC->GetStackWalkCurrentFrameInfo(this->m_pSFIHandle, NULL, &ft));
if (ft == IDacDbiInterface::kAtEndOfStack)
{
// We have already unwound to the next frame when we materialize the CordbNativeFrame
// for the current frame. So we just need to clear the cache because we are already at
// the next frame.
if (m_pCachedFrame != NULL)
{
m_pCachedFrame.Clear();
}
m_cachedHR = S_OK;
m_fIsOneFrameAhead = false;
ThrowHR(CORDBG_E_PAST_END_OF_STACK);
}
else
{
IDacDbiInterface * pDAC = GetProcess()->GetDAC();
IDacDbiInterface::FrameType ft = IDacDbiInterface::kInvalid;

IfFailThrow(pDAC->GetStackWalkCurrentFrameInfo(this->m_pSFIHandle, NULL, &ft));
if (ft == IDacDbiInterface::kAtEndOfStack)
{
ThrowHR(CORDBG_E_PAST_END_OF_STACK);
}
// update the cahced flag to indicate that we have reached an unwind CONTEXT
m_cachedSetContextFlag = SET_CONTEXT_FLAG_UNWIND_FRAME;
Comment thread
rcj1 marked this conversation as resolved.
Outdated

// update the cahced flag to indicate that we have reached an unwind CONTEXT
m_cachedSetContextFlag = SET_CONTEXT_FLAG_UNWIND_FRAME;

if (UnwindStackFrame())
{
hr = S_OK;
}
else
{
hr = CORDBG_S_AT_END_OF_STACK;
}
if (UnwindStackFrame())
{
hr = S_OK;
}
else
{
hr = CORDBG_S_AT_END_OF_STACK;
}
}
PUBLIC_REENTRANT_API_END(hr);
Expand Down Expand Up @@ -525,17 +469,6 @@ HRESULT CordbStackWalk::GetFrame(ICorDebugFrame ** ppFrame)
}
PUBLIC_REENTRANT_API_END(hr);

if (FAILED(hr))
{
if (m_fIsOneFrameAhead && (m_pCachedFrame == NULL))
{
// We encountered a problem when we try to materialize a CordbNativeFrame.
// Cache the failure HR so that we can return it later if the caller
// calls GetFrame() again or GetContext().
m_cachedHR = hr;
}
}

return hr;
}

Expand All @@ -556,24 +489,6 @@ HRESULT CordbStackWalk::GetFrameWorker(ICorDebugFrame ** ppFrame)

RSInitHolder<CordbFrame> pResultFrame(NULL);

if (m_fIsOneFrameAhead)
{
if (m_pCachedFrame != NULL)
{
pResultFrame.Assign(m_pCachedFrame);
pResultFrame.TransferOwnershipExternal(ppFrame);
return S_OK;
}
else
{
// We encountered a problem when we were trying to initialize the CordbNativeFrame.
// However, the problem occurred after we have unwound the current frame.
// Whatever error code we return, it should be the same one GetContext() returns.
_ASSERTE(FAILED(m_cachedHR));
ThrowHR(m_cachedHR);
}
}

IDacDbiInterface * pDAC = NULL;
Debugger_STRData frameData;
ZeroMemory(&frameData, sizeof(frameData));
Expand Down Expand Up @@ -628,18 +543,6 @@ HRESULT CordbStackWalk::GetFrameWorker(ICorDebugFrame ** ppFrame)

HRESULT hr = S_OK;

// In order to find the FramePointer on x86, we need to unwind to the next frame.
// Technically, only x86 needs to do this, because the x86 runtime stackwalker doesn't uwnind
// one frame ahead of time. However, we are doing this on all platforms to keep things simple.
BOOL fSuccess = UnwindStackFrame();
(void)fSuccess; //prevent "unused variable" error from GCC
_ASSERTE(fSuccess);

m_fIsOneFrameAhead = true;
#if defined(TARGET_X86)
IfFailThrow(pDAC->GetFramePointer(m_pSFIHandle, &frameData.fp));
#endif // TARGET_X86

// currentFuncData contains general information about the method.
// It has no information about any particular jitted instance of the method.
Debugger_FuncData * pFuncData = &(frameData.v.funcData);
Expand Down Expand Up @@ -695,7 +598,6 @@ HRESULT CordbStackWalk::GetFrameWorker(ICorDebugFrame ** ppFrame)
&frameCtx);

pResultFrame.Assign(static_cast<CordbFrame *>(pNativeFrame));
m_pCachedFrame.Assign(static_cast<CordbFrame *>(pNativeFrame));

// @dbgtodo dynamic language debugging
// If we are dealing with a dynamic method (e.g. an IL stub, a LCG method, etc.),
Expand Down Expand Up @@ -811,18 +713,6 @@ HRESULT CordbStackWalk::GetFrameWorker(ICorDebugFrame ** ppFrame)
{
_ASSERTE(frameData.eType == Debugger_STRData::cRuntimeNativeFrame);

// In order to find the FramePointer on x86, we need to unwind to the next frame.
// Technically, only x86 needs to do this, because the x86 runtime stackwalker doesn't uwnind
// one frame ahead of time. However, we are doing this on all platforms to keep things simple.
BOOL fSuccess = UnwindStackFrame();
(void)fSuccess; //prevent "unused variable" error from GCC
_ASSERTE(fSuccess);

m_fIsOneFrameAhead = true;
#if defined(TARGET_X86)
IfFailThrow(pDAC->GetFramePointer(m_pSFIHandle, &frameData.fp));
#endif // TARGET_X86

// Lookup the appdomain that the thread was in when it was executing code for this frame. We pass this
// to the frame when we create it so we can properly resolve locals in that frame later.
CordbAppDomain * pCurrentAppDomain = GetProcess()->GetAppDomain();
Expand All @@ -834,7 +724,6 @@ HRESULT CordbStackWalk::GetFrameWorker(ICorDebugFrame ** ppFrame)
&frameCtx);

pResultFrame.Assign(static_cast<CordbFrame *>(pRuntimeFrame));
m_pCachedFrame.Assign(static_cast<CordbFrame *>(pRuntimeFrame));

STRESS_LOG2(LF_CORDB, LL_INFO1000, "CSW::GFW - runtime unwindable stack frame (%p): 0x%p",
this, pRuntimeFrame);
Expand Down
25 changes: 0 additions & 25 deletions src/coreclr/debug/inc/dacdbiinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1229,31 +1229,6 @@ IDacDbiInterface : public IUnknown

virtual HRESULT STDMETHODCALLTYPE GetStackParameterSize(CORDB_ADDRESS controlPC, OUT ULONG32 * pRetVal) = 0;

//
// Get the FramePointer of the current frame where the stackwalker is stopped at.
//
// Arguments:
// pSFIHandle - the handle to the stackwalker
// pRetVal - [out] The FramePointer of the current frame.
//
// Return Value:
// S_OK on success; otherwise, an appropriate failure HRESULT.
//
// Notes:
// The FramePointer of a stack frame is:
// the stack address of the return address on x86,
// the current SP on AMD64,
//
// On x86, to get the stack address of the return address, we need to unwind one more frame
// and use the SP of the caller frame as the FramePointer of the callee frame. This
// function does NOT do that. It just returns the SP. The caller needs to handle the
// unwinding.
//
// The FramePointer of an explicit frame is just the stack address of the explicit frame.
//

virtual HRESULT STDMETHODCALLTYPE GetFramePointer(StackWalkHandle pSFIHandle, OUT FramePointer * pRetVal) = 0;

//
// Check whether the specified CONTEXT is the CONTEXT of the leaf frame. This function doesn't care
// whether the leaf frame is native or managed.
Expand Down
Loading
Loading