[NFC] Refactor symbol table parsing.

[clayborg]

Summary

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.

[labath]

This is a good cleanup independently of the caching feature.

lldb/include/lldb/Symbol/ObjectFile.h

344

I guess this should be protected, as people shouldn't be calling it directly.

[JDevlieghere]

LGTM

[clayborg]

I had to revert this. There is a deadlock in:

void Module::PreloadSymbols() {
  std::lock_guard<std::recursive_mutex> guard(m_mutex);
  SymbolFile *sym_file = GetSymbolFile();
  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.
  if (Symtab *symtab = sym_file->GetSymtab())
    symtab->PreloadSymbols();
}

That happens only when we are using ELF files since they need to be manually indexed. What happens is:

• Thread #1 calls "sym_file->PreloadSymbols();"
• Thread #2 starts manually indexing the DWARF and tries to load section data for a DWARF section which has relocations and since Module::PreloadSymbols() from thread #1 has the lock.

The backtrace can be seen when debugging:

$ /Users/gclayton/Documents/src/llvm/clean/Debug/bin/lldb --no-lldbinit -S /Users/gclayton/Documents/src/llvm/clean/Debug/tools/lldb/test/Shell/lit-lldb-init debug_rnglists-dwo.o -o image lookup -v -s lookup_rnglists -o exit

This deadlocks with:

(lldb) bt all
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x00000001850f4548 libsystem_kernel.dylib`__psynch_cvwait + 8
    frame #1: 0x000000018512bdac libsystem_pthread.dylib`_pthread_cond_wait + 1248
    frame #2: 0x0000000185085efc libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 28
    frame #3: 0x0000000104ee97a8 LLDB`void std::__1::condition_variable::wait<llvm::ThreadPool::wait()::$_1>(this=0x000000016d3b03f0, __lk=0x000000016d3aff58, __pred=(anonymous class) @ 0x000000016d3aff28)::$_1) at __mutex_base:409:9
    frame #4: 0x0000000104ee9728 LLDB`llvm::ThreadPool::wait(this=0x000000016d3b0338) at ThreadPool.cpp:72:23
    frame #5: 0x0000000104b97d54 LLDB`lldb_private::ManualDWARFIndex::Index(this=0x00000001302129b0) at ManualDWARFIndex.cpp:109:8
    frame #6: 0x0000000104b0ef98 LLDB`lldb_private::ManualDWARFIndex::Preload(this=0x00000001302129b0) at ManualDWARFIndex.h:27:29
    frame #7: 0x0000000104bb5f1c LLDB`SymbolFileDWARF::PreloadSymbols(this=0x0000000130815400) at SymbolFileDWARF.cpp:2071:12
    frame #8: 0x0000000103c60dcc LLDB`lldb_private::Module::PreloadSymbols(this=0x000000013020d238) at Module.cpp:1383:13
    frame #9: 0x0000000104266f44 LLDB`lldb_private::TargetList::CreateTargetInternal(debugger=0x000000013302d200, user_exe_path=(Data = "debug_rnglists-dwo.o", Length = 20), specified_arch=0x000000016d3b1478, load_dependent_files=eLoadDependentsDefault, platform_sp=std::__1::shared_ptr<lldb_private::Platform>::element_type @ 0x0000000130705b70 strong=7 weak=1, target_sp=std::__1::shared_ptr<lldb_private::Target>::element_type @ 0x0000000130811600 strong=1 weak=2) at TargetList.cpp:371:24
    frame #10: 0x0000000104266648 LLDB`lldb_private::TargetList::CreateTargetInternal(debugger=0x000000013302d200, user_exe_path=(Data = "debug_rnglists-dwo.o", Length = 20), triple_str=(Data = "", Length = 0), load_dependent_files=eLoadDependentsDefault, platform_options=0x0000000133044dc0, target_sp=std::__1::shared_ptr<lldb_private::Target>::element_type @ 0x0000000130811600 strong=1 weak=2) at TargetList.cpp:280:10
    frame #11: 0x0000000104265ae0 LLDB`lldb_private::TargetList::CreateTarget(this=0x000000013302d2d0, debugger=0x000000013302d200, user_exe_path=(Data = "debug_rnglists-dwo.o", Length = 20), triple_str=(Data = "", Length = 0), load_dependent_files=eLoadDependentsDefault, platform_options=0x0000000133044dc0, target_sp=std::__1::shared_ptr<lldb_private::Target>::element_type @ 0x0000000130811600 strong=1 weak=2) at TargetList.cpp:52:17
    frame #12: 0x0000000107d2d2a8 LLDB`CommandObjectTargetCreate::DoExecute(this=0x0000000133044c00, command=0x000000016d3b22a0, result=0x000000016d3b28c0) at CommandObjectTarget.cpp:314:45
    frame #13: 0x0000000103efc228 LLDB`lldb_private::CommandObjectParsed::Execute(this=0x0000000133044c00, args_string="\"debug_rnglists-dwo.o\"", result=0x000000016d3b28c0) at CommandObject.cpp:995:19
    frame #14: 0x0000000103ebd234 LLDB`lldb_private::CommandInterpreter::HandleCommand(this=0x0000000132834c90, command_line="target create \"debug_rnglists-dwo.o\"", lazy_add_to_history=eLazyBoolCalculate, result=0x000000016d3b28c0) at CommandInterpreter.cpp:1971:14
    frame #15: 0x0000000103ec17fc LLDB`lldb_private::CommandInterpreter::IOHandlerInputComplete(this=0x0000000132834c90, io_handler=0x0000000130508c18, line="target create \"debug_rnglists-dwo.o\"") at CommandInterpreter.cpp:3021:3
    frame #16: 0x0000000103bffcd8 LLDB`lldb_private::IOHandlerEditline::Run(this=0x0000000130508c18) at IOHandler.cpp:576:22
    frame #17: 0x0000000103b8fbb8 LLDB`lldb_private::Debugger::RunIOHandlers(this=0x000000013302d200) at Debugger.cpp:877:16
    frame #18: 0x0000000103ec2eac LLDB`lldb_private::CommandInterpreter::RunCommandInterpreter(this=0x0000000132834c90, options=0x00000001305085a0) at CommandInterpreter.cpp:3267:16
    frame #19: 0x000000010349d85c LLDB`lldb::SBDebugger::RunCommandInterpreter(this=0x000000016d3b31e0, options=0x000000016d3b2ec0) at SBDebugger.cpp:1267:14
    frame #20: 0x0000000102a50da0 lldb`Driver::MainLoop(this=0x000000016d3b31c0) at Driver.cpp:635:20
    frame #21: 0x0000000102a51a60 lldb`main(argc=9, argv=0x000000016d3b3720) at Driver.cpp:965:26
    frame #22: 0x0000000185149430 libdyld.dylib`start + 4
  thread #2, name = 'lldb.debugger.event-handler'
    frame #0: 0x00000001850f4548 libsystem_kernel.dylib`__psynch_cvwait + 8
    frame #1: 0x000000018512bdac libsystem_pthread.dylib`_pthread_cond_wait + 1248
    frame #2: 0x0000000185085efc libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 28
    frame #3: 0x000000010435286c LLDB`lldb_private::Listener::GetEventInternal(this=0x0000000132834b50, timeout=0x000000016dbbae58, broadcaster=0x0000000000000000, broadcaster_names=0x0000000000000000, num_broadcaster_names=0, event_type_mask=0, event_sp=nullptr) at Listener.cpp:364:28
    frame #4: 0x0000000104352cd0 LLDB`lldb_private::Listener::GetEvent(this=0x0000000132834b50, event_sp=nullptr, timeout=0x000000016dbbae58) at Listener.cpp:399:10
    frame #5: 0x0000000103b930a0 LLDB`lldb_private::Debugger::DefaultEventHandler(this=0x000000013302d200) at Debugger.cpp:1510:22
    frame #6: 0x0000000103b933cc LLDB`lldb_private::Debugger::EventHandlerThread(arg=0x000000013302d200) at Debugger.cpp:1563:22
    frame #7: 0x0000000103e3060c LLDB`lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(arg=0x0000000130508780) at HostNativeThreadBase.cpp:65:10
    frame #8: 0x00000001081c90f0 LLDB`lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(arg=0x0000000130508780) at HostThreadMacOSX.mm:67:10
    frame #9: 0x000000018512b878 libsystem_pthread.dylib`_pthread_start + 320
  thread #3
    frame #0: 0x00000001850f3a48 libsystem_kernel.dylib`__psynch_mutexwait + 8
    frame #1: 0x0000000185128918 libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_wait + 88
    frame #2: 0x0000000185126244 libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_slow + 232
    frame #3: 0x00000001850b8c88 libc++.1.dylib`std::__1::recursive_mutex::lock() + 16
    frame #4: 0x000000010401a354 LLDB`std::__1::lock_guard<std::__1::recursive_mutex>::lock_guard(this=0x000000016e3c6080, __m=0x000000013020d250) at __mutex_base:91:27
    frame #5: 0x0000000104017a40 LLDB`std::__1::lock_guard<std::__1::recursive_mutex>::lock_guard(this=0x000000016e3c6080, __m=0x000000013020d250) at __mutex_base:91:21
    frame #6: 0x0000000104016ef8 LLDB`lldb_private::ObjectFile::GetSymtab(this=0x0000000130706470) at ObjectFile.cpp:735:43
    frame #7: 0x0000000104771f1c LLDB`ObjectFileELF::RelocateSection(this=0x0000000130706470, section=0x0000000130706e10) at ObjectFileELF.cpp:2852:56
    frame #8: 0x000000010401771c LLDB`lldb_private::ObjectFile::ReadSectionData(this=0x0000000130706470, section=0x0000000130706e10, section_data=0x000000016e3c6538) at ObjectFile.cpp:527:5
    frame #9: 0x0000000104773eb8 LLDB`ObjectFileELF::ReadSectionData(this=0x0000000130706470, section=0x0000000130706e10, section_data=0x000000016e3c6538) at ObjectFileELF.cpp:3317:31
    frame #10: 0x0000000103cd0210 LLDB`lldb_private::Section::GetSectionData(this=0x0000000130706e10, section_data=0x000000016e3c6538) at Section.cpp:395:24
    frame #11: 0x0000000104b4a648 LLDB`LoadSection(section_list=0x0000000130706200, section_type=eSectionTypeDWARFDebugInfoDwo) at DWARFContext.cpp:26:15
    frame #12: 0x0000000104b4a4f0 LLDB`lldb_private::DWARFContext::LoadOrGetSection(this=0x000000016e3c67a0)::$_0::operator()() const at DWARFContext.cpp:36:19
    frame #13: 0x0000000104b4a444 LLDB`decltype(__f=0x000000016e3c67a0)::$_0>(fp)()) std::__1::__invoke<lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0>(lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0&&) at type_traits:3747:1
    frame #14: 0x0000000104b4a41c LLDB`void std::__1::__call_once_param<std::__1::tuple<lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0&&> >::__execute<>(this=0x000000016e3c6740, (null)=__tuple_indices<> @ 0x000000016e3c669f) at mutex:629:9
    frame #15: 0x0000000104b4a3ec LLDB`std::__1::__call_once_param<std::__1::tuple<lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0&&> >::operator(this=0x000000016e3c6740)() at mutex:621:9
    frame #16: 0x0000000104b4a2b8 LLDB`void std::__1::__call_once_proxy<std::__1::tuple<lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0&&> >(__vp=0x000000016e3c6740) at mutex:657:5
    frame #17: 0x00000001850b92b0 libc++.1.dylib`std::__1::__call_once(unsigned long volatile&, void*, void (*)(void*)) + 180
    frame #18: 0x0000000104b4a1f0 LLDB`void std::__1::call_once<lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0>(__flag=0x000000013200d1d0, __func=0x000000016e3c67a0)::$_0&&) at mutex:675:9
    frame #19: 0x0000000104b472b8 LLDB`void llvm::call_once<lldb_private::DWARFContext::LoadOrGetSection(llvm::Optional<lldb::SectionType>, llvm::Optional<lldb::SectionType>, lldb_private::DWARFContext::SectionData&)::$_0>(flag=0x000000013200d1d0, F=0x000000016e3c67a0)::$_0&&) at Threading.h:92:5
    frame #20: 0x0000000104b47274 LLDB`lldb_private::DWARFContext::LoadOrGetSection(this=0x000000013200d0b8, main_section_type=Optional<lldb::SectionType> @ 0x000000016e3c67d8, dwo_section_type=Optional<lldb::SectionType> @ 0x000000016e3c67d0, data=0x000000013200d1d0) at DWARFContext.cpp:34:3
    frame #21: 0x0000000104b47558 LLDB`lldb_private::DWARFContext::getOrLoadDebugInfoData(this=0x000000013200d0b8) at DWARFContext.cpp:69:10
    frame #22: 0x0000000104b5b3d8 LLDB`DWARFDebugInfo::ParseUnitsFor(this=0x00000001307068f0, section=DebugInfo) at DWARFDebugInfo.cpp:74:45
    frame #23: 0x0000000104b63980 LLDB`DWARFDebugInfo::ParseUnitHeadersIfNeeded(this=0x000000016e3c6a80)::$_0::operator()() const at DWARFDebugInfo.cpp:100:5
    frame #24: 0x0000000104b63918 LLDB`decltype(__f=0x000000016e3c6a80)::$_0>(fp)()) std::__1::__invoke<DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0>(DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0&&) at type_traits:3747:1
    frame #25: 0x0000000104b638f0 LLDB`void std::__1::__call_once_param<std::__1::tuple<DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0&&> >::__execute<>(this=0x000000016e3c6a20, (null)=__tuple_indices<> @ 0x000000016e3c697f) at mutex:629:9
    frame #26: 0x0000000104b638c0 LLDB`std::__1::__call_once_param<std::__1::tuple<DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0&&> >::operator(this=0x000000016e3c6a20)() at mutex:621:9
    frame #27: 0x0000000104b6378c LLDB`void std::__1::__call_once_proxy<std::__1::tuple<DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0&&> >(__vp=0x000000016e3c6a20) at mutex:657:5
    frame #28: 0x00000001850b92b0 libc++.1.dylib`std::__1::__call_once(unsigned long volatile&, void*, void (*)(void*)) + 180
    frame #29: 0x0000000104b636c4 LLDB`void std::__1::call_once<DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0>(__flag=0x0000000130706900, __func=0x000000016e3c6a80)::$_0&&) at mutex:675:9
    frame #30: 0x0000000104b5b9c0 LLDB`void llvm::call_once<DWARFDebugInfo::ParseUnitHeadersIfNeeded()::$_0>(flag=0x0000000130706900, F=0x000000016e3c6a80)::$_0&&) at Threading.h:92:5
    frame #31: 0x0000000104b5b984 LLDB`DWARFDebugInfo::ParseUnitHeadersIfNeeded(this=0x00000001307068f0) at DWARFDebugInfo.cpp:99:3
    frame #32: 0x0000000104b5c048 LLDB`DWARFDebugInfo::ContainsTypeUnits(this=0x00000001307068f0) at DWARFDebugInfo.cpp:173:3
    frame #33: 0x0000000104bff524 LLDB`SymbolFileDWARFDwo::FindSingleCompileUnit(this=0x000000013200d018) at SymbolFileDWARFDwo.cpp:62:19
    frame #34: 0x0000000104bff48c LLDB`SymbolFileDWARFDwo::GetDWOCompileUnitForHash(this=0x000000013200d018, hash=16045690984229367821) at SymbolFileDWARFDwo.cpp:49:26
    frame #35: 0x0000000104b7ce44 LLDB`DWARFUnit::ExtractUnitDIEIfNeeded(this=0x000000013081e400) at DWARFUnit.cpp:84:40
    frame #36: 0x0000000104b804a4 LLDB`DWARFUnit::GetDwoSymbolFile(this=0x000000013081e400) at DWARFUnit.cpp:817:3
    frame #37: 0x0000000104b98538 LLDB`lldb_private::ManualDWARFIndex::IndexUnit(this=0x00000001302129b0, unit=0x000000013081e400, dwp=0x0000000000000000, set=0x0000000130213240) at ManualDWARFIndex.cpp:144:50
    frame #38: 0x0000000104ba2354 LLDB`lldb_private::ManualDWARFIndex::Index(this=0x00000001303047c8, cu_idx=0)::$_1::operator()(unsigned long) const at ManualDWARFIndex.cpp:80:5
    frame #39: 0x0000000104ba22e0 LLDB`decltype(__f=0x00000001303047c8, __args=0x00000001303047f0)::$_1&>(fp)(std::__1::forward<unsigned long&>(fp0))) std::__1::__invoke<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&>(lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&) at type_traits:3747:1
    frame #40: 0x0000000104ba229c LLDB`std::__1::__bind_return<lldb_private::ManualDWARFIndex::Index()::$_1, std::__1::tuple<unsigned long>, std::__1::tuple<>, __is_valid_bind_return<lldb_private::ManualDWARFIndex::Index()::$_1, std::__1::tuple<unsigned long>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<lldb_private::ManualDWARFIndex::Index(__f=0x00000001303047c8, __bound_args=size=1, (null)=__tuple_indices<0> @ 0x000000016e3c6d5f, __args=size=0)::$_1, std::__1::tuple<unsigned long>, 0ul, std::__1::tuple<> >(lldb_private::ManualDWARFIndex::Index()::$_1&, std::__1::tuple<unsigned long>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&) at functional:2852:12
    frame #41: 0x0000000104ba2254 LLDB`std::__1::__bind_return<lldb_private::ManualDWARFIndex::Index()::$_1, std::__1::tuple<unsigned long>, std::__1::tuple<>, __is_valid_bind_return<lldb_private::ManualDWARFIndex::Index()::$_1, std::__1::tuple<unsigned long>, std::__1::tuple<> >::value>::type std::__1::__bind<lldb_private::ManualDWARFIndex::Index(this=0x00000001303047c8)::$_1&, unsigned long&>::operator()<>() at functional:2885:20
    frame #42: 0x0000000104ba2210 LLDB`decltype(__f=0x00000001303047c8)::$_1&, unsigned long&>&>(fp)()) std::__1::__invoke<std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&>&>(std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&>&) at type_traits:3747:1
    frame #43: 0x0000000104ba21c4 LLDB`void std::__1::__invoke_void_return_wrapper<void>::__call<std::__1::__bind<lldb_private::ManualDWARFIndex::Index(__args=0x00000001303047c8)::$_1&, unsigned long&>&>(std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&>&) at __functional_base:348:9
    frame #44: 0x0000000104ba219c LLDB`std::__1::__function::__alloc_func<std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&>, std::__1::allocator<std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&> >, void ()>::operator(this=0x00000001303047c8)() at functional:1553:16
    frame #45: 0x0000000104ba1054 LLDB`std::__1::__function::__func<std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&>, std::__1::allocator<std::__1::__bind<lldb_private::ManualDWARFIndex::Index()::$_1&, unsigned long&> >, void ()>::operator(this=0x00000001303047c0)() at functional:1727:12
    frame #46: 0x0000000103f4f47c LLDB`std::__1::__function::__value_func<void ()>::operator(this=0x0000000130304898)() const at functional:1880:16
    frame #47: 0x0000000103f4f414 LLDB`std::__1::function<void ()>::operator(this=0x0000000130304898)() const at functional:2555:12
    frame #48: 0x0000000104eee758 LLDB`decltype(__f=0x0000000130304898)>&>(fp)()) std::__1::__invoke<std::__1::function<void ()>&>(std::__1::function<void ()>&) at type_traits:3747:1
    frame #49: 0x0000000104eee288 LLDB`std::__1::__packaged_task_func<std::__1::function<void ()>, std::__1::allocator<std::__1::function<void ()> >, void ()>::operator(this=0x0000000130304890)() at future:1683:12
    frame #50: 0x0000000104eed45c LLDB`std::__1::__packaged_task_function<void ()>::operator(this=0x000000016e3c6f20)() const at future:1865:12
    frame #51: 0x0000000104eecc58 LLDB`std::__1::packaged_task<void ()>::operator(this=0x000000016e3c6f20)() at future:2085:9
    frame #52: 0x0000000104eeca24 LLDB`llvm::ThreadPool::ThreadPool(this=0x00000001302128c0)::$_0::operator()() const at ThreadPool.cpp:51:9
    frame #53: 0x0000000104eec900 LLDB`void llvm::thread::Apply<llvm::ThreadPool::ThreadPool(llvm::ThreadPoolStrategy)::$_0>(Callee=size=1, (null)=std::__1::index_sequence<> @ 0x000000016e3c6f6f)::$_0>&, std::__1::integer_sequence<unsigned long>) at thread.h:42:5
    frame #54: 0x0000000104eec8cc LLDB`void llvm::thread::GenericThreadProxy<std::__1::tuple<llvm::ThreadPool::ThreadPool(llvm::ThreadPoolStrategy)::$_0> >(Ptr=0x00000001302128c0) at thread.h:50:5
    frame #55: 0x0000000104eec5bc LLDB`void* llvm::thread::ThreadProxy<std::__1::tuple<llvm::ThreadPool::ThreadPool(llvm::ThreadPoolStrategy)::$_0> >(Ptr=0x00000001302128c0) at thread.h:60:5
    frame #56: 0x000000018512b878 libsystem_pthread.dylib`_pthread_start + 320

The issue is the ObjectFileELF::GetSymtab() used to _not_ take the module lock, which is really bad. With this new patch is did take the module lock. So we had very dangerous multi-threading issues before with ObjectFileELF::GetSymtab() not being threadsafe which could explain flakiness.

The main issue is how to fix this. @labath: let me know if you have any ideas on how to fix this. We are asking the symbol file to pre-load the symbols first, and _then_ asking the symbol table to preload itself. Do you remember why? The comment "Prime the symbol file first, since it adds symbols to the symbol table" but the SymbolFile::PreloadSymbols() doesn't seem to do that anymore. Maybe this was changed? I see the symbol file can add symbols via SymbolFile::AddSymbols() as seen in:

Symtab *SymbolFile::GetSymtab() {
  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
  if (m_symtab)
    return m_symtab;

  // Fetch the symtab from the main object file.
  m_symtab = GetMainObjectFile()->GetSymtab();

  // Then add our symbols to it.
  if (m_symtab)
    AddSymbols(*m_symtab);

  return m_symtab;
}

So one solution is to load the normal symbol table first in Module::PreloadSymbols():

void Module::PreloadSymbols() {
  std::lock_guard<std::recursive_mutex> guard(m_mutex);
  SymbolFile *sym_file = GetSymbolFile();
  if (!sym_file)
    return;
  // Load the symbol table first so it can be available to any thread without a lock
  if (Symtab *symtab = sym_file->GetSymtab())
    symtab->PreloadSymbols();

  // Now let the symbol file preload its symbols
  sym_file->PreloadSymbols();
}

Then we would need to modify the ObjectFile::GetSymtab() to be able to check m_symtab_up without taking the module lock:

Symtab *ObjectFile::GetSymtab() {
  // We don't need to take the module lock if the symbol table pointer is
  // already set in the std::unique_ptr since it is thread safe for access. This
  // prevents deadlocks where the symbol table has already been parsed and
  // someone has the module lock acquired and another thread spins up to do
  // something that requires the symbol table. This currently happens in the
  // manual DWARF indexing where it will spin up a thread to index the DWARF and
  // that thread might ask for the symbol table in
  // ObjectFileELF::RelocateSection() which requires the symbol table and causes
  // a deadlock.
  if (auto symtab = m_symtab_up.get())
    return symtab;

  ModuleSP module_sp(GetModule());
  if (module_sp) {
    std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
    // Check the unique pointer again in case two threads made it to the above
    // lock guard and one thread actually acquired the lock first and was
    // populating the symbol table. By the time we get to this line of code
    // the symbol table will either already be parsed or we need to parse it.

    // Here we detect if the symbol table was parsed while the lock was held
    // on this thread.
    if (auto symtab = m_symtab_up.get())
      return symtab;

    // We haven't parse the symbol table yet.
    ElapsedTime elapsed(module_sp->GetSymtabParseTime());
    // Store the symbol table in a local unique pointer until the symbol table
    // has been parsed, and then store the value into m_symtab_up.
    auto symtab_up = std::make_unique<Symtab>(this);
    std::lock_guard<std::recursive_mutex> symtab_guard(
        symtab_up->GetMutex());
    ParseSymtab(*symtab_up);
    symtab_up->Finalize();
    m_symtab_up = std::move(symtab_up);
  }
  return m_symtab_up.get();
}

Any input would be appreciated!

[clayborg]

I resubmitted with a fix in https://reviews.llvm.org/D114288

lldb/include/lldb/Symbol/ObjectFile.h

public:
 
   /// Notify the ObjectFile that the file addresses in the Sections for this
   /// module have been changed.
   virtual void SectionFileAddressesChanged() {}
 
   /// 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.
   ///
   virtual void RelocateSection(lldb_private::Section *section);
 
   /// Appends a Symbol for the specified so_addr to the symbol table.
   ///
   /// If verify_unique is false, the symbol table is not searched to determine

lldb/include/lldb/Symbol/Symtab.h

Symbol *FindFirstSymbolWithNameAndType(ConstString name,
                                          Debug symbol_debug_type,
                                          Visibility symbol_visibility);
   Symbol *FindSymbolAtFileAddress(lldb::addr_t file_addr);
   Symbol *FindSymbolContainingFileAddress(lldb::addr_t file_addr);
   void ForEachSymbolContainingFileAddress(
       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,
                               NameToIndexMap &name_to_index_map) const;
 
   ObjectFile *GetObjectFile() { return m_objfile; }
 
 protected:

lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h

public:
   uint32_t GetAddressByteSize() const override {
     return m_arch.GetAddressByteSize();
   }
 
   AddressClass GetAddressClass(lldb::addr_t file_addr) override {
     return AddressClass::eInvalid;
   }
 
-  Symtab *GetSymtab() override;
+  void ParseSymtab(lldb_private::Symtab &symtab) override;
 
   bool IsStripped() override { return false; }
 
   void CreateSections(SectionList &unified_section_list) override;
 
   void Dump(Stream *s) override {}
 
   ArchSpec GetArchitecture() override { return m_arch; }

lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp

ObjectFileBreakpad::ObjectFileBreakpad(const ModuleSP &module_sp,
     : ObjectFile(module_sp, file, offset, length, data_sp, data_offset),
       m_arch(std::move(arch)), m_uuid(std::move(uuid)) {}
 
 bool ObjectFileBreakpad::ParseHeader() {
   // We already parsed the header during initialization.
   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) {
   if (m_sections_up)
     return;
   m_sections_up = std::make_unique<SectionList>();
 
   llvm::Optional<Record::Kind> current_section;

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

public:
   lldb::ByteOrder GetByteOrder() const override;
 
   bool IsExecutable() const override;
 
   uint32_t GetAddressByteSize() const override;
 
   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;
 
   void CreateSections(lldb_private::SectionList &unified_section_list) override;
 
   void Dump(lldb_private::Stream *s) override;
 
   lldb_private::ArchSpec GetArchitecture() override;
 
   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;
 
   lldb_private::Address
   GetImageInfoAddress(lldb_private::Target *target) override;
 
   lldb_private::Address GetEntryPointAddress() override;
private:
   unsigned PLTRelocationType();
 
   static lldb_private::Status
   RefineModuleDetailsFromNote(lldb_private::DataExtractor &data,
                               lldb_private::ArchSpec &arch_spec,
                               lldb_private::UUID &uuid);
 
   bool AnySegmentHasPhysicalAddress();
-  
   /// Takes the .gnu_debugdata and returns the decompressed object file that is
   /// stored within that section.
   ///
   /// \returns either the decompressed object file stored within the
   /// .gnu_debugdata section or \c nullptr if an error occured or if there's no
   /// section with that name.
   std::shared_ptr<ObjectFileELF> GetGnuDebugDataObjectFile();
 };
 
 #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_ELF_OBJECTFILEELF_H

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

if (GetData(rel->GetFileOffset(), rel->GetFileSize(), rel_data) &&
       GetData(debug->GetFileOffset(), debug->GetFileSize(), debug_data)) {
     ApplyRelocations(thetab, &m_header, rel_hdr, symtab_hdr, debug_hdr,
                      rel_data, symtab_data, debug_data, debug);
   }
 
   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();
+    return module_obj_file->ParseSymtab(lldb_symtab);
 
-  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;
 
-    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);
-    }
+  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) {
-        if (!m_symtab_up)
-          m_symtab_up = std::make_unique<Symtab>(dynsym->GetObjectFile());
-        symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, dynsym);
-      }
-    }
+    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) {
-          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 (reloc_header)
+        ParseTrampolineSymbols(&lldb_symtab, 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);
-    }
+    ParseUnwindSymbols(&lldb_symtab, eh_frame);
 
