Skip to content

Commit 7901f38

Browse files
augusto2112augusto2112
committed
[lldb] Refactor LookupInfo object to be per-language (llvm#168797)
Some months ago, the LookupInfo constructor logic was refactored to not depend on language specific logic, and use languages plugins instead. In this refactor, when the language type is unknown, a single LookupInfo object will handle multiple languages. This doesn't work well, as multiple languages might want to configure the LookupInfo object in different ways. For example, different languages might want to set the m_lookup_name differently from each other, but the previous implementation would pick the first name a language provided, and effectively ignored every other language. Other fields of the LookupInfo object are also configured in incompatible ways. This approach doesn't seem to be a problem upstream, since only the C++/Objective-C language plugins are available, but it broke downstream on the Swift fork, as adding Swift to the list of default languages when the language type is unknown breaks C++ tests. This patch makes it so instead of building a single LookupInfo object for multiple languages, one LookupInfo object is built per language instead. rdar://159531216 (cherry picked from commit d7fb086)
1 parent 45b2f1b commit 7901f38

File tree

16 files changed

+350
-81
lines changed

16 files changed

+350
-81
lines changed

‎lldb/include/lldb/Core/Module.h‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,15 @@ class Module : public std::enable_shared_from_this<Module>,
317317
const ModuleFunctionSearchOptions &options,
318318
SymbolContextList &sc_list);
319319

320+
/// Find functions by a vector of lookup infos.
321+
///
322+
/// If the function is an inlined function, it will have a block,
323+
/// representing the inlined function, and the function will be the
324+
/// containing function. If it is not inlined, then the block will be NULL.
325+
void FindFunctions(const std::vector<LookupInfo> &lookup_infos,
326+
const CompilerDeclContext &parent_decl_ctx,
327+
const ModuleFunctionSearchOptions &options,
328+
SymbolContextList &sc_list);
320329
/// Find functions by name.
321330
///
322331
/// If the function is an inlined function, it will have a block,
@@ -952,8 +961,29 @@ class Module : public std::enable_shared_from_this<Module>,
952961
public:
953962
LookupInfo() = default;
954963

955-
LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
956-
lldb::LanguageType language);
964+
/// Creates a vector of lookup infos for function name resolution.
965+
///
966+
/// \param[in] name
967+
/// The function name to search for. This can be a simple name like
968+
/// "foo" or a qualified name like "Class::method".
969+
///
970+
/// \param[in] name_type_mask
971+
/// A bitmask specifying what types of names to search for
972+
/// (e.g., eFunctionNameTypeFull, eFunctionNameTypeBase,
973+
/// eFunctionNameTypeMethod, eFunctionNameTypeAuto). Multiple types
974+
/// can be combined with bitwise OR.
975+
///
976+
/// \param[in] lang_type
977+
/// The language to create lookups for. If eLanguageTypeUnknown is
978+
/// passed, creates one LookupInfo for each language plugin currently
979+
/// available in LLDB. If a specific language is provided, creates only
980+
// a single LookupInfo for that language.
981+
///
982+
/// \return
983+
/// A vector of LookupInfo objects, one per relevant language.
984+
static std::vector<LookupInfo>
985+
MakeLookupInfos(ConstString name, lldb::FunctionNameType name_type_mask,
986+
lldb::LanguageType lang_type);
957987

958988
ConstString GetName() const { return m_name; }
959989

@@ -994,6 +1024,10 @@ class Module : public std::enable_shared_from_this<Module>,
9941024
/// If \b true, then demangled names that match will need to contain
9951025
/// "m_name" in order to be considered a match
9961026
bool m_match_name_after_lookup = false;
1027+
1028+
private:
1029+
LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
1030+
lldb::LanguageType lang_type);
9971031
};
9981032

9991033
/// Get a unique hash for this module.

‎lldb/include/lldb/Symbol/SymbolContext.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,20 @@ class SymbolContext {
231231

232232
lldb::LanguageType GetLanguage() const;
233233

234+
/// Compares the two symbol contexts, considering that the symbol may or may
235+
/// not be present. If both symbols are present, compare them, if one of the
236+
/// symbols is not present, consider the symbol contexts as equal as long as
237+
/// the other fields are equal.
238+
///
239+
/// This function exists because SymbolContexts are often created without the
240+
/// symbol, which is filled in later on, after its creation.
241+
static bool CompareConsideringPossiblyNullSymbol(const SymbolContext &lhs,
242+
const SymbolContext &rhs);
243+
244+
/// Compares the two symbol contexts, except for the symbol field.
245+
static bool CompareWithoutSymbol(const SymbolContext &lhs,
246+
const SymbolContext &rhs);
247+
234248
/// Find a block that defines the function represented by this symbol
235249
/// context.
236250
///

‎lldb/include/lldb/Symbol/SymbolFile.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,10 @@ class SymbolFile : public PluginInterface {
313313
virtual void FindFunctions(const Module::LookupInfo &lookup_info,
314314
const CompilerDeclContext &parent_decl_ctx,
315315
bool include_inlines, SymbolContextList &sc_list);
316+
virtual void
317+
FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
318+
const CompilerDeclContext &parent_decl_ctx,
319+
bool include_inlines, SymbolContextList &sc_list);
316320
virtual void FindFunctions(const RegularExpression &regex,
317321
bool include_inlines, SymbolContextList &sc_list);
318322
/// Finds imported declarations whose name match \p name.

‎lldb/source/Breakpoint/BreakpointResolverName.cpp‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -218,19 +218,22 @@ StructuredData::ObjectSP BreakpointResolverName::SerializeToStructuredData() {
218218

219219
void BreakpointResolverName::AddNameLookup(ConstString name,
220220
FunctionNameType name_type_mask) {
221-
222-
Module::LookupInfo lookup(name, name_type_mask, m_language);
223-
m_lookups.emplace_back(lookup);
221+
std::vector<Module::LookupInfo> infos =
222+
Module::LookupInfo::MakeLookupInfos(name, name_type_mask, m_language);
223+
llvm::append_range(m_lookups, infos);
224224

225225
auto add_variant_funcs = [&](Language *lang) {
226226
for (Language::MethodNameVariant variant :
227227
lang->GetMethodNameVariants(name)) {
228228
// FIXME: Should we be adding variants that aren't of type Full?
229229
if (variant.GetType() & lldb::eFunctionNameTypeFull) {
230-
Module::LookupInfo variant_lookup(name, variant.GetType(),
231-
lang->GetLanguageType());
232-
variant_lookup.SetLookupName(variant.GetName());
233-
m_lookups.emplace_back(variant_lookup);
230+
std::vector<Module::LookupInfo> variant_lookups =
231+
Module::LookupInfo::MakeLookupInfos(name, variant.GetType(),
232+
lang->GetLanguageType());
233+
llvm::for_each(variant_lookups, [&](auto &variant_lookup) {
234+
variant_lookup.SetLookupName(variant.GetName());
235+
});
236+
llvm::append_range(m_lookups, variant_lookups);
234237
}
235238
}
236239
return true;
@@ -401,14 +404,22 @@ void BreakpointResolverName::GetDescription(Stream *s) {
401404
if (m_match_type == Breakpoint::Regexp)
402405
s->Printf("regex = '%s'", m_regex.GetText().str().c_str());
403406
else {
404-
size_t num_names = m_lookups.size();
405-
if (num_names == 1)
406-
s->Printf("name = '%s'", m_lookups[0].GetName().GetCString());
407+
// Since there may be many lookups objects for the same name breakpoint (one
408+
// per language available), unique them by name, and operate on those unique
409+
// names.
410+
std::vector<ConstString> unique_lookups;
411+
for (auto &lookup : m_lookups) {
412+
if (!llvm::is_contained(unique_lookups, lookup.GetName()))
413+
unique_lookups.push_back(lookup.GetName());
414+
}
415+
if (unique_lookups.size() == 1)
416+
s->Printf("name = '%s'", unique_lookups[0].GetCString());
407417
else {
418+
size_t num_names = unique_lookups.size();
408419
s->Printf("names = {");
409420
for (size_t i = 0; i < num_names; i++) {
410421
s->Printf("%s'%s'", (i == 0 ? "" : ", "),
411-
m_lookups[i].GetName().GetCString());
422+
unique_lookups[i].GetCString());
412423
}
413424
s->Printf("}");
414425
}

‎lldb/source/Core/Module.cpp‎

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -651,26 +651,13 @@ void Module::FindCompileUnits(const FileSpec &path,
651651

652652
Module::LookupInfo::LookupInfo(ConstString name,
653653
FunctionNameType name_type_mask,
654-
LanguageType language)
655-
: m_name(name), m_lookup_name(name), m_language(language) {
654+
LanguageType lang_type)
655+
: m_name(name), m_lookup_name(name), m_language(lang_type) {
656656
std::optional<ConstString> basename;
657-
658-
std::vector<Language *> languages;
659-
{
660-
std::vector<LanguageType> lang_types;
661-
if (language != eLanguageTypeUnknown)
662-
lang_types.push_back(language);
663-
else
664-
lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus};
665-
666-
for (LanguageType lang_type : lang_types) {
667-
if (Language *lang = Language::FindPlugin(lang_type))
668-
languages.push_back(lang);
669-
}
670-
}
657+
Language *lang = Language::FindPlugin(lang_type);
671658

672659
if (name_type_mask & eFunctionNameTypeAuto) {
673-
for (Language *lang : languages) {
660+
if (lang) {
674661
auto info = lang->GetFunctionNameInfo(name);
675662
if (info.first != eFunctionNameTypeNone) {
676663
m_name_type_mask |= info.first;
@@ -687,7 +674,7 @@ Module::LookupInfo::LookupInfo(ConstString name,
687674

688675
} else {
689676
m_name_type_mask = name_type_mask;
690-
for (Language *lang : languages) {
677+
if (lang) {
691678
auto info = lang->GetFunctionNameInfo(name);
692679
if (info.first & m_name_type_mask) {
693680
// If the user asked for FunctionNameTypes that aren't possible,
@@ -696,14 +683,12 @@ Module::LookupInfo::LookupInfo(ConstString name,
696683
// ObjC)
697684
m_name_type_mask &= info.first;
698685
basename = info.second;
699-
break;
700-
}
701-
// Still try and get a basename in case someone specifies a name type mask
702-
// of eFunctionNameTypeFull and a name like "A::func"
703-
if (name_type_mask & eFunctionNameTypeFull &&
704-
info.first != eFunctionNameTypeNone && !basename && info.second) {
686+
} else if (name_type_mask & eFunctionNameTypeFull &&
687+
info.first != eFunctionNameTypeNone && !basename &&
688+
info.second) {
689+
// Still try and get a basename in case someone specifies a name type
690+
// mask of eFunctionNameTypeFull and a name like "A::func"
705691
basename = info.second;
706-
break;
707692
}
708693
}
709694
}
@@ -719,6 +704,36 @@ Module::LookupInfo::LookupInfo(ConstString name,
719704
}
720705
}
721706

707+
std::vector<Module::LookupInfo>
708+
Module::LookupInfo::MakeLookupInfos(ConstString name,
709+
lldb::FunctionNameType name_type_mask,
710+
lldb::LanguageType lang_type) {
711+
std::vector<LanguageType> lang_types;
712+
if (lang_type != eLanguageTypeUnknown) {
713+
lang_types.push_back(lang_type);
714+
} else {
715+
// If the language type was not specified, look up in every language
716+
// available.
717+
Language::ForEach([&](Language *lang) {
718+
auto lang_type = lang->GetLanguageType();
719+
if (!llvm::is_contained(lang_types, lang_type))
720+
lang_types.push_back(lang_type);
721+
return IterationAction::Continue;
722+
});
723+
724+
if (lang_types.empty())
725+
lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus};
726+
}
727+
728+
std::vector<Module::LookupInfo> infos;
729+
infos.reserve(lang_types.size());
730+
for (LanguageType lang_type : lang_types) {
731+
Module::LookupInfo info(name, name_type_mask, lang_type);
732+
infos.push_back(info);
733+
}
734+
return infos;
735+
}
736+
722737
bool Module::LookupInfo::NameMatchesLookupInfo(
723738
ConstString function_name, LanguageType language_type) const {
724739
// We always keep unnamed symbols
@@ -837,18 +852,29 @@ void Module::FindFunctions(const Module::LookupInfo &lookup_info,
837852
}
838853
}
839854

855+
void Module::FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
856+
const CompilerDeclContext &parent_decl_ctx,
857+
const ModuleFunctionSearchOptions &options,
858+
SymbolContextList &sc_list) {
859+
for (auto &lookup_info : lookup_infos)
860+
FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
861+
}
862+
840863
void Module::FindFunctions(ConstString name,
841864
const CompilerDeclContext &parent_decl_ctx,
842865
FunctionNameType name_type_mask,
843866
const ModuleFunctionSearchOptions &options,
844867
SymbolContextList &sc_list) {
845-
const size_t old_size = sc_list.GetSize();
846-
LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
847-
FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
848-
if (name_type_mask & eFunctionNameTypeAuto) {
849-
const size_t new_size = sc_list.GetSize();
850-
if (old_size < new_size)
851-
lookup_info.Prune(sc_list, old_size);
868+
std::vector<LookupInfo> lookup_infos =
869+
LookupInfo::MakeLookupInfos(name, name_type_mask, eLanguageTypeUnknown);
870+
for (auto &lookup_info : lookup_infos) {
871+
const size_t old_size = sc_list.GetSize();
872+
FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
873+
if (name_type_mask & eFunctionNameTypeAuto) {
874+
const size_t new_size = sc_list.GetSize();
875+
if (old_size < new_size)
876+
lookup_info.Prune(sc_list, old_size);
877+
}
852878
}
853879
}
854880

‎lldb/source/Core/ModuleList.cpp‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -645,21 +645,22 @@ void ModuleList::FindFunctions(ConstString name,
645645
FunctionNameType name_type_mask,
646646
const ModuleFunctionSearchOptions &options,
647647
SymbolContextList &sc_list) const {
648-
const size_t old_size = sc_list.GetSize();
649-
650648
if (name_type_mask & eFunctionNameTypeAuto) {
651-
Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
652-
649+
std::vector<Module::LookupInfo> lookup_infos =
650+
Module::LookupInfo::MakeLookupInfos(name, name_type_mask,
651+
eLanguageTypeUnknown);
653652
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
654-
for (const ModuleSP &module_sp : m_modules) {
655-
module_sp->FindFunctions(lookup_info, CompilerDeclContext(), options,
656-
sc_list);
657-
}
658-
659-
const size_t new_size = sc_list.GetSize();
653+
for (const auto &lookup_info : lookup_infos) {
654+
const size_t old_size = sc_list.GetSize();
655+
for (const ModuleSP &module_sp : m_modules) {
656+
module_sp->FindFunctions(lookup_info, CompilerDeclContext(), options,
657+
sc_list);
658+
}
660659

661-
if (old_size < new_size)
662-
lookup_info.Prune(sc_list, old_size);
660+
const size_t new_size = sc_list.GetSize();
661+
if (old_size < new_size)
662+
lookup_info.Prune(sc_list, old_size);
663+
}
663664
} else {
664665
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
665666
for (const ModuleSP &module_sp : m_modules) {
@@ -672,21 +673,24 @@ void ModuleList::FindFunctions(ConstString name,
672673
void ModuleList::FindFunctionSymbols(ConstString name,
673674
lldb::FunctionNameType name_type_mask,
674675
SymbolContextList &sc_list) {
675-
const size_t old_size = sc_list.GetSize();
676-
677676
if (name_type_mask & eFunctionNameTypeAuto) {
678-
Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
677+
std::vector<Module::LookupInfo> lookup_infos =
678+
Module::LookupInfo::MakeLookupInfos(name, name_type_mask,
679+
eLanguageTypeUnknown);
679680

680681
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
681-
for (const ModuleSP &module_sp : m_modules) {
682-
module_sp->FindFunctionSymbols(lookup_info.GetLookupName(),
683-
lookup_info.GetNameTypeMask(), sc_list);
684-
}
682+
for (const auto &lookup_info : lookup_infos) {
683+
const size_t old_size = sc_list.GetSize();
684+
for (const ModuleSP &module_sp : m_modules) {
685+
module_sp->FindFunctionSymbols(lookup_info.GetLookupName(),
686+
lookup_info.GetNameTypeMask(), sc_list);
687+
}
685688

686-
const size_t new_size = sc_list.GetSize();
689+
const size_t new_size = sc_list.GetSize();
687690

688-
if (old_size < new_size)
689-
lookup_info.Prune(sc_list, old_size);
691+
if (old_size < new_size)
692+
lookup_info.Prune(sc_list, old_size);
693+
}
690694
} else {
691695
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
692696
for (const ModuleSP &module_sp : m_modules) {

‎lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ void SymbolFileBreakpad::FindFunctions(
437437
sc.comp_unit = cu_sp.get();
438438
sc.function = func_sp.get();
439439
sc.module_sp = func_sp->CalculateSymbolContextModule();
440-
sc_list.Append(sc);
440+
sc_list.AppendIfUnique(sc, /*merge_symbol_into_function=*/true);
441441
}
442442
}
443443
}

‎lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,14 @@ void DWARFIndex::GetNamespacesWithParents(
173173
});
174174
}
175175

176+
void DWARFIndex::GetFunctions(
177+
const std::vector<Module::LookupInfo> &lookup_infos, SymbolFileDWARF &dwarf,
178+
const CompilerDeclContext &parent_decl_ctx,
179+
llvm::function_ref<IterationAction(DWARFDIE die)> callback) {
180+
for (auto &lookup_info : lookup_infos)
181+
GetFunctions(lookup_info, dwarf, parent_decl_ctx, callback);
182+
}
183+
176184
IterationAction DWARFIndex::ProcessNamespaceDieMatchParents(
177185
const CompilerDeclContext &parent_decl_ctx, DWARFDIE die,
178186
llvm::function_ref<IterationAction(DWARFDIE die)> callback) {

‎lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ class DWARFIndex {
8686
const CompilerDeclContext &parent_decl_ctx,
8787
llvm::function_ref<IterationAction(DWARFDIE die)> callback) = 0;
8888
virtual void
89+
GetFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
90+
SymbolFileDWARF &dwarf,
91+
const CompilerDeclContext &parent_decl_ctx,
92+
llvm::function_ref<IterationAction(DWARFDIE die)> callback);
93+
virtual void
8994
GetFunctions(const RegularExpression &regex,
9095
llvm::function_ref<IterationAction(DWARFDIE die)> callback) = 0;
9196

0 commit comments

Comments
 (0)