diff --git a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs index 38dacce79..a56830f3f 100644 --- a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs +++ b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs @@ -928,7 +928,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)) @@ -951,7 +952,9 @@ namespace System.Runtime.CompilerServices var compilation = CSharpCompilation.Create(Path.GetFileNameWithoutExtension(assemblyName), syntaxTrees, coreDefaultReferences.Select(r => MetadataReference.CreateFromFile(Path.Combine(RefAssembliesToolset.GetPath(CurrentNetCoreVersion.AppVersion), 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, 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