Incomplete loading of DacpGcHeapDetails in ClrDataAccess::ServerGCHeapDetails

[cshung]

The structure DacpGcHeapDetails has many fields:

runtime/src/coreclr/inc/dacprivate.h

Lines 725 to 757 in 1306b03

	struct MSLAYOUT DacpGcHeapDetails
	{
	CLRDATA_ADDRESS heapAddr = 0; // Only filled in in server mode, otherwise NULL
	CLRDATA_ADDRESS alloc_allocated = 0;
	CLRDATA_ADDRESS mark_array = 0;
	CLRDATA_ADDRESS current_c_gc_state = 0;
	CLRDATA_ADDRESS next_sweep_obj = 0;
	CLRDATA_ADDRESS saved_sweep_ephemeral_seg = 0;
	CLRDATA_ADDRESS saved_sweep_ephemeral_start = 0;
	CLRDATA_ADDRESS background_saved_lowest_address = 0;
	CLRDATA_ADDRESS background_saved_highest_address = 0;
	DacpGenerationData generation_table [DAC_NUMBERGENERATIONS] = {};
	CLRDATA_ADDRESS ephemeral_heap_segment = 0;
	CLRDATA_ADDRESS finalization_fill_pointers [DAC_NUMBERGENERATIONS + 3] = {};
	CLRDATA_ADDRESS lowest_address = 0;
	CLRDATA_ADDRESS highest_address = 0;
	CLRDATA_ADDRESS card_table = 0;
	// Use this for workstation mode (DacpGcHeapDat.bServerMode==FALSE).
	HRESULT Request(ISOSDacInterface *sos)
	{
	return sos->GetGCHeapStaticData(this);
	}
	// Use this for Server mode, as there are multiple heaps,
	// and you need to pass a heap address in addr.
	HRESULT Request(ISOSDacInterface *sos, CLRDATA_ADDRESS addr)
	{
	return sos->GetGCHeapDetails(addr, this);
	}
	};

But the ServerGCHeapDetails function only loads a subset of them:

runtime/src/coreclr/debug/daccess/request_svr.cpp

Lines 102 to 144 in 1306b03

	HRESULT ClrDataAccess::ServerGCHeapDetails(CLRDATA_ADDRESS heapAddr, DacpGcHeapDetails *detailsData)
	{
	if (!heapAddr)
	{
	// PREfix.
	return E_INVALIDARG;
	}
	DPTR(dac_gc_heap) pHeap = __DPtr<dac_gc_heap>(TO_TADDR(heapAddr));
	int i;
	//get global information first
	detailsData->heapAddr = heapAddr;
	detailsData->lowest_address = PTR_CDADDR(g_lowest_address);
	detailsData->highest_address = PTR_CDADDR(g_highest_address);
	detailsData->card_table = PTR_CDADDR(g_card_table);
	// now get information specific to this heap (server mode gives us several heaps; we're getting
	// information about only one of them.
	detailsData->alloc_allocated = (CLRDATA_ADDRESS)pHeap->alloc_allocated;
	detailsData->ephemeral_heap_segment = (CLRDATA_ADDRESS)dac_cast<TADDR>(pHeap->ephemeral_heap_segment);
	// get bounds for the different generations
	for (i=0; i<NUMBERGENERATIONS; i++)
	{
	DPTR(dac_generation) generation = ServerGenerationTableIndex(pHeap, i);
	detailsData->generation_table[i].start_segment = (CLRDATA_ADDRESS)dac_cast<TADDR>(generation->start_segment);
	detailsData->generation_table[i].allocation_start = (CLRDATA_ADDRESS)(ULONG_PTR)generation->allocation_start;
	DPTR(gc_alloc_context) alloc_context = dac_cast<TADDR>(generation) + offsetof(dac_generation, allocation_context);
	detailsData->generation_table[i].allocContextPtr = (CLRDATA_ADDRESS)(ULONG_PTR) alloc_context->alloc_ptr;
	detailsData->generation_table[i].allocContextLimit = (CLRDATA_ADDRESS)(ULONG_PTR) alloc_context->alloc_limit;
	}
	DPTR(dac_finalize_queue) fq = pHeap->finalize_queue;
	DPTR(uint8_t*) pFillPointerArray= dac_cast<TADDR>(fq) + offsetof(dac_finalize_queue, m_FillPointers);
	for(i=0; i<(NUMBERGENERATIONS+dac_finalize_queue::ExtraSegCount); i++)
	{
	detailsData->finalization_fill_pointers[i] = (CLRDATA_ADDRESS) pFillPointerArray[i];
	}
	return S_OK;
	}

