From 55ed5cda016f1dd3d730e81165c40d45327e1ba4 Mon Sep 17 00:00:00 2001
From: David Wrighton <davidwr@microsoft.com>
Date: Fri, 7 Aug 2020 13:26:55 -0700
Subject: [PATCH] Fix covariant override disassembly (#40502)

- This introduces a new path where ildasm must generate a full token description instead of relying on ilasm to produce the signature from the method body
---
 src/coreclr/src/ildasm/dasm.cpp                    | 104 +++++++++++++++++++--
 .../UnitTest/CompatibleWithTest.ilproj             |   2 -
 2 files changed, 95 insertions(+), 11 deletions(-)

diff --git a/src/coreclr/src/ildasm/dasm.cpp b/src/coreclr/src/ildasm/dasm.cpp
index da3d12f..eecd1ed 100644
--- a/src/coreclr/src/ildasm/dasm.cpp
+++ b/src/coreclr/src/ildasm/dasm.cpp
@@ -3300,7 +3300,9 @@ void DumpGenericParsCA(mdToken tok, void* GUICookie/*=NULL*/)
     } //end if(g_fShowCA)
 }
 
-// Sets *pbOverridingTypeSpec to TRUE if we are overriding a method declared by a type spec.
+// Sets *pbOverridingTypeSpec to TRUE if we are overriding a method declared by a type spec or
+// if the method has a signature which does not exactly match between the overrider and overridee.
+// That case is commonly caused by covariant overrides.
 // In that case the syntax is slightly different (there are additional 'method' keywords).
 // Refer to Expert .NET 2.0 IL Assembler page 242.
 void PrettyPrintOverrideDecl(ULONG i, __inout __nullterminated char* szString, void* GUICookie, mdToken tkOverrider,
@@ -3320,6 +3322,11 @@ void PrettyPrintOverrideDecl(ULONG i, __inout __nullterminated char* szString, v
 
     if(g_pImport->IsValidToken(tkDecl))
     {
+        bool needsFullTokenPrint = false;
+        bool hasTkDeclParent = false;
+
+        // Determine if the decl is a typespec method, in which case the "method" syntax + full token print
+        // must be used to generate the disassembly.
         if(SUCCEEDED(g_pImport->GetParentToken(tkDecl,&tkDeclParent)))
         {
             if(g_pImport->IsValidToken(tkDeclParent))
@@ -3334,20 +3341,99 @@ void PrettyPrintOverrideDecl(ULONG i, __inout __nullterminated char* szString, v
                 {
                     if(TypeFromToken(tkDeclParent)==mdtTypeSpec)
                     {
-                        szptr += sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr), " %s ",KEYWORD("method"));
-                        PrettyPrintToken(szString,tkDecl,g_pImport,GUICookie,tkOverrider);
-
-                        *pbOverridingTypeSpec = TRUE;
-                        return;
+                        needsFullTokenPrint = true;
                     }
-                    PrettyPrintToken(szString, tkDeclParent, g_pImport,GUICookie,tkOverrider);
-                    strcat_s(szString, SZSTRING_SIZE,"::");
-                    szptr = &szString[strlen(szString)];
+                    hasTkDeclParent = true;
                 }
             }
             else
                 szptr+=sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr),"%s",ERRORMSG("INVALID OVERRIDDEN METHOD'S PARENT TOKEN"));
         }
+
+        // Determine if the sig of the decl does not match the sig of the body
+        // In that case the full "method" syntax must be used
+        if ((TypeFromToken(tkOverrider) == mdtMethodDef) && !needsFullTokenPrint)
+        {
+            PCCOR_SIGNATURE pComSigDecl;
+            ULONG cComSigDecl;
+            mdToken tkDeclSigTok = tkDecl;
+            bool successfullyGotDeclSig = false;
+            bool successfullyGotBodySig = false;
+
+            if (TypeFromToken(tkDeclSigTok) == mdtMethodSpec)
+            {
+                mdToken         meth=0;
+                if (SUCCEEDED(g_pImport->GetMethodSpecProps(tkDeclSigTok, &meth, NULL, NULL)))
+                {
+                    tkDeclSigTok = meth;
+                }
+            }
+
+            if (TypeFromToken(tkDeclSigTok) == mdtMethodDef)
+            {
+                if (SUCCEEDED(g_pImport->GetSigOfMethodDef(tkDeclSigTok, &cComSigDecl, &pComSigDecl)))
+                {
+                    successfullyGotDeclSig = true;
+                }
+            }
+            else if (TypeFromToken(tkDeclSigTok) == mdtMemberRef)
+            {
+                const char *pszMemberNameUnused;
+                if (SUCCEEDED(g_pImport->GetNameAndSigOfMemberRef(
+                    tkDeclSigTok,
+                    &pComSigDecl,
+                    &cComSigDecl,
+                    &pszMemberNameUnused)))
+                {
+                    successfullyGotDeclSig = true;
+                }
+            }
+
+            PCCOR_SIGNATURE pComSigBody;
+            ULONG cComSigBody;
+            if (SUCCEEDED(g_pImport->GetSigOfMethodDef(tkOverrider, &cComSigBody, &pComSigBody)))
+            {
+                successfullyGotBodySig = true;
+            }
+
+            if (successfullyGotDeclSig && successfullyGotBodySig)
+            {
+                if (cComSigBody != cComSigDecl)
+                {
+                    needsFullTokenPrint = true;
+                }
+                else if (memcmp(pComSigBody, pComSigDecl, cComSigBody) != 0)
+                {
+                    needsFullTokenPrint = true;
+                }
+
+                // Signature are binary identical, full sig printing not needed
+            }
+            else
+            {
+                szptr+=sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr),"%s",ERRORMSG("INVALID BODY OR DECL SIG"));
+            }
+        }
+
+        if (needsFullTokenPrint)
+        {
+            // In this case, the shortcut syntax cannot be used, and a full token must be printed.
+            // Print the full token and return.
+            szptr += sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr), " %s ",KEYWORD("method"));
+            PrettyPrintToken(szString,tkDecl,g_pImport,GUICookie,tkOverrider);
+
+            *pbOverridingTypeSpec = TRUE;
+            return;
+        }
+
+        if (hasTkDeclParent)
+        {
+            // If the tkDeclParent was successfully retrieved during parent discovery print it here.
+            PrettyPrintToken(szString, tkDeclParent, g_pImport,GUICookie,tkOverrider);
+            strcat_s(szString, SZSTRING_SIZE,"::");
+            szptr = &szString[strlen(szString)];
+        }
+
         if(TypeFromToken(tkDecl) == mdtMethodSpec)
         {
             mdToken         meth=0;
diff --git a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.ilproj b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.ilproj
index 78d81b0..2a6c419 100644
--- a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.ilproj
+++ b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.ilproj
@@ -3,8 +3,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Disable for ilasm round-trip test failure: https://github.com/dotnet/runtime/issues/38508 -->
-    <IlasmRoundTripIncompatible>true</IlasmRoundTripIncompatible>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="CompatibleWithTest.il" />
-- 
2.7.4