[clr-interp] Implement Managed Return Value support for the interpreter by kotlarmilos · Pull Request #127760 · dotnet/runtime

kotlarmilos · 2026-05-04T15:11:08Z

Description

GetReturnValueLiveOffset returns the native offsets at which the return value of a call site is live, so the debugger can show the value a method just returned when stepping out. It walks the IL to native map looking for CALL_INSTRUCTION entries to find the post-call IP, then reads the value from the return register. The interpreter does not emit CALL_INSTRUCTION map entries and does not return values in registers, so the feature did not work for any interpreted method.

EmitCodeIns now emits a CALL_INSTRUCTION map entry for each managed call opcode. On the DI side, CordbNativeCode::GetReturnValueLiveOffsetImpl recognizes these entries and computes the post-call IP using a per-opcode slot length table built from intops.def. A new CordbNativeCode::GetInterpreterCallDvarOffset decodes the call at that IP and returns the FP-relative byte offset of the call's destination var.

The IL to native map previously emitted entries only at IL offsets where the evaluation stack was empty. It now emits one entry per IL offset and tags STACK_EMPTY only when the stack actually was empty. The stepper only stops on STACK_EMPTY, so stepping is unchanged, but breakpoint binding and IP to source mapping now also work at non-empty-stack offsets.

This fixes the following interpreter debugger test failures:

MRV.TestArrays
MRV.TestAsyncMethods
MRV.TestClasses
MRV.TestComplexGenerics
MRV.TestEnums
MRV.TestGenerics
MRV.TestMisc
MRV.TestNativeCalls
MRV.TestPointers
MRV.TestPrimitiveTypes
MRV.TestReflectionWithAbstractMethod
MRV.TestRefs
MRV.TestStructs

dotnet-policy-service · 2026-05-04T15:12:46Z

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR extends CoreCLR interpreter debug info so managed return values can be surfaced for interpreted calls, and broadens IL/native mapping beyond stack-empty offsets. It fits into the debugger/interpreter pipeline by teaching the interpreter to emit richer boundary data and teaching DI to decode interpreter callsites.

Changes:

Adds interpreter-side IL/native map capacity tracking and emits per-IL-offset mappings plus CALL_INSTRUCTION entries for interpreter call opcodes.
Adds DI-side interpreter opcode-length decoding and synthetic stack-home lookup for interpreter return values.
Wires CordbJITILFrame to read interpreted return values from FP-relative dvars instead of native return registers.

Review found a blocking correctness issue: the new CALL_INSTRUCTION tagging in compiler.cpp also marks compiler-inserted helper calls (for example the helper emitted by EmitPushLdvirtftn), so GetReturnValueLiveOffset can report extra offsets for a single IL callsite and one of those offsets corresponds to the helper’s function-pointer result rather than the user call’s return value.

Show a summary per file

File	Description
`src/coreclr/interpreter/compiler.h`	Adds native-map capacity tracking for interpreter-emitted debug boundaries.
`src/coreclr/interpreter/compiler.cpp`	Emits expanded IL/native mappings and interpreter callsite markers.
`src/coreclr/debug/di/rsthread.cpp`	Reads interpreted return values from synthetic FP-relative variable homes.
`src/coreclr/debug/di/rspriv.h`	Declares the new interpreter-specific DI helper.
`src/coreclr/debug/di/module.cpp`	Decodes interpreter callsite lengths and dvar offsets in DI.

Copilot's findings

Files reviewed: 5/5 changed files
Comments generated: 1

Copilot

Copilot's findings

Files reviewed: 5/5 changed files
Comments generated: 3

Copilot

Copilot's findings

Files reviewed: 5/5 changed files
Comments generated: 1

BrzVlad · 2026-05-06T04:42:59Z

-                int offset = GetCallInstructionLength(nativeBuffer, fetched);
+                int offset = -1;
+#ifdef FEATURE_INTERPRETER
+                if (fetched >= sizeof(int32_t))


This looks like a hack. Surely there must be a way to determine whether the method that this code belongs to is interpreted or not. Otherwise we just rely on chance not to collide with native encoding.

Yes I agree. Added an isInterpreted flag on NativeCodeFunctionData which is populated from EECodeInfo::IsInterpretedCode() in the DAC.

BrzVlad · 2026-05-06T04:48:10Z

+        int32_t dvarOffset = 0;
+        HRESULT hrInterp = pCode->GetInterpreterCallDvarOffset(ILoffset, currentOffset, &dvarOffset);
+        if (FAILED(hrInterp))
+            return hrInterp;


Am I reading it correctly that if the assumed interpreter decoding fails, then we straight up fail the entire method, rather than going through to the compiled code fallback ? We should also figure out how to reliably determine if the method is interpreted or not so we don't rely on this type of behavior.

Yes. Updated code to propagate any decode failure rather than silently falling into the register-based path

BrzVlad · 2026-05-06T04:53:08Z

-    INTERP_INST_FLAG_EMPTY_IL_STACK    = 0x04
+    INTERP_INST_FLAG_EMPTY_IL_STACK    = 0x04,
+    // The instruction is a user IL call site reportable to ICorDebugCode3::GetReturnValueLiveOffset
+    INTERP_INST_FLAG_CALL_INSTRUCTION   = 0x08


This naming is very confusing. We should either use the already existing INTERP_INST_FLAG_CALL or rename this one to suggset that it is for debug reporting purposes only (Maybe insert DBG somewhere in the name)

INTERP_INST_FLAG_CALL is set on every call IR including compiler-inserted helpers. Renamed to
INTERP_INST_FLAG_DBG_CALL_INSTRUCTION to make the debug-only purpose explicit

BrzVlad · 2026-05-06T04:58:45Z

                m_pILToNativeMap[m_ILToNativeMapSize].nativeOffset = nativeOffset;
-                m_pILToNativeMap[m_ILToNativeMapSize].source = ICorDebugInfo::STACK_EMPTY;
+                m_pILToNativeMap[m_ILToNativeMapSize].source =
+                    (ICorDebugInfo::SourceTypes)(ICorDebugInfo::STACK_EMPTY | ICorDebugInfo::CALL_INSTRUCTION);


Why are we putting STACK_EMPTY unconditionally here ?

I was trying to keep the same behavior as before the change. Actually, the first IL to native map entry at the same IL offset already has STACK_EMPTY when the IL stack is empty, so the second entry only needs to mark the CALL_INSTRUCTION.

GetReturnValueLiveOffset returns the native offsets at which the return value of a call site is live, so the debugger can show the value a method just returned when stepping out. It walks the IL to native map looking for CALL_INSTRUCTION-tagged entries to find the post-call IP, then reads the value from the architectural return register. The interpreter does not emit CALL_INSTRUCTION map entries and does not return values in registers, so the feature did not work for any interpreted method. This change wires it up. EmitCodeIns now emits a CALL_INSTRUCTION-flagged map entry for each managed call opcode (INTOP_CALL, INTOP_CALLVIRT, INTOP_CALLI, etc.). On the DI side, CordbNativeCode::GetReturnValueLiveOffsetImpl recognizes these entries and computes the post-call IP using a per-opcode slot length table built from intops.def. A new CordbNativeCode::GetInterpreterCallDvarOffset decodes the call at that IP and returns the FP-relative byte offset of the call's destination var. CordbJITILFrame::GetReturnValueForILOffsetImpl then fabricates NativeVarInfo{VLT_STK, REGNUM_FP, dvarOffset} and routes through the existing GetNativeVariable, which already reads FP-relative slots correctly out of an interpreter frame. Two compiler-side cleanups come along with it. The IL to native map previously emitted entries only at IL offsets where the evaluation stack was empty. It now emits one entry per IL offset and tags STACK_EMPTY only when the stack actually was empty. The stepper only stops on STACK_EMPTY, so stepping is unchanged, but breakpoint binding and IP to source mapping now also work at non-empty-stack offsets. Separately, NewIns was burning m_isFirstInstForEmptyILStack on emit-nop pseudo-ops like INTOP_DEF, which left the first real IR at the same IL offset (the leading newobj of `var x = new Foo();` for example) untagged and made step-over skip past such statements. The flag is now consumed only when a real IR instruction is emitted. This fixes the following interpreter debugger test failures: - MRV.TestArrays - MRV.TestAsyncMethods - MRV.TestClasses - MRV.TestComplexGenerics - MRV.TestEnums - MRV.TestGenerics - MRV.TestMisc - MRV.TestNativeCalls - MRV.TestPointers - MRV.TestPrimitiveTypes - MRV.TestReflectionWithAbstractMethod - MRV.TestRefs - MRV.TestStructs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 8/8 changed files
Comments generated: 2

Caller uses IfFailRet which treats S_FALSE as success. With the IsInterpreted() gating in place, neither S_FALSE return path is reachable on the happy path, so propagate them as real errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 4, 2026 15:11

Copilot started reviewing on behalf of kotlarmilos May 4, 2026 15:11 View session

github-actions Bot added the area-CodeGen-Interpreter-coreclr label May 4, 2026

dotnet-policy-service Bot assigned kotlarmilos May 4, 2026

kotlarmilos changed the title ~~[interp] Implement Managed Return Value support for the interpreter~~ [clr-interp] Implement managed return value support for the interpreter May 4, 2026

kotlarmilos changed the title ~~[clr-interp] Implement managed return value support for the interpreter~~ [clr-interp] Implement Managed Return Value support for the interpreter May 4, 2026

kotlarmilos added this to the 11.0.0 milestone May 4, 2026

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread src/coreclr/interpreter/compiler.cpp Outdated

kotlarmilos closed this May 4, 2026

kotlarmilos reopened this May 5, 2026

kotlarmilos force-pushed the interp-mrv-return-value branch from fe82178 to 3c1ac90 Compare May 5, 2026 09:27

kotlarmilos marked this pull request as ready for review May 5, 2026 12:51

kotlarmilos requested a review from BrzVlad as a code owner May 5, 2026 12:51

Copilot AI review requested due to automatic review settings May 5, 2026 12:51

kotlarmilos requested review from janvorli and kg as code owners May 5, 2026 12:51

Copilot started reviewing on behalf of kotlarmilos May 5, 2026 12:51 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread src/coreclr/interpreter/compiler.cpp Outdated

Comment thread src/coreclr/interpreter/compiler.cpp

Comment thread src/coreclr/interpreter/compiler.cpp Outdated

kotlarmilos force-pushed the interp-mrv-return-value branch from 3c1ac90 to 9f2787a Compare May 5, 2026 14:09

Copilot AI review requested due to automatic review settings May 5, 2026 14:15

kotlarmilos force-pushed the interp-mrv-return-value branch from 9f2787a to 946174b Compare May 5, 2026 14:15

Copilot started reviewing on behalf of kotlarmilos May 5, 2026 14:15 View session

kotlarmilos force-pushed the interp-mrv-return-value branch from 946174b to 9bffeb9 Compare May 5, 2026 14:15

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread src/coreclr/interpreter/compiler.cpp Outdated

BrzVlad reviewed May 6, 2026

View reviewed changes

kotlarmilos force-pushed the interp-mrv-return-value branch from 9bffeb9 to 4d2200a Compare May 6, 2026 09:59

janvorli reviewed May 6, 2026

View reviewed changes

Comment thread src/coreclr/debug/di/module.cpp Outdated

janvorli reviewed May 6, 2026

View reviewed changes

Comment thread src/coreclr/debug/di/module.cpp Outdated

Copilot AI review requested due to automatic review settings May 6, 2026 11:18

kotlarmilos force-pushed the interp-mrv-return-value branch from 4d2200a to 8afd1eb Compare May 6, 2026 11:18

Copilot started reviewing on behalf of kotlarmilos May 6, 2026 11:18 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Comment thread src/coreclr/debug/di/rsthread.cpp

Comment thread src/coreclr/debug/di/module.cpp

kotlarmilos force-pushed the interp-mrv-return-value branch from 8afd1eb to a1932f7 Compare May 6, 2026 11:53

kotlarmilos force-pushed the interp-mrv-return-value branch from a1932f7 to 23409b8 Compare May 6, 2026 11:55

Conversation

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

Uh oh!

Description

Uh oh!

dotnet-policy-service Bot commented May 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

BrzVlad May 6, 2026

Choose a reason for hiding this comment

Uh oh!

kotlarmilos May 6, 2026

Choose a reason for hiding this comment

Uh oh!

BrzVlad May 6, 2026

Choose a reason for hiding this comment

Uh oh!

kotlarmilos May 6, 2026

Choose a reason for hiding this comment

Uh oh!

BrzVlad May 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!

kotlarmilos May 6, 2026

Choose a reason for hiding this comment

Uh oh!

BrzVlad May 6, 2026

Choose a reason for hiding this comment

Uh oh!

kotlarmilos May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kotlarmilos commented May 4, 2026 •

edited

Loading

BrzVlad May 6, 2026 •

edited

Loading