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

src/coreclr/debug/di/rspriv.h

@@ -6994,6 +6994,19 @@ class CordbNativeFrame : public CordbFrame, public ICorDebugNativeFrame, public
                                             CordbType * pType,
                                             ICorDebugValue **ppValue);
 
+    // Build a value that lives in two registers, where either register may be an
+    // integer or a floating-point register (e.g. a 16-byte struct returned in
+    // XMM0+XMM1 on Unix x64, or a mixed int/fp multi-register return). lowReg/highReg
+    // hold the low/high 8 bytes of the value; when the corresponding *IsFloat flag is
+    // true the register is a 0-based fp register index, otherwise it is a
+    // CorDebugRegister.
+    HRESULT GetLocalTwoRegisterValue(DWORD lowReg,
+                                            bool lowIsFloat,
+                                            DWORD highReg,
+                                            bool highIsFloat,
+                                            CordbType * pType,
+                                            ICorDebugValue **ppValue);
+
 
     CORDB_ADDRESS GetLSStackAddress(ICorDebugInfo::RegNum regNum, signed offset);
 
@@ -7877,6 +7890,71 @@ class RegRegValueHome: public RegValueHome
     const RegisterInfo               m_reg2Info;
 }; // class RegRegValueHome
 
+// class TwoRegisterValueHome
+// EnregisteredValueHome for a value that lives in two registers where at least one is a
+// floating-point register (e.g. a 16-byte struct returned in XMM0+XMM1 on Unix x64, or a
+// mixed int/fp multi-register return).
+// Floating-point register contents are not reachable through the integer register display, so
+// rather than referencing live registers this home captures a snapshot of the 16-byte value
+// (low 8 bytes followed by high 8 bytes) when it is created. The snapshot is used to populate
+// the value's local object copy and is cloned for field access. Writing back to a
+// multi-register return value is not supported.
+class TwoRegisterValueHome: public EnregisteredValueHome
+{
+public:
+    // initializing constructor
+    // Arguments:
+    //     input:  pFrame - frame to which the value belongs
+    //             pValue - pointer to the snapshot bytes (low 8 bytes followed by high 8 bytes)
+    //             size   - number of valid bytes pointed to by pValue
+    TwoRegisterValueHome(const CordbNativeFrame * pFrame, const BYTE * pValue, ULONG32 size):
+        EnregisteredValueHome(pFrame)
+    {
+        _ASSERTE(size <= sizeof(m_value));
+        memset(m_value, 0, sizeof(m_value));
+        if (pValue != NULL)
+        {
+            memcpy(m_value, pValue, (size < sizeof(m_value)) ? size : (ULONG32)sizeof(m_value));
+        }
+    };
+
+    // copy constructor
+    TwoRegisterValueHome(const TwoRegisterValueHome * pRemoteRegAddr):
+        EnregisteredValueHome(pRemoteRegAddr->m_pFrame)
+    {
+        memcpy(m_value, pRemoteRegAddr->m_value, sizeof(m_value));
+    };
+
+    // make a copy of this instance of TwoRegisterValueHome
+    virtual
+    TwoRegisterValueHome * Clone() const { return new TwoRegisterValueHome(*this); };
+
+    // writing back to a multi-register return value is not supported
+    virtual
+    void SetEnregisteredValue(MemoryRange newValue, DT_CONTEXT * pContext, bool fIsSigned)
+    {
+        ThrowHR(CORDBG_E_SET_VALUE_NOT_ALLOWED_ON_NONLEAF_FRAME);
+    };
+
+    // Gets the snapshot value and returns it to the caller
+    virtual
+    void GetEnregisteredValue(MemoryRange valueOutBuffer);
+
+    // initializing an instance of RemoteAddress is not supported for a local snapshot
+    virtual
+    void CopyToIPCEType(RemoteAddress * pRegAddr)
+    {
+        ThrowHR(E_NOTIMPL);
+    };
+
+    //-------------------------------------
+    // data members
+    //-------------------------------------
+private:
+    // Snapshot of the value: low 8 bytes followed by high 8 bytes.
+    BYTE m_value[2 * sizeof(double)];
+}; // class TwoRegisterValueHome
+
 // class RegAndMemBaseValueHome
 // derived from RegValueHome, this class is also a base class for RegMemValueHome
 // and MemRegValueHome, which add a memory location for reg-mem or mem-reg values

src/coreclr/debug/di/rsthread.cpp

