Skip to content

Fix #2823 and add a PDB verification mode to nugetfuzz - #4124

Merged
siegfriedpammer merged 2 commits into
masterfrom
fix/2823-async-catch-handler
Sep 9, 2026
Merged

Fix #2823 and add a PDB verification mode to nugetfuzz#4124
siegfriedpammer merged 2 commits into
masterfrom
fix/2823-async-catch-handler

Conversation

@siegfriedpammer

@siegfriedpammer siegfriedpammer commented Sep 8, 2026

Copy link
Copy Markdown
Member

Fixes #2823

Problem

The async stepping blob's first field is the compiler-generated catch handler's IL offset plus one, or 0 when there is nothing to record. ILSpy wrote the raw offset. A consumer decodes it as value - 1 and resolves that to an instruction: Mono.Cecil gets null and throws ArgumentNullException while reading the body. That is the crash in #2823 - it fires on every MoveNext of an async state machine, which is why the assembly opens fine in ILSpy but kills ILLink.

Nothing in the repo would have caught it. PdbGenerationTestRunner compares sequence points on hand-written fixtures and never looks at the custom debug information blobs, and no tool ever generated a PDB for a real assembly and asked whether a consumer can read it.

Solution

Two commits: the check that finds this class of defect, and the fix.

The fix. AsyncDebugInfo.CatchHandlerOffset now holds the offset it is named after, or -1 for "none", and BuildBlob applies the bias - the model the compiler uses. The handler is recorded only where an escaping exception is unlikely to be observed by anyone: an async void method, and an async entry point.

Recording it more widely would be worse than recording it nowhere: an async Task method returns its exception through the Task, so the debugger would break on exceptions user code catches. Measured across csc 1.3.2 to 5.10, one program per compiler:

csc async void normal async Task async Task Main
1.3.2 0x88 0x00 (no async Main in C# 6)
2.10.0 0x80 0x00 0x00
3.11.0 0x80 0x00 0x80
4.14.0 0x80 0x00 0x80
5.10.0 0x80 0x00 0x80

The entry point token names the synchronous <Main> shim that exists because the runtime will not take .entrypoint on an async method, so the method to record is the one that shim calls. IsAsyncEntryPoint reads that call rather than matching the shim's siblings by name - name matching gets a Main overload beside the real entry point wrong, and that case is in the fixture.

IsCompilerGeneratedMainMethod is unchanged.

The tests compare against the C# compiler's own PDB. PdbGenerationTestRunner generates ILSpy's PDB for the compiler's dll, so both blobs describe the same IL and the field compares directly - a same-assembly oracle that was already in the fixture, just never used for anything but sequence points. AsyncSteppingCatchHandler covers a library, AsyncSteppingEntryPoint an executable with all four shapes; Tester.CompileCSharpWithPdb now honours CompilerOptions.Library, which it had been ignoring, so a fixture can ask for an entry point.

nugetfuzz --pdb replaces the type-by-type sweep with a PDB check, reusing that tool's corpus, download and reference-pack machinery. Mono.Cecil has to read every method body through the generated PDB, and a lint checks what the PDB claims against the assembly: IL offsets on instruction boundaries and inside the method, sequence points increasing and pointing at real text in the embedded source, local slots within the local signature, scopes nested, async stepping decoding to a real catch handler, the hoisted-local scope table reaching the highest slot the state machine's field names declare, a single-rooted import scope table. --pdb-lint runs the same checks against a PDB that already exists - the calibration gate, since a lint is only worth its findings if it is silent on correct input.

Verification

--pdb-lint over 198 compiler-written PDBs, plus an async-Main and a top-level executable 0 findings
--pdb Microsoft.Extensions.Http before / after the fix ArgumentNullException … InstructionOffset..ctor + 3 async findings / none
generated vs compiler blob for async Main, a top-level program, and a Main overload identical
each new test with the code it covers disabled red
ICSharpCode.Decompiler.Tests 3606 tests, 0 failed, 45 skipped

The other findings this turned up are pre-existing and filed separately rather than addressed here: #4120, #4121, #4122, #4123.

  • At least one test covering the code changed

🤖 Generated with Claude Code

@siegfriedpammer
siegfriedpammer force-pushed the fix/2823-async-catch-handler branch from 271a9d5 to 4c4c860 Compare September 8, 2026 16:42
The PDB writer had no coverage beyond hand-written fixtures whose sequence
points are compared to the compiler's, and nothing ever asked whether a
consumer can read what it emits. Issue #2823 is the consequence: a PDB that
loads fine in ILSpy kills ILLink, and it took a reporter's own tool to find
out.

--pdb reuses the corpus, download and reference-pack machinery already in
nugetfuzz and replaces the type sweep with two checks: Mono.Cecil - the
consumer ILLink uses - has to read every method body through the generated
PDB, and a lint has to find everything the PDB claims true of the assembly.

--pdb-lint exists because a lint is only worth its findings if it is silent
on correct input. It runs the same checks against a PDB the compiler wrote,
and three of the checks written here were wrong until it said so - including
the async one, which flagged the compiler's own PDB for any executable with
an async Main, a case 190 nuget packages could not contain because a library
has no entry point.

Assisted-by: Claude:claude-opus-5:Claude Code
@siegfriedpammer
siegfriedpammer force-pushed the fix/2823-async-catch-handler branch from 4c4c860 to c7cf38e Compare September 8, 2026 18:12
…d it

The async stepping blob's first field is the compiler-generated catch
handler's IL offset plus one, and 0 when there is nothing to record. ILSpy
wrote the raw offset, so a consumer decoding it as (value - 1) resolved an
address in the middle of an instruction: Mono.Cecil throws
ArgumentNullException while reading the body, which is why the reported
assembly opened in ILSpy but killed ILLink on every MoveNext it had.
CatchHandlerOffset now holds the offset it is named after, or -1 for "none",
and BuildBlob applies the bias - the same model the compiler uses.

It is recorded only where an escaping exception is unlikely to be observed:
an async void method, and an async entry point. Recording it more widely
would be worse than recording it nowhere, because an async Task method
returns its exception through the Task and the debugger would then break on
exceptions user code catches. Measured over csc 1.3.2 to 5.10: async void is
handler+1 and a normal async Task is 0 in every version; only async Main
changed, from 0 to handler+1 between 2.10 and 3.11.

The entry point token names the synchronous '<Main>' shim that exists because
the runtime will not take .entrypoint on an async method, so the method to
record is the one the shim calls. Reading that call is exact; matching the
shim's siblings by name is not, and gets a 'Main' overload beside the real
entry point wrong.

Both tests compare against the compiler's own PDB for the same assembly, so
the blobs describe the same IL and the field compares directly.

Assisted-by: Claude:claude-opus-5:Claude Code
@siegfriedpammer
siegfriedpammer force-pushed the fix/2823-async-catch-handler branch from c7cf38e to 4e2f281 Compare September 9, 2026 07:14
@siegfriedpammer
siegfriedpammer merged commit 2e9d137 into master Sep 9, 2026
15 checks passed
@christophwille
christophwille deleted the fix/2823-async-catch-handler branch September 9, 2026 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ilspycmd] Broken PDB generated for MoveNext when using --genpdb

1 participant