From c3b9989460ec524e34512fccd56859d2c3874b57 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Tue, 6 Oct 2026 08:22:07 -0700 Subject: [PATCH 1/9] Write the managed ilasm PDB to a file beside the output With /DEBUG or /PDB, the managed ilasm embedded the Portable PDB in the image and named "assembly.pdb" in the CodeView entry. Native ilasm writes the PDB as a separate file named after the output, and records the full path of that file in the CodeView entry. Match native ilasm: - The assembler returns the serialized PDB in CompilationResult.PortablePdb instead of embedding it, and the CLI writes it to .pdb beside the output. - The CodeView entry names the PDB path given in the new Options.PdbFilePath. The CLI passes the full path of .pdb. Library callers that do not set it get OutputFileName with its extension replaced by .pdb, or assembly.pdb when there is no output file name. - A PdbChecksum entry follows the CodeView entry. It holds the SHA-256 hash of the PDB with its 20-byte id zeroed, which is the hash the PortablePdbBuilder id provider computes, so the deterministic id and the checksum come from one hash. - With /DET, a Reproducible entry follows, as native ilasm and the C# compiler write it. - An image built without a PDB has no debug directory. ManagedPEBuilder adds a Reproducible-only directory to a deterministic image when it is given none; the new ILAssemblerPEBuilder suppresses that, for the standard and the vtfixup/export builders. The CLI's file output moves into OutputWriter, which the tests compile: - The image is written and closed first. The PDB is then written to a new temporary file in the same directory and renamed over .pdb, so .pdb never holds a partial PDB. - When the image is written without a PDB, an existing .pdb is deleted only if it belongs to the image that was replaced: that image names the PDB's id in its CodeView entry. Any other file is kept. When the assembly fails and produces no image, nothing is written or deleted; with /ERR, an image written despite errors is handled as after a successful assembly. Native ilasm deletes .PDB whether or not it belongs to the replaced image, and also when the assembly fails, and on Linux that is a different file from .pdb; that can remove a PDB that ilasm did not write, so it is not reproduced. - An output named like its PDB (-OUTPUT=Min.pdb with /DEBUG) is refused instead of being overwritten by the PDB. /PDB still produces only the PDB: it adds no DebuggableAttribute and leaves the JIT settings unchanged. Tests now pin that. The README documents the PDB behaviour, and the doc comments state the DebuggableAttribute modes correctly: /DEBUG=IMPL is 0x103, Default | IgnoreSymbolStoreSequencePoints | DisableOptimizations, and does not enable Edit and Continue. Tests: the unit tests read the PDB from the compilation result and pin the debug directory entries and their order, the CodeView path and id, the checksum, deterministic output and the output writer's file handling, including which PDB it deletes and that it closes the image before touching the PDB. Seeded theories check the same rules over generated programs and option combinations, and the output writer over an enumeration of output names and pre-existing output and PDB states. src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs runs native and managed ilasm end to end. Its managed cases are conditional on CORE_ROOT having the managed ilasm, which is built only where the SDK tools are; cases native ilasm does not satisfy run against the managed ilasm only. --- .../ilasm/PortablePdb/IlasmPdbFileTester.cs | 337 ++++++++++++++ .../PortablePdb/TestFiles/TestPdbFile1.il | 20 + .../PortablePdb/TestFiles/TestPdbFileError.il | 22 + .../TestFiles/TestPdbFileMissingInclude.il | 14 + .../TestFiles/TestPdbFileRecoverableError.il | 27 ++ src/tests/ilasm/ilasm_tests.csproj | 17 + src/tools/ilasm/README.md | 12 + .../Actions/GrammarActions.BuildImage.cs | 107 +++-- .../src/ILAssembler/CompilationResult.cs | 19 +- .../src/ILAssembler/ILAssemblerPEBuilder.cs | 65 +++ src/tools/ilasm/src/ILAssembler/Options.cs | 44 +- .../src/ILAssembler/VTableExportPEBuilder.cs | 4 +- src/tools/ilasm/src/ilasm/IlasmRootCommand.cs | 2 +- src/tools/ilasm/src/ilasm/OutputWriter.cs | 290 ++++++++++++ src/tools/ilasm/src/ilasm/Program.cs | 22 +- .../tests/ILAssembler.Tests/AssemblyTests.cs | 18 +- .../ILAssembler.Tests/CompilerOptionsTests.cs | 171 +++++++- .../DocumentCompilerTestHelpers.cs | 65 ++- .../GeneratedCases/PdbCaseGenerator.cs | 217 +++++++++ .../GeneratedCases/PdbGeneratedCaseTests.cs | 255 +++++++++++ .../ILAssembler.Tests.csproj | 5 +- .../tests/ILAssembler.Tests/ModuleTests.cs | 8 +- .../ILAssembler.Tests/OutputWriterTests.cs | 412 ++++++++++++++++++ .../ILAssembler.Tests/SourceDirectiveTests.cs | 96 ++-- 24 files changed, 2107 insertions(+), 142 deletions(-) create mode 100644 src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs create mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il create mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il create mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il create mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il create mode 100644 src/tools/ilasm/src/ILAssembler/ILAssemblerPEBuilder.cs create mode 100644 src/tools/ilasm/src/ilasm/OutputWriter.cs create mode 100644 src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs create mode 100644 src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs create mode 100644 src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs diff --git a/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs b/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs new file mode 100644 index 00000000000000..6653e3994e4437 --- /dev/null +++ b/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs @@ -0,0 +1,337 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; +using System.Security.Cryptography; +using Xunit; + +namespace IlasmPortablePdbTests +{ + // Runs ilasm end to end and checks the PDB file it writes and the image's debug directory. + // "Native" cases run CORE_ROOT/ilasm. "Managed" cases run CORE_ROOT/managed-ilasm/ilasm, which is built only + // where the SDK tools are (SdkToolsSupported in eng/Subsets.props), so they run only when CORE_ROOT has it. + // Cases that native ilasm does not satisfy run only against the managed ilasm and say why. + // The tests are static: the merged test runner creates an instance of a test class only to dispose it. + public class IlasmPdbFileTester + { + private const string TestDir = "TestFiles"; + private const string Native = "native"; + private const string Managed = "managed"; + + private static readonly string s_ilasmFile = "ilasm" + (OperatingSystem.IsWindows() ? ".exe" : string.Empty); + + // Whether CORE_ROOT has the managed ilasm; the condition of every managed case. + public static bool HasManagedIlasm => + Environment.GetEnvironmentVariable("CORE_ROOT") is { Length: > 0 } coreRoot && + File.Exists(Path.Combine(coreRoot, "managed-ilasm", s_ilasmFile)); + + private static string GetIlasm(string kind) => + IlasmPortablePdbTesterCommon.GetIlasmFullPath( + Environment.GetEnvironmentVariable("CORE_ROOT"), + kind == Managed ? Path.Combine("managed-ilasm", s_ilasmFile) : s_ilasmFile); + + // A fresh directory per test and ilasm, so tests do not see each other's outputs. + private static string CreateOutputDirectory(string testName, string kind) + { + string directory = Path.Combine(Environment.CurrentDirectory, "IlasmPdbFileTester", $"{testName}-{kind}"); + if (Directory.Exists(directory)) + { + Directory.Delete(directory, recursive: true); + } + + Directory.CreateDirectory(directory); + return directory; + } + + private static int RunIlasm(string kind, string ilSource, string outputPath, string switches) + { + string ilSourcePath = Path.Combine(Environment.CurrentDirectory, TestDir, ilSource); + Assert.True(File.Exists(ilSourcePath)); + + var startInfo = new ProcessStartInfo + { + UseShellExecute = false, + WorkingDirectory = Environment.CurrentDirectory, + FileName = GetIlasm(kind), + Arguments = $"-nologo -quiet -dll -output={outputPath} {switches} {ilSourcePath}", + }; + + using Process ilasm = Process.Start(startInfo); + ilasm.WaitForExit(); + return ilasm.ExitCode; + } + + private static PEReader ReadImage(string path) => new PEReader(ImmutableArray.Create(File.ReadAllBytes(path))); + + private static DebugDirectoryEntryType[] GetDebugDirectoryTypes(string imagePath) + { + using PEReader pe = ReadImage(imagePath); + return pe.ReadDebugDirectory().Select(entry => entry.Type).ToArray(); + } + + private static bool HasDebuggableAttribute(string imagePath) + { + using PEReader pe = ReadImage(imagePath); + MetadataReader reader = pe.GetMetadataReader(); + foreach (CustomAttributeHandle handle in reader.GetAssemblyDefinition().GetCustomAttributes()) + { + CustomAttribute attribute = reader.GetCustomAttribute(handle); + if (attribute.Constructor.Kind == HandleKind.MemberReference) + { + MemberReference constructor = reader.GetMemberReference((MemberReferenceHandle)attribute.Constructor); + if (constructor.Parent.Kind == HandleKind.TypeReference && + reader.GetString(reader.GetTypeReference((TypeReferenceHandle)constructor.Parent).Name) == "DebuggableAttribute") + { + return true; + } + } + } + + return false; + } + + // With -DEBUG or -PDB the PDB is a file beside the output, the CodeView entry names its full path and + // carries its id, and nothing is embedded in the image. + [Theory] + [InlineData("-debug")] + [InlineData("-pdb")] + public static void Native_PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string pdbSwitch) => + PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(Native, pdbSwitch); + + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData("-debug")] + [InlineData("-pdb")] + public static void Managed_PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string pdbSwitch) => + PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(Managed, pdbSwitch); + + private static void PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string kind, string pdbSwitch) + { + string directory = CreateOutputDirectory(nameof(PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt) + pdbSwitch, kind); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); + + Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, pdbSwitch)); + + Assert.True(File.Exists(pdb)); + using PEReader pe = ReadImage(dll); + ImmutableArray entries = pe.ReadDebugDirectory(); + Assert.DoesNotContain(entries, entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); + DebugDirectoryEntry codeViewEntry = Assert.Single(entries, entry => entry.Type == DebugDirectoryEntryType.CodeView); + CodeViewDebugDirectoryData codeView = pe.ReadCodeViewDebugDirectoryData(codeViewEntry); + Assert.Equal(pdb, codeView.Path); + + using MetadataReaderProvider pdbProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(File.ReadAllBytes(pdb))); + BlobContentId pdbId = new BlobContentId(pdbProvider.GetMetadataReader().DebugMetadataHeader.Id); + Assert.Equal(pdbId.Guid, codeView.Guid); + Assert.Equal(pdbId.Stamp, codeViewEntry.Stamp); + } + + // The debug directory is CodeView then PdbChecksum, and under -DET a Reproducible entry follows. + [Theory] + [InlineData(false)] + [InlineData(true)] + public static void Native_PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(bool deterministic) => + PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(Native, deterministic); + + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData(false)] + [InlineData(true)] + public static void Managed_PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(bool deterministic) => + PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(Managed, deterministic); + + private static void PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(string kind, bool deterministic) + { + string directory = CreateOutputDirectory(nameof(PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic) + deterministic, kind); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + + Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, deterministic ? "-debug -det" : "-debug")); + + DebugDirectoryEntryType[] expected = deterministic + ? new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible } + : new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum }; + Assert.Equal(expected, GetDebugDirectoryTypes(dll)); + } + + // The PdbChecksum entry is SHA-256 of the PDB file with its 20-byte id zeroed (PE-COFF.md). + // Managed only: native ilasm hashes the PDB before its content is final (#135211). + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData("-debug")] + [InlineData("-debug -det")] + public static void Managed_PdbSwitch_PdbChecksumIsSha256OfThePdbWithItsIdZeroed(string switches) + { + string directory = CreateOutputDirectory(nameof(Managed_PdbSwitch_PdbChecksumIsSha256OfThePdbWithItsIdZeroed) + switches.Replace(" ", string.Empty), Managed); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); + + Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, switches)); + + using PEReader pe = ReadImage(dll); + DebugDirectoryEntry checksumEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.PdbChecksum); + PdbChecksumDebugDirectoryData checksum = pe.ReadPdbChecksumDebugDirectoryData(checksumEntry); + byte[] pdbBytes = File.ReadAllBytes(pdb); + using (MetadataReaderProvider pdbProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(pdbBytes))) + { + DebugMetadataHeader header = pdbProvider.GetMetadataReader().DebugMetadataHeader; + Array.Clear(pdbBytes, header.IdStartOffset, header.Id.Length); + } + + Assert.Equal("SHA256", checksum.AlgorithmName); + Assert.Equal(SHA256.HashData(pdbBytes), checksum.Checksum.ToArray()); + } + + // -PDB produces the PDB without adding a DebuggableAttribute; -DEBUG adds one. + [Theory] + [InlineData("-pdb", false)] + [InlineData("-debug", true)] + public static void Native_PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string pdbSwitch, bool expectAttribute) => + PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(Native, pdbSwitch, expectAttribute); + + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData("-pdb", false)] + [InlineData("-debug", true)] + public static void Managed_PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string pdbSwitch, bool expectAttribute) => + PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(Managed, pdbSwitch, expectAttribute); + + private static void PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string kind, string pdbSwitch, bool expectAttribute) + { + string directory = CreateOutputDirectory(nameof(PdbSwitch_AddsADebuggableAttributeOnlyWithDebug) + pdbSwitch, kind); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + + Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, pdbSwitch)); + + Assert.Equal(expectAttribute, HasDebuggableAttribute(dll)); + } + + // A PDB left by an earlier build is replaced when ilasm writes a new one. + [Fact] + public static void Native_StalePdb_IsReplacedWhenAPdbIsWritten() => StalePdb_IsReplacedWhenAPdbIsWritten(Native); + + [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + public static void Managed_StalePdb_IsReplacedWhenAPdbIsWritten() => StalePdb_IsReplacedWhenAPdbIsWritten(Managed); + + private static void StalePdb_IsReplacedWhenAPdbIsWritten(string kind) + { + string directory = CreateOutputDirectory(nameof(StalePdb_IsReplacedWhenAPdbIsWritten), kind); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); + File.WriteAllBytes(pdb, new byte[] { 0xDE, 0xAD }); + + Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, "-debug")); + + using PEReader pe = ReadImage(dll); + Assert.True(pe.TryOpenAssociatedPortablePdb(dll, path => File.OpenRead(path), out MetadataReaderProvider pdbProvider, out string pdbPath)); + pdbProvider.Dispose(); + Assert.Equal(pdb, pdbPath); + } + + // Without a PDB switch, a successful build deletes the PDB of the image it replaces. + // Managed only: native ilasm deletes .PDB, which on a case-sensitive file system is not this file. + [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + public static void Managed_StalePdb_IsDeletedWhenItsImageIsReplacedWithoutAPdb() + { + string directory = CreateOutputDirectory(nameof(Managed_StalePdb_IsDeletedWhenItsImageIsReplacedWithoutAPdb), Managed); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); + Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, "-debug")); + Assert.True(File.Exists(pdb)); + + Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, string.Empty)); + + Assert.True(File.Exists(dll)); + Assert.False(File.Exists(pdb)); + } + + // Without a PDB switch, a PDB beside an output that did not exist before belongs to no image ilasm replaced, + // so it is kept. + // Managed only: native ilasm deletes .PDB whether or not it belongs to the replaced image. + [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + public static void Managed_UnrelatedPdb_IsKeptWhenNoImageIsReplaced() + { + string directory = CreateOutputDirectory(nameof(Managed_UnrelatedPdb_IsKeptWhenNoImageIsReplaced), Managed); + string otherDll = Path.Combine(directory, "Other.dll"); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); + Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", otherDll, "-debug")); + File.Move(Path.Combine(directory, "Other.pdb"), pdb); + byte[] unrelatedPdb = File.ReadAllBytes(pdb); + + Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, string.Empty)); + + Assert.Equal(unrelatedPdb, File.ReadAllBytes(pdb)); + } + + // When assembly fails, the existing output and PDB are left as they were: after an error in the source + // (TestPdbFileError.il) and after an exception (TestPdbFileMissingInclude.il, whose include is missing). + // Managed only: native ilasm deletes .PDB when assembly fails. + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData("TestPdbFileError.il", "")] + [InlineData("TestPdbFileError.il", "-debug")] + [InlineData("TestPdbFileMissingInclude.il", "-debug")] + public static void Managed_FailedBuild_LeavesTheExistingOutputAndPdbUnchanged(string ilSource, string switches) + { + string name = Path.GetFileNameWithoutExtension(ilSource); + string directory = CreateOutputDirectory($"{nameof(Managed_FailedBuild_LeavesTheExistingOutputAndPdbUnchanged)}-{name}{switches}", Managed); + string dll = Path.Combine(directory, name + ".dll"); + string pdb = Path.Combine(directory, name + ".pdb"); + byte[] existingDll = { 0x4D, 0x5A }; + byte[] existingPdb = { 0xDE, 0xAD }; + File.WriteAllBytes(dll, existingDll); + File.WriteAllBytes(pdb, existingPdb); + + Assert.NotEqual(0, RunIlasm(Managed, ilSource, dll, switches)); + + Assert.Equal(existingDll, File.ReadAllBytes(dll)); + Assert.Equal(existingPdb, File.ReadAllBytes(pdb)); + } + + // With -ERR, an image written despite errors is handled as after a successful build: with -DEBUG its PDB is + // written beside it, and without a PDB switch the PDB of the image it replaces is deleted. + // Managed only: native ilasm deletes .PDB, which on a case-sensitive file system is not this file. + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData("-err -debug")] + [InlineData("-err")] + public static void Managed_ErrorTolerantBuildWithErrors_HandlesThePdbAsASuccessfulBuildDoes(string switches) + { + string directory = CreateOutputDirectory(nameof(Managed_ErrorTolerantBuildWithErrors_HandlesThePdbAsASuccessfulBuildDoes) + switches.Replace(" ", string.Empty), Managed); + string dll = Path.Combine(directory, "TestPdbFileRecoverableError.dll"); + string pdb = Path.Combine(directory, "TestPdbFileRecoverableError.pdb"); + Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, "-debug")); + Assert.NotEqual(0, RunIlasm(Managed, "TestPdbFileRecoverableError.il", dll, "-debug")); + + Assert.Equal(0, RunIlasm(Managed, "TestPdbFileRecoverableError.il", dll, switches)); + + using PEReader pe = ReadImage(dll); + MetadataReader reader = pe.GetMetadataReader(); + Assert.Equal("TestPdbFileRecoverableError", reader.GetString(reader.GetAssemblyDefinition().Name)); + if (switches.Contains("-debug")) + { + Assert.True(pe.TryOpenAssociatedPortablePdb(dll, path => File.OpenRead(path), out MetadataReaderProvider pdbProvider, out string pdbPath)); + pdbProvider.Dispose(); + Assert.Equal(pdb, pdbPath); + } + else + { + Assert.False(File.Exists(pdb)); + } + } + + // An output named like its PDB is refused rather than overwritten by the PDB. + // Managed only: native ilasm writes the image and then overwrites it with the PDB. + [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + public static void Managed_OutputNamedLikeItsPdb_IsRefused() + { + string directory = CreateOutputDirectory(nameof(Managed_OutputNamedLikeItsPdb_IsRefused), Managed); + string output = Path.Combine(directory, "TestPdbFile1.pdb"); + + Assert.NotEqual(0, RunIlasm(Managed, "TestPdbFile1.il", output, "-debug")); + + Assert.False(File.Exists(output)); + } + } +} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il new file mode 100644 index 00000000000000..b48b097d7ae6f1 --- /dev/null +++ b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime +{ +} + +.assembly TestPdbFile1 +{ +} + +.class public auto ansi beforefieldinit TestPdbFileType + extends [System.Runtime]System.Object +{ + .method public hidebysig static int32 Foo() cil managed + { + ldc.i4.1 + ret + } +} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il new file mode 100644 index 00000000000000..ab0195908c4706 --- /dev/null +++ b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// This source does not assemble: the branch target is not defined. + +.assembly extern System.Runtime +{ +} + +.assembly TestPdbFileError +{ +} + +.class public auto ansi beforefieldinit TestPdbFileErrorType + extends [System.Runtime]System.Object +{ + .method public hidebysig static void Foo() cil managed + { + br UNDEFINED_LABEL + ret + } +} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il new file mode 100644 index 00000000000000..5fb82ff05319dd --- /dev/null +++ b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// This source does not assemble: the included file does not exist. + +#include "TestPdbFileMissingInclude.inc" + +.assembly extern System.Runtime +{ +} + +.assembly TestPdbFileMissingInclude +{ +} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il new file mode 100644 index 00000000000000..e03b166645d375 --- /dev/null +++ b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// This source has an error that the error-tolerant option (-ERR) assembles past: the exported type names an +// assembly reference that is not declared. + +.assembly extern System.Runtime +{ +} + +.assembly TestPdbFileRecoverableError +{ +} + +.class extern public MissingType +{ + .assembly extern MissingAssembly +} + +.class public auto ansi beforefieldinit TestPdbFileRecoverableErrorType + extends [System.Runtime]System.Object +{ + .method public hidebysig static void Foo() cil managed + { + ret + } +} diff --git a/src/tests/ilasm/ilasm_tests.csproj b/src/tests/ilasm/ilasm_tests.csproj index 6e4d9a17c241af..6388f8c2c2c7c6 100644 --- a/src/tests/ilasm/ilasm_tests.csproj +++ b/src/tests/ilasm/ilasm_tests.csproj @@ -12,6 +12,7 @@ + @@ -64,6 +65,22 @@ Always TestFiles\%(FileName)%(Extension) + + Always + TestFiles\%(FileName)%(Extension) + + + Always + TestFiles\%(FileName)%(Extension) + + + Always + TestFiles\%(FileName)%(Extension) + + + Always + TestFiles\%(FileName)%(Extension) + Always %(Filename).il diff --git a/src/tools/ilasm/README.md b/src/tools/ilasm/README.md index 203eaecda412cf..7790db4b4b34a6 100644 --- a/src/tools/ilasm/README.md +++ b/src/tools/ilasm/README.md @@ -20,6 +20,18 @@ If you modify any `.g4` grammar files (rare), you must regenerate the parser and This will update the generated files before building the main project. +## Debug information (PDB) + +ilasm writes debug information as a separate Portable PDB file beside the output, as native ilasm does for an output with an extension, and the image gets the same debug directory entry types in the same order. The PDB's content is narrower than native ilasm's: it records only the documents and sequence points given by `.line` directives (native ilasm also records the `.il` source file and its lines) and has no local scopes. + +- **Which switches produce a PDB.** `--debug` (`-DEBUG`, `-DEBUG=IMPL`, `-DEBUG=OPT`) and `--pdb` (`-PDB`). +- **Where it goes.** The PDB is a separate file named after the output with its extension replaced by `.pdb`, in the same directory (`Min.pdb` for `Min.dll` or `Min`). For an output without an extension in a directory whose name has a dot, native ilasm differs: it cuts the whole path at its last dot, so `-OUTPUT=a.b/Min` gives it `a.pdb` beside the `a.b` directory, where ilasm writes `a.b/Min.pdb`. The PDB is not embedded in the image. ilasm writes and closes the image first, then writes the PDB to a new temporary file in the same directory and renames it to `.pdb`, so `.pdb` never holds a partial PDB. If the PDB cannot be written, the run fails and the new image sits beside the previous `.pdb`, if there was one; ilasm tries to delete the temporary file, which can remain if that fails or if the process ends before the rename. +- **An output named like its PDB.** If the output path itself ends in `.pdb` (`-OUTPUT=Min.pdb`) and a PDB is requested, the PDB would overwrite the image, so ilasm reports an error and writes nothing. The comparison ignores case on Windows and macOS. +- **What the image records.** The image's debug directory has these entries, in this order: a CodeView entry that names the full path of the PDB file and carries the PDB's id; a PdbChecksum entry with the SHA-256 hash of the PDB file with its 20-byte id zeroed; and, with `--deterministic`, a Reproducible entry. Debuggers and symbol tools use them to find the PDB and to check that it matches the image. When no PDB is produced, the image has no debug directory, with or without `--deterministic`. +- **`--pdb` compared with `--debug`.** `--pdb` produces the PDB but does not itself add a `DebuggableAttribute`, so on its own it does not change how the JIT compiles the assembly; with `--debug` as well, the attribute is still added. `--debug` adds a `DebuggableAttribute` that disables JIT optimization (`0x101`). `--debug-mode impl` (`-DEBUG=IMPL`) also tells the JIT to use implicit sequence points instead of those in the PDB (`0x103`), and `--debug-mode opt` (`-DEBUG=OPT`) keeps JIT optimization enabled with implicit sequence points (`0x3`). These are native ilasm's values. The attribute is added only when the source declares an assembly (`.assembly`). +- **Stale PDBs.** When ilasm writes an image without a PDB, because none was requested, it then deletes an existing `.pdb` only if that PDB belongs to the image being replaced: the image previously at the output path names the PDB's id in its CodeView entry. This keeps a PDB from an earlier build from sitting beside the new image. Any other file at that path is kept: a PDB beside an output that did not exist before, the PDB of another image, or a file that is not a Portable PDB. An output that is itself named like its PDB (`-OUTPUT=Min.pdb`) is the new image and is not deleted. A PDB that cannot be deleted is left in place without failing the run. When the assembly fails and produces no output, ilasm writes nothing and deletes nothing; with `--error` (`-ERR`) an image is written despite the errors, and its PDB is handled as for a successful assembly. If writing the image itself fails, the existing PDB is left as it was, and the image may be partial. +- **`--deterministic`.** The PDB id and the image's MVID and timestamp are derived from content hashes, so the same input assembled to the same output path gives byte-identical image and PDB files. The CodeView entry records the full path of the PDB, so assembling to a different directory changes the image (the PDB is unchanged). + --- For more details, see the main repository README or contact the maintainers. diff --git a/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs b/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs index c35daba4713e83..86254d6b0e6ac5 100644 --- a/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs +++ b/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs @@ -137,8 +137,8 @@ internal sealed partial class GrammarActions entryPoint = (MethodDefinitionHandle)_entityRegistry.EntryPoint.Handle; } - // Build debug directory if we have any debug info - DebugDirectoryBuilder? debugDirectoryBuilder = BuildDebugDirectory(entryPoint, out _); + // Build the PDB and the debug directory that references it, if a PDB is requested + DebugDirectoryBuilder? debugDirectoryBuilder = BuildDebugDirectory(entryPoint, out ImmutableArray? portablePdb); Func, BlobContentId>? deterministicIdProvider = _options.Deterministic ? GetDeterministicContentId @@ -177,7 +177,7 @@ internal sealed partial class GrammarActions exports: exports, dataLabelFixups: validatedDataLabelFixups); - return (_diagnostics.ToImmutable(), new CompilationResult(peBuilder, mvidFixup)); + return (_diagnostics.ToImmutable(), new CompilationResult(peBuilder, mvidFixup, portablePdb)); } // Apply CorFlags from options or directive @@ -187,7 +187,7 @@ internal sealed partial class GrammarActions standardCorFlags |= CorFlags.Prefers32Bit; } - ManagedPEBuilder standardBuilder = new( + ILAssemblerPEBuilder standardBuilder = new( header, rootBuilder, ilStream, @@ -198,10 +198,13 @@ internal sealed partial class GrammarActions debugDirectoryBuilder: debugDirectoryBuilder, deterministicIdProvider: deterministicIdProvider); - return (_diagnostics.ToImmutable(), new CompilationResult(standardBuilder, mvidFixup)); + return (_diagnostics.ToImmutable(), new CompilationResult(standardBuilder, mvidFixup, portablePdb)); } private static BlobContentId GetDeterministicContentId(IEnumerable content) + => BlobContentId.FromHash(ComputeSha256(content)); + + private static byte[] ComputeSha256(IEnumerable content) { using IncrementalHash hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA256); foreach (Blob blob in content) @@ -209,7 +212,7 @@ private static BlobContentId GetDeterministicContentId(IEnumerable content hash.AppendData(blob.GetBytes()); } - return BlobContentId.FromHash(hash.GetHashAndReset()); + return hash.GetHashAndReset(); } private ImmutableArray ValidateVTableFixups(Machine machine) @@ -554,9 +557,36 @@ private ImmutableArray ValidateExports( } } - private DebugDirectoryBuilder? BuildDebugDirectory(MethodDefinitionHandle entryPoint, out int debugDataSize) + /// + /// Builds the Portable PDB and the debug directory that references it, when a PDB is requested. + /// + /// + /// + /// A PDB is produced when or is set, or when + /// a method has .line sequence points. Otherwise this returns , the image + /// has no debug directory (), and is + /// . + /// + /// + /// The PDB is returned in for the caller to write as a separate file; + /// it is not embedded in the image. The debug directory has these entries, in this order: + /// a CodeView entry that names the PDB file () and carries the PDB id; + /// a PdbChecksum entry holding the SHA-256 hash of the PDB with its 20-byte id zeroed + /// (the PDB Checksum Debug Directory Entry in docs/design/specs/PE-COFF.md); and, with + /// , a Reproducible entry. + /// + /// + /// With , the PDB id is derived from that same hash + /// (), so the same input gives the same PDB + /// bytes. The PDB bytes do not depend on ; the image does, because + /// the CodeView entry records that path, so the same input and the same PDB path give the same image + /// bytes. Without , the id's GUID is random. The checksum is the + /// content hash in both cases. + /// + /// + private DebugDirectoryBuilder? BuildDebugDirectory(MethodDefinitionHandle entryPoint, out ImmutableArray? portablePdb) { - debugDataSize = 0; + portablePdb = null; // Check if we have any methods with debug info bool hasDebugInfo = false; @@ -583,40 +613,63 @@ private ImmutableArray ValidateExports( // Get row counts from main metadata for the portable PDB var typeSystemRowCounts = _metadataBuilder.GetRowCounts(); - Func, BlobContentId> pdbIdProvider = _options.Deterministic - ? GetDeterministicContentId - : _ => new BlobContentId(Guid.NewGuid(), 0x04030201); + // PortablePdbBuilder hands the id provider the serialized PDB with its 20-byte id zeroed. + // The SHA-256 of that content is the PdbChecksum value, and the deterministic id. + byte[]? pdbContentHash = null; + BlobContentId PdbIdProvider(IEnumerable content) + { + pdbContentHash = ComputeSha256(content); + return _options.Deterministic + ? BlobContentId.FromHash(pdbContentHash) + : new BlobContentId(Guid.NewGuid(), 0x04030201); + } // Create the portable PDB var pdbBuilder = new PortablePdbBuilder( _pdbBuilder, typeSystemRowCounts, entryPoint, - idProvider: pdbIdProvider); + idProvider: PdbIdProvider); var pdbBlob = new BlobBuilder(); var pdbContentId = pdbBuilder.Serialize(pdbBlob); + byte[] pdbChecksum = pdbContentHash + ?? throw new InvalidOperationException("The Portable PDB was serialized without calling its id provider."); - // Create debug directory with embedded PDB + // Reference the PDB file from the image: CodeView, then PdbChecksum, then (deterministic only) + // Reproducible, as native ilasm and the C# compiler do. var debugDirectoryBuilder = new DebugDirectoryBuilder(); debugDirectoryBuilder.AddCodeViewEntry( - $"assembly.pdb", + GetPdbFilePath(), pdbContentId, pdbBuilder.FormatVersion); - debugDirectoryBuilder.AddEmbeddedPortablePdbEntry(pdbBlob, pdbBuilder.FormatVersion); + debugDirectoryBuilder.AddPdbChecksumEntry( + "SHA256", + ImmutableArray.Create(pdbChecksum)); + if (_options.Deterministic) + { + debugDirectoryBuilder.AddReproducibleEntry(); + } - // Calculate debug data size: - // 2 debug directory entries (28 bytes each) + CodeView data (~24 bytes) + Embedded PDB data (compressed pdbBlob + 8 header) - // CodeView entry: signature (4) + guid (16) + age (4) + path (variable, ~12 for "assembly.pdb\0") - const int debugDirEntrySize = 28; - int codeViewDataSize = 4 + 16 + 4 + "assembly.pdb".Length + 1; // signature + guid + age + path + null - int embeddedPdbHeaderSize = 8; // MPDB signature (4) + uncompressed size (4) - // The embedded PDB is compressed, estimate conservatively as same size - int embeddedPdbDataSize = embeddedPdbHeaderSize + pdbBlob.Count; + portablePdb = pdbBlob.ToImmutableArray(); + return debugDirectoryBuilder; + } - debugDataSize = (2 * debugDirEntrySize) + codeViewDataSize + embeddedPdbDataSize; + /// + /// Gets the PDB path recorded in the CodeView entry: when set, + /// otherwise with its extension replaced by .pdb, + /// otherwise assembly.pdb. + /// + private string GetPdbFilePath() + { + if (_options.PdbFilePath is not null) + { + return _options.PdbFilePath; + } - return debugDirectoryBuilder; + return _options.OutputFileName is not null + ? Path.ChangeExtension(_options.OutputFileName, ".pdb") + : "assembly.pdb"; } private void BuildPdbMetadata() @@ -744,7 +797,7 @@ private static BlobBuilder EncodeSequencePoints(List private void ApplyDebuggableAttribute() { @@ -758,7 +811,7 @@ private void ApplyDebuggableAttribute() // EnableEditAndContinue = 0x04, DisableOptimizations = 0x100 const int DebuggingModesDefault = 0x101; // Default | DisableOptimizations const int DebuggingModesOpt = 0x03; // Default | IgnoreSymbolStoreSequencePoints - const int DebuggingModesImpl = 0x103; // Default | DisableOptimizations | EnableEditAndContinue + const int DebuggingModesImpl = 0x103; // Default | IgnoreSymbolStoreSequencePoints | DisableOptimizations int debuggingModes = _options.DebugMode switch { diff --git a/src/tools/ilasm/src/ILAssembler/CompilationResult.cs b/src/tools/ilasm/src/ILAssembler/CompilationResult.cs index 1edfbdee73db84..261f8b65dfa083 100644 --- a/src/tools/ilasm/src/ILAssembler/CompilationResult.cs +++ b/src/tools/ilasm/src/ILAssembler/CompilationResult.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Immutable; using System.Reflection.Metadata; using System.Reflection.PortableExecutable; @@ -12,12 +13,28 @@ public sealed class CompilationResult private readonly PEBuilder _peBuilder; private readonly Blob _mvidFixup; - internal CompilationResult(PEBuilder peBuilder, Blob mvidFixup) + internal CompilationResult(PEBuilder peBuilder, Blob mvidFixup, ImmutableArray? portablePdb) { _peBuilder = peBuilder; _mvidFixup = mvidFixup; + PortablePdb = portablePdb; } + /// + /// Gets the serialized Portable PDB for the image, or when no PDB was requested. + /// + /// + /// A PDB is produced when or is set, or when + /// the source has .line sequence points. It is not embedded in the image: the image's debug directory + /// holds a CodeView entry that names the PDB path and carries this PDB's id, followed by a PdbChecksum + /// entry with the SHA-256 hash of these bytes with the 20-byte PDB id zeroed, and, with + /// , a Reproducible entry. The PDB path is + /// when set; otherwise it is with its + /// extension replaced by .pdb, or assembly.pdb when no output file name is set. + /// The caller writes these bytes to the path named in the CodeView entry. + /// + public ImmutableArray? PortablePdb { get; } + /// Serializes the compiled image into the specified builder. /// The builder that receives the serialized image. /// The content identifier of the serialized image. diff --git a/src/tools/ilasm/src/ILAssembler/ILAssemblerPEBuilder.cs b/src/tools/ilasm/src/ILAssembler/ILAssemblerPEBuilder.cs new file mode 100644 index 00000000000000..ab7dfd1dab9e0c --- /dev/null +++ b/src/tools/ilasm/src/ILAssembler/ILAssemblerPEBuilder.cs @@ -0,0 +1,65 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using System.Reflection.PortableExecutable; + +namespace ILAssembler; + +/// +/// The for every image the assembler writes. The image has a debug +/// directory only when the assembler supplies one. +/// +/// +/// Given no , a deterministic adds a debug +/// directory with a Reproducible entry of its own. Native ilasm writes a debug directory only with a PDB, so +/// this builder passes an empty instead, and it clears the PE header's debug +/// directory entry whenever the directory has no entries, which an empty builder would otherwise leave with a +/// non-zero address and size 0: an image built without a PDB has no debug directory, deterministic or not. +/// +internal class ILAssemblerPEBuilder : ManagedPEBuilder +{ + public ILAssemblerPEBuilder( + PEHeaderBuilder header, + MetadataRootBuilder metadataRootBuilder, + BlobBuilder ilStream, + BlobBuilder? mappedFieldData = null, + BlobBuilder? managedResources = null, + ResourceSectionBuilder? nativeResources = null, + DebugDirectoryBuilder? debugDirectoryBuilder = null, + int strongNameSignatureSize = 128, + MethodDefinitionHandle entryPoint = default, + CorFlags flags = CorFlags.ILOnly, + Func, BlobContentId>? deterministicIdProvider = null) + : base( + header, + metadataRootBuilder, + ilStream, + mappedFieldData, + managedResources, + nativeResources, + debugDirectoryBuilder ?? new DebugDirectoryBuilder(), + strongNameSignatureSize, + entryPoint, + flags, + deterministicIdProvider) + { + } + + protected override PEDirectoriesBuilder GetDirectories() + { + PEDirectoriesBuilder directories = base.GetDirectories(); + + // A debug directory without entries is no debug directory. Clearing it is idempotent, so a later call + // (VTableExportPEBuilder calls this again while serializing .reloc) sees the same directories. + if (directories.DebugTable.Size == 0) + { + directories.DebugTable = default; + } + + return directories; + } +} diff --git a/src/tools/ilasm/src/ILAssembler/Options.cs b/src/tools/ilasm/src/ILAssembler/Options.cs index ca86c3750778c5..9dfe54a13458a1 100644 --- a/src/tools/ilasm/src/ILAssembler/Options.cs +++ b/src/tools/ilasm/src/ILAssembler/Options.cs @@ -11,13 +11,15 @@ namespace ILAssembler public enum DebugMode { /// - /// Implicit sequence points - enables edit and continue. - /// Produces DebuggingModes = Default | DisableOptimizations | EnableEditAndContinue (0x103). + /// Implicit sequence points: JIT optimization is disabled, and the JIT uses implicit sequence points + /// rather than those in the PDB. Edit and Continue is not enabled. + /// Produces DebuggingModes = Default | IgnoreSymbolStoreSequencePoints | DisableOptimizations (0x103). /// Impl, /// - /// Optimized debugging - enables JIT optimization while preserving debug info. + /// Optimized debugging - enables JIT optimization while preserving debug info; the JIT uses implicit + /// sequence points rather than those in the PDB. /// Produces DebuggingModes = Default | IgnoreSymbolStoreSequencePoints (0x03). /// Opt @@ -101,21 +103,51 @@ public sealed class Options public string? MetadataVersion { get; set; } /// - /// Enable debug mode: create PDB, disable JIT optimization. + /// Enable debug mode: produce a Portable PDB and add a DebuggableAttribute to the assembly + /// that disables JIT optimization (see for the other settings). /// + /// + /// The DebuggableAttribute is added only when the source declares an assembly (.assembly); + /// a module without one gets no attribute. + /// The PDB is returned in , not embedded in the image. + /// The image references it through its debug directory; see . + /// public bool Debug { get; set; } /// - /// Create PDB file without enabling debug info tracking. + /// Produce a Portable PDB without enabling debug info tracking: this option does not itself add a + /// DebuggableAttribute, so on its own it leaves the JIT settings of the assembly unchanged. /// + /// + /// Combined with or , the attribute those options add is + /// still added. The PDB is returned in , not embedded in + /// the image. + /// public bool Pdb { get; set; } /// - /// Debug mode: Impl for implicit sequence points, Opt to enable JIT optimization. + /// Debug mode, as native ilasm's /DEBUG=IMPL and /DEBUG=OPT: selects the modes of the + /// DebuggableAttribute. /// When null with Debug=true, uses default (DisableOptimizations). /// + /// + /// A non-null value adds a DebuggableAttribute with the selected debugging modes, + /// as does, and likewise only when the source declares an assembly. + /// public DebugMode? DebugMode { get; set; } + /// + /// The path of the Portable PDB file, recorded in the image's CodeView debug directory entry + /// when a PDB is produced (see ). + /// + /// + /// The assembler does not write this file; the caller writes + /// to it. The command-line tool passes the full path of the output file with its extension + /// replaced by .pdb. When null, the CodeView entry names with + /// its extension replaced by .pdb, or assembly.pdb when no output file name is set. + /// + public string? PdbFilePath { get; set; } + /// /// Override the name of the compiled assembly. /// diff --git a/src/tools/ilasm/src/ILAssembler/VTableExportPEBuilder.cs b/src/tools/ilasm/src/ILAssembler/VTableExportPEBuilder.cs index db9a394c34ace7..d294b4939753cc 100644 --- a/src/tools/ilasm/src/ILAssembler/VTableExportPEBuilder.cs +++ b/src/tools/ilasm/src/ILAssembler/VTableExportPEBuilder.cs @@ -15,10 +15,10 @@ namespace ILAssembler; /// -/// A PE builder that extends with VTable fixups, +/// A PE builder that extends with VTable fixups, /// unmanaged exports, and data label reference fixups. /// -internal sealed class VTableExportPEBuilder : ManagedPEBuilder +internal sealed class VTableExportPEBuilder : ILAssemblerPEBuilder { private const string RelocationSectionName = ".reloc"; private const string SDataSectionName = ".sdata"; diff --git a/src/tools/ilasm/src/ilasm/IlasmRootCommand.cs b/src/tools/ilasm/src/ilasm/IlasmRootCommand.cs index a13a12a8d1841f..a0c0dc33375d12 100644 --- a/src/tools/ilasm/src/ilasm/IlasmRootCommand.cs +++ b/src/tools/ilasm/src/ilasm/IlasmRootCommand.cs @@ -23,7 +23,7 @@ internal sealed class IlasmRootCommand : RootCommand new("--debug", "-g") { Description = "Disable JIT optimization, create PDB file, use sequence points from PDB" }; public Option DebugMode { get; } = - new("--debug-mode") { Description = "Debug mode: 'impl' (implicit sequence points) or 'opt' (enable JIT optimization)" }; + new("--debug-mode") { Description = "Debug mode: 'impl' (disable JIT optimization, JIT uses implicit sequence points instead of those in the PDB) or 'opt' (enable JIT optimization, JIT uses implicit sequence points)" }; public Option Optimize { get; } = new("--optimize", "-O") { Description = "Optimize long instructions to short" }; diff --git a/src/tools/ilasm/src/ilasm/OutputWriter.cs b/src/tools/ilasm/src/ilasm/OutputWriter.cs new file mode 100644 index 00000000000000..03c7eb780a5c64 --- /dev/null +++ b/src/tools/ilasm/src/ilasm/OutputWriter.cs @@ -0,0 +1,290 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; + +namespace ILAssembler; + +/// +/// Writes an assembled image and its Portable PDB to disk. +/// +/// +/// +/// The PDB is a separate file beside the output (), as with native ilasm. +/// is called only when the assembly +/// produced an image: when it succeeded, or despite errors with the error-tolerant option (--error, +/// /ERR), in which case the image and PDB are handled exactly as for a successful assembly. When the assembly +/// produces no image, the tool calls nothing here and leaves the existing output and PDB as they are. +/// +/// +/// The image is written and closed before the PDB is touched, so a failure while writing or closing the image +/// leaves the existing PDB as it was; the image itself may then be partial. The PDB is written to a new temporary +/// file in the same directory and then renamed over <output>.pdb, so the PDB path holds either the +/// file that was there before or the complete new PDB. If writing or renaming the temporary file fails, the run +/// fails with the new image already written, and deleting the temporary file is attempted; it can remain if that +/// deletion fails or the process ends before the rename. Because the PDB is renamed into place, a symbolic or hard +/// link at <output>.pdb is replaced by a regular file and the link's target keeps its old content; +/// deleting a PDB (below) likewise removes a symbolic link, not its target. +/// +/// +/// When no PDB is produced, an existing <output>.pdb is deleted after the image is written only if it +/// belongs to the image being replaced: the image previously at the output path has a CodeView entry whose PDB id +/// (GUID and stamp) equals the id of that Portable PDB. Any other file at the PDB path is left in place, including +/// a PDB beside an output that did not exist before, a PDB of another image, and a file that is not a Portable PDB. +/// +/// +internal static class OutputWriter +{ + /// + /// Gets the full path of the PDB for an output: the output path with its extension replaced by .pdb. + /// + public static string GetPdbPath(string outputPath) => Path.GetFullPath(Path.ChangeExtension(outputPath, ".pdb")); + + /// + /// Gets whether names the output file itself, as it does for an output named + /// Min.pdb. + /// + /// + /// The comparison ignores case on Windows and macOS, whose default file systems do, so that + /// Min.PDB and Min.pdb are treated as the same file there; elsewhere it is exact. + /// + public static bool IsOutputPath(string pdbPath, string outputPath) => + string.Equals( + pdbPath, + Path.GetFullPath(outputPath), + OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); + + /// + /// Writes the image to , then writes to + /// or, when there is no PDB, deletes the PDB of the image that was replaced. + /// + /// The path of the image. + /// The path of the PDB, from . + /// Writes the image to the stream it is given. + /// The PDB, or when none was produced. + /// What was written or deleted. + /// + /// When a PDB is produced and names the output file itself, nothing is written + /// and is returned. When no PDB is produced and + /// names the output file, that file is the new image and is not deleted. + /// A PDB that belongs to the replaced image but cannot be deleted is left in place without failing the write. + /// + public static OutputWriteResult Write(string outputPath, string pdbPath, Action writeImage, ImmutableArray? portablePdb) => + Write(outputPath, pdbPath, writeImage, portablePdb, File.Create); + + /// + /// , with the stream the image is + /// written to created by . + /// + internal static OutputWriteResult Write( + string outputPath, + string pdbPath, + Action writeImage, + ImmutableArray? portablePdb, + Func createImageStream) + { + bool pdbPathIsOutputPath = IsOutputPath(pdbPath, outputPath); + if (portablePdb is not null && pdbPathIsOutputPath) + { + return OutputWriteResult.PdbWouldOverwriteOutput; + } + + // Which PDB the image being replaced refers to, read before the image is overwritten. Without a new + // PDB, only that PDB is deleted. + BlobContentId? replacedImagePdbId = portablePdb is null && !pdbPathIsOutputPath ? TryReadCodeViewPdbId(outputPath) : null; + + // Write and close the image before touching the PDB. If this throws, the PDB is left as it was. + using (Stream imageStream = createImageStream(outputPath)) + { + writeImage(imageStream); + } + + if (portablePdb is { } pdb) + { + WritePdb(pdbPath, GetTemporaryPdbPath(pdbPath), pdb); + return OutputWriteResult.ImageAndPdbWritten; + } + + if (replacedImagePdbId is { } pdbId && TryReadPortablePdbId(pdbPath) == pdbId && TryDelete(pdbPath)) + { + return OutputWriteResult.ImageWrittenStalePdbDeleted; + } + + return OutputWriteResult.ImageWritten; + } + + /// + /// Gets the id (GUID and stamp) of the PDB that the image at refers to in its + /// first CodeView entry, or when there is no such file, it cannot be read, or + /// finds no id in it. + /// + internal static BlobContentId? TryReadCodeViewPdbId(string imagePath) + { + try + { + if (!File.Exists(imagePath)) + { + return null; + } + + using FileStream stream = File.OpenRead(imagePath); + return TryReadCodeViewPdbId(stream); + } + catch (Exception ex) when (ex is BadImageFormatException or IOException or UnauthorizedAccessException) + { + return null; + } + } + + /// + /// Gets the id (GUID and stamp) of the PDB that the image in refers to in its first + /// CodeView entry, or when the stream is longer than bytes, + /// holds a COFF object file rather than a PE image, or the image has no CodeView entry. + /// + /// The stream holds neither a PE image nor a COFF object file. + internal static BlobContentId? TryReadCodeViewPdbId(Stream stream) + { + // System.Reflection.Metadata reads at most int.MaxValue bytes and throws ArgumentException for a longer stream. + if (stream.Length > int.MaxValue) + { + return null; + } + + using var peReader = new PEReader(stream, PEStreamOptions.PrefetchEntireImage | PEStreamOptions.LeaveOpen); + + // A file that does not start with "MZ" is read as a COFF object file (twenty zero bytes read as one). It has + // no PE header and so no debug directory, and ReadDebugDirectory requires the PE header. + if (peReader.PEHeaders.IsCoffOnly) + { + return null; + } + + foreach (DebugDirectoryEntry entry in peReader.ReadDebugDirectory()) + { + if (entry.Type == DebugDirectoryEntryType.CodeView) + { + return new BlobContentId(peReader.ReadCodeViewDebugDirectoryData(entry).Guid, entry.Stamp); + } + } + + return null; + } + + /// + /// Gets the id of the Portable PDB at , or when there is no + /// such file, it cannot be read, or finds no id in it. + /// + internal static BlobContentId? TryReadPortablePdbId(string pdbPath) + { + try + { + if (!File.Exists(pdbPath)) + { + return null; + } + + using FileStream stream = File.OpenRead(pdbPath); + return TryReadPortablePdbId(stream); + } + catch (Exception ex) when (ex is BadImageFormatException or IOException or UnauthorizedAccessException) + { + return null; + } + } + + /// + /// Gets the id of the Portable PDB in , or when the stream is + /// longer than bytes or holds metadata that is not a Portable PDB. + /// + /// The stream does not hold metadata. + internal static BlobContentId? TryReadPortablePdbId(Stream stream) + { + // System.Reflection.Metadata reads at most int.MaxValue bytes and throws ArgumentException for a longer stream. + if (stream.Length > int.MaxValue) + { + return null; + } + + using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbStream( + stream, + MetadataStreamOptions.PrefetchMetadata | MetadataStreamOptions.LeaveOpen); + DebugMetadataHeader? header = provider.GetMetadataReader().DebugMetadataHeader; + return header is null ? null : new BlobContentId(header.Id); + } + + /// + /// Gets a path in the PDB's directory for the temporary file the PDB is written to before it is renamed. + /// + internal static string GetTemporaryPdbPath(string pdbPath) => + Path.Combine(Path.GetDirectoryName(pdbPath)!, $"{Path.GetFileName(pdbPath)}.{Path.GetRandomFileName()}.tmp"); + + /// + /// Writes to a new file at and renames it over + /// . + /// + /// + /// The temporary file is created only if no file exists at . If the write or + /// the rename fails, deleting the temporary file is attempted only when this call created it, and + /// is left as it was. + /// + internal static void WritePdb(string pdbPath, string temporaryPath, ImmutableArray pdb) + { + bool created = false; + try + { + using (var stream = new FileStream(temporaryPath, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + { + created = true; + stream.Write(pdb.AsSpan()); + } + + File.Move(temporaryPath, pdbPath, overwrite: true); + } + catch + { + if (created) + { + TryDelete(temporaryPath); + } + + throw; + } + } + + private static bool TryDelete(string path) + { + try + { + File.Delete(path); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return false; + } + } +} + +/// +/// What wrote or deleted. +/// +internal enum OutputWriteResult +{ + /// The image was written. No PDB was produced, and no PDB was deleted. + ImageWritten, + + /// + /// The image was written. No PDB was produced, and the PDB of the image it replaced was deleted. + /// + ImageWrittenStalePdbDeleted, + + /// The image and then its PDB were written. + ImageAndPdbWritten, + + /// Nothing was written: the PDB path names the output file itself. + PdbWouldOverwriteOutput, +} diff --git a/src/tools/ilasm/src/ilasm/Program.cs b/src/tools/ilasm/src/ilasm/Program.cs index 0e058a3f988808..c63c3430e33a0e 100644 --- a/src/tools/ilasm/src/ilasm/Program.cs +++ b/src/tools/ilasm/src/ilasm/Program.cs @@ -63,6 +63,10 @@ public int Run() int exitCode = 0; try { + // As in native ilasm, the PDB is .pdb beside the output, and the + // image's CodeView entry records its full path. + string pdbPath = OutputWriter.GetPdbPath(outputPath); + // Report each file being assembled foreach (string file in inputFiles) { @@ -161,6 +165,7 @@ public int Run() options.PseudoAttributes = Get(_command.PseudoAttributes); options.Fold = Get(_command.Fold); options.OutputFileName = Path.GetFileName(outputPath); + options.PdbFilePath = pdbPath; // Set up include path for #include directive resolution string? includePath = Get(_command.IncludePath); @@ -248,11 +253,22 @@ byte[] LoadResource(string path) return 1; } - // Write output - using var outputStream = File.Create(outputPath); + // Write the image, then the PDB beside it (or, when there is none, remove the PDB of the image + // that was replaced). + // Every failure above returns before this, leaving the existing files as they are. var blobBuilder = new BlobBuilder(); compilationResult.Serialize(blobBuilder); - blobBuilder.WriteContentTo(outputStream); + OutputWriteResult written = OutputWriter.Write( + outputPath, + pdbPath, + stream => blobBuilder.WriteContentTo(stream), + compilationResult.PortablePdb); + if (written == OutputWriteResult.PdbWouldOverwriteOutput) + { + Console.Error.WriteLine($"Error: The PDB file '{pdbPath}' would overwrite the output file"); + Console.Error.WriteLine("***** FAILURE *****"); + return 1; + } if (hasErrors) { diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/AssemblyTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/AssemblyTests.cs index 89e03a857314ea..fd3496b447f5c3 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/AssemblyTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/AssemblyTests.cs @@ -454,10 +454,22 @@ .line 1 "deterministic.il" Pdb = true, }; - ImmutableArray firstPdb = DocumentCompilerTestHelpers.CompileAndGetEmbeddedPortablePdb(source, options); - ImmutableArray secondPdb = DocumentCompilerTestHelpers.CompileAndGetEmbeddedPortablePdb(source, options); + CompilationResult first = DocumentCompilerTestHelpers.CompileAndGetResult(source, options); + CompilationResult second = DocumentCompilerTestHelpers.CompileAndGetResult(source, options); + ImmutableArray firstPdb = DocumentCompilerTestHelpers.GetPortablePdb(first); + ImmutableArray secondPdb = DocumentCompilerTestHelpers.GetPortablePdb(second); Assert.Equal(firstPdb, secondPdb); + Assert.Equal(GetCodeViewPdbId(first), GetCodeViewPdbId(second)); + } + + private static (Guid Guid, uint Stamp) GetCodeViewPdbId(CompilationResult result) + { + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + DebugDirectoryEntry codeViewEntry = Assert.Single( + pe.ReadDebugDirectory(), + entry => entry.Type == DebugDirectoryEntryType.CodeView); + return (pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Guid, codeViewEntry.Stamp); } [Theory] @@ -483,7 +495,7 @@ .method public static void Main() cil managed Pdb = true, }; - ImmutableArray pdb = DocumentCompilerTestHelpers.CompileAndGetEmbeddedPortablePdb(source, options); + ImmutableArray pdb = DocumentCompilerTestHelpers.CompileAndGetPortablePdb(source, options); using MetadataReaderProvider pdbProvider = MetadataReaderProvider.FromPortablePdbImage(pdb); BlobContentId pdbId = new(pdbProvider.GetMetadataReader().DebugMetadataHeader!.Id); diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs index 488dcb482a0658..2ec109c2494815 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs @@ -2,12 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Reflection; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using System.Reflection.PortableExecutable; +using System.Security.Cryptography; using Xunit; using DocumentCompilerTestHelpers = ILAssembler.Tests.DocumentCompilerTestHelpers; @@ -44,6 +46,20 @@ .class public auto ansi Test extends [mscorlib]System.Object reader.GetBlobBytes(attribute.Value)); } + private const string PortablePdbSource = """ + .assembly extern System.Runtime { } + .assembly test { } + .class public auto ansi beforefieldinit Test + { + .method public static void M() cil managed + { + .line 10 'test.cs' + nop + ret + } + } + """; + [Fact] public void AssemblyNameMetadataVersionAndModuleNameOptions_AreApplied() { @@ -364,7 +380,7 @@ .method public static void DifferentBounds() cil managed } [Fact] - public void PdbOption_EmitsEmbeddedPortablePdbWithoutLineDirectives() + public void PdbOption_ProducesPortablePdbWithoutLineDirectives() { string source = """ .assembly test { } @@ -378,17 +394,160 @@ .method public static void M() cil managed } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options { Pdb = true }); - - var debugDirectory = pe.ReadDebugDirectory(); - var embeddedPdbEntry = debugDirectory.Single(entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdbEntry); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Pdb = true }); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); Assert.Empty(pdbReader.Documents); Assert.NotEmpty(pdbReader.MethodDebugInformation); } + [Fact] + public void PdbOption_DoesNotAddDebuggableAttribute() + { + string source = """ + .assembly extern System.Runtime { } + .assembly test { } + .class public auto ansi beforefieldinit Test + { + } + """; + + using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options { Pdb = true }); + var reader = pe.GetMetadataReader(); + + Assert.Empty(reader.GetAssemblyDefinition().GetCustomAttributes()); + } + + [Fact] + public void WithoutDebugOrPdb_DeterministicImageWithExportsHasNoDebugDirectory() + { + // .vtfixup and .export images are built by VTableExportPEBuilder rather than the standard builder; + // without a PDB they too have no debug directory, not even ManagedPEBuilder's default Reproducible one. + string source = """ + .assembly test { } + .assembly extern mscorlib { } + .data VT = int32(0) + .vtfixup [1] int32 fromunmanaged at VT + .class public auto ansi Test extends [mscorlib]System.Object + { + .method public static void ExportedMethod() cil managed + { + .vtentry 1 : 1 + .export [1] + ret + } + } + """; + + using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options { Deterministic = true }); + DirectoryEntry debugTable = pe.PEHeaders.PEHeader!.DebugTableDirectory; + + Assert.Contains(pe.PEHeaders.SectionHeaders, section => section.Name == ".sdata"); + Assert.Equal((0, 0), (debugTable.RelativeVirtualAddress, debugTable.Size)); + } + + [Fact] + public void PortablePdb_IsReferencedByCodeViewThenPdbChecksumAndNotEmbedded() + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options { Debug = true }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + + Assert.Equal( + new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum }, + pe.ReadDebugDirectory().Select(entry => entry.Type)); + } + + [Fact] + public void PortablePdb_Deterministic_IsReferencedByCodeViewThenPdbChecksumThenReproducible() + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options { Debug = true, Deterministic = true }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + + Assert.Equal( + new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible }, + pe.ReadDebugDirectory().Select(entry => entry.Type)); + } + + [Fact] + public void PortablePdb_CodeViewEntry_NamesPdbFilePath() + { + string pdbFilePath = Path.Combine(Path.GetTempPath(), "out", "Output.pdb"); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options + { + Debug = true, + OutputFileName = "Output.dll", + PdbFilePath = pdbFilePath, + }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + DebugDirectoryEntry codeViewEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.CodeView); + + Assert.Equal(pdbFilePath, pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Path); + } + + [Theory] + [InlineData("Output.dll", "Output.pdb")] + [InlineData("Output", "Output.pdb")] + [InlineData(null, "assembly.pdb")] + public void PortablePdb_CodeViewEntry_WithoutPdbFilePath_NamesPdbAfterOutputFileName(string? outputFileName, string expectedPath) + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options + { + Debug = true, + OutputFileName = outputFileName, + }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + DebugDirectoryEntry codeViewEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.CodeView); + + Assert.Equal(expectedPath, pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Path); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void PortablePdb_CodeViewEntry_CarriesPdbId(bool deterministic) + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options + { + Debug = true, + Deterministic = deterministic, + }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + DebugDirectoryEntry codeViewEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.CodeView); + CodeViewDebugDirectoryData codeView = pe.ReadCodeViewDebugDirectoryData(codeViewEntry); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); + BlobContentId pdbId = new(pdbProvider.GetMetadataReader().DebugMetadataHeader!.Id); + + Assert.Equal(pdbId.Guid, codeView.Guid); + Assert.Equal(pdbId.Stamp, codeViewEntry.Stamp); + Assert.Equal(1, codeView.Age); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void PortablePdb_PdbChecksumEntry_IsSha256OfPdbWithIdZeroed(bool deterministic) + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options + { + Debug = true, + Deterministic = deterministic, + }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + DebugDirectoryEntry checksumEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.PdbChecksum); + PdbChecksumDebugDirectoryData checksum = pe.ReadPdbChecksumDebugDirectoryData(checksumEntry); + + byte[] pdb = DocumentCompilerTestHelpers.GetPortablePdb(result).ToArray(); + using (var pdbProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(pdb))) + { + DebugMetadataHeader header = pdbProvider.GetMetadataReader().DebugMetadataHeader!; + Array.Clear(pdb, header.IdStartOffset, header.Id.Length); + } + + Assert.Equal("SHA256", checksum.AlgorithmName); + Assert.Equal(SHA256.HashData(pdb), checksum.Checksum.ToArray()); + } + [Fact] public void DebugModeImpl_EmitsDebuggableAttributeBlob() { diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTestHelpers.cs b/src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTestHelpers.cs index 12c1ddd870ee40..04f6d5e41838fc 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTestHelpers.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTestHelpers.cs @@ -5,8 +5,6 @@ using System.Buffers.Binary; using System.Collections.Generic; using System.Collections.Immutable; -using System.IO; -using System.IO.Compression; using System.Linq; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; @@ -115,26 +113,55 @@ internal static ImmutableArray CompileAndGetImageBytes(string source, Opti return Compile(source, options); } - internal static ImmutableArray CompileAndGetEmbeddedPortablePdb(string source, Options options) + internal static CompilationResult CompileAndGetResult(string source, Options options) { - using PEReader pe = CompileAndGetReader(source, options); - DebugDirectoryEntry embeddedPdbEntry = pe.ReadDebugDirectory() - .Single(entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - ImmutableArray embeddedPdb = pe.GetSectionData(embeddedPdbEntry.DataRelativeVirtualAddress) - .GetContent(0, embeddedPdbEntry.DataSize); - - const int EmbeddedPdbHeaderSize = 8; - ReadOnlySpan embeddedPdbBytes = embeddedPdb.AsSpan(); - int uncompressedSize = BinaryPrimitives.ReadInt32LittleEndian(embeddedPdbBytes.Slice(sizeof(int))); - using MemoryStream compressedStream = new(embeddedPdbBytes.Slice(EmbeddedPdbHeaderSize).ToArray()); - using DeflateStream deflateStream = new(compressedStream, CompressionMode.Decompress); - using MemoryStream pdbStream = new(uncompressedSize); - deflateStream.CopyTo(pdbStream); - Assert.Equal(uncompressedSize, pdbStream.Length); - - return ImmutableArray.CreateRange(pdbStream.ToArray()); + var (diagnostics, result) = CompileWithDiagnostics(source, options); + Assert.Empty(diagnostics); + Assert.NotNull(result); + return result!; + } + + /// Gets the Portable PDB that the compilation returned for writing beside the image. + internal static ImmutableArray GetPortablePdb(CompilationResult result) + { + Assert.True(result.PortablePdb.HasValue, "Expected the compilation to produce a Portable PDB"); + return result.PortablePdb!.Value; } + internal static MetadataReaderProvider GetPortablePdbReaderProvider(CompilationResult result) + => MetadataReaderProvider.FromPortablePdbImage(GetPortablePdb(result)); + + internal static ImmutableArray CompileAndGetPortablePdb(string source, Options options) + => GetPortablePdb(CompileAndGetResult(source, options)); + + /// + /// Compiles a program with one method, with a PDB and deterministically, and returns the image and the PDB + /// that its CodeView entry refers to. Different names give different PDB ids: the method's sequence point + /// is in a document named after it, and the PDB does not record method names. + /// + internal static (ImmutableArray Image, ImmutableArray Pdb) CompileImageAndPdb(string name) + { + CompilationResult result = CompileAndGetResult(OneMethodSource(name, $".line 1 '{name}.cs'"), new Options { Debug = true, Deterministic = true }); + return (Serialize(result), GetPortablePdb(result)); + } + + /// Compiles a program with one method without a PDB: the image has no debug directory. + internal static ImmutableArray CompileImageWithoutPdb(string name) + => Compile(OneMethodSource(name, lineDirective: string.Empty), new Options { Deterministic = true }); + + private static string OneMethodSource(string methodName, string lineDirective) => $$""" + .assembly extern System.Runtime { } + .assembly test { } + .class public auto ansi beforefieldinit Test + { + .method public static void {{methodName}}() cil managed + { + {{lineDirective}} + ret + } + } + """; + internal static int GetFirstTokenOperand(PEReader pe, MetadataReader reader, string methodName, ILOpcode targetOpcode) { var method = reader.MethodDefinitions diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs new file mode 100644 index 00000000000000..1e6358911960dd --- /dev/null +++ b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs @@ -0,0 +1,217 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Text; +using Xunit; + +namespace ILAssembler.Tests.GeneratedCases +{ + /// A method of a generated program, with an optional .line directive. + public sealed record GeneratedMethod(string? Document, int Line) + { + public override string ToString() => Document is null ? "-" : $"{Document}@{Line}"; + } + + /// A small valid IL program: one class with one or more methods. + public sealed record GeneratedProgram(ImmutableArray Methods) + { + public GeneratedProgram WithExtraMethod() => new(Methods.Add(new GeneratedMethod(null, 0))); + + public string ToSource() + { + var source = new StringBuilder(); + source.AppendLine(".assembly extern System.Runtime { }"); + source.AppendLine(".assembly test { }"); + source.AppendLine(".class public auto ansi beforefieldinit Test"); + source.AppendLine("{"); + for (int i = 0; i < Methods.Length; i++) + { + source.AppendLine($" .method public static void M{i}() cil managed"); + source.AppendLine(" {"); + if (Methods[i].Document is { } document) + { + source.AppendLine($" .line {Methods[i].Line} '{document}'"); + } + + source.AppendLine(" nop"); + source.AppendLine(" ret"); + source.AppendLine(" }"); + } + + source.AppendLine("}"); + return source.ToString(); + } + + public override string ToString() => string.Join(" ", Methods); + } + + /// The PDB-related assembler options of a generated compilation. + public sealed record GeneratedOptions( + bool Debug, + DebugMode? DebugMode, + bool Pdb, + bool Deterministic, + string? OutputFileName, + string? PdbFilePath) + { + /// Whether an option that produces a PDB is set ( or ). + public bool RequestsPdb => Debug || Pdb; + + /// The PDB path the CodeView entry is expected to name. + public string ExpectedCodeViewPath => + PdbFilePath ?? (OutputFileName is not null ? Path.ChangeExtension(OutputFileName, ".pdb") : "assembly.pdb"); + + public Options ToOptions() => new() + { + Debug = Debug, + DebugMode = DebugMode, + Pdb = Pdb, + Deterministic = Deterministic, + OutputFileName = OutputFileName, + PdbFilePath = PdbFilePath, + }; + + public override string ToString() => + $"{(Debug ? "debug " : "")}{(DebugMode is { } mode ? $"mode={mode} " : "")}{(Pdb ? "pdb " : "")}{(Deterministic ? "det " : "")}" + + $"out={OutputFileName ?? "-"} pdbpath={PdbFilePath ?? "-"}"; + } + + /// A generated program and options. + public sealed record CompileCase(GeneratedProgram Program, GeneratedOptions Options) + { + public override string ToString() => $"{Options} | {Program}"; + } + + /// A file system state at the PDB path before the output is written. + public enum ExistingPdb + { + Absent, + + /// A Portable PDB ('s). + File, + + Directory, + } + + /// A file system state at the output path before the output is written. + public enum ExistingOutput + { + Absent, + + /// The image whose CodeView entry refers to the PDB at the PDB path (). + ImageOfThePdb, + + /// An image whose CodeView entry refers to another PDB. + ImageOfAnotherPdb, + + /// A file that is not an image. + NotAnImage, + + /// A file that reads as a COFF object file: no PE header, so no debug directory. + CoffObject, + } + + /// + /// The inputs of the generated-case theories. Compilation cases come from a with a fixed + /// seed, so every run builds the same cases and a failing case is reproduced by its index; the display name + /// also describes it. The output writer's states are few, so they are enumerated exhaustively. + /// + public static class PdbCaseGenerator + { + private const int Seed = 20261006; + private const int CaseCount = 100; + + private static readonly string[] s_documents = ["a.cs", "b.cs", "dir/c.cs"]; + private static readonly DebugMode?[] s_debugModes = [null, ILAssembler.DebugMode.Impl, ILAssembler.DebugMode.Opt]; + private static readonly string?[] s_outputFileNames = [null, "Output.dll", "Output.exe", "Output", "Out.put.dll"]; + private static readonly string?[] s_pdbFilePaths = [null, "/out/Output.pdb", "C:\\out\\Output.pdb", "Output.pdb"]; + + /// Programs with any options. + public static ImmutableArray AnyCases { get; } = Generate(new Random(Seed), requirePdb: false); + + /// Programs with options that request a PDB. + public static ImmutableArray PdbRequestedCases { get; } = Generate(new Random(Seed + 1), requirePdb: true); + + public static TheoryData AnyCaseData => ToTheoryData(AnyCases); + + public static TheoryData PdbRequestedCaseData => ToTheoryData(PdbRequestedCases); + + /// + /// An image and the PDB its CodeView entry refers to, which the output writer states place at the output and + /// PDB paths, and an image of another PDB. + /// + public static (ImmutableArray Image, ImmutableArray Pdb) ExistingPair { get; } = DocumentCompilerTestHelpers.CompileImageAndPdb("A"); + + public static ImmutableArray ImageOfAnotherPdb { get; } = DocumentCompilerTestHelpers.CompileImageAndPdb("B").Image; + + /// Every output name, pre-existing output state, pre-existing PDB path state and PDB presence. + public static TheoryData OutputWriteStates + { + get + { + var data = new TheoryData(); + foreach (string outputFileName in new[] { "Min.dll", "Min.exe", "Min" }) + { + foreach (ExistingOutput existingOutput in Enum.GetValues()) + { + foreach (ExistingPdb existingPdb in Enum.GetValues()) + { + foreach (bool withPdb in new[] { false, true }) + { + data.Add(outputFileName, existingOutput, existingPdb, withPdb); + } + } + } + } + + return data; + } + } + + private static TheoryData ToTheoryData(ImmutableArray cases) + { + var data = new TheoryData(); + for (int i = 0; i < cases.Length; i++) + { + data.Add(i, cases[i].ToString()); + } + + return data; + } + + private static ImmutableArray Generate(Random random, bool requirePdb) + { + var cases = ImmutableArray.CreateBuilder(CaseCount); + while (cases.Count < CaseCount) + { + var methods = ImmutableArray.CreateBuilder(); + int methodCount = random.Next(1, 6); + for (int i = 0; i < methodCount; i++) + { + methods.Add(random.Next(2) == 0 + ? new GeneratedMethod(null, 0) + : new GeneratedMethod(s_documents[random.Next(s_documents.Length)], random.Next(1, 100_000))); + } + + var options = new GeneratedOptions( + Debug: random.Next(2) == 0, + DebugMode: s_debugModes[random.Next(s_debugModes.Length)], + Pdb: random.Next(2) == 0, + Deterministic: random.Next(2) == 0, + OutputFileName: s_outputFileNames[random.Next(s_outputFileNames.Length)], + PdbFilePath: s_pdbFilePaths[random.Next(s_pdbFilePaths.Length)]); + + if (!requirePdb || options.RequestsPdb) + { + cases.Add(new CompileCase(new GeneratedProgram(methods.ToImmutable()), options)); + } + } + + return cases.MoveToImmutable(); + } + } +} diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs new file mode 100644 index 00000000000000..1e537907994e36 --- /dev/null +++ b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs @@ -0,0 +1,255 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; +using System.Security.Cryptography; +using Xunit; + +// The description parameter is not read: it names the generated case in the test's display name. +#pragma warning disable xUnit1026 + +namespace ILAssembler.Tests.GeneratedCases +{ + /// + /// The PDB rules, checked over seeded generated programs and option combinations + /// (), and for the output writer over every combination of output name, + /// pre-existing output, pre-existing PDB path and PDB presence that + /// enumerates. + /// + public class PdbGeneratedCaseTests + { + private static readonly ImmutableArray s_newPdb = [0x42, 0x53, 0x4A, 0x42]; + + private static (CompilationResult Result, ImmutableArray Image) Compile(GeneratedProgram program, GeneratedOptions options) + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(program.ToSource(), options.ToOptions()); + return (result, DocumentCompilerTestHelpers.Serialize(result)); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.PdbRequestedCaseData), MemberType = typeof(PdbCaseGenerator))] + public void RequestedPdb_IsReferencedByCodeViewThenPdbChecksumThenReproducibleIffDeterministic(int index, string description) + { + CompileCase input = PdbCaseGenerator.PdbRequestedCases[index]; + (_, ImmutableArray image) = Compile(input.Program, input.Options); + using var pe = new PEReader(image); + + DebugDirectoryEntryType[] expected = input.Options.Deterministic + ? [DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible] + : [DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum]; + Assert.Equal(expected, pe.ReadDebugDirectory().Select(entry => entry.Type)); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.PdbRequestedCaseData), MemberType = typeof(PdbCaseGenerator))] + public void RequestedPdb_CodeViewEntryCarriesThePdbId(int index, string description) + { + CompileCase input = PdbCaseGenerator.PdbRequestedCases[index]; + (CompilationResult result, ImmutableArray image) = Compile(input.Program, input.Options); + using var pe = new PEReader(image); + DebugDirectoryEntry codeViewEntry = pe.ReadDebugDirectory().Single(entry => entry.Type == DebugDirectoryEntryType.CodeView); + using MetadataReaderProvider pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); + BlobContentId pdbId = new(pdbProvider.GetMetadataReader().DebugMetadataHeader!.Id); + + Assert.Equal((pdbId.Guid, pdbId.Stamp), (pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Guid, codeViewEntry.Stamp)); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.PdbRequestedCaseData), MemberType = typeof(PdbCaseGenerator))] + public void RequestedPdb_CodeViewEntryNamesThePdbPathOrItsFallback(int index, string description) + { + CompileCase input = PdbCaseGenerator.PdbRequestedCases[index]; + (_, ImmutableArray image) = Compile(input.Program, input.Options); + using var pe = new PEReader(image); + DebugDirectoryEntry codeViewEntry = pe.ReadDebugDirectory().Single(entry => entry.Type == DebugDirectoryEntryType.CodeView); + + Assert.Equal(input.Options.ExpectedCodeViewPath, pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Path); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.PdbRequestedCaseData), MemberType = typeof(PdbCaseGenerator))] + public void RequestedPdb_PdbChecksumIsSha256OfThePdbWithItsIdZeroed(int index, string description) + { + CompileCase input = PdbCaseGenerator.PdbRequestedCases[index]; + (CompilationResult result, ImmutableArray image) = Compile(input.Program, input.Options); + using var pe = new PEReader(image); + DebugDirectoryEntry checksumEntry = pe.ReadDebugDirectory().Single(entry => entry.Type == DebugDirectoryEntryType.PdbChecksum); + PdbChecksumDebugDirectoryData checksum = pe.ReadPdbChecksumDebugDirectoryData(checksumEntry); + + byte[] pdb = DocumentCompilerTestHelpers.GetPortablePdb(result).ToArray(); + using (MetadataReaderProvider pdbProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(pdb))) + { + DebugMetadataHeader header = pdbProvider.GetMetadataReader().DebugMetadataHeader!; + Array.Clear(pdb, header.IdStartOffset, header.Id.Length); + } + + Assert.Equal("SHA256", checksum.AlgorithmName); + Assert.Equal(SHA256.HashData(pdb), checksum.Checksum.ToArray()); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.AnyCaseData), MemberType = typeof(PdbCaseGenerator))] + public void Deterministic_CompilingTwice_GivesIdenticalImageAndPdb(int index, string description) + { + CompileCase input = PdbCaseGenerator.AnyCases[index]; + GeneratedOptions options = input.Options with { Deterministic = true }; + + (CompilationResult firstResult, ImmutableArray firstImage) = Compile(input.Program, options); + (CompilationResult secondResult, ImmutableArray secondImage) = Compile(input.Program, options); + + Assert.Equal(firstImage, secondImage); + Assert.Equal(firstResult.PortablePdb.HasValue, secondResult.PortablePdb.HasValue); + if (firstResult.PortablePdb is { } firstPdb) + { + Assert.Equal(firstPdb, secondResult.PortablePdb!.Value); + } + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.PdbRequestedCaseData), MemberType = typeof(PdbCaseGenerator))] + public void Deterministic_DifferentSources_GiveDifferentImagesAndPdbIds(int index, string description) + { + CompileCase input = PdbCaseGenerator.PdbRequestedCases[index]; + GeneratedOptions options = input.Options with { Deterministic = true }; + + (CompilationResult firstResult, ImmutableArray firstImage) = Compile(input.Program, options); + (CompilationResult secondResult, ImmutableArray secondImage) = Compile(input.Program.WithExtraMethod(), options); + + Assert.NotEqual(firstImage, secondImage); + using MetadataReaderProvider firstPdb = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(firstResult); + using MetadataReaderProvider secondPdb = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(secondResult); + Assert.NotEqual( + firstPdb.GetMetadataReader().DebugMetadataHeader!.Id, + secondPdb.GetMetadataReader().DebugMetadataHeader!.Id); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.OutputWriteStates), MemberType = typeof(PdbCaseGenerator))] + public void OutputWriter_NeverLeavesATemporaryFile(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) + { + foreach (bool imageWriteFails in new[] { false, true }) + { + RunOutputWrite(outputFileName, existingOutput, existingPdb, withPdb, imageWriteFails, (directory, _, _) => + Assert.DoesNotContain(Directory.EnumerateFileSystemEntries(directory), path => path.EndsWith(".tmp", StringComparison.Ordinal))); + } + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.OutputWriteStates), MemberType = typeof(PdbCaseGenerator))] + public void OutputWriter_WhenTheImageWriteFails_LeavesThePdbPathUnchanged(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) + { + RunOutputWrite(outputFileName, existingOutput, existingPdb, withPdb, imageWriteFails: true, (_, pdbPath, exception) => + { + Assert.NotNull(exception); + switch (existingPdb) + { + case ExistingPdb.Absent: + Assert.False(Path.Exists(pdbPath)); + break; + case ExistingPdb.File: + Assert.Equal(PdbCaseGenerator.ExistingPair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + break; + case ExistingPdb.Directory: + Assert.True(Directory.Exists(pdbPath)); + break; + } + }); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.OutputWriteStates), MemberType = typeof(PdbCaseGenerator))] + public void OutputWriter_AfterTheImageIsWritten_LeavesTheNewPdbOrDeletesOnlyTheReplacedImagesPdb(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) + { + RunOutputWrite(outputFileName, existingOutput, existingPdb, withPdb, imageWriteFails: false, (_, pdbPath, exception) => + { + if (existingPdb == ExistingPdb.Directory) + { + // A directory is never deleted; with a PDB, renaming over it fails. + Assert.Equal(withPdb, exception is not null); + Assert.True(Directory.Exists(pdbPath)); + } + else if (withPdb) + { + Assert.Null(exception); + Assert.Equal(s_newPdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + else if (existingPdb == ExistingPdb.File && existingOutput != ExistingOutput.ImageOfThePdb) + { + // The PDB does not belong to the image that was replaced, so it stays. + Assert.Null(exception); + Assert.Equal(PdbCaseGenerator.ExistingPair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + else + { + Assert.Null(exception); + Assert.False(File.Exists(pdbPath)); + } + }); + } + + private static void RunOutputWrite( + string outputFileName, + ExistingOutput existingOutput, + ExistingPdb existingPdb, + bool withPdb, + bool imageWriteFails, + Action check) + { + string directory = Directory.CreateTempSubdirectory("ilasm-output-").FullName; + try + { + string outputPath = Path.Combine(directory, outputFileName); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + switch (existingOutput) + { + case ExistingOutput.ImageOfThePdb: + File.WriteAllBytes(outputPath, PdbCaseGenerator.ExistingPair.Image.ToArray()); + break; + case ExistingOutput.ImageOfAnotherPdb: + File.WriteAllBytes(outputPath, PdbCaseGenerator.ImageOfAnotherPdb.ToArray()); + break; + case ExistingOutput.NotAnImage: + File.WriteAllBytes(outputPath, [0x01]); + break; + case ExistingOutput.CoffObject: + File.WriteAllBytes(outputPath, new byte[20]); + break; + } + + switch (existingPdb) + { + case ExistingPdb.File: + File.WriteAllBytes(pdbPath, PdbCaseGenerator.ExistingPair.Pdb.ToArray()); + break; + case ExistingPdb.Directory: + Directory.CreateDirectory(pdbPath); + break; + } + + Exception? exception = Record.Exception(() => OutputWriter.Write( + outputPath, + pdbPath, + stream => + { + stream.WriteByte(0x4D); + if (imageWriteFails) + { + throw new IOException("Injected image write failure"); + } + }, + withPdb ? s_newPdb : null)); + + check(directory, pdbPath, exception); + } + finally + { + Directory.Delete(directory, recursive: true); + } + } + } +} diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj index f7867dac960e75..d0886ad184c72c 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj +++ b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj @@ -14,7 +14,7 @@ - + IlasmRootCommand.cs @@ -22,6 +22,9 @@ NativeCommandLine.cs + + OutputWriter.cs + diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs index 70f4a11e203b75..b75fb060cc99d3 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs @@ -219,7 +219,8 @@ ldc.i4.s 42 } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); var reader = pe.GetMetadataReader(); var moduleType = reader.GetTypeDefinition(MetadataTokens.TypeDefinitionHandle(1)); var globalMethod = reader.GetMethodDefinition(Assert.Single(moduleType.GetMethods())); @@ -240,10 +241,7 @@ ldc.i4.s 42 Assert.Empty(attribute.FixedArguments); Assert.Empty(attribute.NamedArguments); - var embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - using var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); Assert.Contains( "global.cs", diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs new file mode 100644 index 00000000000000..535bf8f41eb37c --- /dev/null +++ b/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs @@ -0,0 +1,412 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Reflection.PortableExecutable; +using Xunit; + +namespace ILAssembler.Tests; + +public sealed class OutputWriterTests : IDisposable +{ + private static readonly byte[] s_image = [0x4D, 0x5A, 0x01, 0x02]; + private static readonly ImmutableArray s_pdb = [0x42, 0x53, 0x4A, 0x42, 0x03]; + private static readonly byte[] s_stale = [0xDE, 0xAD]; + + // An image and the PDB its CodeView entry refers to, a second such pair with another PDB id, and an image + // without a debug directory. + private static readonly (ImmutableArray Image, ImmutableArray Pdb) s_pair = DocumentCompilerTestHelpers.CompileImageAndPdb("A"); + private static readonly (ImmutableArray Image, ImmutableArray Pdb) s_otherPair = DocumentCompilerTestHelpers.CompileImageAndPdb("B"); + private static readonly ImmutableArray s_imageWithoutPdb = DocumentCompilerTestHelpers.CompileImageWithoutPdb("A"); + + private readonly string _directory = Directory.CreateTempSubdirectory("ilasm-output-").FullName; + + public void Dispose() => Directory.Delete(_directory, recursive: true); + + private string PathOf(string fileName) => Path.Combine(_directory, fileName); + + private string[] FileNames() => Directory.EnumerateFileSystemEntries(_directory).Select(Path.GetFileName).Order().ToArray()!; + + private static void WriteImage(Stream stream) => stream.Write(s_image); + + [Theory] + [InlineData("Min.dll", "Min.pdb")] + [InlineData("Min", "Min.pdb")] + [InlineData("a.b/Min", "a.b/Min.pdb")] + public void GetPdbPath_ReplacesTheExtensionAndReturnsTheFullPath(string outputPath, string expectedPdbPath) + { + Assert.Equal(Path.GetFullPath(expectedPdbPath), OutputWriter.GetPdbPath(outputPath)); + } + + [Fact] + public void Write_WithPdb_WritesTheImageAndThenThePdb() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + bool pdbExistedDuringImageWrite = true; + + OutputWriteResult result = OutputWriter.Write( + outputPath, + pdbPath, + stream => + { + pdbExistedDuringImageWrite = File.Exists(pdbPath); + WriteImage(stream); + }, + s_pdb); + + Assert.Equal(OutputWriteResult.ImageAndPdbWritten, result); + Assert.False(pdbExistedDuringImageWrite); + Assert.Equal(s_image, File.ReadAllBytes(outputPath)); + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + } + + [Fact] + public void Write_WithPdb_ReplacesAnExistingPdb() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + + OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithPdb_RenamesANewFileOverTheExistingPdbInsteadOfRewritingIt() + { + // Renaming a completed temporary file over the PDB path is what keeps a failed write from leaving a + // partial PDB. It is observable as a reader of the old PDB still seeing the old content afterwards. + // Renaming over a file that is open depends on the Windows version, so this runs elsewhere. + if (OperatingSystem.IsWindows()) + { + return; + } + + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + using var oldPdbReader = new FileStream(pdbPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + + OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + byte[] oldContent = new byte[s_stale.Length]; + oldPdbReader.ReadExactly(oldContent); + Assert.Equal(s_stale, oldContent); + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithoutPdb_DeletesThePdbOfTheImageItReplaces() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWrittenStalePdbDeleted, result); + Assert.Equal(new[] { "Min.dll" }, FileNames()); + } + + [Fact] + public void Write_WithoutPdb_KeepsAPdbWhenNoImageIsReplaced() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithoutPdb_KeepsAPdbThatTheReplacedImageDoesNotReference() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_otherPair.Image.ToArray()); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + Assert.NotEqual(OutputWriter.TryReadPortablePdbId(pdbPath), OutputWriter.TryReadCodeViewPdbId(outputPath)); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedImageHasNoDebugDirectory() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_imageWithoutPdb.ToArray()); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedFileIsNotAnImage() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_stale); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedFileIsACoffObject() + { + // System.Reflection.Metadata reads a file that does not start with "MZ" as a COFF object file, which has no + // PE header and so no debug directory. Twenty zero bytes read as one with no sections. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, new byte[20]); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + using (var coff = new PEReader(File.ReadAllBytes(outputPath).ToImmutableArray())) + { + Assert.True(coff.PEHeaders.IsCoffOnly); + } + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_image, File.ReadAllBytes(outputPath)); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void TryReadCodeViewPdbId_ReturnsNullForAFileLongerThanAnImageCanBe() + { + // System.Reflection.Metadata rejects a stream longer than int.MaxValue bytes with an ArgumentException. + using var stream = new LengthOnlyStream(int.MaxValue + 1L); + + Assert.Null(OutputWriter.TryReadCodeViewPdbId(stream)); + } + + [Fact] + public void TryReadPortablePdbId_ReturnsNullForAFileLongerThanAPdbCanBe() + { + using var stream = new LengthOnlyStream(int.MaxValue + 1L); + + Assert.Null(OutputWriter.TryReadPortablePdbId(stream)); + } + + [Fact] + public void Write_WithoutPdb_KeepsAFileThatIsNotAPortablePdb() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + File.WriteAllBytes(pdbPath, s_stale); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Write_WhenTheImageWriteFails_LeavesTheExistingPdbUnchanged(bool withPdb) + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + + Assert.Throws(() => OutputWriter.Write( + outputPath, + pdbPath, + stream => + { + stream.WriteByte(0x4D); + throw new IOException("Injected image write failure"); + }, + withPdb ? s_pdb : null)); + + Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Write_WhenClosingTheImageFails_LeavesTheExistingPdbUnchanged(bool withPdb) + { + // The image is closed before the PDB is touched: a PDB written, or the replaced image's PDB deleted, + // before the image stream is disposed would show here. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + + Assert.Throws(() => OutputWriter.Write( + outputPath, + pdbPath, + WriteImage, + withPdb ? s_pdb : null, + path => new ThrowOnDisposeStream(File.Create(path)))); + + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WhenThePdbCannotBeWritten_RemovesOnlyItsTemporaryFile() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + + // A directory at the PDB path makes the final rename fail after the temporary file is written. + Directory.CreateDirectory(pdbPath); + + Assert.ThrowsAny(() => OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb)); + + Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + Assert.True(Directory.Exists(pdbPath)); + } + + [Fact] + public void WritePdb_WhenItsTemporaryPathExists_LeavesThePdbUnchanged() + { + string pdbPath = PathOf("Min.pdb"); + string temporaryPath = PathOf("Min.pdb.existing.tmp"); + File.WriteAllBytes(pdbPath, s_stale); + File.WriteAllBytes(temporaryPath, s_image); + + Assert.Throws(() => OutputWriter.WritePdb(pdbPath, temporaryPath, s_pdb)); + + Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void WritePdb_WhenItsTemporaryPathExists_KeepsTheFileThere() + { + string pdbPath = PathOf("Min.pdb"); + string temporaryPath = PathOf("Min.pdb.existing.tmp"); + File.WriteAllBytes(temporaryPath, s_image); + + Assert.Throws(() => OutputWriter.WritePdb(pdbPath, temporaryPath, s_pdb)); + + Assert.Equal(s_image, File.ReadAllBytes(temporaryPath)); + } + + [Fact] + public void Write_WithPdbNamedLikeTheOutput_WritesNothing() + { + string outputPath = PathOf("Min.pdb"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_stale); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(s_stale, File.ReadAllBytes(outputPath)); + } + + [Fact] + public void Write_WithoutPdb_KeepsAnOutputNamedLikeThePdb() + { + string outputPath = PathOf("Min.pdb"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_image, File.ReadAllBytes(outputPath)); + } + + [Fact] + public void Write_WithoutPdb_NeverDeletesTheOutputItWrote() + { + // The output is named like its PDB, the image it replaces refers to a PDB, and the bytes written are that + // PDB's: the file at the PDB path then reads as the replaced image's PDB, but it is the new output. + string outputPath = PathOf("Min.pdb"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, stream => stream.Write(s_pair.Pdb.AsSpan()), portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(outputPath)); + } + + [Fact] + public void Write_WithPdb_TreatsAnOutputDifferingFromThePdbOnlyInCaseAsTheDefaultFileSystemDoes() + { + // Min.PDB and Min.pdb are one file on the default Windows and macOS file systems, and two elsewhere. + bool caseInsensitive = OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); + string outputPath = PathOf("Min.PDB"); + string pdbPath = OutputWriter.GetPdbPath(outputPath); + + OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(caseInsensitive ? OutputWriteResult.PdbWouldOverwriteOutput : OutputWriteResult.ImageAndPdbWritten, result); + } + + // A stream that writes through to another and fails when it is closed. + private sealed class ThrowOnDisposeStream(Stream inner) : Stream + { + public override bool CanRead => inner.CanRead; + public override bool CanSeek => inner.CanSeek; + public override bool CanWrite => inner.CanWrite; + public override long Length => inner.Length; + + public override long Position + { + get => inner.Position; + set => inner.Position = value; + } + + public override void Flush() => inner.Flush(); + public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, count); + public override long Seek(long offset, SeekOrigin origin) => inner.Seek(offset, origin); + public override void SetLength(long value) => inner.SetLength(value); + public override void Write(byte[] buffer, int offset, int count) => inner.Write(buffer, offset, count); + + protected override void Dispose(bool disposing) + { + inner.Dispose(); + throw new IOException("Injected image close failure"); + } + } + + // A readable, seekable stream that reports a length and holds no data, so a length check can be tested without + // a file of that size. + private sealed class LengthOnlyStream(long length) : Stream + { + public override bool CanRead => true; + public override bool CanSeek => true; + public override bool CanWrite => false; + public override long Length => length; + public override long Position { get; set; } + + public override void Flush() { } + public override int Read(byte[] buffer, int offset, int count) => throw new InvalidOperationException("The stream was read."); + public override long Seek(long offset, SeekOrigin origin) => Position = origin switch + { + SeekOrigin.Begin => offset, + SeekOrigin.Current => Position + offset, + _ => length + offset, + }; + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } +} diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs index 3a2af847b13177..4cf5887af48fc2 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs @@ -37,7 +37,7 @@ br END END: ret """); using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbImage( - DocumentCompilerTestHelpers.CompileAndGetEmbeddedPortablePdb(source, new Options { Optimize = optimize })); + DocumentCompilerTestHelpers.CompileAndGetPortablePdb(source, new Options { Optimize = optimize })); MetadataReader reader = provider.GetMetadataReader(); SequencePoint[] points = reader.GetMethodDebugInformation(reader.MethodDebugInformation.Single()) .GetSequencePoints().ToArray(); @@ -159,7 +159,7 @@ .method public static void TestMethod() cil managed [Fact] - public void PdbGeneration_WithLineAndLanguageDirectives_CreatesValidEmbeddedPdb() + public void PdbGeneration_WithLineAndLanguageDirectives_CreatesValidPdb() { string source = $$""" .assembly test { } @@ -178,17 +178,10 @@ .line 20 "test.cs" } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); - - // Verify debug directory exists with embedded PDB - var debugDirectory = pe.ReadDebugDirectory(); - Assert.NotEmpty(debugDirectory); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); - var embeddedPdbEntry = debugDirectory.FirstOrDefault(e => e.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - Assert.NotEqual(default, embeddedPdbEntry); - - // Read the embedded PDB and verify contents - var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdbEntry); + // Read the PDB and verify contents + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); // Verify document exists with correct name and language @@ -205,7 +198,7 @@ .line 20 "test.cs" } [Fact] - public void PdbGeneration_LanguageWithVendorAndDocumentType_CreatesValidEmbeddedPdb() + public void PdbGeneration_LanguageWithVendorAndDocumentType_CreatesValidPdb() { string source = $$""" .assembly test { } @@ -224,17 +217,10 @@ .line 20 "test.cs" } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); - - // Verify debug directory exists with embedded PDB - var debugDirectory = pe.ReadDebugDirectory(); - Assert.NotEmpty(debugDirectory); - - var embeddedPdbEntry = debugDirectory.FirstOrDefault(e => e.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - Assert.NotEqual(default, embeddedPdbEntry); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); - // Read the embedded PDB and verify contents - var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdbEntry); + // Read the PDB and verify contents + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); // Verify document exists with correct name and language @@ -266,12 +252,12 @@ .method public static void TestMethod() cil managed } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); - // Verify no embedded PDB when no debug directives - var debugDirectory = pe.ReadDebugDirectory(); - var embeddedPdbEntry = debugDirectory.FirstOrDefault(e => e.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - Assert.Equal(default, embeddedPdbEntry); + // Verify no PDB and no debug directory when no debug directives + Assert.Null(result.PortablePdb); + Assert.Empty(pe.ReadDebugDirectory()); } @@ -351,12 +337,12 @@ .method public static void TestMethod() cil managed } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); var reader = pe.GetMetadataReader(); var methodHandle = reader.MethodDefinitions.Single(handle => reader.GetString(reader.GetMethodDefinition(handle).Name) == "TestMethod"); - var embeddedPdbEntry = pe.ReadDebugDirectory().Single(entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - using var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdbEntry); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); var debugHandle = MetadataTokens.MethodDebugInformationHandle(MetadataTokens.GetRowNumber(methodHandle)); var debugInfo = pdbReader.GetMethodDebugInformation(debugHandle); @@ -404,14 +390,12 @@ .method public static void M() cil managed } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); var reader = pe.GetMetadataReader(); var methodHandle = reader.MethodDefinitions .Single(handle => reader.GetString(reader.GetMethodDefinition(handle).Name) == "M"); - var embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - using var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); var debugInformation = pdbReader.GetMethodDebugInformation( MetadataTokens.MethodDebugInformationHandle(MetadataTokens.GetRowNumber(methodHandle))); @@ -444,11 +428,8 @@ .line 10 "document.cs" } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); - var embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - using var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); var document = pdbReader.GetDocument(Assert.Single(pdbReader.Documents)); @@ -476,11 +457,8 @@ .line 10 } """; - using var pe = DocumentCompilerTestHelpers.CompileAndGetReader(source, new Options()); - var embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - using var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); var document = pdbReader.GetDocument(Assert.Single(pdbReader.Documents)); @@ -531,10 +509,7 @@ .line 20 "doc2.cs" var imageBuilder = new BlobBuilder(); image!.Serialize(imageBuilder); using var pe = new PEReader(imageBuilder.ToImmutableArray()); - var embeddedPdbEntry = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - using var pdbProvider = pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdbEntry); + using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(image); var pdbReader = pdbProvider.GetMetadataReader(); var documentNames = pdbReader.Documents .Select(handle => pdbReader.GetString(pdbReader.GetDocument(handle).Name)) @@ -588,14 +563,8 @@ .line 20 "second.vb" Assert.Empty(diagnostics); Assert.NotNull(result); - BlobBuilder image = new(); - result!.Serialize(image); - using PEReader pe = new(image.ToImmutableArray()); - DebugDirectoryEntry embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); using MetadataReaderProvider pdbProvider = - pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result!); MetadataReader pdbReader = pdbProvider.GetMetadataReader(); Dictionary documentLanguages = pdbReader.Documents.ToDictionary( handle => pdbReader.GetString(pdbReader.GetDocument(handle).Name), @@ -651,11 +620,8 @@ .line 20 .Single(handle => reader.GetString(reader.GetMethodDefinition(handle).Name) == "M1"); MethodDefinitionHandle secondMethod = reader.MethodDefinitions .Single(handle => reader.GetString(reader.GetMethodDefinition(handle).Name) == "M2"); - DebugDirectoryEntry embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); using MetadataReaderProvider pdbProvider = - pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); MetadataReader pdbReader = pdbProvider.GetMetadataReader(); MethodDebugInformation firstDebugInformation = pdbReader.GetMethodDebugInformation( @@ -703,14 +669,8 @@ .line 10 "document.cs" Assert.Contains(diagnostics, diagnostic => diagnostic.Severity == DiagnosticSeverity.Error); Assert.NotNull(result); - var image = new BlobBuilder(); - result!.Serialize(image); - using var pe = new PEReader(image.ToImmutableArray()); - DebugDirectoryEntry embeddedPdb = Assert.Single( - pe.ReadDebugDirectory(), - entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); using MetadataReaderProvider pdbProvider = - pe.ReadEmbeddedPortablePdbDebugDirectoryData(embeddedPdb); + DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result!); MetadataReader pdbReader = pdbProvider.GetMetadataReader(); Document document = pdbReader.GetDocument(Assert.Single(pdbReader.Documents)); From 39f0b96b390f394cf5ca76f40442afa0036d6675 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Tue, 6 Oct 2026 08:23:31 -0700 Subject: [PATCH 2/9] Generate a PDB only when /DEBUG or /PDB is given The managed ilasm produced a PDB whenever the source had .line directives, even without /DEBUG or /PDB. Native ilasm produces a PDB only when one of those switches is given. Match native ilasm: a PDB is produced only with --debug, --debug-mode or --pdb. Without them the image has no debug directory, with or without --deterministic, and the .line directives are still parsed and validated but their sequence points are not emitted. --debug-mode alone counts as --debug, as it already did for the DebuggableAttribute. Tests that relied on the implicit PDB now pass Debug = true. New unit tests pin that .line alone produces no PDB, that --debug-mode alone produces one, and that an image without a PDB has no debug directory and no debug data, deterministic or not, with or without .line. Seeded theories over generated programs and options pin that a PDB is produced exactly when a switch requests it and that an image without one has no debug directory and no debug data. IlasmPdbFileTester checks both ilasms end to end without a switch, with and without .line and /DET (the managed ilasm when CORE_ROOT has it). The README and the doc comments say which switches produce a PDB. --- .../ilasm/PortablePdb/IlasmPdbFileTester.cs | 30 +++++++++++++ .../PortablePdb/TestFiles/TestPdbFileLine.il | 21 +++++++++ src/tests/ilasm/ilasm_tests.csproj | 4 ++ src/tools/ilasm/README.md | 2 +- .../Actions/GrammarActions.BuildImage.cs | 25 +++-------- .../src/ILAssembler/CompilationResult.cs | 6 +-- src/tools/ilasm/src/ILAssembler/Options.cs | 5 ++- .../ILAssembler.Tests/CompilerOptionsTests.cs | 45 +++++++++++++++++++ .../GeneratedCases/PdbCaseGenerator.cs | 4 +- .../GeneratedCases/PdbGeneratedCaseTests.cs | 36 +++++++++++++++ .../tests/ILAssembler.Tests/ModuleTests.cs | 2 +- .../ILAssembler.Tests/SourceDirectiveTests.cs | 38 +++++++++++++--- 12 files changed, 184 insertions(+), 34 deletions(-) create mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il diff --git a/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs b/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs index 6653e3994e4437..78745d551c2f10 100644 --- a/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs +++ b/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs @@ -185,6 +185,36 @@ public static void Managed_PdbSwitch_PdbChecksumIsSha256OfThePdbWithItsIdZeroed( Assert.Equal(SHA256.HashData(pdbBytes), checksum.Checksum.ToArray()); } + // Without -DEBUG or -PDB there is no PDB file and no debug directory, even with .line directives or -DET. + [Theory] + [InlineData("TestPdbFile1.il", "")] + [InlineData("TestPdbFile1.il", "-det")] + [InlineData("TestPdbFileLine.il", "")] + [InlineData("TestPdbFileLine.il", "-det")] + public static void Native_NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(string ilSource, string switches) => + NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(Native, ilSource, switches); + + [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] + [InlineData("TestPdbFile1.il", "")] + [InlineData("TestPdbFile1.il", "-det")] + [InlineData("TestPdbFileLine.il", "")] + [InlineData("TestPdbFileLine.il", "-det")] + public static void Managed_NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(string ilSource, string switches) => + NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(Managed, ilSource, switches); + + private static void NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(string kind, string ilSource, string switches) + { + string directory = CreateOutputDirectory($"{nameof(NoPdbSwitch_WritesNoPdbAndNoDebugDirectory)}-{Path.GetFileNameWithoutExtension(ilSource)}{switches}", kind); + string dll = Path.Combine(directory, "Output.dll"); + + Assert.Equal(0, RunIlasm(kind, ilSource, dll, switches)); + + Assert.Equal(new[] { "Output.dll" }, Directory.GetFiles(directory).Select(Path.GetFileName).ToArray()); + using PEReader pe = ReadImage(dll); + DirectoryEntry debugTable = pe.PEHeaders.PEHeader.DebugTableDirectory; + Assert.Equal((0, 0), (debugTable.RelativeVirtualAddress, debugTable.Size)); + } + // -PDB produces the PDB without adding a DebuggableAttribute; -DEBUG adds one. [Theory] [InlineData("-pdb", false)] diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il new file mode 100644 index 00000000000000..a324d4d1134818 --- /dev/null +++ b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime +{ +} + +.assembly TestPdbFileLine +{ +} + +.class public auto ansi beforefieldinit TestPdbFileLineType + extends [System.Runtime]System.Object +{ + .method public hidebysig static int32 Foo() cil managed + { + .line 10,10 : 9,10 'TestPdbFileLine.cs' + ldc.i4.1 + ret + } +} diff --git a/src/tests/ilasm/ilasm_tests.csproj b/src/tests/ilasm/ilasm_tests.csproj index 6388f8c2c2c7c6..17e0231aaecd43 100644 --- a/src/tests/ilasm/ilasm_tests.csproj +++ b/src/tests/ilasm/ilasm_tests.csproj @@ -73,6 +73,10 @@ Always TestFiles\%(FileName)%(Extension) + + Always + TestFiles\%(FileName)%(Extension) + Always TestFiles\%(FileName)%(Extension) diff --git a/src/tools/ilasm/README.md b/src/tools/ilasm/README.md index 7790db4b4b34a6..7ebfc774324d3f 100644 --- a/src/tools/ilasm/README.md +++ b/src/tools/ilasm/README.md @@ -24,7 +24,7 @@ This will update the generated files before building the main project. ilasm writes debug information as a separate Portable PDB file beside the output, as native ilasm does for an output with an extension, and the image gets the same debug directory entry types in the same order. The PDB's content is narrower than native ilasm's: it records only the documents and sequence points given by `.line` directives (native ilasm also records the `.il` source file and its lines) and has no local scopes. -- **Which switches produce a PDB.** `--debug` (`-DEBUG`, `-DEBUG=IMPL`, `-DEBUG=OPT`) and `--pdb` (`-PDB`). +- **Which switches produce a PDB.** `--debug` (`-DEBUG`), `--debug-mode impl|opt` (`-DEBUG=IMPL`, `-DEBUG=OPT`) and `--pdb` (`-PDB`). Without one of them there is no PDB and no debug directory: `.line` directives are still parsed and checked, but their sequence points are not emitted. - **Where it goes.** The PDB is a separate file named after the output with its extension replaced by `.pdb`, in the same directory (`Min.pdb` for `Min.dll` or `Min`). For an output without an extension in a directory whose name has a dot, native ilasm differs: it cuts the whole path at its last dot, so `-OUTPUT=a.b/Min` gives it `a.pdb` beside the `a.b` directory, where ilasm writes `a.b/Min.pdb`. The PDB is not embedded in the image. ilasm writes and closes the image first, then writes the PDB to a new temporary file in the same directory and renames it to `.pdb`, so `.pdb` never holds a partial PDB. If the PDB cannot be written, the run fails and the new image sits beside the previous `.pdb`, if there was one; ilasm tries to delete the temporary file, which can remain if that fails or if the process ends before the rename. - **An output named like its PDB.** If the output path itself ends in `.pdb` (`-OUTPUT=Min.pdb`) and a PDB is requested, the PDB would overwrite the image, so ilasm reports an error and writes nothing. The comparison ignores case on Windows and macOS. - **What the image records.** The image's debug directory has these entries, in this order: a CodeView entry that names the full path of the PDB file and carries the PDB's id; a PdbChecksum entry with the SHA-256 hash of the PDB file with its 20-byte id zeroed; and, with `--deterministic`, a Reproducible entry. Debuggers and symbol tools use them to find the PDB and to check that it matches the image. When no PDB is produced, the image has no debug directory, with or without `--deterministic`. diff --git a/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs b/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs index 86254d6b0e6ac5..72380f15cc557f 100644 --- a/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs +++ b/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs @@ -562,10 +562,10 @@ private ImmutableArray ValidateExports( /// /// /// - /// A PDB is produced when or is set, or when - /// a method has .line sequence points. Otherwise this returns , the image - /// has no debug directory (), and is - /// . + /// A PDB is produced when , or + /// is set; .line directives alone do not produce one. Otherwise this + /// returns , the image has no debug directory (), + /// and is . /// /// /// The PDB is returned in for the caller to write as a separate file; @@ -588,20 +588,9 @@ private ImmutableArray ValidateExports( { portablePdb = null; - // Check if we have any methods with debug info - bool hasDebugInfo = false; - foreach (var entity in _entityRegistry.GetSeenEntities(TableIndex.MethodDef)) - { - if (entity is EntityRegistry.MethodDefinitionEntity method && - method.DebugInfo.SequencePoints.Count > 0) - { - hasDebugInfo = true; - break; - } - } - - // Generate PDB if we have debug info OR if --debug/--pdb options are set - bool generatePdb = hasDebugInfo || _options.Debug || _options.Pdb; + // As in native ilasm, only /DEBUG (any mode) or /PDB produces a PDB. Without them, sequence + // points from .line directives are parsed and validated but not emitted. + bool generatePdb = _options.Debug || _options.DebugMode is not null || _options.Pdb; if (!generatePdb) { return null; diff --git a/src/tools/ilasm/src/ILAssembler/CompilationResult.cs b/src/tools/ilasm/src/ILAssembler/CompilationResult.cs index 261f8b65dfa083..452e094e72d34e 100644 --- a/src/tools/ilasm/src/ILAssembler/CompilationResult.cs +++ b/src/tools/ilasm/src/ILAssembler/CompilationResult.cs @@ -24,9 +24,9 @@ internal CompilationResult(PEBuilder peBuilder, Blob mvidFixup, ImmutableArray when no PDB was requested. /// /// - /// A PDB is produced when or is set, or when - /// the source has .line sequence points. It is not embedded in the image: the image's debug directory - /// holds a CodeView entry that names the PDB path and carries this PDB's id, followed by a PdbChecksum + /// A PDB is produced when , or + /// is set; .line directives alone do not produce one. + /// It is not embedded in the image: the image's debug directory holds a CodeView entry that names the PDB path and carries this PDB's id, followed by a PdbChecksum /// entry with the SHA-256 hash of these bytes with the 20-byte PDB id zeroed, and, with /// , a Reproducible entry. The PDB path is /// when set; otherwise it is with its diff --git a/src/tools/ilasm/src/ILAssembler/Options.cs b/src/tools/ilasm/src/ILAssembler/Options.cs index 9dfe54a13458a1..77ab49b3aeba8d 100644 --- a/src/tools/ilasm/src/ILAssembler/Options.cs +++ b/src/tools/ilasm/src/ILAssembler/Options.cs @@ -131,8 +131,9 @@ public sealed class Options /// When null with Debug=true, uses default (DisableOptimizations). /// /// - /// A non-null value adds a DebuggableAttribute with the selected debugging modes, - /// as does, and likewise only when the source declares an assembly. + /// A non-null value implies : it produces a Portable PDB and adds a + /// DebuggableAttribute with the selected debugging modes, likewise only when the source + /// declares an assembly. /// public DebugMode? DebugMode { get; set; } diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs index 2ec109c2494815..9fd011d9960992 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/CompilerOptionsTests.cs @@ -402,6 +402,51 @@ .method public static void M() cil managed Assert.NotEmpty(pdbReader.MethodDebugInformation); } + [Theory] + [InlineData(DebugMode.Impl)] + [InlineData(DebugMode.Opt)] + public void DebugModeWithoutDebug_ProducesPortablePdb(DebugMode debugMode) + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(PortablePdbSource, new Options { DebugMode = debugMode }); + + Assert.NotNull(result.PortablePdb); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void WithoutDebugOrPdb_ImageHasNoDebugDirectory(bool deterministic, bool lineDirective) + { + string source = lineDirective ? PortablePdbSource : PortablePdbSource.Replace(".line 10 'test.cs'", string.Empty); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Deterministic = deterministic }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + DirectoryEntry debugTable = pe.PEHeaders.PEHeader!.DebugTableDirectory; + + // As in native ilasm: no debug directory at all, not even the Reproducible entry that + // ManagedPEBuilder adds by default to a deterministic image. + Assert.Equal((0, 0), (debugTable.RelativeVirtualAddress, debugTable.Size)); + } + + [Fact] + public void WithoutDebugOrPdb_DeterministicImageCarriesNoDebugData() + { + // Given no debug directory, ManagedPEBuilder writes a Reproducible entry into a deterministic image. + // Clearing the PE header's debug directory alone would leave that entry's bytes in .text, so the + // deterministic image must have the same .text size as a nondeterministic one. + string source = PortablePdbSource.Replace(".line 10 'test.cs'", string.Empty); + + static int TextSize(string source, bool deterministic) + { + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Deterministic = deterministic }); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + return pe.PEHeaders.SectionHeaders.Single(section => section.Name == ".text").VirtualSize; + } + + Assert.Equal(TextSize(source, deterministic: false), TextSize(source, deterministic: true)); + } + [Fact] public void PdbOption_DoesNotAddDebuggableAttribute() { diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs index 1e6358911960dd..822ca4b7786ade 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs @@ -58,8 +58,8 @@ public sealed record GeneratedOptions( string? OutputFileName, string? PdbFilePath) { - /// Whether an option that produces a PDB is set ( or ). - public bool RequestsPdb => Debug || Pdb; + /// Whether a switch that produces a PDB is set (/DEBUG in any mode, or /PDB). + public bool RequestsPdb => Debug || DebugMode is not null || Pdb; /// The PDB path the CodeView entry is expected to name. public string ExpectedCodeViewPath => diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs index 1e537907994e36..8a8e0a452d6d4f 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs @@ -31,6 +31,42 @@ private static (CompilationResult Result, ImmutableArray Image) Compile(Ge return (result, DocumentCompilerTestHelpers.Serialize(result)); } + [Theory] + [MemberData(nameof(PdbCaseGenerator.AnyCaseData), MemberType = typeof(PdbCaseGenerator))] + public void Pdb_IsProducedExactlyWhenASwitchRequestsIt(int index, string description) + { + CompileCase input = PdbCaseGenerator.AnyCases[index]; + + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(input.Program.ToSource(), input.Options.ToOptions()); + + Assert.Equal(input.Options.RequestsPdb, result.PortablePdb.HasValue); + } + + [Theory] + [MemberData(nameof(PdbCaseGenerator.AnyCaseData), MemberType = typeof(PdbCaseGenerator))] + public void WithoutAPdbSwitch_ImageHasNoDebugDirectory(int index, string description) + { + CompileCase input = PdbCaseGenerator.AnyCases[index]; + GeneratedOptions options = input.Options with { Debug = false, DebugMode = null, Pdb = false }; + + (_, ImmutableArray image) = Compile(input.Program, options); + using var pe = new PEReader(image); + DirectoryEntry debugTable = pe.PEHeaders.PEHeader!.DebugTableDirectory; + + Assert.Equal((0, 0), (debugTable.RelativeVirtualAddress, debugTable.Size)); + + // No debug data either: the Reproducible entry ManagedPEBuilder writes by default into a deterministic + // image would make its .text larger than a nondeterministic one's. + Assert.Equal(TextSize(input.Program, options with { Deterministic = false }), TextSize(input.Program, options with { Deterministic = true })); + } + + private static int TextSize(GeneratedProgram program, GeneratedOptions options) + { + (_, ImmutableArray image) = Compile(program, options); + using var pe = new PEReader(image); + return pe.PEHeaders.SectionHeaders.Single(section => section.Name == ".text").VirtualSize; + } + [Theory] [MemberData(nameof(PdbCaseGenerator.PdbRequestedCaseData), MemberType = typeof(PdbCaseGenerator))] public void RequestedPdb_IsReferencedByCodeViewThenPdbChecksumThenReproducibleIffDeterministic(int index, string description) diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs index b75fb060cc99d3..695b0cb6b3bb46 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/ModuleTests.cs @@ -219,7 +219,7 @@ ldc.i4.s 42 } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); var reader = pe.GetMetadataReader(); var moduleType = reader.GetTypeDefinition(MetadataTokens.TypeDefinitionHandle(1)); diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs index 4cf5887af48fc2..31e0f82ec28b47 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs @@ -37,7 +37,7 @@ br END END: ret """); using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbImage( - DocumentCompilerTestHelpers.CompileAndGetPortablePdb(source, new Options { Optimize = optimize })); + DocumentCompilerTestHelpers.CompileAndGetPortablePdb(source, new Options { Optimize = optimize, Debug = true })); MetadataReader reader = provider.GetMetadataReader(); SequencePoint[] points = reader.GetMethodDebugInformation(reader.MethodDebugInformation.Single()) .GetSequencePoints().ToArray(); @@ -178,7 +178,7 @@ .line 20 "test.cs" } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); // Read the PDB and verify contents using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); @@ -217,7 +217,7 @@ .line 20 "test.cs" } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); // Read the PDB and verify contents using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); @@ -260,6 +260,30 @@ .method public static void TestMethod() cil managed Assert.Empty(pe.ReadDebugDirectory()); } + [Fact] + public void LineDirective_WithoutDebugOrPdb_ProducesNoPdb() + { + string source = """ + .assembly test { } + .class public auto ansi beforefieldinit Test + { + .method public static void TestMethod() cil managed + { + .line 10 "test.cs" + nop + ret + } + } + """; + + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); + + // As in native ilasm, .line alone produces no PDB: only /DEBUG or /PDB does. + Assert.Null(result.PortablePdb); + Assert.Empty(pe.ReadDebugDirectory()); + } + [Fact] public void StringEscape_NewlineInLdstr() @@ -337,7 +361,7 @@ .method public static void TestMethod() cil managed } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); var reader = pe.GetMetadataReader(); var methodHandle = reader.MethodDefinitions.Single(handle => reader.GetString(reader.GetMethodDefinition(handle).Name) == "TestMethod"); @@ -390,7 +414,7 @@ .method public static void M() cil managed } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); using var pe = new PEReader(DocumentCompilerTestHelpers.Serialize(result)); var reader = pe.GetMetadataReader(); var methodHandle = reader.MethodDefinitions @@ -428,7 +452,7 @@ .line 10 "document.cs" } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); var document = pdbReader.GetDocument(Assert.Single(pdbReader.Documents)); @@ -457,7 +481,7 @@ .line 10 } """; - CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options()); + CompilationResult result = DocumentCompilerTestHelpers.CompileAndGetResult(source, new Options { Debug = true }); using var pdbProvider = DocumentCompilerTestHelpers.GetPortablePdbReaderProvider(result); var pdbReader = pdbProvider.GetMetadataReader(); var document = pdbReader.GetDocument(Assert.Single(pdbReader.Documents)); From 4c298ff72a1121b7c507fbbd8ea0f254f899e0ec Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Tue, 6 Oct 2026 08:24:24 -0700 Subject: [PATCH 3/9] Assemble the PDB determinism check to its own image in the IL round-trip harness The "Test PDB determinism" step assembled with -DEBUG to -output=.pdb and hashed that file. With native ilasm the PDB is .pdb, the same path, so the PDB overwrote the image and the step hashed the PDB. With the previous managed ilasm the step hashed the image, which had the PDB embedded, so it also checked that the image assembled with -DEBUG repeats. The managed ilasm now writes the PDB beside the output and refuses an output path the PDB would overwrite, so the step failed with "ILASM failed with exit code 1" in the managedilasmroundtrip scenario. Assemble to IL-RT/.pdbcheck.dll instead and hash both that image and the PDB that ilasm writes beside it, IL-RT/.pdbcheck.pdb, and fail the step if either differs between the two runs. Native and managed ilasm both write that pair, so the step checks the -DEBUG image and the PDB for both again. The step also no longer overwrites the test's own .pdb, the C# compiler's PDB, in the test directory. The rest of the round trip is unchanged. --- src/tests/Common/CLRTest.Jit.targets | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/tests/Common/CLRTest.Jit.targets b/src/tests/Common/CLRTest.Jit.targets index 034a69bdd480fd..a53a8f4f8efe35 100644 --- a/src/tests/Common/CLRTest.Jit.targets +++ b/src/tests/Common/CLRTest.Jit.targets @@ -430,9 +430,11 @@ for inputAssemblyName in glob.glob("*.dll"): if not is_managed_debug_assembly(inputAssemblyName): ilasmSwitches = ilasmSwitches + " -DEBUG" - pdbName = inputAssemblyName.replace('.dll', '.pdb') + # Assemble to an image of its own and hash that image and the PDB that ilasm writes beside it. + pdbCheckAssemblyName = os.path.join("IL-RT", assemblyNameWithoutExtension + ".pdbcheck.dll") + pdbName = os.path.join("IL-RT", assemblyNameWithoutExtension + ".pdbcheck.pdb") - ilasm_args = f'{ilasm_path} -output={pdbName} {ilasmSwitches} {disassemblyName}' + ilasm_args = f'{ilasm_path} -output={pdbCheckAssemblyName} {ilasmSwitches} {disassemblyName}' print(ilasm_args) proc = subprocess.Popen(ilasm_args, shell=True) @@ -446,9 +448,10 @@ for inputAssemblyName in glob.glob("*.dll"): print(f"ILASM failed with exit code {proc.returncode}") sys.exit(1) + imageHash = hash_file(pdbCheckAssemblyName) hash = hash_file(pdbName) - ilasm_args = f'{ilasm_path} -output={pdbName} {ilasmSwitches} {disassemblyName}' + ilasm_args = f'{ilasm_path} -output={pdbCheckAssemblyName} {ilasmSwitches} {disassemblyName}' print(ilasm_args) proc = subprocess.Popen(ilasm_args, shell=True) @@ -462,6 +465,10 @@ for inputAssemblyName in glob.glob("*.dll"): print(f"ILASM failed with exit code {proc.returncode}") sys.exit(1) + if imageHash != hash_file(pdbCheckAssemblyName): + print("ILASM determinism with -DEBUG failed") + sys.exit(1) + if hash != hash_file(pdbName): print("ILASM PDB determinism failed") sys.exit(1) From 5fe16545a3a5a0d8b93f769a34c446b88fd04c77 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Tue, 6 Oct 2026 14:31:45 -0700 Subject: [PATCH 4/9] Split the output writer into a stream-based type in ILAssembler and a path-based type in ilasm OutputWriter moves to the ILAssembler library and works on streams only: it writes and closes the image, then writes the PDB, or, when no PDB was produced, deletes the existing PDB only if the replaced image's CodeView entry names its id. It refuses to write a PDB over the output. The caller supplies the output and the PDB through the new IOutputStreams interface, so every decision can be tested with in-memory streams. OutputFileWriter in the ilasm tool backs those streams with files: it resolves .pdb, opens the existing image and PDB, creates the output, writes the PDB to a temporary file in the same directory and renames it into place, and deletes a stale PDB. Program calls it. The behaviour of the tool is unchanged. The tests that need no file system now run on streams in OutputWriterTests; OutputFileWriterTests keeps the path, temporary file, rename, deletion and same-file tests, and the seeded output-writer theories run against OutputFileWriter. --- .../ilasm/src/ILAssembler/OutputWriter.cs | 253 ++++++++++ src/tools/ilasm/src/ilasm/OutputFileWriter.cs | 138 ++++++ src/tools/ilasm/src/ilasm/OutputWriter.cs | 290 ------------ src/tools/ilasm/src/ilasm/Program.cs | 4 +- .../GeneratedCases/PdbCaseGenerator.cs | 4 +- .../GeneratedCases/PdbGeneratedCaseTests.cs | 16 +- .../ILAssembler.Tests.csproj | 4 +- .../OutputFileWriterTests.cs | 189 ++++++++ .../ILAssembler.Tests/OutputWriterTests.cs | 446 +++++++++--------- 9 files changed, 820 insertions(+), 524 deletions(-) create mode 100644 src/tools/ilasm/src/ILAssembler/OutputWriter.cs create mode 100644 src/tools/ilasm/src/ilasm/OutputFileWriter.cs delete mode 100644 src/tools/ilasm/src/ilasm/OutputWriter.cs create mode 100644 src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs diff --git a/src/tools/ilasm/src/ILAssembler/OutputWriter.cs b/src/tools/ilasm/src/ILAssembler/OutputWriter.cs new file mode 100644 index 00000000000000..79da0880119f11 --- /dev/null +++ b/src/tools/ilasm/src/ILAssembler/OutputWriter.cs @@ -0,0 +1,253 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; + +namespace ILAssembler; + +/// +/// Writes an assembled image and its Portable PDB through the streams of an , and +/// decides what happens to an existing PDB when no PDB is produced. +/// +/// +/// +/// This type orders the writes and makes the decisions; it does not touch the file system. The caller supplies the +/// output and the PDB as streams: the ilasm tool backs them with the output file and <output>.pdb +/// beside it. +/// +/// +/// The image is written and its stream closed before the PDB is written, read or deleted, so a failure while +/// writing or closing the image leaves the existing PDB as it was; the image itself may then be partial. +/// +/// +/// When no PDB is produced, the existing PDB is deleted after the image is written only if it belongs to the image +/// being replaced: the output as it was before the new image was written is a PE image whose first CodeView entry +/// has a PDB id (GUID and stamp) equal to the id of that Portable PDB. Anything else at the PDB path is left in +/// place, including a PDB when there was no previous output, a PDB of another image, and a file that is not a +/// Portable PDB. A previous output or PDB that cannot be opened or read counts as having no id, so it never causes a +/// deletion and never fails the write. +/// +/// +/// When a PDB is produced and the PDB path names the output itself (), +/// nothing is written and is returned. When no PDB is +/// produced and the PDB path names the output, the file there is the new image, so it is neither read as a PDB nor +/// deleted. +/// +/// +public static class OutputWriter +{ + /// + /// Writes the image to , then writes + /// with or, when there is no PDB, deletes the PDB of the image that was + /// replaced. + /// + /// The output and its PDB. + /// Writes the image to the stream it is given. + /// The PDB, or when none was produced. + /// What was written or deleted. + /// + /// An exception from , from creating or closing the output, or from + /// propagates to the caller. A PDB that belongs to the replaced image but + /// cannot be deleted ( returns ) is left in place + /// without failing the write. + /// + public static OutputWriteResult Write(IOutputStreams output, Action writeImage, ImmutableArray? portablePdb) + { + bool pdbPathIsOutputPath = output.PdbPathIsOutputPath; + if (portablePdb is not null && pdbPathIsOutputPath) + { + return OutputWriteResult.PdbWouldOverwriteOutput; + } + + // Which PDB the image being replaced refers to, read before the image is overwritten. Without a new + // PDB, only that PDB is deleted. + BlobContentId? replacedImagePdbId = portablePdb is null && !pdbPathIsOutputPath ? TryReadPdbIdOfExistingOutput(output) : null; + + // Write and close the image before touching the PDB. If this throws, the PDB is left as it was. + using (Stream imageStream = output.CreateOutput()) + { + writeImage(imageStream); + } + + if (portablePdb is { } pdb) + { + output.WritePdb(pdb); + return OutputWriteResult.ImageAndPdbWritten; + } + + if (replacedImagePdbId is { } pdbId && TryReadIdOfExistingPdb(output) == pdbId && output.TryDeletePdb()) + { + return OutputWriteResult.ImageWrittenStalePdbDeleted; + } + + return OutputWriteResult.ImageWritten; + } + + /// + /// Gets the PDB id that the existing output refers to, or when there is no existing + /// output, it cannot be opened or read, or finds no id in it. + /// + private static BlobContentId? TryReadPdbIdOfExistingOutput(IOutputStreams output) + { + try + { + using Stream? stream = output.OpenExistingOutput(); + return stream is null ? null : TryReadCodeViewPdbId(stream); + } + catch (Exception ex) when (ex is BadImageFormatException or IOException or UnauthorizedAccessException) + { + return null; + } + } + + /// + /// Gets the id of the existing PDB, or when there is no existing PDB, it cannot be opened + /// or read, or finds no id in it. + /// + private static BlobContentId? TryReadIdOfExistingPdb(IOutputStreams output) + { + try + { + using Stream? stream = output.OpenExistingPdb(); + return stream is null ? null : TryReadPortablePdbId(stream); + } + catch (Exception ex) when (ex is BadImageFormatException or IOException or UnauthorizedAccessException) + { + return null; + } + } + + /// + /// Gets the id (GUID and stamp) of the PDB that the image in refers to in its first + /// CodeView entry, or when the stream is longer than bytes, + /// holds a COFF object file rather than a PE image, or the image has no CodeView entry. + /// + /// The stream holds neither a PE image nor a COFF object file. + internal static BlobContentId? TryReadCodeViewPdbId(Stream stream) + { + // System.Reflection.Metadata reads at most int.MaxValue bytes and throws ArgumentException for a longer stream. + if (stream.Length > int.MaxValue) + { + return null; + } + + using var peReader = new PEReader(stream, PEStreamOptions.PrefetchEntireImage | PEStreamOptions.LeaveOpen); + + // A file that does not start with "MZ" is read as a COFF object file (twenty zero bytes read as one). It has + // no PE header and so no debug directory, and ReadDebugDirectory requires the PE header. + if (peReader.PEHeaders.IsCoffOnly) + { + return null; + } + + foreach (DebugDirectoryEntry entry in peReader.ReadDebugDirectory()) + { + if (entry.Type == DebugDirectoryEntryType.CodeView) + { + return new BlobContentId(peReader.ReadCodeViewDebugDirectoryData(entry).Guid, entry.Stamp); + } + } + + return null; + } + + /// + /// Gets the id of the Portable PDB in , or when the stream is + /// longer than bytes or holds metadata that is not a Portable PDB. + /// + /// The stream does not hold metadata. + internal static BlobContentId? TryReadPortablePdbId(Stream stream) + { + // System.Reflection.Metadata reads at most int.MaxValue bytes and throws ArgumentException for a longer stream. + if (stream.Length > int.MaxValue) + { + return null; + } + + using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbStream( + stream, + MetadataStreamOptions.PrefetchMetadata | MetadataStreamOptions.LeaveOpen); + DebugMetadataHeader? header = provider.GetMetadataReader().DebugMetadataHeader; + return header is null ? null : new BlobContentId(header.Id); + } +} + +/// +/// The output and its PDB, as streams, for . +/// +/// +/// uses these members in this order, each at most once: +/// ; , only when no PDB is produced; +/// ; then either , or followed by +/// when the existing PDB belongs to the replaced image. It disposes every stream it is +/// given. +/// +public interface IOutputStreams +{ + /// + /// Gets whether the PDB path names the output itself (an output named Min.pdb, for example), so that + /// writing a PDB would overwrite the image. + /// + bool PdbPathIsOutputPath { get; } + + /// + /// Opens the output as it is before the new image is written, for reading, or returns + /// when there is no output yet. + /// + /// + /// An , or + /// from this method or from reading the stream counts as an output with no + /// PDB id. + /// + Stream? OpenExistingOutput(); + + /// + /// Creates the output, replacing any existing one, and returns the stream the image is written to. + /// + Stream CreateOutput(); + + /// + /// Writes to the PDB path, replacing what is there. + /// + void WritePdb(ImmutableArray pdb); + + /// + /// Opens the file at the PDB path for reading, or returns when there is none. + /// + /// + /// An , or + /// from this method or from reading the stream counts as a file with no + /// PDB id. + /// + Stream? OpenExistingPdb(); + + /// + /// Deletes the file at the PDB path. + /// + /// if it was deleted; if it could not be. + bool TryDeletePdb(); +} + +/// +/// What wrote or deleted. +/// +public enum OutputWriteResult +{ + /// The image was written. No PDB was produced, and no PDB was deleted. + ImageWritten, + + /// + /// The image was written. No PDB was produced, and the PDB of the image it replaced was deleted. + /// + ImageWrittenStalePdbDeleted, + + /// The image and then its PDB were written. + ImageAndPdbWritten, + + /// Nothing was written: the PDB path names the output itself. + PdbWouldOverwriteOutput, +} diff --git a/src/tools/ilasm/src/ilasm/OutputFileWriter.cs b/src/tools/ilasm/src/ilasm/OutputFileWriter.cs new file mode 100644 index 00000000000000..df4d687defdc6b --- /dev/null +++ b/src/tools/ilasm/src/ilasm/OutputFileWriter.cs @@ -0,0 +1,138 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; + +namespace ILAssembler; + +/// +/// Writes an assembled image and its Portable PDB to files: the output, and <output>.pdb beside it. +/// +/// +/// +/// The PDB is a separate file beside the output (), as with native ilasm. +/// is called only when the assembly produced an image: when it succeeded, or despite errors with +/// the error-tolerant option (--error, /ERR), in which case the image and PDB are handled exactly as for +/// a successful assembly. When the assembly produces no image, the tool calls nothing here and leaves the existing +/// output and PDB as they are. +/// +/// +/// orders the writes and decides which existing PDB is deleted; this type gives it +/// the two files as streams. An output or PDB that does not exist is opened as none. +/// +/// +/// The PDB is written to a new temporary file in the same directory and then renamed over <output>.pdb, +/// so the PDB path holds either the file that was there before or the complete new PDB. If writing or renaming the +/// temporary file fails, the run fails with the new image already written, and deleting the temporary file is +/// attempted; it can remain if that deletion fails or the process ends before the rename. Because the PDB is renamed +/// into place, a symbolic or hard link at <output>.pdb is replaced by a regular file and the link's +/// target keeps its old content; deleting a stale PDB likewise removes a symbolic link, not its target. A stale PDB +/// that cannot be deleted is left in place without failing the write. +/// +/// +internal static class OutputFileWriter +{ + /// + /// Gets the full path of the PDB for an output: the output path with its extension replaced by .pdb. + /// + public static string GetPdbPath(string outputPath) => Path.GetFullPath(Path.ChangeExtension(outputPath, ".pdb")); + + /// + /// Gets whether names the output file itself, as it does for an output named + /// Min.pdb. + /// + /// + /// The comparison ignores case on Windows and macOS, whose default file systems do, so that + /// Min.PDB and Min.pdb are treated as the same file there; elsewhere it is exact. + /// + public static bool IsOutputPath(string pdbPath, string outputPath) => + string.Equals( + pdbPath, + Path.GetFullPath(outputPath), + OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); + + /// + /// Writes the image to , then writes to + /// or, when there is no PDB, deletes the PDB of the image that was replaced, as + /// decides. + /// + /// The path of the image. + /// The path of the PDB, from . + /// Writes the image to the stream it is given. + /// The PDB, or when none was produced. + /// What was written or deleted. + public static OutputWriteResult Write(string outputPath, string pdbPath, Action writeImage, ImmutableArray? portablePdb) => + OutputWriter.Write(new OutputFiles(outputPath, pdbPath), writeImage, portablePdb); + + /// + /// Gets a path in the PDB's directory for the temporary file the PDB is written to before it is renamed. + /// + internal static string GetTemporaryPdbPath(string pdbPath) => + Path.Combine(Path.GetDirectoryName(pdbPath)!, $"{Path.GetFileName(pdbPath)}.{Path.GetRandomFileName()}.tmp"); + + /// + /// Writes to a new file at and renames it over + /// . + /// + /// + /// The temporary file is created only if no file exists at . If the write or + /// the rename fails, deleting the temporary file is attempted only when this call created it, and + /// is left as it was. + /// + internal static void WritePdb(string pdbPath, string temporaryPath, ImmutableArray pdb) + { + bool created = false; + try + { + using (var stream = new FileStream(temporaryPath, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + { + created = true; + stream.Write(pdb.AsSpan()); + } + + File.Move(temporaryPath, pdbPath, overwrite: true); + } + catch + { + if (created) + { + TryDelete(temporaryPath); + } + + throw; + } + } + + private static bool TryDelete(string path) + { + try + { + File.Delete(path); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return false; + } + } + + private static FileStream? OpenReadIfExists(string path) => File.Exists(path) ? File.OpenRead(path) : null; + + /// The output file and the PDB file, as the streams writes through. + private sealed class OutputFiles(string outputPath, string pdbPath) : IOutputStreams + { + public bool PdbPathIsOutputPath => IsOutputPath(pdbPath, outputPath); + + public Stream? OpenExistingOutput() => OpenReadIfExists(outputPath); + + public Stream CreateOutput() => File.Create(outputPath); + + public void WritePdb(ImmutableArray pdb) => OutputFileWriter.WritePdb(pdbPath, GetTemporaryPdbPath(pdbPath), pdb); + + public Stream? OpenExistingPdb() => OpenReadIfExists(pdbPath); + + public bool TryDeletePdb() => TryDelete(pdbPath); + } +} diff --git a/src/tools/ilasm/src/ilasm/OutputWriter.cs b/src/tools/ilasm/src/ilasm/OutputWriter.cs deleted file mode 100644 index 03c7eb780a5c64..00000000000000 --- a/src/tools/ilasm/src/ilasm/OutputWriter.cs +++ /dev/null @@ -1,290 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Immutable; -using System.IO; -using System.Reflection.Metadata; -using System.Reflection.PortableExecutable; - -namespace ILAssembler; - -/// -/// Writes an assembled image and its Portable PDB to disk. -/// -/// -/// -/// The PDB is a separate file beside the output (), as with native ilasm. -/// is called only when the assembly -/// produced an image: when it succeeded, or despite errors with the error-tolerant option (--error, -/// /ERR), in which case the image and PDB are handled exactly as for a successful assembly. When the assembly -/// produces no image, the tool calls nothing here and leaves the existing output and PDB as they are. -/// -/// -/// The image is written and closed before the PDB is touched, so a failure while writing or closing the image -/// leaves the existing PDB as it was; the image itself may then be partial. The PDB is written to a new temporary -/// file in the same directory and then renamed over <output>.pdb, so the PDB path holds either the -/// file that was there before or the complete new PDB. If writing or renaming the temporary file fails, the run -/// fails with the new image already written, and deleting the temporary file is attempted; it can remain if that -/// deletion fails or the process ends before the rename. Because the PDB is renamed into place, a symbolic or hard -/// link at <output>.pdb is replaced by a regular file and the link's target keeps its old content; -/// deleting a PDB (below) likewise removes a symbolic link, not its target. -/// -/// -/// When no PDB is produced, an existing <output>.pdb is deleted after the image is written only if it -/// belongs to the image being replaced: the image previously at the output path has a CodeView entry whose PDB id -/// (GUID and stamp) equals the id of that Portable PDB. Any other file at the PDB path is left in place, including -/// a PDB beside an output that did not exist before, a PDB of another image, and a file that is not a Portable PDB. -/// -/// -internal static class OutputWriter -{ - /// - /// Gets the full path of the PDB for an output: the output path with its extension replaced by .pdb. - /// - public static string GetPdbPath(string outputPath) => Path.GetFullPath(Path.ChangeExtension(outputPath, ".pdb")); - - /// - /// Gets whether names the output file itself, as it does for an output named - /// Min.pdb. - /// - /// - /// The comparison ignores case on Windows and macOS, whose default file systems do, so that - /// Min.PDB and Min.pdb are treated as the same file there; elsewhere it is exact. - /// - public static bool IsOutputPath(string pdbPath, string outputPath) => - string.Equals( - pdbPath, - Path.GetFullPath(outputPath), - OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); - - /// - /// Writes the image to , then writes to - /// or, when there is no PDB, deletes the PDB of the image that was replaced. - /// - /// The path of the image. - /// The path of the PDB, from . - /// Writes the image to the stream it is given. - /// The PDB, or when none was produced. - /// What was written or deleted. - /// - /// When a PDB is produced and names the output file itself, nothing is written - /// and is returned. When no PDB is produced and - /// names the output file, that file is the new image and is not deleted. - /// A PDB that belongs to the replaced image but cannot be deleted is left in place without failing the write. - /// - public static OutputWriteResult Write(string outputPath, string pdbPath, Action writeImage, ImmutableArray? portablePdb) => - Write(outputPath, pdbPath, writeImage, portablePdb, File.Create); - - /// - /// , with the stream the image is - /// written to created by . - /// - internal static OutputWriteResult Write( - string outputPath, - string pdbPath, - Action writeImage, - ImmutableArray? portablePdb, - Func createImageStream) - { - bool pdbPathIsOutputPath = IsOutputPath(pdbPath, outputPath); - if (portablePdb is not null && pdbPathIsOutputPath) - { - return OutputWriteResult.PdbWouldOverwriteOutput; - } - - // Which PDB the image being replaced refers to, read before the image is overwritten. Without a new - // PDB, only that PDB is deleted. - BlobContentId? replacedImagePdbId = portablePdb is null && !pdbPathIsOutputPath ? TryReadCodeViewPdbId(outputPath) : null; - - // Write and close the image before touching the PDB. If this throws, the PDB is left as it was. - using (Stream imageStream = createImageStream(outputPath)) - { - writeImage(imageStream); - } - - if (portablePdb is { } pdb) - { - WritePdb(pdbPath, GetTemporaryPdbPath(pdbPath), pdb); - return OutputWriteResult.ImageAndPdbWritten; - } - - if (replacedImagePdbId is { } pdbId && TryReadPortablePdbId(pdbPath) == pdbId && TryDelete(pdbPath)) - { - return OutputWriteResult.ImageWrittenStalePdbDeleted; - } - - return OutputWriteResult.ImageWritten; - } - - /// - /// Gets the id (GUID and stamp) of the PDB that the image at refers to in its - /// first CodeView entry, or when there is no such file, it cannot be read, or - /// finds no id in it. - /// - internal static BlobContentId? TryReadCodeViewPdbId(string imagePath) - { - try - { - if (!File.Exists(imagePath)) - { - return null; - } - - using FileStream stream = File.OpenRead(imagePath); - return TryReadCodeViewPdbId(stream); - } - catch (Exception ex) when (ex is BadImageFormatException or IOException or UnauthorizedAccessException) - { - return null; - } - } - - /// - /// Gets the id (GUID and stamp) of the PDB that the image in refers to in its first - /// CodeView entry, or when the stream is longer than bytes, - /// holds a COFF object file rather than a PE image, or the image has no CodeView entry. - /// - /// The stream holds neither a PE image nor a COFF object file. - internal static BlobContentId? TryReadCodeViewPdbId(Stream stream) - { - // System.Reflection.Metadata reads at most int.MaxValue bytes and throws ArgumentException for a longer stream. - if (stream.Length > int.MaxValue) - { - return null; - } - - using var peReader = new PEReader(stream, PEStreamOptions.PrefetchEntireImage | PEStreamOptions.LeaveOpen); - - // A file that does not start with "MZ" is read as a COFF object file (twenty zero bytes read as one). It has - // no PE header and so no debug directory, and ReadDebugDirectory requires the PE header. - if (peReader.PEHeaders.IsCoffOnly) - { - return null; - } - - foreach (DebugDirectoryEntry entry in peReader.ReadDebugDirectory()) - { - if (entry.Type == DebugDirectoryEntryType.CodeView) - { - return new BlobContentId(peReader.ReadCodeViewDebugDirectoryData(entry).Guid, entry.Stamp); - } - } - - return null; - } - - /// - /// Gets the id of the Portable PDB at , or when there is no - /// such file, it cannot be read, or finds no id in it. - /// - internal static BlobContentId? TryReadPortablePdbId(string pdbPath) - { - try - { - if (!File.Exists(pdbPath)) - { - return null; - } - - using FileStream stream = File.OpenRead(pdbPath); - return TryReadPortablePdbId(stream); - } - catch (Exception ex) when (ex is BadImageFormatException or IOException or UnauthorizedAccessException) - { - return null; - } - } - - /// - /// Gets the id of the Portable PDB in , or when the stream is - /// longer than bytes or holds metadata that is not a Portable PDB. - /// - /// The stream does not hold metadata. - internal static BlobContentId? TryReadPortablePdbId(Stream stream) - { - // System.Reflection.Metadata reads at most int.MaxValue bytes and throws ArgumentException for a longer stream. - if (stream.Length > int.MaxValue) - { - return null; - } - - using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbStream( - stream, - MetadataStreamOptions.PrefetchMetadata | MetadataStreamOptions.LeaveOpen); - DebugMetadataHeader? header = provider.GetMetadataReader().DebugMetadataHeader; - return header is null ? null : new BlobContentId(header.Id); - } - - /// - /// Gets a path in the PDB's directory for the temporary file the PDB is written to before it is renamed. - /// - internal static string GetTemporaryPdbPath(string pdbPath) => - Path.Combine(Path.GetDirectoryName(pdbPath)!, $"{Path.GetFileName(pdbPath)}.{Path.GetRandomFileName()}.tmp"); - - /// - /// Writes to a new file at and renames it over - /// . - /// - /// - /// The temporary file is created only if no file exists at . If the write or - /// the rename fails, deleting the temporary file is attempted only when this call created it, and - /// is left as it was. - /// - internal static void WritePdb(string pdbPath, string temporaryPath, ImmutableArray pdb) - { - bool created = false; - try - { - using (var stream = new FileStream(temporaryPath, FileMode.CreateNew, FileAccess.Write, FileShare.None)) - { - created = true; - stream.Write(pdb.AsSpan()); - } - - File.Move(temporaryPath, pdbPath, overwrite: true); - } - catch - { - if (created) - { - TryDelete(temporaryPath); - } - - throw; - } - } - - private static bool TryDelete(string path) - { - try - { - File.Delete(path); - return true; - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - return false; - } - } -} - -/// -/// What wrote or deleted. -/// -internal enum OutputWriteResult -{ - /// The image was written. No PDB was produced, and no PDB was deleted. - ImageWritten, - - /// - /// The image was written. No PDB was produced, and the PDB of the image it replaced was deleted. - /// - ImageWrittenStalePdbDeleted, - - /// The image and then its PDB were written. - ImageAndPdbWritten, - - /// Nothing was written: the PDB path names the output file itself. - PdbWouldOverwriteOutput, -} diff --git a/src/tools/ilasm/src/ilasm/Program.cs b/src/tools/ilasm/src/ilasm/Program.cs index c63c3430e33a0e..a5e7f34e108ee4 100644 --- a/src/tools/ilasm/src/ilasm/Program.cs +++ b/src/tools/ilasm/src/ilasm/Program.cs @@ -65,7 +65,7 @@ public int Run() { // As in native ilasm, the PDB is .pdb beside the output, and the // image's CodeView entry records its full path. - string pdbPath = OutputWriter.GetPdbPath(outputPath); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); // Report each file being assembled foreach (string file in inputFiles) @@ -258,7 +258,7 @@ byte[] LoadResource(string path) // Every failure above returns before this, leaving the existing files as they are. var blobBuilder = new BlobBuilder(); compilationResult.Serialize(blobBuilder); - OutputWriteResult written = OutputWriter.Write( + OutputWriteResult written = OutputFileWriter.Write( outputPath, pdbPath, stream => blobBuilder.WriteContentTo(stream), diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs index 822ca4b7786ade..c827b370eea69a 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbCaseGenerator.cs @@ -118,7 +118,7 @@ public enum ExistingOutput /// /// The inputs of the generated-case theories. Compilation cases come from a with a fixed /// seed, so every run builds the same cases and a failing case is reproduced by its index; the display name - /// also describes it. The output writer's states are few, so they are enumerated exhaustively. + /// also describes it. The output file writer's states are few, so they are enumerated exhaustively. /// public static class PdbCaseGenerator { @@ -141,7 +141,7 @@ public static class PdbCaseGenerator public static TheoryData PdbRequestedCaseData => ToTheoryData(PdbRequestedCases); /// - /// An image and the PDB its CodeView entry refers to, which the output writer states place at the output and + /// An image and the PDB its CodeView entry refers to, which the output file writer states place at the output and /// PDB paths, and an image of another PDB. /// public static (ImmutableArray Image, ImmutableArray Pdb) ExistingPair { get; } = DocumentCompilerTestHelpers.CompileImageAndPdb("A"); diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs index 8a8e0a452d6d4f..ec51c9594909ac 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/GeneratedCases/PdbGeneratedCaseTests.cs @@ -17,9 +17,9 @@ namespace ILAssembler.Tests.GeneratedCases { /// /// The PDB rules, checked over seeded generated programs and option combinations - /// (), and for the output writer over every combination of output name, - /// pre-existing output, pre-existing PDB path and PDB presence that - /// enumerates. + /// (), and for the output file writer () over every + /// combination of output name, pre-existing output, pre-existing PDB path and PDB presence that + /// enumerates. /// public class PdbGeneratedCaseTests { @@ -166,7 +166,7 @@ public void Deterministic_DifferentSources_GiveDifferentImagesAndPdbIds(int inde [Theory] [MemberData(nameof(PdbCaseGenerator.OutputWriteStates), MemberType = typeof(PdbCaseGenerator))] - public void OutputWriter_NeverLeavesATemporaryFile(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) + public void OutputFileWriter_NeverLeavesATemporaryFile(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) { foreach (bool imageWriteFails in new[] { false, true }) { @@ -177,7 +177,7 @@ public void OutputWriter_NeverLeavesATemporaryFile(string outputFileName, Existi [Theory] [MemberData(nameof(PdbCaseGenerator.OutputWriteStates), MemberType = typeof(PdbCaseGenerator))] - public void OutputWriter_WhenTheImageWriteFails_LeavesThePdbPathUnchanged(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) + public void OutputFileWriter_WhenTheImageWriteFails_LeavesThePdbPathUnchanged(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) { RunOutputWrite(outputFileName, existingOutput, existingPdb, withPdb, imageWriteFails: true, (_, pdbPath, exception) => { @@ -199,7 +199,7 @@ public void OutputWriter_WhenTheImageWriteFails_LeavesThePdbPathUnchanged(string [Theory] [MemberData(nameof(PdbCaseGenerator.OutputWriteStates), MemberType = typeof(PdbCaseGenerator))] - public void OutputWriter_AfterTheImageIsWritten_LeavesTheNewPdbOrDeletesOnlyTheReplacedImagesPdb(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) + public void OutputFileWriter_AfterTheImageIsWritten_LeavesTheNewPdbOrDeletesOnlyTheReplacedImagesPdb(string outputFileName, ExistingOutput existingOutput, ExistingPdb existingPdb, bool withPdb) { RunOutputWrite(outputFileName, existingOutput, existingPdb, withPdb, imageWriteFails: false, (_, pdbPath, exception) => { @@ -240,7 +240,7 @@ private static void RunOutputWrite( try { string outputPath = Path.Combine(directory, outputFileName); - string pdbPath = OutputWriter.GetPdbPath(outputPath); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); switch (existingOutput) { case ExistingOutput.ImageOfThePdb: @@ -267,7 +267,7 @@ private static void RunOutputWrite( break; } - Exception? exception = Record.Exception(() => OutputWriter.Write( + Exception? exception = Record.Exception(() => OutputFileWriter.Write( outputPath, pdbPath, stream => diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj index d0886ad184c72c..a845a4021aeccf 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj +++ b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj @@ -22,8 +22,8 @@ NativeCommandLine.cs - - OutputWriter.cs + + OutputFileWriter.cs diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs new file mode 100644 index 00000000000000..28190773130d80 --- /dev/null +++ b/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs @@ -0,0 +1,189 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using Xunit; + +namespace ILAssembler.Tests; + +/// +/// Tests of on real files: the PDB path, the temporary file and rename, deletion, +/// and how paths that name the same file are recognized. The decisions it delegates to +/// are tested on streams in . +/// +public sealed class OutputFileWriterTests : IDisposable +{ + private static readonly byte[] s_image = [0x4D, 0x5A, 0x01, 0x02]; + private static readonly ImmutableArray s_pdb = [0x42, 0x53, 0x4A, 0x42, 0x03]; + private static readonly byte[] s_stale = [0xDE, 0xAD]; + + // An image and the PDB its CodeView entry refers to. + private static readonly (ImmutableArray Image, ImmutableArray Pdb) s_pair = DocumentCompilerTestHelpers.CompileImageAndPdb("A"); + + private readonly string _directory = Directory.CreateTempSubdirectory("ilasm-output-").FullName; + + public void Dispose() => Directory.Delete(_directory, recursive: true); + + private string PathOf(string fileName) => Path.Combine(_directory, fileName); + + private string[] FileNames() => Directory.EnumerateFileSystemEntries(_directory).Select(Path.GetFileName).Order().ToArray()!; + + private static void WriteImage(Stream stream) => stream.Write(s_image); + + [Theory] + [InlineData("Min.dll", "Min.pdb")] + [InlineData("Min", "Min.pdb")] + [InlineData("a.b/Min", "a.b/Min.pdb")] + public void GetPdbPath_ReplacesTheExtensionAndReturnsTheFullPath(string outputPath, string expectedPdbPath) + { + Assert.Equal(Path.GetFullPath(expectedPdbPath), OutputFileWriter.GetPdbPath(outputPath)); + } + + [Fact] + public void Write_WithPdb_WritesTheImageAndThePdbBesideIt() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.ImageAndPdbWritten, result); + Assert.Equal(s_image, File.ReadAllBytes(outputPath)); + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + } + + [Fact] + public void Write_WithPdb_ReplacesAnExistingPdb() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + + OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithPdb_RenamesANewFileOverTheExistingPdbInsteadOfRewritingIt() + { + // Renaming a completed temporary file over the PDB path is what keeps a failed write from leaving a + // partial PDB. It is observable as a reader of the old PDB still seeing the old content afterwards. + // Renaming over a file that is open depends on the Windows version, so this runs elsewhere. + if (OperatingSystem.IsWindows()) + { + return; + } + + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + using var oldPdbReader = new FileStream(pdbPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + + OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + byte[] oldContent = new byte[s_stale.Length]; + oldPdbReader.ReadExactly(oldContent); + Assert.Equal(s_stale, oldContent); + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithoutPdb_DeletesThePdbOfTheImageItReplaces() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWrittenStalePdbDeleted, result); + Assert.Equal(new[] { "Min.dll" }, FileNames()); + } + + [Fact] + public void Write_WhenThePdbCannotBeWritten_RemovesOnlyItsTemporaryFile() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + + // A directory at the PDB path makes the final rename fail after the temporary file is written. + Directory.CreateDirectory(pdbPath); + + Assert.ThrowsAny(() => OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb)); + + Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + Assert.True(Directory.Exists(pdbPath)); + } + + [Fact] + public void WritePdb_WhenItsTemporaryPathExists_LeavesThePdbUnchanged() + { + string pdbPath = PathOf("Min.pdb"); + string temporaryPath = PathOf("Min.pdb.existing.tmp"); + File.WriteAllBytes(pdbPath, s_stale); + File.WriteAllBytes(temporaryPath, s_image); + + Assert.Throws(() => OutputFileWriter.WritePdb(pdbPath, temporaryPath, s_pdb)); + + Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void WritePdb_WhenItsTemporaryPathExists_KeepsTheFileThere() + { + string pdbPath = PathOf("Min.pdb"); + string temporaryPath = PathOf("Min.pdb.existing.tmp"); + File.WriteAllBytes(temporaryPath, s_image); + + Assert.Throws(() => OutputFileWriter.WritePdb(pdbPath, temporaryPath, s_pdb)); + + Assert.Equal(s_image, File.ReadAllBytes(temporaryPath)); + } + + [Fact] + public void Write_WithPdbNamedLikeTheOutput_WritesNothing() + { + string outputPath = PathOf("Min.pdb"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_stale); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(s_stale, File.ReadAllBytes(outputPath)); + } + + [Fact] + public void Write_WithoutPdb_NeverDeletesTheOutputItWrote() + { + // The output is named like its PDB, the image it replaces refers to a PDB, and the bytes written are that + // PDB's: the file at the PDB path then reads as the replaced image's PDB, but it is the new output. + string outputPath = PathOf("Min.pdb"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, stream => stream.Write(s_pair.Pdb.AsSpan()), portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(outputPath)); + } + + [Fact] + public void Write_WithPdb_TreatsAnOutputDifferingFromThePdbOnlyInCaseAsTheDefaultFileSystemDoes() + { + // Min.PDB and Min.pdb are one file on the default Windows and macOS file systems, and two elsewhere. + bool caseInsensitive = OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); + string outputPath = PathOf("Min.PDB"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(caseInsensitive ? OutputWriteResult.PdbWouldOverwriteOutput : OutputWriteResult.ImageAndPdbWritten, result); + } +} diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs index 535bf8f41eb37c..54fb999248925a 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs @@ -2,16 +2,22 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.IO; -using System.Linq; using System.Reflection.PortableExecutable; using Xunit; namespace ILAssembler.Tests; -public sealed class OutputWriterTests : IDisposable +/// +/// Tests of on in-memory streams, without the file system: the order of the writes, +/// which existing PDB is deleted, and the refusal to overwrite the output with its PDB. +/// +public class OutputWriterTests { + private const string CloseOutput = "CloseOutput"; + private static readonly byte[] s_image = [0x4D, 0x5A, 0x01, 0x02]; private static readonly ImmutableArray s_pdb = [0x42, 0x53, 0x4A, 0x42, 0x03]; private static readonly byte[] s_stale = [0xDE, 0xAD]; @@ -22,205 +28,227 @@ public sealed class OutputWriterTests : IDisposable private static readonly (ImmutableArray Image, ImmutableArray Pdb) s_otherPair = DocumentCompilerTestHelpers.CompileImageAndPdb("B"); private static readonly ImmutableArray s_imageWithoutPdb = DocumentCompilerTestHelpers.CompileImageWithoutPdb("A"); - private readonly string _directory = Directory.CreateTempSubdirectory("ilasm-output-").FullName; - - public void Dispose() => Directory.Delete(_directory, recursive: true); - - private string PathOf(string fileName) => Path.Combine(_directory, fileName); - - private string[] FileNames() => Directory.EnumerateFileSystemEntries(_directory).Select(Path.GetFileName).Order().ToArray()!; - private static void WriteImage(Stream stream) => stream.Write(s_image); - [Theory] - [InlineData("Min.dll", "Min.pdb")] - [InlineData("Min", "Min.pdb")] - [InlineData("a.b/Min", "a.b/Min.pdb")] - public void GetPdbPath_ReplacesTheExtensionAndReturnsTheFullPath(string outputPath, string expectedPdbPath) - { - Assert.Equal(Path.GetFullPath(expectedPdbPath), OutputWriter.GetPdbPath(outputPath)); - } + // The previous output is the image of the PDB at the PDB path. + private static MemoryOutputStreams ImageAndItsPdb() => new() { Output = s_pair.Image.ToArray(), Pdb = s_pair.Pdb.ToArray() }; [Fact] - public void Write_WithPdb_WritesTheImageAndThenThePdb() + public void Write_WithPdb_WritesTheImageAndThePdb() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - bool pdbExistedDuringImageWrite = true; + var output = new MemoryOutputStreams(); - OutputWriteResult result = OutputWriter.Write( - outputPath, - pdbPath, - stream => - { - pdbExistedDuringImageWrite = File.Exists(pdbPath); - WriteImage(stream); - }, - s_pdb); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, s_pdb); Assert.Equal(OutputWriteResult.ImageAndPdbWritten, result); - Assert.False(pdbExistedDuringImageWrite); - Assert.Equal(s_image, File.ReadAllBytes(outputPath)); - Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); - Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + Assert.Equal(s_image, output.Output); + Assert.Equal(s_pdb.ToArray(), output.Pdb); } [Fact] - public void Write_WithPdb_ReplacesAnExistingPdb() + public void Write_WithPdb_WritesThePdbAfterTheImageIsClosed() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(pdbPath, s_stale); + var output = new MemoryOutputStreams(); - OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + OutputWriter.Write(output, WriteImage, s_pdb); - Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(new[] { nameof(IOutputStreams.CreateOutput), CloseOutput, nameof(IOutputStreams.WritePdb) }, output.Calls); } [Fact] - public void Write_WithPdb_RenamesANewFileOverTheExistingPdbInsteadOfRewritingIt() + public void Write_WithPdb_DoesNotReadTheExistingOutput() { - // Renaming a completed temporary file over the PDB path is what keeps a failed write from leaving a - // partial PDB. It is observable as a reader of the old PDB still seeing the old content afterwards. - // Renaming over a file that is open depends on the Windows version, so this runs elsewhere. - if (OperatingSystem.IsWindows()) - { - return; - } + MemoryOutputStreams output = ImageAndItsPdb(); - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(pdbPath, s_stale); - using var oldPdbReader = new FileStream(pdbPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + OutputWriter.Write(output, WriteImage, s_pdb); - OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); - - byte[] oldContent = new byte[s_stale.Length]; - oldPdbReader.ReadExactly(oldContent); - Assert.Equal(s_stale, oldContent); - Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.DoesNotContain(nameof(IOutputStreams.OpenExistingOutput), output.Calls); } [Fact] public void Write_WithoutPdb_DeletesThePdbOfTheImageItReplaces() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + MemoryOutputStreams output = ImageAndItsPdb(); - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWrittenStalePdbDeleted, result); - Assert.Equal(new[] { "Min.dll" }, FileNames()); + Assert.Null(output.Pdb); + Assert.Equal(s_image, output.Output); + } + + [Fact] + public void Write_WithoutPdb_ReadsTheReplacedImageBeforeCreatingTheOutput() + { + MemoryOutputStreams output = ImageAndItsPdb(); + + OutputWriter.Write(output, WriteImage, portablePdb: null); + + int read = output.Calls.IndexOf(nameof(IOutputStreams.OpenExistingOutput)); + int create = output.Calls.IndexOf(nameof(IOutputStreams.CreateOutput)); + Assert.True(read >= 0 && read < create, string.Join(", ", output.Calls)); } [Fact] public void Write_WithoutPdb_KeepsAPdbWhenNoImageIsReplaced() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + var output = new MemoryOutputStreams { Pdb = s_pair.Pdb.ToArray() }; - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] public void Write_WithoutPdb_KeepsAPdbThatTheReplacedImageDoesNotReference() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_otherPair.Image.ToArray()); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); - Assert.NotEqual(OutputWriter.TryReadPortablePdbId(pdbPath), OutputWriter.TryReadCodeViewPdbId(outputPath)); + var output = new MemoryOutputStreams { Output = s_otherPair.Image.ToArray(), Pdb = s_pair.Pdb.ToArray() }; - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedImageHasNoDebugDirectory() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_imageWithoutPdb.ToArray()); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + var output = new MemoryOutputStreams { Output = s_imageWithoutPdb.ToArray(), Pdb = s_pair.Pdb.ToArray() }; - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] - public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedFileIsNotAnImage() + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedOutputIsNotAnImage() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_stale); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + var output = new MemoryOutputStreams { Output = s_stale, Pdb = s_pair.Pdb.ToArray() }; - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] - public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedFileIsACoffObject() + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedOutputIsACoffObject() { - // System.Reflection.Metadata reads a file that does not start with "MZ" as a COFF object file, which has no + // System.Reflection.Metadata reads bytes that do not start with "MZ" as a COFF object file, which has no // PE header and so no debug directory. Twenty zero bytes read as one with no sections. - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, new byte[20]); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); - using (var coff = new PEReader(File.ReadAllBytes(outputPath).ToImmutableArray())) + byte[] coff = new byte[20]; + using (var reader = new PEReader(coff.ToImmutableArray())) { - Assert.True(coff.PEHeaders.IsCoffOnly); + Assert.True(reader.PEHeaders.IsCoffOnly); } - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + var output = new MemoryOutputStreams { Output = coff, Pdb = s_pair.Pdb.ToArray() }; + + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_image, File.ReadAllBytes(outputPath)); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] - public void TryReadCodeViewPdbId_ReturnsNullForAFileLongerThanAnImageCanBe() + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedOutputIsLongerThanAnImageCanBe() { // System.Reflection.Metadata rejects a stream longer than int.MaxValue bytes with an ArgumentException. - using var stream = new LengthOnlyStream(int.MaxValue + 1L); + var output = new MemoryOutputStreams + { + Pdb = s_pair.Pdb.ToArray(), + OpenExistingOutputOverride = () => new LengthOnlyStream(int.MaxValue + 1L), + }; - Assert.Null(OutputWriter.TryReadCodeViewPdbId(stream)); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] - public void TryReadPortablePdbId_ReturnsNullForAFileLongerThanAPdbCanBe() + public void Write_WithoutPdb_KeepsAFileLongerThanAPdbCanBe() { - using var stream = new LengthOnlyStream(int.MaxValue + 1L); + var output = new MemoryOutputStreams + { + Output = s_pair.Image.ToArray(), + OpenExistingPdbOverride = () => new LengthOnlyStream(int.MaxValue + 1L), + }; + + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); - Assert.Null(OutputWriter.TryReadPortablePdbId(stream)); + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.DoesNotContain(nameof(IOutputStreams.TryDeletePdb), output.Calls); } [Fact] public void Write_WithoutPdb_KeepsAFileThatIsNotAPortablePdb() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); - File.WriteAllBytes(pdbPath, s_stale); + var output = new MemoryOutputStreams { Output = s_pair.Image.ToArray(), Pdb = s_stale }; - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + Assert.Equal(s_stale, output.Pdb); + } + + [Fact] + public void Write_WithoutPdb_DeletesNothingWhenThereIsNoPdb() + { + var output = new MemoryOutputStreams { Output = s_pair.Image.ToArray() }; + + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.DoesNotContain(nameof(IOutputStreams.TryDeletePdb), output.Calls); + } + + public static TheoryData OpenFailures { get; } = new() { typeof(IOException), typeof(UnauthorizedAccessException) }; + + [Theory] + [MemberData(nameof(OpenFailures))] + public void Write_WithoutPdb_KeepsAPdbWhenTheReplacedOutputCannotBeOpened(Type exceptionType) + { + var output = new MemoryOutputStreams + { + Pdb = s_pair.Pdb.ToArray(), + OpenExistingOutputOverride = () => throw (Exception)Activator.CreateInstance(exceptionType)!, + }; + + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); + } + + [Theory] + [MemberData(nameof(OpenFailures))] + public void Write_WithoutPdb_KeepsAPdbThatCannotBeOpened(Type exceptionType) + { + var output = new MemoryOutputStreams + { + Output = s_pair.Image.ToArray(), + OpenExistingPdbOverride = () => throw (Exception)Activator.CreateInstance(exceptionType)!, + }; + + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.DoesNotContain(nameof(IOutputStreams.TryDeletePdb), output.Calls); + } + + [Fact] + public void Write_WithoutPdb_ReportsAPdbThatCannotBeDeletedAsNotDeleted() + { + MemoryOutputStreams output = ImageAndItsPdb(); + output.PdbDeleteFails = true; + + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); + + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Theory] @@ -228,13 +256,10 @@ public void Write_WithoutPdb_KeepsAFileThatIsNotAPortablePdb() [InlineData(false)] public void Write_WhenTheImageWriteFails_LeavesTheExistingPdbUnchanged(bool withPdb) { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(pdbPath, s_stale); + MemoryOutputStreams output = ImageAndItsPdb(); Assert.Throws(() => OutputWriter.Write( - outputPath, - pdbPath, + output, stream => { stream.WriteByte(0x4D); @@ -242,8 +267,7 @@ public void Write_WhenTheImageWriteFails_LeavesTheExistingPdbUnchanged(bool with }, withPdb ? s_pdb : null)); - Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); - Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Theory] @@ -253,143 +277,125 @@ public void Write_WhenClosingTheImageFails_LeavesTheExistingPdbUnchanged(bool wi { // The image is closed before the PDB is touched: a PDB written, or the replaced image's PDB deleted, // before the image stream is disposed would show here. - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); - File.WriteAllBytes(pdbPath, s_pair.Pdb.ToArray()); + MemoryOutputStreams output = ImageAndItsPdb(); + output.OutputCloseFails = true; - Assert.Throws(() => OutputWriter.Write( - outputPath, - pdbPath, - WriteImage, - withPdb ? s_pdb : null, - path => new ThrowOnDisposeStream(File.Create(path)))); + Assert.Throws(() => OutputWriter.Write(output, WriteImage, withPdb ? s_pdb : null)); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(pdbPath)); + Assert.Equal(s_pair.Pdb.ToArray(), output.Pdb); } [Fact] - public void Write_WhenThePdbCannotBeWritten_RemovesOnlyItsTemporaryFile() + public void Write_WithPdbWhenThePdbPathNamesTheOutput_WritesNothing() { - string outputPath = PathOf("Min.dll"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - - // A directory at the PDB path makes the final rename fail after the temporary file is written. - Directory.CreateDirectory(pdbPath); + var output = new MemoryOutputStreams { Output = s_stale, PdbPathIsOutputPath = true }; - Assert.ThrowsAny(() => OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb)); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, s_pdb); - Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); - Assert.True(Directory.Exists(pdbPath)); - } - - [Fact] - public void WritePdb_WhenItsTemporaryPathExists_LeavesThePdbUnchanged() - { - string pdbPath = PathOf("Min.pdb"); - string temporaryPath = PathOf("Min.pdb.existing.tmp"); - File.WriteAllBytes(pdbPath, s_stale); - File.WriteAllBytes(temporaryPath, s_image); - - Assert.Throws(() => OutputWriter.WritePdb(pdbPath, temporaryPath, s_pdb)); - - Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(s_stale, output.Output); + Assert.Empty(output.Calls); } [Fact] - public void WritePdb_WhenItsTemporaryPathExists_KeepsTheFileThere() + public void Write_WithoutPdbWhenThePdbPathNamesTheOutput_WritesTheImageAndDoesNotReadTheReplacedOutput() { - string pdbPath = PathOf("Min.pdb"); - string temporaryPath = PathOf("Min.pdb.existing.tmp"); - File.WriteAllBytes(temporaryPath, s_image); + // The file at the PDB path is the new image, so the replaced output's PDB id must not be compared with it. + var output = new MemoryOutputStreams { Output = s_pair.Image.ToArray(), PdbPathIsOutputPath = true }; - Assert.Throws(() => OutputWriter.WritePdb(pdbPath, temporaryPath, s_pdb)); + OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); - Assert.Equal(s_image, File.ReadAllBytes(temporaryPath)); + Assert.Equal(OutputWriteResult.ImageWritten, result); + Assert.Equal(s_image, output.Output); + Assert.DoesNotContain(nameof(IOutputStreams.OpenExistingOutput), output.Calls); } - [Fact] - public void Write_WithPdbNamedLikeTheOutput_WritesNothing() + // The output and the PDB in memory, recording the calls made on it in order, with CloseOutput when the output + // stream is disposed. Output and Pdb are the bytes at each path, or null when there is no file; creating the + // output empties it, as creating a file does, and it holds what was written once the stream is closed. + private sealed class MemoryOutputStreams : IOutputStreams { - string outputPath = PathOf("Min.pdb"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_stale); - - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); - - Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); - Assert.Equal(s_stale, File.ReadAllBytes(outputPath)); - } + public byte[]? Output { get; set; } - [Fact] - public void Write_WithoutPdb_KeepsAnOutputNamedLikeThePdb() - { - string outputPath = PathOf("Min.pdb"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); + public byte[]? Pdb { get; set; } - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, portablePdb: null); + public bool PdbPathIsOutputPath { get; init; } - Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_image, File.ReadAllBytes(outputPath)); - } + public bool OutputCloseFails { get; set; } - [Fact] - public void Write_WithoutPdb_NeverDeletesTheOutputItWrote() - { - // The output is named like its PDB, the image it replaces refers to a PDB, and the bytes written are that - // PDB's: the file at the PDB path then reads as the replaced image's PDB, but it is the new output. - string outputPath = PathOf("Min.pdb"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); - File.WriteAllBytes(outputPath, s_pair.Image.ToArray()); + public bool PdbDeleteFails { get; set; } - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, stream => stream.Write(s_pair.Pdb.AsSpan()), portablePdb: null); + // Called instead of opening Output or Pdb, to supply a stream that cannot be held in memory or to fail. + public Func? OpenExistingOutputOverride { get; init; } - Assert.Equal(OutputWriteResult.ImageWritten, result); - Assert.Equal(s_pair.Pdb.ToArray(), File.ReadAllBytes(outputPath)); - } + public Func? OpenExistingPdbOverride { get; init; } - [Fact] - public void Write_WithPdb_TreatsAnOutputDifferingFromThePdbOnlyInCaseAsTheDefaultFileSystemDoes() - { - // Min.PDB and Min.pdb are one file on the default Windows and macOS file systems, and two elsewhere. - bool caseInsensitive = OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); - string outputPath = PathOf("Min.PDB"); - string pdbPath = OutputWriter.GetPdbPath(outputPath); + public List Calls { get; } = []; - OutputWriteResult result = OutputWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + public Stream? OpenExistingOutput() + { + Calls.Add(nameof(OpenExistingOutput)); + return OpenExistingOutputOverride is { } open ? open() : Output is null ? null : new MemoryStream(Output, writable: false); + } - Assert.Equal(caseInsensitive ? OutputWriteResult.PdbWouldOverwriteOutput : OutputWriteResult.ImageAndPdbWritten, result); - } + public Stream CreateOutput() + { + Calls.Add(nameof(CreateOutput)); + Output = []; + return new OutputStream(this); + } - // A stream that writes through to another and fails when it is closed. - private sealed class ThrowOnDisposeStream(Stream inner) : Stream - { - public override bool CanRead => inner.CanRead; - public override bool CanSeek => inner.CanSeek; - public override bool CanWrite => inner.CanWrite; - public override long Length => inner.Length; + public void WritePdb(ImmutableArray pdb) + { + Calls.Add(nameof(WritePdb)); + Pdb = pdb.ToArray(); + } - public override long Position + public Stream? OpenExistingPdb() { - get => inner.Position; - set => inner.Position = value; + Calls.Add(nameof(OpenExistingPdb)); + return OpenExistingPdbOverride is { } open ? open() : Pdb is null ? null : new MemoryStream(Pdb, writable: false); } - public override void Flush() => inner.Flush(); - public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, count); - public override long Seek(long offset, SeekOrigin origin) => inner.Seek(offset, origin); - public override void SetLength(long value) => inner.SetLength(value); - public override void Write(byte[] buffer, int offset, int count) => inner.Write(buffer, offset, count); + public bool TryDeletePdb() + { + Calls.Add(nameof(TryDeletePdb)); + if (PdbDeleteFails) + { + return false; + } + + Pdb = null; + return true; + } - protected override void Dispose(bool disposing) + private sealed class OutputStream(MemoryOutputStreams owner) : MemoryStream { - inner.Dispose(); - throw new IOException("Injected image close failure"); + private bool _closed; + + protected override void Dispose(bool disposing) + { + if (disposing && !_closed) + { + _closed = true; + owner.Output = ToArray(); + owner.Calls.Add(CloseOutput); + base.Dispose(disposing); + if (owner.OutputCloseFails) + { + throw new IOException("Injected image close failure"); + } + + return; + } + + base.Dispose(disposing); + } } } // A readable, seekable stream that reports a length and holds no data, so a length check can be tested without - // a file of that size. + // that many bytes. private sealed class LengthOnlyStream(long length) : Stream { public override bool CanRead => true; From 5b403a517dd75602e19ddd3fa64b37a06dc90432 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Tue, 6 Oct 2026 14:33:34 -0700 Subject: [PATCH 5/9] Move the ilasm PDB file tests into ILAssembler.Tests The end-to-end PDB tests that ran the managed ilasm as a process from src/tests/ilasm now run the command line in process in ILAssembler.Tests (CommandLinePdbFileTests): Program.cs is linked into the test project, as the command-line parsing already is, and each test assembles inline IL sources into its own temporary directory. They cover the PDB beside the output and its CodeView entry, the debug directory order, the checksum, no PDB without a switch (with and without .line and -DET), the DebuggableAttribute, stale-PDB replacement and deletion, an unrelated PDB kept, failed builds, -ERR, the refusal of an output named like its PDB, and byte-identical -DET repeats. The rows that ran native ilasm as a control are dropped; native ilasm is not part of ILAssembler.Tests. src/tests/ilasm is back to what it was before this change. --- .../ilasm/PortablePdb/IlasmPdbFileTester.cs | 367 ---------------- .../PortablePdb/TestFiles/TestPdbFile1.il | 20 - .../PortablePdb/TestFiles/TestPdbFileError.il | 22 - .../PortablePdb/TestFiles/TestPdbFileLine.il | 21 - .../TestFiles/TestPdbFileMissingInclude.il | 14 - .../TestFiles/TestPdbFileRecoverableError.il | 27 -- src/tests/ilasm/ilasm_tests.csproj | 21 - .../CommandLinePdbFileTests.cs | 414 ++++++++++++++++++ .../ILAssembler.Tests.csproj | 7 +- 9 files changed, 420 insertions(+), 493 deletions(-) delete mode 100644 src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs delete mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il delete mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il delete mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il delete mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il delete mode 100644 src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il create mode 100644 src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs diff --git a/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs b/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs deleted file mode 100644 index 78745d551c2f10..00000000000000 --- a/src/tests/ilasm/PortablePdb/IlasmPdbFileTester.cs +++ /dev/null @@ -1,367 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Immutable; -using System.Diagnostics; -using System.IO; -using System.Linq; -using System.Reflection.Metadata; -using System.Reflection.PortableExecutable; -using System.Security.Cryptography; -using Xunit; - -namespace IlasmPortablePdbTests -{ - // Runs ilasm end to end and checks the PDB file it writes and the image's debug directory. - // "Native" cases run CORE_ROOT/ilasm. "Managed" cases run CORE_ROOT/managed-ilasm/ilasm, which is built only - // where the SDK tools are (SdkToolsSupported in eng/Subsets.props), so they run only when CORE_ROOT has it. - // Cases that native ilasm does not satisfy run only against the managed ilasm and say why. - // The tests are static: the merged test runner creates an instance of a test class only to dispose it. - public class IlasmPdbFileTester - { - private const string TestDir = "TestFiles"; - private const string Native = "native"; - private const string Managed = "managed"; - - private static readonly string s_ilasmFile = "ilasm" + (OperatingSystem.IsWindows() ? ".exe" : string.Empty); - - // Whether CORE_ROOT has the managed ilasm; the condition of every managed case. - public static bool HasManagedIlasm => - Environment.GetEnvironmentVariable("CORE_ROOT") is { Length: > 0 } coreRoot && - File.Exists(Path.Combine(coreRoot, "managed-ilasm", s_ilasmFile)); - - private static string GetIlasm(string kind) => - IlasmPortablePdbTesterCommon.GetIlasmFullPath( - Environment.GetEnvironmentVariable("CORE_ROOT"), - kind == Managed ? Path.Combine("managed-ilasm", s_ilasmFile) : s_ilasmFile); - - // A fresh directory per test and ilasm, so tests do not see each other's outputs. - private static string CreateOutputDirectory(string testName, string kind) - { - string directory = Path.Combine(Environment.CurrentDirectory, "IlasmPdbFileTester", $"{testName}-{kind}"); - if (Directory.Exists(directory)) - { - Directory.Delete(directory, recursive: true); - } - - Directory.CreateDirectory(directory); - return directory; - } - - private static int RunIlasm(string kind, string ilSource, string outputPath, string switches) - { - string ilSourcePath = Path.Combine(Environment.CurrentDirectory, TestDir, ilSource); - Assert.True(File.Exists(ilSourcePath)); - - var startInfo = new ProcessStartInfo - { - UseShellExecute = false, - WorkingDirectory = Environment.CurrentDirectory, - FileName = GetIlasm(kind), - Arguments = $"-nologo -quiet -dll -output={outputPath} {switches} {ilSourcePath}", - }; - - using Process ilasm = Process.Start(startInfo); - ilasm.WaitForExit(); - return ilasm.ExitCode; - } - - private static PEReader ReadImage(string path) => new PEReader(ImmutableArray.Create(File.ReadAllBytes(path))); - - private static DebugDirectoryEntryType[] GetDebugDirectoryTypes(string imagePath) - { - using PEReader pe = ReadImage(imagePath); - return pe.ReadDebugDirectory().Select(entry => entry.Type).ToArray(); - } - - private static bool HasDebuggableAttribute(string imagePath) - { - using PEReader pe = ReadImage(imagePath); - MetadataReader reader = pe.GetMetadataReader(); - foreach (CustomAttributeHandle handle in reader.GetAssemblyDefinition().GetCustomAttributes()) - { - CustomAttribute attribute = reader.GetCustomAttribute(handle); - if (attribute.Constructor.Kind == HandleKind.MemberReference) - { - MemberReference constructor = reader.GetMemberReference((MemberReferenceHandle)attribute.Constructor); - if (constructor.Parent.Kind == HandleKind.TypeReference && - reader.GetString(reader.GetTypeReference((TypeReferenceHandle)constructor.Parent).Name) == "DebuggableAttribute") - { - return true; - } - } - } - - return false; - } - - // With -DEBUG or -PDB the PDB is a file beside the output, the CodeView entry names its full path and - // carries its id, and nothing is embedded in the image. - [Theory] - [InlineData("-debug")] - [InlineData("-pdb")] - public static void Native_PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string pdbSwitch) => - PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(Native, pdbSwitch); - - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData("-debug")] - [InlineData("-pdb")] - public static void Managed_PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string pdbSwitch) => - PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(Managed, pdbSwitch); - - private static void PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string kind, string pdbSwitch) - { - string directory = CreateOutputDirectory(nameof(PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt) + pdbSwitch, kind); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); - - Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, pdbSwitch)); - - Assert.True(File.Exists(pdb)); - using PEReader pe = ReadImage(dll); - ImmutableArray entries = pe.ReadDebugDirectory(); - Assert.DoesNotContain(entries, entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); - DebugDirectoryEntry codeViewEntry = Assert.Single(entries, entry => entry.Type == DebugDirectoryEntryType.CodeView); - CodeViewDebugDirectoryData codeView = pe.ReadCodeViewDebugDirectoryData(codeViewEntry); - Assert.Equal(pdb, codeView.Path); - - using MetadataReaderProvider pdbProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(File.ReadAllBytes(pdb))); - BlobContentId pdbId = new BlobContentId(pdbProvider.GetMetadataReader().DebugMetadataHeader.Id); - Assert.Equal(pdbId.Guid, codeView.Guid); - Assert.Equal(pdbId.Stamp, codeViewEntry.Stamp); - } - - // The debug directory is CodeView then PdbChecksum, and under -DET a Reproducible entry follows. - [Theory] - [InlineData(false)] - [InlineData(true)] - public static void Native_PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(bool deterministic) => - PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(Native, deterministic); - - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData(false)] - [InlineData(true)] - public static void Managed_PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(bool deterministic) => - PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(Managed, deterministic); - - private static void PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(string kind, bool deterministic) - { - string directory = CreateOutputDirectory(nameof(PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic) + deterministic, kind); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - - Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, deterministic ? "-debug -det" : "-debug")); - - DebugDirectoryEntryType[] expected = deterministic - ? new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible } - : new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum }; - Assert.Equal(expected, GetDebugDirectoryTypes(dll)); - } - - // The PdbChecksum entry is SHA-256 of the PDB file with its 20-byte id zeroed (PE-COFF.md). - // Managed only: native ilasm hashes the PDB before its content is final (#135211). - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData("-debug")] - [InlineData("-debug -det")] - public static void Managed_PdbSwitch_PdbChecksumIsSha256OfThePdbWithItsIdZeroed(string switches) - { - string directory = CreateOutputDirectory(nameof(Managed_PdbSwitch_PdbChecksumIsSha256OfThePdbWithItsIdZeroed) + switches.Replace(" ", string.Empty), Managed); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); - - Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, switches)); - - using PEReader pe = ReadImage(dll); - DebugDirectoryEntry checksumEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.PdbChecksum); - PdbChecksumDebugDirectoryData checksum = pe.ReadPdbChecksumDebugDirectoryData(checksumEntry); - byte[] pdbBytes = File.ReadAllBytes(pdb); - using (MetadataReaderProvider pdbProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(pdbBytes))) - { - DebugMetadataHeader header = pdbProvider.GetMetadataReader().DebugMetadataHeader; - Array.Clear(pdbBytes, header.IdStartOffset, header.Id.Length); - } - - Assert.Equal("SHA256", checksum.AlgorithmName); - Assert.Equal(SHA256.HashData(pdbBytes), checksum.Checksum.ToArray()); - } - - // Without -DEBUG or -PDB there is no PDB file and no debug directory, even with .line directives or -DET. - [Theory] - [InlineData("TestPdbFile1.il", "")] - [InlineData("TestPdbFile1.il", "-det")] - [InlineData("TestPdbFileLine.il", "")] - [InlineData("TestPdbFileLine.il", "-det")] - public static void Native_NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(string ilSource, string switches) => - NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(Native, ilSource, switches); - - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData("TestPdbFile1.il", "")] - [InlineData("TestPdbFile1.il", "-det")] - [InlineData("TestPdbFileLine.il", "")] - [InlineData("TestPdbFileLine.il", "-det")] - public static void Managed_NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(string ilSource, string switches) => - NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(Managed, ilSource, switches); - - private static void NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(string kind, string ilSource, string switches) - { - string directory = CreateOutputDirectory($"{nameof(NoPdbSwitch_WritesNoPdbAndNoDebugDirectory)}-{Path.GetFileNameWithoutExtension(ilSource)}{switches}", kind); - string dll = Path.Combine(directory, "Output.dll"); - - Assert.Equal(0, RunIlasm(kind, ilSource, dll, switches)); - - Assert.Equal(new[] { "Output.dll" }, Directory.GetFiles(directory).Select(Path.GetFileName).ToArray()); - using PEReader pe = ReadImage(dll); - DirectoryEntry debugTable = pe.PEHeaders.PEHeader.DebugTableDirectory; - Assert.Equal((0, 0), (debugTable.RelativeVirtualAddress, debugTable.Size)); - } - - // -PDB produces the PDB without adding a DebuggableAttribute; -DEBUG adds one. - [Theory] - [InlineData("-pdb", false)] - [InlineData("-debug", true)] - public static void Native_PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string pdbSwitch, bool expectAttribute) => - PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(Native, pdbSwitch, expectAttribute); - - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData("-pdb", false)] - [InlineData("-debug", true)] - public static void Managed_PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string pdbSwitch, bool expectAttribute) => - PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(Managed, pdbSwitch, expectAttribute); - - private static void PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string kind, string pdbSwitch, bool expectAttribute) - { - string directory = CreateOutputDirectory(nameof(PdbSwitch_AddsADebuggableAttributeOnlyWithDebug) + pdbSwitch, kind); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - - Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, pdbSwitch)); - - Assert.Equal(expectAttribute, HasDebuggableAttribute(dll)); - } - - // A PDB left by an earlier build is replaced when ilasm writes a new one. - [Fact] - public static void Native_StalePdb_IsReplacedWhenAPdbIsWritten() => StalePdb_IsReplacedWhenAPdbIsWritten(Native); - - [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - public static void Managed_StalePdb_IsReplacedWhenAPdbIsWritten() => StalePdb_IsReplacedWhenAPdbIsWritten(Managed); - - private static void StalePdb_IsReplacedWhenAPdbIsWritten(string kind) - { - string directory = CreateOutputDirectory(nameof(StalePdb_IsReplacedWhenAPdbIsWritten), kind); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); - File.WriteAllBytes(pdb, new byte[] { 0xDE, 0xAD }); - - Assert.Equal(0, RunIlasm(kind, "TestPdbFile1.il", dll, "-debug")); - - using PEReader pe = ReadImage(dll); - Assert.True(pe.TryOpenAssociatedPortablePdb(dll, path => File.OpenRead(path), out MetadataReaderProvider pdbProvider, out string pdbPath)); - pdbProvider.Dispose(); - Assert.Equal(pdb, pdbPath); - } - - // Without a PDB switch, a successful build deletes the PDB of the image it replaces. - // Managed only: native ilasm deletes .PDB, which on a case-sensitive file system is not this file. - [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - public static void Managed_StalePdb_IsDeletedWhenItsImageIsReplacedWithoutAPdb() - { - string directory = CreateOutputDirectory(nameof(Managed_StalePdb_IsDeletedWhenItsImageIsReplacedWithoutAPdb), Managed); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); - Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, "-debug")); - Assert.True(File.Exists(pdb)); - - Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, string.Empty)); - - Assert.True(File.Exists(dll)); - Assert.False(File.Exists(pdb)); - } - - // Without a PDB switch, a PDB beside an output that did not exist before belongs to no image ilasm replaced, - // so it is kept. - // Managed only: native ilasm deletes .PDB whether or not it belongs to the replaced image. - [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - public static void Managed_UnrelatedPdb_IsKeptWhenNoImageIsReplaced() - { - string directory = CreateOutputDirectory(nameof(Managed_UnrelatedPdb_IsKeptWhenNoImageIsReplaced), Managed); - string otherDll = Path.Combine(directory, "Other.dll"); - string dll = Path.Combine(directory, "TestPdbFile1.dll"); - string pdb = Path.Combine(directory, "TestPdbFile1.pdb"); - Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", otherDll, "-debug")); - File.Move(Path.Combine(directory, "Other.pdb"), pdb); - byte[] unrelatedPdb = File.ReadAllBytes(pdb); - - Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, string.Empty)); - - Assert.Equal(unrelatedPdb, File.ReadAllBytes(pdb)); - } - - // When assembly fails, the existing output and PDB are left as they were: after an error in the source - // (TestPdbFileError.il) and after an exception (TestPdbFileMissingInclude.il, whose include is missing). - // Managed only: native ilasm deletes .PDB when assembly fails. - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData("TestPdbFileError.il", "")] - [InlineData("TestPdbFileError.il", "-debug")] - [InlineData("TestPdbFileMissingInclude.il", "-debug")] - public static void Managed_FailedBuild_LeavesTheExistingOutputAndPdbUnchanged(string ilSource, string switches) - { - string name = Path.GetFileNameWithoutExtension(ilSource); - string directory = CreateOutputDirectory($"{nameof(Managed_FailedBuild_LeavesTheExistingOutputAndPdbUnchanged)}-{name}{switches}", Managed); - string dll = Path.Combine(directory, name + ".dll"); - string pdb = Path.Combine(directory, name + ".pdb"); - byte[] existingDll = { 0x4D, 0x5A }; - byte[] existingPdb = { 0xDE, 0xAD }; - File.WriteAllBytes(dll, existingDll); - File.WriteAllBytes(pdb, existingPdb); - - Assert.NotEqual(0, RunIlasm(Managed, ilSource, dll, switches)); - - Assert.Equal(existingDll, File.ReadAllBytes(dll)); - Assert.Equal(existingPdb, File.ReadAllBytes(pdb)); - } - - // With -ERR, an image written despite errors is handled as after a successful build: with -DEBUG its PDB is - // written beside it, and without a PDB switch the PDB of the image it replaces is deleted. - // Managed only: native ilasm deletes .PDB, which on a case-sensitive file system is not this file. - [ConditionalTheory(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - [InlineData("-err -debug")] - [InlineData("-err")] - public static void Managed_ErrorTolerantBuildWithErrors_HandlesThePdbAsASuccessfulBuildDoes(string switches) - { - string directory = CreateOutputDirectory(nameof(Managed_ErrorTolerantBuildWithErrors_HandlesThePdbAsASuccessfulBuildDoes) + switches.Replace(" ", string.Empty), Managed); - string dll = Path.Combine(directory, "TestPdbFileRecoverableError.dll"); - string pdb = Path.Combine(directory, "TestPdbFileRecoverableError.pdb"); - Assert.Equal(0, RunIlasm(Managed, "TestPdbFile1.il", dll, "-debug")); - Assert.NotEqual(0, RunIlasm(Managed, "TestPdbFileRecoverableError.il", dll, "-debug")); - - Assert.Equal(0, RunIlasm(Managed, "TestPdbFileRecoverableError.il", dll, switches)); - - using PEReader pe = ReadImage(dll); - MetadataReader reader = pe.GetMetadataReader(); - Assert.Equal("TestPdbFileRecoverableError", reader.GetString(reader.GetAssemblyDefinition().Name)); - if (switches.Contains("-debug")) - { - Assert.True(pe.TryOpenAssociatedPortablePdb(dll, path => File.OpenRead(path), out MetadataReaderProvider pdbProvider, out string pdbPath)); - pdbProvider.Dispose(); - Assert.Equal(pdb, pdbPath); - } - else - { - Assert.False(File.Exists(pdb)); - } - } - - // An output named like its PDB is refused rather than overwritten by the PDB. - // Managed only: native ilasm writes the image and then overwrites it with the PDB. - [ConditionalFact(typeof(IlasmPdbFileTester), nameof(HasManagedIlasm))] - public static void Managed_OutputNamedLikeItsPdb_IsRefused() - { - string directory = CreateOutputDirectory(nameof(Managed_OutputNamedLikeItsPdb_IsRefused), Managed); - string output = Path.Combine(directory, "TestPdbFile1.pdb"); - - Assert.NotEqual(0, RunIlasm(Managed, "TestPdbFile1.il", output, "-debug")); - - Assert.False(File.Exists(output)); - } - } -} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il deleted file mode 100644 index b48b097d7ae6f1..00000000000000 --- a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFile1.il +++ /dev/null @@ -1,20 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -.assembly extern System.Runtime -{ -} - -.assembly TestPdbFile1 -{ -} - -.class public auto ansi beforefieldinit TestPdbFileType - extends [System.Runtime]System.Object -{ - .method public hidebysig static int32 Foo() cil managed - { - ldc.i4.1 - ret - } -} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il deleted file mode 100644 index ab0195908c4706..00000000000000 --- a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileError.il +++ /dev/null @@ -1,22 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -// This source does not assemble: the branch target is not defined. - -.assembly extern System.Runtime -{ -} - -.assembly TestPdbFileError -{ -} - -.class public auto ansi beforefieldinit TestPdbFileErrorType - extends [System.Runtime]System.Object -{ - .method public hidebysig static void Foo() cil managed - { - br UNDEFINED_LABEL - ret - } -} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il deleted file mode 100644 index a324d4d1134818..00000000000000 --- a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileLine.il +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -.assembly extern System.Runtime -{ -} - -.assembly TestPdbFileLine -{ -} - -.class public auto ansi beforefieldinit TestPdbFileLineType - extends [System.Runtime]System.Object -{ - .method public hidebysig static int32 Foo() cil managed - { - .line 10,10 : 9,10 'TestPdbFileLine.cs' - ldc.i4.1 - ret - } -} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il deleted file mode 100644 index 5fb82ff05319dd..00000000000000 --- a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileMissingInclude.il +++ /dev/null @@ -1,14 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -// This source does not assemble: the included file does not exist. - -#include "TestPdbFileMissingInclude.inc" - -.assembly extern System.Runtime -{ -} - -.assembly TestPdbFileMissingInclude -{ -} diff --git a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il b/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il deleted file mode 100644 index e03b166645d375..00000000000000 --- a/src/tests/ilasm/PortablePdb/TestFiles/TestPdbFileRecoverableError.il +++ /dev/null @@ -1,27 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -// This source has an error that the error-tolerant option (-ERR) assembles past: the exported type names an -// assembly reference that is not declared. - -.assembly extern System.Runtime -{ -} - -.assembly TestPdbFileRecoverableError -{ -} - -.class extern public MissingType -{ - .assembly extern MissingAssembly -} - -.class public auto ansi beforefieldinit TestPdbFileRecoverableErrorType - extends [System.Runtime]System.Object -{ - .method public hidebysig static void Foo() cil managed - { - ret - } -} diff --git a/src/tests/ilasm/ilasm_tests.csproj b/src/tests/ilasm/ilasm_tests.csproj index 17e0231aaecd43..6e4d9a17c241af 100644 --- a/src/tests/ilasm/ilasm_tests.csproj +++ b/src/tests/ilasm/ilasm_tests.csproj @@ -12,7 +12,6 @@ - @@ -65,26 +64,6 @@ Always TestFiles\%(FileName)%(Extension) - - Always - TestFiles\%(FileName)%(Extension) - - - Always - TestFiles\%(FileName)%(Extension) - - - Always - TestFiles\%(FileName)%(Extension) - - - Always - TestFiles\%(FileName)%(Extension) - - - Always - TestFiles\%(FileName)%(Extension) - Always %(Filename).il diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs new file mode 100644 index 00000000000000..85ef88867ed612 --- /dev/null +++ b/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs @@ -0,0 +1,414 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.CommandLine; +using System.IO; +using System.Linq; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; +using System.Security.Cryptography; +using Xunit; + +namespace ILAssembler.Tests; + +/// +/// Runs the ilasm command line () in process, end to end, and checks the files it writes: the PDB +/// beside the output, the image's debug directory, the deletion of a stale PDB, failed and error-tolerant builds, and +/// the refusal of an output named like its PDB. Each test has its own temporary directory, with the IL sources in +/// src and the outputs in out. +/// +public sealed class CommandLinePdbFileTests : IDisposable +{ + private const string Source = """ + .assembly extern System.Runtime + { + } + + .assembly TestPdbFile1 + { + } + + .class public auto ansi beforefieldinit TestPdbFileType + extends [System.Runtime]System.Object + { + .method public hidebysig static int32 Foo() cil managed + { + ldc.i4.1 + ret + } + } + """; + + private const string SourceWithLineDirective = """ + .assembly extern System.Runtime + { + } + + .assembly TestPdbFileLine + { + } + + .class public auto ansi beforefieldinit TestPdbFileLineType + extends [System.Runtime]System.Object + { + .method public hidebysig static int32 Foo() cil managed + { + .line 10,10 : 9,10 'TestPdbFileLine.cs' + ldc.i4.1 + ret + } + } + """; + + // Does not assemble: the branch target is not defined. + private const string SourceWithError = """ + .assembly extern System.Runtime + { + } + + .assembly TestPdbFileError + { + } + + .class public auto ansi beforefieldinit TestPdbFileErrorType + extends [System.Runtime]System.Object + { + .method public hidebysig static void Foo() cil managed + { + br UNDEFINED_LABEL + ret + } + } + """; + + // Does not assemble: the included file does not exist, so assembly ends with an exception. + private const string SourceWithMissingInclude = """ + #include "TestPdbFileMissingInclude.inc" + + .assembly extern System.Runtime + { + } + + .assembly TestPdbFileMissingInclude + { + } + """; + + // Has an error that the error-tolerant option (-ERR) assembles past: the exported type names an assembly + // reference that is not declared. + private const string SourceWithRecoverableError = """ + .assembly extern System.Runtime + { + } + + .assembly TestPdbFileRecoverableError + { + } + + .class extern public MissingType + { + .assembly extern MissingAssembly + } + + .class public auto ansi beforefieldinit TestPdbFileRecoverableErrorType + extends [System.Runtime]System.Object + { + .method public hidebysig static void Foo() cil managed + { + ret + } + } + """; + + private readonly string _directory = Directory.CreateTempSubdirectory("ilasm-cli-").FullName; + + public CommandLinePdbFileTests() + { + Directory.CreateDirectory(Path.Combine(_directory, "src")); + Directory.CreateDirectory(Path.Combine(_directory, "out")); + } + + public void Dispose() => Directory.Delete(_directory, recursive: true); + + private string OutputPath(string fileName) => Path.Combine(_directory, "out", fileName); + + private string[] OutputFileNames() => Directory.EnumerateFileSystemEntries(Path.Combine(_directory, "out")).Select(Path.GetFileName).Order().ToArray()!; + + // Writes the source to src/.il and assembles it as "ilasm -nologo -quiet -dll -output= + // " does, returning the exit code. + private int RunIlasm(string source, string sourceName, string outputPath, string switches) + { + string sourcePath = Path.Combine(_directory, "src", sourceName + ".il"); + File.WriteAllText(sourcePath, source); + string[] args = ["-nologo", "-quiet", "-dll", $"-output={outputPath}", .. switches.Split(' ', StringSplitOptions.RemoveEmptyEntries), sourcePath]; + + var command = new IlasmRootCommand(); + ParseResult result = command.Parse(NativeCommandLine.Normalize(args, command)); + Assert.Empty(result.Errors); + return new Program(command, result).Run(); + } + + private int RunIlasm(string outputPath, string switches) => RunIlasm(Source, "TestPdbFile1", outputPath, switches); + + private static PEReader ReadImage(string path) => new(ImmutableArray.Create(File.ReadAllBytes(path))); + + private static BlobContentId ReadPdbId(string pdbPath) + { + using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(File.ReadAllBytes(pdbPath))); + return new BlobContentId(provider.GetMetadataReader().DebugMetadataHeader!.Id); + } + + // The path of the PDB that System.Reflection.Metadata finds for the image, as a debugger would find it. + private static string? FindAssociatedPdb(string imagePath) + { + using PEReader pe = ReadImage(imagePath); + if (!pe.TryOpenAssociatedPortablePdb(imagePath, File.OpenRead, out MetadataReaderProvider? provider, out string? pdbPath)) + { + return null; + } + + provider?.Dispose(); + return pdbPath; + } + + private static bool HasDebuggableAttribute(string imagePath) + { + using PEReader pe = ReadImage(imagePath); + MetadataReader reader = pe.GetMetadataReader(); + foreach (CustomAttributeHandle handle in reader.GetAssemblyDefinition().GetCustomAttributes()) + { + CustomAttribute attribute = reader.GetCustomAttribute(handle); + if (attribute.Constructor.Kind == HandleKind.MemberReference) + { + MemberReference constructor = reader.GetMemberReference((MemberReferenceHandle)attribute.Constructor); + if (constructor.Parent.Kind == HandleKind.TypeReference && + reader.GetString(reader.GetTypeReference((TypeReferenceHandle)constructor.Parent).Name) == "DebuggableAttribute") + { + return true; + } + } + } + + return false; + } + + // With -DEBUG or -PDB the PDB is a file beside the output, the CodeView entry names its full path and carries + // its id, and nothing is embedded in the image. + [Theory] + [InlineData("-debug")] + [InlineData("-pdb")] + public void PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string pdbSwitch) + { + string dll = OutputPath("TestPdbFile1.dll"); + string pdb = OutputPath("TestPdbFile1.pdb"); + + Assert.Equal(0, RunIlasm(dll, pdbSwitch)); + + Assert.Equal(new[] { "TestPdbFile1.dll", "TestPdbFile1.pdb" }, OutputFileNames()); + using PEReader pe = ReadImage(dll); + ImmutableArray entries = pe.ReadDebugDirectory(); + Assert.DoesNotContain(entries, entry => entry.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); + DebugDirectoryEntry codeViewEntry = Assert.Single(entries, entry => entry.Type == DebugDirectoryEntryType.CodeView); + CodeViewDebugDirectoryData codeView = pe.ReadCodeViewDebugDirectoryData(codeViewEntry); + Assert.Equal(pdb, codeView.Path); + BlobContentId pdbId = ReadPdbId(pdb); + Assert.Equal((pdbId.Guid, pdbId.Stamp), (codeView.Guid, codeViewEntry.Stamp)); + } + + // The debug directory is CodeView then PdbChecksum, and under -DET a Reproducible entry follows. + [Theory] + [InlineData(false)] + [InlineData(true)] + public void PdbSwitch_DebugDirectoryIsCodeViewThenPdbChecksumThenReproducibleWhenDeterministic(bool deterministic) + { + string dll = OutputPath("TestPdbFile1.dll"); + + Assert.Equal(0, RunIlasm(dll, deterministic ? "-debug -det" : "-debug")); + + DebugDirectoryEntryType[] expected = deterministic + ? [DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible] + : [DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum]; + using PEReader pe = ReadImage(dll); + Assert.Equal(expected, pe.ReadDebugDirectory().Select(entry => entry.Type)); + } + + // The PdbChecksum entry is SHA-256 of the PDB file with its 20-byte id zeroed (PE-COFF.md). + [Theory] + [InlineData("-debug")] + [InlineData("-debug -det")] + public void PdbSwitch_PdbChecksumIsSha256OfThePdbFileWithItsIdZeroed(string switches) + { + string dll = OutputPath("TestPdbFile1.dll"); + string pdb = OutputPath("TestPdbFile1.pdb"); + + Assert.Equal(0, RunIlasm(dll, switches)); + + using PEReader pe = ReadImage(dll); + DebugDirectoryEntry checksumEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.PdbChecksum); + PdbChecksumDebugDirectoryData checksum = pe.ReadPdbChecksumDebugDirectoryData(checksumEntry); + byte[] pdbBytes = File.ReadAllBytes(pdb); + using (MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(pdbBytes))) + { + DebugMetadataHeader header = provider.GetMetadataReader().DebugMetadataHeader!; + Array.Clear(pdbBytes, header.IdStartOffset, header.Id.Length); + } + + Assert.Equal("SHA256", checksum.AlgorithmName); + Assert.Equal(SHA256.HashData(pdbBytes), checksum.Checksum.ToArray()); + } + + // -DET gives byte-identical image and PDB files when the same source is assembled twice to the same output. + [Fact] + public void Deterministic_AssemblingTwice_WritesIdenticalImageAndPdb() + { + string dll = OutputPath("TestPdbFile1.dll"); + string pdb = OutputPath("TestPdbFile1.pdb"); + Assert.Equal(0, RunIlasm(dll, "-debug -det")); + byte[] firstImage = File.ReadAllBytes(dll); + byte[] firstPdb = File.ReadAllBytes(pdb); + + Assert.Equal(0, RunIlasm(dll, "-debug -det")); + + Assert.Equal(firstImage, File.ReadAllBytes(dll)); + Assert.Equal(firstPdb, File.ReadAllBytes(pdb)); + } + + // Without -DEBUG or -PDB there is no PDB file and no debug directory, even with .line directives or -DET. + [Theory] + [InlineData(false, "")] + [InlineData(false, "-det")] + [InlineData(true, "")] + [InlineData(true, "-det")] + public void NoPdbSwitch_WritesNoPdbAndNoDebugDirectory(bool withLineDirective, string switches) + { + string dll = OutputPath("Output.dll"); + + Assert.Equal(0, withLineDirective + ? RunIlasm(SourceWithLineDirective, "TestPdbFileLine", dll, switches) + : RunIlasm(dll, switches)); + + Assert.Equal(new[] { "Output.dll" }, OutputFileNames()); + using PEReader pe = ReadImage(dll); + DirectoryEntry debugTable = pe.PEHeaders.PEHeader!.DebugTableDirectory; + Assert.Equal((0, 0), (debugTable.RelativeVirtualAddress, debugTable.Size)); + } + + // -PDB produces the PDB without adding a DebuggableAttribute; -DEBUG adds one. + [Theory] + [InlineData("-pdb", false)] + [InlineData("-debug", true)] + public void PdbSwitch_AddsADebuggableAttributeOnlyWithDebug(string pdbSwitch, bool expectAttribute) + { + string dll = OutputPath("TestPdbFile1.dll"); + + Assert.Equal(0, RunIlasm(dll, pdbSwitch)); + + Assert.Equal(expectAttribute, HasDebuggableAttribute(dll)); + } + + // A PDB left by an earlier build is replaced when ilasm writes a new one. + [Fact] + public void StalePdb_IsReplacedWhenAPdbIsWritten() + { + string dll = OutputPath("TestPdbFile1.dll"); + string pdb = OutputPath("TestPdbFile1.pdb"); + File.WriteAllBytes(pdb, [0xDE, 0xAD]); + + Assert.Equal(0, RunIlasm(dll, "-debug")); + + Assert.Equal(pdb, FindAssociatedPdb(dll)); + } + + // Without a PDB switch, a successful build deletes the PDB of the image it replaces. + [Fact] + public void StalePdb_IsDeletedWhenItsImageIsReplacedWithoutAPdb() + { + string dll = OutputPath("TestPdbFile1.dll"); + Assert.Equal(0, RunIlasm(dll, "-debug")); + Assert.Equal(new[] { "TestPdbFile1.dll", "TestPdbFile1.pdb" }, OutputFileNames()); + + Assert.Equal(0, RunIlasm(dll, string.Empty)); + + Assert.Equal(new[] { "TestPdbFile1.dll" }, OutputFileNames()); + } + + // Without a PDB switch, a PDB beside an output that did not exist before belongs to no image ilasm replaced, so it + // is kept. + [Fact] + public void UnrelatedPdb_IsKeptWhenNoImageIsReplaced() + { + string pdb = OutputPath("TestPdbFile1.pdb"); + Assert.Equal(0, RunIlasm(OutputPath("Other.dll"), "-debug")); + File.Move(OutputPath("Other.pdb"), pdb); + byte[] unrelatedPdb = File.ReadAllBytes(pdb); + + Assert.Equal(0, RunIlasm(OutputPath("TestPdbFile1.dll"), string.Empty)); + + Assert.Equal(unrelatedPdb, File.ReadAllBytes(pdb)); + } + + // When assembly fails, the existing output and PDB are left as they were: after an error in the source and after + // an exception (an include that does not exist). + [Theory] + [InlineData("Error", "")] + [InlineData("Error", "-debug")] + [InlineData("MissingInclude", "-debug")] + public void FailedBuild_LeavesTheExistingOutputAndPdbUnchanged(string failure, string switches) + { + string source = failure == "Error" ? SourceWithError : SourceWithMissingInclude; + string dll = OutputPath("Output.dll"); + string pdb = OutputPath("Output.pdb"); + byte[] existingDll = [0x4D, 0x5A]; + byte[] existingPdb = [0xDE, 0xAD]; + File.WriteAllBytes(dll, existingDll); + File.WriteAllBytes(pdb, existingPdb); + + Assert.NotEqual(0, RunIlasm(source, "TestPdbFile" + failure, dll, switches)); + + Assert.Equal(existingDll, File.ReadAllBytes(dll)); + Assert.Equal(existingPdb, File.ReadAllBytes(pdb)); + } + + // With -ERR, an image written despite errors is handled as after a successful build: with -DEBUG its PDB is + // written beside it, and without a PDB switch the PDB of the image it replaces is deleted. + [Theory] + [InlineData("-err -debug")] + [InlineData("-err")] + public void ErrorTolerantBuildWithErrors_HandlesThePdbAsASuccessfulBuildDoes(string switches) + { + string dll = OutputPath("TestPdbFileRecoverableError.dll"); + string pdb = OutputPath("TestPdbFileRecoverableError.pdb"); + Assert.Equal(0, RunIlasm(dll, "-debug")); + Assert.NotEqual(0, RunIlasm(SourceWithRecoverableError, "TestPdbFileRecoverableError", dll, "-debug")); + + Assert.Equal(0, RunIlasm(SourceWithRecoverableError, "TestPdbFileRecoverableError", dll, switches)); + + using (PEReader pe = ReadImage(dll)) + { + MetadataReader reader = pe.GetMetadataReader(); + Assert.Equal("TestPdbFileRecoverableError", reader.GetString(reader.GetAssemblyDefinition().Name)); + } + + if (switches.Contains("-debug", StringComparison.Ordinal)) + { + Assert.Equal(pdb, FindAssociatedPdb(dll)); + } + else + { + Assert.False(File.Exists(pdb)); + } + } + + // An output named like its PDB is refused rather than overwritten by the PDB. + [Fact] + public void OutputNamedLikeItsPdb_IsRefused() + { + string output = OutputPath("TestPdbFile1.pdb"); + + Assert.NotEqual(0, RunIlasm(output, "-debug")); + + Assert.Empty(OutputFileNames()); + } +} diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj index a845a4021aeccf..bcda70c01347b8 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj +++ b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj @@ -6,6 +6,8 @@ enable false + + false @@ -14,8 +16,11 @@ - + + + Program.cs + IlasmRootCommand.cs From f790795a86148e21ee803de8bb9d70435e4a6819 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Tue, 6 Oct 2026 15:00:01 -0700 Subject: [PATCH 6/9] State when each output stream is used, and pin the writer test pair's PDB ids The IOutputStreams remarks now say exactly when OutputWriter.Write calls each member: OpenExistingOutput only when no PDB is produced and the PDB path is not the output path; CreateOutput unless the PDB would overwrite the output; WritePdb when a PDB is produced; otherwise OpenExistingPdb only when the replaced output yielded a PDB id, and TryDeletePdb only when that id equals the existing PDB's id. The stream test that keeps a PDB the replaced image does not reference again asserts its precondition, which the file-based test had before the split: the replaced image's CodeView entry and the PDB at the PDB path carry different ids. The ids are read with PEReader and MetadataReaderProvider rather than the writer's own readers. The two pairs differ by construction: they are compiled deterministically from different sources, and a deterministic PDB id is a hash of the PDB. --- .../ilasm/src/ILAssembler/OutputWriter.cs | 8 +++++--- .../ILAssembler.Tests/OutputWriterTests.cs | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/tools/ilasm/src/ILAssembler/OutputWriter.cs b/src/tools/ilasm/src/ILAssembler/OutputWriter.cs index 79da0880119f11..c90ac31ca343c9 100644 --- a/src/tools/ilasm/src/ILAssembler/OutputWriter.cs +++ b/src/tools/ilasm/src/ILAssembler/OutputWriter.cs @@ -181,9 +181,11 @@ public static OutputWriteResult Write(IOutputStreams output, Action writ /// /// /// uses these members in this order, each at most once: -/// ; , only when no PDB is produced; -/// ; then either , or followed by -/// when the existing PDB belongs to the replaced image. It disposes every stream it is +/// ; , only when no PDB is produced and +/// is ; , unless the PDB would +/// overwrite the output; then either , when a PDB is produced, or +/// , only when the replaced output yielded a PDB id, followed by +/// , only when that id equals the id of the existing PDB. It disposes every stream it is /// given. /// public interface IOutputStreams diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs index 54fb999248925a..2c0cb154ad78cb 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/OutputWriterTests.cs @@ -5,6 +5,8 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.IO; +using System.Linq; +using System.Reflection.Metadata; using System.Reflection.PortableExecutable; using Xunit; @@ -33,6 +35,21 @@ public class OutputWriterTests // The previous output is the image of the PDB at the PDB path. private static MemoryOutputStreams ImageAndItsPdb() => new() { Output = s_pair.Image.ToArray(), Pdb = s_pair.Pdb.ToArray() }; + // The PDB id (GUID and stamp) that an image's CodeView entry refers to, and the id of a Portable PDB, read + // without the writer's own readers. + private static BlobContentId CodeViewPdbId(ImmutableArray image) + { + using var reader = new PEReader(image); + DebugDirectoryEntry codeView = reader.ReadDebugDirectory().Single(entry => entry.Type == DebugDirectoryEntryType.CodeView); + return new BlobContentId(reader.ReadCodeViewDebugDirectoryData(codeView).Guid, codeView.Stamp); + } + + private static BlobContentId PortablePdbId(ImmutableArray pdb) + { + using MetadataReaderProvider provider = MetadataReaderProvider.FromPortablePdbImage(pdb); + return new BlobContentId(provider.GetMetadataReader().DebugMetadataHeader!.Id); + } + [Fact] public void Write_WithPdb_WritesTheImageAndThePdb() { @@ -103,6 +120,8 @@ public void Write_WithoutPdb_KeepsAPdbWhenNoImageIsReplaced() [Fact] public void Write_WithoutPdb_KeepsAPdbThatTheReplacedImageDoesNotReference() { + // The replaced image refers to a PDB, but not to the one at the PDB path. + Assert.NotEqual(PortablePdbId(s_pair.Pdb), CodeViewPdbId(s_otherPair.Image)); var output = new MemoryOutputStreams { Output = s_otherPair.Image.ToArray(), Pdb = s_pair.Pdb.ToArray() }; OutputWriteResult result = OutputWriter.Write(output, WriteImage, portablePdb: null); From 3f6053f90b6825c84292f96b49ed21f6672d4fe5 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Wed, 7 Oct 2026 15:46:19 -0700 Subject: [PATCH 7/9] Refuse an output that is a symbolic link to its PDB, and report the tests that cannot run as skipped Review feedback on the output file writer: - IsOutputPath compared the output and PDB paths as strings, so an output that is a symbolic link to .pdb was not recognized: the image was written through the link into the PDB's file, the PDB was then renamed over it, and the image was lost although the run reported success. IsOutputPath now also follows the chain of symbolic links that starts at the output path (relative targets against the link's directory, dangling targets included, at most 40 links) and compares each target with the PDB path, so such an output gets the existing "would overwrite the output file" refusal. A missing path counts as no link; any other failure to read a link propagates, so an output that cannot be inspected is not written. A link at the PDB path and a hard link between the two names need no check, because the PDB is renamed into place rather than written through either name; tests pin both. - The rename test returned early on Windows under a plain [Fact], so it was reported as passed there without running. It now uses [ConditionalFact] and is reported as skipped on Windows. The new symbolic-link tests are conditioned on whether the process can create a symbolic link, so they run on Windows where that is permitted and are reported as skipped otherwise; the hard-link test runs everywhere. The test project references Microsoft.DotNet.XUnitExtensions for the attribute, as the coreclr tool test projects do. --- .../ilasm/src/ILAssembler/OutputWriter.cs | 7 +- src/tools/ilasm/src/ilasm/OutputFileWriter.cs | 80 +++++++- .../ILAssembler.Tests.csproj | 1 + .../OutputFileWriterTests.cs | 174 +++++++++++++++++- 4 files changed, 243 insertions(+), 19 deletions(-) diff --git a/src/tools/ilasm/src/ILAssembler/OutputWriter.cs b/src/tools/ilasm/src/ILAssembler/OutputWriter.cs index c90ac31ca343c9..14f93a3eda4092 100644 --- a/src/tools/ilasm/src/ILAssembler/OutputWriter.cs +++ b/src/tools/ilasm/src/ILAssembler/OutputWriter.cs @@ -50,7 +50,8 @@ public static class OutputWriter /// The PDB, or when none was produced. /// What was written or deleted. /// - /// An exception from , from creating or closing the output, or from + /// An exception from propagates to the caller before anything is + /// written. An exception from , from creating or closing the output, or from /// propagates to the caller. A PDB that belongs to the replaced image but /// cannot be deleted ( returns ) is left in place /// without failing the write. @@ -191,8 +192,8 @@ public static OutputWriteResult Write(IOutputStreams output, Action writ public interface IOutputStreams { /// - /// Gets whether the PDB path names the output itself (an output named Min.pdb, for example), so that - /// writing a PDB would overwrite the image. + /// Gets whether the PDB path names the output itself (an output named Min.pdb, for example, or an output + /// that is a symbolic link to the PDB path), so that writing a PDB would overwrite the image. /// bool PdbPathIsOutputPath { get; } diff --git a/src/tools/ilasm/src/ilasm/OutputFileWriter.cs b/src/tools/ilasm/src/ilasm/OutputFileWriter.cs index df4d687defdc6b..4aa7e04125e5eb 100644 --- a/src/tools/ilasm/src/ilasm/OutputFileWriter.cs +++ b/src/tools/ilasm/src/ilasm/OutputFileWriter.cs @@ -27,31 +27,93 @@ namespace ILAssembler; /// so the PDB path holds either the file that was there before or the complete new PDB. If writing or renaming the /// temporary file fails, the run fails with the new image already written, and deleting the temporary file is /// attempted; it can remain if that deletion fails or the process ends before the rename. Because the PDB is renamed -/// into place, a symbolic or hard link at <output>.pdb is replaced by a regular file and the link's -/// target keeps its old content; deleting a stale PDB likewise removes a symbolic link, not its target. A stale PDB +/// into place, a symbolic or hard link at <output>.pdb is replaced by a regular file and writing the PDB +/// leaves the link's target as it was; deleting a stale PDB likewise removes a symbolic link, not its target. A stale PDB /// that cannot be deleted is left in place without failing the write. /// +/// +/// The image, by contrast, is written through a symbolic link at the output path. An output that leads to +/// <output>.pdb through symbolic links is therefore treated like an output named <output>.pdb +/// (): no PDB is written over it, and it is not deleted as a stale PDB. +/// /// internal static class OutputFileWriter { + /// + /// The most symbolic links follows from the output path, which is the limit Linux + /// applies when it resolves a path. A cycle of links ends the walk here too. + /// + internal const int MaxSymbolicLinks = 40; + /// /// Gets the full path of the PDB for an output: the output path with its extension replaced by .pdb. /// public static string GetPdbPath(string outputPath) => Path.GetFullPath(Path.ChangeExtension(outputPath, ".pdb")); /// - /// Gets whether names the output file itself, as it does for an output named - /// Min.pdb. + /// Gets whether writing the image to would write it at , + /// where the PDB then replaces it: the output is named like its PDB (an output named Min.pdb), or it is a + /// symbolic link that leads to the PDB path. /// /// + /// + /// The full output path is compared with the PDB path, and so is each target in the chain of symbolic links that + /// starts at the output path, whether or not the last target exists. A relative link target is resolved against + /// the directory of the link. Only the links at the output path and at its targets are followed; the directories + /// in each path are compared as written. + /// + /// + /// The walk ends at a path that is not a symbolic link, at a path that does not exist (a + /// or counts as no link), or after + /// links. Any other failure to read a link, such as an + /// or , propagates, so that an output that + /// cannot be inspected is not written. + /// + /// + /// A symbolic link at the PDB path needs no check, because the PDB is renamed into place and never written + /// through a link. Nor does a hard link between the two names: the image is written through the output's name + /// and the PDB then replaces the other name, which leaves the image under the output's name. + /// + /// /// The comparison ignores case on Windows and macOS, whose default file systems do, so that /// Min.PDB and Min.pdb are treated as the same file there; elsewhere it is exact. + /// /// - public static bool IsOutputPath(string pdbPath, string outputPath) => - string.Equals( - pdbPath, - Path.GetFullPath(outputPath), - OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); + public static bool IsOutputPath(string pdbPath, string outputPath) + { + StringComparison comparison = OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + string? path = Path.GetFullPath(outputPath); + for (int links = 0; path is not null && links <= MaxSymbolicLinks; links++) + { + if (string.Equals(pdbPath, path, comparison)) + { + return true; + } + + path = TryGetSymbolicLinkTarget(path); + } + + return false; + } + + /// + /// Gets the full path of the target of the symbolic link at , or + /// when there is no link there or nothing exists at . + /// + /// Any other failure to read the link propagates. + private static string? TryGetSymbolicLinkTarget(string path) + { + try + { + return File.ResolveLinkTarget(path, returnFinalTarget: false)?.FullName; + } + catch (Exception ex) when (ex is FileNotFoundException or DirectoryNotFoundException) + { + return null; + } + } /// /// Writes the image to , then writes to diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj index bcda70c01347b8..2bc00fa2b90048 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj +++ b/src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj @@ -12,6 +12,7 @@ + diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs index 28190773130d80..d0284b62509605 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using System.IO; using System.Linq; +using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace ILAssembler.Tests; @@ -23,6 +24,9 @@ public sealed class OutputFileWriterTests : IDisposable // An image and the PDB its CodeView entry refers to. private static readonly (ImmutableArray Image, ImmutableArray Pdb) s_pair = DocumentCompilerTestHelpers.CompileImageAndPdb("A"); + // Whether this process can create a symbolic link, probed once on first use (see CanCreateSymbolicLinks). + private static readonly Lazy s_canCreateSymbolicLinks = new(ProbeSymbolicLinkCreation); + private readonly string _directory = Directory.CreateTempSubdirectory("ilasm-output-").FullName; public void Dispose() => Directory.Delete(_directory, recursive: true); @@ -33,6 +37,49 @@ public sealed class OutputFileWriterTests : IDisposable private static void WriteImage(Stream stream) => stream.Write(s_image); + /// + /// The condition of the tests that are skipped on Windows, each of which says why. + /// + public static bool IsNotWindows => !OperatingSystem.IsWindows(); + + /// + /// The condition of the symbolic-link tests: whether this process can create a symbolic link to a file, which on + /// Windows needs a privilege or developer mode. Probed once, by creating one in a new temporary directory; a + /// file-system failure at any step, including creating that directory, counts as no. + /// + public static bool CanCreateSymbolicLinks => s_canCreateSymbolicLinks.Value; + + private static bool ProbeSymbolicLinkCreation() + { + string? directory = null; + try + { + directory = Directory.CreateTempSubdirectory("ilasm-symlink-probe-").FullName; + string target = Path.Combine(directory, "target"); + File.WriteAllBytes(target, []); + File.CreateSymbolicLink(Path.Combine(directory, "link"), target); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return false; + } + finally + { + if (directory is not null) + { + try + { + Directory.Delete(directory, recursive: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Removing the probe's directory is best effort; the answer does not depend on it. + } + } + } + } + [Theory] [InlineData("Min.dll", "Min.pdb")] [InlineData("Min", "Min.pdb")] @@ -68,17 +115,12 @@ public void Write_WithPdb_ReplacesAnExistingPdb() Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); } - [Fact] + [ConditionalFact(typeof(OutputFileWriterTests), nameof(IsNotWindows))] public void Write_WithPdb_RenamesANewFileOverTheExistingPdbInsteadOfRewritingIt() { // Renaming a completed temporary file over the PDB path is what keeps a failed write from leaving a // partial PDB. It is observable as a reader of the old PDB still seeing the old content afterwards. - // Renaming over a file that is open depends on the Windows version, so this runs elsewhere. - if (OperatingSystem.IsWindows()) - { - return; - } - + // Renaming over a file that is open depends on the Windows version, so this is skipped there. string outputPath = PathOf("Min.dll"); string pdbPath = OutputFileWriter.GetPdbPath(outputPath); File.WriteAllBytes(pdbPath, s_stale); @@ -186,4 +228,122 @@ public void Write_WithPdb_TreatsAnOutputDifferingFromThePdbOnlyInCaseAsTheDefaul Assert.Equal(caseInsensitive ? OutputWriteResult.PdbWouldOverwriteOutput : OutputWriteResult.ImageAndPdbWritten, result); } + + [ConditionalFact(typeof(OutputFileWriterTests), nameof(CanCreateSymbolicLinks))] + public void Write_WithPdb_WritesNothingWhenTheOutputIsASymbolicLinkToThePdb() + { + // Writing the image through the link would put it in Min.pdb, which the PDB then replaces. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + File.CreateSymbolicLink(outputPath, pdbPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(pdbPath, new FileInfo(outputPath).LinkTarget); + Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + Assert.Equal(new[] { "Min.dll", "Min.pdb" }, FileNames()); + } + + [ConditionalFact(typeof(OutputFileWriterTests), nameof(CanCreateSymbolicLinks))] + public void Write_WithPdb_ResolvesARelativeSymbolicLinkAgainstTheLinksDirectory() + { + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + Directory.CreateDirectory(PathOf("sub")); + File.WriteAllBytes(pdbPath, s_stale); + File.CreateSymbolicLink(outputPath, Path.Combine("sub", "..", "Min.pdb")); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(s_stale, File.ReadAllBytes(pdbPath)); + } + + [ConditionalFact(typeof(OutputFileWriterTests), nameof(CanCreateSymbolicLinks))] + public void Write_WithPdb_WritesNothingWhenTheOutputIsASymbolicLinkToAPdbThatDoesNotExistYet() + { + // Creating the output through the dangling link would create Min.pdb, which the PDB then replaces. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.CreateSymbolicLink(outputPath, pdbPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(new[] { "Min.dll" }, FileNames()); + } + + [ConditionalFact(typeof(OutputFileWriterTests), nameof(CanCreateSymbolicLinks))] + public void Write_WithPdb_WritesNothingWhenTheOutputLeadsThroughThePdbPathToAnotherFile() + { + // Min.dll -> Min.pdb -> Other.dll: the image would go to Other.dll, but replacing the link at Min.pdb with the + // PDB would leave Min.dll naming the PDB. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + string otherPath = PathOf("Other.dll"); + File.WriteAllBytes(otherPath, s_stale); + File.CreateSymbolicLink(pdbPath, otherPath); + File.CreateSymbolicLink(outputPath, pdbPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.PdbWouldOverwriteOutput, result); + Assert.Equal(s_stale, File.ReadAllBytes(otherPath)); + } + + [ConditionalFact(typeof(OutputFileWriterTests), nameof(CanCreateSymbolicLinks))] + public void Write_WithPdb_ReplacesASymbolicLinkToTheOutputAtThePdbPath() + { + // The PDB is renamed into place, so a link at the PDB path is replaced rather than written through. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(outputPath, s_stale); + File.CreateSymbolicLink(pdbPath, outputPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.ImageAndPdbWritten, result); + Assert.Equal(s_image, File.ReadAllBytes(outputPath)); + Assert.Null(new FileInfo(pdbPath).LinkTarget); + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void Write_WithPdb_KeepsTheImageWhenTheOutputAndThePdbAreHardLinks() + { + // The image is written through Min.dll into the shared file, and the PDB then replaces the name Min.pdb. + string outputPath = PathOf("Min.dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + File.WriteAllBytes(pdbPath, s_stale); + File.CreateHardLink(outputPath, pdbPath); + + OutputWriteResult result = OutputFileWriter.Write(outputPath, pdbPath, WriteImage, s_pdb); + + Assert.Equal(OutputWriteResult.ImageAndPdbWritten, result); + Assert.Equal(s_image, File.ReadAllBytes(outputPath)); + Assert.Equal(s_pdb.ToArray(), File.ReadAllBytes(pdbPath)); + } + + [Fact] + public void IsOutputPath_TreatsAnOutputInAMissingDirectoryAsNoLink() + { + string outputPath = PathOf(Path.Combine("missing", "Min.dll")); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + + Assert.False(OutputFileWriter.IsOutputPath(pdbPath, outputPath)); + } + + [ConditionalFact(typeof(OutputFileWriterTests), nameof(IsNotWindows))] + public void IsOutputPath_PropagatesAFailureToInspectTheOutputOtherThanItsAbsence() + { + // A file name longer than the file system allows cannot be inspected, and that is not a missing output. On + // Unix it fails with ENAMETOOLONG; the error Windows reports for it is not pinned here, so this is skipped + // there. + string outputPath = PathOf(new string('a', 300) + ".dll"); + string pdbPath = OutputFileWriter.GetPdbPath(outputPath); + + Assert.Throws(() => OutputFileWriter.IsOutputPath(pdbPath, outputPath)); + } } From 29bed49aa77bc0eb27d4803b80009b922f24fbf9 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Thu, 8 Oct 2026 16:00:48 -0700 Subject: [PATCH 8/9] Record the PDB file name for deterministic builds The review asked that a deterministic build not record the PDB's full path, in the way the native linker's /PDBALTPATH:%_PDB% does, and that a non-deterministic build keep it. The image's CodeView entry now names the PDB's file name and extension (Min.pdb) with --deterministic, and its full path otherwise. The PDB file is still written beside the output in both modes. Native ilasm records the full path in both modes, so the deterministic rule is a deliberate difference from it. So a deterministic image no longer depends on the directory it is written to: the same input assembled to two directories gives byte-identical images, as it already gave byte-identical PDBs. The command line makes the choice, because it is the one place that knows the PDB's full path; the assembler records Options.PdbFilePath as given. New command-line tests pin, under -DET: the file name with -DEBUG and -PDB; the file name for a relative output path (reported as skipped where no relative path reaches the temporary directory); that System.Reflection.Metadata finds the PDB beside the image from that name, with the matching id; and identical images and PDBs across two output directories. The existing full-path test now states that it covers builds without -DET. The README and the doc comments state what is recorded in each mode. --- src/tools/ilasm/README.md | 4 +- .../Actions/GrammarActions.BuildImage.cs | 7 +- .../src/ILAssembler/CompilationResult.cs | 4 +- src/tools/ilasm/src/ILAssembler/Options.cs | 19 +++- src/tools/ilasm/src/ilasm/Program.cs | 9 +- .../CommandLinePdbFileTests.cs | 90 ++++++++++++++++++- 6 files changed, 117 insertions(+), 16 deletions(-) diff --git a/src/tools/ilasm/README.md b/src/tools/ilasm/README.md index 7ebfc774324d3f..f6c9eba4cf8840 100644 --- a/src/tools/ilasm/README.md +++ b/src/tools/ilasm/README.md @@ -27,10 +27,10 @@ ilasm writes debug information as a separate Portable PDB file beside the output - **Which switches produce a PDB.** `--debug` (`-DEBUG`), `--debug-mode impl|opt` (`-DEBUG=IMPL`, `-DEBUG=OPT`) and `--pdb` (`-PDB`). Without one of them there is no PDB and no debug directory: `.line` directives are still parsed and checked, but their sequence points are not emitted. - **Where it goes.** The PDB is a separate file named after the output with its extension replaced by `.pdb`, in the same directory (`Min.pdb` for `Min.dll` or `Min`). For an output without an extension in a directory whose name has a dot, native ilasm differs: it cuts the whole path at its last dot, so `-OUTPUT=a.b/Min` gives it `a.pdb` beside the `a.b` directory, where ilasm writes `a.b/Min.pdb`. The PDB is not embedded in the image. ilasm writes and closes the image first, then writes the PDB to a new temporary file in the same directory and renames it to `.pdb`, so `.pdb` never holds a partial PDB. If the PDB cannot be written, the run fails and the new image sits beside the previous `.pdb`, if there was one; ilasm tries to delete the temporary file, which can remain if that fails or if the process ends before the rename. - **An output named like its PDB.** If the output path itself ends in `.pdb` (`-OUTPUT=Min.pdb`) and a PDB is requested, the PDB would overwrite the image, so ilasm reports an error and writes nothing. The comparison ignores case on Windows and macOS. -- **What the image records.** The image's debug directory has these entries, in this order: a CodeView entry that names the full path of the PDB file and carries the PDB's id; a PdbChecksum entry with the SHA-256 hash of the PDB file with its 20-byte id zeroed; and, with `--deterministic`, a Reproducible entry. Debuggers and symbol tools use them to find the PDB and to check that it matches the image. When no PDB is produced, the image has no debug directory, with or without `--deterministic`. +- **What the image records.** The image's debug directory has these entries, in this order: a CodeView entry that names the PDB file and carries the PDB's id; a PdbChecksum entry with the SHA-256 hash of the PDB file with its 20-byte id zeroed; and, with `--deterministic`, a Reproducible entry. Debuggers and symbol tools use them to find the PDB and to check that it matches the image. Without `--deterministic`, the CodeView entry names the full path of the PDB file, as native ilasm does. With `--deterministic`, it names only the PDB's file name and extension (`Min.pdb`), because a deterministic image must not depend on the directory it is written to. Debuggers look for a PDB of that name beside the image, symbol servers look it up by that name and the PDB id, and the PdbChecksum entry verifies that the PDB found matches the image. Here ilasm deliberately differs from native ilasm, which records the full path in both modes, and follows the native linker's `/PDBALTPATH:%_PDB%` convention instead. The PDB file itself is written beside the output in both modes. When no PDB is produced, the image has no debug directory, with or without `--deterministic`. - **`--pdb` compared with `--debug`.** `--pdb` produces the PDB but does not itself add a `DebuggableAttribute`, so on its own it does not change how the JIT compiles the assembly; with `--debug` as well, the attribute is still added. `--debug` adds a `DebuggableAttribute` that disables JIT optimization (`0x101`). `--debug-mode impl` (`-DEBUG=IMPL`) also tells the JIT to use implicit sequence points instead of those in the PDB (`0x103`), and `--debug-mode opt` (`-DEBUG=OPT`) keeps JIT optimization enabled with implicit sequence points (`0x3`). These are native ilasm's values. The attribute is added only when the source declares an assembly (`.assembly`). - **Stale PDBs.** When ilasm writes an image without a PDB, because none was requested, it then deletes an existing `.pdb` only if that PDB belongs to the image being replaced: the image previously at the output path names the PDB's id in its CodeView entry. This keeps a PDB from an earlier build from sitting beside the new image. Any other file at that path is kept: a PDB beside an output that did not exist before, the PDB of another image, or a file that is not a Portable PDB. An output that is itself named like its PDB (`-OUTPUT=Min.pdb`) is the new image and is not deleted. A PDB that cannot be deleted is left in place without failing the run. When the assembly fails and produces no output, ilasm writes nothing and deletes nothing; with `--error` (`-ERR`) an image is written despite the errors, and its PDB is handled as for a successful assembly. If writing the image itself fails, the existing PDB is left as it was, and the image may be partial. -- **`--deterministic`.** The PDB id and the image's MVID and timestamp are derived from content hashes, so the same input assembled to the same output path gives byte-identical image and PDB files. The CodeView entry records the full path of the PDB, so assembling to a different directory changes the image (the PDB is unchanged). +- **`--deterministic`.** The PDB id and the image's MVID and timestamp are derived from content hashes, so the same input assembled to an output with the same file name gives byte-identical image and PDB files, in any directory: the CodeView entry records only the PDB's file name. --- diff --git a/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs b/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs index 72380f15cc557f..823b6ab76e66c7 100644 --- a/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs +++ b/src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.BuildImage.cs @@ -579,8 +579,9 @@ private ImmutableArray ValidateExports( /// With , the PDB id is derived from that same hash /// (), so the same input gives the same PDB /// bytes. The PDB bytes do not depend on ; the image does, because - /// the CodeView entry records that path, so the same input and the same PDB path give the same image - /// bytes. Without , the id's GUID is random. The checksum is the + /// the CodeView entry records that path as given, so the same input and the same PDB path give the + /// same image bytes, and a PDB path that is a file name alone keeps the image independent of its + /// directory. Without , the id's GUID is random. The checksum is the /// content hash in both cases. /// /// @@ -645,7 +646,7 @@ BlobContentId PdbIdProvider(IEnumerable content) } /// - /// Gets the PDB path recorded in the CodeView entry: when set, + /// Gets the PDB path recorded in the CodeView entry: as given when set, /// otherwise with its extension replaced by .pdb, /// otherwise assembly.pdb. /// diff --git a/src/tools/ilasm/src/ILAssembler/CompilationResult.cs b/src/tools/ilasm/src/ILAssembler/CompilationResult.cs index 452e094e72d34e..58c17171bfa303 100644 --- a/src/tools/ilasm/src/ILAssembler/CompilationResult.cs +++ b/src/tools/ilasm/src/ILAssembler/CompilationResult.cs @@ -29,9 +29,9 @@ internal CompilationResult(PEBuilder peBuilder, Blob mvidFixup, ImmutableArray, a Reproducible entry. The PDB path is - /// when set; otherwise it is with its + /// as given when set; otherwise it is with its /// extension replaced by .pdb, or assembly.pdb when no output file name is set. - /// The caller writes these bytes to the path named in the CodeView entry. + /// The caller writes these bytes to the file that path names; a file name alone names a file beside the image. /// public ImmutableArray? PortablePdb { get; } diff --git a/src/tools/ilasm/src/ILAssembler/Options.cs b/src/tools/ilasm/src/ILAssembler/Options.cs index 77ab49b3aeba8d..67c3aab70109ce 100644 --- a/src/tools/ilasm/src/ILAssembler/Options.cs +++ b/src/tools/ilasm/src/ILAssembler/Options.cs @@ -95,6 +95,13 @@ public sealed class Options /// /// Produce deterministic outputs. /// + /// + /// The same input and options give the same image and PDB bytes. The image records + /// in its CodeView entry, or, when that is null, the fallback it describes + /// ( with its extension replaced by .pdb, or assembly.pdb), so it + /// depends on that path; + /// the PDB does not. + /// public bool Deterministic { get; set; } /// @@ -142,10 +149,14 @@ public sealed class Options /// when a PDB is produced (see ). /// /// - /// The assembler does not write this file; the caller writes - /// to it. The command-line tool passes the full path of the output file with its extension - /// replaced by .pdb. When null, the CodeView entry names with - /// its extension replaced by .pdb, or assembly.pdb when no output file name is set. + /// The CodeView entry records this value as given. The assembler does not write the PDB file; the + /// caller writes to the file this path names, where a + /// file name alone names a file beside the image. The command-line tool writes the PDB to the output + /// path with its extension replaced by .pdb, and passes the full path of that file, or, with + /// , only its file name and extension, so that a deterministic image does + /// not depend on the directory it is written to. When null, the CodeView entry names + /// with its extension replaced by .pdb, or assembly.pdb + /// when no output file name is set. /// public string? PdbFilePath { get; set; } diff --git a/src/tools/ilasm/src/ilasm/Program.cs b/src/tools/ilasm/src/ilasm/Program.cs index a5e7f34e108ee4..cbaacc304bdb2b 100644 --- a/src/tools/ilasm/src/ilasm/Program.cs +++ b/src/tools/ilasm/src/ilasm/Program.cs @@ -63,8 +63,11 @@ public int Run() int exitCode = 0; try { - // As in native ilasm, the PDB is .pdb beside the output, and the - // image's CodeView entry records its full path. + // As in native ilasm, the PDB is .pdb beside the output, and without + // --deterministic the image's CodeView entry records the PDB's full path. With --deterministic it + // records only the PDB's file name and extension, so that a deterministic image does not depend on + // the directory it is written to. Native ilasm records the full path in both modes; this follows + // the native linker's /PDBALTPATH:%_PDB% convention instead. string pdbPath = OutputFileWriter.GetPdbPath(outputPath); // Report each file being assembled @@ -165,7 +168,7 @@ public int Run() options.PseudoAttributes = Get(_command.PseudoAttributes); options.Fold = Get(_command.Fold); options.OutputFileName = Path.GetFileName(outputPath); - options.PdbFilePath = pdbPath; + options.PdbFilePath = options.Deterministic ? Path.GetFileName(pdbPath) : pdbPath; // Set up include path for #include directive resolution string? includePath = Get(_command.IncludePath); diff --git a/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs b/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs index 85ef88867ed612..41eabd95f861b3 100644 --- a/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs +++ b/src/tools/ilasm/tests/ILAssembler.Tests/CommandLinePdbFileTests.cs @@ -9,6 +9,7 @@ using System.Reflection.Metadata; using System.Reflection.PortableExecutable; using System.Security.Cryptography; +using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace ILAssembler.Tests; @@ -124,6 +125,13 @@ .method public hidebysig static void Foo() cil managed private readonly string _directory = Directory.CreateTempSubdirectory("ilasm-cli-").FullName; + /// + /// Whether a relative path leads from the working directory to the temporary directory, where each test's files + /// are. On Windows there is none when the two are on different drives. + /// + public static bool TemporaryDirectoryHasARelativePath => + !Path.IsPathRooted(Path.GetRelativePath(Environment.CurrentDirectory, Path.GetTempPath())); + public CommandLinePdbFileTests() { Directory.CreateDirectory(Path.Combine(_directory, "src")); @@ -194,8 +202,8 @@ private static bool HasDebuggableAttribute(string imagePath) return false; } - // With -DEBUG or -PDB the PDB is a file beside the output, the CodeView entry names its full path and carries - // its id, and nothing is embedded in the image. + // With -DEBUG or -PDB the PDB is a file beside the output, the CodeView entry names its full path (without -DET) + // and carries its id, and nothing is embedded in the image. [Theory] [InlineData("-debug")] [InlineData("-pdb")] @@ -217,6 +225,84 @@ public void PdbSwitch_WritesThePdbBesideTheOutputAndReferencesIt(string pdbSwitc Assert.Equal((pdbId.Guid, pdbId.Stamp), (codeView.Guid, codeViewEntry.Stamp)); } + // With -DET the PDB is still written beside the output, but the CodeView entry names only its file name and + // extension, so the image does not depend on the directory it is written to; it still carries the PDB's id. + [Theory] + [InlineData("-debug -det")] + [InlineData("-pdb -det")] + public void Deterministic_CodeViewEntryNamesThePdbFileNameOnly(string switches) + { + string dll = OutputPath("TestPdbFile1.dll"); + string pdb = OutputPath("TestPdbFile1.pdb"); + + Assert.Equal(0, RunIlasm(dll, switches)); + + Assert.Equal(new[] { "TestPdbFile1.dll", "TestPdbFile1.pdb" }, OutputFileNames()); + using PEReader pe = ReadImage(dll); + DebugDirectoryEntry codeViewEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.CodeView); + CodeViewDebugDirectoryData codeView = pe.ReadCodeViewDebugDirectoryData(codeViewEntry); + Assert.Equal("TestPdbFile1.pdb", codeView.Path); + BlobContentId pdbId = ReadPdbId(pdb); + Assert.Equal((pdbId.Guid, pdbId.Stamp), (codeView.Guid, codeViewEntry.Stamp)); + } + + // With -DET a relative output path still gives a CodeView entry with no directory part, and the PDB is written + // beside the output. The output path is relative to the working directory of the test process, which is not the + // output's directory. Where no relative path leads there, the test is skipped rather than run with an absolute path. + [ConditionalFact(typeof(CommandLinePdbFileTests), nameof(TemporaryDirectoryHasARelativePath))] + public void Deterministic_RelativeOutputPath_CodeViewEntryNamesThePdbFileNameOnly() + { + string directory = OutputPath("sub"); + Directory.CreateDirectory(directory); + string relativeDll = Path.GetRelativePath(Environment.CurrentDirectory, Path.Combine(directory, "TestPdbFile1.dll")); + Assert.False(Path.IsPathRooted(relativeDll), relativeDll); + + Assert.Equal(0, RunIlasm(relativeDll, "-debug -det")); + + Assert.Equal(new[] { "TestPdbFile1.dll", "TestPdbFile1.pdb" }, Directory.EnumerateFiles(directory).Select(Path.GetFileName).Order().ToArray()); + using PEReader pe = ReadImage(Path.Combine(directory, "TestPdbFile1.dll")); + DebugDirectoryEntry codeViewEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.CodeView); + Assert.Equal("TestPdbFile1.pdb", pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Path); + } + + // With -DET a debugger still finds the PDB from the bare CodeView name: System.Reflection.Metadata probes the + // image's directory for that name, and the PDB it opens has the id that the CodeView entry carries. + [Fact] + public void Deterministic_TheAssociatedPdbIsFoundBesideTheImage() + { + string directory = OutputPath("sub"); + Directory.CreateDirectory(directory); + string dll = Path.Combine(directory, "TestPdbFile1.dll"); + + Assert.Equal(0, RunIlasm(dll, "-debug -det")); + + using PEReader pe = ReadImage(dll); + Assert.True(pe.TryOpenAssociatedPortablePdb(dll, File.OpenRead, out MetadataReaderProvider? provider, out string? pdbPath)); + using (provider) + { + Assert.Equal(Path.Combine(directory, "TestPdbFile1.pdb"), pdbPath); + DebugDirectoryEntry codeViewEntry = Assert.Single(pe.ReadDebugDirectory(), entry => entry.Type == DebugDirectoryEntryType.CodeView); + BlobContentId openedPdbId = new(provider!.GetMetadataReader().DebugMetadataHeader!.Id); + Assert.Equal((pe.ReadCodeViewDebugDirectoryData(codeViewEntry).Guid, codeViewEntry.Stamp), (openedPdbId.Guid, openedPdbId.Stamp)); + } + } + + // -DET gives byte-identical image and PDB files when the same source is assembled to two different directories. + [Fact] + public void Deterministic_AssemblingToTwoDirectories_WritesIdenticalImageAndPdb() + { + string firstDll = OutputPath(Path.Combine("first", "TestPdbFile1.dll")); + string secondDll = OutputPath(Path.Combine("second", "TestPdbFile1.dll")); + Directory.CreateDirectory(Path.GetDirectoryName(firstDll)!); + Directory.CreateDirectory(Path.GetDirectoryName(secondDll)!); + + Assert.Equal(0, RunIlasm(firstDll, "-debug -det")); + Assert.Equal(0, RunIlasm(secondDll, "-debug -det")); + + Assert.Equal(File.ReadAllBytes(firstDll), File.ReadAllBytes(secondDll)); + Assert.Equal(File.ReadAllBytes(Path.ChangeExtension(firstDll, ".pdb")), File.ReadAllBytes(Path.ChangeExtension(secondDll, ".pdb"))); + } + // The debug directory is CodeView then PdbChecksum, and under -DET a Reproducible entry follows. [Theory] [InlineData(false)] From 09d4a5992c9446f767939e35327417f96c687174 Mon Sep 17 00:00:00 2001 From: Peter Shrosbree Date: Fri, 9 Oct 2026 10:37:50 -0700 Subject: [PATCH 9/9] Leave the ilasm README unchanged The README is for build instructions, not for the tool's features, so this restores it to main's text. The behaviour it described is stated in the doc comments of the options, which are unchanged. --- src/tools/ilasm/README.md | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/tools/ilasm/README.md b/src/tools/ilasm/README.md index f6c9eba4cf8840..203eaecda412cf 100644 --- a/src/tools/ilasm/README.md +++ b/src/tools/ilasm/README.md @@ -20,18 +20,6 @@ If you modify any `.g4` grammar files (rare), you must regenerate the parser and This will update the generated files before building the main project. -## Debug information (PDB) - -ilasm writes debug information as a separate Portable PDB file beside the output, as native ilasm does for an output with an extension, and the image gets the same debug directory entry types in the same order. The PDB's content is narrower than native ilasm's: it records only the documents and sequence points given by `.line` directives (native ilasm also records the `.il` source file and its lines) and has no local scopes. - -- **Which switches produce a PDB.** `--debug` (`-DEBUG`), `--debug-mode impl|opt` (`-DEBUG=IMPL`, `-DEBUG=OPT`) and `--pdb` (`-PDB`). Without one of them there is no PDB and no debug directory: `.line` directives are still parsed and checked, but their sequence points are not emitted. -- **Where it goes.** The PDB is a separate file named after the output with its extension replaced by `.pdb`, in the same directory (`Min.pdb` for `Min.dll` or `Min`). For an output without an extension in a directory whose name has a dot, native ilasm differs: it cuts the whole path at its last dot, so `-OUTPUT=a.b/Min` gives it `a.pdb` beside the `a.b` directory, where ilasm writes `a.b/Min.pdb`. The PDB is not embedded in the image. ilasm writes and closes the image first, then writes the PDB to a new temporary file in the same directory and renames it to `.pdb`, so `.pdb` never holds a partial PDB. If the PDB cannot be written, the run fails and the new image sits beside the previous `.pdb`, if there was one; ilasm tries to delete the temporary file, which can remain if that fails or if the process ends before the rename. -- **An output named like its PDB.** If the output path itself ends in `.pdb` (`-OUTPUT=Min.pdb`) and a PDB is requested, the PDB would overwrite the image, so ilasm reports an error and writes nothing. The comparison ignores case on Windows and macOS. -- **What the image records.** The image's debug directory has these entries, in this order: a CodeView entry that names the PDB file and carries the PDB's id; a PdbChecksum entry with the SHA-256 hash of the PDB file with its 20-byte id zeroed; and, with `--deterministic`, a Reproducible entry. Debuggers and symbol tools use them to find the PDB and to check that it matches the image. Without `--deterministic`, the CodeView entry names the full path of the PDB file, as native ilasm does. With `--deterministic`, it names only the PDB's file name and extension (`Min.pdb`), because a deterministic image must not depend on the directory it is written to. Debuggers look for a PDB of that name beside the image, symbol servers look it up by that name and the PDB id, and the PdbChecksum entry verifies that the PDB found matches the image. Here ilasm deliberately differs from native ilasm, which records the full path in both modes, and follows the native linker's `/PDBALTPATH:%_PDB%` convention instead. The PDB file itself is written beside the output in both modes. When no PDB is produced, the image has no debug directory, with or without `--deterministic`. -- **`--pdb` compared with `--debug`.** `--pdb` produces the PDB but does not itself add a `DebuggableAttribute`, so on its own it does not change how the JIT compiles the assembly; with `--debug` as well, the attribute is still added. `--debug` adds a `DebuggableAttribute` that disables JIT optimization (`0x101`). `--debug-mode impl` (`-DEBUG=IMPL`) also tells the JIT to use implicit sequence points instead of those in the PDB (`0x103`), and `--debug-mode opt` (`-DEBUG=OPT`) keeps JIT optimization enabled with implicit sequence points (`0x3`). These are native ilasm's values. The attribute is added only when the source declares an assembly (`.assembly`). -- **Stale PDBs.** When ilasm writes an image without a PDB, because none was requested, it then deletes an existing `.pdb` only if that PDB belongs to the image being replaced: the image previously at the output path names the PDB's id in its CodeView entry. This keeps a PDB from an earlier build from sitting beside the new image. Any other file at that path is kept: a PDB beside an output that did not exist before, the PDB of another image, or a file that is not a Portable PDB. An output that is itself named like its PDB (`-OUTPUT=Min.pdb`) is the new image and is not deleted. A PDB that cannot be deleted is left in place without failing the run. When the assembly fails and produces no output, ilasm writes nothing and deletes nothing; with `--error` (`-ERR`) an image is written despite the errors, and its PDB is handled as for a successful assembly. If writing the image itself fails, the existing PDB is left as it was, and the image may be partial. -- **`--deterministic`.** The PDB id and the image's MVID and timestamp are derived from content hashes, so the same input assembled to an output with the same file name gives byte-identical image and PDB files, in any directory: the CodeView entry records only the PDB's file name. - --- For more details, see the main repository README or contact the maintainers.