Move UnwindTable from ObjectFile to Module

[labath]

Summary

This is a preparatory step to enable adding extra unwind strategies by
symbol file plugins. This has been discussed on the lldb-dev mailing
list: http://lists.llvm.org/pipermail/lldb-dev/2019-February/014703.html.

[clayborg]

You changed the API to return a pointer which can be NULL so we either need to check all returns for this or change the API to return a reference to a default constructed UnwindTable. I like the latter option, but I will leave that up to you. Since we are moving functionality into symbol files, we might want to opt for the latter to allow us to populate the UnwindTable as needed from any source when ever that source becomes available (like adding a symbol file to a module after the fact).

include/lldb/Core/Module.h

709 ↗

(On Diff #186481)

I would vote to return a "UnwindTable&" and make a const and non const version of this. Otherwise we have to NULL check any call to this API.

1108–1110 ↗

(On Diff #186481)

Why not just make this an instance to avoid having to null check all accessor calls?:

UnwindTable m_unwind_table;

source/Commands/CommandObjectTarget.cpp

3536–3537 ↗

(On Diff #186481)

Either NULL check or change the API?

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

2865–2866 ↗

(On Diff #186481)

Either NULL check or change the API?

source/Plugins/Process/Utility/RegisterContextLLDB.cpp

242–243 ↗

(On Diff #186481)

Either NULL check or change the API?

674–675 ↗

(On Diff #186481)

Either NULL check or change the API?

776–777 ↗

(On Diff #186481)

Either NULL check or change the API?

795 ↗

(On Diff #186481)

Either NULL check or change the API?

805 ↗

(On Diff #186481)

Either NULL check or change the API?

[labath]

Changed Module::GetUnwindTable to return a referece. I also changed the
UnwindTable to internally store a Module& (instead of ObjectFile&), to make sure
it always has something to hold on to.

[labath]

include/lldb/Core/Module.h

709 ↗

(On Diff #186481)

There isn't much point in having a const version of this, as all UnwindTable methods are non-const.

[clayborg]

Just a question of if we need an optional unwind table instance instead of just an instance. I am fine either way.

include/lldb/Core/Module.h

1108–1110 ↗

(On Diff #186599)

Any reason to not just have a UnwindTable instance here? The accessor can't fail, so one must be created anyway right?

[labath]

include/lldb/Core/Module.h

1108–1110 ↗

(On Diff #186599)

The difference is in when it gets created. The regular instance would have to be created together with the Module. However, that shouldn't really matter, as UnwindTable is internally lazily initialized as well, so I'll just remove the optional. We may need to do something smarter here anyway once we start re-initializing the unwind info due to symbol file changes.

lldb/trunk/include/lldb/Core/Module.h

public:
   ///
   /// If the Section file addresses for a module are updated, this method
   /// should be called.  Any parts of the module, object file, or symbol file
   /// that has cached those file addresses must invalidate or update its
   /// cache.
   //------------------------------------------------------------------
   virtual void SectionFileAddressesChanged();
 
+  //------------------------------------------------------------------
+  /// Returns a reference to the UnwindTable for this Module
+  ///
+  /// The UnwindTable contains FuncUnwinders objects for any function in this
+  /// Module.  If a FuncUnwinders object hasn't been created yet (i.e. the
+  /// function has yet to be unwound in a stack walk), it will be created when
+  /// requested.  Specifically, we do not create FuncUnwinders objects for
+  /// functions until they are needed.
+  ///
+  /// @return
+  ///     Returns the unwind table for this module. If this object has no
+  ///     associated object file, an empty UnwindTable is returned.
+  //------------------------------------------------------------------
+  UnwindTable &GetUnwindTable() { return m_unwind_table; }
+
   llvm::VersionTuple GetVersion();
 
   //------------------------------------------------------------------
   /// Load an object file from memory.
   ///
   /// If available, the size of the object file in memory may be passed to
   /// avoid additional round trips to process memory. If the size is not
   /// provided, a default value is used. This value should be large enough to
protected:
   ConstString m_object_name; ///< The name an object within this module that is
                              ///selected, or empty of the module is represented
                              ///by \a m_file.
   uint64_t m_object_offset;
   llvm::sys::TimePoint<> m_object_mod_time;
   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
+  UnwindTable m_unwind_table{*this}; ///< 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>
       m_old_symfiles; ///< If anyone calls Module::SetSymbolFileFileSpec() and
                       ///changes the symbol file,
   ///< we need to keep all old symbol files around in case anyone has type
   ///references to them
   TypeSystemMap m_type_system_map;   ///< A map of any type systems associated

lldb/trunk/include/lldb/Symbol/ObjectFile.h

public:
   ///
   /// @return
   ///     Returns \b true if the header was parsed successfully, \b
   ///     false otherwise.
   //------------------------------------------------------------------
   virtual bool ParseHeader() = 0;
 
   //------------------------------------------------------------------
-  /// Returns a reference to the UnwindTable for this ObjectFile
-  ///
-  /// The UnwindTable contains FuncUnwinders objects for any function in this
-  /// ObjectFile.  If a FuncUnwinders object hasn't been created yet (i.e. the
-  /// function has yet to be unwound in a stack walk), it will be created when
-  /// requested.  Specifically, we do not create FuncUnwinders objects for
-  /// functions until they are needed.
-  ///
-  /// @return
-  ///     Returns the unwind table for this object file.
-  //------------------------------------------------------------------
-  virtual lldb_private::UnwindTable &GetUnwindTable() { return m_unwind_table; }
-
-  //------------------------------------------------------------------
   /// Returns if the function bounds for symbols in this symbol file are
   /// likely accurate.
   ///
   /// The unwinder can emulate the instructions of functions to understand
   /// prologue/epilogue code sequences, where registers are spilled on the
   /// stack, etc.  This feature relies on having the correct start addresses
   /// of all functions.  If the ObjectFile has a way to tell that symbols have
   /// been stripped and there's no way to reconstruct start addresses (e.g.
protected:
   Strata m_strata;
   lldb::addr_t m_file_offset; ///< The offset in bytes into the file, or the
                               ///address in memory
   lldb::addr_t m_length; ///< The length of this object file if it is known (can
                          ///be zero if length is unknown or can't be
                          ///determined).
   DataExtractor
       m_data; ///< The data for this object file so things can be parsed lazily.
-  lldb_private::UnwindTable m_unwind_table; /// < Table of FuncUnwinders objects
-                                            /// created for this ObjectFile's
-                                            /// functions
   lldb::ProcessWP m_process_wp;
   const lldb::addr_t m_memory_addr;
   std::unique_ptr<lldb_private::SectionList> m_sections_up;
   std::unique_ptr<lldb_private::Symtab> m_symtab_up;
   uint32_t m_synthetic_symbol_idx;
 
   //------------------------------------------------------------------
   /// Sets the architecture for a module.  At present the architecture can

lldb/trunk/include/lldb/Symbol/UnwindTable.h

 namespace lldb_private {
 
 // A class which holds all the FuncUnwinders objects for a given ObjectFile.
 // The UnwindTable is populated with FuncUnwinders objects lazily during the
 // debug session.
 
 class UnwindTable {
 public:
-  UnwindTable(ObjectFile &objfile);
+  /// Create an Unwind table using the data in the given module.
+  explicit UnwindTable(Module &module);
+
   ~UnwindTable();
 
   lldb_private::DWARFCallFrameInfo *GetEHFrameInfo();
   lldb_private::DWARFCallFrameInfo *GetDebugFrameInfo();
 
   lldb_private::CompactUnwindInfo *GetCompactUnwindInfo();
 
   ArmUnwindInfo *GetArmUnwindInfo();
private:
   void Initialize();
   llvm::Optional<AddressRange> GetAddressRange(const Address &addr,
                                                SymbolContext &sc);
 
   typedef std::map<lldb::addr_t, lldb::FuncUnwindersSP> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
-  ObjectFile &m_object_file;
+  Module &m_module;
   collection m_unwinds;
 
   bool m_initialized; // delay some initialization until ObjectFile is set up
   std::mutex m_mutex;
 
   std::unique_ptr<DWARFCallFrameInfo> m_eh_frame_up;
   std::unique_ptr<DWARFCallFrameInfo> m_debug_frame_up;
   std::unique_ptr<CompactUnwindInfo> m_compact_unwind_up;
   std::unique_ptr<ArmUnwindInfo> m_arm_unwind_up;
 
   DISALLOW_COPY_AND_ASSIGN(UnwindTable);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_UnwindTable_h

lldb/trunk/source/Commands/CommandObjectTarget.cpp

for (uint32_t idx = 0; idx < num_matches; idx++) {
       ConstString funcname(sc.GetFunctionName());
       if (funcname.IsEmpty())
         continue;
       addr_t start_addr = range.GetBaseAddress().GetLoadAddress(target);
       if (abi)
         start_addr = abi->FixCodeAddress(start_addr);
 
       FuncUnwindersSP func_unwinders_sp(
-          sc.module_sp->GetObjectFile()
-              ->GetUnwindTable()
+          sc.module_sp->GetUnwindTable()
               .GetUncachedFuncUnwindersContainingAddress(start_addr, sc));
       if (!func_unwinders_sp)
         continue;
 
       result.GetOutputStream().Printf(
           "UNWIND PLANS for %s`%s (start addr 0x%" PRIx64 ")\n\n",
           sc.module_sp->GetPlatformFileSpec().GetFilename().AsCString(),
           funcname.AsCString(), start_addr);

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

if (symbol) {
         if (m_symtab_up == nullptr)
           m_symtab_up.reset(new Symtab(reloc_section->GetObjectFile()));
 
         ParseTrampolineSymbols(m_symtab_up.get(), symbol_id, reloc_header,
                                reloc_id);
       }
     }
 
-    DWARFCallFrameInfo *eh_frame = GetUnwindTable().GetEHFrameInfo();
-    if (eh_frame) {
+    if (DWARFCallFrameInfo *eh_frame =
+            GetModule()->GetUnwindTable().GetEHFrameInfo()) {
       if (m_symtab_up == nullptr)
         m_symtab_up.reset(new Symtab(this));
       ParseUnwindSymbols(m_symtab_up.get(), 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)

lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp

if (!ReadFrameAddress(row_register_kind, active_row->GetCFAValue(), m_cfa)) {
     // Try the fall back unwind plan since the
     // full unwind plan failed.
     FuncUnwindersSP func_unwinders_sp;
     UnwindPlanSP call_site_unwind_plan;
     bool cfa_status = false;
 
     if (m_sym_ctx_valid) {
       func_unwinders_sp =
-          pc_module_sp->GetObjectFile()
-              ->GetUnwindTable()
-              .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx);
+          pc_module_sp->GetUnwindTable().GetFuncUnwindersContainingAddress(
+              m_current_pc, m_sym_ctx);
     }
 
     if (func_unwinders_sp.get() != nullptr)
       call_site_unwind_plan = func_unwinders_sp->GetUnwindPlanAtCallSite(
           process->GetTarget(), m_current_offset_backed_up_one);
 
     if (call_site_unwind_plan.get() != nullptr) {
       m_fallback_unwind_plan_sp = call_site_unwind_plan;
UnwindPlanSP RegisterContextLLDB::GetFastUnwindPlanForFrame() {
   if (!m_current_pc.IsValid() || !pc_module_sp ||
       pc_module_sp->GetObjectFile() == NULL)
     return unwind_plan_sp;
 
   if (IsFrameZero())
     return unwind_plan_sp;
 
   FuncUnwindersSP func_unwinders_sp(
-      pc_module_sp->GetObjectFile()
-          ->GetUnwindTable()
-          .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx));
+      pc_module_sp->GetUnwindTable().GetFuncUnwindersContainingAddress(
+          m_current_pc, m_sym_ctx));
   if (!func_unwinders_sp)
     return unwind_plan_sp;
 
   // If we're in _sigtramp(), unwinding past this frame requires special
   // knowledge.
   if (m_frame_type == eTrapHandlerFrame || m_frame_type == eDebuggerFrame)
     return unwind_plan_sp;
 
if (!m_current_pc.IsValid() || !pc_module_sp ||
       pc_module_sp->GetObjectFile() == NULL) {
     m_frame_type = eNormalFrame;
     return arch_default_unwind_plan_sp;
   }
 
   FuncUnwindersSP func_unwinders_sp;
   if (m_sym_ctx_valid) {
     func_unwinders_sp =
-        pc_module_sp->GetObjectFile()
-            ->GetUnwindTable()
-            .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx);
+        pc_module_sp->GetUnwindTable().GetFuncUnwindersContainingAddress(
+            m_current_pc, m_sym_ctx);
   }
 
   // No FuncUnwinders available for this pc (stripped function symbols, lldb
   // could not augment its function table with another source, like
   // LC_FUNCTION_STARTS or eh_frame in ObjectFileMachO). See if eh_frame or the
   // .ARM.exidx tables have unwind information for this address, else fall back
   // to the architectural default unwind.
   if (!func_unwinders_sp) {
     m_frame_type = eNormalFrame;
 
     if (!pc_module_sp || !pc_module_sp->GetObjectFile() ||
         !m_current_pc.IsValid())
       return arch_default_unwind_plan_sp;
 
     // Even with -fomit-frame-pointer, we can try eh_frame to get back on
     // track.
     DWARFCallFrameInfo *eh_frame =
-        pc_module_sp->GetObjectFile()->GetUnwindTable().GetEHFrameInfo();
+        pc_module_sp->GetUnwindTable().GetEHFrameInfo();
     if (eh_frame) {
       unwind_plan_sp = std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
       if (eh_frame->GetUnwindPlan(m_current_pc, *unwind_plan_sp))
         return unwind_plan_sp;
       else
         unwind_plan_sp.reset();
     }
 
     ArmUnwindInfo *arm_exidx =
-        pc_module_sp->GetObjectFile()->GetUnwindTable().GetArmUnwindInfo();
+        pc_module_sp->GetUnwindTable().GetArmUnwindInfo();
     if (arm_exidx) {
       unwind_plan_sp = std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
       if (arm_exidx->GetUnwindPlan(exe_ctx.GetTargetRef(), m_current_pc,
                                    *unwind_plan_sp))
         return unwind_plan_sp;
       else
         unwind_plan_sp.reset();
     }

lldb/trunk/source/Symbol/ObjectFile.cpp

 ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
                        const FileSpec *file_spec_ptr,
                        lldb::offset_t file_offset, lldb::offset_t length,
                        const lldb::DataBufferSP &data_sp,
                        lldb::offset_t data_offset)
     : ModuleChild(module_sp),
       m_file(), // This file could be different from the original module's file
       m_type(eTypeInvalid), m_strata(eStrataInvalid),
-      m_file_offset(file_offset), m_length(length), m_data(),
-      m_unwind_table(*this), m_process_wp(),
+      m_file_offset(file_offset), m_length(length), m_data(), m_process_wp(),
       m_memory_addr(LLDB_INVALID_ADDRESS), m_sections_up(), m_symtab_up(),
       m_synthetic_symbol_idx(0) {
   if (file_spec_ptr)
     m_file = *file_spec_ptr;
   if (data_sp)
     m_data.SetData(data_sp, data_offset, length);
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
   if (log)
     log->Printf("%p ObjectFile::ObjectFile() module = %p (%s), file = %s, "
                 "file_offset = 0x%8.8" PRIx64 ", size = %" PRIu64,
                 static_cast<void *>(this), static_cast<void *>(module_sp.get()),
                 module_sp->GetSpecificationDescription().c_str(),
                 m_file ? m_file.GetPath().c_str() : "<NULL>", m_file_offset,
                 m_length);
 }
 
 ObjectFile::ObjectFile(const lldb::ModuleSP &module_sp,
                        const ProcessSP &process_sp, lldb::addr_t header_addr,
                        DataBufferSP &header_data_sp)
     : ModuleChild(module_sp), m_file(), m_type(eTypeInvalid),
       m_strata(eStrataInvalid), m_file_offset(0), m_length(0), m_data(),
-      m_unwind_table(*this), m_process_wp(process_sp),
-      m_memory_addr(header_addr), m_sections_up(), m_symtab_up(),
-      m_synthetic_symbol_idx(0) {
+      m_process_wp(process_sp), m_memory_addr(header_addr), m_sections_up(),
+      m_symtab_up(), m_synthetic_symbol_idx(0) {
   if (header_data_sp)
     m_data.SetData(header_data_sp, 0, header_data_sp->GetByteSize());
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
   if (log)
     log->Printf("%p ObjectFile::ObjectFile() module = %p (%s), process = %p, "
                 "header_addr = 0x%" PRIx64,
                 static_cast<void *>(this), static_cast<void *>(module_sp.get()),
                 module_sp->GetSpecificationDescription().c_str(),

lldb/trunk/source/Symbol/UnwindTable.cpp

 
 // There is one UnwindTable object per ObjectFile. It contains a list of Unwind
 // objects -- one per function, populated lazily -- for the ObjectFile. Each
 // Unwind object has multiple UnwindPlans for different scenarios.
 
 using namespace lldb;
 using namespace lldb_private;
 
-UnwindTable::UnwindTable(ObjectFile &objfile)
-    : m_object_file(objfile), m_unwinds(), m_initialized(false), m_mutex(),
+UnwindTable::UnwindTable(Module &module)
+    : m_module(module), m_unwinds(), m_initialized(false), m_mutex(),
       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)
     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)
+    return;
 
-  SectionList *sl = m_object_file.GetSectionList();
+  SectionList *sl = object_file->GetSectionList();
   if (!sl)
     return;
 
   SectionSP sect = sl->FindSectionByType(eSectionTypeEHFrame, true);
   if (sect.get()) {
     m_eh_frame_up.reset(
-        new DWARFCallFrameInfo(m_object_file, sect, DWARFCallFrameInfo::EH));
+        new DWARFCallFrameInfo(*object_file, sect, DWARFCallFrameInfo::EH));
   }
 
   sect = sl->FindSectionByType(eSectionTypeDWARFDebugFrame, true);
   if (sect) {
     m_debug_frame_up.reset(
-        new DWARFCallFrameInfo(m_object_file, sect, DWARFCallFrameInfo::DWARF));
+        new DWARFCallFrameInfo(*object_file, sect, DWARFCallFrameInfo::DWARF));
   }
 
   sect = sl->FindSectionByType(eSectionTypeCompactUnwind, true);
   if (sect) {
-    m_compact_unwind_up.reset(new CompactUnwindInfo(m_object_file, sect));
+    m_compact_unwind_up.reset(new 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.reset(new ArmUnwindInfo(m_object_file, sect, sect_extab));
+      m_arm_unwind_up.reset(new ArmUnwindInfo(*object_file, sect, sect_extab));
     }
   }
 }
 
 UnwindTable::~UnwindTable() {}
 
 llvm::Optional<AddressRange> UnwindTable::GetAddressRange(const Address &addr,
                                                           SymbolContext &sc) {
UnwindTable::GetUncachedFuncUnwindersContainingAddress(const Address &addr,
   if (!range_or)
     return nullptr;
 
   return std::make_shared<FuncUnwinders>(*this, *range_or);
 }
 
 void UnwindTable::Dump(Stream &s) {
   std::lock_guard<std::mutex> guard(m_mutex);
-  s.Printf("UnwindTable for '%s':\n",
-           m_object_file.GetFileSpec().GetPath().c_str());
+  s.Format("UnwindTable for '{0}':\n", m_module.GetFileSpec());
   const_iterator begin = m_unwinds.begin();
   const_iterator end = m_unwinds.end();
   for (const_iterator pos = begin; pos != end; ++pos) {
     s.Printf("[%u] 0x%16.16" PRIx64 "\n", (unsigned)std::distance(begin, pos),
              pos->first);
   }
   s.EOL();
 }
CompactUnwindInfo *UnwindTable::GetCompactUnwindInfo() {
   return m_compact_unwind_up.get();
 }
 
 ArmUnwindInfo *UnwindTable::GetArmUnwindInfo() {
   Initialize();
   return m_arm_unwind_up.get();
 }
 
-ArchSpec UnwindTable::GetArchitecture() {
-  return m_object_file.GetArchitecture();
-}
+ArchSpec UnwindTable::GetArchitecture() { return m_module.GetArchitecture(); }
 
 bool UnwindTable::GetAllowAssemblyEmulationUnwindPlans() {
-  return m_object_file.AllowAssemblyEmulationUnwindPlans();
+  if (ObjectFile *object_file = m_module.GetObjectFile())
+    return object_file->AllowAssemblyEmulationUnwindPlans();
+  return false;
 }