[PDB] Parse UDT symbols and pointers to members (combined patch)

[aleksandr.urakov]

Summary

In this patch I've tried to combine the best ideas from D49368 and D49410, so it implements following:

• Completion of UDTs from a PDB with a filling of a layout info;
• Pointers to members;
• Fixes the bug relating to a virtual base offset reading from vbtable. The offset was treated as an unsigned, but it can be a negative sometimes.

[aleksandr.urakov]

lit/SymbolFile/PDB/udt-layout.test

1–51 ↗

(On Diff #157945)

I've preserved this Windows-only test (but also have included other non-execution tests, which may become cross-platform some later, as Pavel has mentioned at D49410), but if you'll find it unnecessary I'll remove it.

[aleksandr.urakov]

Added support of a MSInheritance attribute.

[aleksandr.urakov]

Ping!

Can you review this, please?

[zturner]

I am OOO this week and only have access to mobile, so hopefully someone
else can review it

[aleksandr.urakov]

Unfortunately, there was no people yet, who can review this :)

Ping! Can anyone review this, please?

[zturner]

Ok I’ll take a look later today then when i get in

[aleksandr.urakov]

Thanks a lot!

[teemperor]

@aleksandr.urakov Not sure if you are already working of this, but just FYI this patch introduced a few compiler warnings:

/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:55:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]       
  default:                                                                                                                                                                                                 
  ^                                                                                                                                                                                                        
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:216:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]      
  default:                                                                                                                                                                                                 
  ^                                                                                                                                                                                                        
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:229:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]      
  default:                                                                                                                                                                                                 
  ^                                                                                                                                                                                                        
3 warnings generated.
 20% [125/610] Building CXX object tools/lldb/source/Symbol/CMakeFiles/lldbSymbol.dir/ClangASTContext.cpp.o                                                                                               
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:6547:45: warning: comparison of integers of different signs: 'const int32_t' (aka 'const int') and 'unsigned int' [-Wsig$-compare]
                            if (base_offset != UINT32_MAX) {
                                ~~~~~~~~~~~ ^  ~~~~~~~~~~
/Users/teemperor/llvm/sidestuff/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:6760:34: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int' [-Wsign-compare]  
                  if (bit_offset == UINT32_MAX)
                      ~~~~~~~~~~ ^  ~~~~~~~~~~
2 warnings generated.

[aleksandr.urakov]

Thanks for catching that!

I didn't know about that warnings because I build LLVM with MSVC compiler, but it doesn't introduce such warnings. Now I've fixed the problem with rLLDB339994.

[stella.stamenova]

I am not 100% sure yet, but this change is the most likely culprit for one of the SymbolFile tests to start failing, namely: SymbolFile/DWARF/dwarf5-index-is-used.cpp. This fails for us on both Windows and Linux with the following:

FAIL: lldb :: SymbolFile/DWARF/dwarf5-index-is-used.cpp (42214 of 43490)
******************** TEST 'lldb :: SymbolFile/DWARF/dwarf5-index-is-used.cpp' FAILED ********************
Script:
--
: 'RUN: at line 5';   clang /vstsdrive/_work/11/s/llvm/tools/lldb/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp -g -c -o /vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp.o --target=x86_64-pc-linux -mllvm -accel-tables=Dwarf
: 'RUN: at line 6';   ld.lld /vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp.o -o /vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp
: 'RUN: at line 7';   /vstsdrive/_work/11/b/LLVMBuild/bin/lldb-test symbols /vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp | /vstsdrive/_work/11/b/LLVMBuild/bin/FileCheck /vstsdrive/_work/11/s/llvm/tools/lldb/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
--
Exit Code: 1

Command Output (stderr):
--
/vstsdrive/_work/11/s/llvm/tools/lldb/lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp:9:11: error: CHECK: expected string not found in input
// CHECK: Name Index
          ^
<stdin>:1:1: note: scanning from here
Module: /vstsdrive/_work/11/b/LLVMBuild/tools/lldb/lit/SymbolFile/DWARF/Output/dwarf5-index-is-used.cpp.tmp
^
<stdin>:36:5: note: possible intended match here
IDX name type flags addr offset size link info addralgn entsize Name
    ^

--

********************

I tried looking through the results from the bots to see if the test is passing online and as far as I can tell, none of the bots that are currently online have ran it or if they did, the results got lost because of the failures from the new vscode tests.

[aleksandr.urakov]

Yes, I've seen today that this test is failing now, but if I'm not mistaken this have happened some later than this patch was committed. But I'm not sure, I'll look at this more precisely tomorrow, thanks!

[aleksandr.urakov]

It's also possible that the test has been failing since some commit in another repository (e.g. clang, lld or llvm). It seems that it was ok several days ago...

[stella.stamenova]

In D49980#1208096, @aleksandr.urakov wrote:

It's also possible that the test has been failing since some commit in another repository (e.g. clang, lld or llvm). It seems that it was ok several days ago...

I think you are right. I've been trying to isolate when the failure started happening and it appears to be one of the changes made on Aug. 20 in one of the repositories (still trying to narrow that down).

[aleksandr.urakov]

It seems that the cause of the failure is rL340206, but I'm not sure if the adding of -gpubnames switch to clang will be a correct fix for that?

lldb/trunk/include/lldb/Symbol/ClangASTContext.h

#define LLDB_INVALID_DECL_LEVEL UINT32_MAX
   clang::CXXMethodDecl *
   AddMethodToCXXRecordType(lldb::opaque_compiler_type_t type, const char *name,
                            const char *mangled_name,
                            const CompilerType &method_type,
                            lldb::AccessType access, bool is_virtual,
                            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,
                            lldb::AccessType access, bool is_virtual,
                            bool base_of_class);
 
   static void DeleteBaseClassSpecifiers(clang::CXXBaseSpecifier **base_classes,
                                         unsigned num_base_classes);

lldb/trunk/lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

+// To avoid linking MSVC specific libs, we don't test virtual/override methods
+// that needs vftable support in this file.
+
+// Enum.
+enum Enum { RED, GREEN, BLUE };
+Enum EnumVar;
+
+// Union.
+union Union {
+  short Row;
+  unsigned short Col;
+  int Line : 16; // Test named bitfield.
+  short : 8;     // Unnamed bitfield symbol won't be generated in PDB.
+  long Table;
+};
+Union UnionVar;
+
+// Struct.
+struct Struct;
+typedef Struct StructTypedef;
+
+struct Struct {
+  bool A;
+  unsigned char UCharVar;
+  unsigned int UIntVar;
+  long long LongLongVar;
+  Enum EnumVar; // Test struct has UDT member.
+  int array[10];
+};
+struct Struct StructVar;
+
+struct _List; // Forward declaration.
+struct Complex {
+  struct _List *array[90];
+  struct { // Test unnamed struct. MSVC treats it as `int x`
+    int x;
+  };
+  union { // Test unnamed union. MSVC treats it as `int a; float b;`
+    int a;
+    float b;
+  };
+};
+struct Complex c;
+
+struct _List { // Test doubly linked list.
+  struct _List *current;
+  struct _List *previous;
+  struct _List *next;
+};
+struct _List ListVar;
+
+typedef struct {
+  int a;
+} UnnamedStruct; // Test unnamed typedef-ed struct.
+UnnamedStruct UnnanmedVar;
+
+// Class.
+namespace MemberTest {
+class Base {
+public:
+  Base() {}
+  ~Base() {}
+
+public:
+  int Get() { return 0; }
+
+protected:
+  int a;
+};
+class Friend {
+public:
+  int f() { return 3; }
+};
+class Class : public Base { // Test base class.
+  friend Friend;
+  static int m_static; // Test static member variable.
+public:
+  Class() : m_public(), m_private(), m_protected() {}
+  explicit Class(int a) { m_public = a; } // Test first reference of m_public.
+  ~Class() {}
+
+  static int StaticMemberFunc(int a, ...) {
+    return 1;
+  } // Test static member function.
+  int Get() { return 1; }
+  int f(Friend c) { return c.f(); }
+  inline bool operator==(const Class &rhs) const // Test operator.
+  {
+    return (m_public == rhs.m_public);
+  }
+
+public:
+  int m_public;
+  struct Struct m_struct;
+
+private:
+  Union m_union;
+  int m_private;
+
+protected:
+  friend class Friend;
+  int m_protected;
+};
+} // namespace MemberTest
+
+int main() {
+  MemberTest::Base B1;
+  B1.Get();
+  MemberTest::Class::StaticMemberFunc(1, 10, 2);
+  return 0;
+}

lldb/trunk/lit/SymbolFile/PDB/Inputs/PointerTypeTest.cpp

+int main() {
+  // Test pointer to array.
+  int array[2][4];
+  int(*array_pointer)[2][4] = &array;
+
+  struct ST {
+    int a;
+    int f(int x) { return 1; }
+  };
+
+  ST s = {10};
+
+  // Test pointer to a local.
+  int *p_int = &s.a;
+
+  // Test pointer to data member.
+  int ST::*p_member_field = &ST::a;
+
+  // Test pointer to member function.
+  int (ST::*p_member_method)(int) = &ST::f;
+
+  return 0;
+}

lldb/trunk/lit/SymbolFile/PDB/Inputs/UdtLayoutTest.cpp

+struct A {
+  explicit A(int u) { _u._u3 = u; }
+  A(const A &) = default;
+  virtual ~A() = default;
+
+private:
+  union U {
+    char _u1;
+    short _u2;
+    int _u3;
+  };
+
+  A::U _u;
+};
+
+#pragma pack(push, 1)
+template <int I> struct B : public virtual A {
+  B(char a, unsigned short b, int c) : A(a + b + c), _a(a), _b(b), _c(c) {}
+
+private:
+  char _a;
+  unsigned short : 3;
+  unsigned short _b : 6;
+  unsigned short : 4;
+  int _c;
+};
+#pragma pack(pop)
+
+#pragma pack(push, 16)
+class C : private virtual B<0>, public virtual B<1>, private B<2>, public B<3> {
+public:
+  C(char x, char y, char z)
+      : A(x - y + z), B<0>(x, y, z), B<1>(x * 2, y * 2, z * 2),
+        B<2>(x * 3, y * 3, z * 3), B<3>(x * 4, y * 4, z * 4), _x(x * 5),
+        _y(y * 5), _z(z * 5) {}
+
+  static int abc;
+
+private:
+  int _x;
+  short _y;
+  char _z;
+};
+int C::abc = 123;
+#pragma pack(pop)
+
+class List {
+public:
+  List() = default;
+  List(List *p, List *n, C v) : Prev(p), Next(n), Value(v) {}
+
+private:
+  List *Prev = nullptr;
+  List *Next = nullptr;
+  C Value{1, 2, 3};
+};
+
+int main() {
+  List ls[16];
+  return 0;
+}

lldb/trunk/lit/SymbolFile/PDB/Inputs/UdtLayoutTest.script

+breakpoint set --file UdtLayoutTest.cpp --line 60
+run
+target variable
+frame variable

lldb/trunk/lit/SymbolFile/PDB/class-layout.test

+REQUIRES: windows
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o %T/ClassLayoutTest.cpp.obj
+RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/ClassLayoutTest.cpp.exe
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=ENUM %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=UNION %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=STRUCT %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=COMPLEX %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=LIST %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=UNNAMED-STRUCT %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=BASE %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=FRIEND %s
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck --check-prefix=CLASS %s
+
+CHECK: Module [[MOD:.*]]
+CHECK: {{^[0-9A-F]+}}: SymbolVendor ([[MOD]])
+CHECK: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\ClassLayoutTest.cpp'
+
+ENUM:  name = "Enum", size = 4,  decl = ClassLayoutTest.cpp:5
+ENUM-SAME: enum Enum {
+ENUM:    RED,
+ENUM:    GREEN,
+ENUM:    BLUE
+ENUM:}
+
+UNION:  name = "Union", size = 4, decl = ClassLayoutTest.cpp:9
+UNION-SAME: union Union {
+UNION:    short Row;
+UNION:    unsigned short Col;
+UNION:    int Line : 16;
+UNION:    long Table;
+UNION:}
+
+STRUCT:  name = "Struct", size = 64, decl = ClassLayoutTest.cpp:22
+STRUCT-SAME: struct Struct {
+STRUCT:    bool A;
+STRUCT:    unsigned char UCharVar;
+STRUCT:    unsigned int UIntVar;
+STRUCT:    long long LongLongVar;
+STRUCT:    Enum EnumVar;
+STRUCT:    int array[10];
+STRUCT:}
+
+COMPLEX:  name = "Complex", size = 368, decl = ClassLayoutTest.cpp:33
+COMPLEX-SAME: struct Complex {
+COMPLEX:    _List *array[90];
+COMPLEX:    int x;
+COMPLEX:    int a;
+COMPLEX:    float b;
+COMPLEX:}
+
+LIST:  name = "_List", size = 12, decl = ClassLayoutTest.cpp:45
+LIST-SAME: struct _List {
+LIST:    _List *current;
+LIST:    _List *previous;
+LIST:    _List *next;
+LIST:}
+
+UNNAMED-STRUCT:  name = "UnnamedStruct", size = 4, decl = ClassLayoutTest.cpp:52
+UNNAMED-STRUCT-SAME: struct UnnamedStruct {
+UNNAMED-STRUCT:   int a;
+UNNAMED-STRUCT:}
+
+BASE:  name = "MemberTest::Base", size = 4,  decl = ClassLayoutTest.cpp:59
+BASE-SAME: class MemberTest::Base {
+BASE:    int a;
+BASE:    void {{.*}}Base();
+BASE:    {{.*}}~Base();
+BASE:    int {{.*}}Get();
+BASE:}
+
+FRIEND:  name = "MemberTest::Friend", size = 1, decl = ClassLayoutTest.cpp:70
+FRIEND-SAME: class MemberTest::Friend {
+FRIEND:    int f();
+FRIEND: }
+
+CLASS:  name = "MemberTest::Class", size = 88, decl = ClassLayoutTest.cpp:74
+CLASS-SAME: class MemberTest::Class : public MemberTest::Base {
+CLASS:    static int m_static;
+CLASS:    int m_public;
+CLASS:    Struct m_struct;
+CLASS:    Union m_union;
+CLASS:    int m_private;
+CLASS:    int m_protected;
+CLASS:    void Class();
+CLASS:    void Class(int);
+CLASS:    ~MemberTest::Class();
+CLASS:    static int {{.*}}StaticMemberFunc(int, ...);
+CLASS:    int Get();
+CLASS:    int f(MemberTest::Friend);
+CLASS:    bool operator==(const MemberTest::Class &)
+CLASS:}

lldb/trunk/lit/SymbolFile/PDB/pointers.test

+REQUIRES: windows
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/PointerTypeTest.cpp /o %T/PointerTypeTest.cpp.obj
+RUN: link %T/PointerTypeTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/PointerTypeTest.cpp.exe
+RUN: lldb-test symbols %T/PointerTypeTest.cpp.exe | FileCheck %s
+RUN: lldb-test symbols %T/PointerTypeTest.cpp.exe | FileCheck --check-prefix=MAIN-ST-F %s
+RUN: lldb-test symbols %T/PointerTypeTest.cpp.exe | FileCheck --check-prefix=MAIN-ST %s
+RUN: lldb-test symbols %T/PointerTypeTest.cpp.exe | FileCheck --check-prefix=MAIN %s
+RUN: lldb-test symbols %T/PointerTypeTest.cpp.exe | FileCheck --check-prefix=F %s
+
+CHECK: Module [[MOD:.*]]
+CHECK: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\PointerTypeTest.cpp'
+
+MAIN-ST-F:  name = "main::ST::f"
+MAIN-ST-F-SAME: decl = PointerTypeTest.cpp:8
+MAIN-ST-F-SAME: compiler_type = {{.*}} int (int)
+
+MAIN-ST:  name = "main::ST", size = 4, decl = PointerTypeTest.cpp:6, compiler_type = {{.*}} struct main::ST {
+MAIN-ST-NEXT: int a;
+MAIN-ST-NEXT: int {{.*}}f(int);
+MAIN-ST-NEXT:}
+
+MAIN:   Function{[[FID1:.*]]}, mangled = _main
+MAIN-NEXT:  Block{[[FID1]]}
+MAIN:     Variable{{.*}}, name = "array_pointer"
+MAIN-SAME:    (int (*)[2][4]), scope = local
+MAIN:     Variable{{.*}}, name = "p_int"
+MAIN-SAME:    (int *), scope = local
+MAIN:     Variable{{.*}}, name = "p_member_field"
+MAIN-SAME:    (int main::ST::*), scope = local
+MAIN:     Variable{{.*}}, name = "p_member_method"
+MAIN-SAME:    (int (main::ST::*)(int)), scope = local
+
+F:   Function{[[FID2:.*]]}, demangled = {{.*}}f(int)
+F-NEXT:  Block{[[FID2]]}
+F:     Variable{{.*}}, name = "this"
+F-SAME:    (main::ST *), scope = parameter, location = {{.*}}, artificial
+F:     Variable{{.*}}, name = "x"
+F-SAME:    (int), scope = parameter, decl = PointerTypeTest.cpp:8

lldb/trunk/lit/SymbolFile/PDB/udt-layout.test

+REQUIRES: windows
+RUN: clang-cl /Zi %S/Inputs/UdtLayoutTest.cpp /o %t.exe
+RUN: %lldb -b -s %S/Inputs/UdtLayoutTest.script -- %t.exe | FileCheck %s
+
+CHECK:(int) int C::abc = 123
+CHECK:(List [16]) ls = {
+CHECK:  [15] = {
+CHECK:    Prev = 0x00000000
+CHECK:    Next = 0x00000000
+CHECK:    Value = {
+CHECK:      B<0> = {
+CHECK:        A = {
+CHECK:          _u = (_u1 = '\x02', _u2 = 2, _u3 = 2)
+CHECK:        }
+CHECK:        _a = '\x01'
+CHECK:        _b = 2
+CHECK:        _c = 3
+CHECK:      }
+CHECK:      B<1> = {
+CHECK:        A = {
+CHECK:          _u = (_u1 = '\x02', _u2 = 2, _u3 = 2)
+CHECK:        }
+CHECK:        _a = '\x02'
+CHECK:        _b = 4
+CHECK:        _c = 6
+CHECK:      }
+CHECK:      B<2> = {
+CHECK:        A = {
+CHECK:          _u = (_u1 = '\x02', _u2 = 2, _u3 = 2)
+CHECK:        }
+CHECK:        _a = '\x03'
+CHECK:        _b = 6
+CHECK:        _c = 9
+CHECK:      }
+CHECK:      B<3> = {
+CHECK:        A = {
+CHECK:          _u = (_u1 = '\x02', _u2 = 2, _u3 = 2)
+CHECK:        }
+CHECK:        _a = '\x04'
+CHECK:        _b = 8
+CHECK:        _c = 12
+CHECK:      }
+CHECK:      A = {
+CHECK:        _u = (_u1 = '\x02', _u2 = 2, _u3 = 2)
+CHECK:      }
+CHECK:      _x = 5
+CHECK:      _y = 10
+CHECK:      _z = '\x0f'
+CHECK:    }
+CHECK:  }
+CHECK:}

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

default:
       break;
     }
   }
   if (template_param_infos.args.empty())
     return false;
   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<clang::FunctionProtoType>(
-      context.getCanonicalType(m1->getType()));
-
-  const auto *m2Type =
-    llvm::cast<clang::FunctionProtoType>(
-      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<clang::CXXRecordDecl>(
-            specifier->getType()->getAs<clang::RecordType>()->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<clang::CXXMethodDecl>(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<clang::CXXMethodDecl>(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) {
   SymbolFileDWARF *dwarf = die.GetDWARF();
 
   std::lock_guard<std::recursive_mutex> guard(
       dwarf->GetObjectFile()->GetModule()->GetMutex());
 
ClangASTImporter::LayoutInfo layout_info;
           // Clang will copy each CXXBaseSpecifier in "base_classes" so we have
           // to free them all.
           ClangASTContext::DeleteBaseClassSpecifiers(&base_classes.front(),
                                                      base_classes.size());
         }
       }
     }
 
-    addMethodOverrides(m_ast, clang_type);
+    m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType());
     ClangASTContext::BuildIndirectFields(clang_type);
     ClangASTContext::CompleteTagDeclarationDefinition(clang_type);
 
     if (!layout_info.field_offsets.empty() ||
         !layout_info.base_offsets.empty() ||
         !layout_info.vbase_offsets.empty()) {
       if (type)
         layout_info.bit_size = type->GetByteSize() * 8;

lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.h

 
 namespace lldb_private {
 class ClangASTContext;
 class CompilerType;
 } // namespace lldb_private
 
 namespace llvm {
 namespace pdb {
+template <typename ChildType> class ConcreteSymbolEnumerator;
+
 class PDBSymbol;
 class PDBSymbolData;
+class PDBSymbolFunc;
+class PDBSymbolTypeBaseClass;
 class PDBSymbolTypeBuiltin;
+class PDBSymbolTypeUDT;
 } // namespace pdb
 } // namespace llvm
 
 class PDBASTParser {
 public:
   PDBASTParser(lldb_private::ClangASTContext &ast);
   ~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<lldb::opaque_compiler_type_t, lldb::user_id_t>
+      ClangTypeToUidMap;
+  typedef llvm::pdb::ConcreteSymbolEnumerator<llvm::pdb::PDBSymbolData>
+      PDBDataSymbolEnumerator;
+  typedef llvm::pdb::ConcreteSymbolEnumerator<llvm::pdb::PDBSymbolTypeBaseClass>
+      PDBBaseClassSymbolEnumerator;
+  typedef llvm::pdb::ConcreteSymbolEnumerator<llvm::pdb::PDBSymbolFunc>
+      PDBFuncSymbolEnumerator;
+
   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;
+  void AddRecordMethods(
+      lldb_private::SymbolFile &symbol_file,
+      lldb_private::CompilerType &record_type,
+      PDBFuncSymbolEnumerator &methods_enum) 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

lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

 //===-- PDBASTParser.cpp ----------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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/ClangExternalASTSourceCommon.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/Declaration.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeSystem.h"
 
 #include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
 #include "llvm/DebugInfo/PDB/PDBSymbol.h"
int TranslateUdtKind(PDB_UdtType pdb_kind) {
   case PDB_UdtType::Class:
     return clang::TTK_Class;
   case PDB_UdtType::Struct:
     return clang::TTK_Struct;
   case PDB_UdtType::Union:
     return clang::TTK_Union;
   case PDB_UdtType::Interface:
     return clang::TTK_Interface;
+  default:
+    llvm_unreachable("unsuported PDB UDT type");
   }
-  return -1;
 }
 
 lldb::Encoding TranslateBuiltinEncoding(PDB_BuiltinType type) {
   switch (type) {
   case PDB_BuiltinType::Float:
     return lldb::eEncodingIEEE754;
   case PDB_BuiltinType::Int:
   case PDB_BuiltinType::Long:
if (!src_file_up)
     return false;
 
   FileSpec spec(src_file_up->getFileName(), /*resolve_path*/ false);
   decl.SetFile(spec);
   decl.SetColumn(first_line_up->getColumnNumber());
   decl.SetLine(first_line_up->getLineNumber());
   return true;
 }
+
+AccessType TranslateMemberAccess(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;
+  }
+}
+
+AccessType GetDefaultAccessibilityForUdtKind(PDB_UdtType udt_kind) {
+  switch (udt_kind) {
+  case PDB_UdtType::Struct:
+  case PDB_UdtType::Union:
+    return eAccessPublic;
+  case PDB_UdtType::Class:
+  case PDB_UdtType::Interface:
+    return eAccessPrivate;
+  default:
+    llvm_unreachable("unsupported PDB UDT type");
+  }
+}
+
+AccessType GetAccessibilityForUdt(const PDBSymbolTypeUDT &udt) {
+  AccessType access = TranslateMemberAccess(udt.getAccess());
+  if (access != lldb::eAccessNone || !udt.isNested())
+    return access;
+
+  auto parent = udt.getClassParent();
+  if (!parent)
+    return lldb::eAccessNone;
+
+  auto parent_udt = llvm::dyn_cast<PDBSymbolTypeUDT>(parent.get());
+  if (!parent_udt)
+    return lldb::eAccessNone;
+
+  return GetDefaultAccessibilityForUdtKind(parent_udt->getUdtKind());
+}
+
+clang::MSInheritanceAttr::Spelling GetMSInheritance(
+    const PDBSymbolTypeUDT &udt) {
+  int base_count = 0;
+  bool has_virtual = false;
+
+  auto bases_enum = udt.findAllChildren<PDBSymbolTypeBaseClass>();
+  if (bases_enum) {
+    while (auto base = bases_enum->getNext()) {
+      base_count++;
+      has_virtual |= base->isVirtualBaseClass();
+    }
+  }
+
+  if (has_virtual)
+    return clang::MSInheritanceAttr::Keyword_virtual_inheritance;
+  if (base_count > 1)
+    return clang::MSInheritanceAttr::Keyword_multiple_inheritance;
+  return clang::MSInheritanceAttr::Keyword_single_inheritance;
+}
 } // namespace
 
 PDBASTParser::PDBASTParser(lldb_private::ClangASTContext &ast) : m_ast(ast) {}
 
 PDBASTParser::~PDBASTParser() {}
 
 // DebugInfoASTParser interface
 
 lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const PDBSymbol &type) {
   // PDB doesn't maintain enough information to robustly rebuild the entire
   // tree, and this is most problematic when it comes to figure out the right
   // DeclContext to put a type in.  So for now, everything goes in the
   // translation unit decl as a fully qualified type.
   clang::DeclContext *tu_decl_ctx = m_ast.GetTranslationUnitDecl();
   Declaration decl;
 
   switch (type.getSymTag()) {
+  case PDB_SymType::BaseClass: {
+    auto symbol_file = m_ast.GetSymbolFile();
+    if (!symbol_file)
+      return nullptr;
+
+    auto ty = symbol_file->ResolveTypeUID(type.getRawSymbol().getTypeId());
+    return ty ? ty->shared_from_this() : nullptr;
+  } break;
   case PDB_SymType::UDT: {
     auto udt = llvm::dyn_cast<PDBSymbolTypeUDT>(&type);
     assert(udt);
-    AccessType access = lldb::eAccessPublic;
-    PDB_UdtType udt_kind = udt->getUdtKind();
-    auto tag_type_kind = TranslateUdtKind(udt_kind);
-    if (tag_type_kind == -1)
+
+    // Note that, unnamed UDT being typedef-ed is generated as a UDT symbol
+    // other than a Typedef symbol in PDB. For example,
+    //    typedef union { short Row; short Col; } Union;
+    // is generated as a named UDT in PDB:
+    //    union Union { short Row; short Col; }
+    // Such symbols will be handled here.
+
+    // Some UDT with trival ctor has zero length. Just ignore.
+    if (udt->getLength() == 0)
       return nullptr;
 
-    if (udt_kind == PDB_UdtType::Class)
-      access = lldb::eAccessPrivate;
+    // Ignore unnamed-tag UDTs.
+    if (udt->getName().empty())
+      return nullptr;
+
+    auto access = GetAccessibilityForUdt(*udt);
+
+    auto tag_type_kind = TranslateUdtKind(udt->getUdtKind());
+
+    ClangASTMetadata metadata;
+    metadata.SetUserID(type.getSymIndexId());
+    metadata.SetIsDynamicCXXType(false);
 
     CompilerType clang_type = m_ast.CreateRecordType(
         tu_decl_ctx, access, udt->getName().c_str(), tag_type_kind,
-        lldb::eLanguageTypeC_plus_plus, nullptr);
+        lldb::eLanguageTypeC_plus_plus, &metadata);
+    assert(clang_type.IsValid());
+
+    if (udt->isConstType())
+      clang_type = clang_type.AddConstModifier();
+
+    if (udt->isVolatileType())
+      clang_type = clang_type.AddVolatileModifier();
+
+    clang::CXXRecordDecl *record_decl =
+      m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
+    assert(record_decl);
+    auto inheritance_attr = clang::MSInheritanceAttr::CreateImplicit(
+        *m_ast.getASTContext(), GetMSInheritance(*udt));
+    record_decl->addAttr(inheritance_attr);
+
+    ClangASTContext::StartTagDeclarationDefinition(clang_type);
+
+    Type::ResolveStateTag type_resolve_state_tag;
+    auto children = udt->findAllChildren();
+    if (!children || children->getChildCount() == 0) {
+      // PDB does not have symbol of forwarder. We assume we get an udt w/o any
+      // fields. Just complete it at this point.
+      ClangASTContext::CompleteTagDeclarationDefinition(clang_type);
+
+      m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false);
+
+      type_resolve_state_tag = Type::eResolveStateFull;
+    } else {
+      // Add the type to the forward declarations. It will help us to avoid
+      // an endless recursion in CompleteTypeFromUdt function.
+      auto clang_type_removed_fast_quals =
+          ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType();
+      m_forward_decl_clang_type_to_uid[clang_type_removed_fast_quals] =
+          type.getSymIndexId();
 
       m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
 
+      type_resolve_state_tag = Type::eResolveStateForward;
+    }
+
+    GetDeclarationForSymbol(type, decl);
     return std::make_shared<lldb_private::Type>(
         type.getSymIndexId(), m_ast.GetSymbolFile(),
         ConstString(udt->getName()), udt->getLength(), nullptr,
         LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, clang_type,
-        lldb_private::Type::eResolveStateForward);
+        type_resolve_state_tag);
   } break;
   case PDB_SymType::Enum: {
     auto enum_type = llvm::dyn_cast<PDBSymbolTypeEnum>(&type);
     assert(enum_type);
     auto underlying_type_up = enum_type->getUnderlyingType();
     if (!underlying_type_up)
       return nullptr;
     lldb::Encoding encoding =
lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const PDBSymbol &type) {
   case PDB_SymType::PointerType: {
     auto *pointer_type = llvm::dyn_cast<PDBSymbolTypePointer>(&type);
     assert(pointer_type);
     Type *pointee_type = m_ast.GetSymbolFile()->ResolveTypeUID(
         pointer_type->getPointeeType()->getSymIndexId());
     if (!pointee_type)
       return nullptr;
 
+    if (pointer_type->isPointerToDataMember() ||
+        pointer_type->isPointerToMemberFunction()) {
+      auto class_parent_uid = pointer_type->getRawSymbol().getClassParentId();
+      auto class_parent_type =
+          m_ast.GetSymbolFile()->ResolveTypeUID(class_parent_uid);
+      assert(class_parent_type);
+
+      CompilerType pointer_ast_type;
+      pointer_ast_type = ClangASTContext::CreateMemberPointerType(
+          class_parent_type->GetLayoutCompilerType(),
+          pointee_type->GetForwardCompilerType());
+      assert(pointer_ast_type);
+
+      return std::make_shared<lldb_private::Type>(
+          pointer_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(),
+          pointer_type->getLength(), nullptr, LLDB_INVALID_UID,
+          lldb_private::Type::eEncodingIsUID, decl, pointer_ast_type,
+          lldb_private::Type::eResolveStateForward);
+    }
+
     CompilerType pointer_ast_type;
     pointer_ast_type = pointee_type->GetFullCompilerType();
     if (pointer_type->isReference())
       pointer_ast_type = pointer_ast_type.GetLValueReferenceType();
     else if (pointer_type->isRValueReference())
       pointer_ast_type = pointer_ast_type.GetRValueReferenceType();
     else
       pointer_ast_type = pointer_ast_type.GetPointerType();
return std::make_shared<lldb_private::Type>(
         lldb_private::Type::eResolveStateFull);
   } break;
   default:
     break;
   }
   return nullptr;
 }
 
