Skip to content

Commit 15fca71

Browse files
augusto2112augusto2112
committed
[lldb] Refactor LookupInfo object to be per-language
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
1 parent 1262acf commit 15fca71

File tree

13 files changed

+224
-79
lines changed

13 files changed

+224
-79
lines changed

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

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,22 @@ class Module : public std::enable_shared_from_this<Module>,
315315
const ModuleFunctionSearchOptions &options,
316316
SymbolContextList &sc_list);
317317

318+
/// Find functions by a vector of lookup infos.
319+
///
320+
/// If the function is an inlined function, it will have a block,
321+
/// representing the inlined function, and the function will be the
322+
/// containing function. If it is not inlined, then the block will be NULL.
323+
///
324+
/// \param[in] lookup_infos
325+
/// The vector of lookup infos of the function we are looking for.
326+
///
327+
/// \param[out] sc_list
328+
/// A symbol context list that gets filled in with all of the
329+
/// matches.
330+
void FindFunctions(const std::vector<LookupInfo> &lookup_infos,
331+
const CompilerDeclContext &parent_decl_ctx,
332+
const ModuleFunctionSearchOptions &options,
333+
SymbolContextList &sc_list);
318334
/// Find functions by name.
319335
///
320336
/// If the function is an inlined function, it will have a block,
@@ -917,8 +933,29 @@ class Module : public std::enable_shared_from_this<Module>,
917933
public:
918934
LookupInfo() = default;
919935

920-
LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
921-
lldb::LanguageType language);
936+
/// Creates a vector of lookup infos for function name resolution.
937+
///
938+
/// \param[in] name
939+
/// The function name to search for. This can be a simple name like
940+
/// "foo" or a qualified name like "Class::method".
941+
///
942+
/// \param[in] name_type_mask
943+
/// A bitmask specifying what types of names to search for
944+
/// (e.g., eFunctionNameTypeFull, eFunctionNameTypeBase,
945+
/// eFunctionNameTypeMethod, eFunctionNameTypeAuto). Multiple types
946+
/// can be combined with bitwise OR.
947+
///
948+
/// \param[in] requested_lang_type
949+
/// The language to create lookups for. If eLanguageTypeUnknown is
950+
/// passed, creates one LookupInfo for each language plugin currently
951+
/// available in LLDB. If a specific language is provided, creates only
952+
// a single LookupInfo for that language.
953+
///
954+
/// \return
955+
/// A vector of LookupInfo objects, one per relevant language.
956+
static std::vector<LookupInfo>
957+
MakeLookupInfos(ConstString name, lldb::FunctionNameType name_type_mask,
958+
lldb::LanguageType requested_lang_type);
922959

923960
ConstString GetName() const { return m_name; }
924961

@@ -959,6 +996,10 @@ class Module : public std::enable_shared_from_this<Module>,
959996
/// If \b true, then demangled names that match will need to contain
960997
/// "m_name" in order to be considered a match
961998
bool m_match_name_after_lookup = false;
999+
1000+
private:
1001+
LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
1002+
lldb::LanguageType lang_type);
9621003
};
9631004

9641005
/// 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
@@ -309,6 +309,10 @@ class SymbolFile : public PluginInterface {
309309
virtual void FindFunctions(const Module::LookupInfo &lookup_info,
310310
const CompilerDeclContext &parent_decl_ctx,
311311
bool include_inlines, SymbolContextList &sc_list);
312+
virtual void
313+
FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
314+
const CompilerDeclContext &parent_decl_ctx,
315+
bool include_inlines, SymbolContextList &sc_list);
312316
virtual void FindFunctions(const RegularExpression &regex,
313317
bool include_inlines, SymbolContextList &sc_list);
314318

‎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 IterationAction::Continue;
@@ -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
@@ -643,26 +643,13 @@ void Module::FindCompileUnits(const FileSpec &path,
643643

644644
Module::LookupInfo::LookupInfo(ConstString name,
645645
FunctionNameType name_type_mask,
646-
LanguageType language)
647-
: m_name(name), m_lookup_name(name), m_language(language) {
646+
LanguageType lang_type)
647+
: m_name(name), m_lookup_name(name), m_language(lang_type) {
648648
std::optional<ConstString> basename;
649-
650-
std::vector<Language *> languages;
651-
{
652-
std::vector<LanguageType> lang_types;
653-
if (language != eLanguageTypeUnknown)
654-
lang_types.push_back(language);
655-
else
656-
lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus};
657-
658-
for (LanguageType lang_type : lang_types) {
659-
if (Language *lang = Language::FindPlugin(lang_type))
660-
languages.push_back(lang);
661-
}
662-
}
649+
Language *lang = Language::FindPlugin(lang_type);
663650

664651
if (name_type_mask & eFunctionNameTypeAuto) {
665-
for (Language *lang : languages) {
652+
if (lang) {
666653
auto info = lang->GetFunctionNameInfo(name);
667654
if (info.first != eFunctionNameTypeNone) {
668655
m_name_type_mask |= info.first;
@@ -679,7 +666,7 @@ Module::LookupInfo::LookupInfo(ConstString name,
679666

680667
} else {
681668
m_name_type_mask = name_type_mask;
682-
for (Language *lang : languages) {
669+
if (lang) {
683670
auto info = lang->GetFunctionNameInfo(name);
684671
if (info.first & m_name_type_mask) {
685672
// If the user asked for FunctionNameTypes that aren't possible,
@@ -688,14 +675,12 @@ Module::LookupInfo::LookupInfo(ConstString name,
688675
// ObjC)
689676
m_name_type_mask &= info.first;
690677
basename = info.second;
691-
break;
692-
}
693-
// Still try and get a basename in case someone specifies a name type mask
694-
// of eFunctionNameTypeFull and a name like "A::func"
695-
if (name_type_mask & eFunctionNameTypeFull &&
696-
info.first != eFunctionNameTypeNone && !basename && info.second) {
678+
} else if (name_type_mask & eFunctionNameTypeFull &&
679+
info.first != eFunctionNameTypeNone && !basename &&
680+
info.second) {
681+
// Still try and get a basename in case someone specifies a name type
682+
// mask of eFunctionNameTypeFull and a name like "A::func"
697683
basename = info.second;
698-
break;
699684
}
700685
}
701686
}
@@ -711,6 +696,36 @@ Module::LookupInfo::LookupInfo(ConstString name,
711696
}
712697
}
713698

699+
std::vector<Module::LookupInfo>
700+
Module::LookupInfo::MakeLookupInfos(ConstString name,
701+
lldb::FunctionNameType name_type_mask,
702+
lldb::LanguageType requested_lang_type) {
703+
std::vector<LanguageType> lang_types;
704+
if (requested_lang_type != eLanguageTypeUnknown) {
705+
lang_types.push_back(requested_lang_type);
706+
} else {
707+
// If the language type was not specified, look up in every language
708+
// available.
709+
Language::ForEach([&](Language *lang) {
710+
auto lang_type = lang->GetLanguageType();
711+
if (!llvm::is_contained(lang_types, lang_type))
712+
lang_types.push_back(lang_type);
713+
return IterationAction::Continue;
714+
});
715+
716+
if (lang_types.empty())
717+
lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus};
718+
}
719+
720+
std::vector<Module::LookupInfo> infos;
721+
infos.reserve(lang_types.size());
722+
for (LanguageType lang_type : lang_types) {
723+
Module::LookupInfo info(name, name_type_mask, lang_type);
724+
infos.push_back(info);
725+
}
726+
return infos;
727+
}
728+
714729
bool Module::LookupInfo::NameMatchesLookupInfo(
715730
ConstString function_name, LanguageType language_type) const {
716731
// We always keep unnamed symbols
@@ -824,18 +839,29 @@ void Module::FindFunctions(const Module::LookupInfo &lookup_info,
824839
}
825840
}
826841

842+
void Module::FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
843+
const CompilerDeclContext &parent_decl_ctx,
844+
const ModuleFunctionSearchOptions &options,
845+
SymbolContextList &sc_list) {
846+
for (auto &lookup_info : lookup_infos)
847+
FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
848+
}
849+
827850
void Module::FindFunctions(ConstString name,
828851
const CompilerDeclContext &parent_decl_ctx,
829852
FunctionNameType name_type_mask,
830853
const ModuleFunctionSearchOptions &options,
831854
SymbolContextList &sc_list) {
832-
const size_t old_size = sc_list.GetSize();
833-
LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
834-
FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
835-
if (name_type_mask & eFunctionNameTypeAuto) {
836-
const size_t new_size = sc_list.GetSize();
837-
if (old_size < new_size)
838-
lookup_info.Prune(sc_list, old_size);
855+
std::vector<LookupInfo> lookup_infos =
856+
LookupInfo::MakeLookupInfos(name, name_type_mask, eLanguageTypeUnknown);
857+
for (auto &lookup_info : lookup_infos) {
858+
const size_t old_size = sc_list.GetSize();
859+
FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
860+
if (name_type_mask & eFunctionNameTypeAuto) {
861+
const size_t new_size = sc_list.GetSize();
862+
if (old_size < new_size)
863+
lookup_info.Prune(sc_list, old_size);
864+
}
839865
}
840866
}
841867

‎lldb/source/Core/ModuleList.cpp‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -453,21 +453,22 @@ void ModuleList::FindFunctions(ConstString name,
453453
FunctionNameType name_type_mask,
454454
const ModuleFunctionSearchOptions &options,
455455
SymbolContextList &sc_list) const {
456-
const size_t old_size = sc_list.GetSize();
457-
458456
if (name_type_mask & eFunctionNameTypeAuto) {
459-
Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
460-
457+
std::vector<Module::LookupInfo> lookup_infos =
458+
Module::LookupInfo::MakeLookupInfos(name, name_type_mask,
459+
eLanguageTypeUnknown);
461460
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
462-
for (const ModuleSP &module_sp : m_modules) {
463-
module_sp->FindFunctions(lookup_info, CompilerDeclContext(), options,
464-
sc_list);
465-
}
466-
467-
const size_t new_size = sc_list.GetSize();
461+
for (const auto &lookup_info : lookup_infos) {
462+
const size_t old_size = sc_list.GetSize();
463+
for (const ModuleSP &module_sp : m_modules) {
464+
module_sp->FindFunctions(lookup_info, CompilerDeclContext(), options,
465+
sc_list);
466+
}
468467

469-
if (old_size < new_size)
470-
lookup_info.Prune(sc_list, old_size);
468+
const size_t new_size = sc_list.GetSize();
469+
if (old_size < new_size)
470+
lookup_info.Prune(sc_list, old_size);
471+
}
471472
} else {
472473
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
473474
for (const ModuleSP &module_sp : m_modules) {
@@ -480,21 +481,24 @@ void ModuleList::FindFunctions(ConstString name,
480481
void ModuleList::FindFunctionSymbols(ConstString name,
481482
lldb::FunctionNameType name_type_mask,
482483
SymbolContextList &sc_list) {
483-
const size_t old_size = sc_list.GetSize();
484-
485484
if (name_type_mask & eFunctionNameTypeAuto) {
486-
Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
485+
std::vector<Module::LookupInfo> lookup_infos =
486+
Module::LookupInfo::MakeLookupInfos(name, name_type_mask,
487+
eLanguageTypeUnknown);
487488

488489
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
489-
for (const ModuleSP &module_sp : m_modules) {
490-
module_sp->FindFunctionSymbols(lookup_info.GetLookupName(),
491-
lookup_info.GetNameTypeMask(), sc_list);
492-
}
490+
for (const auto &lookup_info : lookup_infos) {
491+
const size_t old_size = sc_list.GetSize();
492+
for (const ModuleSP &module_sp : m_modules) {
493+
module_sp->FindFunctionSymbols(lookup_info.GetLookupName(),
494+
lookup_info.GetNameTypeMask(), sc_list);
495+
}
493496

494-
const size_t new_size = sc_list.GetSize();
497+
const size_t new_size = sc_list.GetSize();
495498

496-
if (old_size < new_size)
497-
lookup_info.Prune(sc_list, old_size);
499+
if (old_size < new_size)
500+
lookup_info.Prune(sc_list, old_size);
501+
}
498502
} else {
499503
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
500504
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)