[clr-interp] Support for breakpoints and stepping by matouskozak · Pull Request #123251 · dotnet/runtime

matouskozak · 2026-01-16T08:39:04Z

Summary

This PR adds debugger support for the CoreCLR interpreter, enabling IDE breakpoints and single-stepping functionality.

Key Changes

Breakpoint Support:

Enable breakpoints in interpreter via INTOP_BREAKPOINT opcode injection
Add IL offset 0 → IR offset 0 mapping for method entry breakpoints
Refactor ApplyPatch/UnapplyPatch for interpreter code patches
Add activation flag for interpreter patches (needed because opcode 0 is valid INTOP_RET)

Single-Stepping Support:

Add InterpreterStepHelper class to encapsulate step setup logic
Per-pframe context bypass address/opcode fields for resuming past breakpoints
Implement TrapInterpreterCodeStep for step-in/step-over/step-out
Call OnMethodEnter to notify debugger (needed for step-in on virtual calls)

Interpreter Compiler Changes:

Keep NOP instructions in debug builds (don't collapse them)

Stack Walking:

Workaround interpreter stack walking issues ea1faed (TODO: revert when Cleanup EH / stackwalk interpreter frame handling #124296 is merged)

Testing

Verified IDE breakpoints work with interpreted code
Single-stepping (step-in, step-over, step-out) functional
No regressions to existing debugger support verified locally using diagnostictests

Notes/TODOs

Stack walking workaround needs to be reverted once proper fix is in place
m_interpActivated should be generalized to all patches (stop using opcode == 0 as "not active") [debugger] Replace PRDIsEmpty(opcode) activation check with explicit m_activated flag #124499
s_interpOpLen / opcode name tables in executioncontrol.cpp are debug logging aids — TODO remove
Long-term plan: Extract JIT-specific code into HardwareExecutionControl derived class, create InterpreterExecutionControl for interpreter support

Copilot

Pull request overview

This work-in-progress pull request adds support for managed debugger breakpoints in the CoreCLR interpreter. The changes extend the existing user breakpoint support (e.g., Debugger.Break()) to support IDE breakpoints and enable setting breakpoints when the program is stopped.

Changes:

Adds interpreter single-step thread state flag and supporting methods
Introduces new INTOP_SINGLESTEP opcode for step-over operations
Implements InterpreterWalker to analyze interpreter bytecode for debugger stepping
Modifies breakpoint execution logic to distinguish between IDE breakpoints and step-out breakpoints
Enables JIT completion notifications for interpreter code
Pre-inserts IL offset 0 entry in the IL-to-native map to support method entry breakpoints

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
src/coreclr/vm/threads.h	Adds TSNC_InterpreterSingleStep thread state flag and related methods
src/coreclr/vm/jitinterface.cpp	Removes interpreter code exclusion from JITComplete notifications
src/coreclr/vm/interpexec.cpp	Implements breakpoint and single-step handling with opcode replacement
src/coreclr/vm/codeman.h	Adds IsInterpretedCode() helper method
src/coreclr/interpreter/intops.h	Adds helper functions to classify interpreter opcodes
src/coreclr/interpreter/inc/intops.def	Defines INTOP_SINGLESTEP opcode
src/coreclr/interpreter/compiler.cpp	Pre-inserts IL offset 0 mapping for method entry breakpoints
src/coreclr/debug/ee/interpreterwalker.h	Declares InterpreterWalker class for bytecode analysis
src/coreclr/debug/ee/interpreterwalker.cpp	Implements bytecode walker for debugger stepping operations
src/coreclr/debug/ee/functioninfo.cpp	Uses GetInterpreterCodeFromInterpreterPrecodeIfPresent for code address
src/coreclr/debug/ee/executioncontrol.h	Defines BreakpointInfo structure and GetBreakpointInfo method
src/coreclr/debug/ee/executioncontrol.cpp	Implements INTOP_SINGLESTEP patch support and breakpoint info retrieval
src/coreclr/debug/ee/controller.h	Includes interpreterwalker.h header
src/coreclr/debug/ee/controller.cpp	Implements TrapStep for interpreter using InterpreterWalker
src/coreclr/debug/ee/CMakeLists.txt	Adds interpreterwalker source files to build

src/coreclr/vm/interpexec.cpp

src/coreclr/interpreter/compiler.cpp

src/coreclr/debug/ee/controller.cpp

src/coreclr/vm/interpexec.cpp

src/coreclr/debug/ee/controller.cpp

src/coreclr/debug/ee/CMakeLists.txt

src/coreclr/debug/ee/interpreterwalker.cpp

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.

src/coreclr/vm/interpexec.cpp

src/coreclr/debug/ee/executioncontrol.h

src/coreclr/debug/ee/executioncontrol.cpp

src/coreclr/debug/ee/interpreterwalker.cpp

src/coreclr/vm/interpexec.cpp

src/coreclr/vm/threads.cpp

src/coreclr/debug/ee/interpreterwalker.cpp

src/coreclr/vm/interpexec.cpp

noahfalk · 2026-01-31T07:35:47Z

src/coreclr/debug/ee/controller.cpp

+                    {
+                        // Indirect call (CALLVIRT, CALLI, CALLDELEGATE) - cannot determine target statically
+                        // Use JMC backstop to catch method entry
+                        // TODO: Could we do better? Why we can't use StubManagers to trace indirect calls?


You can StubManagers if there is one that recognizes the code pattern being used to make the indirect call. If the indirect call doesn't need too many instructions to reach the destination you could also enable single-stepping and get there that way.

src/coreclr/debug/ee/controller.cpp

src/coreclr/debug/ee/executioncontrol.cpp

src/coreclr/debug/ee/interpreterwalker.cpp

src/coreclr/debug/ee/executioncontrol.cpp

src/coreclr/debug/ee/controller.cpp

noahfalk · 2026-02-03T12:05:16Z

src/coreclr/debug/ee/executioncontrol.cpp

+    patch->opcode = currentOpcode; // Save original opcode
+
+    // Check if this is a single-step patch by looking at the controller's thread's interpreter SS flag.
+    Thread* pThread = patch->controller->GetThread();


I think of this interface as being an abstraction that mirrors how the debugger would normally interact with hardware. By default it would be ApplyPatch(address) and the native implementation would be something like:

VirtualProtect(address, ...); // make it writable CORDbgInsertBreakpoint(address); VirtualProtect(address, ...); // make it readonly again

Maybe we'd make it a little more sophisticated so that ApplyPatch is also responsible for saving the old opcode in an output parameter. I would not expect this method to get access to the complete DebuggerPatch object or for the implementation to look at fields like patch->controller. If the debugger needs to do single stepping I'd expect either:

The debugger uses an explicit API different from this one to turn single stepping on. The interpreter could use special opcodes as an implementation detail if it wanted to, but it would be responsible for doing all the bookkeeping on its own (without using DebuggerPatchTable).

The debugger emulates single-stepping behavior using an IExecutionControl API that only has breakpoints. In that case the debugger would call this API to set a INTOP_BREAKPOINT instruction. Later when the interpretter sends back the BreakpointHit callback the debugger code would be responsible for looking up the patch at that address and interpretting the breakpoint as the completion of a single step operation. Other architectures do that single step emulation by copying the instruction to a separate buffer but it could also be done inline if the interpretter is going to have a special feature that enables executing the original opcode on a per-thread basis.

The debugger emulates single-stepping behavior using an IExecutionControl API that only has breakpoints. In that case the debugger would call this API to set a INTOP_BREAKPOINT instruction.

Makes sense, I refactored the code to only use INTOP_BREAKPOINT.

Later when the interpretter sends back the BreakpointHit callback the debugger code would be responsible for looking up the patch at that address and interpretting the breakpoint as the completion of a single step operation.

I think this should happen naturally and get handled in

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

Lines 7420 to 7449 in 35b1dd2

if (IsInRange(offset, m_range, m_rangeCount, &info) ||

ShouldContinueStep( &info, offset))

{

LOG((LF_CORDB, LL_INFO10000,

"Intermediate step patch hit at 0x%x\n", offset));

if (!TrapStep(&info, m_stepIn))

TrapStepNext(&info);

EnableUnwind(m_fp);

return TPR_IGNORE;

}

else

{

LOG((LF_CORDB, LL_INFO10000, "Step patch hit at 0x%x\n", offset));

// For a JMC stepper, we have an additional constraint:

// skip non-user code. So if we're still in non-user code, then

// we've got to keep going

DebuggerMethodInfo * dmi = g_pDebugger->GetOrCreateMethodInfo(module, md);

if ((dmi != NULL) && DetectHandleNonUserCode(&info, dmi))

{

return TPR_IGNORE;

}

StackTraceTicket ticket(patch);

PrepareForSendEvent(ticket);

return TPR_TRIGGER;

}

. I'm not yet sure if we will need to do something special to handle breakpoint (used for stepping) and breakpoint (regular) on debugger side for interpreter but this check should handle the notificaion when we the stepping operation is completed.

Other architectures do that single step emulation by copying the instruction to a separate buffer but it could also be done inline if the interpretter is going to have a special feature that enables executing the original opcode on a per-thread basis.

I've added a thread storage for the original opcode which gets retrieved during the interpreter execution loop and re-executed when needed.

Does this align with what you had in mind?

a thread storage for the original opcode which gets retrieved during the interpreter execution loop

You can have multiple threads running the same interpreter code. What happens when a second thread hits a breakpoint and the original code is saved in a thread local storage of some other thread?

My understanding is, that if non-stepping threads hits INTOP_BREAKPOINT, they would still call to debugger but they will NOT trigger MatchPatch flow which calls TriggerPatch which sends debugger notification (only the thread which is stepping should do that), instead it will jump to ActivatePatchSkip which sets the bypass opcode (reads it from the patch) and allows non-stepping threads to continue execution.

It's not the most efficient solution, we could optimize this by having some kind of book-keeping solution with {address, original opcode} on the side which would be accessible in the interpreter execution loop to fast track non-stepping threads bypass flow.

It's not the most efficient solution

I wouldn't worry about optimizing it, the performance requirements for breakpoint bypass aren't very high. At the end if we can use VS to do ~10 steps/sec thats a good speed. (And even if we can't breakpoint bypass is unlikely to be the bottleneck)

noahfalk · 2026-02-03T12:43:08Z

src/coreclr/vm/interpexec.cpp

-                    InterpBreakpoint(ip, pFrame, stack, pInterpreterFrame);
-                    break;
+                    Thread* pThread = GetThread();
+                    bpInfo = execControl->GetBreakpointInfo(ip);


I think we want to get rid of this GetBreakpointInfo() call. Ideally at the interpreter layer all breakpoints will be handled uniformly. To get there:

bpInfo.isStepOut - The interpreter shouldn't be responsible for figuring out what a breakpoint is used for. I see this flag is being used to control some IP adjustment implicitly via the FaultingExceptionFrame but I don't think we want the interpreter doing that either. Instead we can probably just make a contract that the interpreter always reports the IP as being the address where the breakpoint was set. If the debugger needs to sometimes adjust the IP and sometimes not it can be responsible to figure out which breakpoint addresses need that treatment after receiving the Breakpoint callback. It already has access to all the same info this GetBreakpointInfo() API is looking up. Fwiw, I don't yet understand why an adjustment is needed in this case or why it would be conditional, but if there is something conditional that needs to be done the debugger code seems like the right place for it.

bpInfo.originalOpcode - We probably need a more generalized mechanism for the debugger to tell the interpreter when to execute the original opcode. For example we could have some special fields in the CONTEXT only used by interpreter that stores an address and an opcode. The meaning of those fields would be "if the opcode at address X is a breakpoint, execute opcode Y instead". This allows the debugger to enable the breakpoint bypass behavior at any time, not just after a breakpoint was previously hit. The debugger doesn't always skip breakpoints immediately after hitting them. We might need to define some explicit InterpretterCONTEXT or Get/SetXYZ() functions that define where these fields get placed in the CONTEXT data blob.

For example we could have some special fields in the CONTEXT only used by interpreter that stores an address and an opcode. The meaning of those fields would be "if the opcode at address X is a breakpoint, execute opcode Y instead". This allows the debugger to enable the breakpoint bypass behavior at any time, not just after a breakpoint was previously hit. The debugger doesn't always skip breakpoints immediately after hitting them. We might need to define some explicit InterpretterCONTEXT or Get/SetXYZ() functions that define where these fields get placed in the CONTEXT data blob.

@noahfalk Do we need to keep multiple address + opcode pairs or is one enough? I'm thinking that we could just record the current address + opcode when we call ActivatePatchSkip but not sure if there is a scenario where this would break.

If we don't need more than one pair, I was thinking we could store it on the thread like thread->SetInterpreterBypass((const int32_t*)PC, (int32_t)patch->opcode); and then check in interpexec we would just get the opcode and execute it.

I'm ~75% confident that storing a single address will be sufficient

src/coreclr/vm/interpexec.cpp

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.

src/coreclr/vm/threads.h

src/coreclr/interpreter/compiler.cpp

src/coreclr/debug/ee/controller.cpp

src/coreclr/debug/ee/interpreterwalker.cpp

src/coreclr/debug/ee/controller.cpp

src/coreclr/debug/ee/executioncontrol.cpp

src/coreclr/debug/ee/controller.cpp

- New InterpreterWalker class decodes bytecode control flow for stepping - Update TrapStep to use InterpreterWalker for interpreted code - Add per-thread TSNC_InterpreterSingleStep flag for step tracking - ApplyPatch now uses INTOP_SINGLESTEP vs INTOP_BREAKPOINT based on flag - Handle INTOP_SINGLESTEP in interpreter execution loop

- needed for step-in support in virtual calls

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 12 comments.

src/coreclr/debug/ee/controller.cpp

src/coreclr/interpreter/compiler.cpp

src/coreclr/debug/ee/controller.h

src/coreclr/debug/ee/interpreterwalker.cpp

src/coreclr/debug/ee/controller.cpp

src/coreclr/debug/ee/interpreterwalker.cpp

src/coreclr/vm/interpexec.h

src/coreclr/vm/interpexec.cpp

src/coreclr/interpreter/compiler.cpp

src/coreclr/debug/ee/controller.cpp

jkotas · 2026-02-17T15:00:05Z

src/coreclr/vm/interpexec.h

    const int32_t *ip; // This ip is updated only when execution can leave the frame
    PTR_InterpMethodContextFrame pNext;

+#if defined(DEBUGGING_SUPPORTED) && !defined(TARGET_BROWSER)


Are these TARGET_BROWSER ifdefs still needed? I would expect that the CORDebuggerAttached checks should take care of wasm.

If they are still needed for some reason, should it be TARGET_WASM instead? I would expect that the problem is wasm specific, not browser specific.

They are no longer needed. Previously, wasm wasn't linking the debugger files but that appears to be fixed now. Removed && !defined(TARGET_BROWSER).

Edit: actually some are still needed because of the wasm exclusion

runtime/src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt

Line 58 in 6b548dc

set(LIB_CORDBEE cordbee_wks)

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

src/coreclr/debug/ee/controller.cpp

noahfalk · 2026-02-18T08:25:42Z

src/coreclr/debug/ee/controller.cpp

+            {
+                // Indirect call - enable JMC backstop
+                LOG((LF_CORDB,LL_INFO10000,"DS::TICS: Indirect call, enabling MethodEnter backstop\n"));
+                EnableJMCBackStop(info->m_activeFrame.md);


Is it possible that this indirect call will lead to a jitted or R2R method instead of an interpretted one? If so we need a tracking item to make this more robust because not all jitted/R2R code will emit the MethodEnter callback.

Created a tracking issue to support mixed mode debugging #124547. For the moment, we are not planning to support stepping from interpreter to R2R code in .NET 11 but good to track this for future work if we decide to enable it.

noahfalk · 2026-02-18T08:36:07Z

src/coreclr/debug/ee/executioncontrol.cpp

 }

+// Assume controller lock is held by caller
 bool InterpreterExecutionControl::ApplyPatch(DebuggerControllerPatch* patch)


Doesn't have to be in this PR, but I'd prefer if these ExecutionControl methods don't have DebuggerControllerPatch as the parameter to make the interface clearer. Something like:

void ApplyPatch(address, [out] originalOpcode)
void UnapplyPatch(address, originalOpcode)
void BypassPatch(context, address, originalOpcode)

Yes, makes sense #124546

noahfalk · 2026-02-18T08:45:55Z

src/coreclr/debug/ee/executioncontrol.h

    virtual bool UnapplyPatch(DebuggerControllerPatch* patch) override;

+    // Set bypass on the InterpMethodContextFrame so the interpreter executes
+    // the original opcode instead of re-hitting the breakpoint.


This comment should clarify that the bypass is automatically cleared after the first time the breakpoint is bypassed.

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.

src/coreclr/vm/stackwalk.cpp

src/coreclr/debug/ee/controller.cpp

BrzVlad · 2026-02-20T08:24:02Z

src/coreclr/debug/ee/executioncontrol.cpp

+    InterpMethodContextFrame *pFrame = (InterpMethodContextFrame *)GetSP(filterCtx);
+    _ASSERTE(pFrame != NULL);
+
+    pFrame->SetBypass((const int32_t*)patch->address, (int32_t)patch->opcode);


My understanding is that a bypass is set on the frame triggering a breakpoint. Once we continue execution, we immediately clear the bypass. Does this mean we can have at most one bypass, per thread, at any given time ? In that case it seems that we could move the fields to some less hot structure (like InterpThreadContext) ?

There are a few places where debugger modifies CONTEXT (SetIP, func eval) so if we would use thread context they would need to save and restore the thread data in addition to handling the context. However, both setip and func eval currently don't work for interpreter so they will possibly require interpreter specific changes anyways. I originally had it as part of InterpThreadContext but later moved to use InterpMethodContextFrame to align with rest of debugger. If it would cause a perf issues I can move it back to InterpThreadContext and revisit this when addressing setip and func eval functionality.

@BrzVlad Is the concern the extra added fields to InterpMethodContextFrame and increased size?

Yes, that was the primary concern. But also from a conceptual standpoint, where we are allocating these slots in common frame allocation structure, for them to pretty much never be used. And if only one such pair can be in use at one time, it might add confusion to have an unlimited number of these pair reserved (one per each interp frame).

I moved it to InterpThreadContext as it currently doesn't introduce any extra cost on the debugger side. We can revisit moving it elsewhere if the debugger side of things became too complex because of this. fyi: @noahfalk

BrzVlad · 2026-02-20T08:34:24Z

src/coreclr/interpreter/compiler.cpp

        {
+            // In debug builds, emit INTOP_NOP to give sequence points unique native offsets
+            // for proper debugger stepping on `{` and `}` braces
+            if (ins->opcode == INTOP_NOP && m_corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_DEBUG_CODE))


Is this duplicating logic from normal code emit ? Also, if we are actually ending up generating code, then this opcode doesn't really seem to be an emit nop. Would a better approach be to have a new placeholder opcode, that we emit instead for CEE_NOP ? Then the offset mapping happens implicitly with existing code.

Replaced with INTOP_DEBUG_SEQ_POINT which made it much cleaner. Thanks.

BrzVlad · 2026-02-20T08:39:21Z

src/coreclr/vm/interpexec.cpp

@@ -1186,8 +1008,11 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
            // It will be useful for testing e.g. the debug info at various locations in the current method, so let's
            // keep it for such purposes until we don't need it anymore.
            pFrame->ip = (int32_t*)ip;
-
-            switch (*ip)
+            opcode = ip[0];


We have a bunch of places in the exec loop where we use *ip again, inside switch cases. My assumption is that, when a breakpoint is added, the interpreter execution is already suspended. Also, I don't believe we actually have suspension points between switch (*ip) and the following *ip uses. However, this seems somewhat fragile and bound to change. I believe it would make sense to conservatively replace other uses of *ip with opcode so we don't risk them incorrectly observe a potentially added INTOP_BREAKPOINT that would break the opcode execution logic.

Replaced the ocurrances of *ip with opcode.

src/coreclr/vm/interpexec.cpp

BrzVlad · 2026-02-20T08:48:06Z

src/coreclr/vm/interpexec.cpp

+                    // Notify debugger of method entry for step-in support for indirect calls.
+                    if (CORDebuggerAttached() && g_pDebugInterface != NULL && g_pDebugInterface->IsMethodEnterEnabled())
+                    {
+                        g_pDebugInterface->OnMethodEnter((void*)ip);


I don't remember, was there a reason for this not being a dedicated opcode at the start of all methods, when debugging is enabled ? We would remove the need for any checks when there is no debugger. I'm assuming we would have the same runtime binary for app debug/release builds, right ? So DEBUGGING_SUPPORTED would always be defined in production ?

I've previously hit an issue with this approach where the reported ip was not pointing to method entry, I've since solved the issue. Please take a look.

BrzVlad · 2026-02-20T10:24:01Z

src/coreclr/debug/ee/controller.cpp

+        case InterpreterStepHelper::SSR_NeedStepIn:
+            // Handle step-in for calls
+            if (pCallTarget != NULL)
+            {


I'm wondering, if we already have a working mechanism for indirect calls, whether it is beneficial to have this separate direct call approach. Do we expect to gain relevant benefits from this ? For the sake of simplicity and stability, maybe we can use just the general approach.

Do we expect to gain relevant benefits from this ?

It should be slightly faster as we don't have to rely on the global counter.

For the sake of simplicity and stability, maybe we can use just the general approach.

Yes, since we now have the method entry opcode I think we can simplify even though there is going to be slight perf penalty on debugger side.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/debug/ee/controller.h:569

DebuggerControllerPatch::IsActivated relies on m_interpActivated for interpreter patches to avoid treating opcode==0 as “not active”. However, there are activation paths where a patch can become logically active without going through InterpreterExecutionControl::ApplyPatch (e.g., when another patch already exists at the same address and activation is skipped by copying the opcode). In those cases, if the original opcode is 0 (INTOP_RET), IsActivated will still return false, breaking the invariants that bound patches are activated. Ensure m_interpActivated is set/cleared consistently for all interpreter patch activation/deactivation paths, not just ApplyPatch/UnapplyPatch.

    bool IsActivated()
    {
#ifdef FEATURE_INTERPRETER
        // m_interpActivated is set only by InterpreterExecutionControl::ApplyPatch
        if (m_interpActivated)
        {
            _ASSERTE(address != NULL);
            return TRUE;
        }
#endif // FEATURE_INTERPRETER

        // Patch is activate if we've stored a non-zero opcode
        // Note: this might be a problem as opcode 0 may be a valid opcode (see issue 366221).
        if( PRDIsEmpty(opcode) ) {
            return FALSE;
        }

        // Patch is active, so it must also be bound
        _ASSERTE( address != NULL );
        return TRUE;
    }

Copilot · 2026-02-24T12:05:15Z

src/coreclr/debug/ee/executioncontrol.cpp

+        LOG((LF_CORDB, LL_INFO1000, "InterpreterEC::ApplyPatch Patch already applied at %p\n",
+            patch->address));
+        return false;


InterpreterExecutionControl::ApplyPatch returns false when the target slot already contains INTOP_BREAKPOINT. DebuggerController::ActivatePatch doesn’t check ApplyPatch’s return value and asserts the patch is activated afterwards, so this can leave the patch in an unactivated state (and potentially with opcode still empty) while the code is patched. Consider treating this case as an already-applied patch (e.g., fetch/copy the saved original opcode from the existing patch entry and mark this patch activated) or make it a hard assert/unreachable if it should never happen under the controller lock.

Suggested change

LOG((LF_CORDB, LL_INFO1000, "InterpreterEC::ApplyPatch Patch already applied at %p\n",

patch->address));

return false;

// Treat this as an already-applied patch: record the current opcode (the breakpoint)

// and mark the patch as activated without modifying the bytecode slot again.

patch->opcode = currentOpcode;

patch->m_interpActivated = true;

LOG((LF_CORDB, LL_INFO1000, "InterpreterEC::ApplyPatch Patch already applied at %p\n",

patch->address));

return true;

src/coreclr/debug/ee/controller.cpp

matouskozak self-assigned this Jan 16, 2026

matouskozak added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) area-Diagnostics-coreclr labels Jan 16, 2026

matouskozak mentioned this pull request Jan 16, 2026

[clr-interp] Implement interpreter debugging support in CoreCLR #120842

Open

10 tasks

matouskozak force-pushed the interpreter-breakpoints branch from ed5ce4c to dce1adc Compare January 16, 2026 08:52

Copilot AI review requested due to automatic review settings January 30, 2026 10:59

Copilot started reviewing on behalf of matouskozak January 30, 2026 11:00 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 30, 2026 11:19

Copilot started reviewing on behalf of matouskozak January 30, 2026 11:20 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

noahfalk reviewed Jan 31, 2026

View reviewed changes

src/coreclr/debug/ee/executioncontrol.cpp Outdated Show resolved Hide resolved

noahfalk reviewed Feb 3, 2026

View reviewed changes

matouskozak mentioned this pull request Feb 4, 2026

[clr-interp][debugging] Use address of opcode instead of interpreter precode in dji.m_addrOfCode #123998

Open

Copilot AI review requested due to automatic review settings February 5, 2026 10:38

Copilot started reviewing on behalf of matouskozak February 5, 2026 10:39 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

matouskozak added 8 commits February 6, 2026 18:52

refactor ExecutionControl

57f1c6e

enable breakpoints in the interpreter

cb3e9a3

support for regular breakpoints with interpreter

8513f2a

insert IL offset 0 pointing to IR 0

74235f6

disable interpreter breakpoint support for BROWSER

dc240a1

disable CheckRegDisplaySP for interpreter TODO

b0ff0e5

call OnMethodEnter to notify debugger

e3474fc

- needed for step-in support in virtual calls

matouskozak requested a review from tommcdon February 17, 2026 14:40

Copilot started reviewing on behalf of matouskozak February 17, 2026 14:41 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

jkotas reviewed Feb 17, 2026

View reviewed changes

add assert remove browser exclusion

a020a26

Copilot AI review requested due to automatic review settings February 17, 2026 16:13

matouskozak force-pushed the interpreter-breakpoints branch from 1d33785 to a020a26 Compare February 17, 2026 16:13

Copilot started reviewing on behalf of matouskozak February 17, 2026 16:14 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

exclude executioncontrol for target wasm

821f05c

build-analysis bot mentioned this pull request Feb 17, 2026

[android][clr] No peer certificates when executing System.Net.Http.Functional.Tests on Android emulator #124526

Open

noahfalk approved these changes Feb 18, 2026

View reviewed changes

matouskozak added 2 commits February 18, 2026 12:17

prevent calls to (Un)ApplyTraceFlag for interp

c674607

extend comments

28e604f

Copilot AI review requested due to automatic review settings February 18, 2026 12:29

Copilot started reviewing on behalf of matouskozak February 18, 2026 12:30 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

BrzVlad reviewed Feb 20, 2026

View reviewed changes

matouskozak added 4 commits February 23, 2026 15:58

use opcode var instead of *ip in interp exec loop

e9c23fb

simplify interpreter debug sequence points

e717eff

Add INTOP_DEBUG_METHOD_ENTER opcode to mark entry

903bd96

Move breakpoint bypass to InterpThreadContext

bbc1c9f

Copilot AI review requested due to automatic review settings February 24, 2026 11:56

matouskozak force-pushed the interpreter-breakpoints branch from 53c3009 to bbc1c9f Compare February 24, 2026 11:56

Copilot started reviewing on behalf of matouskozak February 24, 2026 11:57 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

matouskozak added 2 commits February 24, 2026 13:19

use "JMC backstop" for step-in unconditionally

6461cda

use %zx instead of %x for size_t offsets in logs

3440998

matouskozak requested review from BrzVlad and noahfalk February 24, 2026 17:27

	if (IsInRange(offset, m_range, m_rangeCount, &info) \|\|
	ShouldContinueStep( &info, offset))
	{
	LOG((LF_CORDB, LL_INFO10000,
	"Intermediate step patch hit at 0x%x\n", offset));

	if (!TrapStep(&info, m_stepIn))
	TrapStepNext(&info);

	EnableUnwind(m_fp);
	return TPR_IGNORE;
	}
	else
	{
	LOG((LF_CORDB, LL_INFO10000, "Step patch hit at 0x%x\n", offset));

	// For a JMC stepper, we have an additional constraint:
	// skip non-user code. So if we're still in non-user code, then
	// we've got to keep going
	DebuggerMethodInfo * dmi = g_pDebugger->GetOrCreateMethodInfo(module, md);

	if ((dmi != NULL) && DetectHandleNonUserCode(&info, dmi))
	{
	return TPR_IGNORE;
	}

	StackTraceTicket ticket(patch);
	PrepareForSendEvent(ticket);
	return TPR_TRIGGER;
	}

Comments

Conversation

matouskozak commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

Testing

Notes/TODOs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matouskozak Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matouskozak Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matouskozak commented Jan 16, 2026 •

edited

Loading

matouskozak Feb 6, 2026 •

edited

Loading

matouskozak Feb 4, 2026 •

edited

Loading

matouskozak Feb 17, 2026 •

edited

Loading