From 93904ecfc5fc5ec7030f060b00df54824278903a Mon Sep 17 00:00:00 2001 From: Ty Overby Date: Mon, 22 Feb 2016 15:32:39 -0800 Subject: [PATCH] Handle PE writing exceptions by appending a diagnostic --- .../CSharp/Portable/CSharpResources.resx | 5 +- .../CSharp/Portable/Errors/ErrorCode.cs | 1 + .../CSharp/Portable/Errors/MessageProvider.cs | 1 + .../Attributes/EmitTestStrongNameProvider.cs | 4 +- .../Test/Emit/Emit/CompilationEmitTests.cs | 33 +++++++- .../CSharp/Test/Emit/Emit/OutputStreams.cs | 67 --------------- .../Core/Portable/Compilation/Compilation.cs | 18 +--- .../Diagnostic/CommonMessageProvider.cs | 1 + .../Core/Portable/PEWriter/PeWriter.cs | 14 +++- .../VisualBasic/Portable/Errors/Errors.vb | 1 + .../Portable/Errors/MessageProvider.vb | 6 ++ .../VisualBasic/Portable/VBResources.resx | 5 +- .../Attributes/EmitTestStrongNameProvider.vb | 3 +- .../Test/Emit/Emit/CompilationEmitTests.vb | 84 +++++++++++++++++++ .../Shared/Mocks/TestMessageProvider.cs | 8 ++ src/Test/Utilities/Shared/Pe/BrokenStream.cs | 74 ++++++++++++++++ 16 files changed, 233 insertions(+), 92 deletions(-) create mode 100644 src/Test/Utilities/Shared/Pe/BrokenStream.cs diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index 1d96642596d..74de8bc06e2 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -4675,4 +4675,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ Combined length of user strings used by the program exceeds allowed limit. Try to decrease use of string literals. - \ No newline at end of file + + An error occurred while writing the output file. + + diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index 729180bee5e..b0d2a1c103b 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1320,5 +1320,6 @@ internal enum ErrorCode ERR_InvalidPathMap = 8101, ERR_PublicSignButNoKey = 8102, ERR_TooManyUserStrings = 8103, + ERR_PeWritingFailure = 8104, } } diff --git a/src/Compilers/CSharp/Portable/Errors/MessageProvider.cs b/src/Compilers/CSharp/Portable/Errors/MessageProvider.cs index 4a4d0ff96aa..a83bce0681c 100644 --- a/src/Compilers/CSharp/Portable/Errors/MessageProvider.cs +++ b/src/Compilers/CSharp/Portable/Errors/MessageProvider.cs @@ -202,6 +202,7 @@ public override void ReportDuplicateMetadataReferenceWeak(DiagnosticBag diagnost public override int ERR_MetadataNameTooLong { get { return (int)ErrorCode.ERR_MetadataNameTooLong; } } public override int ERR_EncReferenceToAddedMember { get { return (int)ErrorCode.ERR_EncReferenceToAddedMember; } } public override int ERR_TooManyUserStrings { get { return (int)ErrorCode.ERR_TooManyUserStrings; } } + public override int ERR_PeWritingFailure { get { return (int)ErrorCode.ERR_PeWritingFailure; } } public override void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute) { diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs b/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs index 6f67bc26092..24e6fa4fb79 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs @@ -54,8 +54,8 @@ class C options: options); comp.VerifyEmitDiagnostics( - // error CS7028: Error signing output with public key from container 'RoslynTestContainer' -- This is a test IOException - Diagnostic(ErrorCode.ERR_PublicKeyContainerFailure).WithArguments("RoslynTestContainer", "This is a test IOException").WithLocation(1, 1)); + // error CS8104: An error occurred while writing the Portable Executable file. + Diagnostic(ErrorCode.ERR_PeWritingFailure).WithArguments("This is a test IOException").WithLocation(1, 1)); } } } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index d32cf1cd11a..2f17b18265e 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Immutable; using System.IO; using System.Linq; using System.Reflection; @@ -2699,10 +2698,16 @@ public void BrokenOutStream() var compilation = CreateCompilationWithMscorlib(source); var output = new BrokenStream(); - Assert.Throws(() => compilation.Emit(output)); + var result = compilation.Emit(output); + result.Diagnostics.Verify( + // error CS8104: An error occurred while writing the Portable Executable file. + Diagnostic(ErrorCode.ERR_PeWritingFailure).WithArguments("I/O error occurred.").WithLocation(1, 1)); output.BreakHow = 1; - Assert.Throws(() => compilation.Emit(output)); + result = compilation.Emit(output); + result.Diagnostics.Verify( + // error CS8104: An error occurred while writing the Portable Executable file. + Diagnostic(ErrorCode.ERR_PeWritingFailure).WithArguments("Specified method is not supported.").WithLocation(1, 1)); // disposed stream is not writable var outReal = new MemoryStream(); @@ -2949,5 +2954,27 @@ static int M(dynamic d, object o) AssertEx.Equal(expectedNames, actualNames); } } + + [Fact] + [WorkItem(3240, "https://github.com/dotnet/roslyn/pull/8227")] + public void FailingEmitter() + { + string source = @" +public class X +{ + public static void Main() + { + + } +}"; + var compilation = CreateCompilationWithMscorlib(source); + var broken = new BrokenStream(); + broken.BreakHow = 0; + var result = compilation.Emit(broken); + Assert.False(result.Success); + result.Diagnostics.Verify( + // error CS8104: An error occurred while writing the Portable Executable file. + Diagnostic(ErrorCode.ERR_PeWritingFailure).WithArguments("I/O error occurred.").WithLocation(1, 1)); + } } } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/OutputStreams.cs b/src/Compilers/CSharp/Test/Emit/Emit/OutputStreams.cs index 26880dfaa98..674fd11abe8 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/OutputStreams.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/OutputStreams.cs @@ -20,71 +20,4 @@ public Stream GetXmlInclude(SyntaxTree syntaxTree, string xmlIncludeFile) throw new NotImplementedException(); } } - - internal class BrokenStream : Stream - { - public int BreakHow; - - public override bool CanRead - { - get { return true; } - } - - public override bool CanSeek - { - get { return true; } - } - - public override bool CanWrite - { - get { return true; } - } - - public override void Flush() - { - } - - public override long Length - { - get - { - return 0; - } - } - - public override long Position - { - get - { - return 0; - } - set - { - if (BreakHow == 1) - throw new NotSupportedException(); - } - } - - public override int Read(byte[] buffer, int offset, int count) - { - return 0; - } - - public override long Seek(long offset, SeekOrigin origin) - { - return 0; - } - - public override void SetLength(long value) - { - if (BreakHow == 2) - throw new IOException(); - } - - public override void Write(byte[] buffer, int offset, int count) - { - if (BreakHow == 0) - throw new IOException(); - } - } } diff --git a/src/Compilers/Core/Portable/Compilation/Compilation.cs b/src/Compilers/Core/Portable/Compilation/Compilation.cs index 59d287c537a..18796e9b251 100644 --- a/src/Compilers/Core/Portable/Compilation/Compilation.cs +++ b/src/Compilers/Core/Portable/Compilation/Compilation.cs @@ -1844,17 +1844,8 @@ private static EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool su { Debug.Assert(Options.StrongNameProvider != null); - // Targeted try-catch for errors during CreateInputStream as found in TFS 1140649 - // TODO: Put this wrapping in PeWriter to catch all potential PE writing exceptions - try - { - signingInputStream = Options.StrongNameProvider.CreateInputStream(); - retStream = signingInputStream; - } - catch (Exception e) - { - throw new Cci.PeWritingException(e); - } + signingInputStream = Options.StrongNameProvider.CreateInputStream(); + retStream = signingInputStream; } else { @@ -1917,9 +1908,8 @@ private static EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool su } catch (Cci.PeWritingException e) { - // Targeted fix for TFS 1140649 - // TODO: Add resource and better error message for a variety of PE exceptions - diagnostics.Add(StrongNameKeys.GetError(StrongNameKeys.KeyFilePath, StrongNameKeys.KeyContainer, e.Message, MessageProvider)); + diagnostics.Add(MessageProvider.CreateDiagnostic(MessageProvider.ERR_PeWritingFailure, Location.None, e.Message)); + return false; } catch (ResourceException e) { diff --git a/src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs b/src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs index 573ecf52a0c..8b089af9feb 100644 --- a/src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs +++ b/src/Compilers/Core/Portable/Diagnostic/CommonMessageProvider.cs @@ -203,6 +203,7 @@ public DiagnosticInfo FilterDiagnosticInfo(DiagnosticInfo diagnosticInfo, Compil public abstract int ERR_MetadataNameTooLong { get; } public abstract int ERR_EncReferenceToAddedMember { get; } public abstract int ERR_TooManyUserStrings { get; } + public abstract int ERR_PeWritingFailure { get; } public abstract void ReportInvalidAttributeArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int parameterIndex, AttributeData attribute); public abstract void ReportInvalidNamedArgument(DiagnosticBag diagnostics, SyntaxNode attributeSyntax, int namedArgumentIndex, ITypeSymbol attributeClass, string parameterName); diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs index c04bd639ab0..a42fb3c599b 100644 --- a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis; using Roslyn.Utilities; using EmitContext = Microsoft.CodeAnalysis.Emit.EmitContext; +using Microsoft.CodeAnalysis.CodeGen; namespace Microsoft.Cci { @@ -88,10 +89,17 @@ internal sealed class PeWriter // If PDB writer is given, we have to have PDB path. Debug.Assert(nativePdbWriterOpt == null || pdbPathOpt != null); - var peWriter = new PeWriter(context.Module.Properties, context.Module.Win32Resources, context.Module.Win32ResourceSection, pdbPathOpt, deterministic); - var mdWriter = FullMetadataWriter.Create(context, messageProvider, allowMissingMethodBodies, deterministic, getPortablePdbStreamOpt != null, cancellationToken); + try + { + var peWriter = new PeWriter(context.Module.Properties, context.Module.Win32Resources, context.Module.Win32ResourceSection, pdbPathOpt, deterministic); + var mdWriter = FullMetadataWriter.Create(context, messageProvider, allowMissingMethodBodies, deterministic, getPortablePdbStreamOpt != null, cancellationToken); - return peWriter.WritePeToStream(mdWriter, getPeStream, getPortablePdbStreamOpt, nativePdbWriterOpt); + return peWriter.WritePeToStream(mdWriter, getPeStream, getPortablePdbStreamOpt, nativePdbWriterOpt); + } + catch (Exception ex) when (!(ex is PdbWritingException || ex is ResourceException || ex is PermissionSetFileReadException)) + { + throw new PeWritingException(ex); + } } private bool WritePeToStream(MetadataWriter mdWriter, Func getPeStream, Func getPortablePdbStreamOpt, PdbWriter nativePdbWriterOpt) diff --git a/src/Compilers/VisualBasic/Portable/Errors/Errors.vb b/src/Compilers/VisualBasic/Portable/Errors/Errors.vb index 2c8ac032348..efb2e6f67f6 100644 --- a/src/Compilers/VisualBasic/Portable/Errors/Errors.vb +++ b/src/Compilers/VisualBasic/Portable/Errors/Errors.vb @@ -1682,6 +1682,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic ERR_InvalidPathMap = 37253 ERR_PublicSignNoKey = 37254 ERR_TooManyUserStrings = 37255 + ERR_PeWritingFailure = 37256 ERR_LastPlusOne diff --git a/src/Compilers/VisualBasic/Portable/Errors/MessageProvider.vb b/src/Compilers/VisualBasic/Portable/Errors/MessageProvider.vb index b1a90827c2c..c6bbeb1a62b 100644 --- a/src/Compilers/VisualBasic/Portable/Errors/MessageProvider.vb +++ b/src/Compilers/VisualBasic/Portable/Errors/MessageProvider.vb @@ -480,6 +480,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Return ERRID.ERR_TooManyUserStrings End Get End Property + + Public Overrides ReadOnly Property ERR_PeWritingFailure As Integer + Get + Return ERRID.ERR_PeWritingFailure + End Get + End Property End Class End Namespace diff --git a/src/Compilers/VisualBasic/Portable/VBResources.resx b/src/Compilers/VisualBasic/Portable/VBResources.resx index a9dcac8d3a5..7d49b24dd86 100644 --- a/src/Compilers/VisualBasic/Portable/VBResources.resx +++ b/src/Compilers/VisualBasic/Portable/VBResources.resx @@ -5347,4 +5347,7 @@ Combined length of user strings used by the program exceeds allowed limit. Try to decrease use of string or XML literals. - \ No newline at end of file + + An error occurred while writing the output file. + + diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb index e5b9efc6c2b..f79884e8ffb 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb @@ -54,7 +54,8 @@ End Class , options:=options) comp.VerifyEmitDiagnostics( - Diagnostic(ERRID.ERR_PublicKeyContainerFailure).WithArguments("RoslynTestContainer", "This is a test IOException").WithLocation(1, 1)) + Diagnostic(ERRID.ERR_PeWritingFailure).WithArguments("This is a test IOException").WithLocation(1, 1)) + End Sub End Class diff --git a/src/Compilers/VisualBasic/Test/Emit/Emit/CompilationEmitTests.vb b/src/Compilers/VisualBasic/Test/Emit/Emit/CompilationEmitTests.vb index 79f1c616cc7..1af628835f5 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Emit/CompilationEmitTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Emit/CompilationEmitTests.vb @@ -2923,5 +2923,89 @@ End Class End Using End Sub + Private Class FailingStream + Inherits Stream + + Shared exception As IOException = New IOException() + + Public Overrides ReadOnly Property CanRead As Boolean + Get + Return True + End Get + End Property + + Public Overrides ReadOnly Property CanSeek As Boolean + Get + Return True + End Get + End Property + + Public Overrides ReadOnly Property CanWrite As Boolean + Get + Return True + End Get + End Property + + Public Overrides ReadOnly Property Length As Long + Get + Throw exception + End Get + End Property + + Public Overrides Property Position As Long + Get + Throw exception + End Get + Set(value As Long) + Throw exception + End Set + End Property + + Public Overrides Sub Flush() + Throw exception + End Sub + + Public Overrides Sub SetLength(value As Long) + Throw exception + End Sub + + Public Overrides Sub Write(buffer() As Byte, offset As Integer, count As Integer) + Throw exception + End Sub + + Public Overrides Function Read(buffer() As Byte, offset As Integer, count As Integer) As Integer + Throw exception + End Function + + Public Overrides Function Seek(offset As Long, origin As SeekOrigin) As Long + Throw exception + End Function + End Class + + + Public Sub FailingEmitter() + ' Check that Compilation.Emit actually produces compilation errors. + Dim compilation = CompilationUtils.CreateCompilationWithMscorlibAndVBRuntime( + + +Module M1 + Sub Main() + End Sub +End Module + + ) + + Dim emitResult As EmitResult + + Using output = New BrokenStream() + output.BreakHow = 0 + emitResult = compilation.Emit(output, Nothing, Nothing, Nothing) + End Using + + CompilationUtils.AssertTheseDiagnostics(emitResult.Diagnostics, + +BC37256: An error occurred while writing the Portable Executable file. +) + End Sub End Class End Namespace diff --git a/src/Test/Utilities/Shared/Mocks/TestMessageProvider.cs b/src/Test/Utilities/Shared/Mocks/TestMessageProvider.cs index 4e459dcebf6..291679c387a 100644 --- a/src/Test/Utilities/Shared/Mocks/TestMessageProvider.cs +++ b/src/Test/Utilities/Shared/Mocks/TestMessageProvider.cs @@ -362,5 +362,13 @@ public override int ERR_TooManyUserStrings throw new NotImplementedException(); } } + + public override int ERR_PeWritingFailure + { + get + { + throw new NotImplementedException(); + } + } } } diff --git a/src/Test/Utilities/Shared/Pe/BrokenStream.cs b/src/Test/Utilities/Shared/Pe/BrokenStream.cs new file mode 100644 index 00000000000..24c000eff7f --- /dev/null +++ b/src/Test/Utilities/Shared/Pe/BrokenStream.cs @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO; + +namespace Roslyn.Test.Utilities +{ + internal class BrokenStream : Stream + { + public int BreakHow; + + public override bool CanRead + { + get { return true; } + } + + public override bool CanSeek + { + get { return true; } + } + + public override bool CanWrite + { + get { return true; } + } + + public override void Flush() + { + } + + public override long Length + { + get + { + return 0; + } + } + + public override long Position + { + get + { + return 0; + } + set + { + if (BreakHow == 1) + throw new NotSupportedException(); + } + } + + public override int Read(byte[] buffer, int offset, int count) + { + return 0; + } + + public override long Seek(long offset, SeekOrigin origin) + { + return 0; + } + + public override void SetLength(long value) + { + if (BreakHow == 2) + throw new IOException(); + } + + public override void Write(byte[] buffer, int offset, int count) + { + if (BreakHow == 0) + throw new IOException(); + } + } +} -- GitLab