Fix cross-bitness address truncation in ClrDataAddress conversions

[max-charlamb]

Summary

Refactor ClrDataAddress to correctly handle sign-extended addresses from the DAC and eliminate cross-platform ABI ambiguity at the COM interop boundary. This prevents silent address truncation when inspecting 32-bit targets and ensures correct marshalling on all platforms.

Problem

ClrDataAddress used implicit operators that converted through nint/nuint:

• operator ulong: (nuint)Value — truncates 64-bit addresses to 32 bits on x86 hosts
• operator ClrDataAddress(ulong): (nint)value — same truncation on input

This corrupts all addresses when a 32-bit host reads a 64-bit dump, causing:

• GetExportSymbolAddress() returning 0
• PEImage being null
• Could not find DotNetRuntimeContractDescriptor errors

Additionally, the DAC's sign-extended 32-bit addresses (e.g., 0xFFFFFFFF_80123456) caused OverflowException in checked arithmetic contexts in DacDataTargetCOM.ReadVirtual.

ClrDataAddress Rules

1. Sign Extension

The legacy DAC sign-extends 32-bit target addresses on the wire:

• 0x80123456 becomes 0xFFFFFFFF_80123456 (CLRDATA_ADDRESS)
• FromAddress(addr, target) applies sign extension: (ulong)(long)(int)(uint)addr
• ToAddress(target) strips it: (ulong)(uint)_value

2. ABI Boundary — Managed-Only Type

ClrDataAddress never crosses the ABI boundary by value. Vtable slots use plain ulong (annotated /*ClrDataAddress*/) because single-field struct passing differs across platform ABIs. Managed wrappers convert via:

• addr.ToInteropAddress() — unwrap to raw ulong (no transform)
• ClrDataAddress.FromInteropAddress(raw) — wrap from raw ulong (no transform)

Pointer forms (ref/out/ClrDataAddress*) are safe as-is and preserved in vtable signatures.

3. Confinement

ClrDataAddress exists only in DacInterface/ (vtable wrappers, struct definitions) and DacImplementation/ (conversion boundary). It never leaks into AbstractDac/, Implementation/, or the public API. The rest of ClrMD uses plain ulong for all addresses.

4. Data Flow

DacImplementation (ulong)
    | FromAddress(addr, _target)
    v
SosDac wrapper (ClrDataAddress)
    | .ToInteropAddress()
    v
Vtable call (ulong on the wire) --> Native DAC
    |
Returns via out struct fields (ClrDataAddress)
    | .ToAddress(_target)
    v
DacImplementation (ulong, clean target address)
    |
Public API (ulong: ClrObject.Address, ClrType.MethodTable, etc.)

Changes

ClrDataAddress API

• Removed implicit operators between ClrDataAddress and ulong
• Added ToAddress(TargetProperties) / FromAddress(ulong, TargetProperties) — sign-extension-aware conversions
• Added ToInteropAddress() / FromInteropAddress(ulong) — raw wire-value helpers for the ABI boundary
• Added ClrDataAddress.Null property — replaces default for clarity
• Added IsNull, IEquatable<ClrDataAddress>, hex DebuggerDisplay
• Added TargetProperties struct — holds pointer size, threaded through DAC layer
• Added comprehensive design documentation as block comment

Vtable Signatures (all SosDac files)

Flipped every by-value ClrDataAddress parameter to ulong /*ClrDataAddress*/:

• SosDac.cs — 48 vtable slots, 47 wrapper call sites
• SosDac6.cs, SosDac8.cs, SosDac13.cs, SosDac14.cs, SOSDac13Old.cs
• ClrDataProcess.cs — including CLRDATA_ENUM handle corrections
• DacDataTargetCOM.cs — address-typed slots annotated

Verified slot-by-slot against sospriv.idl in dotnet/runtime.

DacImplementation Conversion Sites

• All callers use ClrDataAddress.FromAddress(ulong, _target) to wrap addresses for DAC calls
• All DAC return values / struct fields use .ToAddress(_target) to unwrap
• Replaced all default ClrDataAddress usages with ClrDataAddress.Null
• Removed all 28 redundant TargetProperties target = _target local copies

Inbound DAC Callback Fix

• DacDataTargetCOM.ReadVirtual: wrap (long)address in unchecked(...) to prevent OverflowException on sign-extended 32-bit addresses

Testing

• Build: 0 errors, 0 warnings on both netstandard2.0 and net10.0
• x64 tests: 308 pass / 138 fail — matches main baseline (pre-existing SingleFileBasicArrayTests failures unrelated)
• x86: known OverflowException in ClrDataAddress.ToAddress for checked context — pre-existing, fix pending (unchecked((uint)_value))

Follow-up from #1421

This PR addresses the broader ClrDataAddress design issue discovered after #1421 (IntPtr.Size fixes) was merged.

[max-charlamb]

Follow-up to #1421 — we missed this case. MinidumpMemoryDescriptor uses ClrDataAddress for its StartAddress and DataSize64 fields, which truncates 64-bit addresses on 32-bit hosts via nint/nuint casts. This wasn't caught in #1421 because those fields aren't IntPtr.Size-dependent — the truncation happens inside ClrDataAddress's implicit conversions.

[leculver]

Let's separate out the MinidumpMemoryDescriptor into its own change.

This is a pretty messy change for something I don't really care if it works or not (32bit process debugging an x64 process). I don't mind supporting it if it's something you all want, but I want to make sure it doesn't overcomplicate the code.

[leculver]

This will be incredibly tricky to get right.

I think I'd like to see a more explicit converter class which we create once when the DacLibrary. This converter can hang off of DacLibrary but should be passed to ClrDataProcess and all SosDac instances. This way we have an encapsulated/explicit way of handling this conversion throughout.

Danger: structs and CLRDATA_ADDRESS are not interchangable on 32bit linux ABI (x86 sysv and arm32, if I remember correctly). These are passed by hidden pointer instead of in a register. So if ClrDataAddress is ever returned from a function, you cannot use ClrDataAddress at that boundary point. This also applies to ClrDataAddress as a parameter (every dac method) which is why it's defined as ulong in vtables.

Danger: ClrDataAddress value parameters (which is basically every dac method) also expects a sign extended value at the boundary. The current code works (probably by accident), so we have to be careful not to break it. For example:

 inline TADDR CLRDATA_ADDRESS_TO_TADDR(CLRDATA_ADDRESS cdAddr)
 {
 #ifndef HOST_64BIT
     INT64 iSignedAddr = (INT64)cdAddr;
     if (iSignedAddr > INT_MAX || iSignedAddr < INT_MIN)
         DacError(E_INVALIDARG);
 #endif
     return (TADDR)cdAddr;
 }

Walk 0x80000000 (a valid 3-GB-range 32-bit target address) through this:

• Sign-extended input 0xFFFFFFFF80000000 -> signed -2147483648 -> in [INT_MIN, INT_MAX] -> truncates to 0x80000000. Works.
• Un-sign-extended input 0x0000000080000000 -> signed +2147483648 -> > INT_MAX -> DacError(E_INVALIDARG). Fails.

Below is a rough sketch of what I'm thinking, though this is untested. You can modify this design if you want, but the key points:

1. ClrDataAddress has no way to get the value out of it. It's opaque except to ClrDataAddress.Converter.
2. An instance of the converter should be built once and passed around.
3. All structs which have a ClrDataAddress should define it as such.
4. Current inputs to legacy dac functions are all CLRDATA_ADDRESS and expect the sign extended value.
5. Values returned from the dac are ClrDataAddress and convert with ToAddress.
6. Values given to the dac need to be defined as ClrDataAddress somewhere to prevent mistakes, but pass through ToInteropAddress when passed to the raw method on the vtable.

(Or feel free to come up with something that works better. Just keep in mind that touching this fragile system is a hot stove of pain. You are more than welcome to tackle this, we just need to make sure it's right with the legacy dac and test it as best we can.)

/// <summary>
/// A wrapper around the raw 64-bit value produced by the DAC to represent an address. Due to historic/legacy
/// reasons, some DACs (legacy DACs) produce a sign-extended 64-bit value even when the target process is 32-bit.
/// This struct encapsulates that value and provides a converter to handle both legacy and modern DACs correctly.
/// </summary>
/// <param name="_value">The raw 64 bits produced by the DAC.  May be sign-extended from a  32-bit target address
/// when the backing DAC is a legacy DAC.</param>
[StructLayout(LayoutKind.Sequential)]
[DebuggerDisplay("{Value,h}")]
internal readonly struct ClrDataAddress(ulong _value) : IEquatable<ClrDataAddress>
{
    public bool IsNull => _value == 0;
    public bool Equals(ClrDataAddress other) => _value == other._value; 
    public override bool Equals(object? obj) => obj is ClrDataAddress cda && Equals(cda);
    public override int GetHashCode() => _value.GetHashCode();
    public static bool operator ==(ClrDataAddress left, ClrDataAddress right) => left.Equals(right);
    public static bool operator !=(ClrDataAddress left, ClrDataAddress right) => !left.Equals(right);
    public override string ToString() => $"0x{_value:x}";

    /// <summary>
    /// Constructs a ClrDataAddress.Converter for the given DAC configuration.  
    /// </summary>
    /// <param name="_signExtend">Should be false if cDac, true otherwise.</param>
    /// <param name="_pointerSize">The pointer size of the target process.</param>
    public sealed class Converter(bool _signExtend, int _pointerSize)
    {
        /// <summary>
        /// The target pointer size this converter was built for, in bytes.
        /// </summary>
        public int PointerSize => _pointerSize;

        /// <summary>
        /// True if this converter un-sign-extends legacy DAC output.  Exposed
        /// primarily for diagnostics/asserts; callers should normally just call
        /// <see cref="ToAddress"/> / <see cref="FromAddress"/>.
        /// </summary>
        public bool IsLegacy => _signExtend;

        /// <summary>
        /// Returns the un-sign-extended value of a ClrDataAddress to pass to a dac function.
        /// </summary>
        public ulong ToInteropAddress(ClrDataAddress cda) => cda._value;

        /// <summary>
        /// Converts a DAC-produced address to the real target address that
        /// the rest of ClrMD should see.
        /// </summary>
        public ulong ToAddress(ClrDataAddress cda)
        {
            if (!_signExtend)
                return cda.Value;

            return _pointerSize == 4 ? (uint)cda.Value : cda.Value;
        }

        /// <summary>
        /// Converts a canonical target address to the bit pattern the DAC
        /// expects back on its interface.
        /// </summary>
        public ClrDataAddress FromAddress(ulong address)
        {
            if (!_signExtend)
                return new ClrDataAddress(address);
                
            return _pointerSize == 4
                ? new ClrDataAddress(unchecked((ulong)(long)(int)address))
                : new ClrDataAddress(address);
        }
    }
}

[max-charlamb]

Let's separate out the MinidumpMemoryDescriptor into its own change.

This is a pretty messy change for something I don't really care if it works or not (32bit process debugging an x64 process). I don't mind supporting it if it's something you all want, but I want to make sure it doesn't overcomplicate the code.

Agreed that this is messy and that 32 bit processes debugging a 64 bit process is not an important scenario. The larger reason I want to fix this is supporting 64 bit processes debugging a 32 bit dump. Today it mostly works, but only when the 32 bit address is not in the top half of the address (and the top half of bits are filled with 1s). The 32 bit host targeting a 64 bit process serves as a proxy for the more common scenario.

Danger: structs and CLRDATA_ADDRESS are not interchangable on 32bit linux ABI (x86 sysv and arm32, if I remember correctly). These are passed by hidden pointer instead of in a register. So if ClrDataAddress is ever returned from a function, you cannot use ClrDataAddress at that boundary point. This also applies to ClrDataAddress as a parameter (every dac method) which is why it's defined as ulong in vtables.

We handle this in the cDAC by using custom source generated marshallers to marshal the ClrDataAddress object as a ulong. I can see if there is an equivalent that we can use that works on netstandard.

Danger: ClrDataAddress value parameters (which is basically every dac method) also expects a sign extended value at the boundary. The current code works (probably by accident), so we have to be careful not to break it.

The cDAC maintains the sign-extension of the legacy DAC APIs which use ClrDataAddress, so there shouldn't be any behavioral differences between legacy and 'modern' ISOSDacInterface APIs.

I appreciate the feedback. I'll rework this to try and make it cleaner using the converter approach. Thanks.

[max-charlamb]

Update: Rebased onto latest origin/main (picks up #1424 high-bit test harness). Dropped one commit (Fix 64-bit address truncation in MinidumpMemoryDescriptor) as patch already upstream.

New in this push:

• DacDataTargetCOM.ReadVirtual (net6.0+): wrap (long)address in unchecked to avoid OverflowException in checked mode when the DAC passes a sign-extended 32-bit CLRDATA_ADDRESS. Surfaced by running HeapTests.HighBit_HeapHasObjectsAbove2Gb under x86.

Validation:

• Clean build (0 warnings, 0 errors) on netstandard2.0 + net10.0.
• HeapTests.HighBit_HeapHasObjectsAbove2Gb passes under x86 HighBitHost.

[leculver]

Just some pre-review things I found.

[leculver]

Oh, I think SOSDac12.cs was entirely missed?

[leculver]

One issue and three nitpicks.

It will likely be a day or two before I hit approve, I have to cleanly get the 4.0 change through.

[leculver]

LGTM. Thanks for the hard work on this one!