diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index ed1e382d6c7..fdcecdceb10 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Fix `--pathmap` not producing path-independent binaries for portable PDBs: the debug directory was reserved from the original PDB path rather than the mapped one, so output retained padding proportional to the original path's length. ([PR #20509](https://github.com/dotnet/fsharp/pull/20509)) * Fix `NativePtr.stackalloc` nested in a larger expression (e.g. a call argument or the right of an assignment) producing an assembly that throws `InvalidProgramException` at load. ([Issue #8083](https://github.com/dotnet/fsharp/issues/8083), [PR #20302](https://github.com/dotnet/fsharp/pull/20302)) * Fix internal error "Unexpected generalized type variables when compiling an active pattern" when an active pattern is used in a `let` binding whose right-hand side is a generic value, e.g. `let (T) = id`. Such a binding is now checked like the equivalent `match` and is not generalized. ([Issue #16856](https://github.com/dotnet/fsharp/issues/16856), [PR #20383](https://github.com/dotnet/fsharp/pull/20383)) * Fix Release-only (`--optimize+`) `System.InvalidProgramException` from `Seq.collect` / `yield!` over a value-type (struct) collection implementing `seq<'T>` (e.g. `ImmutableArray<_>`) when materialised with `List.ofSeq` / `Seq.toList` / `Seq.toArray` or a list/array comprehension. The collector lowering now boxes a struct sub-collection to `seq<'T>` before calling `AddMany`/`AddManyAndClose` (matching the coercion the type checker already inserts for `yield!`), and uses `unit` as the try/finally result type instead of the body type (removing a spurious `ldnull` store). ([Issue #20203](https://github.com/dotnet/fsharp/issues/20203)) diff --git a/src/Compiler/AbstractIL/ilwrite.fs b/src/Compiler/AbstractIL/ilwrite.fs index b057eef8715..0f83d230c1f 100644 --- a/src/Compiler/AbstractIL/ilwrite.fs +++ b/src/Compiler/AbstractIL/ilwrite.fs @@ -4073,17 +4073,21 @@ let writeBinaryAux (stream: Stream, options: options, modul, normalizeAssemblyRe (if options.deterministic then sizeof_IMAGE_DEBUG_DIRECTORY else 0) ) next - // The debug data is given to us by the PDB writer and appears to - // typically be the type of the data plus the PDB file name. We fill - // this in after we've written the binary. We approximate the size according - // to what PDB writers seem to require and leave extra space just in case... - let debugDataJustInCase = 40 - let debugDataChunk, next = - chunk (align 0x4 (match options.pdbfile with - | None -> 0 - | Some f -> (24 - + System.Text.Encoding.Unicode.GetByteCount f // See bug 748444 - + debugDataJustInCase))) next + // Portable CodeView data contains a 24-byte header followed by the + // mapped UTF-8 path and its terminator. Reserving from the original path + // would retain checkout-specific padding even after applying a path map. + let debugDataSize = + match options.pdbfile with + | None -> 0 + | Some f when options.portablePDB -> + let debugPath = + if options.embeddedPDB then !!(Path.GetFileName f) + else PathMap.apply options.pathMap f + 24 + System.Text.Encoding.UTF8.GetByteCount debugPath + 1 + | Some f -> + // Keep the conservative reservation for the native PDB writer. + 24 + System.Text.Encoding.Unicode.GetByteCount f + 40 // See bug 748444 + let debugDataChunk, next = chunk (align 0x4 debugDataSize) next let debugChecksumPdbChunk, next = chunk (align 0x4 (match pdbInfoOpt with diff --git a/tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/determinism/determinism.fs b/tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/determinism/determinism.fs index be9006bc946..758bb3f2054 100644 --- a/tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/determinism/determinism.fs +++ b/tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/determinism/determinism.fs @@ -184,3 +184,59 @@ module Determinism Assert.Equal(readMvid dll1, readMvid dll2) finally try Directory.Delete(tempRoot, true) with _ -> () + + [] + [] + [] + [] + let ``Path mapping removes original PDB path length from the entire binary`` debugType unicodeMappedPath = + let mappedRoot = + if unicodeMappedPath then "/mapped/" + String.replicate 128 "日本語" + else "/mapped" + let tempRoot = + // The space is deliberate: argument quoting must survive a temp root + // containing spaces (e.g. TEMP under "C:\Users\First Last"). + Path.Combine(Path.GetTempPath(), "fsharp pdb path length " + Guid.NewGuid().ToString("N")) + try + let compileIn directory value = + let workDir = Path.Combine(tempRoot, directory) + Directory.CreateDirectory workDir |> ignore + let source = Path.Combine(workDir, "Library.fs") + let output = Path.Combine(workDir, "Library.dll") + File.WriteAllText(source, $"module Library\nlet value = {value}\n") + let defaultOpts = CompilerAssert.DefaultProjectOptions(TargetFramework.Current).OtherOptions + let result = runFscProcess [ + yield! defaultOpts |> Array.toList + yield "--target:library" + yield "--deterministic+" + yield $"--debug:{debugType}" + yield $"--pathmap:{workDir}={mappedRoot}" + yield $"-o:{output}" + yield source + ] + if result.ExitCode <> 0 then + failwithf "fsc exit %d\nstdout:%s\nstderr:%s" result.ExitCode result.StdOut result.StdErr + use stream = File.OpenRead output + use pe = new PEReader(stream) + let codeViewEntry = + pe.ReadDebugDirectory() + |> Seq.find (fun entry -> entry.Type = DebugDirectoryEntryType.CodeView) + let codeView = pe.ReadCodeViewDebugDirectoryData codeViewEntry + let expectedPdbPath = + if debugType = "embedded" then "Library.pdb" + else mappedRoot + "/Library.pdb" + Assert.Equal(expectedPdbPath, codeView.Path) + output + + let first = compileIn "short" 1 + let second = compileIn "a-much-longer-output-directory" 1 + Assert.True(File.ReadAllBytes first = File.ReadAllBytes second, "Mapped DLL bytes must agree, including debug directory layout") + if debugType = "portable" then + Assert.True( + File.ReadAllBytes(Path.ChangeExtension(first, "pdb")) = File.ReadAllBytes(Path.ChangeExtension(second, "pdb")), + "Mapped portable PDB bytes must agree") + + let changed = compileIn "changed-source" 2 + Assert.False(File.ReadAllBytes first = File.ReadAllBytes changed, "A real source change must still change the binary") + finally + if Directory.Exists tempRoot then Directory.Delete(tempRoot, true) diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index 2d7b7e077cb..4292338e698 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -2412,14 +2412,25 @@ $ code --diff {outFile} {expectedFile} /// Result type for CLI subprocess execution (runFsiProcess / runFscProcess). type ProcessResult = { ExitCode: int; StdOut: string; StdErr: string } + /// Quote an argument that contains whitespace so the child process's command line + /// parser sees it as a single token. Arguments routinely embed paths with spaces, + /// e.g. a temp directory under "C:\Users\First Last", or the .NET Framework + /// reference assemblies under "C:\Program Files (x86)". + let private quoteArg (arg: string) = + if arg |> Seq.exists Char.IsWhiteSpace && not (arg.StartsWith("\"", StringComparison.Ordinal)) then + "\"" + arg + "\"" + else + arg + /// Run an F# tool (FSI or FSC) as a subprocess. Shared helper for runFsiProcess / runFscProcess. let private runToolProcess (toolPath: string) (args: string list) : ProcessResult = + let quotedArgs = args |> List.map quoteArg |> String.concat " " #if NETCOREAPP let exe = TestFramework.initialConfig.DotNetExe - let arguments = toolPath + " " + (args |> String.concat " ") + let arguments = quoteArg toolPath + " " + quotedArgs #else let exe = toolPath - let arguments = args |> String.concat " " + let arguments = quotedArgs #endif let exitCode, stdout, stderr = Commands.executeProcess exe arguments (Directory.GetCurrentDirectory()) { ExitCode = exitCode; StdOut = stdout; StdErr = stderr }