aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: zturner, rnk, stella.stamenova.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, teemperor, abidh.

This patch makes virtual bases to be added in the correct order to the bases 
list. It is important because `VTableContext` (`MicrosoftVTableContext` in our 
case) uses then the order of virtual bases in the list to restore the virtual 
table indexes. These indexes are used then to resolve the layout of the virtual 
bases.

We haven't enough information about offsets of virtual bases regarding to the 
object (moreover, in a common case we can't rely on such information, see the 
example here: https://reviews.llvm.org/D53506#1272306 ), but there should be 
enough information to restore the layout of the virtual bases from the indexes 
in runtime. After D53506 <https://reviews.llvm.org/D53506> this information is 
used whenever possible, so there should be no problems with virtual bases' 
fields reading.

I have some problems with the test, I'll describe them exactly in the test's 
text below. Do you have them too or is this a specific problem with my setup?


Repository:
  rLLDB LLDB

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
@@ -49,6 +49,7 @@
   PdbAstBuilder &m_ast_builder;
   llvm::pdb::TpiStream &m_tpi;
   std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> m_bases;
+  std::map<uint64_t, std::unique_ptr<clang::CXXBaseSpecifier>> m_vbases;
   ClangASTImporter::LayoutInfo m_layout;
 
 public:
@@ -66,7 +67,9 @@
 
 private:
   clang::QualType AddBaseClassForTypeIndex(llvm::codeview::TypeIndex ti,
-                                           llvm::codeview::MemberAccess access);
+                                           llvm::codeview::MemberAccess access,
+                                           bool is_virtual,
+                                           uint64_t vtable_idx);
 };
 
 } // namespace npdb
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,
+    bool is_virtual, uint64_t vtable_idx) {
   PdbTypeSymId type_id(ti);
   clang::QualType qt = m_ast_builder.GetOrCreateType(type_id);
 
@@ -58,17 +59,24 @@
 
   std::unique_ptr<clang::CXXBaseSpecifier> base_spec =
       m_ast_builder.clang().CreateBaseClassSpecifier(
-          qt.getAsOpaquePtr(), TranslateMemberAccess(access), false,
+          qt.getAsOpaquePtr(), TranslateMemberAccess(access), is_virtual,
           udt_cvt.kind() == LF_CLASS);
   lldbassert(base_spec);
-  m_bases.push_back(std::move(base_spec));
+
+  // Process virtual bases separately because their order in the result vector
+  // must correspond to the indexes in the vbtable.
+  if (is_virtual)
+    m_vbases[vtable_idx] = std::move(base_spec);
+  else
+    m_bases.push_back(std::move(base_spec));
+
   return qt;
 }
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            BaseClassRecord &base) {
   clang::QualType base_qt =
-      AddBaseClassForTypeIndex(base.Type, base.getAccess());
+      AddBaseClassForTypeIndex(base.Type, base.getAccess(), false, 0);
 
   auto decl =
       m_ast_builder.clang().GetAsCXXRecordDecl(base_qt.getAsOpaquePtr());
@@ -82,9 +90,9 @@
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            VirtualBaseClassRecord &base) {
-  AddBaseClassForTypeIndex(base.BaseType, base.getAccess());
+  AddBaseClassForTypeIndex(base.BaseType, base.getAccess(), true,
+                           base.VTableIndex);
 
-  // FIXME: Handle virtual base offsets.
   return Error::success();
 }
 
@@ -177,6 +185,10 @@
 }
 
 void UdtRecordCompleter::complete() {
+  // Flush all virtual bases using the correct order.
+  for (auto &pair : m_vbases)
+    m_bases.push_back(std::move(pair.second));
+
   ClangASTContext &clang = m_ast_builder.clang();
   clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(),
                             std::move(m_bases));
Index: lit/SymbolFile/NativePDB/tag-types.cpp
===================================================================
--- lit/SymbolFile/NativePDB/tag-types.cpp
+++ lit/SymbolFile/NativePDB/tag-types.cpp
@@ -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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to