Skip to content
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.

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

Add patch for runtime ilasm generation fix #1899

Merged
merged 1 commit into from
Nov 19, 2020
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
From 874540a2205e227c1ccb8a08b4e50789de7611eb Mon Sep 17 00:00:00 2001
From: Brian Sullivan <briansul@microsoft.com>
Date: Tue, 17 Nov 2020 14:11:55 -0800
Subject: [PATCH] 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.
---
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 9748bf7..d6acf8e 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 constariant
+// 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 60aa014..8f87df6 100644
--- a/src/coreclr/src/ilasm/assembler.h
+++ b/src/coreclr/src/ilasm/assembler.h
@@ -1289,7 +1289,7 @@ public:

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 41df30f..10fd753 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);
}


--
1.8.3.1