Skip to content

Fix ilasm PdbChecksum and the PDB ID under -DET - #135212

Open
pcshrosbree wants to merge 1 commit into
dotnet:mainfrom
pcshrosbree:fix/ilasm-pdb-checksum
Open

pcshrosbree wants to merge 1 commit into
dotnet:mainfrom
pcshrosbree:fix/ilasm-pdb-checksum

Conversation

@pcshrosbree

Copy link
Copy Markdown
Contributor

ilasm's PdbChecksum debug directory entry hashes only the #Pdb stream, whereas PE-COFF.md requires a hash of the whole PDB file with its 20-byte ID zeroed, so the checksum never verifies. With -DET, the PDB ID's stamp also stays 0 while the CodeView entry carries the deterministic stamp, so PEReader.TryOpenAssociatedPortablePdb rejects the pair.

Change

  • PortablePdbWriter::ComputeSha256PdbChecksum replaces ComputeSha256PdbStreamChecksum. It zeroes the PDB ID in the #Pdb heap, serializes the PDB with GetSaveSize and SaveToMemory, hashes the result and restores the ID. At that point the PDB is final apart from its ID, and Assembler::SavePdbFile later saves it through the same emitter.
  • IILAsmPortablePdbWriter::ChangePdbStreamGuid becomes ChangePdbStreamId(guid, stamp). It is backed by PdbHeap::SetDataId, which writes all 20 bytes with the same big-endian handling as PdbHeap::SetData. -DET uses it to write both fields.
  • The now-unused ComputeSha256PdbStreamChecksum and PdbHeap::ComputeSha256Checksum are removed.
  • TestPortablePdbDebugDirectory now runs with and without -DET. It compares all 20 bytes of the PDB ID with the CodeView entry (before, it compared only the GUID), and checks the PdbChecksum entry against SHA-256 of the PDB file with its ID zeroed.

Notes

  • The output changes only where it was wrong. Assembling the same IL with the base and the fixed ilasm in one directory, a -DET PDB differs only in its 4 stamp bytes, and the DLL only in its 32 checksum bytes. So -DET output is not byte-identical to an older ilasm's.
  • The PDB is now serialized twice. On a 552 KB PDB (2,500 methods, 200 documents), the difference was within noise: a median of 48 ms against 49 ms per ilasm run, over 5 runs each.
  • The IILAsmPortablePdbWriter vtable changes but its IID does not. ilasm is the interface's only consumer, and it links mdcompiler_ppdb statically. I can give the interface a new IID if you prefer.

Validation (Linux x64)

ilasm_tests against a Checked Core_Root, with counts taken from ilasm_tests.testResults.xml:

ilasm Result
base 4 of 15 fail: the two non--DET cases on the checksum, and the two -DET cases on the stamp
fixed 15 of 15 pass

Each new assertion was also checked to fail with the corresponding part of the fix reverted.

Nine IL inputs were assembled with and without -DET, covering no sequence points, no methods, several documents with an entry point, a 552 KB PDB, /DEBUG=OPT, /DEBUG=IMPL and /PDB. On every output:

  • the checksum matches only the PE-COFF.md rule;
  • the PDB ID matches the CodeView entry;
  • TryOpenAssociatedPortablePdb succeeds.

Two -DET runs in the same directory give identical bytes.

Not run: Windows, macOS, Arm64 or a big-endian target (the BIGENDIAN branch of SetDataId is compiled out on x64), and the full src/tests suite.

Resolves #135211

Note

This change, its validation and this description were prepared with AI assistance (Anthropic Claude and OpenAI Codex) under my direction. AI agents ran the builds, tests and probes on my machine. I reviewed the diff, the results and this text before posting.

ilasm computed the PdbChecksum debug directory entry over the #Pdb stream
only, while PE-COFF.md requires the hash of the entire PDB file with its
20-byte PDB ID zeroed. Under -DET it also wrote only the deterministic GUID
into the #Pdb stream, so the PDB ID's timestamp stayed 0 while the CodeView
entry carried the deterministic stamp, and
PEReader.TryOpenAssociatedPortablePdb rejected the pair.

Compute the checksum by serializing the PDB with its ID temporarily zeroed,
and write both the GUID and the timestamp into the #Pdb stream.

Extend TestPortablePdbDebugDirectory to compare all 20 bytes of the PDB ID
with the CodeView entry and to verify the PdbChecksum entry, with and without
-DET.
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 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

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.

@jkotas

jkotas commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

@pcshrosbree ilasm is being rewritten in C#. C/C++ implementation of ilasm is going to be deleted soon.

You may want to check whether the issues you are running into are fixed in the C# rewrite https://github.com/dotnet/runtime/tree/main/src/tools/ilasm , and if not - fix them there instead.

