Skip to content

Various fixes for generics handling #3185

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

Merged
merged 1 commit into from
Jun 20, 2025
Merged

Various fixes for generics handling #3185

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
183 changes: 118 additions & 65 deletions src/CLR/Core/TypeSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1266,32 +1266,55 @@ bool CLR_RT_MethodDef_Instance::InitializeFromIndex(
{
NATIVE_PROFILE_CLR_CORE();

// Look up the TypeSpec's cross‐reference to find the *open* generic definition and its assembly
CLR_INDEX tsAsmIdx = typeSpec.Assembly();
if (tsAsmIdx == 0 || tsAsmIdx > (int)g_CLR_RT_TypeSystem.c_MaxAssemblies)
CLR_RT_TypeSpec_Instance tsInst;

if (!tsInst.InitializeFromIndex(typeSpec))
{
return false;
}
CLR_RT_Assembly *tsAsm = g_CLR_RT_TypeSystem.m_assemblies[tsAsmIdx - 1];

int tsRow = (int)typeSpec.TypeSpec();
const auto &xref = tsAsm->crossReferenceTypeSpec[tsRow];
// parse the signature to get to the GENERICINST and then the definition token
CLR_RT_Assembly *tsAsm = tsInst.assembly;
auto tsRec = tsAsm->GetTypeSpec(typeSpec.TypeSpec());
CLR_RT_SignatureParser parser;
parser.Initialize_TypeSpec(tsAsm, tsRec);

// xref.ownerType is the open‐generic TypeDef_Index of Span`1, List`1, etc.
CLR_RT_TypeDef_Index ownerType = xref.ownerType;
CLR_RT_SignatureParser::Element elem;
if (FAILED(parser.Advance(elem)))
{
return false;
}

// Compute the MethodDef_Index bound to the owner’s assembly
CLR_RT_TypeDef_Index ownerTypeIdx;
CLR_RT_MethodDef_Index mdRebound;

// pack the new assembly index (ownerType.Assembly()) with the original row
mdRebound.data = (ownerType.Assembly() << 24) | md.Method();
if (elem.DataType == DATATYPE_VAR)
{
CLR_RT_TypeDef_Index realOwner;
NanoCLRDataType dummyDT;

if (!tsAsm->FindGenericParamAtTypeSpec(typeSpec.TypeSpec(), elem.GenericParamPosition, realOwner, dummyDT))
{
return false;
}

ownerTypeIdx = realOwner;
}
else
{
// elem.Class.data now contains the TypeDef/TypeRef token for the generic definition
ownerTypeIdx.data = elem.Class.data;
}

// rebind the method onto the *declaring* assembly of the generic
mdRebound.data = (ownerTypeIdx.Assembly() << 24) | md.Method();

if (!InitializeFromIndex(mdRebound))
{
return false;
}

// set the generic‐type context *before* we lose it
// remember the TypeSpec so this is available when needed
this->genericType = &typeSpec;

return true;
Expand Down Expand Up @@ -1401,7 +1424,7 @@ bool CLR_RT_MethodDef_Instance::ResolveToken(
return false;
}

Set(assemblyIndex - 1, methodIndex.Method());
Set(assemblyIndex, methodIndex.Method());
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical bug fix: Corrected assembly index calculation

The removal of - 1 from assemblyIndex fixes an off-by-one error when setting the method index for TypeSpec-owned method references. This ensures the correct assembly is referenced.

🤖 Prompt for AI Agents 
In src/CLR/Core/TypeSystem.cpp at line 1427, fix the off-by-one error by
removing the subtraction of 1 from assemblyIndex when calling Set with
assemblyIndex and methodIndex.Method(). This correction ensures the correct
assembly is referenced for TypeSpec-owned method references.

assembly = g_CLR_RT_TypeSystem.m_assemblies[assemblyIndex - 1];
target = assembly->GetMethodDef(methodIndex.Method());
}
Expand Down Expand Up @@ -3278,7 +3301,7 @@ HRESULT CLR_RT_Assembly::ResolveMethodRef()
#endif
}

CLR_UINT32 dummyAssemblyIndex = 0xffff;
CLR_UINT32 dummyAssemblyIndex = 0xffffffff;

if (typeSpecInstance.assembly->FindMethodDef(
typeSpecInstance.target,
Expand Down Expand Up @@ -4770,6 +4793,7 @@ bool CLR_RT_Assembly::FindGenericParamAtTypeSpec(
// element.Class was filled from the VAR position
typeDef = element.Class;
dataType = element.DataType;

return true;
}

Expand Down Expand Up @@ -5002,15 +5026,24 @@ bool CLR_RT_Assembly::FindMethodDef(

// switch to the assembly that declared this TypeSpec
CLR_RT_Assembly *declAssm = tsInstance.assembly;
const CLR_RECORD_TYPEDEF *td =
(const CLR_RECORD_TYPEDEF *)declAssm->GetTable(TBL_TypeDef) + tsInstance.typeDefIndex;

if (declAssm->FindMethodDef(td, methodName, base, sig, index))
CLR_INDEX typeDefIdx = tsInstance.typeDefIndex;

// validate that it really is in-range
if (typeDefIdx >= declAssm->tablesSize[TBL_TypeDef])
{
// doesn't seem to be, jump to TypeSpec parsing
goto try_typespec;
}

if (declAssm->FindMethodDef(declAssm->GetTypeDef(typeDefIdx), methodName, base, sig, index))
{
assmIndex = declAssm->assemblyIndex;
return true;
}

try_typespec:

// parse the TypeSpec signature to get the *definition* token of the generic type:
CLR_RT_SignatureParser parser{};
parser.Initialize_TypeSpec(this, ts);
Expand Down Expand Up @@ -6233,18 +6266,41 @@ HRESULT CLR_RT_TypeSystem::BuildTypeName(const CLR_RT_TypeSpec_Index &typeIndex,

for (int i = 0; i < parser.GenParamCount; i++)
{
// read the next element (should be either VAR, MVAR, or a concrete type)
parser.Advance(element);

#if defined(VIRTUAL_DEVICE)
NANOCLR_CHECK_HRESULT(QueueStringToBuffer(szBuffer, iBuffer, c_CLR_RT_DataTypeLookup[element.DataType].m_name));
#endif
if (element.DataType == DATATYPE_VAR)
{
// resolve the !T against our *closed* typeIndex
CLR_RT_TypeDef_Index realTd;
NanoCLRDataType realDt;

// this will bind !T→System.Int32, etc.
instance.assembly->FindGenericParamAtTypeSpec(
typeIndex.TypeSpec(), // closed instantiation row
element.GenericParamPosition, // the !N slot
realTd,
realDt);

// now print the *actual* type name
BuildTypeName(realTd, szBuffer, iBuffer);
}
else
{
// concrete type (e.g. a nested generic, a value type, another class...)
CLR_RT_TypeDef_Index td;
td.data = element.Class.data;

BuildTypeName(td, szBuffer, iBuffer);
}

if (i + 1 < parser.GenParamCount)
{
NANOCLR_CHECK_HRESULT(QueueStringToBuffer(szBuffer, iBuffer, ","));
}
}

CLR_SafeSprintf(szBuffer, iBuffer, ">");
NANOCLR_CHECK_HRESULT(QueueStringToBuffer(szBuffer, iBuffer, ">"));

NANOCLR_NOCLEANUP();
}
Expand Down Expand Up @@ -6320,69 +6376,66 @@ HRESULT CLR_RT_TypeSystem::BuildMethodName(
NATIVE_PROFILE_CLR_CORE();
NANOCLR_HEADER();

CLR_RT_TypeDef_Instance declTypeInst{};
CLR_RT_MethodDef_Instance tempMD{};
CLR_RT_MethodDef_Instance inst{};
CLR_RT_TypeDef_Index declTypeIdx;
CLR_RT_TypeDef_Instance instOwner{};
bool useGeneric = false;

if (genericType == nullptr)
// find out which type declares this method
if (!tempMD.InitializeFromIndex(md))
{
if (inst.InitializeFromIndex(md) == false)
{
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}

CLR_RT_TypeDef_Instance instOwner{};
if (instOwner.InitializeFromMethod(inst) == false)
{
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}

NANOCLR_CHECK_HRESULT(BuildTypeName(instOwner, szBuffer, iBuffer));

CLR_SafeSprintf(szBuffer, iBuffer, "::%s", inst.assembly->GetString(inst.target->name));
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}
else
{
CLR_INDEX tsAsmIdx = genericType->Assembly();
CLR_RT_Assembly *genericTypeAssembly = g_CLR_RT_TypeSystem.m_assemblies[tsAsmIdx - 1];

CLR_RT_SignatureParser parser;
parser.Initialize_TypeSpec(genericTypeAssembly, genericTypeAssembly->GetTypeSpec(genericType->TypeSpec()));

CLR_RT_SignatureParser::Element element;

// get type
parser.Advance(element);

CLR_RT_TypeDef_Index typeDef;
typeDef.data = element.Class.data;
if (!declTypeInst.InitializeFromMethod(tempMD))
{
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}

BuildTypeName(typeDef, szBuffer, iBuffer);
declTypeIdx.Set(md.Assembly(), declTypeInst.assembly->crossReferenceMethodDef[md.Method()].GetOwner());

NANOCLR_CHECK_HRESULT(QueueStringToBuffer(szBuffer, iBuffer, "<"));
if (genericType != nullptr && genericType->data != 0xffffffff)
{
// parse TypeSpec to get its TypeDef
CLR_RT_TypeSpec_Instance tsInst = {};

for (int i = 0; i < parser.GenParamCount; i++)
if (tsInst.InitializeFromIndex(*genericType))
{
parser.Advance(element);

#if defined(VIRTUAL_DEVICE)
NANOCLR_CHECK_HRESULT(
QueueStringToBuffer(szBuffer, iBuffer, c_CLR_RT_DataTypeLookup[element.DataType].m_name));
#endif
if (i + 1 < parser.GenParamCount)
if (tsInst.typeDefIndex == declTypeIdx.Type())
{
NANOCLR_CHECK_HRESULT(QueueStringToBuffer(szBuffer, iBuffer, ","));
useGeneric = true;
}
}
}

if (inst.InitializeFromIndex(md, *genericType) == false)
if (!useGeneric)
{
// fallback: non generic method or a different type
if (!inst.InitializeFromIndex(md))
{
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}
}
else
{
// method belong to a closed generic type, so bind it to the appropriate TypeSpec
if (!inst.InitializeFromIndex(md, *genericType))
{
// this is a MethodDef for a generic type, but the generic type is not bound to an assembly
// so we cannot build the name of the method.
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}
}

CLR_SafeSprintf(szBuffer, iBuffer, ">::%s", genericTypeAssembly->GetString(inst.target->name));
if (instOwner.InitializeFromMethod(inst) == false)
{
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}

NANOCLR_CHECK_HRESULT(BuildTypeName(instOwner, szBuffer, iBuffer));

CLR_SafeSprintf(szBuffer, iBuffer, "::%s", inst.assembly->GetString(inst.target->name));

NANOCLR_NOCLEANUP();
}

Expand Down