[codeview] Improve readability of type record assembly

Adds the method MCStreamer::EmitBinaryData, which is usually an alias
for EmitBytes. In the MCAsmStreamer case, it is overridden to emit hex
dump output like this:
        .byte   0x0e, 0x00, 0x08, 0x10
        .byte   0x03, 0x00, 0x00, 0x00
        .byte   0x00, 0x00, 0x00, 0x00
        .byte   0x00, 0x10, 0x00, 0x00

Also, when verbose asm comments are enabled, this patch prints the dump
output for each comment before its record, like this:
        # ArgList (0x1000) {
        #   TypeLeafKind: LF_ARGLIST (0x1201)
        #   NumArgs: 0
        #   Arguments [
        #   ]
        # }
        .byte   0x06, 0x00, 0x01, 0x12
        .byte   0x00, 0x00, 0x00, 0x00

This should make debugging easier and testing more convenient.

Reviewers: aaboud

Subscribers: majnemer, zturner, amccarth, aaboud, llvm-commits

Differential Revision: http://reviews.llvm.org/D20711

llvm-svn: 271313

llvm/include/llvm/DebugInfo/CodeView/MemoryTypeTableBuilder.h

@@ -10,39 +10,15 @@
 #ifndef LLVM_DEBUGINFO_CODEVIEW_MEMORYTYPETABLEBUILDER_H
 #define LLVM_DEBUGINFO_CODEVIEW_MEMORYTYPETABLEBUILDER_H
 
-#include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
-#include <functional>
-#include <memory>
-#include <unordered_map>
 #include <vector>
 
 namespace llvm {
 namespace codeview {
 
 class MemoryTypeTableBuilder : public TypeTableBuilder {
-public:
-  class Record {
-  public:
-    explicit Record(llvm::StringRef RData);
-
-    const char *data() const { return Data.get(); }
-    uint16_t size() const { return Size; }
-
-  private:
-    uint16_t Size;
-    std::unique_ptr<char[]> Data;
-  };
-
-private:
-  class RecordHash : std::unary_function<llvm::StringRef, size_t> {
-  public:
-    size_t operator()(llvm::StringRef Val) const {
-      return static_cast<size_t>(llvm::hash_value(Val));
-    }
-  };
-
 public:
   MemoryTypeTableBuilder() {}
 
@@ -51,8 +27,8 @@ public:
   template <typename TFunc> void ForEachRecord(TFunc Func) {
     uint32_t Index = TypeIndex::FirstNonSimpleIndex;
 
-    for (const std::unique_ptr<Record> &R : Records) {
+    for (StringRef R : Records) {
-      Func(TypeIndex(Index), R.get());
+      Func(TypeIndex(Index), R);
       ++Index;
     }
   }
@@ -61,8 +37,9 @@ protected:
   TypeIndex writeRecord(llvm::StringRef Data) override;
 
 private:
-  std::vector<std::unique_ptr<Record>> Records;
+  std::vector<StringRef> Records;
-  std::unordered_map<llvm::StringRef, TypeIndex, RecordHash> HashedRecords;
+  BumpPtrAllocator RecordStorage;
+  DenseMap<StringRef, TypeIndex> HashedRecords;
 };
 
 } // end namespace codeview

llvm/include/llvm/DebugInfo/CodeView/TypeDumper.h

@@ -23,8 +23,8 @@ namespace codeview {
 /// Dumper for CodeView type streams found in COFF object files and PDB files.
 class CVTypeDumper {
 public:
-  CVTypeDumper(ScopedPrinter &W, bool PrintRecordBytes)
+  CVTypeDumper(ScopedPrinter *W, bool PrintRecordBytes)
-      : W(&W), PrintRecordBytes(PrintRecordBytes) {}
+      : W(W), PrintRecordBytes(PrintRecordBytes) {}
 
   StringRef getTypeName(TypeIndex TI);
   void printTypeIndex(StringRef FieldName, TypeIndex TI);

llvm/include/llvm/MC/MCStreamer.h

@@ -523,6 +523,10 @@ public:
   /// etc.
   virtual void EmitBytes(StringRef Data);
 
+  /// Functionally identical to EmitBytes. When emitting textual assembly, this
+  /// method uses .byte directives instead of .ascii or .asciz for readability.
+  virtual void EmitBinaryData(StringRef Data);
+
   /// \brief Emit the expression \p Value into the output as a native
   /// integer of the given \p Size bytes.
   ///

llvm/include/llvm/Support/ScopedPrinter.h

@@ -78,7 +78,12 @@ public:
     IndentLevel = std::max(0, IndentLevel - Levels);
   }
 
+  void resetIndent() { IndentLevel = 0; }
+
+  void setPrefix(StringRef P) { Prefix = P; }
+
   void printIndent() {
+    OS << Prefix;
     for (int i = 0; i < IndentLevel; ++i)
       OS << "  ";
   }
@@ -332,6 +337,7 @@ private:
 
   raw_ostream &OS;
   int IndentLevel;
+  StringRef Prefix;
 };
 
 template <>

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

@@ -15,12 +15,14 @@
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/CodeView/Line.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeDumper.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCSectionCOFF.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/COFF.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetFrameLowering.h"
@@ -282,22 +284,34 @@ void CodeViewDebug::emitTypeInformation() {
   OS.SwitchSection(Asm->getObjFileLowering().getCOFFDebugTypesSection());
   emitCodeViewMagicVersion();
 
+  SmallString<8> CommentPrefix;
+  if (OS.isVerboseAsm()) {
+    CommentPrefix += '\t';
+    CommentPrefix += Asm->MAI->getCommentString();
+    CommentPrefix += ' ';
+  }
+
+  CVTypeDumper CVTD(nullptr, /*PrintRecordBytes=*/false);
   TypeTable.ForEachRecord(
-      [&](TypeIndex Index, const MemoryTypeTableBuilder::Record *R) {
+      [&](TypeIndex Index, StringRef Record) {
-        // Each record should be 4 byte aligned. We achieve that by emitting
+        if (OS.isVerboseAsm()) {
-        // LF_PAD padding bytes. The on-disk record size includes the padding
+          // Emit a block comment describing the type record for readability.
-        // bytes so that consumers don't have to skip past them.
+          SmallString<512> CommentBlock;
-        uint64_t RecordSize = R->size() + 2;
+          raw_svector_ostream CommentOS(CommentBlock);
-        uint64_t AlignedSize = alignTo(RecordSize, 4);
+          ScopedPrinter SP(CommentOS);
-        uint64_t AlignedRecordSize = AlignedSize - 2;
+          SP.setPrefix(CommentPrefix);
-        assert(AlignedRecordSize < (1 << 16) && "type record size overflow");
+          CVTD.setPrinter(&SP);
-        OS.AddComment("Type record length");
+          bool DumpSuccess =
-        OS.EmitIntValue(AlignedRecordSize, 2);
+              CVTD.dump({Record.bytes_begin(), Record.bytes_end()});
-        OS.AddComment("Type record data");
+          (void)DumpSuccess;
-        OS.EmitBytes(StringRef(R->data(), R->size()));
+          assert(DumpSuccess && "produced malformed type record");
-        // Pad the record with LF_PAD bytes.
+          // emitRawComment will insert its own tab and comment string before
-        for (unsigned I = AlignedSize - RecordSize; I > 0; --I)
+          // the first line, so strip off our first one. It also prints its own
-          OS.EmitIntValue(LF_PAD0 + I, 1);
+          // newline.
+          OS.emitRawComment(
+              CommentOS.str().drop_front(CommentPrefix.size() - 1).rtrim());
+        }
+        OS.EmitBinaryData(Record);
       });
 }
 

llvm/lib/DebugInfo/CodeView/MemoryTypeTableBuilder.cpp

@@ -13,23 +13,34 @@
 using namespace llvm;
 using namespace codeview;
 
-MemoryTypeTableBuilder::Record::Record(StringRef RData)
-    : Size(RData.size()), Data(new char[RData.size()]) {
-  memcpy(Data.get(), RData.data(), RData.size());
-}
-
 TypeIndex MemoryTypeTableBuilder::writeRecord(StringRef Data) {
+  assert(Data.size() <= UINT16_MAX);
   auto I = HashedRecords.find(Data);
   if (I != HashedRecords.end()) {
     return I->second;
   }
 
-  std::unique_ptr<Record> R(new Record(Data));
+  // The record provided by the user lacks the 2 byte size field prefix and is
+  // not padded to 4 bytes. Ultimately, that is what gets emitted in the object
+  // file, so pad it out now.
+  const int SizeOfRecLen = 2;
+  const int Align = 4;
+  int TotalSize = alignTo(Data.size() + SizeOfRecLen, Align);
+  assert(TotalSize - SizeOfRecLen <= UINT16_MAX);
+  char *Mem =
+      reinterpret_cast<char *>(RecordStorage.Allocate(TotalSize, Align));
+  *reinterpret_cast<ulittle16_t *>(Mem) = uint16_t(TotalSize - SizeOfRecLen);
+  memcpy(Mem + SizeOfRecLen, Data.data(), Data.size());
+  for (int I = Data.size() + SizeOfRecLen; I < TotalSize; ++I)
+    Mem[I] = LF_PAD0 + (TotalSize - I);
 
   TypeIndex TI(static_cast<uint32_t>(Records.size()) +
                TypeIndex::FirstNonSimpleIndex);
-  HashedRecords.insert(std::make_pair(StringRef(R->data(), R->size()), TI));
+
-  Records.push_back(std::move(R));
+  // Use only the data supplied by the user as a key to the hash table, so that
+  // future lookups will succeed.
+  HashedRecords.insert(std::make_pair(StringRef(Mem + SizeOfRecLen, Data.size()), TI));
+  Records.push_back(StringRef(Mem, TotalSize));
 
   return TI;
 }

llvm/lib/MC/MCAsmStreamer.cpp

@@ -162,6 +162,8 @@ public:
   void EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                       unsigned ByteAlignment = 0) override;
 
+  void EmitBinaryData(StringRef Data) override;
+
   void EmitBytes(StringRef Data) override;
 
   void EmitValueImpl(const MCExpr *Value, unsigned Size,
@@ -709,6 +711,20 @@ void MCAsmStreamer::EmitBytes(StringRef Data) {
   EmitEOL();
 }
 
+void MCAsmStreamer::EmitBinaryData(StringRef Data) {
+  // This is binary data. Print it in a grid of hex bytes for readability.
+  const size_t Cols = 4;
+  for (size_t I = 0, EI = alignTo(Data.size(), Cols); I < EI; I += Cols) {
+    size_t J = I, EJ = std::min(I + Cols, Data.size());
+    assert(EJ > 0);
+    OS << MAI->getData8bitsDirective();
+    for (; J < EJ - 1; ++J)
+      OS << format("0x%02x", uint8_t(Data[J])) << ", ";
+    OS << format("0x%02x", uint8_t(Data[J]));
+    EmitEOL();
+  }
+}
+
 void MCAsmStreamer::EmitIntValue(uint64_t Value, unsigned Size) {
   EmitValue(MCConstantExpr::create(Value, getContext()), Size);
 }

llvm/lib/MC/MCStreamer.cpp

@@ -761,6 +761,7 @@ void MCStreamer::EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
 void MCStreamer::ChangeSection(MCSection *, const MCExpr *) {}
 void MCStreamer::EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {}
 void MCStreamer::EmitBytes(StringRef Data) {}
+void MCStreamer::EmitBinaryData(StringRef Data) { EmitBytes(Data); }
 void MCStreamer::EmitValueImpl(const MCExpr *Value, unsigned Size, SMLoc Loc) {
   visitUsedExpr(*Value);
 }

llvm/test/DebugInfo/COFF/inlining.ll

@@ -39,6 +39,50 @@
 ; ASM: addl    $7, "?x@@3HC"
 ; ASM: .cv_loc 0 1 17 1                # t.cpp:17:1
 
+; ASM: .section .debug$T,"dr"
+; ASM: .long 4 # Debug section magic
+; ASM: # ArgList (0x1000) {
+; ASM: #   TypeLeafKind: LF_ARGLIST (0x1201)
+; ASM: #   NumArgs: 0
+; ASM: #   Arguments [
+; ASM: #   ]
+; ASM: # }
+; ASM: .byte   0x06, 0x00, 0x01, 0x12
+; ASM: .byte   0x00, 0x00, 0x00, 0x00
+; ASM: # Procedure (0x1001) {
+; ASM: #   TypeLeafKind: LF_PROCEDURE (0x1008)
+; ASM: #   ReturnType: void (0x3)
+; ASM: #   CallingConvention: NearC (0x0)
+; ASM: #   FunctionOptions [ (0x0)
+; ASM: #   ]
+; ASM: #   NumParameters: 0
+; ASM: #   ArgListType: () (0x1000)
+; ASM: # }
+; ASM: .byte   0x0e, 0x00, 0x08, 0x10
+; ASM: .byte   0x03, 0x00, 0x00, 0x00
+; ASM: .byte   0x00, 0x00, 0x00, 0x00
+; ASM: .byte   0x00, 0x10, 0x00, 0x00
+; ASM: # FuncId (0x1002) {
+; ASM: #   TypeLeafKind: LF_FUNC_ID (0x1601)
+; ASM: #   ParentScope: 0x0
+; ASM: #   FunctionType: void () (0x1001)
+; ASM: #   Name: bar
+; ASM: # }
+; ASM: .byte   0x0e, 0x00, 0x01, 0x16
+; ASM: .byte   0x00, 0x00, 0x00, 0x00
+; ASM: .byte   0x01, 0x10, 0x00, 0x00
+; ASM: .byte   0x62, 0x61, 0x72, 0x00
+; ASM: # FuncId (0x1003) {
+; ASM: #   TypeLeafKind: LF_FUNC_ID (0x1601)
+; ASM: #   ParentScope: 0x0
+; ASM: #   FunctionType: void () (0x1001)
+; ASM: #   Name: foo
+; ASM: # }
+; ASM: .byte   0x0e, 0x00, 0x01, 0x16
+; ASM: .byte   0x00, 0x00, 0x00, 0x00
+; ASM: .byte   0x01, 0x10, 0x00, 0x00
+; ASM: .byte   0x66, 0x6f, 0x6f, 0x00
+
 ; ASM: .section .debug$S,"dr"
 ; ASM: .long   246                     # Inlinee lines subsection
 ; ASM: .long   [[inline_end:.*]]-[[inline_beg:.*]] #

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

@@ -639,7 +639,7 @@ static Error dumpStructure(RawSession &RS) {
   if (auto EC = dumpNamedStream(P, File))
     return EC;
 
-  codeview::CVTypeDumper TD(P, false);
+  codeview::CVTypeDumper TD(&P, false);
   if (auto EC = dumpTpiStream(P, File, TD, StreamTPI))
     return EC;
   if (auto EC = dumpTpiStream(P, File, TD, StreamIPI))

llvm/tools/llvm-readobj/COFFDumper.cpp

@@ -63,7 +63,7 @@ public:
   friend class COFFObjectDumpDelegate;
   COFFDumper(const llvm::object::COFFObjectFile *Obj, ScopedPrinter &Writer)
       : ObjDumper(Writer), Obj(Obj),
-        CVTD(Writer, opts::CodeViewSubsectionBytes) {}
+        CVTD(&Writer, opts::CodeViewSubsectionBytes) {}
 
   void printFileHeaders() override;
   void printSections() override;
@@ -1495,13 +1495,10 @@ void llvm::dumpCodeViewMergedTypes(
   // Flatten it first, then run our dumper on it.
   ListScope S(Writer, "MergedTypeStream");
   SmallString<0> Buf;
-  CVTypes.ForEachRecord([&](TypeIndex TI, MemoryTypeTableBuilder::Record *R) {
+  CVTypes.ForEachRecord([&](TypeIndex TI, StringRef Record) {
-    // The record data doesn't include the 16 bit size.
+    Buf.append(Record.begin(), Record.end());
-    Buf.push_back(R->size() & 0xff);
-    Buf.push_back((R->size() >> 8) & 0xff);
-    Buf.append(R->data(), R->data() + R->size());
   });
-  CVTypeDumper CVTD(Writer, opts::CodeViewSubsectionBytes);
+  CVTypeDumper CVTD(&Writer, opts::CodeViewSubsectionBytes);
   if (!CVTD.dump({Buf.str().bytes_begin(), Buf.str().bytes_end()})) {
     Writer.flush();
     error(object_error::parse_failed);