Skip to content

Fix closed generic instance corruption and generic type-param cast #3452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
danielmeza wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Fix closed generic instance corruption and generic type-param cast #3452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37bc0ae
Fix generic-instance corruption from synchronized methods + complete …
danielmeza Jun 24, 2026
fe27eb7
Resolve generic type-param cast through same-assembly interface dispatch
danielmeza Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/CLR/Core/CLR_RT_HeapBlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,10 @@ CLR_UINT32 CLR_RT_HeapBlock::GetHashCode(CLR_RT_HeapBlock *ptr, bool fRecurse, C

case DATATYPE_CLASS:
case DATATYPE_VALUETYPE:
// a boxed generic value type (the dedicated DATATYPE_GENERICINST heap object) keeps its
// declaring TypeDef in the object header (SetObjectCls), so it hashes by its inline fields
// exactly like a value-type instance.
case DATATYPE_GENERICINST:
{
CLR_RT_TypeDef_Instance cls{};
cls.InitializeFromIndex(ptr->ObjectCls());
Expand Down Expand Up @@ -1462,6 +1466,9 @@ bool CLR_RT_HeapBlock::ObjectsEqual(

switch (leftDataType)
{
// a boxed generic value type (DATATYPE_GENERICINST) compares by its inline fields like a
// value type; keys of the same closed type share TypeDef + layout, so the field walk is valid.
case DATATYPE_GENERICINST:
case DATATYPE_VALUETYPE:
if (pArgLeft.ObjectCls().data == pArgRight.ObjectCls().data)
{
Expand Down Expand Up @@ -1914,6 +1921,7 @@ CLR_INT32 CLR_RT_HeapBlock::Compare_Values(const CLR_RT_HeapBlock &left, const C

case DATATYPE_CLASS:
case DATATYPE_VALUETYPE:
case DATATYPE_GENERICINST: // boxed generic value type: order by reference like class/value-type
case DATATYPE_SZARRAY:
case DATATYPE_WEAKCLASS:
return CompareValues_Pointers(&left, &right);
Expand Down
16 changes: 14 additions & 2 deletions src/CLR/Core/CLR_RT_HeapBlock_Lock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ HRESULT CLR_RT_HeapBlock_Lock::CreateInstance(
{
case DATATYPE_VALUETYPE:
case DATATYPE_CLASS:
ptr->SetObjectLock(lock);
// A generic instance overloads the object-lock slot to store its closed TypeSpec
// (see SetGenericInstanceType / ObjectGenericType), so it cannot also hold a monitor
// lock pointer. Writing the lock here would clobber the TypeSpec and break every
// subsequent generic field / method resolution on the instance.
if (ptr->IsAGenericInstance() == false)
{
ptr->SetObjectLock(lock);
}
break;

default:
Expand Down Expand Up @@ -138,7 +145,12 @@ void CLR_RT_HeapBlock_Lock::ChangeOwner()
{
case DATATYPE_VALUETYPE:
case DATATYPE_CLASS:
ptr->SetObjectLock(nullptr);
// See CreateInstance: a generic instance never stored a lock here (the slot holds its
// closed TypeSpec), so do not clear it or the TypeSpec would be wiped.
if (ptr->IsAGenericInstance() == false)
{
ptr->SetObjectLock(nullptr);
}
break;

default:
Expand Down
36 changes: 21 additions & 15 deletions src/CLR/Core/Execution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2813,6 +2813,14 @@ HRESULT CLR_RT_ExecutionEngine::NewGenericInstanceObject(
// Associate the instance with its declaring type for reflection & casting utilities.
giHeader->SetObjectCls(instance);

// Record the closed TypeSpec on the instance and mark it as a generic instance.
// SetObjectCls writes objectHeader.cls (offset 0) and clears objectHeader.lock (offset 4);
// SetGenericInstanceType then stores the TypeSpec into the slot that overlaps the lock,
// leaving the declaring TypeDef intact. Without this, ObjectGenericType() (used by clone,
// type recovery and casting) and IsAGenericInstance() would not work for this representation.
giHeader->SetGenericInstanceType(*genericInstance);
giHeader->SetFlags(CLR_RT_HeapBlock::HB_GenericInstance);

//
// Initialize field types, from last to first.
//
Expand All @@ -2822,12 +2830,6 @@ HRESULT CLR_RT_ExecutionEngine::NewGenericInstanceObject(

fieldCursor = reinterpret_cast<CLR_RT_HeapBlock *>(giHeader) + totFields;

CLR_Debug::Printf(
"DBG GenericInst: NewGenericInstanceObject type='%s' totFields=%d giHeader=%08X\r\n",
instance.assembly->GetString(instance.target->name),
totFields,
(uintptr_t)giHeader);

while (--totFields > 0)
{
while (clsFields == 0)
Expand All @@ -2851,15 +2853,7 @@ HRESULT CLR_RT_ExecutionEngine::NewGenericInstanceObject(
target--;
clsFields--;

CLR_Debug::Printf(
"DBG GenericInst: InitField field='%s' type='%s' cursor=%08X\r\n",
assm->GetString(target->name),
assm->GetString(target->type),
(uintptr_t)fieldCursor);

NANOCLR_CHECK_HRESULT(InitializeReference(*fieldCursor, target, assm, genericInstance));

CLR_Debug::Printf("DBG GenericInst: InitField done dt=%d\r\n", (int)fieldCursor->DataType());
}

if (instance.HasFinalizer())
Expand Down Expand Up @@ -2900,6 +2894,7 @@ HRESULT CLR_RT_ExecutionEngine::CloneObject(CLR_RT_HeapBlock &reference, const C

case DATATYPE_VALUETYPE:
case DATATYPE_CLASS:
case DATATYPE_GENERICINST:
{
//
// Save the pointer to the object to clone, in case 'reference' and 'source' point to the same block.
Expand All @@ -2910,7 +2905,7 @@ HRESULT CLR_RT_ExecutionEngine::CloneObject(CLR_RT_HeapBlock &reference, const C
safeSource.SetObjectReference(obj);
CLR_RT_ProtectFromGC gc(safeSource);

if (obj->IsAGenericInstance())
if (obj->IsAGenericInstance() || dt == DATATYPE_GENERICINST)
{
// instanciate the generic type
genericInstance.InitializeFromIndex(obj->ObjectGenericType());
Expand Down Expand Up @@ -3154,8 +3149,19 @@ CLR_RT_HeapBlock_Lock *CLR_RT_ExecutionEngine::FindLockObject(CLR_RT_HeapBlock &
{
case DATATYPE_VALUETYPE:
case DATATYPE_CLASS:
// A generic instance (either a dedicated DATATYPE_GENERICINST object or a class flagged
// HB_GenericInstance) overloads the lock slot to store its closed TypeSpec, so it can
// never carry a real monitor lock. Returning null here (instead of reading the slot as a
// pointer) avoids dereferencing the TypeSpec value as a CLR_RT_HeapBlock_Lock.
if (ptr->IsAGenericInstance())
{
return nullptr;
}
return ptr->ObjectLock();

case DATATYPE_GENERICINST:
return nullptr;

default:
// the remaining data types aren't to be handled
break;
Expand Down
9 changes: 9 additions & 0 deletions src/CLR/Core/GarbageCollector_ComputeReachabilityGraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,18 @@ bool CLR_RT_GarbageCollector::ComputeReachabilityGraphForMultipleBlocks(CLR_RT_H

case DATATYPE_CLASS:
case DATATYPE_VALUETYPE:
case DATATYPE_GENERICINST:
//
// This is the real object, mark all its fields.
//
// DATATYPE_GENERICINST is the dedicated heap representation of a closed generic
// instance (see CLR_RT_ExecutionEngine::NewGenericInstanceObject). Just like a
// DATATYPE_CLASS / DATATYPE_VALUETYPE object, block[0] is the header and block[1..]
// are the instance fields. Without this case the object falls through to 'default'
// and its fields are never marked, so any reference-typed field (e.g. the hidden
// backing delegate of a field-like event) is reclaimed and left dangling, crashing
// with LoadProhibited on the next access.
//
lst = ptr + 1;
num = ptr->DataSize() - 1;
break;
Expand Down
11 changes: 8 additions & 3 deletions src/CLR/Core/GarbageCollector_Info.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,13 @@ void CLR_RT_GarbageCollector::ValidateCluster(CLR_RT_HeapCluster *hc)
NANOCLR_DEBUG_STOP();
}

// Check for overlapping blocks, if this is not a class or value type
// Check for overlapping blocks, if this is not a class, value type, or generic instance —
// those reuse the Next()/Prev() header slot for object data, so it isn't a real free-list link.
// First the next block
CLR_RT_HeapBlock_Node const *nextPtr = ptr->Next();
if ((ptr->DataType() != DATATYPE_VALUETYPE && ptr->DataType() != DATATYPE_CLASS) && nextPtr)
if ((ptr->DataType() != DATATYPE_VALUETYPE && ptr->DataType() != DATATYPE_CLASS &&
ptr->DataType() != DATATYPE_GENERICINST) &&
nextPtr)
{
// is the next pointer before or after the current block?
if (nextPtr < ptr)
Expand All @@ -134,7 +137,9 @@ void CLR_RT_GarbageCollector::ValidateCluster(CLR_RT_HeapCluster *hc)

// now the previous block
CLR_RT_HeapBlock_Node const *prevPtr = ptr->Prev();
if ((ptr->DataType() != DATATYPE_VALUETYPE && ptr->DataType() != DATATYPE_CLASS) && prevPtr)
if ((ptr->DataType() != DATATYPE_VALUETYPE && ptr->DataType() != DATATYPE_CLASS &&
ptr->DataType() != DATATYPE_GENERICINST) &&
prevPtr)
{
// is the previous pointer before or after the current block?
if (prevPtr < ptr)
Expand Down
88 changes: 70 additions & 18 deletions src/CLR/Core/Interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,11 @@ bool CLR_RT_HeapBlock::InitObject()
return false;
}

if (dt == DATATYPE_VALUETYPE && obj->IsBoxed() == false)
if ((dt == DATATYPE_VALUETYPE || dt == DATATYPE_GENERICINST) && obj->IsBoxed() == false)
{
// A closed generic value-type instance (DATATYPE_GENERICINST) has the same header + inline-field
// layout as a value type, so zero each field block, not just the header. Skipping this leaves stale
// reference/value fields (e.g. a Nullable<T> initialized via initobj).
CLR_UINT32 num = obj->DataSize();

while (--num)
Expand Down Expand Up @@ -2245,28 +2248,47 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
// 7. Object is a class instance (generic or not)
if (obj && obj->DataType() == DATATYPE_CLASS)
{
CLR_RT_TypeDef_Index objCls = obj->ObjectCls();
// 7a. A closed generic instance stores its OWN exact closed TypeSpec
// (set by NewObject via SetGenericInstanceType). That is the authoritative
// generic context for an explicit-interface dispatch through a non-generic
// interface (e.g. DataTemplate<ItemVm>::IDataTemplate.Build, called from a
// non-generic Repeater), where neither the caller nor the interface carries
// a TypeSpec. Use it directly so a (T)item cast inside the callee can
// resolve VAR !0 -> the real type arg instead of throwing CLR_E_INVALID_CAST.
if (obj->IsAGenericInstance())
{
const CLR_RT_TypeSpec_Index &objTS = obj->ObjectGenericType();
if (NANOCLR_INDEX_IS_VALID(objTS))
{
effectiveCallerGeneric = &objTS;
}
}

// 8. Object has a valid TypeDef
if (NANOCLR_INDEX_IS_VALID(objCls))
if (effectiveCallerGeneric == nullptr)
{
// NOW search for a TypeSpec that matches this object's TypeDef
// This is only needed for closed generic instances like List<int>
for (int i = 0; i < assm->tablesSize[TBL_TypeSpec]; i++)
CLR_RT_TypeDef_Index objCls = obj->ObjectCls();

// 8. Object has a valid TypeDef
if (NANOCLR_INDEX_IS_VALID(objCls))
{
const CLR_RT_TypeSpec_Index *tsIdx =
&assm->crossReferenceTypeSpec[i].genericType;
if (NANOCLR_INDEX_IS_VALID(*tsIdx))
// Fallback: search for a TypeSpec that matches this object's TypeDef
// (covers instances not flagged as a closed generic).
for (int i = 0; i < assm->tablesSize[TBL_TypeSpec]; i++)
{
CLR_RT_TypeSpec_Instance tsInst{};
if (tsInst.InitializeFromIndex(*tsIdx) &&
NANOCLR_INDEX_IS_VALID(tsInst.genericTypeDef) &&
tsInst.genericTypeDef.data == objCls.data)
const CLR_RT_TypeSpec_Index *tsIdx =
&assm->crossReferenceTypeSpec[i].genericType;
if (NANOCLR_INDEX_IS_VALID(*tsIdx))
{
// Found a TypeSpec in the caller's assembly that matches the
// object's class
effectiveCallerGeneric = tsIdx;
break;
CLR_RT_TypeSpec_Instance tsInst{};
if (tsInst.InitializeFromIndex(*tsIdx) &&
NANOCLR_INDEX_IS_VALID(tsInst.genericTypeDef) &&
tsInst.genericTypeDef.data == objCls.data)
{
// Found a TypeSpec in the caller's assembly that matches
// the object's class
effectiveCallerGeneric = tsIdx;
break;
}
}
}
}
Expand Down Expand Up @@ -2498,6 +2520,36 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
{
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}

// Same-assembly interface dispatch uses a MethodDef token, so the
// CALLVIRT pre-pass above (guarded to TBL_MethodRef) does not run and
// genericType is left null. Recover the closed TypeSpec from the
// receiver, the same way the genericType branch above does, so a
// (T)item cast inside an explicit interface impl on a closed generic
// (e.g. DataTemplate<ItemVm>::IDataTemplate.Build) resolves VAR !0
// instead of throwing CLR_E_INVALID_CAST.
if (calleeInst.genericType == nullptr)
{
CLR_RT_HeapBlock *thisHeap = pThis[0].Dereference();
if (thisHeap != nullptr && thisHeap->IsAGenericInstance())
{
const CLR_RT_TypeSpec_Index &objTS = thisHeap->ObjectGenericType();
if (NANOCLR_INDEX_IS_VALID(objTS))
{
CLR_RT_TypeSpec_Instance tsObj{};
CLR_RT_TypeDef_Instance declObj{};
if (tsObj.InitializeFromIndex(objTS) &&
declObj.InitializeFromMethod(calleeInst) &&
tsObj.genericTypeDef.data == declObj.data)
{
calleeInst.InitializeFromIndex(
calleeReal,
objTS,
&stack->m_call);
}
}
}
}
}

// Restore the array element type after reinitializing calleeInst
Expand Down
8 changes: 7 additions & 1 deletion src/CLR/Core/TypeSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3310,6 +3310,10 @@ HRESULT CLR_RT_TypeDescriptor::InitializeFromObject(const CLR_RT_HeapBlock &ref)

case DATATYPE_VALUETYPE:
case DATATYPE_CLASS:
case DATATYPE_GENERICINST:
// A closed generic instance laid out as a dedicated DATATYPE_GENERICINST heap object
// still records its declaring (open) TypeDef in the object header via SetObjectCls, so
// recover the type the same way as a regular class/value-type instance.
cls = &obj->ObjectCls();
break;

Expand Down Expand Up @@ -3520,8 +3524,10 @@ HRESULT CLR_RT_TypeDescriptor::ExtractTypeIndexFromObject(const CLR_RT_HeapBlock

NANOCLR_CHECK_HRESULT(CLR_RT_TypeDescriptor::ExtractObjectAndDataType(obj, dt));

if (dt == DATATYPE_VALUETYPE || dt == DATATYPE_CLASS)
if (dt == DATATYPE_VALUETYPE || dt == DATATYPE_CLASS || dt == DATATYPE_GENERICINST)
{
// DATATYPE_GENERICINST objects keep their declaring TypeDef in the object header (SetObjectCls),
// so the type index is recovered exactly like a class/value-type instance.
res = obj->ObjectCls();
}
else
Expand Down
Loading