[Lldb-commits] [PATCH] D56904: [NativePDB] Process virtual bases in the correct order

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov updated this revision to Diff 184061.
aleksandr.urakov added a comment.

Thanks for the comments, I think that they make sense. I've updated the patch.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56904/new/

https://reviews.llvm.org/D56904

Files:
  lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit
  lit/SymbolFile/NativePDB/tag-types.cpp
  source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h


Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
===================================================================
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
@@ -35,6 +35,9 @@
 class PdbAstBuilder;
 
 class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks {
+  using IndexedBase =
+      std::pair<uint64_t, std::unique_ptr<clang::CXXBaseSpecifier>>;
+
   union UdtTagRecord {
     UdtTagRecord() {}
     llvm::codeview::UnionRecord ur;
@@ -47,7 +50,7 @@
   clang::TagDecl &m_tag_decl;
   PdbAstBuilder &m_ast_builder;
   llvm::pdb::TpiStream &m_tpi;
-  std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> m_bases;
+  std::vector<IndexedBase> m_bases;
   ClangASTImporter::LayoutInfo m_layout;
 
 public:
@@ -64,8 +67,9 @@
   void complete();
 
 private:
-  clang::QualType AddBaseClassForTypeIndex(llvm::codeview::TypeIndex ti,
-                                           llvm::codeview::MemberAccess access);
+  clang::QualType AddBaseClassForTypeIndex(
+      llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access,
+      llvm::Optional<uint64_t> vtable_idx = llvm::Optional<uint64_t>());
   void AddMethod(llvm::StringRef name, llvm::codeview::TypeIndex type_idx,
                  llvm::codeview::MemberAccess access,
                  llvm::codeview::MethodOptions options,
Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===================================================================
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -50,7 +50,8 @@
 }
 
 clang::QualType UdtRecordCompleter::AddBaseClassForTypeIndex(
-    llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access) {
+    llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access,
+    llvm::Optional<uint64_t> vtable_idx) {
   PdbTypeSymId type_id(ti);
   clang::QualType qt = m_ast_builder.GetOrCreateType(type_id);
 
@@ -58,10 +59,13 @@
 
   std::unique_ptr<clang::CXXBaseSpecifier> base_spec =
       m_ast_builder.clang().CreateBaseClassSpecifier(
-          qt.getAsOpaquePtr(), TranslateMemberAccess(access), false,
-          udt_cvt.kind() == LF_CLASS);
+          qt.getAsOpaquePtr(), TranslateMemberAccess(access),
+          vtable_idx.hasValue(), udt_cvt.kind() == LF_CLASS);
   lldbassert(base_spec);
-  m_bases.push_back(std::move(base_spec));
+
+  m_bases.push_back(
+      std::make_pair(vtable_idx.getValueOr(0), std::move(base_spec)));
+
   return qt;
 }
 
@@ -98,9 +102,8 @@
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            VirtualBaseClassRecord &base) {
-  AddBaseClassForTypeIndex(base.BaseType, base.getAccess());
+  AddBaseClassForTypeIndex(base.BaseType, base.getAccess(), base.VTableIndex);
 
-  // FIXME: Handle virtual base offsets.
   return Error::success();
 }
 
@@ -209,9 +212,19 @@
 }
 
 void UdtRecordCompleter::complete() {
+  // Ensure the correct order for virtual bases.
+  std::stable_sort(m_bases.begin(), m_bases.end(),
+                   [](const IndexedBase &lhs, const IndexedBase &rhs) {
+                     return lhs.first < rhs.first;
+                   });
+
+  std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> bases;
+  bases.reserve(m_bases.size());
+  for (auto &ib : m_bases)
+    bases.push_back(std::move(ib.second));
+
   ClangASTContext &clang = m_ast_builder.clang();
-  clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(),
-                            std::move(m_bases));
+  clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(), std::move(bases));
 
   clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQualType());
   ClangASTContext::BuildIndirectFields(m_derived_ct);
Index: lit/SymbolFile/NativePDB/tag-types.cpp
===================================================================
--- lit/SymbolFile/NativePDB/tag-types.cpp
+++ lit/SymbolFile/NativePDB/tag-types.cpp
@@ -2,7 +2,7 @@
 // REQUIRES: lld
 
 // Test that we can display tag types.
-// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
+// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN:     %p/Inputs/tag-types.lldbinit | FileCheck %s
 
@@ -115,6 +115,12 @@
 
 unsigned Derived2::StaticDataMember = 0;
 
+// Test virtual inheritance.
+class DerivedVirtual1 : public virtual Class {};
+
+// Test the correctness of the virtual bases order.
+class DerivedVirtual2 : public DerivedVirtual1, public virtual OneMember {};
+
 // Test scoped enums and unscoped enums.
 enum class EnumInt {
   A = 1,
@@ -133,6 +139,8 @@
   Union U;
   Derived D;
   Derived2 D2;
+  DerivedVirtual1 DV1;
+  DerivedVirtual2 DV2;
   EnumInt EI;
   EnumShort ES;
   
@@ -221,6 +229,12 @@
 // CHECK-NEXT: class Derived2 : protected Class, private Struct {
 // CHECK-NEXT:     static unsigned int StaticDataMember;
 // CHECK-NEXT: }
+// CHECK-NEXT: (lldb) type lookup -- DerivedVirtual1
+// CHECK-NEXT: class DerivedVirtual1 : virtual public Class {
+// CHECK-NEXT: }
+// CHECK-NEXT: (lldb) type lookup -- DerivedVirtual2
+// CHECK-NEXT: class DerivedVirtual2 : public DerivedVirtual1, virtual public Class, virtual public OneMember {
+// CHECK-NEXT: }
 // CHECK-NEXT: (lldb) type lookup -- EnumInt
 // CHECK-NEXT: enum EnumInt {
 // CHECK-NEXT:     A,
Index: lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit
===================================================================
--- lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit
+++ lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit
@@ -3,6 +3,8 @@
 type lookup -- Union
 type lookup -- Derived
 type lookup -- Derived2
+type lookup -- DerivedVirtual1
+type lookup -- DerivedVirtual2
 type lookup -- EnumInt
 type lookup -- EnumShort
 type lookup -- InvalidType

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits