zturner updated this revision to Diff 172511.
zturner added a comment.
I just added a new test which dumps the clang AST and makes sure no bogus
records get introduced. Since this is already LGTM'ed I'm assuming this is
good to go, but since it now depends on https://reviews.llvm.org/D54072, I
figure I might as well update this diff since I can't commit it yet.
https://reviews.llvm.org/D54053
Files:
lldb/lit/SymbolFile/NativePDB/Inputs/ast-reconstruction.lldbinit
lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp
lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.cpp
lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -177,7 +177,8 @@
lldb::TypeSP CreateProcedureType(PdbSymUid type_uid,
const llvm::codeview::ProcedureRecord &pr);
lldb::TypeSP
- CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name, size_t size,
+ CreateClassStructUnion(PdbSymUid type_uid, llvm::StringRef name,
+ llvm::StringRef unique_name, size_t size,
clang::TagTypeKind ttk,
clang::MSInheritanceAttr::Spelling inheritance);
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -51,6 +51,7 @@
#include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h"
+#include "MangledAST.h"
#include "PdbSymUid.h"
#include "PdbUtil.h"
#include "UdtRecordCompleter.h"
@@ -731,27 +732,11 @@
}
lldb::TypeSP SymbolFileNativePDB::CreateClassStructUnion(
- PdbSymUid type_uid, llvm::StringRef name, size_t size,
- clang::TagTypeKind ttk, clang::MSInheritanceAttr::Spelling inheritance) {
+ PdbSymUid type_uid, llvm::StringRef name, llvm::StringRef unique_name,
+ size_t size, clang::TagTypeKind ttk,
+ clang::MSInheritanceAttr::Spelling inheritance) {
- // Ignore unnamed-tag UDTs.
- name = DropNameScope(name);
- if (name.empty())
- return nullptr;
-
- clang::DeclContext *decl_context = m_clang->GetTranslationUnitDecl();
-
- lldb::AccessType access =
- (ttk == clang::TTK_Class) ? lldb::eAccessPrivate : lldb::eAccessPublic;
-
- ClangASTMetadata metadata;
- metadata.SetUserID(type_uid.toOpaqueId());
- metadata.SetIsDynamicCXXType(false);
-
- CompilerType ct =
- m_clang->CreateRecordType(decl_context, access, name.str().c_str(), ttk,
- lldb::eLanguageTypeC_plus_plus, &metadata);
- lldbassert(ct.IsValid());
+ CompilerType ct = CreateClangDeclFromMangledName(*m_clang, unique_name);
clang::CXXRecordDecl *record_decl =
m_clang->GetAsCXXRecordDecl(ct.GetOpaqueQualType());
@@ -771,7 +756,7 @@
// FIXME: Search IPI stream for LF_UDT_MOD_SRC_LINE.
Declaration decl;
return std::make_shared<Type>(type_uid.toOpaqueId(), m_clang->GetSymbolFile(),
- ConstString(name), size, nullptr,
+ ct.GetTypeName(), size, nullptr,
LLDB_INVALID_UID, Type::eEncodingIsUID, decl,
ct, Type::eResolveStateForward);
}
@@ -782,14 +767,15 @@
clang::MSInheritanceAttr::Spelling inheritance =
GetMSInheritance(m_index->tpi().typeCollection(), cr);
- return CreateClassStructUnion(type_uid, cr.getName(), cr.getSize(), ttk,
- inheritance);
+ return CreateClassStructUnion(type_uid, cr.getName(), cr.getUniqueName(),
+ cr.getSize(), ttk, inheritance);
}
lldb::TypeSP SymbolFileNativePDB::CreateTagType(PdbSymUid type_uid,
const UnionRecord &ur) {
return CreateClassStructUnion(
- type_uid, ur.getName(), ur.getSize(), clang::TTK_Union,
+ type_uid, ur.getName(), ur.getUniqueName(), ur.getSize(),
+ clang::TTK_Union,
clang::MSInheritanceAttr::Spelling::Keyword_single_inheritance);
}
Index: lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h
===================================================================
--- /dev/null
+++ lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.h
@@ -0,0 +1,30 @@
+//===-- MangledAST.h --------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_MANGLEDAST_H
+#define LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_MANGLEDAST_H
+
+namespace clang {
+class TagDecl;
+} // namespace clang
+
+namespace llvm {
+class StringRef;
+}
+
+namespace lldb_private {
+class ClangASTContext;
+class CompilerType;
+namespace npdb {
+CompilerType CreateClangDeclFromMangledName(ClangASTContext &clang,
+ llvm::StringRef mangled);
+} // namespace npdb
+} // namespace lldb_private
+
+#endif // LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_MANGLEDAST_H
Index: lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.cpp
===================================================================
--- /dev/null
+++ lldb/source/Plugins/SymbolFile/NativePDB/MangledAST.cpp
@@ -0,0 +1,91 @@
+//===-- MangledAST.cpp ------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MangledAST.h"
+
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/lldb-enumerations.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/MicrosoftDemangle.h"
+#include "llvm/Demangle/MicrosoftDemangleNodes.h"
+#include "llvm/Demangle/StringView.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::npdb;
+using namespace llvm::ms_demangle;
+
+static std::string render(const Node &node) {
+ OutputStream OS;
+ initializeOutputStream(nullptr, nullptr, OS, 1024);
+ node.output(OS, OF_Default);
+ OS << '\0';
+ return {OS.getBuffer()};
+}
+
+static clang::TagTypeKind TranslateTagKind(TagKind tk) {
+ switch (tk) {
+ case TagKind::Class:
+ return clang::TagTypeKind::TTK_Class;
+ case TagKind::Enum:
+ return clang::TagTypeKind::TTK_Enum;
+ case TagKind::Struct:
+ return clang::TagTypeKind::TTK_Struct;
+ case TagKind::Union:
+ return clang::TagTypeKind::TTK_Union;
+ }
+ llvm_unreachable("Unreachable!");
+}
+
+static clang::DeclContext *CreateDeclContext(ClangASTContext &clang,
+ llvm::ArrayRef<Node *> scope) {
+ clang::DeclContext *decl_context = clang.GetTranslationUnitDecl();
+ if (scope.empty())
+ return decl_context;
+
+ clang::DeclContext *top_level_decl = decl_context;
+ // For now we're just going to pretend that each component of the scope is a
+ // namespace. This isn't actually true, as they could be functions, nested
+ // classes with template parameters, or many other things. But it's a nice
+ // simplifying assumption that lets us make incremental progress while keeping
+ // things simple.
+ for (Node *ns : scope) {
+ std::string name = render(*ns);
+ decl_context =
+ clang.GetUniqueNamespaceDeclaration(name.c_str(), decl_context);
+ }
+ return decl_context;
+}
+
+CompilerType
+lldb_private::npdb::CreateClangDeclFromMangledName(ClangASTContext &clang,
+ llvm::StringRef mangled) {
+ StringView sv(mangled.data(), mangled.size());
+ Demangler demangler;
+ TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
+ if (!ttn)
+ return CompilerType{};
+
+ NodeArrayNode &name_components = *ttn->QualifiedName->Components;
+
+ llvm::ArrayRef<Node *> components(name_components.Nodes,
+ name_components.Count);
+
+ lldb::AccessType access =
+ (ttn->Tag == TagKind::Class) ? lldb::eAccessPrivate : lldb::eAccessPublic;
+
+ std::string name = render(*components.back());
+
+ llvm::ArrayRef<Node *> scope = components.drop_back();
+ clang::TagTypeKind ttk = TranslateTagKind(ttn->Tag);
+ clang::DeclContext *decl_context = CreateDeclContext(clang, scope);
+ return clang.CreateRecordType(decl_context, access, name.c_str(), ttk,
+ lldb::eLanguageTypeC_plus_plus, nullptr);
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
+++ lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
@@ -1,5 +1,6 @@
add_lldb_library(lldbPluginSymbolFileNativePDB PLUGIN
CompileUnitIndex.cpp
+ MangledAST.cpp
PdbIndex.cpp
PdbUtil.cpp
SymbolFileNativePDB.cpp
Index: lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp
===================================================================
--- lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp
+++ lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp
@@ -81,31 +81,23 @@
// classes nested in namespaces and inner classes
-// FIXME: LLDB with native pdb plugin doesn't currently resolve nested names
-// correctly, because it requires creating clang::NamespaceDecl or
-// clang::RecordDecl for the outer namespace or classes. PDB doesn't contain
-// sufficient information to distinguish namespace scopes from nested class
-// scopes, so the best we can hope for is a heuristic reconstruction of the
-// clang AST based on demangling the type's unique name. However, this is
-// as-yet unimplemented in the native PDB plugin, so for now all of these will
-// all just look like `S` when LLDB prints them.
auto e = &three<A::B::S*, B::A::S*, A::C::S&>;
-// CHECK: (S *(*)(S *, S &)) e = {{.*}}
+// CHECK: (A::B::S *(*)(B::A::S *, A::C::S &)) e = {{.*}}
auto f = &three<A::C::S&, A::B::S*, B::A::S*>;
-// CHECK: (S &(*)(S *, S *)) f = {{.*}}
+// CHECK: (A::C::S &(*)(A::B::S *, B::A::S *)) f = {{.*}}
auto g = &three<B::A::S*, A::C::S&, A::B::S*>;
-// CHECK: (S *(*)(S &, S *)) g = {{.*}}
+// CHECK: (B::A::S *(*)(A::C::S &, A::B::S *)) g = {{.*}}
// parameter types that are themselves template instantiations.
auto h = &four<TC<void>, TC<int>, TC<TC<int>>, TC<A::B::S>>;
// Note the awkward space in TC<TC<int> >. This is because this is how template
// instantiations are emitted by the compiler, as the fully instantiated name.
// Only via reconstruction of the AST through the mangled type name (see above
// comment) can we hope to do better than this).
-// CHECK: (TC<void> (*)(TC<int>, TC<TC<int> >, S>)) h = {{.*}}
+// CHECK: (TC<void> (*)(TC<int>, TC<struct TC<int>>, TC<struct A::B::S>)) h = {{.*}}
auto i = &nullary<A::B::S>;
-// CHECK: (S (*)()) i = {{.*}}
+// CHECK: (A::B::S (*)()) i = {{.*}}
// Make sure we can handle types that don't have complete debug info.
Index: lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
===================================================================
--- /dev/null
+++ lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
@@ -0,0 +1,124 @@
+// clang-format off
+// REQUIRES: lld
+
+// RUN: clang-cl /Z7 /GS- /GR- /c -Xclang -fkeep-static-consts /Fo%t.obj -- %s
+// RUN: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-NS1 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-NS2 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-TAGRECORD %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-C %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-U %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-S %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/ast-reconstruction.lldbinit 2>&1 | FileCheck \
+// RUN: --check-prefix=NODUPE-E %s
+
+
+struct S {};
+class C {};
+union U {};
+enum class E {
+ A = 1
+};
+
+namespace NS1 {
+ namespace NS2 {
+ struct TagRecord { };
+ }
+
+ struct Tag2 {
+ class TagRecord {};
+ };
+}
+
+struct NS2 {
+ struct NS1 {
+ union TagRecord {};
+ };
+ enum TagRecord {};
+};
+
+S GlobalS;
+C GlobalC;
+U GlobalU;
+E GlobalE = E::A;
+
+NS1::NS2::TagRecord ABR;
+NS1::Tag2::TagRecord ATR;
+NS2::NS1::TagRecord BAR;
+NS2::TagRecord BT;
+
+// CHECK: TranslationUnitDecl
+// CHECK: |-NamespaceDecl {{.*}} NS1
+// CHECK: | |-NamespaceDecl {{.*}} NS2
+// CHECK: | | `-CXXRecordDecl {{.*}} struct TagRecord definition
+// CHECK: | | `-MSInheritanceAttr {{.*}} Implicit __single_inheritance
+// CHECK: | `-NamespaceDecl {{.*}} Tag2
+// CHECK: | `-CXXRecordDecl {{.*}} class TagRecord definition
+// CHECK: | `-MSInheritanceAttr {{.*}} Implicit __single_inheritance
+// CHECK: |-NamespaceDecl {{.*}} NS2
+// CHECK: | `-NamespaceDecl {{.*}} NS1
+// CHECK: | `-CXXRecordDecl {{.*}} union TagRecord definition
+// CHECK: | `-MSInheritanceAttr {{.*}} Implicit __single_inheritance
+// CHECK: |-EnumDecl {{.*}} TagRecord
+// CHECK: |-CXXRecordDecl {{.*}} class C definition
+// CHECK: | `-MSInheritanceAttr {{.*}} Implicit __single_inheritance
+// CHECK: |-CXXRecordDecl {{.*}} union U definition
+// CHECK: | `-MSInheritanceAttr {{.*}} Implicit __single_inheritance
+// CHECK: |-CXXRecordDecl {{.*}} struct S definition
+// CHECK: | `-MSInheritanceAttr {{.*}} Implicit __single_inheritance
+// CHECK: |-EnumDecl {{.*}} E
+// CHECK: `-<undeserialized declarations>
+
+// NODUPE-NS1: TranslationUnitDecl
+// NODUPE-NS1: |-NamespaceDecl {{.*}} NS1
+// NODUPE-NS1-NOT: |-NamespaceDecl {{.*}} NS1
+// NODUPE-NS1: `-<undeserialized declarations>
+
+// NODUPE-NS2: TranslationUnitDecl
+// NODUPE-NS2: |-NamespaceDecl {{.*}} NS1
+// NODUPE-NS2-NOT: |-NamespaceDecl {{.*}} NS1
+// NODUPE-NS2: `-<undeserialized declarations>
+
+// NODUPE-TAGRECORD: TranslationUnitDecl
+// NODUPE-TAGRECORD: | | `-CXXRecordDecl {{.*}} struct TagRecord definition
+// NODUPE-TAGRECORD-NOT: | | `-CXXRecordDecl {{.*}} struct TagRecord definition
+// NODUPE-TAGRECORD: `-<undeserialized declarations>
+
+// NODUPE-C: TranslationUnitDecl
+// NODUPE-C: |-CXXRecordDecl {{.*}} class C definition
+// NODUPE-C-NOT: |-CXXRecordDecl {{.*}} class C definition
+// NODUPE-C: `-<undeserialized declarations>
+
+// NODUPE-U: TranslationUnitDecl
+// NODUPE-U: |-CXXRecordDecl {{.*}} union U definition
+// NODUPE-U-NOT: |-CXXRecordDecl {{.*}} union U definition
+// NODUPE-U: `-<undeserialized declarations>
+
+// NODUPE-S: TranslationUnitDecl
+// NODUPE-S: |-CXXRecordDecl {{.*}} struct S definition
+// NODUPE-S-NOT: |-CXXRecordDecl {{.*}} struct S definition
+// NODUPE-S: `-<undeserialized declarations>
+
+// NODUPE-E: TranslationUnitDecl
+// NODUPE-E: |-EnumDecl {{.*}} E
+// NODUPE-E-NOT: |-EnumDecl {{.*}} E
+// NODUPE-E: `-<undeserialized declarations>
+
+int main(int argc, char **argv) {
+ return 0;
+}
Index: lldb/lit/SymbolFile/NativePDB/Inputs/ast-reconstruction.lldbinit
===================================================================
--- /dev/null
+++ lldb/lit/SymbolFile/NativePDB/Inputs/ast-reconstruction.lldbinit
@@ -0,0 +1,23 @@
+target variable ABR
+target variable ATR
+target variable BAR
+target variable BT
+target variable GlobalC
+target variable GlobalU
+target variable GlobalS
+target variable GlobalE
+
+target modules dump ast
+
+target variable ABR
+target variable ATR
+target variable BAR
+target variable BT
+target variable GlobalC
+target variable GlobalU
+target variable GlobalS
+target variable GlobalE
+
+target modules dump ast
+
+quit
\ No newline at end of file
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
