[release/6.0] Add explicit null-check for tailcalls to VSD (#62769)

* Add explicit null-check for tailcalls to VSD

There is already a comment that this is necessary, but it is only being
done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD
as well.

Fix #61486

* Revert cleanup to make potential backport easier

Co-authored-by: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index f6bc9a4..ba5f450 100644 (file)
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -7896,6 +7896,15 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
         // Avoid potential extra work for the return (for example, vzeroupper)
         call->gtType = TYP_VOID;
 
+        // The runtime requires that we perform a null check on the `this` argument before
+        // tail calling to a virtual dispatch stub. This requirement is a consequence of limitations
+        // in the runtime's ability to map an AV to a NullReferenceException if
+        // the AV occurs in a dispatch stub that has unmanaged caller.
+        if (call->IsVirtualStub())
+        {
+            call->gtFlags |= GTF_CALL_NULLCHECK;
+        }
+
         // Do some target-specific transformations (before we process the args,
         // etc.) for the JIT helper case.
         if (tailCallViaJitHelper)
@@ -8622,15 +8631,6 @@ void Compiler::fgMorphTailCallViaJitHelper(GenTreeCall* call)
     JITDUMP("fgMorphTailCallViaJitHelper (before):\n");
     DISPTREE(call);
 
-    // The runtime requires that we perform a null check on the `this` argument before
-    // tail calling  to a virtual dispatch stub. This requirement is a consequence of limitations
-    // in the runtime's ability to map an AV to a NullReferenceException if
-    // the AV occurs in a dispatch stub that has unmanaged caller.
-    if (call->IsVirtualStub())
-    {
-        call->gtFlags |= GTF_CALL_NULLCHECK;
-    }
-
     // For the helper-assisted tail calls, we need to push all the arguments
     // into a single list, and then add a few extra at the beginning or end.
     //

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs
new file mode 100644 (file)
index 0000000..3285ee9
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs
@@ -0,0 +1,45 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+
+public class Runtime_61486
+{
+    public static int Main()
+    {
+        var my = new My(new My(null));
+        var m = my.GetType().GetMethod("M");
+        try
+        {
+            m.Invoke(my, null);
+            return -1;
+        }
+        catch (TargetInvocationException ex) when (ex.InnerException is NullReferenceException)
+        {
+            return 100;
+        }
+    }
+
+    public interface IFace
+    {
+        void M();
+    }
+
+    public class My : IFace
+    {
+        private IFace _face;
+
+        public My(IFace face)
+        {
+            _face = face;
+        }
+
+        // We cannot handle a null ref inside a VSD if the caller is not
+        // managed frame. This test is verifying that JIT null checks ahead of
+        // time in this case.
+        [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+        public void M() => _face.M();
+    }
+}

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj
new file mode 100644 (file)
index 0000000..6946bed
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj
@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>