Teach `llvm-pdbutil pretty -native` about `-injected-sources`

[thakis]

Summary

pretty -native -injected-sources -injected-source-content works with this patch, and produces identical output to the dia version

• I'd like to get feedback on the high-level design here, since I'm not familiar with how things are supposed to be done in this code. In particular, does the split between NativeEnumInjectedSources and InjectedSourceStream make sense? Are these names consistent with the rest of the code?

• The injected source _writing_ is done in PDBFileBuilder. Now that InjectedSourceStream exists, should there be a InjectedSourceStreamBuilder that does the building part? (If so, probably in a follow-up)

(I wish I had had this when working on PR41626, and when I was looking at natvis stuff again the other day I realized I had almost forgotten all this stuff again. So I figured I'd try to implement this while I still remember it.)

[thakis]

llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp

937 ↗

(On Diff #208735)

(This is needed because NativeSession returns nullptr if there are no injected sources. DIASession also has a return nullptr in one case, but I guess it's never hit in practice?)

[thakis]

-injected-source-content works

[thakis]

wip

[thakis]

better

[thakis]

rebase, done

[thakis]

Probably time to call ETIMEOUT on zturner. rnk, can you look at this one too given you already have seen all the prerequisite CLs? I'm pretty happy with this by now.

[thakis]

remove a dead function

[rnk]

I'd like to get feedback on the high-level design here, since I'm not familiar with how things are supposed to be done in this code. In particular, does the split between NativeEnumInjectedSources and InjectedSourceStream make sense? Are these names consistent with the rest of the code?

Looks consistent with the existing DIA interface implementations to me, so I'd go with it.

The injected source _writing_ is done in PDBFileBuilder. Now that InjectedSourceStream exists, should there be a InjectedSourceStreamBuilder that does the building part? (If so, probably in a follow-up)

Sounds like a good follow-up.

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

40 ↗

(On Diff #209930)

I think this class only uses I->second throughout, so would it be simpler to take a reference to the hash table value? I think it's SrcHeaderBlockEntry, right? I think that would be more readable, since we aren't iterating, we shouldn't need an iterator, we're just looking at one source file entry.

50 ↗

(On Diff #209930)

Hm. I guess perhaps we should've allowed error propagation out of these APIs. I guess that's not fixable without an interface change.

[thakis]

give NativeInjectedSource a SrcHeaderBlockEntry instead of a const_iterator

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

40 ↗

(On Diff #209930)

Good suggestion, done.

[zturner]

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

50 ↗

(On Diff #209930)

The thinking at the time was that anything that might generate an error should happen during load, not on lazy evaluation of the properties. That's why most of the classes have a initialize or reload() static method which returns an Expected<T>, so that all validation can be done up front. I'm open to the possibility that that wasn't the best decision though, but it kept the code simple and handled most of the cases we would expect to see in practice.

[thakis]

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

50 ↗

(On Diff #209930)

Thanks for chiming in! That makes sense to me.

I changed the code to verify that all referenced strings exist in reload() and removed the error handling here. I didn't (yet) do this for getCode() since that'd duplicate quite a bit of code. From your comment, it kind of sounds like the intended design is that InjectedSourceStream should grab _all_ data from disk and store it in memory, and the NativeEnumInjectedSources should refer to that. Is that accurate? If so, InjectedSourceStream should probably have a

struct InjectedSource {
  uint32_t CRC32;
  StringRef Name;
  StringRef ObjName;
  StringRef VName;
  PDB_SourceCompression Compression;
  llvm::MemoryBuffer Contents;
};

and store a name->InjectedSource map, and check that the CRC32 matches the CRC32 of Contents at load time too. Does that sound right?

If so, is there an existing method to load the contents of a stream into a MemoryBuffer somewhere?

[thakis]

half-address zturner comment

[thakis]

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

50 ↗

(On Diff #209930)

Looking at this some more: For just natvis files, loading everything into memory seems fine, but the docs for IDiaInjectedSource kind of suggest that it's possible to store the source code of all cc files compiled to obj files in the pdb, and loading that eagerly would be quite a bit of data. OTOH, it's only loaded eagerly if PDBFile::getInjectedSourceStream() is called, and the only caller of it currently walks all entries anyhow.

[rnk]

lgtm, up to you if you want to wait for @zturner. I like the new error handling approach. :)

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

50 ↗

(On Diff #209930)

If so, is there an existing method to load the contents of a stream into a MemoryBuffer somewhere?

I don't think so, we moved data into and out of the stream APIs with ArrayRef<uint8_t>, typically stored in BumpPtrAllocators.

[thakis]

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

50 ↗

(On Diff #209930)

Thanks, I'll land this for now as it makes llvm-pdbutil more useful, and then I can iterate from there.

I found out how to get source files into /src/headerblock, it's a C# thing (but it still doesn't set obj):

C:\src\llvm-mono>type Hello.cs
using System;
namespace HelloWorld {
class Hello {
  static void Main() { Console.WriteLine("Hello World!"); }
}
}

C:\src\llvm-mono>csc Hello.cs /debug
Microsoft (R) Visual C# Compiler version 2.10.0.0 (b9fb1610)
Copyright (C) Microsoft Corporation. All rights reserved.


C:\src\llvm-mono>csc Hello.cs /debug /nologo

C:\src\llvm-mono>out\gn\bin\llvm-pdbutil.exe pretty -injected-sources Hello.pdb
Summary for C:\src\llvm-mono\Hello.pdb
  Size: 11776 bytes
  Guid: {9F3B98FA-6D65-D34E-8CF9-86994C5C9569}
  Age: 1
  Attributes: HasPrivateSymbols
---INJECTED SOURCES---
  C:\src\llvm-mono\Hello.cs (92 bytes): obj=<null>, vname=c:\src\llvm-mono\hello.cs, crc=3766985073, compression=Unknown (101)

(llvm-mono just means "monorepo"; in the c# context it could lead to confusion I suppose :P)

[zturner]

llvm/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

50 ↗

(On Diff #209930)

I don't think you need a MemoryBuffer, because a MemoryBuffer has to be contiguous, which would force a copy out of the mapped PDB.

That's what the BinaryStreamRef stuff is for, it's essentially a wrapper over a PDB file, and logical offset+length in the PDB file. You can read from it as if it were a memory buffer, but it handles the case where the buffer isn't contiguous in the backing file for you and only copies when necessary.

So your idea is basically correct, except I'd store a BinaryStreamRef instead of a MemoryBuffer. I think BinaryStreamReader has a method called readBinaryStreamRef (or something similar), that takes a length and just returns you a binary stream ref that is safe to store (at least until you close the master PDBFile).

DbiStream::initialize() is a good example, because it does stuff like this already.

50 ↗

(On Diff #209930)

Yea it's fine to land this now and iterate on it. FWIW, I think it's probably *more* work to get everything into a MemoryBuffer than it is to just save off a BinaryStreamRef, and then it'll just work no matter what or how much stuff you store in there. But maybe I'm biased :)

And yes, it's possible to store arbitrary file content into the injected sources. I had the idea that we could do this for generated code (for example the output of protoc), this way you could debug into code that is generated as part of build step and won't ever be archived on source server. In theory Injected Sources are the way to do this, but the question is really whether the debugger supports it. It probably does for C#, but not so sure about C++. If it does, it would be really cool though.

[grimar]

llvm/trunk/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

49

FWIW, it is not correct to handle LLVM errors like that.
This code fails if the code is compiled with LLVM_ENABLE_ABI_BREAKING_CHECKS in Release (i.e. when asserts are disabled).

The problem is that getStringForID returns Expected<StringRef> and Expected value must always
be checked, even if it is in success state.

Test failture log (found with check-llvm call):

********************
FAIL: LLVM :: tools/llvm-pdbutil/injected-sources-native.test (33257 of 33768)
******************** TEST 'LLVM :: tools/llvm-pdbutil/injected-sources-native.test' FAILED ********************
Script:
--
: 'RUN: at line 4';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/InjectedSource.pdb | /home/umb/LLVM/build/bin/FileCheck /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
: 'RUN: at line 6';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/ClassLayoutTest.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=NEGATIVE /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
: 'RUN: at line 33';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/dotnet_hashonly.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=HASH /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
: 'RUN: at line 45';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/dotnet_contents_uncompressed.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=UNCOMP /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
: 'RUN: at line 62';   /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content    /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/dotnet_contents_compressed.pdb | /home/umb/LLVM/build/bin/FileCheck --check-prefix=COMP /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test
--
Exit Code: 2

Command Output (stderr):
--
Expected<T> must be checked before access or destruction.
Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed).
Stack dump:
0.	Program arguments: /home/umb/LLVM/build/bin/llvm-pdbutil pretty -native -injected-sources -injected-source-content /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/Inputs/InjectedSource.pdb 
 #0 0x000055b389798f5a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/home/umb/LLVM/build/bin/llvm-pdbutil+0x2acf5a)
 #1 0x000055b389796c84 llvm::sys::RunSignalHandlers() (/home/umb/LLVM/build/bin/llvm-pdbutil+0x2aac84)
 #2 0x000055b389796e05 SignalHandler(int) (/home/umb/LLVM/build/bin/llvm-pdbutil+0x2aae05)
 #3 0x00007faef95ebdd0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12dd0)
 #4 0x00007faef8cb4077 raise /build/glibc-B9XfQf/glibc-2.28/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x00007faef8c95535 abort /build/glibc-B9XfQf/glibc-2.28/stdlib/abort.c:81:7
 #6 0x000055b389587813 llvm::Expected<llvm::StringRef>::fatalUncheckedExpected() const (/home/umb/LLVM/build/bin/llvm-pdbutil+0x9b813)
 #7 0x000055b3896a4eb8 llvm::pdb::(anonymous namespace)::NativeInjectedSource::getFileName() const (/home/umb/LLVM/build/bin/llvm-pdbutil+0x1b8eb8)
 #8 0x000055b3895bb8ed dumpPretty(llvm::StringRef) (/home/umb/LLVM/build/bin/llvm-pdbutil+0xcf8ed)
 #9 0x000055b3895501c0 main (/home/umb/LLVM/build/bin/llvm-pdbutil+0x641c0)
#10 0x00007faef8c9709b __libc_start_main /build/glibc-B9XfQf/glibc-2.28/csu/../csu/libc-start.c:342:3
#11 0x000055b38956645a _start (/home/umb/LLVM/build/bin/llvm-pdbutil+0x7a45a)
FileCheck error: '-' is empty.
FileCheck command line:  /home/umb/LLVM/build/bin/FileCheck /home/umb/LLVM/llvm/test/tools/llvm-pdbutil/injected-sources-native.test

[grimar]

llvm/trunk/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

49

Basing on the text in assert() (I am not familar with the logic of this tool at all), seems just adding a consumeError call would be sufficient and correct.

llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h

if (IsEnd != R.IsEnd)
       return false;
 
     return (Map == R.Map) && (Index == R.Index);
   }
   const std::pair<uint32_t, ValueT> &operator*() const {
     assert(Map->Present.test(Index));
     return Map->Buckets[Index];
   }
+
+  // Implement postfix op++ in terms of prefix op++ by using the superclass
+  // implementation.
+  using iterator_facade_base<HashTableIterator<ValueT>,
+                             std::forward_iterator_tag,
+                             const std::pair<uint32_t, ValueT>>::operator++;
   HashTableIterator &operator++() {
     while (Index < Map->Buckets.size()) {
       ++Index;
       if (Map->Present.test(Index))
         return *this;
     }
 
     IsEnd = true;
     return *this;
   }
 
 private:
   bool isEnd() const { return IsEnd; }
   uint32_t index() const { return Index; }
 
   const HashTable<ValueT> *Map;
   uint32_t Index;
   bool IsEnd;
 };
 
 template <typename ValueT>
 class HashTable {
-  using const_iterator = HashTableIterator<ValueT>;
-  friend const_iterator;
-
   struct Header {
     support::ulittle32_t Size;
     support::ulittle32_t Capacity;
   };
 
   using BucketList = std::vector<std::pair<uint32_t, ValueT>>;
 
 public:
+  using const_iterator = HashTableIterator<ValueT>;
+  friend const_iterator;
+
   HashTable() { Buckets.resize(8); }
   explicit HashTable(uint32_t Capacity) {
     Buckets.resize(Capacity);
   }
 
   Error load(BinaryStreamReader &Stream) {
     const Header *H;
     if (auto EC = Stream.readObject(H))

llvm/trunk/include/llvm/DebugInfo/PDB/Native/InjectedSourceStream.h

+//===- InjectedSourceStream.h - PDB Headerblock Stream Access ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_PDB_RAW_PDBINJECTEDSOURCESTREAM_H
+#define LLVM_DEBUGINFO_PDB_RAW_PDBINJECTEDSOURCESTREAM_H
+
+#include "llvm/DebugInfo/PDB/Native/HashTable.h"
+#include "llvm/DebugInfo/PDB/Native/RawTypes.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+namespace msf {
+class MappedBlockStream;
+}
+namespace pdb {
+class PDBFile;
+class PDBStringTable;
+
+class InjectedSourceStream {
+public:
+  InjectedSourceStream(std::unique_ptr<msf::MappedBlockStream> Stream);
+  Error reload(const PDBStringTable &Strings);
+
+  using const_iterator = HashTable<SrcHeaderBlockEntry>::const_iterator;
+  const_iterator begin() const { return InjectedSourceTable.begin(); }
+  const_iterator end() const { return InjectedSourceTable.end(); }
+
+  uint32_t size() const { return InjectedSourceTable.size(); }
+
+private:
+  std::unique_ptr<msf::MappedBlockStream> Stream;
+
+  const SrcHeaderBlockHeader* Header;
+  HashTable<SrcHeaderBlockEntry> InjectedSourceTable;
+};
+}
+}
+
+#endif

llvm/trunk/include/llvm/DebugInfo/PDB/Native/NativeEnumInjectedSources.h

+//==- NativeEnumInjectedSources.cpp - Native Injected Source Enumerator --*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_PDB_NATIVE_NATIVEENUMINJECTEDSOURCES_H
+#define LLVM_DEBUGINFO_PDB_NATIVE_NATIVEENUMINJECTEDSOURCES_H
+
+#include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
+#include "llvm/DebugInfo/PDB/IPDBInjectedSource.h"
+#include "llvm/DebugInfo/PDB/Native/InjectedSourceStream.h"
+
+namespace llvm {
+namespace pdb {
+
+class InjectedSourceStream;
+class PDBStringTable;
+
+class NativeEnumInjectedSources : public IPDBEnumChildren<IPDBInjectedSource> {
+public:
+  NativeEnumInjectedSources(PDBFile &File, const InjectedSourceStream &IJS,
+                            const PDBStringTable &Strings);
+
+  uint32_t getChildCount() const override;
+  std::unique_ptr<IPDBInjectedSource>
+  getChildAtIndex(uint32_t Index) const override;
+  std::unique_ptr<IPDBInjectedSource> getNext() override;
+  void reset() override;
+
+private:
+  PDBFile &File;
+  const InjectedSourceStream &Stream;
+  const PDBStringTable &Strings;
+  InjectedSourceStream::const_iterator Cur;
+};
+
+} // namespace pdb
+} // namespace llvm
+
+#endif

llvm/trunk/include/llvm/DebugInfo/PDB/Native/PDBFile.h

 namespace msf {
 class MappedBlockStream;
 }
 
 namespace pdb {
 class DbiStream;
 class GlobalsStream;
 class InfoStream;
+class InjectedSourceStream;
 class PDBStringTable;
 class PDBFileBuilder;
 class PublicsStream;
 class SymbolStream;
 class TpiStream;
 
 class PDBFile : public msf::IMSFFile {
   friend PDBFileBuilder;
public:
   BinaryStreamRef getMsfBuffer() const { return *Buffer; }
 
   ArrayRef<support::ulittle32_t> getDirectoryBlockArray() const;
 
   std::unique_ptr<msf::MappedBlockStream>
   createIndexedStream(uint16_t SN) const;
   Expected<std::unique_ptr<msf::MappedBlockStream>>
   safelyCreateIndexedStream(uint32_t StreamIndex) const;
+  Expected<std::unique_ptr<msf::MappedBlockStream>>
+  safelyCreateNamedStream(StringRef Name);
 
   msf::MSFStreamLayout getStreamLayout(uint32_t StreamIdx) const;
   msf::MSFStreamLayout getFpmStreamLayout() const;
 
   Error parseFileHeaders();
   Error parseStreamData();
 
   Expected<InfoStream &> getPDBInfoStream();
   Expected<DbiStream &> getPDBDbiStream();
   Expected<GlobalsStream &> getPDBGlobalsStream();
   Expected<TpiStream &> getPDBTpiStream();
   Expected<TpiStream &> getPDBIpiStream();
   Expected<PublicsStream &> getPDBPublicsStream();
   Expected<SymbolStream &> getPDBSymbolStream();
   Expected<PDBStringTable &> getStringTable();
+  Expected<InjectedSourceStream &> getInjectedSourceStream();
 
   BumpPtrAllocator &getAllocator() { return Allocator; }
 
   bool hasPDBDbiStream() const;
   bool hasPDBGlobalsStream();
   bool hasPDBInfoStream() const;
   bool hasPDBIpiStream() const;
   bool hasPDBPublicsStream();
   bool hasPDBSymbolStream();
   bool hasPDBTpiStream() const;
   bool hasPDBStringTable();
+  bool hasPDBInjectedSourceStream();
 
   uint32_t getPointerSize();
 
 private:
   std::string FilePath;
   BumpPtrAllocator &Allocator;
 
   std::unique_ptr<BinaryStream> Buffer;
 
   msf::MSFLayout ContainerLayout;
 
   std::unique_ptr<GlobalsStream> Globals;
   std::unique_ptr<InfoStream> Info;
   std::unique_ptr<DbiStream> Dbi;
   std::unique_ptr<TpiStream> Tpi;
   std::unique_ptr<TpiStream> Ipi;
   std::unique_ptr<PublicsStream> Publics;
   std::unique_ptr<SymbolStream> Symbols;
   std::unique_ptr<msf::MappedBlockStream> DirectoryStream;
   std::unique_ptr<msf::MappedBlockStream> StringTableStream;
+  std::unique_ptr<InjectedSourceStream> InjectedSources;
   std::unique_ptr<PDBStringTable> Strings;
 };
 }
 }
 
 #endif

llvm/trunk/lib/DebugInfo/PDB/CMakeLists.txt

add_pdb_impl_folder(Native
   Native/DbiStream.cpp
   Native/DbiStreamBuilder.cpp
   Native/EnumTables.cpp
   Native/GlobalsStream.cpp
   Native/Hash.cpp
   Native/HashTable.cpp
   Native/InfoStream.cpp
   Native/InfoStreamBuilder.cpp
+  Native/InjectedSourceStream.cpp
   Native/ModuleDebugStream.cpp
   Native/NativeCompilandSymbol.cpp
   Native/NativeEnumGlobals.cpp
+  Native/NativeEnumInjectedSources.cpp
   Native/NativeEnumModules.cpp
   Native/NativeEnumTypes.cpp
   Native/NativeExeSymbol.cpp
   Native/NativeRawSymbol.cpp
   Native/NativeSymbolEnumerator.cpp
   Native/NativeTypeArray.cpp
   Native/NativeTypeBuiltin.cpp
   Native/NativeTypeEnum.cpp

llvm/trunk/lib/DebugInfo/PDB/Native/InjectedSourceStream.cpp

+//===- InjectedSourceStream.cpp - PDB Headerblock Stream Access -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DebugInfo/PDB/Native/InjectedSourceStream.h"
+
+#include "llvm/DebugInfo/MSF/MappedBlockStream.h"
+#include "llvm/DebugInfo/PDB/Native/Hash.h"
+#include "llvm/DebugInfo/PDB/Native/PDBStringTable.h"
+#include "llvm/DebugInfo/PDB/Native/RawConstants.h"
+#include "llvm/DebugInfo/PDB/Native/RawTypes.h"
+#include "llvm/Support/BinaryStreamReader.h"
+#include "llvm/Support/Endian.h"
+
+using namespace llvm;
+using namespace llvm::msf;
+using namespace llvm::support;
+using namespace llvm::pdb;
+
+InjectedSourceStream::InjectedSourceStream(
+    std::unique_ptr<MappedBlockStream> Stream)
+    : Stream(std::move(Stream)) {}
+
+Error InjectedSourceStream::reload(const PDBStringTable &Strings) {
+  BinaryStreamReader Reader(*Stream);
+
+  if (auto EC = Reader.readObject(Header))
+    return EC;
+
+  if (Header->Version !=
+      static_cast<uint32_t>(PdbRaw_SrcHeaderBlockVer::SrcVerOne))
+    return make_error<RawError>(raw_error_code::corrupt_file,
+                                "Invalid headerblock header version");
+
+  if (auto EC = InjectedSourceTable.load(Reader))
+    return EC;
+
+  for (const auto& Entry : *this) {
+    if (Entry.second.Size != sizeof(SrcHeaderBlockEntry))
+      return make_error<RawError>(raw_error_code::corrupt_file,
+                                  "Invalid headerbock entry size");
+    if (Entry.second.Version !=
+        static_cast<uint32_t>(PdbRaw_SrcHeaderBlockVer::SrcVerOne))
+      return make_error<RawError>(raw_error_code::corrupt_file,
+                                  "Invalid headerbock entry version");
+
+    // Check that all name references are valid.
+    auto Name = Strings.getStringForID(Entry.second.FileNI);
+    if (!Name)
+      return Name.takeError();
+    auto ObjName = Strings.getStringForID(Entry.second.ObjNI);
+    if (!ObjName)
+      return ObjName.takeError();
+    auto VName = Strings.getStringForID(Entry.second.VFileNI);
+    if (!VName)
+      return VName.takeError();
+  }
+
+  assert(Reader.bytesRemaining() == 0);
+  return Error::success();
+}

llvm/trunk/lib/DebugInfo/PDB/Native/NativeEnumInjectedSources.cpp

+//==- NativeEnumInjectedSources.cpp - Native Injected Source Enumerator --*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DebugInfo/PDB/Native/NativeEnumInjectedSources.h"
+
+#include "llvm/DebugInfo/PDB/Native/InfoStream.h"
+#include "llvm/DebugInfo/PDB/Native/PDBFile.h"
+#include "llvm/DebugInfo/PDB/Native/PDBStringTable.h"
+
+namespace llvm {
+namespace pdb {
+
+namespace {
+
+Expected<std::string> readStreamData(BinaryStream &Stream) {
+  uint32_t Offset = 0, DataLength = Stream.getLength();
+  std::string Result;
+  Result.reserve(DataLength);
+  while (Offset < DataLength) {
+    ArrayRef<uint8_t> Data;
+    if (auto E = Stream.readLongestContiguousChunk(Offset, Data))
+      return std::move(E);
+    Offset += Data.size();
+    Result += toStringRef(Data);
+  }
+  return Result;
+}
+
+class NativeInjectedSource final : public IPDBInjectedSource {
+  const SrcHeaderBlockEntry &Entry;
+  const PDBStringTable &Strings;
+  PDBFile &File;
+
+public:
+  NativeInjectedSource(const SrcHeaderBlockEntry &Entry,
+                       PDBFile &File, const PDBStringTable &Strings)
+      : Entry(Entry), Strings(Strings), File(File) {}
+
+  uint32_t getCrc32() const override { return Entry.CRC; }
+  uint64_t getCodeByteSize() const override { return Entry.FileSize; }
+
+  std::string getFileName() const override {
+    auto Name = Strings.getStringForID(Entry.FileNI);
+    assert(Name && "InjectedSourceStream should have rejected this");
+    return *Name;
+  }
+
+  std::string getObjectFileName() const override {
+    auto ObjName = Strings.getStringForID(Entry.ObjNI);
+    assert(ObjName && "InjectedSourceStream should have rejected this");
+    return *ObjName;
+  }
+
+  std::string getVirtualFileName() const override {
+    auto VName = Strings.getStringForID(Entry.VFileNI);
+    assert(VName && "InjectedSourceStream should have rejected this");
+    return *VName;
+  }
+
+  PDB_SourceCompression getCompression() const override {
+    return static_cast<PDB_SourceCompression>(Entry.Compression);
+  }
+
+  std::string getCode() const override {
+    // Get name of stream storing the data.
+    auto VName = Strings.getStringForID(Entry.VFileNI);
+    assert(VName && "InjectedSourceStream should have rejected this");
+    std::string StreamName = ("/src/files/" + *VName).str();
+
+    // Find stream with that name and read its data.
+    // FIXME: Consider validating (or even loading) all this in
+    // InjectedSourceStream so that no error can happen here.
+    auto ExpectedFileStream = File.safelyCreateNamedStream(StreamName);
+    if (!ExpectedFileStream) {
+      consumeError(ExpectedFileStream.takeError());
+      return "(failed to open data stream)";
+    }
+
+    auto Data = readStreamData(**ExpectedFileStream);
+    if (!Data) {
+      consumeError(Data.takeError());
+      return "(failed to read data)";
+    }
+    return *Data;
+  }
+};
+
+} // namespace
+
+NativeEnumInjectedSources::NativeEnumInjectedSources(
+    PDBFile &File, const InjectedSourceStream &IJS,
+    const PDBStringTable &Strings)
+    : File(File), Stream(IJS), Strings(Strings), Cur(Stream.begin()) {}
+
+uint32_t NativeEnumInjectedSources::getChildCount() const {
+  return static_cast<uint32_t>(Stream.size());
+}
+
+std::unique_ptr<IPDBInjectedSource>
+NativeEnumInjectedSources::getChildAtIndex(uint32_t N) const {
+  if (N >= getChildCount())
+    return nullptr;
+  return make_unique<NativeInjectedSource>(std::next(Stream.begin(), N)->second,
+                                           File, Strings);
+}
+
+std::unique_ptr<IPDBInjectedSource> NativeEnumInjectedSources::getNext() {
+  if (Cur == Stream.end())
+    return nullptr;
+  return make_unique<NativeInjectedSource>((Cur++)->second, File, Strings);
+}
+
+void NativeEnumInjectedSources::reset() { Cur = Stream.begin(); }
+
+}
+}

llvm/trunk/lib/DebugInfo/PDB/Native/NativeSession.cpp

 //===- NativeSession.cpp - Native implementation of IPDBSession -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/NativeSession.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
 #include "llvm/DebugInfo/PDB/Native/NativeCompilandSymbol.h"
+#include "llvm/DebugInfo/PDB/Native/NativeEnumInjectedSources.h"
 #include "llvm/DebugInfo/PDB/Native/NativeEnumTypes.h"
 #include "llvm/DebugInfo/PDB/Native/NativeExeSymbol.h"
 #include "llvm/DebugInfo/PDB/Native/NativeTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/Native/NativeTypeEnum.h"
 #include "llvm/DebugInfo/PDB/Native/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
 #include "llvm/DebugInfo/PDB/Native/SymbolCache.h"
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"

 }
 
 std::unique_ptr<IPDBEnumTables> NativeSession::getEnumTables() const {
   return nullptr;
 }
 
 std::unique_ptr<IPDBEnumInjectedSources>
 NativeSession::getInjectedSources() const {
+  auto ISS = Pdb->getInjectedSourceStream();
+  if (!ISS) {
+    consumeError(ISS.takeError());
     return nullptr;
   }
+  auto Strings = Pdb->getStringTable();
+  if (!Strings) {
+    consumeError(Strings.takeError());
+    return nullptr;
+  }
+  return make_unique<NativeEnumInjectedSources>(*Pdb, *ISS, *Strings);
+}
 
 std::unique_ptr<IPDBEnumSectionContribs>
 NativeSession::getSectionContribs() const {
   return nullptr;
 }
 
 std::unique_ptr<IPDBEnumFrameData>
 NativeSession::getFrameData() const {

llvm/trunk/lib/DebugInfo/PDB/Native/PDBFile.cpp

 //===- PDBFile.cpp - Low level interface to a PDB file ----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/PDBFile.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
 #include "llvm/DebugInfo/PDB/Native/DbiStream.h"
 #include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
 #include "llvm/DebugInfo/PDB/Native/InfoStream.h"
+#include "llvm/DebugInfo/PDB/Native/InjectedSourceStream.h"
 #include "llvm/DebugInfo/PDB/Native/PDBStringTable.h"
 #include "llvm/DebugInfo/PDB/Native/PublicsStream.h"
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
 #include "llvm/DebugInfo/PDB/Native/SymbolStream.h"
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"
 #include "llvm/Support/BinaryStream.h"
 #include "llvm/Support/BinaryStreamArray.h"
 #include "llvm/Support/BinaryStreamReader.h"
if (auto EC = TempSymbols->reload())
       return std::move(EC);
     Symbols = std::move(TempSymbols);
   }
   return *Symbols;
 }
 
 Expected<PDBStringTable &> PDBFile::getStringTable() {
   if (!Strings) {
-    auto IS = getPDBInfoStream();
-    if (!IS)
-      return IS.takeError();
-
-    Expected<uint32_t> ExpectedNSI = IS->getNamedStreamIndex("/names");
-    if (!ExpectedNSI)
-      return ExpectedNSI.takeError();
-    uint32_t NameStreamIndex = *ExpectedNSI;
-
-    auto NS = safelyCreateIndexedStream(NameStreamIndex);
+    auto NS = safelyCreateNamedStream("/names");
     if (!NS)
       return NS.takeError();
 
     auto N = llvm::make_unique<PDBStringTable>();
     BinaryStreamReader Reader(**NS);
     if (auto EC = N->reload(Reader))
       return std::move(EC);
     assert(Reader.bytesRemaining() == 0);
     StringTableStream = std::move(*NS);
     Strings = std::move(N);
   }
   return *Strings;
 }
 
+Expected<InjectedSourceStream &> PDBFile::getInjectedSourceStream() {
+  if (!InjectedSources) {
+    auto IJS = safelyCreateNamedStream("/src/headerblock");
+    if (!IJS)
+      return IJS.takeError();
+
+    auto Strings = getStringTable();
+    if (!Strings)
+      return Strings.takeError();
+
+    auto IJ = llvm::make_unique<InjectedSourceStream>(std::move(*IJS));
+    if (auto EC = IJ->reload(*Strings))
+      return std::move(EC);
+    InjectedSources = std::move(IJ);
+  }
+  return *InjectedSources;
+}
+
 uint32_t PDBFile::getPointerSize() {
   auto DbiS = getPDBDbiStream();
   if (!DbiS)
     return 0;
   PDB_Machine Machine = DbiS->getMachineType();
   if (Machine == PDB_Machine::Amd64)
     return 8;
   return 4;
bool PDBFile::hasPDBStringTable() {
   if (!ExpectedNSI) {
     consumeError(ExpectedNSI.takeError());
     return false;
   }
   assert(*ExpectedNSI < getNumStreams());
   return true;
 }
 
+bool PDBFile::hasPDBInjectedSourceStream() {
+  auto IS = getPDBInfoStream();
+  if (!IS)
+    return false;
+  Expected<uint32_t> ExpectedNSI = IS->getNamedStreamIndex("/src/headerblock");
+  if (!ExpectedNSI) {
+    consumeError(ExpectedNSI.takeError());
+    return false;
+  }
+  assert(*ExpectedNSI < getNumStreams());
+  return true;
+}
+
 /// Wrapper around MappedBlockStream::createIndexedStream() that checks if a
 /// stream with that index actually exists.  If it does not, the return value
 /// will have an MSFError with code msf_error_code::no_stream.  Else, the return
 /// value will contain the stream returned by createIndexedStream().
 Expected<std::unique_ptr<MappedBlockStream>>
 PDBFile::safelyCreateIndexedStream(uint32_t StreamIndex) const {
   if (StreamIndex >= getNumStreams())
     // This rejects kInvalidStreamIndex with an error as well.
     return make_error<RawError>(raw_error_code::no_stream);
   return createIndexedStream(StreamIndex);
 }
+
+Expected<std::unique_ptr<MappedBlockStream>>
+PDBFile::safelyCreateNamedStream(StringRef Name) {
+  auto IS = getPDBInfoStream();
+  if (!IS)
+    return IS.takeError();
+
+  Expected<uint32_t> ExpectedNSI = IS->getNamedStreamIndex(Name);
+  if (!ExpectedNSI)
+    return ExpectedNSI.takeError();
+  uint32_t NameStreamIndex = *ExpectedNSI;
+
+  return safelyCreateIndexedStream(NameStreamIndex);
+}

llvm/trunk/test/tools/llvm-pdbutil/injected-sources-native.test

+; This is identical to injected-sources.test, except that it uses the -native
+; mode of pretty (and hence doesn't require diasdk and runs on all platforms).
+
+; RUN: llvm-pdbutil pretty -native -injected-sources -injected-source-content \
+; RUN:   %p/Inputs/InjectedSource.pdb | FileCheck %s
+; RUN: llvm-pdbutil pretty -native -injected-sources -injected-source-content \
+; RUN:   %p/Inputs/ClassLayoutTest.pdb | FileCheck --check-prefix=NEGATIVE %s
+
+; CHECK:      ---INJECTED SOURCES---
+; CHECK:      c.natvis (140 bytes): obj=<null>, vname=c.natvis, crc=334478030, compression=None
+; CHECK-NEXT: <?xml version="1.0" encoding="utf-8"?>
+; CHECK-NEXT: <AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
+; CHECK-NEXT: </AutoVisualizer>
+; CHECK:      a.natvis (140 bytes): obj=<null>, vname=a.natvis, crc=334478030, compression=None
+; CHECK-NEXT: <?xml version="1.0" encoding="utf-8"?>
+; CHECK-NEXT: <AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
+; CHECK-NEXT: </AutoVisualizer>
+; CHECK:      b.natvis (294 bytes): obj=<null>, vname=b.natvis, crc=2059731902, compression=None
+; CHECK-NEXT: <?xml version="1.0" encoding="utf-8"?>
+; CHECK-NEXT: <AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
+; CHECK-NEXT: <Type Name="Baz">
+; CHECK-NEXT:   <DisplayString>Third test</DisplayString>
+; CHECK-NEXT: </Type>
+; CHECK-NEXT: <Type Name="Buzz">
+; CHECK-NEXT:   <DisplayString>Fourth test</DisplayString>
+; CHECK-NEXT: </Type>
+; CHECK-NEXT: </AutoVisualizer>
+
+; NEGATIVE:      ---INJECTED SOURCES---
+; NEGATIVE-NEXT: There are no injected sources.

llvm/trunk/tools/llvm-pdbutil/llvm-pdbutil.cpp

 }
 
 static std::string stringOr(std::string Str, std::string IfEmpty) {
   return (Str.empty()) ? IfEmpty : Str;
 }
 
 static void dumpInjectedSources(LinePrinter &Printer, IPDBSession &Session) {
   auto Sources = Session.getInjectedSources();
-  if (0 == Sources->getChildCount()) {
+  if (!Sources || !Sources->getChildCount()) {
     Printer.printLine("There are no injected sources.");
     return;
   }
 
   while (auto IS = Sources->getNext()) {
     Printer.NewLine();
     std::string File = stringOr(IS->getFileName(), "<null>");
     uint64_t Size = IS->getCodeByteSize();
static void dumpPretty(StringRef Path) {
   if (opts::pretty::Lines) {
     Printer.NewLine();
   }
   if (opts::pretty::InjectedSources) {
     Printer.NewLine();
     WithColor(Printer, PDB_ColorItem::SectionHeader).get()
         << "---INJECTED SOURCES---";
     AutoIndent Indent1(Printer);
-
-    if (ReaderType == PDB_ReaderType::Native)
-      Printer.printLine(
-          "Injected sources are not supported with the native reader.");
-    else
-      dumpInjectedSources(Printer, *Session);
   }
 
   Printer.NewLine();
   outs().flush();
 }
 
 static void mergePdbs() {
   BumpPtrAllocator Allocator;

llvm/trunk/utils/gn/secondary/llvm/lib/DebugInfo/PDB/BUILD.gn

sources = [
     "Native/DbiStreamBuilder.cpp",
     "Native/EnumTables.cpp",
     "Native/GSIStreamBuilder.cpp",
     "Native/GlobalsStream.cpp",
     "Native/Hash.cpp",
     "Native/HashTable.cpp",
     "Native/InfoStream.cpp",
     "Native/InfoStreamBuilder.cpp",
+    "Native/InjectedSourceStream.cpp",
     "Native/ModuleDebugStream.cpp",
     "Native/NamedStreamMap.cpp",
     "Native/NativeCompilandSymbol.cpp",
     "Native/NativeEnumGlobals.cpp",
+    "Native/NativeEnumInjectedSources.cpp",
     "Native/NativeEnumModules.cpp",
     "Native/NativeEnumTypes.cpp",
     "Native/NativeExeSymbol.cpp",
     "Native/NativeRawSymbol.cpp",
     "Native/NativeSession.cpp",
     "Native/NativeSymbolEnumerator.cpp",
     "Native/NativeTypeArray.cpp",
     "Native/NativeTypeBuiltin.cpp",