+bool PDBASTParser::CompleteTypeFromPDB(
+    lldb_private::CompilerType &compiler_type) {
+  if (GetClangASTImporter().CanImport(compiler_type))
+    return GetClangASTImporter().CompleteType(compiler_type);
+
+  // Remove the type from the forward declarations to avoid
+  // an endless recursion for types like a linked list.
+  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<SymbolFilePDB *>(m_ast.GetSymbolFile());
+  if (!symbol_file)
+    return false;
+
+  std::unique_ptr<PDBSymbol> 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<PDBSymbolTypeUDT>(symbol.get());
+    if (!udt)
+      return false;
+
+    return CompleteTypeFromUDT(*symbol_file, compiler_type, *udt);
+  }
+  default:
+    llvm_unreachable("not a forward clang type decl!");
+  }
+}
+
 bool PDBASTParser::AddEnumValue(CompilerType enum_type,
                                 const PDBSymbolData &enum_value) const {
   Declaration decl;
   Variant v = enum_value.getValue();
   std::string name = enum_value.getName();
   int64_t raw_value;
   switch (v.Type) {
   case PDB_VariantType::Int8:
bool PDBASTParser::AddEnumValue(CompilerType enum_type,
   CompilerType underlying_type =
       m_ast.GetEnumerationIntegerType(enum_type.GetOpaqueQualType());
   uint32_t byte_size = m_ast.getASTContext()->getTypeSize(
       ClangUtil::GetQualType(underlying_type));
   return m_ast.AddEnumerationValueToEnumerationType(
       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 nested_enums = udt.findAllChildren<PDBSymbolTypeUDT>();
+  if (nested_enums)
+    while (auto nested = nested_enums->getNext())
+      symbol_file.ResolveTypeUID(nested->getSymIndexId());
+
+  auto bases_enum = udt.findAllChildren<PDBSymbolTypeBaseClass>();
+  if (bases_enum)
+    AddRecordBases(symbol_file, compiler_type,
+                   TranslateUdtKind(udt.getUdtKind()), *bases_enum,
+                   layout_info);
+
+  auto members_enum = udt.findAllChildren<PDBSymbolData>();
+  if (members_enum)
+    AddRecordMembers(symbol_file, compiler_type, *members_enum, layout_info);
+
+  auto methods_enum = udt.findAllChildren<PDBSymbolFunc>();
+  if (methods_enum)
+    AddRecordMethods(symbol_file, compiler_type, *methods_enum);
+
+  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<bool>(compiler_type);
+
+  GetClangASTImporter().InsertRecordDecl(record_decl, layout_info);
+
+  return static_cast<bool>(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()) {
+    if (member->isCompilerGenerated())
+        continue;
+
+    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 = TranslateMemberAccess(member->getAccess());
+
+    switch (member->getDataKind()) {
+    case PDB_DataKind::Member: {
+      auto location_type = member->getLocationType();
+
+      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));
+
+      break;
+    }
+    case PDB_DataKind::StaticMember:
+      ClangASTContext::AddVariableToRecordType(record_type, member_name.c_str(),
+                                               member_comp_type, access);
+      break;
+    default:
+      llvm_unreachable("unsupported PDB data kind");
+    }
+  }
+}
+
+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<clang::CXXBaseSpecifier *> 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->GetFullCompilerType();
+    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 = TranslateMemberAccess(base->getAccess());
+
+    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());
+  }
+}
+
+void PDBASTParser::AddRecordMethods(
+    lldb_private::SymbolFile &symbol_file,
+    lldb_private::CompilerType &record_type,
+    PDBFuncSymbolEnumerator &methods_enum) const {
+  while (auto method = methods_enum.getNext()) {
+    auto method_type = symbol_file.ResolveTypeUID(method->getSymIndexId());
+    // MSVC specific __vecDelDtor.
+    if (!method_type)
+      break;
+
+    auto method_comp_type = method_type->GetFullCompilerType();
+    if (!method_comp_type.GetCompleteType()) {
+      symbol_file.GetObjectFile()->GetModule()->ReportError(
+          ":: Class '%s' has a method '%s' whose type cannot be completed.",
+          record_type.GetTypeName().GetCString(),
+          method_comp_type.GetTypeName().GetCString());
+      if (ClangASTContext::StartTagDeclarationDefinition(method_comp_type))
+        ClangASTContext::CompleteTagDeclarationDefinition(method_comp_type);
+    }
+
+    // TODO: get mangled name for the method.
+    m_ast.AddMethodToCXXRecordType(
+        record_type.GetOpaqueQualType(), method->getName().c_str(),
+        /*mangled_name*/ nullptr, method_comp_type,
+        TranslateMemberAccess(method->getAccess()), method->isVirtual(),
+        method->isStatic(), method->hasInlineAttribute(),
+        /*is_explicit*/ false, // FIXME: Need this field in CodeView.
+        /*is_attr_used*/ false,
+        /*is_artificial*/ method->isCompilerGenerated());
+  }
+}

lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugEnd.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugStart.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolPublicSymbol.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" // For IsCPPMangledName
 #include "Plugins/SymbolFile/PDB/PDBASTParser.h"
 #include "Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h"
 
 #include <regex>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::pdb;
for (auto tag : tags_to_search) {
         case PDB_SymType::Typedef:
           break;
         default:
           continue;
         }
 
         // This should cause the type to get cached and stored in the `m_types`
         // lookup.
-        if (!ResolveTypeUID(symbol->getSymIndexId()))
-          continue;
-
+        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;
         }
       }
+    }
   };
 
   if (sc.function) {
     auto pdb_func = m_session_up->getConcreteSymbolById<PDBSymbolFunc>(
         sc.function->GetID());
     if (!pdb_func)
       return 0;
     ParseTypesByTagFn(*pdb_func);
if (result) {
     auto type_list = GetTypeList();
     if (type_list)
       type_list->Insert(result);
   }
   return result.get();
 }
 
 bool SymbolFilePDB::CompleteType(lldb_private::CompilerType &compiler_type) {
-  // TODO: Implement this
+  std::lock_guard<std::recursive_mutex> guard(
+      GetObjectFile()->GetModule()->GetMutex());
+
+  ClangASTContext *clang_ast_ctx = llvm::dyn_cast_or_null<ClangASTContext>(
+      GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus));
+  if (!clang_ast_ctx)
     return false;
