diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs index 02e206061fc99d..26b65ca24354c2 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs @@ -323,8 +323,6 @@ internal void SetWaitSleepJoinState() GC.KeepAlive(this); } - // Temporary workaround for https://github.com/dotnet/runtime/issues/122479 - [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] internal void ClearWaitSleepJoinState() { // This method is called when the thread is no longer in a wait, sleep, or join state. diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a83b16e1591cb9..bd5917de11f821 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9151,44 +9151,58 @@ void Compiler::impImportBlockCode(BasicBlock* block) bool hasTailPrefix = (prefixFlags & PREFIX_TAILCALL_EXPLICIT); if (newBBcreatedForTailcallStress && !hasTailPrefix) { - // Do a more detailed evaluation of legality - const bool passedConstraintCheck = - checkTailCallConstraint(opcode, &resolvedToken, - constraintCall ? &constrainedResolvedToken : nullptr); - - // Avoid setting compHasBackwardsJump = true via tail call stress if the method cannot have - // patchpoints. - // - const bool mayHavePatchpoints = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0) && - (JitConfig.TC_OnStackReplacement() > 0) && - compCanHavePatchpoints(); - if (passedConstraintCheck && (mayHavePatchpoints || compHasBackwardJump)) + // Don't stress-tailcall named intrinsics: many of them are imported as + // non-CALL IR nodes (e.g. GC.KeepAlive -> GT_KEEPALIVE), which would + // leave a BBJ_RETURN block that doesn't end in a CALL/RETURN and + // confuse later phases (see + // https://github.com/dotnet/runtime/issues/122479). Suppress both the + // explicit and the implicit tailcall promotion in that case. + if ((callInfo.methodFlags & CORINFO_FLG_INTRINSIC) != 0) { - // 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? + JITDUMP(" (Tailcall stress: skipping intrinsic)"); + passedStressModeValidation = false; + } + else + { + // Do a more detailed evaluation of legality + const bool passedConstraintCheck = + checkTailCallConstraint(opcode, &resolvedToken, + constraintCall ? &constrainedResolvedToken : nullptr); + + // Avoid setting compHasBackwardsJump = true via tail call stress if the method cannot have + // patchpoints. + // + const bool mayHavePatchpoints = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0) && + (JitConfig.TC_OnStackReplacement() > 0) && + compCanHavePatchpoints(); + if (passedConstraintCheck && (mayHavePatchpoints || compHasBackwardJump)) { - // Stress the tailcall. - JITDUMP(" (Tailcall stress: prefixFlags |= PREFIX_TAILCALL_EXPLICIT)"); - prefixFlags |= PREFIX_TAILCALL_EXPLICIT | PREFIX_TAILCALL_STRESS; + // 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 | PREFIX_TAILCALL_STRESS; + } + else + { + // Runtime disallows this tail call + JITDUMP(" (Tailcall stress: runtime preventing tailcall)"); + passedStressModeValidation = false; + } } else { - // Runtime disallows this tail call - JITDUMP(" (Tailcall stress: runtime preventing tailcall)"); + // Constraints disallow this tail call + JITDUMP(" (Tailcall stress: constraint check failed)"); passedStressModeValidation = false; } } - else - { - // Constraints disallow this tail call - JITDUMP(" (Tailcall stress: constraint check failed)"); - passedStressModeValidation = false; - } } } #endif diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_122479/Runtime_122479.cs b/src/tests/JIT/Regression/JitBlue/Runtime_122479/Runtime_122479.cs new file mode 100644 index 00000000000000..73c1466aee562e --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_122479/Runtime_122479.cs @@ -0,0 +1,39 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Under STRESS_TAILCALL the JIT importer would promote a "call+ret" pattern +// to an explicit (or implicit) tailcall before discovering that the callee +// is a named intrinsic (e.g. GC.KeepAlive, which is imported as a +// GT_KEEPALIVE node, not a CALL). When combined with a [SuppressGCTransition] +// P/Invoke earlier in the same block, the resulting BBJ_RETURN ended with a +// non-CALL/non-RETURN statement and tripped an assert in fgInsertStmtNearEnd +// during "Insert GC Polls". +// +// Run under any jitstress / jitstress_tiered leg to validate. + +using System; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Threading; +using Xunit; + +public class Runtime_122479 +{ + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ReproPattern() + { + // Thread.Sleep -> Thread.ClearWaitSleepJoinState which has the + // exact "SuppressGCTransition QCall; GC.KeepAlive(this); ret" shape + // that originally triggered the assert. + Thread.Sleep(0); + } + + [Fact] + public static void TestEntryPoint() + { + for (int i = 0; i < 100; i++) + { + ReproPattern(); + } + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_122479/Runtime_122479.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_122479/Runtime_122479.csproj new file mode 100644 index 00000000000000..de6d5e08882e86 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_122479/Runtime_122479.csproj @@ -0,0 +1,8 @@ + + + True + + + + +