[LLDB][NativePDB] Fix function decl creation for class methods

[zequanwu]

Summary

This is a split of D113724. Calling TypeSystemClang::AddMethodToCXXRecordType
to create function decls for class methods.

lldb-test symbols --dump-ast also triggers that crash.

[labath]

Tagging Shafik in case he wants to look at the AST usage, but overall, this seems to be in line with the discussions on the other patch.

[shafik]

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

1091

I see you are setting these fall to false are you planning on figuring out correct values for them eventually or is that handled somewhere else?

Also looking at what we do when we parse DWARF, we also set the ClangASTMetadata as well.

[zequanwu]

I realized that the test case here is incorrect, because the output of lldb-test symbols dump-ast ... shows void virtual_method() twice. The void virtual_method() without virtual is created by the new code. Because lldb-test symbols dump-ast ... calls Module::ParseAllDebugSymbols() which calls SymbolFileNativePDB::ParseTypes(CompileUnit &comp_unit). It will construct all debug info inside TagDecls found from TPI stream, so the second void virtual_method() shows up.

I tried to use this for testing: lldb-test symbols --find=function --name=full::name --function-flags=full ..., but it doesn't even shows information for function main.

I'm working on another way to fix it.

[zequanwu]

Use lldb-test symbols --find=function --name=full::name --function-flags=full for testing.
Add some information about static/virtual/artificial and access type.

[zequanwu]

Add @rnk for code view records reading part.

[zequanwu]

Handle OverloadedMethodRecord.

[zequanwu]

use lower case for parameters.

[rnk]

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

1084

As a style thing, this class is quite large, I think I would prefer to see this at global scope in an anonymous namespace.

lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

89

I guess the test isn't able to differentiate whether these methods are static, private, etc. =/

We can overlook that for now, but I think now is the time to invest in better testing tools. The lldb-test program can do whatever you need it to, I don't think changing it requires updating that many tests, I think it's something you should look into.

[zequanwu]

Fix style.

lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

89

There is already a way to get those info (static, virtual, except access type) using lldb-test symbols --dump-ast .... It prints Clang::Decl which has those info. However, it has a bug in NativePDB plugin. It prints the methods twice. Maybe we can just check once for each method in the test?

lldb-test symbols --find=functions prints lldb_private::Function which doesn't have those info (static/virtual).

So, we can either just use lldb-test symbols --dump-ast ... but check only once for each method or use lldb-test symbols --find=functions.

Both commands just crash without this change.

[zequanwu]

Select the correct OneMethodRecord for overloaded method by checking TypeIndex.

[zequanwu]

Add test with lldb-test symbols --dump-ast though it printed the methods twice. It is considered a separate bug that could be fixed later.

[zequanwu]

Deduplicate method decls in a class/struct by method's name and CompilerType.

[zequanwu]

lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

89

D115062 adds access type when dumping the ast using lldb-test symbols --dump-ast.

[zequanwu]

Fix stale comments.

[labath]

I am somewhat confused by this deduplication logic. I've read https://reviews.llvm.org/D113930#3136404, so I understand why the duplication is happening, but I don't understand _should_ it be happening. IIUC, SymbolFileNativePDB::ParseTypes tries to force the completion of all types. If it succeeds, then there should be no need to add anything else to that type. In fact, adding methods to an already-complete type sounds like a pretty bad idea.

While I'm not very familiar with PDBs or the process of clang ast creation, it sounds to me like there is something wrong here, and I want to make sure we're not using the deduplication to cover up some other issue. Were you able to understand (and ideally, explain) why we have the two parsing methods and why they end up trying to add the same method twice?

[zequanwu]

In D113930#3173181, @labath wrote:

While I'm not very familiar with PDBs or the process of clang ast creation, it sounds to me like there is something wrong here, and I want to make sure we're not using the deduplication to cover up some other issue. Were you able to understand (and ideally, explain) why we have the two parsing methods and why they end up trying to add the same method twice?

In ModuleParseAllDebugSymbols, these two functions (NativePDB::ParseFunctions and NativePDB::ParseTypes) are called with NativePDB plugin. NativePDB::ParseFunctions parses pdb debug stream, which contains information for those methods existing in the binary. Methods defined in the source files but removed by optimizations will not show up in it. This is where those methods get created at first time. NativePDB::ParseTypes parses pdb type stream, which contains type information for methods even if they are removed by optimizations. All class members are in the class's FieldList type record. ParseTypes visits all classes' FieldList to complete their TagRecord. This is where those methods get created the second time. We can refer to a function's type record (from type stream) using its symbol record (from debug stream) but not the opposite, since it's common multiple functions have the same signature, which will be just one type record for them. If we can change the order of function calls so that ParseFunctions is after ParseTypes, we check if a method is already created or not, not sure if it's okay.

[labath]

Thanks for the explanation. This makes things a lot clearer (though it doesn't convince me that this is the right approach).

I still think that the piecemeal addition of member functions is a problem. While I don't think that changing the parsing order in Module::ParseAllDebugSymbols would be a problem (that function is only called from lldb-test), I think that would only hide the problem. Looking at what happens in the DWARF case, I /think/ (the code is fairly messy, so I could be wrong) that we don't create a clang ast representation for the function in ParseFunctions. We only create an lldb_private::Function (in DWARFASTParserClang::ParseFunctionFromDWARF). The ast entity only gets created in SymbolFileDWARF::ParseTypes (->ParseType->DWARFASTParserClang->ParseTypeFromDWARF). This kinda makes sense since we only need the ast representation of the method when we start dealing its class.

So I have a feeling that the real solution here is to avoid creating an ast entry for the function in the first phase. Then you should be able to create the full class declaration in the second phase, as you will have complete information there.

Does that make sense?

[zequanwu]

In D113930#3174406, @labath wrote:

I still think that the piecemeal addition of member functions is a problem. While I don't think that changing the parsing order in Module::ParseAllDebugSymbols would be a problem (that function is only called from lldb-test), I think that would only hide the problem. Looking at what happens in the DWARF case, I /think/ (the code is fairly messy, so I could be wrong) that we don't create a clang ast representation for the function in ParseFunctions. We only create an lldb_private::Function (in DWARFASTParserClang::ParseFunctionFromDWARF). The ast entity only gets created in SymbolFileDWARF::ParseTypes (->ParseType->DWARFASTParserClang->ParseTypeFromDWARF). This kinda makes sense since we only need the ast representation of the method when we start dealing its class.

From what I tested on dwarf plugin, lldb-test symbols -dump-ast doesn't print any ast decl, except a skeleton class decl. After adding some logs at (https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L1026), it doesn't get executed.

$ lldb-test symbols -dump-ast ast-methods.exe
Module: ast-methods.exe
struct Struct;

Nevertheless I think it works in dwarf because it creates the decls if die is DW_TAG_subprogram/DW_TAG_inlined_subroutine /DW_TAG_subroutine_type (https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L522). We can associate a decl with a die. But we can't associate a type record with a decl when parsing type stream in PDB.

So I have a feeling that the real solution here is to avoid creating an ast entry for the function in the first phase. Then you should be able to create the full class declaration in the second phase, as you will have complete information there.

If we don't create ast entries at ParseFunctions, non-class-member functions will not have ast decls. If we just create ast decls for non-class-member functions in ParseFunctions, we are expecting that ParseTypes is always called after ParseFunctions to complete the decl creation for member functions, which is not right.

[labath]

I am still not sure this is the right solution, but a) I don't know what *is* the right solution; and (b) I don't think this makes the situation worse; so I am hitting accept.

In D113930#3175022, @zequanwu wrote:

In D113930#3174406, @labath wrote:

So I have a feeling that the real solution here is to avoid creating an ast entry for the function in the first phase. Then you should be able to create the full class declaration in the second phase, as you will have complete information there.

If we don't create ast entries at ParseFunctions, non-class-member functions will not have ast decls. If we just create ast decls for non-class-member functions in ParseFunctions, we are expecting that ParseTypes is always called after ParseFunctions to complete the decl creation for member functions, which is not right.

I don't think this is as bad as it may sound at first. For lldb-test and ParseAllDebugSymbols purposes, I would even say its perfectly fine.

Overall I am not too worried about what happens in the ParseAllDebugSymbols scenario. I am more interested in what happens during "normal" operation when all of this is parsed lazily, in response to FindFunctions/FindTypes/... queries. (*) If everything works fine there then I think this would even be preferable, since you generally want to parse only the minimal amount of information. But I don't really even know how all of this works in DWARF, so I don't want to force you to redesign everything.

(*) A corollary of this is that I don't really trust that SymbolFileDWARF is doing "the right thing" in response to ParseAllDebugSymbols, since the main use case is lazy parsing, and ParseAllDebugSymbols only started being used relatively recently.

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h

private:
   TypeSystemClang &m_clang;
 
   ClangASTImporter m_importer;
 
   llvm::DenseMap<TypeIndex, TypeIndex> m_parent_types;
   llvm::DenseMap<clang::Decl *, DeclStatus> m_decl_to_status;
   llvm::DenseMap<lldb::user_id_t, clang::Decl *> m_uid_to_decl;
   llvm::DenseMap<lldb::user_id_t, clang::QualType> m_uid_to_type;
+
+  // From class/struct's opaque_compiler_type_t to a set containing the pairs of
+  // method's name and CompilerType.
+  llvm::DenseMap<lldb::opaque_compiler_type_t,
+                 llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>>
+      m_cxx_record_map;
 };
 
 } // namespace npdb
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_PDBASTBUILDER_H

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 
 using namespace lldb_private;
 using namespace lldb_private::npdb;
 using namespace llvm::codeview;
 using namespace llvm::pdb;
 
