Skip to content

[lldb/DWARF] Remove duplicate type filtering #116989

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.

Sign up for GitHub

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 1 commit into from
Nov 25, 2024
Merged

[lldb/DWARF] Remove duplicate type filtering #116989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,19 @@ void DWARFIndex::GetTypesWithQuery(
bool DWARFIndex::ProcessTypeDIEMatchQuery(
TypeQuery &query, DWARFDIE die,
llvm::function_ref<bool(DWARFDIE die)> callback) {
// Nothing to match from query
if (query.GetContextRef().size() <= 1)
Comment on lines -140 to -141
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This wasn't correct because it wasn't checking the "ComilerContextKind" part of the query. It didn't matter because of the "second line of defense" (which is now gone).

// Check the language, but only if we have a language filter.
if (query.HasLanguage() &&
!query.LanguageMatches(SymbolFileDWARF::GetLanguageFamily(*die.GetCU())))
return true; // Keep iterating over index types, language mismatch.

// Since mangled names are unique, we only need to check if the names are
// the same.
if (query.GetSearchByMangledName()) {
if (die.GetMangledName(/*substitute_name_allowed=*/false) !=
query.GetTypeBasename().GetStringRef())
return true; // Keep iterating over index types, mangled name mismatch.
return callback(die);
}

std::vector<lldb_private::CompilerContext> die_context;
if (query.GetModuleSearch())
Expand Down
52 changes: 2 additions & 50 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2726,39 +2726,8 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
TypeQuery query_full(query);
bool have_index_match = false;
m_index->GetTypesWithQuery(query_full, [&](DWARFDIE die) {
// Check the language, but only if we have a language filter.
if (query.HasLanguage()) {
if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU())))
return true; // Keep iterating over index types, language mismatch.
}

// Since mangled names are unique, we only need to check if the names are
// the same.
if (query.GetSearchByMangledName()) {
if (die.GetMangledName(/*substitute_name_allowed=*/false) !=
query.GetTypeBasename().GetStringRef())
return true; // Keep iterating over index types, mangled name mismatch.
if (Type *matching_type = ResolveType(die, true, true)) {
results.InsertUnique(matching_type->shared_from_this());
return !results.Done(query); // Keep iterating if we aren't done.
}
return true; // Keep iterating over index types, weren't able to resolve
// this type
}

// Check the context matches
std::vector<lldb_private::CompilerContext> die_context;
if (query.GetModuleSearch())
die_context = die.GetDeclContext();
else
die_context = die.GetTypeLookupContext();
assert(!die_context.empty());
Copy link
Contributor

Choose a reason for hiding this comment

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

This assert is missing in DWARFIndex::ProcessTypeDIEMatchQuery, maybe add that back?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't feel strongly about this, but I don't think this should be an assert. Like, we can probably reach this code if a bad (corrupted) index points us to an empty DIE. According to the assertion manifesto this shouldn't crash lldb.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, sounds good.

if (!query.ContextMatches(die_context))
return true; // Keep iterating over index types, context mismatch.

// Try to resolve the type.
if (Type *matching_type = ResolveType(die, true, true)) {
if (matching_type->IsTemplateType()) {
if (!query.GetSearchByMangledName() && matching_type->IsTemplateType()) {
// We have to watch out for case where we lookup a type by basename and
// it matches a template with simple template names. Like looking up
// "Foo" and if we have simple template names then we will match
Expand Down Expand Up @@ -2790,7 +2759,7 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
// With -gsimple-template-names, a templated type's DW_AT_name will not
// contain the template parameters. Try again stripping '<' and anything
// after, filtering out entries with template parameters that don't match.
if (!have_index_match) {
if (!have_index_match && !query.GetSearchByMangledName()) {
// Create a type matcher with a compiler context that is tuned for
// -gsimple-template-names. We will use this for the index lookup and the
// context matching, but will use the original "match" to insert matches
Expand All @@ -2804,23 +2773,6 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
// Copy our match's context and update the basename we are looking for
// so we can use this only to compare the context correctly.
m_index->GetTypesWithQuery(query_simple, [&](DWARFDIE die) {
// Check the language, but only if we have a language filter.
if (query.HasLanguage()) {
if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU())))
return true; // Keep iterating over index types, language mismatch.
}

// Check the context matches
std::vector<lldb_private::CompilerContext> die_context;
if (query.GetModuleSearch())
die_context = die.GetDeclContext();
else
die_context = die.GetTypeLookupContext();
assert(!die_context.empty());
if (!query_simple.ContextMatches(die_context))
return true; // Keep iterating over index types, context mismatch.

// Try to resolve the type.
if (Type *matching_type = ResolveType(die, true, true)) {
ConstString name = matching_type->GetQualifiedName();
// We have found a type that still might not match due to template
Expand Down
Loading