@@ -6787,10 +6787,11 @@ HRESULT CordbNativeFrame::GetLocalDoubleRegisterValue(
             // nickbe
             // 10/31/2002 11:09:42
             //
-            // This assert assumes that the JIT will only partially enregister
-            // objects that have a size equal to twice the size of a register.
+            // The JIT partially enregisters an object across two registers. The
+            // object occupies more than one register (otherwise it would be a
+            // single-register home) and at most two registers' worth of space.
             //
-            _ASSERTE(objectSize == 2 * sizeof(void*));
+            _ASSERTE((objectSize > sizeof(void*)) && (objectSize <= 2 * sizeof(void*)));
         }
     }
 #endif
@@ -7043,6 +7044,108 @@ HRESULT CordbNativeFrame::GetLocalFloatingPointValue(DWORD index,
     return hr;
 }
 
+// Build a value that lives in two registers, where either register may be an
+// integer or a floating-point register (e.g. a 16-byte struct returned in
+// XMM0+XMM1 on Unix x64, or a mixed int/fp multi-register return). The low and
+// high 8-byte halves are gathered from the appropriate register sources into a
+// contiguous local snapshot, then the value is built from that snapshot.
+//
+// Arguments:
+//     lowReg      - the register holding the low 8 bytes. When lowIsFloat is true
+//                   this is a 0-based fp register index, otherwise a CorDebugRegister.
+//     lowIsFloat  - whether the low half is in a floating-point register.
+//     highReg     - the register holding the high 8 bytes. When highIsFloat is true
+//                   this is a 0-based fp register index, otherwise a CorDebugRegister.
+//     highIsFloat - whether the high half is in a floating-point register.
+//     pType       - the type of the value.
+//     ppValue     - [out] the newly created value.
+//
+// Note: This produces a read-only value snapshot (no register value-home for
+//       write-back), which matches how multi-register return values are inspected.
+HRESULT CordbNativeFrame::GetLocalTwoRegisterValue(DWORD            lowReg,
+                                                   bool             lowIsFloat,
+                                                   DWORD            highReg,
+                                                   bool             highIsFloat,
+                                                   CordbType *      pType,
+                                                   ICorDebugValue **ppValue)
+{
+    PUBLIC_REENTRANT_API_ENTRY(this);
+    FAIL_IF_NEUTERED(this);
+    VALIDATE_POINTER_TO_OBJECT(ppValue, ICorDebugValue **);
+    ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess());
+
+    HRESULT hr = S_OK;
+
+    // Snapshot of the value: low 8 bytes followed by high 8 bytes.
+    BYTE valueBuffer[2 * sizeof(double)] = {0};
+
+    EX_TRY
+    {
+        CordbThread * pThread = m_pThread;
+
+        // Ensure the floating-point state is loaded if either half lives in an fp register.
+        if (lowIsFloat || highIsFloat)
+        {
+            if (!pThread->m_fFloatStateValid)
+            {
+                pThread->LoadFloatState();
+            }
+        }
+
+        const DWORD numFloatValues =
+            (DWORD)(sizeof(pThread->m_floatValues) / sizeof(pThread->m_floatValues[0]));
+
+        // Gather the low 8 bytes.
+        if (lowIsFloat)
+        {
+            if (lowReg >= numFloatValues)
+                ThrowHR(E_INVALIDARG);
+            memcpy(valueBuffer, &pThread->m_floatValues[lowReg], sizeof(double));
+        }
+        else
+        {
+            UINT_PTR * pReg = GetAddressOfRegister((CorDebugRegister)lowReg);
+            if (pReg == NULL)
+                ThrowHR(E_INVALIDARG);
+            memcpy(valueBuffer, pReg, sizeof(UINT_PTR));
+        }
+
+        // Gather the high 8 bytes.
+        if (highIsFloat)
+        {
+            if (highReg >= numFloatValues)
+                ThrowHR(E_INVALIDARG);
+            memcpy(valueBuffer + sizeof(double), &pThread->m_floatValues[highReg], sizeof(double));
+        }
+        else
+        {
+            UINT_PTR * pReg = GetAddressOfRegister((CorDebugRegister)highReg);
+            if (pReg == NULL)
+                ThrowHR(E_INVALIDARG);
+            memcpy(valueBuffer + sizeof(double), pReg, sizeof(UINT_PTR));
+        }
+
+        // Build the value from the local snapshot. The value lives in two registers, at
+        // least one of which is a floating-point register, so its contents cannot be
+        // reached through the integer register display. TwoRegisterValueHome captures the
+        // 16-byte snapshot so the value's object copy can be populated and so the home can
+        // be cloned for read-only field access (writing back is not supported).
+        EnregisteredValueHomeHolder pRemoteReg(new TwoRegisterValueHome(this, valueBuffer, sizeof(valueBuffer)));
+        EnregisteredValueHomeHolder * pRegHolder = pRemoteReg.GetAddr();
+
+        CordbValue::CreateValueByType(GetCurrentAppDomain(),
+                                      pType,
+                                      false,
+                                      EMPTY_BUFFER,
+                                      MemoryRange(NULL, 0),
+                                      pRegHolder,
+                                      ppValue);  // throws
+    }
+    EX_CATCH_HRESULT(hr);
+
+    return hr;
+}
+
 //---------------------------------------------------------------------------------------
 //
 // Quick accessor to tell if we're the leaf frame.
@@ -8255,7 +8358,7 @@ HRESULT CordbJITILFrame::GetNativeVariable(CordbType *type,
 #if defined(TARGET_ARM) // @ARMTODO
         hr = E_NOTIMPL;
 #elif defined(TARGET_AMD64)
-        hr = m_nativeFrame->GetLocalFloatingPointValue(pNativeVarInfo->loc.vlReg.vlrReg + REGISTER_AMD64_XMM0,
+        hr = m_nativeFrame->GetLocalFloatingPointValue(ConvertRegNumToCorDebugRegister(pNativeVarInfo->loc.vlReg.vlrReg),
                                                        type, ppValue);
 #elif defined(TARGET_ARM64)
         hr = m_nativeFrame->GetLocalFloatingPointValue(pNativeVarInfo->loc.vlReg.vlrReg + REGISTER_ARM64_V0,
@@ -8295,6 +8398,31 @@ HRESULT CordbJITILFrame::GetNativeVariable(CordbType *type,
         break;
 
     case ICorDebugInfo::VLT_REG_REG:
+#if defined(TARGET_AMD64)
+        {
+            const ICorDebugInfo::RegNum lowReg  = pNativeVarInfo->loc.vlRegReg.vlrrReg1;
+            const ICorDebugInfo::RegNum highReg = pNativeVarInfo->loc.vlRegReg.vlrrReg2;
+            const bool lowIsFloat  = lowReg >= ICorDebugInfo::REGNUM_FP_FIRST;
+            const bool highIsFloat = highReg >= ICorDebugInfo::REGNUM_FP_FIRST;
+
+            if (lowIsFloat || highIsFloat)
+            {
+                // AMD64 extends RegNum with XMM registers, so VLT_REG_REG can
+                // represent mixed int/fp pairs. Other targets still require
+                // dedicated encodings for FP-containing multi-register values.
+                hr = m_nativeFrame->GetLocalTwoRegisterValue(
+                    lowIsFloat ? lowReg - ICorDebugInfo::REGNUM_FP_FIRST
+                               : ConvertRegNumToCorDebugRegister(lowReg),
+                    lowIsFloat,
+                    highIsFloat ? highReg - ICorDebugInfo::REGNUM_FP_FIRST
+                                : ConvertRegNumToCorDebugRegister(highReg),
+                    highIsFloat,
+                    type,
+                    ppValue);
+                break;
+            }
+        }
+#endif
         hr = m_nativeFrame->GetLocalDoubleRegisterValue(
                             ConvertRegNumToCorDebugRegister(pNativeVarInfo->loc.vlRegReg.vlrrReg2),
                             ConvertRegNumToCorDebugRegister(pNativeVarInfo->loc.vlRegReg.vlrrReg1),
@@ -8336,15 +8464,16 @@ HRESULT CordbJITILFrame::GetNativeVariable(CordbType *type,
         break;
 
     case ICorDebugInfo::VLT_FPSTK:
-#if defined(TARGET_ARM) // @ARMTODO
-        hr = E_NOTIMPL;
-#else
-        /*
-        @TODO [Microsoft] We have to make this work!!!!!!!!!!!!!
+#if defined(TARGET_X86)
+        // On x86 floating-point values (including return values) live on the x87
+        // FP stack. vlfReg is the depth from the top of the stack, so add the base
+        // register to form the CorDebugRegister index expected by the helper.
         hr = m_nativeFrame->GetLocalFloatingPointValue(
                          pNativeVarInfo->loc.vlFPstk.vlfReg + REGISTER_X86_FPSTACK_0,
                          type, ppValue);
-                         */
+#elif defined(TARGET_ARM) // @ARMTODO
+        hr = E_NOTIMPL;
+#else
         hr = CORDBG_E_IL_VAR_NOT_AVAILABLE;
 #endif
         break;

src/coreclr/debug/di/rstype.cpp

@@ -1738,9 +1738,46 @@ HRESULT CordbType::ReturnedByValue()
     ULONG32 unboxedSize = 0;
     IfFailRet(GetUnboxedObjectSize(&unboxedSize));
 
+#ifdef TARGET_64BIT
+    // A value type is returned in registers (and is therefore representable by
+    // the managed-return-value debug info) only if it fits in at most two
+    // pointer-sized registers. Larger value types use the return buffer (stack)
+    // path, which the JIT does not currently emit MRV info for.
+    //
+    // On AMD64, the RegNum enum includes FP registers (XMM0-XMM15), so
+    // VLT_REG_REG can encode any combination of int and FP registers for
+    // two-register returns. Single-register returns use VLT_REG / VLT_REG_FP.
+    if (unboxedSize > 2 * sizeof(SIZE_T))
+        return S_FALSE;
+
+    // Whether the value occupies two registers (size in (8, 16] bytes on a
+    // 64-bit target). Single-register (<= pointer-sized) returns only support
+    // the original simple cases: a single integer/pointer-sized non-FP field.
+    // Floating-point and generic (unbound type-parameter) fields are only
+    // encodable for the two-register case (where VLT_REG_REG with unified
+    // RegNum handles all int/FP combinations). Enabling them for single-register
+    // value classes would reach unimplemented paths in the value-home code, so
+    // they remain unsupported there.
+    const bool twoRegister = (unboxedSize > sizeof(SIZE_T));
+
+    // 64-bit targets support multi-field value classes (e.g. ValueTuple<T1, T2>)
+    // returned across two registers.
+    const bool allowMultiField = true;
+#else
+    // 32-bit targets (x86 / arm32): the multi-register FP/mixed managed-return-
+    // value feature (dotnet/runtime#129344) is 64-bit only. Preserve the original
+    // behavior exactly: a value type is representable only if it fits in a single
+    // (pointer-sized) register and has a single non-floating-point field. The
+    // expanded two-register encodings above are inactive here, so broadening the
+    // size/field/FP rules would surface return values that the 32-bit read path
+    // does not support.
     if (unboxedSize > sizeof(SIZE_T))
         return S_FALSE;
 
+    const bool twoRegister = false;
+    const bool allowMultiField = false;
+#endif
+
     mdToken mdClass = m_pClass->GetToken();
 
     int fieldCount = 0;
@@ -1764,8 +1801,14 @@ HRESULT CordbType::ReturnedByValue()
             // !static
             if ((attr & 0x10) == 0)
             {
-                if (fieldCount++)
+                // 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;
+                }
 
                 CorElementType et;
                 SigParser parser(sigBlob, sigLen);
@@ -1778,7 +1821,12 @@ HRESULT CordbType::ReturnedByValue()
                     {
                     case ELEMENT_TYPE_R4:
                     case ELEMENT_TYPE_R8:
-                        unsupported = true;
+                        // Floating-point fields are returned in FP registers.
+                        // A single FP register holding a value class is not
+                        // encodable here (only primitive VLT_REG_FP is), so
+                        // restrict to the two-register multi-reg forms.
+                        if (!twoRegister)
+                            unsupported = true;
                         break;
 
                     case ELEMENT_TYPE_CLASS:
@@ -1787,6 +1835,22 @@ HRESULT CordbType::ReturnedByValue()
                         // OK
                         break;
 
+                    case ELEMENT_TYPE_VAR:
+                    case ELEMENT_TYPE_MVAR:
+                        // The field's type is a generic type parameter (e.g. the
+                        // Item1/Item2 fields of ValueTuple<T1, T2>); the unbound field
+                        // signature does not carry the instantiated type, so we cannot
+                        // tell whether it resolves to an FP type. Only permit it for the
+                        // two-register multi-reg forms, where both the all-FP and mixed
+                        // int/FP paths are implemented (and the read path fails gracefully
+                        // when the value is not actually register-returned). This is
+                        // required to support mixed int/fp returns such as
+                        // ValueTuple<double, int>, while avoiding the
+                        // unimplemented single-FP-register value-class path.
+                        if (!twoRegister)
+                            unsupported = true;
+                        break;
+
                     default:
                         if (!CorIsPrimitiveType(et))
                             unsupported = true;
@@ -1813,7 +1877,7 @@ HRESULT CordbType::ReturnedByValue()
     if (unsupported)
         return S_FALSE;
 
-    return fieldCount <= 1 ? S_OK : S_FALSE;
+    return S_OK;
 }