-    // 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);
-      }
-    }
-
-    m_symtab_up->CalculateSymbolSizes();
-  }
-
-  return m_symtab_up.get();
+    if (is_valid_entry_point && !lldb_symtab.FindSymbolContainingFileAddress(
+      uint64_t symbol_id = lldb_symtab.GetNumSymbols();
+      lldb_symtab.AddSymbol(symbol);
 }
 
 void ObjectFileELF::RelocateSection(lldb_private::Section *section)
 {
   static const char *debug_prefix = ".debug";
 
   // Set relocated bit so we stop getting called, regardless of whether we
   // actually relocate.

lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h

bool SetLoadAddress(lldb_private::Target &target, lldb::addr_t value,
                       bool value_is_offset) override;
 
   lldb::ByteOrder GetByteOrder() const override;
 
   bool IsExecutable() const override;
 
   uint32_t GetAddressByteSize() const override;
 
-  lldb_private::Symtab *GetSymtab() override;
+  void ParseSymtab(lldb_private::Symtab &symtab) override;
 
   bool IsStripped() override;
 
   void CreateSections(lldb_private::SectionList &unified_section_list) override;
 
   void Dump(lldb_private::Stream *s) override;
 
   lldb_private::ArchSpec GetArchitecture() override;

lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp

 ByteOrder ObjectFileJIT::GetByteOrder() const { return m_data.GetByteOrder(); }
 
 bool ObjectFileJIT::IsExecutable() const { return false; }
 
 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) {
+    delegate_sp->PopulateSymtab(this, symtab);
 }
 
 bool ObjectFileJIT::IsStripped() {
   return false; // JIT code that is in a module is never stripped
 }
 
 void ObjectFileJIT::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_up) {

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h

public:
   bool IsDynamicLoader() const;
 
   bool IsSharedCacheBinary() const;
 
   uint32_t GetAddressByteSize() const override;
 
   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;
 
   void CreateSections(lldb_private::SectionList &unified_section_list) override;
 
   void Dump(lldb_private::Stream *s) override;
 
   lldb_private::ArchSpec GetArchitecture() override;

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

case eSymbolTypeObjCIVar:
       return AddressClass::eRuntime;
     case eSymbolTypeReExported:
       return AddressClass::eRuntime;
     }
   }
   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());
     if (module_sp) {
       lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
       for (uint32_t i = 0; i < m_header.ncmds; ++i) {
         const lldb::offset_t load_cmd_offset = offset;
 
ParseNList(DataExtractor &nlist_data, lldb::offset_t &nlist_data_offset,
   nlist.n_sect = nlist_data.GetU8_unchecked(&nlist_data_offset);
   nlist.n_desc = nlist_data.GetU16_unchecked(&nlist_data_offset);
   nlist.n_value = nlist_data.GetAddress_unchecked(&nlist_data_offset);
   return nlist;
 }
 
 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>")));
 
   llvm::MachO::symtab_command symtab_load_command = {0, 0, 0, 0, 0, 0};
   llvm::MachO::linkedit_data_command function_starts_load_command = {0, 0, 0, 0};
   llvm::MachO::linkedit_data_command exports_trie_load_command = {0, 0, 0, 0};
   llvm::MachO::dyld_info_command dyld_info = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
for (i = 0; i < m_header.ncmds; ++i) {
     // Watch for the symbol table load command
     switch (lc.cmd) {
     case LC_SYMTAB:
       symtab_load_command.cmd = lc.cmd;
       symtab_load_command.cmdsize = lc.cmdsize;
       // 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:
     case LC_DYLD_INFO_ONLY:
       if (m_data.GetU32(&offset, &dyld_info.rebase_off, 10)) {
         dyld_info.cmd = lc.cmd;
         dyld_info.cmdsize = lc.cmdsize;
       } else {
for (i = 0; i < m_header.ncmds; ++i) {
 
     default:
       break;
     }
     offset = cmd_offset + lc.cmdsize;
   }
 
   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();
   bool bit_width_32 = addr_byte_size == 4;
   const size_t nlist_byte_size =
       bit_width_32 ? sizeof(struct nlist) : sizeof(struct nlist_64);
 
   DataExtractor nlist_data(nullptr, 0, byte_order, addr_byte_size);
if (process && m_header.filetype != llvm::MachO::MH_OBJECT &&
 
     nlist_data.SetData(m_data, symtab_load_command.symoff,
                        nlist_data_byte_size);
     strtab_data.SetData(m_data, symtab_load_command.stroff,
                         strtab_data_byte_size);
 
     // 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,
                              dyld_info.export_size);
     } else if (exports_trie_load_command.datasize > 0) {
       dyld_trie_data.SetData(m_data, exports_trie_load_command.dataoff,
                              exports_trie_load_command.datasize);
     }
if (dsc_data_sp) {
               local_symbols_entry.nlistCount =
                   dsc_local_symbols_data.GetU32(&offset);
 
               if (header_file_offset == local_symbols_entry.dylibOffset) {
                 unmapped_local_symbols_found = local_symbols_entry.nlistCount;
 
                 // 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 +
                     (nlist_byte_size * local_symbols_entry.nlistStartIndex);
                 uint32_t string_table_offset = local_symbols_info.stringsOffset;
 
                 for (uint32_t nlist_index = 0;
                      nlist_index < local_symbols_entry.nlistCount;
if (dsc_data_sp) {
                         } else {
                           type = eSymbolTypeCompiler;
 
                           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 the size which we already placed
                             // with the original symbol, so don't add it if
                             // we want a minimal symbol table
                             add_nlist = false;
                           }
if (dsc_data_sp) {
                         type = eSymbolTypeSourceFile;
                         if (symbol_name == NULL) {
                           add_nlist = false;
                           if (N_SO_index != UINT32_MAX) {
                             // Set the size of the N_SO to the terminating
                             // 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);
                           }
                           N_NSYM_indexes.clear();
                           N_INCL_indexes.clear();
                           N_BRAC_indexes.clear();
                           N_COMM_indexes.clear();
                           N_FUN_indexes.clear();
if (dsc_data_sp) {
                       case N_EINCL:
                         // include file end: name,,NO_SECT,0,0
                         // Set the size of the N_BINCL to the terminating
                         // index of this 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());
+                              symtab.SymbolAtIndex(N_INCL_indexes.back());
                           symbol_ptr->SetByteSize(sym_idx + 1);
                           symbol_ptr->SetSizeIsSibling(true);
                           N_INCL_indexes.pop_back();
                         }
                         type = eSymbolTypeScopeEnd;
                         break;
 
                       case N_SOL:
if (dsc_data_sp) {
                         // Set the size of the N_LBRAC to the terminating
                         // index of this N_RBRAC so that we can always skip
                         // the entire symbol if we need to navigate more
                         // 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());
+                              symtab.SymbolAtIndex(N_BRAC_indexes.back());
                           symbol_ptr->SetByteSize(sym_idx + 1);
                           symbol_ptr->SetSizeIsSibling(true);
                           N_BRAC_indexes.pop_back();
                         }
                         type = eSymbolTypeScopeEnd;
                         break;
 
                       case N_EXCL:
if (dsc_data_sp) {
                         // end common: name,,n_sect,0,0
                         // Set the size of the N_BCOMM to the terminating
                         // index of this N_ECOMM/N_ECOML 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_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();
                         }
                         type = eSymbolTypeScopeEnd;
                         break;
 
                       case N_LENG:

 #endif
 
   if (nlist_data.GetByteSize() > 0) {
 
     // If the sym array was not created while parsing the DSC unmapped
     // 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) {
       assert(m_dysymtab.ilocalsym == 0);
       nlist_data_offset += (m_dysymtab.nlocalsym * nlist_byte_size);
       nlist_idx = m_dysymtab.nlocalsym;
     } else {
       nlist_idx = 0;
auto ParseSymbolLambda = [&](struct nlist_64 &nlist, uint32_t nlist_idx,
             // out
             N_FUN_indexes.push_back(sym_idx);
           } else {
             type = eSymbolTypeCompiler;
 
             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
               // the size which we already placed with the original symbol,
               // so don't add it if we want a minimal symbol table
               add_nlist = false;
             }
           }
auto ParseSymbolLambda = [&](struct nlist_64 &nlist, uint32_t nlist_idx,
           type = eSymbolTypeSourceFile;
           if (symbol_name == nullptr) {
             add_nlist = false;
             if (N_SO_index != UINT32_MAX) {
               // Set the size of the N_SO to the terminating 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);
             }
             N_NSYM_indexes.clear();
             N_INCL_indexes.clear();
             N_BRAC_indexes.clear();
             N_COMM_indexes.clear();
             N_FUN_indexes.clear();
auto ParseSymbolLambda = [&](struct nlist_64 &nlist, uint32_t nlist_idx,
           break;
 
         case N_EINCL:
           // include file end: name,,NO_SECT,0,0
           // Set the size of the N_BINCL to the terminating index of this
           // 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();
           }
           type = eSymbolTypeScopeEnd;
           break;
 
         case N_SOL:
auto ParseSymbolLambda = [&](struct nlist_64 &nlist, uint32_t nlist_idx,
 
         case N_RBRAC:
           // right bracket: 0,,NO_SECT,nesting level,address Set the size of
           // the N_LBRAC to the terminating index of this N_RBRAC so that we
           // can always skip the entire symbol if we need to navigate more
           // 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();
           }
           type = eSymbolTypeScopeEnd;
           break;
 
         case N_EXCL:
auto ParseSymbolLambda = [&](struct nlist_64 &nlist, uint32_t nlist_idx,
 
         case N_ECOMM:
           // end common: name,,n_sect,0,0
           // Set the size of the N_BCOMM to the terminating index of this
           // N_ECOMM/N_ECOML 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_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();
           }
           type = eSymbolTypeScopeEnd;
           break;
 
         case N_LENG:
#endif
   int trie_symbol_table_augment_count = 0;
   for (auto &e : external_sym_trie_entries) {
     if (symbols_added.find(e.entry.address) == symbols_added.end())
       trie_symbol_table_augment_count++;
   }
 
   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;
 
   // Add symbols from the trie to the symbol table.
   for (auto &e : external_sym_trie_entries) {
     if (symbols_added.find(e.entry.address) != symbols_added.end())
       continue;
 
for (i = 0; i < function_starts_count; ++i) {
       if (symbols_added.find(function_starts.GetEntryRef(i).addr) ==
           symbols_added.end())
         ++num_synthetic_function_symbols;
     }
 
     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 =
             function_starts.GetEntryAtIndex(i);
         if (symbols_added.find(func_start_entry->addr) == symbols_added.end()) {
           addr_t symbol_file_addr = func_start_entry->addr;
           uint32_t symbol_flags = 0;
           if (func_start_entry->data)
if (num_synthetic_function_symbols > 0) {
       }
     }
   }
 
   // Trim our symbols down to just what we ended up with after removing any
   // symbols.
   if (sym_idx < num_syms) {
     num_syms = sym_idx;
-    sym = symtab->Resize(num_syms);
+    sym = symtab.Resize(num_syms);
   }
 
   // Now synthesize indirect symbols
   if (m_dysymtab.nindirectsyms != 0) {
     if (indirect_symbol_index_data.GetByteSize()) {
       NListIndexToSymbolIndexMap::const_iterator end_index_pos =
           m_nlist_idx_to_sym_idx.end();
 
if (indirect_symbol_index_data.GetByteSize()) {
                 continue;
 
               NListIndexToSymbolIndexMap::const_iterator index_pos =
                   m_nlist_idx_to_sym_idx.find(stub_sym_id);
               Symbol *stub_symbol = nullptr;
               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) {
                 Address so_addr(symbol_stub_addr, section_list);
 
                 if (stub_symbol->GetType() == eSymbolTypeUndefined) {
                   // Change the external symbol into a trampoline that makes
                   // sense These symbols were N_UNDF N_EXT, and are useless
                   // to us, so we can re-use them so we don't have to make up
                   // a synthetic symbol for no good reason.
                   if (resolver_addresses.find(symbol_stub_addr) ==
                       resolver_addresses.end())
                     stub_symbol->SetType(eSymbolTypeTrampoline);
                   else
                     stub_symbol->SetType(eSymbolTypeResolver);
                   stub_symbol->SetExternal(false);
                   stub_symbol->GetAddressRef() = so_addr;
                   stub_symbol->SetByteSize(symbol_stub_byte_size);
                 } else {
                   // 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++);
                   sym[sym_idx].GetMangled() = stub_symbol_mangled_name;
                   if (resolver_addresses.find(symbol_stub_addr) ==
                       resolver_addresses.end())
                     sym[sym_idx].SetType(eSymbolTypeTrampoline);
                   else
if (!reexport_trie_entries.empty()) {
     for (const auto &e : reexport_trie_entries) {
       if (e.entry.import_name) {
         // Only add indirect symbols from the Trie entries if we didn't have
         // a N_INDR nlist entry for this already
         if (indirect_symbol_names.find(e.entry.name) ==
             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);
           sym[sym_idx].SetIsSynthetic(true);
           sym[sym_idx].SetReExportedSymbolName(e.entry.import_name);
           if (e.entry.other > 0 && e.entry.other <= dylib_files.GetSize()) {
             sym[sym_idx].SetReExportedSymbolSharedLibrary(
                 dylib_files.GetFileSpecAtIndex(e.entry.other - 1));
           }
           ++sym_idx;
         }
       }
     }
   }
-
-  //        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) {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
     s->Printf("%p: ", static_cast<void *>(this));
     s->Indent();

lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.h

public:
   lldb::ByteOrder GetByteOrder() const override {
     return lldb::eByteOrderLittle;
   }
 
   bool ParseHeader() override { return true; }
 
   bool IsExecutable() const override { return false; }
 
-  Symtab *GetSymtab() override { return nullptr; }
+  void ParseSymtab(lldb_private::Symtab &symtab) override {}
 
   bool IsStripped() override { return false; }
 
   // No section in PDB file.
   void CreateSections(SectionList &unified_section_list) override {}
 
   void Dump(Stream *s) override {}
 

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

public:
 
   bool IsExecutable() const override;
 
   uint32_t GetAddressByteSize() const override;
 
   //    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;
 
   void CreateSections(lldb_private::SectionList &unified_section_list) override;
 
   void Dump(lldb_private::Stream *s) override;
 
   lldb_private::ArchSpec GetArchitecture() override;

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

lldb::offset_t string_file_offset =
         m_coff_header.symoff + (m_coff_header.nsyms * 18) + stroff;
     if (const char *name = m_data.GetCStr(&string_file_offset))
       return name;
     return "";
   }
   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);
+void ObjectFilePECOFF::ParseSymtab(Symtab &symtab) {
 
-          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));
-            }
+      Symbol *symbols = symtab.Resize(num_syms);
 
-            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);
+    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);
-        }
-      }
-      m_symtab_up->CalculateSymbolSizes();
-    }
-  }
-  return m_symtab_up.get();
 }
 
 std::unique_ptr<CallFrameInfo> ObjectFilePECOFF::CreateCallFrameInfo() {
   if (coff_data_dir_exception_table >= m_coff_header_opt.data_dirs.size())
     return {};
 
   data_directory data_dir_exception =
       m_coff_header_opt.data_dirs[coff_data_dir_exception_table];

lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h

public:
   uint32_t GetAddressByteSize() const override {
     return m_arch.GetAddressByteSize();
   }
 
   AddressClass GetAddressClass(lldb::addr_t file_addr) override {
     return AddressClass::eInvalid;
   }
 
-  Symtab *GetSymtab() override;
+  void ParseSymtab(lldb_private::Symtab &symtab) override;
 
   bool IsStripped() override { return !!GetExternalDebugInfoFileSpec(); }
 
   void CreateSections(SectionList &unified_section_list) override;
 
   void Dump(Stream *s) override;
 
   ArchSpec GetArchitecture() override { return m_arch; }

lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp

ObjectFileWasm::ObjectFileWasm(const lldb::ModuleSP &module_sp,
     : ObjectFile(module_sp, process_sp, header_addr, header_data_sp),
       m_arch("wasm32-unknown-unknown-wasm") {}
 
 bool ObjectFileWasm::ParseHeader() {
   // We already parsed the header during initialization.
   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_")) {
     return llvm::StringSwitch<SectionType>(Name)
         .Case("abbrev", eSectionTypeDWARFDebugAbbrev)
         .Case("abbrev.dwo", eSectionTypeDWARFDebugAbbrevDwo)
         .Case("addr", eSectionTypeDWARFDebugAddr)
         .Case("aranges", eSectionTypeDWARFDebugAranges)

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

public:
   }
   bool ParseHeader() override { return true; }
   Type CalculateType() override { return eTypeUnknown; }
   Strata CalculateStrata() override { return eStrataUnknown; }
   uint32_t GetDependentModules(FileSpecList &file_list) override { return 0; }
   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(); }
 
   uint32_t GetAddressByteSize() const override {
     return m_arch.GetAddressByteSize();
   }
 
   Address GetBaseAddress() override {

lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

for (llvm::StringRef line : lines(Record::Public)) {
     if (auto record = PublicRecord::parse(line))
       add_symbol(record->Address, llvm::None, record->Name);
     else
       LLDB_LOG(log, "Failed to parse: {0}. Skipping record.", line);
   }
 
   for (Symbol &symbol : symbols)
     symtab.AddSymbol(std::move(symbol));
-  symtab.CalculateSymbolSizes();
+  symtab.Finalize();
 }
 
 llvm::Expected<lldb::addr_t>
 SymbolFileBreakpad::GetParameterStackSize(Symbol &symbol) {
   ParseUnwindData();
   if (auto *entry = m_unwind_data->win.FindEntryThatContains(
           symbol.GetAddress().GetFileAddress())) {
     auto record = StackWinRecord::parse(
void SymbolFileBreakpad::ParseUnwindData() {
   }
   m_unwind_data->win.Sort();
 }
 
 uint64_t SymbolFileBreakpad::GetDebugInfoSize() {
   // Breakpad files are all debug info.
   return m_objfile_sp->GetByteSize();
 }
-

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

symtab.AddSymbol(
                offset,    // value
                size,      // size
                size != 0, // size_is_valid
                false,     // contains_linker_annotations
                0          // flags
                ));
   }
 
-  symtab.CalculateSymbolSizes();
   symtab.Finalize();
 }
 
 void SymbolFilePDB::FindTypes(
     lldb_private::ConstString name, const CompilerDeclContext &parent_decl_ctx,
     uint32_t max_matches,
     llvm::DenseSet<lldb_private::SymbolFile *> &searched_symbol_files,
     lldb_private::TypeMap &types) {

lldb/source/Symbol/ObjectFile.cpp

void llvm::format_provider<ObjectFile::Strata>::format(
   case ObjectFile::eStrataRawImage:
     OS << "raw image";
     break;
   case ObjectFile::eStrataJIT:
     OS << "jit";
     break;
   }
 }
+
+
+Symtab *ObjectFile::GetSymtab() {
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+    std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
+    if (!m_symtab_up) {
+      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);
+      m_symtab_up->Finalize();
+    }
+  }
+  return m_symtab_up.get();
+}

lldb/source/Symbol/Symtab.cpp

if (num_entries > 0) {
       }
 
       // Sort again in case the range size changes the ordering
       m_file_addr_to_index.Sort();
     }
   }
 }
 
-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) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   if (!m_file_addr_to_index_computed)
     InitAddressIndexes();
 
   const FileRangeToIndexMap::Entry *entry =