Fix crossgen2 incorrect handling of ldftn to external managed methods

src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/LocalMethodImport.cs → src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DelayLoadMethodImport.cs

@@ -11,12 +11,12 @@
 
 namespace ILCompiler.DependencyAnalysis.ReadyToRun
 {
-    public class LocalMethodImport : DelayLoadHelperImport, IMethodNode
+    public class DelayLoadMethodImport : DelayLoadHelperImport, IMethodNode
     {
         private readonly MethodWithGCInfo _localMethod;
         private readonly MethodWithToken _method;
 
-        public LocalMethodImport(
+        public DelayLoadMethodImport(
             NodeFactory factory,
             ReadyToRunFixupKind fixupKind,
             MethodWithToken method,
@@ -48,14 +48,22 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
             {
                 yield return entry;
             }
-            yield return new DependencyListEntry(_localMethod, "Local method import");
+            if (_localMethod != null)
+                yield return new DependencyListEntry(_localMethod, "Local method import");
         }
 
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
-            int result = comparer.Compare(_localMethod, ((LocalMethodImport)other)._localMethod);
-            if (result != 0)
-                return result;
+            if ((_localMethod != null) && (((DelayLoadMethodImport)other)._localMethod != null))
+            {
+                int result = comparer.Compare(_localMethod, ((DelayLoadMethodImport)other)._localMethod);
+                if (result != 0)
+                    return result;
+            }
+            else if (_localMethod != null)
+                return 1;
+            else if (((DelayLoadMethodImport)other)._localMethod != null)
+                return -1;
 
             return base.CompareToImpl(other, comparer);
         }

src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/PrecodeMethodImport.cs

@@ -55,14 +55,22 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
             {
                 yield return entry;
             }
-            yield return new DependencyListEntry(_localMethod, "Precode Method Import");
+            if (_localMethod != null)
+                yield return new DependencyListEntry(_localMethod, "Precode Method Import");
         }
 
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
-            int result = comparer.Compare(_localMethod, ((PrecodeMethodImport)other)._localMethod);
-            if (result != 0)
-                return result;
+            if ((_localMethod != null) && (((PrecodeMethodImport)other)._localMethod != null))
+            {
+                int result = comparer.Compare(_localMethod, ((PrecodeMethodImport)other)._localMethod);
+                if (result != 0)
+                    return result;
+            }
+            else if (_localMethod != null)
+                return 1;
+            else if (((PrecodeMethodImport)other)._localMethod != null)
+                return -1;
 
             return base.CompareToImpl(other, comparer);
         }

src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs

@@ -351,36 +351,30 @@ private IMethodNode CreateMethodEntrypoint(TypeAndMethod key)
             bool isInstantiatingStub = key.IsInstantiatingStub;
             bool isPrecodeImportRequired = key.IsPrecodeImportRequired;
             MethodDesc compilableMethod = method.Method.GetCanonMethodTarget(CanonicalFormKind.Specific);
+            MethodWithGCInfo methodWithGCInfo = null;
+
             if (CompilationModuleGroup.ContainsMethodBody(compilableMethod, false))
             {
-                if (isPrecodeImportRequired)
-                {
-                    return new PrecodeMethodImport(
-                        this,
-                        ReadyToRunFixupKind.MethodEntry,
-                        method,
-                        CompiledMethodNode(compilableMethod),
-                        isUnboxingStub,
-                        isInstantiatingStub);
-                }
-                else
-                {
-                    return new LocalMethodImport(
-                        this,
-                        ReadyToRunFixupKind.MethodEntry,
-                        method,
-                        CompiledMethodNode(compilableMethod),
-                        isUnboxingStub,
-                        isInstantiatingStub);
-                }
+                methodWithGCInfo = CompiledMethodNode(compilableMethod);
+            }
+
+            if (isPrecodeImportRequired)
+            {
+                return new PrecodeMethodImport(
+                    this,
+                    ReadyToRunFixupKind.MethodEntry,
+                    method,
+                    methodWithGCInfo,
+                    isUnboxingStub,
+                    isInstantiatingStub);
             }
             else
             {
-                // First time we see a given external method - emit indirection cell and the import entry
-                return new ExternalMethodImport(
+                return new DelayLoadMethodImport(
                     this,
                     ReadyToRunFixupKind.MethodEntry,
                     method,
+                    methodWithGCInfo,
                     isUnboxingStub,
                     isInstantiatingStub);
             }
@@ -409,9 +403,9 @@ public IEnumerable<MethodWithGCInfo> EnumerateCompiledMethods(EcmaModule moduleT
 
                 IMethodNode methodNodeDebug = MethodEntrypoint(new MethodWithToken(method, moduleToken, constrainedType: null), false, false, false);
                 MethodWithGCInfo methodCodeNodeDebug = methodNodeDebug as MethodWithGCInfo;
-                if (methodCodeNodeDebug == null && methodNodeDebug is LocalMethodImport localMethodImport)
+                if (methodCodeNodeDebug == null && methodNodeDebug is DelayLoadMethodImport DelayLoadMethodImport)
                 {
-                    methodCodeNodeDebug = localMethodImport.MethodCodeNode;
+                    methodCodeNodeDebug = DelayLoadMethodImport.MethodCodeNode;
                 }
                 if (methodCodeNodeDebug == null && methodNodeDebug is PrecodeMethodImport precodeMethodImport)
                 {

src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj

@@ -126,7 +126,6 @@
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\ImportThunk.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\DelegateCtorSignature.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\DevirtualizationManager.cs" />
-    <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\ExternalMethodImport.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\FieldFixupSignature.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\HeaderNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\ImportSectionNode.cs" />
@@ -136,7 +135,7 @@
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\InstanceEntryPointTableNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\OwnerCompositeExecutableNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\PrecodeMethodImport.cs" />
-    <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\LocalMethodImport.cs" />
+    <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\DelayLoadMethodImport.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\ManifestMetadataTableNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\MethodEntryPointTableNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ReadyToRun\MethodFixupSignature.cs" />

src/coreclr/tests/src/readytorun/crossgen2/Program.cs

@@ -1237,6 +1237,31 @@ private static string EmitTextFileForTesting()
         return file;
     }
 
+    private static bool DelegateFromAnotherModuleTest()
+    {
+        // This test tests referencing a method from another module while creating a delegate.
+        Action del = HelperClass.DelegateReferencedMethod;
+        string delegateMethodString = del.Method.ToString();
+        Console.WriteLine(delegateMethodString);
+        if (!delegateMethodString.Contains("DelegateReferencedMethod"))
+            return false;
+        else
+            return true;
+    }
+
+    private static bool FunctionPointerFromAnotherModuleTest()
+    {
+        // This test tests referencing a method from another module while creating a function pointer.
+        // Function pointers to managed functions should be stable, and result in calling the right function
+        IntPtr initialFunctionPointer = HelperILCode.GetFunctionPointerFromOtherModule();
+        HelperILCode.CallFunctionPointer(initialFunctionPointer);
+        HelperILCode.CallFunctionPointer(initialFunctionPointer);
+        HelperILCode.CallFunctionPointer(initialFunctionPointer);
+        IntPtr finalFunctionPointer = HelperILCode.GetFunctionPointerFromOtherModule();
+
+        return finalFunctionPointer == initialFunctionPointer;
+    }
+
     public static int Main(string[] args)
     {
         _passedTests = new List<string>();
@@ -1296,6 +1321,8 @@ public static int Main(string[] args)
         RunTest("ObjectToStringOnGenericParamTestSByte", ObjectToStringOnGenericParamTestSByte());
         RunTest("ObjectToStringOnGenericParamTestVersionBubbleLocalStruct", ObjectToStringOnGenericParamTestVersionBubbleLocalStruct());
         RunTest("EnumValuesToStringTest", EnumValuesToStringTest());
+        RunTest("DelegateFromAnotherModuleTest", DelegateFromAnotherModuleTest());
+        RunTest("FunctionPointerFromAnotherModuleTest", FunctionPointerFromAnotherModuleTest());
 
         File.Delete(TextFileName);
 

src/coreclr/tests/src/readytorun/crossgen2/crossgen2smoke.csproj

@@ -14,17 +14,25 @@
     <!-- This is an explicit crossgen test -->
     <CrossGenTest>false</CrossGenTest>
   </PropertyGroup>
+  <ItemGroup>
+    <ProjectReference Include=".\helperdll.csproj" />
+    <ProjectReference Include=".\helperildll.ilproj" />
+  </ItemGroup>
   <ItemGroup>
     <Compile Include="Program.cs" />
   </ItemGroup>
   <PropertyGroup>
     <CLRTestBatchPreCommands><![CDATA[
 $(CLRTestBatchPreCommands)
-%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll -r:%Core_Root%\*.dll -o:crossgen2smoke.dll crossgen2smoke.ildll
+copy helperildll.dll helperildll.ildll
+%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll -r:%Core_Root%\*.dll -r:helperdll.dll -o:helperildll.dll helperildll.ildll
+%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll -r:%Core_Root%\*.dll -r:helperdll.dll -r:helperildll.dll -o:crossgen2smoke.dll crossgen2smoke.ildll
 ]]></CLRTestBatchPreCommands>
     <BashCLRTestPreCommands><![CDATA[
 $(BashCLRTestPreCommands)
-$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll -r:$CORE_ROOT/*.dll -o:crossgen2smoke.dll crossgen2smoke.ildll
+cp helperildll.dll helperildll.ildll
+$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll -r:$CORE_ROOT/*.dll -r:helperdll.dll -o:helperildll.dll helperildll.ildll
+$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll -r:$CORE_ROOT/*.dll -r:helperdll.dll -r:helperildll.dll -o:crossgen2smoke.dll crossgen2smoke.ildll
 ]]></BashCLRTestPreCommands>
   </PropertyGroup>
 </Project>

src/coreclr/tests/src/readytorun/crossgen2/helperdll.cs

@@ -0,0 +1,22 @@
+// 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;
+
+public class HelperClass
+{
+    // This method is used to test whether or not a method from a separate module 
+    // referenced via Delegate is handled correctly. Do not call this method directly.
+    public static void DelegateReferencedMethod()
+    {
+        Console.WriteLine("In helper method");
+    }
+
+    // This method is used to test whether or not a method from a separate module 
+    // referenced as a function pointer is handled correctly. Do not call this method directly
+    public static void FunctionPointerReferencedMethod()
+    {
+        Console.WriteLine("In function pointer method");
+    }
+}

src/coreclr/tests/src/readytorun/crossgen2/helperdll.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>library</OutputType>
+    <CLRTestKind>SharedLibrary</CLRTestKind>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include=".\helperdll.cs" />
+  </ItemGroup>
+</Project>

src/coreclr/tests/src/readytorun/crossgen2/helperildll.il

@@ -0,0 +1,25 @@
+// 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.
+
+.assembly extern mscorlib { }
+.assembly extern helperdll { }
+
+.assembly helperildll {}
+.module helperildll.dll
+
+.class auto ansi public beforefieldinit HelperILCode
+       extends [mscorlib]System.Object
+{
+  .method public hidebysig static native int GetFunctionPointerFromOtherModule() cil managed noinlining
+  {
+     ldftn      void [helperdll]HelperClass::FunctionPointerReferencedMethod()
+     ret
+  }
+  .method public hidebysig static void CallFunctionPointer(native int) cil managed noinlining
+  {
+     ldarg.0
+     calli void()
+     ret
+  }
+}

src/coreclr/tests/src/readytorun/crossgen2/helperildll.ilproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk.IL">
+  <PropertyGroup>
+    <OutputType>Library</OutputType>
+    <CLRTestKind>SharedLibrary</CLRTestKind>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="helperildll.il" />
+  </ItemGroup>
+</Project>

src/coreclr/tests/src/readytorun/determinism/crossgen2determinism.csproj

@@ -18,16 +18,16 @@
     <CLRTestBatchPreCommands><![CDATA[
 $(CLRTestBatchPreCommands)
 set CoreRT_DeterminismSeed=1
-%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll --map -r:%Core_Root%\*.dll -o:crossgen2smoke1.ildll ..\..\crossgen2\crossgen2smoke\crossgen2smoke.ildll
+%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll --map -r:%Core_Root%\*.dll -r:..\..\crossgen2\crossgen2smoke\helperdll.dll -r:..\..\crossgen2\crossgen2smoke\helperildll.dll -o:crossgen2smoke1.ildll ..\..\crossgen2\crossgen2smoke\crossgen2smoke.ildll
 set CoreRT_DeterminismSeed=2
-%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll --map -r:%Core_Root%\*.dll -o:crossgen2smoke2.ildll ..\..\crossgen2\crossgen2smoke\crossgen2smoke.ildll
+%Core_Root%\CoreRun.exe %Core_Root%\crossgen2\crossgen2.dll --map -r:%Core_Root%\*.dll -r:..\..\crossgen2\crossgen2smoke\helperdll.dll -r:..\..\crossgen2\crossgen2smoke\helperildll.dll -o:crossgen2smoke2.ildll ..\..\crossgen2\crossgen2smoke\crossgen2smoke.ildll
 ]]></CLRTestBatchPreCommands>
     <BashCLRTestPreCommands><![CDATA[
 $(BashCLRTestPreCommands)
 export CoreRT_DeterminismSeed=1
-$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll --map -r:$CORE_ROOT/*.dll -o:crossgen2smoke1.ildll ../../crossgen2/crossgen2smoke/crossgen2smoke.ildll
+$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll --map -r:$CORE_ROOT/*.dll -r:../../crossgen2/crossgen2smoke/helperdll.dll -r:../../crossgen2/crossgen2smoke/helperildll.dll -o:crossgen2smoke1.ildll ../../crossgen2/crossgen2smoke/crossgen2smoke.ildll
 export CoreRT_DeterminismSeed=2
-$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll --map -r:$CORE_ROOT/*.dll -o:crossgen2smoke2.ildll ../../crossgen2/crossgen2smoke/crossgen2smoke.ildll
+$CORE_ROOT/corerun $CORE_ROOT/crossgen2/crossgen2.dll --map -r:$CORE_ROOT/*.dll -r:../../crossgen2/crossgen2smoke/helperdll.dll -r:../../crossgen2/crossgen2smoke/helperildll.dll -o:crossgen2smoke2.ildll ../../crossgen2/crossgen2smoke/crossgen2smoke.ildll
 ]]></BashCLRTestPreCommands>
   </PropertyGroup>
 </Project>