diff --git a/ICSharpCode.Decompiler.PowerShell/DecompilationErrorReporting.cs b/ICSharpCode.Decompiler.PowerShell/DecompilationErrorReporting.cs new file mode 100644 index 000000000..5399d268b --- /dev/null +++ b/ICSharpCode.Decompiler.PowerShell/DecompilationErrorReporting.cs @@ -0,0 +1,39 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Collections.Generic; +using System.Management.Automation; + +namespace ICSharpCode.Decompiler.PowerShell +{ + static class DecompilationErrorReporting + { + /// + /// Raises one non-terminating error per member the decompiler could not handle. The output + /// is produced either way - with the error text in place of the affected code - so without + /// this a script would take known-broken source for a clean decompilation. + /// + public static void WriteDecompilationErrors(this Cmdlet cmdlet, IReadOnlyList errors) + { + foreach (var error in errors) + { + cmdlet.WriteError(new ErrorRecord(error, ErrorIds.DecompilationFailed, ErrorCategory.NotSpecified, null)); + } + } + } +} diff --git a/ICSharpCode.Decompiler.PowerShell/GetDecompiledProjectCmdlet.cs b/ICSharpCode.Decompiler.PowerShell/GetDecompiledProjectCmdlet.cs index ccad637bf..ebea95183 100644 --- a/ICSharpCode.Decompiler.PowerShell/GetDecompiledProjectCmdlet.cs +++ b/ICSharpCode.Decompiler.PowerShell/GetDecompiledProjectCmdlet.cs @@ -17,6 +17,7 @@ // DEALINGS IN THE SOFTWARE. using System; +using System.Collections.Generic; using System.IO; using System.Management.Automation; using System.Threading; @@ -96,6 +97,7 @@ namespace ICSharpCode.Decompiler.PowerShell task.Wait(); WriteProgress(new ProgressRecord(1, "Decompiling " + fileName, "Decompilation finished") { RecordType = ProgressRecordType.Completed }); + this.WriteDecompilationErrors(errors); } catch (Exception e) { @@ -104,6 +106,8 @@ namespace ICSharpCode.Decompiler.PowerShell } } + private IReadOnlyList errors = Array.Empty(); + private void DoDecompile(string path) { MetadataFile module = Decompiler.TypeSystem.MainModule.MetadataFile; @@ -112,7 +116,14 @@ namespace ICSharpCode.Decompiler.PowerShell decompiler.ProgressIndicator = this; fileName = module.FileName; completed = 0; - decompiler.DecompileProject(module, path); + try + { + decompiler.DecompileProject(module, path); + } + finally + { + errors = decompiler.Errors; + } } } } diff --git a/ICSharpCode.Decompiler.PowerShell/GetDecompiledSourceCmdlet.cs b/ICSharpCode.Decompiler.PowerShell/GetDecompiledSourceCmdlet.cs index e8a8e87eb..ae1b46045 100644 --- a/ICSharpCode.Decompiler.PowerShell/GetDecompiledSourceCmdlet.cs +++ b/ICSharpCode.Decompiler.PowerShell/GetDecompiledSourceCmdlet.cs @@ -53,6 +53,7 @@ namespace ICSharpCode.Decompiler.PowerShell } WriteObject(output.ToString()); + this.WriteDecompilationErrors(Decompiler.Errors); } catch (Exception e) { diff --git a/ICSharpCode.Decompiler.Tests/DecompilationErrorRecoveryTests.cs b/ICSharpCode.Decompiler.Tests/DecompilationErrorRecoveryTests.cs new file mode 100644 index 000000000..a0fcf17b7 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/DecompilationErrorRecoveryTests.cs @@ -0,0 +1,145 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.IO; +using System.Linq; + +using ICSharpCode.Decompiler.CSharp; +using ICSharpCode.Decompiler.CSharp.OutputVisitor; +using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.IL; +using ICSharpCode.Decompiler.IL.Transforms; +using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.TypeSystem; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests +{ + /// + /// A method body that cannot be decompiled must not take the surrounding type - or, when + /// exporting a project, the surrounding assembly - down with it. The failure is turned into + /// output the user can copy into a bug report, and decompilation continues. + /// + [TestFixture] + public class DecompilationErrorRecoveryTests + { + const string SimulatedFailure = "Simulated transform failure"; + + [Test] + public void FailingMethodBodyKeepsTheRestOfTheType() + { + var decompiler = CreateDecompiler(); + decompiler.ILTransforms.Add(new ThrowingILTransform("CleanUpFileName")); + + string code = decompiler.DecompileTypeAsString( + new FullTypeName("ICSharpCode.Decompiler.CSharp.ProjectDecompiler.WholeProjectDecompiler")); + + using (Assert.EnterMultipleScope()) + { + Assert.That(code, Does.Contain(SimulatedFailure), "the exception text must show up in the output"); + Assert.That(code, Does.Contain(CSharpDecompiler.DecompilationErrorReportUrl), "users need to be told where to report this"); + Assert.That(code, Does.Contain("public static string CleanUpFileName"), "the failing member keeps its signature"); + Assert.That(code, Does.Contain("DecompileProject"), "the other members of the type are unaffected"); + } + } + + [Test] + public void FailingMethodBodyIsRecordedAsError() + { + var decompiler = CreateDecompiler(); + decompiler.ILTransforms.Add(new ThrowingILTransform("CleanUpFileName")); + + decompiler.DecompileTypeAsString( + new FullTypeName("ICSharpCode.Decompiler.CSharp.ProjectDecompiler.WholeProjectDecompiler")); + + var error = decompiler.Errors.Single(); + Assert.That(error.Message, Does.Contain("CleanUpFileName")); + } + + /// + /// describes the decompilation that just ran, so a + /// reused instance must not report the previous one's failures against it. + /// + [Test] + public void ErrorsCoverOnlyTheLastDecompilation() + { + var decompiler = CreateDecompiler(); + var failing = new ThrowingILTransform("CleanUpFileName"); + decompiler.ILTransforms.Add(failing); + decompiler.DecompileTypeAsString( + new FullTypeName("ICSharpCode.Decompiler.CSharp.ProjectDecompiler.WholeProjectDecompiler")); + + decompiler.ILTransforms.Remove(failing); + decompiler.DecompileTypeAsString( + new FullTypeName("ICSharpCode.Decompiler.CSharp.ProjectDecompiler.WholeProjectDecompiler")); + + Assert.That(decompiler.Errors, Is.Empty); + } + + /// + /// A member whose output throws is replaced by the error text, and writing carries on with + /// the rest of the type - a file cut off mid-member would leave the braces around it open + /// and every later type unreadable. + /// + [Test] + public void FailingOutputKeepsTheFileWellFormed() + { + var decompiler = CreateDecompiler(); + var syntaxTree = decompiler.DecompileType( + new FullTypeName("ICSharpCode.Decompiler.CSharp.ProjectDecompiler.WholeProjectDecompiler")); + + // A member that cannot be written: an expression node with no children to write. + var victim = syntaxTree.Descendants.OfType().First(m => m.Name == "CleanUpFileName"); + victim.Body.Statements.Clear(); + victim.Body.Statements.Add(new ExpressionStatement(new BinaryOperatorExpression())); + + var writer = new StringWriter(); + var outputVisitor = new ErrorTolerantOutputVisitor(writer, new DecompilerSettings().CSharpFormattingOptions); + syntaxTree.AcceptVisitor(outputVisitor); + string code = writer.ToString(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(outputVisitor.Errors, Has.Count.EqualTo(1), "the failure is reported to the caller"); + Assert.That(code, Does.Contain(CSharpDecompiler.DecompilationErrorReportUrl), "and shows up in the file"); + Assert.That(code, Does.Contain("DecompileProject"), "the members after the failing one are still written"); + Assert.That(code.Count(c => c == '{'), Is.EqualTo(code.Count(c => c == '}')), + "every brace the failed member opened is closed again"); + } + } + + static CSharpDecompiler CreateDecompiler() + { + var module = new PEFile("ICSharpCode.Decompiler.dll"); + var settings = new DecompilerSettings(); + var typeSystem = new DecompilerTypeSystem(module, new UniversalAssemblyResolver(null, false, null), settings); + return new CSharpDecompiler(typeSystem, settings); + } + + sealed class ThrowingILTransform(string methodName) : IILTransform + { + public void Run(ILFunction function, ILTransformContext context) + { + if (function.Parent == null && function.Method?.Name == methodName) + throw new InvalidOperationException(SimulatedFailure); + } + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WholeProjectDecompilerTests.cs b/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WholeProjectDecompilerTests.cs index 55e4f96f8..4d0d25c8e 100644 --- a/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WholeProjectDecompilerTests.cs +++ b/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WholeProjectDecompilerTests.cs @@ -19,9 +19,14 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; +using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp.ProjectDecompiler; +using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.CSharp.Transforms; using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.TypeSystem; using NUnit.Framework; @@ -68,6 +73,78 @@ public sealed class WholeProjectDecompilerTests } } + /// + /// Everything an export can fail at - decompiling a source file, creating one, the assembly-info + /// file, a resource - is reported and skipped; the export itself always runs to completion, so a + /// single unsupported member cannot cost the user the whole project (issue #3510). + /// + [Test] + public void FailuresDoNotAbortTheExport() + { + string targetDirectory = Path.Combine(Environment.CurrentDirectory, Path.GetRandomFileName()); + TestFriendlyProjectDecompiler decompiler = new(new UniversalAssemblyResolver(null, false, null)); + decompiler.ConfigureDecompiler = d => d.AstTransforms.Add(new ThrowingAstTransform(nameof(WholeProjectDecompiler))); + decompiler.FailResourceEnumeration = true; + decompiler.FailFileCreationFor = new[] { nameof(TargetServices) + ".cs", "AssemblyInfo.cs" }; + + StringWriter projectFileWriter = new(); + decompiler.DecompileProject(new PEFile("ICSharpCode.Decompiler.dll"), targetDirectory, projectFileWriter); + AssertDirectoryDoesntExist(targetDirectory); + + string failedFile = Path.Combine(targetDirectory, "ICSharpCode.Decompiler.CSharp.ProjectDecompiler", $"{nameof(WholeProjectDecompiler)}.cs"); + using (Assert.EnterMultipleScope()) + { + Assert.That(decompiler.Errors.Select(e => e.InnerException?.Message), Is.EquivalentTo(new[] { + ThrowingAstTransform.Failure, + TestFriendlyProjectDecompiler.ResourceFailure, + TestFriendlyProjectDecompiler.FileCreationFailure + nameof(TargetServices) + ".cs", + TestFriendlyProjectDecompiler.FileCreationFailure + "AssemblyInfo.cs", + })); + Assert.That(decompiler.Files[failedFile].ToString(), Does.Contain(ThrowingAstTransform.Failure), + "the error text takes the place of the file's contents"); + Assert.That(decompiler.Files, Has.Count.GreaterThan(100), "all other files are still written"); + Assert.That(projectFileWriter.ToString(), Does.Contain(" + /// A resource that cannot be written must cost that resource alone. Recovering around the + /// enumeration cannot do this - an iterator is finished once it throws - so the export has to + /// recover per resource, and this pins that. + /// + [Test] + public void OneFailingResourceDoesNotDropTheOthers() + { + string targetDirectory = Path.Combine(Environment.CurrentDirectory, Path.GetRandomFileName()); + TestFriendlyProjectDecompiler decompiler = new(new UniversalAssemblyResolver(null, false, null)); + decompiler.FailResourceWriting = true; + + StringWriter projectFileWriter = new(); + // Two embedded .resources containers and nothing else, so both go through WriteResourceToFile + // and the test never touches the disk. + decompiler.DecompileProject(new PEFile("Microsoft.DiaSymReader.Converter.Xml.dll"), targetDirectory, projectFileWriter); + AssertDirectoryDoesntExist(targetDirectory); + + using (Assert.EnterMultipleScope()) + { + Assert.That(decompiler.WrittenResources, Has.Count.EqualTo(2), + "the resource after the failing one is still written"); + Assert.That(decompiler.Errors.Select(e => e.InnerException?.Message), + Is.EqualTo(new[] { TestFriendlyProjectDecompiler.ResourceFailure })); + } + } + + sealed class ThrowingAstTransform(string typeName) : IAstTransform + { + public const string Failure = "Simulated AST transform failure"; + + public void Run(AstNode rootNode, TransformContext context) + { + if (rootNode.Descendants.OfType().Any(td => td.Name == typeName)) + throw new InvalidOperationException(Failure); + } + } + static void AssertDirectoryDoesntExist(string directory) { if (Directory.Exists(directory)) @@ -81,9 +158,19 @@ public sealed class WholeProjectDecompilerTests { public Dictionary Files { get; } = []; public HashSet Directories { get; } = []; + public Action? ConfigureDecompiler { get; set; } + + protected override CSharpDecompiler CreateDecompiler(DecompilerTypeSystem ts) + { + var decompiler = base.CreateDecompiler(ts); + ConfigureDecompiler?.Invoke(decompiler); + return decompiler; + } protected override TextWriter CreateFile(string path) { + if (FailFileCreationFor.Any(name => path.EndsWith(name, StringComparison.Ordinal))) + throw new IOException(FileCreationFailure + Path.GetFileName(path)); StringWriter writer = new(); lock (Files) { @@ -102,6 +189,37 @@ public sealed class WholeProjectDecompilerTests protected override IEnumerable WriteMiscellaneousFilesInProject(PEFile module) => []; - protected override IEnumerable WriteResourceFilesInProject(MetadataFile module) => []; + public const string ResourceFailure = "Simulated resource failure"; + public const string FileCreationFailure = "Simulated file creation failure: "; + + public bool FailResourceEnumeration { get; set; } + + public bool FailResourceWriting { get; set; } + + public string[] FailFileCreationFor { get; set; } = Array.Empty(); + + public List WrittenResources { get; } = []; + + protected override IEnumerable WriteResourceFilesInProject(MetadataFile module) + { + if (FailResourceWriting) + return base.WriteResourceFilesInProject(module); + return FailResourceEnumeration + ? Enumerable.Range(0, 1).Select(_ => throw new InvalidOperationException(ResourceFailure)) + : []; + } + + // Fails on the first resource only, so the test can tell "recovered per resource" from + // "gave up on the rest of them". + protected override IEnumerable WriteResourceToFile(string fileName, string resourceName, Stream entryStream) + { + if (WrittenResources.Count == 0) + { + WrittenResources.Add(fileName); + throw new InvalidOperationException(ResourceFailure); + } + WrittenResources.Add(fileName); + return new[] { new ProjectItemInfo("EmbeddedResource", fileName) }; + } } } diff --git a/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs b/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs index 6fc0feea9..56cc26fd6 100644 --- a/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs +++ b/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs @@ -19,6 +19,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Linq; using System.Reflection.PortableExecutable; using System.Text.RegularExpressions; using System.Threading; @@ -204,6 +205,9 @@ namespace ICSharpCode.Decompiler.Roundtrip decompiler.StrongNameKeyFile = Path.Combine(inputDir, snkFilePath); } decompiler.DecompileProject(module, decompiledDir); + // The exporter reports what it could not decompile instead of throwing, so + // without this a decompiler crash produces a stub and the round trip passes. + Assert.That(decompiler.Errors.Select(e => e.ToString()), Is.Empty); Console.WriteLine($"Decompiled {fileToRoundtrip} in {w.Elapsed.TotalSeconds:f2}"); projectFile = Path.Combine(decompiledDir, module.Name + ".csproj"); } diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index b613b71f5..6c9a27aef 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -290,6 +290,59 @@ namespace ICSharpCode.Decompiler.CSharp get { return astTransforms; } } + /// + /// Method bodies that could not be decompiled. Instead of aborting the surrounding type, + /// such a member is emitted with the error text in place of its body (see + /// ) and the exception is collected here, so callers + /// decompiling many members - the project exporter above all - can tell the user how many + /// members are affected. + /// + public IReadOnlyList Errors => errors; + + readonly List errors = new List(); + + /// + /// Where users are asked to report decompilation failures; part of the error text emitted + /// into the output, because a failure nobody reports is a failure nobody fixes. + /// + public const string DecompilationErrorReportUrl = "https://github.com/icsharpcode/ILSpy/issues/new"; + + /// + /// The headline a front end puts above the list of failures it recovered from. Shared so the + /// UI, the command line and any other consumer say the same thing and point at the same URL. + /// + public static IEnumerable GetErrorSummaryLines(int errorCount) + { + yield return $"{errorCount} error(s) occurred; the affected code was replaced by the error text in the output."; + yield return $"Please report them at {DecompilationErrorReportUrl}:"; + } + + /// + /// The one-line description of a single failure, so the UI and the command line name it the + /// same way. + /// + public static string GetErrorHeadline(DecompilerException error) + { + if (error == null) + throw new ArgumentNullException(nameof(error)); + return error.InnerException == null ? error.Message : $"{error.Message}: {error.InnerException.Message}"; + } + + /// + /// Renders as the lines of a comment block: an explanation, the + /// request to report it, and the full exception including its stack trace, which is what + /// makes such a report actionable. + /// + internal static IEnumerable GetErrorCommentLines(Exception error) + { + yield return "ILSpy could not decompile this. Please report the exception below,"; + yield return "along with the assembly it came from, at " + DecompilationErrorReportUrl; + foreach (string line in error.ToString().Split('\n')) + { + yield return line.TrimEnd('\r'); + } + } + /// /// Creates a new instance from the given using the given . /// @@ -752,6 +805,10 @@ namespace ICSharpCode.Decompiler.CSharp DecompileRun CreateDecompileRun(HashSet namespaces) { + // Every public Decompile* entry point starts here, so this is where the failures of the + // previous one stop counting - otherwise a reused instance reports them again against + // members that decompiled cleanly. + errors.Clear(); List resolvedNamespaces = new List(); foreach (var ns in namespaces) { @@ -2283,9 +2340,34 @@ namespace ICSharpCode.Decompiler.CSharp CleanUpMethodDeclaration(entityDecl, body, function, localSettings.DecompileMemberBodies); } - catch (Exception innerException) when (!(innerException is OperationCanceledException || innerException is DecompilerException)) + catch (Exception innerException) when (!(innerException is OperationCanceledException)) { - throw new DecompilerException(module, method, innerException); + // One method the decompiler cannot handle must not cost the user the type or, when + // exporting a project, the assembly around it: keep the signature, put the error in + // front of it, and let the remaining members decompile. + errors.Add(innerException as DecompilerException ?? new DecompilerException(module, method, innerException)); + entityDecl.GetChild(Slots.Body)?.Remove(); + if (settings.DecompileMemberBodies) + { + // The error goes where the code would have been, the same way a warning about the + // code does - and the body keeps the member's shape intact. + var errorBody = new BlockStatement(); + var errorStatement = new EmptyStatement(); + foreach (string line in GetErrorCommentLines(innerException)) + { + errorStatement.AddTrailingTrivia(new Comment(" " + line)); + } + errorBody.Statements.Add(errorStatement); + entityDecl.AddChild(errorBody, Slots.Body); + } + else + { + // Definitions-only output has no body to put the error in. + foreach (string line in GetErrorCommentLines(innerException)) + { + entityDecl.AddLeadingTrivia(new Comment(" " + line)); + } + } } } diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/ErrorTolerantOutputVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/ErrorTolerantOutputVisitor.cs new file mode 100644 index 000000000..4aa3edf0f --- /dev/null +++ b/ICSharpCode.Decompiler/CSharp/OutputVisitor/ErrorTolerantOutputVisitor.cs @@ -0,0 +1,138 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.IO; + +using ICSharpCode.Decompiler.CSharp.Syntax; + +#nullable enable + +namespace ICSharpCode.Decompiler.CSharp.OutputVisitor +{ + /// + /// Writes a syntax tree like , but a member whose output throws + /// is replaced by the error text instead of ending the file half-written. Writing resumes with + /// the next member, so the reader still gets the rest of the type and a file that closes every + /// brace it opened. + /// + /// + /// The failures are collected in . An from the + /// underlying writer is not something to recover from - every following write would fail the + /// same way - so it is left to propagate. + /// + public class ErrorTolerantOutputVisitor : CSharpOutputVisitor + { + readonly List errors = new List(); + int braceDepth; + + public ErrorTolerantOutputVisitor(TextWriter textWriter, CSharpFormattingOptions formattingPolicy) + : base(textWriter, formattingPolicy) + { + } + + /// + /// The failures that took the place of a member, in the order they were written. + /// + public IReadOnlyList Errors => errors; + + protected override void OpenBrace(BraceStyle style, bool newLine = true) + { + base.OpenBrace(style, newLine); + braceDepth++; + } + + protected override void CloseBrace(BraceStyle style, bool unindent = true) + { + base.CloseBrace(style, unindent); + braceDepth--; + } + + public override void VisitTypeDeclaration(TypeDeclaration typeDeclaration) + => Write(typeDeclaration, base.VisitTypeDeclaration); + + public override void VisitDelegateDeclaration(DelegateDeclaration delegateDeclaration) + => Write(delegateDeclaration, base.VisitDelegateDeclaration); + + public override void VisitConstructorDeclaration(ConstructorDeclaration constructorDeclaration) + => Write(constructorDeclaration, base.VisitConstructorDeclaration); + + public override void VisitDestructorDeclaration(DestructorDeclaration destructorDeclaration) + => Write(destructorDeclaration, base.VisitDestructorDeclaration); + + public override void VisitEnumMemberDeclaration(EnumMemberDeclaration enumMemberDeclaration) + => Write(enumMemberDeclaration, base.VisitEnumMemberDeclaration); + + public override void VisitExtensionDeclaration(ExtensionDeclaration extensionDeclaration) + => Write(extensionDeclaration, base.VisitExtensionDeclaration); + + public override void VisitEventDeclaration(EventDeclaration eventDeclaration) + => Write(eventDeclaration, base.VisitEventDeclaration); + + public override void VisitCustomEventDeclaration(CustomEventDeclaration customEventDeclaration) + => Write(customEventDeclaration, base.VisitCustomEventDeclaration); + + public override void VisitFieldDeclaration(FieldDeclaration fieldDeclaration) + => Write(fieldDeclaration, base.VisitFieldDeclaration); + + public override void VisitFixedFieldDeclaration(FixedFieldDeclaration fixedFieldDeclaration) + => Write(fixedFieldDeclaration, base.VisitFixedFieldDeclaration); + + public override void VisitIndexerDeclaration(IndexerDeclaration indexerDeclaration) + => Write(indexerDeclaration, base.VisitIndexerDeclaration); + + public override void VisitMethodDeclaration(MethodDeclaration methodDeclaration) + => Write(methodDeclaration, base.VisitMethodDeclaration); + + public override void VisitOperatorDeclaration(OperatorDeclaration operatorDeclaration) + => Write(operatorDeclaration, base.VisitOperatorDeclaration); + + public override void VisitPropertyDeclaration(PropertyDeclaration propertyDeclaration) + => Write(propertyDeclaration, base.VisitPropertyDeclaration); + + void Write(T node, Action write) where T : AstNode + { + int braces = braceDepth; + int containers = containerStack.Count; + try + { + write(node); + } + catch (Exception ex) when (!(ex is OperationCanceledException || ex is IOException)) + { + errors.Add(ex); + // The failed member left the writer inside its own nodes and braces: unwind both, so + // what follows is written at the level the member started at. + while (containerStack.Count > containers) + { + writer.EndNode(containerStack.Pop()); + } + while (braceDepth > braces) + { + CloseBrace(BraceStyle.NextLine); + } + NewLine(); + foreach (string line in CSharpDecompiler.GetErrorCommentLines(ex)) + { + writer.WriteComment(CommentType.SingleLine, " " + line); + } + } + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/ProjectDecompiler/WholeProjectDecompiler.cs b/ICSharpCode.Decompiler/CSharp/ProjectDecompiler/WholeProjectDecompiler.cs index 84f85968e..9264e109a 100644 --- a/ICSharpCode.Decompiler/CSharp/ProjectDecompiler/WholeProjectDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/ProjectDecompiler/WholeProjectDecompiler.cs @@ -151,8 +151,82 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler // per-run members HashSet directories = new HashSet(Platform.FileNameComparer); + readonly List errors = new List(); readonly IProjectFileWriter projectWriter; + /// + /// Everything that went wrong during the last . + /// An export never aborts on a member, file or resource it cannot handle; it writes the + /// error text where the content would have gone and continues, so a single unsupported + /// method still yields a complete project. Callers should show this list to the user - + /// otherwise the failures ship silently and never get reported. + /// + public IReadOnlyList Errors => errors; + + void RecordError(DecompilerException error) + { + lock (errors) + { + errors.Add(error); + } + } + + /// + /// Yields the items of until one of them throws; the failure is + /// recorded instead of aborting the export. + /// + IEnumerable RecordingErrors(IEnumerable items, MetadataFile file, string what) + { + using var enumerator = items.GetEnumerator(); + bool lastMoveFailed = false; + while (true) + { + T item; + try + { + if (!enumerator.MoveNext()) + yield break; + item = enumerator.Current; + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + RecordError(ex as DecompilerException ?? new DecompilerException(file, $"Error writing {what}", ex)); + // Skip the item that failed and try the next one, but give up once two attempts + // in a row fail: an enumerator that throws without advancing - which nothing + // stops an override from being - would otherwise loop forever. + if (lastMoveFailed) + yield break; + lastMoveFailed = true; + continue; + } + lastMoveFailed = false; + yield return item; + } + } + + /// + /// Puts the error text where the file's contents would have gone. The writer itself may be + /// what failed - a full disk, a stream already closed - so a second failure while reporting + /// the first is dropped rather than allowed to take the export down. + /// + static void WriteErrorComment(TextWriter? writer, Exception error) + { + if (writer == null) + return; + try + { + // The failure may have interrupted the output visitor mid-line. + writer.WriteLine(); + foreach (string line in CSharpDecompiler.GetErrorCommentLines(error)) + { + writer.WriteLine("// " + line); + } + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + } + } + public void DecompileProject(MetadataFile file, string targetDirectory, CancellationToken cancellationToken = default(CancellationToken)) { string projectFileName = Path.Combine(targetDirectory, CleanUpFileName(file.Name, ".csproj")); @@ -182,7 +256,8 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler { TargetDirectory = targetDirectory; directories.Clear(); - var resources = WriteResourceFilesInProject(file).ToList(); + errors.Clear(); + var resources = RecordingErrors(WriteResourceFilesInProject(file), file, "resource files").ToList(); resourceFileCount = resources.Count; var files = WriteCodeFilesInProject(file, resources.SelectMany(r => r.PartialTypes ?? Enumerable.Empty()).ToList(), cancellationToken).ToList(); codeFileCount = files.Count; @@ -190,7 +265,7 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler var module = file as PEFile; if (module != null) { - files.AddRange(WriteMiscellaneousFilesInProject(module)); + files.AddRange(RecordingErrors(WriteMiscellaneousFilesInProject(module), file, "miscellaneous files")); } if (StrongNameKeyFile != null) { @@ -277,6 +352,7 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler var progressReporter = ProgressIndicator; var progress = new DecompilationProgress { TotalUnits = files.Count, Title = "Exporting project..." }; DecompilerTypeSystem ts = new DecompilerTypeSystem(module, AssemblyResolver, Settings); + var missingFiles = new ConcurrentBag(); var workList = new HashSet(); var processedTypes = new HashSet(); ProcessFiles(files); @@ -290,7 +366,21 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler progress.TotalUnits = files.Count; } - return files.Select(f => new ProjectItemInfo("Compile", f.Key)).Concat(WriteAssemblyInfo(ts, cancellationToken)); + // The assembly-level attributes are a single file like any other: failing to decompile + // them costs that file, not the export. + IEnumerable assemblyInfo; + try + { + assemblyInfo = WriteAssemblyInfo(ts, cancellationToken); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + RecordError(ex as DecompilerException ?? new DecompilerException(module, "Error decompiling the module and assembly attributes", ex)); + assemblyInfo = Enumerable.Empty(); + } + + return files.Select(f => f.Key).Except(missingFiles, Platform.FileNameComparer) + .Select(f => new ProjectItemInfo("Compile", f)).Concat(assemblyInfo); string GetFileFileNameForHandle(TypeDefinitionHandle h) { @@ -325,10 +415,14 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler delegate (IGrouping file) { var declaredTypes = file.ToArray(); DecompilerEventSource.Log.ProjectFileStart(file.Key, declaredTypes.Length); + // Everything that can fail for this one file - creating it included, which is + // where a path too long for the file system surfaces - belongs inside the try. + TextWriter? w = null; + CSharpDecompiler? decompiler = null; try { - using var w = CreateFile(Path.Combine(TargetDirectory, file.Key)); - CSharpDecompiler decompiler = CreateDecompiler(ts); + w = CreateFile(Path.Combine(TargetDirectory, file.Key)); + decompiler = CreateDecompiler(ts); foreach (var partialType in partialTypes) { @@ -356,14 +450,44 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler } } - syntaxTree.AcceptVisitor(new CSharpOutputVisitor(w, Settings.CSharpFormattingOptions)); + // A member the output visitor cannot write is replaced by the error text + // rather than truncating the file where it failed. + var outputVisitor = new ErrorTolerantOutputVisitor(w, Settings.CSharpFormattingOptions); + syntaxTree.AcceptVisitor(outputVisitor); + foreach (var outputError in outputVisitor.Errors) + { + RecordError(new DecompilerException(module, $"Error writing '{file.Key}'", outputError)); + } } - catch (Exception innerException) when (!(innerException is OperationCanceledException || innerException is DecompilerException)) + catch (Exception innerException) when (!(innerException is OperationCanceledException)) { - throw new DecompilerException(module, $"Error decompiling for '{file.Key}'", innerException); + // Whatever the decompiler could not cope with here, the remaining files + // are unaffected and the user still gets a complete project; the error + // takes the place of the file's contents. + RecordError(innerException as DecompilerException ?? new DecompilerException(module, $"Error decompiling for '{file.Key}'", innerException)); + if (w == null) + { + // Nothing was written, so nothing can carry the error text - and the + // project must not claim a file that is not there. + missingFiles.Add(file.Key); + } + WriteErrorComment(w, innerException); } finally { + foreach (var error in decompiler?.Errors ?? (IReadOnlyList)Array.Empty()) + { + RecordError(error); + } + try + { + w?.Dispose(); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + // Dispose flushes: on a full disk this is where the write actually fails. + RecordError(new DecompilerException(module, $"Error writing '{file.Key}'", ex)); + } DecompilerEventSource.Log.ProjectFileStop(file.Key); } progress.Status = file.Key; @@ -379,77 +503,97 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler { foreach (var r in module.Resources.Where(r => r.ResourceType == ResourceType.Embedded)) { - Stream? stream = r.TryOpenStream(); - if (stream == null) + List items; + try + { + items = WriteResourceFileInProject(r).ToList(); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + // One resource nobody can decode - a mangled .resources blob, a BAML stream the + // decompiler chokes on - costs that resource, not the ones behind it. + RecordError(ex as DecompilerException ?? new DecompilerException(module, $"Error writing resource '{r.Name}'", ex)); continue; + } + foreach (var item in items) + { + yield return item; + } + } + } + + IEnumerable WriteResourceFileInProject(Resource r) + { + Stream? stream = r.TryOpenStream(); + if (stream == null) + yield break; - stream.Position = 0; + stream.Position = 0; - if (r.Name.EndsWith(".resources", StringComparison.OrdinalIgnoreCase)) + if (r.Name.EndsWith(".resources", StringComparison.OrdinalIgnoreCase)) + { + bool decodedIntoIndividualFiles; + var individualResources = new List(); + try { - bool decodedIntoIndividualFiles; - var individualResources = new List(); - try + var resourcesFile = new ResourcesFile(stream); + if (resourcesFile.AllEntriesAreStreams()) { - var resourcesFile = new ResourcesFile(stream); - if (resourcesFile.AllEntriesAreStreams()) + foreach (var (name, value) in resourcesFile) { - foreach (var (name, value) in resourcesFile) + string fileName = SanitizeFileName(name); + string? dirName = Path.GetDirectoryName(fileName); + if (!string.IsNullOrEmpty(dirName) && directories.Add(dirName)) { - string fileName = SanitizeFileName(name); - string? dirName = Path.GetDirectoryName(fileName); - if (!string.IsNullOrEmpty(dirName) && directories.Add(dirName)) - { - CreateDirectory(Path.Combine(TargetDirectory, dirName)); - } - Stream entryStream = (Stream)value!; - entryStream.Position = 0; - individualResources.AddRange( - WriteResourceToFile(fileName, name, entryStream)); + CreateDirectory(Path.Combine(TargetDirectory, dirName)); } - decodedIntoIndividualFiles = true; - } - else - { - decodedIntoIndividualFiles = false; + Stream entryStream = (Stream)value!; + entryStream.Position = 0; + individualResources.AddRange( + WriteResourceToFile(fileName, name, entryStream)); } + decodedIntoIndividualFiles = true; } - catch (BadImageFormatException) - { - decodedIntoIndividualFiles = false; - } - catch (EndOfStreamException) + else { decodedIntoIndividualFiles = false; } - if (decodedIntoIndividualFiles) - { - foreach (var entry in individualResources) - { - yield return entry; - } - } - else + } + catch (BadImageFormatException) + { + decodedIntoIndividualFiles = false; + } + catch (EndOfStreamException) + { + decodedIntoIndividualFiles = false; + } + if (decodedIntoIndividualFiles) + { + foreach (var entry in individualResources) { - stream.Position = 0; - string fileName = GetFileNameForResource(r.Name); - foreach (var entry in WriteResourceToFile(fileName, r.Name, stream)) - { - yield return entry; - } + yield return entry; } } else { + stream.Position = 0; string fileName = GetFileNameForResource(r.Name); - using (FileStream fs = new FileStream(Path.Combine(TargetDirectory, fileName), FileMode.Create, FileAccess.Write)) + foreach (var entry in WriteResourceToFile(fileName, r.Name, stream)) { - stream.Position = 0; - stream.CopyTo(fs); + yield return entry; } - yield return new ProjectItemInfo("EmbeddedResource", fileName).With("LogicalName", r.Name); } } + else + { + string fileName = GetFileNameForResource(r.Name); + using (FileStream fs = new FileStream(Path.Combine(TargetDirectory, fileName), FileMode.Create, FileAccess.Write)) + { + stream.Position = 0; + stream.CopyTo(fs); + } + yield return new ProjectItemInfo("EmbeddedResource", fileName).With("LogicalName", r.Name); + } } protected virtual IEnumerable WriteResourceToFile(string fileName, string resourceName, Stream entryStream) @@ -519,25 +663,56 @@ namespace ICSharpCode.Decompiler.CSharp.ProjectDecompiler if (resources == null) yield break; - byte[]? appIcon = CreateApplicationIcon(resources); - if (appIcon != null) - { + // Each file is written on its own, so the one that fails is the only one lost. + foreach (var item in TryWrite(module, "app.ico", () => { + byte[]? appIcon = CreateApplicationIcon(resources); + if (appIcon == null) + return null; File.WriteAllBytes(Path.Combine(TargetDirectory, "app.ico"), appIcon); - yield return new ProjectItemInfo("ApplicationIcon", "app.ico"); + return new ProjectItemInfo("ApplicationIcon", "app.ico"); + })) + { + yield return item; } - byte[]? appManifest = CreateApplicationManifest(resources); - if (appManifest != null && !IsDefaultApplicationManifest(appManifest)) - { + foreach (var item in TryWrite(module, "app.manifest", () => { + byte[]? appManifest = CreateApplicationManifest(resources); + if (appManifest == null || IsDefaultApplicationManifest(appManifest)) + return null; File.WriteAllBytes(Path.Combine(TargetDirectory, "app.manifest"), appManifest); - yield return new ProjectItemInfo("ApplicationManifest", "app.manifest"); + return new ProjectItemInfo("ApplicationManifest", "app.manifest"); + })) + { + yield return item; } - var appConfig = module.FileName + ".config"; - if (File.Exists(appConfig)) - { + foreach (var item in TryWrite(module, "app.config", () => { + var appConfig = module.FileName + ".config"; + if (!File.Exists(appConfig)) + return null; File.Copy(appConfig, Path.Combine(TargetDirectory, "app.config"), overwrite: true); - yield return new ProjectItemInfo("ApplicationConfig", Path.GetFileName(appConfig)); + return new ProjectItemInfo("ApplicationConfig", Path.GetFileName(appConfig)); + })) + { + yield return item; + } + } + + IEnumerable TryWrite(MetadataFile module, string what, Func write) + { + ProjectItemInfo? item; + try + { + item = write(); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + RecordError(ex as DecompilerException ?? new DecompilerException(module, $"Error writing '{what}'", ex)); + yield break; + } + if (item.HasValue) + { + yield return item.Value; } } diff --git a/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs b/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs index 7400f798e..9ecc66986 100644 --- a/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs +++ b/ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs @@ -163,6 +163,11 @@ Examples: [Option("-r|--referencepath ", "Path to a directory containing dependencies of the assembly that is being decompiled.", CommandOptionType.MultipleValue)] public string[] ReferencePaths { get; } + [Option("--ignore-decompilation-errors", "Exit with success even when parts of the assembly could not be decompiled. " + + "The affected code carries the error text in the output and the failures are listed on stderr either way; " + + "only the exit status changes.", CommandOptionType.NoValue)] + public bool IgnoreDecompilationErrorsFlag { get; } + [Option("--no-dead-code", "Remove dead code.", CommandOptionType.NoValue)] public bool RemoveDeadCode { get; } @@ -254,7 +259,7 @@ Examples: { string projectFileName = Path.Combine(outputDirectory, Path.GetFileNameWithoutExtension(InputAssemblyNames[0]) + ".csproj"); DecompileAsProject(InputAssemblyNames[0], projectFileName); - return 0; + return ExitCodeForDecompilationErrors(); } var projects = new List(); foreach (var file in InputAssemblyNames) @@ -265,7 +270,7 @@ Examples: projects.Add(new ProjectItem(projectFileName, projectId.PlatformName, projectId.Guid, projectId.TypeGuid)); } SolutionCreator.WriteSolutionFile(Path.Combine(outputDirectory, Path.GetFileNameWithoutExtension(outputDirectory) + ".sln"), projects); - return 0; + return ExitCodeForDecompilationErrors(); } else if (GenerateDiagrammer) { @@ -295,7 +300,7 @@ Examples: if (result != 0) return result; } - return 0; + return ExitCodeForDecompilationErrors(); } } catch (Exception ex) @@ -622,6 +627,34 @@ Examples: return 0; } + readonly List decompilationErrors = new(); + + /// + /// Lists what the decompiler could not handle. The output was still written - with the error + /// text in place of the affected code - so this is the only sign anything went wrong, and + /// keeps it from passing silently in a script. + /// + void ReportDecompilationErrors(string assemblyFileName, IReadOnlyList errors) + { + if (errors.Count == 0) + return; + decompilationErrors.AddRange(errors); + Console.Error.WriteLine($"While decompiling {assemblyFileName}:"); + foreach (string line in CSharpDecompiler.GetErrorSummaryLines(errors.Count)) + { + Console.Error.WriteLine(line); + } + foreach (var error in errors) + { + Console.Error.WriteLine(" " + CSharpDecompiler.GetErrorHeadline(error)); + } + } + + int ExitCodeForDecompilationErrors() + { + return decompilationErrors.Count == 0 || IgnoreDecompilationErrorsFlag ? 0 : ProgramExitCodes.EX_SOFTWARE; + } + ProjectId DecompileAsProject(string assemblyFileName, string projectFileName) { var module = new PEFile(assemblyFileName); @@ -645,8 +678,11 @@ Examples: { decompiler = new WholeProjectDecompiler(settings, resolver, null, resolver, debugInfo); } - using (var projectFileWriter = new StreamWriter(File.OpenWrite(projectFileName))) - return decompiler.DecompileProject(module, Path.GetDirectoryName(projectFileName), projectFileWriter); + ProjectId projectId; + using (var projectFileWriter = new StreamWriter(File.Create(projectFileName))) + projectId = decompiler.DecompileProject(module, Path.GetDirectoryName(projectFileName), projectFileWriter); + ReportDecompilationErrors(assemblyFileName, decompiler.Errors); + return projectId; } int Decompile(string assemblyFileName, TextWriter output, string typeName = null) @@ -656,6 +692,7 @@ Examples: if (typeName == null) { output.Write(decompiler.DecompileWholeModuleAsString()); + ReportDecompilationErrors(assemblyFileName, decompiler.Errors); return 0; } @@ -666,6 +703,7 @@ Examples: } output.Write(decompiler.DecompileTypeAsString(typeDefinition.FullTypeName)); + ReportDecompilationErrors(assemblyFileName, decompiler.Errors); return 0; } @@ -680,6 +718,7 @@ Examples: } output.Write(decompiler.DecompileAsString(handle)); + ReportDecompilationErrors(assemblyFileName, decompiler.Errors); return 0; } diff --git a/ICSharpCode.ILSpyCmd/README.md b/ICSharpCode.ILSpyCmd/README.md index c32cef8b0..c821eb643 100644 --- a/ICSharpCode.ILSpyCmd/README.md +++ b/ICSharpCode.ILSpyCmd/README.md @@ -57,6 +57,9 @@ Options: -ds|--decompiler-setting Set a decompiler setting. Use multiple times to set multiple settings. -r|--referencepath Path to a directory containing dependencies of the assembly that is being decompiled. + --ignore-decompilation-errors Exit with success even when parts of the assembly could not be decompiled. The + affected code carries the error text in the output and the failures are listed + on stderr either way; only the exit status changes. --no-dead-code Remove dead code. --no-dead-stores Remove dead stores. -d|--dump-package Dump package assemblies into a folder. This requires the output directory