[clr-interp] Implement Managed Return Value support for the interpreter#127760
[clr-interp] Implement Managed Return Value support for the interpreter#127760kotlarmilos wants to merge 3 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg |
There was a problem hiding this comment.
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_INSTRUCTIONentries for interpreter call opcodes. - Adds DI-side interpreter opcode-length decoding and synthetic stack-home lookup for interpreter return values.
- Wires
CordbJITILFrameto 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
fe82178 to
3c1ac90
Compare
3c1ac90 to
9f2787a
Compare
9f2787a to
946174b
Compare
946174b to
9bffeb9
Compare
9bffeb9 to
4d2200a
Compare
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>
4d2200a to
8afd1eb
Compare
8afd1eb to
a1932f7
Compare
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>
a1932f7 to
23409b8
Compare
Description
GetReturnValueLiveOffsetreturns 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 forCALL_INSTRUCTIONentries to find the post-call IP, then reads the value from the return register. The interpreter does not emitCALL_INSTRUCTIONmap entries and does not return values in registers, so the feature did not work for any interpreted method.EmitCodeInsnow emits aCALL_INSTRUCTIONmap entry for each managed call opcode. On the DI side,CordbNativeCode::GetReturnValueLiveOffsetImplrecognizes these entries and computes the post-call IP using a per-opcode slot length table built fromintops.def. A newCordbNativeCode::GetInterpreterCallDvarOffsetdecodes 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_EMPTYonly when the stack actually was empty. The stepper only stops onSTACK_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.TestArraysMRV.TestAsyncMethodsMRV.TestClassesMRV.TestComplexGenericsMRV.TestEnumsMRV.TestGenericsMRV.TestMiscMRV.TestNativeCallsMRV.TestPointersMRV.TestPrimitiveTypesMRV.TestReflectionWithAbstractMethodMRV.TestRefsMRV.TestStructs