Skip to content

Conversation

@max-charlamb
Copy link
Member

Saves datadescriptors json blobs in minidumps.

This enumeration algorithm is basic and hardcodes the number of sub-descriptors on the main datadescriptor. Fully recursing the subdescriptors the same way as the cDAC would require parsing the JSON blob. I believe this is fine for now and in the future, we'd move this code over to the cDAC where it is already parsed.

My biggest concern is bloating minidump file size when this won't be used in most scenarios for now.

@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds enumeration support for cDAC (Contract-based DAC) data descriptors in minidumps. The main purpose is to ensure that cDAC contract descriptor data structures are included in minidump memory reports, making them available for debugging.

Key changes:

  • Adds a new EnumMemDataDescriptors method that enumerates the main ContractDescriptor and its subdescriptors
  • Integrates the new enumeration into the minidump generation workflow
  • Documents the ordering requirement for subdescriptor pointers in the global structure

Reviewed Changes

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

File Description
src/coreclr/vm/datadescriptor/datadescriptor.inc Adds a comment documenting that subdescriptor pointers must be the last pointers in the global structure
src/coreclr/debug/daccess/enummem.cpp Implements EnumMemDataDescriptors to enumerate ContractDescriptor structures and integrates it into the minidump workflow
src/coreclr/debug/daccess/dacimpl.h Declares the new EnumMemDataDescriptors method
src/coreclr/debug/daccess/CMakeLists.txt Adds include directory for contract-descriptor.h header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants