[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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@271313 91177308-0d34-0410-b5e6-96231b3b80d8

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 @@ class MemoryTypeTableBuilder : public TypeTableBuilder {
   template <typename TFunc> void ForEachRecord(TFunc Func) {
     uint32_t Index = TypeIndex::FirstNonSimpleIndex;
 
-    for (const std::unique_ptr<Record> &R : Records) {
-      Func(TypeIndex(Index), R.get());
+    for (StringRef R : Records) {
+      Func(TypeIndex(Index), R);
       ++Index;
     }
   }
@@ -61,8 +37,9 @@ class MemoryTypeTableBuilder : public TypeTableBuilder {
   TypeIndex writeRecord(llvm::StringRef Data) override;
 
 private:
-  std::vector<std::unique_ptr<Record>> Records;
-  std::unordered_map<llvm::StringRef, TypeIndex, RecordHash> HashedRecords;
+  std::vector<StringRef> Records;
+  BumpPtrAllocator RecordStorage;
+  DenseMap<StringRef, TypeIndex> HashedRecords;
 };
 
 } // end namespace codeview

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)
-      : W(&W), PrintRecordBytes(PrintRecordBytes) {}
+  CVTypeDumper(ScopedPrinter *W, bool PrintRecordBytes)
+      : W(W), PrintRecordBytes(PrintRecordBytes) {}
 
   StringRef getTypeName(TypeIndex TI);
   void printTypeIndex(StringRef FieldName, TypeIndex TI);

include/llvm/MC/MCStreamer.h

@@ -523,6 +523,10 @@ class MCStreamer {
   /// 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.
   ///

include/llvm/Support/ScopedPrinter.h

@@ -78,7 +78,12 @@ class ScopedPrinter {
     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 @@ class ScopedPrinter {
 
   raw_ostream &OS;
   int IndentLevel;
+  StringRef Prefix;
 };
 
 template <>

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) {
-        // Each record should be 4 byte aligned. We achieve that by emitting
-        // LF_PAD padding bytes. The on-disk record size includes the padding
-        // bytes so that consumers don't have to skip past them.
-        uint64_t RecordSize = R->size() + 2;
-        uint64_t AlignedSize = alignTo(RecordSize, 4);
-        uint64_t AlignedRecordSize = AlignedSize - 2;
-        assert(AlignedRecordSize < (1 << 16) && "type record size overflow");
-        OS.AddComment("Type record length");
-        OS.EmitIntValue(AlignedRecordSize, 2);
-        OS.AddComment("Type record data");
-        OS.EmitBytes(StringRef(R->data(), R->size()));
-        // Pad the record with LF_PAD bytes.
-        for (unsigned I = AlignedSize - RecordSize; I > 0; --I)
-          OS.EmitIntValue(LF_PAD0 + I, 1);
+      [&](TypeIndex Index, StringRef Record) {
+        if (OS.isVerboseAsm()) {
+          // Emit a block comment describing the type record for readability.
+          SmallString<512> CommentBlock;
+          raw_svector_ostream CommentOS(CommentBlock);
+          ScopedPrinter SP(CommentOS);
+          SP.setPrefix(CommentPrefix);
+          CVTD.setPrinter(&SP);
+          bool DumpSuccess =
+              CVTD.dump({Record.bytes_begin(), Record.bytes_end()});
+          (void)DumpSuccess;
+          assert(DumpSuccess && "produced malformed type record");
+          // emitRawComment will insert its own tab and comment string before
+          // the first line, so strip off our first one. It also prints its own
+          // newline.
+          OS.emitRawComment(
+              CommentOS.str().drop_front(CommentPrefix.size() - 1).rtrim());
+        }
+        OS.EmitBinaryData(Record);
       });
 }
 

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;
 }

lib/MC/MCAsmStreamer.cpp

@@ -162,6 +162,8 @@ class MCAsmStreamer final : public MCStreamer {
   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);
 }

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);
 }

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:.*]] #

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))

tools/llvm-readobj/COFFDumper.cpp

@@ -63,7 +63,7 @@ class COFFDumper : public ObjDumper {
   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) {
-    // The record data doesn't include the 16 bit size.
-    Buf.push_back(R->size() & 0xff);
-    Buf.push_back((R->size() >> 8) & 0xff);
-    Buf.append(R->data(), R->data() + R->size());
+  CVTypes.ForEachRecord([&](TypeIndex TI, StringRef Record) {
+    Buf.append(Record.begin(), Record.end());
   });
-  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);