Repository navigation
JIT: Treat tailcalled delegate receivers as escaping - #136408
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7bb954bf-2cce-4297-acb5-e4f5e2cb32b2
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7bb954bf-2cce-4297-acb5-e4f5e2cb32b2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7bb954bf-2cce-4297-acb5-e4f5e2cb32b2
|
PTAL @dotnet/jit-contrib trivial fix, no diffs . |
adamperlin
left a comment
There was a problem hiding this comment.
This lgtm from what I understand. Is it worth adding a regression test for the second failure case?
|
Thanks for the quick fix! I reported #136373. While narrowing it down we hit a second shape of the same bug, and I'd like to make sure this PR covers it before it merges. The second shape: no factory in tail position. A plain C# helper ( Results (Windows 11 x64, 2026-10-23):
All with Repro: two projects, 96 lines (click to expand)
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net9.0;net10.0</TargetFrameworks>
<Optimize>true</Optimize>
<DebugType>none</DebugType>
</PropertyGroup>
</Project>
using System;
public static class Alg
{
// A plain C# helper: an ordinary call to the delegate, no tail prefix, so the JIT can inline it.
public static TResult Let<T, TResult>(T value, Func<T, TResult> f) => f(value);
}
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFrameworks>net9.0;net10.0</TargetFrameworks>
<Optimize>true</Optimize>
<Tailcalls>true</Tailcalls>
<DebugType>none</DebugType>
<RollForward>LatestPatch</RollForward>
</PropertyGroup>
<ItemGroup>
<Compile Include="Program.fs" />
<ProjectReference Include="../Helpers/Helpers.csproj" />
</ItemGroup>
</Project>
// Second failure mode: a delegate stack-allocated behind a guarded devirtualization, used after its frame is gone.
// Run with DOTNET_FastTailCalls=0, which sends every explicit tail call through the tailcall helpers. Top.Call's tail
// call into Outer.Call, and Outer.Call's into Root.Invoke, then make Root.Invoke run under a tail-call dispatcher.
// (no argument) warm Alg.Let up first (so its delegate call gets a profile and can be GDV'd), then call Top.Call
// nowarm call Top.Call without warming Alg.Let up (the delegate comes back from a real call, on the heap)
// outer warm up, then call Outer.Call (one dispatcher level fewer)
// direct warm up, then call Root.Invoke directly (no dispatcher above it)
open System
open System.Runtime.CompilerServices
open System.Threading
[<Sealed>]
type W(full: decimal) =
member _.Full = full
let levelOf (reversed: (decimal * string)[]) (w: W) (win: decimal) =
let basis = win / w.Full
match reversed |> Array.tryFind (fun (minimum, _) -> minimum <= basis) with
| Some (_, level) -> level
| None -> "none"
type Factory() =
// The delegate is created inside the lambda handed to Alg.Let. The lambda captures `entries` (an object), so the
// JIT can guess its target and inline it once Alg.Let's delegate call has a profile.
[<MethodImpl(MethodImplOptions.AggressiveInlining)>]
static member Make(entries: (decimal * string)[]) : Func<W, decimal, string> =
Alg.Let(entries |> Array.sortBy fst, Func<(decimal * string)[], Func<W, decimal, string>>(fun sorted ->
let reversed = Array.rev sorted
Func<W, decimal, string>(fun w win -> if entries.Length = 0 then "none" else levelOf reversed w win)))
type Root(entries: (decimal * string)[]) =
// NoInlining only keeps the F# compiler from copying this into Outer; it doesn't restrict what the JIT inlines here.
[<MethodImpl(MethodImplOptions.NoInlining)>]
member _.Invoke(w: W, win: int64) : string = Factory.Make(entries).Invoke(w, decimal win) // tail. callvirt Invoke
type Outer(root: Root) =
member _.Call(w: W, win: int64) : string = root.Invoke(w, win) // tail. callvirt
type Top(outer: Outer) =
member _.Call(w: W, win: int64) : string = outer.Call(w, win) // tail. callvirt
let say (s: string) = Console.Out.WriteLine s; Console.Out.Flush()
[<EntryPoint>]
let main argv =
let mode = if argv.Length > 0 then argv[0] else "dispatcher"
let fastTailCalls = Environment.GetEnvironmentVariable "DOTNET_FastTailCalls"
say $"runtime {Environment.Version} {Runtime.InteropServices.RuntimeInformation.OSArchitecture}, mode {mode}, DOTNET_FastTailCalls={fastTailCalls}"
let entries = [| 0M, "Small"; 5M, "Big" |]
let w = W 100M
if mode <> "nowarm" then
// Calls in rounds with pauses, so Alg.Let tiers up (instrumented Tier0, then Tier1) in the background.
for _ in 1 .. 160 do
for _ in 1 .. 250 do Factory.Make(entries).Invoke(w, 700M) |> ignore
Thread.Sleep 50
say "warmed Alg.Let up"
let root = Root entries
let outer = Outer root
let top = Top outer
RuntimeHelpers.PrepareMethod(typeof<Top>.GetMethod("Call").MethodHandle)
RuntimeHelpers.PrepareMethod(typeof<Outer>.GetMethod("Call").MethodHandle)
RuntimeHelpers.PrepareMethod(typeof<Root>.GetMethod("Invoke").MethodHandle)
say "calling"
let mutable result = ""
for _ in 1 .. 200000 do
result <- if mode = "direct" then root.Invoke(w, 700L) elif mode = "outer" then outer.Call(w, 700L) else top.Call(w, 700L)
say $"returned {result}"
0Run: Since the delegate here is the receiver of the tail-called Backport request. Could this be considered for servicing in
Once this is merged, I'm happy to run both repros and our real crash path against a daily build and report back here, if that helps the backport. |
|
this shape is covered too:
I'll backport it to 11.0 (although, it won't make it to GA as it's too late, only with the first servicing patch), but the bar is high for 10.0, given there is a workaround I am not sure it can meet it. |
|
Excellent, I'm glad it covers that case too, and thank you for taking it to 11.0! On 10.0, I'd like to make the case once, since I think the workaround is weaker than it looks:
If it would help the servicing bar, I'm glad to verify the fix on a 10.0 daily or servicing build against both repros and our production code path. |
Fixes #136373.
Treat delegate receivers of explicit tail calls as escaping. Tailcall helpers require the original delegate and can use it after the caller's frame returns, so stack allocation is unsafe.
Adds a small F# regression test that tailcalls a freshly created delegate with enough arguments to require the helpers.