Modified IsForeignTypeUnit to return a std::optional and modify to work with new DWARF changes.

Author: clayborg

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

@@ -61,53 +61,50 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   return result;
 }
 
-bool
-DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry,
-                                        DWARFTypeUnit *&foreign_tu) const {
-  foreign_tu = nullptr;
+std::optional<DWARFTypeUnit *>
+DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry) const {
   std::optional<uint64_t> type_sig = entry.getForeignTUTypeSignature();
   if (!type_sig.has_value())
-    return false;
+    return std::nullopt;
   auto dwp_sp = m_debug_info.GetDwpSymbolFile();
-  if (dwp_sp) {
-    // We have a .dwp file, just get the type unit from there.
-    foreign_tu = dwp_sp->DebugInfo().GetTypeUnitForHash(*type_sig);
-  } else {
-    // We have a .dwo file that contains the type unit.
-    foreign_tu = nullptr; // TODO: fixme before checkin
+  if (!dwp_sp) {
+    // No .dwp file, we need to load the .dwo file.
+
+    // 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> unit_offset = entry.getForeignTUSkeletonCUOffset();
+    if (!unit_offset)
+      return nullptr; // Return NULL, this is a type unit, but couldn't find it.
+    DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
+                                                 *unit_offset);
+    if (!cu)
+      return nullptr; // Return NULL, this is a type unit, but couldn't find it.
+    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);
   }
-  if (foreign_tu == nullptr)
-    return true;
-  // If this entry represents a foreign type unit, 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 file, 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 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. We rely on each DebugNames::Entry either having
-  // both a DW_IDX_type_unit and a DW_IDX_compile_unit, or the .debug_names
-  // table has only a single compile unit with multiple type units. Once
-  // we find the skeleton compile unit, we make sure the DW_AT_dwo_name
-  // attributes match.
-  const llvm::DWARFDebugNames::NameIndex *name_index = entry.getNameIndex();
-  assert(name_index);
-  // Ask the entry for the skeleton compile unit offset.
+  // 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.
+
   std::optional<uint64_t> cu_offset = entry.getForeignTUSkeletonCUOffset();
-  // If the entry doesn't specify the skeleton compile unit offset, then check
-  // if the .debug_names table only has one compile unit. If so, then this is
-  // the skeleton compile unit we should used.
-  if (!cu_offset && name_index->getCUCount() == 1)
-    cu_offset = name_index->getCUOffset(0);
-
-  // If we couldn't find the skeleton compile unit offset, be safe and say there
-  // is no match. We don't want to use an invalid DIE offset on the wrong type
-  // unit.
   if (cu_offset) {
     DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::DebugInfo, *cu_offset);
     if (cu) {
@@ -117,27 +114,30 @@ DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry,
           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)
-        foreign_tu = nullptr; // Ignore this entry, the DWO name doesn't match.
-    } else {
-      foreign_tu = nullptr; // Ignore this entry, we can find the skeleton CU
+      if (cu_dwo_name == tu_dwo_name)
+        return foreign_tu; // We found a match!
     }
-  } else {
-    foreign_tu = nullptr; // Ignore this entry, we can find the skeleton CU
   }
-  return true;
+  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 = IsForeignTypeUnit(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)
     unit_offset = entry.getLocalTUOffset();
-  DWARFUnit *cu =
-      m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
-  return cu ? &cu->GetNonSkeletonUnit() : nullptr;
+  if (unit_offset) {
+    if (DWARFUnit *cu =
+        m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset))
+      return &cu->GetNonSkeletonUnit();
+  }
+  return nullptr;
 }
 
 DWARFDIE DebugNamesDWARFIndex::GetDIE(const DebugNames::Entry &entry) const {
@@ -349,14 +349,13 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
     if (!isType(entry.tag()))
       continue;
 
-
-    DWARFTypeUnit *foreign_tu = nullptr;
-    if (IsForeignTypeUnit(entry, foreign_tu)) {
-      // If we get a NULL foreign_tu back, the entry doesn't match the type unit
-      // in the .dwp file.
-      if (!foreign_tu)
+    // 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 = IsForeignTypeUnit(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

@@ -100,15 +100,17 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   /// \param[in] entry
   ///   The accelerator table entry to check.
   ///
-  /// \param[out] foreign_tu
-  ///   A reference to the foreign type unit pointer that will be filled in
-  ///   with a valid type unit if the entry matches the type unit, or filled in
-  ///   with NULL if the entry isn't valid for the type unit that ended up in
-  ///   the .dwp file.
-  ///
   /// \returns
-  ///   True if \a entry represents a foreign type unit, false otherwise.
-  bool IsForeignTypeUnit(const DebugNames::Entry &entry, DWARFTypeUnit *&foreign_tu) const;
+  ///   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 *>
+  IsForeignTypeUnit(const DebugNames::Entry &entry) const;
 
   DWARFTypeUnit *GetForeignTypeUnit(const DebugNames::Entry &entry) const;
 

llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp

@@ -678,15 +678,19 @@ DWARFDebugNames::Entry::getForeignTUSkeletonCUOffset() const {
     return std::nullopt;
   // Lookup the DW_IDX_compile_unit and make sure we have one, if we don't
   // we don't default to returning the first compile unit like getCUOffset().
+  std::optional<uint64_t> CUIndex;
   std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_compile_unit);
-  if (!Off)
-    return std::nullopt;
+  if (Off) {
+    CUIndex = Off->getAsUnsignedConstant();
+  } else {
+    // Check if there is only 1 CU and return that. Most .o files generate one
+    // .debug_names table per source file where there is 1 CU and many TUs.
+    if (NameIdx->getCUCount() == 1)
+      CUIndex = 0;
+  }
   // Extract the CU index and return the right CU offset.
-  if (std::optional<uint64_t> CUIndex = Off->getAsUnsignedConstant()) {
-    if (*CUIndex >= NameIdx->getCUCount())
-      return std::nullopt;
+  if (CUIndex && *CUIndex < NameIdx->getCUCount())
     return NameIdx->getCUOffset(*CUIndex);
-  }
   return std::nullopt;
 }