https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/87740
>From badd915257bb192add91696e0b8530c057bd385f Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 c37cc91e08ed1..056c6d4b0605f 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 4706b55d38ea9..4d4555a338252 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 1d17f20670eed..d815d345b08ee 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 a27a414ecdd19..94fce2ed9c175 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 92275600f99ca..103e157d3cac5 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 0126e587e52d8..3f0bb39dfc20c 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 0f3eab0186c4e..4b4f7117f5c5d 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 bb2d949677d98..b81aaf0c20df5 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 0000000000000..3662059166790 --- /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 0d447a78cdc61..6c7132eb35021 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 ac19ac7932971..e120b59ccb4e6 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 86f4e88a37cb02412564da0781764983a07e1a50 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 d815d345b08ee..a2541c96cf77f 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 94fce2ed9c175..984dbd84417b9 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 6c7132eb35021..bc9b11ea89587 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 e120b59ccb4e6..01d68ec5bcbf3 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 391a3490e423d07e772c41f234353ca0e70b1a1c Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 056c6d4b0605f..28df305a0c771 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 a2541c96cf77f..9802ea097acea 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 984dbd84417b9..22f881e66c2bc 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 4b4f7117f5c5d..db33ccd04b5cc 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 2532d36369d9c8da24175f4c5a1c41d37e6e7d58 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 9802ea097acea..0350305f6e913 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 22f881e66c2bc..311eba5062d2c 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 01d68ec5bcbf3..7c26836978568 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 2ad1c7afa38c0126f0fe3bc784108d229729ebf9 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 3662059166790..30f91de15fa4b 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 81cfe2dc74c286c1dad97518a0bf98a17105c03f Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 28df305a0c771..f7df38d240191 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 4d4555a338252..598739bf3cb95 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 0350305f6e913..029b1fbb3d8ff 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 311eba5062d2c..93ba3f5a66883 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 103e157d3cac5..118ed97f802f7 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 3f0bb39dfc20c..d8c4a22ab21f7 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 db33ccd04b5cc..ddee74b280b62 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 30f91de15fa4b..90e8220837abe 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 6b371fa516d6a1ed2ee56a742a6f49cc5d8436e0 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 029b1fbb3d8ff..ea9227f2bcd5c 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 93ba3f5a66883..6233c053f5f00 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 118ed97f802f7..d581d3773ab23 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 ddee74b280b62..fce68ac85f14a 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 90e8220837abe..046395670d046 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 bc9b11ea89587..7765a0607c3f9 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 7c26836978568..0ea73ad9c23a4 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 a896cba9c082d85386edbd0bbe1a6b023318f086 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 ea9227f2bcd5c..23dd5c4bfb11c 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 5abf50cb9d4b6d60321d4919806ca1fcf0ccb0f7 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 23dd5c4bfb11c..a7cf00b6cdc33 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 6233c053f5f00..cb15c1d4f994b 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 fce68ac85f14a..bbcba495f9960 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 b81aaf0c20df5..51c22912066b4 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 71c9997e4c82c..365cba6495982 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 1500540424b52..6b7f672aaa57a 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 234a1ad215bcb96bdceda375eca5b00cf82dd3ff Mon Sep 17 00:00:00 2001 From: paperchalice <liujunchan...@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 70030e7eebfefe771ce741d3c11154273a8df623 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 a7cf00b6cdc33..7e66b3dccf97f 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 7765a0607c3f9..0117c90e77f99 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 0ea73ad9c23a4..7fba00d0e457b 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 67686b426c2e41e50e19c4b570060003de44c119 Mon Sep 17 00:00:00 2001 From: Greg Clayton <clayb...@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 bbcba495f9960..bd81618ac914d 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 046395670d046..8dd5a5472ed4c 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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits