Skip to content

[cDAC] Relax HRESULT validation to allow divergent failure codes #125236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
max-charlamb merged 1 commit into from
Mar 6, 2026
Merged

[cDAC] Relax HRESULT validation to allow divergent failure codes #125236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ int IXCLRDataFrame.GetNumArguments(uint* numArgs)
{
uint numArgsLocal;
int hrLocal = _legacyImpl.GetNumArguments(&numArgsLocal);
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
if (hr == HResults.S_OK)
Debug.Assert(*numArgs == numArgsLocal, $"cDAC: {*numArgs}, DAC: {numArgsLocal}");
}
Expand Down Expand Up @@ -162,7 +162,7 @@ int IXCLRDataFrame.GetNumLocalVariables(uint* numLocals)
{
uint numLocalsLocal;
int hrLocal = _legacyImpl.GetNumLocalVariables(&numLocalsLocal);
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
if (hr == HResults.S_OK)
Debug.Assert(*numLocals == numLocalsLocal, $"cDAC: {*numLocals}, DAC: {numLocalsLocal}");
}
Expand Down Expand Up @@ -221,7 +221,7 @@ int IXCLRDataFrame.GetMethodInstance(out IXCLRDataMethodInstance? method)
#if DEBUG
if (_legacyImpl is not null)
{
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
}
#endif

Expand Down
11 changes: 5 additions & 6 deletions ...ive/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ int IXCLRDataMethodInstance.GetTokenAndScope(uint* token, void** /*IXCLRDataModu
void* legacyModPtr = null;
int hrLocal = _legacyImpl.GetTokenAndScope(validateToken ? &tokenLocal : null, validateMod ? &legacyModPtr : null);

Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);

if (validateToken)
{
Expand Down Expand Up @@ -172,7 +172,7 @@ int IXCLRDataMethodInstance.GetName(uint flags, uint bufLen, uint* nameLen, char
hrLocal = _legacyImpl.GetName(flags, bufLen, &nameLenLocal, nameBuf is null ? null : pNameBufLocal);
}

Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
if (nameLen is not null)
Debug.Assert(nameLenLocal == *nameLen, $"cDAC: {*nameLen:x}, DAC: {nameLenLocal:x}");

Expand Down Expand Up @@ -270,8 +270,7 @@ int IXCLRDataMethodInstance.GetILOffsetsByAddress(ClrDataAddress address, uint o
validateIlOffsets ? localIlOffsetsPtr : null);
}

// DAC function returns odd failure codes it doesn't make sense to match directly
Debug.Assert(hrLocal == hr || (hrLocal < 0 && hr < 0), $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);

if (hr == HResults.S_OK)
{
Expand Down Expand Up @@ -362,7 +361,7 @@ int IXCLRDataMethodInstance.GetILAddressMap(uint mapLen, uint* mapNeeded, [In, O
uint mapNeededLocal;
ClrDataILAddressMap[]? mapsLocal = mapLen > 0 ? new ClrDataILAddressMap[mapLen] : null;
int hrLocal = _legacyImpl.GetILAddressMap(mapLen, &mapNeededLocal, mapsLocal);
Debug.Assert(hrLocal == hr, $"HResult - cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);

if (hr == HResults.S_OK)
{
Expand Down Expand Up @@ -428,7 +427,7 @@ int IXCLRDataMethodInstance.GetRepresentativeEntryAddress(ClrDataAddress* addr)
ClrDataAddress addrLocal;
int hrLocal = _legacyImpl.GetRepresentativeEntryAddress(&addrLocal);

Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
Debug.Assert(addrLocal == *addr, $"cDAC: {*addr:x}, DAC: {addrLocal:x}");
}
#endif
Expand Down
6 changes: 3 additions & 3 deletions src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataStackWalk.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ int IXCLRDataStackWalk.GetContext(uint contextFlags, uint contextBufSize, uint*
{
byte[] localContextBuf = new byte[contextBufSize];
int hrLocal = _legacyImpl.GetContext(contextFlags, contextBufSize, null, localContextBuf);
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);

if (hr == HResults.S_OK)
{
Expand Down Expand Up @@ -136,7 +136,7 @@ int IXCLRDataStackWalk.Next()
{
int hrLocal = _legacyImpl.Next();
#if DEBUG
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
#endif
}

Expand Down Expand Up @@ -174,7 +174,7 @@ int IXCLRDataStackWalk.Request(uint reqCode, uint inBufferSize, byte* inBuffer,
{
hrLocal = _legacyImpl.Request(reqCode, inBufferSize, inBuffer, outBufferSize, localOutBufferPtr);
}
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);

for (int i = 0; i < outBufferSize; i++)
{
Expand Down
46 changes: 46 additions & 0 deletions src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/DebugExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.IO;
using System.Runtime.CompilerServices;

namespace Microsoft.Diagnostics.DataContractReader.Legacy;

internal enum HResultValidationMode
{
/// <summary>
/// HRESULTs must match exactly.
/// </summary>
Exact,

/// <summary>
/// Success HRESULTs must match exactly, but any two failing HRESULTs (negative values) are considered equivalent.
/// This is the recommended default because the cDAC and native DAC may use different exception types for the
/// same invalid input (e.g., InvalidOperationException vs E_INVALIDARG), producing different failing HRESULTs.
/// </summary>
AllowDivergentFailures,
}

internal static class DebugExtensions
{
extension(Debug)
{
[Conditional("DEBUG")]
internal static void ValidateHResult(
int cdacHr,
int dacHr,
HResultValidationMode mode = HResultValidationMode.AllowDivergentFailures,
[CallerFilePath] string? filePath = null,
[CallerLineNumber] int lineNumber = 0)
{
bool match = mode switch
{
HResultValidationMode.Exact => cdacHr == dacHr,
HResultValidationMode.AllowDivergentFailures => cdacHr == dacHr || (cdacHr < 0 && dacHr < 0),
_ => cdacHr == dacHr,
};
Debug.Assert(match, $"HResult mismatch - cDAC: 0x{unchecked((uint)cdacHr):X8}, DAC: 0x{unchecked((uint)dacHr):X8} ({Path.GetFileName(filePath)}:{lineNumber})");
}
}
}
12 changes: 6 additions & 6 deletions ...naged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ int IXCLRDataProcess.StartEnumMethodInstancesByAddress(ClrDataAddress address, /
#if DEBUG
if (_legacyProcess is not null)
{
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
}
#endif
return hr;
Expand Down Expand Up @@ -440,7 +440,7 @@ int IXCLRDataProcess.EnumMethodInstanceByAddress(ulong* handle, out IXCLRDataMet
#if DEBUG
if (_legacyProcess is not null)
{
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
}
#endif

Expand Down Expand Up @@ -550,7 +550,7 @@ int IXCLRDataProcess.GetOtherNotificationFlags(uint* flags)
{
uint flagsLocal;
int hrLocal = _legacyProcess.GetOtherNotificationFlags(&flagsLocal);
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
Debug.Assert(*flags == flagsLocal);
}
#endif
Expand Down Expand Up @@ -595,7 +595,7 @@ int IXCLRDataProcess.SetOtherNotificationFlags(uint flags)
{
hrLocal = ex.HResult;
}
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
if (hr == HResults.S_OK)
{
Debug.Assert(flags == flagsLocal);
Expand Down Expand Up @@ -650,7 +650,7 @@ int IXCLRDataProcess2.GetGcNotification(GcEvtArgs* gcEvtArgs)
if (_legacyProcess2 is not null)
{
int hrLocal = _legacyProcess2.GetGcNotification(gcEvtArgs);
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
}
#endif
return hr;
Expand All @@ -674,7 +674,7 @@ int IXCLRDataProcess2.SetGcNotification(GcEvtArgs gcEvtArgs)
{
// update the DAC cache
int hrLocal = _legacyProcess2.SetGcNotification(gcEvtArgs);
Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
Debug.ValidateHResult(hr, hrLocal);
}
#endif
return hr;
Expand Down
Loading