Skip to content

Move FuncEvalFrame/DebuggerU2MCatchHandlerFrame detection to SfiNextWorker#127683

Open
tommcdon wants to merge 4 commits intodotnet:mainfrom
tommcdon:dev/tommcdon/debugNestedFuncEval
Open

Move FuncEvalFrame/DebuggerU2MCatchHandlerFrame detection to SfiNextWorker#127683
tommcdon wants to merge 4 commits intodotnet:mainfrom
tommcdon:dev/tommcdon/debugNestedFuncEval

Conversation

@tommcdon
Copy link
Copy Markdown
Member

@tommcdon tommcdon commented May 2, 2026

The FuncEvalFrame and DebuggerU2MCatchHandlerFrame detection in NotifyExceptionPassStarted relied on stale StackFrameIterator state from the end of pass 1. The iterator is no longer guaranteed to be in SFITER_FRAME_FUNCTION state at that point, causing the debugger to never receive the CATCH_HANDLER_FOUND notification during func eval.

Move the check to SfiNextWorker where it runs during pass 1 at the native transition boundary with live iterator state and direct access to the frame chain.

Found using internal Visual Studio testing

Fixes #125777

…orker

The FuncEvalFrame and DebuggerU2MCatchHandlerFrame detection in
NotifyExceptionPassStarted relied on stale StackFrameIterator state
from the end of pass 1. The iterator is no longer guaranteed to be in
SFITER_FRAME_FUNCTION state at that point, causing the debugger to
never receive the CATCH_HANDLER_FOUND notification during func eval.

Move the check to SfiNextWorker where it runs during pass 1 at the
native transition boundary with live iterator state and direct access
to the frame chain. This matches the original design intent from
PR dotnet#102470.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a debugger notification regression for exceptions thrown during func eval by moving FuncEvalFrame / DebuggerU2MCatchHandlerFrame detection from NotifyExceptionPassStarted (which could observe stale StackFrameIterator state after pass 1) into SfiNextWorker, where the check runs during pass 1 at the native-transition boundary with live iterator state and direct access to the explicit frame chain.

Changes:

  • Removed FuncEvalFrame / DebuggerU2MCatchHandlerFrame detection from NotifyExceptionPassStarted where StackFrameIterator state could be stale.
  • Added pass-1 native-transition-time detection in SfiNextWorker and issues NotifyOfCHFFilter when the next explicit frame is FuncEvalFrame (skipping ProtectValueClassFrame) or the topmost DebuggerU2MCatchHandlerFrame.

Comment thread src/coreclr/vm/exceptionhandling.cpp Outdated
Comment thread src/coreclr/vm/exceptionhandling.cpp Outdated
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Copilot AI review requested due to automatic review settings May 3, 2026 23:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/coreclr/vm/exceptionhandling.cpp:3657

  • The NotifyExceptionPassStarted pass-2 branch now has an else { for the reverse-pinvoke case that appears to be missing the expected closing braces for the enclosing else // passNumber == 2 block / function. As written, the brace structure around this empty else block doesn’t look balanced and would fail to compile. Please ensure the reverse-pinvoke else block is properly closed and the surrounding scopes are correctly terminated (or remove the empty else entirely if it’s no longer needed).
    }
}

NOINLINE static void NotifyFunctionEnterHelper(StackFrameIterator *pThis, Thread *pThread, ExInfo *pExInfo)

Comment on lines 3654 to 3656
}
}

Comment on lines +4031 to +4037
pNotifyFrame = pNotifyFrame->PtrNextFrame();
_ASSERTE(pNotifyFrame != FRAME_TOP);
}
if ((pNotifyFrame->GetFrameIdentifier() == FrameIdentifier::FuncEvalFrame) || IsTopmostDebuggerU2MCatchHandlerFrame(pNotifyFrame))
{
EEToDebuggerExceptionInterfaceWrapper::NotifyOfCHFFilter((EXCEPTION_POINTERS *)&pTopExInfo->m_ptrs, pNotifyFrame);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid feedback?

Copilot AI review requested due to automatic review settings May 3, 2026 23:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

if (pNotifyFrame->GetFrameIdentifier() == FrameIdentifier::ProtectValueClassFrame)
{
pNotifyFrame = pNotifyFrame->PtrNextFrame();
_ASSERTE(pNotifyFrame != FRAME_TOP);
Comment on lines +4019 to +4020
// explicit frame is a FuncEvalFrame or DebuggerU2MCatchHandlerFrame. The debugger needs this notification
// while the managed stack frames are still present so it can inspect the failure location.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing DEBUG_EXCEPTION_CATCH_HANDLER_FOUND events for exceptions during func eval

3 participants