Issue with .net core and portable format

[sawilde]

As you may know OpenCover uses Mono.Cecil to leverage the reading of PDB files and I was hoping that the latest release would resolve the issue identified with handling .net core (OpenCover/opencover#610)

I upgraded to 0.9.6.3 through nuget (as it appeared work on portable format had been done between the the 0.9.6.1 and 0.9.6.3 releases) but the library is throwing an OutOfMemoryException("Array dimensions exceeded supported range.").

   at Microsoft.Cci.Pdb.MsfDirectory..ctor(PdbReader reader, PdbFileHeader head, BitAccess bits)
   at Microsoft.Cci.Pdb.PdbFile.LoadFunctions(Stream read, Dictionary`2& tokenToSourceMapping, String& sourceServerData, Int32& age, Guid& guid)
   at Mono.Cecil.Pdb.PdbReader.PopulateFunctions()
   at Mono.Cecil.Pdb.PdbReader.ProcessDebugHeader(ImageDebugDirectory directory, Byte[] header)
   at Mono.Cecil.ModuleDefinition.ProcessDebugHeader()
   at Mono.Cecil.ModuleReader.ReadSymbols(ModuleDefinition module, ReaderParameters parameters)
   at Mono.Cecil.ModuleReader.CreateModuleFrom(Image image, ReaderParameters parameters)
   at Mono.Cecil.ModuleDefinition.ReadModule(String fileName, ReaderParameters parameters)
   at Mono.Cecil.AssemblyDefinition.ReadAssembly(String fileName, ReaderParameters parameters)
   at OpenCover.Framework.Symbols.CecilSymbolManager.LoadSourceAssembly() in C:\Projects\opencover.git\working\main\OpenCover.Framework\Symbols\CecilSymbolManager.cs:line 126

I tried to investigate myself but the mainline has moved on and broken backward compatibility (SequencePoint related) and I can't tell from your git history nor your releases which commit aligns with the nuget package 0.9.6.3.

Can you help by either

1. resolution of the issue
2. identify which commit was used to build the 0.9.6.3 release so that I can investigate the issue myself.

[jbevain]

@sawilde hey Shaun,

As you mentioned, in .NET Core, the pdb format changed. There's nothing in 0.9.6 to handle the portable pdb files. What's happening is that Cecil is trying to read a portable pdb file with the native pdb reader, and the native pdb reader crashes while misinterpreting the file. This particular exception has been fixed in master through #279.

To answer your question, the 0.9.6 nuget is maintained through the 0.9.6-nuget branch. 0.9.6.3 has been built off 0f397db.

Now, to really fix the .NET Core scenario, you're going to need support for portable pdbs which are in master and aimed at being released through 0.10 based on the master, which is now fairly stable.

There are breaking changes in 0.10 however. To help you update, please find bellow the current breaking change log:

Custom Attributes on interface implementation for winmd files

We used to have:

class TypeDefinition {
   public Collection<TypeReference> Interfaces { get; }
}

We now have:

class TypeDefinition {
   public Collection<InterfaceImplementation> Interfaces { get; }
}

class InterfaceImplementation {
  public TypeReference InterfaceTypes { get; }
  public Collection<CustomAttribute> CustomAttributes { get; }
}

That's an easy change.

Lower Memory Usage

I've merged this branch, meaning that by default a ModuleDefinition keeps an open handle on its file and needs to be disposed. The default behavior for a DefaultAssemblyResolver, which is caching, is to dispose all cached assemblies when the resolver is disposed.

You may override that by setting the InMemory = true of the ReaderParameters, but that will lead to more memory fragmentation.

Because of this, if you were previously reading and writing an assembly on the same file, you need to change how you read and write your assembly to:

// ReaderParameters { ReadWrite = true } is necessary to later write the file
using (var module = ModuleDefinition.ReadModule(file, new ReaderParameters { ReadWrite = true })) 
{
    // Modify the assembly

    module.Write (); // Write to the same file that was used to open the file
}

Portable Pdb Support

This is a large change that introduces support for the new portable pdb information. The nice thing is that when users transition to the new format, all features are supported, and you can debug a rewritten assembly without loss of debug information.

It also simplifies quite a bit the debug symbol reader / writer. For 0.11, I'll look into improving the native pdb story.

Things that went away:

Instruction.SequencePoint
VariableReference.Name

The information moved to MethodDefinition.DebugInformation. You can use instead:

MethodDebugInformation.GetSequencePoint(Instruction)
MethodDebugInformation.TryGetName(VariableDefinition, out string)

[jbevain]

One thing that master doesn't have yet, is a way to automatically select whether is should use the native pdb reader or a portable pdb reader. The current workaround right now is to:

private void ReadSymbols (ModuleDefinition module, string moduleFileName)
{
    ISymbolReaderProvider readerProvider = null;

    if (!File.Exists (pdb))
        return;

    const uint PortablePdbSignature = 0x424a5342;

    if (ReadFirst4Bytes(pdb) == PortablePdbSignature)
      readerProvider = new PortablePdbReaderProvider ();
    else
      readerProvider = new PdbReaderProvider ();

    module.ReadSymbols (readerProvider.GetSymbolReader (module, moduleFileName));
}

[jbevain]

One sensible thing to do would be to cherry-pick the fix with the pdb header check to throw a proper exception in 0.9.6-nuget.

[sawilde]

So, if I understand you correctly, to support the new portable pdb format I should take the latest master and build from that branch (what will later become 0.10.) and have custom assemblies of Mono.Cecil until the 0.10. package is released through nuget?

One sensible thing to do would be to cherry-pick the fix with the pdb header check to throw a proper exception in 0.9.6-nuget.

That would help future diagnosis - OpenCover's approach to any exceptions during the handling of symbols is to just treat the assembly as non-coverable.

Instruction.SequencePoint

I use this property a lot as these are the key instrumentation points for OpenCover

Do you have an indication when the new branch will be ready via nuget? I can prep my work now based on the master but I like to release against proper packages rather than a snowflake build - I just need to set expectations with my users who are looking for this functionality.

[jbevain]

If you want support for portable pdb right now then yes, your only option is to build Cecil, as there's no released version that supports it.

I'll be pushing a 0.10-pre version of the package this week-end or this week.

Instruction.SequencePoint has been removed, but as mentioned above, you can replace that with:

method.DebugInformation.GetSequencePoint(instruction)

[sawilde]

awesome - I'll get ahead using a custom build and then link in with your packages later - thx

[manu-st]

Quick question about the removal of Name in VariableReference. We create code that declares variable using "new VariableDefinition ("a", ...);" What is the best replacement for this?

[jbevain]

@manu-silicon you'll need to create a corresponding VariableDebugInformation, and add it to the ScopeDebugInformation that maps to the life time of the variable (you can add it to the root scope to set this name to the variable for the entire method).

[jbevain]

The first 0.10 beta is available, with portable pdb support: http://cecil.pe/post/149243207656/mono-cecil-010-beta-1

[facedbook]

I have a question about PdbFileHeader class, it seems not to support portable pdb format since the magic is "BSJB", not "Microsoft C/C++ MSF 7.00"

[facedbook]

in NativePdbReader, the magic is 0x53445352

[jbevain]

@facedbook NativePdbReader doesn't support Portable Pdb. You need to either use a PortablePdbReaderProvider or a DefaultSymbolReaderProvider to properly read the symbols.

[facedbook]

@jbevain thank you for your answer! Sorry i don't know where to ask questions in github. I want to port a c# coverage tool to support .Net Core.

public void Initialize(string assemblyFilePath)
{
	using (var pdbFileStream = File.OpenRead(Path.ChangeExtension(assemblyFilePath, "pdb")))
	{
		Dictionary<uint, PdbTokenLine> tokenToSourceMapping;
		string svrData;
		int age;
		Guid guid;

		//_pdbFunctions = PdbFile.LoadFunctions(pdbFileStream, true).ToDictionary(func => func.token, func => func);
		_pdbFunctions = PdbFile.LoadFunctions(pdbFileStream, out tokenToSourceMapping, out svrData, out age, out guid).ToDictionary(func => func.token, func => func);
	}
}

But i still cannot figure out how to use PortablePdbReaderProvider to archive the same goal, i.e. read all functions

[jbevain]

@facedbook there's nothing builtin. The Portable Pdb support uses the Cecil object model to represent debug symbol.

You need to work with the method.DebugInformation.