https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/87344
This patch introduces a new `IterationMarker` enum (happy to take alternative name suggestions), which callbacks, like the one in `SymbolFileDWARFDebugMap::ForEachSymbolFile` can return in order to indicate whether the caller should continue iterating or bail. For now this patch just changes the `ForEachSymbolFile` callback to use this new enum. In the future we could change the various `DWARFIndex::GetXXX` callbacks to do the same. This makes the callbacks easier to read and hopefully reduces the chance of bugs like https://github.com/llvm/llvm-project/pull/87177. >From 9ebae9e48ae327ab16e1e7480a3e51b945b990c7 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Tue, 2 Apr 2024 13:50:09 +0100 Subject: [PATCH] [lldb][SymbolFileDWARFDebugMap] Introduce enum to indicate whether to continue iteration of object files This patch introduces a new `IterationMarker` enum (happy to take alternative name suggestions), which callbacks, like the one in `SymbolFileDWARFDebugMap::ForEachSymbolFile` can return in order to indicate whether the caller should continue iterating or bail. For now this patch just changes the `ForEachSymbolFile` callback to use this new enum. In the future we could change the various `DWARFIndex::GetXXX` callbacks to do the same. This makes the callbacks easier to read and hopefully reduces the chance of bugs like https://github.com/llvm/llvm-project/pull/87177. --- lldb/include/lldb/lldb-enumerations.h | 7 ++ .../DWARF/SymbolFileDWARFDebugMap.cpp | 73 ++++++++++--------- .../DWARF/SymbolFileDWARFDebugMap.h | 6 +- 3 files changed, 49 insertions(+), 37 deletions(-) diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 646f7bfda98475..eea5f44c0e192c 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1339,6 +1339,13 @@ enum AddressMaskRange { eAddressMaskRangeAll = eAddressMaskRangeAny, }; +/// Useful for callbacks whose return type indicates +/// whether to continue iteration or short-circuit. +enum class IterationMarker { + eContinue = 0, + eStop, +}; + } // namespace lldb #endif // LLDB_LLDB_ENUMERATIONS_H diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 4bc2cfd60688a8..d40be5d718aa6e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -37,6 +37,7 @@ #include "LogChannelDWARF.h" #include "SymbolFileDWARF.h" +#include "lldb/lldb-enumerations.h" #include <memory> #include <optional> @@ -803,13 +804,13 @@ SymbolFileDWARFDebugMap::GetDynamicArrayInfoForUID( bool SymbolFileDWARFDebugMap::CompleteType(CompilerType &compiler_type) { bool success = false; if (compiler_type) { - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { if (oso_dwarf->HasForwardDeclForCompilerType(compiler_type)) { oso_dwarf->CompleteType(compiler_type); success = true; - return true; + return IterationMarker::eStop; } - return false; + return IterationMarker::eContinue; }); } return success; @@ -915,7 +916,7 @@ void SymbolFileDWARFDebugMap::FindGlobalVariables( std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); uint32_t total_matches = 0; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { const uint32_t old_size = variables.GetSize(); oso_dwarf->FindGlobalVariables(name, parent_decl_ctx, max_matches, variables); @@ -925,18 +926,18 @@ void SymbolFileDWARFDebugMap::FindGlobalVariables( // Are we getting all matches? if (max_matches == UINT32_MAX) - return false; // Yep, continue getting everything + return IterationMarker::eContinue; // Yep, continue getting everything // If we have found enough matches, lets get out if (max_matches >= total_matches) - return true; + return IterationMarker::eStop; // Update the max matches for any subsequent calls to find globals in any // other object files with DWARF max_matches -= oso_matches; } - return false; + return IterationMarker::eContinue; }); } @@ -945,7 +946,7 @@ void SymbolFileDWARFDebugMap::FindGlobalVariables( VariableList &variables) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); uint32_t total_matches = 0; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { const uint32_t old_size = variables.GetSize(); oso_dwarf->FindGlobalVariables(regex, max_matches, variables); @@ -955,18 +956,18 @@ void SymbolFileDWARFDebugMap::FindGlobalVariables( // Are we getting all matches? if (max_matches == UINT32_MAX) - return false; // Yep, continue getting everything + return IterationMarker::eContinue; // Yep, continue getting everything // If we have found enough matches, lets get out if (max_matches >= total_matches) - return true; + return IterationMarker::eStop; // Update the max matches for any subsequent calls to find globals in any // other object files with DWARF max_matches -= oso_matches; } - return false; + return IterationMarker::eContinue; }); } @@ -1071,7 +1072,7 @@ void SymbolFileDWARFDebugMap::FindFunctions( LLDB_SCOPED_TIMERF("SymbolFileDWARFDebugMap::FindFunctions (name = %s)", lookup_info.GetLookupName().GetCString()); - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { uint32_t sc_idx = sc_list.GetSize(); oso_dwarf->FindFunctions(lookup_info, parent_decl_ctx, include_inlines, sc_list); @@ -1079,7 +1080,7 @@ void SymbolFileDWARFDebugMap::FindFunctions( RemoveFunctionsWithModuleNotEqualTo(m_objfile_sp->GetModule(), sc_list, sc_idx); } - return false; + return IterationMarker::eContinue; }); } @@ -1090,7 +1091,7 @@ void SymbolFileDWARFDebugMap::FindFunctions(const RegularExpression ®ex, LLDB_SCOPED_TIMERF("SymbolFileDWARFDebugMap::FindFunctions (regex = '%s')", regex.GetText().str().c_str()); - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { uint32_t sc_idx = sc_list.GetSize(); oso_dwarf->FindFunctions(regex, include_inlines, sc_list); @@ -1098,7 +1099,7 @@ void SymbolFileDWARFDebugMap::FindFunctions(const RegularExpression ®ex, RemoveFunctionsWithModuleNotEqualTo(m_objfile_sp->GetModule(), sc_list, sc_idx); } - return false; + return IterationMarker::eContinue; }); } @@ -1121,9 +1122,9 @@ void SymbolFileDWARFDebugMap::GetTypes(SymbolContextScope *sc_scope, oso_dwarf->GetTypes(sc_scope, type_mask, type_list); } } else { - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { oso_dwarf->GetTypes(sc_scope, type_mask, type_list); - return false; + return IterationMarker::eContinue; }); } } @@ -1141,9 +1142,9 @@ SymbolFileDWARFDebugMap::ParseCallEdgesInFunction( TypeSP SymbolFileDWARFDebugMap::FindDefinitionTypeForDWARFDeclContext( const DWARFDIE &die) { TypeSP type_sp; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die); - return ((bool)type_sp); + return type_sp ? IterationMarker::eStop : IterationMarker::eContinue; }); return type_sp; } @@ -1152,13 +1153,13 @@ bool SymbolFileDWARFDebugMap::Supports_DW_AT_APPLE_objc_complete_type( SymbolFileDWARF *skip_dwarf_oso) { if (m_supports_DW_AT_APPLE_objc_complete_type == eLazyBoolCalculate) { m_supports_DW_AT_APPLE_objc_complete_type = eLazyBoolNo; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { if (skip_dwarf_oso != oso_dwarf && oso_dwarf->Supports_DW_AT_APPLE_objc_complete_type(nullptr)) { m_supports_DW_AT_APPLE_objc_complete_type = eLazyBoolYes; - return true; + return IterationMarker::eStop; } - return false; + return IterationMarker::eContinue; }); } return m_supports_DW_AT_APPLE_objc_complete_type == eLazyBoolYes; @@ -1217,10 +1218,10 @@ TypeSP SymbolFileDWARFDebugMap::FindCompleteObjCDefinitionTypeForDIE( if (!must_be_implementation) { TypeSP type_sp; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { type_sp = oso_dwarf->FindCompleteObjCDefinitionTypeForDIE( die, type_name, must_be_implementation); - return (bool)type_sp; + return type_sp ? IterationMarker::eStop : IterationMarker::eContinue; }); return type_sp; @@ -1231,9 +1232,10 @@ TypeSP SymbolFileDWARFDebugMap::FindCompleteObjCDefinitionTypeForDIE( void SymbolFileDWARFDebugMap::FindTypes(const TypeQuery &query, TypeResults &results) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { oso_dwarf->FindTypes(query, results); - return results.Done(query); // Keep iterating if we aren't done. + return results.Done(query) ? IterationMarker::eStop + : IterationMarker::eContinue; }); } @@ -1243,23 +1245,24 @@ CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace( std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); CompilerDeclContext matching_namespace; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { matching_namespace = oso_dwarf->FindNamespace(name, parent_decl_ctx, only_root_namespaces); - return (bool)matching_namespace; + return matching_namespace ? IterationMarker::eStop + : IterationMarker::eContinue; }); return matching_namespace; } void SymbolFileDWARFDebugMap::DumpClangAST(Stream &s) { - ForEachSymbolFile([&s](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&s](SymbolFileDWARF *oso_dwarf) { oso_dwarf->DumpClangAST(s); // The underlying assumption is that DumpClangAST(...) will obtain the // AST from the underlying TypeSystem and therefore we only need to do // this once and can stop after the first iteration hence we return true. - return true; + return IterationMarker::eStop; }); } @@ -1391,7 +1394,7 @@ void SymbolFileDWARFDebugMap::ParseDeclsForContext( lldb_private::CompilerDeclContext decl_ctx) { ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { oso_dwarf->ParseDeclsForContext(decl_ctx); - return false; // Keep iterating + return IterationMarker::eContinue; }); } @@ -1519,14 +1522,14 @@ SymbolFileDWARFDebugMap::AddOSOARanges(SymbolFileDWARF *dwarf2Data, ModuleList SymbolFileDWARFDebugMap::GetDebugInfoModules() { ModuleList oso_modules; - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { ObjectFile *oso_objfile = oso_dwarf->GetObjectFile(); if (oso_objfile) { ModuleSP module_sp = oso_objfile->GetModule(); if (module_sp) oso_modules.Append(module_sp); } - return false; // Keep iterating + return IterationMarker::eContinue; }); return oso_modules; } @@ -1579,8 +1582,8 @@ Status SymbolFileDWARFDebugMap::CalculateFrameVariableError(StackFrame &frame) { void SymbolFileDWARFDebugMap::GetCompileOptions( std::unordered_map<lldb::CompUnitSP, lldb_private::Args> &args) { - ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) { oso_dwarf->GetCompileOptions(args); - return false; + return IterationMarker::eContinue; }); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index d639ee500080d5..de3c1b45597a28 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -20,6 +20,7 @@ #include "UniqueDWARFASTType.h" #include "lldb/Utility/StructuredData.h" +#include "lldb/lldb-enumerations.h" class DWARFASTParserClang; @@ -235,11 +236,12 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon { // If closure returns "false", iteration continues. If it returns // "true", iteration terminates. - void ForEachSymbolFile(std::function<bool(SymbolFileDWARF *)> closure) { + void ForEachSymbolFile( + std::function<lldb::IterationMarker(SymbolFileDWARF *)> closure) { for (uint32_t oso_idx = 0, num_oso_idxs = m_compile_unit_infos.size(); oso_idx < num_oso_idxs; ++oso_idx) { if (SymbolFileDWARF *oso_dwarf = GetSymbolFileByOSOIndex(oso_idx)) { - if (closure(oso_dwarf)) + if (closure(oso_dwarf) == lldb::IterationMarker::eStop) return; } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits