Add MRV debug info for multi-register FP/mixed struct returns on Unix x64 by tommcdon · Pull Request #129992 · dotnet/runtime

tommcdon · 2026-06-29T18:19:52Z

Adds managed-return-value (MRV) debug-info encoding for value-class returns that live in two registers. Additionally, this change implements CordbJITILFrame::GetNativeVariable for x86 x87 FP-stack return/local variables (missed scenario from #129321)

Fixes #129344

… x64 Adds managed-return-value (MRV) debug-info encoding for value-class returns that live in two registers where at least one is a floating-point register (e.g. a 16-byte struct returned in XMM0+XMM1, or a mixed ValueTuple<double,int> returned in XMM0+RAX on SysV x64). Previously these were silently skipped. - New VarLocType values VLT_REG_FP_REG_FP, VLT_REG_FP_REG, VLT_REG_REG_FP. - JIT producer (scopeinfo/codegencommon) emits the appropriate encoding via storeVariableInTwoRegisters instead of skipping non-integer register pairs. - Serializer (debuginfostore) encodes the two register indices. - DBI consumer reconstructs the value via a snapshot-based TwoRegisterValueHome (64-bit only). - CordbType::ReturnedByValue permits FP/generic fields only for two-register (>8 byte) returns, leaving single-register value classes unsupported. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CordbJITILFrame::GetNativeVariable returned CORDBG_E_IL_VAR_NOT_AVAILABLE for the VLT_FPSTK location kind, leaving the implementation commented out since the initial CoreCLR commit. On x86, floating-point values (including return values, surfaced via GetReturnValueForILOffset) live on the x87 FP stack and are reported as VLT_FPSTK, so inspecting a float/double return or local failed with an unavailable-variable error. Enable the existing GetLocalFloatingPointValue path for TARGET_X86, which already handles loading the x87 stack state and R4/R8 conversion. Behavior on non-x86 targets is unchanged (ARM remains E_NOTIMPL; others retain the CORDBG_E_IL_VAR_NOT_AVAILABLE fallback, where VLT_FPSTK is never produced). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

dotnet-policy-service · 2026-06-29T18:21:16Z

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR extends CoreCLR debug-info (ICorDebugInfo VarLocType / MRV encoding) to represent value-class locations spanning two registers where at least one register is a floating-point register, enabling managed-return-value inspection for Unix x64 multi-reg FP/mixed struct returns and adding x86 x87 FP-stack (VLT_FPSTK) support in CordbJITILFrame::GetNativeVariable.

Changes:

Add new VarLocType encodings (VLT_REG_FP_REG_FP, VLT_REG_FP_REG, VLT_REG_REG_FP) and thread them through varloc equality, debug-info encoding/decoding, and tooling (R2R/AOT readers/writers).
Update the JIT’s scope/return-value debug-info emission to use a new storeVariableInTwoRegisters helper that can encode int+fp / fp+fp register pairs (not just int+int).
Add debugger-side decoding/building of values for the new encodings via a two-register snapshot home, plus implement the missing x86 VLT_FPSTK read path.

Reviewed changes

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

Show a summary per file

File	Description
src/coreclr/vm/util.cpp	Extend VarLoc equality to treat new two-register FP/mixed loc types like `VLT_REG_REG`.
src/coreclr/vm/debuginfostore.cpp	Encode the two register indices for new loc types using the existing `vlRegReg` layout.
src/coreclr/tools/r2rdump/Extensions.cs	Add display handling for new loc types (currently prints raw indices).
src/coreclr/tools/Common/JitInterface/CorInfoTypes.VarInfo.cs	Add new `VarLocType` enum values for tooling/shared interface parity.
src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/DebugInfoTypes.cs	Mirror new `VarLocType` values for R2R reflection reader.
src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/DebugInfo.cs	Parse new loc types as two `uint` payload fields.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DebugInfoTableNode.cs	Emit new loc types with two register fields in var blobs.
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/CodeView/CodeViewSymbolsBuilder.cs	Recognize new loc types in CodeView symbol emission logic.
src/coreclr/jit/scopeinfo.cpp	Add `storeVariableInTwoRegisters`; emit mixed/fp two-reg locations for SysV AMD64 params; update equality/dumps/assert-sync.
src/coreclr/jit/ee_il_dll.cpp	Print new loc types correctly in JIT debug dumps by mapping fp indices back to fp regs.
src/coreclr/jit/codegeninterface.h	Add new loc type enum members and the new `storeVariableInTwoRegisters` API.
src/coreclr/jit/codegencommon.cpp	Emit MRV return location using `storeVariableInTwoRegisters`; keep 32-bit guardrails.
src/coreclr/inc/cordebuginfo.h	Add new `VarLocType` values and document the `vlRegReg` reuse contract for fp/mixed pairs.
src/coreclr/debug/ee/functioninfo.cpp	Add logging for the new loc types in native-var dumps.
src/coreclr/debug/ee/debugger.cpp	Treat new loc types as “enregistered” for valuetype size/classification decisions.
src/coreclr/debug/di/valuehome.cpp	Make reg-reg reads tolerant of struct sizes < 2 registers; add `TwoRegisterValueHome::GetEnregisteredValue`; relax offset assumption for multi-reg returns.
src/coreclr/debug/di/rstype.cpp	Expand `CordbType::ReturnedByValue` gating for 64-bit multi-reg return scenarios, including mixed and generic-parameter fields (two-reg only).
src/coreclr/debug/di/rsthread.cpp	Add `GetLocalTwoRegisterValue`; handle new loc types in `GetNativeVariable`; implement x86 `VLT_FPSTK` read path.
src/coreclr/debug/di/rspriv.h	Declare `GetLocalTwoRegisterValue` and introduce `TwoRegisterValueHome`.

jakobbotsch · 2026-06-29T23:07:42Z

+        VLT_REG_FP_REG_FP, // variable lives in two fp registers (e.g. a 16-byte struct returned in XMM0+XMM1 on Unix x64)
+        VLT_REG_FP_REG,    // low part lives in an fp register, high part in an int register (mixed multi-reg return)
+        VLT_REG_REG_FP,    // low part lives in an int register, high part in an fp register (mixed multi-reg return)
+


Would it be messier to make RegNum include FP registers too instead? It would likely look quite a bit more natural on the JIT side.

This representation also does not scale very well, e.g. HFAs on arm32/arm64 can be returned in up to 4 registers (albeit not mixed registers). Also struct locals can have their fields enregistered in an arbitrary number of mixed registers, so a representation like this excludes us from ever trying to represent that.

Replaced VLT_REG_FP_REG* with a unified RegNum encoding, setting us up for future multi-register encodings on arm.

Replace the three new VarLocType values (VLT_REG_FP_REG_FP, VLT_REG_FP_REG, VLT_REG_REG_FP) with a unified approach that extends the RegNum enum to include FP registers on AMD64 (XMM0-XMM15). This allows VLT_REG_REG to encode any combination of int and FP registers without new VarLocType variants, addressing scalability concerns for future multi-register scenarios (HFAs, multi-field enregistration). Changes: - Add REGNUM_XMM0-REGNUM_XMM15 to RegNum enum (AMD64 only) - Extend g_JITToCorDbgReg to map FP RegNum to CorDebugRegister - Add mapRegNumToDebugRegNum helper in JIT scopeinfo - Simplify storeVariableInRegisters to handle int+FP uniformly - Update DBI VLT_REG_REG path to detect FP registers and use GetLocalTwoRegisterValue for mixed/FP multi-reg values - Remove VLT_REG_FP_REG_FP/VLT_REG_FP_REG/VLT_REG_REG_FP from all consumers (DBI, EE, R2R tools, debuginfostore) x86 is unchanged (no FP registers in RegNum; uses VLT_FPSTK). Tested: x86 Windows, x64 Windows, x64 Linux, ARM64 Windows — all MRV diagnostic tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ireg-fp-debuginfo

Use ICorDebugInfo::RegNum parameter type and static_cast<unsigned> for the cross-enum comparison in the VLT_REG_REG debug display lambda. Fixes -Werror,-Wenum-compare on clang (Linux builds). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

+                // On 32-bit targets, only single-field value classes are
+                // representable (matching the original behavior). More than one
+                // non-static field is unsupported there.
+                if (!allowMultiField && fieldCount++ != 0)
+                {
+                    unsupported = true;
                    break;
+                }


+    // The low half occupies the first register-sized chunk; the high half the second.
+    // The out buffer may be smaller than two registers (e.g. a 12-byte struct returned
+    // in two 8-byte registers), so clamp each copy to the bytes that actually remain.
+    const SIZE_T cbReg = sizeof(*lowWordAddr);
+    const SIZE_T cbTotal = valueOutBuffer.Size();
+    _ASSERTE(cbTotal <= 2 * cbReg);


Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

     */
-    _ASSERTE(offset == 0);
    pRemoteReg.Assign(m_pRemoteRegAddr->Clone());


+    // Two-register returns are encoded via VLT_REG_REG (two integer registers),
+    // VLT_REG_FP_REG_FP (two FP registers), or the mixed VLT_REG_FP_REG /
+    // VLT_REG_REG_FP forms. Single-register returns use VLT_REG / VLT_REG_FP.


+    // A two-register value occupies more than one register's worth of space
+    // and at most two registers' worth. On x86 this is 8 bytes (2*4), on
+    // x64 this is up to 16 bytes (2*8).
+    _ASSERTE((newValue.Size() > sizeof(void*)) && (newValue.Size() <= 2 * sizeof(void*)));
    _ASSERTE(REG_SIZE == sizeof(void*));


Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/debug/di/valuehome.cpp:795

RegisterValueHome::CreateInternalValue now allows offset != 0 for multi-field values, but it still clones and passes the parent EnregisteredValueHome as the field's home. That home describes the full register pair, not the sub-field, so it can enable incorrect write-back if the caller tries to SetValue on a field at a non-zero offset. For non-zero offsets, prefer returning a value backed only by the provided local snapshot (no enregistered home), keeping field inspection correct while avoiding unsafe register writes.

    pRemoteReg.Assign(m_pRemoteRegAddr->Clone());

    EnregisteredValueHomeHolder * pRegHolder = pRemoteReg.GetAddr();

    // create a value for the member field.

+            // On non-AMD64, only int registers are supported in RegNum.
+            if (genIsValidIntReg(reg1))
            {
-                // Integer register parameter. On SysV x64, the second segment
-                // may be in an XMM register for mixed struct passing — drop it
-                // since VLT_REG_REG cannot encode FP registers.
-                // TODO: Supporting this case is tracked by https://github.com/dotnet/runtime/issues/129344
-                if (reg2 != REG_NA && !genIsValidIntReg(reg2))
-                {
-                    reg2 = REG_NA;
-                }
                varLocation.storeVariableInRegisters(reg1, reg2);
            }


+#ifdef TARGET_AMD64
+    assert(genIsValidIntReg(reg) || genIsValidFloatReg(reg));
+    assert((otherReg == REG_NA) || genIsValidIntReg(otherReg) || genIsValidFloatReg(otherReg));
+#else
    assert(genIsValidIntReg(reg));


- scopeinfo.cpp: Use static_cast<unsigned> for cross-enum comparisons in VLT_REG_REG debug display (same fix as ee_il_dll.cpp). - valuehome.cpp: Relax SetEnregisteredValue assert to accept values up to 2*REG_SIZE and safely handle partial high-register fills. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

On non-AMD64 platforms (ARM64, LoongArch64, RISC-V), the JIT can pass FP registers to storeVariableInRegisters for float/double return values and FP-homed parameters. Previously this asserted genIsValidIntReg. Now handles all platforms uniformly: - Single FP register: encodes as VLT_REG_FP with 0-based index (non-AMD64) or unified RegNum (AMD64) - Two-register with any FP on non-AMD64: VLT_INVALID (graceful bail-out since VLT_REG_REG can only encode int regs there) - Simplifies psiBegProlog caller to a single platform-independent guard Fixes NativeAOT ILC assert on LoongArch64/RISC-V and runtime test crashes on ARM64/x86/x64 Checked no_tiered_compilation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

    // Update the proper registers.
    SetContextRegister(pContext, m_reg1Info.m_kRegNumber, highPart); // throws


+    assert(genIsValidIntReg(reg) || genIsValidFloatReg(reg));
+    assert((otherReg == REG_NA) || genIsValidIntReg(otherReg) || genIsValidFloatReg(otherReg));


        case TYP_FLOAT:
        case TYP_DOUBLE:
-            // VLT_REG_FP uses a 0-based FP register index; the DBI adds the
-            // platform-specific XMM0/V0 base when converting to CorDebugRegister.
            this->vlType       = VLT_REG_FP;
-            this->vlReg.vlrReg = (regNumber)(varDsc->GetRegNum() - REG_FP_FIRST);
+            this->vlReg.vlrReg = static_cast<regNumber>(mapRegNumToDebugRegNum(varDsc->GetRegNum()));
            break;


+            this->vlType       = VLT_REG_FP;
+            this->vlReg.vlrReg = static_cast<regNumber>(mapRegNumToDebugRegNum(varDsc->GetRegNum()));
            break;


+    //
+    // Two-register returns are encoded via VLT_REG_REG (two integer registers),
+    // VLT_REG_FP_REG_FP (two FP registers), or the mixed VLT_REG_FP_REG /
+    // VLT_REG_REG_FP forms. Single-register returns use VLT_REG / VLT_REG_FP.


            if (varLoc->vlType == VLT_REG_BYREF)
            {
                printf(" byref");
            }
            break;


            if (var->loc.vlType == (ICorDebugInfo::VarLocType)CodeGenInterface::VLT_REG_BYREF)
            {
                printf(" byref");
            }
            break;


- mapRegNumToDebugRegNum: return REGNUM_COUNT sentinel for XMM16-31 (AVX-512 registers not representable in the RegNum enum) - storeVariableInRegisters: check for sentinel and emit VLT_INVALID - siFillRegisterVarLoc: same guard for float/double and SIMD locals - rstype.cpp: update comments to reflect unified RegNum encoding (removed references to deleted VLT_REG_FP_REG* types) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

tommcdon and others added 2 commits June 26, 2026 20:08

tommcdon added this to the 11.0.0 milestone Jun 29, 2026

tommcdon requested review from jakobbotsch, max-charlamb, rcj1 and thaystg June 29, 2026 18:19

tommcdon self-assigned this Jun 29, 2026

tommcdon requested a review from MichalStrehovsky as a code owner June 29, 2026 18:19

Copilot AI review requested due to automatic review settings June 29, 2026 18:19

tommcdon added the area-Diagnostics-coreclr label Jun 29, 2026

Copilot started reviewing on behalf of tommcdon June 29, 2026 18:20 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

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

Comment thread src/coreclr/tools/r2rdump/Extensions.cs Outdated

jakobbotsch reviewed Jun 29, 2026

View reviewed changes

tommcdon and others added 3 commits July 2, 2026 10:11

Merge remote-tracking branch 'origin/main' into dev/tommcdon/mrv-mult…

0c7b4eb

…ireg-fp-debuginfo

Copilot AI review requested due to automatic review settings July 3, 2026 14:16

Copilot started reviewing on behalf of tommcdon July 3, 2026 14:17 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

tommcdon force-pushed the dev/tommcdon/mrv-multireg-fp-debuginfo branch from a3d69e0 to 1f4a621 Compare July 3, 2026 16:33

Copilot AI review requested due to automatic review settings July 3, 2026 16:33

Copilot started reviewing on behalf of tommcdon July 3, 2026 16:33 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

build-analysis Bot mentioned this pull request Jul 3, 2026

[ci-scan] Test failure: System.Formats.Tar.Tests.TarFile_ExtractToDirectory_File_Tests.ExtractToDirectory_RejectsChainedSymlinkDirectoryTraversal_WithNestedFile #130177

Open

tommcdon force-pushed the dev/tommcdon/mrv-multireg-fp-debuginfo branch from 1f4a621 to d206fe1 Compare July 4, 2026 01:02

Copilot AI review requested due to automatic review settings July 4, 2026 02:12

tommcdon force-pushed the dev/tommcdon/mrv-multireg-fp-debuginfo branch from d206fe1 to cab5285 Compare July 4, 2026 02:12

Copilot started reviewing on behalf of tommcdon July 4, 2026 02:13 View session

Copilot AI reviewed Jul 4, 2026

View reviewed changes

tommcdon force-pushed the dev/tommcdon/mrv-multireg-fp-debuginfo branch from cab5285 to 52ddfbc Compare July 4, 2026 02:28

This was referenced Jul 4, 2026

LibraryImportGenerator.Unit.Tests crashing on linux-x64 mono interpreter #100800

Open

Multiple Helix work items fail on maccatalyst/tvos CoreCLR Release #126460

Open

Copilot AI review requested due to automatic review settings July 5, 2026 11:56

Copilot started reviewing on behalf of tommcdon July 5, 2026 11:57 View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add MRV debug info for multi-register FP/mixed struct returns on Unix x64#129992

Add MRV debug info for multi-register FP/mixed struct returns on Unix x64#129992
tommcdon wants to merge 8 commits into
dotnet:mainfrom
tommcdon:dev/tommcdon/mrv-multireg-fp-debuginfo

tommcdon commented Jun 29, 2026

Uh oh!

dotnet-policy-service Bot commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

jakobbotsch Jun 29, 2026

Uh oh!

jakobbotsch Jun 29, 2026

Uh oh!

tommcdon Jul 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Update the proper registers.
		SetContextRegister(pContext, m_reg1Info.m_kRegNumber, highPart); // throws

		assert(genIsValidIntReg(reg) \|\| genIsValidFloatReg(reg));
		assert((otherReg == REG_NA) \|\| genIsValidIntReg(otherReg) \|\| genIsValidFloatReg(otherReg));

Uh oh!

Conversation

tommcdon commented Jun 29, 2026

Uh oh!

dotnet-policy-service Bot commented Jun 29, 2026

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!

jakobbotsch Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

jakobbotsch Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

tommcdon Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants