Skip to content

Add cDAC dump collection provider - #132584

Draft
max-charlamb wants to merge 17 commits into
dotnet:mainfrom
max-charlamb:cdac-dump-creator
Draft

Add cDAC dump collection provider#132584
max-charlamb wants to merge 17 commits into
dotnet:mainfrom
max-charlamb:cdac-dump-creator

Conversation

@max-charlamb

@max-charlamb max-charlamb commented Aug 20, 2026

Copy link
Copy Markdown
Member

Summary

  • add a NativeAOT EnumMemory provider backed by the managed cDAC contracts
  • publish it as the platform mscordaccore binary when UseCdacDumpCollectProvider=true
  • support Windows, Linux, and macOS dump creation without shipping the legacy DAC
  • collect dump memory by tracing contract reads and explicitly exercising the contracts required for later analysis
  • preserve existing DAC and cDAC SOS coverage while adding a separate collector-enabled SOS leg

Memory policy

  • mini and triage dumps contain the memory reached through the relevant cDAC contracts
  • heap dumps additionally exercise GC and other heap-specific contracts
  • raw in-memory symbol streams are explicitly included only in heap dumps, matching the legacy policy where broad private/read-write mapping collection captured them
  • full dumps continue to use the platform's full-memory collection path

Runtime integration

CoreCLR publishes the provider under the existing mscordaccore name and omits the native DAC only when the opt-in property is enabled. Default builds continue to use the legacy DAC.

createdump locates DotNetRuntimeContractDescriptor in regular, single-file, and NativeAOT layouts.

Createdump now probes IXCLRDataProcess as an optional provider capability. The EnumMemory provider returns E_NOINTERFACE, allowing dump creation to continue without a build-time CDAC_DUMP_COLLECTOR distinction. Until smaller targeted APIs are added, createdump does not:

  • add or rename managed module mappings or record managed module timestamps, image sizes, MVIDs, and main-module identity
  • enrich crash reports with managed frames, method details, or managed exception object, type, and HRESULT information

These limitations affect createdump's native bookkeeping and optional crash-report JSON. The collector independently captures the contract memory needed for subsequent SOS, ClrMD, and cDAC dump analysis.

Diagnostics CI

The existing cDAC, cDAC_verify, and DAC SOS legs remain unchanged and consume the standard runtime build. A separate AllSubsets_CoreCLR_EnumMemory build publishes uniquely suffixed _enum_memory artifacts for the new cDAC_EnumMemory leg.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Windows NativeAOT mscordaccore.dll provider backed by cDAC contracts for dump collection and metadata, integrated with CoreCLR.

Changes:

  • Adds COM entrypoints and cDAC-based memory enumeration.
  • Collects managed objects, methods, types, modules, and mini metadata.
  • Integrates provider builds, resources, installation, and solution configuration.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 12 comments.

Show a summary per file
File Summary / final review note
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.ICLRDataEnumMemoryRegions.cs Forwards legacy memory-region enumeration calls.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs Adds callback declarations. Critical: the typed callback may break legacy COM marshalling. Nit: the new public API lacks approval.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/CdacErrorLogger.cs Adds legacy diagnostics logging. Nit: appears unused.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/ObjectCollector.cs Collects objects and type names. Moderate: cancellation HRESULTs are swallowed.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MiniMetadataWriter.cs Writes DacStreams mini metadata.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/Microsoft.Diagnostics.DataContractReader.DumpCollect.csproj Defines the NativeAOT provider project.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MethodCollector.cs Collects method dependencies and names. Moderate: cancellation HRESULTs are swallowed.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs Bridges cDAC reads and callbacks. Critical: collection mode is incorrectly derived, causing mini and triage dumps to run heap phases.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/GenerateWindowsVersionResource.ps1 Generates the Windows version resource.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/Entrypoints.cs Exposes the COM entrypoint. Critical: IXCLRDataProcess initialization fails. Nit: integration coverage is missing.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/DumpCreator.cs Coordinates dump collection. Critical: mini dumps omit managed code regions and module PE/debug data.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/DumpCollectLogger.cs Provides provider diagnostics.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/ContractDescriptorLocator.cs Locates the runtime contract descriptor.
src/native/managed/cdac/cdac.slnx Adds the provider to the cDAC solution.
src/coreclr/runtime.proj Publishes and configures the provider. Critical: Windows debug-subset builds publish neither the managed replacement nor an installed native DAC.
src/coreclr/dlls/mscoree/coreclr/GenClrDebugResource.ps1 Generates the CLR debug resource.
src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt Embeds provider resource identity.
src/coreclr/dlls/mscordac/CMakeLists.txt Adjusts native DAC installation behavior.
Suppressed comments (6)

