From: Zachary Turner <zturner@google.com>
Date: Mon, 24 Apr 2017 17:47:24 +0000 (+0000)
Subject: [llvm-pdbdump] Re-write the record layout code to be more resilient.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0e8b7073e9ec0d43329b10e31a9b38d62f51119f;p=llvm

[llvm-pdbdump] Re-write the record layout code to be more resilient.

This reworks the way virtual bases are handled, and also the way
padding is detected across multiple levels of aggregates, producing
a much more accurate result.

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

diff --git a/include/llvm/DebugInfo/PDB/DIA/DIARawSymbol.h b/include/llvm/DebugInfo/PDB/DIA/DIARawSymbol.h
index c0633cbdfa5..3710eb29e7f 100644
--- a/include/llvm/DebugInfo/PDB/DIA/DIARawSymbol.h
+++ b/include/llvm/DebugInfo/PDB/DIA/DIARawSymbol.h
@@ -102,7 +102,8 @@ public:
   uint32_t getVirtualBaseDispIndex() const override;
   uint32_t getVirtualBaseOffset() const override;
   uint32_t getVirtualTableShapeId() const override;
-  std::unique_ptr<PDBSymbolTypeVTable> getVirtualBaseTableType() const override;
+  std::unique_ptr<PDBSymbolTypeBuiltin>
+  getVirtualBaseTableType() const override;
   PDB_DataKind getDataKind() const override;
   PDB_SymType getSymTag() const override;
   PDB_UniqueId getGuid() const override;
diff --git a/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h b/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h
index 4c28e194bc7..fab086c62c7 100644
--- a/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h
+++ b/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h
@@ -113,7 +113,7 @@ public:
   virtual Variant getValue() const = 0;
   virtual uint32_t getVirtualBaseDispIndex() const = 0;
   virtual uint32_t getVirtualBaseOffset() const = 0;
-  virtual std::unique_ptr<PDBSymbolTypeVTable>
+  virtual std::unique_ptr<PDBSymbolTypeBuiltin>
   getVirtualBaseTableType() const = 0;
   virtual uint32_t getVirtualTableShapeId() const = 0;
   virtual PDB_DataKind getDataKind() const = 0;
diff --git a/include/llvm/DebugInfo/PDB/Native/NativeRawSymbol.h b/include/llvm/DebugInfo/PDB/Native/NativeRawSymbol.h
index cffb5d09d22..e1e78035ff3 100644
--- a/include/llvm/DebugInfo/PDB/Native/NativeRawSymbol.h
+++ b/include/llvm/DebugInfo/PDB/Native/NativeRawSymbol.h
@@ -101,7 +101,8 @@ public:
   uint32_t getVirtualBaseDispIndex() const override;
   uint32_t getVirtualBaseOffset() const override;
   uint32_t getVirtualTableShapeId() const override;
-  std::unique_ptr<PDBSymbolTypeVTable> getVirtualBaseTableType() const override;
+  std::unique_ptr<PDBSymbolTypeBuiltin>
+  getVirtualBaseTableType() const override;
   PDB_DataKind getDataKind() const override;
   PDB_SymType getSymTag() const override;
   PDB_UniqueId getGuid() const override;
diff --git a/include/llvm/DebugInfo/PDB/UDTLayout.h b/include/llvm/DebugInfo/PDB/UDTLayout.h
index e3dcba50bd1..8283113cfe1 100644
--- a/include/llvm/DebugInfo/PDB/UDTLayout.h
+++ b/include/llvm/DebugInfo/PDB/UDTLayout.h
@@ -15,6 +15,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/DenseMap.h"
 
 #include <list>
 #include <memory>
@@ -32,40 +33,62 @@ class PDBSymbolTypeVTable;
 
 class ClassLayout;
 class BaseClassLayout;
-class StorageItemBase;
+class LayoutItemBase;
 class UDTLayoutBase;
 
