Skip to content

Fix rebinding logic in MethodDef_Instance::InitializeFromIndex() #3221

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
+47 −5
Open

Fix rebinding logic in MethodDef_Instance::InitializeFromIndex() #3221

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
20fc89d
Fix rebinding logic in MethodDef_Instance::InitializeFromIndex()
josesimoes Oct 17, 2025
b34c1f4
DumpOpCode now is using generic type from call
josesimoes Oct 29, 2025
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
50 changes: 46 additions & 4 deletions src/CLR/Core/TypeSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ bool CLR_RT_MethodDef_Instance::InitializeFromIndex(
}

CLR_RT_TypeDef_Index ownerTypeIdx;
CLR_RT_MethodDef_Index mdRebound;
CLR_RT_MethodDef_Index mdResolved;

if (elem.DataType == DATATYPE_VAR)
{
Expand All @@ -1661,10 +1661,52 @@ bool CLR_RT_MethodDef_Instance::InitializeFromIndex(
ownerTypeIdx.data = elem.Class.data;
}

// rebind the method onto the *declaring* assembly of the generic
mdRebound.data = (ownerTypeIdx.Assembly() << 24) | md.Method();
// try to find the correct method reference
if (ownerTypeIdx.Assembly() != md.Assembly())
{
// Cross-assembly case: need to find the corresponding MethodRef/MethodDef
CLR_RT_Assembly *originalAssm = g_CLR_RT_TypeSystem.m_assemblies[md.Assembly() - 1];
CLR_RT_Assembly *targetAssm = g_CLR_RT_TypeSystem.m_assemblies[ownerTypeIdx.Assembly() - 1];

// Get the original method information
const CLR_RECORD_METHODDEF *originalMD = originalAssm->GetMethodDef(md.Method());
const char *methodName = originalAssm->GetString(originalMD->name);

// Try to find the method in the target assembly by looking through MethodRefs
bool found = false;
for (int i = 0; i < targetAssm->tablesSize[TBL_MethodRef]; i++)
{
const CLR_RECORD_METHODREF *mr = targetAssm->GetMethodRef(i);
const char *refName = targetAssm->GetString(mr->name);

if (!strcmp(methodName, refName))
{
// Found a potential match, now check if it resolves to our original method
CLR_RT_MethodRef_CrossReference &crossRef = targetAssm->crossReferenceMethodRef[i];

if (crossRef.target.data == md.data)
{
// This MethodRef points to our original method!
mdResolved.data = md.data;
found = true;
break;
Comment on lines +1684 to +1692
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

Method matching only compares names without checking signatures. This can lead to incorrect matches when multiple overloaded methods exist with the same name but different signatures. The code should use signature comparison similar to FindMethodDef (lines 5703-5710) which uses CLR_RT_SignatureParser and CLR_RT_TypeSystem::MatchSignature.

Suggested change
// Found a potential match, now check if it resolves to our original method
CLR_RT_MethodRef_CrossReference &crossRef = targetAssm->crossReferenceMethodRef[i];
if (crossRef.target.data == md.data)
{
// This MethodRef points to our original method!
mdResolved.data = md.data;
found = true;
break;
// Compare signatures to ensure correct method match
CLR_RT_SignatureParser sigParserOriginal;
CLR_RT_SignatureParser sigParserCandidate;
// Parse the signature of the original method
sigParserOriginal.Initialize_Method(originalAssm, originalMD);
// Parse the signature of the candidate method reference
sigParserCandidate.Initialize_MethodRef(targetAssm, mr);
if (CLR_RT_TypeSystem::MatchSignature(sigParserOriginal, sigParserCandidate))
{
// Found a potential match, now check if it resolves to our original method
CLR_RT_MethodRef_CrossReference &crossRef = targetAssm->crossReferenceMethodRef[i];
if (crossRef.target.data == md.data)
{
// This MethodRef points to our original method!
mdResolved.data = md.data;
found = true;
break;
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

@copilot Method signatures will be different depending on a type being a closed or open generic... signatures can't be compared on an exact match.

}
}
Comment on lines +1682 to +1694
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The method resolution logic is incorrect. When a matching MethodRef is found (line 1687), the code sets mdResolved.data to md.data (the original method), but it should construct a new method index using the target assembly and the MethodRef index. The resolved method should be: mdResolved.data = (ownerTypeIdx.Assembly() << 24) | i; This defeats the purpose of the cross-assembly resolution.

Copilot uses AI. Check for mistakes.
}

if (!found)
{
// Fallback: keep the original method reference
mdResolved.data = md.data;
}
}
else
{
// Same assembly case: no rebinding needed
mdResolved.data = md.data;
}

if (!InitializeFromIndex(mdRebound))
if (!InitializeFromIndex(mdResolved))
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/CLR/Diagnostics/Info.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ void CLR_RT_Assembly::DumpOpcodeDirect(
if (mdInst.ResolveToken(token, call.assembly, call.genericType))
{
// mdInst now holds the target MethodDef (or MethodSpec) plus any genericType.
CLR_RT_DUMP::METHOD(mdInst, mdInst.genericType);
CLR_RT_DUMP::METHOD(mdInst, call.genericType);
}
else
{
Expand Down