Fix for GitHub issue 76673 and regression test - incorrect static virtual method...

The issue tracks incorrect ordering of static virtual method lookup
methods in the CoreCLR runtime. According to the default interface
implementation ECMA 335 addendum, default interface implementation
resolution should only be used after the "old rules" failed to
resolve the method. In other words resolution on classes must go
first.

In the pre-existing implementation the for loop used to traverse
the chain of base classes was incorrectly combining
resolution on classes with default interface implementation
resolution. The fix is to walk the entire chain of base classes
first and only if that fails, attempt the default interface
implementation resolution.

diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp
index 3837a94..c56d1fb 100644 (file)
--- a/src/coreclr/vm/methodtable.cpp
+++ b/src/coreclr/vm/methodtable.cpp
@@ -8269,23 +8269,24 @@ MethodTable::ResolveVirtualStaticMethod(
                         }
                     }
                 }
+            }
 
-                BOOL haveUniqueDefaultImplementation = pMT->FindDefaultInterfaceImplementation(
-                    pInterfaceMD,
-                    pInterfaceType,
-                    &pMD,
-                    /* allowVariance */ allowVariantMatches,
-                    /* throwOnConflict */ uniqueResolution == nullptr,
-                    level);
-                if (haveUniqueDefaultImplementation || (pMD != nullptr && (verifyImplemented || uniqueResolution != nullptr)))
+            MethodDesc *pMDDefaultImpl = nullptr;
+            BOOL haveUniqueDefaultImplementation = FindDefaultInterfaceImplementation(
+                pInterfaceMD,
+                pInterfaceType,
+                &pMDDefaultImpl,
+                /* allowVariance */ allowVariantMatches,
+                /* throwOnConflict */ uniqueResolution == nullptr,
+                level);
+            if (haveUniqueDefaultImplementation || (pMDDefaultImpl != nullptr && (verifyImplemented || uniqueResolution != nullptr)))
+            {
+                // We tolerate conflicts upon verification of implemented SVMs so that they only blow up when actually called at execution time.
+                if (uniqueResolution != nullptr)
                 {
-                    // We tolerate conflicts upon verification of implemented SVMs so that they only blow up when actually called at execution time.
-                    if (uniqueResolution != nullptr)
-                    {
-                        *uniqueResolution = haveUniqueDefaultImplementation;
-                    }
-                    return pMD;
+                    *uniqueResolution = haveUniqueDefaultImplementation;
                 }
+                return pMDDefaultImpl;
             }
         }
 

diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_76673.cs b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_76673.cs
new file mode 100644 (file)
index 0000000..6e3c922
--- /dev/null
+++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_76673.cs
@@ -0,0 +1,46 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Regression test for a bug found in Roslyn testing; as of authoting
+// this test CoreCLR static virtual method support apparently has a bug
+// causing "Testing Test1" to return "I1.M1" from a call to M1 instead
+// of the expected "Test2.M1".
+
+interface I1
+{
+    static virtual string M1() { return "I1.M1"; }
+    static abstract int M2();
+}
+
+class Test1 : Test2, I1
+{
+    static public int M1() { return 0; }
+    static public ref int M2() { throw null; }
+
+    static int Main()
+    {
+        System.Console.WriteLine("Testing Test2");
+        bool ok2 = Test<Test2>();
+        System.Console.WriteLine("Testing Test1");
+        bool ok1 = Test<Test1>();
+        return ok2 && ok1 ? 100 : 1;
+    }
+
+    static bool Test<T>() where T : I1
+    {
+        string m1 = T.M1();
+        int m2 = T.M2();
+        System.Console.WriteLine("T.M1 returns {0} ('Test2.M1' expected); T.M2 return {1} (2 expected)", m1, m2);
+        return (m1 == "Test2.M1" && m2 == 2);
+    }
+
+}
+
+class Test2 : I1
+{
+    static string I1.M1()
+    {
+        return "Test2.M1";
+    }
+    static int I1.M2() => 2;
+}

diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_76673.csproj b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_76673.csproj
new file mode 100644 (file)
index 0000000..570644f
--- /dev/null
+++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_76673.csproj
@@ -0,0 +1,8 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>