From ecd9b6dc6d5b6be579c4e0233784baddbe4359dc Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 22 Mar 2018 23:52:41 -0700 Subject: [PATCH] Remove workarounds --- .../Test/Emit/Emit/CompilationEmitTests.cs | 3 +- .../Test/Emit/Emit/DeterministicTests.cs | 2 +- .../CSharp/Test/Emit/PDB/PortablePdbTests.cs | 5 +- .../Core/Portable/Compilation/Compilation.cs | 2 +- .../PEWriter/DebugDirectoryExtensions.cs | 112 ---------------- .../Core/Portable/PEWriter/PeWriter.cs | 124 +----------------- .../Test/Emit/PDB/PortablePdbTests.vb | 5 +- src/Test/PdbUtilities/Reader/PdbValidation.cs | 1 - 8 files changed, 9 insertions(+), 245 deletions(-) delete mode 100644 src/Compilers/Core/Portable/PEWriter/DebugDirectoryExtensions.cs diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index 7a40be3c339..7a095560d71 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -16,7 +16,6 @@ using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Emit; using Microsoft.CodeAnalysis.Test.Utilities; -using Roslyn.Reflection.PortableExecutable; using Roslyn.Test.Utilities; using Roslyn.Utilities; using Xunit; @@ -2110,7 +2109,7 @@ public void RefAssembly_AllowEmbeddingPdb() var result = comp.Emit(output, metadataPEStream: metadataOutput, options: EmitOptions.Default.WithDebugInformationFormat(DebugInformationFormat.Embedded).WithIncludePrivateMembers(false)); - VerifyEmbeddedDebugInfo(output, new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryExtensions.PdbChecksumEntryType, DebugDirectoryEntryType.EmbeddedPortablePdb }); + VerifyEmbeddedDebugInfo(output, new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.EmbeddedPortablePdb }); VerifyEmbeddedDebugInfo(metadataOutput, new DebugDirectoryEntryType[] { DebugDirectoryEntryType.Reproducible }); } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs index be5158e6299..813ff6900b7 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs @@ -148,7 +148,7 @@ public void PlatformChangeTimestamp() PEReader peReader1 = new PEReader(result1.pe); PEReader peReader2 = new PEReader(result2.pe); Assert.Equal(Machine.Amd64, peReader1.PEHeaders.CoffHeader.Machine); - Assert.Equal((Machine)0xAA64, peReader2.PEHeaders.CoffHeader.Machine); + Assert.Equal(Machine.Arm64, peReader2.PEHeaders.CoffHeader.Machine); Assert.NotEqual(peReader1.PEHeaders.CoffHeader.TimeDateStamp, peReader2.PEHeaders.CoffHeader.TimeDateStamp); } diff --git a/src/Compilers/CSharp/Test/Emit/PDB/PortablePdbTests.cs b/src/Compilers/CSharp/Test/Emit/PDB/PortablePdbTests.cs index d471c0ce63c..e42e9443d53 100644 --- a/src/Compilers/CSharp/Test/Emit/PDB/PortablePdbTests.cs +++ b/src/Compilers/CSharp/Test/Emit/PDB/PortablePdbTests.cs @@ -13,7 +13,6 @@ using Microsoft.CodeAnalysis.Debugging; using Microsoft.CodeAnalysis.Emit; using Microsoft.CodeAnalysis.Test.Utilities; -using Roslyn.Reflection.PortableExecutable; using Roslyn.Test.Utilities; using Xunit; @@ -173,7 +172,7 @@ public static void Main() { var entries = peReader.ReadDebugDirectory(); - AssertEx.Equal(new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryExtensions.PdbChecksumEntryType, DebugDirectoryEntryType.EmbeddedPortablePdb }, entries.Select(e => e.Type)); + AssertEx.Equal(new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.EmbeddedPortablePdb }, entries.Select(e => e.Type)); var codeView = entries[0]; var checksum = entries[1]; @@ -233,7 +232,7 @@ public static void Main() { var entries = peReader.ReadDebugDirectory(); - AssertEx.Equal(new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryExtensions.PdbChecksumEntryType, DebugDirectoryEntryType.Reproducible, DebugDirectoryEntryType.EmbeddedPortablePdb }, entries.Select(e => e.Type)); + AssertEx.Equal(new[] { DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible, DebugDirectoryEntryType.EmbeddedPortablePdb }, entries.Select(e => e.Type)); var codeView = entries[0]; var checksum = entries[1]; diff --git a/src/Compilers/Core/Portable/Compilation/Compilation.cs b/src/Compilers/Core/Portable/Compilation/Compilation.cs index d77c756e391..496ede90646 100644 --- a/src/Compilers/Core/Portable/Compilation/Compilation.cs +++ b/src/Compilers/Core/Portable/Compilation/Compilation.cs @@ -1564,7 +1564,7 @@ internal void SetupWin32Resources(CommonPEModuleBuilder moduleBeingBuilt, Stream switch (platform) { case Platform.Arm64: - machine = (Machine)0xAA64; //Machine.Arm64; https://github.com/dotnet/roslyn/issues/25185 + machine = Machine.Arm64; break; case Platform.Arm: diff --git a/src/Compilers/Core/Portable/PEWriter/DebugDirectoryExtensions.cs b/src/Compilers/Core/Portable/PEWriter/DebugDirectoryExtensions.cs deleted file mode 100644 index e759b2595da..00000000000 --- a/src/Compilers/Core/Portable/PEWriter/DebugDirectoryExtensions.cs +++ /dev/null @@ -1,112 +0,0 @@ -// 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.Diagnostics; -using System.IO; -using System.Reflection; -using System.Reflection.Metadata; -using System.Reflection.PortableExecutable; -using Roslyn.Utilities; - -namespace Roslyn.Reflection.PortableExecutable -{ - // TODO: move to SRM: https://github.com/dotnet/roslyn/issues/24712 - internal static class DebugDirectoryExtensions - { - private static Lazy<(FieldInfo, Action)> s_debugDirectoryBuilderMembers - = new Lazy<(FieldInfo, Action)>(() => - { - var type = typeof(DebugDirectoryBuilder).GetTypeInfo(); - return ( - type.GetDeclaredField("_dataBuilder"), - (Action) - type.GetDeclaredMethod("AddEntry", typeof(DebugDirectoryEntryType), typeof(uint), typeof(uint), typeof(int)). - CreateDelegate(typeof(Action))); - }); - - internal const DebugDirectoryEntryType PdbChecksumEntryType = (DebugDirectoryEntryType)19; - - public static void AddPdbChecksumEntry(this DebugDirectoryBuilder builder, string algorithmName, ImmutableArray checksum) - { - var (dataBuilder, addEntry) = s_debugDirectoryBuilderMembers.Value; - int dataSize = WritePdbChecksumData((BlobBuilder)dataBuilder.GetValue(builder), algorithmName, checksum); - addEntry(builder, PdbChecksumEntryType, 0x00000001, 0x00000000, dataSize); - } - - private static int WritePdbChecksumData(BlobBuilder builder, string algorithmName, ImmutableArray checksum) - { - int start = builder.Count; - - // NUL-terminated algorithm name: - builder.WriteUTF8(algorithmName, allowUnpairedSurrogates: true); - builder.WriteByte(0); - - // checksum: - builder.WriteBytes(checksum); - - return builder.Count - start; - } - - /// - /// Reads the data pointed to by the specified Debug Directory entry and interprets them as PDB Checksum entry. - /// - /// is not a PDB Checksum entry. - /// Bad format of the data. - /// IO error while reading from the underlying stream. - /// PE image not available. - internal static PdbChecksumDebugDirectoryData ReadPdbChecksumDebugDirectoryData(this PEReader peReader, DebugDirectoryEntry entry) - { - if (entry.Type != PdbChecksumEntryType) - { - throw new ArgumentException("Unexpected debug directory entry type", nameof(entry)); - } - - var peImage = peReader.GetEntireImage(); - - int dataOffset = peReader.IsLoadedImage ? entry.DataRelativeVirtualAddress : entry.DataPointer; - var reader = peImage.GetReader(dataOffset, entry.DataSize); - - int nameLength = reader.IndexOf(0); - if (nameLength <= 0) - { - throw new BadImageFormatException("Invalid PDB Checksum data format"); - } - - string algorithmName = reader.ReadUTF8(nameLength); - - // NUL - reader.Offset += 1; - - var checksum = reader.ReadBytes(reader.RemainingBytes).ToImmutableArray(); - if (checksum.Length == 0) - { - throw new BadImageFormatException("Invalid PDB Checksum data format"); - } - - return new PdbChecksumDebugDirectoryData(algorithmName, checksum); - } - } - - internal readonly struct PdbChecksumDebugDirectoryData - { - /// - /// Checksum algorithm name. - /// - public string AlgorithmName { get; } - - /// - /// Checksum of the associated PDB. - /// - public ImmutableArray Checksum { get; } - - internal PdbChecksumDebugDirectoryData(string algorithmName, ImmutableArray checksum) - { - Debug.Assert(!string.IsNullOrEmpty(algorithmName)); - Debug.Assert(!checksum.IsDefaultOrEmpty); - - AlgorithmName = algorithmName; - Checksum = checksum; - } - } -} diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs index 08844179a74..90e9b1fd219 100644 --- a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs @@ -15,7 +15,6 @@ using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Emit; -using Roslyn.Reflection.PortableExecutable; using static Microsoft.CodeAnalysis.SigningUtilities; using EmitContext = Microsoft.CodeAnalysis.Emit.EmitContext; using Microsoft.DiaSymReader; @@ -31,44 +30,6 @@ public PeWritingException(Exception inner) internal static class PeWriter { - // Remove this function when https://github.com/dotnet/roslyn/issues/25185 is fixed - private static byte ReadByteFromBlobBuilder(BlobBuilder blobBuilder, int offset) - { - BlobBuilder.Blobs blobs = blobBuilder.GetBlobs(); - int startOffsetOfBlob = 0; - foreach (Blob b in blobs) - { - ArraySegment bytesInBlob = b.GetBytes(); - int offsetInBlob = offset - startOffsetOfBlob; - - if (offsetInBlob < bytesInBlob.Count) - return ((IList)bytesInBlob)[offsetInBlob]; - - startOffsetOfBlob += bytesInBlob.Count; - } - throw new IndexOutOfRangeException(); - } - - // Remove this function when https://github.com/dotnet/roslyn/issues/25185 is fixed - private static void WriteByteToBlobBuilder(BlobBuilder blobBuilder, int offset, byte data) - { - BlobBuilder.Blobs blobs = blobBuilder.GetBlobs(); - int startOffsetOfBlob = 0; - foreach (Blob b in blobs) - { - ArraySegment bytesInBlob = b.GetBytes(); - int offsetInBlob = offset - startOffsetOfBlob; - - if (offsetInBlob < bytesInBlob.Count) - { - ((IList)bytesInBlob)[offsetInBlob] = data; - return; - } - - startOffsetOfBlob += bytesInBlob.Count; - } - } - internal static bool WritePeToStream( EmitContext context, CommonMessageProvider messageProvider, @@ -158,17 +119,8 @@ private static void WriteByteToBlobBuilder(BlobBuilder blobBuilder, int offset, ushort portablePdbVersion = 0; var metadataRootBuilder = mdWriter.GetRootBuilder(); - Machine SRMVisibleMachine = properties.Machine; - - // When attempting to write an Arm64 PE file, tell the PEBuilder that an Amd64 pe is being written instead - // then replace the bits in the produced PE header with the correct bits. This will allow an Arm64 pe to be - // generated without requiring the System.Reflection.Metadata dll to be updated to a newer version. - // Remove this code when https://github.com/dotnet/roslyn/issues/25185 is fixed - if (SRMVisibleMachine == (Machine)0xAA64) - SRMVisibleMachine = Machine.Amd64; - var peHeaderBuilder = new PEHeaderBuilder( - machine: SRMVisibleMachine, + machine: properties.Machine, sectionAlignment: properties.SectionAlignment, fileAlignment: properties.FileAlignment, imageBase: properties.BaseAddress, @@ -288,61 +240,13 @@ private static void WriteByteToBlobBuilder(BlobBuilder blobBuilder, int offset, var peBlob = new BlobBuilder(); var peContentId = peBuilder.Serialize(peBlob, out Blob mvidSectionFixup); - - // Remove this code when https://github.com/dotnet/roslyn/issues/25185 is fixed - if (SRMVisibleMachine != properties.Machine) - { - // ARM64 pe - // Update Amd64 machine type in PE header to 0xAA64 - // Machine type in PEHeader is located at... - // 128 bytes (dos header) - // 4 bytes (PE Signature) - // 2 bytes (Machine) - // 2 bytes (NumberOfSections) - // 4 bytes (TimeDateStamp) - int machineOffset = 128 + 4; - int timeDateStampOffset = 128 + 4 + 2 + 2; - ushort amd64MachineType = (ushort)Machine.Amd64; - Debug.Assert(ReadByteFromBlobBuilder(peBlob, machineOffset) == (byte)(amd64MachineType)); - Debug.Assert(ReadByteFromBlobBuilder(peBlob, machineOffset + 1) == (byte)(amd64MachineType >> 8)); - - ushort realMachineType = (ushort)properties.Machine; - byte lsbMachineType = (byte)realMachineType; - byte msbMachineType = (byte)(realMachineType >> 8); - - // This code path is currently only expected to be used for ARM64 - Debug.Assert(lsbMachineType == 0x64); - Debug.Assert(msbMachineType == 0xAA); - - WriteByteToBlobBuilder(peBlob, machineOffset, lsbMachineType); - WriteByteToBlobBuilder(peBlob, machineOffset + 1, msbMachineType); - - if (peIdProvider != null) - { - // Patch timestamp in COFF Header to all zeroes - WriteByteToBlobBuilder(peBlob, timeDateStampOffset, 0); - WriteByteToBlobBuilder(peBlob, timeDateStampOffset + 1, 0); - WriteByteToBlobBuilder(peBlob, timeDateStampOffset + 2, 0); - WriteByteToBlobBuilder(peBlob, timeDateStampOffset + 3, 0); - - peContentId = peIdProvider(peBlob.GetBlobs()); - - // patch timestamp in COFF header: - uint newTimestamp = peContentId.Stamp; - WriteByteToBlobBuilder(peBlob, timeDateStampOffset, (byte)newTimestamp); - WriteByteToBlobBuilder(peBlob, timeDateStampOffset + 1, (byte)(newTimestamp >> 8)); - WriteByteToBlobBuilder(peBlob, timeDateStampOffset + 2, (byte)(newTimestamp >> 16)); - WriteByteToBlobBuilder(peBlob, timeDateStampOffset + 3, (byte)(newTimestamp >> 24)); - } - } - + PatchModuleVersionIds(mvidFixup, mvidSectionFixup, mvidStringFixup, peContentId.Guid); if (privateKeyOpt != null && corFlags.HasFlag(CorFlags.StrongNameSigned)) { Debug.Assert(strongNameProvider.Capability == SigningCapability.SignsPeBuilder); strongNameProvider.SignPeBuilder(peBuilder, peBlob, privateKeyOpt.Value); - FixupChecksum(peBuilder, peBlob); } try @@ -357,30 +261,6 @@ private static void WriteByteToBlobBuilder(BlobBuilder blobBuilder, int offset, return true; } - private static void FixupChecksum(ExtendedPEBuilder peBuilder, BlobBuilder peBlob) - { - // Checksum fixup, workaround for https://github.com/dotnet/corefx/issues/25829 - // Tracked by https://github.com/dotnet/roslyn/issues/23762 - // Since the checksum is calculated before signing in the PEBuilder, - // we need to redo the calculation and write in the correct checksum - Blob checksumBlob = getChecksumBlob(peBuilder); - - ArraySegment checksumSegment = checksumBlob.GetBytes(); - uint oldChecksum = BitConverter.ToUInt32(checksumSegment.Array, checksumSegment.Offset); - uint newChecksum = CalculateChecksum(peBlob, checksumBlob); - new BlobWriter(checksumBlob).WriteUInt32(newChecksum); - - // If this assert fires, the above bug has been fixed and this workaround should - // be removed - Debug.Assert(oldChecksum != newChecksum); - - Blob getChecksumBlob(PEBuilder builder) - => (Blob)typeof(PEBuilder).GetRuntimeFields() - .Where(f => f.Name == "_lazyChecksum") - .Single() - .GetValue(builder); - } - private static MethodInfo s_calculateChecksumMethod; // internal for testing internal static uint CalculateChecksum(BlobBuilder peBlob, Blob checksumBlob) diff --git a/src/Compilers/VisualBasic/Test/Emit/PDB/PortablePdbTests.vb b/src/Compilers/VisualBasic/Test/Emit/PDB/PortablePdbTests.vb index 0a234431bd2..02cb4a13916 100644 --- a/src/Compilers/VisualBasic/Test/Emit/PDB/PortablePdbTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/PDB/PortablePdbTests.vb @@ -8,7 +8,6 @@ Imports System.Text Imports Microsoft.CodeAnalysis.Debugging Imports Microsoft.CodeAnalysis.Emit Imports Microsoft.CodeAnalysis.Test.Utilities -Imports Roslyn.Reflection.PortableExecutable Imports Roslyn.Test.Utilities Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests.PDB @@ -82,7 +81,7 @@ End Class Using peReader = New PEReader(peBlob) Dim entries = peReader.ReadDebugDirectory() - AssertEx.Equal({DebugDirectoryEntryType.CodeView, DebugDirectoryExtensions.PdbChecksumEntryType, DebugDirectoryEntryType.EmbeddedPortablePdb}, entries.Select(Function(e) e.Type)) + AssertEx.Equal({DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.EmbeddedPortablePdb}, entries.Select(Function(e) e.Type)) Dim codeView = entries(0) Dim checksum = entries(1) @@ -135,7 +134,7 @@ End Class Using peReader = New PEReader(peBlob) Dim entries = peReader.ReadDebugDirectory() - AssertEx.Equal({DebugDirectoryEntryType.CodeView, DebugDirectoryExtensions.PdbChecksumEntryType, DebugDirectoryEntryType.Reproducible, DebugDirectoryEntryType.EmbeddedPortablePdb}, entries.Select(Function(e) e.Type)) + AssertEx.Equal({DebugDirectoryEntryType.CodeView, DebugDirectoryEntryType.PdbChecksum, DebugDirectoryEntryType.Reproducible, DebugDirectoryEntryType.EmbeddedPortablePdb}, entries.Select(Function(e) e.Type)) Dim codeView = entries(0) Dim checksum = entries(1) diff --git a/src/Test/PdbUtilities/Reader/PdbValidation.cs b/src/Test/PdbUtilities/Reader/PdbValidation.cs index f9712c4c6bb..a088d6c112d 100644 --- a/src/Test/PdbUtilities/Reader/PdbValidation.cs +++ b/src/Test/PdbUtilities/Reader/PdbValidation.cs @@ -20,7 +20,6 @@ using Microsoft.DiaSymReader; using Microsoft.DiaSymReader.Tools; using Microsoft.Metadata.Tools; -using Roslyn.Reflection.PortableExecutable; using Roslyn.Test.PdbUtilities; using Roslyn.Test.Utilities; using Roslyn.Utilities; -- GitLab