Skip to content
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

Jump to bottom

Ensure GCHeap related debugging still works even when we use newer CLRGC to target older runtimes #88457

Merged
merged 4 commits into from
Jul 13, 2023
Merged

Ensure GCHeap related debugging still works even when we use newer CLRGC to target older runtimes #88457

merged 4 commits into from
Jul 13, 2023

Conversation

cshung
Copy link
Member

@cshung cshung commented Jul 6, 2023

This change makes sure GCHeap-related debugging still works even when we use newer CLRGC to target older runtimes by:

  • Placing all the newer fields at the end of GcDacVars and dac_gcheap, and
  • Making sure GC never populate fields that do not exist on older versions.

@cshung cshung requested a review from leculver July 6, 2023 03:14
@cshung cshung self-assigned this Jul 6, 2023
@ghost
Copy link

ghost commented Jul 6, 2023

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

Issue Details

This change makes sure GCHeap-related debugging still works even when we use newer CLRGC to target older runtimes by:

  • Placing all the newer fields at the end of GcDacVars and dac_gcheap, and
  • Making sure GC never populate fields that do not exist on older versions.
Author: cshung
Assignees: cshung
Labels:

area-Diagnostics-coreclr
Milestone: -

@cshung cshung marked this pull request as draft July 6, 2023 03:14
@cshung cshung marked this pull request as ready for review July 10, 2023 19:14
@cshung cshung requested review from noahfalk and removed request for leculver July 10, 2023 19:15
noahfalk
noahfalk approved these changes Jul 11, 2023
View reviewed changes
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.

A few suggestions inline but LGTM.

fyi @dotnet/dotnet-diag

src/coreclr/gc/dac_gcheap_fields.h Outdated Show resolved Hide resolved
src/coreclr/gc/gcinterface.dacvars.def Outdated Show resolved Hide resolved
src/coreclr/gc/dac_gcheap_fields.h Outdated Show resolved Hide resolved
src/coreclr/gc/gcinterface.dacvars.def Show resolved Hide resolved
@cshung @noahfalk
Apply suggestions from code review
26312c5 
Co-authored-by: Noah Falk <noahfalk@users.noreply.github.com>
This was referenced Jul 11, 2023
Closed
Closed
Closed
@mikem8361
Copy link
Member

Is this going to be merged? It would be nice if .NET 8 doesn't have this problem going forward.

Maoni0
Maoni0 approved these changes Jul 13, 2023
View reviewed changes
Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

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

:shipit:

This was referenced Jul 13, 2023
Closed
Closed
@cshung cshung merged commit 4ee9eef into dotnet:main Jul 13, 2023
96 of 107 checks passed
@cshung cshung deleted the public/fix-debug branch July 13, 2023 23:03
@cshung cshung mentioned this pull request Aug 4, 2023
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@noahfalk noahfalk noahfalk approved these changes

@Maoni0 Maoni0 Maoni0 approved these changes

Assignees

@cshung cshung

Labels
area-Diagnostics-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cshung @mikem8361 @noahfalk @Maoni0