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

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 24 09:56:07 PDT 2024


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

>From 3f99b41eac0e04e15bdb99bea2ee75703936ea00 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/12] Add support for using foreign type units in
 .debug_names.

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 crashers that happen due to PeekDIEName() using wrong offsets
---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp       | 18 ++++
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h |  2 +
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 ++++++++++++-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  6 +-
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp     |  6 +-
 .../SymbolFile/DWARF/ManualDWARFIndex.h       |  7 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 66 ++++++++------
 .../SymbolFile/DWARF/SymbolFileDWARF.h        |  9 ++
 .../DWARF/x86/dwp-foreign-type-units.cpp      | 91 +++++++++++++++++++
 .../DebugInfo/DWARF/DWARFAcceleratorTable.h   | 11 +++
 .../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
 11 files changed, 257 insertions(+), 37 deletions(-)
 create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed12..056c6d4b0605f8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section section,
   return result;
 }
 
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
+  // Make sure we get the correct SymbolFileDWARF from the DIERef before
+  // asking for information from a debug info object. We might start with the
+  // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+  // might be pointing to a specific .dwo file or to the .dwp file. So this
+  // makes sure we get the right SymbolFileDWARF instance before finding the
+  // DWARFUnit that contains the offset. If we just use this object to do the
+  // search, we might be using the wrong .debug_info section from the wrong
+  // file with an offset meant for a different section.
+  SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+  return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+                                                       die_ref.die_offset());
+}
+
 DWARFUnit *
 DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
                                            dw_offset_t die_offset) {
@@ -232,6 +246,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());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea98..4d4555a3382529 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/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;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed4..d815d345b08ee7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
       module, std::move(index_up), debug_names, debug_str, dwarf));
 }
 
+
+llvm::DenseSet<uint64_t>
+DebugNamesDWARFIndex::GetTypeUnitSigs(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,17 +60,22 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   return result;
 }
 
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const {
+  std::optional<uint64_t> type_sig = entry.getForeignTUTypeSignature();
+  if (type_sig)
+    if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
+      return dwp_sp->DebugInfo().GetTypeUnitForHash(*type_sig);
+  return nullptr;
+}
+
 DWARFUnit *
 DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
   // 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;
-  }
-
   DWARFUnit *cu =
       m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
   return cu ? &cu->GetNonSkeletonUnit() : nullptr;
@@ -274,6 +291,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
     if (!isType(entry.tag()))
       continue;
 
+
+    DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+    if (foreign_tu) {
+      // 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.
+      const llvm::DWARFDebugNames::NameIndex *name_index = entry.getNameIndex();
+      if (name_index == nullptr)
+        continue;
+      // In order to determine if the type unit that ended up in a .dwp file
+      // is valid, we need to grab the type unit and check the attribute on the
+      // type unit matches the .dwo file. For this to happen we rely on each
+      // .dwo file having its own .debug_names table with a single compile unit
+      // and multiple type units. This is the only way we can tell if a type
+      // unit came from a specific .dwo file.
+      if (name_index->getCUCount() == 1) {
+        dw_offset_t cu_offset = name_index->getCUOffset(0);
+        DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::DebugInfo,
+                                                     cu_offset);
+        if (cu) {
+          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)
+            continue; // Ignore this entry, the CU DWO doesn't match the TU DWO
+        }
+      }
+    }
     // Grab at most one extra parent, subsequent parents are not necessary to
     // test equality.
     std::optional<llvm::SmallVector<Entry, 4>> parent_chain =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index a27a414ecdd193..94fce2ed9c175c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/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),
+                   GetTypeUnitSigs(*m_debug_names_up)) {}
 
   DWARFDebugInfo &m_debug_info;
 
@@ -85,6 +86,8 @@ class DebugNamesDWARFIndex : public DWARFIndex {
 
   DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
   DWARFDIE GetDIE(const DebugNames::Entry &entry) const;
+  DWARFTypeUnit *GetForeignTypeUnit(const DebugNames::Entry &entry) const;
+  // std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const;
   bool ProcessEntry(const DebugNames::Entry &entry,
                     llvm::function_ref<bool(DWARFDIE die)> callback);
 
@@ -97,6 +100,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
                                   llvm::StringRef name);
 
   static llvm::DenseSet<dw_offset_t> GetUnits(const DebugNames &debug_names);
+  static llvm::DenseSet<uint64_t> GetTypeUnitSigs(const DebugNames &debug_names);
 };
 
 } // namespace dwarf
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 92275600f99caa..103e157d3cac59 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -60,8 +60,10 @@ 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.count(tu->GetTypeHash()) == 0)
+          units_to_index.push_back(tu);
+      }
     }
   }
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
index 0126e587e52d85..3f0bb39dfc20c7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ b/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(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;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 0f3eab0186c4e8..4b4f7117f5c5de 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -693,7 +693,6 @@ llvm::DWARFDebugAbbrev *SymbolFileDWARF::DebugAbbrev() {
   if (debug_abbrev_data.GetByteSize() == 0)
     return nullptr;
 
-  ElapsedTime elapsed(m_parse_time);
   auto abbr =
       std::make_unique<llvm::DWARFDebugAbbrev>(debug_abbrev_data.GetAsLLVM());
   llvm::Error error = abbr->parse();
@@ -1727,14 +1726,7 @@ 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
@@ -1742,30 +1734,51 @@ SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
   // 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->GetBaseSymbolFile().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
 
-    if (*file_index == DIERef::k_file_index_mask)
-      symbol_file = GetDwpSymbolFile().get(); // DWP case
-    else
-      symbol_file = this->DebugInfo()
-                        .GetUnitAtIndex(*die_ref.file_index())
+      // Handle the .dwo file case correctly
+      return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
                         ->GetDwoSymbolFile(); // DWO case
-  } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
-    return DWARFDIE();
+    }
   }
+  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);
+  return DWARFDIE();
 }
 
 /// Return the DW_AT_(GNU_)dwo_id.
@@ -2721,7 +2734,6 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
       die_context = die.GetDeclContext();
     else
       die_context = die.GetTypeLookupContext();
-    assert(!die_context.empty());
     if (!query.ContextMatches(die_context))
       return true; // Keep iterating over index types, context mismatch.
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index bb2d949677d985..b81aaf0c20df50 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/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.
+  SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref);
+
   virtual DWARFDIE GetDIE(const DIERef &die_ref);
 
   DWARFDIE GetDIE(lldb::user_id_t uid);
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
new file mode 100644
index 00000000000000..36620591667901
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -0,0 +1,91 @@
+// REQUIRES: lld
+
+// This test will make a type that will be compiled differently into two
+// different .dwo files in a type unit with the same type hash, but with
+// differing contents. I have discovered that the hash for the type unit is
+// simply based off of the typename and doesn't seem to differ when the contents
+// differ, so that will help us test foreign type units in the .debug_names
+// section of the main executable. When a DWP file is made, only one type unit
+// will be kept and the type unit that is kept has the .dwo file name that it
+// came from. When LLDB loads the foreign type units, it needs to verify that
+// any entries from foreign type units come from the right .dwo file. We test
+// this since the contents of type units are not always the same even though
+// they have the same type hash. We don't want invalid accelerator table entries
+// to come from one .dwo file and be used on a type unit from another since this
+// could cause invalid lookups to happen. LLDB knows how to track down which
+// .dwo file a type unit comes from by looking at the DW_AT_dwo_name attribute
+// in the DW_TAG_type_unit.
+
+// Now test with DWARF5
+// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf \
+// RUN:   -fdebug-types-section -gpubnames -c %s -o %t.main.o
+// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT \
+// RUN:   -fdebug-types-section -gpubnames -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t
+
+// First we check when we make the .dwp file with %t.main.dwo first so it will
+// pick the type unit from %t.main.dwo. Verify we find only the types from
+// %t.main.dwo's type unit.
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.dwp
+// RUN: %lldb \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -o "type lookup FloatType" \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -b %t | FileCheck %s
+// CHECK: (lldb) type lookup IntegerType
+// CHECK-NEXT: int
+// CHECK-NEXT: (lldb) type lookup FloatType
+// CHECK-NEXT: double
+// CHECK-NEXT: (lldb) type lookup IntegerType
+// CHECK-NEXT: int
+
+// Next we check when we make the .dwp file with %t.foo.dwo first so it will
+// pick the type unit from %t.main.dwo. Verify we find only the types from
+// %t.main.dwo's type unit.
+// RUN: llvm-dwp %t.foo.dwo %t.main.dwo -o %t.dwp
+// RUN: %lldb \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -o "type lookup FloatType" \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -b %t | FileCheck %s --check-prefix=VARIANT
+
+// VARIANT: (lldb) type lookup IntegerType
+// VARIANT-NEXT: unsigned int
+// VARIANT-NEXT: (lldb) type lookup FloatType
+// VARIANT-NEXT: float
+// VARIANT-NEXT: (lldb) type lookup IntegerType
+// VARIANT-NEXT: unsigned int
+
+
+// We need to do this so we end with a type unit in each .dwo file and that has
+// the same signature but different contents. When we make the .dwp file, then
+// one of the type units will end up in the .dwp file and we will have
+// .debug_names accelerator tables for both type units and we need to ignore
+// the type units .debug_names entries that don't match the .dwo file whose
+// copy of the type unit ends up in the final .dwp file. To do this, LLDB will
+// look at the type unit and take the DWO name attribute and make sure it
+// matches, and if it doesn't, it will ignore the accelerator table entry.
+struct CustomType {
+  // We switch the order of "FloatType" and "IntegerType" so that if we do
+  // end up reading the wrong accelerator table entry, that we would end up
+  // getting an invalid offset and not find anything, or the offset would have
+  // matched and we would find the wrong thing.
+#ifdef VARIANT
+  typedef float FloatType;
+  typedef unsigned IntegerType;
+#else
+  typedef int IntegerType;
+  typedef double FloatType;
+#endif
+  IntegerType x;
+  FloatType y;
+};
+
+#ifdef VARIANT
+int foo() {
+#else
+int main() {
+#endif
+  CustomType c = {1, 2.0};
+  return 0;
+}
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 0d447a78cdc614..6c7132eb350215 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -64,6 +64,14 @@ class DWARFAcceleratorTable {
       return std::nullopt;
     }
 
+    /// Returns the type signature of the Type Unit associated with this
+    /// Accelerator Entry or std::nullopt if the Type Unit offset is not
+    /// recorded in this Accelerator Entry.
+    virtual std::optional<uint64_t> getForeignTUTypeSignature() const {
+      // Default return for accelerator tables that don't support type units.
+      return std::nullopt;
+    }
+
     /// Returns the Tag of the Debug Info Entry associated with this
     /// Accelerator Entry or std::nullopt if the Tag is not recorded in this
     /// Accelerator Entry.
@@ -433,8 +441,11 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     Entry(const NameIndex &NameIdx, const Abbrev &Abbr);
 
   public:
+    const NameIndex *getNameIndex() const { return NameIdx; }
     std::optional<uint64_t> getCUOffset() const override;
     std::optional<uint64_t> getLocalTUOffset() const override;
+    std::optional<uint64_t> getForeignTUTypeSignature() const override;
+
     std::optional<dwarf::Tag> getTag() const override { return tag(); }
 
     /// Returns the Index into the Compilation Unit list of the owning Name
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index ac19ac79329718..e120b59ccb4e6d 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -657,6 +657,19 @@ std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUOffset() const {
   return NameIdx->getLocalTUOffset(*Index);
 }
 
+std::optional<uint64_t>
+DWARFDebugNames::Entry::getForeignTUTypeSignature() const {
+  std::optional<uint64_t> Index = getLocalTUIndex();
+  const uint32_t NumLocalTUs = NameIdx->getLocalTUCount();
+  if (!Index || *Index < NumLocalTUs)
+    return std::nullopt;  // Invalid TU index or TU index is for a local TU
+  // The foreign TU index is the TU index minus the number of local TUs.
+  const uint64_t ForeignTUIndex = *Index - NumLocalTUs;
+  if (ForeignTUIndex >= NameIdx->getForeignTUCount())
+    return std::nullopt;  // Invalid foreign TU index.
+  return NameIdx->getForeignTUSignature(ForeignTUIndex);
+}
+
 std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUIndex() const {
   if (std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_type_unit))
     return Off->getAsUnsignedConstant();

>From d122c3a8bbc57b08f00e391ce58a1480b6c78e82 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Sun, 5 May 2024 09:36:56 -0700
Subject: [PATCH 02/12] Add support for BOLT generated .debug_names.

BOLT creates a single .debug_names table where foreign type units have both a DW_IDX_type_unit and a DW_IDX_compile_unit to uniquely identify the .dwo file that the type unit came from.
---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 110 +++++++++++-------
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  25 ++++
 .../DebugInfo/DWARF/DWARFAcceleratorTable.h   |  20 ++++
 .../DebugInfo/DWARF/DWARFAcceleratorTable.cpp |  20 ++++
 4 files changed, 134 insertions(+), 41 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index d815d345b08ee7..a2541c96cf77f5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -60,13 +60,71 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   return result;
 }
 
-DWARFTypeUnit *
-DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const {
+bool
+DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry,
+                                        DWARFTypeUnit *&foreign_tu) const {
+  foreign_tu = nullptr;
   std::optional<uint64_t> type_sig = entry.getForeignTUTypeSignature();
-  if (type_sig)
-    if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
-      return dwp_sp->DebugInfo().GetTypeUnitForHash(*type_sig);
-  return nullptr;
+  if (!type_sig.has_value())
+    return false;
+  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 (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.
+  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) {
+      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)
+        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
+    }
+  } else {
+    foreign_tu = nullptr; // Ignore this entry, we can find the skeleton CU
+  }
+  return true;
 }
 
 DWARFUnit *
@@ -292,42 +350,12 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
       continue;
 
 
-    DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
-    if (foreign_tu) {
-      // 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.
-      const llvm::DWARFDebugNames::NameIndex *name_index = entry.getNameIndex();
-      if (name_index == nullptr)
+    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)
         continue;
-      // In order to determine if the type unit that ended up in a .dwp file
-      // is valid, we need to grab the type unit and check the attribute on the
-      // type unit matches the .dwo file. For this to happen we rely on each
-      // .dwo file having its own .debug_names table with a single compile unit
-      // and multiple type units. This is the only way we can tell if a type
-      // unit came from a specific .dwo file.
-      if (name_index->getCUCount() == 1) {
-        dw_offset_t cu_offset = name_index->getCUOffset(0);
-        DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::DebugInfo,
-                                                     cu_offset);
-        if (cu) {
-          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)
-            continue; // Ignore this entry, the CU DWO doesn't match the TU DWO
-        }
-      }
     }
     // Grab at most one extra parent, subsequent parents are not necessary to
     // test equality.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 94fce2ed9c175c..984dbd84417b92 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -88,6 +88,31 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   DWARFDIE GetDIE(const DebugNames::Entry &entry) const;
   DWARFTypeUnit *GetForeignTypeUnit(const DebugNames::Entry &entry) const;
   // std::optional<DIERef> ToDIERef(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 true or false to indicate this. The \a foreign_tu pointer will be
+  /// filled in if this entry 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.
+  ///
+  /// \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;
+
+  DWARFTypeUnit *GetForeignTypeUnit(const DebugNames::Entry &entry) const;
+
+  std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const;
   bool ProcessEntry(const DebugNames::Entry &entry,
                     llvm::function_ref<bool(DWARFDIE die)> callback);
 
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 6c7132eb350215..bc9b11ea89587b 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -72,6 +72,25 @@ class DWARFAcceleratorTable {
       return std::nullopt;
     }
 
+    // Returns the the CU offset for a foreign TU.
+    //
+    // Entries that represent foreign type units can have both a
+    // DW_IDX_compile_unit and a DW_IDX_type_unit. In this case the
+    // DW_IDX_compile_unit represents the skeleton CU offset for the .dwo file
+    // that matches this foreign type unit entry. The type unit will have a
+    // DW_AT_dwo_name attribute that must match the attribute in the skeleton
+    // CU. This function is needed be because the getCUOffset() method will
+    // return the first CU if there is no DW_IDX_compile_unit attribute in this
+    // entry, and it won't return a value CU offset if there is a
+    // DW_IDX_type_unit. But this function will return std::nullopt if there is
+    // no DW_IDX_compile_unit attribute or if this doesn't represent a foreign
+    // type unit.
+    virtual std::optional<uint64_t> getForeignTUSkeletonCUOffset() const {
+      // Default return for accelerator tables that don't support type units.
+      return std::nullopt;
+    }
+
+
     /// Returns the Tag of the Debug Info Entry associated with this
     /// Accelerator Entry or std::nullopt if the Tag is not recorded in this
     /// Accelerator Entry.
@@ -445,6 +464,7 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     std::optional<uint64_t> getCUOffset() const override;
     std::optional<uint64_t> getLocalTUOffset() const override;
     std::optional<uint64_t> getForeignTUTypeSignature() const override;
+    std::optional<uint64_t> getForeignTUSkeletonCUOffset() const override;
 
     std::optional<dwarf::Tag> getTag() const override { return tag(); }
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index e120b59ccb4e6d..01d68ec5bcbf3e 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -670,6 +670,26 @@ DWARFDebugNames::Entry::getForeignTUTypeSignature() const {
   return NameIdx->getForeignTUSignature(ForeignTUIndex);
 }
 
+
+std::optional<uint64_t>
+DWARFDebugNames::Entry::getForeignTUSkeletonCUOffset() const {
+  // Must have a DW_IDX_type_unit and it must be a foreign type unit.
+  if (!getForeignTUTypeSignature())
+    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<DWARFFormValue> Off = lookup(dwarf::DW_IDX_compile_unit);
+  if (!Off)
+    return std::nullopt;
+  // 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;
+    return NameIdx->getCUOffset(*CUIndex);
+  }
+  return std::nullopt;
+}
+
 std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUIndex() const {
   if (std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_type_unit))
     return Off->getAsUnsignedConstant();

>From 91c3093248fa090c35bdff2edeedf83aa8cf5633 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Mon, 3 Jun 2024 16:05:06 -0700
Subject: [PATCH 03/12] Finish merges with upstream HEAD.

---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp       | 26 +++++++++----------
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp |  1 +
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  2 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  3 ++-
 4 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 056c6d4b0605f8..28df305a0c771d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,19 +222,19 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section section,
   return result;
 }
 
-DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-  // Make sure we get the correct SymbolFileDWARF from the DIERef before
-  // asking for information from a debug info object. We might start with the
-  // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
-  // might be pointing to a specific .dwo file or to the .dwp file. So this
-  // makes sure we get the right SymbolFileDWARF instance before finding the
-  // DWARFUnit that contains the offset. If we just use this object to do the
-  // search, we might be using the wrong .debug_info section from the wrong
-  // file with an offset meant for a different section.
-  SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
-  return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
-                                                       die_ref.die_offset());
-}
+// DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
+//   // Make sure we get the correct SymbolFileDWARF from the DIERef before
+//   // asking for information from a debug info object. We might start with the
+//   // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+//   // might be pointing to a specific .dwo file or to the .dwp file. So this
+//   // makes sure we get the right SymbolFileDWARF instance before finding the
+//   // DWARFUnit that contains the offset. If we just use this object to do the
+//   // search, we might be using the wrong .debug_info section from the wrong
+//   // file with an offset meant for a different section.
+//   SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+//   return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+//                                                        die_ref.die_offset());
+// }
 
 DWARFUnit *
 DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index a2541c96cf77f5..9802ea097acead 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/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"
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 984dbd84417b92..22f881e66c2bcc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -86,7 +86,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
 
   DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
   DWARFDIE GetDIE(const DebugNames::Entry &entry) const;
-  DWARFTypeUnit *GetForeignTypeUnit(const DebugNames::Entry &entry) const;
+
   // std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const;
 
   /// Checks if an entry is a foreign TU and fetch the type unit.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 4b4f7117f5c5de..db33ccd04b5cc5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1777,7 +1777,8 @@ SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref);
   if (symbol_file)
-    return symbol_file->DebugInfo().GetDIE(die_ref);
+    return symbol_file->DebugInfo().GetDIE(die_ref.section(),
+                                           die_ref.die_offset());
   return DWARFDIE();
 }
 

>From 7cdf24a70e9fef9d10bd6578bd460cf786366f4c Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 7 Jun 2024 14:36:22 -0700
Subject: [PATCH 04/12] Modified IsForeignTypeUnit to return a std::optional
 and modify to work with new DWARF changes.

---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 117 +++++++++---------
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  18 +--
 .../DebugInfo/DWARF/DWARFAcceleratorTable.cpp |  16 ++-
 3 files changed, 78 insertions(+), 73 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 9802ea097acead..0350305f6e913f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/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 {
@@ -350,14 +350,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 =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 22f881e66c2bcc..311eba5062d2c1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/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;
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 01d68ec5bcbf3e..7c268369785681 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/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;
 }
 

>From 843e0be87550e98a0bf13414d67593f05c257752 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 7 Jun 2024 15:45:10 -0700
Subject: [PATCH 05/12] Add tests for .dwo files.

---
 .../DWARF/x86/dwp-foreign-type-units.cpp      | 71 ++++++++++++++-----
 1 file changed, 54 insertions(+), 17 deletions(-)

diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 36620591667901..30f91de15fa4bb 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -23,21 +23,53 @@
 // RUN:   -fdebug-types-section -gpubnames -c %s -o %t.foo.o
 // RUN: ld.lld %t.main.o %t.foo.o -o %t
 
-// First we check when we make the .dwp file with %t.main.dwo first so it will
+// Check when have no .dwp file that we can find the types in both .dwo files.
+// RUN: rm -f %t.dwp
+// RUN: %lldb \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -o "type lookup FloatType" \
+// RUN:   -o "type lookup CustomType" \
+// RUN:   -b %t | FileCheck %s --check-prefix=NODWP
+// NODWP: (lldb) type lookup IntegerType
+// NODWP-NEXT: int
+// NODWP-NEXT: unsigned int
+// NODWP: (lldb) type lookup FloatType
+// NODWP-NEXT: double
+// NODWP-NEXT: float
+// NODWP: (lldb) type lookup CustomType
+// NODWP-NEXT: struct CustomType {
+// NODWP-NEXT:     typedef int IntegerType;
+// NODWP-NEXT:     typedef double FloatType;
+// NODWP-NEXT:     CustomType::IntegerType x;
+// NODWP-NEXT:     CustomType::FloatType y;
+// NODWP-NEXT: }
+// NODWP-NEXT: struct CustomType {
+// NODWP-NEXT:     typedef unsigned int IntegerType;
+// NODWP-NEXT:     typedef float FloatType;
+// NODWP-NEXT:     CustomType::IntegerType x;
+// NODWP-NEXT:     CustomType::FloatType y;
+// NODWP-NEXT: }
+
+// Check when we make the .dwp file with %t.main.dwo first so it will
 // pick the type unit from %t.main.dwo. Verify we find only the types from
 // %t.main.dwo's type unit.
 // RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.dwp
 // RUN: %lldb \
 // RUN:   -o "type lookup IntegerType" \
 // RUN:   -o "type lookup FloatType" \
-// RUN:   -o "type lookup IntegerType" \
-// RUN:   -b %t | FileCheck %s
-// CHECK: (lldb) type lookup IntegerType
-// CHECK-NEXT: int
-// CHECK-NEXT: (lldb) type lookup FloatType
-// CHECK-NEXT: double
-// CHECK-NEXT: (lldb) type lookup IntegerType
-// CHECK-NEXT: int
+// RUN:   -o "type lookup CustomType" \
+// RUN:   -b %t | FileCheck %s --check-prefix=DWPMAIN
+// DWPMAIN: (lldb) type lookup IntegerType
+// DWPMAIN-NEXT: int
+// DWPMAIN: (lldb) type lookup FloatType
+// DWPMAIN-NEXT: double
+// DWPMAIN: (lldb) type lookup CustomType
+// DWPMAIN-NEXT: struct CustomType {
+// DWPMAIN-NEXT:     typedef int IntegerType;
+// DWPMAIN-NEXT:     typedef double FloatType;
+// DWPMAIN-NEXT:     CustomType::IntegerType x;
+// DWPMAIN-NEXT:     CustomType::FloatType y;
+// DWPMAIN-NEXT: }
 
 // Next we check when we make the .dwp file with %t.foo.dwo first so it will
 // pick the type unit from %t.main.dwo. Verify we find only the types from
@@ -46,15 +78,20 @@
 // RUN: %lldb \
 // RUN:   -o "type lookup IntegerType" \
 // RUN:   -o "type lookup FloatType" \
-// RUN:   -o "type lookup IntegerType" \
-// RUN:   -b %t | FileCheck %s --check-prefix=VARIANT
+// RUN:   -o "type lookup CustomType" \
+// RUN:   -b %t | FileCheck %s --check-prefix=DWPFOO
 
-// VARIANT: (lldb) type lookup IntegerType
-// VARIANT-NEXT: unsigned int
-// VARIANT-NEXT: (lldb) type lookup FloatType
-// VARIANT-NEXT: float
-// VARIANT-NEXT: (lldb) type lookup IntegerType
-// VARIANT-NEXT: unsigned int
+// DWPFOO: (lldb) type lookup IntegerType
+// DWPFOO-NEXT: unsigned int
+// DWPFOO: (lldb) type lookup FloatType
+// DWPFOO-NEXT: float
+// DWPFOO: (lldb) type lookup CustomType
+// DWPFOO-NEXT: struct CustomType {
+// DWPFOO-NEXT:     typedef unsigned int IntegerType;
+// DWPFOO-NEXT:     typedef float FloatType;
+// DWPFOO-NEXT:     CustomType::IntegerType x;
+// DWPFOO-NEXT:     CustomType::FloatType y;
+// DWPFOO-NEXT: }
 
 
 // We need to do this so we end with a type unit in each .dwo file and that has

>From 04363bace262e4a8e94f2e91f39c5a5fbac4f8d8 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 7 Jun 2024 16:01:29 -0700
Subject: [PATCH 06/12] After merge, address review comments and remove dead
 code.

---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp       | 16 +-----------
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h |  2 +-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp |  2 +-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   | 18 +++++--------
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp     |  2 +-
 .../SymbolFile/DWARF/ManualDWARFIndex.h       |  2 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  2 ++
 .../DWARF/x86/dwp-foreign-type-units.cpp      | 26 +++++++++----------
 8 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 28df305a0c771d..f7df38d2401914 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,20 +222,6 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section section,
   return result;
 }
 
-// DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-//   // Make sure we get the correct SymbolFileDWARF from the DIERef before
-//   // asking for information from a debug info object. We might start with the
-//   // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
-//   // might be pointing to a specific .dwo file or to the .dwp file. So this
-//   // makes sure we get the right SymbolFileDWARF instance before finding the
-//   // DWARFUnit that contains the offset. If we just use this object to do the
-//   // search, we might be using the wrong .debug_info section from the wrong
-//   // file with an offset meant for a different section.
-//   SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
-//   return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
-//                                                        die_ref.die_offset());
-// }
-
 DWARFUnit *
 DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
                                            dw_offset_t die_offset) {
@@ -246,7 +232,7 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
   return result;
 }
 
-const std::shared_ptr<SymbolFileDWARFDwo> DWARFDebugInfo::GetDwpSymbolFile() {
+const std::shared_ptr<SymbolFileDWARFDwo> &DWARFDebugInfo::GetDwpSymbolFile() {
   return m_dwarf.GetDwpSymbolFile();
 }
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4d4555a3382529..598739bf3cb958 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,7 +52,7 @@ class DWARFDebugInfo {
 
   const DWARFDebugAranges &GetCompileUnitAranges();
 
-  const std::shared_ptr<SymbolFileDWARFDwo> GetDwpSymbolFile();
+  const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile();
 
 protected:
   typedef std::vector<DWARFUnitSP> UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 0350305f6e913f..029b1fbb3d8ffb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -37,7 +37,7 @@ DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
 
 
 llvm::DenseSet<uint64_t>
-DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames &debug_names) {
+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();
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 311eba5062d2c1..93ba3f5a668830 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -71,7 +71,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
         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),
-                   GetTypeUnitSigs(*m_debug_names_up)) {}
+                   GetTypeUnitSignatures(*m_debug_names_up)) {}
 
   DWARFDebugInfo &m_debug_info;
 
@@ -87,15 +87,14 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
   DWARFDIE GetDIE(const DebugNames::Entry &entry) const;
 
-  // std::optional<DIERef> ToDIERef(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 true or false to indicate this. The \a foreign_tu pointer will be
-  /// filled in if this entry 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.
+  /// 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.
@@ -112,9 +111,6 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   std::optional<DWARFTypeUnit *>
   IsForeignTypeUnit(const DebugNames::Entry &entry) const;
 
-  DWARFTypeUnit *GetForeignTypeUnit(const DebugNames::Entry &entry) const;
-
-  std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const;
   bool ProcessEntry(const DebugNames::Entry &entry,
                     llvm::function_ref<bool(DWARFDIE die)> callback);
 
@@ -127,7 +123,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
                                   llvm::StringRef name);
 
   static llvm::DenseSet<dw_offset_t> GetUnits(const DebugNames &debug_names);
-  static llvm::DenseSet<uint64_t> GetTypeUnitSigs(const DebugNames &debug_names);
+  static llvm::DenseSet<uint64_t> GetTypeUnitSignatures(const DebugNames &debug_names);
 };
 
 } // namespace dwarf
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 103e157d3cac59..118ed97f802f7e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -61,7 +61,7 @@ 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))) {
-        if (m_type_sigs_to_avoid.count(tu->GetTypeHash()) == 0)
+        if (!m_type_sigs_to_avoid.contains(tu->GetTypeHash()))
           units_to_index.push_back(tu);
       }
     }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
index 3f0bb39dfc20c7..d8c4a22ab21f7b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -25,7 +25,7 @@ class ManualDWARFIndex : public DWARFIndex {
                    llvm::DenseSet<uint64_t> type_sigs_to_avoid = {})
       : DWARFIndex(module), m_dwarf(&dwarf),
         m_units_to_avoid(std::move(units_to_avoid)),
-        m_type_sigs_to_avoid(type_sigs_to_avoid) {}
+        m_type_sigs_to_avoid(std::move(type_sigs_to_avoid)) {}
 
   void Preload() override { Index(); }
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index db33ccd04b5cc5..ddee74b280b621 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -693,6 +693,7 @@ llvm::DWARFDebugAbbrev *SymbolFileDWARF::DebugAbbrev() {
   if (debug_abbrev_data.GetByteSize() == 0)
     return nullptr;
 
+  ElapsedTime elapsed(m_parse_time);
   auto abbr =
       std::make_unique<llvm::DWARFDebugAbbrev>(debug_abbrev_data.GetAsLLVM());
   llvm::Error error = abbr->parse();
@@ -2735,6 +2736,7 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
       die_context = die.GetDeclContext();
     else
       die_context = die.GetTypeLookupContext();
+    assert(!die_context.empty());
     if (!query.ContextMatches(die_context))
       return true; // Keep iterating over index types, context mismatch.
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 30f91de15fa4bb..90e8220837abe5 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -2,19 +2,19 @@
 
 // This test will make a type that will be compiled differently into two
 // different .dwo files in a type unit with the same type hash, but with
-// differing contents. I have discovered that the hash for the type unit is
-// simply based off of the typename and doesn't seem to differ when the contents
-// differ, so that will help us test foreign type units in the .debug_names
-// section of the main executable. When a DWP file is made, only one type unit
-// will be kept and the type unit that is kept has the .dwo file name that it
-// came from. When LLDB loads the foreign type units, it needs to verify that
-// any entries from foreign type units come from the right .dwo file. We test
-// this since the contents of type units are not always the same even though
-// they have the same type hash. We don't want invalid accelerator table entries
-// to come from one .dwo file and be used on a type unit from another since this
-// could cause invalid lookups to happen. LLDB knows how to track down which
-// .dwo file a type unit comes from by looking at the DW_AT_dwo_name attribute
-// in the DW_TAG_type_unit.
+// differing contents. Clang's type unit signature is based only on the mangled
+// name of the type, regardless of the contents of the type, so that will help
+// us test foreign type units in the .debug_names section of the main
+// executable. When a DWP file is made, only one type unit will be kept and the
+// type unit that is kept has the .dwo file name that it came from. When LLDB
+// loads the foreign type units, it needs to verify that any entries from
+// foreign type units come from the right .dwo file. We test this since the
+// contents of type units are not always the same even though they have the same
+// type hash. We don't want invalid accelerator table entries to come from one
+// .dwo file and be used on a type unit from another since this could cause
+// invalid lookups to happen. LLDB knows how to track down which .dwo file a
+// type unit comes from by looking at the DW_AT_dwo_name attribute in the
+// DW_TAG_type_unit.
 
 // Now test with DWARF5
 // RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf \

>From 46cb76bde8953cbb92a9b4f0626d086d75f3cc9e Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 7 Jun 2024 16:02:26 -0700
Subject: [PATCH 07/12] Ran clang-format.

---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp      | 14 +++++++-------
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h        |  3 ++-
 .../Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp  |  3 ++-
 .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp   |  7 ++++---
 .../DWARF/x86/dwp-foreign-type-units.cpp           |  1 -
 .../llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h   |  1 -
 llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp |  5 ++---
 7 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 029b1fbb3d8ffb..ea9227f2bcd5cd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -35,7 +35,6 @@ 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;
@@ -77,8 +76,8 @@ DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry) const {
     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);
+    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();
@@ -86,7 +85,8 @@ DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry) const {
     // 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);
+    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
@@ -133,8 +133,8 @@ DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
   if (!unit_offset)
     unit_offset = entry.getLocalTUOffset();
   if (unit_offset) {
-    if (DWARFUnit *cu =
-        m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset))
+    if (DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
+                                                     *unit_offset))
       return &cu->GetNonSkeletonUnit();
   }
   return nullptr;
@@ -355,7 +355,7 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
     // didn't match.
     std::optional<DWARFTypeUnit *> foreign_tu = IsForeignTypeUnit(entry);
     if (foreign_tu && foreign_tu.value() == nullptr)
-        continue;
+      continue;
 
     // Grab at most one extra parent, subsequent parents are not necessary to
     // test equality.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 93ba3f5a668830..6233c053f5f005 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -123,7 +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);
+  static llvm::DenseSet<uint64_t>
+  GetTypeUnitSignatures(const DebugNames &debug_names);
 };
 
 } // namespace dwarf
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 118ed97f802f7e..d581d3773ab23e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -60,7 +60,8 @@ 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))) {
+      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);
       }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index ddee74b280b621..fce68ac85f14a6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1753,7 +1753,7 @@ SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
   if (file_index) {
     SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
     if (debug_map) {
-        // We have a SymbolFileDWARFDebugMap, so let it find the right file
+      // We have a SymbolFileDWARFDebugMap, so let it find the right file
       return debug_map->GetSymbolFileByOSOIndex(*file_index);
     } else {
       // Handle the .dwp file case correctly
@@ -1761,8 +1761,9 @@ SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
         return GetDwpSymbolFile().get(); // DWP case
 
       // Handle the .dwo file case correctly
-      return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
-                        ->GetDwoSymbolFile(); // DWO case
+      return DebugInfo()
+          .GetUnitAtIndex(*die_ref.file_index())
+          ->GetDwoSymbolFile(); // DWO case
     }
   }
   return this;
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 90e8220837abe5..046395670d0465 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -93,7 +93,6 @@
 // DWPFOO-NEXT:     CustomType::FloatType y;
 // DWPFOO-NEXT: }
 
-
 // We need to do this so we end with a type unit in each .dwo file and that has
 // the same signature but different contents. When we make the .dwp file, then
 // one of the type units will end up in the .dwp file and we will have
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index bc9b11ea89587b..7765a0607c3f97 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -90,7 +90,6 @@ class DWARFAcceleratorTable {
       return std::nullopt;
     }
 
-
     /// Returns the Tag of the Debug Info Entry associated with this
     /// Accelerator Entry or std::nullopt if the Tag is not recorded in this
     /// Accelerator Entry.
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 7c268369785681..0ea73ad9c23a44 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -662,15 +662,14 @@ DWARFDebugNames::Entry::getForeignTUTypeSignature() const {
   std::optional<uint64_t> Index = getLocalTUIndex();
   const uint32_t NumLocalTUs = NameIdx->getLocalTUCount();
   if (!Index || *Index < NumLocalTUs)
-    return std::nullopt;  // Invalid TU index or TU index is for a local TU
+    return std::nullopt; // Invalid TU index or TU index is for a local TU
   // The foreign TU index is the TU index minus the number of local TUs.
   const uint64_t ForeignTUIndex = *Index - NumLocalTUs;
   if (ForeignTUIndex >= NameIdx->getForeignTUCount())
-    return std::nullopt;  // Invalid foreign TU index.
+    return std::nullopt; // Invalid foreign TU index.
   return NameIdx->getForeignTUSignature(ForeignTUIndex);
 }
 
-
 std::optional<uint64_t>
 DWARFDebugNames::Entry::getForeignTUSkeletonCUOffset() const {
   // Must have a DW_IDX_type_unit and it must be a foreign type unit.

>From e0fd0698b8b4530283be9a614bf5e69694102b02 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Mon, 10 Jun 2024 17:32:54 -0700
Subject: [PATCH 08/12] Respond to user comments.

---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 48 +++++++++----------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index ea9227f2bcd5cd..23dd5c4bfb11c0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -65,21 +65,23 @@ DebugNamesDWARFIndex::IsForeignTypeUnit(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.getForeignTUSkeletonCUOffset();
+  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.
-
-    // 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.
@@ -104,20 +106,14 @@ DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry) const {
   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 (cu_offset) {
-    DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::DebugInfo, *cu_offset);
-    if (cu) {
-      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!
-    }
-  }
+  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.
 }
 

>From 720f654cd5621b3163d736c40c26b1ac004556d2 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 11 Jun 2024 11:12:39 -0700
Subject: [PATCH 09/12] Address user feedback.

- Make GetDIERefSymbolFile() virtual in SymbolFileDWARF and override in SymbolFileDWARFDwo
- Rename IsForeignTypeUnit to GetForeignTypeUnit
---
 .../Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp       | 6 +++---
 lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h | 2 +-
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp    | 2 +-
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h      | 2 +-
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +++++
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h   | 2 ++
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 23dd5c4bfb11c0..a7cf00b6cdc332 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -61,7 +61,7 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
 }
 
 std::optional<DWARFTypeUnit *>
-DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry) const {
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const {
   std::optional<uint64_t> type_sig = entry.getForeignTUTypeSignature();
   if (!type_sig.has_value())
     return std::nullopt;
@@ -120,7 +120,7 @@ DebugNamesDWARFIndex::IsForeignTypeUnit(const DebugNames::Entry &entry) const {
 DWARFUnit *
 DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
 
-  if (std::optional<DWARFTypeUnit *> foreign_tu = IsForeignTypeUnit(entry))
+  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
@@ -349,7 +349,7 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
     // 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);
+    std::optional<DWARFTypeUnit *> foreign_tu = GetForeignTypeUnit(entry);
     if (foreign_tu && foreign_tu.value() == nullptr)
       continue;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 6233c053f5f005..cb15c1d4f994b3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -109,7 +109,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   ///     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;
+  GetForeignTypeUnit(const DebugNames::Entry &entry) const;
 
   bool ProcessEntry(const DebugNames::Entry &entry,
                     llvm::function_ref<bool(DWARFDIE die)> callback);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index fce68ac85f14a6..bbcba495f9960d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1748,7 +1748,7 @@ SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
   // to let the base symbol file handle this.
   SymbolFileDWARFDwo *dwo = llvm::dyn_cast_or_null<SymbolFileDWARFDwo>(this);
   if (dwo)
-    return dwo->GetBaseSymbolFile().GetDIERefSymbolFile(die_ref);
+    return dwo->GetDIERefSymbolFile(die_ref);
 
   if (file_index) {
     SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index b81aaf0c20df50..51c22912066b4f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -248,7 +248,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
   /// 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.
-  SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref);
+  virtual SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref);
 
   virtual DWARFDIE GetDIE(const DIERef &die_ref);
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index 71c9997e4c82c1..365cba64959823 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/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);
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
index 1500540424b524..6b7f672aaa57ac 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ b/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;
 

>From 6f786cc2f6cc6adb58d1e2f165a204cfe82c0d3b Mon Sep 17 00:00:00 2001
From: paperchalice <liujunchang97 at outlook.com>
Date: Mon, 10 Jun 2024 22:12:33 +0800
Subject: [PATCH 10/12] Revert "[Misc] Use `LLVM_ENABLE_ABI_BREAKING_CHECKS`
 correctly" (#94982)

Reverts llvm/llvm-project#94212
Some codes assume that `NDEBUG` implies
`LLVM_ENABLE_ABI_BREAKING_CHECKS`, thus #94212 breaks some build bots.

>From 49eb373fa946b3341964c0ae3657d6c58a805e40 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Thu, 20 Jun 2024 15:21:35 -0700
Subject: [PATCH 11/12] Address user feedback.

---
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp |  2 +-
 .../DebugInfo/DWARF/DWARFAcceleratorTable.h   | 30 ++++--------
 .../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 47 ++++++++-----------
 3 files changed, 30 insertions(+), 49 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index a7cf00b6cdc332..7e66b3dccf97fa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -70,7 +70,7 @@ DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const {
   // 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.getForeignTUSkeletonCUOffset();
+  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.
 
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 7765a0607c3f97..0117c90e77f990 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -72,24 +72,6 @@ class DWARFAcceleratorTable {
       return std::nullopt;
     }
 
-    // Returns the the CU offset for a foreign TU.
-    //
-    // Entries that represent foreign type units can have both a
-    // DW_IDX_compile_unit and a DW_IDX_type_unit. In this case the
-    // DW_IDX_compile_unit represents the skeleton CU offset for the .dwo file
-    // that matches this foreign type unit entry. The type unit will have a
-    // DW_AT_dwo_name attribute that must match the attribute in the skeleton
-    // CU. This function is needed be because the getCUOffset() method will
-    // return the first CU if there is no DW_IDX_compile_unit attribute in this
-    // entry, and it won't return a value CU offset if there is a
-    // DW_IDX_type_unit. But this function will return std::nullopt if there is
-    // no DW_IDX_compile_unit attribute or if this doesn't represent a foreign
-    // type unit.
-    virtual std::optional<uint64_t> getForeignTUSkeletonCUOffset() const {
-      // Default return for accelerator tables that don't support type units.
-      return std::nullopt;
-    }
-
     /// Returns the Tag of the Debug Info Entry associated with this
     /// Accelerator Entry or std::nullopt if the Tag is not recorded in this
     /// Accelerator Entry.
@@ -463,10 +445,13 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     std::optional<uint64_t> getCUOffset() const override;
     std::optional<uint64_t> getLocalTUOffset() const override;
     std::optional<uint64_t> getForeignTUTypeSignature() const override;
-    std::optional<uint64_t> getForeignTUSkeletonCUOffset() const override;
-
     std::optional<dwarf::Tag> getTag() const override { return tag(); }
 
+    // Special function that will return the related CU offset needed type 
+    // units. This gets used to find the .dwo file that originated the entries
+    // for a given type unit.
+    std::optional<uint64_t> getRelatedCUOffset() const;
+
     /// Returns the Index into the Compilation Unit list of the owning Name
     /// Index or std::nullopt if this Accelerator Entry does not have an
     /// associated Compilation Unit. It is up to the user to verify that the
@@ -478,6 +463,11 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     /// attribute.
     std::optional<uint64_t> getCUIndex() const;
 
+    /// Similar functionality to getCUIndex() but without the DW_IDX_type_unit
+    /// restriction. This allows us to get the associated a compilation unit
+    /// index for an entry that is a type unit.
+    std::optional<uint64_t> getRelatedCUIndex() const;
+
     /// Returns the Index into the Local Type Unit list of the owning Name
     /// Index or std::nullopt if this Accelerator Entry does not have an
     /// associated Type Unit. It is up to the user to verify that the
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 0ea73ad9c23a44..7fba00d0e457b6 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -630,19 +630,26 @@ std::optional<uint64_t> DWARFDebugNames::Entry::getDIEUnitOffset() const {
   return std::nullopt;
 }
 
-std::optional<uint64_t> DWARFDebugNames::Entry::getCUIndex() const {
+std::optional<uint64_t> DWARFDebugNames::Entry::getRelatedCUIndex() const {
+  // Return the DW_IDX_compile_unit attribute value if it is specified.
   if (std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_compile_unit))
     return Off->getAsUnsignedConstant();
   // In a per-CU index, the entries without a DW_IDX_compile_unit attribute
-  // implicitly refer to the single CU, but only if we don't have a
-  // DW_IDX_type_unit.
-  if (lookup(dwarf::DW_IDX_type_unit).has_value())
-    return std::nullopt;
+  // implicitly refer to the single CU. 
   if (NameIdx->getCUCount() == 1)
     return 0;
   return std::nullopt;
 }
 
+std::optional<uint64_t> DWARFDebugNames::Entry::getCUIndex() const {
+  // Return the DW_IDX_compile_unit attribute value but only if we don't have a
+  // DW_IDX_type_unit attribute. Use Entry::getRelatedCUIndex() to get the
+  // associated CU index if this behaviour is not desired.
+  if (lookup(dwarf::DW_IDX_type_unit).has_value())
+    return std::nullopt;
+  return getRelatedCUIndex();
+}
+
 std::optional<uint64_t> DWARFDebugNames::Entry::getCUOffset() const {
   std::optional<uint64_t> Index = getCUIndex();
   if (!Index || *Index >= NameIdx->getCUCount())
@@ -650,6 +657,13 @@ std::optional<uint64_t> DWARFDebugNames::Entry::getCUOffset() const {
   return NameIdx->getCUOffset(*Index);
 }
 
+std::optional<uint64_t> DWARFDebugNames::Entry::getRelatedCUOffset() const {
+  std::optional<uint64_t> Index = getRelatedCUIndex();
+  if (!Index || *Index >= NameIdx->getCUCount())
+    return std::nullopt;
+  return NameIdx->getCUOffset(*Index);
+}
+
 std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUOffset() const {
   std::optional<uint64_t> Index = getLocalTUIndex();
   if (!Index || *Index >= NameIdx->getLocalTUCount())
@@ -670,29 +684,6 @@ DWARFDebugNames::Entry::getForeignTUTypeSignature() const {
   return NameIdx->getForeignTUSignature(ForeignTUIndex);
 }
 
-std::optional<uint64_t>
-DWARFDebugNames::Entry::getForeignTUSkeletonCUOffset() const {
-  // Must have a DW_IDX_type_unit and it must be a foreign type unit.
-  if (!getForeignTUTypeSignature())
-    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) {
-    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 (CUIndex && *CUIndex < NameIdx->getCUCount())
-    return NameIdx->getCUOffset(*CUIndex);
-  return std::nullopt;
-}
-
 std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUIndex() const {
   if (std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_type_unit))
     return Off->getAsUnsignedConstant();

>From 9a1d8cfbf656e33ae6b65a234c5ea121659b43e8 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 21 Jun 2024 09:48:56 -0700
Subject: [PATCH 12/12] Respond to user feeback.

---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 27 +++++++------------
 .../DWARF/x86/dwp-foreign-type-units.cpp      | 15 +++--------
 2 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index bbcba495f9960d..bd81618ac914da 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1744,27 +1744,18 @@ SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
   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) {
-    SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
-    if (debug_map) {
       // We have a SymbolFileDWARFDebugMap, so let it find the right file
+\    if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
       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
-    }
+    
+    // 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;
 }
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
index 046395670d0465..8dd5a5472ed4c6 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
@@ -9,14 +9,13 @@
 // type unit that is kept has the .dwo file name that it came from. When LLDB
 // loads the foreign type units, it needs to verify that any entries from
 // foreign type units come from the right .dwo file. We test this since the
-// contents of type units are not always the same even though they have the same
-// type hash. We don't want invalid accelerator table entries to come from one
-// .dwo file and be used on a type unit from another since this could cause
+// contents of type units are not always the same even though they have the
+// same type hash. We don't want invalid accelerator table entries to come from
+// one .dwo file and be used on a type unit from another since this could cause
 // invalid lookups to happen. LLDB knows how to track down which .dwo file a
 // type unit comes from by looking at the DW_AT_dwo_name attribute in the
 // DW_TAG_type_unit.
 
-// Now test with DWARF5
 // RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf \
 // RUN:   -fdebug-types-section -gpubnames -c %s -o %t.main.o
 // RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT \
@@ -93,14 +92,6 @@
 // DWPFOO-NEXT:     CustomType::FloatType y;
 // DWPFOO-NEXT: }
 
-// We need to do this so we end with a type unit in each .dwo file and that has
-// the same signature but different contents. When we make the .dwp file, then
-// one of the type units will end up in the .dwp file and we will have
-// .debug_names accelerator tables for both type units and we need to ignore
-// the type units .debug_names entries that don't match the .dwo file whose
-// copy of the type unit ends up in the final .dwp file. To do this, LLDB will
-// look at the type unit and take the DWO name attribute and make sure it
-// matches, and if it doesn't, it will ignore the accelerator table entry.
 struct CustomType {
   // We switch the order of "FloatType" and "IntegerType" so that if we do
   // end up reading the wrong accelerator table entry, that we would end up



More information about the lldb-commits mailing list