Skip to content

[clr-interp] Implement Managed Return Value support for the interpreter - #127760

Merged
kotlarmilos merged 7 commits into
dotnet:mainfrom
kotlarmilos:interp-mrv-return-value
Jun 18, 2026
Merged

[clr-interp] Implement Managed Return Value support for the interpreter#127760
kotlarmilos merged 7 commits into
dotnet:mainfrom
kotlarmilos:interp-mrv-return-value

Conversation

@kotlarmilos

@kotlarmiloskotlarmilos commented May 4, 2026

Copy link
Copy Markdown
Member

Description

This PR adds managed return value support for interpreted methods so the debugger can surface the value a method just returned when stepping over or out of a call. Building on the variable-home contract from #128479 where EmitCall flags each value-returning IL call/callvirt site. The interpreter stores every return value in that slot, so the generic DI consumer reads it like any other variable home with no interpreter-specific disassembly or ABI logic.

Testing

Ran the MRV debugger tests against a local Checked CoreCLR build with DOTNET_InterpMode=3 and all active MRV tests pass under the interpreter:

  • 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

CopilotAI review requested due to automatic review settings May 4, 2026 15:11
@kotlarmiloskotlarmilos changed the title [interp] Implement Managed Return Value support for the interpreter[clr-interp] Implement managed return value support for the interpreterMay 4, 2026
@kotlarmiloskotlarmilos changed the title [clr-interp] Implement managed return value support for the interpreter[clr-interp] Implement Managed Return Value support for the interpreterMay 4, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@kotlarmiloskotlarmilos added this to the 11.0.0 milestone May 4, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 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
FileDescription
src/coreclr/interpreter/compiler.hAdds native-map capacity tracking for interpreter-emitted debug boundaries.
src/coreclr/interpreter/compiler.cppEmits expanded IL/native mappings and interpreter callsite markers.
src/coreclr/debug/di/rsthread.cppReads interpreted return values from synthetic FP-relative variable homes.
src/coreclr/debug/di/rspriv.hDeclares the new interpreter-specific DI helper.
src/coreclr/debug/di/module.cppDecodes interpreter callsite lengths and dvar offsets in DI.

Copilot's findings

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

Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

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

Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
@kotlarmilos
kotlarmilosforce-pushed the interp-mrv-return-value branch from 3c1ac90 to 9f2787aCompareMay 5, 2026 14:09
CopilotAI review requested due to automatic review settings May 5, 2026 14:15
@kotlarmilos
kotlarmilosforce-pushed the interp-mrv-return-value branch from 9f2787a to 946174bCompareMay 5, 2026 14:15
@kotlarmilos
kotlarmilosforce-pushed the interp-mrv-return-value branch from 946174b to 9bffeb9CompareMay 5, 2026 14:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

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

Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated

@noahfalknoahfalk left a comment

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.

@kotlarmilos - If possible lets hold off on this one until we settle what data contract we want here overall. If getting this in is blocking making progress elsewhere let me know but I'm guessing its relatively isolated. If we do wind up embedding this interpreter info into the debugger code we'll probably want to define some new IDacDbiInterface APIs and put the logic that understands interpreter disassembly in the DAC instead.

Comment threadsrc/coreclr/debug/di/module.cpp Outdated
@kotlarmilos
kotlarmilos marked this pull request as draft May 12, 2026 13:57
kotlarmilosand others added 2 commits June 11, 2026 10:34
Rework interpreter managed-return-value (MRV) support to use the
CALL_RETURN_ILNUM native-variable contract introduced in dotnet#128479
instead of the obsolete CALL_INSTRUCTION source-map encoding.
The interpreter now emits one ICorDebugInfo::CALL_RETURN_ILNUM
NativeVarInfo entry per value-returning managed call site through the
standard setVars path. Each entry describes the call's destination var
(an FP-relative stack slot) as the return value home, with
callReturnValueILOffset set to the call's IL offset and startOffset set
to the post-call native offset. The generic DI consumer
(GetReturnValueVariableHomes / GetNativeVariable) handles the VLT_STK
location identically to ordinary interpreter locals, so all of the PR's
former DI-side instruction-decoding additions are obsolete and dropped
(reverted to main): module.cpp, rsthread.cpp, rspriv.h, dacdbiimpl.cpp,
and the dacdbistructures isInterpreted plumbing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 11, 2026 08:41

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment threadsrc/coreclr/interpreter/compiler.cpp Outdated
kotlarmilosand others added 2 commits June 11, 2026 11:28
The debugger resolves managed return values only for IL call/callvirt
opcodes (CordbNativeCode::GetCallSignature), so CALL_RETURN_ILNUM entries
emitted for calli sites are never consumed and only grow the var table.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 11, 2026 09:36
@kotlarmilos
kotlarmilos marked this pull request as ready for review June 11, 2026 09:37
@kotlarmilos

Copy link
Copy Markdown
MemberAuthor

@kotlarmilos - If possible lets hold off on this one until we settle what data contract we want here overall. If getting this in is blocking making progress elsewhere let me know but I'm guessing its relatively isolated. If we do wind up embedding this interpreter info into the debugger code we'll probably want to define some new IDacDbiInterface APIs and put the logic that understands interpreter disassembly in the DAC instead.

This PR is rebased on #128479, in which EmitCall flags each IL call/callvirt site that returns a value.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new

@noahfalknoahfalk left a comment

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.

Looks great!

@kotlarmilos
kotlarmilos merged commit 50bc3b3 into dotnet:mainJun 18, 2026
111 of 113 checks passed
eiriktsarpalis pushed a commit that referenced this pull request Jul 15, 2026
…er (#127760)
## Description
This PR adds managed return value support for interpreted methods so the
debugger can surface the value a method just returned when stepping over
or out of a call. Building on the variable-home contract from #128479
where `EmitCall` flags each value-returning IL `call`/`callvirt` site.
The interpreter stores every return value in that slot, so the generic
DI consumer reads it like any other variable home with no
interpreter-specific disassembly or ABI logic.
## Testing
Ran the MRV debugger tests against a local Checked CoreCLR build with
`DOTNET_InterpMode=3` and all active MRV tests pass under the
interpreter:
- 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>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 20, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kotlarmilos@BrzVlad@noahfalk@janvorli