Skip to content

Write the managed ilasm PDB to a separate file, as native ilasm does - #135289

Merged
jkoritzinsky merged 9 commits into
dotnet:mainfrom
pcshrosbree:fix/managed-ilasm-pdb-file
Oct 10, 2026
Merged

jkoritzinsky merged 9 commits into
dotnet:mainfrom
pcshrosbree:fix/managed-ilasm-pdb-file

Conversation

@pcshrosbree

@pcshrosbree pcshrosbree commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

In #135212, @jkotas pointed out that native ilasm is being replaced by the C# rewrite in src/tools/ilasm and suggested fixing issues there. I listed three PDB differences between the managed and native ilasm, and @jkoritzinsky confirmed that they are unintended and should be fixed in managed ilasm. This PR fixes them.

Changes

Before After
-DEBUG / -PDB PDB embedded in the image (EmbeddedPortablePdb entry) PDB written to <output>.pdb beside the output, not embedded, as native ilasm
.line without -DEBUG or -PDB embedded PDB produced no PDB and no debug directory, as native ilasm; .line is still parsed and validated
CodeView path always assembly.pdb full path of the PDB file, as native ilasm; under -DET only its file name (Min.pdb), a deliberate difference that follows the linker's /PDBALTPATH:%_PDB%
  • Debug directory. It is CodeView, then PdbChecksum, then, with -DET, Reproducible: the same entry types in the same order as native ilasm.
    • The checksum is SHA-256 of the PDB with its 20-byte id zeroed (PE-COFF.md).
    • It is the hash the PortablePdbBuilder id provider computes, so under -DET the PDB id and the checksum come from one hash.
  • No PDB, no debug directory. An image built without a PDB has no debug directory, with or without -DET. ManagedPEBuilder adds a Reproducible-only directory to a deterministic image when it is given none. A small ILAssemblerPEBuilder subclass, which the vtfixup/export builder also derives from, clears a debug directory with no entries.
  • Library and CLI. PDB generation does not touch the file system.
    • CompilationResult.PortablePdb returns the PDB bytes, the new Options.PdbFilePath supplies the CodeView path, and the CLI writes the file.
    • Library callers that don't set PdbFilePath get the output file name with a .pdb extension, or assembly.pdb.
  • File output is split in two: OutputWriter in ILAssembler works on streams and makes the decisions below, and OutputFileWriter in ilasm supplies the files:
    • Write order. It writes and closes the image, then writes the PDB to a new temporary file in the same directory and renames it over <output>.pdb, so <output>.pdb never holds a partial PDB. If the PDB cannot be written, the run fails with the new image beside the previous PDB, and deleting the temporary file is attempted.
    • Stale PDBs. After writing an image without a PDB, it deletes an existing <output>.pdb only if it belongs to the image that was replaced, that is, if the previous image's CodeView entry names that PDB's id. Any other file is kept, including when the previous output is not a PE image it can read (a COFF object file or a file over 2 GB, for example), and an output that is itself named like its PDB is never deleted. A PDB that cannot be deleted is left in place without failing the run. When assembly fails and produces no output, it writes and deletes nothing; with -ERR, an image produced despite errors is written and its PDB is handled as for a successful assembly.
      • Native ilasm deletes <output>.PDB whenever it writes no PDB, including when assembly fails. I dropped that deliberately, because it can remove a PDB that ilasm did not write or that belongs to an image that was not replaced.
      • On Linux native deletes <output>.PDB, so it has not been removing .pdb files there.
    • Name collisions. If the PDB path would be the output path (-OUTPUT=Min.pdb -DEBUG), it reports an error and writes nothing, where native ilasm overwrites the image with the PDB. The comparison ignores case on Windows and macOS.
  • Switches. --debug-mode alone produces a PDB, as it already implied --debug for the DebuggableAttribute. -PDB still adds no DebuggableAttribute of its own.
  • Docs.
    • The options have doc comments.
    • The DebugMode.Impl docs said it enables Edit and Continue. IMPL is 0x103, Default | IgnoreSymbolStoreSequencePoints | DisableOptimizations, and the docs now say so.
    • The --debug-mode help says what the modes do to the JIT.

Round-trip harness (needed for this PR to merge).

  • src/tests/Common/CLRTest.Jit.targets checked PDB determinism by assembling with -DEBUG -output=<name>.pdb and hashing that file. That works with native ilasm only because the PDB overwrites the image.
  • Managed ilasm now refuses that output path, so the managedilasmroundtrip lane would fail.
  • The third commit assembles that step to IL-RT/<name>.pdbcheck.dll and hashes both that image and the .pdb beside it, which both ilasms write. The previous managed run had also checked the -DEBUG image, by hashing it with the PDB embedded.
  • I ran the changed step by hand with native and managed ilasm. Both repeat byte-identically, and a run without -DET fails the image comparison, and with the image comparison removed fails the PDB comparison, as it should. I could not run the lanes themselves. In the round-trip lane, reassembling a non-debug test assembly without -DEBUG now removes the compiler's <name>.pdb, which belongs to the replaced image; the reassembled image has no debug directory, so nothing reads it, and the previous step overwrote that file anyway.

Determinism. With -DET the CodeView entry records only the PDB's file name, so the same input assembled to outputs of the same name in different directories gives byte-identical images and PDBs (native ilasm records the full path in both modes; this follows the linker's /PDBALTPATH:%_PDB% convention instead).

Remaining differences from native ilasm:

  • PdbChecksum entry timestamp: 0 in managed (DebugDirectoryBuilder), the PDB stamp in native. The managed entry is new in this PR.
  • Nondeterministic PDB stamp: the constant 0x04030201 in managed, the clock in native.
  • -DET PDB id: the PDB content hash in managed, the metadata hash in native.
  • -OUTPUT=a.b/Min: native writes a.pdb in the parent directory; managed writes a.b/Min.pdb. That naming is deliberate.
  • CodeView path under -DET: the PDB's file name in managed, its full path in native. That is deliberate (see Determinism above).
  • Stale PDB removal: the ownership rule above, versus native's unconditional <output>.PDB deletion.
  • PDB content: managed has no local scopes, uses one document per method, and does not record the .il source as a document.
  • Netmodules: native emits the DebuggableAttribute on AssemblyAttributesGoHere; managed emits it only with .assembly.

Testing (Linux x64)

  • ILAssembler.Tests: 1216 before; 2124 at commit 1, 2332 at commits 2 and 3, 2344 at commit 4, 2366 at commits 5 and 6, 2374 at commit 7, and 2379 at commit 8, all passing on Linux; on Windows two file-system tests skip, five symbolic-link tests skip unless the process can create symbolic links, and the relative-output test skips if the temporary directory is on another drive than the working directory.
    • Unit tests read the PDB from the compilation result. They pin:

      • the entry order, with and without -DET;
      • the CodeView path, its fallback, and its id;
      • the checksum, against the PDB with its id zeroed;
      • no PDB from .line alone;
      • no debug directory or debug data without a PDB, including through the vtfixup/export builder;
      • -PDB without a DebuggableAttribute;
      • deterministic PDB and image bytes.
    • OutputWriterTests tests the output writer on in-memory streams, without the file system. It covers:

      • the write order, including closing the image before touching the PDB (an injected close failure);
      • which existing PDB it deletes, including when the replaced output is a COFF object, longer than 2 GB, or cannot be opened;
      • the refusal to write a PDB over the output.
    • OutputFileWriterTests covers the file side: the PDB path, the temporary file and rename, deletion, and the case rule.

    • Seeded theories check the same rules over generated programs and option combinations (800 cases). They also check the file writer over an enumeration of output names and pre-existing output and PDB states (270 cases). Each case is named by its index and description.

    • CommandLinePdbFileTests runs the command line in process on IL sources in a temporary directory (27 cases). It covers:

      • the PDB beside the output and its CodeView entry;
      • under -DET, a CodeView entry with only the PDB's file name, for absolute and relative output paths, and the PDB found beside the image from that name;
      • the entry order and the checksum;
      • no PDB without a switch, with .line and with -DET;
      • -PDB without a DebuggableAttribute;
      • stale-PDB replacement and deletion, and an unrelated PDB kept;
      • failed builds;
      • -ERR with a recoverable error;
      • the refusal of an output named like its PDB;
      • byte-identical -DET repeats, and byte-identical -DET images and PDBs across two output directories.

      The cases that ran native ilasm as a control were dropped when these tests moved out of src/tests/ilasm.

    • I ran 59 selected mutations, one at a time: 19 in the compiler (checksum, entry order, gating), 32 in the two output writers and 8 in the command line. 58 were detected. The survivor removes a file-existence check before opening a file; it is equivalent for stable files, because a missing file raises an IOException that the library counts as no id.

  • I ran the built CLI end to end and compared it with native ilasm, both as built from this branch and with Fix ilasm PdbChecksum and the PDB ID under -DET #135212's fix. I checked the files written, the entries, the checksum, byte-identical -DET repeats, and stale-PDB handling.
  • src/tests/ilasm/PortablePdb against managed ilasm: 0/10 before, 4/10 after.
    • Both TestPortablePdbDebugDirectory cases now pass.
    • The six failures have three causes, all PDB content this PR does not touch: local scopes (4 cases), a method whose sequence points span two documents (TestDocuments1), and the .il source as a document (TestPortablePdbDocuments). Fixes for those are in progress separately and will follow as their own PRs.

Not run: Windows, macOS, Arm64, the round-trip lanes, or the full src/tests suite. The ilasm pipeline runs on this PR and covers the round-trip lanes on eight platforms and ILAssembler.Tests on Windows x64.

Questions:

  • Is ImmutableArray<byte>? CompilationResult.PortablePdb the shape you want, or ImmutableArray<byte> with IsDefault?
  • Would you rather have an option in System.Reflection.Metadata for "no debug directory" than the ILAssemblerPEBuilder subclass? ManagedPEBuilder has no public way to decline its default Reproducible entry.
  • Following the review, the CodeView entry now holds only the PDB's file name under -DET, and its full path otherwise.
  • Is deleting <output>.pdb only when it belongs to the replaced image the behaviour you want?
  • Is the --debug-mode help wording right?

Note

This change was prepared with AI assistance (Anthropic Claude and OpenAI Codex) under my direction. AI agents wrote the code and ran the build and tests on my machine. I reviewed the change before posting.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 6, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib
See info in area-owners.md if you want to be subscribed.

@jkoritzinsky

Copy link
Copy Markdown
Member

Instead of adding tests under src/tests/ilasm, please add the new tests under ILAssembler.Tests. That test suite is significantly cheaper to run and is the way forward for our testing for managed ilasm.

Comment thread src/tools/ilasm/src/ilasm/OutputWriter.cs Outdated
@pcshrosbree

Copy link
Copy Markdown
Contributor Author

Thanks, both are done. The output writer is now two types: OutputWriter in ILAssembler works only on streams, through a small IOutputStreams interface, and makes every decision (write order, which existing PDB to delete, and the refusal to write a PDB over the output), all unit-tested on MemoryStreams in OutputWriterTests; OutputFileWriter in ilasm supplies the files, writes the PDB through a temporary file and a rename, and does the deletion. The library types are public because ILAssembler has no InternalsVisibleTo and the tool already calls only its public types (DocumentCompiler, CompilationResult); I can switch to InternalsVisibleTo if you prefer. The end-to-end tests moved from src/tests/ilasm into ILAssembler.Tests as CommandLinePdbFileTests, which links Program.cs and runs the command line in process against a temporary directory, so src/tests/ilasm matches main again. I dropped the rows that ran native ilasm as a control, since native ilasm has no place in ILAssembler.Tests. Linking Program.cs also needed <GenerateProgramFile>false</GenerateProgramFile> in the test project; I'm happy to revisit either choice.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

An output symlink targeting its PDB path can cause a successful run to lose the generated image.

2 open findings
What changed in this PR

Updates managed ilasm to emit separate Portable PDB files and align debug-directory behavior with native ilasm.

Changes:

  • Returns and writes standalone PDB data with CodeView/checksum entries.
  • Adds safe PDB replacement and stale-PDB handling.
  • Expands unit, generated, CLI, and round-trip tests.
File Description
SourceDirectiveTests.cs Updates source-directive PDB tests.
OutputWriterTests.cs Tests stream-based output decisions.
OutputFileWriterTests.cs Tests filesystem output behavior.
ModuleTests.cs Reads standalone module PDBs.
ILAssembler.Tests.csproj Links CLI output components.
PdbGeneratedCaseTests.cs Adds generated PDB/output tests.
PdbCaseGenerator.cs Generates deterministic test cases.
DocumentCompilerTestHelpers.cs Adds standalone-PDB helpers.
CompilerOptionsTests.cs Tests PDB options and directories.
CommandLinePdbFileTests.cs Adds end-to-end CLI coverage.
AssemblyTests.cs Updates deterministic PDB tests.
VTableExportPEBuilder.cs Uses the shared PE builder.
OutputWriter.cs Implements stream-based output handling.
Options.cs Documents and adds PDB path options.
ILAssemblerPEBuilder.cs Suppresses empty debug directories.
CompilationResult.cs Exposes generated PDB bytes.
GrammarActions.BuildImage.cs Builds standalone PDBs and references.
Program.cs Writes image and PDB outputs.
OutputFileWriter.cs Implements filesystem output handling.
IlasmRootCommand.cs Clarifies debug-mode help.
README.md Documents PDB behavior.
CLRTest.Jit.targets Updates round-trip determinism checks.

🧠 Review effort: Balanced

Comment thread src/tools/ilasm/src/ilasm/OutputFileWriter.cs Outdated
Comment thread src/tools/ilasm/tests/ILAssembler.Tests/OutputFileWriterTests.cs Outdated

@jkoritzinsky jkoritzinsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more request. Otherwise looking good to me so far.

Comment thread src/tools/ilasm/src/ilasm/Program.cs Outdated

@jkoritzinsky jkoritzinsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last change, then this will be good to merge.

Comment thread src/tools/ilasm/README.md Outdated
@jkoritzinsky
jkoritzinsky enabled auto-merge (squash) October 9, 2026 17:48
auto-merge was automatically disabled October 9, 2026 20:02

Head branch was pushed to by a user without write access

@pcshrosbree
pcshrosbree force-pushed the fix/managed-ilasm-pdb-file branch from da5e802 to 8d4f4e2 Compare October 9, 2026 20:02
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 <output>.pdb beside
  the output.
- The CodeView entry names the PDB path given in the new
  Options.PdbFilePath. The CLI passes the full path of <output>.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
  <output>.pdb, so <output>.pdb never holds a partial PDB.
- When the image is written without a PDB, an existing <output>.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 <output>.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.
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.
…rip harness

The "Test PDB determinism" step assembled with -DEBUG to -output=<name>.pdb
and hashed that file. With native ilasm the PDB is <output without
extension>.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/<name>.pdbcheck.dll instead and hash both that image
and the PDB that ilasm writes beside it, IL-RT/<name>.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
<name>.pdb, the C# compiler's PDB, in the test directory. The rest of the
round trip is unchanged.
… 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 <output>.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.
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.
… 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.
…ests 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 <output>.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.
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.
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.
@pcshrosbree
pcshrosbree force-pushed the fix/managed-ilasm-pdb-file branch from 8d4f4e2 to 09d4a59 Compare October 9, 2026 20:31
@pcshrosbree

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #135115 landed (now 09d4a59): the only differences are context lines in Program.cs and the position of a test constant in CompilerOptionsTests.cs; the code is unchanged.

