From 53a66be0819745beff06cfacc80f9fa5bb7f6909 Mon Sep 17 00:00:00 2001 From: jcouv Date: Sat, 29 Apr 2017 23:19:34 -0700 Subject: [PATCH 1/9] Add .mvid section to PE and remove dependency on MetadataReader --- .../Test/Emit/CSharpCompilerEmitTest.csproj | 3 + .../Test/Emit/Emit/CompilationEmitTests.cs | 32 ++++ .../Core/MSBuildTask/CopyRefAssembly.cs | 6 +- .../Core/MSBuildTask/MSBuildTask.csproj | 1 + src/Compilers/Core/MSBuildTask/MvidReader.cs | 143 ++++++++++++++++++ src/Compilers/Core/MSBuildTask/project.json | 1 - .../Core/Portable/PEWriter/PeWriter.cs | 106 ++++++++++++- 7 files changed, 280 insertions(+), 12 deletions(-) create mode 100755 src/Compilers/Core/MSBuildTask/MvidReader.cs diff --git a/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj b/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj index 2b69608a19d3d..89673c5a909f2 100644 --- a/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj +++ b/src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj @@ -55,6 +55,9 @@ + + Emit\MvidReader.cs + diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index abc39c4f664aa..8ce846650ab69 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -262,9 +262,11 @@ internal static void Main() VerifyEntryPoint(output, expectZero: false); VerifyMethods(output, new[] { "void C.Main()", "C..ctor()" }); + VerifyMVID(output, hasMvidSection: false); VerifyEntryPoint(metadataOutput, expectZero: true); VerifyMethods(metadataOutput, new[] { "C..ctor()" }); + VerifyMVID(metadataOutput, hasMvidSection: true); } void VerifyEntryPoint(MemoryStream stream, bool expectZero) @@ -275,6 +277,34 @@ void VerifyEntryPoint(MemoryStream stream, bool expectZero) } } + /// + /// Extract the MVID using two different methods (PEReader and MvidReader) and compare them. + /// We only expect an .mvid section in ref assemblies. + /// + void VerifyMVID(MemoryStream stream, bool hasMvidSection) + { + Guid mvidFromModuleDefinition; + stream.Position = 0; + using (var reader = new PEReader(stream)) + { + var metadataReader = reader.GetMetadataReader(); + mvidFromModuleDefinition = metadataReader.GetGuid(metadataReader.GetModuleDefinition().Mvid); + + stream.Position = 0; + var mvidFromMvidReader = BuildTasks.MvidReader.ReadAssemblyMvid(stream); + + if (hasMvidSection) + { + Assert.Equal(mvidFromModuleDefinition, mvidFromMvidReader); + } + else + { + Assert.NotEqual(Guid.Empty, mvidFromModuleDefinition); + Assert.Equal(Guid.Empty, mvidFromMvidReader); + } + } + } + [Fact] public void EmitRefAssembly_PrivatePropertySetter() { @@ -296,6 +326,8 @@ public class C VerifyMethods(output, new[] { "System.Int32 C.k__BackingField", "System.Int32 C.PrivateSetter.get", "void C.PrivateSetter.set", "C..ctor()", "System.Int32 C.PrivateSetter { get; private set; }" }); VerifyMethods(metadataOutput, new[] { "System.Int32 C.PrivateSetter.get", "C..ctor()", "System.Int32 C.PrivateSetter { get; }" }); + VerifyMVID(output, hasMvidSection: false); + VerifyMVID(metadataOutput, hasMvidSection: true); } } diff --git a/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs b/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs index 1c2f34c70646a..ef0107f44ff8d 100644 --- a/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs +++ b/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs @@ -2,8 +2,6 @@ using System; using System.IO; -using System.Reflection.Metadata; -using System.Reflection.PortableExecutable; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; @@ -85,10 +83,8 @@ private bool Copy() private Guid ExtractMvid(string path) { using (FileStream source = File.OpenRead(path)) - using (var reader = new PEReader(source)) { - var metadataReader = reader.GetMetadataReader(); - return metadataReader.GetGuid(metadataReader.GetModuleDefinition().Mvid); + return MvidReader.ReadAssemblyMvid(source); } } } diff --git a/src/Compilers/Core/MSBuildTask/MSBuildTask.csproj b/src/Compilers/Core/MSBuildTask/MSBuildTask.csproj index cbced79b1b505..9c161e6aa519b 100644 --- a/src/Compilers/Core/MSBuildTask/MSBuildTask.csproj +++ b/src/Compilers/Core/MSBuildTask/MSBuildTask.csproj @@ -55,6 +55,7 @@ + diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs new file mode 100755 index 0000000000000..831202cc18812 --- /dev/null +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -0,0 +1,143 @@ +// 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.Diagnostics; +using System.IO; + +namespace Microsoft.CodeAnalysis.BuildTasks +{ + public sealed class MvidReader : BinaryReader + { + public MvidReader(Stream stream) : base(stream) + { + } + + public static Guid ReadAssemblyMvid(Stream stream) + { + try + { + var mvidReader = new MvidReader(stream); + return mvidReader.FindMvid(); + } + catch (EndOfStreamException) + { + } + + return Guid.Empty; + } + + public Guid FindMvid() + { + Guid empty = Guid.Empty; + + // DOS Header (64), DOS Stub (64), PE Signature (4), COFF Header (20), PE Header (224) + if (BaseStream.Length < 64 + 64 + 4 + 20 + 224) + { + return empty; + } + + // DOS Header: PE (2) + if (ReadUInt16() != 0x5a4d) + { + return empty; + } + + // DOS Header: Start (58) + Skip(58); + + // DOS Header: Address of PE Signature + MoveTo(ReadUInt32()); + + // PE Signature ('P' 'E' null null) + if (ReadUInt32() != 0x00004550) + { + return empty; + } + + // COFF Header: Machine (2) + Skip(2); + + // COFF Header: NumberOfSections (2) + ushort sections = ReadUInt16(); + + // COFF Header: TimeDateStamp (4), PointerToSymbolTable (4), NumberOfSymbols (4) + Skip(12); + + // COFF Header: OptionalHeaderSize (2) + ushort optionalHeaderSize = ReadUInt16(); + + // COFF Header: Characteristics (2) + Skip(2); + + // Optional header + Skip(optionalHeaderSize); + + // Section headers + return FindMvidInSections(sections); + } + + string ReadSectionName() + { + int length = 8; + int read = 0; + var buffer = new char[length]; + var bytes = ReadBytes(length); + while (read < length) + { + var current = bytes[read]; + if (current == 0) + break; + + buffer[read++] = (char)current; + } + + return new string(buffer, 0, read); + } + + Guid FindMvidInSections(ushort count) + { + for (int i = 0; i < count; i++) + { + // Section: Name (8) + string name = ReadSectionName(); + + // Section: VirtualSize (4) + uint virtualSize = ReadUInt32(); + + // Section: VirtualAddress (4), SizeOfRawData (4) + Skip(8); + + // Section: PointerToRawData (4) + uint pointerToRawData = ReadUInt32(); + + // Section: PointerToRelocations (4), PointerToLineNumbers (4), NumberOfRelocations (2), + // NumberOfLineNumbers (2), Characteristics (4) + Skip(16); + + if (name.Equals(".mvid", StringComparison.Ordinal)) + { + // The .mvid section only stores a Guid + Debug.Assert(virtualSize == 16); + + BaseStream.Position = pointerToRawData; + byte[] guidBytes = new byte[16]; + BaseStream.Read(guidBytes, 0, 16); + + return new Guid(guidBytes); + } + } + + return Guid.Empty; + } + + public void Skip(int bytes) + { + BaseStream.Seek(bytes, SeekOrigin.Current); + } + + public void MoveTo(uint position) + { + BaseStream.Seek(position, SeekOrigin.Begin); + } + } +} diff --git a/src/Compilers/Core/MSBuildTask/project.json b/src/Compilers/Core/MSBuildTask/project.json index c9efa1ac00175..ac0c51a5b3082 100644 --- a/src/Compilers/Core/MSBuildTask/project.json +++ b/src/Compilers/Core/MSBuildTask/project.json @@ -15,7 +15,6 @@ "System.IO.Pipes": "4.3.0", "System.Linq": "4.3.0", "System.Reflection": "4.3.0", - "System.Reflection.Metadata": "1.4.2", "System.Security.AccessControl": "4.3.0", "System.Security.Cryptography.Algorithms": "4.3.0", "System.Security.Principal.Windows": "4.3.0", diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs index cd9011bb617ab..344667b4f8e49 100644 --- a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs @@ -69,7 +69,7 @@ internal static bool WritePeToStream( MethodDefinitionHandle entryPointHandle; MethodDefinitionHandle debugEntryPointHandle; mdWriter.GetEntryPoints(out entryPointHandle, out debugEntryPointHandle); - + if (!debugEntryPointHandle.IsNil) { nativePdbWriterOpt?.SetEntryPoint((uint)MetadataTokens.GetToken(debugEntryPointHandle)); @@ -186,7 +186,7 @@ internal static bool WritePeToStream( debugDirectoryBuilder = null; } - var peBuilder = new ManagedPEBuilder( + var peBuilder = new ExtendedPEBuilder( peHeaderBuilder, metadataRootBuilder, ilBuilder, @@ -197,12 +197,13 @@ internal static bool WritePeToStream( CalculateStrongNameSignatureSize(context.Module), entryPointHandle, properties.CorFlags, - deterministicIdProvider); + deterministicIdProvider, + metadataOnly && !context.IncludePrivateMembers); var peBlob = new BlobBuilder(); - var peContentId = peBuilder.Serialize(peBlob); + var peContentId = peBuilder.Serialize(peBlob, out Blob mvidSectionFixup); - PatchModuleVersionIds(mvidFixup, mvidStringFixup, peContentId.Guid); + PatchModuleVersionIds(mvidFixup, mvidSectionFixup, mvidStringFixup, peContentId.Guid); try { @@ -216,7 +217,7 @@ internal static bool WritePeToStream( return true; } - private static void PatchModuleVersionIds(Blob guidFixup, Blob stringFixup, Guid mvid) + private static void PatchModuleVersionIds(Blob guidFixup, Blob guidSectionFixup, Blob stringFixup, Guid mvid) { if (!guidFixup.IsDefault) { @@ -225,6 +226,13 @@ private static void PatchModuleVersionIds(Blob guidFixup, Blob stringFixup, Guid Debug.Assert(writer.RemainingBytes == 0); } + if (!guidSectionFixup.IsDefault) + { + var writer = new BlobWriter(guidSectionFixup); + writer.WriteGuid(mvid); + Debug.Assert(writer.RemainingBytes == 0); + } + if (!stringFixup.IsDefault) { var writer = new BlobWriter(stringFixup); @@ -322,5 +330,91 @@ private static int CalculateStrongNameSignatureSize(CommonPEModuleBuilder module return (keySize < 128 + 32) ? 128 : keySize - 32; } + + /// + /// This PEBuilder adds an .mvid section. + /// + private class ExtendedPEBuilder : ManagedPEBuilder + { + private const string MvidSectionName = ".mvid"; + public const int SizeOfGuid = 16; + + // When the section is built with a placeholder, the placeholder blob is saved for later fixing up. + private Blob _mvidSectionFixup = default(Blob); + + // Only include the .mvid section in ref assemblies + private bool _withMvidSection; + + public ExtendedPEBuilder( + PEHeaderBuilder header, + MetadataRootBuilder metadataRootBuilder, + BlobBuilder ilStream, + BlobBuilder mappedFieldData, + BlobBuilder managedResources, + ResourceSectionBuilder nativeResources, + DebugDirectoryBuilder debugDirectoryBuilder, + int strongNameSignatureSize, + MethodDefinitionHandle entryPoint, + CorFlags flags, + Func, BlobContentId> deterministicIdProvider, + bool withMvidSection) + : base(header, metadataRootBuilder, ilStream, mappedFieldData, managedResources, nativeResources, + debugDirectoryBuilder, strongNameSignatureSize, entryPoint, flags, deterministicIdProvider) + { + _withMvidSection = withMvidSection; + } + + protected override ImmutableArray
CreateSections() + { + var baseSections = base.CreateSections(); + + if (_withMvidSection) + { + var builder = ArrayBuilder
.GetInstance(baseSections.Length + 1); + + builder.Add(new Section(MvidSectionName, SectionCharacteristics.MemRead | + SectionCharacteristics.ContainsInitializedData | + SectionCharacteristics.MemDiscardable)); + + builder.AddRange(baseSections); + return builder.ToImmutableAndFree(); + } + else + { + return baseSections; + } + } + + protected override BlobBuilder SerializeSection(string name, SectionLocation location) + { + if (name.Equals(MvidSectionName, StringComparison.Ordinal)) + { + Debug.Assert(_withMvidSection); + return SerializeMvidSection(location); + } + + return base.SerializeSection(name, location); + } + + internal BlobContentId Serialize(BlobBuilder peBlob, out Blob mvidSectionFixup) + { + var result = base.Serialize(peBlob); + mvidSectionFixup = _mvidSectionFixup; + return result; + } + + private BlobBuilder SerializeMvidSection(SectionLocation location) + { + var sectionBuilder = new BlobBuilder(); + + // The guid will be filled in later: + _mvidSectionFixup = sectionBuilder.ReserveBytes(SizeOfGuid); + var mvidWriter = new BlobWriter(_mvidSectionFixup); + mvidWriter.WriteBytes(0, _mvidSectionFixup.Length); + Debug.Assert(mvidWriter.RemainingBytes == 0); + + return sectionBuilder; + } + } } } From 67c31d30b3427ef538c679827809affc95b52b2f Mon Sep 17 00:00:00 2001 From: jcouv Date: Sun, 30 Apr 2017 21:49:39 -0700 Subject: [PATCH 2/9] Address PR feedback from Tomas and Jared --- .../Test/Emit/Emit/CompilationEmitTests.cs | 12 +-- .../Test/Emit/Emit/DeterministicTests.cs | 25 ++++- .../Core/MSBuildTask/CopyRefAssembly.cs | 2 +- src/Compilers/Core/MSBuildTask/MvidReader.cs | 58 ++++------ .../Core/Portable/CodeAnalysis.csproj | 1 + .../Portable/PEWriter/ExtendedPEBuilder.cs | 100 ++++++++++++++++++ .../Core/Portable/PEWriter/PeWriter.cs | 87 --------------- 7 files changed, 153 insertions(+), 132 deletions(-) create mode 100755 src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index 8ce846650ab69..a83a0f1025fee 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -262,11 +262,11 @@ internal static void Main() VerifyEntryPoint(output, expectZero: false); VerifyMethods(output, new[] { "void C.Main()", "C..ctor()" }); - VerifyMVID(output, hasMvidSection: false); + VerifyMvid(output, hasMvidSection: false); VerifyEntryPoint(metadataOutput, expectZero: true); VerifyMethods(metadataOutput, new[] { "C..ctor()" }); - VerifyMVID(metadataOutput, hasMvidSection: true); + VerifyMvid(metadataOutput, hasMvidSection: true); } void VerifyEntryPoint(MemoryStream stream, bool expectZero) @@ -281,7 +281,7 @@ void VerifyEntryPoint(MemoryStream stream, bool expectZero) /// Extract the MVID using two different methods (PEReader and MvidReader) and compare them. /// We only expect an .mvid section in ref assemblies. /// - void VerifyMVID(MemoryStream stream, bool hasMvidSection) + private void VerifyMvid(MemoryStream stream, bool hasMvidSection) { Guid mvidFromModuleDefinition; stream.Position = 0; @@ -291,7 +291,7 @@ void VerifyMVID(MemoryStream stream, bool hasMvidSection) mvidFromModuleDefinition = metadataReader.GetGuid(metadataReader.GetModuleDefinition().Mvid); stream.Position = 0; - var mvidFromMvidReader = BuildTasks.MvidReader.ReadAssemblyMvid(stream); + var mvidFromMvidReader = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(stream); if (hasMvidSection) { @@ -326,8 +326,8 @@ public class C VerifyMethods(output, new[] { "System.Int32 C.k__BackingField", "System.Int32 C.PrivateSetter.get", "void C.PrivateSetter.set", "C..ctor()", "System.Int32 C.PrivateSetter { get; private set; }" }); VerifyMethods(metadataOutput, new[] { "System.Int32 C.PrivateSetter.get", "C..ctor()", "System.Int32 C.PrivateSetter { get; }" }); - VerifyMVID(output, hasMvidSection: false); - VerifyMVID(metadataOutput, hasMvidSection: true); + VerifyMvid(output, hasMvidSection: false); + VerifyMvid(metadataOutput, hasMvidSection: true); } } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs index 2d98c539a82f6..a3a6be5cbb7b4 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs @@ -18,14 +18,19 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Emit public class DeterministicTests : EmitMetadataTestBase { private Guid CompiledGuid(string source, string assemblyName, bool debug) + { + return CompiledGuid(source, assemblyName, options: debug ? TestOptions.DebugExe : TestOptions.ReleaseExe); + } + + private Guid CompiledGuid(string source, string assemblyName, CSharpCompilationOptions options, EmitOptions emitOptions = null) { var compilation = CreateCompilation(source, assemblyName: assemblyName, references: new[] { MscorlibRef }, - options: (debug ? TestOptions.DebugExe : TestOptions.ReleaseExe).WithDeterministic(true)); + options: options.WithDeterministic(true)); Guid result = default(Guid); - base.CompileAndVerify(compilation, validator: a => + base.CompileAndVerify(compilation, emitOptions: emitOptions, validator: a => { var module = a.Modules[0]; result = module.GetModuleVersionIdOrThrow(); @@ -104,6 +109,22 @@ public static void Main(string[] args) {} Assert.NotEqual(mvid3, mvid7); } + [Fact] + public void RefAssembly() + { + var source = +@"class Program +{ + public static void Main(string[] args) {} + CHANGE +}"; + var emitRefAssembly = EmitOptions.Default.WithEmitMetadataOnly(true).WithIncludePrivateMembers(false); + + var mvid1 = CompiledGuid(source.Replace("CHANGE", ""), "X1", TestOptions.DebugDll, emitRefAssembly); + var mvid2 = CompiledGuid(source.Replace("CHANGE", "private void M() { }"), "X1", TestOptions.DebugDll, emitRefAssembly); + Assert.Equal(mvid1, mvid2); + } + const string CompareAllBytesEmitted_Source = @" using System; using System.Linq; diff --git a/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs b/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs index ef0107f44ff8d..5b12dc838c517 100644 --- a/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs +++ b/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs @@ -84,7 +84,7 @@ private Guid ExtractMvid(string path) { using (FileStream source = File.OpenRead(path)) { - return MvidReader.ReadAssemblyMvid(source); + return MvidReader.ReadAssemblyMvidOrEmpty(source); } } } diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index 831202cc18812..e1c815a400bf1 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Text; namespace Microsoft.CodeAnalysis.BuildTasks { @@ -12,12 +13,12 @@ public MvidReader(Stream stream) : base(stream) { } - public static Guid ReadAssemblyMvid(Stream stream) + public static Guid ReadAssemblyMvidOrEmpty(Stream stream) { try { var mvidReader = new MvidReader(stream); - return mvidReader.FindMvid(); + return mvidReader.TryFindMvid(); } catch (EndOfStreamException) { @@ -26,7 +27,7 @@ public static Guid ReadAssemblyMvid(Stream stream) return Guid.Empty; } - public Guid FindMvid() + public Guid TryFindMvid() { Guid empty = Guid.Empty; @@ -76,46 +77,24 @@ public Guid FindMvid() return FindMvidInSections(sections); } - string ReadSectionName() - { - int length = 8; - int read = 0; - var buffer = new char[length]; - var bytes = ReadBytes(length); - while (read < length) - { - var current = bytes[read]; - if (current == 0) - break; - - buffer[read++] = (char)current; - } - - return new string(buffer, 0, read); - } - - Guid FindMvidInSections(ushort count) + private Guid FindMvidInSections(ushort count) { for (int i = 0; i < count; i++) { // Section: Name (8) - string name = ReadSectionName(); - - // Section: VirtualSize (4) - uint virtualSize = ReadUInt32(); - - // Section: VirtualAddress (4), SizeOfRawData (4) - Skip(8); + byte[] name = ReadBytes(8); + if (name.Length == 8 && name[0] == '.' && + name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd') + { + // Section: VirtualSize (4) + uint virtualSize = ReadUInt32(); - // Section: PointerToRawData (4) - uint pointerToRawData = ReadUInt32(); + // Section: VirtualAddress (4), SizeOfRawData (4) + Skip(8); - // Section: PointerToRelocations (4), PointerToLineNumbers (4), NumberOfRelocations (2), - // NumberOfLineNumbers (2), Characteristics (4) - Skip(16); + // Section: PointerToRawData (4) + uint pointerToRawData = ReadUInt32(); - if (name.Equals(".mvid", StringComparison.Ordinal)) - { // The .mvid section only stores a Guid Debug.Assert(virtualSize == 16); @@ -125,6 +104,13 @@ Guid FindMvidInSections(ushort count) return new Guid(guidBytes); } + else + { + // Section: VirtualSize (4), VirtualAddress (4), SizeOfRawData (4), + // PointerToRawData (4), PointerToRelocations (4), PointerToLineNumbers (4), + // NumberOfRelocations (2), NumberOfLineNumbers (2), Characteristics (4) + Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4); + } } return Guid.Empty; diff --git a/src/Compilers/Core/Portable/CodeAnalysis.csproj b/src/Compilers/Core/Portable/CodeAnalysis.csproj index 1581cbd140c2f..d871379ace22a 100644 --- a/src/Compilers/Core/Portable/CodeAnalysis.csproj +++ b/src/Compilers/Core/Portable/CodeAnalysis.csproj @@ -140,6 +140,7 @@ + diff --git a/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs b/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs new file mode 100755 index 0000000000000..e75878e0ede90 --- /dev/null +++ b/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs @@ -0,0 +1,100 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Diagnostics; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using System.Reflection.PortableExecutable; +using Microsoft.CodeAnalysis; + +namespace Microsoft.Cci +{ + /// + /// This PEBuilder adds an .mvid section. + /// + internal sealed class ExtendedPEBuilder + : ManagedPEBuilder + { + private const string MvidSectionName = ".mvid"; + public const int SizeOfGuid = 16; + + // When the section is built with a placeholder, the placeholder blob is saved for later fixing up. + private Blob _mvidSectionFixup = default(Blob); + + // Only include the .mvid section in ref assemblies + private bool _withMvidSection; + + public ExtendedPEBuilder( + PEHeaderBuilder header, + MetadataRootBuilder metadataRootBuilder, + BlobBuilder ilStream, + BlobBuilder mappedFieldData, + BlobBuilder managedResources, + ResourceSectionBuilder nativeResources, + DebugDirectoryBuilder debugDirectoryBuilder, + int strongNameSignatureSize, + MethodDefinitionHandle entryPoint, + CorFlags flags, + Func, BlobContentId> deterministicIdProvider, + bool withMvidSection) + : base(header, metadataRootBuilder, ilStream, mappedFieldData, managedResources, nativeResources, + debugDirectoryBuilder, strongNameSignatureSize, entryPoint, flags, deterministicIdProvider) + { + _withMvidSection = withMvidSection; + } + + protected override ImmutableArray
CreateSections() + { + var baseSections = base.CreateSections(); + + if (_withMvidSection) + { + var builder = ArrayBuilder
.GetInstance(baseSections.Length + 1); + + builder.Add(new Section(MvidSectionName, SectionCharacteristics.MemRead | + SectionCharacteristics.ContainsInitializedData | + SectionCharacteristics.MemDiscardable)); + + builder.AddRange(baseSections); + return builder.ToImmutableAndFree(); + } + else + { + return baseSections; + } + } + + protected override BlobBuilder SerializeSection(string name, SectionLocation location) + { + if (name.Equals(MvidSectionName, StringComparison.Ordinal)) + { + Debug.Assert(_withMvidSection); + return SerializeMvidSection(location); + } + + return base.SerializeSection(name, location); + } + + internal BlobContentId Serialize(BlobBuilder peBlob, out Blob mvidSectionFixup) + { + var result = base.Serialize(peBlob); + mvidSectionFixup = _mvidSectionFixup; + return result; + } + + private BlobBuilder SerializeMvidSection(SectionLocation location) + { + var sectionBuilder = new BlobBuilder(); + + // The guid will be filled in later: + _mvidSectionFixup = sectionBuilder.ReserveBytes(SizeOfGuid); + var mvidWriter = new BlobWriter(_mvidSectionFixup); + mvidWriter.WriteBytes(0, _mvidSectionFixup.Length); + Debug.Assert(mvidWriter.RemainingBytes == 0); + + return sectionBuilder; + } + } +} diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs index 344667b4f8e49..a84e660f48385 100644 --- a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.Diagnostics; using System.IO; using System.Linq; @@ -330,91 +329,5 @@ private static int CalculateStrongNameSignatureSize(CommonPEModuleBuilder module return (keySize < 128 + 32) ? 128 : keySize - 32; } - - /// - /// This PEBuilder adds an .mvid section. - /// - private class ExtendedPEBuilder : ManagedPEBuilder - { - private const string MvidSectionName = ".mvid"; - public const int SizeOfGuid = 16; - - // When the section is built with a placeholder, the placeholder blob is saved for later fixing up. - private Blob _mvidSectionFixup = default(Blob); - - // Only include the .mvid section in ref assemblies - private bool _withMvidSection; - - public ExtendedPEBuilder( - PEHeaderBuilder header, - MetadataRootBuilder metadataRootBuilder, - BlobBuilder ilStream, - BlobBuilder mappedFieldData, - BlobBuilder managedResources, - ResourceSectionBuilder nativeResources, - DebugDirectoryBuilder debugDirectoryBuilder, - int strongNameSignatureSize, - MethodDefinitionHandle entryPoint, - CorFlags flags, - Func, BlobContentId> deterministicIdProvider, - bool withMvidSection) - : base(header, metadataRootBuilder, ilStream, mappedFieldData, managedResources, nativeResources, - debugDirectoryBuilder, strongNameSignatureSize, entryPoint, flags, deterministicIdProvider) - { - _withMvidSection = withMvidSection; - } - - protected override ImmutableArray
CreateSections() - { - var baseSections = base.CreateSections(); - - if (_withMvidSection) - { - var builder = ArrayBuilder
.GetInstance(baseSections.Length + 1); - - builder.Add(new Section(MvidSectionName, SectionCharacteristics.MemRead | - SectionCharacteristics.ContainsInitializedData | - SectionCharacteristics.MemDiscardable)); - - builder.AddRange(baseSections); - return builder.ToImmutableAndFree(); - } - else - { - return baseSections; - } - } - - protected override BlobBuilder SerializeSection(string name, SectionLocation location) - { - if (name.Equals(MvidSectionName, StringComparison.Ordinal)) - { - Debug.Assert(_withMvidSection); - return SerializeMvidSection(location); - } - - return base.SerializeSection(name, location); - } - - internal BlobContentId Serialize(BlobBuilder peBlob, out Blob mvidSectionFixup) - { - var result = base.Serialize(peBlob); - mvidSectionFixup = _mvidSectionFixup; - return result; - } - - private BlobBuilder SerializeMvidSection(SectionLocation location) - { - var sectionBuilder = new BlobBuilder(); - - // The guid will be filled in later: - _mvidSectionFixup = sectionBuilder.ReserveBytes(SizeOfGuid); - var mvidWriter = new BlobWriter(_mvidSectionFixup); - mvidWriter.WriteBytes(0, _mvidSectionFixup.Length); - Debug.Assert(mvidWriter.RemainingBytes == 0); - - return sectionBuilder; - } - } } } From 51a3c3f86788de1f052fb1e1dca7bd1575da8e28 Mon Sep 17 00:00:00 2001 From: jcouv Date: Sun, 30 Apr 2017 22:45:43 -0700 Subject: [PATCH 3/9] Update refout docs --- docs/compilers/CSharp/CommandLine.md | 2 ++ docs/compilers/Visual Basic/CommandLine.md | 2 ++ docs/features/refout.md | 11 ++++++----- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/compilers/CSharp/CommandLine.md b/docs/compilers/CSharp/CommandLine.md index 8244ce7bfc71e..5f78b84f8a6ae 100644 --- a/docs/compilers/CSharp/CommandLine.md +++ b/docs/compilers/CSharp/CommandLine.md @@ -4,6 +4,7 @@ | ---- | ---- | | **OUTPUT FILES** | | `/out:`*file* | Specify output file name (default: base name of file with main class or first file) +| `/refout:`*file* | Specify the reference assembly's output file name | `/target:exe` | Build a console executable (default) (Short form: `/t:exe`) | `/target:winexe` | Build a Windows executable (Short form: `/t:winexe` ) | `/target:library` | Build a library (Short form: `/t:library`) @@ -32,6 +33,7 @@ | `/debug`:{`full`|`pdbonly`|`portable`} | Specify debugging type (`full` is default, and enables attaching a debugger to a running program. `portable` is a cross-platform format) | `/optimize`{`+`|`-`} | Enable optimizations (Short form: `/o`) | `/deterministic` | Produce a deterministic assembly (including module version GUID and timestamp) +| `/refonly | Produce a reference assembly, instead of a full assembly, as the primary output | **ERRORS AND WARNINGS** | `/warnaserror`{`+`|`-`} | Report all warnings as errors | `/warnaserror`{`+`|`-`}`:`*warn list* | Report specific warnings as errors diff --git a/docs/compilers/Visual Basic/CommandLine.md b/docs/compilers/Visual Basic/CommandLine.md index 0a7afcefd67da..e7c44304aa4fa 100644 --- a/docs/compilers/Visual Basic/CommandLine.md +++ b/docs/compilers/Visual Basic/CommandLine.md @@ -4,6 +4,7 @@ | ---- | ---- | | **OUTPUT FILE** | `/out:`*file* | Specifies the output file name. +| `/refout:`*file* | Specify the reference assembly's output file name | `/target:exe` | Create a console application (default). (Short form: `/t`) | `/target:winexe` | Create a Windows application. | `/target:library` | Create a library assembly. @@ -34,6 +35,7 @@ | `/debug:portable` | Emit debugging information in the portable format. | `/debug:pdbonly` | Emit PDB file only. | `/deterministic` | Produce a deterministic assembly (including module version GUID and timestamp) +| `/refonly | Produce a reference assembly, instead of a full assembly, as the primary output | **ERRORS AND WARNINGS** | `/nowarn` | Disable all warnings. | `/nowarn:`*number_list* | Disable a list of individual warnings. diff --git a/docs/features/refout.md b/docs/features/refout.md index 9e533c52c72f7..0da4d809079a1 100644 --- a/docs/features/refout.md +++ b/docs/features/refout.md @@ -32,7 +32,7 @@ Two mutually exclusive command-line parameters will be added to `csc.exe` and `v - `/refout` - `/refonly` -The `/refout` parameter specifies a file path where the ref assembly should be output. This translates to `metadataPeStream` in the `Emit` API (see details below). +The `/refout` parameter specifies a file path where the ref assembly should be output. This translates to `metadataPeStream` in the `Emit` API (see details below). The filename for the ref assembly should generally match that of the primary assembly, but it can be in a different folder. The `/refonly` parameter is a flag that indicates that a ref assembly should be output instead of an implementation assembly. The `/refonly` parameter is not allowed together with the `/refout` parameter, as it doesn't make sense to have both the primary and secondary outputs be ref assemblies. Also, the `/refonly` parameter silently disables outputting PDBs, as ref assemblies cannot be executed. @@ -47,13 +47,14 @@ The `CoreCompile` target will support a new output, called `IntermediateRefAssem The `Csc` task will support a new output, called `OutputRefAssembly`, which parallels the existing `OutputAssembly`. Both of those basically map to the `/refout` command-line parameter. -An additional task, called `CopyRefAssembly`, will be provided along with the existing `Csc` task. It takes a `SourcePath` and a `DestinationPath` and generally copies the file from the source over to the destination. But if it can determine that the contents of those two files match, then the destination file is left untouched. +An additional task, called `CopyRefAssembly`, will be provided along with the existing `Csc` task. It takes a `SourcePath` and a `DestinationPath` and generally copies the file from the source over to the destination. But if it can determine that the contents of those two files match (by comparing their MVIDs, see details below), then the destination file is left untouched. ### CodeAnalysis APIs It is already possible to produce metadata-only assemblies by using `EmitOptions.EmitMetadataOnly`, which is used in IDE scenarios with cross-language dependencies. The compiler will be updated to honour the `EmitOptions.IncludePrivateMembers` flag as well. When combined with `EmitMetadataOnly` or a `metadataPeStream` in `Emit`, a ref assembly will be produced. -The diagnostic check for emitting methods lacking a body (`void M();`) will be moved from declaration diagnostics to regular diagnostics, so that code will successfully emit with `EmitMetadataOnly`. +The diagnostic check for emitting methods lacking a body (`void M();`) will be filtered from declaration diagnostics, so that code will successfully emit with `EmitMetadataOnly`. Later on, the `EmitOptions.TolerateErrors` flag will allow emitting error types as well. +`Emit` is also modified to produce a new PE section called ".mvid" containing a copy of the MVID, when producing ref assemblies. This makes it easy for `CopyRefAssembly` to extract and compare MVIDs from ref assemblies. Going back to the 4 driving scenarios: 1. For a regular compilation, `EmitMetadataOnly` is left to `false` and no `metadataPeStream` is passed into `Emit`. @@ -67,10 +68,10 @@ As mentioned above, there may be further refinements after C# 7.1: - produce ref assemblies even when there are errors outside method bodies (emitting error types when `EmitOptions.TolerateErrors` is set) ## Open questions -- should explicit method implementations be included in ref assemblies? +- should explicit method implementations be included in ref assemblies? (answer: no. The interfaces that are declared as implemented are what matter to consuming compilations) - Non-public attributes on public APIs (emit attribute based on accessibility rule) - ref assemblies and NoPia -- `/refout` and `/addmodule`, should we disallow this combination? +- `/refout` and `/addmodule`, should we disallow this combination? (answer: yes. This will not be supported in C# 7.1) ## Related issues - Produce ref assemblies from command-line and msbuild (https://github.com/dotnet/roslyn/issues/2184) From 4de0d32b580a04d1fb123ced65ebd4ec3e2e34d2 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 1 May 2017 12:23:55 -0700 Subject: [PATCH 4/9] Address PR feedback from Tomas --- .../CSharp/Test/Emit/Emit/CompilationEmitTests.cs | 1 - src/Compilers/Core/MSBuildTask/MvidReader.cs | 14 ++++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index a83a0f1025fee..d2fae1c7f34d6 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -49,7 +49,6 @@ public void Main() Diagnostic(ErrorCode.ERR_AssgLvalueExpected, "x")); } - [Fact] public void CompilationEmitWithQuotedMainType() { diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index e1c815a400bf1..f075346ffd5eb 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -84,7 +84,7 @@ private Guid FindMvidInSections(ushort count) // Section: Name (8) byte[] name = ReadBytes(8); if (name.Length == 8 && name[0] == '.' && - name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd') + name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') { // Section: VirtualSize (4) uint virtualSize = ReadUInt32(); @@ -104,13 +104,11 @@ private Guid FindMvidInSections(ushort count) return new Guid(guidBytes); } - else - { - // Section: VirtualSize (4), VirtualAddress (4), SizeOfRawData (4), - // PointerToRawData (4), PointerToRelocations (4), PointerToLineNumbers (4), - // NumberOfRelocations (2), NumberOfLineNumbers (2), Characteristics (4) - Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4); - } + + // Section: VirtualSize (4), VirtualAddress (4), SizeOfRawData (4), + // PointerToRawData (4), PointerToRelocations (4), PointerToLineNumbers (4), + // NumberOfRelocations (2), NumberOfLineNumbers (2), Characteristics (4) + Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4); } return Guid.Empty; From ec9b2d4e482e6ca87374730a97a298ad00999c24 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 1 May 2017 15:17:46 -0700 Subject: [PATCH 5/9] Address PR feedback from Chuck and Aleksey --- docs/features/refout.md | 5 +- .../Test/Emit/Emit/CompilationEmitTests.cs | 19 ++- src/Compilers/Core/MSBuildTask/MvidReader.cs | 108 ++++++++---------- 3 files changed, 69 insertions(+), 63 deletions(-) diff --git a/docs/features/refout.md b/docs/features/refout.md index 0da4d809079a1..5dd6c37d5c6f6 100644 --- a/docs/features/refout.md +++ b/docs/features/refout.md @@ -37,11 +37,13 @@ The `/refout` parameter specifies a file path where the ref assembly should be o The `/refonly` parameter is a flag that indicates that a ref assembly should be output instead of an implementation assembly. The `/refonly` parameter is not allowed together with the `/refout` parameter, as it doesn't make sense to have both the primary and secondary outputs be ref assemblies. Also, the `/refonly` parameter silently disables outputting PDBs, as ref assemblies cannot be executed. The `/refonly` parameter translates to `EmitMetadataOnly` being `true`, and `IncludePrivateMembers` being `false` in the `Emit` API (see details below). +Neither `/refonly` nor `/refout` are permitted with `/target:module` or `/addmodule` options. When the compiler produces documentation, the contents produced will match the APIs that go into the primary output. In other words, the documentation will be filtered down when using the `/refonly` parameter. The compilation from the command-line will either produce both assemblies (implementation and ref) or neither. There is no "partial success" scenario. + ### CscTask/CoreCompile The `CoreCompile` target will support a new output, called `IntermediateRefAssembly`, which parallels the existing `IntermediateAssembly`. The `Csc` task will support a new output, called `OutputRefAssembly`, which parallels the existing `OutputAssembly`. @@ -68,10 +70,9 @@ As mentioned above, there may be further refinements after C# 7.1: - produce ref assemblies even when there are errors outside method bodies (emitting error types when `EmitOptions.TolerateErrors` is set) ## Open questions -- should explicit method implementations be included in ref assemblies? (answer: no. The interfaces that are declared as implemented are what matter to consuming compilations) +- should explicit method implementations be included in ref assemblies? - Non-public attributes on public APIs (emit attribute based on accessibility rule) - ref assemblies and NoPia -- `/refout` and `/addmodule`, should we disallow this combination? (answer: yes. This will not be supported in C# 7.1) ## Related issues - Produce ref assemblies from command-line and msbuild (https://github.com/dotnet/roslyn/issues/2184) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index d2fae1c7f34d6..1c7da18965727 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -282,23 +282,22 @@ void VerifyEntryPoint(MemoryStream stream, bool expectZero) /// private void VerifyMvid(MemoryStream stream, bool hasMvidSection) { - Guid mvidFromModuleDefinition; stream.Position = 0; using (var reader = new PEReader(stream)) { var metadataReader = reader.GetMetadataReader(); - mvidFromModuleDefinition = metadataReader.GetGuid(metadataReader.GetModuleDefinition().Mvid); + Guid mvidFromModuleDefinition = metadataReader.GetGuid(metadataReader.GetModuleDefinition().Mvid); stream.Position = 0; var mvidFromMvidReader = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(stream); + Assert.NotEqual(Guid.Empty, mvidFromModuleDefinition); if (hasMvidSection) { Assert.Equal(mvidFromModuleDefinition, mvidFromMvidReader); } else { - Assert.NotEqual(Guid.Empty, mvidFromModuleDefinition); Assert.Equal(Guid.Empty, mvidFromMvidReader); } } @@ -590,6 +589,20 @@ private void CompareAssemblies(string sourceTemplate, string change1, string cha { AssertEx.NotEqual(image1, image2, message: $"Expecting difference for includePrivateMembers={includePrivateMembers} case, but they matched."); } + + var mvid1 = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(new MemoryStream(image1.DangerousGetUnderlyingArray())); + var mvid2 = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(new MemoryStream(image2.DangerousGetUnderlyingArray())); + + if (!includePrivateMembers) + { + Assert.NotEqual(Guid.Empty, mvid1); + Assert.Equal(expectMatch, mvid1 == mvid2); + } + else + { + Assert.Equal(Guid.Empty, mvid1); + Assert.Equal(Guid.Empty, mvid2); + } } [Fact] diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index f075346ffd5eb..7ceb3b43aedc7 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -3,125 +3,117 @@ using System; using System.Diagnostics; using System.IO; -using System.Text; namespace Microsoft.CodeAnalysis.BuildTasks { - public sealed class MvidReader : BinaryReader + public static class MvidReader { - public MvidReader(Stream stream) : base(stream) - { - } - public static Guid ReadAssemblyMvidOrEmpty(Stream stream) { - try - { - var mvidReader = new MvidReader(stream); - return mvidReader.TryFindMvid(); - } - catch (EndOfStreamException) - { - } - - return Guid.Empty; + return ReadAssemblyMvidOrEmpty(new BinaryReader(stream)); } - public Guid TryFindMvid() + private static Guid ReadAssemblyMvidOrEmpty(BinaryReader reader) { Guid empty = Guid.Empty; - // DOS Header (64), DOS Stub (64), PE Signature (4), COFF Header (20), PE Header (224) - if (BaseStream.Length < 64 + 64 + 4 + 20 + 224) + // DOS Header (64), DOS Stub (64), PE Signature (4), COFF Header (20), PE Header (224), 1x Section Header (40) + if (reader.BaseStream.Length < 64 + 64 + 4 + 20 + 224 + 40) { return empty; } // DOS Header: PE (2) - if (ReadUInt16() != 0x5a4d) + if (reader.ReadUInt16() != 0x5a4d) { return empty; } // DOS Header: Start (58) - Skip(58); + Skip(58, reader); // DOS Header: Address of PE Signature - MoveTo(ReadUInt32()); + MoveTo(reader.ReadUInt32(), reader); // PE Signature ('P' 'E' null null) - if (ReadUInt32() != 0x00004550) + if (reader.ReadUInt32() != 0x00004550) { return empty; } // COFF Header: Machine (2) - Skip(2); + Skip(2, reader); // COFF Header: NumberOfSections (2) - ushort sections = ReadUInt16(); + ushort sections = reader.ReadUInt16(); // COFF Header: TimeDateStamp (4), PointerToSymbolTable (4), NumberOfSymbols (4) - Skip(12); + Skip(12, reader); // COFF Header: OptionalHeaderSize (2) - ushort optionalHeaderSize = ReadUInt16(); + ushort optionalHeaderSize = reader.ReadUInt16(); // COFF Header: Characteristics (2) - Skip(2); + Skip(2, reader); // Optional header - Skip(optionalHeaderSize); + Skip(optionalHeaderSize, reader); // Section headers - return FindMvidInSections(sections); + return FindMvidInSections(sections, reader); } - private Guid FindMvidInSections(ushort count) + private static Guid FindMvidInSections(ushort count, BinaryReader reader) { - for (int i = 0; i < count; i++) + // .mvid section must be first, if it's there + // Section: Name (8) + byte[] name = reader.ReadBytes(8); + if (name.Length == 8 && name[0] == '.' && + name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') { - // Section: Name (8) - byte[] name = ReadBytes(8); - if (name.Length == 8 && name[0] == '.' && - name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') - { - // Section: VirtualSize (4) - uint virtualSize = ReadUInt32(); + // Section: VirtualSize (4) + uint virtualSize = reader.ReadUInt32(); - // Section: VirtualAddress (4), SizeOfRawData (4) - Skip(8); + // Section: VirtualAddress (4), SizeOfRawData (4) + Skip(8, reader); - // Section: PointerToRawData (4) - uint pointerToRawData = ReadUInt32(); + // Section: PointerToRawData (4) + uint pointerToRawData = reader.ReadUInt32(); - // The .mvid section only stores a Guid - Debug.Assert(virtualSize == 16); + // The .mvid section only stores a Guid + if (virtualSize != 16) + { + Debug.Assert(false); + return Guid.Empty; + } - BaseStream.Position = pointerToRawData; + if (MoveTo(pointerToRawData, reader)) + { byte[] guidBytes = new byte[16]; - BaseStream.Read(guidBytes, 0, 16); - - return new Guid(guidBytes); + if (reader.BaseStream.Read(guidBytes, 0, 16) == 16) + { + return new Guid(guidBytes); + } } - - // Section: VirtualSize (4), VirtualAddress (4), SizeOfRawData (4), - // PointerToRawData (4), PointerToRelocations (4), PointerToLineNumbers (4), - // NumberOfRelocations (2), NumberOfLineNumbers (2), Characteristics (4) - Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4); } return Guid.Empty; } - public void Skip(int bytes) + private static void Skip(int bytes, BinaryReader reader) { - BaseStream.Seek(bytes, SeekOrigin.Current); + reader.BaseStream.Seek(bytes, SeekOrigin.Current); } - public void MoveTo(uint position) + private static bool MoveTo(uint position, BinaryReader reader) { - BaseStream.Seek(position, SeekOrigin.Begin); + if (reader.BaseStream.Length < position) + { + return false; + } + + reader.BaseStream.Seek(position, SeekOrigin.Begin); + return true; } } } From b1ac2aa7d333ae1ae508ae9cb0eefa0550604bdc Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 1 May 2017 15:52:48 -0700 Subject: [PATCH 6/9] Clarify header format --- docs/features/refout.md | 1 - src/Compilers/Core/MSBuildTask/MvidReader.cs | 17 +++++++++++------ .../Core/Portable/PEWriter/ExtendedPEBuilder.cs | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/features/refout.md b/docs/features/refout.md index 5dd6c37d5c6f6..9ab450afc9ffc 100644 --- a/docs/features/refout.md +++ b/docs/features/refout.md @@ -43,7 +43,6 @@ When the compiler produces documentation, the contents produced will match the A The compilation from the command-line will either produce both assemblies (implementation and ref) or neither. There is no "partial success" scenario. - ### CscTask/CoreCompile The `CoreCompile` target will support a new output, called `IntermediateRefAssembly`, which parallels the existing `IntermediateAssembly`. The `Csc` task will support a new output, called `OutputRefAssembly`, which parallels the existing `OutputAssembly`. diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index 7ceb3b43aedc7..53e517ede841d 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -23,17 +23,17 @@ private static Guid ReadAssemblyMvidOrEmpty(BinaryReader reader) return empty; } - // DOS Header: PE (2) - if (reader.ReadUInt16() != 0x5a4d) + // DOS Header: Magic number (2) + if (reader.ReadUInt16() != 0x5a4d) // "MZ" { return empty; } - // DOS Header: Start (58) - Skip(58, reader); + // DOS Header: Address of PE Signature (at 0x3C) + MoveTo(0x3C, reader); + uint lfanew = reader.ReadUInt32(); - // DOS Header: Address of PE Signature - MoveTo(reader.ReadUInt32(), reader); + MoveTo(lfanew, reader); // jump over the MS DOS Stub to the PE Signature // PE Signature ('P' 'E' null null) if (reader.ReadUInt32() != 0x00004550) @@ -65,6 +65,11 @@ private static Guid ReadAssemblyMvidOrEmpty(BinaryReader reader) private static Guid FindMvidInSections(ushort count, BinaryReader reader) { + if (count == 0) + { + return Guid.Empty; + } + // .mvid section must be first, if it's there // Section: Name (8) byte[] name = reader.ReadBytes(8); diff --git a/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs b/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs index e75878e0ede90..608d66faed3c2 100755 --- a/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs +++ b/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs @@ -24,7 +24,7 @@ internal sealed class ExtendedPEBuilder private Blob _mvidSectionFixup = default(Blob); // Only include the .mvid section in ref assemblies - private bool _withMvidSection; + private readonly bool _withMvidSection; public ExtendedPEBuilder( PEHeaderBuilder header, From 4c02a2d3289bfc43d26e4e4491893b11ba7165e8 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 1 May 2017 17:48:07 -0700 Subject: [PATCH 7/9] Add test for .mvid section not being first --- .../Test/Emit/Emit/CompilationEmitTests.cs | 76 +++++++++++++++++++ src/Compilers/Core/MSBuildTask/MvidReader.cs | 58 +++++++------- 2 files changed, 108 insertions(+), 26 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index 1c7da18965727..9a01a8e55c68b 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -1,6 +1,7 @@ // 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.Generic; using System.Collections.Immutable; using System.IO; using System.Linq; @@ -276,6 +277,81 @@ void VerifyEntryPoint(MemoryStream stream, bool expectZero) } } + private class TestResourceSectionBuilder : ResourceSectionBuilder + { + public TestResourceSectionBuilder() + { + } + + protected override void Serialize(BlobBuilder builder, SectionLocation location) + { + builder.WriteInt32(0x12345678); + builder.WriteInt32(location.PointerToRawData); + builder.WriteInt32(location.RelativeVirtualAddress); + } + } + + private class TestPEBuilder : ManagedPEBuilder + { + public static readonly Guid s_mvid = Guid.Parse("a78fa2c3-854e-42bf-8b8d-75a450a6dc18"); + + public TestPEBuilder(PEHeaderBuilder header, + MetadataRootBuilder metadataRootBuilder, + BlobBuilder ilStream, + ResourceSectionBuilder nativeResources) + : base(header, metadataRootBuilder, ilStream, nativeResources: nativeResources) + { + } + + protected override ImmutableArray
CreateSections() + { + return base.CreateSections().Add( + new Section(".mvid", SectionCharacteristics.MemRead | + SectionCharacteristics.ContainsInitializedData | + SectionCharacteristics.MemDiscardable)); + } + + protected override BlobBuilder SerializeSection(string name, SectionLocation location) + { + if (name.Equals(".mvid", StringComparison.Ordinal)) + { + var sectionBuilder = new BlobBuilder(); + sectionBuilder.WriteGuid(s_mvid); + return sectionBuilder; + } + + return base.SerializeSection(name, location); + } + } + + [Fact] + public void MvidSectionNotFirst() + { + var ilBuilder = new BlobBuilder(); + var metadataBuilder = new MetadataBuilder(); + + var peBuilder = new TestPEBuilder( + PEHeaderBuilder.CreateLibraryHeader(), + new MetadataRootBuilder(metadataBuilder), + ilBuilder, + nativeResources: new TestResourceSectionBuilder()); + + var peBlob = new BlobBuilder(); + peBuilder.Serialize(peBlob); + + var peStream = new MemoryStream(); + peBlob.WriteContentTo(peStream); + + peStream.Position = 0; + var peReader = new PEReader(peStream); + AssertEx.Equal(new[] { ".text", ".rsrc", ".reloc", ".mvid" }, + peReader.PEHeaders.SectionHeaders.Select(h => h.Name)); + + peStream.Position = 0; + var mvid = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(peStream); + Assert.Equal(TestPEBuilder.s_mvid, mvid); + } + /// /// Extract the MVID using two different methods (PEReader and MvidReader) and compare them. /// We only expect an .mvid section in ref assemblies. diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index 53e517ede841d..f149ed8ec59a6 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -65,41 +65,47 @@ private static Guid ReadAssemblyMvidOrEmpty(BinaryReader reader) private static Guid FindMvidInSections(ushort count, BinaryReader reader) { - if (count == 0) + for (int i = 0; i < count; i++) { - return Guid.Empty; - } - // .mvid section must be first, if it's there - // Section: Name (8) - byte[] name = reader.ReadBytes(8); - if (name.Length == 8 && name[0] == '.' && - name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') - { - // Section: VirtualSize (4) - uint virtualSize = reader.ReadUInt32(); + // .mvid section must be first, if it's there + // Section: Name (8) + byte[] name = reader.ReadBytes(8); + if (name.Length == 8 && name[0] == '.' && + name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') + { + // Section: VirtualSize (4) + uint virtualSize = reader.ReadUInt32(); - // Section: VirtualAddress (4), SizeOfRawData (4) - Skip(8, reader); + // Section: VirtualAddress (4), SizeOfRawData (4) + Skip(8, reader); - // Section: PointerToRawData (4) - uint pointerToRawData = reader.ReadUInt32(); + // Section: PointerToRawData (4) + uint pointerToRawData = reader.ReadUInt32(); - // The .mvid section only stores a Guid - if (virtualSize != 16) - { - Debug.Assert(false); - return Guid.Empty; - } + // The .mvid section only stores a Guid + if (virtualSize != 16) + { + Debug.Assert(false); + return Guid.Empty; + } - if (MoveTo(pointerToRawData, reader)) - { - byte[] guidBytes = new byte[16]; - if (reader.BaseStream.Read(guidBytes, 0, 16) == 16) + if (MoveTo(pointerToRawData, reader)) { - return new Guid(guidBytes); + byte[] guidBytes = new byte[16]; + if (reader.BaseStream.Read(guidBytes, 0, 16) == 16) + { + return new Guid(guidBytes); + } } } + else + { + // Section: VirtualSize (4), VirtualAddress (4), SizeOfRawData (4), + // PointerToRawData (4), PointerToRelocations (4), PointerToLineNumbers (4), + // NumberOfRelocations (2), NumberOfLineNumbers (2), Characteristics (4) + Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4, reader); + } } return Guid.Empty; From 7df9479433092b819ebccd3e4bb9d171639a065d Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 2 May 2017 12:23:32 -0700 Subject: [PATCH 8/9] No end-of-stream exception --- .../Test/Emit/Emit/CompilationEmitTests.cs | 14 +- src/Compilers/Core/MSBuildTask/MvidReader.cs | 168 +++++++++++++----- 2 files changed, 132 insertions(+), 50 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index 9a01a8e55c68b..77ed494cf7dc7 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -343,13 +343,15 @@ public void MvidSectionNotFirst() peBlob.WriteContentTo(peStream); peStream.Position = 0; - var peReader = new PEReader(peStream); - AssertEx.Equal(new[] { ".text", ".rsrc", ".reloc", ".mvid" }, - peReader.PEHeaders.SectionHeaders.Select(h => h.Name)); + using (var peReader = new PEReader(peStream)) + { + AssertEx.Equal(new[] { ".text", ".rsrc", ".reloc", ".mvid" }, + peReader.PEHeaders.SectionHeaders.Select(h => h.Name)); - peStream.Position = 0; - var mvid = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(peStream); - Assert.Equal(TestPEBuilder.s_mvid, mvid); + peStream.Position = 0; + var mvid = BuildTasks.MvidReader.ReadAssemblyMvidOrEmpty(peStream); + Assert.Equal(TestPEBuilder.s_mvid, mvid); + } } /// diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index f149ed8ec59a6..4ffba702ca374 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -8,6 +8,8 @@ namespace Microsoft.CodeAnalysis.BuildTasks { public static class MvidReader { + private static readonly Guid s_empty = Guid.Empty; + public static Guid ReadAssemblyMvidOrEmpty(Stream stream) { return ReadAssemblyMvidOrEmpty(new BinaryReader(stream)); @@ -15,49 +17,69 @@ public static Guid ReadAssemblyMvidOrEmpty(Stream stream) private static Guid ReadAssemblyMvidOrEmpty(BinaryReader reader) { - Guid empty = Guid.Empty; - - // DOS Header (64), DOS Stub (64), PE Signature (4), COFF Header (20), PE Header (224), 1x Section Header (40) - if (reader.BaseStream.Length < 64 + 64 + 4 + 20 + 224 + 40) - { - return empty; - } - // DOS Header: Magic number (2) - if (reader.ReadUInt16() != 0x5a4d) // "MZ" + if (!ReadUInt16(reader, out ushort magicNumber) || magicNumber != 0x5a4d) // "MZ" { - return empty; + return s_empty; } // DOS Header: Address of PE Signature (at 0x3C) - MoveTo(0x3C, reader); - uint lfanew = reader.ReadUInt32(); + if (!MoveTo(0x3C, reader)) + { + return s_empty; + } + if (!ReadUInt32(reader, out uint lfanew)) + { + return s_empty; + } - MoveTo(lfanew, reader); // jump over the MS DOS Stub to the PE Signature + // jump over the MS DOS Stub to the PE Signature + if (!MoveTo(lfanew, reader)) + { + return s_empty; + } // PE Signature ('P' 'E' null null) - if (reader.ReadUInt32() != 0x00004550) + if (!ReadUInt32(reader, out uint peSig) || peSig != 0x00004550) { - return empty; + return s_empty; } // COFF Header: Machine (2) - Skip(2, reader); + if (!Skip(2, reader)) + { + return s_empty; + } // COFF Header: NumberOfSections (2) - ushort sections = reader.ReadUInt16(); + if (!ReadUInt16(reader, out ushort sections)) + { + return s_empty; + } // COFF Header: TimeDateStamp (4), PointerToSymbolTable (4), NumberOfSymbols (4) - Skip(12, reader); + if (!Skip(12, reader)) + { + return s_empty; + } // COFF Header: OptionalHeaderSize (2) - ushort optionalHeaderSize = reader.ReadUInt16(); + if (!ReadUInt16(reader, out ushort optionalHeaderSize)) + { + return s_empty; + } // COFF Header: Characteristics (2) - Skip(2, reader); + if (!Skip(2, reader)) + { + return s_empty; + } // Optional header - Skip(optionalHeaderSize, reader); + if (!Skip(optionalHeaderSize, reader)) + { + return s_empty; + } // Section headers return FindMvidInSections(sections, reader); @@ -67,58 +89,116 @@ private static Guid FindMvidInSections(ushort count, BinaryReader reader) { for (int i = 0; i < count; i++) { - - // .mvid section must be first, if it's there // Section: Name (8) - byte[] name = reader.ReadBytes(8); + if (!ReadBytes(reader, 8, out byte[] name)) + { + return s_empty; + } + if (name.Length == 8 && name[0] == '.' && name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') { // Section: VirtualSize (4) - uint virtualSize = reader.ReadUInt32(); + if (!ReadUInt32(reader, out uint virtualSize) || virtualSize != 16) + { + // The .mvid section only stores a Guid + return s_empty; + } // Section: VirtualAddress (4), SizeOfRawData (4) - Skip(8, reader); - - // Section: PointerToRawData (4) - uint pointerToRawData = reader.ReadUInt32(); - - // The .mvid section only stores a Guid - if (virtualSize != 16) + if (!Skip(8, reader)) { - Debug.Assert(false); - return Guid.Empty; + return s_empty; } - if (MoveTo(pointerToRawData, reader)) + // Section: PointerToRawData (4) + if (!ReadUInt32(reader, out uint pointerToRawData)) { - byte[] guidBytes = new byte[16]; - if (reader.BaseStream.Read(guidBytes, 0, 16) == 16) - { - return new Guid(guidBytes); - } + return s_empty; } + + return ReadMvidSection(reader, pointerToRawData); } else { // Section: VirtualSize (4), VirtualAddress (4), SizeOfRawData (4), // PointerToRawData (4), PointerToRelocations (4), PointerToLineNumbers (4), // NumberOfRelocations (2), NumberOfLineNumbers (2), Characteristics (4) - Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4, reader); + if (!Skip(4 + 4 + 4 + 4 + 4 + 4 + 2 + 2 + 4, reader)) + { + return s_empty; + } } } - return Guid.Empty; + return s_empty; + } + + private static Guid ReadMvidSection(BinaryReader reader, uint pointerToMvidSection) + { + if (!MoveTo(pointerToMvidSection, reader)) + { + return s_empty; + } + + if (!ReadBytes(reader, 16, out byte[] guidBytes)) + { + return s_empty; + } + + return new Guid(guidBytes); } - private static void Skip(int bytes, BinaryReader reader) + private static bool ReadUInt16(BinaryReader reader, out ushort output) { + if (reader.BaseStream.Position + 2 >= reader.BaseStream.Length) + { + output = 0; + return false; + } + + output = reader.ReadUInt16(); + return true; + } + + private static bool ReadUInt32(BinaryReader reader, out uint output) + { + if (reader.BaseStream.Position + 4 >= reader.BaseStream.Length) + { + output = 0; + return false; + } + + output = reader.ReadUInt32(); + return true; + } + + private static bool ReadBytes(BinaryReader reader, int count, out byte[] output) + { + if (reader.BaseStream.Position + count >= reader.BaseStream.Length) + { + output = null; + return false; + } + + output = reader.ReadBytes(count); + return true; + } + + private static bool Skip(int bytes, BinaryReader reader) + { + if (reader.BaseStream.Position + bytes >= reader.BaseStream.Length) + { + return false; + } + reader.BaseStream.Seek(bytes, SeekOrigin.Current); + return true; } private static bool MoveTo(uint position, BinaryReader reader) { - if (reader.BaseStream.Length < position) + if (position >= reader.BaseStream.Length) { return false; } From 786dbe7e25be7fe95b941e8973939cdecdd6c4af Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 2 May 2017 12:41:59 -0700 Subject: [PATCH 9/9] More feedback --- src/Compilers/Core/MSBuildTask/MvidReader.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index 4ffba702ca374..745fdf24fbb11 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -28,13 +28,13 @@ private static Guid ReadAssemblyMvidOrEmpty(BinaryReader reader) { return s_empty; } - if (!ReadUInt32(reader, out uint lfanew)) + if (!ReadUInt32(reader, out uint pointerToPeSignature)) { return s_empty; } // jump over the MS DOS Stub to the PE Signature - if (!MoveTo(lfanew, reader)) + if (!MoveTo(pointerToPeSignature, reader)) { return s_empty; }