Revert "[NFC] Refactor symbol table parsing."

This reverts commit 951b107eedab1829f18049443f03339dbb0db165.

Buildbots were failing, there is a deadlock in /Users/gclayton/Documents/src/llvm/clean/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s when ELF files try to relocate things.

1 files changed, 134 insertions, 111 deletions

diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index ec57e0c1f54e..8e0f228a988f 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2687,132 +2687,155 @@ unsigned ObjectFileELF::RelocateDebugSections(const ELFSectionHeader *rel_hdr,
   return 0;
 }
 
-void ObjectFileELF::ParseSymtab(Symtab &lldb_symtab) {
+Symtab *ObjectFileELF::GetSymtab() {
   ModuleSP module_sp(GetModule());
   if (!module_sp)
-    return;
-
-  Progress progress(
-      llvm::formatv("Parsing symbol table for {0}",
-                    m_file.GetFilename().AsCString("<Unknown>")));
-  ElapsedTime elapsed(module_sp->GetSymtabParseTime());
+    return nullptr;
 
   // 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->ParseSymtab(lldb_symtab);
+    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;
 
-  SectionList *section_list = module_sp->GetSectionList();
-  if (!section_list)
-    return;
+    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);
+    }
 
-  uint64_t symbol_id = 0;
-  std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
+    // 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);
+      }
+    }
 
-  // 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);
+    // 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);
+        }
+      }
+    }
+
+    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);
     }
-  }
 
-  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;
+    // 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);
       }
-      lldb_symtab.AddSymbol(symbol);
     }
+
+    m_symtab_up->CalculateSymbolSizes();
   }
+
+  return m_symtab_up.get();
 }
 
 void ObjectFileELF::RelocateSection(lldb_private::Section *section)