Skip to content

Commit

Permalink
Address PR feedback from Chuck and Aleksey
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed May 1, 2017
1 parent 4de0d32 commit ec9b2d4
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 63 deletions.
5 changes: 3 additions & 2 deletions docs/features/refout.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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)
Expand Down
19 changes: 16 additions & 3 deletions src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,22 @@ void VerifyEntryPoint(MemoryStream stream, bool expectZero)
/// </summary>
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);
}
}
Expand Down Expand Up @@ -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]
Expand Down
108 changes: 50 additions & 58 deletions src/Compilers/Core/MSBuildTask/MvidReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

0 comments on commit ec9b2d4

Please sign in to comment.