[llvm-pdbdump] Display padding bytes on record layout

[zturner]

Summary

When dumping classes, show where padding occurs, and at the end of the class print statistics about how many bytes total of padding exist in a class.

Since PDB doesn't specifically contain information about padding, we have to mimic this by sort of reversing a small portion of the record layout algorithm (e.g. looking at offsets and sizes and trying to determine whether something is part of the same field or a new field).

[zturner]

Use a better method for detecting padding. This is less error prone and more efficient. It also allows us to analyze the padding one time up front, which allows us to filter record display based on whether or not the record has padding (e.g. only dump those records that have padding).

[amccarth]

LGTM

With this, it seems llvm-pdbdump is becoming a more sophisticated analysis tool rather than just a dumper. It makes me wonder whether this is the best tool to do this kind of analysis. On the one hand, it works for any binary with a PDB, whether it's built by clang or MSVC. On the other hand, it's not useful for clang-built binaries targeting other platforms. It almost seems like it should be done looking at the IR rather than the final debug info.

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp

109

Below, you explicitly test the results of BitVector::find_first by comparing to -1, but here it's >= 0.

[rnk]

Is it possible to write a test with an existing PDB in Inputs?

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp

81

Why fall through to break? Seems less special to break.

llvm/tools/llvm-pdbdump/LinePrinter.h

inline raw_ostream &operator<<(LinePrinter &Printer, const T &Item) {
   Printer.getStream() << Item;
   return Printer.getStream();
 }
 
 enum class PDB_ColorItem {
   None,
   Address,
   Type,
+  Comment,
+  Padding,
   Keyword,
   Offset,
   Identifier,
   Path,
   SectionHeader,
   LiteralValue,
   Register,
 };

llvm/tools/llvm-pdbdump/LinePrinter.cpp

if (UseColor)
     OS.resetColor();
 }
 
 void WithColor::applyColor(PDB_ColorItem C) {
   switch (C) {
   case PDB_ColorItem::None:
     OS.resetColor();
     return;
+  case PDB_ColorItem::Comment:
+    OS.changeColor(raw_ostream::GREEN, false);
+    return;
   case PDB_ColorItem::Address:
     OS.changeColor(raw_ostream::YELLOW, /*bold=*/true);
     return;
   case PDB_ColorItem::Keyword:
     OS.changeColor(raw_ostream::MAGENTA, true);
     return;
   case PDB_ColorItem::Register:
   case PDB_ColorItem::Offset:
     OS.changeColor(raw_ostream::YELLOW, false);
     return;
   case PDB_ColorItem::Type:
     OS.changeColor(raw_ostream::CYAN, true);
     return;
   case PDB_ColorItem::Identifier:
     OS.changeColor(raw_ostream::CYAN, false);
     return;
   case PDB_ColorItem::Path:
     OS.changeColor(raw_ostream::CYAN, false);
     return;
+  case PDB_ColorItem::Padding:
   case PDB_ColorItem::SectionHeader:
     OS.changeColor(raw_ostream::RED, true);
     return;
   case PDB_ColorItem::LiteralValue:
     OS.changeColor(raw_ostream::GREEN, true);
     return;
   }
 }

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.h

public:
   void dump(const PDBSymbolData &Symbol) override;
   void dump(const PDBSymbolTypeEnum &Symbol) override;
   void dump(const PDBSymbolFunc &Symbol) override;
   void dump(const PDBSymbolTypeTypedef &Symbol) override;
   void dump(const PDBSymbolTypeUDT &Symbol) override;
   void dump(const PDBSymbolTypeVTable &Symbol) override;
 
 private:
+  void moveToNextField(const PDBSymbolData &Data);
   LinePrinter &Printer;
+
+  uint64_t TotalPadding = 0;
+  uint64_t PaddingOccurrences = 0;
+  uint64_t FieldStart = 0;
+  uint64_t FieldEnd = 0;
 };
 }
 }
 #endif

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp

 
 #include "LinePrinter.h"
 #include "PrettyEnumDumper.h"
 #include "PrettyFunctionDumper.h"
 #include "PrettyTypedefDumper.h"
 #include "PrettyVariableDumper.h"
 #include "llvm-pdbdump.h"
 
+#include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/PDBExtras.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeBaseClass.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeVTable.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Format.h"
 
 using namespace llvm;
 using namespace llvm::pdb;
 
 ClassDefinitionDumper::ClassDefinitionDumper(LinePrinter &P)
     : PDBSymDumper(true), Printer(P) {}
 
 void ClassDefinitionDumper::start(const PDBSymbolTypeUDT &Class) {
   assert(opts::pretty::ClassFormat !=
          opts::pretty::ClassDefinitionFormat::None);
 
+  uint32_t Size = Class.getLength();
   std::string Name = Class.getName();
+  WithColor(Printer, PDB_ColorItem::Comment).get() << "// sizeof = " << Size;
+  Printer.NewLine();
+
   WithColor(Printer, PDB_ColorItem::Keyword).get() << Class.getUdtKind() << " ";
   WithColor(Printer, PDB_ColorItem::Type).get() << Class.getName();
 
   auto Bases = Class.findAllChildren<PDBSymbolTypeBaseClass>();
   if (Bases->getChildCount() > 0) {
     Printer.Indent();
     Printer.NewLine();
     Printer << ":";
if (Bases->getChildCount() > 0) {
     }
     Printer.Unindent();
   }
 
   Printer << " {";
   auto Children = Class.findAllChildren();
   Printer.Indent();
   int DumpedCount = 0;
+  bool IsLayoutMode = opts::pretty::ClassFormat ==
+                      opts::pretty::ClassDefinitionFormat::LayoutOnly;
+
   while (auto Child = Children->getNext()) {
+    bool ShowInLayoutMode = false;
 
-    if (opts::pretty::ClassFormat ==
-        opts::pretty::ClassDefinitionFormat::LayoutOnly) {
-      if (auto Data = dyn_cast<PDBSymbolData>(Child.get())) {
-        switch (Data->getLocationType()) {
-        case PDB_LocType::ThisRel:
-        case PDB_LocType::BitField:
-          break;
-        default:
-          // All other types of data field do not occupy any storage (e.g. are
-          // const),
-          // so in layout mode we skip them.
-          continue;
-        }
-      } else {
-        // Only data symbols affect record layout, so skip any non-data symbols
-        // if
-        // we're in record layout mode.
-        continue;
+        moveToNextField(*Data);
+        ShowInLayoutMode = true;
+        LLVM_FALLTHROUGH;
+        break;
       }
     }
+    if (IsLayoutMode && !ShowInLayoutMode)
+      continue;
 
     if (auto Func = dyn_cast<PDBSymbolFunc>(Child.get())) {
       if (Func->isCompilerGenerated() && opts::pretty::ExcludeCompilerGenerated)
         continue;
 
       if (Func->getLength() == 0 && !Func->isPureVirtual() &&
           !Func->isIntroVirtualFunction())
         continue;
     }
 
     ++DumpedCount;
     Child->dump(*this);
   }
 
   Printer.Unindent();
   if (DumpedCount > 0)
     Printer.NewLine();
   Printer << "}";
+  if (PaddingOccurrences > 0 && Class.getLength() > 0) {
+    APFloat Percent(100.0f * (float)TotalPadding / (float)Class.getLength());
+    SmallString<8> FloatStr;
+    Percent.toString(FloatStr);
+    Printer.NewLine();
+    WithColor(Printer, PDB_ColorItem::Padding).get()
+        << "// Padding: " << TotalPadding << " bytes in " << PaddingOccurrences
+        << " occurrences.  ";
+    WithColor(Printer, PDB_ColorItem::Padding).get()
+        << "// " << FloatStr << "% of total class size.";
+  }
+  Printer.NewLine();
+}
+
+void ClassDefinitionDumper::moveToNextField(const PDBSymbolData &Data) {
+  uint64_t Start = Data.getOffset();
+  uint64_t End = Start;
+  if (Data.getLocationType() == PDB_LocType::ThisRel) {
+    auto VarType = Data.getType();
+    uint64_t Size = VarType->getRawSymbol().getLength();
+    End += Size;
+  }
+
+  if (Start < FieldEnd) {
+    // If this field starts anywhere in the span of the current field's storage,
+    // then it must be some kind of union, so we consider it part of the same
+    // field.  In that case, extend the range of this field to cover the new
+    // field as well.
+    FieldEnd = std::max(FieldEnd, End);
+  } else {
+    // Otherwise, it's a new field.  If there is space between the current
+    // field's end and the new field's start, record padding bytes.  Only do
+    // if we're not at offset 0.  That indicates space from the base class or
+    // vtable, and we don't really want to treat that as padding.  In the future
+    // we should print out information about vtable and virtual base layout to
+    // make this more clear.
+    if (Start > FieldEnd && FieldEnd != 0) {
+      Printer.NewLine();
+      auto Padding = Start - FieldEnd;
+      ++PaddingOccurrences;
+      TotalPadding += Padding;
+      WithColor(Printer, PDB_ColorItem::Padding).get()
+          << "<padding bytes> // sizeof = " << Padding;
+    }
+    FieldStart = Start;
+    FieldEnd = End;
+  }
 }
 
 void ClassDefinitionDumper::dump(const PDBSymbolTypeBaseClass &Symbol) {}
 
 void ClassDefinitionDumper::dump(const PDBSymbolData &Symbol) {
   VariableDumper Dumper(Printer);
   Dumper.start(Symbol);
 }

llvm/tools/llvm-pdbdump/PrettyVariableDumper.cpp

 void VariableDumper::start(const PDBSymbolData &Var) {
   if (Var.isCompilerGenerated() && opts::pretty::ExcludeCompilerGenerated)
     return;
   if (Printer.IsSymbolExcluded(Var.getName()))
     return;
 
   auto VarType = Var.getType();
 
+  uint64_t Length = VarType->getRawSymbol().getLength();
+
   switch (auto LocType = Var.getLocationType()) {
   case PDB_LocType::Static:
     Printer.NewLine();
     Printer << "data [";
     WithColor(Printer, PDB_ColorItem::Address).get()
         << format_hex(Var.getVirtualAddress(), 10);
-    Printer << "] ";
+    Printer << ", sizeof=" << Length << "] ";
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "static ";
     dumpSymbolTypeAndName(*VarType, Var.getName());
     break;
   case PDB_LocType::Constant:
     if (isa<PDBSymbolTypeEnum>(*VarType))
       break;
     Printer.NewLine();
-    Printer << "data ";
+    Printer << "data [sizeof=" << Length << "] ";
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "const ";
     dumpSymbolTypeAndName(*VarType, Var.getName());
     Printer << " = ";
     WithColor(Printer, PDB_ColorItem::LiteralValue).get() << Var.getValue();
     break;
   case PDB_LocType::ThisRel:
     Printer.NewLine();
     Printer << "data ";
     WithColor(Printer, PDB_ColorItem::Offset).get()
-        << "+" << format_hex(Var.getOffset(), 4) << " ";
+        << "+" << format_hex(Var.getOffset(), 4) << " [sizeof=" << Length
+        << "] ";
     dumpSymbolTypeAndName(*VarType, Var.getName());
     break;
   case PDB_LocType::BitField:
     Printer.NewLine();
     Printer << "data ";
     WithColor(Printer, PDB_ColorItem::Offset).get()
-        << "+" << format_hex(Var.getOffset(), 4) << " ";
+        << "+" << format_hex(Var.getOffset(), 4) << " [sizeof = " << Length
+        << "] ";
     dumpSymbolTypeAndName(*VarType, Var.getName());
     Printer << " : ";
     WithColor(Printer, PDB_ColorItem::LiteralValue).get() << Var.getLength();
     break;
   default:
     Printer.NewLine();
-    Printer << "data ";
+    Printer << "data [sizeof=" << Length << "] ";
     Printer << "unknown(" << LocType << ") ";
     WithColor(Printer, PDB_ColorItem::Identifier).get() << Var.getName();
     break;
   }
 }
 
 void VariableDumper::dump(const PDBSymbolTypeBuiltin &Symbol) {
   BuiltinDumper Dumper(Printer);