Merge pull request swiftlang#9121 from jasonmolenda/cp/r128876433-dont-release-unwindtable-from-module

[lldb] Change Module to have a concrete UnwindTable, update (llvm#101130)

Author: JDevlieghere

lldb/include/lldb/Core/Module.h

@@ -1041,9 +1041,9 @@ class Module : public std::enable_shared_from_this<Module>,
   lldb::ObjectFileSP m_objfile_sp; ///< A shared pointer to the object file
                                    /// parser for this module as it may or may
                                    /// not be shared with the SymbolFile
-  std::optional<UnwindTable> m_unwind_table; ///< Table of FuncUnwinders
-                                             /// objects created for this
-                                             /// Module's functions
+  UnwindTable m_unwind_table;      ///< Table of FuncUnwinders
+                                   /// objects created for this
+                                   /// Module's functions
   lldb::SymbolVendorUP
       m_symfile_up; ///< A pointer to the symbol vendor for this module.
   std::vector<lldb::SymbolVendorUP>

lldb/include/lldb/Symbol/UnwindTable.h

@@ -57,9 +57,9 @@ 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();
+  /// Called after an ObjectFile/SymbolFile has been added to a Module to add
+  /// any new unwind sections that may now be available.
+  void ModuleWasUpdated();
 
 private:
   void Dump(Stream &s);
@@ -75,7 +75,11 @@ class UnwindTable {
   Module &m_module;
   collection m_unwinds;
 
-  bool m_initialized; // delay some initialization until ObjectFile is set up
+  bool m_scanned_all_unwind_sources; // true when we have looked at the
+                                     // ObjectFile and SymbolFile for all
+                                     // sources of unwind information; false if
+                                     // we haven't done that yet, or one of the
+                                     // files has been updated in the Module.
   std::mutex m_mutex;
 
   std::unique_ptr<CallFrameInfo> m_object_file_unwind_up;

lldb/source/Core/Module.cpp

@@ -136,7 +136,8 @@ Module *Module::GetAllocatedModuleAtIndex(size_t idx) {
 }
 
 Module::Module(const ModuleSpec &module_spec)
-    : m_file_has_changed(false), m_first_file_changed_log(false) {
+    : m_unwind_table(*this), m_file_has_changed(false),
+      m_first_file_changed_log(false) {
   // Scope for locker below...
   {
     std::lock_guard<std::recursive_mutex> guard(
@@ -242,8 +243,8 @@ Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
                const llvm::sys::TimePoint<> &object_mod_time)
     : m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
       m_arch(arch), m_file(file_spec), m_object_offset(object_offset),
-      m_object_mod_time(object_mod_time), m_file_has_changed(false),
-      m_first_file_changed_log(false) {
+      m_object_mod_time(object_mod_time), m_unwind_table(*this),
+      m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
     std::lock_guard<std::recursive_mutex> guard(
@@ -262,7 +263,9 @@ Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
               m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")");
 }
 
-Module::Module() : m_file_has_changed(false), m_first_file_changed_log(false) {
+Module::Module()
+    : m_unwind_table(*this), m_file_has_changed(false),
+      m_first_file_changed_log(false) {
   std::lock_guard<std::recursive_mutex> guard(
       GetAllocationModuleCollectionMutex());
   GetModuleCollection().push_back(this);
@@ -331,6 +334,8 @@ ObjectFile *Module::GetMemoryObjectFile(const lldb::ProcessSP &process_sp,
           // Augment the arch with the target's information in case
           // we are unable to extract the os/environment from memory.
           m_arch.MergeFrom(process_sp->GetTarget().GetArchitecture());
+
+          m_unwind_table.ModuleWasUpdated();
         } else {
           error.SetErrorString("unable to find suitable object file plug-in");
         }
@@ -1046,8 +1051,7 @@ 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();
+        m_unwind_table.ModuleWasUpdated();
       }
     }
   }
@@ -1345,6 +1349,8 @@ ObjectFile *Module::GetObjectFile() {
           // more specific than the generic COFF architecture, only merge in
           // those values that overwrite unspecified unknown values.
           m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
+
+          m_unwind_table.ModuleWasUpdated();
         } else {
           ReportError("failed to load objfile for {0}",
                       GetFileSpec().GetPath().c_str());
@@ -1374,12 +1380,9 @@ void Module::SectionFileAddressesChanged() {
 }
 
 UnwindTable &Module::GetUnwindTable() {
-  if (!m_unwind_table) {
-    if (!m_symfile_spec)
-      SymbolLocator::DownloadSymbolFileAsync(GetUUID());
-    m_unwind_table.emplace(*this);
-  }
-  return *m_unwind_table;
+  if (!m_symfile_spec)
+    SymbolLocator::DownloadSymbolFileAsync(GetUUID());
+  return m_unwind_table;
 }
 
 SectionList *Module::GetUnifiedSectionList() {

lldb/source/Symbol/UnwindTable.cpp

@@ -30,70 +30,27 @@ using namespace lldb;
 using namespace lldb_private;
 
 UnwindTable::UnwindTable(Module &module)
-    : m_module(module), m_unwinds(), m_initialized(false), m_mutex(),
-      m_object_file_unwind_up(), m_eh_frame_up(), m_compact_unwind_up(),
-      m_arm_unwind_up() {}
+    : m_module(module), m_unwinds(), m_scanned_all_unwind_sources(false),
+      m_mutex(), m_object_file_unwind_up(), m_eh_frame_up(),
+      m_compact_unwind_up(), m_arm_unwind_up() {}
 
 // We can't do some of this initialization when the ObjectFile is running its
 // ctor; delay doing it until needed for something.
-
 void UnwindTable::Initialize() {
-  if (m_initialized)
+  if (m_scanned_all_unwind_sources)
     return;
 
   std::lock_guard<std::mutex> guard(m_mutex);
 
-  if (m_initialized) // check again once we've acquired the lock
-    return;
-  m_initialized = true;
-  ObjectFile *object_file = m_module.GetObjectFile();
-  if (!object_file)
+  if (m_scanned_all_unwind_sources) // check again once we've acquired the lock
     return;
 
-  m_object_file_unwind_up = object_file->CreateCallFrameInfo();
-
-  SectionList *sl = m_module.GetSectionList();
-  if (!sl)
-    return;
-
-  SectionSP sect = sl->FindSectionByType(eSectionTypeEHFrame, true);
-  if (sect.get()) {
-    m_eh_frame_up = std::make_unique<DWARFCallFrameInfo>(
-        *object_file, sect, DWARFCallFrameInfo::EH);
-  }
-
-  sect = sl->FindSectionByType(eSectionTypeDWARFDebugFrame, true);
-  if (sect) {
-    m_debug_frame_up = std::make_unique<DWARFCallFrameInfo>(
-        *object_file, sect, DWARFCallFrameInfo::DWARF);
-  }
-
-  sect = sl->FindSectionByType(eSectionTypeCompactUnwind, true);
-  if (sect) {
-    m_compact_unwind_up =
-        std::make_unique<CompactUnwindInfo>(*object_file, sect);
-  }
-
-  sect = sl->FindSectionByType(eSectionTypeARMexidx, true);
-  if (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);
-    }
-  }
-}
-
-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;
 
+  m_scanned_all_unwind_sources = true;
+
   if (!m_object_file_unwind_up)
     m_object_file_unwind_up = object_file->CreateCallFrameInfo();
 
@@ -102,22 +59,19 @@ void UnwindTable::Update() {
     return;
 
   SectionSP sect = sl->FindSectionByType(eSectionTypeEHFrame, true);
-  if (!m_eh_frame_up && sect) {
+  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) {
+  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) {
+  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) {
@@ -129,6 +83,11 @@ void UnwindTable::Update() {
   }
 }
 
+void UnwindTable::ModuleWasUpdated() {
+  std::lock_guard<std::mutex> guard(m_mutex);
+  m_scanned_all_unwind_sources = false;
+}
+
 UnwindTable::~UnwindTable() = default;
 
 std::optional<AddressRange>