Fix #2823 and add a PDB verification mode to nugetfuzz

ICSharpCode.Decompiler.Tests/Helpers/Tester.cs

@@ -932,7 +932,8 @@ internal static DecompilerSettings GetSettings(CompilerOptions cscOptions)
 			}
 		}
 
-		public static void CompileCSharpWithPdb(string assemblyName, Dictionary<string, string> sourceFiles, CompilerOptions compilerOptions = CompilerOptions.None)
+		public static void CompileCSharpWithPdb(string assemblyName, Dictionary<string, string> sourceFiles,
+			CompilerOptions compilerOptions = CompilerOptions.None)
 		{
 			var parseOptions = new CSharpParseOptions(languageVersion: Microsoft.CodeAnalysis.CSharp.LanguageVersion.Latest);
 			if (compilerOptions.HasFlag(CompilerOptions.EnableRuntimeAsync))
@@ -955,7 +956,9 @@ public static void CompileCSharpWithPdb(string assemblyName, Dictionary<string,
 			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,

ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs

@@ -427,7 +427,7 @@ public void RuntimeAsync()
 			string outputBase = Path.Combine(TestCasePath, nameof(RuntimeAsync) + ".expected");
 			Tester.CompileCSharpWithPdb(outputBase, new Dictionary<string, string> {
 				{ 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 @@ public void RuntimeAsync()
 			}
 		}
 
+		[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<string, uint> 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<string, uint> offsets)
+				=> string.Join("\n", offsets.OrderBy(pair => pair.Key, StringComparer.Ordinal)
+					.Select(pair => $"{pair.Key}: 0x{pair.Value:x}"));
+		}
+
+		/// <summary>
+		/// The catch handler offset out of every MethodSteppingInformation blob, keyed by the name of
+		/// the method that carries it.
+		/// </summary>
+		private static Dictionary<string, uint> ReadCatchHandlerOffsets(MetadataReader pdb, MetadataReader pe)
+		{
+			var offsets = new Dictionary<string, uint>();
+			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<DecompilationProgress>
 		{
 			private Action<DecompilationProgress> reportFunc;
@@ -585,17 +678,19 @@ public void MemberInitializerEvents()
 			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<string, string> {
 				{ 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");

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<int> 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");
+	}
+}

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);
+		}
+	}
+}

ICSharpCode.Decompiler/DebugInfo/AsyncDebugInfo.cs

@@ -25,6 +25,10 @@ namespace ICSharpCode.Decompiler.DebugInfo
 {
 	public readonly struct AsyncDebugInfo
 	{
+		/// <summary>
+		/// 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.
+		/// </summary>
 		public readonly int CatchHandlerOffset;
 		public readonly ImmutableArray<Await> Awaits;
 
@@ -49,7 +53,9 @@ public Await(int yieldOffset, int resumeOffset)
 		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);

ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs

@@ -25,6 +25,7 @@
 
 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 @@ internal static bool IsCompilerGeneratedMainMethod(MetadataFile module, MethodDe
 			return method == entrypoint && metadata.GetString(definition.Name).Equals("<Main>", 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 @@ public void Run(ILFunction function, ILTransformContext context)
 			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 @@ public void Run(ILFunction function, ILTransformContext context)
 			}
 
 			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,

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=<file>` (append findings as JSONL instead of writing a
 per-run HTML report), `NUGETFUZZ_HTML=<file>` (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