Handle generics in methodimpls for default interface methods (#20404)

The existing logic looking at MethodImpls to find the default interface implementation was not correct in the presence of generics. The MethodImpl records we build during type load only contain inexact MethodDescs for the declMethod that are not suitable for comparisons.

The fix is to pipe through the token and resolve the declaring method from it at the time of dispatch.

11 files changed, 537 insertions, 64 deletions

diff --git a/src/vm/methodimpl.cpp b/src/vm/methodimpl.cpp
index b3f8451e22..9cd452c056 100644
--- a/src/vm/methodimpl.cpp
+++ b/src/vm/methodimpl.cpp
@@ -164,7 +164,8 @@ void MethodImpl::SetSize(LoaderHeap *pHeap, AllocMemTracker *pamTracker, DWORD s
     if(size > 0) {
         // An array of DWORDs, the first entry representing count, and the rest representing slot numbers
         S_SIZE_T cbCountAndSlots = S_SIZE_T(sizeof(DWORD)) +        // DWORD for the total count of slots
-                                    S_SIZE_T(size) * S_SIZE_T(sizeof(DWORD)); // DWORD each for the slot numbers
+                                    S_SIZE_T(size) * S_SIZE_T(sizeof(DWORD)) + // DWORD each for the slot numbers
+                                    S_SIZE_T(size) * S_SIZE_T(sizeof(mdToken)); // Token each for the method tokens
 
         // MethodDesc* for each of the implemented methods
         S_SIZE_T cbMethodDescs = S_SIZE_T(size) * S_SIZE_T(sizeof(RelativePointer<MethodDesc *>));
@@ -190,7 +191,7 @@ void MethodImpl::SetSize(LoaderHeap *pHeap, AllocMemTracker *pamTracker, DWORD s
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////
-void MethodImpl::SetData(DWORD* slots, RelativePointer<MethodDesc*>* md)
+void MethodImpl::SetData(DWORD* slots, mdToken* tokens, RelativePointer<MethodDesc*>* md)
 {
     CONTRACTL {
         NOTHROW;
@@ -202,6 +203,9 @@ void MethodImpl::SetData(DWORD* slots, RelativePointer<MethodDesc*>* md)
     DWORD *pdwSize = pdwSlots.GetValue();
     DWORD dwSize = *pdwSize;
     memcpy(&(pdwSize[1]), slots, dwSize*sizeof(DWORD));
+    
+    // Copy tokens that correspond to the slots above
+    memcpy(&(pdwSize[1 + dwSize]), tokens, dwSize*sizeof(mdToken));
 
     RelativePointer<MethodDesc *> *pImplMD = pImplementedMD.GetValue();
 
@@ -219,7 +223,7 @@ void MethodImpl::Save(DataImage *image)
     DWORD size = GetSize();
     _ASSERTE(size > 0);
 
-    image->StoreStructure(pdwSlots.GetValue(), (size+1)*sizeof(DWORD),
+    image->StoreStructure(pdwSlots.GetValue(), (size+1)*sizeof(DWORD)+size*sizeof(mdToken),
                                     DataImage::ITEM_METHOD_DESC_COLD,
                                     sizeof(DWORD));
     image->StoreStructure(pImplementedMD.GetValue(), size*sizeof(RelativePointer<MethodDesc*>),
@@ -277,7 +281,7 @@ MethodImpl::EnumMemoryRegions(CLRDataEnumMemoryFlags flags)
     {
         ULONG32 numSlots = GetSize();
         DacEnumMemoryRegion(dac_cast<TADDR>(GetSlotsRawNonNull()),
-                            (numSlots + 1) * sizeof(DWORD));
+                            (numSlots + 1) * sizeof(DWORD) + numSlots * sizeof(mdToken));
 
         if (GetImpMDs().IsValid())
         {
diff --git a/src/vm/methodimpl.h b/src/vm/methodimpl.h
index ce915eaf9d..195093ac55 100644
--- a/src/vm/methodimpl.h
+++ b/src/vm/methodimpl.h
@@ -24,7 +24,7 @@ class MethodImpl
     friend class NativeImageDumper;
 #endif
 
-    RelativePointer<PTR_DWORD>            pdwSlots;       // Maintains the slots in sorted order, the first entry is the size
+    RelativePointer<PTR_DWORD>            pdwSlots;       // Maintains the slots and tokens in sorted order, the first entry is the size
     RelativePointer<DPTR( RelativePointer<PTR_MethodDesc> )> pImplementedMD;
 
 public:
@@ -46,6 +46,8 @@ public:
             { WRAPPER_NO_CONTRACT; if (IsValid()) m_iCur++; }
         inline WORD GetSlot()
             { WRAPPER_NO_CONTRACT; CONSISTENCY_CHECK(IsValid()); _ASSERTE(FitsIn<WORD>(m_pImpl->GetSlots()[m_iCur])); return static_cast<WORD>(m_pImpl->GetSlots()[m_iCur]); }
+        inline mdToken GetToken()
+            { WRAPPER_NO_CONTRACT; CONSISTENCY_CHECK(IsValid()); return m_pImpl->GetTokens()[m_iCur]; }
         inline MethodDesc *GetMethodDesc()
             { WRAPPER_NO_CONTRACT; return m_pImpl->GetMethodDesc(m_iCur, (PTR_MethodDesc) m_pMD); }
     };
@@ -109,10 +111,26 @@ public:
 #ifndef DACCESS_COMPILE 
 
     ///////////////////////////////////////////////////////////////////////////////////////
+    inline mdToken* GetTokens()
+    {
+        CONTRACTL{
+            NOTHROW;
+            GC_NOTRIGGER;
+            PRECONDITION(CheckPointer(this));
+            SUPPORTS_DAC;
+        } CONTRACTL_END;
+
+        if (pdwSlots.IsNull())
+            return NULL;
+        else
+            return (mdToken*)(GetSlotsRawNonNull() + 1 + *GetSlotsRawNonNull());
+    }
+
+    ///////////////////////////////////////////////////////////////////////////////////////
     void SetSize(LoaderHeap *pHeap, AllocMemTracker *pamTracker, DWORD size);
 
     ///////////////////////////////////////////////////////////////////////////////////////
-    void SetData(DWORD* slots, RelativePointer<MethodDesc*> * md);
+    void SetData(DWORD* slots, mdToken* tokens, RelativePointer<MethodDesc*> * md);
 
 #endif // !DACCESS_COMPILE
 
diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp
index aa4fc8c003..57f24959fa 100644
--- a/src/vm/methodtable.cpp
+++ b/src/vm/methodtable.cpp
@@ -7138,8 +7138,11 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
                 {
                     if (pCurMT->HasSameTypeDefAs(pInterfaceMT))
                     {
-                        // Generic variance match - we'll instantiate pCurMD with the right type arguments later
-                        pCurMD = pInterfaceMD;
+                        if (!pInterfaceMD->IsAbstract())
+                        {
+                            // Generic variance match - we'll instantiate pCurMD with the right type arguments later
+                            pCurMD = pInterfaceMD;
+                        }
                     }
                     else
                     {
@@ -7148,43 +7151,62 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
                         // Implicit override in default interface methods are not allowed
                         //
                         MethodIterator methodIt(pCurMT);
-                        for (; methodIt.IsValid(); methodIt.Next())
+                        for (; methodIt.IsValid() && pCurMD == NULL; methodIt.Next())
                         {
                             MethodDesc *pMD = methodIt.GetMethodDesc();
                             int targetSlot = pInterfaceMD->GetSlot();
 
-                            if (pMD->IsMethodImpl())
+                            // If this is not a MethodImpl, it can't be implementing the method we're looking for
+                            if (!pMD->IsMethodImpl())
+                                continue;
+                                
+                            // We have a MethodImpl - iterate over all the declarations it's implementing,
+                            // looking for the interface method we need.
+                            MethodImpl::Iterator it(pMD);
+                            for (; it.IsValid() && pCurMD == NULL; it.Next())
                             {
-                                MethodImpl::Iterator it(pMD);
-                                for (; it.IsValid(); it.Next())
-                                {
-                                    MethodDesc *pDeclMD = it.GetMethodDesc();
+                                MethodDesc *pDeclMD = it.GetMethodDesc();
 
-                                    if (pDeclMD->GetSlot() != targetSlot)
-                                        continue;
+                                // Is this the right slot?
+                                if (pDeclMD->GetSlot() != targetSlot)
+                                    continue;
 
-                                    MethodTable *pDeclMT = pDeclMD->GetMethodTable();
-                                    if (pDeclMT->ContainsGenericVariables())
-                                    {
-                                        TypeHandle thInstDeclMT = ClassLoader::LoadGenericInstantiationThrowing(
-                                            pDeclMT->GetModule(),
-                                            pDeclMT->GetCl(),
-                                            pCurMT->GetInstantiation());
-                                        MethodTable *pInstDeclMT = thInstDeclMT.GetMethodTable();
-                                        if (pInstDeclMT == pInterfaceMT)
-                                        {
-                                            // This is a matching override. We'll instantiate pCurMD later
-                                            pCurMD = pMD;
-                                            break;
-                                        }
-                                    }
-                                    else if (pDeclMD == pInterfaceMD)
+                                // Is this the right interface?
+                                if (!pDeclMD->HasSameMethodDefAs(pInterfaceMD))
+                                    continue;
+
+                                if (pInterfaceMD->HasClassInstantiation())
+                                {
+                                    // pInterfaceMD will be in the canonical form, so we need to check the specific
+                                    // instantiation against pInterfaceMT.
+                                    //
+                                    // The parent of pDeclMD is unreliable for this purpose because it may or
+                                    // may not be canonicalized. Let's go from the metadata.
+
+                                    SigTypeContext typeContext = SigTypeContext(pCurMT);
+
+                                    mdTypeRef tkParent;
+                                    IfFailThrow(pMD->GetModule()->GetMDImport()->GetParentToken(it.GetToken(), &tkParent));
+
+                                    MethodTable* pDeclMT = ClassLoader::LoadTypeDefOrRefOrSpecThrowing(
+                                        pMD->GetModule(),
+                                        tkParent,
+                                        &typeContext).AsMethodTable();
+
+                                    // We do CanCastToInterface to also cover variance.
+                                    // We already know this is a method on the same type definition as the (generic)
+                                    // interface but we need to make sure the instantiations match.
+                                    if (pDeclMT->CanCastToInterface(pInterfaceMT))
                                     {
-                                        // Exact match override 
+                                        // We have a match
                                         pCurMD = pMD;
-                                        break;
                                     }
-                                } 
+                                }
+                                else
+                                {
+                                    // No generics involved. If the method definitions match, it's a match.
+                                    pCurMD = pMD;
+                                }
                             }
                         }
                     }
diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp
index 3f54c2f427..b452f3b88a 100644
--- a/src/vm/methodtablebuilder.cpp
+++ b/src/vm/methodtablebuilder.cpp
@@ -5560,7 +5560,7 @@ MethodTableBuilder::ProcessInexactMethodImpls()
                     continue;
 
                 // Otherwise, record the method impl discovery if the match is 
-                bmtMethodImpl->AddMethodImpl(*it, declMethod, GetStackingAllocator());
+                bmtMethodImpl->AddMethodImpl(*it, declMethod, bmtMetaData->rgMethodImplTokens[m].methodDecl, GetStackingAllocator());
             }
 
             if (!fMatchFound && bmtMetaData->rgMethodImplTokens[m].fThrowIfUnmatchedDuringInexactMethodImplProcessing)
@@ -5875,7 +5875,7 @@ MethodTableBuilder::ProcessMethodImpls()
                         BuildMethodTableThrowException(IDS_CLASSLOAD_MI_MUSTBEVIRTUAL, it.Token());
                     }
 
-                    bmtMethodImpl->AddMethodImpl(*it, declMethod, GetStackingAllocator());
+                    bmtMethodImpl->AddMethodImpl(*it, declMethod, mdDecl, GetStackingAllocator());
                 }
             }
         }
@@ -6186,6 +6186,7 @@ MethodTableBuilder::PlaceMethodImpls()
     DWORD dwMaxSlotSize = IsInterface() ? bmtMethod->dwNumberMethodImpls : bmtVT->cVirtualSlots;
 
     DWORD * slots = new (&GetThread()->m_MarshalAlloc) DWORD[dwMaxSlotSize];
+    mdToken * tokens = new (&GetThread()->m_MarshalAlloc) mdToken[dwMaxSlotSize];
     RelativePointer<MethodDesc *> * replaced = new (&GetThread()->m_MarshalAlloc) RelativePointer<MethodDesc*>[dwMaxSlotSize];
 
     DWORD iEntry = 0;
@@ -6200,6 +6201,8 @@ MethodTableBuilder::PlaceMethodImpls()
     while (true)
     {   // collect information until we reach the next body
 
+        tokens[slotIndex] = bmtMethodImpl->GetDeclarationToken(iEntry);
+
         // Get the declaration part of the method impl. It will either be a token
         // (declaration is on this type) or a method desc.
         bmtMethodHandle hDeclMethod = bmtMethodImpl->GetDeclarationMethod(iEntry);
@@ -6281,7 +6284,7 @@ MethodTableBuilder::PlaceMethodImpls()
         if(iEntry == bmtMethodImpl->pIndex)
         {
             // We hit the end of the list so dump the current data and leave
-            WriteMethodImplData(pCurImplMethod, slotIndex, slots, replaced);
+            WriteMethodImplData(pCurImplMethod, slotIndex, slots, tokens, replaced);
             break;
         }
         else
@@ -6291,7 +6294,7 @@ MethodTableBuilder::PlaceMethodImpls()
             if (pNextImplMethod != pCurImplMethod)
             {
                 // If we're moving on to a new body, dump the current data and reset the counter
-                WriteMethodImplData(pCurImplMethod, slotIndex, slots, replaced);
+                WriteMethodImplData(pCurImplMethod, slotIndex, slots, tokens, replaced);
                 slotIndex = 0;
             }
 
@@ -6306,6 +6309,7 @@ MethodTableBuilder::WriteMethodImplData(
     bmtMDMethod * pImplMethod, 
     DWORD         cSlots, 
     DWORD *       rgSlots, 
+    mdToken *     rgTokens,
     RelativePointer<MethodDesc *> * rgDeclMD)
 {
     STANDARD_VM_CONTRACT;
@@ -6355,12 +6359,16 @@ MethodTableBuilder::WriteMethodImplData(
                     DWORD sTmp = rgSlots[i];
                     rgSlots[i] = rgSlots[min];
                     rgSlots[min] = sTmp;
+
+                    mdToken tTmp = rgTokens[i];
+                    rgTokens[i] = rgTokens[min];
+                    rgTokens[min] = tTmp;
                 }
             }
         }
 
         // Go and set the method impl
-        pImpl->SetData(rgSlots, rgDeclMD);
+        pImpl->SetData(rgSlots, rgTokens, rgDeclMD);
 
         GetHalfBakedClass()->SetContainsMethodImpls();
     }
@@ -10751,39 +10759,81 @@ MethodTableBuilder::SetupMethodTable2(
 
 // Returns true if there is at least one default implementation for this interface method
 // We don't care about conflicts at this stage in order to avoid impact type load performance
-BOOL MethodTableBuilder::HasDefaultInterfaceImplementation(MethodDesc *pDeclMD)
+BOOL MethodTableBuilder::HasDefaultInterfaceImplementation(bmtRTType *pDeclType, MethodDesc *pDeclMD)
 {
     STANDARD_VM_CONTRACT;
 
 #ifdef FEATURE_DEFAULT_INTERFACES
     // If the interface method is already non-abstract, we are done
-    if (pDeclMD->IsDefaultInterfaceMethod())
+    if (!pDeclMD->IsAbstract())
         return TRUE;
 
-    MethodTable *pDeclMT = pDeclMD->GetMethodTable();
+    int targetSlot = pDeclMD->GetSlot();
 
-    // Otherwise, traverse the list of interfaces and see if there is at least one override 
-    bmtInterfaceInfo::MapIterator intIt = bmtInterface->IterateInterfaceMap();
-    for (; !intIt.AtEnd(); intIt.Next())
+    // Iterate over all the interfaces this type implements
+    bmtInterfaceEntry * pItfEntry = NULL;
+    for (DWORD i = 0; i < bmtInterface->dwInterfaceMapSize; i++)
     {
-        MethodTable *pIntfMT = intIt->GetInterfaceType()->GetMethodTable();
-        if (pIntfMT->GetClass()->ContainsMethodImpls() && pIntfMT->CanCastToInterface(pDeclMT))
+        bmtRTType * pCurItf = bmtInterface->pInterfaceMap[i].GetInterfaceType();
+
+        // Go over the methods on the interface
+        MethodTable::IntroducedMethodIterator methIt(pCurItf->GetMethodTable());
+        for (; methIt.IsValid(); methIt.Next())
         {
-            MethodTable::MethodIterator methodIt(pIntfMT);
-            for (; methodIt.IsValid(); methodIt.Next())
+            MethodDesc * pPotentialImpl = methIt.GetMethodDesc();
+
+            // If this interface method is not a MethodImpl, it can't possibly implement
+            // the interface method we are looking for
+            if (!pPotentialImpl->IsMethodImpl())
+                continue;
+
+            // Go over all the decls this MethodImpl is implementing
+            MethodImpl::Iterator it(pPotentialImpl);
+            for (; it.IsValid(); it.Next())
             {
-                MethodDesc *pMD = methodIt.GetMethodDesc();
-                if (pMD->IsMethodImpl())
+                MethodDesc *pPotentialDecl = it.GetMethodDesc();
+
+                // Check this is a decl with the right slot
+                if (pPotentialDecl->GetSlot() != targetSlot)
+                    continue;
+
+                // Find out what interface this default implementation is implementing
+                mdToken tkParent;
+                IfFailThrow(GetModule()->GetMDImport()->GetParentToken(it.GetToken(), &tkParent));
+
+                // We can only load the approximate interface at this point
+                MethodTable * pPotentialInterfaceMT = ClassLoader::LoadTypeDefOrRefOrSpecThrowing(
+                    GetModule(),
+                    tkParent,
+                    &bmtGenerics->typeContext,
+                    ClassLoader::ThrowIfNotFound,
+                    ClassLoader::PermitUninstDefOrRef,
+                    ClassLoader::LoadTypes,
+                    CLASS_LOAD_APPROXPARENTS,
+                    TRUE).GetMethodTable()->GetCanonicalMethodTable();
+
+                // Is this a default implementation for the interface we are looking for?
+                if (pDeclType->GetMethodTable()->HasSameTypeDefAs(pPotentialInterfaceMT))
                 {
-                    MethodImpl::Iterator it(pMD);
-                    for (; it.IsValid(); it.Next())
+                    // If the type is not generic, matching defs are all we need
+                    if (!pDeclType->GetMethodTable()->HasInstantiation())
+                        return TRUE;
+
+                    // If this is generic, we need to compare under substitutions
+                    Substitution curItfSubs(tkParent, GetModule(), &pCurItf->GetSubstitution());
+
+                    // Type Equivalence is not respected for this comparision as you can have multiple type equivalent interfaces on a class
+                    TokenPairList newVisited = TokenPairList::AdjustForTypeEquivalenceForbiddenScope(NULL);
+                    if (MetaSig::CompareTypeDefsUnderSubstitutions(
+                        pPotentialInterfaceMT, pDeclType->GetMethodTable(),
+                        &curItfSubs, &pDeclType->GetSubstitution(),
+                        &newVisited))
                     {
-                        if (it.GetMethodDesc() == pDeclMD)
-                            return TRUE;
+                        return TRUE;
                     }
                 }
             }
-        }
+        }        
     }
 #endif // FEATURE_DEFAULT_INTERFACES
 
@@ -10866,7 +10916,7 @@ void MethodTableBuilder::VerifyVirtualMethodsImplemented(MethodTable::MethodData
                 {
                     MethodDesc *pMD = it.GetDeclMethodDesc();
 
-                    if (!HasDefaultInterfaceImplementation(pMD))
+                    if (!HasDefaultInterfaceImplementation(intIt->GetInterfaceType(), pMD))
                         BuildMethodTableThrowException(IDS_CLASSLOAD_NOTIMPLEMENTED, pMD->GetNameOnNonArrayClass());
                 }
             }
@@ -11508,7 +11558,7 @@ void MethodTableBuilder::GetCoClassAttribInfo()
 
 //*******************************************************************************
 void MethodTableBuilder::bmtMethodImplInfo::AddMethodImpl(
-    bmtMDMethod * pImplMethod, bmtMethodHandle declMethod,
+    bmtMDMethod * pImplMethod, bmtMethodHandle declMethod, mdToken declToken,
     StackingAllocator * pStackingAllocator)
 {
     STANDARD_VM_CONTRACT;
@@ -11534,7 +11584,7 @@ void MethodTableBuilder::bmtMethodImplInfo::AddMethodImpl(
         rgEntries = rgEntriesNew;
         cMaxIndex = newEntriesCount;
     }
-    rgEntries[pIndex++] = Entry(pImplMethod, declMethod);
+    rgEntries[pIndex++] = Entry(pImplMethod, declMethod, declToken);
 }
 
 //*******************************************************************************
diff --git a/src/vm/methodtablebuilder.h b/src/vm/methodtablebuilder.h
index 1a21b8e3bc..e7ef13b484 100644
--- a/src/vm/methodtablebuilder.h
+++ b/src/vm/methodtablebuilder.h
@@ -2086,16 +2086,20 @@ private:
         {
             bmtMethodHandle declMethod;
             bmtMDMethod *   pImplMethod;
+            mdToken         declToken;
 
             Entry(bmtMDMethod *   pImplMethodIn,
-                  bmtMethodHandle declMethodIn)
+                  bmtMethodHandle declMethodIn,
+                  mdToken declToken)
               : declMethod(declMethodIn),
-                pImplMethod(pImplMethodIn)
+                pImplMethod(pImplMethodIn),
+                declToken(declToken)
               {}
 
             Entry()
               : declMethod(),
-                pImplMethod(NULL)
+                pImplMethod(NULL),
+                declToken()
               {}
         };
 
@@ -2129,6 +2133,7 @@ private:
         AddMethodImpl(
             bmtMDMethod * pImplMethod,
             bmtMethodHandle declMethod,
+            mdToken declToken,
             StackingAllocator * pStackingAllocator);
 
         //-----------------------------------------------------------------------------------------
@@ -2139,6 +2144,13 @@ private:
             { LIMITED_METHOD_CONTRACT; _ASSERTE(i < pIndex); return rgEntries[i].declMethod; }
 
         //-----------------------------------------------------------------------------------------
+        // Get the decl method for a particular methodimpl entry.
+        mdToken
+        GetDeclarationToken(
+            DWORD i)
+            { LIMITED_METHOD_CONTRACT; _ASSERTE(i < pIndex); return rgEntries[i].declToken; }
+
+        //-----------------------------------------------------------------------------------------
         // Get the impl method for a particular methodimpl entry.
         bmtMDMethod *
         GetImplementationMethod(
@@ -2720,6 +2732,7 @@ private:
         bmtMDMethod *       pImplMethod,
         DWORD               cSlots,
         DWORD *             rgSlots,
+        mdToken *           rgTokens,
         RelativePointer<MethodDesc *> *       rgDeclMD);
 
     // --------------------------------------------------------------------------------------------
@@ -2837,7 +2850,7 @@ private:
     VOID HandleGCForValueClasses(
         MethodTable **);
 
-    BOOL HasDefaultInterfaceImplementation(MethodDesc *pIntfMD);
+    BOOL HasDefaultInterfaceImplementation(bmtRTType *pIntfType, MethodDesc *pIntfMD);
     VOID VerifyVirtualMethodsImplemented(MethodTable::MethodData * hMTData);
 
     VOID CheckForTypeEquivalence(
diff --git a/tests/src/Regressions/coreclr/16354/notimplemented.il b/tests/src/Regressions/coreclr/16354/notimplemented.il
new file mode 100644
index 0000000000..2459e8c056
--- /dev/null
+++ b/tests/src/Regressions/coreclr/16354/notimplemented.il
@@ -0,0 +1,72 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+// Makes sure that a type that implements IFoo<string> through a default
+// interface method and declares it also implements IFoo<ValueType>
+// (but it actually doesn't) will not be loadable.
+
+.assembly extern System.Runtime { }
+
+.assembly notimplemented { }
+
+.class interface private abstract auto ansi IFoo`1<T>
+{
+  .method public hidebysig newslot abstract virtual instance void Frob() cil managed
+  {
+  }
+}
+
+.class interface private abstract auto ansi IBar
+       implements class IFoo`1<class [System.Runtime]System.String>
+{
+  .method public hidebysig newslot virtual final instance void Frob() cil managed
+  {
+    .override class IFoo`1<class [System.Runtime]System.String>::Frob
+    ret
+  }
+}
+
+.class private auto ansi beforefieldinit Fooer
+       extends [System.Runtime]System.Object
+       implements IBar, class IFoo`1<class [System.Runtime]System.ValueType>
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [System.Runtime]System.Object::.ctor()
+    ret
+  }
+}
+
+.method private hidebysig static void LoadFooer() cil managed noinlining
+{
+  newobj instance void Fooer::.ctor()
+  pop
+  ret
+}
+
+.method public hidebysig static int32 Main() cil managed
+{
+  .entrypoint
+
+  .try
+  {
+    call void LoadFooer()
+    leave DidNotThrow
+  }
+  catch [System.Runtime]System.TypeLoadException
+  {
+    pop
+    leave Okay
+  }
+
+Okay:
+  ldc.i4 100
+  ret
+
+DidNotThrow:
+  ldc.i4.m1
+  ret
+}
diff --git a/tests/src/Regressions/coreclr/16354/notimplemented.ilproj b/tests/src/Regressions/coreclr/16354/notimplemented.ilproj
new file mode 100644
index 0000000000..79d5309397
--- /dev/null
+++ b/tests/src/Regressions/coreclr/16354/notimplemented.ilproj
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{85DFC527-4DB1-595E-A7D7-E94EE1F8140D}</ProjectGuid>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+
+  <ItemGroup>
+    <Compile Include="notimplemented.il" />
+  </ItemGroup>
+
+
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>
diff --git a/tests/src/Regressions/coreclr/16355/boring.il b/tests/src/Regressions/coreclr/16355/boring.il
new file mode 100644
index 0000000000..0e950f03d0
--- /dev/null
+++ b/tests/src/Regressions/coreclr/16355/boring.il
@@ -0,0 +1,130 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+.assembly extern System.Runtime { }
+
+.assembly boring { }
+
+.class interface private abstract auto ansi IFoo`1<T>
+{
+  .method public hidebysig newslot abstract virtual instance int32 Frob() cil managed
+  {
+  }
+
+  .method public hidebysig newslot virtual instance int32 Bark() cil managed
+  {
+    ldc.i4.m1
+    ret
+  }
+}
+
+.class interface private abstract auto ansi IBar
+       implements class IFoo`1<class [System.Runtime]System.String>,
+                  class IFoo`1<class [System.Runtime]System.Object>,
+                  class IFoo`1<class [System.Runtime]System.ValueType>
+{
+  .method public hidebysig newslot virtual final instance int32 FrobImpl1() cil managed
+  {
+    .override class IFoo`1<class [System.Runtime]System.String>::Frob
+    .override class IFoo`1<class [System.Runtime]System.Object>::Frob
+    ldc.i4.1
+    ret
+  }
+
+  .method public hidebysig newslot virtual final instance int32 FrobImpl2() cil managed
+  {
+    .override class IFoo`1<class [System.Runtime]System.ValueType>::Frob
+    ldc.i4.2
+    ret
+  }
+
+  .method public hidebysig newslot virtual final instance int32 BarkImpl1() cil managed
+  {
+    .override class IFoo`1<class [System.Runtime]System.String>::Bark
+    ldc.i4.3
+    ret
+  }
+
+  .method public hidebysig newslot virtual final instance int32 BarkImpl2() cil managed
+  {
+    .override class IFoo`1<class [System.Runtime]System.ValueType>::Bark
+    ldc.i4.4
+    ret
+  }
+}
+
+.class private auto ansi beforefieldinit Fooer
+       extends [System.Runtime]System.Object
+       implements IBar
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [System.Runtime]System.Object::.ctor()
+    ret
+  }
+}
+
+.method public hidebysig static int32 Main() cil managed
+{
+  .entrypoint
+
+  newobj instance void class Fooer::.ctor()
+  callvirt instance int32 class IFoo`1<string>::Frob()
+  ldc.i4.1
+  ceq
+  brtrue IFoo_String_Frob_Okay
+  ldc.i4.1
+  ret
+IFoo_String_Frob_Okay:
+
+  newobj instance void class Fooer::.ctor()
+  callvirt instance int32 class IFoo`1<object>::Frob()
+  ldc.i4.1
+  ceq
+  brtrue IFoo_Object_Frob_Okay
+  ldc.i4.2
+  ret
+IFoo_Object_Frob_Okay:
+
+  newobj instance void class Fooer::.ctor()
+  callvirt instance int32 class IFoo`1<class [System.Runtime]System.ValueType>::Frob()
+  ldc.i4.2
+  ceq
+  brtrue IFoo_ValueType_Frob_Okay
+  ldc.i4.3
+  ret
+IFoo_ValueType_Frob_Okay:
+
+  newobj instance void class Fooer::.ctor()
+  callvirt instance int32 class IFoo`1<string>::Bark()
+  ldc.i4.3
+  ceq
+  brtrue IFoo_String_Bark_Okay
+  ldc.i4.4
+  ret
+IFoo_String_Bark_Okay:
+
+  newobj instance void class Fooer::.ctor()
+  callvirt instance int32 class IFoo`1<object>::Bark()
+  ldc.i4.m1
+  ceq
+  brtrue IFoo_Object_Bark_Okay
+  ldc.i4.5
+  ret
+IFoo_Object_Bark_Okay:
+
+  newobj instance void class Fooer::.ctor()
+  callvirt instance int32 class IFoo`1<class [System.Runtime]System.ValueType>::Bark()
+  ldc.i4.4
+  ceq
+  brtrue IFoo_ValueType_Bark_Okay
+  ldc.i4.6
+  ret
+IFoo_ValueType_Bark_Okay:
+
+  ldc.i4 100
+  ret
+}
diff --git a/tests/src/Regressions/coreclr/16355/boring.ilproj b/tests/src/Regressions/coreclr/16355/boring.ilproj
new file mode 100644
index 0000000000..6bf3f70e6f
--- /dev/null
+++ b/tests/src/Regressions/coreclr/16355/boring.ilproj
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{85DFC527-4DB1-595E-A7D7-E94EE1F8140D}</ProjectGuid>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+
+  <ItemGroup>
+    <Compile Include="boring.il" />
+  </ItemGroup>
+
+
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>
diff --git a/tests/src/Regressions/coreclr/16355/variance.il b/tests/src/Regressions/coreclr/16355/variance.il
new file mode 100644
index 0000000000..068933db43
--- /dev/null
+++ b/tests/src/Regressions/coreclr/16355/variance.il
@@ -0,0 +1,59 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+.assembly extern System.Runtime { }
+
+.assembly variance { }
+
+.class interface private abstract auto ansi IContravariant`1<- T>
+{
+  .method public hidebysig newslot abstract virtual instance valuetype [System.Runtime]System.RuntimeTypeHandle Frob(!T t) cil managed
+  {
+  }
+}
+
+.class interface private abstract auto ansi IBar`1<- T>
+       implements class IContravariant`1<!T>
+{
+  .method public hidebysig newslot virtual final instance valuetype [System.Runtime]System.RuntimeTypeHandle Frob(!T t) cil managed
+  {
+    .override class IContravariant`1<!T>::Frob
+    ldtoken !T
+    ret
+  }
+}
+
+
+.class private auto ansi beforefieldinit Fooer
+       extends [System.Runtime]System.Object
+       implements class IBar`1<class [System.Runtime]System.Object>
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [System.Runtime]System.Object::.ctor()
+    ret
+  }
+}
+
+.method public hidebysig static int32 Main() cil managed
+{
+  .entrypoint
+  .locals (valuetype [System.Runtime]System.RuntimeTypeHandle)
+  newobj instance void class Fooer::.ctor()
+  ldnull
+  callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle class IContravariant`1<string>::Frob(!0)
+  stloc.0
+  ldloca 0
+  ldtoken object
+  call instance bool valuetype [System.Runtime]System.RuntimeTypeHandle::Equals(valuetype [System.Runtime]System.RuntimeTypeHandle)
+  brtrue Okay
+  ldc.i4.m1
+  ret
+
+Okay:
+  ldc.i4 100
+  ret
+}
diff --git a/tests/src/Regressions/coreclr/16355/variance.ilproj b/tests/src/Regressions/coreclr/16355/variance.ilproj
new file mode 100644
index 0000000000..60513c7004
--- /dev/null
+++ b/tests/src/Regressions/coreclr/16355/variance.ilproj
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{85DFC527-4DB1-595E-A7D7-E94EE1F8140D}</ProjectGuid>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+
+  <ItemGroup>
+    <Compile Include="variance.il" />
+  </ItemGroup>
+
+
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>