@jkoritzinsky
jkoritzinsky merged commit 9b108d9 into dotnet:main Oct 10, 2026
108 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 13, 2026
jkoritzinsky pushed a commit that referenced this pull request Oct 15, 2026
…s as documents, LocalSignature row id (#135297)

Part of making the managed ilasm (`src/tools/ilasm`) a drop-in for
native ilasm, as discussed in #135212. There @jkotas pointed to the
managed rewrite as the way forward, @jkoritzinsky confirmed that PDB
differences from native ilasm are unintended, and @am11 suggested
combining efforts.

#135289 (the separate `.pdb` file) has merged; this PR's seven commits
are now the whole diff; the last three address review feedback.

Without this PR, with the separate-PDB fix applied, the runtime's
PortablePdb test cases (`IlasmPortablePdbTester` in
`src/tests/ilasm/PortablePdb`) pass 10/10 with native ilasm and 4/10
with the managed one, measured by running those cases against both
ilasms on Linux x64. This PR takes the document-related failures and one
spec defect.

1. **A method can span documents.** Before: one document per method (the
file of its first `.line`), so a method whose `.line` directives named
two files had every point in the first. After: each point records the
document current when its directive is applied. A method spanning
documents has `Document = nil`, an `InitialDocument` in the blob header,
and a `document-record` before each non-hidden point whose document
changes, which is what native writes (`DefineSequencePoints`).
2. **`''` means "same file".** Before: `.line N : C ''` created a
document named `''`. ildasm writes that form when the file did not
change, including on a method's first `.line`. After: an empty name
keeps the current document, as native's `SetSourceFileName` does.
3. **Documents.** Before: documents were created only for files a
method's first point used, in MethodDef order, keyed by name and
language; their language was empty without `.language`; and the input
`.il` file was never a document. After, as in native: the input file, by
full path, is document #1; every `.line`/`#line` file is defined when
its directive is applied, wherever it appears and whether or not a point
uses it; documents are in encounter order, keyed by name
(case-sensitive); each keeps the language current at its first
definition, which defaults to IL assembly
(`af046cd3-d0e1-11d2-977c-00a0c9b4d50c`).
4. **LocalSignature.** Before: the sequence-points blob header always
had `LocalSignature = 0`. After: it is the StandAloneSig row id of the
body's local signature, as the Portable PDB spec requires and native
writes, or 0 without locals. This includes bodies shared by `/FOLD`.
5. **Methods without a body.** Before: a `.line` inside an abstract or
`pinvokeimpl` method, or in a body with directives but no instructions,
gave it a sequence point at IL offset 0 of a body it does not have.
After: such a method has no sequence points; the directive still names
its file, as in native.
6. **`--pathmap`** (from review). `--pathmap <path>=<sourcePath>[,...]`
replaces path prefixes in the paths the debug information records, every
PDB document name and the PDB path in the CodeView entry, with the C#
compiler's `-pathmap` rules: each side is completed with a trailing
separator so a key matches whole directories only, the first matching
entry wins, and the result's separators follow the replacement's. With
`/DET`, where the CodeView entry already names only the PDB's file name,
mapping the directory of the inputs gives byte-identical image and PDB
files wherever the tree is. Files are still read and written at their
real paths.

**Behaviour changes to be aware of:**
- With `/DEBUG` or `/PDB`, every PDB now has the input file as a
document, even without `.line` directives.
- The CLI now passes each input file's full path to the assembler, so it
can name the input document by full path. As a result, error messages
also name the input by its full path, whereas native ilasm prints the
path as given on the command line. I'm happy to keep the diagnostic path
as given and pass the document name separately if that is preferred.

**Deliberate difference from native:** a method with no sequence points,
or without a body, keeps a nil `Document`, as the spec says; native
names the current document.

**Other differences from native ilasm** found in the same survey, not
taken by any of these PRs, are tracked in #135314.

**Not in this PR (follow-ups):**
- LocalScope/LocalVariable rows, explicit local slots and lexical
scopes.
- Implicit sequence points for the `.il` source's own lines.
- `#include`d files as documents. In particular, a file named by a
`.line` inside an include currently stays current after the include
ends, whereas native re-establishes the including file.
- `#line` auto-increment.
- `.line` end-column defaults.
- Two cases that come from the managed ilasm recording sequence points
when a directive is applied rather than when instructions are emitted;
the implicit-points work will change this: a `.line` after a method's
last instruction gives a point at the end offset, where native gives
none; a class-level `.line` followed by a method without `.line` gives
that method's first instruction no point, where native gives it the
class-level coordinates.

**Testing:**
- **Unit tests.** New `PdbDocumentTests` reads the PDB back with
System.Reflection.Metadata, and for the encoding also reads the blob's
raw records. It covers: the nil Document, the InitialDocument and each
point's document (A→B→A switching; hidden points, with no
document-record); two directives at one offset; `''` within a method and
on its first `.line`; files named at top level, class level or by a
replaced directive; encounter order (not MethodDef order) and
case-sensitive names; the default language, `.language` for later
documents and for the next input file, and the first language sticking;
one document per input file; abstract and `pinvokeimpl` methods getting
no points; no points for a body with directives but no instructions,
while an error-tolerant compile keeps the points of a method that still
has its instructions; identical PDB bytes under `/DET`; the
LocalSignature row id: rows 1 and 2, 0 without locals, `/FOLD`, and no
blob for a method with locals but no `.line`. Each new behaviour has a
test that fails when the behaviour is removed; I checked this by
breaking each one in turn. Existing tests that checked documents now
assert the complete ordered list of document names, input file first.
- **Generated cases.** 200 seeded programs, some split across two input
files and some with abstract methods. Their methods move between a few
documents with named, `''`, unnamed and hidden `.line` directives (in
the full form native accepts), class-level directives, `.language` and
locals. The generator computes the expected documents and points
independently. Theories check each point's document, the method's
Document, the InitialDocument, the document table and the
LocalSignature.
- **Command line.** New `CommandLinePdbDocumentTests` runs the ilasm
command line in process, as `CommandLinePdbFileTests` does, and reads
the PDB it writes. It checks what only the command line decides: without
any `.line`, the input file is the only document, by full path; an input
given by a path relative to the current directory (written under it, so
the argument is never rooted, without setting the current directory) is
named by its full path; two input files are two documents in
command-line order, before the document of a `.line` file; and the
`TestDocuments2` case end to end (`''` on a method's first `.line` keeps
the previous method's document). Each of these fails when the behaviour
it checks is broken. `PortablePdbTestReader` can now also open an image
and PDB from files. `MethodTests` now also checks that the PDB header's
entry point is the `.entrypoint` method's MethodDef, not the first
MethodDef, and nil without one; the runtime's `TestPdbDebugDirectory2`
case was the only test of this.
- **`--pathmap`.** `PathMapTests` checks the parsing and mapping rules
as strings; `PdbDocumentTests` and `CompilerOptionsTests` check the
document names and the CodeView path on the library;
`CommandLinePathMapTests` runs the command line in process: the same
source in two directories, each mapped to `/_/`, assembled with `-DET
-DEBUG`, gives byte-identical image and PDB files; the option can be
given more than once and the entries accumulate in order; a malformed
value fails before anything is written; the PDB is written at its real
path. Each rule has a test that fails when the rule is removed.
- **Runtime PortablePdb cases (a measurement, not a change in this
PR).** This PR changes nothing under `src/tests/ilasm`. I ran that
directory's `IlasmPortablePdbTester` cases against both ilasms on Linux
x64, outside the tree: native passes 10/10, and the managed ilasm passes
4/10 at #135289's head and 6/10 with this PR; the four still red are
`TestPortablePdbLocalScope(TestLocalScopes1..4.il)`, which the
local-scope PR takes. The two cases I added while measuring,
`TestDocuments2` and an input given by a relative path, passed with both
ilasms and are now in `CommandLinePdbDocumentTests`.

> [!NOTE]
> This change was prepared with AI assistance (Anthropic Claude and
OpenAI Codex) under my direction. AI agents wrote the code and ran the
build and tests on my machine. I reviewed the change before posting.
jkoritzinsky pushed a commit that referenced this pull request Oct 16, 2026
…ly, and emit LocalScope/LocalVariable rows (#135311)

Part of making the managed ilasm (`src/tools/ilasm`) a drop-in
replacement for native ilasm, as discussed in #135212. There @jkotas
pointed to the managed rewrite as the way forward, @jkoritzinsky
confirmed that PDB differences from native ilasm are unintended, and
@am11 suggested combining efforts.

#135297 (PDB documents) and #135289 (the separate `.pdb` file) have both
merged, and this branch now sits directly on main: the two commits here
are the whole diff.

With the document changes (#135297) applied, the managed ilasm still
failed the four `TestPortablePdbLocalScope` cases of the runtime's
PortablePdb tests (`src/tests/ilasm/PortablePdb`): it passed 6 of those
10 cases, where native ilasm passes all 10. I measured this by running
those cases against both ilasms on Linux x64, outside the tree, at
#135297's head before this change; this PR does not change
`src/tests/ilasm`. The cause is three defects in local handling:

1. **No LocalScope or LocalVariable rows.** Before: none were written.
After: as in native ilasm, each lexical scope that declares a named
local gets a LocalScope row from the offset at its `{` to the offset at
its `}`. The method body as a whole is a scope, and so are `.try`,
`catch`, `filter`, `finally` and `fault` bodies. Each named local in the
scope gets a LocalVariable row (Index = slot, attributes 0). Rows are
sorted by start offset, then longest first. Scope information is kept
whenever a PDB is requested, by any of the switches that request one.
2. **Explicit slots were dropped.** Before: `.locals init ([3] int32 x)`
was appended as the next slot. In `TestLocalScopes4.il`, the name
`LOCAL_12` and `stloc.3` meant different variables, and the signature
had 6 slots instead of 5. After, as in native `EmitLocals`:
   - `[n]` takes slot `n`, padding the table;
   - a local without `[n]` takes the slot after the highest one;
- a slot freed when its scope closes can be declared again with the same
type;
- native's diagnostics apply: the "Local var slot n is in use" warning
(a slot is in use from a declaration until any scope that declared it
closes, as in native), and the "Local var slot n: type conflict" and
"Undefined type of local var slot n in method m" errors;
- a `...` sentinel in a `.locals` list no longer takes a slot. Before,
it took one with an invalid type byte and shifted every later local;
native ilasm ignores it;
   - on parameters, `[n]` keeps its meaning.
3. **Nested blocks did not scope names.** Before: when the method had a
top-level `.locals`, an inner `.locals` declaring the same name resolved
to the outer slot, and inner names stayed visible after the block.
After: each block has its own name scope, searched innermost first; the
first declaration of a name in a scope wins, and names are
case-sensitive.

**Items 2 and 3 change the emitted IL** for inputs with sparse or reused
slots, with shadowed or out-of-scope names, or with a sentinel in
`.locals`. A name declared in a block and used after it is now an error,
as in native ilasm. ildasm always writes sequential slots and unique
names, so a round trip through ildasm cannot show these cases.

**Deliberate differences from native**, listed in
`MANAGED-ILASM-FIXES.md`:
- A scope without instructions gets no LocalScope row. Native writes
length 0, and the spec requires a positive length.
- Within a scope, each name gets a LocalVariable row at the slot its
first declaration has, unless an earlier row of the scope already
describes that slot. A name refers to its first declaration in the
scope, so a row never describes a local that its name does not refer to.
Later declarations of a name, and unnamed locals, take no part and keep
no other name out. Native writes a row for every named local, so a scope
that declares a name or a slot twice gets duplicate names or indices,
which the spec forbids.
- Slots run from 0 to 65535. An explicit slot outside that range (other
than `[-1]`, which means none) is reported as written and the local
takes the next slot. The range is checked on the literal as written, so
a literal too large for 32 bits is reported too; native reads
`[4294967295]` as `[-1]` and `[0x10000000000000000]` as slot 0 without a
diagnostic. A local whose next slot would be past 65535 is reported and
not declared. Native accepts such slots, which no local instruction can
name; it writes slot 65536's PDB index as 0 and does not finish on other
negative values.

**Other differences from native ilasm** found in the same survey, not
taken by any of these PRs, are tracked in #135314.

**A question:** the spec says a method's first scope spans the whole
body. Like native ilasm, this change writes no whole-body scope when the
method-level `.locals` names nothing (`TestLocalScopes3` expects that).
Should a root scope always be written instead?

**A note on output:** `TestLocalScopes4.il` now triggers the in-use
warning, as it does with native ilasm. The CLI's diagnostic rendering
prints the location verbosely (the whole source text); that is a
separate fix.

**Testing:**
- **Unit tests: 71 new rows.**
- `LocalTests` (33): slot rules and diagnostics, including the in-use
flag's native behaviour, the type a slot keeps after a conflict,
out-of-range slots reported as written (including literals too large for
32 bits, one of which would otherwise read as `[-1]`), slot 65535 in
decimal, hexadecimal and octal, and the 65535 boundary with and without
a PDB; a sentinel in `.locals`, and a sentinel-prefixed type, which is a
local; shadowing; out-of-scope names; duplicate names; case-sensitive
names.
  - `LocalScopeTests` (38), read back with System.Reflection.Metadata:
- scope extent: root span, block offsets, sibling blocks, a body without
instructions;
    - row order: sorting, and order for the same range;
- which locals get rows: unnamed locals and unnamed-only blocks, empty
blocks, duplicate and crossed-duplicate names and slots, a later
declaration of a name or an unnamed local never keeping another name's
row out, case-distinct names, a sentinel, Index equal to `[n]` up to
65535, attributes;
- exception-handling bodies: `.try`, `catch`, `finally`, `filter`, a
block inside `.try`, and a program with a slot gap, a slot declared by
both a `.try` body and a `catch` body, an unnamed local and a name
declared again in the `catch` body, checked against the rows native
ilasm writes for it and against its local signature;
    - per-scope ownership across methods;
- options: every switch that requests a PDB (`/DEBUG`, `/PDB`, and the
debug mode alone) gives the same rows and the same IL; `/OPTIMIZE`
offsets; `/DET` byte-identical PDBs; `/FOLD` with each folded method
keeping its own names.
- I broke each rule in turn and checked that a test fails. Of those
changes, the only ones no test notices are the PDB-only allocation
gates, which do not change any output.
- **Generated cases:** 200 seeded programs (196 of them with LocalScope
rows) with blocks nested up to four deep, explicit, padded and reused
slots, unnamed locals, and shadowed and duplicated names. The theories
compare the local signature, every named reference and the
LocalScope/LocalVariable rows (including their count) against a model of
native ilasm's rules, and check that the table is sorted and that each
method's scopes nest or are disjoint. That is 5 theories, 1,000 rows.
- **Comparison with native ilasm, measured outside the tree:** I ran the
runtime's PortablePdb cases (`src/tests/ilasm/PortablePdb`) against both
ilasms on Linux x64, with the four `TestPortablePdbLocalScope` cases
also run against the managed ilasm, and a fifth local-scope case for a
slot gap, a reused slot, an unnamed local, `.try` and `catch` bodies and
a shadowed name. Before this change the managed ilasm failed all five;
after it, both ilasms pass all 14 `IlasmPortablePdbTester` cases, so the
managed ilasm passes the original 10 where it passed 6. The fifth case
is now the `LocalScopeTests` fact
`SlotGapReusedSlotAndShadowedNameInTryAndCatchBodies_GiveTheRowsOfNativeIlasm`.
This PR changes nothing under `src/tests/ilasm`.

> [!NOTE]
> This change was prepared with AI assistance (Anthropic Claude and
OpenAI Codex) under my direction. AI agents wrote the code and ran the
build and tests on my machine. I reviewed the change before posting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-ILTools-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants