[SymbolFilePDB] Fix null array access when parsing the type of a function without any arguments, i.e. 'int main()' and add support to test it

[asmith]

Summary

• Fix a null array access bug. This happens when creating the lldb type for a function that has no argument.
• Implement SymbolFilePDB::ParseTypes method. Using lldb-test symbols will show all supported types in the target.
• Create lldb types for variadic function, PDBSymbolTypePointer, PDBSymbolTypeBuiltin
• The underlying builtin type for PDBSymbolTypeEnum is always Int, correct it with the very first enumerator's encoding if any. This is more accurate when the underlying type is not signed or another integer type.
• Fix a bug when the compiler type is not created based on PDB_BuiltinType. For example, basic type long is of same width as int in a 32-bit target, and the compiler type of former one will be represented by the one generated for latter if using the default method. Introduce a static function GetBuiltinTypeForPDBEncodingAndBitSize to correct this issue.
• Basic type long double and double have the same bit size in MSVC and there is no information in a PDB to distinguish them. The compiler type of the former one is represented by the latter's.
• There is no line informaton about typedef, enum etc in a PDB and the source and line information for them are not shown.
• There is no information about scoped enumeration. The compiler type is represented as an unscoped one.

[zturner]

This is another example where we could test it easily if lldb-test could dump this. Are you willing to take a stab at this?

[davide]

Indeed, this needs to be tested.

[asmith]

This fix is part of a larger set of changes to retrieve the type for a function signature and I don't see how to test for this without those changes. With all the other changes, lldb-test fails without this fix and passes with it. So it's implicitly already tested.

[zturner]

This was originally written as a unit test because at the time we didn't have lldb-test. To be honest I think it's time to remove these checked in binaries and convert everything to FileCheck tests. There's a couple of reasons this is more practical. For starters, the binaries can be really large and having lots of large binaries in the repo is a real drag for developers. Secondly, we've recently had some bug reports where some virus scan software is flagging our executables as malicious due to some heuristics. So while you're just modifying an existing binary / test, if it's not a ton of work I would really prefer standardizing on lldb-test + FileCheck for this sort of thing. Did you explore that at all?

While there's a little higher up front cost because you have to go update the lldb-test tool to support this, after that work is done it will make writing future tests very easy. All you would need is something like this:

REQUIRES: system-windows
RUN: clang-cl /Z7 /c %S/Inputs/foo.cpp /o %T/foo.cpp.obj
RUN: link /DEBUG /PDB:%T/foo.cpp.pdb /Fo:%T/foo.cpp.exe
RUN: lldb-test symbols -types foo.cpp.exe | FileCheck %s

CHECK: int(*FuncPointerTypedef)();
CHECK: int(*VariadicFuncPointerTypedef)(char,...);

[asmith]

Do you prefer to drop the test changes and corresponding binaries?
For an llvm-lit test, which directory do you recommend for the new file?

[asmith]

The test is added and this is ready to go. Let me know if there are any other changes. Thanks!

[zturner]

Sorry for forgetting about this! lgtm

[asmith]

Revision was approved by zturner in the comments

[majnemer]

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

162–163

Copy paste bug?

[labath]

I have reverted this because this breaks the lldb-Unit::SymbolFilePDBTests.TestTypedefs test http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7715. My guess is that this is because you did not update the checked-in exe to account for the changes in it's source. I'll leave up to you and Zachary to figure out whether you actually want to update the exe or test everything using the lldb-test approach, but in the mean time, we cannot have failing tests in the repository.

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

+// typedef
+typedef unsigned long ULongArrayTypedef[10];
+ULongArrayTypedef ULongArrayVar;
+
+typedef long double*& RefTypedef;
+long double* LongDoublePtrVar = 0;
+RefTypedef RefVar = LongDoublePtrVar;
+
+typedef long long (*FuncPtrTypedef)(int&, unsigned char**, short[], const double, volatile bool);
+FuncPtrTypedef FuncVar;
+
+typedef char (*VarArgsFuncTypedef)(void*, long, unsigned short, unsigned int, ...);
+VarArgsFuncTypedef VarArgsFuncVar;
+
+typedef float (*VarArgsFuncTypedefA)(...);
+VarArgsFuncTypedefA VarArgsFuncVarA;
+
+// unscoped enum
+enum Enum { RED, GREEN, BLUE };
+Enum EnumVar;
+
+enum EnumConst { LOW, NORMAL = 10, HIGH };
+EnumConst EnumConstVar;
+
+enum EnumEmpty {};
+EnumEmpty EnumEmptyVar;
+
+enum EnumUChar : unsigned char { ON, OFF, AUTO };
+EnumUChar EnumCharVar;
+
+// scoped enum
+enum class EnumClass { YES, NO, DEFAULT };
+EnumClass EnumClassVar;
+
+enum struct EnumStruct { red, blue, black };
+EnumStruct EnumStructVar;
+  
+int main() {
+  return 0;
+}

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

+REQUIRES: windows
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/SimpleTypesTest.cpp /o %T/SimpleTypesTest.cpp.enums.obj
+RUN: link %T/SimpleTypesTest.cpp.enums.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/SimpleTypesTest.cpp.enums.exe
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.enums.exe | FileCheck %s
+
+; FIXME: PDB does not have information about scoped enumeration (Enum class) so the  
+; compiler type used is the same as the one for unscoped enumeration.
+
+CHECK: Module [[CU:.*]]
+CHECK-DAG: {{^[0-9A-F]+}}: SymbolVendor ([[CU]])
+CHECK:      Type{{.*}} , name = "Enum", size = 4, compiler_type = {{.*}} enum Enum {
+CHECK-NEXT:    RED,
+CHECK-NEXT:    GREEN,
+CHECK-NEXT:    BLUE
+CHECK-NEXT:}
+
+CHECK:      Type{{.*}} , name = "EnumConst", size = 4, compiler_type = {{.*}} enum EnumConst {
+CHECK-NEXT:    LOW,
+CHECK-NEXT:    NORMAL,
+CHECK-NEXT:    HIGH
+CHECK-NEXT:}
+
+CHECK:      Type{{.*}} , name = "EnumEmpty", size = 4, compiler_type = {{.*}} enum EnumEmpty {
+CHECK-NEXT:}
+
+CHECK:      Type{{.*}} , name = "EnumUChar", size = 1, compiler_type = {{.*}} enum EnumUChar {
+CHECK-NEXT:    ON,
+CHECK-NEXT:    OFF,
+CHECK-NEXT:    AUTO
+CHECK-NEXT:}
+
+; Note that `enum EnumClass` is tested instead of `enum class EnumClass`
+CHECK:      Type{{.*}} , name = "EnumClass", size = 4, compiler_type = {{.*}} enum EnumClass {
+CHECK-NEXT:    YES,
+CHECK-NEXT:    NO,
+CHECK-NEXT:    DEFAULT
+CHECK-NEXT:}
+
+CHECK:      Type{{.*}} , name = "EnumStruct", size = 4, compiler_type = {{.*}} enum EnumStruct {
+CHECK-NEXT:    red,
+CHECK-NEXT:    blue,
+CHECK-NEXT:    black
+CHECK-NEXT:}
+
+CHECK-DAG: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\SimpleTypesTest.cpp'

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

+REQUIRES: windows
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/SimpleTypesTest.cpp /o %T/SimpleTypesTest.cpp.typedefs.obj
+RUN: link %T/SimpleTypesTest.cpp.typedefs.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/SimpleTypesTest.cpp.typedefs.exe
+RUN: lldb-test symbols %T/SimpleTypesTest.cpp.typedefs.exe | FileCheck %s
+
+; Generate 32-bit target
+
+; FIXME: PDB does not have line information for typedef statements so source
+; and line information for them is not tested.
+
+; Note, types `long double` and `double` have same bit size in MSVC and there
+; is no information in the PDB to distinguish them. So the compiler type for 
+; both of them is the same.
+
+CHECK: Module [[CU:.*]]
+CHECK-DAG:  {{^[0-9A-F]+}}: SymbolVendor ([[CU]])
+CHECK:      Type{{.*}} , name = "unsigned long", size = 4, compiler_type = {{.*}} unsigned long
+CHECK-NEXT: Type{{.*}} , size = 40, compiler_type = {{.*}} unsigned long [10]
+CHECK-NEXT: Type{{.*}} , name = "ULongArrayTypedef", compiler_type = {{.*}} typedef ULongArrayTypedef
+
+; Note: compiler_type of `long double` is represented by the one for `double`
+CHECK:      Type{{.*}} , name = "double", size = 8, compiler_type = {{.*}} double
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} double *
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} double *&
+CHECK-DAG: Type{{.*}} , name = "RefTypedef", compiler_type = {{.*}} typedef RefTypedef
+
+CHECK:      Type{{.*}} , name = "int", size = 4, compiler_type = {{.*}} int
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} int &
+CHECK-NEXT: Type{{.*}} , name = "unsigned char", size = 1, compiler_type = {{.*}} unsigned char
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} unsigned char *
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} unsigned char **
+CHECK-NEXT: Type{{.*}} , name = "short", size = 2, compiler_type = {{.*}} short
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} short *
+CHECK-NEXT: Type{{.*}} , name = "const double", size = 8, compiler_type = {{.*}} const double
+CHECK-NEXT: Type{{.*}} , name = "volatile bool", size = 1, compiler_type = {{.*}} volatile _Bool
+CHECK-NEXT: Type{{.*}} , name = "long long", size = 8, compiler_type = {{.*}} long long
+CHECK-NEXT: Type{{.*}} , compiler_type = {{.*}} long long (int &, unsigned char **, short *, const double, volatile _Bool)
+CHECK-DAG:  Type{{.*}} , name = "FuncPtrTypedef", compiler_type = {{.*}} typedef FuncPtrTypedef
+
+CHECK:      Type{{.*}} , name = "void", compiler_type = {{.*}} void
+CHECK-NEXT: Type{{.*}} , size = 4, compiler_type = {{.*}} void *
+CHECK-NEXT: Type{{.*}} , name = "long", size = 4, compiler_type = {{.*}} long
+CHECK-NEXT: Type{{.*}} , name = "unsigned short", size = 2, compiler_type = {{.*}} unsigned short
+CHECK-NEXT: Type{{.*}} , name = "unsigned int", size = 4, compiler_type = {{.*}} unsigned int
+CHECK-NEXT: Type{{.*}} , name = "signed char", size = 1, compiler_type = {{.*}} signed char
+CHECK-NEXT: Type{{.*}} , compiler_type = {{.*}} signed char (void *, long, unsigned short, unsigned int, ...)
+CHECK-DAG:  Type{{.*}} , size = 4, compiler_type = {{.*}} signed char (*)(void *, long, unsigned short, unsigned int, ...)
+CHECK-DAG:  Type{{.*}} , name = "VarArgsFuncTypedef", compiler_type = {{.*}} typedef VarArgsFuncTypedef
+
+CHECK:      Type{{.*}} , name = "float", size = 4, compiler_type = {{.*}} float
+CHECK-NEXT: Type{{.*}} , compiler_type = {{.*}} float (...)
+CHECK-DAG:  Type{{.*}} , size = 4, compiler_type = {{.*}} float (*)(...)
+CHECK-DAG:  Type{{.*}} , name = "VarArgsFuncTypedefA", compiler_type = {{.*}} typedef VarArgsFuncTypedefA
+
+CHECK-DAG: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\SimpleTypesTest.cpp'

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

 
 #include "llvm/DebugInfo/PDB/PDBSymbol.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeArray.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
 using namespace lldb;
 using namespace lldb_private;
-using namespace llvm;
 using namespace llvm::pdb;
 
 namespace {
 int TranslateUdtKind(PDB_UdtType pdb_kind) {
   switch (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;
   }
-  return clang::TTK_Class;
+  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:
   case PDB_BuiltinType::Char:
     return lldb::eEncodingSint;
   case PDB_BuiltinType::Bool:
   case PDB_BuiltinType::UInt:
   case PDB_BuiltinType::ULong:
   case PDB_BuiltinType::HResult:
     return lldb::eEncodingUint;
   default:
     return lldb::eEncodingInvalid;
   }
 }
+
+lldb::Encoding TranslateEnumEncoding(PDB_VariantType type) {
+  switch (type) {
+  case PDB_VariantType::Int8:
+  case PDB_VariantType::Int16:
+  case PDB_VariantType::Int32:
+  case PDB_VariantType::Int64:
+    return lldb::eEncodingSint;
+
+  case PDB_VariantType::UInt8:
+  case PDB_VariantType::UInt16:
+  case PDB_VariantType::UInt32:
+  case PDB_VariantType::UInt64:
+    return lldb::eEncodingUint;
+
+  default:
+    break;
+  }
+
+  return lldb::eEncodingSint;
+}
+
+CompilerType GetBuiltinTypeForPDBEncodingAndBitSize(
+    ClangASTContext *clang_ast, const PDBSymbolTypeBuiltin *pdb_type,
+    Encoding encoding, uint32_t width) {
+  if (!pdb_type)
+    return CompilerType();
+  if (!clang_ast)
+    return CompilerType();
+  auto *ast = clang_ast->getASTContext();
+  if (!ast)
+    return CompilerType();
+
+  switch (pdb_type->getBuiltinType()) {
+  default: break;
+  case PDB_BuiltinType::None:
+    return CompilerType();
+  case PDB_BuiltinType::Void:
+    // FIXME: where is non-zero size of `void` from?
+    if (width == 0)
+      return clang_ast->GetBasicType(eBasicTypeVoid);
+  case PDB_BuiltinType::Bool:
+    return clang_ast->GetBasicType(eBasicTypeBool);
+  case PDB_BuiltinType::Long:
+    if (width == ast->getTypeSize(ast->LongTy))
+      return CompilerType(ast, ast->LongTy);
+    if (width == ast->getTypeSize(ast->LongLongTy))
+      return CompilerType(ast, ast->LongLongTy);
+    break;
+  case PDB_BuiltinType::ULong:
+    if (width == ast->getTypeSize(ast->UnsignedLongTy))
+      return CompilerType(ast, ast->UnsignedLongTy);
+    if (width == ast->getTypeSize(ast->UnsignedLongLongTy))
+      return CompilerType(ast, ast->UnsignedLongLongTy);
+    break;
+  case PDB_BuiltinType::WCharT:
+    if (width == ast->getTypeSize(ast->WCharTy))
+      return CompilerType(ast, ast->WCharTy);
+    break;
+  case PDB_BuiltinType::Float:
+    // Note: types `long double` and `double` have same bit size in MSVC and there
+    // is no information in the PDB to distinguish them. So when falling back
+    // to default search, the compiler type of `long double` will be represented by
+    // the one generated for `double`.
+    break;
+  }
+  // If there is no match on PDB_BuiltinType, fall back to default search
+  // by encoding and width only
+  return clang_ast->GetBuiltinTypeForEncodingAndBitSize(encoding, width);
+}
+
+ConstString GetPDBBuiltinTypeName(const PDBSymbolTypeBuiltin *pdb_type,
+                                  CompilerType &compiler_type) {
+  if (!pdb_type)
+    return compiler_type.GetTypeName();
+
+  PDB_BuiltinType kind = pdb_type->getBuiltinType();
+  switch (kind) {
+  default: break;
+  case PDB_BuiltinType::Currency:
+    return ConstString("CURRENCY");
+  case PDB_BuiltinType::Date:
+    return ConstString("DATE");
+  case PDB_BuiltinType::Variant:
+    return ConstString("VARIANT");
+  case PDB_BuiltinType::Complex:
+    return ConstString("complex");
+  case PDB_BuiltinType::Bitfield:
+    return ConstString("bitfield");
+  case PDB_BuiltinType::BSTR:
+    return ConstString("BSTR");
+  case PDB_BuiltinType::HResult:
+    return ConstString("HRESULT");
+  case PDB_BuiltinType::BCD:
+    return ConstString("HRESULT");
+  case PDB_BuiltinType::None:
+    return ConstString("...");
+  }
+  return compiler_type.GetTypeName();
+}
 }
 
 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;
 
   if (auto udt = llvm::dyn_cast<PDBSymbolTypeUDT>(&type)) {
     AccessType access = lldb::eAccessPublic;
     PDB_UdtType udt_kind = udt->getUdtKind();
+    auto tag_type_kind = TranslateUdtKind(udt_kind);
+    if (tag_type_kind == -1)
+      return nullptr;
 
     if (udt_kind == PDB_UdtType::Class)
       access = lldb::eAccessPrivate;
 
     CompilerType clang_type = m_ast.CreateRecordType(
-        tu_decl_ctx, access, udt->getName().c_str(), TranslateUdtKind(udt_kind),
+        tu_decl_ctx, access, udt->getName().c_str(), tag_type_kind,
         lldb::eLanguageTypeC_plus_plus, nullptr);
 
     m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
 
     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);
   } else if (auto enum_type = llvm::dyn_cast<PDBSymbolTypeEnum>(&type)) {
-    std::string name = enum_type->getName();
+    auto underlying_type_up = enum_type->getUnderlyingType();
+    if (!underlying_type_up)
+      return nullptr;
     lldb::Encoding encoding =
-        TranslateBuiltinEncoding(enum_type->getBuiltinType());
+        TranslateBuiltinEncoding(underlying_type_up->getBuiltinType());
+    // FIXME: Type of underlying builtin is always `Int`. We correct it with
+    // the very first enumerator's encoding if any.
+    auto first_child = enum_type->findOneChild<PDBSymbolData>();
+    if (first_child) {
+      encoding = TranslateEnumEncoding(first_child->getValue().Type);
+    }
+    std::string name = enum_type->getName();
     uint64_t bytes = enum_type->getLength();
-    CompilerType builtin_type =
-        m_ast.GetBuiltinTypeForEncodingAndBitSize(encoding, bytes * 8);
+    CompilerType builtin_type;
+    if (bytes > 0)
+      builtin_type = GetBuiltinTypeForPDBEncodingAndBitSize(
+           &m_ast, underlying_type_up.get(), encoding, bytes * 8);
+    else
+      builtin_type = m_ast.GetBasicType(eBasicTypeInt);
+    // FIXME: PDB does not have information about scoped enumeration (Enum Class).
+    // Set it false for now.
+    bool isScoped = false;
 
     CompilerType ast_enum = m_ast.CreateEnumerationType(
-        name.c_str(), tu_decl_ctx, decl, builtin_type, false);
+        name.c_str(), tu_decl_ctx, decl, builtin_type, isScoped);
     auto enum_values = enum_type->findAllChildren<PDBSymbolData>();
+    if (enum_values) {
       while (auto enum_value = enum_values->getNext()) {
         if (enum_value->getDataKind() != PDB_DataKind::Constant)
           continue;
         AddEnumValue(ast_enum, *enum_value);
       }
+    }
+    if (ClangASTContext::StartTagDeclarationDefinition(ast_enum))
+      ClangASTContext::CompleteTagDeclarationDefinition(ast_enum);
 
     return std::make_shared<lldb_private::Type>(
         type.getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name), bytes,
         nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl,
         ast_enum, lldb_private::Type::eResolveStateFull);
   } else if (auto type_def = llvm::dyn_cast<PDBSymbolTypeTypedef>(&type)) {
     lldb_private::Type *target_type =
         m_ast.GetSymbolFile()->ResolveTypeUID(type_def->getTypeId());
if (auto udt = llvm::dyn_cast<PDBSymbolTypeUDT>(&type)) {
     return std::make_shared<lldb_private::Type>(
         type_def->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name),
         bytes, nullptr, target_type->GetID(),
         lldb_private::Type::eEncodingIsTypedefUID, decl, ast_typedef,
         lldb_private::Type::eResolveStateFull);
   } else if (auto func_sig = llvm::dyn_cast<PDBSymbolTypeFunctionSig>(&type)) {
     auto arg_enum = func_sig->getArguments();
     uint32_t num_args = arg_enum->getChildCount();
-    std::vector<CompilerType> arg_list(num_args);
-    while (auto arg = arg_enum->getNext()) {
+    std::vector<CompilerType> arg_list;
+
+    bool is_variadic = func_sig->isCVarArgs();
+    // Drop last variadic argument.
+    if (is_variadic)
+      --num_args;
+    for (int arg_idx = 0; arg_idx < num_args; arg_idx++) {
+      auto arg = arg_enum->getChildAtIndex(arg_idx);
+      if (!arg)
+        break;
       lldb_private::Type *arg_type =
           m_ast.GetSymbolFile()->ResolveTypeUID(arg->getSymIndexId());
       // If there's some error looking up one of the dependent types of this
       // function signature, bail.
       if (!arg_type)
         return nullptr;
       CompilerType arg_ast_type = arg_type->GetFullCompilerType();
       arg_list.push_back(arg_ast_type);
     }
+    lldbassert(arg_list.size() <= num_args);
+
     auto pdb_return_type = func_sig->getReturnType();
     lldb_private::Type *return_type =
         m_ast.GetSymbolFile()->ResolveTypeUID(pdb_return_type->getSymIndexId());
     // If there's some error looking up one of the dependent types of this
     // function signature, bail.
     if (!return_type)
       return nullptr;
     CompilerType return_ast_type = return_type->GetFullCompilerType();
     uint32_t type_quals = 0;
     if (func_sig->isConstType())
       type_quals |= clang::Qualifiers::Const;
     if (func_sig->isVolatileType())
       type_quals |= clang::Qualifiers::Volatile;
     CompilerType func_sig_ast_type = m_ast.CreateFunctionType(
-        return_ast_type, &arg_list[0], num_args, false, type_quals);
+        return_ast_type, arg_list.data(), arg_list.size(), is_variadic,
+        type_quals);
 
     return std::make_shared<lldb_private::Type>(
         func_sig->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), 0,
         nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl,
         func_sig_ast_type, lldb_private::Type::eResolveStateFull);
   } else if (auto array_type = llvm::dyn_cast<PDBSymbolTypeArray>(&type)) {
     uint32_t num_elements = array_type->getCount();
     uint32_t element_uid = array_type->getElementType()->getSymIndexId();
     uint32_t bytes = array_type->getLength();
 
     lldb_private::Type *element_type =
         m_ast.GetSymbolFile()->ResolveTypeUID(element_uid);
     if (!element_type)
       return nullptr;
     CompilerType element_ast_type = element_type->GetFullCompilerType();
     CompilerType array_ast_type =
         m_ast.CreateArrayType(element_ast_type, num_elements, false);
     return std::make_shared<lldb_private::Type>(
         array_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(),
         bytes, nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID,
         decl, array_ast_type, lldb_private::Type::eResolveStateFull);
+  } else if (auto *builtin_type = llvm::dyn_cast<PDBSymbolTypeBuiltin>(&type)) {
+    PDB_BuiltinType builtin_kind = builtin_type->getBuiltinType();
+    if (builtin_kind == PDB_BuiltinType::None)
+      return nullptr;
+
+    uint64_t bytes = builtin_type->getLength();
+    Encoding encoding = TranslateBuiltinEncoding(builtin_kind);
+    CompilerType builtin_ast_type = GetBuiltinTypeForPDBEncodingAndBitSize(
+        &m_ast, builtin_type, encoding, bytes * 8);
+
+    Type::EncodingDataType encoding_data_type = Type::eEncodingIsUID;
+    if (builtin_type->isConstType()) {
+      encoding_data_type = Type::eEncodingIsConstUID;
+      builtin_ast_type = builtin_ast_type.AddConstModifier();
+    }
+    if (builtin_type->isVolatileType()) {
+      encoding_data_type = Type::eEncodingIsVolatileUID;
+      builtin_ast_type = builtin_ast_type.AddVolatileModifier();
+    }
+    auto type_name = GetPDBBuiltinTypeName(builtin_type, builtin_ast_type);
+
+    return std::make_shared<lldb_private::Type>(
+        builtin_type->getSymIndexId(), m_ast.GetSymbolFile(), type_name,
+        bytes, nullptr, LLDB_INVALID_UID, encoding_data_type,
+        decl, builtin_ast_type, lldb_private::Type::eResolveStateFull);
+  } else if (auto *pointer_type = llvm::dyn_cast<PDBSymbolTypePointer>(&type)) {
+    Type *pointee_type = m_ast.GetSymbolFile()->ResolveTypeUID(
+        pointer_type->getPointeeType()->getSymIndexId());
+    if (!pointee_type)
+      return nullptr;
+
+    CompilerType pointer_ast_type;
+    Type::EncodingDataType encoding_data_type = Type::eEncodingIsPointerUID;
+    if (pointer_type->isReference()) {
+      encoding_data_type = Type::eEncodingIsLValueReferenceUID;
+      pointer_ast_type =
+          pointee_type->GetFullCompilerType().GetLValueReferenceType();
+    } else
+      pointer_ast_type = pointee_type->GetFullCompilerType().GetPointerType();
+
+    return std::make_shared<lldb_private::Type>(
+        pointer_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(),
+        pointer_type->getLength(), nullptr, LLDB_INVALID_UID,
+        encoding_data_type, decl, pointer_ast_type,
+        lldb_private::Type::eResolveStateFull);
   }
   return nullptr;
 }
 
 bool PDBASTParser::AddEnumValue(CompilerType enum_type,
                                 const PDBSymbolData &enum_value) const {
   Declaration decl;
   Variant v = enum_value.getValue();

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

 #include "lldb/Core/PluginManager.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
+#include "lldb/Symbol/TypeList.h"
 #include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/IPDBDataStream.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
 #include "llvm/DebugInfo/PDB/IPDBTable.h"

 
 size_t
 SymbolFilePDB::ParseFunctionBlocks(const lldb_private::SymbolContext &sc) {
   // TODO: Implement this
   return size_t();
 }
 
 size_t SymbolFilePDB::ParseTypes(const lldb_private::SymbolContext &sc) {
-  // TODO: Implement this
-  return size_t();
+  lldbassert(sc.module_sp.get());
+  size_t num_added = 0;
+  auto results_up = m_session_up->getGlobalScope()->findAllChildren();
+  if (!results_up)
+    return 0;
+  while (auto symbol_up = results_up->getNext()) {
+    switch (symbol_up->getSymTag()) {
+    case PDB_SymType::Enum:
+    case PDB_SymType::UDT:
+    case PDB_SymType::Typedef:
+      break;
+    default:
+      continue;
+    }
+
+    auto type_uid = symbol_up->getSymIndexId();
+    if (m_types.find(type_uid) != m_types.end())
+      continue;
+
+    // This should cause the type to get cached and stored in the `m_types`
+    // lookup.
+    if (!ResolveTypeUID(symbol_up->getSymIndexId()))
+      continue;
+
+    ++num_added;
+  }
+  return num_added;
 }
 
 size_t
 SymbolFilePDB::ParseVariablesForContext(const lldb_private::SymbolContext &sc) {
   // TODO: Implement this
   return size_t();
 }
 
lldb_private::Type *SymbolFilePDB::ResolveTypeUID(lldb::user_id_t type_uid) {
   if (!pdb)
     return nullptr;
 
   auto pdb_type = m_session_up->getSymbolById(type_uid);
   if (pdb_type == nullptr)
     return nullptr;
 
   lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
-  if (result.get())
+  if (result.get()) {
     m_types.insert(std::make_pair(type_uid, result));
+    auto type_list = GetTypeList();
+    type_list->Insert(result);
+  }
   return result.get();
 }
 
 bool SymbolFilePDB::CompleteType(lldb_private::CompilerType &compiler_type) {
   // TODO: Implement this
   return false;
 }
 

 }
 
 size_t SymbolFilePDB::FindTypes(
     const std::vector<lldb_private::CompilerContext> &contexts, bool append,
     lldb_private::TypeMap &types) {
   return 0;
 }
 
-lldb_private::TypeList *SymbolFilePDB::GetTypeList() { return nullptr; }
+lldb_private::TypeList *SymbolFilePDB::GetTypeList() {
+  return m_obj_file->GetModule()->GetTypeList();
+}
 
 size_t SymbolFilePDB::GetTypes(lldb_private::SymbolContextScope *sc_scope,
                                uint32_t type_mask,
                                lldb_private::TypeList &type_list) {
   return size_t();
 }
 
 lldb_private::TypeSystem *

lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp

public:
   };
   ShortEnum se;
 };
 
 int test_func(int a, int b) { return a + b; }
 
 typedef Class ClassTypedef;
 typedef NS::NSClass NSClassTypedef;
+typedef int(*FuncPointerTypedef)();
+typedef int(*VariadicFuncPointerTypedef)(char,...);
+FuncPointerTypedef GlobalFunc;
+VariadicFuncPointerTypedef GlobalVariadicFunc;
 int GlobalArray[10];
 
 static const int sizeof_NSClass = sizeof(NS::NSClass);
 static const int sizeof_Class = sizeof(Class);
 static const int sizeof_NestedClass = sizeof(Class::NestedClass);
 static const int sizeof_Enum = sizeof(Enum);
 static const int sizeof_ShortEnum = sizeof(ShortEnum);
 static const int sizeof_ClassTypedef = sizeof(ClassTypedef);
 static const int sizeof_NSClassTypedef = sizeof(NSClassTypedef);
+static const int sizeof_FuncPointerTypedef = sizeof(FuncPointerTypedef);
+static const int sizeof_VariadicFuncPointerTypedef = sizeof(VariadicFuncPointerTypedef);
 static const int sizeof_GlobalArray = sizeof(GlobalArray);
 
 int main(int argc, char **argv) {
   ShortEnum e1;
   Enum e2;
   Class c1;
   Class::NestedClass c2;
   NS::NSClass c3;
 
   ClassTypedef t1;
   NSClassTypedef t2;
   return test_func(1, 2);
 }

lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

TEST_F(SymbolFilePDBTests, TestTypedefs) {
   SymbolVendor *plugin = module->GetSymbolVendor();
   SymbolFilePDB *symfile =
       static_cast<SymbolFilePDB *>(plugin->GetSymbolFile());
   llvm::pdb::IPDBSession &session = symfile->GetPDBSession();
   SymbolContext sc;
   llvm::DenseSet<SymbolFile *> searched_files;
   TypeMap results;
 
-  const char *TypedefsToCheck[] = {"ClassTypedef", "NSClassTypedef"};
+  const char *TypedefsToCheck[] = {
+      "ClassTypedef", "NSClassTypedef",
+      "FuncPointerTypedef", "VariadicFuncPointerTypedef"
+  };
   for (auto Typedef : TypedefsToCheck) {
     TypeMap results;
     EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString(Typedef), nullptr, false,
                                      0, searched_files, results));
     EXPECT_EQ(1u, results.GetSize());
     lldb::TypeSP typedef_type = results.GetTypeAtIndex(0);
     EXPECT_EQ(ConstString(Typedef), typedef_type->GetName());
     CompilerType compiler_type = typedef_type->GetFullCompilerType();