Comparing with the workstation GC, so code is also questionable - for example - do we need to check of the validity of the finalization queue?

runtime/src/coreclr/debug/daccess/request.cpp

Lines 2772 to 2828 in 72a26f9

	HRESULT
	ClrDataAccess::GetGCHeapStaticData(struct DacpGcHeapDetails *detailsData)
	{
	if (detailsData == NULL)
	return E_INVALIDARG;
	SOSDacEnter();
	detailsData->lowest_address = PTR_CDADDR(g_lowest_address);
	detailsData->highest_address = PTR_CDADDR(g_highest_address);
	detailsData->card_table = PTR_CDADDR(g_card_table);
	detailsData->heapAddr = NULL;
	detailsData->alloc_allocated = (CLRDATA_ADDRESS)*g_gcDacGlobals->alloc_allocated;
	detailsData->ephemeral_heap_segment = (CLRDATA_ADDRESS)*g_gcDacGlobals->ephemeral_heap_segment;
	detailsData->mark_array = (CLRDATA_ADDRESS)*g_gcDacGlobals->mark_array;
	detailsData->current_c_gc_state = (CLRDATA_ADDRESS)*g_gcDacGlobals->current_c_gc_state;
	detailsData->next_sweep_obj = (CLRDATA_ADDRESS)*g_gcDacGlobals->next_sweep_obj;
	if (g_gcDacGlobals->saved_sweep_ephemeral_seg != nullptr)
	{
	detailsData->saved_sweep_ephemeral_seg = (CLRDATA_ADDRESS)*g_gcDacGlobals->saved_sweep_ephemeral_seg;
	detailsData->saved_sweep_ephemeral_start = (CLRDATA_ADDRESS)*g_gcDacGlobals->saved_sweep_ephemeral_start;
	}
	else
	{
	// with regions, we don't have these variables anymore
	// use special value -1 in saved_sweep_ephemeral_seg to signal the region case
	detailsData->saved_sweep_ephemeral_seg = (CLRDATA_ADDRESS)-1;
	detailsData->saved_sweep_ephemeral_start = 0;
	}
	detailsData->background_saved_lowest_address = (CLRDATA_ADDRESS)*g_gcDacGlobals->background_saved_lowest_address;
	detailsData->background_saved_highest_address = (CLRDATA_ADDRESS)*g_gcDacGlobals->background_saved_highest_address;
	for (unsigned int i=0; i < *g_gcDacGlobals->max_gen + 2; i++)
	{
	DPTR(dac_generation) generation = GenerationTableIndex(g_gcDacGlobals->generation_table, i);
	detailsData->generation_table[i].start_segment = (CLRDATA_ADDRESS) dac_cast<TADDR>(generation->start_segment);
	detailsData->generation_table[i].allocation_start = (CLRDATA_ADDRESS) generation->allocation_start;
	DPTR(gc_alloc_context) alloc_context = dac_cast<TADDR>(generation) + offsetof(dac_generation, allocation_context);
	detailsData->generation_table[i].allocContextPtr = (CLRDATA_ADDRESS)alloc_context->alloc_ptr;
	detailsData->generation_table[i].allocContextLimit = (CLRDATA_ADDRESS)alloc_context->alloc_limit;
	}
	if (g_gcDacGlobals->finalize_queue.IsValid())
	{
	DPTR(dac_finalize_queue) fq = Dereference(g_gcDacGlobals->finalize_queue);
	DPTR(uint8_t*) fillPointersTable = dac_cast<TADDR>(fq) + offsetof(dac_finalize_queue, m_FillPointers);
	for (unsigned int i = 0; i<(*g_gcDacGlobals->max_gen + 2 + dac_finalize_queue::ExtraSegCount); i++)
	{
	detailsData->finalization_fill_pointers[i] = (CLRDATA_ADDRESS)*TableIndex(fillPointersTable, i, sizeof(uint8_t*));
	}
	}
	SOSDacLeave();
	return hr;
	}

In particular, the fact that saved_sweep_ephemeral_seg is not loaded during ServerGCHeapDetails is leading SOS to believe a region build of the GC to be a segment build, leading to subsequent errors.

Therefore saved_sweep_ephemeral_seg has to be fixed. The other fields are less clear. As far as I understand the SOS code, it looks like those fields are also accessed in the server GC case as well, so we should fix those as well.

@dotnet/gc for awareness

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

Issue Details

---

The structure DacpGcHeapDetails has many fields:

runtime/src/coreclr/inc/dacprivate.h

Lines 726 to 757 in 1306b03

	{
	CLRDATA_ADDRESS heapAddr = 0; // Only filled in in server mode, otherwise NULL
	CLRDATA_ADDRESS alloc_allocated = 0;
	CLRDATA_ADDRESS mark_array = 0;
	CLRDATA_ADDRESS current_c_gc_state = 0;
	CLRDATA_ADDRESS next_sweep_obj = 0;
	CLRDATA_ADDRESS saved_sweep_ephemeral_seg = 0;
	CLRDATA_ADDRESS saved_sweep_ephemeral_start = 0;
	CLRDATA_ADDRESS background_saved_lowest_address = 0;
	CLRDATA_ADDRESS background_saved_highest_address = 0;
	DacpGenerationData generation_table [DAC_NUMBERGENERATIONS] = {};
	CLRDATA_ADDRESS ephemeral_heap_segment = 0;
	CLRDATA_ADDRESS finalization_fill_pointers [DAC_NUMBERGENERATIONS + 3] = {};
	CLRDATA_ADDRESS lowest_address = 0;
	CLRDATA_ADDRESS highest_address = 0;
	CLRDATA_ADDRESS card_table = 0;
	// Use this for workstation mode (DacpGcHeapDat.bServerMode==FALSE).
	HRESULT Request(ISOSDacInterface *sos)
	{
	return sos->GetGCHeapStaticData(this);
	}
	// Use this for Server mode, as there are multiple heaps,
	// and you need to pass a heap address in addr.
	HRESULT Request(ISOSDacInterface *sos, CLRDATA_ADDRESS addr)
	{
	return sos->GetGCHeapDetails(addr, this);
	}
	};

But the ServerGCHeapDetails function only loads a subset of them:

runtime/src/coreclr/debug/daccess/request_svr.cpp

Lines 102 to 144 in 1306b03

	HRESULT ClrDataAccess::ServerGCHeapDetails(CLRDATA_ADDRESS heapAddr, DacpGcHeapDetails *detailsData)
	{
	if (!heapAddr)
	{
	// PREfix.
	return E_INVALIDARG;
	}
	DPTR(dac_gc_heap) pHeap = __DPtr<dac_gc_heap>(TO_TADDR(heapAddr));
	int i;
	//get global information first
	detailsData->heapAddr = heapAddr;
	detailsData->lowest_address = PTR_CDADDR(g_lowest_address);
	detailsData->highest_address = PTR_CDADDR(g_highest_address);
	detailsData->card_table = PTR_CDADDR(g_card_table);
	// now get information specific to this heap (server mode gives us several heaps; we're getting
	// information about only one of them.
	detailsData->alloc_allocated = (CLRDATA_ADDRESS)pHeap->alloc_allocated;
	detailsData->ephemeral_heap_segment = (CLRDATA_ADDRESS)dac_cast<TADDR>(pHeap->ephemeral_heap_segment);
	// get bounds for the different generations
	for (i=0; i<NUMBERGENERATIONS; i++)
	{
	DPTR(dac_generation) generation = ServerGenerationTableIndex(pHeap, i);
	detailsData->generation_table[i].start_segment = (CLRDATA_ADDRESS)dac_cast<TADDR>(generation->start_segment);
	detailsData->generation_table[i].allocation_start = (CLRDATA_ADDRESS)(ULONG_PTR)generation->allocation_start;
	DPTR(gc_alloc_context) alloc_context = dac_cast<TADDR>(generation) + offsetof(dac_generation, allocation_context);
	detailsData->generation_table[i].allocContextPtr = (CLRDATA_ADDRESS)(ULONG_PTR) alloc_context->alloc_ptr;
	detailsData->generation_table[i].allocContextLimit = (CLRDATA_ADDRESS)(ULONG_PTR) alloc_context->alloc_limit;
	}
	DPTR(dac_finalize_queue) fq = pHeap->finalize_queue;
	DPTR(uint8_t*) pFillPointerArray= dac_cast<TADDR>(fq) + offsetof(dac_finalize_queue, m_FillPointers);
	for(i=0; i<(NUMBERGENERATIONS+dac_finalize_queue::ExtraSegCount); i++)
	{
	detailsData->finalization_fill_pointers[i] = (CLRDATA_ADDRESS) pFillPointerArray[i];
	}
	return S_OK;
	}

Comparing with the workstation GC, so code is also questionable - for example - do we need to check of the validity of the finalization queue?

runtime/src/coreclr/debug/daccess/request.cpp

Lines 2772 to 2828 in 72a26f9

	HRESULT
	ClrDataAccess::GetGCHeapStaticData(struct DacpGcHeapDetails *detailsData)
	{
	if (detailsData == NULL)
	return E_INVALIDARG;
	SOSDacEnter();
	detailsData->lowest_address = PTR_CDADDR(g_lowest_address);
	detailsData->highest_address = PTR_CDADDR(g_highest_address);
	detailsData->card_table = PTR_CDADDR(g_card_table);
	detailsData->heapAddr = NULL;
	detailsData->alloc_allocated = (CLRDATA_ADDRESS)*g_gcDacGlobals->alloc_allocated;
	detailsData->ephemeral_heap_segment = (CLRDATA_ADDRESS)*g_gcDacGlobals->ephemeral_heap_segment;
	detailsData->mark_array = (CLRDATA_ADDRESS)*g_gcDacGlobals->mark_array;
	detailsData->current_c_gc_state = (CLRDATA_ADDRESS)*g_gcDacGlobals->current_c_gc_state;
	detailsData->next_sweep_obj = (CLRDATA_ADDRESS)*g_gcDacGlobals->next_sweep_obj;
	if (g_gcDacGlobals->saved_sweep_ephemeral_seg != nullptr)
	{
	detailsData->saved_sweep_ephemeral_seg = (CLRDATA_ADDRESS)*g_gcDacGlobals->saved_sweep_ephemeral_seg;
	detailsData->saved_sweep_ephemeral_start = (CLRDATA_ADDRESS)*g_gcDacGlobals->saved_sweep_ephemeral_start;
	}
	else
	{
	// with regions, we don't have these variables anymore
	// use special value -1 in saved_sweep_ephemeral_seg to signal the region case
	detailsData->saved_sweep_ephemeral_seg = (CLRDATA_ADDRESS)-1;
	detailsData->saved_sweep_ephemeral_start = 0;
	}
	detailsData->background_saved_lowest_address = (CLRDATA_ADDRESS)*g_gcDacGlobals->background_saved_lowest_address;
	detailsData->background_saved_highest_address = (CLRDATA_ADDRESS)*g_gcDacGlobals->background_saved_highest_address;
	for (unsigned int i=0; i < *g_gcDacGlobals->max_gen + 2; i++)
	{
	DPTR(dac_generation) generation = GenerationTableIndex(g_gcDacGlobals->generation_table, i);
	detailsData->generation_table[i].start_segment = (CLRDATA_ADDRESS) dac_cast<TADDR>(generation->start_segment);
	detailsData->generation_table[i].allocation_start = (CLRDATA_ADDRESS) generation->allocation_start;
	DPTR(gc_alloc_context) alloc_context = dac_cast<TADDR>(generation) + offsetof(dac_generation, allocation_context);
	detailsData->generation_table[i].allocContextPtr = (CLRDATA_ADDRESS)alloc_context->alloc_ptr;
	detailsData->generation_table[i].allocContextLimit = (CLRDATA_ADDRESS)alloc_context->alloc_limit;
	}
	if (g_gcDacGlobals->finalize_queue.IsValid())
	{
	DPTR(dac_finalize_queue) fq = Dereference(g_gcDacGlobals->finalize_queue);
	DPTR(uint8_t*) fillPointersTable = dac_cast<TADDR>(fq) + offsetof(dac_finalize_queue, m_FillPointers);
	for (unsigned int i = 0; i<(*g_gcDacGlobals->max_gen + 2 + dac_finalize_queue::ExtraSegCount); i++)
	{
	detailsData->finalization_fill_pointers[i] = (CLRDATA_ADDRESS)*TableIndex(fillPointersTable, i, sizeof(uint8_t*));
	}
	}
	SOSDacLeave();
	return hr;
	}

In particular, the fact that saved_sweep_ephemeral_seg is not loaded during ServerGCHeapDetails is leading SOS to believe a region build of the GC to be a segment build, leading to subsequent errors.

Therefore saved_sweep_ephemeral_seg has to be fixed. The other fields are less clear. As far as I understand the SOS code, it looks like those fields are also accessed in the server GC case as well, so we should fix those as well.

@dotnet/gc for awareness

Author:	cshung
Assignees:	-
Labels:	area-Diagnostics-coreclr
Milestone:	-

[tommcdon]

@cshung Thank you for the bug report. Feel free to submit a PR correcting the inaccuracies in the DAC.
cc @mikem8361

[noahfalk]

thanks @cshung! 👍