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 ca309e95875..c71d44a7069 100644 --- a/src/Compiler/AbstractIL/ilreflect.fs +++ b/src/Compiler/AbstractIL/ilreflect.fs @@ -1658,7 +1658,17 @@ 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..0e537038401 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/FixedBindings/FixedBindings.fs @@ -157,6 +157,77 @@ module FixedBindings = IL_001a: ret } """ ] + [] + 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 + { + + .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