https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/87740
>From d69497fc66ce092fd75fcbe7c64460a49a6e2172 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@gmail.com> Date: Sat, 30 Mar 2024 10:50:34 -0700 Subject: [PATCH 1/2] 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 | 16 +++- .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 + .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 68 +++++++++++++- .../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 5 +- .../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +- .../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 65 +++++++------ .../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, 260 insertions(+), 33 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 44febcfac3b000..0e111c8ec47f45 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp @@ -223,7 +223,17 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section section, } DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) { - return GetUnitContainingDIEOffset(die_ref.section(), die_ref.die_offset()); + // 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 * @@ -236,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 c1f0cb0203fb76..b7f99ce6282b1f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h @@ -58,6 +58,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 4da0d56fdcacb4..ba35b3b872e6c6 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,6 +60,15 @@ 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; +} + std::optional<DIERef> DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const { // Look for a DWARF unit offset (CU offset or local TU offset) as they are @@ -55,8 +76,15 @@ DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const { std::optional<uint64_t> unit_offset = entry.getCUOffset(); if (!unit_offset) { unit_offset = entry.getLocalTUOffset(); - if (!unit_offset) + if (!unit_offset) { + if (DWARFTypeUnit *tu = GetForeignTypeUnit(entry)) { + if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset()) + return DIERef(tu->GetSymbolFileDWARF().GetFileIndex(), + DIERef::Section::DebugInfo, + tu->GetOffset() + *die_offset); + } return std::nullopt; + } } DWARFUnit *cu = @@ -273,6 +301,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 b54dd1162d20ab..6b48ce4eea1875 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h @@ -71,7 +71,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; @@ -84,6 +85,7 @@ class DebugNamesDWARFIndex : public DWARFIndex { std::unique_ptr<DebugNames> m_debug_names_up; ManualDWARFIndex m_fallback; + 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 +99,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 1164bc62682a9a..b828b56cc36606 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(); @@ -1726,14 +1725,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 @@ -1741,29 +1733,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); - 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); + // 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. @@ -2717,7 +2731,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 2f8f80f8765cb8..a3e34278ec9422 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 f1d4fc72d5a727..2fe33ca6d29c30 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 9c65d85985f1bb..54d8f54affb52a 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp @@ -666,6 +666,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 42528eb2dcd69f44d0135a092d28b94368d0582e Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@gmail.com> Date: Sun, 5 May 2024 09:36:56 -0700 Subject: [PATCH 2/2] 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 | 153 +++++++++++------- .../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 22 +++ .../DebugInfo/DWARF/DWARFAcceleratorTable.h | 20 +++ .../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 20 +++ 4 files changed, 161 insertions(+), 54 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index ba35b3b872e6c6..e13d8868684f0e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -60,31 +60,109 @@ 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; } std::optional<DIERef> DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const { + + // All entries need to have a DIE offset. If they don't, we can't do anything + // with this entry. If we have a foreign TU, then this is the DIE offset + // within the type unit in the .dwo file, otherwise if we have a CU or a TU, + // it is the DIE offset within the .debug_info section. + std::optional<uint64_t> die_offset = entry.getDIEUnitOffset(); + if (!die_offset) + return std::nullopt; + + // Look for a foreign type unit first because a DebugNames::Entry might have + // both a DW_IDX_type_unit and a DW_IDX_compile_unit. If this is the case, + // then the DW_IDX_compile_unit specifies the .dwo file that the type unit + // originally came from in a .dwp file. DWP files get a single type unit and + // all other type units that have the same type signature do not make it into + // the .dwp file so we need to ignore any type unit .debug_names entries that + // do not match. + 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) + return DIERef(foreign_tu->GetSymbolFileDWARF().GetFileIndex(), + DIERef::Section::DebugInfo, + foreign_tu->GetOffset() + *die_offset); + return std::nullopt; + } + // 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(); - if (!unit_offset) { - if (DWARFTypeUnit *tu = GetForeignTypeUnit(entry)) { - if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset()) - return DIERef(tu->GetSymbolFileDWARF().GetFileIndex(), - DIERef::Section::DebugInfo, - tu->GetOffset() + *die_offset); - } + if (!unit_offset) return std::nullopt; - } } DWARFUnit *cu = @@ -93,11 +171,8 @@ DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const { return std::nullopt; cu = &cu->GetNonSkeletonUnit(); - if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset()) - return DIERef(cu->GetSymbolFileDWARF().GetFileIndex(), - DIERef::Section::DebugInfo, cu->GetOffset() + *die_offset); - - return std::nullopt; + return DIERef(cu->GetSymbolFileDWARF().GetFileIndex(), + DIERef::Section::DebugInfo, cu->GetOffset() + *die_offset); } bool DebugNamesDWARFIndex::ProcessEntry( @@ -302,42 +377,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 6b48ce4eea1875..7acdf62e361718 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h @@ -85,7 +85,29 @@ class DebugNamesDWARFIndex : public DWARFIndex { std::unique_ptr<DebugNames> m_debug_names_up; ManualDWARFIndex m_fallback; + /// 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 2fe33ca6d29c30..52b4243f6f87d7 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 54d8f54affb52a..482d03833d739e 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp @@ -679,6 +679,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(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits