[Lldb-commits] [lldb] 7e6df41 - [NFC] Refactor symbol table parsing.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 30 13:54:40 PST 2021
Author: Greg Clayton
Date: 2021-11-30T13:54:32-08:00
New Revision: 7e6df41f655e51c984b92bbd9c19a6fb851f35d4
URL: https://github.com/llvm/llvm-project/commit/7e6df41f655e51c984b92bbd9c19a6fb851f35d4
DIFF: https://github.com/llvm/llvm-project/commit/7e6df41f655e51c984b92bbd9c19a6fb851f35d4.diff
LOG: [NFC] Refactor symbol table parsing.
Symbol table parsing has evolved over the years and many plug-ins contained duplicate code in the ObjectFile::GetSymtab() that used to be pure virtual. With this change, the "Symbtab *ObjectFile::GetSymtab()" is no longer virtual and will end up calling a new "void ObjectFile::ParseSymtab(Symtab &symtab)" pure virtual function to actually do the parsing. This helps centralize the code for parsing the symbol table and allows the ObjectFile base class to do all of the common work, like taking the necessary locks and creating the symbol table object itself. Plug-ins now just need to parse when they are asked to parse as the ParseSymtab function will only get called once.
This is a retry of the original patch https://reviews.llvm.org/D113965 which was reverted. There was a deadlock in the Manual DWARF indexing code during symbol preloading where the module was asked on the main thread to preload its symbols, and this would in turn cause the DWARF manual indexing to use a thread pool to index all of the compile units, and if there were relocations on the debug information sections, these threads could ask the ObjectFile to load section contents, which could cause a call to ObjectFileELF::RelocateSection() which would ask for the symbol table from the module and it would deadlock. We can't lock the module in ObjectFile::GetSymtab(), so the solution I am using is to use a llvm::once_flag to create the symbol table object once and then lock the Symtab object. Since all APIs on the symbol table use this lock, this will prevent anyone from using the symbol table before it is parsed and finalized and will avoid the deadlock I mentioned. ObjectFileELF::GetSymtab() was never locking the module lock before and would put off creating the symbol table until somewhere inside ObjectFileELF::GetSymtab(). Now we create it one time inside of the ObjectFile::GetSymtab() and immediately lock it which should be safe enough. This avoids the deadlocks and still provides safety.
Differential Revision: https://reviews.llvm.org/D114288
Added:
Modified:
lldb/include/lldb/Symbol/ObjectFile.h
lldb/include/lldb/Symbol/Symtab.h
lldb/source/Core/Module.cpp
lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/ObjectFile.cpp
lldb/source/Symbol/Symtab.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h
index 4ccd7f92064d5..0a8b38b2c6428 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -19,6 +19,7 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/UUID.h"
#include "lldb/lldb-private.h"
+#include "llvm/Support/Threading.h"
#include "llvm/Support/VersionTuple.h"
namespace lldb_private {
@@ -322,12 +323,26 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
/// Gets the symbol table for the currently selected architecture (and
/// object for archives).
///
- /// Symbol table parsing can be deferred by ObjectFile instances until this
- /// accessor is called the first time.
+ /// This function will manage when ParseSymtab(...) is called to actually do
+ /// the symbol table parsing in each plug-in. This function will take care of
+ /// taking all the necessary locks and finalizing the symbol table when the
+ /// symbol table does get parsed.
///
/// \return
/// The symbol table for this object file.
- virtual Symtab *GetSymtab() = 0;
+ Symtab *GetSymtab();
+
+ /// Parse the symbol table into the provides symbol table object.
+ ///
+ /// Symbol table parsing will be done once when this function is called by
+ /// each object file plugin. All of the necessary locks will already be
+ /// acquired before this function is called and the symbol table object to
+ /// populate is supplied as an argument and doesn't need to be created by
+ /// each plug-in.
+ ///
+ /// \param
+ /// The symbol table to populate.
+ virtual void ParseSymtab(Symtab &symtab) = 0;
/// Perform relocations on the section if necessary.
///
@@ -708,7 +723,12 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
const lldb::addr_t m_memory_addr;
std::unique_ptr<lldb_private::SectionList> m_sections_up;
std::unique_ptr<lldb_private::Symtab> m_symtab_up;
- uint32_t m_synthetic_symbol_idx;
+ /// We need a llvm::once_flag that we can use to avoid locking the module
+ /// lock and deadlocking LLDB. See comments in ObjectFile::GetSymtab() for
+ /// the full details. We also need to be able to clear the symbol table, so we
+ /// need to use a std::unique_ptr to a llvm::once_flag so if we clear the
+ /// symbol table, we can have a new once flag to use when it is created again.
+ std::unique_ptr<llvm::once_flag> m_symtab_once_up;
/// Sets the architecture for a module. At present the architecture can
/// only be set if it is invalid. It is not allowed to switch from one
diff --git a/lldb/include/lldb/Symbol/Symtab.h b/lldb/include/lldb/Symbol/Symtab.h
index e1ad0dfd2eb8d..e5d21c1bf4b38 100644
--- a/lldb/include/lldb/Symbol/Symtab.h
+++ b/lldb/include/lldb/Symbol/Symtab.h
@@ -119,20 +119,13 @@ class Symtab {
lldb::addr_t file_addr, std::function<bool(Symbol *)> const &callback);
void FindFunctionSymbols(ConstString name, uint32_t name_type_mask,
SymbolContextList &sc_list);
- void CalculateSymbolSizes();
void SortSymbolIndexesByValue(std::vector<uint32_t> &indexes,
bool remove_duplicates) const;
static void DumpSymbolHeader(Stream *s);
- void Finalize() {
- // Shrink to fit the symbols so we don't waste memory
- if (m_symbols.capacity() > m_symbols.size()) {
- collection new_symbols(m_symbols.begin(), m_symbols.end());
- m_symbols.swap(new_symbols);
- }
- }
+ void Finalize();
void AppendSymbolNamesToMap(const IndexCollection &indexes,
bool add_demangled, bool add_mangled,
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index bd0a667171a5a..cbecbb9aa5fed 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1379,12 +1379,15 @@ void Module::PreloadSymbols() {
if (!sym_file)
return;
- // Prime the symbol file first, since it adds symbols to the symbol table.
- sym_file->PreloadSymbols();
-
- // Now we can prime the symbol table.
+ // Load the object file symbol table and any symbols from the SymbolFile that
+ // get appended using SymbolFile::AddSymbols(...).
if (Symtab *symtab = sym_file->GetSymtab())
symtab->PreloadSymbols();
+
+ // Now let the symbol file preload its data and the symbol table will be
+ // available without needing to take the module lock.
+ sym_file->PreloadSymbols();
+
}
void Module::SetSymbolFileFileSpec(const FileSpec &file) {
diff --git a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
index bad730512ff49..ce701fd823fdf 100644
--- a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
+++ b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
@@ -116,9 +116,10 @@ bool ObjectFileBreakpad::ParseHeader() {
return true;
}
-Symtab *ObjectFileBreakpad::GetSymtab() {
- // TODO
- return nullptr;
+void ObjectFileBreakpad::ParseSymtab(Symtab &symtab) {
+ // Nothing to do for breakpad files, all information is parsed as debug info
+ // which means "lldb_private::Function" objects are used, or symbols are added
+ // by the SymbolFileBreakpad::AddSymbols(...) function in the symbol file.
}
void ObjectFileBreakpad::CreateSections(SectionList &unified_section_list) {
diff --git a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
index c320c7ad3e2e7..f04e0b4dd7a7a 100644
--- a/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
+++ b/lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
@@ -71,7 +71,7 @@ class ObjectFileBreakpad : public ObjectFile {
return AddressClass::eInvalid;
}
- Symtab *GetSymtab() override;
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
bool IsStripped() override { return false; }
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 8e0f228a988f2..96e94ef08a45e 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2687,155 +2687,131 @@ unsigned ObjectFileELF::RelocateDebugSections(const ELFSectionHeader *rel_hdr,
return 0;
}
-Symtab *ObjectFileELF::GetSymtab() {
+void ObjectFileELF::ParseSymtab(Symtab &lldb_symtab) {
ModuleSP module_sp(GetModule());
if (!module_sp)
- return nullptr;
+ return;
+
+ Progress progress(
+ llvm::formatv("Parsing symbol table for {0}",
+ m_file.GetFilename().AsCString("<Unknown>")));
+ ElapsedTime elapsed(module_sp->GetSymtabParseTime());
// We always want to use the main object file so we (hopefully) only have one
// cached copy of our symtab, dynamic sections, etc.
ObjectFile *module_obj_file = module_sp->GetObjectFile();
if (module_obj_file && module_obj_file != this)
- return module_obj_file->GetSymtab();
-
- if (m_symtab_up == nullptr) {
- Progress progress(
- llvm::formatv("Parsing symbol table for {0}",
- m_file.GetFilename().AsCString("<Unknown>")));
- ElapsedTime elapsed(module_sp->GetSymtabParseTime());
- SectionList *section_list = module_sp->GetSectionList();
- if (!section_list)
- return nullptr;
+ return module_obj_file->ParseSymtab(lldb_symtab);
- uint64_t symbol_id = 0;
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
-
- // Sharable objects and dynamic executables usually have 2 distinct symbol
- // tables, one named ".symtab", and the other ".dynsym". The dynsym is a
- // smaller version of the symtab that only contains global symbols. The
- // information found in the dynsym is therefore also found in the symtab,
- // while the reverse is not necessarily true.
- Section *symtab =
- section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
- if (symtab) {
- m_symtab_up = std::make_unique<Symtab>(symtab->GetObjectFile());
- symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
- }
-
- // The symtab section is non-allocable and can be stripped, while the
- // .dynsym section which should always be always be there. To support the
- // minidebuginfo case we parse .dynsym when there's a .gnu_debuginfo
- // section, nomatter if .symtab was already parsed or not. This is because
- // minidebuginfo normally removes the .symtab symbols which have their
- // matching .dynsym counterparts.
- if (!symtab ||
- GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata"))) {
- Section *dynsym =
- section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
- .get();
- if (dynsym) {
- if (!m_symtab_up)
- m_symtab_up = std::make_unique<Symtab>(dynsym->GetObjectFile());
- symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
- }
- }
+ SectionList *section_list = module_sp->GetSectionList();
+ if (!section_list)
+ return;
- // DT_JMPREL
- // If present, this entry's d_ptr member holds the address of
- // relocation
- // entries associated solely with the procedure linkage table.
- // Separating
- // these relocation entries lets the dynamic linker ignore them during
- // process initialization, if lazy binding is enabled. If this entry is
- // present, the related entries of types DT_PLTRELSZ and DT_PLTREL must
- // also be present.
- const ELFDynamic *symbol = FindDynamicSymbol(DT_JMPREL);
- if (symbol) {
- // Synthesize trampoline symbols to help navigate the PLT.
- addr_t addr = symbol->d_ptr;
- Section *reloc_section =
- section_list->FindSectionContainingFileAddress(addr).get();
- if (reloc_section) {
- user_id_t reloc_id = reloc_section->GetID();
- const ELFSectionHeaderInfo *reloc_header =
- GetSectionHeaderByIndex(reloc_id);
- if (reloc_header) {
- if (m_symtab_up == nullptr)
- m_symtab_up =
- std::make_unique<Symtab>(reloc_section->GetObjectFile());
-
- ParseTrampolineSymbols(m_symtab_up.get(), symbol_id, reloc_header,
- reloc_id);
- }
- }
- }
+ uint64_t symbol_id = 0;
- if (DWARFCallFrameInfo *eh_frame =
- GetModule()->GetUnwindTable().GetEHFrameInfo()) {
- if (m_symtab_up == nullptr)
- m_symtab_up = std::make_unique<Symtab>(this);
- ParseUnwindSymbols(m_symtab_up.get(), eh_frame);
+ // Sharable objects and dynamic executables usually have 2 distinct symbol
+ // tables, one named ".symtab", and the other ".dynsym". The dynsym is a
+ // smaller version of the symtab that only contains global symbols. The
+ // information found in the dynsym is therefore also found in the symtab,
+ // while the reverse is not necessarily true.
+ Section *symtab =
+ section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get();
+ if (symtab)
+ symbol_id += ParseSymbolTable(&lldb_symtab, symbol_id, symtab);
+
+ // The symtab section is non-allocable and can be stripped, while the
+ // .dynsym section which should always be always be there. To support the
+ // minidebuginfo case we parse .dynsym when there's a .gnu_debuginfo
+ // section, nomatter if .symtab was already parsed or not. This is because
+ // minidebuginfo normally removes the .symtab symbols which have their
+ // matching .dynsym counterparts.
+ if (!symtab ||
+ GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata"))) {
+ Section *dynsym =
+ section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
+ .get();
+ if (dynsym)
+ symbol_id += ParseSymbolTable(&lldb_symtab, symbol_id, dynsym);
+ }
+
+ // DT_JMPREL
+ // If present, this entry's d_ptr member holds the address of
+ // relocation
+ // entries associated solely with the procedure linkage table.
+ // Separating
+ // these relocation entries lets the dynamic linker ignore them during
+ // process initialization, if lazy binding is enabled. If this entry is
+ // present, the related entries of types DT_PLTRELSZ and DT_PLTREL must
+ // also be present.
+ const ELFDynamic *symbol = FindDynamicSymbol(DT_JMPREL);
+ if (symbol) {
+ // Synthesize trampoline symbols to help navigate the PLT.
+ addr_t addr = symbol->d_ptr;
+ Section *reloc_section =
+ section_list->FindSectionContainingFileAddress(addr).get();
+ if (reloc_section) {
+ user_id_t reloc_id = reloc_section->GetID();
+ const ELFSectionHeaderInfo *reloc_header =
+ GetSectionHeaderByIndex(reloc_id);
+ if (reloc_header)
+ ParseTrampolineSymbols(&lldb_symtab, symbol_id, reloc_header, reloc_id);
}
+ }
- // If we still don't have any symtab then create an empty instance to avoid
- // do the section lookup next time.
- if (m_symtab_up == nullptr)
- m_symtab_up = std::make_unique<Symtab>(this);
-
- // In the event that there's no symbol entry for the entry point we'll
- // artificially create one. We delegate to the symtab object the figuring
- // out of the proper size, this will usually make it span til the next
- // symbol it finds in the section. This means that if there are missing
- // symbols the entry point might span beyond its function definition.
- // We're fine with this as it doesn't make it worse than not having a
- // symbol entry at all.
- if (CalculateType() == eTypeExecutable) {
- ArchSpec arch = GetArchitecture();
- auto entry_point_addr = GetEntryPointAddress();
- bool is_valid_entry_point =
- entry_point_addr.IsValid() && entry_point_addr.IsSectionOffset();
- addr_t entry_point_file_addr = entry_point_addr.GetFileAddress();
- if (is_valid_entry_point && !m_symtab_up->FindSymbolContainingFileAddress(
- entry_point_file_addr)) {
- uint64_t symbol_id = m_symtab_up->GetNumSymbols();
- // Don't set the name for any synthetic symbols, the Symbol
- // object will generate one if needed when the name is accessed
- // via accessors.
- SectionSP section_sp = entry_point_addr.GetSection();
- Symbol symbol(
- /*symID=*/symbol_id,
- /*name=*/llvm::StringRef(), // Name will be auto generated.
- /*type=*/eSymbolTypeCode,
- /*external=*/true,
- /*is_debug=*/false,
- /*is_trampoline=*/false,
- /*is_artificial=*/true,
- /*section_sp=*/section_sp,
- /*offset=*/0,
- /*size=*/0, // FDE can span multiple symbols so don't use its size.
- /*size_is_valid=*/false,
- /*contains_linker_annotations=*/false,
- /*flags=*/0);
- // When the entry point is arm thumb we need to explicitly set its
- // class address to reflect that. This is important because expression
- // evaluation relies on correctly setting a breakpoint at this
- // address.
- if (arch.GetMachine() == llvm::Triple::arm &&
- (entry_point_file_addr & 1)) {
- symbol.GetAddressRef().SetOffset(entry_point_addr.GetOffset() ^ 1);
- m_address_class_map[entry_point_file_addr ^ 1] =
- AddressClass::eCodeAlternateISA;
- } else {
- m_address_class_map[entry_point_file_addr] = AddressClass::eCode;
- }
- m_symtab_up->AddSymbol(symbol);
+ if (DWARFCallFrameInfo *eh_frame =
+ GetModule()->GetUnwindTable().GetEHFrameInfo()) {
+ ParseUnwindSymbols(&lldb_symtab, eh_frame);
+ }
+
+ // In the event that there's no symbol entry for the entry point we'll
+ // artificially create one. We delegate to the symtab object the figuring
+ // out of the proper size, this will usually make it span til the next
+ // symbol it finds in the section. This means that if there are missing
+ // symbols the entry point might span beyond its function definition.
+ // We're fine with this as it doesn't make it worse than not having a
+ // symbol entry at all.
+ if (CalculateType() == eTypeExecutable) {
+ ArchSpec arch = GetArchitecture();
+ auto entry_point_addr = GetEntryPointAddress();
+ bool is_valid_entry_point =
+ entry_point_addr.IsValid() && entry_point_addr.IsSectionOffset();
+ addr_t entry_point_file_addr = entry_point_addr.GetFileAddress();
+ if (is_valid_entry_point && !lldb_symtab.FindSymbolContainingFileAddress(
+ entry_point_file_addr)) {
+ uint64_t symbol_id = lldb_symtab.GetNumSymbols();
+ // Don't set the name for any synthetic symbols, the Symbol
+ // object will generate one if needed when the name is accessed
+ // via accessors.
+ SectionSP section_sp = entry_point_addr.GetSection();
+ Symbol symbol(
+ /*symID=*/symbol_id,
+ /*name=*/llvm::StringRef(), // Name will be auto generated.
+ /*type=*/eSymbolTypeCode,
+ /*external=*/true,
+ /*is_debug=*/false,
+ /*is_trampoline=*/false,
+ /*is_artificial=*/true,
+ /*section_sp=*/section_sp,
+ /*offset=*/0,
+ /*size=*/0, // FDE can span multiple symbols so don't use its size.
+ /*size_is_valid=*/false,
+ /*contains_linker_annotations=*/false,
+ /*flags=*/0);
+ // When the entry point is arm thumb we need to explicitly set its
+ // class address to reflect that. This is important because expression
+ // evaluation relies on correctly setting a breakpoint at this
+ // address.
+ if (arch.GetMachine() == llvm::Triple::arm &&
+ (entry_point_file_addr & 1)) {
+ symbol.GetAddressRef().SetOffset(entry_point_addr.GetOffset() ^ 1);
+ m_address_class_map[entry_point_file_addr ^ 1] =
+ AddressClass::eCodeAlternateISA;
+ } else {
+ m_address_class_map[entry_point_file_addr] = AddressClass::eCode;
}
+ lldb_symtab.AddSymbol(symbol);
}
-
- m_symtab_up->CalculateSymbolSizes();
}
-
- return m_symtab_up.get();
}
void ObjectFileELF::RelocateSection(lldb_private::Section *section)
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
index 5738e5cf60d57..554f623ec8af1 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -110,7 +110,7 @@ class ObjectFileELF : public lldb_private::ObjectFile {
lldb_private::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
- lldb_private::Symtab *GetSymtab() override;
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
bool IsStripped() override;
@@ -123,7 +123,7 @@ class ObjectFileELF : public lldb_private::ObjectFile {
lldb_private::UUID GetUUID() override;
/// Return the contents of the .gnu_debuglink section, if the object file
- /// contains it.
+ /// contains it.
llvm::Optional<lldb_private::FileSpec> GetDebugLink();
uint32_t GetDependentModules(lldb_private::FileSpecList &files) override;
@@ -278,8 +278,9 @@ class ObjectFileELF : public lldb_private::ObjectFile {
/// number of dynamic symbols parsed.
size_t ParseDynamicSymbols();
- /// Populates m_symtab_up will all non-dynamic linker symbols. This method
- /// will parse the symbols only once. Returns the number of symbols parsed.
+ /// Populates the symbol table with all non-dynamic linker symbols. This
+ /// method will parse the symbols only once. Returns the number of symbols
+ /// parsed.
unsigned ParseSymbolTable(lldb_private::Symtab *symbol_table,
lldb::user_id_t start_id,
lldb_private::Section *symtab);
@@ -384,7 +385,7 @@ class ObjectFileELF : public lldb_private::ObjectFile {
lldb_private::UUID &uuid);
bool AnySegmentHasPhysicalAddress();
-
+
/// Takes the .gnu_debugdata and returns the decompressed object file that is
/// stored within that section.
///
diff --git a/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp b/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
index bec0099517c8f..ca93374548891 100644
--- a/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
+++ b/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
@@ -106,23 +106,10 @@ uint32_t ObjectFileJIT::GetAddressByteSize() const {
return m_data.GetAddressByteSize();
}
-Symtab *ObjectFileJIT::GetSymtab() {
- ModuleSP module_sp(GetModule());
- if (module_sp) {
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- if (m_symtab_up == nullptr) {
- ElapsedTime elapsed(module_sp->GetSymtabParseTime());
- m_symtab_up = std::make_unique<Symtab>(this);
- std::lock_guard<std::recursive_mutex> symtab_guard(
- m_symtab_up->GetMutex());
- ObjectFileJITDelegateSP delegate_sp(m_delegate_wp.lock());
- if (delegate_sp)
- delegate_sp->PopulateSymtab(this, *m_symtab_up);
- // TODO: get symbols from delegate
- m_symtab_up->Finalize();
- }
- }
- return m_symtab_up.get();
+void ObjectFileJIT::ParseSymtab(Symtab &symtab) {
+ ObjectFileJITDelegateSP delegate_sp(m_delegate_wp.lock());
+ if (delegate_sp)
+ delegate_sp->PopulateSymtab(this, symtab);
}
bool ObjectFileJIT::IsStripped() {
diff --git a/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h b/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
index 03ac001988a01..be31139df549e 100644
--- a/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
+++ b/lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
@@ -67,7 +67,7 @@ class ObjectFileJIT : public lldb_private::ObjectFile {
uint32_t GetAddressByteSize() const override;
- lldb_private::Symtab *GetSymtab() override;
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
bool IsStripped() override;
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 6d6ad1a6cc002..573e763f1c0fd 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1311,22 +1311,6 @@ AddressClass ObjectFileMachO::GetAddressClass(lldb::addr_t file_addr) {
return AddressClass::eUnknown;
}
-Symtab *ObjectFileMachO::GetSymtab() {
- ModuleSP module_sp(GetModule());
- if (module_sp) {
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- if (m_symtab_up == nullptr) {
- ElapsedTime elapsed(module_sp->GetSymtabParseTime());
- m_symtab_up = std::make_unique<Symtab>(this);
- std::lock_guard<std::recursive_mutex> symtab_guard(
- m_symtab_up->GetMutex());
- ParseSymtab();
- m_symtab_up->Finalize();
- }
- }
- return m_symtab_up.get();
-}
-
bool ObjectFileMachO::IsStripped() {
if (m_dysymtab.cmd == 0) {
ModuleSP module_sp(GetModule());
@@ -2233,12 +2217,12 @@ ParseNList(DataExtractor &nlist_data, lldb::offset_t &nlist_data_offset,
enum { DebugSymbols = true, NonDebugSymbols = false };
-size_t ObjectFileMachO::ParseSymtab() {
+void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s",
m_file.GetFilename().AsCString(""));
ModuleSP module_sp(GetModule());
if (!module_sp)
- return 0;
+ return;
Progress progress(llvm::formatv("Parsing symbol table for {0}",
m_file.GetFilename().AsCString("<Unknown>")));
@@ -2288,7 +2272,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// Read in the rest of the symtab load command
if (m_data.GetU32(&offset, &symtab_load_command.symoff, 4) ==
nullptr) // fill in symoff, nsyms, stroff, strsize fields
- return 0;
+ return;
break;
case LC_DYLD_INFO:
@@ -2347,12 +2331,11 @@ size_t ObjectFileMachO::ParseSymtab() {
}
if (!symtab_load_command.cmd)
- return 0;
+ return;
- Symtab *symtab = m_symtab_up.get();
SectionList *section_list = GetSectionList();
if (section_list == nullptr)
- return 0;
+ return;
const uint32_t addr_byte_size = m_data.GetAddressByteSize();
const ByteOrder byte_order = m_data.GetByteOrder();
@@ -2489,7 +2472,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// We shouldn't have exports data from both the LC_DYLD_INFO command
// AND the LC_DYLD_EXPORTS_TRIE command in the same binary:
- lldbassert(!((dyld_info.export_size > 0)
+ lldbassert(!((dyld_info.export_size > 0)
&& (exports_trie_load_command.datasize > 0)));
if (dyld_info.export_size > 0) {
dyld_trie_data.SetData(m_data, dyld_info.export_off,
@@ -2881,10 +2864,10 @@ size_t ObjectFileMachO::ParseSymtab() {
// The normal nlist code cannot correctly size the Symbols
// array, we need to allocate it here.
- sym = symtab->Resize(
+ sym = symtab.Resize(
symtab_load_command.nsyms + m_dysymtab.nindirectsyms +
unmapped_local_symbols_found - m_dysymtab.nlocalsym);
- num_syms = symtab->GetNumSymbols();
+ num_syms = symtab.GetNumSymbols();
nlist_data_offset =
local_symbols_info.nlistOffset +
@@ -3016,7 +2999,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// original
// STAB entry so we don't have
// to hunt for it later
- symtab->SymbolAtIndex(N_FUN_indexes.back())
+ symtab.SymbolAtIndex(N_FUN_indexes.back())
->SetByteSize(nlist.n_value);
N_FUN_indexes.pop_back();
// We don't really need the end function STAB as
@@ -3096,7 +3079,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// index of this N_SO so that we can always skip
// the entire N_SO if we need to navigate more
// quickly at the source level when parsing STABS
- symbol_ptr = symtab->SymbolAtIndex(N_SO_index);
+ symbol_ptr = symtab.SymbolAtIndex(N_SO_index);
symbol_ptr->SetByteSize(sym_idx);
symbol_ptr->SetSizeIsSibling(true);
}
@@ -3203,7 +3186,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// quickly at the source level when parsing STABS
if (!N_INCL_indexes.empty()) {
symbol_ptr =
- symtab->SymbolAtIndex(N_INCL_indexes.back());
+ symtab.SymbolAtIndex(N_INCL_indexes.back());
symbol_ptr->SetByteSize(sym_idx + 1);
symbol_ptr->SetSizeIsSibling(true);
N_INCL_indexes.pop_back();
@@ -3268,7 +3251,7 @@ size_t ObjectFileMachO::ParseSymtab() {
nlist.n_value);
if (!N_BRAC_indexes.empty()) {
symbol_ptr =
- symtab->SymbolAtIndex(N_BRAC_indexes.back());
+ symtab.SymbolAtIndex(N_BRAC_indexes.back());
symbol_ptr->SetByteSize(sym_idx + 1);
symbol_ptr->SetSizeIsSibling(true);
N_BRAC_indexes.pop_back();
@@ -3306,7 +3289,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// parsing STABS
if (!N_COMM_indexes.empty()) {
symbol_ptr =
- symtab->SymbolAtIndex(N_COMM_indexes.back());
+ symtab.SymbolAtIndex(N_COMM_indexes.back());
symbol_ptr->SetByteSize(sym_idx + 1);
symbol_ptr->SetSizeIsSibling(true);
N_COMM_indexes.pop_back();
@@ -3806,8 +3789,8 @@ size_t ObjectFileMachO::ParseSymtab() {
// symbols, create it now.
if (sym == nullptr) {
sym =
- symtab->Resize(symtab_load_command.nsyms + m_dysymtab.nindirectsyms);
- num_syms = symtab->GetNumSymbols();
+ symtab.Resize(symtab_load_command.nsyms + m_dysymtab.nindirectsyms);
+ num_syms = symtab.GetNumSymbols();
}
if (unmapped_local_symbols_found) {
@@ -3941,7 +3924,7 @@ size_t ObjectFileMachO::ParseSymtab() {
if (!N_FUN_indexes.empty()) {
// Copy the size of the function into the original STAB entry
// so we don't have to hunt for it later
- symtab->SymbolAtIndex(N_FUN_indexes.back())
+ symtab.SymbolAtIndex(N_FUN_indexes.back())
->SetByteSize(nlist.n_value);
N_FUN_indexes.pop_back();
// We don't really need the end function STAB as it contains
@@ -4015,7 +3998,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// N_SO so that we can always skip the entire N_SO if we need
// to navigate more quickly at the source level when parsing
// STABS
- symbol_ptr = symtab->SymbolAtIndex(N_SO_index);
+ symbol_ptr = symtab.SymbolAtIndex(N_SO_index);
symbol_ptr->SetByteSize(sym_idx);
symbol_ptr->SetSizeIsSibling(true);
}
@@ -4109,7 +4092,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// N_EINCL so that we can always skip the entire symbol if we need
// to navigate more quickly at the source level when parsing STABS
if (!N_INCL_indexes.empty()) {
- symbol_ptr = symtab->SymbolAtIndex(N_INCL_indexes.back());
+ symbol_ptr = symtab.SymbolAtIndex(N_INCL_indexes.back());
symbol_ptr->SetByteSize(sym_idx + 1);
symbol_ptr->SetSizeIsSibling(true);
N_INCL_indexes.pop_back();
@@ -4168,7 +4151,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// quickly at the source level when parsing STABS
symbol_section = section_info.GetSection(nlist.n_sect, nlist.n_value);
if (!N_BRAC_indexes.empty()) {
- symbol_ptr = symtab->SymbolAtIndex(N_BRAC_indexes.back());
+ symbol_ptr = symtab.SymbolAtIndex(N_BRAC_indexes.back());
symbol_ptr->SetByteSize(sym_idx + 1);
symbol_ptr->SetSizeIsSibling(true);
N_BRAC_indexes.pop_back();
@@ -4202,7 +4185,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// we need to navigate more quickly at the source level when
// parsing STABS
if (!N_COMM_indexes.empty()) {
- symbol_ptr = symtab->SymbolAtIndex(N_COMM_indexes.back());
+ symbol_ptr = symtab.SymbolAtIndex(N_COMM_indexes.back());
symbol_ptr->SetByteSize(sym_idx + 1);
symbol_ptr->SetSizeIsSibling(true);
N_COMM_indexes.pop_back();
@@ -4658,7 +4641,7 @@ size_t ObjectFileMachO::ParseSymtab() {
if (num_syms < sym_idx + trie_symbol_table_augment_count) {
num_syms = sym_idx + trie_symbol_table_augment_count;
- sym = symtab->Resize(num_syms);
+ sym = symtab.Resize(num_syms);
}
uint32_t synthetic_sym_id = symtab_load_command.nsyms;
@@ -4707,7 +4690,7 @@ size_t ObjectFileMachO::ParseSymtab() {
if (num_synthetic_function_symbols > 0) {
if (num_syms < sym_idx + num_synthetic_function_symbols) {
num_syms = sym_idx + num_synthetic_function_symbols;
- sym = symtab->Resize(num_syms);
+ sym = symtab.Resize(num_syms);
}
for (i = 0; i < function_starts_count; ++i) {
const FunctionStarts::Entry *func_start_entry =
@@ -4762,7 +4745,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// symbols.
if (sym_idx < num_syms) {
num_syms = sym_idx;
- sym = symtab->Resize(num_syms);
+ sym = symtab.Resize(num_syms);
}
// Now synthesize indirect symbols
@@ -4807,11 +4790,11 @@ size_t ObjectFileMachO::ParseSymtab() {
if (index_pos != end_index_pos) {
// We have a remapping from the original nlist index to a
// current symbol index, so just look this up by index
- stub_symbol = symtab->SymbolAtIndex(index_pos->second);
+ stub_symbol = symtab.SymbolAtIndex(index_pos->second);
} else {
// We need to lookup a symbol using the original nlist symbol
// index since this index is coming from the S_SYMBOL_STUBS
- stub_symbol = symtab->FindSymbolByID(stub_sym_id);
+ stub_symbol = symtab.FindSymbolByID(stub_sym_id);
}
if (stub_symbol) {
@@ -4834,7 +4817,7 @@ size_t ObjectFileMachO::ParseSymtab() {
// Make a synthetic symbol to describe the trampoline stub
Mangled stub_symbol_mangled_name(stub_symbol->GetMangled());
if (sym_idx >= num_syms) {
- sym = symtab->Resize(++num_syms);
+ sym = symtab.Resize(++num_syms);
stub_symbol = nullptr; // this pointer no longer valid
}
sym[sym_idx].SetID(synthetic_sym_id++);
@@ -4873,7 +4856,7 @@ size_t ObjectFileMachO::ParseSymtab() {
indirect_symbol_names.end()) {
// Make a synthetic symbol to describe re-exported symbol.
if (sym_idx >= num_syms)
- sym = symtab->Resize(++num_syms);
+ sym = symtab.Resize(++num_syms);
sym[sym_idx].SetID(synthetic_sym_id++);
sym[sym_idx].GetMangled() = Mangled(e.entry.name);
sym[sym_idx].SetType(eSymbolTypeReExported);
@@ -4888,18 +4871,6 @@ size_t ObjectFileMachO::ParseSymtab() {
}
}
}
-
- // StreamFile s(stdout, false);
- // s.Printf ("Symbol table before CalculateSymbolSizes():\n");
- // symtab->Dump(&s, NULL, eSortOrderNone);
- // Set symbol byte sizes correctly since mach-o nlist entries don't have
- // sizes
- symtab->CalculateSymbolSizes();
-
- // s.Printf ("Symbol table after CalculateSymbolSizes():\n");
- // symtab->Dump(&s, NULL, eSortOrderNone);
-
- return symtab->GetNumSymbols();
}
void ObjectFileMachO::Dump(Stream *s) {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
index 29f07b0546a71..b3a941a957222 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -92,7 +92,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
lldb_private::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
- lldb_private::Symtab *GetSymtab() override;
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
bool IsStripped() override;
diff --git a/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h b/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
index 36e71e21332fb..da999d2b55a75 100644
--- a/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
+++ b/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h
@@ -68,7 +68,7 @@ class ObjectFilePDB : public ObjectFile {
bool IsExecutable() const override { return false; }
- Symtab *GetSymtab() override { return nullptr; }
+ void ParseSymtab(lldb_private::Symtab &symtab) override {}
bool IsStripped() override { return false; }
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 5c7489e35049b..f69f3c300ae78 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -589,139 +589,125 @@ llvm::StringRef ObjectFilePECOFF::GetSectionName(const section_header_t §) {
return hdr_name;
}
-// GetNListSymtab
-Symtab *ObjectFilePECOFF::GetSymtab() {
- ModuleSP module_sp(GetModule());
- if (module_sp) {
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- if (m_symtab_up == nullptr) {
- ElapsedTime elapsed(module_sp->GetSymtabParseTime());
- SectionList *sect_list = GetSectionList();
- m_symtab_up = std::make_unique<Symtab>(this);
- std::lock_guard<std::recursive_mutex> guard(m_symtab_up->GetMutex());
-
- const uint32_t num_syms = m_coff_header.nsyms;
-
- if (m_file && num_syms > 0 && m_coff_header.symoff > 0) {
- const uint32_t symbol_size = 18;
- const size_t symbol_data_size = num_syms * symbol_size;
- // Include the 4-byte string table size at the end of the symbols
- DataExtractor symtab_data =
- ReadImageData(m_coff_header.symoff, symbol_data_size + 4);
- lldb::offset_t offset = symbol_data_size;
- const uint32_t strtab_size = symtab_data.GetU32(&offset);
- if (strtab_size > 0) {
- DataExtractor strtab_data = ReadImageData(
- m_coff_header.symoff + symbol_data_size, strtab_size);
-
- offset = 0;
- std::string symbol_name;
- Symbol *symbols = m_symtab_up->Resize(num_syms);
- for (uint32_t i = 0; i < num_syms; ++i) {
- coff_symbol_t symbol;
- const uint32_t symbol_offset = offset;
- const char *symbol_name_cstr = nullptr;
- // If the first 4 bytes of the symbol string are zero, then they
- // are followed by a 4-byte string table offset. Else these
- // 8 bytes contain the symbol name
- if (symtab_data.GetU32(&offset) == 0) {
- // Long string that doesn't fit into the symbol table name, so
- // now we must read the 4 byte string table offset
- uint32_t strtab_offset = symtab_data.GetU32(&offset);
- symbol_name_cstr = strtab_data.PeekCStr(strtab_offset);
- symbol_name.assign(symbol_name_cstr);
- } else {
- // Short string that fits into the symbol table name which is 8
- // bytes
- offset += sizeof(symbol.name) - 4; // Skip remaining
- symbol_name_cstr = symtab_data.PeekCStr(symbol_offset);
- if (symbol_name_cstr == nullptr)
- break;
- symbol_name.assign(symbol_name_cstr, sizeof(symbol.name));
- }
- symbol.value = symtab_data.GetU32(&offset);
- symbol.sect = symtab_data.GetU16(&offset);
- symbol.type = symtab_data.GetU16(&offset);
- symbol.storage = symtab_data.GetU8(&offset);
- symbol.naux = symtab_data.GetU8(&offset);
- symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
- if ((int16_t)symbol.sect >= 1) {
- Address symbol_addr(sect_list->FindSectionByID(symbol.sect),
- symbol.value);
- symbols[i].GetAddressRef() = symbol_addr;
- symbols[i].SetType(MapSymbolType(symbol.type));
- }
-
- if (symbol.naux > 0) {
- i += symbol.naux;
- offset += symbol.naux * symbol_size;
- }
- }
+void ObjectFilePECOFF::ParseSymtab(Symtab &symtab) {
+ SectionList *sect_list = GetSectionList();
+ const uint32_t num_syms = m_coff_header.nsyms;
+ if (m_file && num_syms > 0 && m_coff_header.symoff > 0) {
+ const uint32_t symbol_size = 18;
+ const size_t symbol_data_size = num_syms * symbol_size;
+ // Include the 4-byte string table size at the end of the symbols
+ DataExtractor symtab_data =
+ ReadImageData(m_coff_header.symoff, symbol_data_size + 4);
+ lldb::offset_t offset = symbol_data_size;
+ const uint32_t strtab_size = symtab_data.GetU32(&offset);
+ if (strtab_size > 0) {
+ DataExtractor strtab_data = ReadImageData(
+ m_coff_header.symoff + symbol_data_size, strtab_size);
+
+ offset = 0;
+ std::string symbol_name;
+ Symbol *symbols = symtab.Resize(num_syms);
+ for (uint32_t i = 0; i < num_syms; ++i) {
+ coff_symbol_t symbol;
+ const uint32_t symbol_offset = offset;
+ const char *symbol_name_cstr = nullptr;
+ // If the first 4 bytes of the symbol string are zero, then they
+ // are followed by a 4-byte string table offset. Else these
+ // 8 bytes contain the symbol name
+ if (symtab_data.GetU32(&offset) == 0) {
+ // Long string that doesn't fit into the symbol table name, so
+ // now we must read the 4 byte string table offset
+ uint32_t strtab_offset = symtab_data.GetU32(&offset);
+ symbol_name_cstr = strtab_data.PeekCStr(strtab_offset);
+ symbol_name.assign(symbol_name_cstr);
+ } else {
+ // Short string that fits into the symbol table name which is 8
+ // bytes
+ offset += sizeof(symbol.name) - 4; // Skip remaining
+ symbol_name_cstr = symtab_data.PeekCStr(symbol_offset);
+ if (symbol_name_cstr == nullptr)
+ break;
+ symbol_name.assign(symbol_name_cstr, sizeof(symbol.name));
+ }
+ symbol.value = symtab_data.GetU32(&offset);
+ symbol.sect = symtab_data.GetU16(&offset);
+ symbol.type = symtab_data.GetU16(&offset);
+ symbol.storage = symtab_data.GetU8(&offset);
+ symbol.naux = symtab_data.GetU8(&offset);
+ symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
+ if ((int16_t)symbol.sect >= 1) {
+ Address symbol_addr(sect_list->FindSectionByID(symbol.sect),
+ symbol.value);
+ symbols[i].GetAddressRef() = symbol_addr;
+ symbols[i].SetType(MapSymbolType(symbol.type));
+ }
+
+ if (symbol.naux > 0) {
+ i += symbol.naux;
+ offset += symbol.naux * symbol_size;
}
}
+ }
+ }
- // Read export header
- if (coff_data_dir_export_table < m_coff_header_opt.data_dirs.size() &&
- m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmsize > 0 &&
- m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmaddr > 0) {
- export_directory_entry export_table;
- uint32_t data_start =
- m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmaddr;
-
- DataExtractor symtab_data = ReadImageDataByRVA(
- data_start, m_coff_header_opt.data_dirs[0].vmsize);
- lldb::offset_t offset = 0;
-
- // Read export_table header
- export_table.characteristics = symtab_data.GetU32(&offset);
- export_table.time_date_stamp = symtab_data.GetU32(&offset);
- export_table.major_version = symtab_data.GetU16(&offset);
- export_table.minor_version = symtab_data.GetU16(&offset);
- export_table.name = symtab_data.GetU32(&offset);
- export_table.base = symtab_data.GetU32(&offset);
- export_table.number_of_functions = symtab_data.GetU32(&offset);
- export_table.number_of_names = symtab_data.GetU32(&offset);
- export_table.address_of_functions = symtab_data.GetU32(&offset);
- export_table.address_of_names = symtab_data.GetU32(&offset);
- export_table.address_of_name_ordinals = symtab_data.GetU32(&offset);
-
- bool has_ordinal = export_table.address_of_name_ordinals != 0;
-
- lldb::offset_t name_offset = export_table.address_of_names - data_start;
- lldb::offset_t name_ordinal_offset =
- export_table.address_of_name_ordinals - data_start;
-
- Symbol *symbols = m_symtab_up->Resize(export_table.number_of_names);
-
- std::string symbol_name;
-
- // Read each export table entry
- for (size_t i = 0; i < export_table.number_of_names; ++i) {
- uint32_t name_ordinal =
- has_ordinal ? symtab_data.GetU16(&name_ordinal_offset) : i;
- uint32_t name_address = symtab_data.GetU32(&name_offset);
-
- const char *symbol_name_cstr =
- symtab_data.PeekCStr(name_address - data_start);
- symbol_name.assign(symbol_name_cstr);
+ // Read export header
+ if (coff_data_dir_export_table < m_coff_header_opt.data_dirs.size() &&
+ m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmsize > 0 &&
+ m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmaddr > 0) {
+ export_directory_entry export_table;
+ uint32_t data_start =
+ m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmaddr;
- lldb::offset_t function_offset = export_table.address_of_functions -
- data_start +
- sizeof(uint32_t) * name_ordinal;
- uint32_t function_rva = symtab_data.GetU32(&function_offset);
+ DataExtractor symtab_data = ReadImageDataByRVA(
+ data_start, m_coff_header_opt.data_dirs[0].vmsize);
+ lldb::offset_t offset = 0;
- Address symbol_addr(m_coff_header_opt.image_base + function_rva,
- sect_list);
- symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
- symbols[i].GetAddressRef() = symbol_addr;
- symbols[i].SetType(lldb::eSymbolTypeCode);
- symbols[i].SetDebug(true);
- }
- }
- m_symtab_up->CalculateSymbolSizes();
+ // Read export_table header
+ export_table.characteristics = symtab_data.GetU32(&offset);
+ export_table.time_date_stamp = symtab_data.GetU32(&offset);
+ export_table.major_version = symtab_data.GetU16(&offset);
+ export_table.minor_version = symtab_data.GetU16(&offset);
+ export_table.name = symtab_data.GetU32(&offset);
+ export_table.base = symtab_data.GetU32(&offset);
+ export_table.number_of_functions = symtab_data.GetU32(&offset);
+ export_table.number_of_names = symtab_data.GetU32(&offset);
+ export_table.address_of_functions = symtab_data.GetU32(&offset);
+ export_table.address_of_names = symtab_data.GetU32(&offset);
+ export_table.address_of_name_ordinals = symtab_data.GetU32(&offset);
+
+ bool has_ordinal = export_table.address_of_name_ordinals != 0;
+
+ lldb::offset_t name_offset = export_table.address_of_names - data_start;
+ lldb::offset_t name_ordinal_offset =
+ export_table.address_of_name_ordinals - data_start;
+
+ Symbol *symbols = symtab.Resize(export_table.number_of_names);
+
+ std::string symbol_name;
+
+ // Read each export table entry
+ for (size_t i = 0; i < export_table.number_of_names; ++i) {
+ uint32_t name_ordinal =
+ has_ordinal ? symtab_data.GetU16(&name_ordinal_offset) : i;
+ uint32_t name_address = symtab_data.GetU32(&name_offset);
+
+ const char *symbol_name_cstr =
+ symtab_data.PeekCStr(name_address - data_start);
+ symbol_name.assign(symbol_name_cstr);
+
+ lldb::offset_t function_offset = export_table.address_of_functions -
+ data_start +
+ sizeof(uint32_t) * name_ordinal;
+ uint32_t function_rva = symtab_data.GetU32(&function_offset);
+
+ Address symbol_addr(m_coff_header_opt.image_base + function_rva,
+ sect_list);
+ symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
+ symbols[i].GetAddressRef() = symbol_addr;
+ symbols[i].SetType(lldb::eSymbolTypeCode);
+ symbols[i].SetDebug(true);
}
}
- return m_symtab_up.get();
}
std::unique_ptr<CallFrameInfo> ObjectFilePECOFF::CreateCallFrameInfo() {
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index 876b83264f6fb..b7de24d1d418c 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -107,7 +107,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
// virtual lldb_private::AddressClass
// GetAddressClass (lldb::addr_t file_addr);
- lldb_private::Symtab *GetSymtab() override;
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
bool IsStripped() override;
diff --git a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
index 0e6329885528b..7445f8311c500 100644
--- a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -246,7 +246,7 @@ bool ObjectFileWasm::ParseHeader() {
return true;
}
-Symtab *ObjectFileWasm::GetSymtab() { return nullptr; }
+void ObjectFileWasm::ParseSymtab(Symtab &symtab) {}
static SectionType GetSectionTypeFromName(llvm::StringRef Name) {
if (Name.consume_front(".debug_") || Name.consume_front(".zdebug_")) {
diff --git a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
index 44939b6d4ea0e..d7b5bc22caadb 100644
--- a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -78,7 +78,7 @@ class ObjectFileWasm : public ObjectFile {
return AddressClass::eInvalid;
}
- Symtab *GetSymtab() override;
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
bool IsStripped() override { return !!GetExternalDebugInfoFileSpec(); }
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 08cf58b0a84a1..37ee5466c5b98 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -73,7 +73,7 @@ class PlaceholderObjectFile : public ObjectFile {
bool IsExecutable() const override { return false; }
ArchSpec GetArchitecture() override { return m_arch; }
UUID GetUUID() override { return m_uuid; }
- Symtab *GetSymtab() override { return m_symtab_up.get(); }
+ void ParseSymtab(lldb_private::Symtab &symtab) override {}
bool IsStripped() override { return true; }
ByteOrder GetByteOrder() const override { return m_arch.GetByteOrder(); }
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index b07674af3bd9f..9d23f1baf9315 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -500,7 +500,7 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) {
for (Symbol &symbol : symbols)
symtab.AddSymbol(std::move(symbol));
- symtab.CalculateSymbolSizes();
+ symtab.Finalize();
}
llvm::Expected<lldb::addr_t>
@@ -927,4 +927,3 @@ uint64_t SymbolFileBreakpad::GetDebugInfoSize() {
// Breakpad files are all debug info.
return m_objfile_sp->GetByteSize();
}
-
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 0a75310c3bde5..8c20244a6c446 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2067,6 +2067,13 @@ uint32_t SymbolFileDWARF::ResolveSymbolContext(
}
void SymbolFileDWARF::PreloadSymbols() {
+ // Get the symbol table for the symbol file prior to taking the module lock
+ // so that it is available without needing to take the module lock. The DWARF
+ // indexing might end up needing to relocate items when DWARF sections are
+ // loaded as they might end up getting the section contents which can call
+ // ObjectFileELF::RelocateSection() which in turn will ask for the symbol
+ // table and can cause deadlocks.
+ GetSymtab();
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
m_index->Preload();
}
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 45dfc4b9a1525..db0ae241be7ec 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1421,7 +1421,6 @@ void SymbolFilePDB::AddSymbols(lldb_private::Symtab &symtab) {
));
}
- symtab.CalculateSymbolSizes();
symtab.Finalize();
}
diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp
index 101af01341a20..bfab741b0d667 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -244,7 +244,7 @@ ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
m_type(eTypeInvalid), m_strata(eStrataInvalid),
m_file_offset(file_offset), m_length(length), m_data(), m_process_wp(),
m_memory_addr(LLDB_INVALID_ADDRESS), m_sections_up(), m_symtab_up(),
- m_synthetic_symbol_idx(0) {
+ m_symtab_once_up(new llvm::once_flag()) {
if (file_spec_ptr)
m_file = *file_spec_ptr;
if (data_sp)
@@ -265,7 +265,7 @@ ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
: ModuleChild(module_sp), m_file(), m_type(eTypeInvalid),
m_strata(eStrataInvalid), m_file_offset(0), m_length(0), m_data(),
m_process_wp(process_sp), m_memory_addr(header_addr), m_sections_up(),
- m_symtab_up(), m_synthetic_symbol_idx(0) {
+ m_symtab_up(), m_symtab_once_up(new llvm::once_flag()) {
if (header_data_sp)
m_data.SetData(header_data_sp, 0, header_data_sp->GetByteSize());
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
@@ -571,11 +571,13 @@ bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
void ObjectFile::ClearSymtab() {
ModuleSP module_sp(GetModule());
if (module_sp) {
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
LLDB_LOGF(log, "%p ObjectFile::ClearSymtab () symtab = %p",
static_cast<void *>(this),
static_cast<void *>(m_symtab_up.get()));
+ // Since we need to clear the symbol table, we need a new llvm::once_flag
+ // instance so we can safely create another symbol table
+ m_symtab_once_up.reset(new llvm::once_flag());
m_symtab_up.reset();
}
}
@@ -715,3 +717,33 @@ void llvm::format_provider<ObjectFile::Strata>::format(
break;
}
}
+
+
+Symtab *ObjectFile::GetSymtab() {
+ ModuleSP module_sp(GetModule());
+ if (module_sp) {
+ // We can't take the module lock in ObjectFile::GetSymtab() or we can
+ // deadlock in DWARF indexing when any file asks for the symbol table from
+ // an object file. This currently happens in the preloading of symbols in
+ // SymbolFileDWARF::PreloadSymbols() because the main thread will take the
+ // module lock, and then threads will be spun up to index the DWARF and
+ // any of those threads might end up trying to relocate items in the DWARF
+ // sections which causes ObjectFile::GetSectionData(...) to relocate section
+ // data which requires the symbol table.
+ //
+ // So to work around this, we create the symbol table one time using
+ // llvm::once_flag, lock it, and then set the unique pointer. Any other
+ // thread that gets ahold of the symbol table before parsing is done, will
+ // not be able to access the symbol table contents since all APIs in Symtab
+ // are protected by a mutex in the Symtab object itself.
+ llvm::call_once(*m_symtab_once_up, [&]() {
+ ElapsedTime elapsed(module_sp->GetSymtabParseTime());
+ Symtab *symtab = new Symtab(this);
+ std::lock_guard<std::recursive_mutex> symtab_guard(symtab->GetMutex());
+ m_symtab_up.reset(symtab);
+ ParseSymtab(*m_symtab_up);
+ m_symtab_up->Finalize();
+ });
+ }
+ return m_symtab_up.get();
+}
diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 19c1fee2bb381..c67955523bfb1 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -997,10 +997,15 @@ void Symtab::InitAddressIndexes() {
}
}
-void Symtab::CalculateSymbolSizes() {
+void Symtab::Finalize() {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
- // Size computation happens inside InitAddressIndexes.
+ // Calculate the size of symbols inside InitAddressIndexes.
InitAddressIndexes();
+ // Shrink to fit the symbols so we don't waste memory
+ if (m_symbols.capacity() > m_symbols.size()) {
+ collection new_symbols(m_symbols.begin(), m_symbols.end());
+ m_symbols.swap(new_symbols);
+ }
}
Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr) {
More information about the lldb-commits
mailing list