JIT: use stress mode tail call validation info for implicit tail calls (#25093)

If a call site fails tail call stress validation, don't consider it for
implicit tail calling either.

In normal jitting we defer this level of validation until impImportCall
to avoid duplicating work.

This avoids an assert when we have invalid IL.

Fixes #25027.

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 83942d6..bbb0d14 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -13787,8 +13787,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 constraintCall = (prefixFlags & PREFIX_CONSTRAINED) != 0;
 
                 bool newBBcreatedForTailcallStress;
+                bool passedStressModeValidation;
 
                 newBBcreatedForTailcallStress = false;
+                passedStressModeValidation    = true;
 
                 if (compIsForInlining())
                 {
@@ -13816,22 +13818,41 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             (OPCODE)getU1LittleEndian(codeAddr + sz) == CEE_RET; // Next opcode is a CEE_RET
 
                         bool hasTailPrefix = (prefixFlags & PREFIX_TAILCALL_EXPLICIT);
-                        if (newBBcreatedForTailcallStress && !hasTailPrefix && // User hasn't set "tail." prefix yet.
-                            verCheckTailCallConstraint(opcode, &resolvedToken,
-                                                       constraintCall ? &constrainedResolvedToken : nullptr,
-                                                       true) // Is it legal to do tailcall?
-                            )
+                        if (newBBcreatedForTailcallStress && !hasTailPrefix)
                         {
-                            CORINFO_METHOD_HANDLE declaredCalleeHnd = callInfo.hMethod;
-                            bool                  isVirtual         = (callInfo.kind == CORINFO_VIRTUALCALL_STUB) ||
-                                             (callInfo.kind == CORINFO_VIRTUALCALL_VTABLE);
-                            CORINFO_METHOD_HANDLE exactCalleeHnd = isVirtual ? nullptr : declaredCalleeHnd;
-                            if (info.compCompHnd->canTailCall(info.compMethodHnd, declaredCalleeHnd, exactCalleeHnd,
-                                                              hasTailPrefix)) // Is it legal to do tailcall?
+                            // Do a more detailed evaluation of legality
+                            const bool returnFalseIfInvalid = true;
+                            const bool passedConstraintCheck =
+                                verCheckTailCallConstraint(opcode, &resolvedToken,
+                                                           constraintCall ? &constrainedResolvedToken : nullptr,
+                                                           returnFalseIfInvalid);
+
+                            if (passedConstraintCheck)
+                            {
+                                // Now check with the runtime
+                                CORINFO_METHOD_HANDLE declaredCalleeHnd = callInfo.hMethod;
+                                bool                  isVirtual         = (callInfo.kind == CORINFO_VIRTUALCALL_STUB) ||
+                                                 (callInfo.kind == CORINFO_VIRTUALCALL_VTABLE);
+                                CORINFO_METHOD_HANDLE exactCalleeHnd = isVirtual ? nullptr : declaredCalleeHnd;
+                                if (info.compCompHnd->canTailCall(info.compMethodHnd, declaredCalleeHnd, exactCalleeHnd,
+                                                                  hasTailPrefix)) // Is it legal to do tailcall?
+                                {
+                                    // Stress the tailcall.
+                                    JITDUMP(" (Tailcall stress: prefixFlags |= PREFIX_TAILCALL_EXPLICIT)");
+                                    prefixFlags |= PREFIX_TAILCALL_EXPLICIT;
+                                }
+                                else
+                                {
+                                    // Runtime disallows this tail call
+                                    JITDUMP(" (Tailcall stress: runtime preventing tailcall)");
+                                    passedStressModeValidation = false;
+                                }
+                            }
+                            else
                             {
-                                // Stress the tailcall.
-                                JITDUMP(" (Tailcall stress: prefixFlags |= PREFIX_TAILCALL_EXPLICIT)");
-                                prefixFlags |= PREFIX_TAILCALL_EXPLICIT;
+                                // Constraints disallow this tail call
+                                JITDUMP(" (Tailcall stress: constraint check failed)");
+                                passedStressModeValidation = false;
                             }
                         }
                     }
@@ -13841,9 +13862,16 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 bool isRecursive;
                 isRecursive = !compIsForInlining() && (callInfo.hMethod == info.compMethodHnd);
 
-                // Note that when running under tail call stress, a call will be marked as explicit tail prefixed
-                // hence will not be considered for implicit tail calling.
-                if (impIsImplicitTailCallCandidate(opcode, codeAddr + sz, codeEndp, prefixFlags, isRecursive))
+                // If we've already disqualified this call as a tail call under tail call stress,
+                // don't consider it for implicit tail calling either.
+                //
+                // When not running under tail call stress, we may mark this call as an implicit
+                // tail call candidate. We'll do an "equivalent" validation during impImportCall.
+                //
+                // Note that when running under tail call stress, a call marked as explicit
+                // tail prefixed will not be considered for implicit tail calling.
+                if (passedStressModeValidation &&
+                    impIsImplicitTailCallCandidate(opcode, codeAddr + sz, codeEndp, prefixFlags, isRecursive))
                 {
                     if (compIsForInlining())
                     {

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027.il b/tests/src/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027.il
new file mode 100644 (file)
index 0000000..351af3c
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027.il
@@ -0,0 +1,92 @@
+// 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.
+//
+// GitHub 25027: Tail call stress shouldn't cause asserts 
+// in the presence of invalid IL
+
+.assembly extern System.Private.CoreLib { auto }
+.assembly extern mscorlib { auto }
+.assembly GitHub_25027 {}
+
+.class private auto ansi beforefieldinit X
+       extends [System.Private.CoreLib]System.Object
+{
+  .method public hidebysig static int32  Main() cil managed
+  {
+    .entrypoint
+    .locals init ([0] int32 result)
+    ldc.i4.m1
+    stloc.0
+
+    .try
+    {
+      call       int32 X::F()
+      stloc.0
+      leave.s    join
+
+    }
+    catch [System.Private.CoreLib]System.InvalidProgramException 
+    {
+      pop
+      ldc.i4.s   100
+      stloc.0
+      leave.s    join
+    }
+
+    join:
+
+    ldloc.0
+    ret
+  }
+
+  // F is intentionally made big enough so that it doesn't get inlined into
+  // Main without needing to be marked "noinlining"
+
+  .method private hidebysig static int32 
+          F() cil managed
+  {
+    .locals init ([0] int32 r,
+             [1] int32 i)
+    ldc.i4.0
+    stloc.0
+    ldc.i4.0
+    stloc.1
+    br.s test
+
+    loop:  
+
+    ldloc.0
+    ldloc.1
+    ldloc.1
+    mul
+    add
+    stloc.0
+    ldloc.1
+    ldc.i4.1
+    add
+    stloc.1
+
+    test:
+
+    ldloc.1
+    ldc.i4.s   10
+    ble.s      loop
+
+    ldloc.0
+
+    // invalid IL and invalid tail call site
+
+    dup
+    dup
+    call       void X::G(int32)
+    ret
+  }
+
+  .method private hidebysig static void
+          G(int32 x) cil managed
+  {
+    ret
+  }
+
+}

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027.ilproj b/tests/src/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027.ilproj
new file mode 100644 (file)
index 0000000..28e89d2
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027.ilproj
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="GitHub_25027.il" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>
\ No newline at end of file