+
+  PDBASTParser *pdb =
+      llvm::dyn_cast<PDBASTParser>(clang_ast_ctx->GetPDBParser());
+  if (!pdb)
+    return false;
+
+  return pdb->CompleteTypeFromPDB(compiler_type);
 }
 
 lldb_private::CompilerDecl SymbolFilePDB::GetDeclForUID(lldb::user_id_t uid) {
   return lldb_private::CompilerDecl();
 }
 
 lldb_private::CompilerDeclContext
 SymbolFilePDB::GetDeclContextForUID(lldb::user_id_t uid) {

lldb/trunk/source/Symbol/ClangASTContext.cpp

return language == eLanguageTypeUnknown || // Clang is the default type system
          Language::LanguageIsObjC(language) ||
          Language::LanguageIsPascal(language) ||
          // Use Clang for Rust until there is a proper language plugin for it
          language == eLanguageTypeRust ||
          language == eLanguageTypeExtRenderScript ||
          // 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<clang::FunctionProtoType>(
+      context.getCanonicalType(m1->getType()));
+
+  const auto *m2Type = llvm::cast<clang::FunctionProtoType>(
+      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<clang::CXXRecordDecl>(
+                specifier->getType()->getAs<clang::RecordType>()->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<clang::CXXMethodDecl>(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<clang::CXXMethodDecl>(overridden_decl));
+  }
+}
 }
 
 typedef lldb_private::ThreadSafeDenseMap<clang::ASTContext *, ClangASTContext *>
     ClangASTMap;
 
 static ClangASTMap &GetASTMap() {
   static ClangASTMap *g_map_ptr = nullptr;
   static llvm::once_flag g_once_flag;
CompilerType ClangASTContext::GetChildCompilerTypeAtIndex(
   const clang::Type::TypeClass parent_type_class =
       parent_qual_type->getTypeClass();
   child_bitfield_bit_size = 0;
   child_bitfield_bit_offset = 0;
   child_is_base_class = false;
   language_flags = 0;
 
   const bool idx_is_valid = idx < GetNumChildren(type, omit_empty_base_classes);
-  uint32_t bit_offset;
+  int32_t bit_offset;
   switch (parent_type_class) {
   case clang::Type::Builtin:
     if (idx_is_valid) {
       switch (llvm::cast<clang::BuiltinType>(parent_qual_type)->getKind()) {
       case clang::BuiltinType::ObjCId:
       case clang::BuiltinType::ObjCClass:
         child_name = "isa";
         child_byte_size =
if (idx_is_valid && GetCompleteType(type)) {
                           if (vbtable_ptr != LLDB_INVALID_ADDRESS) {
                             // Get the index into the virtual base table. The
                             // index is the index in uint32_t from vbtable_ptr
                             const unsigned vbtable_index =
                                 msoft_vtable_ctx->getVBTableIndex(
                                     cxx_record_decl, base_class_decl);
                             const lldb::addr_t base_offset_addr =
                                 vbtable_ptr + vbtable_index * 4;
-                            const uint32_t base_offset =
-                                process->ReadUnsignedIntegerFromMemory(
+                            const int32_t base_offset =
+                                process->ReadSignedIntegerFromMemory(
                                     base_offset_addr, 4, UINT32_MAX, err);
                             if (base_offset != UINT32_MAX) {
                               handled = true;
                               bit_offset = base_offset * 8;
                             }
                           }
                         }
                       } else {
                         clang::ItaniumVTableContext *itanium_vtable_ctx =
                             static_cast<clang::ItaniumVTableContext *>(
                                 vtable_ctx);
                         if (vtable_ptr_addr) {
                           const lldb::addr_t vtable_ptr =
                               process->ReadPointerFromMemory(vtable_ptr_addr,
                                                              err);
                           if (vtable_ptr != LLDB_INVALID_ADDRESS) {
                             clang::CharUnits base_offset_offset =
                                 itanium_vtable_ctx->getVirtualBaseOffsetOffset(
                                     cxx_record_decl, base_class_decl);
                             const lldb::addr_t base_offset_addr =
                                 vtable_ptr + base_offset_offset.getQuantity();
                             const uint32_t base_offset_size =
                                 process->GetAddressByteSize();
-                            const uint64_t base_offset =
-                                process->ReadUnsignedIntegerFromMemory(
+                            const int64_t base_offset =
+                                process->ReadSignedIntegerFromMemory(
                                     base_offset_addr, base_offset_size,
                                     UINT32_MAX, err);
                             if (base_offset < UINT32_MAX) {
                               handled = true;
                               bit_offset = base_offset * 8;
                             }
                           }
                         }
#endif
   //    cxx_record_decl->hasTrivialCopyConstructor());
   //    printf ("decl->hasTrivialCopyAssignment()  = %i\n",
   //    cxx_record_decl->hasTrivialCopyAssignment());
   //    printf ("decl->hasTrivialDestructor()      = %i\n",
   //    cxx_record_decl->hasTrivialDestructor());
   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 *
 ClangASTContext::CreateBaseClassSpecifier(lldb::opaque_compiler_type_t type,
                                           AccessType access, bool is_virtual,
                                           bool base_of_class) {
   if (type)
     return new clang::CXXBaseSpecifier(
bool ClangASTContext::LayoutRecordType(
     void *baton, const clang::RecordDecl *record_decl, uint64_t &bit_size,
     uint64_t &alignment,
     llvm::DenseMap<const clang::FieldDecl *, uint64_t> &field_offsets,
     llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
         &base_offsets,
     llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
         &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);
 }
 
 //----------------------------------------------------------------------
 // CompilerDecl override functions
 //----------------------------------------------------------------------
 
 ConstString ClangASTContext::DeclGetName(void *opaque_decl) {
   if (opaque_decl) {