From 8c58fc2347820ce48e09605d8adddb993df9ebb5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?=
 <MichalStrehovsky@users.noreply.github.com>
Date: Wed, 28 Dec 2022 16:19:51 +0900
Subject: [PATCH] Fix generic dataflow for generic virtual methods (#80009)

Generic virtual method analysis happens on top of canonical forms, so we would miss the fact that `IFoo.SomeMethod<SomeType>` was called and only see `IFoo.SomeMethod<__Canon>`.
---
 .../RuntimeMethodHandleNode.cs                |  5 ++++
 .../Compiler/UsageBasedMetadataManager.cs     |  4 +--
 .../SmokeTests/TrimmingBehaviors/Dataflow.cs  | 26 +++++++++++++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/RuntimeMethodHandleNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/RuntimeMethodHandleNode.cs
index bda295744e7db..c01ba9ed2dccf 100644
--- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/RuntimeMethodHandleNode.cs
+++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/RuntimeMethodHandleNode.cs
@@ -53,6 +53,11 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
             {
                 dependencies ??= new DependencyList();
                 dependencies.Add(factory.GVMDependencies(_targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "GVM dependencies for runtime method handle");
+
+                // GVM analysis happens on canonical forms, but this is potentially injecting new genericness
+                // into the system. Ensure reflection analysis can still see this.
+                if (_targetMethod.IsAbstract)
+                    factory.MetadataManager.GetDependenciesDueToMethodCodePresence(ref dependencies, factory, _targetMethod, methodIL: null);
             }
 
             factory.MetadataManager.GetDependenciesDueToLdToken(ref dependencies, factory, _targetMethod);
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
index 55c425ea3017b..fad631ad5f7b7 100644
--- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
+++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
@@ -567,9 +567,9 @@ protected override void GetDependenciesDueToMethodCodePresenceInternal(ref Depen
 
             Debug.Assert(methodIL != null || method.IsAbstract || method.IsPInvoke || method.IsInternalCall);
 
-            if (methodIL != null && scanReflection)
+            if (scanReflection)
             {
-                if (FlowAnnotations.RequiresDataflowAnalysis(method))
+                if (methodIL != null && FlowAnnotations.RequiresDataflowAnalysis(method))
                 {
                     AddDataflowDependency(ref dependencies, factory, methodIL, "Method has annotated parameters");
                 }
diff --git a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs
index 344a29740877c..8e7f289db270f 100644
--- a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs
+++ b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs
@@ -243,6 +243,13 @@ public static void AlsoKeptMethod() { }
             private static void RemovedMethod() { }
         }
 
+        class Type4WithPublicKept
+        {
+            public static void KeptMethod() { }
+            public static void AlsoKeptMethod() { }
+            private static void RemovedMethod() { }
+        }
+
         struct Struct1WithPublicKept
         {
             public static void KeptMethod() { }
@@ -276,6 +283,21 @@ public static void Keep<U>()
             }
         }
 
+        static IKeepPublicThroughGvm s_keepPublicThroughGvm = new KeepPublicThroughGvm();
+
+        interface IKeepPublicThroughGvm
+        {
+            void Keep<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>();
+        }
+
+        class KeepPublicThroughGvm : IKeepPublicThroughGvm
+        {
+            public void Keep<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>()
+            {
+                Assert.NotNull(typeof(T).GetMethod("KeptMethod", BindingFlags.Public | BindingFlags.Static));
+            }
+        }
+
         public static void Run()
         {
             new KeepsNonPublic();
@@ -293,6 +315,10 @@ public static void Run()
             KeepsPublic<Type3WithPublicKept>.Keep<object>();
             Assert.Equal(2, typeof(Type3WithPublicKept).CountMethods());
             Assert.Equal(2, typeof(Type3WithPublicKept).CountPublicMethods());
+
+            s_keepPublicThroughGvm.Keep<Type4WithPublicKept>();
+            Assert.Equal(2, typeof(Type4WithPublicKept).CountMethods());
+            Assert.Equal(2, typeof(Type4WithPublicKept).CountPublicMethods());
         }
     }