src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt:250

  • When CLR_DUMP_COLLECT_PROVIDER_PATH is set, the command consumes that external provider at line 249, but this dependency list only tracks the native mscordaccore target. Republishing/replacing the managed provider without rebuilding that target can leave clr_debug_resource.bin with stale DAC timestamp/image-size identity, so incremental builds may embed an invalid provider identity. Add the selected provider path to DEPENDS (while retaining the target dependency for the fallback case).
        DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/GenClrDebugResource.ps1" mscordaccore mscordbi

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/DumpCreator.cs:60

  • The try covers an entire phase, so one unreadable module, thread, or stack frame exits the phase and skips every later item. That undermines the stated best-effort behavior and can omit most regions from a dump because of one corrupt target structure. Catch and log failures at the individual-item loops (while preserving cancellation), leaving this wrapper for phase-level failures.
        try
        {
            enumerate();
            DumpCollectLogger.Log($"Completed {phase} enumeration.");
        }

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MethodCollector.cs:27

  • Both dependency enumeration calls run before the only guarded naming operation. If one frame points at unreadable/corrupt MethodDesc data, the exception escapes CaptureMethod, aborts the entire threads phase through DumpCreator.TryEnumerate, and prevents later threads from being collected. Handle failures per frame/dependency (while propagating cancellation) so one bad frame does not discard the rest of a best-effort dump.
        EnumerateMethodDependencies(methodDesc);
        EnumerateMethodDescDataDependencies(methodDesc);

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MethodCollector.cs:83

  • The existing native DacStreams producer truncates a method name at the first ( and appends () before calling DacMdCacheAddEEName (see MethodDesc::EnumMemoryRegions), but this path stores the full signature. The replacement provider therefore produces different names for !clrstack and other consumers than the established mini-metadata format; apply the same truncation before adding the method name.
            TypeNameBuilder.AppendMethodInternal(
                _target,
                name,
                method,
                TypeNameFormat.FormatSignature |

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MiniMetadataWriter.cs:19

  • The new NativeAOT provider and its COM callback paths have no automated tests. Existing DacStreamsTests exercise the reader, but not the generated COM ABI, callback HRESULT/cancellation behavior, callback2 updates, pointer-size serialization, or the end-to-end mini/heap collection. Add focused tests (or a Windows end-to-end test) before relying on the manual dump check for this low-level provider.
    public static void Write(

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/ObjectCollector.cs:79

  • Because the type and parent pointers come from target memory, this loop has no termination guard. A corrupt or cyclic MethodTable parent chain will spin forever, and the surrounding best-effort catch cannot recover from a hang; track visited MethodTable addresses or cap the traversal, as the thread walk does.
        while (type.Address != TargetPointer.Null)
Comment thread src/coreclr/runtime.proj Outdated
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:33

  • clrFlags is documented as reserved/ignored by the native implementation, and createdump always passes CLRDATA_ENUM_MEM_HEAP2 even for --normal and --triage (crashinfo.cpp:378-383). As written, every non-full createdump request therefore runs EnumerateGC, loader heaps, sync blocks, and stress-log collection, turning mini/triage dumps into heap-sized dumps. Select heap enumeration from MiniDumpWithPrivateReadWriteMemory, as the existing DAC does.
            bool includeHeap =
                clrFlags is CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP or CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP2
                || (miniDumpFlags & MiniDumpWithPrivateReadWriteMemory) != 0;

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/ObjectCollector.cs:68

  • This catch also consumes COR_E_OPERATIONCANCELED thrown by _emitter.Add above. Cancellation while collecting an exception object is therefore ignored and dump generation continues, unlike every TryEnumerate path. Filter cancellation out so it reaches EnumMemoryRegions and is returned to the caller.
        catch (System.Exception)
        {
        }
Comment thread src/coreclr/debug/createdump/crashinfo.cpp Outdated
@max-charlamb
max-charlamb marked this pull request as ready for review August 25, 2026 12:39
Copilot AI review requested due to automatic review settings August 25, 2026 12:39
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 6 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/coreclr/pal/prebuilt/inc/clrdata.h:155

  • This file identifies itself as MIDL-generated at line 4, while the source src/coreclr/inc/clrdata.idl is unchanged. Revert this whitespace-only generated-output churn rather than modifying the prebuilt artifact directly.
    src/coreclr/pal/prebuilt/idl/clrdata_i.cpp:26
  • This file identifies itself as MIDL-generated at line 4, while the source src/coreclr/inc/clrdata.idl is unchanged. Revert this whitespace-only generated-output churn rather than modifying the prebuilt artifact directly.

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:35

  • This mode selection collapses every non-heap request into the regular mini path. CLRDATA_ENUM_MEM_TRIAGE/MiniDumpFilterTriage are valid triage modes, and MiniDumpWithFullAuxiliaryState has separate native behavior (enummem.cpp:2044-2061). Since this DLL replaces the auxiliary provider, those requests silently receive the wrong region set. Dispatch supported modes explicitly, or reject/fallback for unsupported modes rather than treating them as mini.
            bool includeHeap =
                clrFlags is CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP or CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP2
                || (miniDumpFlags & MiniDumpWithPrivateReadWriteMemory) != 0;

            new DumpCreator(target, runtimeModule, includeHeap, emitter).EnumerateMemoryRegions();
Copilot AI review requested due to automatic review settings August 25, 2026 13:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Suppressed comments (10)

Previously missed (7) — in code that hasn't changed since the last review.

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/ObjectCollector.cs:68

  • This catch also swallows COR_E_OPERATIONCANCELED raised by _emitter.Add or an implicit target read. The caller's cancellation is then ignored and dump collection continues; the surrounding DumpCreator.TryEnumerate explicitly preserves cancellation, so this catch must do the same.
        catch (System.Exception)
        {
        }

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/ObjectCollector.cs:160

  • Target reads performed while formatting the type name can invoke the emitter and throw COR_E_OPERATIONCANCELED; this blanket catch suppresses that cancellation and lets collection continue. Preserve cancellation here just as DumpCreator.TryEnumerate does.
        catch (System.Exception)
        {
        }

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MethodCollector.cs:91

  • Formatting a method performs target reads, so this catch can consume a COR_E_OPERATIONCANCELED from the enumeration callback. That prevents cancellation from reaching EnumMemoryRegions and causes more callbacks after the caller requested termination.
        catch (System.Exception)
        {
        }

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:126

  • This passes zero-extended TargetPointer.Value addresses to a callback whose address type is CLRDATA_ADDRESS. On 32-bit targets, that ABI requires sign extension (TO_CDADDR in dacimpl.h:42; ConversionExtensions.ToClrDataAddress follows the same rule), and this provider is enabled for x86. Regions above 0x7fffffff will therefore be reported under the wrong 64-bit address. Normalize callback addresses using the target pointer size before invoking the callback.

This issue also appears on line 174 of the same file.

            int hr = _enumMemoryRegion(callback, address, chunkSize);

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MethodCollector.cs:87

  • Mini-metadata method names must match the native DAC's compatibility shape. The native implementation formats the signature but then truncates at the first ( and appends ) because !analyze assumes that argument-list form (method.cpp:3965-3975). Writing the full signature here changes fallback symbols; truncate it before adding the name.
                TypeNameFormat.FormatSignature |
                TypeNameFormat.FormatNamespace |
                TypeNameFormat.FormatFullInst);

src/coreclr/pal/prebuilt/inc/clrdata.h:155

  • This file identifies itself as “ALWAYS GENERATED” at line 4, and the PR only removes generator-produced whitespace without changing clrdata.idl. Revert this unrelated generated-file edit rather than hand-modifying prebuilt output.
    src/coreclr/pal/prebuilt/idl/clrdata_i.cpp:26
  • This is an “ALWAYS GENERATED” MIDL output file (line 4), and these whitespace-only removals have no corresponding source IDL change. Revert the generated-file churn.

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:33

  • clrFlags is intentionally ignored by the existing DAC; ClrDataAccess::EnumMemoryRegions selects heap mode only from MiniDumpWithPrivateReadWriteMemory (enummem.cpp:1991-2062). Callers such as createdump pass CLRDATA_ENUM_MEM_HEAP2 for every non-full dump, so consulting it here can turn a normal/triage dump into a heap dump. Derive this solely from miniDumpFlags to remain a drop-in provider.
            bool includeHeap =
                clrFlags is CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP or CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP2
                || (miniDumpFlags & MiniDumpWithPrivateReadWriteMemory) != 0;

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:35

  • The provider never distinguishes MiniDumpFilterTriage, so triage requests run the normal path. That path emits ExceptionData.Message, both stack-trace strings, Watson buckets, and module paths; the native triage worker deliberately omits the message and rewrites stack traces to remove file information (enummem.cpp:524-539, 638-653) because triage dumps must be PII-free. Pass a dump mode into DumpCreator and implement the triage-specific omissions/updates before enabling this provider.
            new DumpCreator(target, runtimeModule, includeHeap, emitter).EnumerateMemoryRegions();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:174

  • UpdateMemoryRegion also takes a CLRDATA_ADDRESS, but bufferAddress.Value is zero-extended on x86. If the mini-metadata buffer is above 2 GB, the update targets a different 64-bit address and the dump retains invalid/stale stream data. Apply the same target-width sign extension used for enumeration callbacks.
                hr = updateMemoryRegion(callback2, address, (uint)buffer.Length, bufferPointer);
Copilot AI review requested due to automatic review settings August 25, 2026 13:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MiniMetadataWriter.cs:46

  • Stopping at the first name that does not fit drops every later entry, even though later names may be shorter and fit in the remaining mini-metadata buffer. The native writer rejects only the individual oversized entry and continues accepting subsequent names (daccess.cpp:2412-2421). Continue here instead so one long generic name does not discard unrelated method/type names.
            if (offset > buffer.Length - entrySize)
                break;

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/DumpCreator.cs:245

  • This cleanup-list traversal has no cycle detection. Since dump collection is explicitly best-effort for partially unreadable or corrupt targets, a repeated LinkNext value makes dump creation loop forever instead of returning a partial dump. Track visited cleanup blocks, as the thread traversal already does, and stop when an address repeats.
        TargetPointer cleanup = syncBlock.GetSyncBlockFromCleanupList();
        while (cleanup != TargetPointer.Null)
            cleanup = syncBlock.GetNextSyncBlock(cleanup);

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:35

  • The non-heap provider path is not validated with an actual mini/triage dump. The existing dump harness only generates Heap and Full dumps (DumpTests.targets:223-228), and Full dumps already contain process memory, so they cannot detect omitted regions in this path. Add a Mini (and ideally Triage) dump case that verifies stack walking and mini-metadata names before making this provider the default.
            bool includeHeap =
                clrFlags is CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP or CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP2
                || (miniDumpFlags & MiniDumpWithPrivateReadWriteMemory) != 0;

            new DumpCreator(target, runtimeModule, includeHeap, emitter).EnumerateMemoryRegions();
Comment thread src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Copilot AI review requested due to automatic review settings August 25, 2026 13:55
Max Charlamb added 2 commits August 25, 2026 10:00
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:35

  • Mode selection ignores MiniDumpFilterTriage (0x00100000), although the native DAC derives triage mode from miniDumpFlags and treats the clrFlags argument as reserved. A triage request therefore follows the normal mini path here, and DumpCreator emits full module paths and exception message/stack-trace strings instead of applying the native triage PII filtering. Decode the triage flag and propagate that mode so sensitive regions are skipped or poisoned consistently with EnumMemoryRegionsWorkerMicroTriage; add triage dump coverage as well.
            bool includeHeap =
                clrFlags is CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP or CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP2
                || (miniDumpFlags & MiniDumpWithPrivateReadWriteMemory) != 0;

            new DumpCreator(target, runtimeModule, includeHeap, emitter).EnumerateMemoryRegions();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs:13

  • This introduces a new public enum and changes the public COM interface parameter type, but the PR does not link an issue with the api-approved label. New public managed surface needs an approved API shape; otherwise keep the existing int signature and use an internal enum inside the provider.
public enum CLRDataEnumMemoryFlags
Add runtime contract descriptor discovery and statically link the PAL into createdump so the managed dump collector can replace the legacy DAC across supported platforms.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Copilot AI review requested due to automatic review settings August 25, 2026 18:37
@max-charlamb
max-charlamb removed the request for review from noahfalk August 25, 2026 18:42
@max-charlamb
max-charlamb marked this pull request as draft August 25, 2026 18:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/MemoryRegionEnumerator.cs:33

  • clrFlags cannot determine heap mode here: createdump passes CLRDATA_ENUM_MEM_HEAP2 for every non-full dump (src/coreclr/debug/createdump/crashinfo.cpp:381-386), while the legacy DAC derives MINI/HEAP2/TRIAGE from miniDumpFlags (src/coreclr/debug/daccess/enummem.cpp:2044-2061). Consequently, normal mini and triage dumps enter the heap branch; triage can include the managed heap despite its size/privacy filtering. Derive the complete mode from miniDumpFlags, propagate triage semantics into DumpCreator, and add triage dump coverage.
            bool includeHeap =
                clrFlags is CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP or CLRDataEnumMemoryFlags.CLRDATA_ENUM_MEM_HEAP2
                || (miniDumpFlags & MiniDumpWithPrivateReadWriteMemory) != 0;

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DumpCollect/DumpCreator.cs:245

  • The cleanup list is target-controlled and may contain a cycle when the process state is corrupt. This loop then never terminates (and cached contract reads can keep returning the same nodes), contradicting the collector's best-effort handling of corrupt targets. Track visited addresses, as the thread traversal already does.
        TargetPointer cleanup = syncBlock.GetSyncBlockFromCleanupList();
        while (cleanup != TargetPointer.Null)
            cleanup = syncBlock.GetNextSyncBlock(cleanup);
Comment thread eng/pipelines/runtime-diagnostics.yml Outdated
Avoid duplicate ARM64 PAL symbols, bound corrupt SyncBlock cleanup traversal, and sign-extend 32-bit addresses passed to dump callbacks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Max Charlamb added 2 commits August 26, 2026 15:20
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Order the DAC PAL before the PAL archive so GNU ld resolves the bundled libunwind symbols, and document the remaining expected CI failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Mach-O symbol lookup returns a relocated process address, so use it directly instead of adding the module base a second time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Comment thread eng/pipelines/runtime-diagnostics.yml Outdated
# legs that run on top of it: cDAC, cDAC_verify, DAC.
# Each leg sets testInterpreter: true so interpreter coverage
# is exercised inline (no separate Interpreter leg).
# -rc release -lc release -clrinterpreter) consumed by the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you have a plan in mind for how this work would eventually get checked in? Right now it looks like we are disabling DAC and cDAC_verify testing which I'd be very wary of doing until we are confident that the fragile implementation of DAC/DBI will not ship in the next version of .NET and we are no longer getting value from cDAC_verify style comparison testing. I do think we are getting there, but now seemed too soon.

A few options I thought of:

  • treat it as a proof of concept, test it manually, but then leave it disabled for now until the rest of work to remove DAC/DBI is ready.
  • make use of a feature flag (UseCdacDumpCollectProvider) and have test legs that run both with the flag enabled and disabled. To keep CI costs down we'd probably only want it opt-in or on a low scheduled cadence.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we get a good enough signal, feature flag it would be my vote.

goto exit;
}
#ifndef CDAC_DUMP_COLLECTOR
hr = pfnCLRDataCreateInstance(__uuidof(IXCLRDataProcess), dataTarget, (void**)&m_pClrDataProcess);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some functionality loss around not implementing this right? Is it a TODO for later or there is an alternative plan for how this info gets populated?

For example: https://github.com/max-charlamb/runtime/blob/a482c7e7d81de431dadc9beaf7ac7a97685812ac/src/coreclr/debug/createdump/crashinfo.cpp#L406

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added TODO: [cdac] for:

  • CrashInfo::EnumerateManagedModules()
  • CrashInfo::UnwindAllThreads()
    which require the IXCLRDataProcess interface.
AddRef();
return S_OK;
}
else if (InterfaceId == IID_ICLRContractLocator)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend we try to phase out this interface and instead define cDAC API exports that take the contract address directly as a parameter in the method signature. This means on Windows we'd need two entrypoints (one that implements the MAP extensibility point and one that createdump calls) but I think that is still clearer sometimes passing in this interface and sometimes not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to match the pattern of the new dbgshim api which passes in the descriptoraddress directly.

add_dependencies(coreclr clr_debug_resources)
add_dependencies(coreclr_static clr_debug_resources mscordaccore)
add_dependencies(coreclr_static clr_debug_resources)
if(NOT CLR_DUMP_COLLECT_PROVIDER_PATH)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I missed this at first - moving this right after the clr_debug_resource target was defined would help it be visible.

if (flags.HasFlag(ModuleFlags.ReflectionEmit))
_emitter.RegisterMetadataRange(ecmaMetadata.GetReadWriteSavedMetadataAddress(module));

if (loader.TryGetSymbolStream(module, out TargetPointer symbolBuffer, out uint symbolSize))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did the original DAC code capture dynamic/in-memory symbol streams? I would not have expected that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DAC does not explicitly enumerate the in-memory symbol buffer, but it is included in heap dumps.

For Windows heap dumps, createdump requests MiniDumpWithPrivateReadWriteMemory and passes it to MiniDumpWriteDump. DbgHelp documents that this flag scans the virtual address space for PAGE_READWRITE memory.

On Unix, createdump directly adds every read/write native mapping for a heap dump. Since CGrowableStream allocates its buffer using new char[], the buffer is captured.

I updated the collector, so the symbol buffer is explicitly emitted only when heap collection is enabled. Mini dumps should no longer include it. While this isn't explicitly required, I like having more explicit enumeration.

loader.GetFileName(module);

if (loader.TryGetLoadedImageContents(module, out _, out _, out _))
_emitter.RegisterMetadataRange(ecmaMetadata.GetReadOnlyMetadataAddress(module));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we do similar masking in the DAC implementation of the dump reader? Does something differ so the masking matters here and didn't matter before?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the legacy DAC does essentially the same thing.

When it creates the host-side metadata importer, it reads the target metadata using DacInstantiateTypeByAddressNoReport. That creates a normal DAC cache entry but marks it with the noReport bit. When the DAC reports memory that was implicitly touched during enumeration, DumpAllInstances skips entries with noReport set. The comment there notes that metadata is the only memory currently excluded this way.

if (bytesRead != (uint)buffer.Length)
return HResults.E_FAIL;

if (emitter.ShouldEmitTargetRead(address, bytesRead))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know DAC does this implicit recording of every piece of memory that got touched while doing the exploration and I think thats a fine starting point for a port. I'm hoping that we'd be able to move away from this eventually (in a future PR) and more precisely document exactly what memory goes into the dump. Right now its unclear to me how much memory gets swept in through this mechanism and of that how much is actually needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a pretty reasonable way of doing the enumeration. It directly connects what is expected to work to the collection. If we change the commands to require more memory, the enumeration is automatically updated.

Agreed we can discuss other approaches moving forwards.

Max Charlamb added 3 commits August 28, 2026 11:37
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Max Charlamb added 5 commits August 31, 2026 10:51
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Add an explicit descriptor-address entrypoint while retaining the existing discovery path for compatibility. Remove createdump's ICLRContractLocator implementation and the corresponding native IDL surface.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d56e1d60-7e1a-4353-8d3f-27deac700b4f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment