Always use portable tailcalling mechanism for delegate tailcalls (#70269

) * Always use portable tailcalling mechanism for delegate tailcalls It is rare but possible to have delegates that point to VSD stubs. Since VSD stubs on x86 may disassemble the call-site we cannot allow tail calling these with the old mechanism. Fix #70259 * Add regression test * Calm down the test Tailcall through built-in delegates is best-effort. Instantiating stubs may break the chain and on ARM32 so may wrapper delegate stubs. The latter happens for this particular test. Change the test so that it is just verifying we do not hit the assert. * Disable regression test on Mono * Clean up some ildasm output
dotnet · Jun 7, 2022 · caff7b0 · caff7b0
1 parent da573d1
commit caff7b0
Showing 6 changed files with 191 additions and 4 deletions.
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -5649,7 +5649,7 @@ class Compiler
 #endif
     bool     fgCheckStmtAfterTailCall();
     GenTree* fgMorphTailCallViaHelpers(GenTreeCall* call, CORINFO_TAILCALL_HELPERS& help);
-    bool fgCanTailCallViaJitHelper();
+    bool fgCanTailCallViaJitHelper(GenTreeCall* call);
     void fgMorphTailCallViaJitHelper(GenTreeCall* call);
     GenTree* fgCreateCallDispatcherAndGetResult(GenTreeCall*          origCall,
                                                 CORINFO_METHOD_HANDLE callTargetStubHnd,

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
@@ -6645,7 +6645,7 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
 
         // On x86 we have a faster mechanism than the general one which we use
         // in almost all cases. See fgCanTailCallViaJitHelper for more information.
-        if (fgCanTailCallViaJitHelper())
+        if (fgCanTailCallViaJitHelper(call))
         {
             tailCallViaJitHelper = true;
         }
@@ -17688,10 +17688,13 @@ bool Compiler::fgCheckStmtAfterTailCall()
 // fgCanTailCallViaJitHelper: check whether we can use the faster tailcall
 // JIT helper on x86.
 //
+// Arguments:
+//   call - the tailcall
+//
 // Return Value:
 //    'true' if we can; or 'false' if we should use the generic tailcall mechanism.
 //
-bool Compiler::fgCanTailCallViaJitHelper()
+bool Compiler::fgCanTailCallViaJitHelper(GenTreeCall* call)
 {
 #if !defined(TARGET_X86) || defined(UNIX_X86_ABI)
     // On anything except windows X86 we have no faster mechanism available.
@@ -17700,11 +17703,22 @@ bool Compiler::fgCanTailCallViaJitHelper()
     // For R2R make sure we go through portable mechanism that the 'EE' side
     // will properly turn into a runtime JIT.
     if (opts.IsReadyToRun())
+    {
         return false;
+    }
 
     // The JIT helper does not properly handle the case where localloc was used.
     if (compLocallocUsed)
+    {
         return false;
+    }
+
+    // Delegate calls may go through VSD stub in rare cases. Those look at the
+    // call site so we cannot use the JIT helper.
+    if (call->IsDelegateInvoke())
+    {
+        return false;
+    }
 
     return true;
 #endif

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259.cs b/src/tests/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259.cs
@@ -0,0 +1,48 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Note: This test file is the source of the Runtime_70259.il file. It requires
+// InlineIL.Fody to compile. It is not used as anything but a reference of that
+// IL file.
+
+using InlineIL;
+using System;
+using System.Runtime.CompilerServices;
+
+class Runtime_70259
+{
+    private static int Main()
+    {
+        // This creates an open delegate that goes through shuffle thunk and
+        // then VSD stub. On arm32 it also goes through wrapper delegate.
+        Func<IFace, int> del = typeof(IFace).GetMethod("Method").CreateDelegate<Func<IFace, int>>();
+
+        // We need a normal call here to get a call site of the form `call
+        // [rel32]` which triggers the bug.
+        return CallMe(del);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static int CallMe(Func<IFace, int> del)
+    {
+        IL.Push(del);
+        IL.Push(new C());
+        IL.Emit.Tail();
+        IL.Emit.Call(new MethodRef(typeof(Func<IFace, int>), "Invoke"));
+        return IL.Return<int>();
+    }
+
+    interface IFace
+    {
+        int Method();
+    }
+
+    class C : IFace
+    {
+        public virtual int Method()
+        {
+            return 100;
+        }
+    }
+}
+
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259.il b/src/tests/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259.il
@@ -0,0 +1,110 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// See Runtime_70259.cs.
+
+//  Microsoft (R) .NET IL Disassembler.  Version 7.0.0-dev
+
+
+
+// Metadata version: v4.0.30319
+.assembly extern System.Runtime
+{
+  .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )                         // .?_....:
+  .ver 7:0:0:0
+}
+.assembly Runtime_70259
+{
+}
+.module Runtime_70259.dll
+// MVID: {78ECA09B-26A9-44BB-9FD2-D94B639A44D5}
+.custom instance void [System.Runtime]System.Security.UnverifiableCodeAttribute::.ctor() = ( 01 00 00 00 )
+.imagebase 0x00400000
+.file alignment 0x00000200
+.stackreserve 0x00100000
+.subsystem 0x0003       // WINDOWS_CUI
+.corflags 0x00000001    //  ILONLY
+// Image base: 0x000001F5DF740000
+
+
+// =============== CLASS MEMBERS DECLARATION ===================
+
+.class private auto ansi beforefieldinit Runtime_70259
+       extends [System.Runtime]System.Object
+{
+  .class interface abstract auto ansi nested private IFace
+  {
+    .method public hidebysig newslot abstract virtual
+            instance int32  Method() cil managed
+    {
+    } // end of method IFace::Method
+
+  } // end of class IFace
+
+  .class auto ansi nested private beforefieldinit C
+         extends [System.Runtime]System.Object
+         implements Runtime_70259/IFace
+  {
+    .method public hidebysig newslot virtual
+            instance int32  Method() cil managed
+    {
+      // Code size       3 (0x3)
+      .maxstack  8
+      IL_0000:  ldc.i4.s   100
+      IL_0002:  ret
+    } // end of method C::Method
+
+    .method public hidebysig specialname rtspecialname
+            instance void  .ctor() cil managed
+    {
+      // Code size       7 (0x7)
+      .maxstack  8
+      IL_0000:  ldarg.0
+      IL_0001:  call       instance void [System.Runtime]System.Object::.ctor()
+      IL_0006:  ret
+    } // end of method C::.ctor
+
+  } // end of class C
+
+  .method private hidebysig static int32
+          Main() cil managed
+  {
+    .entrypoint
+    // Code size       31 (0x1f)
+    .maxstack  8
+    IL_0000:  ldtoken    Runtime_70259/IFace
+    IL_0005:  call       class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
+    IL_000a:  ldstr      "Method"
+    IL_000f:  call       instance class [System.Runtime]System.Reflection.MethodInfo [System.Runtime]System.Type::GetMethod(string)
+    IL_0014:  callvirt   instance !!0 [System.Runtime]System.Reflection.MethodInfo::CreateDelegate<class [System.Runtime]System.Func`2<class Runtime_70259/IFace,int32>>()
+    IL_0019:  call       int32 Runtime_70259::CallMe(class [System.Runtime]System.Func`2<class Runtime_70259/IFace,int32>)
+    IL_001e:  ret
+  } // end of method Runtime_70259::Main
+
+  .method private hidebysig static int32
+          CallMe(class [System.Runtime]System.Func`2<class Runtime_70259/IFace,int32> del) cil managed noinlining
+  {
+    // Code size       14 (0xe)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  newobj     instance void Runtime_70259/C::.ctor()
+    IL_0006:  tail.
+    IL_0008:  call       instance !1 class [System.Runtime]System.Func`2<class Runtime_70259/IFace,int32>::Invoke(!0)
+    IL_000d:  ret
+  } // end of method Runtime_70259::CallMe
+
+  .method public hidebysig specialname rtspecialname
+          instance void  .ctor() cil managed
+  {
+    // Code size       7 (0x7)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  call       instance void [System.Runtime]System.Object::.ctor()
+    IL_0006:  ret
+  } // end of method Runtime_70259::.ctor
+
+} // end of class Runtime_70259
+
+// =============================================================
+
+// *********** DISASSEMBLY COMPLETE ***********************
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259.ilproj b/src/tests/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259.ilproj
@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk.IL">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>Full</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+</Project>
diff --git a/src/tests/issues.targets b/src/tests/issues.targets
@@ -1499,7 +1499,7 @@
         <ExcludeList Include="$(XunitTestBinBase)/JIT/SIMD/ShiftOperations/*">
           <Issue>There is a known undefined behavior with shifts and 0xFFFFFFFF overflows, so skip the test for mono.</Issue>
         </ExcludeList>
-      
+
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/TieredCompilation/BasicTestWithMcj/*">
             <Issue>Tests features specific to coreclr</Issue>
         </ExcludeList>
@@ -2219,6 +2219,9 @@
         <ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/BestFitMapping/Assembly_True_False/Assembly_True_False/*">
             <Issue>Mono doesn't support interop BestFitMapping and ThrowOnUnmappableChar attributes</Issue>
         </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259/*">
+            <Issue>https://github.com/dotnet/runtime/issues/70279</Issue>
+        </ExcludeList>
     </ItemGroup>
 
     <!-- Known failures for mono runtime on Windows -->