Skip to content

Fix interpreter SetIP breakpoint handling #131784

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
matouskozak wants to merge 1 commit into
base: main
Choose a base branch
from
+25 −7
Open

Fix interpreter SetIP breakpoint handling #131784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions src/coreclr/interpreter/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,8 +1072,12 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArray<Reloc*
if (ilOffset < (uint32_t)m_ILCodeSizeFromILHeader)
{
uint32_t nativeOffset = ConvertOffset(ins->nativeOffset);
bool isEmptyILStack = (ins->flags & INTERP_INST_FLAG_EMPTY_IL_STACK) != 0;
bool isFilterOrCatchEntry = m_pCBB->isFilterOrCatchFuncletEntry && (ilOffset == (uint32_t)m_pCBB->ilOffset);

// Only emit mapping entries at IL offsets where the evaluation stack is empty
if ((ins->flags & INTERP_INST_FLAG_EMPTY_IL_STACK) &&
// or at filter and catch entries, where the exception object is on the stack.
if ((isEmptyILStack || isFilterOrCatchEntry) &&
((m_ILToNativeMapSize == 0) || (m_pILToNativeMap[m_ILToNativeMapSize - 1].ilOffset != ilOffset)))
{
// This code assumes that instructions for the same IL offset are emitted in a single run without
Expand All @@ -1096,7 +1100,9 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArray<Reloc*

m_pILToNativeMap[m_ILToNativeMapSize].ilOffset = ilOffset;
m_pILToNativeMap[m_ILToNativeMapSize].nativeOffset = nativeOffset;
m_pILToNativeMap[m_ILToNativeMapSize].source = ICorDebugInfo::STACK_EMPTY;
m_pILToNativeMap[m_ILToNativeMapSize].source = isEmptyILStack
? ICorDebugInfo::STACK_EMPTY
: ICorDebugInfo::SOURCE_TYPE_INVALID;
m_ILToNativeMapSize++;
}

Expand Down
22 changes: 17 additions & 5 deletions src/coreclr/vm/interpexec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,11 +845,16 @@ static void InterpBreakpoint(const int32_t *ip, const InterpMethodContextFrame *
EX_END_CATCH
pThread->SetFilterContext(NULL);

// The debugger may have moved execution via SetIP. If so, drop the bypass
// (it was set up for the original IP) and resume at the new context via
// ResumeAfterCatchException.
// The debugger may have moved execution via SetIP. Preserve a bypass
// created for the destination before resuming at the new context.
if ((GetIP(&ctx) != (PCODE)ip) || (GetSP(&ctx) != (DWORD64)pFrame))
{
if (GetIP(&ctx) == (PCODE)savedBypassAddress)

@BrzVlad BrzVlad Aug 6, 2026 •
edited
Loading

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.

In which scenario does this happen ? Say the debugger wants to continue execution at a different ip. Then it would just set the ip to the new location and it just starts executing from there. What would be the point of setting a bypass. If the target instruction is a breakpoint, sounds like it should just execute it normally.

Maybe the debugger still wants to execute the original instruction that was trapped, before dispatching to the new location ? In that case, isn't savedBypassAddress pointing to the original breakpoint location ? Or is savedBypassAddress always pointing to the new target with the opcode from the other trapping instruction, suggesting that the debugger has enough information to always set the bypass to the right values. In which case, why are we doing this equality check, should we just set the bypass unconditionally, unifying with the code below ?

@matouskozak matouskozak Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In which scenario does this happen ?

This is for the case when SetIP moves execution to a place that already has a breakpoint. After debugger resumes we would hit the breakpoint again.

If the target instruction is a breakpoint, sounds like it should just execute it normally.

I'm not sure we can because we need to exit through ThrowResumeAfterCatchException to move to the new IP, thus skipping the regular breakpoint bypass path.

Maybe the debugger still wants to execute the original instruction that was trapped, before dispatching to the new location ?

I'm not sure that's what debugger want, I would expect that if execution is changed via SetIP, we just move to the new location.

should we just set the bypass unconditionally, unifying with the code below ?

We only need the bypass in case we go to breakpoint with SetIP other case will execute normally.

{
pThreadContext->m_bypassAddress = savedBypassAddress;
pThreadContext->m_bypassOpcode = savedBypassOpcode;
}

ThrowResumeAfterCatchException(GetSP(&ctx), GetIP(&ctx));
}

Expand Down Expand Up @@ -1500,10 +1505,17 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
INTOP_CASE(INTOP_BREAKPOINT)
{
pFrame->ip = ip;
LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Hit breakpoint at IP %p\n", ip));
InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame);

int32_t bypassOpcode = 0;
if (pThreadContext->HasBypass(ip, &bypassOpcode))

@janvorli janvorli Aug 6, 2026

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.

It is not clear to me why we want to execute the bypass byte code. Do we do that for JITted code too? I would expect that in native code, if I set a breakpoint at say a division instruction, we would not execute that instruction until we resume after breakpoint. Why would the interpreter be different?

@matouskozak matouskozak Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This bypass dispatch was intended for a case where SetIP redirects to a location with another breakpoint. In that case, when we resume execution via ThrowResumeAfterCatchException, we would hit the breakpoint at SetIP location, bypassing it here we can dispatch to the underlying opcode instead of hitting another breakpoint.

I thought this would be safe, as I don't expect we would have more than one bypass set at given ip and so if there is a bypass already set when we hit INTOP_BREAKPOINT it must have come from SetIP.

@tommcdon tommcdon Aug 7, 2026

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.

Do we do that for JITted code too?

Yes we do have the same patch-skip concept for JITted code, implemented at the native instruction level.

I would expect that in native code, if I set a breakpoint at say a division instruction, we would not execute that instruction until we resume after breakpoint. Why would the interpreter be different?

Correct. We do not execute the patched instruction when stopping at the breakpoint. The bypass is installed when the debugger resumes execution and executes the saved original instruction once. With SetIP, if the destination contains a breakpoint patch, the interpreter needs the same behavior; otherwise, it immediately reports that destination breakpoint instead of executing the requested instruction.

{
LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Pre-callback bypass at IP %p with opcode 0x%x\n", ip, bypassOpcode));
pThreadContext->ClearBypass();
INTOP_DISPATCH(bypassOpcode);
}

LOG((LF_CORDB, LL_INFO10000, "InterpExecMethod: Hit breakpoint at IP %p\n", ip));
InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame);

// After debugger callback, check if bypass was set on the thread context
if (pThreadContext->HasBypass(ip, &bypassOpcode))
Expand Down
Loading