Index: include/lldb/Symbol/ClangASTContext.h =================================================================== --- include/lldb/Symbol/ClangASTContext.h +++ include/lldb/Symbol/ClangASTContext.h @@ -831,6 +831,8 @@ bool is_static, bool is_inline, bool is_explicit, bool is_attr_used, bool is_artificial); + void AddMethodOverridesForCXXRecordType(lldb::opaque_compiler_type_t type); + // C++ Base Classes clang::CXXBaseSpecifier * CreateBaseClassSpecifier(lldb::opaque_compiler_type_t type, Index: lit/SymbolFile/PDB/Inputs/UdtCompletionTest.cpp =================================================================== --- /dev/null +++ lit/SymbolFile/PDB/Inputs/UdtCompletionTest.cpp @@ -0,0 +1,61 @@ +union U { + char _u1; + short _u2; + int _u3; +}; + +struct A { + explicit A(int u) { _u._u3 = u; } + A(const A &) = default; + virtual ~A() = default; + +private: + U _u; +}; + +#pragma pack(push, 1) +struct B : public virtual A { + B(char a, char b, char c) : A(a + b + c), _a(a), _b(b), _c(c) {} + B(const B &) = default; + +private: + char _a; + unsigned short : 3; + unsigned short _b : 6; + unsigned short : 4; + int _c; +}; +#pragma pack(pop) + +#pragma pack(push, 16) +class C : public virtual A, private B { +public: + C(char x, char y, char z) + : A(x - y + z), B(x, y, z), _x(x * 2), _y(y * 2), _z(z * 2) {} + C(const C &) = default; + + static int abc; + +private: + int _x; + short _y; + char _z; +}; +int C::abc = 123; +#pragma pack(pop) + +class List { +public: + List(List *p, List *n, C v) : Prev(p), Next(n), Value(v) {} + +private: + List *Prev; + List *Next; + C Value; +}; + +int main(int argc, char *argv[]) { + List l{nullptr, nullptr, C{1, 2, 3}}; + + return 0; +} Index: lit/SymbolFile/PDB/Inputs/UdtCompletionTest.script =================================================================== --- /dev/null +++ lit/SymbolFile/PDB/Inputs/UdtCompletionTest.script @@ -0,0 +1,4 @@ +breakpoint set --file UdtCompletionTest.cpp --line 60 +run +target variable +frame variable Index: lit/SymbolFile/PDB/udt-completion.test =================================================================== --- /dev/null +++ lit/SymbolFile/PDB/udt-completion.test @@ -0,0 +1,26 @@ +REQUIRES: windows +RUN: clang-cl /Zi %S/Inputs/UdtCompletionTest.cpp /o %t.exe +RUN: %lldb -b -s %S/Inputs/UdtCompletionTest.script -- %t.exe | FileCheck %s + +CHECK: (int) int C::abc = 123 + +CHECK: (List) l = { +CHECK: Prev = 0x00000000 +CHECK: Next = 0x00000000 +CHECK: Value = { +CHECK: A = { +CHECK: _u = (_u1 = '\x02', _u2 = 2, _u3 = 2) +CHECK: } +CHECK: B = { +CHECK: A = { +CHECK: _u = (_u1 = '\x02', _u2 = 2, _u3 = 2) +CHECK: } +CHECK: _a = '\x01' +CHECK: _b = 2 +CHECK: _c = 3 +CHECK: } +CHECK: _x = 2 +CHECK: _y = 4 +CHECK: _z = '\x06' +CHECK: } +CHECK: } Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2108,95 +2108,6 @@ return template_param_infos.args.size() == template_param_infos.names.size(); } -// Checks whether m1 is an overload of m2 (as opposed to an override). This is -// called by addOverridesForMethod to distinguish overrides (which share a -// vtable entry) from overloads (which require distinct entries). -static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) { - // FIXME: This should detect covariant return types, but currently doesn't. - lldbassert(&m1->getASTContext() == &m2->getASTContext() && - "Methods should have the same AST context"); - clang::ASTContext &context = m1->getASTContext(); - - const auto *m1Type = - llvm::cast( - context.getCanonicalType(m1->getType())); - - const auto *m2Type = - llvm::cast( - context.getCanonicalType(m2->getType())); - - auto compareArgTypes = - [&context](const clang::QualType &m1p, const clang::QualType &m2p) { - return context.hasSameType(m1p.getUnqualifiedType(), - m2p.getUnqualifiedType()); - }; - - // FIXME: In C++14 and later, we can just pass m2Type->param_type_end() - // as a fourth parameter to std::equal(). - return (m1->getNumParams() != m2->getNumParams()) || - !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(), - m2Type->param_type_begin(), compareArgTypes); -} - -// If decl is a virtual method, walk the base classes looking for methods that -// decl overrides. This table of overridden methods is used by IRGen to -// determine the vtable layout for decl's parent class. -static void addOverridesForMethod(clang::CXXMethodDecl *decl) { - if (!decl->isVirtual()) - return; - - clang::CXXBasePaths paths; - - auto find_overridden_methods = - [decl](const clang::CXXBaseSpecifier *specifier, clang::CXXBasePath &path) { - if (auto *base_record = - llvm::dyn_cast( - specifier->getType()->getAs()->getDecl())) { - - clang::DeclarationName name = decl->getDeclName(); - - // If this is a destructor, check whether the base class destructor is - // virtual. - if (name.getNameKind() == clang::DeclarationName::CXXDestructorName) - if (auto *baseDtorDecl = base_record->getDestructor()) { - if (baseDtorDecl->isVirtual()) { - path.Decls = baseDtorDecl; - return true; - } else - return false; - } - - // Otherwise, search for name in the base class. - for (path.Decls = base_record->lookup(name); !path.Decls.empty(); - path.Decls = path.Decls.slice(1)) { - if (auto *method_decl = - llvm::dyn_cast(path.Decls.front())) - if (method_decl->isVirtual() && !isOverload(decl, method_decl)) { - path.Decls = method_decl; - return true; - } - } - } - - return false; - }; - - if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) { - for (auto *overridden_decl : paths.found_decls()) - decl->addOverriddenMethod( - llvm::cast(overridden_decl)); - } -} - -// If clang_type is a CXXRecordDecl, builds the method override list for each -// of its virtual methods. -static void addMethodOverrides(ClangASTContext &ast, CompilerType &clang_type) { - if (auto *record = - ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType())) - for (auto *method : record->methods()) - addOverridesForMethod(method); -} - bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, lldb_private::Type *type, CompilerType &clang_type) { @@ -2405,7 +2316,7 @@ } } - addMethodOverrides(m_ast, clang_type); + m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType()); ClangASTContext::BuildIndirectFields(clang_type); ClangASTContext::CompleteTagDeclarationDefinition(clang_type); Index: source/Plugins/SymbolFile/PDB/PDBASTParser.h =================================================================== --- source/Plugins/SymbolFile/PDB/PDBASTParser.h +++ source/Plugins/SymbolFile/PDB/PDBASTParser.h @@ -28,9 +28,13 @@ namespace llvm { namespace pdb { +template class ConcreteSymbolEnumerator; + class PDBSymbol; class PDBSymbolData; +class PDBSymbolTypeBaseClass; class PDBSymbolTypeBuiltin; +class PDBSymbolTypeUDT; } // namespace pdb } // namespace llvm @@ -40,13 +44,40 @@ ~PDBASTParser(); lldb::TypeSP CreateLLDBTypeFromPDBType(const llvm::pdb::PDBSymbol &type); + bool CompleteTypeFromPDB(lldb_private::CompilerType &compiler_type); + + lldb_private::ClangASTImporter &GetClangASTImporter() { + return m_ast_importer; + } private: + typedef llvm::DenseMap + ClangTypeToUidMap; + typedef llvm::pdb::ConcreteSymbolEnumerator + PDBDataSymbolEnumerator; + typedef llvm::pdb::ConcreteSymbolEnumerator + PDBBaseClassSymbolEnumerator; + bool AddEnumValue(lldb_private::CompilerType enum_type, const llvm::pdb::PDBSymbolData &data) const; + bool CompleteTypeFromUDT(lldb_private::SymbolFile &symbol_file, + lldb_private::CompilerType &compiler_type, + llvm::pdb::PDBSymbolTypeUDT &udt); + void AddRecordMembers( + lldb_private::SymbolFile &symbol_file, + lldb_private::CompilerType &record_type, + PDBDataSymbolEnumerator &members_enum, + lldb_private::ClangASTImporter::LayoutInfo &layout_info) const; + void AddRecordBases( + lldb_private::SymbolFile &symbol_file, + lldb_private::CompilerType &record_type, + int record_kind, + PDBBaseClassSymbolEnumerator &bases_enum, + lldb_private::ClangASTImporter::LayoutInfo &layout_info) const; lldb_private::ClangASTContext &m_ast; lldb_private::ClangASTImporter m_ast_importer; + ClangTypeToUidMap m_forward_decl_clang_type_to_uid; }; #endif // LLDB_PLUGINS_SYMBOLFILE_PDB_PDBASTPARSER_H Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp =================================================================== --- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp +++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp @@ -9,10 +9,13 @@ #include "PDBASTParser.h" +#include "SymbolFilePDB.h" + #include "clang/AST/CharUnits.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "lldb/Core/Module.h" #include "lldb/Symbol/ClangASTContext.h" #include "lldb/Symbol/ClangUtil.h" #include "lldb/Symbol/Declaration.h" @@ -199,6 +202,19 @@ decl.SetLine(first_line_up->getLineNumber()); return true; } + +AccessType TranslateUdtAccess(PDB_MemberAccess access) { + switch (access) { + case PDB_MemberAccess::Private: + return eAccessPrivate; + case PDB_MemberAccess::Protected: + return eAccessProtected; + case PDB_MemberAccess::Public: + return eAccessPublic; + default: + return eAccessNone; + } +} } // namespace PDBASTParser::PDBASTParser(lldb_private::ClangASTContext &ast) : m_ast(ast) {} @@ -232,10 +248,17 @@ tu_decl_ctx, access, udt->getName().c_str(), tag_type_kind, lldb::eLanguageTypeC_plus_plus, nullptr); + ClangASTContext::StartTagDeclarationDefinition(clang_type); + + auto clang_type_removed_fast_quals = + ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(); + auto sym_uid = type.getSymIndexId(); + m_forward_decl_clang_type_to_uid[clang_type_removed_fast_quals] = sym_uid; + m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true); return std::make_shared( - type.getSymIndexId(), m_ast.GetSymbolFile(), + sym_uid, m_ast.GetSymbolFile(), ConstString(udt->getName()), udt->getLength(), nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, clang_type, lldb_private::Type::eResolveStateForward); @@ -471,6 +494,46 @@ return nullptr; } +bool PDBASTParser::CompleteTypeFromPDB( + lldb_private::CompilerType &compiler_type) { + if (GetClangASTImporter().CanImport(compiler_type)) + return GetClangASTImporter().CompleteType(compiler_type); + + CompilerType compiler_type_no_qualifiers = + ClangUtil::RemoveFastQualifiers(compiler_type); + auto uid_it = m_forward_decl_clang_type_to_uid.find( + compiler_type_no_qualifiers.GetOpaqueQualType()); + if (uid_it == m_forward_decl_clang_type_to_uid.end()) + return true; + + auto symbol_file = static_cast(m_ast.GetSymbolFile()); + if (!symbol_file) + return false; + + std::unique_ptr symbol = + symbol_file->GetPDBSession().getSymbolById(uid_it->getSecond()); + if (!symbol) + return false; + + m_forward_decl_clang_type_to_uid.erase(uid_it); + + ClangASTContext::SetHasExternalStorage(compiler_type.GetOpaqueQualType(), + false); + + switch (symbol->getSymTag()) { + case PDB_SymType::UDT: { + auto udt = llvm::dyn_cast(symbol.get()); + if (!udt) + return false; + + return CompleteTypeFromUDT(*symbol_file, compiler_type, *udt); + } + default: + assert(false && "not a forward clang type decl!"); + return false; + } +} + bool PDBASTParser::AddEnumValue(CompilerType enum_type, const PDBSymbolData &enum_value) const { Declaration decl; @@ -513,3 +576,142 @@ enum_type.GetOpaqueQualType(), underlying_type, decl, name.c_str(), raw_value, byte_size * 8); } + +bool PDBASTParser::CompleteTypeFromUDT( + lldb_private::SymbolFile &symbol_file, + lldb_private::CompilerType &compiler_type, + llvm::pdb::PDBSymbolTypeUDT &udt) { + ClangASTImporter::LayoutInfo layout_info; + layout_info.bit_size = udt.getLength() * 8; + + auto kind = TranslateUdtKind(udt.getUdtKind()); + if (kind == -1) + kind = clang::TTK_Class; + m_ast.SetTagTypeKind(ClangUtil::GetQualType(compiler_type), kind); + + auto members_enum = udt.findAllChildren(); + if (members_enum) + AddRecordMembers(symbol_file, compiler_type, *members_enum, layout_info); + + auto bases_enum = udt.findAllChildren(); + if (bases_enum) + AddRecordBases(symbol_file, compiler_type, kind, *bases_enum, layout_info); + + m_ast.AddMethodOverridesForCXXRecordType(compiler_type.GetOpaqueQualType()); + ClangASTContext::BuildIndirectFields(compiler_type); + ClangASTContext::CompleteTagDeclarationDefinition(compiler_type); + + clang::CXXRecordDecl *record_decl = + m_ast.GetAsCXXRecordDecl(compiler_type.GetOpaqueQualType()); + if (!record_decl) + return static_cast(compiler_type); + + GetClangASTImporter().InsertRecordDecl(record_decl, layout_info); + + return static_cast(compiler_type); +} + +void PDBASTParser::AddRecordMembers( + lldb_private::SymbolFile &symbol_file, + lldb_private::CompilerType &record_type, + PDBDataSymbolEnumerator &members_enum, + lldb_private::ClangASTImporter::LayoutInfo &layout_info) const { + while (auto member = members_enum.getNext()) { + auto member_name = member->getName(); + + auto member_type = symbol_file.ResolveTypeUID(member->getTypeId()); + if (!member_type) + continue; + + auto member_comp_type = member_type->GetLayoutCompilerType(); + if (!member_comp_type.GetCompleteType()) { + symbol_file.GetObjectFile()->GetModule()->ReportError( + ":: Class '%s' has a member '%s' of type '%s' " + "which does not have a complete definition.", + record_type.GetTypeName().GetCString(), member_name.c_str(), + member_comp_type.GetTypeName().GetCString()); + if (ClangASTContext::StartTagDeclarationDefinition(member_comp_type)) + ClangASTContext::CompleteTagDeclarationDefinition(member_comp_type); + } + + auto access = TranslateUdtAccess(member->getAccess()); + if (access == eAccessNone) + access = eAccessPublic; + + auto location_type = member->getLocationType(); + if (location_type == PDB_LocType::ThisRel || + location_type == PDB_LocType::BitField) { + auto bit_size = member->getLength(); + if (location_type == PDB_LocType::ThisRel) + bit_size *= 8; + + auto decl = ClangASTContext::AddFieldToRecordType( + record_type, member_name.c_str(), member_comp_type, access, bit_size); + if (!decl) + continue; + + auto offset = member->getOffset() * 8; + if (location_type == PDB_LocType::BitField) + offset += member->getBitPosition(); + + layout_info.field_offsets.insert(std::make_pair(decl, offset)); + } else + ClangASTContext::AddVariableToRecordType(record_type, member_name.c_str(), + member_comp_type, access); + } +} + +void PDBASTParser::AddRecordBases( + lldb_private::SymbolFile &symbol_file, + lldb_private::CompilerType &record_type, int record_kind, + PDBBaseClassSymbolEnumerator &bases_enum, + lldb_private::ClangASTImporter::LayoutInfo &layout_info) const { + std::vector base_classes; + while (auto base = bases_enum.getNext()) { + auto base_type = symbol_file.ResolveTypeUID(base->getTypeId()); + if (!base_type) + continue; + + auto base_comp_type = base_type->GetLayoutCompilerType(); + if (!base_comp_type.GetCompleteType()) { + symbol_file.GetObjectFile()->GetModule()->ReportError( + ":: Class '%s' has a base class '%s' " + "which does not have a complete definition.", + record_type.GetTypeName().GetCString(), + base_comp_type.GetTypeName().GetCString()); + if (ClangASTContext::StartTagDeclarationDefinition(base_comp_type)) + ClangASTContext::CompleteTagDeclarationDefinition(base_comp_type); + } + + auto access = TranslateUdtAccess(base->getAccess()); + if (access == eAccessNone) + access = eAccessPublic; + + auto is_virtual = base->isVirtualBaseClass(); + + auto base_class_spec = m_ast.CreateBaseClassSpecifier( + base_comp_type.GetOpaqueQualType(), access, is_virtual, + record_kind == clang::TTK_Class); + if (!base_class_spec) + continue; + + base_classes.push_back(base_class_spec); + + if (is_virtual) + continue; + + auto decl = m_ast.GetAsCXXRecordDecl(base_comp_type.GetOpaqueQualType()); + if (!decl) + continue; + + auto offset = clang::CharUnits::fromQuantity(base->getOffset()); + layout_info.base_offsets.insert(std::make_pair(decl, offset)); + } + if (!base_classes.empty()) { + m_ast.SetBaseClassesForClassType(record_type.GetOpaqueQualType(), + &base_classes.front(), + base_classes.size()); + ClangASTContext::DeleteBaseClassSpecifiers(&base_classes.front(), + base_classes.size()); + } +} Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp =================================================================== --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -459,10 +459,13 @@ // This should cause the type to get cached and stored in the `m_types` // lookup. - if (!ResolveTypeUID(symbol->getSymIndexId())) - continue; - - ++num_added; + if (auto type = ResolveTypeUID(symbol->getSymIndexId())) { + // Resolve the type completely to avoid a completion + // (and so a list change, which causes an iterators invalidation) + // during a TypeList dumping + type->GetFullCompilerType(); + ++num_added; + } } } }; @@ -568,8 +571,20 @@ } bool SymbolFilePDB::CompleteType(lldb_private::CompilerType &compiler_type) { - // TODO: Implement this - return false; + std::lock_guard guard( + GetObjectFile()->GetModule()->GetMutex()); + + ClangASTContext *clang_ast_ctx = llvm::dyn_cast_or_null( + GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus)); + if (!clang_ast_ctx) + return false; + + PDBASTParser *pdb = + llvm::dyn_cast(clang_ast_ctx->GetPDBParser()); + if (!pdb) + return false; + + return pdb->CompleteTypeFromPDB(compiler_type); } lldb_private::CompilerDecl SymbolFilePDB::GetDeclForUID(lldb::user_id_t uid) { Index: source/Symbol/ClangASTContext.cpp =================================================================== --- source/Symbol/ClangASTContext.cpp +++ source/Symbol/ClangASTContext.cpp @@ -124,6 +124,84 @@ // Use Clang for D until there is a proper language plugin for it language == eLanguageTypeD; } + +// Checks whether m1 is an overload of m2 (as opposed to an override). This is +// called by addOverridesForMethod to distinguish overrides (which share a +// vtable entry) from overloads (which require distinct entries). +bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) { + // FIXME: This should detect covariant return types, but currently doesn't. + lldbassert(&m1->getASTContext() == &m2->getASTContext() && + "Methods should have the same AST context"); + clang::ASTContext &context = m1->getASTContext(); + + const auto *m1Type = llvm::cast( + context.getCanonicalType(m1->getType())); + + const auto *m2Type = llvm::cast( + context.getCanonicalType(m2->getType())); + + auto compareArgTypes = [&context](const clang::QualType &m1p, + const clang::QualType &m2p) { + return context.hasSameType(m1p.getUnqualifiedType(), + m2p.getUnqualifiedType()); + }; + + // FIXME: In C++14 and later, we can just pass m2Type->param_type_end() + // as a fourth parameter to std::equal(). + return (m1->getNumParams() != m2->getNumParams()) || + !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(), + m2Type->param_type_begin(), compareArgTypes); +} + +// If decl is a virtual method, walk the base classes looking for methods that +// decl overrides. This table of overridden methods is used by IRGen to +// determine the vtable layout for decl's parent class. +void addOverridesForMethod(clang::CXXMethodDecl *decl) { + if (!decl->isVirtual()) + return; + + clang::CXXBasePaths paths; + + auto find_overridden_methods = + [decl](const clang::CXXBaseSpecifier *specifier, + clang::CXXBasePath &path) { + if (auto *base_record = llvm::dyn_cast( + specifier->getType()->getAs()->getDecl())) { + + clang::DeclarationName name = decl->getDeclName(); + + // If this is a destructor, check whether the base class destructor is + // virtual. + if (name.getNameKind() == clang::DeclarationName::CXXDestructorName) + if (auto *baseDtorDecl = base_record->getDestructor()) { + if (baseDtorDecl->isVirtual()) { + path.Decls = baseDtorDecl; + return true; + } else + return false; + } + + // Otherwise, search for name in the base class. + for (path.Decls = base_record->lookup(name); !path.Decls.empty(); + path.Decls = path.Decls.slice(1)) { + if (auto *method_decl = + llvm::dyn_cast(path.Decls.front())) + if (method_decl->isVirtual() && !isOverload(decl, method_decl)) { + path.Decls = method_decl; + return true; + } + } + } + + return false; + }; + + if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) { + for (auto *overridden_decl : paths.found_decls()) + decl->addOverriddenMethod( + llvm::cast(overridden_decl)); + } +} } typedef lldb_private::ThreadSafeDenseMap @@ -8125,6 +8203,13 @@ return cxx_method_decl; } +void ClangASTContext::AddMethodOverridesForCXXRecordType( + lldb::opaque_compiler_type_t type) { + if (auto *record = GetAsCXXRecordDecl(type)) + for (auto *method : record->methods()) + addOverridesForMethod(method); +} + #pragma mark C++ Base Classes clang::CXXBaseSpecifier * @@ -9664,11 +9749,16 @@ llvm::DenseMap &vbase_offsets) { ClangASTContext *ast = (ClangASTContext *)baton; - DWARFASTParserClang *dwarf_ast_parser = - (DWARFASTParserClang *)ast->GetDWARFParser(); - return dwarf_ast_parser->GetClangASTImporter().LayoutRecordType( - record_decl, bit_size, alignment, field_offsets, base_offsets, - vbase_offsets); + lldb_private::ClangASTImporter *importer = nullptr; + if (ast->m_dwarf_ast_parser_ap) + importer = &ast->m_dwarf_ast_parser_ap->GetClangASTImporter(); + if (!importer && ast->m_pdb_ast_parser_ap) + importer = &ast->m_pdb_ast_parser_ap->GetClangASTImporter(); + if (!importer) + return false; + + return importer->LayoutRecordType(record_decl, bit_size, alignment, + field_offsets, base_offsets, vbase_offsets); } //----------------------------------------------------------------------