[pdb] Parse module line info

[zturner]

Summary

Rather than just dumping binary blocks for line info, we now pretty print individual line and column records.

To facilitate this, a couple of changes had to be made:

1. ModuleSubstream got moved from DebugInfo/PDB to DebugInfo/CodeView, and various codeview related types are defined there. It turns out DebugInfo/CodeView/Line.h already defines many of these structures, but this is really old code that is not endian aware, doesn't interact well with StreamInterface and not very helpful for getting stuff out of a PDB. Eventually we should migrate the old readobj COFFDumper code to these new structures, or at least merge their functionality somehow.

2. A ModuleSubstream visitor is introduced. Depending on where your module substream array comes from, different subsets of record types can be expected. We are already hand parsing these substream arrays in many places especially in COFFDumper.cpp. In the future we can migrate these paths to the visitor as well, which should reduce a lot of code in COFFDumper.cpp.

[ruiu]

include/llvm/DebugInfo/CodeView/CodeView.h

16 ↗

(On Diff #59471)

Instead of adding an #include to this header, can you include it into .cpp files that actually need Endian.h?

include/llvm/DebugInfo/CodeView/ModuleSubstream.h

22

Thank you for these comments!

41–42

What is this?

include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h

23

What does this correspond to?

39

I don't think you need !!.

lib/DebugInfo/CodeView/ModuleSubstream.cpp

12

Remove this blank line.

[majnemer]

tools/llvm-pdbdump/llvm-pdbdump.cpp

543–549

Instead of doing this bitmashing yourself, why not construct a LineInfo from N.Flags?

[amccarth]

DebugInfo/CodeView/ModuleSubstreamVisitor.cpp probably shouldn't be empty (or it shouldn't be added).

include/llvm/DebugInfo/CodeView/CodeView.h

16 ↗

(On Diff #59471)

Nothing in this file actually uses the endian types (as far as I can see). I'd include Endian.h in llvm/DebugInfo/CodeView/ModuleSubstream.h and ModuleSubstreamVisitor.h.

include/llvm/DebugInfo/CodeView/StreamArray.h

55

Should this be explicit?

213

this->Index to distinguish from the Index parameter? Ug. Can you give the parameter a distinct name?

include/llvm/DebugInfo/CodeView/StreamReader.h

56

Why U and not Extractor?

tools/llvm-pdbdump/llvm-pdbdump.cpp

579

Should(could) L be a const auto &?

[zturner]

Updated based on suggestions

[majnemer]

tools/llvm-pdbdump/llvm-pdbdump.cpp

545

I'd use the logic in tools/llvm-readobj/COFFDumper.cpp:940:

if (LI.isAlwaysStepInto())
  W.printString("StepInto", StringRef("Always"));
else if (LI.isNeverStepInto())
  W.printString("StepInto", StringRef("Never"));
else
  W.printNumber("LineNumberStart", LI.getStartLine());

You might have to update fewer tests down the road if you choose the same output format that llvm-readobj uses.

[zturner]

I'll address the remainder of the comments in a followup

include/llvm/DebugInfo/CodeView/ModuleSubstream.h

42–43

Added comments

include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h

24

This doesn't correspond to any CodeView types. It's just the ValueType of the array. i.e. when you iterate the array you will be iterating over a sequence of these. The value type can be whatever you want (just depends how you write the extractor), so here I chose to make it something that provides access to the 2 underlying embedded arrays and the offset field.

include/llvm/DebugInfo/CodeView/StreamReader.h

56

Because you don't ever have to think about what to pass for it, and it's not used in the function except for passthrough. You're never explicitly parameterizing the function, it's always deduced based on the type of the VarStreamArray you pass in.

lib/DebugInfo/CodeView/ModuleSubstream.cpp

13

Don't we usually put a blank line between the TU's "primary" header and the rest of the headers? I always do, wonder if I've been doing it wrong.

[zturner]

Address remaining suggestions

[majnemer]

LGTM

include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h

43

What if BlockSize is smaller than LineFileBlockHeader?

include/llvm/DebugInfo/CodeView/StreamArray.h

91–96

Should we have in-class member initializers for these? Seems like it might be less error prone/more compact.

213

Should it be an error for Array.size() to be -1?

[zturner]

include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h

43

That would indicate a corrupt record. So yea, I should check for that.

include/llvm/DebugInfo/CodeView/StreamArray.h

213

If it were easy to make it work, then we should, but to do that we'd probably need to increase the size of the iterator, and I'm not sure it's worth it. I think the only way that can ever happen is if your StreamRef is 2^32-1 bytes long, and I don't know that it's even possible to have a stream that big (I recall there are constants somewhere in the cv header files that determine how big a stream can be based on the block size and whether large blocks are enabled). I can look into it a bit more when I get home, but this would be a pretty exceptional circumstance.

[zturner]

size is unsigned, so it could happen if you had a stream which was exactly
2^32 bytes long, and it was an array of uint8's. :-/ Not that that would
ever happen. I think Rui has a patch up which changes this a little, so
I'll wait until that goes in.

include/llvm/DebugInfo/CodeView/ModuleSubstream.h

+//===- ModuleSubstream.h ----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_CODEVIEW_MODULESUBSTREAM_H
+#define LLVM_DEBUGINFO_CODEVIEW_MODULESUBSTREAM_H
+
+#include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/DebugInfo/CodeView/StreamArray.h"
+#include "llvm/DebugInfo/CodeView/StreamRef.h"
+#include "llvm/Support/Endian.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+namespace codeview {
+
+// Corresponds to the `CV_DebugSSubsectionHeader_t` structure.
+struct ModuleSubsectionHeader {
+  support::ulittle32_t Kind;   // codeview::ModuleSubstreamKind enum
+  support::ulittle32_t Length; // number of bytes occupied by this record.
+};
+
+// Corresponds to the `CV_DebugSLinesHeader_t` structure.
+struct LineSubstreamHeader {
+  support::ulittle32_t RelocOffset;  // Code offset of line contribution.
+  support::ulittle16_t RelocSegment; // Code segment of line contribution.
+  support::ulittle16_t Flags;        // See LineFlags enumeration.
+  support::ulittle32_t CodeSize;     // Code size of this line contribution.
+};
+
+// Corresponds to the `CV_DebugSLinesFileBlockHeader_t` structure.
+struct LineFileBlockHeader {
+  support::ulittle32_t FileOffset;
+  support::ulittle32_t NumLines;  // Number of lines
+  support::ulittle32_t BlockSize; // Code size of block, in bytes.
+  // The following two variable length arrays appear immediately after the
+  // header.  The structure definitions follow.
+  // LineNumberEntry   Lines[NumLines];
+  // ColumnNumberEntry Columns[NumLines];
+};
+
+// Corresponds to `CV_Line_t` structure
+struct LineNumberEntry {
+  support::ulittle32_t Offset; // Offset to start of code bytes for line number
+  support::ulittle32_t Flags;  // Start:24, End:7, IsStatement:1
+};
+
+// Corresponds to `CV_Column_t` structure
+struct ColumnNumberEntry {
+  support::ulittle16_t StartColumn;
+  support::ulittle16_t EndColumn;
+};
+
+class ModuleSubstream {
+public:
+  ModuleSubstream();
+  ModuleSubstream(ModuleSubstreamKind Kind, StreamRef Data);
+  static Error initialize(StreamRef Stream, ModuleSubstream &Info);
+  uint32_t getRecordLength() const;
+  ModuleSubstreamKind getSubstreamKind() const;
+  StreamRef getRecordData() const;
+
+private:
+  ModuleSubstreamKind Kind;
+  StreamRef Data;
+};
+
+template <> struct VarStreamArrayExtractor<ModuleSubstream> {
+  Error operator()(StreamRef Stream, uint32_t &Length,
+                   ModuleSubstream &Info) const {
+    if (auto EC = ModuleSubstream::initialize(Stream, Info))
+      return EC;
+    Length = Info.getRecordLength();
+    return Error::success();
+  }
+};
+
+typedef VarStreamArray<ModuleSubstream> ModuleSubstreamArray;
+}
+}
+
+#endif // LLVM_DEBUGINFO_CODEVIEW_MODULESUBSTREAM_H

include/llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h

+//===- ModuleSubstreamVisitor.h ---------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_CODEVIEW_MODULESUBSTREAMVISITOR_H
+#define LLVM_DEBUGINFO_CODEVIEW_MODULESUBSTREAMVISITOR_H
+
+#include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/DebugInfo/CodeView/CodeViewError.h"
+#include "llvm/DebugInfo/CodeView/Line.h"
+#include "llvm/DebugInfo/CodeView/ModuleSubstream.h"
+#include "llvm/DebugInfo/CodeView/StreamReader.h"
+#include "llvm/DebugInfo/CodeView/StreamRef.h"
+
+namespace llvm {
+namespace codeview {
+
+struct LineColumnEntry {
+  support::ulittle32_t Offset;
+  FixedStreamArray<LineNumberEntry> LineNumbers;
+  FixedStreamArray<ColumnNumberEntry> Columns;
+};
+
+class FileLineInfoExtractor {
+public:
+  FileLineInfoExtractor(const LineSubstreamHeader *Header) : Header(Header) {}
+
+  Error operator()(StreamRef Stream, uint32_t &Len,
+                   LineColumnEntry &Item) const {
+    const LineFileBlockHeader *BlockHeader;
+    StreamReader Reader(Stream);
+    if (auto EC = Reader.readObject(BlockHeader))
+      return EC;
+    bool HasColumn = Header->Flags & LineFlags::HaveColumns;
+    uint32_t LineInfoSize =
+        BlockHeader->NumLines *
+        (sizeof(LineNumberEntry) + (HasColumn ? sizeof(ColumnNumberEntry) : 0));
+    uint32_t Size = BlockHeader->BlockSize - sizeof(LineFileBlockHeader);
+    if (LineInfoSize > Size)
+      return make_error<CodeViewError>(cv_error_code::corrupt_record,
+                                       "Invalid line block record size");
+    // The value recorded in BlockHeader->BlockSize includes the size of
+    // LineFileBlockHeader.
+    Len = BlockHeader->BlockSize;
+    Item.Offset = BlockHeader->FileOffset;
+    if (auto EC = Reader.readArray(Item.LineNumbers, BlockHeader->NumLines))
+      return EC;
+    if (HasColumn) {
+      if (auto EC = Reader.readArray(Item.Columns, BlockHeader->NumLines))
+        return EC;
+    }
+    return Error::success();
+  }
+
+private:
+  const LineSubstreamHeader *Header;
+};
+
+typedef VarStreamArray<LineColumnEntry, FileLineInfoExtractor> LineInfoArray;
+
+class IModuleSubstreamVisitor {
+public:
+  virtual ~IModuleSubstreamVisitor() {}
+
+  virtual Error visitUnknown(ModuleSubstreamKind Kind, StreamRef Data) = 0;
+  virtual Error visitSymbols(StreamRef Data);
+  virtual Error visitLines(StreamRef Data, const LineSubstreamHeader *Header,
+                           LineInfoArray Lines);
+  virtual Error visitStringTable(StreamRef Data);
+  virtual Error visitFileChecksums(StreamRef Data);
+  virtual Error visitFrameData(StreamRef Data);
+  virtual Error visitInlineeLines(StreamRef Data);
+  virtual Error visitCrossScopeImports(StreamRef Data);
+  virtual Error visitCrossScopeExports(StreamRef Data);
+  virtual Error visitILLines(StreamRef Data);
+  virtual Error visitFuncMDTokenMap(StreamRef Data);
+  virtual Error visitTypeMDTokenMap(StreamRef Data);
+  virtual Error visitMergedAssemblyInput(StreamRef Data);
+  virtual Error visitCoffSymbolRVA(StreamRef Data);
+};
+
+Error visitModuleSubstream(const ModuleSubstream &R,
+                           IModuleSubstreamVisitor &V);
+
+} // namespace codeview
+} // namespace llvm
+
+#endif // LLVM_DEBUGINFO_CODEVIEW_MODULESUBSTREAMVISITOR_H

include/llvm/DebugInfo/CodeView/StreamArray.h

template <typename ValueType,
           typename Extractor = VarStreamArrayExtractor<ValueType>>
 class VarStreamArray {
   friend class VarStreamArrayIterator<ValueType, Extractor>;
 
 public:
   typedef VarStreamArrayIterator<ValueType, Extractor> Iterator;
 
   VarStreamArray() {}
+  explicit VarStreamArray(const Extractor &E) : E(E) {}
 
-  VarStreamArray(StreamRef Stream) : Stream(Stream) {}
+  explicit VarStreamArray(StreamRef Stream) : Stream(Stream) {}
+  VarStreamArray(StreamRef Stream, const Extractor &E) : Stream(Stream), E(E) {}
+
+  VarStreamArray(const VarStreamArray<ValueType, Extractor> &Other)
+      : Stream(Other.Stream), E(Other.E) {}
 
   Iterator begin(bool *HadError = nullptr) const {
-    return Iterator(*this, HadError);
+    return Iterator(*this, E, HadError);
   }
 
-  Iterator end() const { return Iterator(); }
+  Iterator end() const { return Iterator(E); }
+
+  const Extractor &getExtractor() const { return E; }
 
 private:
   StreamRef Stream;
+  Extractor E;
 };
 
 template <typename ValueType, typename Extractor> class VarStreamArrayIterator {
   typedef VarStreamArrayIterator<ValueType, Extractor> IterType;
   typedef VarStreamArray<ValueType, Extractor> ArrayType;
 
 public:
-  VarStreamArrayIterator(const ArrayType &Array, bool *HadError = nullptr)
+  VarStreamArrayIterator(const ArrayType &Array, const Extractor &E,
+                         bool *HadError = nullptr)
       : Array(&Array), IterRef(Array.Stream), HasError(false),
-        HadError(HadError) {
+        HadError(HadError), Extract(E) {
     auto EC = Extract(IterRef, ThisLen, ThisValue);
     if (EC) {
       consumeError(std::move(EC));
       markError();
     }
   }
   VarStreamArrayIterator()
       : Array(nullptr), ThisLen(0), ThisValue(), IterRef(), HasError(false),
         HadError(nullptr) {}
+  explicit VarStreamArrayIterator(const Extractor &E)
+      : Array(nullptr), ThisLen(0), ThisValue(), IterRef(), HasError(false),
+        HadError(nullptr), Extract(E) {}
   ~VarStreamArrayIterator() {}
 
   bool operator==(const IterType &R) const {
     if (Array && R.Array) {
       // Both have a valid array, make sure they're same.
       assert(Array == R.Array);
       return IterRef == R.IterRef;
     }

 private:
   StreamRef Stream;
 };
 
 template <typename T> class FixedStreamArrayIterator {
 public:
   FixedStreamArrayIterator(const FixedStreamArray<T> &Array)
       : Array(Array), Index(uint32_t(-1)) {}
-  FixedStreamArrayIterator(const FixedStreamArray<T> &Array, uint32_t Index)
-      : Array(Array), Index(Index) {}
+  FixedStreamArrayIterator(const FixedStreamArray<T> &Array, uint32_t ArrayIndex)
+      : Array(Array), Index(ArrayIndex) {
+    if (Array.size() <= Index)
+      Index = uint32_t(-1);
+  }
 
   bool operator==(const FixedStreamArrayIterator<T> &R) {
     assert(&Array == &R.Array);
     return Index == R.Index;
   }
 
   bool operator!=(const FixedStreamArrayIterator<T> &R) {
     return !(*this == R);

include/llvm/DebugInfo/CodeView/StreamReader.h

public:
   template <typename T> Error readObject(const T *&Dest) {
     ArrayRef<uint8_t> Buffer;
     if (auto EC = readBytes(Buffer, sizeof(T)))
       return EC;
     Dest = reinterpret_cast<const T *>(Buffer.data());
     return Error::success();
   }
 
-  template <typename T>
-  Error readArray(VarStreamArray<T> &Array, uint32_t Size) {
+  template <typename T, typename U>
+  Error readArray(VarStreamArray<T, U> &Array, uint32_t Size) {
     StreamRef S;
     if (auto EC = readStreamRef(S, Size))
       return EC;
-    Array = VarStreamArray<T>(S);
+    Array = VarStreamArray<T, U>(S, Array.getExtractor());
     return Error::success();
   }
 
   template <typename T>
   Error readArray(FixedStreamArray<T> &Array, uint32_t NumItems) {
     if (NumItems == 0) {
       Array = FixedStreamArray<T>();
       return Error::success();

include/llvm/DebugInfo/PDB/Raw/DbiStream.h

 //===- DbiStream.h - PDB Dbi Stream (Stream 3) Access -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_PDB_RAW_PDBDBISTREAM_H
 #define LLVM_DEBUGINFO_PDB_RAW_PDBDBISTREAM_H
 
+#include "llvm/DebugInfo/CodeView/ModuleSubstream.h"
 #include "llvm/DebugInfo/CodeView/StreamArray.h"
 #include "llvm/DebugInfo/CodeView/StreamRef.h"
 #include "llvm/DebugInfo/PDB/PDBTypes.h"
 #include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
 #include "llvm/DebugInfo/PDB/Raw/ModInfo.h"
-#include "llvm/DebugInfo/PDB/Raw/ModuleSubstreamRecord.h"
 #include "llvm/DebugInfo/PDB/Raw/NameHashTable.h"
 #include "llvm/DebugInfo/PDB/Raw/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Raw/RawTypes.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace object {

include/llvm/DebugInfo/PDB/Raw/ModStream.h

 //===- ModStream.h - PDB Module Info Stream Access ------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_PDB_RAW_MODSTREAM_H
 #define LLVM_DEBUGINFO_PDB_RAW_MODSTREAM_H
 
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/DebugInfo/CodeView/CVRecord.h"
+#include "llvm/DebugInfo/CodeView/ModuleSubstream.h"
 #include "llvm/DebugInfo/CodeView/StreamArray.h"
 #include "llvm/DebugInfo/CodeView/StreamRef.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
-#include "llvm/DebugInfo/PDB/Raw/ModuleSubstreamRecord.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace pdb {
 class PDBFile;
 class ModInfo;
 
 class ModStream {
 public:
-  typedef codeview::VarStreamArray<ModuleSubstreamRecord> LineInfoArray;
-
   ModStream(PDBFile &File, const ModInfo &Module);
   ~ModStream();
 
   Error reload();
 
   iterator_range<codeview::CVSymbolArray::Iterator>
   symbols(bool *HadError) const;
 
-  iterator_range<LineInfoArray::Iterator> lines(bool *HadError) const;
+  iterator_range<codeview::ModuleSubstreamArray::Iterator>
+  lines(bool *HadError) const;
 
 private:
   const ModInfo &Mod;
 
   MappedBlockStream Stream;
 
   codeview::CVSymbolArray SymbolsSubstream;
   codeview::StreamRef LinesSubstream;
   codeview::StreamRef C13LinesSubstream;
   codeview::StreamRef GlobalRefsSubstream;
 
-  LineInfoArray LineInfo;
+  codeview::ModuleSubstreamArray LineInfo;
 };
 }
 }
 
 #endif

include/llvm/DebugInfo/PDB/Raw/ModuleSubstreamRecord.h

-//===- ModuleSubstreamRecord.h ----------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_DEBUGINFO_PDB_RAW_MODULESUBSTREAMRECORD_H
-#define LLVM_DEBUGINFO_PDB_RAW_MODULESUBSTREAMRECORD_H
-
-#include "llvm/DebugInfo/CodeView/CodeView.h"
-#include "llvm/DebugInfo/CodeView/StreamArray.h"
-#include "llvm/DebugInfo/CodeView/StreamRef.h"
-#include "llvm/Support/Error.h"
-
-namespace llvm {
-
-namespace pdb {
-class ModuleSubstreamRecord {
-public:
-  ModuleSubstreamRecord();
-  ModuleSubstreamRecord(codeview::ModuleSubstreamKind Kind,
-                        codeview::StreamRef Data);
-  static Error initialize(codeview::StreamRef Stream,
-                          ModuleSubstreamRecord &Info);
-  uint32_t getRecordLength() const;
-  codeview::ModuleSubstreamKind getSubstreamKind() const;
-  codeview::StreamRef getRecordData() const;
-
-private:
-  codeview::ModuleSubstreamKind Kind;
-  codeview::StreamRef Data;
-};
-}
-
-namespace codeview {
-template <> struct VarStreamArrayExtractor<pdb::ModuleSubstreamRecord> {
-  Error operator()(StreamRef Stream, uint32_t &Length,
-                   pdb::ModuleSubstreamRecord &Info) const {
-    if (auto EC = pdb::ModuleSubstreamRecord::initialize(Stream, Info))
-      return EC;
-    Length = Info.getRecordLength();
-    return Error::success();
-  }
-};
-}
-}
-
-#endif // LLVM_DEBUGINFO_PDB_RAW_MODULESUBSTREAMRECORD_H

include/llvm/DebugInfo/PDB/Raw/RawTypes.h

support::ulittle16_t ClassName;     // Byte index of the class name in the
                                       // sstSegName table, or 0xFFFF.
   support::ulittle32_t Offset;        // Byte offset of the logical segment
                                       // within the specified physical segment.
                                       // If group is set in flags, offset is the
                                       // offset of the group.
   support::ulittle32_t SecByteLength; // Byte count of the segment or group.
 };
 
-// Corresponds to the `CV_DebugSSubsectionHeader_t` structure.
-struct ModuleSubsectionHeader {
-  support::ulittle32_t Kind;   // codeview::ModuleSubstreamKind enum
-  support::ulittle32_t Length; // number of bytes occupied by this record.
-};
-
-// Corresponds to the `CV_DebugSLinesHeader_t` structure.
-struct LineTableSubsectionHeader {
-  support::ulittle32_t OffCon;
-  support::ulittle16_t SegCon;
-  support::ulittle16_t Flags;
-  support::ulittle32_t CbCon;
-};
-
-// Corresponds to the `CV_DebugSLinesFileBlockHeader_t` structure.
-struct SourceFileBlockHeader {
-  support::ulittle32_t offFile;
-  support::ulittle32_t nLines;
-  support::ulittle32_t cbBlock;
-  // LineInfo      lines[nLines];
-  // ColumnInfo    columns[nColumns];
-};
-
-// Corresponds to `CV_Line_t` structure
-struct LineInfo {
-  unsigned long Offset; // Offset to start of code bytes for line number
-  unsigned long LinenumStart : 24; // line where statement/expression starts
-  unsigned long
-      DeltaLineEnd : 7;         // delta to line where statement ends (optional)
-  unsigned long FStatement : 1; // true if a statement linenumber, else an
-                                // expression line num
-};
-
-// Corresponds to `CV_Column_t` structure
-struct ColumnInfo {
-  support::ulittle16_t OffColumnStart;
-  support::ulittle16_t OffColumnEnd;
-};
-
 } // namespace pdb
 } // namespace llvm
 
 #endif

lib/DebugInfo/CodeView/CMakeLists.txt

 add_llvm_library(LLVMDebugInfoCodeView
   ByteStream.cpp
   CodeViewError.cpp
   EnumTables.cpp
   FieldListRecordBuilder.cpp
   Line.cpp
   ListRecordBuilder.cpp
   MemoryTypeTableBuilder.cpp
   MethodListRecordBuilder.cpp
+  ModuleSubstream.cpp
+  ModuleSubstreamVisitor.cpp
   RecordSerialization.cpp
   StreamReader.cpp
   SymbolDumper.cpp
   TypeDumper.cpp
   TypeRecord.cpp
   TypeRecordBuilder.cpp
   TypeStreamMerger.cpp
   TypeTableBuilder.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/DebugInfo/CodeView
   )

lib/DebugInfo/CodeView/ModuleSubstream.cpp

+//===- ModuleSubstream.cpp --------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DebugInfo/CodeView/ModuleSubstream.h"
+
+#include "llvm/DebugInfo/CodeView/StreamReader.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+
+ModuleSubstream::ModuleSubstream() : Kind(ModuleSubstreamKind::None) {}
+
+ModuleSubstream::ModuleSubstream(ModuleSubstreamKind Kind, StreamRef Data)
+    : Kind(Kind), Data(Data) {}
+
+Error ModuleSubstream::initialize(StreamRef Stream, ModuleSubstream &Info) {
+  const ModuleSubsectionHeader *Header;
+  StreamReader Reader(Stream);
+  if (auto EC = Reader.readObject(Header))
+    return EC;
+
+  ModuleSubstreamKind Kind =
+      static_cast<ModuleSubstreamKind>(uint32_t(Header->Kind));
+  if (auto EC = Reader.readStreamRef(Info.Data, Header->Length))
+    return EC;
+  Info.Kind = Kind;
+  return Error::success();
+}
+
+uint32_t ModuleSubstream::getRecordLength() const {
+  return sizeof(ModuleSubsectionHeader) + Data.getLength();
+}
+
+ModuleSubstreamKind ModuleSubstream::getSubstreamKind() const { return Kind; }
+
+StreamRef ModuleSubstream::getRecordData() const { return Data; }

lib/DebugInfo/CodeView/ModuleSubstreamVisitor.cpp

+//===- ModuleSubstreamVisitor.cpp -------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+
+Error IModuleSubstreamVisitor::visitSymbols(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::Symbols, Data);
+}
+Error IModuleSubstreamVisitor::visitLines(StreamRef Data,
+                                          const LineSubstreamHeader *Header,
+                                          LineInfoArray Lines) {
+  return visitUnknown(ModuleSubstreamKind::Lines, Data);
+}
+Error IModuleSubstreamVisitor::visitStringTable(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::StringTable, Data);
+}
+Error IModuleSubstreamVisitor::visitFileChecksums(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::FileChecksums, Data);
+}
+Error IModuleSubstreamVisitor::visitFrameData(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::FrameData, Data);
+}
+Error IModuleSubstreamVisitor::visitInlineeLines(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::InlineeLines, Data);
+}
+Error IModuleSubstreamVisitor::visitCrossScopeImports(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::CrossScopeExports, Data);
+}
+Error IModuleSubstreamVisitor::visitCrossScopeExports(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::CrossScopeImports, Data);
+}
+Error IModuleSubstreamVisitor::visitILLines(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::ILLines, Data);
+}
+Error IModuleSubstreamVisitor::visitFuncMDTokenMap(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::FuncMDTokenMap, Data);
+}
+Error IModuleSubstreamVisitor::visitTypeMDTokenMap(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::TypeMDTokenMap, Data);
+}
+Error IModuleSubstreamVisitor::visitMergedAssemblyInput(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::MergedAssemblyInput, Data);
+}
+Error IModuleSubstreamVisitor::visitCoffSymbolRVA(StreamRef Data) {
+  return visitUnknown(ModuleSubstreamKind::CoffSymbolRVA, Data);
+}
+
+Error llvm::codeview::visitModuleSubstream(const ModuleSubstream &R,
+                                           IModuleSubstreamVisitor &V) {
+  switch (R.getSubstreamKind()) {
+  case ModuleSubstreamKind::Symbols:
+    return V.visitSymbols(R.getRecordData());
+  case ModuleSubstreamKind::Lines: {
+    StreamReader Reader(R.getRecordData());
+    const LineSubstreamHeader *Header;
+    if (auto EC = Reader.readObject(Header))
+      return EC;
+    FileLineInfoExtractor E(Header);
+    LineInfoArray LineInfos(E);
+    if (auto EC = Reader.readArray(LineInfos, Reader.bytesRemaining()))
+      return EC;
+    return V.visitLines(R.getRecordData(), Header, LineInfos);
+  }
+  case ModuleSubstreamKind::StringTable:
+    return V.visitStringTable(R.getRecordData());
+  case ModuleSubstreamKind::FileChecksums:
+    return V.visitFileChecksums(R.getRecordData());
+  case ModuleSubstreamKind::FrameData:
+    return V.visitFrameData(R.getRecordData());
+  case ModuleSubstreamKind::InlineeLines:
+    return V.visitInlineeLines(R.getRecordData());
+  case ModuleSubstreamKind::CrossScopeImports:
+    return V.visitCrossScopeImports(R.getRecordData());
+  case ModuleSubstreamKind::CrossScopeExports:
+    return V.visitCrossScopeExports(R.getRecordData());
+  case ModuleSubstreamKind::ILLines:
+    return V.visitILLines(R.getRecordData());
+  case ModuleSubstreamKind::FuncMDTokenMap:
+    return V.visitFuncMDTokenMap(R.getRecordData());
+  case ModuleSubstreamKind::TypeMDTokenMap:
+    return V.visitTypeMDTokenMap(R.getRecordData());
+  case ModuleSubstreamKind::MergedAssemblyInput:
+    return V.visitMergedAssemblyInput(R.getRecordData());
+  case ModuleSubstreamKind::CoffSymbolRVA:
+    return V.visitCoffSymbolRVA(R.getRecordData());
+  default:
+    return V.visitUnknown(R.getSubstreamKind(), R.getRecordData());
+  }
+}

lib/DebugInfo/PDB/CMakeLists.txt

 endif()
 
 add_pdb_impl_folder(Raw
   Raw/DbiStream.cpp
   Raw/EnumTables.cpp
   Raw/InfoStream.cpp
   Raw/MappedBlockStream.cpp
   Raw/ModInfo.cpp
-  Raw/ModuleSubstreamRecord.cpp
   Raw/ModStream.cpp
   Raw/NameHashTable.cpp
   Raw/NameMap.cpp
   Raw/PDBFile.cpp
   Raw/PublicsStream.cpp
   Raw/RawError.cpp
   Raw/RawSession.cpp
   Raw/SymbolStream.cpp

lib/DebugInfo/PDB/Raw/ModStream.cpp

 }
 
 iterator_range<codeview::CVSymbolArray::Iterator>
 ModStream::symbols(bool *HadError) const {
   return llvm::make_range(SymbolsSubstream.begin(HadError),
                           SymbolsSubstream.end());
 }
 
-iterator_range<ModStream::LineInfoArray::Iterator>
+iterator_range<codeview::ModuleSubstreamArray::Iterator>
 ModStream::lines(bool *HadError) const {
   return llvm::make_range(LineInfo.begin(HadError), LineInfo.end());
 }

lib/DebugInfo/PDB/Raw/ModuleSubstreamRecord.cpp

-//===- ModuleSubstreamRecord.cpp --------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "llvm/DebugInfo/PDB/Raw/ModuleSubstreamRecord.h"
-
-#include "llvm/DebugInfo/CodeView/StreamReader.h"
-#include "llvm/DebugInfo/PDB/Raw/RawTypes.h"
-
-using namespace llvm;
-using namespace llvm::codeview;
-using namespace llvm::pdb;
-
-ModuleSubstreamRecord::ModuleSubstreamRecord()
-    : Kind(ModuleSubstreamKind::None) {}
-
-ModuleSubstreamRecord::ModuleSubstreamRecord(ModuleSubstreamKind Kind,
-                                             StreamRef Data)
-    : Kind(Kind), Data(Data) {}
-
-Error ModuleSubstreamRecord::initialize(StreamRef Stream,
-                                        ModuleSubstreamRecord &Info) {
-  const ModuleSubsectionHeader *Header;
-  StreamReader Reader(Stream);
-  if (auto EC = Reader.readObject(Header))
-    return EC;
-
-  ModuleSubstreamKind Kind =
-      static_cast<ModuleSubstreamKind>(uint32_t(Header->Kind));
-  if (auto EC = Reader.readStreamRef(Info.Data, Header->Length))
-    return EC;
-  Info.Kind = Kind;
-  return Error::success();
-}
-
-uint32_t ModuleSubstreamRecord::getRecordLength() const {
-  return sizeof(ModuleSubsectionHeader) + Data.getLength();
-}
-
-ModuleSubstreamKind ModuleSubstreamRecord::getSubstreamKind() const {
-  return Kind;
-}
-
-StreamRef ModuleSubstreamRecord::getRecordData() const { return Data; }

test/DebugInfo/PDB/pdbdump-headers.test

 ; EMPTY-NEXT:             BuildId: 4110
 ; EMPTY-NEXT:           }
 ; EMPTY-NEXT:           Bytes (
 ; EMPTY-NEXT:             0000: 0E100000                             |....|
 ; EMPTY-NEXT:           )
 ; EMPTY-NEXT:         }
 ; EMPTY-NEXT:       ]
 ; EMPTY-NEXT:       LineInfo [
-; EMPTY-NEXT:         {
-; EMPTY-NEXT:           Kind: Lines (0xF2)
+; EMPTY-NEXT:         Lines {
+; EMPTY-NEXT:           FileOffset: 0
+; EMPTY-NEXT:           Line {
+; EMPTY-NEXT:             Offset: 0
+; EMPTY-NEXT:             LineNumberStart: 5
+; EMPTY-NEXT:             EndDelta: 0
+; EMPTY-NEXT:             IsStatement: Yes
+; EMPTY-NEXT:           }
+; EMPTY-NEXT:           Line {
+; EMPTY-NEXT:             Offset: 3
+; EMPTY-NEXT:             LineNumberStart: 6
+; EMPTY-NEXT:             EndDelta: 0
+; EMPTY-NEXT:             IsStatement: Yes
+; EMPTY-NEXT:           }
+; EMPTY-NEXT:           Line {
+; EMPTY-NEXT:             Offset: 8
+; EMPTY-NEXT:             LineNumberStart: 7
+; EMPTY-NEXT:             EndDelta: 0
+; EMPTY-NEXT:             IsStatement: Yes
+; EMPTY-NEXT:           }
 ; EMPTY-NEXT:           Data (
 ; EMPTY-NEXT:             0000: 10000000 01000000 0A000000 00000000  |................|
 ; EMPTY-NEXT:             0010: 03000000 24000000 00000000 05000080  |....$...........|
 ; EMPTY-NEXT:             0020: 03000000 06000080 08000000 07000080  |................|
 ; EMPTY-NEXT:           )
 ; EMPTY-NEXT:         }
-; EMPTY-NEXT:         {
-; EMPTY-NEXT:           Kind: FileChecksums (0xF4)
+; EMPTY-NEXT:         FileChecksums {
 ; EMPTY-NEXT:           Data (
 ; EMPTY-NEXT:             0000: 56000000 1001A0A5 BD0D3ECD 93FC29D1  |V.........>...).|
 ; EMPTY-NEXT:             0010: 9DE826FB F4BC0000                    |..&.....|
 ; EMPTY-NEXT:           )
 ; EMPTY-NEXT:         }
 ; EMPTY-NEXT:       ]
 ; EMPTY-NEXT:     }
 ; EMPTY-NEXT:     {

tools/llvm-pdbdump/llvm-pdbdump.cpp

 #include "VariableDumper.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/config.h"
 #include "llvm/DebugInfo/CodeView/EnumTables.h"
+#include "llvm/DebugInfo/CodeView/Line.h"
+#include "llvm/DebugInfo/CodeView/ModuleSubstreamVisitor.h"
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include "llvm/DebugInfo/CodeView/SymbolDumper.h"
 #include "llvm/DebugInfo/CodeView/TypeDumper.h"
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBRawSymbol.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/PDB.h"

 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
+using namespace llvm::codeview;
 using namespace llvm::pdb;
 
 namespace opts {
 
 enum class PDB_DumpType { ByType, ByObjFile, Both };
 
 cl::list<std::string> InputFilenames(cl::Positional,
                                      cl::desc("<input PDB files>"),
for (auto &Modi : DS.modules()) {
         ModStream ModS(File, Modi.Info);
         if (auto EC = ModS.reload())
           return EC;
 
         if (ShouldDumpSymbols) {
           ListScope SS(P, "Symbols");
           codeview::CVSymbolDumper SD(P, TD, nullptr, false);
           bool HadError = false;
-          for (auto &S : ModS.symbols(&HadError)) {
+          for (const auto &S : ModS.symbols(&HadError)) {
             DictScope DD(P, "");
 
             if (opts::DumpModuleSyms)
               SD.dump(S);
             if (opts::DumpSymRecordBytes)
               P.printBinaryBlock("Bytes", S.Data);
           }
           if (HadError)
             return make_error<RawError>(
                 raw_error_code::corrupt_file,
                 "DBI stream contained corrupt symbol record");
         }
         if (opts::DumpLineInfo) {
           ListScope SS(P, "LineInfo");
           bool HadError = false;
-          for (auto &L : ModS.lines(&HadError)) {
-            DictScope DD(P, "");
-            P.printEnum("Kind", uint32_t(L.getSubstreamKind()),
-                        codeview::getModuleSubstreamKindNames());
+          // Define a locally scoped visitor to print the different
+          // substream types types.
+          class RecordVisitor : public codeview::IModuleSubstreamVisitor {
+          public:
+            RecordVisitor(ScopedPrinter &P) : P(P) {}
+            Error visitUnknown(ModuleSubstreamKind Kind,
+                               StreamRef Data) override {
+              DictScope DD(P, "Unknown");
+              return printBinaryData(Data);
+            }
+            Error visitFileChecksums(StreamRef Data) override {
+              DictScope DD(P, "FileChecksums");
+              return printBinaryData(Data);
+            }
+
+            Error visitLines(StreamRef Data, const LineSubstreamHeader *Header,
+                             LineInfoArray Lines) override {
+              DictScope DD(P, "Lines");
+              for (const auto &L : Lines) {
+                P.printNumber("FileOffset", L.Offset);
+                for (const auto &N : L.LineNumbers) {
+                  DictScope DDD(P, "Line");
+                  LineInfo LI(N.Flags);
+                  P.printNumber("Offset", N.Offset);
+                  if (LI.isAlwaysStepInto())
+                    P.printString("StepInto", StringRef("Always"));
+                  else if (LI.isNeverStepInto())
+                    P.printString("StepInto", StringRef("Never"));
+                  else
+                    P.printNumber("LineNumberStart", LI.getStartLine());
+                  P.printNumber("EndDelta", LI.getLineDelta());
+                  P.printBoolean("IsStatement", LI.isStatement());
+                }
+                for (const auto &C : L.Columns) {
+                  DictScope DDD(P, "Column");
+                  P.printNumber("Start", C.StartColumn);
+                  P.printNumber("End", C.EndColumn);
+                }
+              }
+              return printBinaryData(Data);
+            }
+
+          private:
+            Error printBinaryData(StreamRef Stream) {
               ArrayRef<uint8_t> Data;
-            codeview::StreamReader R(L.getRecordData());
+              StreamReader R(Stream);
               if (auto EC = R.readBytes(Data, R.bytesRemaining())) {
                 return make_error<RawError>(
                     raw_error_code::corrupt_file,
                     "DBI stream contained corrupt line info record");
               }
               P.printBinaryBlock("Data", Data);
+              P.flush();
+              return Error::success();
+            }
+            ScopedPrinter &P;
+          };
+          RecordVisitor V(P);
+          for (const auto &L : ModS.lines(&HadError)) {
+            if (auto EC = codeview::visitModuleSubstream(L, V))
+              return EC;
           }
         }
       }
     }
   }
   return Error::success();
 }
 

tools/llvm-readobj/COFFDumper.cpp

void COFFDumper::printCodeViewSymbolSection(StringRef SectionName,
   uint32_t Magic;
   error(consume(Data, Magic));
   W.printHex("Magic", Magic);
   if (Magic != COFF::DEBUG_SECTION_MAGIC)
     return error(object_error::parse_failed);
 
   initializeFileAndStringTables(Data);
 
+  // TODO: Convert this over to using ModuleSubstreamVisitor.
   while (!Data.empty()) {
     // The section consists of a number of subsection in the following format:
     // |SubSectionType|SubSectionSize|Contents...|
     uint32_t SubType, SubSectionSize;
     error(consume(Data, SubType));
     error(consume(Data, SubSectionSize));
 
     ListScope S(W, "Subsection");