Browse Source

Report compiler-generated names leaking into decompiled output

decompdiff counted the substring "<>" in the output text, which both missed
mangled names that do not contain it ("VB$AnonymousType_0", "<Main>$") and
counted every generic argument list ending in an identifier character. The
shape is matched lexically there because only text is available; nugetfuzz has
the syntax tree, so it applies the decompiler's own identifier rule
(EscapeInvalidIdentifiers.IsValid) to the tree's identifiers instead, and any
hit is output that does not compile. Findings collapse to the shape of the
name because the bracketed part and the digits vary per occurrence, so one
unfolded construct stays one finding rather than one per member it hit.

The report's kind list is also the render loop's only source of sections, so
the PDB bucket added with the PDB verification mode never reached the HTML.

Assisted-by: Claude:claude-opus-5:Claude Code
pull/4128/head
Siegfried Pammer 7 days ago
parent
commit
a6415d1041
  1. 14
      TestTools/decompdiff.cs
  2. 43
      TestTools/nugetfuzz.cs

14
TestTools/decompdiff.cs

@ -255,7 +255,7 @@ summary.AppendLine($"|---|---|---|---|");
summary.AppendLine(MetricRow("lines", oldTotals.Lines, newTotals.Lines)); summary.AppendLine(MetricRow("lines", oldTotals.Lines, newTotals.Lines));
summary.AppendLine(MetricRow("goto statements", oldTotals.Gotos, newTotals.Gotos)); summary.AppendLine(MetricRow("goto statements", oldTotals.Gotos, newTotals.Gotos));
summary.AppendLine(MetricRow("//IL_ warning comments", oldTotals.IlWarnings, newTotals.IlWarnings)); 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();
summary.AppendLine($"types: {unchanged} unchanged, {changed.Count} changed, {newErrors} NEW errors, {fixedErrors} fixed errors, {bothErrors} errored in both"); summary.AppendLine($"types: {unchanged} unchanged, {changed.Count} changed, {newErrors} NEW errors, {fixedErrors} fixed errors, {bothErrors} errored in both");
summary.AppendLine(); 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) 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", "<P>k__BackingField",
// "<Main>$"), 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( public static Metrics Measure(string code) => new(
code.Count(c => c == '\n') + 1, code.Count(c => c == '\n') + 1,
Regex.Matches(code, @"\bgoto ").Count, Regex.Matches(code, @"\bgoto ").Count,
Regex.Matches(code, @"//IL_[0-9a-fA-F]+:").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) 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); => new(a.Lines + b.Lines, a.Gotos + b.Gotos, a.IlWarnings + b.IlWarnings, a.GeneratedNames + b.GeneratedNames);

43
TestTools/nugetfuzz.cs

@ -47,6 +47,8 @@ using System.Text.RegularExpressions;
using ICSharpCode.Decompiler; using ICSharpCode.Decompiler;
using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp;
using ICSharpCode.Decompiler.CSharp.OutputVisitor;
using ICSharpCode.Decompiler.CSharp.Syntax;
using ICSharpCode.Decompiler.DebugInfo; using ICSharpCode.Decompiler.DebugInfo;
using ICSharpCode.Decompiler.Metadata; using ICSharpCode.Decompiler.Metadata;
@ -81,6 +83,7 @@ var installedTfm = NuGetFramework.Parse($"net{Environment.Version.Major}.{Enviro
var net48 = NuGetFramework.Parse("net48"); var net48 = NuGetFramework.Parse("net48");
var reducer = new FrameworkReducer(); var reducer = new FrameworkReducer();
var failures = new Dictionary<string, Finding>(); var failures = new Dictionary<string, Finding>();
var formatting = new DecompilerSettings().CSharpFormattingOptions;
int assemblyCount = 0, typeCount = 0, pdbChecked = 0, pdbSkipped = 0; int assemblyCount = 0, typeCount = 0, pdbChecked = 0, pdbSkipped = 0;
long charCount = 0, refsResolved = 0, refsTotal = 0; long charCount = 0, refsResolved = 0, refsTotal = 0;
bool verbose = Environment.GetEnvironmentVariable("NUGETFUZZ_VERBOSE") != null; bool verbose = Environment.GetEnvironmentVariable("NUGETFUZZ_VERBOSE") != null;
@ -626,17 +629,34 @@ async Task DecompileAssembly(string pkg, string dllPath, List<string> searchDirs
decompiler.CancellationToken = cts.Token; decompiler.CancellationToken = cts.Token;
try try
{ {
var code = decompiler.DecompileTypeAsString(type.FullTypeName); var tree = decompiler.DecompileType(type.FullTypeName);
var code = SyntaxTreeToString(tree);
typeCount++; typeCount++;
charCount += code.Length; charCount += code.Length;
// Compiler-generated types (<Module>, <PrivateImplementationDetails>, // Compiler-generated types (<Module>, <PrivateImplementationDetails>,
// VB$AnonymousType_*, ...) are still decompiled to shake out edge cases, // VB$AnonymousType_*, ...) are still decompiled to shake out edge cases,
// but empty output is normal for them ('<' and '$' match the decompiler's // but empty output is normal for them ('<' and '$' match the decompiler's
// own generated-name detection in SRMExtensions.IsGeneratedName). // 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")); Report(pkg, name, type.FullTypeName.ToString(), new InvalidDataException("empty decompilation output"));
else if (dumpDir != null) else if (dumpDir != null)
File.WriteAllText(Path.Combine(dumpDir, SanitizeFileName($"{pkg}.{type.FullTypeName}.cs")), code); 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<Identifier>()
.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) // ILFunction warnings (unknown result types, stack type mismatches, invalid IL)
// surface in the output as "//IL_xxxx: <message>" comments. // surface in the output as "//IL_xxxx: <message>" comments.
foreach (var warning in Regex.Matches(code, @"//IL_[0-9a-fA-F]+: (.*)") foreach (var warning in Regex.Matches(code, @"//IL_[0-9a-fA-F]+: (.*)")
@ -658,6 +678,13 @@ async Task DecompileAssembly(string pkg, string dllPath, List<string> searchDirs
} }
} }
string SyntaxTreeToString(SyntaxTree syntaxTree)
{
var w = new StringWriter();
syntaxTree.AcceptVisitor(new CSharpOutputVisitor(w, formatting));
return w.ToString();
}
void ReportResolutions(LoggingResolver logResolver) void ReportResolutions(LoggingResolver logResolver)
{ {
var resolutions = logResolver.Resolutions; var resolutions = logResolver.Resolutions;
@ -1264,6 +1291,7 @@ void Report(string pkg, string asm, string type, Exception ex)
var kind = inner is AssertionFailedException ? "ASSERT" var kind = inner is AssertionFailedException ? "ASSERT"
: inner is TimeoutException ? "TIMEOUT" : inner is TimeoutException ? "TIMEOUT"
: inner is DecompilerWarning ? "WARNING" : inner is DecompilerWarning ? "WARNING"
: inner is LeakedName ? "LEAK"
: inner is PdbFinding ? "PDB" : "EXCEPTION"; : inner is PdbFinding ? "PDB" : "EXCEPTION";
var key = $"{kind}|{inner.GetType().Name}|{inner.Message}|{topFrame}"; var key = $"{kind}|{inner.GetType().Name}|{inner.Message}|{topFrame}";
var location = $"{pkg} / {asm} / {type}"; var location = $"{pkg} / {asm} / {type}";
@ -1396,7 +1424,7 @@ static void WriteHtmlReport(string path, List<Finding> findings, int assemblies,
color:#a8791f; border:1px solid #a8791f55; margin-left:6px; } color:#a8791f; border:1px solid #a8791f55; margin-left:6px; }
.ASSERT { border-left:4px solid #d97706; } .EXCEPTION { border-left:4px solid #dc2626; } .ASSERT { border-left:4px solid #d97706; } .EXCEPTION { border-left:4px solid #dc2626; }
.TIMEOUT { border-left:4px solid #7c3aed; } .WARNING { border-left:4px solid #2563eb; } .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; #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; } background:var(--bg); color:var(--fg); font:13px ui-monospace,monospace; }
</style></head><body> </style></head><body>
@ -1407,7 +1435,7 @@ static void WriteHtmlReport(string path, List<Finding> findings, int assemblies,
+ $"({findings.Sum(f => f.Count)} total)" + $"({findings.Sum(f => f.Count)} total)"
+ (dumpDir != null ? $"<br>decompiled sources dumped to {Esc(dumpDir)}" : "") + "</div>"); + (dumpDir != null ? $"<br>decompiled sources dumped to {Esc(dumpDir)}" : "") + "</div>");
html.AppendLine("<input id=filter placeholder='filter by message, type, package or frame'>"); html.AppendLine("<input id=filter placeholder='filter by message, type, package or frame'>");
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(); var group = findings.Where(f => f.Kind == kind).OrderByDescending(f => f.Count).ToList();
if (group.Count == 0) if (group.Count == 0)
@ -1471,6 +1499,13 @@ class AssertionFailedException(string message) : Exception(message);
class DecompilerWarning(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 // 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 // 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. // would turn one bug into one finding per method.

Loading…
Cancel
Save