[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[Greg Clayton via lldb-commits]

================
@@ -1727,45 +1727,61 @@ lldb::ModuleSP SymbolFileDWARF::GetExternalModule(ConstString name) {
   return pos->second;
 }
 
-DWARFDIE
-SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
-  // This method can be called without going through the symbol vendor so we
-  // need to lock the module.
-  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
-
-  SymbolFileDWARF *symbol_file = nullptr;
-
+SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
   // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
   // must make sure we use the correct DWARF file when resolving things. On
   // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
   // SymbolFileDWARF classes, one for each .o file. We can often end up with
   // references to other DWARF objects and we must be ready to receive a
   // "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
   // instance.
+
   std::optional<uint32_t> file_index = die_ref.file_index();
+
+  // If the file index matches, then we have the right SymbolFileDWARF already.
+  // This will work for both .dwo file and DWARF in .o files for mac. Also if
+  // both the file indexes are invalid, then we have a match.
+  if (GetFileIndex() == file_index)
+    return this;
+
+  // If we are currently in a .dwo file and our file index doesn't match we need
+  // to let the base symbol file handle this.
+  SymbolFileDWARFDwo *dwo = llvm::dyn_cast_or_null<SymbolFileDWARFDwo>(this);
+  if (dwo)
+    return dwo->GetDIERefSymbolFile(die_ref);
+
   if (file_index) {
-    if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
-      symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case
-      if (symbol_file)
-        return symbol_file->DebugInfo().GetDIE(die_ref.section(),
-                                               die_ref.die_offset());
-      return DWARFDIE();
+    SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
+    if (debug_map) {
+      // We have a SymbolFileDWARFDebugMap, so let it find the right file
+      return debug_map->GetSymbolFileByOSOIndex(*file_index);
+    } else {
+      // Handle the .dwp file case correctly
+      if (*file_index == DIERef::k_file_index_mask)
+        return GetDwpSymbolFile().get(); // DWP case
+
+      // Handle the .dwo file case correctly
+      return DebugInfo()
+          .GetUnitAtIndex(*die_ref.file_index())
+          ->GetDwoSymbolFile(); // DWO case
     }
+  }
+  return this;
+}
 
-    if (*file_index == DIERef::k_file_index_mask)
-      symbol_file = GetDwpSymbolFile().get(); // DWP case
-    else
-      symbol_file = this->DebugInfo()
-                        .GetUnitAtIndex(*die_ref.file_index())
-                        ->GetDwoSymbolFile(); // DWO case
-  } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
+DWARFDIE
+SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
+  if (die_ref.die_offset() == DW_INVALID_OFFSET)
     return DWARFDIE();
-  }
 
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
+  SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref);
----------------
clayborg wrote:

It only uses the file index component, but that component might not be set in the `DIERef` object and if it isn't, it returns the current SymbolFileDWARF object. We can make a function like:
```
SymbolFileDWARF *GetSymbolFileByFileIndex(std::optional<uint32_t> file_index);
```
The file index only makes sense if it comes from a DIERef object as it understands the value and knows the magic value for the .dwp file, so the above API is much less clear as who else knows what a file index is and would actaully know to ask for the symbol file using such an index? Seems less clear to me than the existing API. Let me know if feel we should still change this

https://github.com/llvm/llvm-project/pull/87740