Skip to content

Commit

Permalink
[lldb] Use LLVM's implementation of AppleTables for apple_debug_types
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@felipepiovezan
felipepiovezan committed Jun 28, 2023
1 parent 98390cc commit e12c701
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 219 deletions.
160 changes: 111 additions & 49 deletions lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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)
Expand All @@ -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 different 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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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
Expand Down
136 changes: 0 additions & 136 deletions lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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())
Expand Down
Loading

0 comments on commit e12c701

Please sign in to comment.