+namespace {
+struct CreateMethodDecl : public TypeVisitorCallbacks {
+  CreateMethodDecl(PdbIndex &m_index, TypeSystemClang &m_clang,
+                   TypeIndex func_type_index,
+                   clang::FunctionDecl *&function_decl,
+                   lldb::opaque_compiler_type_t parent_ty,
+                   llvm::StringRef proc_name, CompilerType func_ct)
+      : m_index(m_index), m_clang(m_clang), func_type_index(func_type_index),
+        function_decl(function_decl), parent_ty(parent_ty),
+        proc_name(proc_name), func_ct(func_ct) {}
+  PdbIndex &m_index;
+  TypeSystemClang &m_clang;
+  TypeIndex func_type_index;
+  clang::FunctionDecl *&function_decl;
+  lldb::opaque_compiler_type_t parent_ty;
+  llvm::StringRef proc_name;
+  CompilerType func_ct;
+
+  llvm::Error visitKnownMember(CVMemberRecord &cvr,
+                               OverloadedMethodRecord &overloaded) override {
+    TypeIndex method_list_idx = overloaded.MethodList;
+
+    CVType method_list_type = m_index.tpi().getType(method_list_idx);
+    assert(method_list_type.kind() == LF_METHODLIST);
+
+    MethodOverloadListRecord method_list;
+    llvm::cantFail(TypeDeserializer::deserializeAs<MethodOverloadListRecord>(
+        method_list_type, method_list));
+
+    for (const OneMethodRecord &method : method_list.Methods) {
+      if (method.getType().getIndex() == func_type_index.getIndex())
+        AddMethod(overloaded.Name, method.getAccess(), method.getOptions(),
+                  method.Attrs);
+    }
+
+    return llvm::Error::success();
+  }
+
+  llvm::Error visitKnownMember(CVMemberRecord &cvr,
+                               OneMethodRecord &record) override {
+    AddMethod(record.getName(), record.getAccess(), record.getOptions(),
+              record.Attrs);
+    return llvm::Error::success();
+  }
+
+  void AddMethod(llvm::StringRef name, MemberAccess access,
+                 MethodOptions options, MemberAttributes attrs) {
+    if (name != proc_name || function_decl)
+      return;
+    lldb::AccessType access_type = TranslateMemberAccess(access);
+    bool is_virtual = attrs.isVirtual();
+    bool is_static = attrs.isStatic();
+    bool is_artificial = (options & MethodOptions::CompilerGenerated) ==
+                         MethodOptions::CompilerGenerated;
+    function_decl = m_clang.AddMethodToCXXRecordType(
+        parent_ty, proc_name,
+        /*mangled_name=*/nullptr, func_ct, /*access=*/access_type,
+        /*is_virtual=*/is_virtual, /*is_static=*/is_static,
+        /*is_inline=*/false, /*is_explicit=*/false,
+        /*is_attr_used=*/false, /*is_artificial=*/is_artificial);
+  }
+};
+} // namespace
+
 static llvm::Optional<PdbCompilandSymId> FindSymbolScope(PdbIndex &index,
                                                          PdbCompilandSymId id) {
   CVSymbol sym = index.ReadSymbolRecord(id);
   if (symbolOpensScope(sym.kind())) {
     // If this exact symbol opens a scope, we can just directly access its
     // parent.
     id.offset = getScopeParentOffset(sym);
     // Global symbols have parent offset of 0.  Return llvm::None to indicate
bool PdbAstBuilder::CompleteTagDecl(clang::TagDecl &tag) {
   TypeIndex field_list_ti = GetFieldListIndex(cvt);
   CVType field_list_cvt = m_index.tpi().getType(field_list_ti);
   if (field_list_cvt.kind() != LF_FIELDLIST)
     return false;
 
   // Visit all members of this class, then perform any finalization necessary
   // to complete the class.
   CompilerType ct = ToCompilerType(tag_qt);
-  UdtRecordCompleter completer(best_ti, ct, tag, *this, m_index);
+  UdtRecordCompleter completer(best_ti, ct, tag, *this, m_index,
+                               m_cxx_record_map);
   auto error =
       llvm::codeview::visitMemberRecordStream(field_list_cvt.data(), completer);
   completer.complete();
 
   status.resolved = true;
   if (!error)
     return true;
 
const clang::FunctionProtoType *func_type =
       llvm::dyn_cast<clang::FunctionProtoType>(qt);
 
   CompilerType func_ct = ToCompilerType(qt);
 
   llvm::StringRef proc_name = proc.Name;
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
 
-  clang::FunctionDecl *function_decl = m_clang.CreateFunctionDeclaration(
+  clang::FunctionDecl *function_decl = nullptr;
+  if (parent->isRecord()) {
+    clang::QualType parent_qt = llvm::dyn_cast<clang::TypeDecl>(parent)
+                                    ->getTypeForDecl()
+                                    ->getCanonicalTypeInternal();
+    lldb::opaque_compiler_type_t parent_opaque_ty =
+        ToCompilerType(parent_qt).GetOpaqueQualType();
+
+    auto iter = m_cxx_record_map.find(parent_opaque_ty);
+    if (iter != m_cxx_record_map.end()) {
+      if (iter->getSecond().contains({proc_name, func_ct})) {
+        return nullptr;
+      }
+    }
+
+    CVType cvt = m_index.tpi().getType(type_id.index);
+    MemberFunctionRecord func_record(static_cast<TypeRecordKind>(cvt.kind()));
+    llvm::cantFail(TypeDeserializer::deserializeAs<MemberFunctionRecord>(
+        cvt, func_record));
+    TypeIndex class_index = func_record.getClassType();
+    CVType parent_cvt = m_index.tpi().getType(class_index);
+    ClassRecord class_record = CVTagRecord::create(parent_cvt).asClass();
+    // If it's a forward reference, try to get the real TypeIndex.
+    if (class_record.isForwardRef()) {
+      llvm::Expected<TypeIndex> eti =
+          m_index.tpi().findFullDeclForForwardRef(class_index);
+      if (eti) {
+        class_record =
+            CVTagRecord::create(m_index.tpi().getType(*eti)).asClass();
+      }
+    }
+    if (!class_record.FieldList.isSimple()) {
+      CVType field_list = m_index.tpi().getType(class_record.FieldList);
+      CreateMethodDecl process(m_index, m_clang, type_id.index, function_decl,
+                               parent_opaque_ty, proc_name, func_ct);
+      if (llvm::Error err = visitMemberRecordStream(field_list.data(), process))
+        llvm::consumeError(std::move(err));
+    }
+
+    if (!function_decl) {
+      function_decl = m_clang.AddMethodToCXXRecordType(
+          parent_opaque_ty, proc_name,
+          /*mangled_name=*/nullptr, func_ct,
+          /*access=*/lldb::AccessType::eAccessPublic,
+          /*is_virtual=*/false, /*is_static=*/false,
+          /*is_inline=*/false, /*is_explicit=*/false,
+          /*is_attr_used=*/false, /*is_artificial=*/false);
+    }
+
+    m_cxx_record_map[parent_opaque_ty].insert({proc_name, func_ct});
+  } else {
+    function_decl = m_clang.CreateFunctionDeclaration(
         parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+    CreateFunctionParameters(func_id, *function_decl,
+                             func_type->getNumParams());
+  }
 
   lldbassert(m_uid_to_decl.count(toOpaqueUid(func_id)) == 0);
   m_uid_to_decl[toOpaqueUid(func_id)] = function_decl;
   DeclStatus status;
   status.resolved = true;
   status.uid = toOpaqueUid(func_id);
   m_decl_to_status.insert({function_decl, status});
 
-  CreateFunctionParameters(func_id, *function_decl, func_type->getNumParams());
-
   return function_decl;
 }
 
 void PdbAstBuilder::CreateFunctionParameters(PdbCompilandSymId func_id,
                                              clang::FunctionDecl &function_decl,
                                              uint32_t param_count) {
   CompilandIndexItem *cii = m_index.compilands().GetCompiland(func_id.modi);
   CVSymbolArray scope =

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h

class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks {
 
   PdbTypeSymId m_id;
   CompilerType &m_derived_ct;
   clang::TagDecl &m_tag_decl;
   PdbAstBuilder &m_ast_builder;
   PdbIndex &m_index;
   std::vector<IndexedBase> m_bases;
   ClangASTImporter::LayoutInfo m_layout;
+  llvm::DenseMap<lldb::opaque_compiler_type_t,
+                 llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>>
+      &m_cxx_record_map;
 
 public:
-  UdtRecordCompleter(PdbTypeSymId id, CompilerType &derived_ct,
-                     clang::TagDecl &tag_decl, PdbAstBuilder &ast_builder,
-                     PdbIndex &index);
+  UdtRecordCompleter(
+      PdbTypeSymId id, CompilerType &derived_ct, clang::TagDecl &tag_decl,
+      PdbAstBuilder &ast_builder, PdbIndex &index,
+      llvm::DenseMap<lldb::opaque_compiler_type_t,
+                     llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>,
+                                    8>> &cxx_record_map);
 
 #define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
   llvm::Error visitKnownMember(llvm::codeview::CVMemberRecord &CVR,            \
                                llvm::codeview::Name##Record &Record) override;
 #define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #include "llvm/DebugInfo/CodeView/CodeViewTypes.def"
 
   void complete();

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

 using namespace llvm::codeview;
 using namespace llvm::pdb;
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::npdb;
 
 using Error = llvm::Error;
 
-UdtRecordCompleter::UdtRecordCompleter(PdbTypeSymId id,
-                                       CompilerType &derived_ct,
-                                       clang::TagDecl &tag_decl,
-                                       PdbAstBuilder &ast_builder,
-                                       PdbIndex &index)
+UdtRecordCompleter::UdtRecordCompleter(
+    PdbTypeSymId id, CompilerType &derived_ct, clang::TagDecl &tag_decl,
+    PdbAstBuilder &ast_builder, PdbIndex &index,
+    llvm::DenseMap<lldb::opaque_compiler_type_t,
+                   llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>>
+        &cxx_record_map)
     : m_id(id), m_derived_ct(derived_ct), m_tag_decl(tag_decl),
-      m_ast_builder(ast_builder), m_index(index) {
+      m_ast_builder(ast_builder), m_index(index),
+      m_cxx_record_map(cxx_record_map) {
   CVType cvt = m_index.tpi().getType(m_id.index);
   switch (cvt.kind()) {
   case LF_ENUM:
     llvm::cantFail(TypeDeserializer::deserializeAs<EnumRecord>(cvt, m_cvr.er));
     break;
   case LF_UNION:
     llvm::cantFail(TypeDeserializer::deserializeAs<UnionRecord>(cvt, m_cvr.ur));
     break;

 }
 
 void UdtRecordCompleter::AddMethod(llvm::StringRef name, TypeIndex type_idx,
                                    MemberAccess access, MethodOptions options,
                                    MemberAttributes attrs) {
   clang::QualType method_qt =
       m_ast_builder.GetOrCreateType(PdbTypeSymId(type_idx));
   m_ast_builder.CompleteType(method_qt);
+  CompilerType method_ct = m_ast_builder.ToCompilerType(method_qt);
+  lldb::opaque_compiler_type_t derived_opaque_ty = m_derived_ct.GetOpaqueQualType();
+  auto iter = m_cxx_record_map.find(derived_opaque_ty);
+  if (iter != m_cxx_record_map.end()) {
+    if (iter->getSecond().contains({name, method_ct})) {
+      return;
+    }
+  }
 
   lldb::AccessType access_type = TranslateMemberAccess(access);
   bool is_artificial = (options & MethodOptions::CompilerGenerated) ==
                        MethodOptions::CompilerGenerated;
   m_ast_builder.clang().AddMethodToCXXRecordType(
-      m_derived_ct.GetOpaqueQualType(), name.data(), nullptr,
-      m_ast_builder.ToCompilerType(method_qt), access_type, attrs.isVirtual(),
-      attrs.isStatic(), false, false, false, is_artificial);
+      derived_opaque_ty, name.data(), nullptr, method_ct,
+      access_type, attrs.isVirtual(), attrs.isStatic(), false, false, false,
+      is_artificial);
+
+  m_cxx_record_map[derived_opaque_ty].insert({name, method_ct});
 }
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            BaseClassRecord &base) {
   clang::QualType base_qt =
       AddBaseClassForTypeIndex(base.Type, base.getAccess());
 
   auto decl =

lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp

 // clang-format off
 // REQUIRES: lld, x86
 
 // RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -GR- -c /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
-// RUN:     %p/Inputs/ast-methods.lldbinit 2>&1 | FileCheck %s
+// RUN:     %p/Inputs/ast-methods.lldbinit 2>&1 | FileCheck %s --check-prefix=AST
+
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols --dump-ast %t.exe | FileCheck %s --check-prefix=SYMBOL
 
 struct Struct {
   void simple_method() {}
 
   virtual void virtual_method() {}
 
   static void static_method() {}
 
   int overloaded_method() {}
   int overloaded_method(char c) {}
   int overloaded_method(char c, int i, ...) {}
 };
 
 Struct s;
 
 int main(int argc, char **argv) {
+  s.simple_method();
+  s.static_method();
+  s.virtual_method();
+  s.overloaded_method();
+  s.overloaded_method('a');
+  s.overloaded_method('a', 1);
   return 0;
 }
 
-// CHECK: TranslationUnitDecl
-// CHECK: |-CXXRecordDecl {{.*}} struct Struct definition
-// CHECK: | |-CXXMethodDecl {{.*}} simple_method 'void (){{.*}}'
-// CHECK: | |-CXXMethodDecl {{.*}} virtual_method 'void (){{.*}}' virtual
-// CHECK: | |-CXXMethodDecl {{.*}} static_method 'void ()' static
-// CHECK: | |-CXXMethodDecl {{.*}} overloaded_method 'int (){{.*}}'
-// CHECK: | |-CXXMethodDecl {{.*}} overloaded_method 'int (char){{.*}}'
-// CHECK: | | `-ParmVarDecl {{.*}} 'char'
-// CHECK: | `-CXXMethodDecl {{.*}} overloaded_method 'int (char, int, ...)'
-// CHECK: |   |-ParmVarDecl {{.*}} 'char'
-// CHECK: |   `-ParmVarDecl {{.*}} 'int'
+// AST: TranslationUnitDecl
+// AST: |-CXXRecordDecl {{.*}} struct Struct definition
+// AST: | |-CXXMethodDecl {{.*}} simple_method 'void (){{.*}}'
+// AST: | |-CXXMethodDecl {{.*}} virtual_method 'void (){{.*}}' virtual
+// AST: | |-CXXMethodDecl {{.*}} static_method 'void ()' static
+// AST: | |-CXXMethodDecl {{.*}} overloaded_method 'int (){{.*}}'
+// AST: | |-CXXMethodDecl {{.*}} overloaded_method 'int (char){{.*}}'
+// AST: | | `-ParmVarDecl {{.*}} 'char'
+// AST: | `-CXXMethodDecl {{.*}} overloaded_method 'int (char, int, ...)'
+// AST: |   |-ParmVarDecl {{.*}} 'char'
+// AST: |   `-ParmVarDecl {{.*}} 'int'
+
+// SYMBOL:      int main(int argc, char **argv);
+// SYMBOL-NEXT: struct Struct {
+// SYMBOL-NEXT:     void simple_method();
+// SYMBOL-NEXT:     static void static_method();
+// SYMBOL-NEXT:     virtual void virtual_method();
+// SYMBOL-NEXT:     int overloaded_method();
+// SYMBOL-NEXT:     int overloaded_method(char);
+// SYMBOL-NEXT:     int overloaded_method(char, int, ...);
+// SYMBOL-NEXT: };

lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

 // clang-format off
 // REQUIRES: lld, x86
 
-// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /GR- /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 
 // RUN: lldb-test symbols --find=function --name=main --function-flags=full %t.exe \
 // RUN:     | FileCheck %s --check-prefix=FIND-MAIN
 
 // RUN: lldb-test symbols --find=function --name=static_fn --function-flags=full %t.exe \
 // RUN:     | FileCheck %s --check-prefix=FIND-STATIC
 
 // RUN: lldb-test symbols --find=function --name=varargs_fn --function-flags=full %t.exe \
 // RUN:     | FileCheck %s --check-prefix=FIND-VAR
 
+// RUN: lldb-test symbols --find=function --name=Struct::simple_method --function-flags=full %t.exe \
+// RUN:     | FileCheck %s --check-prefix=FIND-SIMPLE
+
+// RUN: lldb-test symbols --find=function --name=Struct::virtual_method --function-flags=full %t.exe \
+// RUN:     | FileCheck %s --check-prefix=FIND-VIRTUAL
+
+// RUN: lldb-test symbols --find=function --name=Struct::static_method --function-flags=full %t.exe \
+// RUN:     | FileCheck %s --check-prefix=FIND-STATIC-METHOD
+
+// RUN: lldb-test symbols --find=function --name=Struct::overloaded_method --function-flags=full %t.exe \
+// RUN:     | FileCheck %s --check-prefix=FIND-OVERLOAD
+
+struct Struct {
+  int simple_method() {
+    return 1;
+  }
+
+  virtual int virtual_method() {
+    return 2;
+  }
+
+  static int static_method() {
+    return 3;
+  }
+
+  int overloaded_method() {
+    return 4 + overloaded_method('a') + overloaded_method('a', 1);
+  }
+protected:
+  virtual int overloaded_method(char c) {
+    return 5;
+  }
+private:
+  static int overloaded_method(char c, int i, ...) {
+    return 6;
+  }
+};
+
+Struct s;
+
 static int static_fn() {
   return 42;
 }
 
 int varargs_fn(int x, int y, ...) {
   return x + y;
 }
 
 int main(int argc, char **argv) {
-  return static_fn() + varargs_fn(argc, argc);
+  return static_fn() + varargs_fn(argc, argc) + s.simple_method() +
+  Struct::static_method() + s.virtual_method() + s.overloaded_method();
 }
 
 // FIND-MAIN:      Function: id = {{.*}}, name = "main"
 // FIND-MAIN-NEXT: FuncType: id = {{.*}}, compiler_type = "int (int, char **)"
 
 // FIND-STATIC:      Function: id = {{.*}}, name = "{{.*}}static_fn{{.*}}"
 // FIND-STATIC-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
 
 // FIND-VAR:      Function: id = {{.*}}, name = "{{.*}}varargs_fn{{.*}}"
 // FIND-VAR-NEXT: FuncType: id = {{.*}}, compiler_type = "int (int, int, ...)"
+
+// FIND-SIMPLE:      Function: id = {{.*}}, name = "{{.*}}Struct::simple_method{{.*}}"
+// FIND-SIMPLE-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-VIRTUAL:      Function: id = {{.*}}, name = "{{.*}}Struct::virtual_method{{.*}}"
+// FIND-VIRTUAL-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-STATIC-METHOD:      Function: id = {{.*}}, name = "{{.*}}Struct::static_method{{.*}}"
+// FIND-STATIC-METHOD-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-OVERLOAD: Function: id = {{.*}}, name = "{{.*}}Struct::overloaded_method{{.*}}"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (void)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char, int, ...)"