Skip to content

Conversation

max-charlamb
Copy link
Member

@max-charlamb max-charlamb commented Jul 25, 2025

Implements and adds sub-descriptor as defined by #118126 for the GC. Each variation of the GC (SVR/WKS) has its own descriptor which is set up in PopulateDacVars.

Changes

  • Add implementation mechanism for sub-descriptors in datadescriptor.cpp and cdac-build-tool
  • Add new set of datadescriptors under src/coreclr/gc/datadescriptor mirroring the vm descriptors under src/coreclr/vm/datadescriptor.
  • Additionally build the GC descriptors as part of coreclr.dll
  • Bump gcinterface minor version to support adding new field gc_descriptor

Notes

In order to test the new GC descriptor, implements ISOSDacInterface::GetGCHeapData and ISOSDacInterface::GetGCHeapList.

  • Due to constraints around defining explicit template specializations in a namespace that differs from the one that defined the template, I had to define the cdac_data specializations outside of the GC namespace (in datadescriptor.h). In theory we could move this into the GC namespace, but this conflicts with the existing definition used in the global namespace in coreclr. I thought reusing the same class and moving the specializations was reasonable.
  • There are several times the GC is built without datadescriptors (or where they aren't supported). These include NativeAOT (something in bootstrap builds) and src/coreclr/gc/sample. We most likely need #ifdef out the extern specifiers in these scenarios.
    • Added GC_DESCRIPTOR define for builds when we expect the GCDescriptor.
  • Currently waiting on updating coreclr to target c++17 Proposal: Upgrade C++ compiler feature level to C++ 17 #112419 to resolve build issues.

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 25, 2025
@risc-vv
Copy link

risc-vv commented Jul 25, 2025

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.

@jkotas jkotas added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 27, 2025
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
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

This looked fine to me but we should go over it with Maoni as well.

@max-charlamb max-charlamb requested a review from noahfalk August 20, 2025 13:58
@mangod9
Copy link
Member

mangod9 commented Aug 25, 2025

GC side changes look good to me.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM

@max-charlamb max-charlamb merged commit 61b5219 into dotnet:main Aug 26, 2025
101 checks passed
@max-charlamb max-charlamb deleted the cdac-multi-contract branch August 26, 2025 14:02
@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants