From 25565d97e7d70fe116fd871506959361be5fe126 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Tue, 31 May 2016 18:45:36 +0000 Subject: [PATCH] [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 --- .../CodeView/MemoryTypeTableBuilder.h | 35 +++------------ include/llvm/DebugInfo/CodeView/TypeDumper.h | 4 +- include/llvm/MC/MCStreamer.h | 4 ++ include/llvm/Support/ScopedPrinter.h | 6 +++ lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | 44 ++++++++++++------- .../CodeView/MemoryTypeTableBuilder.cpp | 27 ++++++++---- lib/MC/MCAsmStreamer.cpp | 16 +++++++ lib/MC/MCStreamer.cpp | 1 + test/DebugInfo/COFF/inlining.ll | 44 +++++++++++++++++++ tools/llvm-pdbdump/llvm-pdbdump.cpp | 2 +- tools/llvm-readobj/COFFDumper.cpp | 11 ++--- 11 files changed, 132 insertions(+), 62 deletions(-) diff --git a/include/llvm/DebugInfo/CodeView/MemoryTypeTableBuilder.h b/include/llvm/DebugInfo/CodeView/MemoryTypeTableBuilder.h index 81635076b34..002f885c7c5 100644 --- a/include/llvm/DebugInfo/CodeView/MemoryTypeTableBuilder.h +++ b/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 -#include -#include #include 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 Data; - }; - -private: - class RecordHash : std::unary_function { - public: - size_t operator()(llvm::StringRef Val) const { - return static_cast(llvm::hash_value(Val)); - } - }; - public: MemoryTypeTableBuilder() {} @@ -51,8 +27,8 @@ public: template void ForEachRecord(TFunc Func) { uint32_t Index = TypeIndex::FirstNonSimpleIndex; - for (const std::unique_ptr &R : Records) { - Func(TypeIndex(Index), R.get()); + for (StringRef R : Records) { + Func(TypeIndex(Index), R); ++Index; } } @@ -61,8 +37,9 @@ protected: TypeIndex writeRecord(llvm::StringRef Data) override; private: - std::vector> Records; - std::unordered_map HashedRecords; + std::vector Records; + BumpPtrAllocator RecordStorage; + DenseMap HashedRecords; }; } // end namespace codeview diff --git a/include/llvm/DebugInfo/CodeView/TypeDumper.h b/include/llvm/DebugInfo/CodeView/TypeDumper.h index 77dab8e8d3d..a5e359fd586 100644 --- a/include/llvm/DebugInfo/CodeView/TypeDumper.h +++ b/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); diff --git a/include/llvm/MC/MCStreamer.h b/include/llvm/MC/MCStreamer.h index fb60a1994c4..8b19ccddd19 100644 --- a/include/llvm/MC/MCStreamer.h +++ b/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. /// diff --git a/include/llvm/Support/ScopedPrinter.h b/include/llvm/Support/ScopedPrinter.h index aa338bb6722..b9d8a8db1b6 100644 --- a/include/llvm/Support/ScopedPrinter.h +++ b/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 <> diff --git a/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp index 3f3408eb368..b09925b74c2 100644 --- a/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp +++ b/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); }); } diff --git a/lib/DebugInfo/CodeView/MemoryTypeTableBuilder.cpp b/lib/DebugInfo/CodeView/MemoryTypeTableBuilder.cpp index 9afce92eeb1..8b9e73b94ff 100644 --- a/lib/DebugInfo/CodeView/MemoryTypeTableBuilder.cpp +++ b/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 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(RecordStorage.Allocate(TotalSize, Align)); + *reinterpret_cast(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(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; } diff --git a/lib/MC/MCAsmStreamer.cpp b/lib/MC/MCAsmStreamer.cpp index 5fbecb408f3..c85ffd5182d 100644 --- a/lib/MC/MCAsmStreamer.cpp +++ b/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); } diff --git a/lib/MC/MCStreamer.cpp b/lib/MC/MCStreamer.cpp index 9ed5a5faacd..8872d321d88 100644 --- a/lib/MC/MCStreamer.cpp +++ b/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); } diff --git a/test/DebugInfo/COFF/inlining.ll b/test/DebugInfo/COFF/inlining.ll index c70c79f9cad..a237173f326 100644 --- a/test/DebugInfo/COFF/inlining.ll +++ b/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:.*]] # diff --git a/tools/llvm-pdbdump/llvm-pdbdump.cpp b/tools/llvm-pdbdump/llvm-pdbdump.cpp index af2540903d2..80755043892 100644 --- a/tools/llvm-pdbdump/llvm-pdbdump.cpp +++ b/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)) diff --git a/tools/llvm-readobj/COFFDumper.cpp b/tools/llvm-readobj/COFFDumper.cpp index 0949436a8f3..fccfd465c57 100644 --- a/tools/llvm-readobj/COFFDumper.cpp +++ b/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) { - // 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);