Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 24, 2024
4 changes: 4 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
return result;
}

const std::shared_ptr<SymbolFileDWARFDwo> &DWARFDebugInfo::GetDwpSymbolFile() {
return m_dwarf.GetDwpSymbolFile();
}

DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
Expand Down
2 changes: 2 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class DWARFDebugInfo {

const DWARFDebugAranges &GetCompileUnitAranges();

const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile();

protected:
typedef std::vector<DWARFUnitSP> UnitColl;

Expand Down
93 changes: 86 additions & 7 deletions lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -34,6 +35,17 @@ DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}

llvm::DenseSet<uint64_t>
DebugNamesDWARFIndex::GetTypeUnitSignatures(const DebugNames &debug_names) {
llvm::DenseSet<uint64_t> result;
for (const DebugNames::NameIndex &ni : debug_names) {
const uint32_t num_tus = ni.getForeignTUCount();
for (uint32_t tu = 0; tu < num_tus; ++tu)
result.insert(ni.getForeignTUSignature(tu));
}
return result;
}

llvm::DenseSet<dw_offset_t>
DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
llvm::DenseSet<dw_offset_t> result;
Expand All @@ -48,20 +60,80 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
return result;
}

std::optional<DWARFTypeUnit *>
DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const {
std::optional<uint64_t> type_sig = entry.getForeignTUTypeSignature();
if (!type_sig.has_value())
return std::nullopt;

// Ask the entry for the skeleton compile unit offset and fetch the .dwo
// file from it and get the type unit by signature from there. If we find
// the type unit in the .dwo file, we don't need to check that the
// DW_AT_dwo_name matches because each .dwo file can have its own type unit.
std::optional<uint64_t> cu_offset = entry.getRelatedCUOffset();
if (!cu_offset)
return nullptr; // Return NULL, this is a type unit, but couldn't find it.

DWARFUnit *cu =
m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *cu_offset);
if (!cu)
return nullptr; // Return NULL, this is a type unit, but couldn't find it.

auto dwp_sp = m_debug_info.GetDwpSymbolFile();
if (!dwp_sp) {
// No .dwp file, we need to load the .dwo file.
DWARFUnit &dwo_cu = cu->GetNonSkeletonUnit();
// We don't need the check if the type unit matches the .dwo file if we have
// a .dwo file (not a .dwp), so we can just return the value here.
if (!dwo_cu.IsDWOUnit())
return nullptr; // We weren't able to load the .dwo file.
return dwo_cu.GetSymbolFileDWARF().DebugInfo().GetTypeUnitForHash(
*type_sig);
}
// We have a .dwp file, just get the type unit from there. We need to verify
// that the type unit that ended up in the final .dwp file is the right type
// unit. Type units have signatures which are the same across multiple .dwo
// files, but only one of those type units will end up in the .dwp file. The
// contents of type units for the same type can be different in different .dwo
// files, which means the DIE offsets might not be the same between two
// different type units. So we need to determine if this accelerator table
// matches the type unit that ended up in the .dwp file. If it doesn't match,
// then we need to ignore this accelerator table entry as the type unit that
// is in the .dwp file will have its own index. In order to determine if the
// type unit that ended up in a .dwp file matches this DebugNames::Entry, we
// need to find the skeleton compile unit for this entry.
DWARFTypeUnit *foreign_tu = dwp_sp->DebugInfo().GetTypeUnitForHash(*type_sig);
if (!foreign_tu)
return nullptr; // Return NULL, this is a type unit, but couldn't find it.

DWARFBaseDIE cu_die = cu->GetUnitDIEOnly();
DWARFBaseDIE tu_die = foreign_tu->GetUnitDIEOnly();
llvm::StringRef cu_dwo_name =
cu_die.GetAttributeValueAsString(DW_AT_dwo_name, nullptr);
llvm::StringRef tu_dwo_name =
tu_die.GetAttributeValueAsString(DW_AT_dwo_name, nullptr);
if (cu_dwo_name == tu_dwo_name)
return foreign_tu; // We found a match!
return nullptr; // Return NULL, this is a type unit, but couldn't find it.
}

DWARFUnit *
DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {

if (std::optional<DWARFTypeUnit *> foreign_tu = GetForeignTypeUnit(entry))
return foreign_tu.value();

// Look for a DWARF unit offset (CU offset or local TU offset) as they are
// both offsets into the .debug_info section.
std::optional<uint64_t> unit_offset = entry.getCUOffset();
if (!unit_offset) {
if (!unit_offset)
unit_offset = entry.getLocalTUOffset();
if (!unit_offset)
return nullptr;
if (unit_offset) {
if (DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
*unit_offset))
return &cu->GetNonSkeletonUnit();
}

DWARFUnit *cu =
m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
return cu ? &cu->GetNonSkeletonUnit() : nullptr;
return nullptr;
}

DWARFDIE DebugNamesDWARFIndex::GetDIE(const DebugNames::Entry &entry) const {
Expand Down Expand Up @@ -274,6 +346,13 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
if (!isType(entry.tag()))
continue;

// If we get a NULL foreign_tu back, the entry doesn't match the type unit
// in the .dwp file, or we were not able to load the .dwo file or the DWO ID
// didn't match.
std::optional<DWARFTypeUnit *> foreign_tu = GetForeignTypeUnit(entry);
if (foreign_tu && foreign_tu.value() == nullptr)
continue;

// Grab at most one extra parent, subsequent parents are not necessary to
// test equality.
std::optional<llvm::SmallVector<Entry, 4>> parent_chain =
Expand Down
30 changes: 29 additions & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ class DebugNamesDWARFIndex : public DWARFIndex {
: DWARFIndex(module), m_debug_info(dwarf.DebugInfo()),
m_debug_names_data(debug_names_data), m_debug_str_data(debug_str_data),
m_debug_names_up(std::move(debug_names_up)),
m_fallback(module, dwarf, GetUnits(*m_debug_names_up)) {}
m_fallback(module, dwarf, GetUnits(*m_debug_names_up),
GetTypeUnitSignatures(*m_debug_names_up)) {}

DWARFDebugInfo &m_debug_info;

Expand All @@ -85,6 +86,31 @@ class DebugNamesDWARFIndex : public DWARFIndex {

DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
DWARFDIE GetDIE(const DebugNames::Entry &entry) const;

/// Checks if an entry is a foreign TU and fetch the type unit.
///
/// This function checks if the DebugNames::Entry refers to a foreign TU and
/// returns an optional with a value of the \a entry is a foreign type unit
/// entry. A valid pointer will be returned if this entry is from a .dwo file
/// or if it is from a .dwp file and it matches the type unit's originating
/// .dwo file by verifying that the DW_TAG_type_unit DIE has a DW_AT_dwo_name
/// that matches the DWO name from the originating skeleton compile unit.
///
/// \param[in] entry
/// The accelerator table entry to check.
///
/// \returns
/// A std::optional that has a value if this entry represents a foreign type
/// unit. If the pointer is valid, then we were able to find and match the
/// entry to the type unit in the .dwo or .dwp file. The returned value can
/// have a valid, yet contain NULL in the following cases:
/// - we were not able to load the .dwo file (missing or DWO ID mismatch)
/// - we were able to load the .dwp file, but the type units DWO name
/// doesn't match the originating skeleton compile unit's entry
/// Returns std::nullopt if this entry is not a foreign type unit entry.
std::optional<DWARFTypeUnit *>
GetForeignTypeUnit(const DebugNames::Entry &entry) const;

bool ProcessEntry(const DebugNames::Entry &entry,
llvm::function_ref<bool(DWARFDIE die)> callback);

Expand All @@ -97,6 +123,8 @@ class DebugNamesDWARFIndex : public DWARFIndex {
llvm::StringRef name);

static llvm::DenseSet<dw_offset_t> GetUnits(const DebugNames &debug_names);
static llvm::DenseSet<uint64_t>
GetTypeUnitSignatures(const DebugNames &debug_names);
};

} // namespace dwarf
Expand Down
7 changes: 5 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ void ManualDWARFIndex::Index() {
}
if (dwp_info && dwp_info->ContainsTypeUnits()) {
for (size_t U = 0; U < dwp_info->GetNumUnits(); ++U) {
if (auto *tu = llvm::dyn_cast<DWARFTypeUnit>(dwp_info->GetUnitAtIndex(U)))
units_to_index.push_back(tu);
if (auto *tu =
llvm::dyn_cast<DWARFTypeUnit>(dwp_info->GetUnitAtIndex(U))) {
if (!m_type_sigs_to_avoid.contains(tu->GetTypeHash()))
units_to_index.push_back(tu);
}
}
}

Expand Down
7 changes: 5 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ class SymbolFileDWARFDwo;
class ManualDWARFIndex : public DWARFIndex {
public:
ManualDWARFIndex(Module &module, SymbolFileDWARF &dwarf,
llvm::DenseSet<dw_offset_t> units_to_avoid = {})
llvm::DenseSet<dw_offset_t> units_to_avoid = {},
llvm::DenseSet<uint64_t> type_sigs_to_avoid = {})
: DWARFIndex(module), m_dwarf(&dwarf),
m_units_to_avoid(std::move(units_to_avoid)) {}
m_units_to_avoid(std::move(units_to_avoid)),
m_type_sigs_to_avoid(std::move(type_sigs_to_avoid)) {}

void Preload() override { Index(); }

Expand Down Expand Up @@ -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;
Expand Down
59 changes: 33 additions & 26 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1727,45 +1727,52 @@ lldb::ModuleSP SymbolFileDWARF::GetExternalModule(ConstString name) {
return pos->second;
}

DWARFDIE
SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
// This method can be called without going through the symbol vendor so we
// need to lock the module.
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());

SymbolFileDWARF *symbol_file = nullptr;

SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
// Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
// must make sure we use the correct DWARF file when resolving things. On
// MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
// SymbolFileDWARF classes, one for each .o file. We can often end up with
// references to other DWARF objects and we must be ready to receive a
// "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
// instance.

std::optional<uint32_t> file_index = die_ref.file_index();
if (file_index) {
if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case
if (symbol_file)
return symbol_file->DebugInfo().GetDIE(die_ref.section(),
die_ref.die_offset());
return DWARFDIE();
}

// If the file index matches, then we have the right SymbolFileDWARF already.
// This will work for both .dwo file and DWARF in .o files for mac. Also if
// both the file indexes are invalid, then we have a match.
if (GetFileIndex() == file_index)
return this;

if (file_index) {
// We have a SymbolFileDWARFDebugMap, so let it find the right file
\ if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
return debug_map->GetSymbolFileByOSOIndex(*file_index);

// Handle the .dwp file case correctly
if (*file_index == DIERef::k_file_index_mask)
symbol_file = GetDwpSymbolFile().get(); // DWP case
else
symbol_file = this->DebugInfo()
.GetUnitAtIndex(*die_ref.file_index())
->GetDwoSymbolFile(); // DWO case
} else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
return DWARFDIE();
return GetDwpSymbolFile().get(); // DWP case

// Handle the .dwo file case correctly
return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
->GetDwoSymbolFile(); // DWO case
}
return this;
}

if (symbol_file)
return symbol_file->GetDIE(die_ref);
DWARFDIE
SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
if (die_ref.die_offset() == DW_INVALID_OFFSET)
return DWARFDIE();

return DebugInfo().GetDIE(die_ref.section(), die_ref.die_offset());
// This method can be called without going through the symbol vendor so we
// need to lock the module.
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only uses the file_index component of the DIERef, how about we just pass that (and call the function something like GetSymbolFileByIndex)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only uses the file index component, but that component might not be set in the DIERef object and if it isn't, it returns the current SymbolFileDWARF object. We can make a function like:

SymbolFileDWARF *GetSymbolFileByFileIndex(std::optional<uint32_t> file_index);

The file index only makes sense if it comes from a DIERef object as it understands the value and knows the magic value for the .dwp file, so the above API is much less clear as who else knows what a file index is and would actaully know to ask for the symbol file using such an index? Seems less clear to me than the existing API. Let me know if feel we should still change this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the file index needs to necessarily come from a DIERef. I can imagine other scenarios where it might be interesting to ask for a symbol file, but I think we can deal with it when the time comes.

if (symbol_file)
Comment on lines +1771 to +1772
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar feedback others have given elsewhere, roll the variable declaration into the if condition

return symbol_file->DebugInfo().GetDIE(die_ref.section(),
die_ref.die_offset());
return DWARFDIE();
}

/// Return the DW_AT_(GNU_)dwo_id.
Expand Down
9 changes: 9 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,15 @@ class SymbolFileDWARF : public SymbolFileCommon {
return m_external_type_modules;
}

/// Given a DIERef, find the correct SymbolFileDWARF.
///
/// A DIERef contains a file index that can uniquely identify a N_OSO file for
/// DWARF in .o files on mac, or a .dwo or .dwp file index for split DWARF.
/// Calling this function will find the correct symbol file to use so that
/// further lookups can be done on the correct symbol file so that the DIE
/// offset makes sense in the DIERef.
virtual SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref);

virtual DWARFDIE GetDIE(const DIERef &die_ref);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now GetDIE is virtual and handles SymbolFile resolution, this one doesn't need to be.


DWARFDIE GetDIE(lldb::user_id_t uid);
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading