[cDAC] Read WKS card table from preserved VM global

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.

[steveisok]

Windows CDB /mw single-file dumps preserve the VM g_card_table global but can omit the workstation GC's gc_heap::card_table slot. Map GCHeapCardTable to the preserved VM global in CoreCLR's embedded WKS descriptor so cDAC can return the actual card-table value and continue reading heap data.

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

Tests: CoreCLR Release and Debug builds; cDAC GCTests; focused workstation GC dump test.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the cDAC GC workstation heap contract to treat the GCHeapCardTable global slot as optional when the slot’s target memory can’t be read, returning a null card table pointer while still providing valid heap and generation data.

Changes:

• Make GCHeapWKS.CardTable resilient to an unreadable card-table slot by using TryReadPointer and falling back to TargetPointer.Null.
• Relax DEBUG cross-validation in SOSDacImpl.GetGCHeapStaticData to tolerate a zero card table when the legacy DAC still reports a non-zero value.
• Add unit test coverage for both readable and unreadable card-table slot scenarios, including validating GetGCHeapStaticData output.

File	Description
src/​native/​managed/​cdac/​tests/​UnitTests/​GCTests.cs	Adds tests for readable vs unreadable workstation card-table slot, and validates GetGCHeapStaticData returns a null card table without breaking generation data.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Legacy/​SOSDacImpl.cs	Narrows DEBUG-only cross-validation to allow card_table == 0 for reduced-dump scenarios in GetGCHeapStaticData.
src/​native/​managed/​cdac/​Microsoft.Diagnostics.DataContractReader.Contracts/​Contracts/​GC/​GCHeapWKS.cs	Switches card-table read to TryReadPointer with a TargetPointer.Null fallback to tolerate unreadable target memory.

[rcj1]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

[steveisok]

I don't think this is the root cause of the issue; rather, we are trying to read memory at gc_heap::card_table which we do not preserve in a dump. g_card_table, however, is preserved but cDAC is not reading from it.

Making sure I'm following you - Are you suggesting this PR should remap GCHeapCardTable to g_card_table instead of treating the current slot as optional?

[rcj1]

Yes, I think that is the best solution.

[Copilot]

Copilot review overview

🟢 Approval recommended

Review tier: Lite
Findings: None

[jkotas]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

[steveisok]

Why is the card_table needed in the first place?

This suggests that the fix is not quite right.

From my read it's ISOSDacInterface compat. I'm not sure if we have any consumers who rely on it.

Are you suggesting we report 0 for this?

[max-charlamb]

Why is the card_table needed in the first place?

The standalone GC descriptor continues using gc_heap::card_table because it cannot depend on VM state.

This suggests that the fix is not quite right.

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD. It appears that is only really used to list the regions used by the runtime.

I would prefer not to map this value to the runtime copy (this seems like it would not fix the issue if using a standalone GC), but instead fix dump collection to make sure this value is enumerated.

[jkotas]

This is exposed by ISOSDacInterface13::GetGCBookkeepingMemoryRegions() which is used by CLRMD.

How is this exposed by this API?

This API walks linked list starting at BookkeepingStart:

runtime/docs/design/datacontracts/GC.md

Lines 948 to 979 in b5388ad

	IReadOnlyList<GCMemoryRegionData>IGC.GetGCBookkeepingMemoryRegions()
	{
	List<GCMemoryRegionData>regions=new();
	TargetPointerbkGlobal=target.ReadGlobalPointer("BookkeepingStart");
	if (bkGlobal==TargetPointer.Null) throwE_FAIL;
	TargetPointerbookkeepingStart=target.ReadPointer(bkGlobal);
	if (bookkeepingStart==TargetPointer.Null) throwE_FAIL;
	uintcardTableInfoSize=/* global value "CardTableInfoSize" */;
	uintrecount=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Recount offset */);
	ulongsize=target.ReadNUInt(bookkeepingStart+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=bookkeepingStart, Size=size });
	TargetPointernext=target.ReadPointer(bookkeepingStart+/* CardTableInfo::NextCardTable offset */);
	TargetPointerfirstNext=next;
	intmaxRegions=MaxBookkeepingRegions;
	// Compare next > cardTableInfoSize to guard against underflow when subtracting
	// cardTableInfoSize. Matches native DAC: `while (next > card_table_info_size)`.
	while (next!=TargetPointer.Null&&next>cardTableInfoSize&&maxRegions>0)
	{
	TargetPointerctAddr=next-cardTableInfoSize;
	recount=target.ReadNUInt(ctAddr+/* CardTableInfo::Recount offset */);
	size=target.ReadNUInt(ctAddr+/* CardTableInfo::Size offset */);
	if (recount!=0&&size!=0)
	regions.Add(newGCMemoryRegionData { Start=ctAddr, Size=size });
	next=target.ReadPointer(ctAddr+/* CardTableInfo::NextCardTable offset */);
	if (next==firstNext) break;
	maxRegions--;
	}
	returnregions;
	}

. Where does the raw card table pointer enter the picture?

[rcj1]

Where does the raw card table pointer enter the picture

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

[max-charlamb]

Where does the raw card table pointer enter the picture?

You are right, I misread the function. CLRMD does expose this through ISOSDacInterface::GetGCHeapDetails, but it doesn't look like it is used in the enumeration logic.

[rcj1]

Fwiw it doesn’t look like we use this card table value in the three consumers I checked.

[jkotas]

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1563 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Line 1429 in b5388ad

details->card_table=heapData.CardTable.ToClrDataAddress(_target);

I would expect the card table pointer to be included in the dump with the rest of heapData to handle this path.

[max-charlamb]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides the card_table. Adding this would be a minor gc/dac interface bump.

#132973

I'm fine with either fix. If we go forwards with the datadescriptor sided fix (in this PR), we should leave a note to undo it when we switch over to using the cDAC to enumerate memory.

[steveisok]

I created a draft PR to fix this from the enumeration side. The DAC previously explicitly enumerated the static WKS GC values for everything besides > the card_table. Adding this would be a minor gc/dac interface bump.

I think we need to test the draft PR runtime with CDB and see if it fixes the problem. If it doesn't / we can't somehow fix it, then I think this a short term fix and as you said, we remove it once we switch over.

[noahfalk]

#132973

+1 for that version of the fix (assuming we confirm it works, it seems like it should)

[steveisok]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

[max-charlamb]

I generated a dump w/ cdb and #132973 and read it with cdac. While the card table read was fixed, it exposed a failure on the next read where interesting_data_per_heap + 8 only has the first element preserved.

compact_reasons_per_heap, expand_mechanisms_per_heap, and interesting_mechanism_bits_per_heap from gcinterface.dacvars.def:71-74 are other arrays likely to display similar characteristics.

Given the cdac reader is eager, we probably should enumerate the full WKS diagnostics arrays.

I updated #132973 to also enumerate those arrays. Previously the DAC did not support !dumpgcdata on heap dumps (as those arrays were not enumerated).

I have tested the single file scenario as described in this PR description and the cDAC and DAC are both able to read the heap dumps and support !dumpgcdata.

[steveisok]

@max-charlamb I tested against your PR and it looks good. I'll close this in favor of yours.