[Lldb-commits] [lldb] e12c701 - [lldb] Use LLVM's implementation of AppleTables for apple_debug_types
Felipe de Azevedo Piovezan via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 28 06:19:25 PDT 2023
Author: Felipe de Azevedo Piovezan
Date: 2023-06-28T09:19:14-04:00
New Revision: e12c701ff0405880045f0330fb6ff94e31ec5c47
URL: https://github.com/llvm/llvm-project/commit/e12c701ff0405880045f0330fb6ff94e31ec5c47
DIFF: https://github.com/llvm/llvm-project/commit/e12c701ff0405880045f0330fb6ff94e31ec5c47.diff
LOG: [lldb] Use LLVM's implementation of AppleTables for apple_debug_types
This commit is replacing really old LLDB code, and we've found some odd
behavior while doing this replacement. While the changes here are largely NFC,
there are some subtle changes that fix such odd behavior.
The most curious example of this is the method `FindCompleteObjCClassName`,
which has a flag `must_be_implementation`. This flag was _only_ being respected
for accelerator tables containing the atom `type_flags`, which seems
counter-intuitive. The implementation for DWARF 5 tables does not do that and
neither does the code introduced in this patch.
There were other weird cases, for example, we found boolean logic that was
always true in a code path: look for a `if !has_qualified_name...` deleted
line; that condition was true by simple if/else analysis.
Differential Revision: https://reviews.llvm.org/D153867
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 5cc44ea5b50c8..0cd8b3d71bd4f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -32,10 +32,8 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
std::make_unique<llvm::AppleAcceleratorTable>(
apple_namespaces.GetAsLLVMDWARF(), llvm_debug_str);
- auto apple_types_table_up = std::make_unique<DWARFMappedHash::MemoryTable>(
- apple_types, debug_str, ".apple_types");
- if (!apple_types_table_up->IsValid())
- apple_types_table_up.reset();
+ auto apple_types_table_up = std::make_unique<llvm::AppleAcceleratorTable>(
+ apple_types.GetAsLLVMDWARF(), llvm_debug_str);
auto apple_objc_table_up = std::make_unique<DWARFMappedHash::MemoryTable>(
apple_objc, debug_str, ".apple_objc");
@@ -51,6 +49,7 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
extract_and_check(apple_names_table_up);
extract_and_check(apple_namespaces_table_up);
+ extract_and_check(apple_types_table_up);
if (apple_names_table_up || apple_namespaces_table_up ||
apple_types_table_up || apple_objc_table_up)
@@ -62,16 +61,69 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
return nullptr;
}
+/// Returns true if `tag` is a class_type of structure_type tag.
+static bool IsClassOrStruct(dw_tag_t tag) {
+ return tag == DW_TAG_class_type || tag == DW_TAG_structure_type;
+}
+
+/// Returns true if `entry` has an extractable DW_ATOM_qual_name_hash and it
+/// matches `expected_hash`.
+static bool
+EntryHasMatchingQualhash(const llvm::AppleAcceleratorTable::Entry &entry,
+ uint32_t expected_hash) {
+ std::optional<llvm::DWARFFormValue> form_value =
+ entry.lookup(dwarf::DW_ATOM_qual_name_hash);
+ if (!form_value)
+ return false;
+ std::optional<uint64_t> hash = form_value->getAsUnsignedConstant();
+ return hash && (*hash == expected_hash);
+}
+
+/// Returns true if `entry` has an extractable DW_ATOM_die_tag and it matches
+/// `expected_tag`. We also consider it a match if the tags are
diff erent but
+/// in the set of {TAG_class_type, TAG_struct_type}.
+static bool EntryHasMatchingTag(const llvm::AppleAcceleratorTable::Entry &entry,
+ dw_tag_t expected_tag) {
+ std::optional<llvm::DWARFFormValue> form_value =
+ entry.lookup(dwarf::DW_ATOM_die_tag);
+ if (!form_value)
+ return false;
+ std::optional<uint64_t> maybe_tag = form_value->getAsUnsignedConstant();
+ if (!maybe_tag)
+ return false;
+ auto tag = static_cast<dw_tag_t>(*maybe_tag);
+ return tag == expected_tag ||
+ (IsClassOrStruct(tag) && IsClassOrStruct(expected_tag));
+}
+
+/// Returns true if `entry` has an extractable DW_ATOM_type_flags and the flag
+/// "DW_FLAG_type_implementation" is set.
+static bool
+HasImplementationFlag(const llvm::AppleAcceleratorTable::Entry &entry) {
+ std::optional<llvm::DWARFFormValue> form_value =
+ entry.lookup(dwarf::DW_ATOM_type_flags);
+ if (!form_value)
+ return false;
+ std::optional<uint64_t> Flags = form_value->getAsUnsignedConstant();
+ return Flags &&
+ (*Flags & llvm::dwarf::AcceleratorTable::DW_FLAG_type_implementation);
+}
+
void AppleDWARFIndex::SearchFor(const llvm::AppleAcceleratorTable &table,
llvm::StringRef name,
llvm::function_ref<bool(DWARFDIE die)> callback,
std::optional<dw_tag_t> search_for_tag,
std::optional<uint32_t> search_for_qualhash) {
- assert(!search_for_tag && !search_for_qualhash && "not yet implemented");
auto converted_cb = DIERefCallback(callback, name);
- for (const auto &entry : table.equal_range(name))
+ for (const auto &entry : table.equal_range(name)) {
+ if (search_for_qualhash &&
+ !EntryHasMatchingQualhash(entry, *search_for_qualhash))
+ continue;
+ if (search_for_tag && !EntryHasMatchingTag(entry, *search_for_tag))
+ continue;
if (!converted_cb(entry))
break;
+ }
}
void AppleDWARFIndex::GetGlobalVariables(
@@ -130,18 +182,32 @@ void AppleDWARFIndex::GetCompleteObjCClass(
llvm::function_ref<bool(DWARFDIE die)> callback) {
if (!m_apple_types_up)
return;
- m_apple_types_up->FindCompleteObjCClassByName(
- class_name.GetStringRef(),
- DIERefCallback(callback, class_name.GetStringRef()),
- must_be_implementation);
+
+ llvm::SmallVector<DIERef> decl_dies;
+ auto converted_cb = DIERefCallback(callback, class_name);
+
+ for (const auto &entry : m_apple_types_up->equal_range(class_name)) {
+ if (HasImplementationFlag(entry)) {
+ converted_cb(entry);
+ return;
+ }
+
+ decl_dies.emplace_back(std::nullopt, DIERef::Section::DebugInfo,
+ *entry.getDIESectionOffset());
+ }
+
+ if (must_be_implementation)
+ return;
+ for (DIERef ref : decl_dies)
+ if (!converted_cb(ref))
+ return;
}
void AppleDWARFIndex::GetTypes(
ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) {
if (!m_apple_types_up)
return;
- m_apple_types_up->FindByName(name.GetStringRef(),
- DIERefCallback(callback, name.GetStringRef()));
+ SearchFor(*m_apple_types_up, name, callback);
}
void AppleDWARFIndex::GetTypes(
@@ -151,51 +217,47 @@ void AppleDWARFIndex::GetTypes(
return;
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- const bool has_tag = m_apple_types_up->GetHeader().header_data.ContainsAtom(
- DWARFMappedHash::eAtomTypeTag);
- const bool has_qualified_name_hash =
- m_apple_types_up->GetHeader().header_data.ContainsAtom(
- DWARFMappedHash::eAtomTypeQualNameHash);
-
- const ConstString type_name(context[0].name);
- const dw_tag_t tag = context[0].tag;
- if (has_tag && has_qualified_name_hash) {
- const char *qualified_name = context.GetQualifiedName();
- const uint32_t qualified_name_hash = llvm::djbHash(qualified_name);
+ const bool entries_have_tag =
+ m_apple_types_up->containsAtomType(DW_ATOM_die_tag);
+ const bool entries_have_qual_hash =
+ m_apple_types_up->containsAtomType(DW_ATOM_qual_name_hash);
+
+ llvm::StringRef expected_name = context[0].name;
+
+ if (entries_have_tag && entries_have_qual_hash) {
+ const dw_tag_t expected_tag = context[0].tag;
+ const uint32_t expected_qualname_hash =
+ llvm::djbHash(context.GetQualifiedName());
if (log)
m_module.LogMessage(log, "FindByNameAndTagAndQualifiedNameHash()");
- m_apple_types_up->FindByNameAndTagAndQualifiedNameHash(
- type_name.GetStringRef(), tag, qualified_name_hash,
- DIERefCallback(callback, type_name.GetStringRef()));
+ SearchFor(*m_apple_types_up, expected_name, callback, expected_tag,
+ expected_qualname_hash);
return;
}
- if (has_tag) {
- // When searching for a scoped type (for example,
- // "std::vector<int>::const_iterator") searching for the innermost
- // name alone ("const_iterator") could yield many false
- // positives. By searching for the parent type ("vector<int>")
- // first we can avoid extracting type DIEs from object files that
- // would fail the filter anyway.
- if (!has_qualified_name_hash && (context.GetSize() > 1) &&
- (context[1].tag == DW_TAG_class_type ||
- context[1].tag == DW_TAG_structure_type)) {
- if (m_apple_types_up->FindByName(context[1].name,
- [&](DIERef ref) { return false; }))
- return;
- }
-
- if (log)
- m_module.LogMessage(log, "FindByNameAndTag()");
- m_apple_types_up->FindByNameAndTag(
- type_name.GetStringRef(), tag,
- DIERefCallback(callback, type_name.GetStringRef()));
+ // Historically, if there are no tags, we also ignore qual_hash (why?)
+ if (!entries_have_tag) {
+ SearchFor(*m_apple_names_up, expected_name, callback);
return;
}
- m_apple_types_up->FindByName(
- type_name.GetStringRef(),
- DIERefCallback(callback, type_name.GetStringRef()));
+ // We have a tag but no qual hash.
+
+ // When searching for a scoped type (for example,
+ // "std::vector<int>::const_iterator") searching for the innermost
+ // name alone ("const_iterator") could yield many false
+ // positives. By searching for the parent type ("vector<int>")
+ // first we can avoid extracting type DIEs from object files that
+ // would fail the filter anyway.
+ if ((context.GetSize() > 1) && IsClassOrStruct(context[1].tag))
+ if (m_apple_types_up->equal_range(context[1].name).empty())
+ return;
+
+ if (log)
+ m_module.LogMessage(log, "FindByNameAndTag()");
+ const dw_tag_t expected_tag = context[0].tag;
+ SearchFor(*m_apple_types_up, expected_name, callback, expected_tag);
+ return;
}
void AppleDWARFIndex::GetNamespaces(
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
index 50a8fe22bcb4f..5ff88cd5e20dd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -24,7 +24,7 @@ class AppleDWARFIndex : public DWARFIndex {
AppleDWARFIndex(Module &module,
std::unique_ptr<llvm::AppleAcceleratorTable> apple_names,
std::unique_ptr<llvm::AppleAcceleratorTable> apple_namespaces,
- std::unique_ptr<DWARFMappedHash::MemoryTable> apple_types,
+ std::unique_ptr<llvm::AppleAcceleratorTable> apple_types,
std::unique_ptr<DWARFMappedHash::MemoryTable> apple_objc)
: DWARFIndex(module), m_apple_names_up(std::move(apple_names)),
m_apple_namespaces_up(std::move(apple_namespaces)),
@@ -65,7 +65,7 @@ class AppleDWARFIndex : public DWARFIndex {
private:
std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_names_up;
std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_namespaces_up;
- std::unique_ptr<DWARFMappedHash::MemoryTable> m_apple_types_up;
+ std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_types_up;
std::unique_ptr<DWARFMappedHash::MemoryTable> m_apple_objc_up;
/// Search for entries whose name is `name` in `table`, calling `callback` for
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
index dee644120f699..19d24328a2da6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -23,92 +23,6 @@ bool DWARFMappedHash::ExtractDIEArray(
return true;
}
-void DWARFMappedHash::ExtractDIEArray(
- const DIEInfoArray &die_info_array, const dw_tag_t tag,
- llvm::function_ref<bool(DIERef ref)> callback) {
- if (tag == 0) {
- ExtractDIEArray(die_info_array, callback);
- return;
- }
-
- const size_t count = die_info_array.size();
- for (size_t i = 0; i < count; ++i) {
- const dw_tag_t die_tag = die_info_array[i].tag;
- bool tag_matches = die_tag == 0 || tag == die_tag;
- if (!tag_matches) {
- if (die_tag == DW_TAG_class_type || die_tag == DW_TAG_structure_type)
- tag_matches = tag == DW_TAG_structure_type || tag == DW_TAG_class_type;
- }
- if (tag_matches) {
- if (!callback(DIERef(die_info_array[i])))
- return;
- }
- }
-}
-
-void DWARFMappedHash::ExtractDIEArray(
- const DIEInfoArray &die_info_array, const dw_tag_t tag,
- const uint32_t qualified_name_hash,
- llvm::function_ref<bool(DIERef ref)> callback) {
- if (tag == 0) {
- ExtractDIEArray(die_info_array, callback);
- return;
- }
-
- const size_t count = die_info_array.size();
- for (size_t i = 0; i < count; ++i) {
- if (qualified_name_hash != die_info_array[i].qualified_name_hash)
- continue;
- const dw_tag_t die_tag = die_info_array[i].tag;
- bool tag_matches = die_tag == 0 || tag == die_tag;
- if (!tag_matches) {
- if (die_tag == DW_TAG_class_type || die_tag == DW_TAG_structure_type)
- tag_matches = tag == DW_TAG_structure_type || tag == DW_TAG_class_type;
- }
- if (tag_matches) {
- if (!callback(DIERef(die_info_array[i])))
- return;
- }
- }
-}
-
-void DWARFMappedHash::ExtractClassOrStructDIEArray(
- const DIEInfoArray &die_info_array,
- bool return_implementation_only_if_available,
- llvm::function_ref<bool(DIERef ref)> callback) {
- const size_t count = die_info_array.size();
- for (size_t i = 0; i < count; ++i) {
- const dw_tag_t die_tag = die_info_array[i].tag;
- if (!(die_tag == 0 || die_tag == DW_TAG_class_type ||
- die_tag == DW_TAG_structure_type))
- continue;
- bool is_implementation =
- (die_info_array[i].type_flags & eTypeFlagClassIsImplementation) != 0;
- if (is_implementation != return_implementation_only_if_available)
- continue;
- if (return_implementation_only_if_available) {
- // We found the one true definition for this class, so only return
- // that
- callback(DIERef(die_info_array[i]));
- return;
- }
- if (!callback(DIERef(die_info_array[i])))
- return;
- }
-}
-
-void DWARFMappedHash::ExtractTypesFromDIEArray(
- const DIEInfoArray &die_info_array, uint32_t type_flag_mask,
- uint32_t type_flag_value, llvm::function_ref<bool(DIERef ref)> callback) {
- const size_t count = die_info_array.size();
- for (size_t i = 0; i < count; ++i) {
- if ((die_info_array[i].type_flags & type_flag_mask) == type_flag_value) {
- if (!callback(DIERef(die_info_array[i])))
- return;
- }
- }
-}
-
const char *DWARFMappedHash::GetAtomTypeName(uint16_t atom) {
switch (atom) {
case eAtomTypeNULL:
@@ -414,56 +328,6 @@ bool DWARFMappedHash::MemoryTable::FindByName(
return DWARFMappedHash::ExtractDIEArray(die_info_array, callback);
}
-void DWARFMappedHash::MemoryTable::FindByNameAndTag(
- llvm::StringRef name, const dw_tag_t tag,
- llvm::function_ref<bool(DIERef ref)> callback) {
- DIEInfoArray die_info_array;
- FindByName(name, die_info_array);
- DWARFMappedHash::ExtractDIEArray(die_info_array, tag, callback);
-}
-
-void DWARFMappedHash::MemoryTable::FindByNameAndTagAndQualifiedNameHash(
- llvm::StringRef name, const dw_tag_t tag,
- const uint32_t qualified_name_hash,
- llvm::function_ref<bool(DIERef ref)> callback) {
- DIEInfoArray die_info_array;
- FindByName(name, die_info_array);
- DWARFMappedHash::ExtractDIEArray(die_info_array, tag, qualified_name_hash,
- callback);
-}
-
-void DWARFMappedHash::MemoryTable::FindCompleteObjCClassByName(
- llvm::StringRef name, llvm::function_ref<bool(DIERef ref)> callback,
- bool must_be_implementation) {
- DIEInfoArray die_info_array;
- FindByName(name, die_info_array);
- if (must_be_implementation &&
- GetHeader().header_data.ContainsAtom(eAtomTypeTypeFlags)) {
- // If we have two atoms, then we have the DIE offset and the type flags
- // so we can find the objective C class efficiently.
- DWARFMappedHash::ExtractTypesFromDIEArray(
- die_info_array, UINT32_MAX, eTypeFlagClassIsImplementation, callback);
- return;
- }
- // We don't only want the one true definition, so try and see what we can
- // find, and only return class or struct DIEs. If we do have the full
- // implementation, then return it alone, else return all possible
- // matches.
- bool found_implementation = false;
- DWARFMappedHash::ExtractClassOrStructDIEArray(
- die_info_array, true /*return_implementation_only_if_available*/,
- [&](DIERef ref) {
- found_implementation = true;
- // Here the return value does not matter as we are called at most once.
- return callback(ref);
- });
- if (found_implementation)
- return;
- DWARFMappedHash::ExtractClassOrStructDIEArray(
- die_info_array, false /*return_implementation_only_if_available*/,
- callback);
-}
-
void DWARFMappedHash::MemoryTable::FindByName(llvm::StringRef name,
DIEInfoArray &die_info_array) {
if (name.empty())
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
index 2d28f9770f2a5..eb6d4c81c38dd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
@@ -135,19 +135,6 @@ class DWARFMappedHash {
bool FindByName(llvm::StringRef name,
llvm::function_ref<bool(DIERef ref)> callback);
- void FindByNameAndTag(llvm::StringRef name, const dw_tag_t tag,
- llvm::function_ref<bool(DIERef ref)> callback);
-
- void FindByNameAndTagAndQualifiedNameHash(
- llvm::StringRef name, const dw_tag_t tag,
- const uint32_t qualified_name_hash,
- llvm::function_ref<bool(DIERef ref)> callback);
-
- void
- FindCompleteObjCClassByName(llvm::StringRef name,
- llvm::function_ref<bool(DIERef ref)> callback,
- bool must_be_implementation);
-
protected:
void FindByName(llvm::StringRef name, DIEInfoArray &die_info_array);
@@ -164,25 +151,6 @@ class DWARFMappedHash {
llvm::function_ref<bool(DIERef ref)> callback);
protected:
- static void ExtractDIEArray(const DIEInfoArray &die_info_array,
- const dw_tag_t tag,
- llvm::function_ref<bool(DIERef ref)> callback);
-
- static void ExtractDIEArray(const DIEInfoArray &die_info_array,
- const dw_tag_t tag,
- const uint32_t qualified_name_hash,
- llvm::function_ref<bool(DIERef ref)> callback);
-
- static void
- ExtractClassOrStructDIEArray(const DIEInfoArray &die_info_array,
- bool return_implementation_only_if_available,
- llvm::function_ref<bool(DIERef ref)> callback);
-
- static void
- ExtractTypesFromDIEArray(const DIEInfoArray &die_info_array,
- uint32_t type_flag_mask, uint32_t type_flag_value,
- llvm::function_ref<bool(DIERef ref)> callback);
-
static const char *GetAtomTypeName(uint16_t atom);
};
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 301f773b5e573..ce5d2f6c1457d 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -314,6 +314,13 @@ class AppleAcceleratorTable : public DWARFAcceleratorTable {
/// Return the Atom description, which can be used to interpret the raw values
/// of the Accelerator Entries in this table.
ArrayRef<std::pair<HeaderData::AtomType, HeaderData::Form>> getAtomsDesc();
+
+ /// Returns true iff `AtomTy` is one of the atoms available in Entries of this
+ /// table.
+ bool containsAtomType(HeaderData::AtomType AtomTy) const {
+ return is_contained(make_first_range(HdrData.Atoms), AtomTy);
+ }
+
bool validateForms();
/// Return information related to the DWARF DIE we're looking for when
More information about the lldb-commits
mailing list