diff --git a/.gitignore b/.gitignore index 49032888dca..822b0ef242d 100644 --- a/.gitignore +++ b/.gitignore @@ -142,6 +142,7 @@ positive.exe # Test result files tests/**/TestResults/*.trx +TestResults/*.trx # Standard output/error files in root directory StandardOutput.txt 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..208c34fcc3b 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md @@ -7,6 +7,7 @@ ### Fixed +* Fix F# compiler to prevent tail call emission when pinned locals are present ([PR #XXXX](https://github.com/dotnet/fsharp/pull/XXXX)) * Fix SignatureHash to include constant values in hash computation ([Issue #18758](https://github.com/dotnet/fsharp/issues/18758)) * Fix parsing errors using anonymous records and units of measures ([PR #18543](https://github.com/dotnet/fsharp/pull/18543)) * Fix parsing errors using anonymous records and code quotations ([PR #18603](https://github.com/dotnet/fsharp/pull/18603)) diff --git a/src/Compiler/Checking/TailCallChecks.fs b/src/Compiler/Checking/TailCallChecks.fs index eba18dbeb4d..bb4bebbcf98 100644 --- a/src/Compiler/Checking/TailCallChecks.fs +++ b/src/Compiler/Checking/TailCallChecks.fs @@ -78,6 +78,9 @@ type cenv = /// Values in module that have been marked [] mustTailCall: Zset + + /// Indicates whether the current method has pinned locals that would prevent tail calls + hasPinnedLocals: bool } override x.ToString() = "" @@ -202,6 +205,7 @@ let CheckForNonTailRecCall (cenv: cenv) expr (tailCall: TailCall) = && not (IsValRefIsDllImport cenv.g vref) && not isCCall && not hasByrefArg + && not cenv.hasPinnedLocals noTailCallBlockers // blockers that will prevent the IL level from emitting a tail instruction else @@ -730,11 +734,26 @@ and CheckBinding cenv alwaysCheckNoReraise ctxt (TBind(v, bindRhs, _) as bind) : | Some info -> info | _ -> ValReprInfo.emptyValData + // Check if this binding introduces a pinned local + let cenv = + if v.IsFixed then + { cenv with hasPinnedLocals = true } + else + cenv + CheckLambdas isTop (Some v) cenv v.ShouldInline valReprInfo tailCall alwaysCheckNoReraise bindRhs v.Range v.Type ctxt and CheckBindings cenv binds = for bind in binds do - CheckBinding cenv false PermitByRefExpr.Yes bind + let (TBind(v, _, _)) = bind + // Update the environment if this binding is fixed + let currentCenv = + if v.IsFixed then + { cenv with hasPinnedLocals = true } + else + cenv + + CheckBinding currentCenv false PermitByRefExpr.Yes bind let CheckModuleBinding cenv (isRec: bool) (TBind _ as bind) = @@ -871,6 +890,7 @@ let CheckImplFile (g: TcGlobals, amap, reportErrors, implFileContents) = stackGuard = StackGuard(PostInferenceChecksStackGuardDepth, "CheckImplFile") amap = amap mustTailCall = Zset.empty valOrder + hasPinnedLocals = false } CheckDefnInModule cenv implFileContents diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 08020ebf21b..32b04b8c90f 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -2689,6 +2689,10 @@ type CodeGenBuffer(m: range, mgbuf: AssemblyBuilder, methodName, alreadyUsedArgs j, true | None -> cgbuf.AllocLocal(ranges, ty, isFixed, canBeReallocd), false + /// Check if any locals have been allocated as pinned/fixed + member _.HasPinnedLocals() = + locals |> Seq.exists (fun (_, _, isFixed, _) -> isFixed) + member _.Close() = let instrs = codebuf.ToArray() @@ -4371,6 +4375,7 @@ and GenApp (cenv: cenv) cgbuf eenv (f, fty, tyargs, curriedArgs, m) sequel = isDllImport, isSelfInit, makesNoCriticalTailcalls, + cgbuf, sequel ) else @@ -4482,11 +4487,15 @@ and CanTailcall isDllImport, isSelfInit, makesNoCriticalTailcalls, + cgbuf: CodeGenBuffer, sequel ) = // Can't tailcall with a struct object arg since it involves a byref // Can't tailcall with a .NET 2.0 generic constrained call since it involves a byref + // Can't tailcall when there are pinned locals since the stack frame must remain alive + let hasPinnedLocals = cgbuf.HasPinnedLocals() + if not hasStructObjArg && Option.isNone ccallInfo @@ -4495,6 +4504,7 @@ and CanTailcall && not isDllImport && not isSelfInit && not makesNoCriticalTailcalls + && not hasPinnedLocals && // We can tailcall even if we need to generate "unit", as long as we're about to throw the value away anyway as par of the return. @@ -4693,7 +4703,7 @@ and GenIndirectCall cenv cgbuf eenv (funcTy, tyargs, curriedArgs, m) sequel = check ilxClosureApps let isTailCall = - CanTailcall(false, None, eenv.withinSEH, hasByrefArg, false, false, false, false, sequel) + CanTailcall(false, None, eenv.withinSEH, hasByrefArg, false, false, false, false, cgbuf, sequel) CountCallFuncInstructions() @@ -5431,6 +5441,7 @@ and GenILCall isDllImport, false, makesNoCriticalTailcalls, + cgbuf, sequel ) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/TailCalls.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/TailCalls.fs index 034ab15f37a..0ab38dcc41e 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/TailCalls.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/TailCalls.fs @@ -235,3 +235,44 @@ let run() = let mutable x = 0 in foo(&x,&x,5) """ ] + [] + let ``TailCall 06 - No tail call with pinned locals``() = + FSharp """ +module TailCall06 +let foo(x:int, y) = printfn "%d" x +let run() = + use ptr = fixed [| 1; 2; 3 |] + foo(42, 5) + """ + |> compileWithTailCalls + |> shouldSucceed + |> verifyIL [ + """ + IL_0040: ldc.i4.s 42 + IL_0042: ldc.i4.5 + IL_0043: call void TailCall06::foo(int32, + !!0) + """ + ] + + [] + let ``TailCall 07 - No tail call with pinned locals in recursive function``() = + FSharp """ +module TailCall07 +let rec countdown n = + use ptr = fixed [| n |] + if n <= 0 then + 0 + else + countdown (n - 1) + """ + |> compileWithTailCalls + |> shouldSucceed + |> verifyIL [ + """ +.locals init (native int V_0, + int32[] V_1, + int32& pinned V_2) + """ + ] +