From 4d5c9ad9c3d7ed84efa1307ec158829b95badc6f Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Fri, 16 Jun 2023 15:07:59 -0400 Subject: [PATCH] [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 --- .../Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp | 95 ++++++++++------ .../Plugins/SymbolFile/DWARF/AppleDWARFIndex.h | 25 +++-- .../source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp | 6 + lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h | 2 + .../Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp | 125 --------------------- .../Plugins/SymbolFile/DWARF/HashedNameToDIE.h | 12 -- 6 files changed, 87 insertions(+), 178 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp index 33555d4..5cc44ea 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::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( - 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( + apple_names.GetAsLLVMDWARF(), llvm_debug_str); auto apple_namespaces_table_up = - std::make_unique( - apple_namespaces, debug_str, ".apple_namespaces"); - if (!apple_namespaces_table_up->IsValid()) - apple_namespaces_table_up.reset(); + std::make_unique( + apple_namespaces.GetAsLLVMDWARF(), llvm_debug_str); auto apple_types_table_up = std::make_unique( apple_types, debug_str, ".apple_types"); @@ -43,6 +42,16 @@ std::unique_ptr 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( @@ -53,13 +62,23 @@ std::unique_ptr AppleDWARFIndex::Create( return nullptr; } +void AppleDWARFIndex::SearchFor(const llvm::AppleAcceleratorTable &table, + llvm::StringRef name, + llvm::function_ref callback, + std::optional search_for_tag, + std::optional 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 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 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 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 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 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 ®ex, llvm::function_ref 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 3a5b8ee..50a8fe2 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 @@ public: DWARFDataExtractor apple_namespaces, DWARFDataExtractor apple_types, DWARFDataExtractor apple_objc, DWARFDataExtractor debug_str); - AppleDWARFIndex( - Module &module, std::unique_ptr apple_names, - std::unique_ptr apple_namespaces, - std::unique_ptr apple_types, - std::unique_ptr apple_objc) + AppleDWARFIndex(Module &module, + std::unique_ptr apple_names, + std::unique_ptr apple_namespaces, + std::unique_ptr apple_types, + std::unique_ptr 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 @@ public: void Dump(Stream &s) override; private: - std::unique_ptr m_apple_names_up; - std::unique_ptr m_apple_namespaces_up; + std::unique_ptr m_apple_names_up; + std::unique_ptr m_apple_namespaces_up; std::unique_ptr m_apple_types_up; std::unique_ptr 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 callback, + std::optional search_for_tag = std::nullopt, + std::optional 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 6e957f7..779b524 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 c820793..13fe96d 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 @@ protected: llvm::function_ref 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 9b1497d..dee6441 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 ®ex, - 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 ®ex, - 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 callback) { if (name.empty()) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h index 0006949..2d28f97 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h @@ -132,14 +132,6 @@ public: bool ReadHashData(uint32_t hash_data_offset, HashData &hash_data) const override; - void - AppendAllDIEsThatMatchingRegex(const lldb_private::RegularExpression ®ex, - 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 callback); @@ -157,10 +149,6 @@ public: bool must_be_implementation); protected: - Result AppendHashDataForRegularExpression( - const lldb_private::RegularExpression ®ex, - lldb::offset_t *hash_data_offset_ptr, Pair &pair) const; - void FindByName(llvm::StringRef name, DIEInfoArray &die_info_array); Result GetHashDataForName(llvm::StringRef name, -- 2.7.4