From 8aee6cc3b4fba5fa3fce35f930d22548646010aa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Sep 2025 15:49:05 +0000 Subject: [PATCH 1/3] Initial plan From 7393aafce0036737e40bfeaedb90901388920a03 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Sep 2025 16:22:31 +0000 Subject: [PATCH 2/3] Implement fix to suppress .tail. prefix for methods with pinned locals Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/Compiler/AbstractIL/ilreflect.fs | 11 +++- src/Compiler/AbstractIL/ilwrite.fs | 11 +++- .../EmittedIL/FixedBindings/FixedBindings.fs | 57 +++++++++++++++++++ .../PinnedLocalsTailCallSuppression.fs | 29 ++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/PinnedLocalsTailCallSuppression.fs diff --git a/src/Compiler/AbstractIL/ilreflect.fs b/src/Compiler/AbstractIL/ilreflect.fs index ca309e95875..82b44fd1bd3 100644 --- a/src/Compiler/AbstractIL/ilreflect.fs +++ b/src/Compiler/AbstractIL/ilreflect.fs @@ -1658,7 +1658,16 @@ let emitLocal cenv emEnv (ilG: ILGenerator) (local: ILLocal) = let emitILMethodBody cenv modB emEnv (ilG: ILGenerator) (ilmbody: ILMethodBody) = let localBs = Array.map (emitLocal cenv emEnv ilG) (List.toArray ilmbody.Locals) let emEnv = envSetLocals emEnv localBs - emitCode cenv modB emEnv ilG ilmbody.Code + + // Check if any local is pinned, and if so, suppress tail calls to prevent runtime crashes + let hasPinnedLocals = ilmbody.Locals |> List.exists (fun local -> local.IsPinned) + let cenvForEmission = + if hasPinnedLocals then + { cenv with emitTailcalls = false } + else + cenv + + emitCode cenvForEmission modB emEnv ilG ilmbody.Code let emitMethodBody cenv modB emEnv ilG _name (mbody: MethodBody) = match mbody with diff --git a/src/Compiler/AbstractIL/ilwrite.fs b/src/Compiler/AbstractIL/ilwrite.fs index 1d5111daa50..f5d480eac5e 100644 --- a/src/Compiler/AbstractIL/ilwrite.fs +++ b/src/Compiler/AbstractIL/ilwrite.fs @@ -2360,7 +2360,16 @@ let GenILMethodBody mname cenv env (il: ILMethodBody) = [| |] let imports = GenPdbImports cenv il.DebugImports - let requiredStringFixups, seh, code, seqpoints, scopes = Codebuf.EmitMethodCode cenv imports localSigs env mname il.Code + + // Check if any local is pinned, and if so, suppress tail calls to prevent runtime crashes + let hasPinnedLocals = il.Locals |> List.exists (fun local -> local.IsPinned) + let cenvForEmission = + if hasPinnedLocals then + { cenv with emitTailcalls = false } + else + cenv + + let requiredStringFixups, seh, code, seqpoints, scopes = Codebuf.EmitMethodCode cenvForEmission imports localSigs env mname il.Code let codeSize = code.Length use methbuf = ByteBuffer.Create (codeSize * 3) // Do we use the tiny format? diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs index 9f1c66decee..19f7df60b0e 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs @@ -157,6 +157,63 @@ module FixedBindings = IL_001a: ret } """ ] + [] + [] + [] + let ``Tail calls suppressed with pinned locals`` langVersion = + FsFromPath (__SOURCE_DIRECTORY__ ++ "PinnedLocalsTailCallSuppression.fs") + |> withLangVersion langVersion + |> withOptimize + |> withTailcalls + |> withNoWarn 9 + |> compile + |> verifyIL [""" + .method public static int32 tailCallWithPinnedLocal(int32 x) cil managed + { + + .maxstack 4 + .locals init (int32 V_0, + native int V_1, + int32& pinned V_2) + IL_0000: ldarg.0 + IL_0001: ldc.i4.0 + IL_0002: bgt.s IL_0006 + + IL_0004: ldc.i4.0 + IL_0005: ret + + IL_0006: ldarg.0 + IL_0007: stloc.0 + IL_0008: ldloca.s V_0 + IL_000a: stloc.2 + IL_000b: ldloca.s V_0 + IL_000d: conv.i + IL_000e: stloc.1 + IL_000f: ldarg.0 + IL_0010: ldc.i4.1 + IL_0011: sub + IL_0012: call int32 PinnedLocalsTailCallSuppression::tailCallWithPinnedLocal(int32) + IL_0017: ret + } """; """ + .method public static int32 normalTailCall(int32 x) cil managed + { + + .maxstack 4 + IL_0000: ldarg.0 + IL_0001: ldc.i4.0 + IL_0002: bgt.s IL_0006 + + IL_0004: ldc.i4.0 + IL_0005: ret + + IL_0006: ldarg.0 + IL_0007: ldc.i4.1 + IL_0008: sub + IL_0009: tail. + IL_000a: call int32 PinnedLocalsTailCallSuppression::normalTailCall(int32) + IL_000f: ret + } """ ] + [] [] [] diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/PinnedLocalsTailCallSuppression.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/PinnedLocalsTailCallSuppression.fs new file mode 100644 index 00000000000..197b563152d --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/PinnedLocalsTailCallSuppression.fs @@ -0,0 +1,29 @@ +module PinnedLocalsTailCallSuppression +open Microsoft.FSharp.NativeInterop + +// Test case that should NOT emit .tail. prefix because method has pinned locals +let rec tailCallWithPinnedLocal x = + if x <= 0 then + 0 + else + let mutable thing = x + use ptr = fixed &thing + tailCallWithPinnedLocal (x - 1) // This should NOT be a tail call due to pinned local + +// Test case that SHOULD emit .tail. prefix because method has no pinned locals +let rec normalTailCall x = + if x <= 0 then + 0 + else + normalTailCall (x - 1) // This should be a tail call + +// Test case with nested functions - outer has pinned local, inner should not emit tail calls +let outerWithPinnedLocal x = + let mutable thing = x + use ptr = fixed &thing + + let rec innerRecursive y = + if y <= 0 then 0 + else innerRecursive (y - 1) // This should NOT be tail call due to outer pinned local + + innerRecursive x \ No newline at end of file From 3c9be60023a0a28e314c1152e23b3d03f28a4b74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Sep 2025 16:35:35 +0000 Subject: [PATCH 3/3] Add release notes and format code for pinned locals tail call fix Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../.FSharp.Compiler.Service/10.0.100.md | 1 + src/Compiler/AbstractIL/ilreflect.fs | 11 +++--- .../EmittedIL/FixedBindings/FixedBindings.fs | 38 +++++++++++++------ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md index 06b39cc019d..eba728d9a5d 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md @@ -24,6 +24,7 @@ * Fix Show XML doc for enum fields in external metadata ([Issue #17939](https://github.com/dotnet/fsharp/issues/17939#issuecomment-3137410105), [PR #18800](https://github.com/dotnet/fsharp/pull/18800)) * Fix nullable types formatting in `FSharpType.Format` and tooltips to include parentheses. ([PR #18842](https://github.com/dotnet/fsharp/pull/18842)) * TypeMismatchDiagnosticExtendedData: fix expected and actual types calculation. ([Issue ](https://github.com/dotnet/fsharp/pull/18851)) +* Fix tail call emission when method contains pinned locals to prevent runtime crashes and memory corruption. Methods with pinned locals now suppress the `.tail.` prefix to ensure memory safety. ### Changed * Use `errorR` instead of `error` in `CheckDeclarations.fs` when possible. ([PR #18645](https://github.com/dotnet/fsharp/pull/18645)) diff --git a/src/Compiler/AbstractIL/ilreflect.fs b/src/Compiler/AbstractIL/ilreflect.fs index 82b44fd1bd3..c71d44a7069 100644 --- a/src/Compiler/AbstractIL/ilreflect.fs +++ b/src/Compiler/AbstractIL/ilreflect.fs @@ -1658,15 +1658,16 @@ let emitLocal cenv emEnv (ilG: ILGenerator) (local: ILLocal) = let emitILMethodBody cenv modB emEnv (ilG: ILGenerator) (ilmbody: ILMethodBody) = let localBs = Array.map (emitLocal cenv emEnv ilG) (List.toArray ilmbody.Locals) let emEnv = envSetLocals emEnv localBs - + // Check if any local is pinned, and if so, suppress tail calls to prevent runtime crashes let hasPinnedLocals = ilmbody.Locals |> List.exists (fun local -> local.IsPinned) - let cenvForEmission = - if hasPinnedLocals then + + let cenvForEmission = + if hasPinnedLocals then { cenv with emitTailcalls = false } - else + else cenv - + emitCode cenvForEmission modB emEnv ilG ilmbody.Code let emitMethodBody cenv modB emEnv ilG _name (mbody: MethodBody) = diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs index 19f7df60b0e..0e537038401 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs @@ -157,17 +157,31 @@ module FixedBindings = IL_001a: ret } """ ] - [] - [] - [] - let ``Tail calls suppressed with pinned locals`` langVersion = - FsFromPath (__SOURCE_DIRECTORY__ ++ "PinnedLocalsTailCallSuppression.fs") - |> withLangVersion langVersion - |> withOptimize - |> withTailcalls - |> withNoWarn 9 - |> compile - |> verifyIL [""" + [] + let ``Tail calls suppressed with pinned locals`` () = + CompilerAssert.CompileLibraryAndVerifyILWithOptions([| "/optimize"; "/tailcalls" |], + """ +module PinnedLocalsTailCallSuppression +open Microsoft.FSharp.NativeInterop + +// Test case that should NOT emit .tail. prefix because method has pinned locals +let rec tailCallWithPinnedLocal x = + if x <= 0 then + 0 + else + let mutable thing = x + use ptr = fixed &thing + tailCallWithPinnedLocal (x - 1) // This should NOT be a tail call due to pinned local + +// Test case that SHOULD emit .tail. prefix because method has no pinned locals +let rec normalTailCall x = + if x <= 0 then + 0 + else + normalTailCall (x - 1) // This should be a tail call + """, + (fun verifier -> verifier.VerifyIL [ + """ .method public static int32 tailCallWithPinnedLocal(int32 x) cil managed { @@ -212,7 +226,7 @@ module FixedBindings = IL_0009: tail. IL_000a: call int32 PinnedLocalsTailCallSuppression::normalTailCall(int32) IL_000f: ret - } """ ] + } """; ])) [] []