[lldb][SymbolFileDWARFDebugMap] Introduce enum to indicate whether to continue iteration of object files

Michael137 · Michael137 · commit 9ebae9e48ae3 · 2024-04-02T13:57:59.000+01:00
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 llvm#87177.
diff --git 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
@@ -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,15 +1072,15 @@ 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);
     if (!sc_list.IsEmpty()) {
       RemoveFunctionsWithModuleNotEqualTo(m_objfile_sp->GetModule(), sc_list,
                                           sc_idx);
     }
-    return false;
+    return IterationMarker::eContinue;
   });
 }
 
@@ -1090,15 +1091,15 @@ void SymbolFileDWARFDebugMap::FindFunctions(const RegularExpression &regex,
   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);
     if (!sc_list.IsEmpty()) {
       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
@@ -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;
       }
     }
