Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support reading embedded pdbs #3454

Merged
merged 2 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,21 @@ private static ISymbolReader GetSymbolReader(string binaryPath)
// For remote scenario, also look for pdb in current directory, (esp for UWP)
// The alternate search path should be an input from Adapters, but since it is not so currently adding a HACK
pdbFilePath = !File.Exists(pdbFilePath) ? Path.Combine(Directory.GetCurrentDirectory(), Path.GetFileName(pdbFilePath)) : pdbFilePath;
using var stream = new FileHelper().GetStream(pdbFilePath, FileMode.Open, FileAccess.Read);
if (PortablePdbReader.IsPortable(stream))

if (File.Exists(pdbFilePath))
{
using var stream = new FileHelper().GetStream(pdbFilePath, FileMode.Open, FileAccess.Read);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we're using File everywhere in this file maybe it's better use also here or create on top and reuse for future testability.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just the code that was here before. Are you talking about using System.IO.File to create the stream? Or about using FileHelper to improve testability?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant have all FileHelper or all File usage, but it's not mandatory I don't like the mix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File helper is used here to get the stream via platform abstractions. It is more correct to use FileHelper everywhere here, but that is a bigger change than what I am comfortable with without adding tests. And I don't want to give this more time right now.

if (PortablePdbReader.IsPortable(stream))
{
return new PortableSymbolReader();
}

return new FullSymbolReader();
}
else
{
// If we cannot find the pdb file, it might be embedded in the dll.
return new PortableSymbolReader();
}

return new FullSymbolReader();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ public PortablePdbReader(Stream stream)
_reader = _provider.GetMetadataReader();
}

/// <summary>
/// Reads the pdb using a provided metadata reader, when the pdb is embedded in the dll, or found by
/// path that is in the dll metadata.
/// </summary>
/// <param name="metadataReaderProvider"></param>
public PortablePdbReader(MetadataReaderProvider metadataReaderProvider!!)
{
_provider = metadataReaderProvider;
_reader = _provider.GetMetadataReader();
}

/// <summary>
/// Dispose Metadata reader
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.IO;
using System.Reflection;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;

using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers;
Expand Down Expand Up @@ -91,7 +93,10 @@ private void PopulateCacheForTypeAndMethodSymbols(string binaryPath)
try
{
var pdbFilePath = Path.ChangeExtension(binaryPath, ".pdb");
using var pdbReader = new PortablePdbReader(new FileHelper().GetStream(pdbFilePath, FileMode.Open, FileAccess.Read));
using PortablePdbReader pdbReader = File.Exists(pdbFilePath)
? CreatePortablePdbReaderFromExistingPdbFile(pdbFilePath)
: CreatePortablePdbReaderFromPEData(binaryPath);

// At this point, the assembly should be already loaded into the load context. We query for a reference to
// find the types and cache the symbol information. Let the loader follow default lookup order instead of
// forcing load from a specific path.
Expand Down Expand Up @@ -148,4 +153,37 @@ private void PopulateCacheForTypeAndMethodSymbols(string binaryPath)
throw;
}
}

/// <summary>
/// Reads the pdb data from the dlls itself, either by loading the referenced pdb file, or by reading
/// embedded pdb from the dll itself.
/// </summary>
/// <param name="binaryPath"></param>
/// <returns></returns>
/// <exception cref="InvalidOperationException"></exception>
private static PortablePdbReader CreatePortablePdbReaderFromPEData(string binaryPath)
{
var peReader = new PEReader(new FileStream(binaryPath, FileMode.Open, FileAccess.Read));

// (_) => null is stream reader factory for the pdb. We don't need that stream.
// out _ is the path to the found pdb, we also don't need that.
var hasPdb = peReader.TryOpenAssociatedPortablePdb(binaryPath, (_) => null, out MetadataReaderProvider mp, out _);

// The out parameters don't give additional info about the pdbFile in case it is not found. So we have few reasons to fail:
if (!hasPdb)
{
throw new InvalidOperationException($"Cannot find portable .PDB file for {binaryPath}. This can have multiple reasons:"
+ "\n- The dll was built with <DebugType>portable</DebugType> and the pdb file is missing (it was deleted, or not moved together with the dll)."
+ "\n- The dll was built with <DebugType>embedded</DebugType> and there is some unknown error reading the metadata from the dll."
+ "\n- The sll was built with <DebugType>none</DebugType> and the pdb file was never even emitted during build."
+ "\n- Additionally if your dll is built with <DebugType>full</DebugType>, see FullPdbReader instead.");
}

return new PortablePdbReader(mp);
}

private static PortablePdbReader CreatePortablePdbReaderFromExistingPdbFile(string pdbFilePath)
{
return new PortablePdbReader(new FileHelper().GetStream(pdbFilePath, FileMode.Open, FileAccess.Read));
}
}