[LLDB][NativePDB] Fix missing symbol info when backtrace minidump

[zequanwu]

Summary

I don't know how to add tests for this. I tested locally with a minidump, a pdb
and the executable.
Before this change, the following comand fail to give symbol information for
backtrace because the m_comp_units in m_index->compilands() was always
empty, changed to use GetCompileUnitAtIndex to create CU when failed to find
one at given index.

lldb -O 'target create GoogleDriveFS.exe --core GoogleDriveFS.dmp' -o 'target symbols add GoogleDriveFS.pdb' -o 'bt'

[labath]

IIUC, these are two separate fixes:
a) fix clang ast creation for methods
b) fix symbol context lookup
It would be better to split them into two patches, as each will require a different kind of a test. The ast thing could be tested by running "image dump ast" on an appropriate piece of debug info. I'm hoping that the second one could be tested through "image lookup -a", but I don't know if it'll be as simple as running just that command. I guess you might need to play around with it a bit to find a way to invoke it without first instantiating the compile unit. It's possible you will need more than one compile unit in the test executable.

I don't think either of the fixes is really tied to minidumps, so I hope it will be possible to reproduce the issues by just compiling some c++ (without even running the program).

[zequanwu]

The ast thing could be tested by running "image dump ast" on an appropriate piece of debug info.

I didn't observe any difference before and after this patch with image dump ast tested on https://github.com/llvm/llvm-project/blob/main/lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp.
I found that code path could get executed by FindFunctions when using image lookup -n ... and break ... on class methods, but there is a bug blocking me from adding tests for it. If the name has namespace like this A::foo, it fails to find it because lldb passes "foo" to plugins' FindFunctions and PdbIndex can only find the symbol if full name is provided. So, I need to either fix that bug first or find other ways to trigger this issue.

I'm hoping that the second one could be tested through "image lookup -a"

Yeah, sent out D113821.

[labath]

In D113724#3128929, @zequanwu wrote:

The ast thing could be tested by running "image dump ast" on an appropriate piece of debug info.

I didn't observe any difference before and after this patch with image dump ast tested on https://github.com/llvm/llvm-project/blob/main/lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp.
I found that code path could get executed by FindFunctions when using image lookup -n ... and break ... on class methods, but there is a bug blocking me from adding tests for it. If the name has namespace like this A::foo, it fails to find it because lldb passes "foo" to plugins' FindFunctions and PdbIndex can only find the symbol if full name is provided. So, I need to either fix that bug first or find other ways to trigger this issue.

Maybe you can try lldb-test? It's find functionality was meant to be a more-or-less direct interface to the Find*** methods (lldb-test symbols --find=function --name=full::name --function-flags=???)

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

1039

Could this be moved into the above if block?

[zequanwu]

Splitted into D113821 and D113930.

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

PdbAstBuilder::GetOrCreateFunctionDecl(PdbCompilandSymId func_id) {
   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();
+    // TODO: Get other information for this method.
+    function_decl = m_clang.AddMethodToCXXRecordType(
+        ToCompilerType(parent_qt).GetOpaqueQualType(), proc_name,
+        /*mangled_name=*/nullptr, func_ct, lldb::AccessType::eAccessPublic,
+        /*is_virtual=*/false, /*is_static=*/false,
+        /*is_inline=*/false, /*is_explicit=*/false,
+        /*is_attr_used=*/false, /*is_artificial=*/false);
+  } else {
+    function_decl = m_clang.CreateFunctionDeclaration(
         parent, OptionalClangModuleID(), proc_name, func_ct, storage, false);
+  }
 
   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());
+  if (!parent->isRecord())
+    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);

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

uint32_t SymbolFileNativePDB::ResolveSymbolContext(
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   uint32_t resolved_flags = 0;
   lldb::addr_t file_addr = addr.GetFileAddress();
 
   if (NeedsResolvedCompileUnit(resolve_scope)) {
     llvm::Optional<uint16_t> modi = m_index->GetModuleIndexForVa(file_addr);
     if (!modi)
       return 0;
-    CompilandIndexItem *cci = m_index->compilands().GetCompiland(*modi);
-    if (!cci)
+    CompUnitSP cu_sp = GetCompileUnitAtIndex(modi.getValue());
+    if (!cu_sp)
       return 0;
 
-    sc.comp_unit = GetOrCreateCompileUnit(*cci).get();
+    sc.comp_unit = cu_sp.get();
     resolved_flags |= eSymbolContextCompUnit;
   }
 
   if (resolve_scope & eSymbolContextFunction ||
       resolve_scope & eSymbolContextBlock) {
     lldbassert(sc.comp_unit);
     std::vector<SymbolAndUid> matches = m_index->FindSymbolsByVa(file_addr);
     // Search the matches in reverse.  This way if there are multiple matches