[Lldb-commits] [lldb] 4d5c9ad - [lldb] Use LLVM's implementation of AppleTables for apple_{names, namespaces}

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Wed Jun 28 05:31:40 PDT 2023


Author: Felipe de Azevedo Piovezan
Date: 2023-06-28T08:31:02-04:00
New Revision: 4d5c9ad9c3d7ed84efa1307ec158829b95badc6f

URL: https://github.com/llvm/llvm-project/commit/4d5c9ad9c3d7ed84efa1307ec158829b95badc6f
DIFF: https://github.com/llvm/llvm-project/commit/4d5c9ad9c3d7ed84efa1307ec158829b95badc6f.diff

LOG: [lldb] Use LLVM's implementation of AppleTables for apple_{names,namespaces}

All the new code should match the behavior of the old exactly.

Of note, the custom queries used to be implemented inside `HashedNameToDIE.cpp`
(which is the LLDB implementation of the tables). However, when porting to LLVM,
we believe they don't belong inside the LLVM table implementation:

1. They don't require any knowledge about the table itself
2. They are not relevant for other users of these classes.
3. They use LLDB data structures.

As such, we implement these custom queries inside AppleDWARFIndex.cpp.

Types and Objective-C tables are done separately, as they have slightly
different functionality that require rewriting more code.

Differential Revision: https://reviews.llvm.org/D153866

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
    lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
    lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
    lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
    lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 33555d4f8f4ba..5cc44ea5b50c8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -22,16 +22,15 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
     Module &module, DWARFDataExtractor apple_names,
     DWARFDataExtractor apple_namespaces, DWARFDataExtractor apple_types,
     DWARFDataExtractor apple_objc, DWARFDataExtractor debug_str) {
-  auto apple_names_table_up = std::make_unique<DWARFMappedHash::MemoryTable>(
-      apple_names, debug_str, ".apple_names");
-  if (!apple_names_table_up->IsValid())
-    apple_names_table_up.reset();
+
+  llvm::DataExtractor llvm_debug_str = debug_str.GetAsLLVM();
+
+  auto apple_names_table_up = std::make_unique<llvm::AppleAcceleratorTable>(
+      apple_names.GetAsLLVMDWARF(), llvm_debug_str);
 
   auto apple_namespaces_table_up =
-      std::make_unique<DWARFMappedHash::MemoryTable>(
-          apple_namespaces, debug_str, ".apple_namespaces");
-  if (!apple_namespaces_table_up->IsValid())
-    apple_namespaces_table_up.reset();
+      std::make_unique<llvm::AppleAcceleratorTable>(
+          apple_namespaces.GetAsLLVMDWARF(), llvm_debug_str);
 
   auto apple_types_table_up = std::make_unique<DWARFMappedHash::MemoryTable>(
       apple_types, debug_str, ".apple_types");
@@ -43,6 +42,16 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
   if (!apple_objc_table_up->IsValid())
     apple_objc_table_up.reset();
 
+  auto extract_and_check = [](auto &TablePtr) {
+    if (auto E = TablePtr->extract()) {
+      llvm::consumeError(std::move(E));
+      TablePtr.reset();
+    }
+  };
+
+  extract_and_check(apple_names_table_up);
+  extract_and_check(apple_namespaces_table_up);
+
   if (apple_names_table_up || apple_namespaces_table_up ||
       apple_types_table_up || apple_objc_table_up)
     return std::make_unique<AppleDWARFIndex>(
@@ -53,13 +62,23 @@ std::unique_ptr<AppleDWARFIndex> AppleDWARFIndex::Create(
   return nullptr;
 }
 
+void AppleDWARFIndex::SearchFor(const llvm::AppleAcceleratorTable &table,
+                                llvm::StringRef name,
+                                llvm::function_ref<bool(DWARFDIE die)> callback,
+                                std::optional<dw_tag_t> search_for_tag,
+                                std::optional<uint32_t> search_for_qualhash) {
+  assert(!search_for_tag && !search_for_qualhash && "not yet implemented");
+  auto converted_cb = DIERefCallback(callback, name);
+  for (const auto &entry : table.equal_range(name))
+    if (!converted_cb(entry))
+      break;
+}
+
 void AppleDWARFIndex::GetGlobalVariables(
     ConstString basename, llvm::function_ref<bool(DWARFDIE die)> callback) {
   if (!m_apple_names_up)
     return;
-  m_apple_names_up->FindByName(
-      basename.GetStringRef(),
-      DIERefCallback(callback, basename.GetStringRef()));
+  SearchFor(*m_apple_names_up, basename, callback);
 }
 
 void AppleDWARFIndex::GetGlobalVariables(
@@ -68,11 +87,13 @@ void AppleDWARFIndex::GetGlobalVariables(
   if (!m_apple_names_up)
     return;
 
-  DWARFMappedHash::DIEInfoArray hash_data;
-  m_apple_names_up->AppendAllDIEsThatMatchingRegex(regex, hash_data);
-  // This is not really the DIE name.
-  DWARFMappedHash::ExtractDIEArray(hash_data,
-                                   DIERefCallback(callback, regex.GetText()));
+  DIERefCallbackImpl converted_cb = DIERefCallback(callback, regex.GetText());
+
+  for (const auto &entry : m_apple_names_up->entries())
+    if (std::optional<llvm::StringRef> name = entry.readName();
+        name && Mangled(*name).NameMatches(regex))
+      if (!converted_cb(entry.BaseEntry))
+        return;
 }
 
 void AppleDWARFIndex::GetGlobalVariables(
@@ -81,11 +102,18 @@ void AppleDWARFIndex::GetGlobalVariables(
     return;
 
   const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
-  DWARFMappedHash::DIEInfoArray hash_data;
-  m_apple_names_up->AppendAllDIEsInRange(non_skeleton_cu.GetOffset(),
-                                         non_skeleton_cu.GetNextUnitOffset(),
-                                         hash_data);
-  DWARFMappedHash::ExtractDIEArray(hash_data, DIERefCallback(callback));
+  dw_offset_t lower_bound = non_skeleton_cu.GetOffset();
+  dw_offset_t upper_bound = non_skeleton_cu.GetNextUnitOffset();
+  auto is_in_range = [lower_bound, upper_bound](std::optional<uint32_t> val) {
+    return val.has_value() && *val >= lower_bound && *val < upper_bound;
+  };
+
+  DIERefCallbackImpl converted_cb = DIERefCallback(callback);
+  for (auto entry : m_apple_names_up->entries()) {
+    if (is_in_range(entry.BaseEntry.getDIESectionOffset()))
+      if (!converted_cb(entry.BaseEntry))
+        return;
+  }
 }
 
 void AppleDWARFIndex::GetObjCMethods(
@@ -174,31 +202,30 @@ void AppleDWARFIndex::GetNamespaces(
     ConstString name, llvm::function_ref<bool(DWARFDIE die)> callback) {
   if (!m_apple_namespaces_up)
     return;
-  m_apple_namespaces_up->FindByName(
-      name.GetStringRef(), DIERefCallback(callback, name.GetStringRef()));
+  SearchFor(*m_apple_namespaces_up, name, callback);
 }
 
 void AppleDWARFIndex::GetFunctions(
     const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf,
     const CompilerDeclContext &parent_decl_ctx,
     llvm::function_ref<bool(DWARFDIE die)> callback) {
+  if (!m_apple_names_up)
+    return;
+
   ConstString name = lookup_info.GetLookupName();
-  m_apple_names_up->FindByName(name.GetStringRef(), [&](DIERef die_ref) {
-    return ProcessFunctionDIE(lookup_info, die_ref, dwarf, parent_decl_ctx,
-                              callback);
-  });
+  for (const auto &entry : m_apple_names_up->equal_range(name)) {
+    DIERef die_ref(std::nullopt, DIERef::Section::DebugInfo,
+                   *entry.getDIESectionOffset());
+    if (!ProcessFunctionDIE(lookup_info, die_ref, dwarf, parent_decl_ctx,
+                            callback))
+      return;
+  }
 }
 
 void AppleDWARFIndex::GetFunctions(
     const RegularExpression &regex,
     llvm::function_ref<bool(DWARFDIE die)> callback) {
-  if (!m_apple_names_up)
-    return;
-
-  DWARFMappedHash::DIEInfoArray hash_data;
-  m_apple_names_up->AppendAllDIEsThatMatchingRegex(regex, hash_data);
-  DWARFMappedHash::ExtractDIEArray(hash_data,
-                                   DIERefCallback(callback, regex.GetText()));
+  return GetGlobalVariables(regex, callback);
 }
 
 void AppleDWARFIndex::Dump(Stream &s) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
index 3a5b8eef9cf40..50a8fe22bcb4f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -11,6 +11,7 @@
 
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/HashedNameToDIE.h"
+#include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
 
 namespace lldb_private {
 class AppleDWARFIndex : public DWARFIndex {
@@ -20,11 +21,11 @@ class AppleDWARFIndex : public DWARFIndex {
          DWARFDataExtractor apple_namespaces, DWARFDataExtractor apple_types,
          DWARFDataExtractor apple_objc, DWARFDataExtractor debug_str);
 
-  AppleDWARFIndex(
-      Module &module, std::unique_ptr<DWARFMappedHash::MemoryTable> apple_names,
-      std::unique_ptr<DWARFMappedHash::MemoryTable> apple_namespaces,
-      std::unique_ptr<DWARFMappedHash::MemoryTable> apple_types,
-      std::unique_ptr<DWARFMappedHash::MemoryTable> apple_objc)
+  AppleDWARFIndex(Module &module,
+                  std::unique_ptr<llvm::AppleAcceleratorTable> apple_names,
+                  std::unique_ptr<llvm::AppleAcceleratorTable> apple_namespaces,
+                  std::unique_ptr<DWARFMappedHash::MemoryTable> apple_types,
+                  std::unique_ptr<DWARFMappedHash::MemoryTable> apple_objc)
       : DWARFIndex(module), m_apple_names_up(std::move(apple_names)),
         m_apple_namespaces_up(std::move(apple_namespaces)),
         m_apple_types_up(std::move(apple_types)),
@@ -62,10 +63,20 @@ class AppleDWARFIndex : public DWARFIndex {
   void Dump(Stream &s) override;
 
 private:
-  std::unique_ptr<DWARFMappedHash::MemoryTable> m_apple_names_up;
-  std::unique_ptr<DWARFMappedHash::MemoryTable> m_apple_namespaces_up;
+  std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_names_up;
+  std::unique_ptr<llvm::AppleAcceleratorTable> m_apple_namespaces_up;
   std::unique_ptr<DWARFMappedHash::MemoryTable> m_apple_types_up;
   std::unique_ptr<DWARFMappedHash::MemoryTable> m_apple_objc_up;
+
+  /// Search for entries whose name is `name` in `table`, calling `callback` for
+  /// each match. If `search_for_tag` is provided, ignore entries whose tag is
+  /// not `search_for_tag`. If `search_for_qualhash` is provided, ignore entries
+  /// whose qualified name hash does not match `search_for_qualhash`.
+  /// If `callback` returns false for an entry, the search is interrupted.
+  void SearchFor(const llvm::AppleAcceleratorTable &table, llvm::StringRef name,
+                 llvm::function_ref<bool(DWARFDIE die)> callback,
+                 std::optional<dw_tag_t> search_for_tag = std::nullopt,
+                 std::optional<uint32_t> search_for_qualhash = std::nullopt);
 };
 } // namespace lldb_private
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index 6e957f7eae629..779b52481b856 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -99,6 +99,12 @@ bool DWARFIndex::DIERefCallbackImpl::operator()(DIERef ref) const {
   return true;
 }
 
+bool DWARFIndex::DIERefCallbackImpl::operator()(
+    const llvm::AppleAcceleratorTable::Entry &entry) const {
+  return this->operator()(DIERef(std::nullopt, DIERef::Section::DebugInfo,
+                                 *entry.getDIESectionOffset()));
+}
+
 void DWARFIndex::ReportInvalidDIERef(DIERef ref, llvm::StringRef name) const {
   m_module.ReportErrorIfModifyDetected(
       "the DWARF debug information has been modified (accelerator table had "

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index c8207931a819f..13fe96dae2aa1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -12,6 +12,7 @@
 #include "Plugins/SymbolFile/DWARF/DIERef.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
 #include "Plugins/SymbolFile/DWARF/DWARFFormValue.h"
+#include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
 
 #include "lldb/Core/Module.h"
 #include "lldb/Target/Statistics.h"
@@ -85,6 +86,7 @@ class DWARFIndex {
                        llvm::function_ref<bool(DWARFDIE die)> callback,
                        llvm::StringRef name);
     bool operator()(DIERef ref) const;
+    bool operator()(const llvm::AppleAcceleratorTable::Entry &entry) const;
 
   private:
     const DWARFIndex &m_index;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
index 9b1497d955bcf..dee644120f699 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -404,131 +404,6 @@ DWARFMappedHash::MemoryTable::GetHashDataForName(
   }
 }
 
-DWARFMappedHash::MemoryTable::Result
-DWARFMappedHash::MemoryTable::AppendHashDataForRegularExpression(
-    const lldb_private::RegularExpression &regex,
-    lldb::offset_t *hash_data_offset_ptr, Pair &pair) const {
-  pair.key = m_data.GetU32(hash_data_offset_ptr);
-  // If the key is zero, this terminates our chain of HashData objects for this
-  // hash value.
-  if (pair.key == 0)
-    return eResultEndOfHashData;
-
-  // There definitely should be a string for this string offset, if there
-  // isn't, there is something wrong, return and error.
-  const char *strp_cstr = m_string_table.PeekCStr(pair.key);
-  if (strp_cstr == nullptr)
-    return eResultError;
-
-  const uint32_t count = m_data.GetU32(hash_data_offset_ptr);
-  const size_t min_total_hash_data_size =
-      count * m_header.header_data.GetMinimumHashDataByteSize();
-  if (count > 0 && m_data.ValidOffsetForDataOfSize(*hash_data_offset_ptr,
-                                                   min_total_hash_data_size)) {
-    // The name in the name table may be a mangled name, in which case we
-    // should also compare against the demangled version.  The simplest way to
-    // do that is to use the Mangled class:
-    lldb_private::Mangled mangled_name((llvm::StringRef(strp_cstr)));
-    const bool match = mangled_name.NameMatches(regex);
-
-    if (!match && m_header.header_data.HashDataHasFixedByteSize()) {
-      // If the regex doesn't match and we have fixed size data, we can just
-      // add the total byte size of all HashData objects to the hash data
-      // offset and be done...
-      *hash_data_offset_ptr += min_total_hash_data_size;
-    } else {
-      // If the string does match, or we don't have fixed size data then we
-      // need to read the hash data as a stream. If the string matches we also
-      // append all HashData objects to the value array.
-      for (uint32_t i = 0; i < count; ++i) {
-        DIEInfo die_info;
-        if (m_header.Read(m_data, hash_data_offset_ptr, die_info)) {
-          // Only happened if the HashData of the string matched...
-          if (match)
-            pair.value.push_back(die_info);
-        } else {
-          // Something went wrong while reading the data
-          *hash_data_offset_ptr = UINT32_MAX;
-          return eResultError;
-        }
-      }
-    }
-    // Return the correct response depending on if the string matched or not...
-    if (match) {
-      // The key (cstring) matches and we have lookup results!
-      return eResultKeyMatch;
-    } else {
-      // The key doesn't match, this function will get called again for the
-      // next key/value or the key terminator which in our case is a zero
-      // .debug_str offset.
-      return eResultKeyMismatch;
-    }
-  } else {
-    *hash_data_offset_ptr = UINT32_MAX;
-    return eResultError;
-  }
-}
-
-void DWARFMappedHash::MemoryTable::AppendAllDIEsThatMatchingRegex(
-    const lldb_private::RegularExpression &regex,
-    DIEInfoArray &die_info_array) const {
-  const uint32_t hash_count = m_header.hashes_count;
-  Pair pair;
-  for (uint32_t offset_idx = 0; offset_idx < hash_count; ++offset_idx) {
-    lldb::offset_t hash_data_offset = GetHashDataOffset(offset_idx);
-    while (hash_data_offset != UINT32_MAX) {
-      const lldb::offset_t prev_hash_data_offset = hash_data_offset;
-      Result hash_result =
-          AppendHashDataForRegularExpression(regex, &hash_data_offset, pair);
-      if (prev_hash_data_offset == hash_data_offset)
-        break;
-
-      // Check the result of getting our hash data.
-      switch (hash_result) {
-      case eResultKeyMatch:
-      case eResultKeyMismatch:
-        // Whether we matches or not, it doesn't matter, we keep looking.
-        break;
-
-      case eResultEndOfHashData:
-      case eResultError:
-        hash_data_offset = UINT32_MAX;
-        break;
-      }
-    }
-  }
-  die_info_array.swap(pair.value);
-}
-
-void DWARFMappedHash::MemoryTable::AppendAllDIEsInRange(
-    const uint32_t die_offset_start, const uint32_t die_offset_end,
-    DIEInfoArray &die_info_array) const {
-  const uint32_t hash_count = m_header.hashes_count;
-  for (uint32_t offset_idx = 0; offset_idx < hash_count; ++offset_idx) {
-    bool done = false;
-    lldb::offset_t hash_data_offset = GetHashDataOffset(offset_idx);
-    while (!done && hash_data_offset != UINT32_MAX) {
-      KeyType key = m_data.GetU32(&hash_data_offset);
-      // If the key is zero, this terminates our chain of HashData objects for
-      // this hash value.
-      if (key == 0)
-        break;
-
-      const uint32_t count = m_data.GetU32(&hash_data_offset);
-      for (uint32_t i = 0; i < count; ++i) {
-        DIEInfo die_info;
-        if (m_header.Read(m_data, &hash_data_offset, die_info)) {
-          if (die_info.die_offset == 0)
-            done = true;
-          if (die_offset_start <= die_info.die_offset &&
-              die_info.die_offset < die_offset_end)
-            die_info_array.push_back(die_info);
-        }
-      }
-    }
-  }
-}
-
 bool DWARFMappedHash::MemoryTable::FindByName(
     llvm::StringRef name, llvm::function_ref<bool(DIERef ref)> callback) {
   if (name.empty())

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
index 0006949865def..2d28f9770f2a5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
@@ -132,14 +132,6 @@ class DWARFMappedHash {
     bool ReadHashData(uint32_t hash_data_offset,
                       HashData &hash_data) const override;
 
-    void
-    AppendAllDIEsThatMatchingRegex(const lldb_private::RegularExpression &regex,
-                                   DIEInfoArray &die_info_array) const;
-
-    void AppendAllDIEsInRange(const uint32_t die_offset_start,
-                              const uint32_t die_offset_end,
-                              DIEInfoArray &die_info_array) const;
-
     bool FindByName(llvm::StringRef name,
                     llvm::function_ref<bool(DIERef ref)> callback);
 
@@ -157,10 +149,6 @@ class DWARFMappedHash {
                                 bool must_be_implementation);
 
   protected:
-    Result AppendHashDataForRegularExpression(
-        const lldb_private::RegularExpression &regex,
-        lldb::offset_t *hash_data_offset_ptr, Pair &pair) const;
-
     void FindByName(llvm::StringRef name, DIEInfoArray &die_info_array);
 
     Result GetHashDataForName(llvm::StringRef name,


        


More information about the lldb-commits mailing list