@pcshrosbree

Copy link
Copy Markdown
Contributor Author

Thanks for the pointer. Here is how the managed ilasm at main (b847b8718d0, Release, Linux x64) handles both issues, with the inputs from #135214:

A few PDB differences from native ilasm I'd be happy to follow up on, if they're unintended:

  • -DEBUG and -PDB embed the PDB, where native ilasm writes a separate file (Min.pdb for Min.dll). The tests expect embedding, but the -PDB help text still reads "Create the PDB file without enabling debug info tracking". .line directives also produce an embedded PDB without either switch; native ilasm needs -DEBUG or -PDB.
  • The CodeView entry always names assembly.pdb. Should it use the output's name, such as Min.pdb for Min.dll, for tools that look up a separate or extracted PDB?

Would the native fixes be useful for servicing branches that still ship native ilasm? If not, I'll close this PR and #135227, and leave #135211 and #135214 for you to dispose of.

Note

This check and this reply were prepared with AI assistance (Anthropic Claude and OpenAI Codex) under my direction. AI agents ran the build and the checks on my machine. I reviewed the text before posting.

@jkotas

jkotas commented Oct 5, 2026

Copy link
Copy Markdown
Member

A few PDB differences from native ilasm I'd be happy to follow up on, if they're unintended:

@jkoritzinsky Could you please comment on this?

@jkoritzinsky

Copy link
Copy Markdown
Member

Those changes are definitely unintended and should be fixed in managed ilasm.

@pcshrosbree

Copy link
Copy Markdown
Contributor Author

Thanks @jkoritzinsky. I'm working on the follow-ups for the three PDB differences in managed ilasm:

  • -DEBUG and -PDB write the PDB as a separate file beside the output (Min.pdb for Min.dll), with a PdbChecksum entry alongside the CodeView entry, instead of embedding it;
  • the CodeView entry names that file rather than assembly.pdb;
  • .line directives alone no longer produce a PDB; one is written only with -DEBUG or -PDB, as in native ilasm.

PR to follow against main, with tests and a note in the ilasm README describing the behaviour. I'll leave this PR and #135227 open until you say whether the native fixes are wanted for servicing.

@am11

am11 commented Oct 6, 2026

Copy link
Copy Markdown
Member

for servicing

Our bar for servicing typically depends on whether a bug is a regression from a previous release. Because this is a long-standing behavior, and we are currently rewriting the managed ilasm (followed by ildasm), it would be best to combine our efforts toward completing those rewrites so they can serve as a drop-in replacement for the native implementation before old code is deleted.

@pcshrosbree

Copy link
Copy Markdown
Contributor Author

Thanks @am11. To make sure I read this right: should I close this PR and #135227, and leave #135211 and #135214 to you?

Some context on why I opened them: my current project is hit by both defects in the native ilasm that ships for .NET 8 and .NET 10 (a PdbChecksum entry that does not verify, with a CodeView id that does not match the PDB's, and -DET giving different inputs the same identity), which is what prompted the PR. Is the managed ilasm likely to replace the native one in the .NET 8 or .NET 10 packages in the next few weeks, or should I plan to work around these issues in my project for those releases?

The managed-ilasm PR for the three PDB differences above is in review on my side and will go up shortly.

@pcshrosbree

Copy link
Copy Markdown
Contributor Author

@jkoritzinsky, the managed ilasm fix is up in #135289. With -DEBUG or -PDB the PDB is now written to <output>.pdb beside the output, and .line alone no longer produces one. The CodeView entry names the PDB's full path, as in native ilasm, so a -DET image depends on its output directory. It is followed by PdbChecksum and, with -DET, Reproducible. The PR also adjusts the IL round-trip harness, whose PDB determinism step relied on the PDB overwriting the image.

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.

@pcshrosbree

Copy link
Copy Markdown
Contributor Author

For anyone following the managed ilasm's PDB output from here: the differences from native ilasm that remain after #135289, #135297, #135311 and #135312 are listed in #135314, grouped so they can be picked up one at a time. Nothing beyond those PRs is planned unless a maintainer names it there.

jkoritzinsky pushed a commit that referenced this pull request Oct 10, 2026
…135289)

In #135212, @jkotas [pointed
out](#135212 (comment))
that native ilasm is being replaced by the C# rewrite in
`src/tools/ilasm` and suggested fixing issues there. I
[listed](#135212 (comment))
three PDB differences between the managed and native ilasm, and
@jkoritzinsky
[confirmed](#135212 (comment))
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 #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.
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.

This branch has not been deployed

No deployments
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.

ilasm: PdbChecksum hashes only the #Pdb stream, and -DET leaves the PDB ID's stamp at 0

4 participants