From 629af167c779c2fdffb3a828511a75e7ec57d18f Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 11 Jun 2026 12:13:58 -0400 Subject: [PATCH 1/4] cDAC: GetObjectStringData returns E_INVALIDARG for size-only queries The legacy DAC (ClrDataAccess::GetObjectStringData) returns E_INVALIDARG when no output buffer is available (count == 0) even on a valid string, while still populating *pNeeded. The cDAC implementation returned S_OK, which trips the cDAC-vs-DAC HResult parity assert when CLRMA (clrma under dotnet-dump) sizes an exception string via GetObjectStringData(obj, 0, nullptr, &needed). Mirror the DAC: validate args as it does (so a non-null buffer with count == 0 still reports the needed size), populate *pNeeded via CopyStringToBuffer, then return E_INVALIDARG when count == 0. CLRMA explicitly ignores that HRESULT and uses the reported size. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SOSDacImpl.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index 2007bb81728a1e..d7289d69e3d088 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -3441,11 +3441,25 @@ int ISOSDacInterface.GetObjectStringData(ClrDataAddress obj, uint count, char* s int hr = HResults.S_OK; try { - if (obj == 0 || (stringData == null && pNeeded == null) || (stringData is not null && count <= 0)) + // Mirror ClrDataAccess::GetObjectStringData argument validation: reject a null object, + // or a request that provides neither an output buffer nor a size-out parameter. + if (obj == 0) + throw new ArgumentException(); + if ((stringData == null || count == 0) && pNeeded == null) throw new ArgumentException(); + Contracts.IObject contract = _target.Contracts.Object; string str = contract.GetStringValue(obj.ToTargetPointer(_target)); + + // Always reports the needed size via pNeeded; copies characters only when an output + // buffer (stringData with count > 0) is provided. OutputBufferHelpers.CopyStringToBuffer(stringData, count, pNeeded, str); + + // Match the legacy DAC: a size-only query (count == 0) still reports the needed size + // via *pNeeded but returns E_INVALIDARG rather than S_OK. Callers (e.g. CLRMA) rely on + // this to size their buffer before the real copy. + if (count == 0) + hr = HResults.E_INVALIDARG; } catch (System.Exception ex) { From f4540e3acc7fa2214b3c466032114d2761024295 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 11 Jun 2026 12:44:25 -0400 Subject: [PATCH 2/4] Use AllowCdacSuccess instead of matching the quirk --- .../SOSDacImpl.cs | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index d7289d69e3d088..fe21fc9bb06992 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -3441,25 +3441,11 @@ int ISOSDacInterface.GetObjectStringData(ClrDataAddress obj, uint count, char* s int hr = HResults.S_OK; try { - // Mirror ClrDataAccess::GetObjectStringData argument validation: reject a null object, - // or a request that provides neither an output buffer nor a size-out parameter. - if (obj == 0) - throw new ArgumentException(); - if ((stringData == null || count == 0) && pNeeded == null) + if (obj == 0 || (stringData == null && pNeeded == null) || (stringData is not null && count <= 0)) throw new ArgumentException(); - Contracts.IObject contract = _target.Contracts.Object; string str = contract.GetStringValue(obj.ToTargetPointer(_target)); - - // Always reports the needed size via pNeeded; copies characters only when an output - // buffer (stringData with count > 0) is provided. OutputBufferHelpers.CopyStringToBuffer(stringData, count, pNeeded, str); - - // Match the legacy DAC: a size-only query (count == 0) still reports the needed size - // via *pNeeded but returns E_INVALIDARG rather than S_OK. Callers (e.g. CLRMA) rely on - // this to size their buffer before the real copy. - if (count == 0) - hr = HResults.E_INVALIDARG; } catch (System.Exception ex) { @@ -3476,7 +3462,12 @@ int ISOSDacInterface.GetObjectStringData(ClrDataAddress obj, uint count, char* s { hrLocal = _legacyImpl.GetObjectStringData(obj, count, ptr, &neededLocal); } - Debug.ValidateHResult(hr, hrLocal); + + // The native DAC returns E_INVALIDARG for a size-only query (no output buffer) even on a + // valid string, while still populating *pNeeded; the cDAC returns S_OK for that case. + // Callers (e.g. CLRMA) ignore the HRESULT and use the reported size, so allow the cDAC to + // succeed where the DAC fails rather than replicating the DAC's quirk. + Debug.ValidateHResult(hr, hrLocal, HResultValidationMode.AllowCdacSuccess); if (hr == HResults.S_OK) { Debug.Assert(pNeeded == null || *pNeeded == neededLocal); From b0013910eb488eac856f5ea865bc7e1c3c92106b Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 11 Jun 2026 13:05:48 -0400 Subject: [PATCH 3/4] cDAC: mirror caller arg shape in GetObjectStringData debug validation Invoke the legacy DAC with the same stringData/pNeeded nullness the caller passed to the cDAC, so the debug HRESULT comparison is apples-to-apples and can't manufacture a spurious divergence from substituted arguments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SOSDacImpl.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index fe21fc9bb06992..1fe3beef7ff7ce 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -3456,11 +3456,16 @@ int ISOSDacInterface.GetObjectStringData(ClrDataAddress obj, uint count, char* s if (_legacyImpl is not null) { char[] stringDataLocal = new char[count]; - uint neededLocal; + uint neededLocal = 0; int hrLocal; fixed (char* ptr = stringDataLocal) { - hrLocal = _legacyImpl.GetObjectStringData(obj, count, ptr, &neededLocal); + // Invoke the legacy DAC under the same argument contract the caller gave the cDAC: + // only pass an output buffer when the caller did, and only request the size-out when + // the caller did. This keeps the HRESULT comparison apples-to-apples. + char* stringDataArg = stringData is null ? null : ptr; + uint* pNeededArg = pNeeded is null ? null : &neededLocal; + hrLocal = _legacyImpl.GetObjectStringData(obj, count, stringDataArg, pNeededArg); } // The native DAC returns E_INVALIDARG for a size-only query (no output buffer) even on a From 661ffbf78013dfb95f4402e91962aeb2dde04daa Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 11 Jun 2026 14:35:11 -0400 Subject: [PATCH 4/4] Remove DAC GetObjectStringData size-only-query quirk instead of masking it Rather than tolerating the divergence via AllowCdacSuccess, fix the legacy DAC: ClrDataAccess::GetObjectStringData no longer returns E_INVALIDARG for a size-only query (no output buffer) on a valid string -- it reports the needed size via *pNeeded and succeeds, matching the cDAC. The cDAC debug parity check returns to the default validation mode. Also mirror the caller's stringData/pNeeded nullness into the legacy DAC call in the debug validation, and derive the content-compare length from the cDAC string (neededLocal is only populated when a size-out is requested), avoiding an out-of-range span when pNeeded is null. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/daccess/request.cpp | 5 +---- .../SOSDacImpl.cs | 10 ++++------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 1ebbdf40d9acf2..5bc105a14f814c 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -1613,11 +1613,8 @@ ClrDataAccess::GetObjectStringData(CLRDATA_ADDRESS obj, unsigned int count, _Ino stringData[0] = W('\0'); } } - else - { - hr = E_INVALIDARG; - } + // A size-only query (no output buffer) reports the needed size via pNeeded and succeeds. if (pNeeded) *pNeeded = needed; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index 1fe3beef7ff7ce..08b0eb2b382acd 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -3468,15 +3468,13 @@ int ISOSDacInterface.GetObjectStringData(ClrDataAddress obj, uint count, char* s hrLocal = _legacyImpl.GetObjectStringData(obj, count, stringDataArg, pNeededArg); } - // The native DAC returns E_INVALIDARG for a size-only query (no output buffer) even on a - // valid string, while still populating *pNeeded; the cDAC returns S_OK for that case. - // Callers (e.g. CLRMA) ignore the HRESULT and use the reported size, so allow the cDAC to - // succeed where the DAC fails rather than replicating the DAC's quirk. - Debug.ValidateHResult(hr, hrLocal, HResultValidationMode.AllowCdacSuccess); + Debug.ValidateHResult(hr, hrLocal); if (hr == HResults.S_OK) { Debug.Assert(pNeeded == null || *pNeeded == neededLocal); - Debug.Assert(stringData == null || new ReadOnlySpan(stringDataLocal, 0, (int)neededLocal - 1).SequenceEqual(new string(stringData))); + // Compare against the legacy buffer using the cDAC string length: neededLocal is only + // populated when a size-out was requested from the legacy DAC (mirroring the caller). + Debug.Assert(stringData == null || new ReadOnlySpan(stringDataLocal, 0, new string(stringData).Length).SequenceEqual(new string(stringData))); } } #endif