Skip to content

[lldb] Fix lookup of types in anonymous namespaces with -gsimple-template-names #123054

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
labath merged 3 commits into from
Jan 16, 2025
Merged

[lldb] Fix lookup of types in anonymous namespaces with -gsimple-template-names #123054

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1aff68c
[lldb] Fix lookup of types in anonymous namespaces with -gsimple-temp…
labath Nov 20, 2024
1241bec
test unions
labath Jan 15, 2025
d26f1b0
rename arg
labath Jan 15, 2025
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
44 changes: 30 additions & 14 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include "llvm/ADT/iterator.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h"
#include "llvm/Support/raw_ostream.h"

using namespace lldb_private;
using namespace lldb_private::dwarf;
Expand Down Expand Up @@ -376,7 +378,8 @@ lldb_private::Type *DWARFDIE::ResolveTypeUID(const DWARFDIE &die) const {
return nullptr;
}

static CompilerContext GetContextEntry(DWARFDIE die) {
static CompilerContext GetContextEntry(DWARFDIE die,
bool complete_template_names) {
auto ctx = [die](CompilerContextKind kind) {
return CompilerContext(kind, ConstString(die.GetName()));
};
Expand All @@ -386,11 +389,6 @@ static CompilerContext GetContextEntry(DWARFDIE die) {
return ctx(CompilerContextKind::Module);
case DW_TAG_namespace:
return ctx(CompilerContextKind::Namespace);
case DW_TAG_class_type:
case DW_TAG_structure_type:
return ctx(CompilerContextKind::ClassOrStruct);
case DW_TAG_union_type:
return ctx(CompilerContextKind::Union);
case DW_TAG_enumeration_type:
return ctx(CompilerContextKind::Enum);
case DW_TAG_subprogram:
Expand All @@ -401,12 +399,28 @@ static CompilerContext GetContextEntry(DWARFDIE die) {
return ctx(CompilerContextKind::Typedef);
case DW_TAG_base_type:
return ctx(CompilerContextKind::Builtin);
case DW_TAG_class_type:
case DW_TAG_structure_type:
case DW_TAG_union_type: {
CompilerContextKind kind = die.Tag() == DW_TAG_union_type
? CompilerContextKind::Union
: CompilerContextKind::ClassOrStruct;
llvm::StringRef name = die.GetName();
if (!complete_template_names || name.contains('<'))
Copy link
Member

@Michael137 Michael137 Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just to clarify, in what cases do we have simple-template-names and contain template parameters? Is it because -gsimple-template-names can't always canonicalize some template names?

Copy link
Collaborator Author

@labath labath Jan 15, 2025

Choose a reason for hiding this comment

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

That's part of it -- some names (I'm not sure which ones, but they're probably involve non-type template arguments and things like that) don't have enough information to reconstruct the original name. The other part is that we really don't know whether debug info contains simplified template names or not (even for names that could be simplified). There's no CU-level attribute or anything that would convey that information.

Copy link
Member

@Michael137 Michael137 Jan 15, 2025

Choose a reason for hiding this comment

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

Ah right makes sense

There's no CU-level attribute or anything that would convey that information

I think I asked about this a while back but would it be worth introducing such CU-level DWARF attribute?

Copy link
Collaborator Author

@labath labath Jan 15, 2025

Choose a reason for hiding this comment

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

You were probably asking @dwblaikie as I wasn't involved in the original implementation. Anyway, here's what I think.

If we were able to simplify all template names, then I can see how such an attribute could be useful as a replacement for the contains('<') heuristic, though I believe that is just a performance optimization (avoid searching through children DIEs if we see the name is templated) and shouldn't produce false positives. It could also be used for things like "if all CUs are using the same kind of names then don't bother trying to search for the other name kind", though I'm unsure what difference would it make in practice (especially the search with the full name should be very fast as it will just get zero hits).

As it stands now, I think it's usefulness would be very limited. About the only thing I can think of is skipping the query with the simplified name if all CUs report they are using full names. That may make sense if you're seeing slowdown due to the additional simplified-name queries.

Copy link
Member

@Michael137 Michael137 Jan 15, 2025

Choose a reason for hiding this comment

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

makes sense, thanks!

return CompilerContext(kind, ConstString(name));

std::string name_storage = name.str();
llvm::raw_string_ostream os(name_storage);
llvm::DWARFTypePrinter<DWARFDIE>(os).appendAndTerminateTemplateParameters(
die);
return CompilerContext(kind, ConstString(os.str()));
}
default:
llvm_unreachable("Check tag type in the caller!");
}
}

static void GetDeclContextImpl(DWARFDIE die,
static void GetDeclContextImpl(DWARFDIE die, bool complete_template_names,
llvm::SmallSet<lldb::user_id_t, 4> &seen,
std::vector<CompilerContext> &context) {
// Stop if we hit a cycle.
Expand All @@ -428,7 +442,7 @@ static void GetDeclContextImpl(DWARFDIE die,
case DW_TAG_subprogram:
case DW_TAG_variable:
case DW_TAG_typedef:
context.push_back(GetContextEntry(die));
context.push_back(GetContextEntry(die, complete_template_names));
break;
default:
break;
Expand All @@ -438,15 +452,16 @@ static void GetDeclContextImpl(DWARFDIE die,
}
}

std::vector<CompilerContext> DWARFDIE::GetDeclContext() const {
std::vector<CompilerContext>
DWARFDIE::GetDeclContext(bool complete_template_names) const {
llvm::SmallSet<lldb::user_id_t, 4> seen;
std::vector<CompilerContext> context;
GetDeclContextImpl(*this, seen, context);
GetDeclContextImpl(*this, complete_template_names, seen, context);
std::reverse(context.begin(), context.end());
return context;
}

static void GetTypeLookupContextImpl(DWARFDIE die,
static void GetTypeLookupContextImpl(DWARFDIE die, bool complete_template_names,
llvm::SmallSet<lldb::user_id_t, 4> &seen,
std::vector<CompilerContext> &context) {
// Stop if we hit a cycle.
Expand All @@ -461,7 +476,7 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
case DW_TAG_variable:
case DW_TAG_typedef:
case DW_TAG_base_type:
context.push_back(GetContextEntry(die));
context.push_back(GetContextEntry(die, complete_template_names));
break;

// If any of the tags below appear in the parent chain, stop the decl
Expand All @@ -484,10 +499,11 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
}
}

std::vector<CompilerContext> DWARFDIE::GetTypeLookupContext() const {
std::vector<CompilerContext>
DWARFDIE::GetTypeLookupContext(bool complete_template_names) const {
llvm::SmallSet<lldb::user_id_t, 4> seen;
std::vector<CompilerContext> context;
GetTypeLookupContextImpl(*this, seen, context);
GetTypeLookupContextImpl(*this, complete_template_names, seen, context);
std::reverse(context.begin(), context.end());
return context;
}
Expand Down
20 changes: 18 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,15 @@ class DWARFDIE : public DWARFBaseDIE {
/// Return this DIE's decl context as it is needed to look up types
/// in Clang modules. This context will include any modules or functions that
/// the type is declared in so an exact module match can be efficiently made.
std::vector<CompilerContext> GetDeclContext() const;
///
/// \param[in] complete_template_names
/// If true, augments the returned names with template arguments derived
/// from the child DIEs, if the names don't contained template arguments
/// already. If false, the returned context will contain the names exactly
/// as they are spelled in the debug info, regardless of whether that
/// includes template arguments or not.
std::vector<CompilerContext>
GetDeclContext(bool complete_template_names = false) const;

/// Get a context to a type so it can be looked up.
///
Expand All @@ -85,7 +93,15 @@ class DWARFDIE : public DWARFBaseDIE {
/// appropriate time, like either the translation unit or at a function
/// context. This is designed to allow users to efficiently look for types
/// using a full or partial CompilerContext array.
std::vector<CompilerContext> GetTypeLookupContext() const;
///
/// \param[in] complete_template_names
/// If true, augments the returned names with template arguments derived
/// from the child DIEs, if the names don't contained template arguments
/// already. If false, the returned context will contain the names exactly
/// as they are spelled in the debug info, regardless of whether that
/// includes template arguments or not.
std::vector<CompilerContext>
GetTypeLookupContext(bool complete_template_names = false) const;

DWARFDeclContext GetDWARFDeclContext() const;

Expand Down
19 changes: 5 additions & 14 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 @@ -10,10 +10,8 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
#include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/FileUtilities.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/FormatAdapters.h"
#include "llvm/Support/Threading.h"

Expand Down Expand Up @@ -2740,18 +2738,11 @@ 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.
}
Comment on lines -2743 to -2747
Copy link
Collaborator Author

@labath labath Jan 15, 2025

Choose a reason for hiding this comment

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

This is already checked in the caller.


std::string qualified_name;
llvm::raw_string_ostream os(qualified_name);
llvm::DWARFTypePrinter<DWARFDIE> type_printer(os);
type_printer.appendQualifiedName(die);
TypeQuery die_query(qualified_name, e_exact_match);
if (query.ContextMatches(die_query.GetContextRef()))
std::vector<CompilerContext> qualified_context =
query.GetModuleSearch()
? die.GetDeclContext(/*complete_template_names=*/true)
: die.GetTypeLookupContext(/*complete_template_names=*/true);
if (query.ContextMatches(qualified_context))
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.
Expand Down
5 changes: 5 additions & 0 deletions lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ def do_test(self, debug_flags):
DATA_TYPES_DISPLAYED_CORRECTLY,
substrs=["1 match found"],
)
self.expect(
"image lookup -A -t 'NS::Struct<int>'",
DATA_TYPES_DISPLAYED_CORRECTLY,
substrs=["1 match found"],
)

@skipIf(compiler=no_match("clang"))
@skipIf(compiler_version=["<", "15.0"])
Expand Down
7 changes: 7 additions & 0 deletions lldb/test/API/lang/cpp/nested-template/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ struct Outer {
struct Inner {};
};

namespace NS {
namespace {
template <typename T> struct Struct {};
} // namespace
} // namespace NS

int main() {
Outer::Inner<int> oi;
NS::Struct<int> ns_struct;
}
Loading