https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/86770
In commit 2f63718f8567413a1c596bda803663eb58d6da5a Author: Jason Molenda <jmole...@apple.com> Date: Tue Mar 26 09:07:15 2024 -0700 [lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (#86603) I stopped clearing a Module's UnwindTable when we add a SymbolFile to avoid the memory management problems with adding a symbol file asynchronously while the UnwindTable is being accessed on another thread. This broke the target-symbols-add-unwind.test shell test on Linux which removes the DWARF debub_frame section from a binary, loads it, then loads the unstripped binary with the DWARF debug_frame section and checks that the UnwindPlans for a function include debug_frame. I originally decided that I was willing to sacrifice the possiblity of additional unwind sources from a symbol file because we rely on assembly emulation so heavily, they're rarely critical. But there are targets where we we don't have emluation and rely on things like DWARF debug_frame a lot more, so this probably wasn't a good choice. This patch adds a new UnwindTable::Update method which looks for any new sources of unwind information and adds it to the UnwindTable, and calls that after a new SymbolFile has been added to a Module. >From 450f75c9fcc68bc3590afd21e6eb1abb85d987eb Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Tue, 26 Mar 2024 22:48:58 -0700 Subject: [PATCH] [lldb] Revive shell test after updating UnwindTable In commit 2f63718f8567413a1c596bda803663eb58d6da5a Author: Jason Molenda <jmole...@apple.com> Date: Tue Mar 26 09:07:15 2024 -0700 [lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (#86603) I stopped clearing a Module's UnwindTable when we add a SymbolFile to avoid the memory management problems with adding a symbol file asynchronously while the UnwindTable is being accessed on another thread. This broke the target-symbols-add-unwind.test shell test on Linux which removes the DWARF debub_frame section from a binary, loads it, then loads the unstripped binary with the DWARF debug_frame section and checks that the UnwindPlans for a function include debug_frame. I originally decided that I was willing to sacrifice the possiblity of additional unwind sources from a symbol file because we rely on assembly emulation so heavily, they're rarely critical. But there are targets where we we don't have emluation and rely on things like DWARF debug_frame a lot more, so this probably wasn't a good choice. This patch adds a new UnwindTable::Update method which looks for any new sources of unwind information and adds it to the UnwindTable, and calls that after a new SymbolFile has been added to a Module. --- lldb/include/lldb/Symbol/UnwindTable.h | 4 ++ lldb/source/Core/Module.cpp | 2 + lldb/source/Symbol/UnwindTable.cpp | 45 +++++++++++++++++++ .../SymbolFile/target-symbols-add-unwind.test | 27 +++++++++++ 4 files changed, 78 insertions(+) create mode 100644 lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test diff --git a/lldb/include/lldb/Symbol/UnwindTable.h b/lldb/include/lldb/Symbol/UnwindTable.h index f0ce7047de2d1e..26826e5d1b497c 100644 --- a/lldb/include/lldb/Symbol/UnwindTable.h +++ b/lldb/include/lldb/Symbol/UnwindTable.h @@ -57,6 +57,10 @@ class UnwindTable { ArchSpec GetArchitecture(); + /// Called after a SymbolFile has been added to a Module to add any new + /// unwind sections that may now be available. + void Update(); + private: void Dump(Stream &s); diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index a520523a96521a..9c105b3f0e57a1 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1009,6 +1009,8 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) { m_symfile_up.reset( SymbolVendor::FindPlugin(shared_from_this(), feedback_strm)); m_did_load_symfile = true; + if (m_unwind_table) + m_unwind_table->Update(); } } } diff --git a/lldb/source/Symbol/UnwindTable.cpp b/lldb/source/Symbol/UnwindTable.cpp index 3c1a5187b11054..11bedf3d6052e7 100644 --- a/lldb/source/Symbol/UnwindTable.cpp +++ b/lldb/source/Symbol/UnwindTable.cpp @@ -84,6 +84,51 @@ void UnwindTable::Initialize() { } } +void UnwindTable::Update() { + if (!m_initialized) + return Initialize(); + + std::lock_guard<std::mutex> guard(m_mutex); + + ObjectFile *object_file = m_module.GetObjectFile(); + if (!object_file) + return; + + if (!m_object_file_unwind_up) + m_object_file_unwind_up = object_file->CreateCallFrameInfo(); + + SectionList *sl = m_module.GetSectionList(); + if (!sl) + return; + + SectionSP sect = sl->FindSectionByType(eSectionTypeEHFrame, true); + if (!m_eh_frame_up && sect) { + m_eh_frame_up = std::make_unique<DWARFCallFrameInfo>( + *object_file, sect, DWARFCallFrameInfo::EH); + } + + sect = sl->FindSectionByType(eSectionTypeDWARFDebugFrame, true); + if (!m_debug_frame_up && sect) { + m_debug_frame_up = std::make_unique<DWARFCallFrameInfo>( + *object_file, sect, DWARFCallFrameInfo::DWARF); + } + + sect = sl->FindSectionByType(eSectionTypeCompactUnwind, true); + if (!m_compact_unwind_up && sect) { + m_compact_unwind_up = + std::make_unique<CompactUnwindInfo>(*object_file, sect); + } + + sect = sl->FindSectionByType(eSectionTypeARMexidx, true); + if (!m_arm_unwind_up && sect) { + SectionSP sect_extab = sl->FindSectionByType(eSectionTypeARMextab, true); + if (sect_extab.get()) { + m_arm_unwind_up = + std::make_unique<ArmUnwindInfo>(*object_file, sect, sect_extab); + } + } +} + UnwindTable::~UnwindTable() = default; std::optional<AddressRange> diff --git a/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test b/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test new file mode 100644 index 00000000000000..5420213d405e86 --- /dev/null +++ b/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test @@ -0,0 +1,27 @@ +# TODO: When it's possible to run "image show-unwind" without a running +# process, we can remove the unsupported line below, and hard-code an ELF +# triple in the test. +# UNSUPPORTED: system-windows, system-darwin + +# RUN: cd %T +# RUN: %clang_host %S/Inputs/target-symbols-add-unwind.c -g \ +# RUN: -fno-unwind-tables -fno-asynchronous-unwind-tables \ +# RUN: -o target-symbols-add-unwind.debug +# RUN: llvm-objcopy --strip-debug target-symbols-add-unwind.debug \ +# RUN: target-symbols-add-unwind.stripped +# RUN: %lldb target-symbols-add-unwind.stripped -s %s -o quit | FileCheck %s + +process launch --stop-at-entry +image show-unwind -n main +# CHECK-LABEL: image show-unwind -n main +# CHECK-NOT: debug_frame UnwindPlan: + +target symbols add -s target-symbols-add-unwind.stripped target-symbols-add-unwind.debug +# CHECK-LABEL: target symbols add +# CHECK: symbol file {{.*}} has been added to {{.*}} + +image show-unwind -n main +# CHECK-LABEL: image show-unwind -n main +# CHECK: debug_frame UnwindPlan: +# CHECK-NEXT: This UnwindPlan originally sourced from DWARF CFI +# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits