Skip to content

Commit

Permalink
Fix issue 44646 - ILAsm incorrectly handles method .custom attribute …
Browse files Browse the repository at this point in the history
…when we have generic type constraint (#44850)

* Fix issue 44646 - ILAsm incorrectly handles method .custom attribute when we have generic type constraint

The IL Assembler incorrectly dropped a .custom attribute for the method
This only occurred when the inline syntax for generic type parameters was used in the method definition.
This behavior can also occur for a generic class definition.

* Fix typo in comment
  • Loading branch information
briansull authored Nov 19, 2020
1 parent 5b48cae commit d3e5aa8
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 12 deletions.
48 changes: 38 additions & 10 deletions src/coreclr/src/ilasm/assembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2487,6 +2487,10 @@ BOOL Assembler::IsPortablePdb()
return (m_pdbFormat == PORTABLE) && (m_pPortablePdbWriter != NULL);
}

// This method is called after we have parsed the generic type parameters for either
// a generic class or a generic method. It calls CheckAddGenericParamConstraint on
// each generic parameter constraint that was recorded.
//
void Assembler::RecordTypeConstraints(GenericParamConstraintList* pGPCList, int numTyPars, TyParDescr* tyPars)
{
if (numTyPars > 0)
Expand All @@ -2503,13 +2507,18 @@ void Assembler::RecordTypeConstraints(GenericParamConstraintList* pGPCList, int
for (int j = 0; j < numConstraints; j++)
{
mdToken tkTypeConstraint = ptk[j];
CheckAddGenericParamConstraint(pGPCList, i, tkTypeConstraint);

// pass false for isParamDirective, these constraints are from the class or method definition
//
CheckAddGenericParamConstraint(pGPCList, i, tkTypeConstraint, false);
}
}
}
}
}

// AddGenericParamConstraint is called when we have a .param constraint directive after a class definition
//
void Assembler::AddGenericParamConstraint(int index, char * pStrGenericParam, mdToken tkTypeConstraint)
{
if (!m_pCurClass)
Expand Down Expand Up @@ -2545,13 +2554,20 @@ void Assembler::AddGenericParamConstraint(int index, char * pStrGenericParam, md
return;
}
}
bool newlyAdded = CheckAddGenericParamConstraint(&m_pCurClass->m_GPCList, index, tkTypeConstraint);

// pass true for isParamDirective, we are parsing a .param directive for a class here
//
CheckAddGenericParamConstraint(&m_pCurClass->m_GPCList, index, tkTypeConstraint, true);
}

// returns true if we create a new GenericParamConstraintDescriptor
// reurns false if we return an already existing GenericParamConstraintDescriptor
// CheckAddGenericParamConstraint is called when we have to handle a generic parameter constraint
// When parsing a generic class/method definition isParamDirective is false - we have a generic type constaint
// for this case we do not setup m_pCustomDescrList as a .custom after a generic class/method definition is
// for the class/method
// When isParamDirective is true, we have a .param constraint directive and we will setup m_pCustomDescrList
// and any subsequent .custom is for the generic parameter constrant
//
bool Assembler::CheckAddGenericParamConstraint(GenericParamConstraintList* pGPCList, int index, mdToken tkTypeConstraint)
void Assembler::CheckAddGenericParamConstraint(GenericParamConstraintList* pGPCList, int index, mdToken tkTypeConstraint, bool isParamDirective)
{
_ASSERTE(tkTypeConstraint != 0);
_ASSERTE(index >= 0);
Expand All @@ -2578,18 +2594,30 @@ bool Assembler::CheckAddGenericParamConstraint(GenericParamConstraintList* pGPCL

if (match)
{
m_pCustomDescrList = pGPC->CAList();
return false;
// Found an existing generic parameter constraint
//
if (isParamDirective)
{
// Setup the custom descr list so that we can record
// custom attributes on this generic param contraint
//
m_pCustomDescrList = pGPC->CAList();
}
}
else
{
// not found add it to our list
// not found - add it to our pGPCList
//
GenericParamConstraintDescriptor* pNewGPCDescr = new GenericParamConstraintDescriptor();
pNewGPCDescr->Init(index, tkTypeConstraint);
pGPCList->PUSH(pNewGPCDescr);
m_pCustomDescrList = pNewGPCDescr->CAList();
return true;
if (isParamDirective)
{
// Setup the custom descr list so that we can record
// custom attributes on this generic param contraint
//
m_pCustomDescrList = pNewGPCDescr->CAList();
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/ilasm/assembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1289,7 +1289,7 @@ class Assembler {

void AddGenericParamConstraint(int index, char * pStrGenericParam, mdToken tkTypeConstraint);

bool CheckAddGenericParamConstraint(GenericParamConstraintList* pGPCList, int index, mdToken tkTypeConstraint);
void CheckAddGenericParamConstraint(GenericParamConstraintList* pGPCList, int index, mdToken tkTypeConstraint, bool isParamDirective);

void EmitGenericParamConstraints(int numTyPars, TyParDescr* pTyPars, mdToken tkOwner, GenericParamConstraintList* pGPCL);

Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/src/ilasm/method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ Label *Method::FindLabel(DWORD PC)
return NULL;
}

// Method::AddGenericParamConstraint is called when we have a .param constraint directive after a method definition
//
void Method::AddGenericParamConstraint(int index, char * pStrGenericParam, mdToken tkTypeConstraint)
{
if (index > 0)
Expand Down Expand Up @@ -164,7 +166,10 @@ void Method::AddGenericParamConstraint(int index, char * pStrGenericParam, mdTok
return;
}
}
m_pAssembler->CheckAddGenericParamConstraint(&m_GPCList, index, tkTypeConstraint);

// pass true for isParamDirective, we are parsing a .param directive for a method here
//
m_pAssembler->CheckAddGenericParamConstraint(&m_GPCList, index, tkTypeConstraint, true);
}


Expand Down

0 comments on commit d3e5aa8

Please sign in to comment.