From d3e5aa89f4037f8dff7422685766fdfb38d4feb8 Mon Sep 17 00:00:00 2001 From: Brian Sullivan Date: Thu, 19 Nov 2020 10:09:57 -0800 Subject: [PATCH] Fix issue 44646 - ILAsm incorrectly handles method .custom attribute 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 --- src/coreclr/src/ilasm/assembler.cpp | 48 +++++++++++++++++++++++------ src/coreclr/src/ilasm/assembler.h | 2 +- src/coreclr/src/ilasm/method.cpp | 7 ++++- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/coreclr/src/ilasm/assembler.cpp b/src/coreclr/src/ilasm/assembler.cpp index 9748bf70488d5..2e674dbd608fa 100644 --- a/src/coreclr/src/ilasm/assembler.cpp +++ b/src/coreclr/src/ilasm/assembler.cpp @@ -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) @@ -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) @@ -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); @@ -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(); + } } } diff --git a/src/coreclr/src/ilasm/assembler.h b/src/coreclr/src/ilasm/assembler.h index 60aa014940d5a..8f87df66645cc 100644 --- a/src/coreclr/src/ilasm/assembler.h +++ b/src/coreclr/src/ilasm/assembler.h @@ -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); diff --git a/src/coreclr/src/ilasm/method.cpp b/src/coreclr/src/ilasm/method.cpp index 41df30f510f65..10fd7537f3201 100644 --- a/src/coreclr/src/ilasm/method.cpp +++ b/src/coreclr/src/ilasm/method.cpp @@ -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) @@ -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); }