Add support for using foreign type units in .debug_names. (#87740)

This patch adds support for the new foreign type unit support in
.debug_names. Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have
a .dwp file
- fix type unit lookups for .dwo files
- fix crashers that happen due to PeekDIEName() using wrong offsets where an entry had DW_IDX_comp_unit and DW_IDX_type_unit entries and when we had no type unit support, it would cause us to think it was a normal DIE in .debug_info from the main executable.

---------

Co-authored-by: paperchalice <[email protected]>

Author: clayborg

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp

@@ -232,6 +232,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
   return result;
 }
 
+const std::shared_ptr<SymbolFileDWARFDwo> &DWARFDebugInfo::GetDwpSymbolFile() {
+  return m_dwarf.GetDwpSymbolFile();
+}
+
 DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
   auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
                                std::make_pair(hash, 0u), llvm::less_first());

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

@@ -52,6 +52,8 @@ class DWARFDebugInfo {
 
   const DWARFDebugAranges &GetCompileUnitAranges();
 
+  const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile();
+
 protected:
   typedef std::vector<DWARFUnitSP> UnitColl;
 

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

@@ -10,6 +10,7 @@
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
@@ -34,6 +35,17 @@ DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
       module, std::move(index_up), debug_names, debug_str, dwarf));
 }
 
+llvm::DenseSet<uint64_t>
+DebugNamesDWARFIndex::GetTypeUnitSignatures(const DebugNames &debug_names) {
+  llvm::DenseSet<uint64_t> result;
+  for (const DebugNames::NameIndex &ni : debug_names) {
+    const uint32_t num_tus = ni.getForeignTUCount();
+    for (uint32_t tu = 0; tu < num_tus; ++tu)
+      result.insert(ni.getForeignTUSignature(tu));
+  }
+  return result;
+}
+
 llvm::DenseSet<dw_offset_t>
 DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   llvm::DenseSet<dw_offset_t> result;
@@ -48,20 +60,80 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   return result;
 }
 
+std::optional<DWARFTypeUnit *>
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const {
+  std::optional<uint64_t> type_sig = entry.getForeignTUTypeSignature();
+  if (!type_sig.has_value())
+    return std::nullopt;
+
+  // Ask the entry for the skeleton compile unit offset and fetch the .dwo
+  // file from it and get the type unit by signature from there. If we find
+  // the type unit in the .dwo file, we don't need to check that the
+  // DW_AT_dwo_name matches because each .dwo file can have its own type unit.
+  std::optional<uint64_t> cu_offset = entry.getRelatedCUOffset();
+  if (!cu_offset)
+    return nullptr; // Return NULL, this is a type unit, but couldn't find it.
+
+  DWARFUnit *cu =
+      m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *cu_offset);
+  if (!cu)
+    return nullptr; // Return NULL, this is a type unit, but couldn't find it.
+
+  auto dwp_sp = m_debug_info.GetDwpSymbolFile();
+  if (!dwp_sp) {
+    // No .dwp file, we need to load the .dwo file.
+    DWARFUnit &dwo_cu = cu->GetNonSkeletonUnit();
+    // We don't need the check if the type unit matches the .dwo file if we have
+    // a .dwo file (not a .dwp), so we can just return the value here.
+    if (!dwo_cu.IsDWOUnit())
+      return nullptr; // We weren't able to load the .dwo file.
+    return dwo_cu.GetSymbolFileDWARF().DebugInfo().GetTypeUnitForHash(
+        *type_sig);
+  }
+  // We have a .dwp file, just get the type unit from there. We need to verify
+  // that the type unit that ended up in the final .dwp file is the right type
+  // unit. Type units have signatures which are the same across multiple .dwo
+  // files, but only one of those type units will end up in the .dwp file. The
+  // contents of type units for the same type can be different in different .dwo
+  // files, which means the DIE offsets might not be the same between two
+  // different type units. So we need to determine if this accelerator table
+  // matches the type unit that ended up in the .dwp file. If it doesn't match,
+  // then we need to ignore this accelerator table entry as the type unit that
+  // is in the .dwp file will have its own index. In order to determine if the
+  // type unit that ended up in a .dwp file matches this DebugNames::Entry, we
+  // need to find the skeleton compile unit for this entry.
+  DWARFTypeUnit *foreign_tu = dwp_sp->DebugInfo().GetTypeUnitForHash(*type_sig);
+  if (!foreign_tu)
+    return nullptr; // Return NULL, this is a type unit, but couldn't find it.
+
+  DWARFBaseDIE cu_die = cu->GetUnitDIEOnly();
+  DWARFBaseDIE tu_die = foreign_tu->GetUnitDIEOnly();
+  llvm::StringRef cu_dwo_name =
+      cu_die.GetAttributeValueAsString(DW_AT_dwo_name, nullptr);
+  llvm::StringRef tu_dwo_name =
+      tu_die.GetAttributeValueAsString(DW_AT_dwo_name, nullptr);
+  if (cu_dwo_name == tu_dwo_name)
+    return foreign_tu; // We found a match!
+  return nullptr; // Return NULL, this is a type unit, but couldn't find it.
+}
+
 DWARFUnit *
 DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
+
+  if (std::optional<DWARFTypeUnit *> foreign_tu = GetForeignTypeUnit(entry))
+    return foreign_tu.value();
+
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.
   std::optional<uint64_t> unit_offset = entry.getCUOffset();
-  if (!unit_offset) {
+  if (!unit_offset)
     unit_offset = entry.getLocalTUOffset();
-    if (!unit_offset)
-      return nullptr;
+  if (unit_offset) {
+    if (DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
+                                                     *unit_offset))
+      return &cu->GetNonSkeletonUnit();
   }
-
-  DWARFUnit *cu =
-      m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
-  return cu ? &cu->GetNonSkeletonUnit() : nullptr;
+  return nullptr;
 }
 
 DWARFDIE DebugNamesDWARFIndex::GetDIE(const DebugNames::Entry &entry) const {
@@ -274,6 +346,13 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
     if (!isType(entry.tag()))
       continue;
 
+    // If we get a NULL foreign_tu back, the entry doesn't match the type unit
+    // in the .dwp file, or we were not able to load the .dwo file or the DWO ID
+    // didn't match.
+    std::optional<DWARFTypeUnit *> foreign_tu = GetForeignTypeUnit(entry);
+    if (foreign_tu && foreign_tu.value() == nullptr)
+      continue;
+
     // Grab at most one extra parent, subsequent parents are not necessary to
     // test equality.
     std::optional<llvm::SmallVector<Entry, 4>> parent_chain =

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h

@@ -70,7 +70,8 @@ class DebugNamesDWARFIndex : public DWARFIndex {
       : DWARFIndex(module), m_debug_info(dwarf.DebugInfo()),
         m_debug_names_data(debug_names_data), m_debug_str_data(debug_str_data),
         m_debug_names_up(std::move(debug_names_up)),
-        m_fallback(module, dwarf, GetUnits(*m_debug_names_up)) {}
+        m_fallback(module, dwarf, GetUnits(*m_debug_names_up),
+                   GetTypeUnitSignatures(*m_debug_names_up)) {}
 
   DWARFDebugInfo &m_debug_info;
 
@@ -85,6 +86,31 @@ class DebugNamesDWARFIndex : public DWARFIndex {
 
   DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
   DWARFDIE GetDIE(const DebugNames::Entry &entry) const;
+
+  /// Checks if an entry is a foreign TU and fetch the type unit.
+  ///
+  /// This function checks if the DebugNames::Entry refers to a foreign TU and
+  /// returns an optional with a value of the \a entry is a foreign type unit
+  /// entry. A valid pointer will be returned if this entry is from a .dwo file
+  /// or if it is from a .dwp file and it matches the type unit's originating
+  /// .dwo file by verifying that the DW_TAG_type_unit DIE has a DW_AT_dwo_name
+  /// that matches the DWO name from the originating skeleton compile unit.
+  ///
+  /// \param[in] entry
+  ///   The accelerator table entry to check.
+  ///
+  /// \returns
+  ///   A std::optional that has a value if this entry represents a foreign type
+  ///   unit. If the pointer is valid, then we were able to find and match the
+  ///   entry to the type unit in the .dwo or .dwp file. The returned value can
+  ///   have a valid, yet contain NULL in the following cases:
+  ///   - we were not able to load the .dwo file (missing or DWO ID mismatch)
+  ///   - we were able to load the .dwp file, but the type units DWO name
+  ///     doesn't match the originating skeleton compile unit's entry
+  ///   Returns std::nullopt if this entry is not a foreign type unit entry.
+  std::optional<DWARFTypeUnit *>
+  GetForeignTypeUnit(const DebugNames::Entry &entry) const;
+
   bool ProcessEntry(const DebugNames::Entry &entry,
                     llvm::function_ref<bool(DWARFDIE die)> callback);
 
@@ -97,6 +123,8 @@ class DebugNamesDWARFIndex : public DWARFIndex {
                                   llvm::StringRef name);
 
   static llvm::DenseSet<dw_offset_t> GetUnits(const DebugNames &debug_names);
+  static llvm::DenseSet<uint64_t>
+  GetTypeUnitSignatures(const DebugNames &debug_names);
 };
 
 } // namespace dwarf

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

@@ -60,8 +60,11 @@ void ManualDWARFIndex::Index() {
   }
   if (dwp_info && dwp_info->ContainsTypeUnits()) {
     for (size_t U = 0; U < dwp_info->GetNumUnits(); ++U) {
-      if (auto *tu = llvm::dyn_cast<DWARFTypeUnit>(dwp_info->GetUnitAtIndex(U)))
-        units_to_index.push_back(tu);
+      if (auto *tu =
+              llvm::dyn_cast<DWARFTypeUnit>(dwp_info->GetUnitAtIndex(U))) {
+        if (!m_type_sigs_to_avoid.contains(tu->GetTypeHash()))
+          units_to_index.push_back(tu);
+      }
     }
   }
 

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h

@@ -21,9 +21,11 @@ class SymbolFileDWARFDwo;
 class ManualDWARFIndex : public DWARFIndex {
 public:
   ManualDWARFIndex(Module &module, SymbolFileDWARF &dwarf,
-                   llvm::DenseSet<dw_offset_t> units_to_avoid = {})
+                   llvm::DenseSet<dw_offset_t> units_to_avoid = {},
+                   llvm::DenseSet<uint64_t> type_sigs_to_avoid = {})
       : DWARFIndex(module), m_dwarf(&dwarf),
-        m_units_to_avoid(std::move(units_to_avoid)) {}
+        m_units_to_avoid(std::move(units_to_avoid)),
+        m_type_sigs_to_avoid(std::move(type_sigs_to_avoid)) {}
 
   void Preload() override { Index(); }
 
@@ -170,6 +172,7 @@ class ManualDWARFIndex : public DWARFIndex {
   SymbolFileDWARF *m_dwarf;
   /// Which dwarf units should we skip while building the index.
   llvm::DenseSet<dw_offset_t> m_units_to_avoid;
+  llvm::DenseSet<uint64_t> m_type_sigs_to_avoid;
 
   IndexSet m_set;
   bool m_indexed = false;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -1727,45 +1727,52 @@ 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 (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();
-    }
 
+  // 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 (file_index) {
+      // We have a SymbolFileDWARFDebugMap, so let it find the right file
+\    if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
+      return debug_map->GetSymbolFileByOSOIndex(*file_index);
+    
+    // Handle the .dwp file case correctly
     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) {
-    return DWARFDIE();
+      return GetDwpSymbolFile().get(); // DWP case
+
+    // Handle the .dwo file case correctly
+    return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
+        ->GetDwoSymbolFile(); // DWO case
   }
+  return this;
+}
 
-  if (symbol_file)
-    return symbol_file->GetDIE(die_ref);
+DWARFDIE
+SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
+  if (die_ref.die_offset() == DW_INVALID_OFFSET)
+    return DWARFDIE();
 
-  return DebugInfo().GetDIE(die_ref.section(), die_ref.die_offset());
+  // 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);
+  if (symbol_file)
+    return symbol_file->DebugInfo().GetDIE(die_ref.section(),
+                                           die_ref.die_offset());
+  return DWARFDIE();
 }
 
 /// Return the DW_AT_(GNU_)dwo_id.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

@@ -241,6 +241,15 @@ class SymbolFileDWARF : public SymbolFileCommon {
     return m_external_type_modules;
   }
 
+  /// Given a DIERef, find the correct SymbolFileDWARF.
+  ///
+  /// A DIERef contains a file index that can uniquely identify a N_OSO file for
+  /// DWARF in .o files on mac, or a .dwo or .dwp file index for split DWARF.
+  /// Calling this function will find the correct symbol file to use so that
+  /// further lookups can be done on the correct symbol file so that the DIE
+  /// offset makes sense in the DIERef.
+  virtual SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref);
+
   virtual DWARFDIE GetDIE(const DIERef &die_ref);
 
   DWARFDIE GetDIE(lldb::user_id_t uid);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

@@ -174,3 +174,8 @@ bool SymbolFileDWARFDwo::GetDebugInfoHadFrameVariableErrors() const {
 void SymbolFileDWARFDwo::SetDebugInfoHadFrameVariableErrors() {
   return GetBaseSymbolFile().SetDebugInfoHadFrameVariableErrors();
 }
+
+SymbolFileDWARF *
+SymbolFileDWARFDwo::GetDIERefSymbolFile(const DIERef &die_ref) {
+  return GetBaseSymbolFile().GetDIERefSymbolFile(die_ref);
+}

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

@@ -67,6 +67,8 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
   bool GetDebugInfoHadFrameVariableErrors() const override;
   void SetDebugInfoHadFrameVariableErrors() override;
 
+  SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref) override;
+
 protected:
   DIEToTypePtr &GetDIEToType() override;