Fix logic in compareTypesForCast (#31681)

We weren't careful enough with `__Canon` in some cases, which lead to unsafely
returning `MustNot` when the cast outcome was not determined at jit time.

Add an extra check, update comments, and add some test cases.

Addresses the failures seen in #1195 (which was reverted).

diff --git a/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
index 8554987..63f817f 100644 (file)
--- a/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
+++ b/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
@@ -1758,16 +1758,29 @@ namespace Internal.JitInterface
                     {
                         result = TypeCompareState.Must;
                     }
-                    // For negative results, the unknown type parameter in
-                    // fromClass might match some instantiated interface,
-                    // either directly or via variance.
+                    // We have __Canon parameter(s) in fromClass, somewhere.
                     //
-                    // However, CanCastTo will report failure in such cases since
-                    // __Canon won't match the instantiated type on the
-                    // interface (which can't be __Canon since we screened out
-                    // canonical subtypes for toClass above). So only report
-                    // failure if the interface is not instantiated.
-                    else if (!toType.HasInstantiation)
+                    // In CanCastTo, these __Canon(s) won't match the interface or
+                    // instantiated types on the interface, so CanCastTo may
+                    // return false negatives.
+                    //
+                    // Only report MustNot if the fromClass is not __Canon
+                    // and the interface is not instantiated; then there is
+                    // no way for the fromClass __Canon(s) to confuse things.
+                    //
+                    //    __Canon       -> IBar             May
+                    //    IFoo<__Canon> -> IFoo<string>     May
+                    //    IFoo<__Canon> -> IBar             MustNot
+                    //
+                    else if (fromType.IsCanonicalDefinitionType(CanonicalFormKind.Any))
+                    {
+                        result = TypeCompareState.May;
+                    }
+                    else if (toType.HasInstantiation)
+                    {
+                        result = TypeCompareState.May;
+                    }
+                    else
                     {
                         result = TypeCompareState.MustNot;
                     }

diff --git a/src/coreclr/src/vm/jitinterface.cpp b/src/coreclr/src/vm/jitinterface.cpp
index 65417d1..923fe69 100644 (file)
--- a/src/coreclr/src/vm/jitinterface.cpp
+++ b/src/coreclr/src/vm/jitinterface.cpp
@@ -4495,16 +4495,29 @@ TypeCompareState CEEInfo::compareTypesForCast(
             {
                 result = TypeCompareState::Must;
             }
-            // For negative results, the unknown type parameter in
-            // fromClass might match some instantiated interface,
-            // either directly or via variance.
+            // We have __Canon parameter(s) in fromClass, somewhere.
             //
-            // However, CanCastTo will report failure in such cases since
-            // __Canon won't match the instantiated type on the
-            // interface (which can't be __Canon since we screened out
-            // canonical subtypes for toClass above). So only report
-            // failure if the interface is not instantiated.
-            else if (!toHnd.HasInstantiation())
+            // In CanCastTo, these __Canon(s) won't match the interface or
+            // instantiated types on the interface, so CanCastTo may
+            // return false negatives.
+            //
+            // Only report MustNot if the fromClass is not __Canon
+            // and the interface is not instantiated; then there is
+            // no way for the fromClass __Canon(s) to confuse things.
+            //
+            //    __Canon       -> IBar             May
+            //    IFoo<__Canon> -> IFoo<string>     May
+            //    IFoo<__Canon> -> IBar             MustNot
+            //
+            else if (fromHnd == TypeHandle(g_pCanonMethodTableClass))
+            {
+                result = TypeCompareState::May;
+            }
+            else if (toHnd.HasInstantiation())
+            {
+                result = TypeCompareState::May;
+            }
+            else
             {
                 result = TypeCompareState::MustNot;
             }

diff --git a/src/coreclr/tests/src/JIT/opt/Casts/shared2.cs b/src/coreclr/tests/src/JIT/opt/Casts/shared2.cs
new file mode 100644 (file)
index 0000000..1c155ec
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Casts/shared2.cs
@@ -0,0 +1,122 @@
+// 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.
+
+using System;
+using System.Runtime.CompilerServices;
+
+interface IBar {}
+interface IFoo<T> {}
+class C : IBar {}
+class C<T> : IFoo<T> {}
+struct S {}
+struct SBar : IBar {}
+
+// More tests for shared types passing through compareTypesForCast
+
+class X
+{
+    static int _errors;
+
+    static void IsTrue(bool expression, [CallerLineNumber] int line = 0, [CallerFilePath] string file = "")
+    {
+        if (!expression)
+        {
+            Console.WriteLine($"{file}:L{line} test failed (expected: true).");
+            _errors++;
+        }
+    }
+
+    static void IsFalse(bool expression, [CallerLineNumber] int line = 0, [CallerFilePath] string file = "")
+    {
+        if (expression)
+        {
+            Console.WriteLine($"{file}:L{line} test failed (expected: false).");
+            _errors++;
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool A1<T>()
+    {
+        return typeof(IFoo<string>).IsAssignableFrom(typeof(T));
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool A2<T>()
+    {
+        return typeof(IBar).IsAssignableFrom(typeof(T));
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool I1<T>(T t)
+    {
+        return t is IFoo<string>;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool I2<T>(T t)
+    {
+        return t is IBar;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool C1<T>(T t)
+    {
+        return (t as IFoo<string>) != null;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static bool C2<T>(T t)
+    {
+        return (t as IBar) != null;
+    }
+
+    public static int Main()
+    {
+        var c = new C();
+        var ci = new C<int>();
+        var cs = new C<string>();
+        var s = new S();
+        var sb = new SBar();
+
+        IsTrue(A1<IFoo<string>>());
+        IsFalse(A1<IFoo<int>>());
+        IsFalse(A1<IBar>());
+        IsFalse(A1<S>());
+        IsFalse(A1<SBar>());
+
+        IsTrue(I1(cs));
+        IsFalse(I1(ci));
+        IsFalse(I1(c));
+        IsFalse(I1(s));
+        IsFalse(I1(sb));
+
+        IsTrue(C1(cs));
+        IsFalse(C1(ci));
+        IsFalse(C1(c));
+        IsFalse(C1(s));
+        IsFalse(C1(sb));
+
+        IsFalse(A2<IFoo<string>>());
+        IsFalse(A2<IFoo<int>>());
+        IsTrue(A2<IBar>());
+        IsFalse(A2<S>());
+        IsTrue(A2<SBar>());
+
+        IsFalse(I2(cs));
+        IsFalse(I2(ci));
+        IsTrue(I2(c));
+        IsFalse(I2(s));
+        IsTrue(I2(sb));
+
+        IsFalse(C2(cs));
+        IsFalse(C2(ci));
+        IsTrue(C2(c));
+        IsFalse(C2(s));
+        IsTrue(C2(sb));
+
+        if (_errors == 0) Console.WriteLine("Passed");
+        return _errors > 0 ? -1 : 100;
+    }
+}

diff --git a/src/coreclr/tests/src/JIT/opt/Casts/shared2.csproj b/src/coreclr/tests/src/JIT/opt/Casts/shared2.csproj
new file mode 100644 (file)
index 0000000..97176f9
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Casts/shared2.csproj
@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="shared2.cs" />
+  </ItemGroup>
+</Project>