Fix handling of IDynamicInterfaceCastable wrt CastCache (#108328)

Fixes #108229. The actual fix is the addition of an `if` check where it originally wasn't. I also fixed the other checks for consistency - positive checks are fine to cache, and negative checks against non-interface targets are also fine to cache.
dotnet · Sep 28, 2024 · ba9b3ba · ba9b3ba
1 parent 7c69e7d
commit ba9b3ba
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 16 deletions.
diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs
@@ -1066,10 +1066,14 @@ private static unsafe bool CacheMiss(MethodTable* pSourceType, MethodTable* pTar
             bool result = TypeCast.AreTypesAssignableInternalUncached(pSourceType, pTargetType, variation, &newList);
 
             //
-            // Update the cache
+            // Update the cache. We only consider type-based conversion rules here.
+            // Therefore a negative result cannot rule out convertibility for IDynamicInterfaceCastable.
             //
-            nuint sourceAndVariation = (nuint)pSourceType + (uint)variation;
-            s_castCache.TrySet(sourceAndVariation, (nuint)pTargetType, result);
+            if (result || !(pTargetType->IsInterface && pSourceType->IsIDynamicInterfaceCastable))
+            {
+                nuint sourceAndVariation = (nuint)pSourceType + (uint)variation;
+                s_castCache.TrySet(sourceAndVariation, (nuint)pTargetType, result);
+            }
 
             return result;
         }
@@ -1252,13 +1256,11 @@ internal static unsafe bool AreTypesAssignableInternalUncached(MethodTable* pSou
             }
 
             //
-            // Update the cache
+            // Update the cache. We only consider type-based conversion rules here.
+            // Therefore a negative result cannot rule out convertibility for IDynamicInterfaceCastable.
             //
-            if (!pSourceType->IsIDynamicInterfaceCastable)
+            if (retObj != null || !(pTargetType->IsInterface && pSourceType->IsIDynamicInterfaceCastable))
             {
-                //
-                // Update the cache
-                //
                 nuint sourceAndVariation = (nuint)pSourceType + (uint)AssignmentVariation.BoxedSource;
                 s_castCache.TrySet(sourceAndVariation, (nuint)pTargetType, retObj != null);
             }
@@ -1293,14 +1295,11 @@ private static unsafe object CheckCastAny_NoCacheLookup(MethodTable* pTargetType
                 obj = CheckCastClass(pTargetType, obj);
             }
 
-            if (!pSourceType->IsIDynamicInterfaceCastable)
-            {
-                //
-                // Update the cache
-                //
-                nuint sourceAndVariation = (nuint)pSourceType + (uint)AssignmentVariation.BoxedSource;
-                s_castCache.TrySet(sourceAndVariation, (nuint)pTargetType, true);
-            }
+            //
+            // Update the cache
+            //
+            nuint sourceAndVariation = (nuint)pSourceType + (uint)AssignmentVariation.BoxedSource;
+            s_castCache.TrySet(sourceAndVariation, (nuint)pTargetType, true);
 
             return obj;
         }

diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs b/src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
@@ -42,6 +42,7 @@ public static int Run()
         TestVariantInterfaceOptimizations.Run();
         TestSharedInterfaceMethods.Run();
         TestGenericAnalysis.Run();
+        TestRuntime108229Regression.Run();
         TestCovariantReturns.Run();
         TestDynamicInterfaceCastable.Run();
         TestStaticInterfaceMethodsAnalysis.Run();
@@ -705,6 +706,27 @@ public static void Run()
         }
     }
 
+    class TestRuntime108229Regression
+    {
+        class Shapeshifter : IDynamicInterfaceCastable
+        {
+            public RuntimeTypeHandle GetInterfaceImplementation(RuntimeTypeHandle interfaceType) => throw new NotImplementedException();
+            public bool IsInterfaceImplemented(RuntimeTypeHandle interfaceType, bool throwIfNotImplemented) => true;
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static bool Is(object o) => o is IEnumerable<object>;
+
+        public static void Run()
+        {
+            object o = new Shapeshifter();
+
+            // Call multiple times in case we just flushed the cast cache (when we flush we don't store).
+            if (!Is(o) || !Is(o) || !Is(o))
+                throw new Exception();
+        }
+    }
+
     class TestCovariantReturns
     {
         interface IFoo