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.

Sign up for GitHub

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

Include source files w/o method bodies in the PDB documents #39136

Merged
merged 9 commits into from
Oct 14, 2019
Merged

Include source files w/o method bodies in the PDB documents #39136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d26e2ef
Include source files w/o method bodies in the PDB documents
ivanbasov Oct 8, 2019
68bb295
code review feedback
ivanbasov Oct 9, 2019
a4cef18
updated PDB tests
ivanbasov Oct 9, 2019
7b98102
code review feedback
ivanbasov Oct 10, 2019
a200fc8
code review feedback
ivanbasov Oct 10, 2019
25ee210
skipping windows tests in mac and linux
ivanbasov Oct 10, 2019
c08e2a2
minor refactoring
ivanbasov Oct 10, 2019
eaab9f3
revert skipping C# tests in linus and mac
ivanbasov Oct 10, 2019
28c8b82
disabling skipped test
ivanbasov Oct 14, 2019
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
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Test/Emit/PDB/CheckSumTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ static void Main()
<file id=""2"" name=""USED2.cs"" language=""C#"" checksumAlgorithm=""406ea660-64cf-4c82-b6f0-42d48172a799"" checksum=""AB-00-7F-1D-23-D9"" />
<file id=""3"" name=""b.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""C0-51-F0-6F-D3-ED-44-A2-11-4D-03-70-89-20-A6-05-11-62-14-BE"" />
<file id=""4"" name=""a.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""F0-C4-23-63-A5-89-B9-29-AF-94-07-85-2F-3A-40-D3-70-14-8F-9B"" />
<file id=""5"" name=""UNUSED.cs"" language=""C#"" checksumAlgorithm=""406ea660-64cf-4c82-b6f0-42d48172a799"" checksum=""AB-00-7F-1D-23-D9"" />
</files>
<methods>
<method containingType=""C"" name=""Main"">
Expand Down Expand Up @@ -286,7 +287,7 @@ void M()
</symbols>");
}

[Fact]
[ConditionalFact(typeof(WindowsOnly))]
public void NoResolver()
{
var comp = CSharpCompilation.Create(
Expand All @@ -305,6 +306,7 @@ class C { void M() { } }
<symbols>
<files>
<file id=""1"" name=""a\..\a.cs"" language=""C#"" checksumAlgorithm=""406ea660-64cf-4c82-b6f0-42d48172a799"" checksum=""AB-00-7F-1D-23-D5"" />
<file id=""2"" name=""C:\a\..\b.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""36-39-3C-83-56-97-F2-F0-60-95-A4-A0-32-C6-32-C7-B2-4B-16-92"" />
</files>
<methods>
<method containingType=""C"" name=""M"">
Expand Down
73 changes: 73 additions & 0 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10511,6 +10511,7 @@ public async void M1()
@"<symbols>
<files>
<file id=""1"" name=""C:\Async.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""DB-EB-2A-06-7B-2F-0E-0D-67-8A-00-2C-58-7A-28-06-05-6C-3D-CE"" />
<file id=""2"" name=""HIDDEN.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""8A-92-EE-2F-D6-6F-C0-69-F4-A8-54-CB-11-BE-A3-06-76-2C-9C-98"" />
</files>
<methods>
<method containingType=""C"" name=""M1"">
Expand Down Expand Up @@ -10577,6 +10578,7 @@ partial class C
<symbols>
<files>
<file id=""1"" name=""constructor.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""EA-D6-0A-16-6C-6A-BC-C1-5D-98-0F-B7-4B-78-13-93-FB-C7-C2-5A"" />
<file id=""2"" name=""initializer.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""84-32-24-D7-FE-32-63-BA-41-D5-17-A2-D5-90-23-B8-12-3C-AF-D5"" />
</files>
<methods>
<method containingType=""C"" name="".ctor"">
Expand Down Expand Up @@ -11037,5 +11039,76 @@ public void InvalidCharacterInPdbPath()
Diagnostic(ErrorCode.FTL_InvalidInputFileName).WithArguments("test\\?.pdb").WithLocation(1, 1));
}
}

