From d6a8f32b0269debd5e4b08dfa67c0a8cd89c6018 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 1 Jun 2015 15:24:01 -0700 Subject: [PATCH] Catch exceptions when creating a stream for signing This change is meant to address crashes like the ones in bug 1140649. The crashes seem to happen due to exceptions thrown while getting a temporary output stream to write the PE file to. I have been unable to reproduce the conditions for the crash themselves -- they only seem to happen on a Japanese language Windows with a Japanese language copy of Visual Studio. This fix addresses their symptom by wrapping the CreateInputStream call during signing with a try/catch and then surfacing the exception as a diagnostic for signing failure. We cannot currently add any more localized resources to stabilization, so this fix is meant to be as targeted as possible and can only deliver information through existing resource strings. (cherry picked from commit 801a10a5257ae08eefbfb9b4beb40fa806ec59c4) --- .../Attributes/EmitTestStrongNameProvider.cs | 61 +++++++++++++++++++ .../InternalsVisibleToAndStrongNameTests.cs | 2 +- .../Test/Emit/CSharpCompilerEmitTest.csproj | 5 +- .../Core/Portable/Compilation/Compilation.cs | 19 +++++- .../Core/Portable/PEWriter/PeWriter.cs | 7 +++ .../Attributes/EmitTestStrongNameProvider.vb | 60 ++++++++++++++++++ .../InternalsVisibleToAndStrongNameTests.vb | 18 +++--- .../Test/Emit/BasicCompilerEmitTest.vbproj | 3 +- 8 files changed, 160 insertions(+), 15 deletions(-) create mode 100644 src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs create mode 100644 src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs b/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs new file mode 100644 index 00000000000..6f67bc26092 --- /dev/null +++ b/src/Compilers/CSharp/Test/Emit/Attributes/EmitTestStrongNameProvider.cs @@ -0,0 +1,61 @@ +// 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 Microsoft.CodeAnalysis.CSharp.Test.Utilities; +using System; +using System.IO; +using Xunit; + +namespace Microsoft.CodeAnalysis.CSharp.UnitTests +{ + public partial class InternalsVisibleToAndStrongNameTests : CSharpTestBase + { + /// + /// A strong name provider which throws an IOException while creating + /// the input stream. + /// + private class StrongNameProviderWithBadInputStream : StrongNameProvider + { + private StrongNameProvider _underlyingProvider; + public StrongNameProviderWithBadInputStream(StrongNameProvider underlyingProvider) + { + _underlyingProvider = underlyingProvider; + } + + public override bool Equals(object other) => this == other; + + public override int GetHashCode() => _underlyingProvider.GetHashCode(); + + internal override Stream CreateInputStream() + { + throw new IOException("This is a test IOException"); + } + + internal override StrongNameKeys CreateKeys(string keyFilePath, string keyContainerName, CommonMessageProvider messageProvider) => + _underlyingProvider.CreateKeys(keyFilePath, keyContainerName, messageProvider); + + internal override void SignAssembly(StrongNameKeys keys, Stream inputStream, Stream outputStream) => + _underlyingProvider.SignAssembly(keys, inputStream, outputStream); + } + + [Fact] + public void BadInputStream() + { + string src = @" +class C +{ + public static void Main(string[] args) { } +}"; + var testProvider = new StrongNameProviderWithBadInputStream(s_defaultProvider); + var options = TestOptions.DebugExe + .WithStrongNameProvider(testProvider) + .WithCryptoKeyContainer("RoslynTestContainer"); + + var comp = CreateCompilationWithMscorlib(src, + 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)); + } + } +} diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs b/src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs index 3bbf1da685b..b0d454ef374 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests { - public class InternalsVisibleToAndStrongNameTests : CSharpTestBase + public partial class InternalsVisibleToAndStrongNameTests : CSharpTestBase { #region Helpers diff --git a/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj b/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj index 18ed848605a..0a8f2891840 100644 --- a/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj +++ b/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj @@ -58,7 +58,7 @@ ..\..\..\..\..\packages\xunit.1.9.2\lib\net20\xunit.dll - + true @@ -151,6 +151,7 @@ + @@ -205,4 +206,4 @@ - + \ No newline at end of file diff --git a/src/Compilers/Core/Portable/Compilation/Compilation.cs b/src/Compilers/Core/Portable/Compilation/Compilation.cs index 2531a97f260..ce6bdf39c2b 100644 --- a/src/Compilers/Core/Portable/Compilation/Compilation.cs +++ b/src/Compilers/Core/Portable/Compilation/Compilation.cs @@ -1665,8 +1665,17 @@ private static EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool su { Debug.Assert(Options.StrongNameProvider != null); - signingInputStream = Options.StrongNameProvider.CreateInputStream(); - retStream = signingInputStream; + // 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); + } } else { @@ -1720,6 +1729,12 @@ private static EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool su diagnostics.Add(MessageProvider.CreateDiagnostic(MessageProvider.ERR_PdbWritingFailed, Location.None, ex.Message)); return false; } + 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)); + } catch (ResourceException e) { diagnostics.Add(MessageProvider.CreateDiagnostic(MessageProvider.ERR_CantReadResource, Location.None, e.Message, e.InnerException.Message)); diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs index 46689fd83a6..df8e71eeac5 100644 --- a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs @@ -14,6 +14,13 @@ namespace Microsoft.Cci { + internal sealed class PeWritingException : Exception + { + public PeWritingException(Exception inner) + : base(inner.Message, inner) + { } + } + internal sealed class PeWriter { /// diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb new file mode 100644 index 00000000000..e5b9efc6c2b --- /dev/null +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/EmitTestStrongNameProvider.vb @@ -0,0 +1,60 @@ +' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +Imports System.IO +Imports Microsoft.CodeAnalysis +Imports Microsoft.CodeAnalysis.VisualBasic +Imports Microsoft.CodeAnalysis.VisualBasic.UnitTests + +Partial Public Class InternalsVisibleToAndStrongNameTests + Inherits BasicTestBase + + Private Class StrongNameProviderWithBadInputStream + Inherits StrongNameProvider + Private _underlyingProvider As StrongNameProvider + Public Sub New(underlyingProvider As StrongNameProvider) + _underlyingProvider = underlyingProvider + End Sub + + + Public Overrides Function Equals(other As Object) As Boolean + Return Me Is other + End Function + + Public Overrides Function GetHashCode() As Integer + Return _underlyingProvider.GetHashCode() + End Function + + Friend Overrides Function CreateInputStream() As Stream + Throw New IOException("This is a test IOException") + End Function + + Friend Overrides Function CreateKeys(keyFilePath As String, keyContainerName As String, messageProvider As CommonMessageProvider) As StrongNameKeys + Return _underlyingProvider.CreateKeys(keyFilePath, keyContainerName, messageProvider) + End Function + + Friend Overrides Sub SignAssembly(keys As StrongNameKeys, inputStream As Stream, outputStream As Stream) + _underlyingProvider.SignAssembly(keys, inputStream, outputStream) + End Sub + End Class + + + Public Sub BadInputStream() + Dim testProvider = New StrongNameProviderWithBadInputStream(s_defaultProvider) + Dim options = TestOptions.DebugDll.WithStrongNameProvider(testProvider).WithCryptoKeyContainer("RoslynTestContainer") + + Dim comp = CreateCompilationWithMscorlib( + + + + , options:=options) + + comp.VerifyEmitDiagnostics( + Diagnostic(ERRID.ERR_PublicKeyContainerFailure).WithArguments("RoslynTestContainer", "This is a test IOException").WithLocation(1, 1)) + End Sub + +End Class diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.vb index 1bed2248edb..1726f15b92d 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.vb @@ -13,7 +13,7 @@ Imports Microsoft.CodeAnalysis.VisualBasic Imports Microsoft.CodeAnalysis.VisualBasic.UnitTests Imports Roslyn.Test.Utilities -Public Class InternalsVisibleToAndStrongNameTests +Partial Public Class InternalsVisibleToAndStrongNameTests Inherits BasicTestBase #Region "Helpers" @@ -417,7 +417,7 @@ End Class Dim err = other.GetDeclarationDiagnostics().Single() Assert.Equal(ERRID.ERR_PublicKeyContainerFailure, err.Code) - Assert.Equal(2, Err.Arguments.Count) + Assert.Equal(2, err.Arguments.Count) Assert.Equal("foo", DirectCast(err.Arguments(0), String)) Assert.True(DirectCast(err.Arguments(1), String).EndsWith(" HRESULT: 0x80090016)", StringComparison.Ordinal)) @@ -1652,7 +1652,7 @@ End Class Public Sub Bug1072350() - Dim sourceA As XElement = _ + Dim sourceA As XElement = @@ -1662,7 +1662,7 @@ End Class]]> - Dim sourceB As XElement = _ + Dim sourceB As XElement = Dim ca = CreateCompilationWithMscorlib(sourceA, options:=TestOptions.ReleaseDll) CompileAndVerify(ca) - Dim cb = CreateCompilationWithMscorlib(sourceB, options:=TestOptions.ReleaseExe, references:={new VisualBasicCompilationReference(ca)}) + Dim cb = CreateCompilationWithMscorlib(sourceB, options:=TestOptions.ReleaseExe, references:={New VisualBasicCompilationReference(ca)}) CompileAndVerify(cb, expectedOutput:="42", emitters:=TestEmitters.CCI).Diagnostics.Verify() End Sub Public Sub Bug1072339() - Dim sourceA As XElement = _ + Dim sourceA As XElement = @@ -1692,7 +1692,7 @@ End Class]]> - Dim sourceB As XElement = _ + Dim sourceB As XElement = Dim ca = CreateCompilationWithMscorlib(sourceA, options:=TestOptions.ReleaseDll) CompileAndVerify(ca) - Dim cb = CreateCompilationWithMscorlib(sourceB, options:=TestOptions.ReleaseExe, references:={new VisualBasicCompilationReference(ca)}) + Dim cb = CreateCompilationWithMscorlib(sourceB, options:=TestOptions.ReleaseExe, references:={New VisualBasicCompilationReference(ca)}) CompileAndVerify(cb, expectedOutput:="42", emitters:=TestEmitters.CCI).Diagnostics.Verify() End Sub Public Sub Bug1095618() - Dim source As XElement = _ + Dim source As XElement = diff --git a/src/Compilers/VisualBasic/Test/Emit/BasicCompilerEmitTest.vbproj b/src/Compilers/VisualBasic/Test/Emit/BasicCompilerEmitTest.vbproj index 3804577cdf3..a8899ea4bb5 100644 --- a/src/Compilers/VisualBasic/Test/Emit/BasicCompilerEmitTest.vbproj +++ b/src/Compilers/VisualBasic/Test/Emit/BasicCompilerEmitTest.vbproj @@ -106,6 +106,7 @@ + @@ -281,4 +282,4 @@ - + \ No newline at end of file -- GitLab