From 4f3c6e4d4b68150a1b6adfc53321a041c8cf578b Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 8 Sep 2026 20:11:39 +0200 Subject: [PATCH 1/2] Check generated PDBs against real assemblies with nugetfuzz --pdb 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 --- TestTools/README.md | 38 +++ TestTools/nugetfuzz.cs | 719 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 724 insertions(+), 33 deletions(-) diff --git a/TestTools/README.md b/TestTools/README.md index 5e2d5878d..723a2d834 100644 --- a/TestTools/README.md +++ b/TestTools/README.md @@ -6,6 +6,7 @@ in-repo test suite cannot: it decompiles fixtures we wrote, these decompile what | tool | question it answers | |---|---| | `nugetfuzz.cs` | Does the decompiler *crash* on real code? (asserts, exceptions, IL warnings) | +| `nugetfuzz.cs --pdb` | Is the *PDB* we generate for real code well-formed, and can a consumer read it? | | `decompdiff.cs` | Did a change make the *output* better or worse? (readability across two builds) | | `nuget-top.ps1` | Where do I get a corpus? (downloads the most-downloaded packages) | @@ -40,6 +41,43 @@ Environment variables: `NUGETFUZZ_VERBOSE` (per-type progress), `NUGETFUZZ_DUMP= decompiled C#), `NUGETFUZZ_LEDGER=` (append findings as JSONL instead of writing a per-run HTML report), `NUGETFUZZ_HTML=` (report path), `NUGET_PACKAGES` (package cache). +### Checking generated PDBs + +`--pdb` swaps the type-by-type sweep for a different question: it generates a portable PDB for +each assembly with `PortablePdbWriter` and checks it two ways. First Mono.Cecil - the consumer +ILLink uses, and the one that crashed in #2823 - has to read every method body *through* the PDB, +which is what makes it decode the custom debug information. Then a structural lint over the PDB +metadata checks that what it says is true of the assembly: IL offsets land on instruction +boundaries and inside the method, sequence points increase and point at real text in the embedded +source, local slots exist in the local signature, scopes nest, async stepping information decodes +to a real catch handler of a method whose kickoff shape allows one, the hoisted-local scope table +reaches the highest slot the state machine's field names declare, and the import scope table has +a single root. Findings are reported as one `PDB` kind with a bracketed category in the message. + +```pwsh +dotnet run nugetfuzz.cs -- --pdb Microsoft.Extensions.Http +dotnet run nugetfuzz.cs -- --pdb @crawl/top-200.corpus.txt +``` + +Generating a whole assembly's PDB costs far more than decompiling its types, so `--pdb` is for a +curated corpus, not for the catalog sweep. Assemblies without a CodeView debug directory entry +are skipped: the writer takes the PDB id from that entry and a consumer rejects a PDB whose id +does not match it, the same reason `ilspycmd -genpdb` refuses them. + +The `Debug.Assert` in the writer that fires on real-world input would unwind out of `WritePdb` and +leave nothing to check, so for the duration of that call the assert listener records instead of +throwing. The assertion is still reported; the PDB it produced is still checked. + +`--pdb-lint` runs the same two checks against the PDB an assembly already ships with - beside it +as a `.pdb`, or embedded in the PE. This is how the lint is calibrated, and it is the first thing +to run after touching a check: a PDB the C# compiler wrote must produce **no** findings at all, so +anything reported there is a defect in the lint rather than in ILSpy. + +```pwsh +dotnet run nugetfuzz.cs -- --pdb-lint ../ICSharpCode.Decompiler/bin/Debug/netstandard2.0/ICSharpCode.Decompiler.dll +dotnet run nugetfuzz.cs -- --pdb-lint ~/.cache/nugetfuzz # every dll that ships symbols +``` + ### Sweeping the whole catalog `nugetfuzz-all.ps1` walks the nuget.org catalog and runs `nugetfuzz.cs` on every package id it diff --git a/TestTools/nugetfuzz.cs b/TestTools/nugetfuzz.cs index 68e92f4ad..d075c1382 100644 --- a/TestTools/nugetfuzz.cs +++ b/TestTools/nugetfuzz.cs @@ -18,6 +18,7 @@ #:project ../ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj #:package NuGet.Packaging@* +#:package Mono.Cecil@0.11.6 #:property PublishAot=false // nugetfuzz: downloads nuget packages (sequentially), resolves their dependency @@ -25,10 +26,20 @@ // Microsoft.NETFramework.ReferenceAssemblies packages), then decompiles every // assembly type-by-type and reports Debug.Assert failures / exceptions. // -// usage: dotnet run nugetfuzz.cs -- [--download-only] ... | @packagelist.txt +// With --pdb it instead generates a portable PDB for each assembly and checks it: Mono.Cecil +// (the consumer ILLink uses) has to be able to read every method body through it, and the PDB +// metadata has to satisfy a structural lint. --pdb-lint runs the same checks against a PDB that +// already exists next to the assembly, which is how the lint is calibrated: it must report +// nothing at all for a PDB the C# compiler wrote. +// +// usage: dotnet run nugetfuzz.cs -- [--download-only|--pdb] ... | @packagelist.txt +// dotnet run nugetfuzz.cs -- --pdb-lint ... | @corpus.txt using System.Diagnostics; using System.IO.Compression; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using System.Reflection.PortableExecutable; using System.Net.Http.Json; using System.Text; using System.Text.Json; @@ -36,8 +47,14 @@ using System.Text.RegularExpressions; using ICSharpCode.Decompiler; using ICSharpCode.Decompiler.CSharp; +using ICSharpCode.Decompiler.DebugInfo; using ICSharpCode.Decompiler.Metadata; +// Cecil is used through aliases: its MethodDefinition/SequencePoint/MethodBody names collide with +// System.Reflection.Metadata's and the decompiler's, and every one of the three is used here. +using Cecil = Mono.Cecil; +using CecilCil = Mono.Cecil.Cil; + using NuGet.Frameworks; using NuGet.Packaging; using NuGet.Versioning; @@ -64,7 +81,7 @@ var installedTfm = NuGetFramework.Parse($"net{Environment.Version.Major}.{Enviro var net48 = NuGetFramework.Parse("net48"); var reducer = new FrameworkReducer(); var failures = new Dictionary(); -int assemblyCount = 0, typeCount = 0; +int assemblyCount = 0, typeCount = 0, pdbChecked = 0, pdbSkipped = 0; long charCount = 0, refsResolved = 0, refsTotal = 0; bool verbose = Environment.GetEnvironmentVariable("NUGETFUZZ_VERBOSE") != null; var dumpDir = Environment.GetEnvironmentVariable("NUGETFUZZ_DUMP"); @@ -111,40 +128,58 @@ if (args is ["--report", var ledgerPath, ..]) // Populates the cache without decompiling: the sweep is the slow part, and a corpus // only needs the assemblies on disk. var downloadOnly = args.Contains("--download-only"); -var packages = args - .Where(a => a != "--download-only") +// Generates a PDB per assembly and checks it, instead of decompiling type by type. The two are +// alternatives rather than additions: a whole-assembly PDB is far more expensive than the type +// sweep, and they answer different questions. +var pdbMode = args.Contains("--pdb"); +// Checks the PDB the assembly already ships with. Calibration, not a sweep. +var pdbLint = args.Contains("--pdb-lint"); +var arguments = args + .Where(a => !a.StartsWith("--")) .SelectMany(a => a.StartsWith('@') ? File.ReadAllLines(a[1..]) : new[] { a }) .Select(l => l.Trim()) .Where(l => l.Length > 0 && !l.StartsWith('#')) .ToList(); -if (packages.Count == 0) +if (arguments.Count == 0) { - Console.Error.WriteLine("usage: nugetfuzz [--download-only] ... | @packagelist.txt"); + Console.Error.WriteLine("usage: nugetfuzz [--download-only|--pdb] ... | @packagelist.txt"); + Console.Error.WriteLine(" nugetfuzz --pdb-lint ... | @corpus.txt"); Console.Error.WriteLine(" nugetfuzz --report [out.html]"); return 1; } -foreach (var spec in packages) +if (pdbLint) { - try - { - await ProcessPackage(spec); - } - catch (Exception ex) when ( - ex is InvalidOperationException && ex.Message.Contains("not found") - || ex is InvalidDataException) - { - // Deleted/delisted package or corrupt nupkg on nuget.org - not a decompiler issue. - Console.WriteLine($" skip {spec}: {ex.Message}"); - } - catch (Exception ex) + foreach (var dll in ExpandDlls(arguments)) + LintExistingPdb(dll); +} +else +{ + foreach (var spec in arguments) { - Report(spec, "-", "-", ex); + try + { + await ProcessPackage(spec); + } + catch (Exception ex) when ( + ex is InvalidOperationException && ex.Message.Contains("not found") + || ex is InvalidDataException) + { + // Deleted/delisted package or corrupt nupkg on nuget.org - not a decompiler issue. + Console.WriteLine($" skip {spec}: {ex.Message}"); + } + catch (Exception ex) + { + Report(spec, "-", "-", ex); + } } } Console.WriteLine(); -Console.WriteLine($"=== {assemblyCount} assemblies, {typeCount} types decompiled ({charCount} chars), {refsResolved}/{refsTotal} refs resolved, {failures.Count} distinct failures ({failures.Values.Sum(f => f.Count)} total) ==="); +if (pdbMode || pdbLint) + Console.WriteLine($"=== {assemblyCount} assemblies, {pdbChecked} PDBs checked, {pdbSkipped} skipped, {failures.Count} distinct failures ({failures.Values.Sum(f => f.Count)} total) ==="); +else + Console.WriteLine($"=== {assemblyCount} assemblies, {typeCount} types decompiled ({charCount} chars), {refsResolved}/{refsTotal} refs resolved, {failures.Count} distinct failures ({failures.Values.Sum(f => f.Count)} total) ==="); foreach (var entry in failures.Values.OrderByDescending(f => f.Count)) Console.WriteLine($"{entry.Count,6}x {entry.Describe()}"); // A sweep runs this program once per package, so per-run findings are appended to a @@ -579,6 +614,12 @@ async Task DecompileAssembly(string pkg, string dllPath, List searchDirs } Console.WriteLine($" {name}"); assemblyCount++; + if (pdbMode) + { + CheckGeneratedPdb(pkg, name, module, decompiler, dllPath, orderedDirs); + ReportResolutions(logResolver); + return; + } foreach (var type in decompiler.TypeSystem.MainModule.TopLevelTypeDefinitions.ToList()) { using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60)); @@ -613,24 +654,604 @@ async Task DecompileAssembly(string pkg, string dllPath, List searchDirs Report(pkg, name, type.FullTypeName.ToString(), ex); } } - var resolutions = logResolver.Resolutions; - var unresolved = resolutions.Where(kv => kv.Value == null).Select(kv => kv.Key).OrderBy(k => k).ToList(); - refsTotal += resolutions.Count; - refsResolved += resolutions.Count - unresolved.Count; - Console.WriteLine($" refs: {resolutions.Count - unresolved.Count}/{resolutions.Count} resolved"); - if (verbose) + ReportResolutions(logResolver); + } +} + +void ReportResolutions(LoggingResolver logResolver) +{ + var resolutions = logResolver.Resolutions; + var unresolved = resolutions.Where(kv => kv.Value == null).Select(kv => kv.Key).OrderBy(k => k).ToList(); + refsTotal += resolutions.Count; + refsResolved += resolutions.Count - unresolved.Count; + Console.WriteLine($" refs: {resolutions.Count - unresolved.Count}/{resolutions.Count} resolved"); + if (verbose) + { + foreach (var kv in resolutions.OrderBy(kv => kv.Key)) + Console.WriteLine($" {kv.Key} -> {kv.Value ?? "NOT FOUND"}"); + } + else + { + foreach (var u in unresolved) + Console.WriteLine($" ! unresolved: {u}"); + } +} + +// --------------------------------------------------------------------------------------------- +// PDB verification. Two independent checks over the same PDB: the Cecil round-trip answers "can +// the consumer ILLink uses read this at all", which is how #2823 surfaced; the lint answers "is +// what it reads true of the assembly", which a crash-free but wrong PDB still fails. +// --------------------------------------------------------------------------------------------- + +void CheckGeneratedPdb(string pkg, string asm, PEFile module, CSharpDecompiler decompiler, + string dllPath, List searchDirs) +{ + // The writer takes the PDB id from the PE's CodeView debug directory entry, and a consumer + // rejects a PDB whose id does not match that entry, so without one there is nothing to check + // - the same reason ilspycmd refuses these assemblies. + if (!PortablePdbWriter.HasCodeViewDebugDirectoryEntry(module)) + { + Console.WriteLine(" skip: no CodeView debug directory entry"); + pdbSkipped++; + return; + } + var pdbStream = new MemoryStream(); + var asserts = new List(); + using (var cts = new CancellationTokenSource(TimeSpan.FromMinutes(10))) + { + decompiler.CancellationToken = cts.Token; + Trace.Listeners.Clear(); + Trace.Listeners.Add(new CollectAsserts(asserts)); + try + { + new PortablePdbWriter { NoLogo = true } + .WritePdb(module, decompiler, new DecompilerSettings(), pdbStream); + } + catch (OperationCanceledException) + { + Report(pkg, asm, "", new TimeoutException("PDB generation timed out (10min)")); + return; + } + catch (Exception ex) { - foreach (var kv in resolutions.OrderBy(kv => kv.Key)) - Console.WriteLine($" {kv.Key} -> {kv.Value ?? "NOT FOUND"}"); + Report(pkg, asm, "", ex); + return; } - else + finally { - foreach (var u in unresolved) - Console.WriteLine($" ! unresolved: {u}"); + Trace.Listeners.Clear(); + Trace.Listeners.Add(new ThrowOnAssert()); } } + // Metadata tokens in an assertion message vary per method; without normalising them one + // defect would fill the ledger with a finding per method it fired on. + foreach (var message in asserts.Select(m => Regex.Replace(m, @"\b[0-9A-Fa-f]{8}\b", "")).Distinct()) + Report(pkg, asm, "", new AssertionFailedException(message)); + + pdbChecked++; + var bodies = CollectBodyFacts(dllPath, searchDirs); + CecilRoundTrip(pkg, asm, dllPath, pdbStream, searchDirs); + pdbStream.Position = 0; + using var provider = MetadataReaderProvider.FromPortablePdbStream(pdbStream); + LintPdb(pkg, asm, module.Metadata, provider.GetMetadataReader(), bodies, EntryPoint(module.Reader)); } +// Runs the same two checks against the PDB an assembly already ships with. This is how the lint +// is calibrated: a PDB the C# compiler wrote must produce no findings at all, so anything +// reported here is a defect in the lint rather than in ILSpy. +void LintExistingPdb(string dllPath) +{ + var asm = Path.GetFileName(dllPath); + using var peStream = File.OpenRead(dllPath); + PEReader peReader; + try + { + peReader = new PEReader(peStream); + if (!peReader.HasMetadata) + return; + } + catch (BadImageFormatException) + { + return; + } + using (peReader) + { + // A PDB next to the assembly if there is one, otherwise the one embedded in the PE. + MemoryStream? pdbStream = null; + MetadataReaderProvider? provider = null; + var pdbPath = Path.ChangeExtension(dllPath, ".pdb"); + try + { + if (File.Exists(pdbPath)) + { + var bytes = File.ReadAllBytes(pdbPath); + // "BSJB": a Windows PDB is a different format the portable reader cannot open. + if (bytes.Length < 4 || BitConverter.ToUInt32(bytes, 0) != 0x424A5342) + { + pdbSkipped++; + return; + } + pdbStream = new MemoryStream(bytes); + provider = MetadataReaderProvider.FromPortablePdbStream(pdbStream, MetadataStreamOptions.LeaveOpen); + } + else + { + var embedded = peReader.ReadDebugDirectory() + .FirstOrDefault(e => e.Type == DebugDirectoryEntryType.EmbeddedPortablePdb); + if (embedded.Type != DebugDirectoryEntryType.EmbeddedPortablePdb) + return; + provider = peReader.ReadEmbeddedPortablePdbDebugDirectoryData(embedded); + } + } + catch (Exception ex) when (ex is IOException or BadImageFormatException) + { + pdbSkipped++; + return; + } + using (provider) + using (pdbStream) + { + Console.WriteLine($" {asm}"); + assemblyCount++; + pdbChecked++; + var bodies = CollectBodyFacts(dllPath, null); + CecilRoundTrip("-", asm, dllPath, pdbStream, null); + LintPdb("-", asm, peReader.GetMetadataReader(), provider.GetMetadataReader(), bodies, + EntryPoint(peReader)); + } + } +} + +Cecil.ReaderParameters CecilParameters(List? searchDirs, Stream? symbols, bool readSymbols) +{ + var resolver = new Cecil.DefaultAssemblyResolver(); + foreach (var dir in searchDirs ?? []) + resolver.AddSearchDirectory(dir); + var parameters = new Cecil.ReaderParameters { AssemblyResolver = resolver }; + if (!readSymbols) + return parameters; + parameters.ReadSymbols = true; + if (symbols != null) + { + symbols.Position = 0; + parameters.SymbolReaderProvider = new CecilCil.PortablePdbReaderProvider(); + parameters.SymbolStream = symbols; + } + else + { + parameters.SymbolReaderProvider = new CecilCil.EmbeddedPortablePdbReaderProvider(); + } + return parameters; +} + +// Parses every method body without symbols, so the lint has the assembly's own truth about +// instruction boundaries, exception handlers and local counts to check the PDB against - and +// still has it for the very methods whose debug information breaks the round-trip below. +Dictionary CollectBodyFacts(string dllPath, List? searchDirs) +{ + var facts = new Dictionary(); + try + { + using var assembly = Cecil.AssemblyDefinition.ReadAssembly(dllPath, CecilParameters(searchDirs, null, readSymbols: false)); + foreach (var type in assembly.MainModule.GetTypes()) + { + foreach (var method in type.Methods) + { + if (!method.HasBody) + continue; + try + { + var body = method.Body; + facts[(int)method.MetadataToken.RID] = new BodyFacts( + body.CodeSize, + body.Instructions.Select(i => i.Offset).ToHashSet(), + body.ExceptionHandlers + .Where(h => h.HandlerType == CecilCil.ExceptionHandlerType.Catch) + .Select(h => h.HandlerStart.Offset).ToHashSet(), + body.Variables.Count); + } + catch (Exception) + { + // A body that will not parse without symbols says nothing about the PDB; the + // lint simply has no facts to check that one method against. + } + } + } + } + catch (Exception ex) + { + Console.WriteLine($" ! body scan failed: {ex.GetType().Name}: {FirstLine(ex.Message)}"); + } + return facts; +} + +// Reads every method body through the PDB, which is what makes Cecil decode the custom debug +// information attached to it. This is the path that fails in #2823 and inside ILLink. +void CecilRoundTrip(string pkg, string asm, string dllPath, Stream? pdbStream, List? searchDirs) +{ + Cecil.AssemblyDefinition assembly; + try + { + assembly = Cecil.AssemblyDefinition.ReadAssembly(dllPath, CecilParameters(searchDirs, pdbStream, readSymbols: true)); + } + catch (CecilCil.SymbolsNotMatchingException) + { + Report(pkg, asm, "", new PdbFinding( + "[MATCH] the PDB does not match the assembly's CodeView debug directory entry")); + return; + } + catch (Exception ex) + { + Report(pkg, asm, "", new PdbFinding( + $"[CECIL] {ex.GetType().Name} opening the assembly with the PDB: {FirstLine(ex.Message)}")); + return; + } + using (assembly) + { + foreach (var type in assembly.MainModule.GetTypes()) + { + foreach (var method in type.Methods) + { + if (!method.HasBody) + continue; + try + { + _ = method.Body; + _ = method.DebugInformation.SequencePoints; + } + catch (Cecil.AssemblyResolutionException) + { + // A reference the corpus does not contain, not a defect in the PDB. + } + catch (Exception ex) + { + // The Cecil frame separates distinct decode failures; the method name goes + // into the location, so the message stays the same for all of them. + var frame = (ex.StackTrace ?? "").Split('\n').Select(l => l.Trim()) + .FirstOrDefault(l => l.Contains("Mono.Cecil")) ?? ""; + Report(pkg, asm, method.FullName, new PdbFinding( + $"[CECIL] {ex.GetType().Name} reading a method body through the PDB @ {frame}")); + } + } + } + } +} + +// Structural lint: everything the PDB claims has to be true of the assembly it describes. The +// category is a prefix on the message, so one finding kind covers all of them in the report. +void LintPdb(string pkg, string asm, MetadataReader pe, MetadataReader pdb, Dictionary bodies, + MethodDefinitionHandle entryPoint) +{ + var sourceLines = new Dictionary(); + + string MethodName(int rid) + { + if (rid <= 0 || rid > pe.MethodDefinitions.Count) + return ""; + var method = pe.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle(rid)); + var type = pe.GetTypeDefinition(method.GetDeclaringType()); + var ns = pe.GetString(type.Namespace); + return $"{(ns.Length > 0 ? ns + "." : "")}{pe.GetString(type.Name)}.{pe.GetString(method.Name)}"; + } + + // One report per method per defect: a method with hundreds of sequence points would + // otherwise contribute hundreds of hits for a single mistake. + void Once(HashSet seen, int rid, string message) + { + if (seen.Add(message)) + Report(pkg, asm, MethodName(rid), new PdbFinding(message)); + } + + // The embedded source, as lines, for checking that sequence points point at real text. + string[]? EmbeddedLines(DocumentHandle handle) + { + int key = MetadataTokens.GetRowNumber(handle); + if (sourceLines.TryGetValue(key, out var cached)) + return cached; + string[]? lines = null; + foreach (var infoHandle in pdb.GetCustomDebugInformation(handle)) + { + var info = pdb.GetCustomDebugInformation(infoHandle); + if (pdb.GetGuid(info.Kind) != KnownGuids.EmbeddedSource) + continue; + var blob = pdb.GetBlobBytes(info.Value); + if (blob.Length < 4) + break; + // int32 uncompressed size, then the bytes - deflated when the size is non-zero. + int uncompressedSize = BitConverter.ToInt32(blob, 0); + using var raw = new MemoryStream(blob, 4, blob.Length - 4); + using Stream content = uncompressedSize > 0 + ? new DeflateStream(raw, CompressionMode.Decompress) + : raw; + using var reader = new StreamReader(content, Encoding.UTF8, detectEncodingFromByteOrderMarks: true); + lines = reader.ReadToEnd().Replace("\r\n", "\n").Split('\n'); + break; + } + sourceLines[key] = lines; + return lines; + } + + // The MethodDebugInformation table is parallel to MethodDef: a consumer indexes it by + // method row, so a short table silently shifts every method's debug info. + if (pdb.MethodDebugInformation.Count != pe.MethodDefinitions.Count) + { + Report(pkg, asm, "", new PdbFinding( + "[DOCS] the MethodDebugInformation row count differs from the MethodDef row count")); + } + + bool anySequencePoints = false; + foreach (var infoHandle in pdb.MethodDebugInformation) + { + var info = pdb.GetMethodDebugInformation(infoHandle); + if (info.SequencePointsBlob.IsNil) + continue; + int rid = MetadataTokens.GetRowNumber(infoHandle); + bodies.TryGetValue(rid, out var facts); + var seen = new HashSet(); + int previousOffset = -1; + foreach (var point in info.GetSequencePoints()) + { + anySequencePoints = true; + // Offsets are delta-encoded, so a non-increasing one is not just out of order: + // it aliases into the record that means "the document changed here". + if (point.Offset <= previousOffset) + Once(seen, rid, "[SEQPOINT] sequence point offsets are not strictly increasing"); + previousOffset = point.Offset; + if (facts != null && !facts.InstructionOffsets.Contains(point.Offset)) + Once(seen, rid, "[OFFSET] a sequence point offset is not an instruction boundary"); + if (point.IsHidden) + continue; + if (point.StartLine > point.EndLine + || (point.StartLine == point.EndLine && point.StartColumn >= point.EndColumn)) + { + Once(seen, rid, "[SEQPOINT] a sequence point span is empty or inverted"); + } + var lines = EmbeddedLines(point.Document); + if (lines == null) + continue; + if (point.EndLine > lines.Length) + Once(seen, rid, "[SOURCE] a sequence point points past the end of the embedded source"); + else if (SpanIsBlank(lines, point.StartLine, point.StartColumn, point.EndLine, point.EndColumn)) + Once(seen, rid, "[SOURCE] a sequence point span covers only whitespace"); + } + } + if (anySequencePoints && pdb.Documents.Count == 0) + Report(pkg, asm, "", new PdbFinding("[DOCS] the PDB has sequence points but no documents")); + + // The table is sorted by method, then start offset, then descending length, so the + // enclosing scope of a row is always the innermost one still open. + int currentMethod = 0; + var open = new List<(int Start, int End)>(); + var scopeSeen = new HashSet(); + foreach (var scopeHandle in pdb.LocalScopes) + { + var scope = pdb.GetLocalScope(scopeHandle); + int rid = MetadataTokens.GetRowNumber(scope.Method); + if (rid != currentMethod) + { + currentMethod = rid; + open.Clear(); + scopeSeen.Clear(); + } + bodies.TryGetValue(rid, out var facts); + int start = scope.StartOffset, end = scope.EndOffset; + if (facts != null) + { + if (!facts.InstructionOffsets.Contains(start)) + Once(scopeSeen, rid, "[OFFSET] a local scope starts at an offset that is not an instruction boundary"); + if (end > facts.CodeSize) + Once(scopeSeen, rid, "[OFFSET] a local scope extends past the end of the method body"); + else if (end != facts.CodeSize && !facts.InstructionOffsets.Contains(end)) + Once(scopeSeen, rid, "[OFFSET] a local scope ends at an offset that is not an instruction boundary"); + } + while (open.Count > 0 && open[^1].End <= start) + open.RemoveAt(open.Count - 1); + if (open.Count > 0 && end > open[^1].End) + Once(scopeSeen, rid, "[LOCALS] a local scope is not nested inside its enclosing scope"); + open.Add((start, end)); + var slots = new HashSet(); + foreach (var variableHandle in scope.GetLocalVariables()) + { + var variable = pdb.GetLocalVariable(variableHandle); + if (facts != null && variable.Index >= facts.LocalCount) + Once(scopeSeen, rid, "[LOCALS] a local variable slot is past the end of the method's local signature"); + if (!slots.Add(variable.Index)) + Once(scopeSeen, rid, "[LOCALS] two local variables in one scope share a slot"); + } + } + + // The expression evaluator resolves unqualified names by walking a scope up to the single + // module-level root. A second root means some scopes hang off nothing, and everything + // under them resolves against an empty chain no matter what the other root holds. + // (An import scope carrying no imports at all is legal - a file need not have usings.) + int rootImportScopes = 0; + foreach (var importScopeHandle in pdb.ImportScopes) + { + if (pdb.GetImportScope(importScopeHandle).Parent.IsNil) + rootImportScopes++; + } + if (rootImportScopes > 1) + { + Report(pkg, asm, "", new PdbFinding( + "[IMPORTS] the import scope table has more than one root, so some scopes chain to nothing")); + } + + foreach (var infoHandle in pdb.CustomDebugInformation) + { + var info = pdb.GetCustomDebugInformation(infoHandle); + if (info.Parent.Kind != HandleKind.MethodDefinition) + continue; + int rid = MetadataTokens.GetRowNumber((MethodDefinitionHandle)info.Parent); + bodies.TryGetValue(rid, out var facts); + var kind = pdb.GetGuid(info.Kind); + var seen = new HashSet(); + if (kind == KnownGuids.MethodSteppingInformation) + CheckAsyncStepping(pdb.GetBlobReader(info.Value), rid, facts, seen); + else if (kind == KnownGuids.StateMachineHoistedLocalScopes) + CheckHoistedScopes(pdb.GetBlobReader(info.Value), rid, facts, seen); + } + + void CheckAsyncStepping(BlobReader reader, int rid, BodyFacts? facts, HashSet seen) + { + try + { + if (reader.RemainingBytes < 4) + { + Once(seen, rid, "[ASYNC] the async stepping blob is truncated"); + return; + } + long catchHandler = reader.ReadUInt32(); + if (catchHandler != 0) + { + // The field is the handler's offset plus one, and only for async void + // methods; 0 otherwise. A consumer decodes it as (value - 1), so a raw + // offset resolves to an address in the middle of an instruction. + var kickoff = pdb.GetMethodDebugInformation(MetadataTokens.MethodDebugInformationHandle(rid)) + .GetStateMachineKickoffMethod(); + // The compiler records a handler for an async void method and for an async entry + // point - both are shapes nothing is expected to await, so the debugger should treat + // the exception as user-unhandled - and an async entry point returns Task. + if (!kickoff.IsNil && !IsUserEntryPoint(pe, kickoff, entryPoint) && !ReturnsVoid(pe, kickoff)) + Once(seen, rid, "[ASYNC] async stepping information records a catch handler for a method whose kickoff method does not return void"); + int decoded = (int)catchHandler - 1; + if (facts != null && !facts.InstructionOffsets.Contains(decoded)) + Once(seen, rid, "[OFFSET] the async stepping catch handler does not decode to an instruction boundary"); + else if (facts != null && !facts.CatchHandlerStarts.Contains(decoded)) + Once(seen, rid, "[ASYNC] the async stepping catch handler does not decode to the start of a catch handler"); + } + while (reader.RemainingBytes > 0) + { + int yield = (int)reader.ReadUInt32(); + int resume = (int)reader.ReadUInt32(); + int resumeMethod = reader.ReadCompressedInteger(); + if (facts != null && !facts.InstructionOffsets.Contains(yield)) + Once(seen, rid, "[OFFSET] an async yield offset is not an instruction boundary"); + if (facts != null && !facts.InstructionOffsets.Contains(resume)) + Once(seen, rid, "[OFFSET] an async resume offset is not an instruction boundary"); + if (resumeMethod != rid) + Once(seen, rid, "[ASYNC] an async resume method is not the method carrying the stepping information"); + } + } + catch (BadImageFormatException) + { + Once(seen, rid, "[ASYNC] the async stepping blob is malformed"); + } + } + + void CheckHoistedScopes(BlobReader reader, int rid, BodyFacts? facts, HashSet seen) + { + int rows = 0; + while (reader.RemainingBytes >= 8) + { + int start = (int)reader.ReadUInt32(); + int length = (int)reader.ReadUInt32(); + rows++; + if (facts != null && start + length > facts.CodeSize) + Once(seen, rid, "[OFFSET] a state machine hoisted local scope extends past the end of the method body"); + } + // The debugger indexes this table by the slot number it parses out of the state + // machine's own field names, so rows missing at the end drop those locals entirely. + if (rows < HoistedSlotCount(pe, rid)) + Once(seen, rid, "[STATEMACHINE] the state machine hoisted local scope table has fewer rows than the state machine has hoisted slots"); + } +} + +// True when every character the span covers is whitespace - a sequence point the debugger would +// highlight as an empty stretch of the source it is embedded next to. +static bool SpanIsBlank(string[] lines, int startLine, int startColumn, int endLine, int endColumn) +{ + for (int line = startLine; line <= endLine && line <= lines.Length; line++) + { + var text = lines[line - 1]; + int from = line == startLine ? Math.Min(startColumn - 1, text.Length) : 0; + int to = line == endLine ? Math.Min(endColumn - 1, text.Length) : text.Length; + if (to > from && text[from..to].Trim().Length > 0) + return false; + } + return true; +} + +// A debugger finds a hoisted local by parsing the slot number N out of the state machine's own +// field names and reading row N-1 of the hoisted scope table, so the table has to reach the +// highest slot the debugger will ask for. Only user-visible hoisted fields are ever asked for: +// 5__N holds a hoisted local and <>8__N a hoisted display class, while <>s__N compiler +// temporaries and <>u__N awaiters share the same slot counter but are never looked up - which is +// why the compiler's own table stops at the last user-visible slot rather than at the last field. +static int HoistedSlotCount(MetadataReader pe, int methodRid) +{ + var method = pe.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle(methodRid)); + int max = 0; + foreach (var fieldHandle in pe.GetTypeDefinition(method.GetDeclaringType()).GetFields()) + { + var match = Regex.Match(pe.GetString(pe.GetFieldDefinition(fieldHandle).Name), @"^<.*>[58]__([0-9]+)$"); + if (match.Success) + max = Math.Max(max, int.Parse(match.Groups[1].Value)); + } + return max; +} + +// The assembly's entry point, or nil when there is none or it lives in another module of a +// multi-module assembly (where the token is a File token rather than a method definition). +static MethodDefinitionHandle EntryPoint(PEReader pe) +{ + int token = pe.PEHeaders.CorHeader?.EntryPointTokenOrRelativeVirtualAddress ?? 0; + return (token >> 24) == 0x06 && (token & 0xFFFFFF) != 0 + ? MetadataTokens.MethodDefinitionHandle(token & 0xFFFFFF) + : default; +} + +// Whether a state machine's kickoff is the entry point as the user wrote it. An async entry +// point compiles to the user's method plus a '
' wrapper that awaits it, and it is the +// wrapper the entry point token names, so the kickoff is recognised through it: same declaring +// type, and the only two names the compiler gives a user entry point. +static bool IsUserEntryPoint(MetadataReader pe, MethodDefinitionHandle kickoff, MethodDefinitionHandle entryPoint) +{ + if (entryPoint.IsNil) + return false; + if (kickoff == entryPoint) + return true; + var wrapper = pe.GetMethodDefinition(entryPoint); + if (pe.GetString(wrapper.Name) != "
") + return false; + var definition = pe.GetMethodDefinition(kickoff); + var name = pe.GetString(definition.Name); + return (name == "Main" || name == "
$") + && definition.GetDeclaringType() == wrapper.GetDeclaringType(); +} + +// Reads just the return type out of a method signature: enough to tell an async void kickoff +// method from an async Task one, without a type system or a resolved reference closure. +static bool ReturnsVoid(MetadataReader pe, MethodDefinitionHandle handle) +{ + var reader = pe.GetBlobReader(pe.GetMethodDefinition(handle).Signature); + var header = reader.ReadSignatureHeader(); + if (header.IsGeneric) + reader.ReadCompressedInteger(); + reader.ReadCompressedInteger(); + while (reader.RemainingBytes > 0) + { + byte element = reader.ReadByte(); + // CMOD_REQD / CMOD_OPT may precede the return type. + if (element is 0x1f or 0x20) + { + reader.ReadCompressedInteger(); + continue; + } + return element == 0x01; + } + return false; +} + +// The paths of a corpus argument: a dll, or a directory scanned recursively for dlls. +static IEnumerable ExpandDlls(IEnumerable entries) + => entries + .SelectMany(e => Directory.Exists(e) + ? Directory.EnumerateFiles(e, "*.dll", SearchOption.AllDirectories) + : [e]) + .Where(f => !f.EndsWith(".resources.dll", StringComparison.OrdinalIgnoreCase)) + .Distinct() + .OrderBy(f => f, StringComparer.Ordinal); + void Report(string pkg, string asm, string type, Exception ex) { // Key on the innermost exception so the same defect hit via many members dedupes. @@ -642,7 +1263,8 @@ void Report(string pkg, string asm, string type, Exception ex) .FirstOrDefault(l => l.Contains("ICSharpCode.Decompiler")) ?? ""; var kind = inner is AssertionFailedException ? "ASSERT" : inner is TimeoutException ? "TIMEOUT" - : inner is DecompilerWarning ? "WARNING" : "EXCEPTION"; + : inner is DecompilerWarning ? "WARNING" + : inner is PdbFinding ? "PDB" : "EXCEPTION"; var key = $"{kind}|{inner.GetType().Name}|{inner.Message}|{topFrame}"; var location = $"{pkg} / {asm} / {type}"; if (failures.TryGetValue(key, out var existing)) @@ -774,6 +1396,7 @@ static void WriteHtmlReport(string path, List findings, int assemblies, color:#a8791f; border:1px solid #a8791f55; margin-left:6px; } .ASSERT { border-left:4px solid #d97706; } .EXCEPTION { border-left:4px solid #dc2626; } .TIMEOUT { border-left:4px solid #7c3aed; } .WARNING { border-left:4px solid #2563eb; } + .PDB { border-left:4px solid #0d9488; } #filter { width:100%; padding:8px; margin:8px 0; border:1px solid var(--line); border-radius:6px; background:var(--bg); color:var(--fg); font:13px ui-monospace,monospace; } @@ -841,10 +1464,40 @@ record LedgerEntry(string Record, string Kind, string ExceptionType, string Mess record VersionIndex(string[] versions); +// What the PDB lint needs to know about a method body, read from the assembly alone. +record BodyFacts(int CodeSize, HashSet InstructionOffsets, HashSet CatchHandlerStarts, int LocalCount); + class AssertionFailedException(string message) : Exception(message); class DecompilerWarning(string message) : Exception(message); +// A defect in a generated PDB. The message describes the shape of the defect and never the +// instance that hit it - Report() dedupes on the message, so naming a method or an offset in it +// would turn one bug into one finding per method. +class PdbFinding(string message) : Exception(message); + +// Records assertion failures instead of throwing them. Debug.Assert unwinding out of the middle +// of PortablePdbWriter would leave no PDB to check, and the writer is known to assert on +// real-world input; with Trace.Listeners holding only this one, execution continues past the +// assert and still produces a PDB to look at. The call site has to be captured here: nothing +// throws, so there is no stack left to read afterwards, and a message-less Debug.Assert would +// otherwise be reported as an empty string naming no code at all. +class CollectAsserts(List messages) : TraceListener +{ + public override void Fail(string? message, string? detailMessage) + { + var frame = Environment.StackTrace.Split('\n').Select(l => l.Trim()) + .FirstOrDefault(l => l.Contains("ICSharpCode.Decompiler")) ?? ""; + messages.Add($"{message} {detailMessage}".Trim() + $" @ {frame}"); + } + public override void Write(string? message) + { + } + public override void WriteLine(string? message) + { + } +} + // Resolves assembly references from the given directories (in priority order) before // falling back to the wrapped resolver, and records every resolution and its outcome. // The wrapped UniversalAssemblyResolver consults the installed runtime BEFORE its search From 4e2f28109e0eae18cbbb72cc65eb37fe49e2e983 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 8 Sep 2026 20:12:03 +0200 Subject: [PATCH 2/2] Fix #2823: write the async catch handler offset the way consumers read 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 '
' 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 --- .../Helpers/Tester.cs | 7 +- .../PdbGenerationTestRunner.cs | 105 +++++++++++++++++- .../PdbGen/AsyncSteppingCatchHandler.cs | 30 +++++ .../PdbGen/AsyncSteppingEntryPoint.cs | 57 ++++++++++ .../DebugInfo/AsyncDebugInfo.cs | 8 +- .../IL/ControlFlow/AsyncAwaitDecompiler.cs | 37 +++++- 6 files changed, 235 insertions(+), 9 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingCatchHandler.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingEntryPoint.cs diff --git a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs index c3f0a8076..904e39641 100644 --- a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs +++ b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs @@ -932,7 +932,8 @@ namespace System.Runtime.CompilerServices } } - public static void CompileCSharpWithPdb(string assemblyName, Dictionary sourceFiles, CompilerOptions compilerOptions = CompilerOptions.None) + public static void CompileCSharpWithPdb(string assemblyName, Dictionary sourceFiles, + CompilerOptions compilerOptions = CompilerOptions.None) { var parseOptions = new CSharpParseOptions(languageVersion: Microsoft.CodeAnalysis.CSharp.LanguageVersion.Latest); if (compilerOptions.HasFlag(CompilerOptions.EnableRuntimeAsync)) @@ -955,7 +956,9 @@ namespace System.Runtime.CompilerServices var compilation = CSharpCompilation.Create(Path.GetFileNameWithoutExtension(assemblyName), syntaxTrees, coreDefaultReferences.Select(r => MetadataReference.CreateFromFile(Path.Combine(RefAssembliesToolset.GetPath(CurrentNetCoreAppVersion), r))), new CSharpCompilationOptions( - OutputKind.DynamicallyLinkedLibrary, + compilerOptions.HasFlag(CompilerOptions.Library) + ? OutputKind.DynamicallyLinkedLibrary + : OutputKind.ConsoleApplication, platform: Platform.AnyCpu, optimizationLevel: OptimizationLevel.Release, allowUnsafe: true, diff --git a/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs b/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs index cdffbe502..eb42e6cec 100644 --- a/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs @@ -427,7 +427,7 @@ namespace ICSharpCode.Decompiler.Tests string outputBase = Path.Combine(TestCasePath, nameof(RuntimeAsync) + ".expected"); Tester.CompileCSharpWithPdb(outputBase, new Dictionary { { Path.GetFileName(sourceFile), File.ReadAllText(sourceFile) } - }, CompilerOptions.EnableRuntimeAsync); + }, CompilerOptions.EnableRuntimeAsync | CompilerOptions.Library); string peFileName = outputBase + ".dll"; var module = new PEFile(peFileName); @@ -450,6 +450,99 @@ namespace ICSharpCode.Decompiler.Tests } } + [Test] + public void AsyncSteppingCatchHandler() + { + // The catch handler field of the async stepping blob is the generated handler's IL offset + // plus one, and only for async void methods; 0 otherwise. A consumer decodes it as + // (value - 1), so a raw offset points into the middle of an instruction and Mono.Cecil + // throws while reading the body, taking ILLink with it (#2823). Both PDBs here describe + // the same assembly, so the compiler's value is a direct oracle. + (string peFileName, string pdbFileName) = CompileTestCase(nameof(AsyncSteppingCatchHandler)); + + var module = new PEFile(peFileName); + var resolver = new UniversalAssemblyResolver(peFileName, false, + module.Metadata.DetectTargetFrameworkId(), null, PEStreamOptions.PrefetchEntireImage); + var decompiler = new CSharpDecompiler(module, resolver, new DecompilerSettings()); + + using var generatedPdb = new MemoryStream(); + new PortablePdbWriter { NoLogo = true } + .WritePdb(module, decompiler, new DecompilerSettings(), generatedPdb); + + generatedPdb.Position = 0; + var actual = ReadCatchHandlerOffsets( + MetadataReaderProvider.FromPortablePdbStream(generatedPdb).GetMetadataReader(), module.Metadata); + using var compilerPdb = File.OpenRead(pdbFileName); + var expected = ReadCatchHandlerOffsets( + MetadataReaderProvider.FromPortablePdbStream(compilerPdb).GetMetadataReader(), module.Metadata); + + Assert.That(expected, Is.Not.Empty, "the fixture produced no async stepping information to compare against"); + Assert.That(Format(actual), Is.EqualTo(Format(expected))); + + static string Format(Dictionary offsets) + => string.Join("\n", offsets.OrderBy(pair => pair.Key, StringComparer.Ordinal) + .Select(pair => $"{pair.Key}: 0x{pair.Value:x}")); + } + + [Test] + public void AsyncSteppingEntryPoint() + { + // The compiler records a catch handler for two shapes, not one: an async void method, and + // an async entry point - which returns Task. Both are shapes nothing is expected to await, + // so an exception escaping them should reach the debugger as user-unhandled. The fixture + // holds all three cases with identical bodies, so only the entry-point and return-type + // distinctions can account for a difference. + // Without CompilerOptions.Library the fixture is compiled as an executable, which is what + // gives it an entry point to recognise. + (string peFileName, string pdbFileName) = CompileTestCase(nameof(AsyncSteppingEntryPoint), + CompilerOptions.None); + + var module = new PEFile(peFileName); + var resolver = new UniversalAssemblyResolver(peFileName, false, + module.Metadata.DetectTargetFrameworkId(), null, PEStreamOptions.PrefetchEntireImage); + var decompiler = new CSharpDecompiler(module, resolver, new DecompilerSettings()); + + using var generatedPdb = new MemoryStream(); + new PortablePdbWriter { NoLogo = true } + .WritePdb(module, decompiler, new DecompilerSettings(), generatedPdb); + + generatedPdb.Position = 0; + var actual = ReadCatchHandlerOffsets( + MetadataReaderProvider.FromPortablePdbStream(generatedPdb).GetMetadataReader(), module.Metadata); + using var compilerPdb = File.OpenRead(pdbFileName); + var expected = ReadCatchHandlerOffsets( + MetadataReaderProvider.FromPortablePdbStream(compilerPdb).GetMetadataReader(), module.Metadata); + + Assert.That(expected.Count, Is.EqualTo(4), "the fixture should produce four async state machines"); + Assert.That(expected.Values.Count(offset => offset != 0), Is.EqualTo(2), + "only the async void method and the entry point should carry a catch handler"); + Assert.That(Format(actual), Is.EqualTo(Format(expected))); + + static string Format(Dictionary offsets) + => string.Join("\n", offsets.OrderBy(pair => pair.Key, StringComparer.Ordinal) + .Select(pair => $"{pair.Key}: 0x{pair.Value:x}")); + } + + /// + /// The catch handler offset out of every MethodSteppingInformation blob, keyed by the name of + /// the method that carries it. + /// + private static Dictionary ReadCatchHandlerOffsets(MetadataReader pdb, MetadataReader pe) + { + var offsets = new Dictionary(); + foreach (var handle in pdb.CustomDebugInformation) + { + var cdi = pdb.GetCustomDebugInformation(handle); + if (pdb.GetGuid(cdi.Kind) != KnownGuids.MethodSteppingInformation) + continue; + var method = pe.GetMethodDefinition((MethodDefinitionHandle)cdi.Parent); + var declaringType = pe.GetTypeDefinition(method.GetDeclaringType()); + offsets[$"{pe.GetString(declaringType.Name)}.{pe.GetString(method.Name)}"] + = pdb.GetBlobReader(cdi.Value).ReadUInt32(); + } + return offsets; + } + private class TestProgressReporter : IProgress { private Action reportFunc; @@ -585,17 +678,19 @@ namespace ICSharpCode.Decompiler.Tests TestSequencePoints(knownResidual: true); } - private static void CompileCSharpWithPdb(string outputBase, string sourceFile) + private static void CompileCSharpWithPdb(string outputBase, string sourceFile, + CompilerOptions compilerOptions = CompilerOptions.Library) { Tester.CompileCSharpWithPdb(outputBase, new Dictionary { { Path.GetFileName(sourceFile), File.ReadAllText(sourceFile) } - }); + }, compilerOptions); } - private (string peFileName, string pdbFileName) CompileTestCase(string testName) + private (string peFileName, string pdbFileName) CompileTestCase(string testName, + CompilerOptions compilerOptions = CompilerOptions.Library) { string sourceFile = Path.Combine(TestCasePath, testName + ".cs"); - CompileCSharpWithPdb(Path.Combine(TestCasePath, testName + ".expected"), sourceFile); + CompileCSharpWithPdb(Path.Combine(TestCasePath, testName + ".expected"), sourceFile, compilerOptions); string peFileName = Path.Combine(TestCasePath, testName + ".expected.dll"); string pdbFileName = Path.Combine(TestCasePath, testName + ".expected.pdb"); diff --git a/ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingCatchHandler.cs b/ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingCatchHandler.cs new file mode 100644 index 000000000..8fd1ea915 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingCatchHandler.cs @@ -0,0 +1,30 @@ +using System; +using System.Threading.Tasks; + +internal class AsyncSteppingCatchHandler +{ + public static async Task RunAsync() + { + await Task.Yield(); + Console.WriteLine("run"); + } + + public static async Task SumAsync(int a, int b) + { + try + { + await Task.Yield(); + return a + b; + } + catch (InvalidOperationException) + { + return 0; + } + } + + public static async void FireAndForget() + { + await Task.Yield(); + Console.WriteLine("done"); + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingEntryPoint.cs b/ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingEntryPoint.cs new file mode 100644 index 000000000..01a97b86d --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/PdbGen/AsyncSteppingEntryPoint.cs @@ -0,0 +1,57 @@ +using System; +using System.Threading.Tasks; + +internal class AsyncSteppingEntryPoint +{ + public static async Task Main() + { + try + { + await Task.Yield(); + Console.WriteLine("main"); + } + catch (InvalidOperationException e) + { + Console.WriteLine(e.Message); + } + } + + public static async Task Main(int notTheEntryPoint) + { + try + { + await Task.Yield(); + Console.WriteLine(notTheEntryPoint); + } + catch (InvalidOperationException e) + { + Console.WriteLine(e.Message); + } + } + + public static async Task NotTheEntryPointAsync() + { + try + { + await Task.Yield(); + Console.WriteLine("other"); + } + catch (InvalidOperationException e) + { + Console.WriteLine(e.Message); + } + } + + public static async void FireAndForget() + { + try + { + await Task.Yield(); + Console.WriteLine("done"); + } + catch (InvalidOperationException e) + { + Console.WriteLine(e.Message); + } + } +} diff --git a/ICSharpCode.Decompiler/DebugInfo/AsyncDebugInfo.cs b/ICSharpCode.Decompiler/DebugInfo/AsyncDebugInfo.cs index 94d5f2747..d634e7f66 100644 --- a/ICSharpCode.Decompiler/DebugInfo/AsyncDebugInfo.cs +++ b/ICSharpCode.Decompiler/DebugInfo/AsyncDebugInfo.cs @@ -25,6 +25,10 @@ namespace ICSharpCode.Decompiler.DebugInfo { public readonly struct AsyncDebugInfo { + /// + /// IL offset of the compiler-generated catch handler whose exceptions the debugger should + /// report as user-unhandled, or -1 when there is none to record. + /// public readonly int CatchHandlerOffset; public readonly ImmutableArray Awaits; @@ -49,7 +53,9 @@ namespace ICSharpCode.Decompiler.DebugInfo public BlobBuilder BuildBlob(MethodDefinitionHandle moveNext) { BlobBuilder blob = new BlobBuilder(); - blob.WriteUInt32((uint)CatchHandlerOffset); + // The field is the handler's offset plus one; 0 is the encoding for "none", which is why + // a consumer reading it back subtracts one before resolving it to an instruction. + blob.WriteUInt32((uint)(CatchHandlerOffset + 1)); foreach (var await in Awaits) { blob.WriteUInt32((uint)await.YieldOffset); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs b/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs index f80a60a62..3775d2eec 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs @@ -25,6 +25,7 @@ using System.Reflection.Metadata; using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.DebugInfo; +using ICSharpCode.Decompiler.Disassembler; using ICSharpCode.Decompiler.IL.Transforms; using ICSharpCode.Decompiler.Metadata; using ICSharpCode.Decompiler.TypeSystem; @@ -62,6 +63,29 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow return method == entrypoint && metadata.GetString(definition.Name).Equals("
", StringComparison.Ordinal); } + static bool IsCalledByEntryPoint(MetadataFile module, MethodDefinitionHandle method) + { + var entrypoint = System.Reflection.Metadata.Ecma335.MetadataTokens.MethodDefinitionHandle(module.CorHeader?.EntryPointTokenOrRelativeVirtualAddress ?? 0); + if (entrypoint.IsNil || !IsCompilerGeneratedMainMethod(module, entrypoint)) + return false; + var shim = module.Metadata.GetMethodDefinition(entrypoint); + if (shim.RelativeVirtualAddress == 0) + return false; + var blob = module.GetMethodBody(shim.RelativeVirtualAddress).GetILReader(); + while (blob.RemainingBytes > 0) + { + var code = blob.DecodeOpCode(); + if (code != ILOpCode.Call) + { + blob.SkipOperand(code); + continue; + } + if (MetadataTokenHelpers.EntityHandleOrNil(blob.ReadInt32()) == method) + return true; + } + return false; + } + enum AsyncMethodType { Void, @@ -116,6 +140,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow if (!context.Settings.AsyncAwait) return; // abort if async/await decompilation is disabled this.context = context; + catchHandlerOffset = -1; fieldToParameterMap.Clear(); cachedFieldToParameterMap.Clear(); awaitBlocks.Clear(); @@ -180,7 +205,17 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } awaitDebugInfos.SortBy(row => row.YieldOffset); - function.AsyncDebugInfo = new AsyncDebugInfo(catchHandlerOffset, awaitDebugInfos.ToImmutableArray()); + // The catchHandlerOffset marks the compiler-generated catch block. We need to distinguish + // a few cases: + // 1) async void methods always record the offset + // 2) the kickoff method of the async Main entry point always records the offset + // 3) in all the other cases nothing (-1) is emitted by csc. + var kickoff = function.Method?.MetadataToken ?? default; + bool recordCatchHandler = methodType == AsyncMethodType.Void + || (kickoff.Kind == HandleKind.MethodDefinition + && IsCalledByEntryPoint(context.PEFile, (MethodDefinitionHandle)kickoff)); + function.AsyncDebugInfo = new AsyncDebugInfo(recordCatchHandler ? catchHandlerOffset : -1, + awaitDebugInfos.ToImmutableArray()); } // Runtime-async analog of fieldToParameterMap's `<>4__this` capture: in a struct method,