-class StorageItemBase {
+class LayoutItemBase {
 public:
-  StorageItemBase(const UDTLayoutBase &Parent, const PDBSymbol &Symbol,
-                  const std::string &Name, uint32_t OffsetInParent,
-                  uint32_t Size);
-  virtual ~StorageItemBase() {}
+  LayoutItemBase(const UDTLayoutBase *Parent, const PDBSymbol *Symbol,
+                 const std::string &Name, uint32_t OffsetInParent,
+                 uint32_t Size, bool IsElided);
+  virtual ~LayoutItemBase() {}
 
-  virtual uint32_t deepPaddingSize() const;
+  uint32_t deepPaddingSize() const;
+  virtual uint32_t tailPadding() const;
 
-  const UDTLayoutBase &getParent() const { return Parent; }
+  const UDTLayoutBase *getParent() const { return Parent; }
   StringRef getName() const { return Name; }
   uint32_t getOffsetInParent() const { return OffsetInParent; }
   uint32_t getSize() const { return SizeOf; }
-  const PDBSymbol &getSymbol() const { return Symbol; }
+  uint32_t getLayoutSize() const { return LayoutSize; }
+  const PDBSymbol *getSymbol() const { return Symbol; }
+  const BitVector &usedBytes() const { return UsedBytes; }
+  bool isElided() const { return IsElided; }
+  virtual bool isVBPtr() const { return false; }
+
+  uint32_t containsOffset(uint32_t Off) const {
+    uint32_t Begin = getOffsetInParent();
+    uint32_t End = Begin + getSize();
+    return (Off >= Begin && Off < End);
+  }
 
 protected:
-  const UDTLayoutBase &Parent;
-  const PDBSymbol &Symbol;
+  const PDBSymbol *Symbol = nullptr;
+  const UDTLayoutBase *Parent = nullptr;
   BitVector UsedBytes;
   std::string Name;
   uint32_t OffsetInParent = 0;
   uint32_t SizeOf = 0;
+  uint32_t LayoutSize = 0;
+  bool IsElided = false;
+};
+
+class VBPtrLayoutItem : public LayoutItemBase {
+public:
+  VBPtrLayoutItem(const UDTLayoutBase &Parent,
+                  std::unique_ptr<PDBSymbolTypeBuiltin> Sym, uint32_t Offset,
+                  uint32_t Size);
+  virtual bool isVBPtr() const { return true; }
+
+private:
+  std::unique_ptr<PDBSymbolTypeBuiltin> Type;
 };
 
-class DataMemberLayoutItem : public StorageItemBase {
+class DataMemberLayoutItem : public LayoutItemBase {
 public:
   DataMemberLayoutItem(const UDTLayoutBase &Parent,
                        std::unique_ptr<PDBSymbolData> DataMember);
 
-  virtual uint32_t deepPaddingSize() const;
-
   const PDBSymbolData &getDataMember();
   bool hasUDTLayout() const;
   const ClassLayout &getUDTLayout() const;
@@ -75,77 +98,73 @@ private:
   std::unique_ptr<ClassLayout> UdtLayout;
 };
 
-class VTableLayoutItem : public StorageItemBase {
+class VTableLayoutItem : public LayoutItemBase {
 public:
   VTableLayoutItem(const UDTLayoutBase &Parent,
                    std::unique_ptr<PDBSymbolTypeVTable> VTable);
-  ArrayRef<PDBSymbolFunc *> funcs() const { return VTableFuncs; }
 
   uint32_t getElementSize() const { return ElementSize; }
 
-  void setFunction(uint32_t Index, PDBSymbolFunc &Func) {
-    VTableFuncs[Index] = &Func;
-  }
-
 private:
   uint32_t ElementSize = 0;
-  std::unique_ptr<PDBSymbolTypeVTableShape> Shape;
   std::unique_ptr<PDBSymbolTypeVTable> VTable;
-  std::vector<PDBSymbolFunc *> VTableFuncs;
 };
 
-class UDTLayoutBase {
+class UDTLayoutBase : public LayoutItemBase {
   template <typename T> using UniquePtrVector = std::vector<std::unique_ptr<T>>;
 
 public:
-  UDTLayoutBase(const PDBSymbol &Symbol, const std::string &Name,
-                uint32_t Size);
-
-  uint32_t shallowPaddingSize() const;
-  uint32_t deepPaddingSize() const;
-
-  const BitVector &usedBytes() const { return UsedBytes; }
+  UDTLayoutBase(const UDTLayoutBase *Parent, const PDBSymbol &Sym,
+                const std::string &Name, uint32_t OffsetInParent, uint32_t Size,
+                bool IsElided);
 
-  uint32_t getClassSize() const { return SizeOf; }
+  uint32_t tailPadding() const override;
 
-  ArrayRef<std::unique_ptr<StorageItemBase>> layout_items() const {
-    return ChildStorage;
-  }
-
-  VTableLayoutItem *findVTableAtOffset(uint32_t RelativeOffset);
+  ArrayRef<LayoutItemBase *> layout_items() const { return LayoutItems; }
 
-  StringRef getUDTName() const { return Name; }
+  ArrayRef<BaseClassLayout *> bases() const { return AllBases; }
+  ArrayRef<BaseClassLayout *> regular_bases() const { return NonVirtualBases; }
+  ArrayRef<BaseClassLayout *> virtual_bases() const { return VirtualBases; }
 
-  ArrayRef<BaseClassLayout *> bases() const { return BaseClasses; }
-  ArrayRef<std::unique_ptr<PDBSymbolTypeBaseClass>> vbases() const {
-    return VirtualBases;
-  }
+  uint32_t directVirtualBaseCount() const { return DirectVBaseCount; }
 
   ArrayRef<std::unique_ptr<PDBSymbolFunc>> funcs() const { return Funcs; }
 
   ArrayRef<std::unique_ptr<PDBSymbol>> other_items() const { return Other; }
 
-  const PDBSymbol &getSymbolBase() const { return SymbolBase; }
-
 protected:
+  bool hasVBPtrAtOffset(uint32_t Off) const;
   void initializeChildren(const PDBSymbol &Sym);
 
-  void addChildToLayout(std::unique_ptr<StorageItemBase> Child);
-  void addVirtualOverride(PDBSymbolFunc &Func);
-  void addVirtualIntro(PDBSymbolFunc &Func);
+  void addChildToLayout(std::unique_ptr<LayoutItemBase> Child);
 
-  const PDBSymbol &SymbolBase;
-  std::string Name;
-  uint32_t SizeOf = 0;
+  uint32_t DirectVBaseCount = 0;
 
-  BitVector UsedBytes;
   UniquePtrVector<PDBSymbol> Other;
   UniquePtrVector<PDBSymbolFunc> Funcs;
-  UniquePtrVector<PDBSymbolTypeBaseClass> VirtualBases;
-  UniquePtrVector<StorageItemBase> ChildStorage;
-  std::vector<std::list<StorageItemBase *>> ChildrenPerByte;
-  std::vector<BaseClassLayout *> BaseClasses;
+  UniquePtrVector<LayoutItemBase> ChildStorage;
+  std::vector<LayoutItemBase *> LayoutItems;
+
+  std::vector<BaseClassLayout *> AllBases;
+  ArrayRef<BaseClassLayout *> NonVirtualBases;
+  ArrayRef<BaseClassLayout *> VirtualBases;
+
   VTableLayoutItem *VTable = nullptr;
+  VBPtrLayoutItem *VBPtr = nullptr;
+};
+
+class BaseClassLayout : public UDTLayoutBase {
+public:
+  BaseClassLayout(const UDTLayoutBase &Parent, uint32_t OffsetInParent,
+                  bool Elide, std::unique_ptr<PDBSymbolTypeBaseClass> Base);
+
+  const PDBSymbolTypeBaseClass &getBase() const { return *Base; }
+  bool isVirtualBase() const { return IsVirtualBase; }
+  bool isEmptyBase() { return SizeOf == 1 && LayoutSize == 0; }
+
+private:
+  std::unique_ptr<PDBSymbolTypeBaseClass> Base;
+  bool IsVirtualBase;
 };
 
 class ClassLayout : public UDTLayoutBase {
@@ -161,19 +180,6 @@ private:
   std::unique_ptr<PDBSymbolTypeUDT> OwnedStorage;
   const PDBSymbolTypeUDT &UDT;
 };
-
-class BaseClassLayout : public UDTLayoutBase, public StorageItemBase {
-public:
-  BaseClassLayout(const UDTLayoutBase &Parent,
-                  std::unique_ptr<PDBSymbolTypeBaseClass> Base);
-
-  const PDBSymbolTypeBaseClass &getBase() const { return *Base; }
-  bool isVirtualBase() const { return IsVirtualBase; }
-
-private:
-  std::unique_ptr<PDBSymbolTypeBaseClass> Base;
-  bool IsVirtualBase;
-};
 }
 } // namespace llvm
 
diff --git a/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp b/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp
index 5e8c0bdc171..4e2474c51cb 100644
--- a/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp
+++ b/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp
@@ -14,6 +14,7 @@
 #include "llvm/DebugInfo/PDB/DIA/DIAEnumSymbols.h"
 #include "llvm/DebugInfo/PDB/DIA/DIASession.h"
 #include "llvm/DebugInfo/PDB/PDBExtras.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeVTable.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeVTableShape.h"
@@ -720,7 +721,7 @@ uint32_t DIARawSymbol::getVirtualTableShapeId() const {
   return PrivateGetDIAValue(Symbol, &IDiaSymbol::get_virtualTableShapeId);
 }
 
-std::unique_ptr<PDBSymbolTypeVTable>
+std::unique_ptr<PDBSymbolTypeBuiltin>
 DIARawSymbol::getVirtualBaseTableType() const {
   CComPtr<IDiaSymbol> TableType;
   if (FAILED(Symbol->get_virtualBaseTableType(&TableType)) || !TableType)
@@ -729,7 +730,7 @@ DIARawSymbol::getVirtualBaseTableType() const {
   auto RawVT = llvm::make_unique<DIARawSymbol>(Session, TableType);
   auto Pointer =
       llvm::make_unique<PDBSymbolTypePointer>(Session, std::move(RawVT));
-  return unique_dyn_cast<PDBSymbolTypeVTable>(Pointer->getPointeeType());
+  return unique_dyn_cast<PDBSymbolTypeBuiltin>(Pointer->getPointeeType());
 }
 
 PDB_DataKind DIARawSymbol::getDataKind() const {
diff --git a/lib/DebugInfo/PDB/DIA/DIASession.cpp b/lib/DebugInfo/PDB/DIA/DIASession.cpp
index 6ecf335812b..ef47b92b4f2 100644
--- a/lib/DebugInfo/PDB/DIA/DIASession.cpp
+++ b/lib/DebugInfo/PDB/DIA/DIASession.cpp
@@ -21,12 +21,22 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::pdb;
 
-static Error ErrorFromHResult(HRESULT Result, StringRef Context) {
+template <typename... Ts>
+static Error ErrorFromHResult(HRESULT Result, const char *Str, Ts &&... Args) {
+  SmallString<64> MessageStorage;
+  StringRef Context;
+  if (sizeof...(Args) > 0) {
+    MessageStorage = formatv(Str, std::forward<Ts>(Args)...).str();
+    Context = MessageStorage;
+  } else
+    Context = Str;
+
   switch (Result) {
   case E_PDB_NOT_FOUND:
     return make_error<GenericError>(generic_error_code::invalid_path, Context);
@@ -95,8 +105,9 @@ Error DIASession::createFromPdb(StringRef Path,
 
   const wchar_t *Path16Str = reinterpret_cast<const wchar_t*>(Path16.data());
   HRESULT HR;
-  if (FAILED(HR = DiaDataSource->loadDataFromPdb(Path16Str)))
-    return ErrorFromHResult(HR, "Calling loadDataFromPdb");
+  if (FAILED(HR = DiaDataSource->loadDataFromPdb(Path16Str))) {
+    return ErrorFromHResult(HR, "Calling loadDataFromPdb {0}", Path);
+  }
 
   if (FAILED(HR = DiaDataSource->openSession(&DiaSession)))
     return ErrorFromHResult(HR, "Calling openSession");
diff --git a/lib/DebugInfo/PDB/Native/NativeRawSymbol.cpp b/lib/DebugInfo/PDB/Native/NativeRawSymbol.cpp
index 3aba35adb53..70968d4330b 100644
--- a/lib/DebugInfo/PDB/Native/NativeRawSymbol.cpp
+++ b/lib/DebugInfo/PDB/Native/NativeRawSymbol.cpp
@@ -13,6 +13,7 @@
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/Native/NativeSession.h"
 #include "llvm/DebugInfo/PDB/PDBExtras.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeVTable.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeVTableShape.h"
 #include "llvm/Support/ConvertUTF.h"
@@ -320,7 +321,7 @@ uint32_t NativeRawSymbol::getVirtualTableShapeId() const {
   return 0;
 }
 
-std::unique_ptr<PDBSymbolTypeVTable>
+std::unique_ptr<PDBSymbolTypeBuiltin>
 NativeRawSymbol::getVirtualBaseTableType() const {
   return nullptr;
 }
diff --git a/lib/DebugInfo/PDB/UDTLayout.cpp b/lib/DebugInfo/PDB/UDTLayout.cpp
index 61cef093d4c..6c6bb26ad1b 100644
--- a/lib/DebugInfo/PDB/UDTLayout.cpp
+++ b/lib/DebugInfo/PDB/UDTLayout.cpp
@@ -16,6 +16,7 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeBaseClass.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeVTable.h"
@@ -39,36 +40,48 @@ static uint32_t getTypeLength(const PDBSymbol &Symbol) {
   return RawType.getLength();
 }
 
-StorageItemBase::StorageItemBase(const UDTLayoutBase &Parent,
-                                 const PDBSymbol &Symbol,
-                                 const std::string &Name,
-                                 uint32_t OffsetInParent, uint32_t Size)
-    : Parent(Parent), Symbol(Symbol), Name(Name),
-      OffsetInParent(OffsetInParent), SizeOf(Size) {
+LayoutItemBase::LayoutItemBase(const UDTLayoutBase *Parent,
+                               const PDBSymbol *Symbol, const std::string &Name,
+                               uint32_t OffsetInParent, uint32_t Size,
+                               bool IsElided)
+    : Symbol(Symbol), Parent(Parent), Name(Name),
+      OffsetInParent(OffsetInParent), SizeOf(Size), LayoutSize(Size),
+      IsElided(IsElided) {
   UsedBytes.resize(SizeOf, true);
 }
 
-uint32_t StorageItemBase::deepPaddingSize() const {
-  // sizeof(Field) - sizeof(typeof(Field)) is trailing padding.
-  return SizeOf - getTypeLength(Symbol);
+uint32_t LayoutItemBase::deepPaddingSize() const {
+  return UsedBytes.size() - UsedBytes.count();
+}
+
+uint32_t LayoutItemBase::tailPadding() const {
+  int Last = UsedBytes.find_last();
+
+  return UsedBytes.size() - (Last + 1);
 }
 
+
 DataMemberLayoutItem::DataMemberLayoutItem(
-    const UDTLayoutBase &Parent, std::unique_ptr<PDBSymbolData> DataMember)
-    : StorageItemBase(Parent, *DataMember, DataMember->getName(),
-                      DataMember->getOffset(), getTypeLength(*DataMember)),
-      DataMember(std::move(DataMember)) {
-  auto Type = this->DataMember->getType();
+    const UDTLayoutBase &Parent, std::unique_ptr<PDBSymbolData> Member)
+    : LayoutItemBase(&Parent, Member.get(), Member->getName(),
+                     Member->getOffset(), getTypeLength(*Member), false),
+      DataMember(std::move(Member)) {
+  auto Type = DataMember->getType();
   if (auto UDT = unique_dyn_cast<PDBSymbolTypeUDT>(Type)) {
-    // UDT data members might have padding in between fields, but otherwise
-    // a member should occupy its entire storage.
-    UsedBytes.resize(SizeOf, false);
     UdtLayout = llvm::make_unique<ClassLayout>(std::move(UDT));
+    UsedBytes = UdtLayout->usedBytes();
   }
 }
 
+VBPtrLayoutItem::VBPtrLayoutItem(const UDTLayoutBase &Parent,
+                                 std::unique_ptr<PDBSymbolTypeBuiltin> Sym,
+                                 uint32_t Offset, uint32_t Size)
+    : LayoutItemBase(&Parent, Sym.get(), "<vbptr>", Offset, Size, false),
+      Type(std::move(Sym)) {
+}
+
 const PDBSymbolData &DataMemberLayoutItem::getDataMember() {
-  return *dyn_cast<PDBSymbolData>(&Symbol);
+  return *dyn_cast<PDBSymbolData>(Symbol);
 }
 
 bool DataMemberLayoutItem::hasUDTLayout() const { return UdtLayout != nullptr; }
@@ -77,36 +90,43 @@ const ClassLayout &DataMemberLayoutItem::getUDTLayout() const {
   return *UdtLayout;
 }
 
-uint32_t DataMemberLayoutItem::deepPaddingSize() const {
-  uint32_t Result = StorageItemBase::deepPaddingSize();
-  if (UdtLayout)
-    Result += UdtLayout->deepPaddingSize();
-  return Result;
-}
-
 VTableLayoutItem::VTableLayoutItem(const UDTLayoutBase &Parent,
-                                   std::unique_ptr<PDBSymbolTypeVTable> VTable)
-    : StorageItemBase(Parent, *VTable, "<vtbl>", 0, getTypeLength(*VTable)),
-      VTable(std::move(VTable)) {
-  auto VTableType = cast<PDBSymbolTypePointer>(this->VTable->getType());
+                                   std::unique_ptr<PDBSymbolTypeVTable> VT)
+    : LayoutItemBase(&Parent, VT.get(), "<vtbl>", 0, getTypeLength(*VT), false),
+      VTable(std::move(VT)) {
+  auto VTableType = cast<PDBSymbolTypePointer>(VTable->getType());
   ElementSize = VTableType->getLength();
+}
 
-  Shape =
-      unique_dyn_cast<PDBSymbolTypeVTableShape>(VTableType->getPointeeType());
-  if (Shape)
-    VTableFuncs.resize(Shape->getCount());
+UDTLayoutBase::UDTLayoutBase(const UDTLayoutBase *Parent, const PDBSymbol &Sym,
+                             const std::string &Name, uint32_t OffsetInParent,
+                             uint32_t Size, bool IsElided)
+    : LayoutItemBase(Parent, &Sym, Name, OffsetInParent, Size, IsElided) {
+  // UDT storage comes from a union of all the children's storage, so start out
+  // uninitialized.
+  UsedBytes.reset(0, Size);
+
+  initializeChildren(Sym);
+  if (LayoutSize < Size)
+    UsedBytes.resize(LayoutSize);
 }
 
-UDTLayoutBase::UDTLayoutBase(const PDBSymbol &Symbol, const std::string &Name,
-                             uint32_t Size)
-    : SymbolBase(Symbol), Name(Name), SizeOf(Size) {
-  UsedBytes.resize(Size);
-  ChildrenPerByte.resize(Size);
-  initializeChildren(Symbol);
+uint32_t UDTLayoutBase::tailPadding() const {
+  uint32_t Abs = LayoutItemBase::tailPadding();
+  if (!LayoutItems.empty()) {
+    const LayoutItemBase *Back = LayoutItems.back();
+    uint32_t ChildPadding = Back->LayoutItemBase::tailPadding();
+    if (Abs < ChildPadding)
+      Abs = 0;
+    else
+      Abs -= ChildPadding;
+  }
+  return Abs;
 }
 
 ClassLayout::ClassLayout(const PDBSymbolTypeUDT &UDT)
-    : UDTLayoutBase(UDT, UDT.getName(), UDT.getLength()), UDT(UDT) {}
+    : UDTLayoutBase(nullptr, UDT, UDT.getName(), 0, UDT.getLength(), false),
+      UDT(UDT) {}
 
 ClassLayout::ClassLayout(std::unique_ptr<PDBSymbolTypeUDT> UDT)
     : ClassLayout(*UDT) {
@@ -114,23 +134,17 @@ ClassLayout::ClassLayout(std::unique_ptr<PDBSymbolTypeUDT> UDT)
 }
 
 BaseClassLayout::BaseClassLayout(const UDTLayoutBase &Parent,
-                                 std::unique_ptr<PDBSymbolTypeBaseClass> Base)
-    : UDTLayoutBase(*Base, Base->getName(), Base->getLength()),
-      StorageItemBase(Parent, *Base, Base->getName(), Base->getOffset(),
-                      Base->getLength()),
-      Base(std::move(Base)) {
-  IsVirtualBase = this->Base->isVirtualBaseClass();
-}
-
-uint32_t UDTLayoutBase::shallowPaddingSize() const {
-  return UsedBytes.size() - UsedBytes.count();
-}
-
-uint32_t UDTLayoutBase::deepPaddingSize() const {
-  uint32_t Result = shallowPaddingSize();
-  for (auto &Child : ChildStorage)
-    Result += Child->deepPaddingSize();
-  return Result;
+                                 uint32_t OffsetInParent, bool Elide,
+                                 std::unique_ptr<PDBSymbolTypeBaseClass> B)
+    : UDTLayoutBase(&Parent, *B, B->getName(), OffsetInParent, B->getLength(),
+                    Elide),
+      Base(std::move(B)) {
+  if (isEmptyBase()) {
+    // Special case an empty base so that it doesn't get treated as padding.
+    UsedBytes.resize(1);
+    UsedBytes.set(0);
+  }
+  IsVirtualBase = Base->isVirtualBaseClass();
 }
 
 void UDTLayoutBase::initializeChildren(const PDBSymbol &Sym) {
@@ -138,15 +152,16 @@ void UDTLayoutBase::initializeChildren(const PDBSymbol &Sym) {
   // followed by functions, followed by other.  This ordering is necessary
   // so that bases and vtables get initialized before any functions which
   // may override them.
-
   UniquePtrVector<PDBSymbolTypeBaseClass> Bases;
   UniquePtrVector<PDBSymbolTypeVTable> VTables;
   UniquePtrVector<PDBSymbolData> Members;
+  UniquePtrVector<PDBSymbolTypeBaseClass> VirtualBaseSyms;
+
   auto Children = Sym.findAllChildren();
   while (auto Child = Children->getNext()) {
     if (auto Base = unique_dyn_cast<PDBSymbolTypeBaseClass>(Child)) {
       if (Base->isVirtualBaseClass())
-        VirtualBases.push_back(std::move(Base));
+        VirtualBaseSyms.push_back(std::move(Base));
       else
         Bases.push_back(std::move(Base));
     }
@@ -164,20 +179,33 @@ void UDTLayoutBase::initializeChildren(const PDBSymbol &Sym) {
       Other.push_back(std::move(Child));
   }
 
+  // We don't want to have any re-allocations in the list of bases, so make
+  // sure to reserve enough space so that our ArrayRefs don't get invalidated.
+  AllBases.reserve(Bases.size() + VirtualBaseSyms.size());
+
+  // Only add non-virtual bases to the class first.  Only at the end of the
+  // class, after all non-virtual bases and data members have been added do we
+  // add virtual bases.  This way the offsets are correctly aligned when we go
+  // to lay out virtual bases.
   for (auto &Base : Bases) {
-    auto BL = llvm::make_unique<BaseClassLayout>(*this, std::move(Base));
-    BaseClasses.push_back(BL.get());
+    uint32_t Offset = Base->getOffset();
+    // Non-virtual bases never get elided.
+    auto BL = llvm::make_unique<BaseClassLayout>(*this, Offset, false,
+                                                 std::move(Base));
 
+    AllBases.push_back(BL.get());
     addChildToLayout(std::move(BL));
   }
+  NonVirtualBases = AllBases;
 
-  for (auto &VT : VTables) {
-    auto VTLayout = llvm::make_unique<VTableLayoutItem>(*this, std::move(VT));
+  assert(VTables.size() <= 1);
+  if (!VTables.empty()) {
+    auto VTLayout =
+        llvm::make_unique<VTableLayoutItem>(*this, std::move(VTables[0]));
 
     VTable = VTLayout.get();
 
     addChildToLayout(std::move(VTLayout));
-    continue;
   }
 
   for (auto &Data : Members) {
@@ -186,150 +214,74 @@ void UDTLayoutBase::initializeChildren(const PDBSymbol &Sym) {
     addChildToLayout(std::move(DM));
   }
 
-  for (auto &Func : Funcs) {
-    if (!Func->isVirtual())
-      continue;
+  // Make sure add virtual bases before adding functions, since functions may be
+  // overrides of virtual functions declared in a virtual base, so the VTables
+  // and virtual intros need to be correctly initialized.
+  for (auto &VB : VirtualBaseSyms) {
+    int VBPO = VB->getVirtualBasePointerOffset();
+    if (!hasVBPtrAtOffset(VBPO)) {
+      if (auto VBP = VB->getRawSymbol().getVirtualBaseTableType()) {
+        auto VBPL = llvm::make_unique<VBPtrLayoutItem>(*this, std::move(VBP),
+                                                       VBPO, VBP->getLength());
+        VBPtr = VBPL.get();
+        addChildToLayout(std::move(VBPL));
+      }
+    }
 
-    if (Func->isIntroVirtualFunction())
-      addVirtualIntro(*Func);
-    else
-      addVirtualOverride(*Func);
+    // Virtual bases always go at the end.  So just look for the last place we
+    // ended when writing something, and put our virtual base there.
+    // Note that virtual bases get elided unless this is a top-most derived
+    // class.
+    uint32_t Offset = UsedBytes.find_last() + 1;
+    bool Elide = (Parent != nullptr);
+    auto BL =
+        llvm::make_unique<BaseClassLayout>(*this, Offset, Elide, std::move(VB));
+    AllBases.push_back(BL.get());
+
+    // Only lay this virtual base out directly inside of *this* class if this
+    // is a top-most derived class.  Keep track of it regardless, but only
+    // physically lay it out if it's a topmost derived class.
+    addChildToLayout(std::move(BL));
   }
+  VirtualBases = makeArrayRef(AllBases).drop_front(NonVirtualBases.size());
+
+  if (Parent != nullptr)
+    LayoutSize = UsedBytes.find_last() + 1;
 }
 
-void UDTLayoutBase::addVirtualIntro(PDBSymbolFunc &Func) {
-  // Kind of a hack, but we prefer the more common destructor name that people
-  // are familiar with, e.g. ~ClassName.  It seems there are always both and
-  // the vector deleting destructor overwrites the nice destructor, so just
-  // ignore the vector deleting destructor.
-  if (Func.getName() == "__vecDelDtor")
-    return;
-
-  if (!VTable) {
-    // FIXME: Handle this.  What's most likely happening is we have an intro
-    // virtual in a derived class where the base also has an intro virtual.
-    // In this case the vtable lives in the base.  What we really need is
-    // for each UDTLayoutBase to contain a list of all its vtables, and
-    // then propagate this list up the hierarchy so that derived classes have
-    // direct access to their bases' vtables.
-    return;
+bool UDTLayoutBase::hasVBPtrAtOffset(uint32_t Off) const {
+  if (VBPtr && VBPtr->getOffsetInParent() == Off)
+    return true;
+  for (BaseClassLayout *BL : AllBases) {
+    if (BL->hasVBPtrAtOffset(Off - BL->getOffsetInParent()))
+      return true;
   }
-
-  uint32_t Stride = VTable->getElementSize();
-
-  uint32_t Index = Func.getVirtualBaseOffset();
-  assert(Index % Stride == 0);
-  Index /= Stride;
-
-  VTable->setFunction(Index, Func);
+  return false;
 }
 
-VTableLayoutItem *UDTLayoutBase::findVTableAtOffset(uint32_t RelativeOffset) {
-  if (VTable && VTable->getOffsetInParent() == RelativeOffset)
-    return VTable;
-  for (auto Base : BaseClasses) {
-    uint32_t Begin = Base->getOffsetInParent();
-    uint32_t End = Begin + Base->getSize();
-    if (RelativeOffset < Begin || RelativeOffset >= End)
-      continue;
-
-    return Base->findVTableAtOffset(RelativeOffset - Begin);
-  }
+void UDTLayoutBase::addChildToLayout(std::unique_ptr<LayoutItemBase> Child) {
+  uint32_t Begin = Child->getOffsetInParent();
 
-  return nullptr;
-}
+  if (!Child->isElided()) {
+    BitVector ChildBytes = Child->usedBytes();
 
-void UDTLayoutBase::addVirtualOverride(PDBSymbolFunc &Func) {
-  auto Signature = Func.getSignature();
-  auto ThisAdjust = Signature->getThisAdjust();
-  // ThisAdjust tells us which VTable we're looking for.  Specifically, it's
-  // the offset into the current class of the VTable we're looking for.  So
-  // look through the base hierarchy until we find one such that
-  // AbsoluteOffset(VT) == ThisAdjust
-  VTableLayoutItem *VT = findVTableAtOffset(ThisAdjust);
-  if (!VT) {
-    // FIXME: There really should be a vtable here.  If there's not it probably
-    // means that the vtable is in a virtual base, which we don't yet support.
-    assert(!VirtualBases.empty());
-    return;
-  }
-  int32_t OverrideIndex = -1;
-  // Now we've found the VTable.  Func will not have a virtual base offset set,
-  // so instead we need to compare names and signatures.  We iterate each item
-  // in the VTable.  All items should already have non null entries because they
-  // were initialized by the intro virtual, which was guaranteed to come before.
-  for (auto ItemAndIndex : enumerate(VT->funcs())) {
-    auto Item = ItemAndIndex.value();
-    assert(Item);
-    // If the name doesn't match, this isn't an override.  Note that it's ok
-    // for the return type to not match (e.g. co-variant return).
-    if (Item->getName() != Func.getName()) {
-      if (Item->isDestructor() && Func.isDestructor()) {
-        OverrideIndex = ItemAndIndex.index();
-        break;
-      }
-      continue;
-    }
-    // Now make sure it's the right overload.  Get the signature of the existing
-    // vtable method and make sure it has the same arglist and the same cv-ness.
-    auto ExistingSig = Item->getSignature();
-    if (ExistingSig->isConstType() != Signature->isConstType())
-      continue;
-    if (ExistingSig->isVolatileType() != Signature->isVolatileType())
-      continue;
-
-    // Now compare arguments.  Using the raw bytes of the PDB this would be
-    // trivial
-    // because there is an ArgListId and they should be identical.  But DIA
-    // doesn't
-    // expose this, so the best we can do is iterate each argument and confirm
-    // that
-    // each one is identical.
-    if (ExistingSig->getCount() != Signature->getCount())
-      continue;
-    bool IsMatch = true;
-    auto ExistingEnumerator = ExistingSig->getArguments();
-    auto NewEnumerator = Signature->getArguments();
-    for (uint32_t I = 0; I < ExistingEnumerator->getChildCount(); ++I) {
-      auto ExistingArg = ExistingEnumerator->getNext();
-      auto NewArg = NewEnumerator->getNext();
-      if (ExistingArg->getSymIndexId() != NewArg->getSymIndexId()) {
-        IsMatch = false;
-        break;
-      }
-    }
-    if (!IsMatch)
-      continue;
+    // Suppose the child occupies 4 bytes starting at offset 12 in a 32 byte
+    // class.  When we call ChildBytes.resize(32), the Child's storage will
+    // still begin at offset 0, so we need to shift it left by offset bytes
+    // to get it into the right position.
+    ChildBytes.resize(UsedBytes.size());
+    ChildBytes <<= Child->getOffsetInParent();
+    UsedBytes |= ChildBytes;
 
-    // It's a match!  Stick the new function into the VTable.
-    OverrideIndex = ItemAndIndex.index();
-    break;
-  }
-  if (OverrideIndex == -1) {
-    // FIXME: This is probably due to one of the other FIXMEs in this file.
-    return;
-  }
-  VT->setFunction(OverrideIndex, Func);
-}
+    if (ChildBytes.count() > 0) {
+      auto Loc = std::upper_bound(LayoutItems.begin(), LayoutItems.end(), Begin,
+                                  [](uint32_t Off, const LayoutItemBase *Item) {
+                                    return (Off < Item->getOffsetInParent());
+                                  });
 
-void UDTLayoutBase::addChildToLayout(std::unique_ptr<StorageItemBase> Child) {
-  uint32_t Begin = Child->getOffsetInParent();
-  uint32_t End = Begin + Child->getSize();
-  // Due to the empty base optimization, End might point outside the bounds of
-  // the parent class.  If that happens, just clamp the value.
-  End = std::min(End, getClassSize());
-
-  UsedBytes.set(Begin, End);
-  while (Begin != End) {
-    ChildrenPerByte[Begin].push_back(Child.get());
-    ++Begin;
+      LayoutItems.insert(Loc, Child.get());
+    }
   }
 
-  auto Loc = std::upper_bound(
-      ChildStorage.begin(), ChildStorage.end(), Begin,
-      [](uint32_t Off, const std::unique_ptr<StorageItemBase> &Item) {
-        return Off < Item->getOffsetInParent();
-      });
-
-  ChildStorage.insert(Loc, std::move(Child));
+  ChildStorage.push_back(std::move(Child));
 }
\ No newline at end of file
diff --git a/test/tools/llvm-pdbdump/Inputs/ComplexPaddingTest.cpp b/test/tools/llvm-pdbdump/Inputs/ComplexPaddingTest.cpp
new file mode 100644
index 00000000000..44a4e304ccb
--- /dev/null
+++ b/test/tools/llvm-pdbdump/Inputs/ComplexPaddingTest.cpp
@@ -0,0 +1,48 @@
+// Compile with "cl /c /Zi /GR- ComplexPaddingTest.cpp"
+// Link with "link ComplexPaddingTest.obj /debug /nodefaultlib /entry:main"
+
+#include <stdint.h>
+
+extern "C" using at_exit_handler = void();
+
+int atexit(at_exit_handler handler) { return 0; }
+
+struct TestVB {
+  static void operator delete(void *ptr, size_t sz) {}
+  virtual ~TestVB() {}
+  virtual void IntroFunction1() {}
+  int X;
+} A;
+
+struct TestNVB {
+  static void operator delete(void *ptr, size_t sz) {}
+  virtual ~TestNVB() {}
+  virtual void IntroFunction2() {}
+  int Y;
+} B;
+
+struct TestVBLayout
+    : public virtual TestVB,
+      public TestNVB {
+  static void operator delete(void *ptr, size_t sz) {}
+  int Z;
+} C;
+
+struct TestIVBBase : public virtual TestVB {
+  int A;
+} D;
+
+struct TestIVBDerived : public TestIVBBase {
+  int B;
+} E;
+
+struct TestIVBMergedDerived
+    : public virtual TestVB,
+      public TestIVBBase {
+  int B;
+} F;
+
+int main(int argc, char **argv) {
+
+  return 0;
+}
diff --git a/test/tools/llvm-pdbdump/Inputs/ComplexPaddingTest.pdb b/test/tools/llvm-pdbdump/Inputs/ComplexPaddingTest.pdb
new file mode 100644
index 00000000000..093276bb708
Binary files /dev/null and b/test/tools/llvm-pdbdump/Inputs/ComplexPaddingTest.pdb differ
diff --git a/test/tools/llvm-pdbdump/class-layout.test b/test/tools/llvm-pdbdump/class-layout.test
index c0083d176ea..ef5eb49af90 100644
--- a/test/tools/llvm-pdbdump/class-layout.test
+++ b/test/tools/llvm-pdbdump/class-layout.test
@@ -36,9 +36,8 @@
 
 ; BASE_CLASS_D: ---TYPES---
 ; BASE_CLASS_D: class BaseClassTest::D [sizeof = 8]
-; BASE_CLASS_D-DAG: protected BaseClassTest::B
-; BASE_CLASS_D-DAG: private BaseClassTest::C
-; BASE_CLASS_D-DAG: protected virtual BaseClassTest::A
+; BASE_CLASS_D-NEXT: protected BaseClassTest::B
+; BASE_CLASS_D-NEXT: private BaseClassTest::C
 
 ; UDT_KIND_TEST: ---TYPES---
 ; UDT_KIND_TEST-DAG: union UdtKindTest::C [sizeof = 1] {}
diff --git a/test/tools/llvm-pdbdump/complex-padding-graphical.test b/test/tools/llvm-pdbdump/complex-padding-graphical.test
new file mode 100644
index 00000000000..82881226e66
--- /dev/null
+++ b/test/tools/llvm-pdbdump/complex-padding-graphical.test
@@ -0,0 +1,53 @@
+; RUN: llvm-pdbdump pretty -classes -class-definitions=graphical \
+; RUN:     -include-types=Test %p/Inputs/ComplexPaddingTest.pdb > %t
+
+; RUN: FileCheck -input-file=%t %s -check-prefix=DIRECT_VB_ONLY
+; RUN: FileCheck -input-file=%t %s -check-prefix=DIRECT_VB_AND_NON_VB
+; RUN: FileCheck -input-file=%t %s -check-prefix=INDIRECT_VB
+; RUN: FileCheck -input-file=%t %s -check-prefix=INDIRECT_AND_DIRECT_VB
+
+
+; DIRECT_VB_ONLY:      struct TestIVBBase [sizeof = 16]
+; DIRECT_VB_ONLY-NEXT:   : public virtual TestVB {
+; DIRECT_VB_ONLY-NEXT:   vbptr +0x00 [sizeof=4]
+; DIRECT_VB_ONLY-NEXT:   data +0x04 [sizeof=4] int A
+; DIRECT_VB_ONLY-NEXT:   vbase +0x08 [sizeof=8] TestVB
+; DIRECT_VB_ONLY-NEXT:     vfptr +0x08 [sizeof=4]
+; DIRECT_VB_ONLY-NEXT:     data +0x0c [sizeof=4] int X
+; DIRECT_VB_ONLY-NEXT: }
+
+DIRECT_VB_AND_NON_VB:      struct TestVBLayout [sizeof = 24]
+DIRECT_VB_AND_NON_VB-NEXT:   : public TestNVB
+DIRECT_VB_AND_NON_VB-NEXT:   , public virtual TestVB {
+DIRECT_VB_AND_NON_VB-NEXT:   base +0x00 [sizeof=8] TestNVB
+DIRECT_VB_AND_NON_VB-NEXT:     vfptr +0x00 [sizeof=4]
+DIRECT_VB_AND_NON_VB-NEXT:     data +0x04 [sizeof=4] int Y
+DIRECT_VB_AND_NON_VB-NEXT:   vbptr +0x08 [sizeof=4]
+DIRECT_VB_AND_NON_VB-NEXT:   data +0x0c [sizeof=4] int Z
+DIRECT_VB_AND_NON_VB-NEXT:   vbase +0x10 [sizeof=8] TestVB
+DIRECT_VB_AND_NON_VB-NEXT:     vfptr +0x10 [sizeof=4]
+DIRECT_VB_AND_NON_VB-NEXT:     data +0x14 [sizeof=4] int X
+DIRECT_VB_AND_NON_VB-NEXT: }
+
+INDIRECT_VB:      struct TestIVBDerived [sizeof = 20]
+INDIRECT_VB-NEXT:   : public TestIVBBase {
+INDIRECT_VB-NEXT:   base +0x00 [sizeof=8] TestIVBBase
+INDIRECT_VB-NEXT:     vbptr +0x00 [sizeof=4]
+INDIRECT_VB-NEXT:     data +0x04 [sizeof=4] int A
+INDIRECT_VB-NEXT:   data +0x08 [sizeof=4] int B
+INDIRECT_VB-NEXT:   ivbase +0x0c [sizeof=8] TestVB
+INDIRECT_VB-NEXT:     vfptr +0x0c [sizeof=4]
+INDIRECT_VB-NEXT:     data +0x10 [sizeof=4] int X
+INDIRECT_VB-NEXT: }
+
+INDIRECT_AND_DIRECT_VB:      struct TestIVBMergedDerived [sizeof = 20]
+INDIRECT_AND_DIRECT_VB-NEXT:   : public TestIVBBase
+INDIRECT_AND_DIRECT_VB-NEXT:   , public virtual TestVB {
+INDIRECT_AND_DIRECT_VB-NEXT:   base +0x00 [sizeof=8] TestIVBBase
+INDIRECT_AND_DIRECT_VB-NEXT:     vbptr +0x00 [sizeof=4]
+INDIRECT_AND_DIRECT_VB-NEXT:     data +0x04 [sizeof=4] int A
+INDIRECT_AND_DIRECT_VB-NEXT:   data +0x08 [sizeof=4] int B
+INDIRECT_AND_DIRECT_VB-NEXT:   vbase +0x0c [sizeof=8] TestVB
+INDIRECT_AND_DIRECT_VB-NEXT:     vfptr +0x0c [sizeof=4]
+INDIRECT_AND_DIRECT_VB-NEXT:     data +0x10 [sizeof=4] int X
+INDIRECT_AND_DIRECT_VB-NEXT: }
diff --git a/test/tools/llvm-pdbdump/simple-padding-graphical.test b/test/tools/llvm-pdbdump/simple-padding-graphical.test
index aacb0a33045..222349e75b0 100644
--- a/test/tools/llvm-pdbdump/simple-padding-graphical.test
+++ b/test/tools/llvm-pdbdump/simple-padding-graphical.test
@@ -71,7 +71,6 @@
 
 ; VFPTR:      struct SimplePadVfptr [sizeof = 8] {
 ; VFPTR-NEXT:   vfptr +0x00 [sizeof=4]
-; VFPTR-NEXT:     [0] &SimplePadVfptr::~SimplePadVfptr
 ; VFPTR-NEXT:   data +0x04 [sizeof=4] int X
 ; VFPTR-NEXT: }
 
@@ -89,15 +88,15 @@
 ; MULTIPLE_INHERIT2:      SimplePadMultiInherit2 [sizeof = 16]
 ; MULTIPLE_INHERIT2-NEXT:   : public SimplePadFields1
 ; MULTIPLE_INHERIT2-NEXT:   , public SimplePadFields2 {
-; MULTIPLE_INHERIT2-NEXT:   base +0x00 [sizeof=4] SimplePadFields1
+; MULTIPLE_INHERIT2-NEXT:   base +0x00 [sizeof=3] SimplePadFields1
 ; MULTIPLE_INHERIT2-NEXT:     data +0x00 [sizeof=1] char A
 ; MULTIPLE_INHERIT2-NEXT:     data +0x01 [sizeof=1] char B
 ; MULTIPLE_INHERIT2-NEXT:     data +0x02 [sizeof=1] char C
-; MULTIPLE_INHERIT2-NEXT:     <padding> (1 bytes)
-; MULTIPLE_INHERIT2-NEXT:   base +0x04 [sizeof=8] SimplePadFields2
+; MULTIPLE_INHERIT2-NEXT:   <padding> (1 bytes)
+; MULTIPLE_INHERIT2-NEXT:   base +0x04 [sizeof=5] SimplePadFields2
 ; MULTIPLE_INHERIT2-NEXT:     data +0x04 [sizeof=4] int Y
 ; MULTIPLE_INHERIT2-NEXT:     data +0x08 [sizeof=1] char X
-; MULTIPLE_INHERIT2-NEXT:     <padding> (3 bytes)
+; MULTIPLE_INHERIT2-NEXT:   <padding> (3 bytes)
 ; MULTIPLE_INHERIT2-NEXT:   data +0x0c [sizeof=4] int X
 ; MULTIPLE_INHERIT2-NEXT: }
 
diff --git a/test/tools/llvm-pdbdump/simple-padding-text.test b/test/tools/llvm-pdbdump/simple-padding-text.test
deleted file mode 100644
index b33af565f65..00000000000
--- a/test/tools/llvm-pdbdump/simple-padding-text.test
+++ /dev/null
@@ -1,94 +0,0 @@
-; RUN: llvm-pdbdump pretty -classes -class-definitions=layout-members \
-; RUN:     -include-types=SimplePad %p/Inputs/SimplePaddingTest.pdb > %t
-
-; RUN: FileCheck -input-file=%t %s -check-prefix=NO_PADDING
-; RUN: FileCheck -input-file=%t %s -check-prefix=UNION
-; RUN: FileCheck -input-file=%t %s -check-prefix=NESTED_UNION
-; RUN: FileCheck -input-file=%t %s -check-prefix=PAD_FROM_FIELDS1
-; RUN: FileCheck -input-file=%t %s -check-prefix=PAD_FROM_FIELDS2
-; RUN: FileCheck -input-file=%t %s -check-prefix=NO_PAD_IN_BASE
-; RUN: FileCheck -input-file=%t %s -check-prefix=PAD_IN_DERIVED
-; RUN: FileCheck -input-file=%t %s -check-prefix=EMPTY_BASE
-; RUN: FileCheck -input-file=%t %s -check-prefix=VFPTR
-; RUN: FileCheck -input-file=%t %s -check-prefix=MULTIPLE_INHERIT
-; RUN: FileCheck -input-file=%t %s -check-prefix=MULTIPLE_INHERIT2
-; RUN: FileCheck -input-file=%t %s -check-prefix=DEEP_INHERIT
-; RUN: FileCheck -input-file=%t %s -check-prefix=AGGREGATE
-
-; NO_PADDING:      struct SimplePadNoPadding [sizeof = 8] {
-; NO_PADDING-NEXT:   data +0x00 [sizeof=4] int X
-; NO_PADDING-NEXT:   data +0x04 [sizeof=4] int Y
-; NO_PADDING-NEXT: }
-
-; UNION:      struct SimplePadUnion [sizeof = 16] {
-; UNION-NEXT:   data +0x00 [sizeof=4] int X
-; UNION-NEXT:   data +0x00 [sizeof=8] __int64 Y
-; UNION-NEXT:   data +0x00 [sizeof=16] SimplePadUnion::
-; UNION-NEXT: }
-
-; NESTED_UNION:      struct {{SimplePadUnion::.*}} [sizeof = 16] {
-; NESTED_UNION-NEXT:   data +0x00 [sizeof=4] int X
-; NESTED_UNION-NEXT:   <padding> (4 bytes)
-; NESTED_UNION-NEXT:   data +0x08 [sizeof=8] __int64 Y
-; NESTED_UNION-NEXT: }
-
-; PAD_FROM_FIELDS1:      struct SimplePadFields1 [sizeof = 4] {
-; PAD_FROM_FIELDS1-NEXT:   data +0x00 [sizeof=1] char A
-; PAD_FROM_FIELDS1-NEXT:   data +0x01 [sizeof=1] char B
-; PAD_FROM_FIELDS1-NEXT:   data +0x02 [sizeof=1] char C
-; PAD_FROM_FIELDS1-NEXT:   <padding> (1 bytes)
-; PAD_FROM_FIELDS1-NEXT: }
-
-; PAD_FROM_FIELDS2:      struct SimplePadFields2 [sizeof = 8] {
-; PAD_FROM_FIELDS2-NEXT:   data +0x00 [sizeof=4] int Y
-; PAD_FROM_FIELDS2-NEXT:   data +0x04 [sizeof=1] char X
-; PAD_FROM_FIELDS2-NEXT:   <padding> (3 bytes)
-; PAD_FROM_FIELDS2-NEXT: }
-
-; NO_PAD_IN_BASE:      struct SimplePadBase [sizeof = 4] {
-; NO_PAD_IN_BASE-NEXT:   data +0x00 [sizeof=4] int X
-; NO_PAD_IN_BASE-NEXT: }
-
-; PAD_IN_DERIVED:      struct SimplePadDerived [sizeof = 16]
-; PAD_IN_DERIVED-NEXT:   public SimplePadBase {
-; PAD_IN_DERIVED-NEXT:   <padding> (4 bytes)
-; PAD_IN_DERIVED-NEXT:   data +0x08 [sizeof=8] __int64 Y
-; PAD_IN_DERIVED-NEXT: }
-
-; EMPTY_BASE:      struct SimplePadEmpty [sizeof = 8]
-; EMPTY_BASE-NEXT:   : public SimplePadEmptyBase1
-; EMPTY_BASE-NEXT:   , public SimplePadEmptyBase2 {
-; EMPTY_BASE-NEXT:   <padding> (2 bytes)
-; EMPTY_BASE-NEXT:   data +0x04 [sizeof=4] int X
-; EMPTY_BASE-NEXT: }
-
-; VFPTR:      struct SimplePadVfptr [sizeof = 8] {
-; VFPTR-NEXT:   vfptr +0x00 [sizeof=4]
-; VFPTR-NEXT:   data +0x04 [sizeof=4] int X
-; VFPTR-NEXT: }
-
-; MULTIPLE_INHERIT:      struct SimplePadMultiInherit [sizeof = 8]
-; MULTIPLE_INHERIT-NEXT:   : public NonEmptyBase1
-; MULTIPLE_INHERIT-NEXT:   , public NonEmptyBase2 {
-; MULTIPLE_INHERIT-NEXT:   <padding> (2 bytes)
-; MULTIPLE_INHERIT-NEXT:   data +0x04 [sizeof=4] int X
-; MULTIPLE_INHERIT-NEXT: }
-
-; MULTIPLE_INHERIT2:      SimplePadMultiInherit2 [sizeof = 16]
-; MULTIPLE_INHERIT2-NEXT:   : public SimplePadFields1
-; MULTIPLE_INHERIT2-NEXT:   , public SimplePadFields2 {
-; MULTIPLE_INHERIT2-NEXT:   data +0x0c [sizeof=4] int X
-; MULTIPLE_INHERIT2-NEXT: }
-
-; DEEP_INHERIT:      struct SimplePadTwoLevelInherit [sizeof = 16]
-; DEEP_INHERIT-NEXT:   : public OneLevelInherit {
-; DEEP_INHERIT-NEXT:   <padding> (4 bytes)
-; DEEP_INHERIT-NEXT:   data +0x08 [sizeof=8] __int64 Z
-; DEEP_INHERIT-NEXT: }
-
-
-; AGGREGATE:      struct SimplePadAggregate [sizeof = 8] {
-; AGGREGATE-NEXT:   data +0x00 [sizeof=1] NonEmptyBase1 X
-; AGGREGATE-NEXT:   <padding> (3 bytes)
-; AGGREGATE-NEXT:   data +0x04 [sizeof=4] int Y
-; AGGREGATE-NEXT: }
diff --git a/tools/llvm-pdbdump/LinePrinter.cpp b/tools/llvm-pdbdump/LinePrinter.cpp
index d4a5a8d859e..7fa524400ae 100644
--- a/tools/llvm-pdbdump/LinePrinter.cpp
+++ b/tools/llvm-pdbdump/LinePrinter.cpp
@@ -72,7 +72,7 @@ void LinePrinter::NewLine() {
 }
 
 bool LinePrinter::IsClassExcluded(const ClassLayout &Class) {
-  if (IsTypeExcluded(Class.getUDTName(), Class.getClassSize()))
+  if (IsTypeExcluded(Class.getName(), Class.getSize()))
     return true;
   if (Class.deepPaddingSize() < opts::pretty::PaddingThreshold)
     return true;
diff --git a/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp b/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp
index 9f213a4b4d9..81ef26167a2 100644
--- a/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp
+++ b/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp
@@ -58,25 +58,11 @@ void ClassDefinitionDumper::start(const ClassLayout &Layout) {
   prettyPrintClassOutro(Layout);
 }
 
-static void printBase(LinePrinter &Printer, const PDBSymbolTypeBaseClass &Base,
-                      uint32_t &CurIndex, uint32_t TotalBaseCount,
-                      bool IsVirtual) {
-  Printer << " ";
-  WithColor(Printer, PDB_ColorItem::Keyword).get() << Base.getAccess();
-  if (IsVirtual)
-    WithColor(Printer, PDB_ColorItem::Keyword).get() << " virtual";
-  WithColor(Printer, PDB_ColorItem::Type).get() << " " << Base.getName();
-  if (++CurIndex < TotalBaseCount) {
-    Printer.NewLine();
-    Printer << ",";
-  }
-}
-
 void ClassDefinitionDumper::prettyPrintClassIntro(const ClassLayout &Layout) {
   DumpedAnything = false;
   Printer.NewLine();
 
-  uint32_t Size = Layout.getClassSize();
+  uint32_t Size = Layout.getSize();
   const PDBSymbolTypeUDT &Class = Layout.getClass();
 
   WithColor(Printer, PDB_ColorItem::Keyword).get() << Class.getUdtKind() << " ";
@@ -84,19 +70,22 @@ void ClassDefinitionDumper::prettyPrintClassIntro(const ClassLayout &Layout) {
   WithColor(Printer, PDB_ColorItem::Comment).get() << " [sizeof = " << Size
                                                    << "]";
   uint32_t BaseCount = Layout.bases().size();
-  uint32_t VBaseCount = Layout.vbases().size();
-  uint32_t TotalBaseCount = BaseCount + VBaseCount;
-  if (TotalBaseCount > 0) {
+  if (BaseCount > 0) {
     Printer.Indent();
-    Printer.NewLine();
-    Printer << ":";
-    uint32_t BaseIndex = 0;
+    char NextSeparator = ':';
     for (auto BC : Layout.bases()) {
       const auto &Base = BC->getBase();
-      printBase(Printer, Base, BaseIndex, TotalBaseCount, false);
-    }
-    for (auto &BC : Layout.vbases()) {
-      printBase(Printer, *BC, BaseIndex, TotalBaseCount, true);
+      if (Base.isIndirectVirtualBaseClass())
+        continue;
+
+      Printer.NewLine();
+      Printer << NextSeparator << " ";
+      WithColor(Printer, PDB_ColorItem::Keyword).get() << Base.getAccess();
+      if (BC->isVirtualBase())
+        WithColor(Printer, PDB_ColorItem::Keyword).get() << " virtual";
+
+      WithColor(Printer, PDB_ColorItem::Type).get() << " " << Base.getName();
+      NextSeparator = ',';
     }
 
     Printer.Unindent();
@@ -114,7 +103,7 @@ void ClassDefinitionDumper::prettyPrintClassOutro(const ClassLayout &Layout) {
   Printer.NewLine();
   if (Layout.deepPaddingSize() > 0) {
     APFloat Pct(100.0 * (double)Layout.deepPaddingSize() /
-                (double)Layout.getClassSize());
+                (double)Layout.getSize());
     SmallString<8> PctStr;
     Pct.toString(PctStr, 4);
     WithColor(Printer, PDB_ColorItem::Padding).get()
diff --git a/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.cpp b/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.cpp
index d146ca9d471..f0385f198e0 100644
--- a/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.cpp
+++ b/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.cpp
@@ -53,15 +53,28 @@ bool PrettyClassLayoutGraphicalDumper::start(const UDTLayoutBase &Layout) {
       }
     }
 
-    CurrentItem = Item.get();
-    Item->getSymbol().dump(*this);
+    CurrentItem = Item;
+    if (Item->isVBPtr()) {
+      VTableLayoutItem &Layout = static_cast<VTableLayoutItem &>(*CurrentItem);
+
+      VariableDumper VarDumper(Printer);
+      VarDumper.startVbptr(CurrentAbsoluteOffset, Layout.getSize());
+    } else {
+      if (auto Sym = Item->getSymbol())
+        Sym->dump(*this);
+    }
+
+    if (Item->getLayoutSize() > 0) {
+      uint32_t Prev = RelativeOffset + Item->getLayoutSize() - 1;
+      NextPaddingByte = UseMap.find_next_unset(Prev);
+    }
   }
 
-  if (NextPaddingByte >= 0 && Layout.getClassSize() > 1) {
-    uint32_t Amount = Layout.getClassSize() - NextPaddingByte;
+  auto TailPadding = Layout.tailPadding();
+  if (TailPadding > 0) {
     Printer.NewLine();
-    WithColor(Printer, PDB_ColorItem::Padding).get() << "<padding> (" << Amount
-                                                     << " bytes)";
+    WithColor(Printer, PDB_ColorItem::Padding).get() << "<padding> ("
+      << TailPadding << " bytes)";
     DumpedAnything = true;
   }
 
@@ -85,12 +98,19 @@ void PrettyClassLayoutGraphicalDumper::dump(
   Printer.NewLine();
   BaseClassLayout &Layout = static_cast<BaseClassLayout &>(*CurrentItem);
 
-  std::string Label = Layout.isVirtualBase() ? "vbase" : "base";
+  std::string Label = "base";
+  if (Layout.isVirtualBase()) {
+    Label.insert(Label.begin(), 'v');
+    if (Layout.getBase().isIndirectVirtualBaseClass())
+      Label.insert(Label.begin(), 'i');
+  }
   Printer << Label << " ";
 
+  uint32_t Size = Layout.isEmptyBase() ? 1 : Layout.getLayoutSize();
+
   WithColor(Printer, PDB_ColorItem::Offset).get()
-      << "+" << format_hex(CurrentAbsoluteOffset, 4)
-      << " [sizeof=" << Layout.getSize() << "] ";
+      << "+" << format_hex(CurrentAbsoluteOffset, 4) << " [sizeof=" << Size
+      << "] ";
 
   WithColor(Printer, PDB_ColorItem::Identifier).get() << Layout.getName();
 
@@ -125,27 +145,8 @@ void PrettyClassLayoutGraphicalDumper::dump(const PDBSymbolData &Symbol) {
 void PrettyClassLayoutGraphicalDumper::dump(const PDBSymbolTypeVTable &Symbol) {
   assert(CurrentItem != nullptr);
 
-  VTableLayoutItem &Layout = static_cast<VTableLayoutItem &>(*CurrentItem);
-
   VariableDumper VarDumper(Printer);
   VarDumper.start(Symbol, ClassOffsetZero);
 
-  Printer.Indent();
-  uint32_t Index = 0;
-  for (auto &Func : Layout.funcs()) {
-    Printer.NewLine();
-    std::string Name = Func->getName();
-    auto ParentClass =
-        unique_dyn_cast<PDBSymbolTypeUDT>(Func->getClassParent());
-    assert(ParentClass);
-    WithColor(Printer, PDB_ColorItem::Address).get() << " [" << Index << "] ";
-    WithColor(Printer, PDB_ColorItem::Identifier).get()
-        << "&" << ParentClass->getName();
-    Printer << "::";
-    WithColor(Printer, PDB_ColorItem::Identifier).get() << Name;
-    ++Index;
-  }
-  Printer.Unindent();
-
   DumpedAnything = true;
 }
diff --git a/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.h b/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.h
index 7dfb74c4e14..61b62b1762a 100644
--- a/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.h
+++ b/tools/llvm-pdbdump/PrettyClassLayoutGraphicalDumper.h
@@ -19,7 +19,7 @@ namespace llvm {
 namespace pdb {
 
 class UDTLayoutBase;
-class StorageItemBase;
+class LayoutItemBase;
 class LinePrinter;
 
 class PrettyClassLayoutGraphicalDumper : public PDBSymDumper {
@@ -37,7 +37,7 @@ private:
 
   LinePrinter &Printer;
 
-  StorageItemBase *CurrentItem = nullptr;
+  LayoutItemBase *CurrentItem = nullptr;
   uint32_t ClassOffsetZero = 0;
   uint32_t CurrentAbsoluteOffset = 0;
   bool DumpedAnything = false;
diff --git a/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.cpp b/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.cpp
index 02f31108b0d..ccfd38c1e3c 100644
--- a/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.cpp
+++ b/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.cpp
@@ -59,14 +59,17 @@ bool PrettyClassLayoutTextDumper::start(const ClassLayout &Layout) {
         NextUnusedByte = UseMap.find_next_unset(Off);
       }
     }
-    LayoutItem->getSymbol().dump(*this);
+    if (auto Sym = LayoutItem->getSymbol())
+      Sym->dump(*this);
   }
 
-  if (NextUnusedByte >= 0 && Layout.getClassSize() > 1) {
-    uint32_t Amount = Layout.getClassSize() - NextUnusedByte;
-    Printer.NewLine();
-    WithColor(Printer, PDB_ColorItem::Padding).get() << "<padding> (" << Amount
-                                                     << " bytes)";
+  if (NextUnusedByte >= 0 && Layout.getSize() > 1) {
+    uint32_t Amount = Layout.getSize() - NextUnusedByte;
+    if (Amount > 0) {
+      Printer.NewLine();
+      WithColor(Printer, PDB_ColorItem::Padding).get() << "<padding> ("
+                                                       << Amount << " bytes)";
+    }
     DumpedAnything = true;
   }
 
@@ -116,4 +119,6 @@ void PrettyClassLayoutTextDumper::dump(const PDBSymbolTypeTypedef &Symbol) {
   Dumper.start(Symbol);
 }
 
+void PrettyClassLayoutTextDumper::dump(const PDBSymbolTypeBuiltin &Symbol) {}
+
 void PrettyClassLayoutTextDumper::dump(const PDBSymbolTypeUDT &Symbol) {}
diff --git a/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.h b/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.h
index 56c20f0e843..1adbbffce55 100644
--- a/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.h
+++ b/tools/llvm-pdbdump/PrettyClassLayoutTextDumper.h
@@ -34,6 +34,7 @@ public:
   void dump(const PDBSymbolTypeTypedef &Symbol) override;
   void dump(const PDBSymbolTypeUDT &Symbol) override;
   void dump(const PDBSymbolTypeVTable &Symbol) override;
+  void dump(const PDBSymbolTypeBuiltin &Symbol) override;
 
 private:
   bool DumpedAnything = false;
diff --git a/tools/llvm-pdbdump/PrettyTypeDumper.cpp b/tools/llvm-pdbdump/PrettyTypeDumper.cpp
index ffeef72150d..324a26f9a24 100644
--- a/tools/llvm-pdbdump/PrettyTypeDumper.cpp
+++ b/tools/llvm-pdbdump/PrettyTypeDumper.cpp
@@ -34,17 +34,23 @@ using LayoutPtr = std::unique_ptr<ClassLayout>;
 typedef bool (*CompareFunc)(const LayoutPtr &S1, const LayoutPtr &S2);
 
 static bool CompareNames(const LayoutPtr &S1, const LayoutPtr &S2) {
-  return S1->getUDTName() < S2->getUDTName();
+  return S1->getName() < S2->getName();
 }
 
 static bool CompareSizes(const LayoutPtr &S1, const LayoutPtr &S2) {
-  return S1->getClassSize() < S2->getClassSize();
+  return S1->getSize() < S2->getSize();
 }
 
 static bool ComparePadding(const LayoutPtr &S1, const LayoutPtr &S2) {
   return S1->deepPaddingSize() < S2->deepPaddingSize();
 }
 
+static bool ComparePaddingPct(const LayoutPtr &S1, const LayoutPtr &S2) {
+  double Pct1 = (double)S1->deepPaddingSize() / (double)S1->getSize();
+  double Pct2 = (double)S2->deepPaddingSize() / (double)S2->getSize();
+  return Pct1 < Pct2;
+}
+
 static CompareFunc getComparisonFunc(opts::pretty::ClassSortMode Mode) {
   switch (Mode) {
   case opts::pretty::ClassSortMode::Name:
@@ -53,6 +59,8 @@ static CompareFunc getComparisonFunc(opts::pretty::ClassSortMode Mode) {
     return CompareSizes;
   case opts::pretty::ClassSortMode::Padding:
     return ComparePadding;
+  case opts::pretty::ClassSortMode::PaddingPct:
+    return ComparePaddingPct;
   default:
     return nullptr;
   }
@@ -67,14 +75,18 @@ filterAndSortClassDefs(LinePrinter &Printer, Enumerator &E,
   Filtered.reserve(UnfilteredCount);
   CompareFunc Comp = getComparisonFunc(opts::pretty::ClassOrder);
 
+  if (UnfilteredCount > 10000) {
+    errs() << formatv("Filtering and sorting {0} types", UnfilteredCount);
+    errs().flush();
+  }
   uint32_t Examined = 0;
   uint32_t Discarded = 0;
   while (auto Class = E.getNext()) {
     ++Examined;
     if (Examined % 10000 == 0) {
-      outs() << formatv("Examined {0}/{1} items.  {2} items discarded\n",
+      errs() << formatv("Examined {0}/{1} items.  {2} items discarded\n",
                         Examined, UnfilteredCount, Discarded);
-      outs().flush();
+      errs().flush();
     }
 
     if (Class->getUnmodifiedTypeId() != 0) {
@@ -163,6 +175,9 @@ void TypeDumper::start(const PDBSymbolExe &Exe) {
         dumpClassLayout(*Class);
     } else {
       while (auto Class = Classes->getNext()) {
+        if (Class->getUnmodifiedTypeId() != 0)
+          continue;
+
         if (Printer.IsTypeExcluded(Class->getName(), Class->getLength()))
           continue;
 
@@ -209,7 +224,7 @@ void TypeDumper::dumpClassLayout(const ClassLayout &Class) {
   if (opts::pretty::ClassFormat == opts::pretty::ClassDefinitionFormat::None) {
     Printer.NewLine();
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "class ";
-    WithColor(Printer, PDB_ColorItem::Identifier).get() << Class.getUDTName();
+    WithColor(Printer, PDB_ColorItem::Identifier).get() << Class.getName();
   } else {
     ClassDefinitionDumper Dumper(Printer);
     Dumper.start(Class);
diff --git a/tools/llvm-pdbdump/PrettyVariableDumper.cpp b/tools/llvm-pdbdump/PrettyVariableDumper.cpp
index 76a0d23bf87..70925f4b03d 100644
--- a/tools/llvm-pdbdump/PrettyVariableDumper.cpp
+++ b/tools/llvm-pdbdump/PrettyVariableDumper.cpp
@@ -91,6 +91,14 @@ void VariableDumper::start(const PDBSymbolData &Var, uint32_t Offset) {
   }
 }
 
+void VariableDumper::startVbptr(uint32_t Offset, uint32_t Size) {
+  Printer.NewLine();
+  Printer << "vbptr ";
+
+  WithColor(Printer, PDB_ColorItem::Offset).get()
+      << "+" << format_hex(Offset, 4) << " [sizeof=" << Size << "] ";
+}
+
 void VariableDumper::start(const PDBSymbolTypeVTable &Var, uint32_t Offset) {
   Printer.NewLine();
   Printer << "vfptr ";
diff --git a/tools/llvm-pdbdump/PrettyVariableDumper.h b/tools/llvm-pdbdump/PrettyVariableDumper.h
index 4ba3bc97d85..cacf1ce9577 100644
--- a/tools/llvm-pdbdump/PrettyVariableDumper.h
+++ b/tools/llvm-pdbdump/PrettyVariableDumper.h
@@ -26,6 +26,7 @@ public:
 
   void start(const PDBSymbolData &Var, uint32_t Offset = 0);
   void start(const PDBSymbolTypeVTable &Var, uint32_t Offset = 0);
+  void startVbptr(uint32_t Offset, uint32_t Size);
 
   void dump(const PDBSymbolTypeArray &Symbol) override;
   void dump(const PDBSymbolTypeBuiltin &Symbol) override;
diff --git a/tools/llvm-pdbdump/llvm-pdbdump.cpp b/tools/llvm-pdbdump/llvm-pdbdump.cpp
index 06c2afc0bc7..d51f64065d6 100644
--- a/tools/llvm-pdbdump/llvm-pdbdump.cpp
+++ b/tools/llvm-pdbdump/llvm-pdbdump.cpp
@@ -124,12 +124,15 @@ cl::opt<bool> Typedefs("typedefs", cl::desc("Display typedef types"),
                        cl::cat(TypeCategory), cl::sub(PrettySubcommand));
 cl::opt<ClassSortMode> ClassOrder(
     "class-order", cl::desc("Class sort order"), cl::init(ClassSortMode::None),
-    cl::values(clEnumValN(ClassSortMode::None, "none",
-                          "Undefined / no particular sort order"),
-               clEnumValN(ClassSortMode::Name, "name", "Sort classes by name"),
-               clEnumValN(ClassSortMode::Size, "size", "Sort classes by size"),
-               clEnumValN(ClassSortMode::Padding, "padding",
-                          "Sort classes by amount of padding")),
+    cl::values(
+        clEnumValN(ClassSortMode::None, "none",
+                   "Undefined / no particular sort order"),
+        clEnumValN(ClassSortMode::Name, "name", "Sort classes by name"),
+        clEnumValN(ClassSortMode::Size, "size", "Sort classes by size"),
+        clEnumValN(ClassSortMode::Padding, "padding",
+                   "Sort classes by amount of padding"),
+        clEnumValN(ClassSortMode::PaddingPct, "padding-pct",
+                   "Sort classes by percentage of space consumed by padding")),
     cl::cat(TypeCategory), cl::sub(PrettySubcommand));
 
 cl::opt<ClassDefinitionFormat> ClassFormat(
diff --git a/tools/llvm-pdbdump/llvm-pdbdump.h b/tools/llvm-pdbdump/llvm-pdbdump.h
index a5429a253df..f77124de0ba 100644
--- a/tools/llvm-pdbdump/llvm-pdbdump.h
+++ b/tools/llvm-pdbdump/llvm-pdbdump.h
@@ -19,7 +19,7 @@ namespace opts {
 namespace pretty {
 
 enum class ClassDefinitionFormat { None, Layout, Graphical, Standard };
-enum class ClassSortMode { None, Name, Size, Padding };
+enum class ClassSortMode { None, Name, Size, Padding, PaddingPct };
 
 extern llvm::cl::opt<bool> Compilands;
 extern llvm::cl::opt<bool> Symbols;