diff --git a/TestTools/decompdiff.cs b/TestTools/decompdiff.cs index be366d1c1..d1d722570 100644 --- a/TestTools/decompdiff.cs +++ b/TestTools/decompdiff.cs @@ -255,7 +255,7 @@ summary.AppendLine($"|---|---|---|---|"); summary.AppendLine(MetricRow("lines", oldTotals.Lines, newTotals.Lines)); summary.AppendLine(MetricRow("goto statements", oldTotals.Gotos, newTotals.Gotos)); summary.AppendLine(MetricRow("//IL_ warning comments", oldTotals.IlWarnings, newTotals.IlWarnings)); -summary.AppendLine(MetricRow("compiler-generated name leaks (<>)", oldTotals.GeneratedNames, newTotals.GeneratedNames)); +summary.AppendLine(MetricRow("compiler-generated name leaks", oldTotals.GeneratedNames, newTotals.GeneratedNames)); summary.AppendLine(); summary.AppendLine($"types: {unchanged} unchanged, {changed.Count} changed, {newErrors} NEW errors, {fixedErrors} fixed errors, {bothErrors} errored in both"); summary.AppendLine(); @@ -832,11 +832,21 @@ record ChangedType(string Location, Metrics Old, Metrics New); record struct Metrics(int Lines, int Gotos, int IlWarnings, int GeneratedNames) { + // Names that the decompiler's own identifier rules reject: every character of an + // identifier must be a letter, digit or '_' (EscapeInvalidIdentifiers.IsValid), and the + // mangled ones start with '<' or contain '$' (SRMExtensions.IsGeneratedName). The input + // is output text rather than identifiers, so the shapes are matched lexically: a + // bracketed part followed by an identifier character ("<>c", "

k__BackingField", + // "

$"), or a '$' between identifier characters ("VB$AnonymousType_0"). Real C# + // never puts an identifier character directly after a generic argument list's '>', so + // generic instantiations do not match. + const string GeneratedNamePattern = @"<[A-Za-z0-9_.,<> ]*>[A-Za-z0-9_$]|[A-Za-z0-9_]\$[A-Za-z0-9_$]"; + public static Metrics Measure(string code) => new( code.Count(c => c == '\n') + 1, Regex.Matches(code, @"\bgoto ").Count, Regex.Matches(code, @"//IL_[0-9a-fA-F]+:").Count, - Regex.Matches(code, @"<>").Count); + Regex.Matches(code, GeneratedNamePattern).Count); public static Metrics operator +(Metrics a, Metrics b) => new(a.Lines + b.Lines, a.Gotos + b.Gotos, a.IlWarnings + b.IlWarnings, a.GeneratedNames + b.GeneratedNames); diff --git a/TestTools/nugetfuzz.cs b/TestTools/nugetfuzz.cs index d075c1382..6c4ecb900 100644 --- a/TestTools/nugetfuzz.cs +++ b/TestTools/nugetfuzz.cs @@ -47,6 +47,8 @@ using System.Text.RegularExpressions; using ICSharpCode.Decompiler; using ICSharpCode.Decompiler.CSharp; +using ICSharpCode.Decompiler.CSharp.OutputVisitor; +using ICSharpCode.Decompiler.CSharp.Syntax; using ICSharpCode.Decompiler.DebugInfo; using ICSharpCode.Decompiler.Metadata; @@ -81,6 +83,7 @@ var installedTfm = NuGetFramework.Parse($"net{Environment.Version.Major}.{Enviro var net48 = NuGetFramework.Parse("net48"); var reducer = new FrameworkReducer(); var failures = new Dictionary(); +var formatting = new DecompilerSettings().CSharpFormattingOptions; int assemblyCount = 0, typeCount = 0, pdbChecked = 0, pdbSkipped = 0; long charCount = 0, refsResolved = 0, refsTotal = 0; bool verbose = Environment.GetEnvironmentVariable("NUGETFUZZ_VERBOSE") != null; @@ -626,17 +629,34 @@ async Task DecompileAssembly(string pkg, string dllPath, List searchDirs decompiler.CancellationToken = cts.Token; try { - var code = decompiler.DecompileTypeAsString(type.FullTypeName); + var tree = decompiler.DecompileType(type.FullTypeName); + var code = SyntaxTreeToString(tree); typeCount++; charCount += code.Length; // Compiler-generated types (, , // VB$AnonymousType_*, ...) are still decompiled to shake out edge cases, // but empty output is normal for them ('<' and '$' match the decompiler's // own generated-name detection in SRMExtensions.IsGeneratedName). - if (string.IsNullOrWhiteSpace(code) && !type.Name.StartsWith('<') && !type.Name.Contains('$')) + bool generatedType = type.Name.StartsWith('<') || type.Name.Contains('$'); + if (string.IsNullOrWhiteSpace(code) && !generatedType) Report(pkg, name, type.FullTypeName.ToString(), new InvalidDataException("empty decompilation output")); else if (dumpDir != null) File.WriteAllText(Path.Combine(dumpDir, SanitizeFileName($"{pkg}.{type.FullTypeName}.cs")), code); + // An identifier may only contain letters, digits and '_' (the rule + // EscapeInvalidIdentifiers.IsValid applies for project output). Any other name in + // the tree is a compiler-generated entity the decompiler failed to fold away, and + // the output does not compile. Inside a generated type everything is mangled by + // definition, so only user-written types are checked. + if (!generatedType) + { + foreach (var leak in tree.DescendantsAndSelf.OfType() + .Select(i => i.Name) + .Where(n => !n.All(ch => char.IsLetterOrDigit(ch) || ch == '_')) + .Distinct()) + { + Report(pkg, name, type.FullTypeName.ToString(), new LeakedName(leak)); + } + } // ILFunction warnings (unknown result types, stack type mismatches, invalid IL) // surface in the output as "//IL_xxxx: " comments. foreach (var warning in Regex.Matches(code, @"//IL_[0-9a-fA-F]+: (.*)") @@ -658,6 +678,13 @@ async Task DecompileAssembly(string pkg, string dllPath, List searchDirs } } +string SyntaxTreeToString(SyntaxTree syntaxTree) +{ + var w = new StringWriter(); + syntaxTree.AcceptVisitor(new CSharpOutputVisitor(w, formatting)); + return w.ToString(); +} + void ReportResolutions(LoggingResolver logResolver) { var resolutions = logResolver.Resolutions; @@ -1264,6 +1291,7 @@ void Report(string pkg, string asm, string type, Exception ex) var kind = inner is AssertionFailedException ? "ASSERT" : inner is TimeoutException ? "TIMEOUT" : inner is DecompilerWarning ? "WARNING" + : inner is LeakedName ? "LEAK" : inner is PdbFinding ? "PDB" : "EXCEPTION"; var key = $"{kind}|{inner.GetType().Name}|{inner.Message}|{topFrame}"; var location = $"{pkg} / {asm} / {type}"; @@ -1396,7 +1424,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; } + .PDB { border-left:4px solid #0d9488; } .LEAK { border-left:4px solid #db2777; } #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; } @@ -1407,7 +1435,7 @@ static void WriteHtmlReport(string path, List findings, int assemblies, + $"({findings.Sum(f => f.Count)} total)" + (dumpDir != null ? $"
decompiled sources dumped to {Esc(dumpDir)}" : "") + ""); html.AppendLine(""); - foreach (var kind in new[] { "ASSERT", "EXCEPTION", "TIMEOUT", "WARNING" }) + foreach (var kind in new[] { "ASSERT", "EXCEPTION", "TIMEOUT", "LEAK", "WARNING", "PDB" }) { var group = findings.Where(f => f.Kind == kind).OrderByDescending(f => f.Count).ToList(); if (group.Count == 0) @@ -1471,6 +1499,13 @@ class AssertionFailedException(string message) : Exception(message); class DecompilerWarning(string message) : Exception(message); +// A compiler-generated name that reached the output. The message keeps only the shape of +// the name - the part in angle brackets is the enclosing member and the digits are per +// occurrence - so that one bucket collects every hit of the same unfolded construct. +class LeakedName(string name) + : Exception("leaked compiler-generated name: " + + Regex.Replace(Regex.Replace(name, "<[^<>]*>", "<>"), "[0-9]+", "N")); + // 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.