Add support for embedding sources in Windows PDBs

dotnet · Sep 9, 2016 · 4d174cf · 4d174cf
1 parent c6127d4
commit 4d174cf
Show file tree

Hide file tree

Showing 12 changed files with 71 additions and 49 deletions.
diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx
@@ -4942,7 +4942,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>/sourcelink switch is only supported when emitting Portable PDB (/debug:portable or /debug:embedded must be specified).</value>
   </data>
   <data name="ERR_CannotEmbedWithoutPdb" xml:space="preserve">
-    <value>/embed switch is only supported when emitting Portable PDB (/debug:portable or /debug:embedded).</value>
+    <value>/embed switch is only supported when emitting a PDB.</value>
   </data>
   <data name="ERR_InvalidInstrumentationKind" xml:space="preserve">
     <value>Invalid instrumentation kind: {0}</value>

diff --git a/src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs b/src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs
@@ -1193,9 +1193,7 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
 
             if (embeddedFiles.Count > 0)
             {
-                // Restricted to portable PDBs for now, but the IsPortable condition should be removed
-                // and the error message adjusted accordingly when native PDB support is added.
-                if (!emitPdb || !debugInformationFormat.IsPortable())
+                if (!emitPdb)
                 {
                     AddDiagnostic(diagnostics, ErrorCode.ERR_CannotEmbedWithoutPdb);
                 }

diff --git a/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs b/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
@@ -1732,19 +1732,6 @@ public void Embed()
 
             parsedArgs = DefaultParse(new[] { "/embed:a.txt", "/debug-", "a.cs" }, _baseDirectory);
             parsedArgs.Errors.Verify(Diagnostic(ErrorCode.ERR_CannotEmbedWithoutPdb));
-
-            // These should fail when native PDB support is added.
-            parsedArgs = DefaultParse(new[] { "/embed", "/debug:full", "a.cs" }, _baseDirectory);
-            parsedArgs.Errors.Verify(Diagnostic(ErrorCode.ERR_CannotEmbedWithoutPdb));
-
-            parsedArgs = DefaultParse(new[] { "/embed", "/debug:full", "a.cs" }, _baseDirectory);
-            parsedArgs.Errors.Verify(Diagnostic(ErrorCode.ERR_CannotEmbedWithoutPdb));
-
-            parsedArgs = DefaultParse(new[] { "/embed", "/debug:pdbonly", "a.cs" }, _baseDirectory);
-            parsedArgs.Errors.Verify(Diagnostic(ErrorCode.ERR_CannotEmbedWithoutPdb));
-
-            parsedArgs = DefaultParse(new[] { "/embed", "/debug+", "a.cs" }, _baseDirectory);
-            parsedArgs.Errors.Verify(Diagnostic(ErrorCode.ERR_CannotEmbedWithoutPdb));
         }
 
         [Theory]

diff --git a/src/Compilers/CSharp/Test/Symbol/Compilation/CompilationAPITests.cs b/src/Compilers/CSharp/Test/Symbol/Compilation/CompilationAPITests.cs
@@ -299,8 +299,14 @@ public void Emit_BadArgs()
 
             Assert.Throws<ArgumentException>("embeddedTexts", () => comp.Emit(
                 peStream: new MemoryStream(),
-                pdbStream: new MemoryStream(),
-                options: EmitOptions.Default.WithDebugInformationFormat(DebugInformationFormat.Pdb),
+                pdbStream: null,
+                options: null,
+                embeddedTexts: new[] { EmbeddedText.FromStream("_", new MemoryStream()) }));
+
+            Assert.Throws<ArgumentException>("embeddedTexts", () => comp.Emit(
+                peStream: new MemoryStream(),
+                pdbStream: null,
+                options: null,
                 embeddedTexts: new[] { EmbeddedText.FromStream("_", new MemoryStream()) }));
 
             Assert.Throws<ArgumentException>("embeddedTexts", () => comp.Emit(

diff --git a/src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs b/src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs
diff --git a/src/Compilers/Core/Portable/CodeAnalysisResources.resx b/src/Compilers/Core/Portable/CodeAnalysisResources.resx
@@ -549,8 +549,8 @@
   <data name="StreamIsTooLong" xml:space="preserve">
     <value>Stream is too long.</value>
   </data>
-  <data name="EmbeddedTextsRequirePortablePdb" xml:space="preserve">
-    <value>Embedded texts are only supported when emitting Portable PDB.</value>
+  <data name="EmbeddedTextsRequirePdb" xml:space="preserve">
+    <value>Embedded texts are only supported when emitting a PDB.</value>
   </data>
   <data name="TheStreamCannotBeWrittenTo" xml:space="preserve">
     <value>The stream cannot be written to.</value>

diff --git a/src/Compilers/Core/Portable/Compilation/Compilation.cs b/src/Compilers/Core/Portable/Compilation/Compilation.cs
@@ -1840,13 +1840,12 @@ public EmitResult Emit(
                     throw new ArgumentException(CodeAnalysisResources.StreamMustSupportRead, nameof(sourceLinkStream));
                 }
             }
+
             if (embeddedTexts != null && !embeddedTexts.IsEmpty())
             {
-                if (options == null || 
-                    options.DebugInformationFormat == DebugInformationFormat.Pdb ||
-                    options.DebugInformationFormat == DebugInformationFormat.PortablePdb && pdbStream == null)
+                if (pdbStream == null && options?.DebugInformationFormat != DebugInformationFormat.Embedded)
                 {
-                    throw new ArgumentException(CodeAnalysisResources.EmbeddedTextsRequirePortablePdb, nameof(embeddedTexts));
+                    throw new ArgumentException(CodeAnalysisResources.EmbeddedTextsRequirePdb, nameof(embeddedTexts));
                 }
             }
 

diff --git a/src/Compilers/Core/Portable/NativePdbWriter/PdbWriter.cs b/src/Compilers/Core/Portable/NativePdbWriter/PdbWriter.cs
@@ -963,7 +963,7 @@ private ISymUnmanagedDocumentWriter GetDocumentWriter(DebugSourceDocument docume
                     try
                     {
                         var algorithmId = info.ChecksumAlgorithmId;
-                        var checksum = info.Checksum.ToArray();
+                        var checksum = ImmutableByteArrayInterop.DangerousGetUnderlyingArray(info.Checksum);
                         var checksumSize = (uint)checksum.Length;
                         writer.SetCheckSum(algorithmId, checksumSize, checksum);
                         if (_callLogger.LogOperation(OP.SetCheckSum))
@@ -979,8 +979,38 @@ private ISymUnmanagedDocumentWriter GetDocumentWriter(DebugSourceDocument docume
                     }
                 }
 
-                // embedded text not currently supported for native PDB and we should have validated that
-                Debug.Assert(info.EmbeddedTextBlob.IsDefault);
+                if (!info.EmbeddedTextBlob.IsDefault)
+                {
+                    try
+                    {
+                        uint blobSize = (uint)info.EmbeddedTextBlob.Length;
+
+                        writer.SetSource(blobSize, ImmutableByteArrayInterop.DangerousGetUnderlyingArray(info.EmbeddedTextBlob));
+
+                        if (_callLogger.LogOperation(OP.SetSource))
+                        {
+                            _callLogger.LogArgument(blobSize);
+
+                            // Since we only have embedded text for documents that have a computed checksum,
+                            // (otherwise we'd have raised ERR_EncodinglessSyntaxTree), we can rely on
+                            // having already logged the checksum (cryptographic hash) and skip writing the
+                            // entire embedded text to the log (which can be large).
+                            Debug.Assert(document.IsComputedChecksum);
+                            Debug.Assert(!info.Checksum.IsDefault);
+
+                            // We do, however, log the first 4 bytes which encode formatting that is not
+                            // included in the checksum.
+                            _callLogger.LogArgument(info.EmbeddedTextBlob[0]);
+                            _callLogger.LogArgument(info.EmbeddedTextBlob[1]);
+                            _callLogger.LogArgument(info.EmbeddedTextBlob[2]);
+                            _callLogger.LogArgument(info.EmbeddedTextBlob[3]);
+                        }
+                    }
+                    catch (Exception ex)
+                    {
+                        throw new PdbWritingException(ex);
+                    }
+                }
             }
 
             return writer;
@@ -1494,6 +1524,22 @@ public void WriteDefinitionLocations(MultiDictionary<DebugSourceDocument, Defini
             }
         }
 
+        /// <summary>
+        /// Write document entries for any embedded text document that does 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 correspodning sequence points.
+        /// </remarks>
+        public void WriteRemainingEmbeddedDocuments(IEnumerable<DebugSourceDocument> embeddedDocuments)
+        {
+            foreach (var document in embeddedDocuments)
+            {
+                Debug.Assert(!document.GetSourceInfo().EmbeddedTextBlob.IsDefault);
+                GetDocumentWriter(document);
+            }
+        }
+
         #endregion
     }
 }
diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs
@@ -90,8 +90,7 @@ public static bool WritePeToStream(
 #endif
                 }
 
-                // embedded text not currently supported for native PDB and we should have validated that
-                Debug.Assert(!mdWriter.Module.DebugDocumentsBuilder.EmbeddedDocuments.Any());
+                nativePdbWriterOpt.WriteRemainingEmbeddedDocuments(mdWriter.Module.DebugDocumentsBuilder.EmbeddedDocuments);
             }
 
             Stream peStream = getPeStream();

diff --git a/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb b/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb
@@ -2593,19 +2593,6 @@ End Class")
 
             parsedArgs = DefaultParse({"/embed:a.txt", "/debug-", "a.vb"}, _baseDirectory)
             parsedArgs.Errors.Verify(Diagnostic(ERRID.ERR_CannotEmbedWithoutPdb))
-
-            ' These should fail when native PDB support is added.
-            parsedArgs = DefaultParse({"/embed", "/debug:full", "a.vb"}, _baseDirectory)
-            parsedArgs.Errors.Verify(Diagnostic(ERRID.ERR_CannotEmbedWithoutPdb))
-
-            parsedArgs = DefaultParse({"/embed", "/debug:full", "a.vb"}, _baseDirectory)
-            parsedArgs.Errors.Verify(Diagnostic(ERRID.ERR_CannotEmbedWithoutPdb))
-
-            parsedArgs = DefaultParse({"/embed", "/debug:pdbonly", "a.vb"}, _baseDirectory)
-            parsedArgs.Errors.Verify(Diagnostic(ERRID.ERR_CannotEmbedWithoutPdb))
-
-            parsedArgs = DefaultParse({"/embed", "/debug+", "a.vb"}, _baseDirectory)
-            parsedArgs.Errors.Verify(Diagnostic(ERRID.ERR_CannotEmbedWithoutPdb))
         End Sub
 
         <Theory>

diff --git a/src/Compilers/VisualBasic/Test/Semantic/Compilation/CompilationAPITests.vb b/src/Compilers/VisualBasic/Test/Semantic/Compilation/CompilationAPITests.vb
@@ -267,8 +267,8 @@ End Namespace
 
             Assert.Throws(Of ArgumentException)("embeddedTexts", Sub() comp.Emit(
                 peStream:=New MemoryStream(),
-                pdbStream:=New MemoryStream(),
-                options:=EmitOptions.Default.WithDebugInformationFormat(DebugInformationFormat.Pdb),
+                pdbStream:=Nothing,
+                options:=Nothing,
                 embeddedTexts:={EmbeddedText.FromStream("_", New MemoryStream())}))
 
             Assert.Throws(Of ArgumentException)("embeddedTexts", Sub() comp.Emit(