[Fact]
[WorkItem(38954, "https://github.com/dotnet/roslyn/issues/38954")]
public void FilesOneWithNoMethodBody()
{
string source1 = WithWindowsLineBreaks(@"
using System;

class C
{
public static void Main()
{
Console.WriteLine();
}
}
");
string source2 = WithWindowsLineBreaks(@"
// no code
");

var tree1 = Parse(source1, "f:/build/goo.cs");
var tree2 = Parse(source2, "f:/build/nocode.cs");
var c = CreateCompilation(new[] { tree1, tree2 }, options: TestOptions.DebugDll);

c.VerifyPdb(@"
<symbols>
<files>
<file id=""1"" name=""f:/build/goo.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""5D-7D-CF-1B-79-12-0E-0A-80-13-E0-98-7E-5C-AA-3B-63-D8-7E-4F"" />
<file id=""2"" name=""f:/build/nocode.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""8B-1D-3F-75-E0-A8-8F-90-B2-D3-52-CF-71-9B-17-29-3C-70-7A-42"" />
</files>
<methods>
<method containingType=""C"" name=""Main"">
<customDebugInfo>
<using>
<namespace usingCount=""1"" />
</using>
</customDebugInfo>
<sequencePoints>
<entry offset=""0x0"" startLine=""7"" startColumn=""5"" endLine=""7"" endColumn=""6"" document=""1"" />
<entry offset=""0x1"" startLine=""8"" startColumn=""9"" endLine=""8"" endColumn=""29"" document=""1"" />
<entry offset=""0x7"" startLine=""9"" startColumn=""5"" endLine=""9"" endColumn=""6"" document=""1"" />
</sequencePoints>
<scope startOffset=""0x0"" endOffset=""0x8"">
<namespace name=""System"" />
</scope>
</method>
</methods>
</symbols>
");
}

[Fact]
[WorkItem(38954, "https://github.com/dotnet/roslyn/issues/38954")]
public void SingleFileWithNoMethodBody()
{
string source = WithWindowsLineBreaks(@"
// no code
");

var tree = Parse(source, "f:/build/nocode.cs");
var c = CreateCompilation(new[] { tree }, options: TestOptions.DebugDll);

c.VerifyPdb(@"
<symbols>
<files>
<file id=""1"" name=""f:/build/nocode.cs"" language=""C#"" checksumAlgorithm=""SHA1"" checksum=""8B-1D-3F-75-E0-A8-8F-90-B2-D3-52-CF-71-9B-17-29-3C-70-7A-42"" />
</files>
<methods />
</symbols>
");
}
}
}
5 changes: 0 additions & 5 deletions src/Compilers/Core/Portable/Compilation/Compilation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2068,8 +2068,6 @@ internal bool CreateDebugDocuments(DebugDocumentsBuilder documentsBuilder, IEnum
// takes priority over the syntax tree pass, which will not embed.
if (!embeddedTexts.IsEmpty())
{
var embeddedDocuments = ArrayBuilder<Cci.DebugSourceDocument>.GetInstance();

foreach (var text in embeddedTexts)
{
Debug.Assert(!string.IsNullOrEmpty(text.FilePath));
Expand All @@ -2083,11 +2081,8 @@ internal bool CreateDebugDocuments(DebugDocumentsBuilder documentsBuilder, IEnum
() => text.GetDebugSourceInfo());

documentsBuilder.AddDebugDocument(document);
embeddedDocuments.Add(document);
}
}

documentsBuilder.EmbeddedDocuments = embeddedDocuments.ToImmutableAndFree();
}

// Add debug documents for all trees with distinct paths.
Expand Down
14 changes: 4 additions & 10 deletions src/Compilers/Core/Portable/Emit/DebugDocumentsBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
using Roslyn.Utilities;
using System;
using System.Collections.Concurrent;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Collections.Generic;

namespace Microsoft.CodeAnalysis.Emit
{
Expand All @@ -19,7 +18,6 @@ internal sealed class DebugDocumentsBuilder
private readonly ConcurrentDictionary<string, Cci.DebugSourceDocument> _debugDocuments;
private readonly ConcurrentCache<(string, string), string> _normalizedPathsCache;
private readonly SourceReferenceResolver _resolverOpt;
private ImmutableArray<Cci.DebugSourceDocument> _embeddedDocuments;

public DebugDocumentsBuilder(SourceReferenceResolver resolverOpt, bool isDocumentNameCaseSensitive)
{
Expand All @@ -31,13 +29,6 @@ public DebugDocumentsBuilder(SourceReferenceResolver resolverOpt, bool isDocumen
StringComparer.OrdinalIgnoreCase);

_normalizedPathsCache = new ConcurrentCache<(string, string), string>(16);
_embeddedDocuments = ImmutableArray<Cci.DebugSourceDocument>.Empty;
}

internal ImmutableArray<Cci.DebugSourceDocument> EmbeddedDocuments
{
get { return _embeddedDocuments; }
set { Debug.Assert(value != null); _embeddedDocuments = value; }
}

internal int DebugDocumentCount => _debugDocuments.Count;
Expand All @@ -47,6 +38,9 @@ internal void AddDebugDocument(Cci.DebugSourceDocument document)
_debugDocuments.Add(document.Location, document);
}

internal IReadOnlyDictionary<string, Cci.DebugSourceDocument> DebugDocuments
=> _debugDocuments;

internal Cci.DebugSourceDocument TryGetDebugDocument(string path, string basePath)
{
return TryGetDebugDocumentForNormalizedPath(NormalizeDebugDocumentPath(path, basePath));
Expand Down
21 changes: 13 additions & 8 deletions src/Compilers/Core/Portable/NativePdbWriter/PdbWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;
using System.Runtime.InteropServices;
using System.Security.Cryptography;
using System.Text;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -572,6 +571,11 @@ private int GetDocumentIndex(DebugSourceDocument document)
return documentIndex;
}

return AddDocumentIndex(document);
}

private int AddDocumentIndex(DebugSourceDocument document)
{
Guid algorithmId;
ReadOnlySpan<byte> checksum;
ReadOnlySpan<byte> embeddedSource;
Expand All @@ -597,7 +601,7 @@ private int GetDocumentIndex(DebugSourceDocument document)
embeddedSource = null;
}

documentIndex = _symWriter.DefineDocument(
int documentIndex = _symWriter.DefineDocument(
document.Location,
document.Language,
document.LanguageVendor,
Expand Down Expand Up @@ -742,18 +746,19 @@ public void EmbedSourceLink(Stream stream)
}

/// <summary>
/// Write document entries for any embedded text document that does not yet have an entry.
/// Write document entries for all debug documents that do not yet have an entry.
/// </summary>
/// <remarks>
/// This is done after serializing method debug info to ensure that we embed all requested
/// text even if there are no corresponding sequence points.
/// </remarks>
public void WriteRemainingEmbeddedDocuments(IEnumerable<DebugSourceDocument> embeddedDocuments)
public void WriteRemainingDebugDocuments(IReadOnlyDictionary<string, DebugSourceDocument> documents)
{
foreach (var document in embeddedDocuments)
foreach (var kvp in documents
.Where(kvp => !_documentIndex.ContainsKey(kvp.Value))
.OrderBy(kvp => kvp.Key))
Copy link
Member

Choose a reason for hiding this comment

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

OrderBy [](start = 17, length = 7)

I didn't understand why we need ordering. There are other methods that invoke AddDocumentIndex and don't seem to do so in any particular order (namely GetDocumentIndex).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to resolve #39136 (comment)
I think there is some kind of ordering in documents for ones having sequence points likely governed by sequence points ordering. And we want to have some consistence for the order of documents without sequence points. Let them be ordered by file names. @tmat may want to comment more.

Copy link
Member

Choose a reason for hiding this comment

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

Any time we produce output based on dictionary enumeration we need to order the results in order to achieve determinism.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Thanks

{
Debug.Assert(!document.GetSourceInfo().EmbeddedTextBlob.IsDefault);
GetDocumentIndex(document);
AddDocumentIndex(kvp.Value);
}
}
}
Expand Down
50 changes: 29 additions & 21 deletions src/Compilers/Core/Portable/PEWriter/MetadataWriter.PortablePdb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,44 +727,52 @@ private void SerializeDeltaLinesAndColumns(BlobBuilder writer, SequencePoint seq

private DocumentHandle GetOrAddDocument(DebugSourceDocument document, Dictionary<DebugSourceDocument, DocumentHandle> index)
{
DocumentHandle documentHandle;
if (!index.TryGetValue(document, out documentHandle))
if (index.TryGetValue(document, out var documentHandle))
{
DebugSourceInfo info = document.GetSourceInfo();
return documentHandle;
}

documentHandle = _debugMetadataOpt.AddDocument(
name: _debugMetadataOpt.GetOrAddDocumentName(document.Location),
hashAlgorithm: info.Checksum.IsDefault ? default(GuidHandle) : _debugMetadataOpt.GetOrAddGuid(info.ChecksumAlgorithmId),
hash: info.Checksum.IsDefault ? default(BlobHandle) : _debugMetadataOpt.GetOrAddBlob(info.Checksum),
language: _debugMetadataOpt.GetOrAddGuid(document.Language));
return AddDocument(document, index);
}

index.Add(document, documentHandle);
private DocumentHandle AddDocument(DebugSourceDocument document, Dictionary<DebugSourceDocument, DocumentHandle> index)
{
DocumentHandle documentHandle;
DebugSourceInfo info = document.GetSourceInfo();

if (info.EmbeddedTextBlob != null)
{
_debugMetadataOpt.AddCustomDebugInformation(
parent: documentHandle,
kind: _debugMetadataOpt.GetOrAddGuid(PortableCustomDebugInfoKinds.EmbeddedSource),
value: _debugMetadataOpt.GetOrAddBlob(info.EmbeddedTextBlob));
}
documentHandle = _debugMetadataOpt.AddDocument(
name: _debugMetadataOpt.GetOrAddDocumentName(document.Location),
hashAlgorithm: info.Checksum.IsDefault ? default(GuidHandle) : _debugMetadataOpt.GetOrAddGuid(info.ChecksumAlgorithmId),
hash: info.Checksum.IsDefault ? default(BlobHandle) : _debugMetadataOpt.GetOrAddBlob(info.Checksum),
language: _debugMetadataOpt.GetOrAddGuid(document.Language));

index.Add(document, documentHandle);

if (info.EmbeddedTextBlob != null)
{
_debugMetadataOpt.AddCustomDebugInformation(
parent: documentHandle,
kind: _debugMetadataOpt.GetOrAddGuid(PortableCustomDebugInfoKinds.EmbeddedSource),
value: _debugMetadataOpt.GetOrAddBlob(info.EmbeddedTextBlob));
}

return documentHandle;
}

/// <summary>
/// Add document entries for any embedded text document that does not yet have an entry.
/// Add document entries for all debug documents that do not yet have an entry.
/// </summary>
/// <remarks>
/// This is done after serializing method debug info to ensure that we embed all requested
/// text even if there are no corresponding sequence points.
/// </remarks>
public void AddRemainingEmbeddedDocuments(IEnumerable<DebugSourceDocument> documents)
public void AddRemainingDebugDocuments(IReadOnlyDictionary<string, DebugSourceDocument> documents)
{
foreach (var document in documents)
foreach (var kvp in documents
.Where(kvp => !_documentIndex.ContainsKey(kvp.Value))
.OrderBy(kvp => kvp.Key))
{
Debug.Assert(document.GetSourceInfo().EmbeddedTextBlob != null);
GetOrAddDocument(document, _documentIndex);
AddDocument(kvp.Value, _documentIndex);
}
}
Copy link
Member

@tmat tmat Oct 9, 2019 •
edited by ivanbasov
Loading

Choose a reason for hiding this comment

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

Why do we need two methods? They seem to be doing the same thing. #Resolved


Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/Core/Portable/PEWriter/PeWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ internal static bool WritePeToStream(
#endif
}

nativePdbWriterOpt.WriteRemainingEmbeddedDocuments(mdWriter.Module.DebugDocumentsBuilder.EmbeddedDocuments);
nativePdbWriterOpt.WriteRemainingDebugDocuments(mdWriter.Module.DebugDocumentsBuilder.DebugDocuments);
}

Stream peStream = getPeStream();
Expand Down Expand Up @@ -151,7 +151,7 @@ internal static bool WritePeToStream(
BlobBuilder portablePdbToEmbed = null;
if (mdWriter.EmitPortableDebugMetadata)
{
mdWriter.AddRemainingEmbeddedDocuments(mdWriter.Module.DebugDocumentsBuilder.EmbeddedDocuments);
mdWriter.AddRemainingDebugDocuments(mdWriter.Module.DebugDocumentsBuilder.DebugDocuments);

// The algorithm must be specified for deterministic builds (checked earlier).
Debug.Assert(!isDeterministic || context.Module.PdbChecksumAlgorithm.Name != null);
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/VisualBasic/Test/Emit/PDB/ChecksumTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ End Class
<files>
<file id="1" name="b:\base\line.vb" language="VB"/>
<file id="2" name="q:\absolute\line.vb" language="VB"/>
<file id="3" name="b:\base\b.vb" language="VB" checksumAlgorithm="SHA1" checksum="F9-90-00-9D-9E-45-97-F2-3D-67-1C-D8-47-A8-9B-DA-4A-91-AA-7F"/>
</files>
<methods>
<method containingType="C" name="M">
Expand Down Expand Up @@ -312,6 +313,7 @@ End Class
<file id="3" name="b:\base\c.vb" language="VB" checksumAlgorithm="406ea660-64cf-4c82-b6f0-42d48172a79a" checksum="AB-00-7F-1D-23-DC"/>
<file id="4" name="b:\base\d.vb" language="VB" checksumAlgorithm="406ea660-64cf-4c82-b6f0-42d48172a79a" checksum="AB-00-7F-1D-23-DD"/>
<file id="5" name="b:\base\e.vb" language="VB" checksumAlgorithm="406ea660-64cf-4c82-b6f0-42d48172a79a" checksum="AB-00-7F-1D-23-DE"/>
<file id="6" name="b:\base\file.vb" language="VB" checksumAlgorithm="SHA1" checksum="C2-46-C6-34-F6-20-D3-FE-28-B9-D8-62-0F-A9-FB-2F-89-E7-48-23"/>
</files>
<methods>
<method containingType="C" name="M">
Expand Down Expand Up @@ -363,6 +365,7 @@ End Class
<file id="1" name="a.vb" language="VB" checksumAlgorithm="406ea660-64cf-4c82-b6f0-42d48172a79a" checksum="AB-00-7F-1D-23-DA"/>
<file id="2" name="./a.vb" language="VB"/>
<file id="3" name="b.vb" language="VB"/>
<file id="4" name="file.vb" language="VB" checksumAlgorithm="SHA1" checksum="23-C1-6B-94-B0-D4-06-26-C8-D2-82-21-63-07-53-11-4D-5A-02-BC"/>
</files>
<methods>
<method containingType="C" name="M">
Expand Down
Loading