From eaa0ad672e3935d79c00c8c181329e4f0630945b Mon Sep 17 00:00:00 2001 From: Aleksandr Urakov Date: Tue, 12 Feb 2019 08:17:11 +0000 Subject: [PATCH] [NativePDB] Process virtual bases in the correct order Summary: 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 this information is used whenever possible, so there should be no problems with virtual bases' fields reading. Reviewers: zturner, rnk, stella.stamenova Subscribers: abidh, teemperor, lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D56904 llvm-svn: 353806 --- .../NativePDB/Inputs/tag-types.lldbinit | 2 ++ lldb/lit/SymbolFile/NativePDB/tag-types.cpp | 16 +++++++++- .../NativePDB/UdtRecordCompleter.cpp | 29 ++++++++++++++----- .../SymbolFile/NativePDB/UdtRecordCompleter.h | 10 +++++-- 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit b/lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit index c50e18101b1b3..a7ec496447efa 100644 --- a/lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit +++ b/lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit @@ -3,6 +3,8 @@ type lookup -- Class type lookup -- Union type lookup -- Derived type lookup -- Derived2 +type lookup -- DerivedVirtual1 +type lookup -- DerivedVirtual2 type lookup -- EnumInt type lookup -- EnumShort type lookup -- InvalidType diff --git a/lldb/lit/SymbolFile/NativePDB/tag-types.cpp b/lldb/lit/SymbolFile/NativePDB/tag-types.cpp index 127ad5980925e..db981eafa6e5b 100644 --- a/lldb/lit/SymbolFile/NativePDB/tag-types.cpp +++ b/lldb/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 @@ class Derived2 : protected Class, private Struct { 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 @@ int main(int argc, char **argv) { Union U; Derived D; Derived2 D2; + DerivedVirtual1 DV1; + DerivedVirtual2 DV2; EnumInt EI; EnumShort ES; @@ -221,6 +229,12 @@ int main(int argc, char **argv) { // 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, diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp index 37be3bc9cdfd5..ba5c14b96039a 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -50,7 +50,8 @@ UdtRecordCompleter::UdtRecordCompleter(PdbTypeSymId id, } clang::QualType UdtRecordCompleter::AddBaseClassForTypeIndex( - llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access) { + llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access, + llvm::Optional vtable_idx) { PdbTypeSymId type_id(ti); clang::QualType qt = m_ast_builder.GetOrCreateType(type_id); @@ -58,10 +59,13 @@ clang::QualType UdtRecordCompleter::AddBaseClassForTypeIndex( std::unique_ptr 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, 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 @@ Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr, } 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> 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); diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h index 43015f9e25b17..b11e813c1228c 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h +++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h @@ -35,6 +35,9 @@ namespace npdb { class PdbAstBuilder; class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { + using IndexedBase = + std::pair>; + union UdtTagRecord { UdtTagRecord() {} llvm::codeview::UnionRecord ur; @@ -47,7 +50,7 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { clang::TagDecl &m_tag_decl; PdbAstBuilder &m_ast_builder; llvm::pdb::TpiStream &m_tpi; - std::vector> m_bases; + std::vector m_bases; ClangASTImporter::LayoutInfo m_layout; public: @@ -64,8 +67,9 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { 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 vtable_idx = llvm::Optional()); void AddMethod(llvm::StringRef name, llvm::codeview::TypeIndex type_idx, llvm::codeview::MemberAccess access, llvm::codeview::MethodOptions options,