From b749a419270813af256e93856cb7b0471c4d7e7a Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 24 Feb 2022 22:42:21 -0800 Subject: [PATCH 1/3] GetTypeField should return ELEMENT_TYPE_BYREF. Did not handle the ByReference special case. --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 40 ++++++++++++++---------- src/coreclr/debug/ee/debugger.cpp | 8 ++--- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 1ff143cf7cce16..13c486c67d857e 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -5416,11 +5416,11 @@ GENERICS_TYPE_TOKEN DacDbiInterfaceImpl::ResolveExactGenericArgsToken(DWORD if (dwExactGenericArgsTokenIndex == 0) { - // In a rare case of VS4Mac debugging VS4Mac ARM64 optimized code we get a null generics argument token. We aren't sure - // why the token is null, it may be a bug or it may be by design in the runtime. In the interest of time we are working - // around the issue rather than investigating the root cause. This workaround should only cause us to degrade generic - // types from exact type parameters to approximate or canonical type parameters. In the future if we discover this issue - // is happening more frequently than we expect or the workaround is more impactful than we expect we may need to remove + // In a rare case of VS4Mac debugging VS4Mac ARM64 optimized code we get a null generics argument token. We aren't sure + // why the token is null, it may be a bug or it may be by design in the runtime. In the interest of time we are working + // around the issue rather than investigating the root cause. This workaround should only cause us to degrade generic + // types from exact type parameters to approximate or canonical type parameters. In the future if we discover this issue + // is happening more frequently than we expect or the workaround is more impactful than we expect we may need to remove // this workaround and resolve the underlying issue. if (rawToken == 0) { @@ -6388,7 +6388,7 @@ HRESULT DacHeapWalker::MoveToNextObject() mCurrObj += mCurrSize; // Check to see if we are in the correct bounds. - bool isGen0 = IsRegionGCEnabled() ? (mHeaps[mCurrHeap].Segments[mCurrSeg].Generation == 0) : + bool isGen0 = IsRegionGCEnabled() ? (mHeaps[mCurrHeap].Segments[mCurrSeg].Generation == 0) : (mHeaps[mCurrHeap].Gen0Start <= mCurrObj && mHeaps[mCurrHeap].Gen0End > mCurrObj); if (isGen0) @@ -6480,7 +6480,7 @@ HRESULT DacHeapWalker::NextSegment() mCurrObj = mHeaps[mCurrHeap].Segments[mCurrSeg].Start; - bool isGen0 = IsRegionGCEnabled() ? (mHeaps[mCurrHeap].Segments[mCurrSeg].Generation == 0) : + bool isGen0 = IsRegionGCEnabled() ? (mHeaps[mCurrHeap].Segments[mCurrSeg].Generation == 0) : (mHeaps[mCurrHeap].Gen0Start <= mCurrObj && mHeaps[mCurrHeap].Gen0End > mCurrObj); if (isGen0) @@ -6904,7 +6904,7 @@ HRESULT DacDbiInterfaceImpl::GetHeapSegments(OUT DacDbiArrayList *p _ASSERTE(eph < heaps[i].SegmentCount); if (heaps[i].Segments[eph].Start != heaps[i].Gen1Start) total++; - } + } } pSegments->Alloc(total); @@ -7166,32 +7166,38 @@ HRESULT DacDbiInterfaceImpl::GetObjectFields(COR_TYPEID id, ULONG32 celt, COR_FI for (ULONG32 i = 0; i < cFields; ++i) { FieldDesc *pField = fieldDescIterator.Next(); - layout[i].token = pField->GetMemberDef(); - layout[i].offset = (ULONG32)pField->GetOffset() + (fReferenceType ? Object::GetOffsetOfFirstField() : 0); + + COR_FIELD* corField = layout + i; + corField->token = pField->GetMemberDef(); + corField->offset = (ULONG32)pField->GetOffset() + (fReferenceType ? Object::GetOffsetOfFirstField() : 0); TypeHandle fieldHandle = pField->LookupFieldTypeHandle(); if (fieldHandle.IsNull()) { - layout[i].id.token1 = 0; - layout[i].id.token2 = 0; - layout[i].fieldType = (CorElementType)0; + corField->id = {}; + corField->fieldType = (CorElementType)0; + } + else if (fieldHandle.IsByRef()) + { + corField->id = {}; + corField->fieldType = ELEMENT_TYPE_BYREF; } else { PTR_MethodTable mt = fieldHandle.GetMethodTable(); - layout[i].fieldType = mt->GetInternalCorElementType(); - layout[i].id.token1 = (ULONG64)mt.GetAddr(); + corField->fieldType = mt->GetInternalCorElementType(); + corField->id.token1 = (ULONG64)mt.GetAddr(); if (!mt->IsArray()) { - layout[i].id.token2 = 0; + corField->id.token2 = 0; } else { TypeHandle hnd = mt->GetArrayElementTypeHandle(); PTR_MethodTable cmt = hnd.GetMethodTable(); - layout[i].id.token2 = (ULONG64)cmt.GetAddr(); + corField->id.token2 = (ULONG64)cmt.GetAddr(); } } } diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index bd96c50fc59d31..ad72af94a46ab7 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -12056,11 +12056,11 @@ TypeHandle Debugger::TypeDataWalk::ReadTypeHandle() case ELEMENT_TYPE_ARRAY: case ELEMENT_TYPE_SZARRAY: th = g_pEEInterface->LoadArrayType(data->data.elementType, typar, data->data.ArrayTypeData.arrayRank); - break; - case ELEMENT_TYPE_PTR: - case ELEMENT_TYPE_BYREF: + break; + case ELEMENT_TYPE_PTR: + case ELEMENT_TYPE_BYREF: th = g_pEEInterface->LoadPointerOrByrefType(data->data.elementType, typar); - break; + break; default: _ASSERTE(0); } From ccf0cae401598690e0eb07f8c8c1e6c1ce393e2d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 25 Feb 2022 13:28:35 -0800 Subject: [PATCH 2/3] Add details for byref COR_FIELD. --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 13c486c67d857e..f8dd52d70207b8 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -7180,7 +7180,14 @@ HRESULT DacDbiInterfaceImpl::GetObjectFields(COR_TYPEID id, ULONG32 celt, COR_FI } else if (fieldHandle.IsByRef()) { - corField->id = {}; + TypeHandle tgtType = fieldHandle.GetTypeParam(); + corField->id.token1 = CoreLibBinder::GetElementType(ELEMENT_TYPE_I).GetAddr(); + if (!tgtType.IsNull()) + { + PTR_MethodTable cmt = tgtType.GetMethodTable(); + corField->id.token2 = (ULONG64)cmt.GetAddr(); + } + corField->fieldType = ELEMENT_TYPE_BYREF; } else From 12fed4d626271e10f801b612e1d02b34a4741ef2 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 25 Feb 2022 15:57:33 -0800 Subject: [PATCH 3/3] Remove setting of token2 in all cases. This value is not used in any scenario and therefore is more confusing than helpful. Add a commment about the expectations of the returned values. --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 25 ++++++------------------ 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index f8dd52d70207b8..be132fe0975831 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -7180,32 +7180,19 @@ HRESULT DacDbiInterfaceImpl::GetObjectFields(COR_TYPEID id, ULONG32 celt, COR_FI } else if (fieldHandle.IsByRef()) { - TypeHandle tgtType = fieldHandle.GetTypeParam(); - corField->id.token1 = CoreLibBinder::GetElementType(ELEMENT_TYPE_I).GetAddr(); - if (!tgtType.IsNull()) - { - PTR_MethodTable cmt = tgtType.GetMethodTable(); - corField->id.token2 = (ULONG64)cmt.GetAddr(); - } - corField->fieldType = ELEMENT_TYPE_BYREF; + // All ByRefs intentionally return IntPtr's MethodTable. + corField->id.token1 = CoreLibBinder::GetElementType(ELEMENT_TYPE_I).GetAddr(); + corField->id.token2 = 0; } else { + // Note that pointer types are handled in this path. + // IntPtr's MethodTable is set for all pointer types and is expected. PTR_MethodTable mt = fieldHandle.GetMethodTable(); corField->fieldType = mt->GetInternalCorElementType(); corField->id.token1 = (ULONG64)mt.GetAddr(); - - if (!mt->IsArray()) - { - corField->id.token2 = 0; - } - else - { - TypeHandle hnd = mt->GetArrayElementTypeHandle(); - PTR_MethodTable cmt = hnd.GetMethodTable(); - corField->id.token2 = (ULONG64)cmt.GetAddr(); - } + corField->id.token2 = 0; } }