[lldb-dev] [PATCH] factor methods in DynamicLoaderPOSIXDYLD into base class

Steve Pucci spucci at google.com
Mon Jan 27 14:32:35 PST 2014


Hi,

I'd like to have access to a number of methods in DynamicLoaderPOSIXDYLD
from the new class I'm working on, DynamicLoaderGDBServer.  These methods
have no dependency on DynamicLoaderPOSIXDYLD, with two exceptions noted
below, so I'm proposing to move them into the base class DynamicLoader.

The two exceptions are the methods UpdateLoadedSections and UnloadSections;
in each case there is one line of code that is special to the derived
class, and the rest of the code in the method is generic to the base class.
 In each case I created a XXXCommon() method on the base class with the
common code, and created a virtual method XXX() on the base class, which in
DynamicLoaderPOSIXDYLD will call the common code and then execute its one
line of specialized code.

This patch is intended to have no functional difference whatsoever.  All
276 tests that are enabled for Ubuntu pass successfully.

Thanks,
   Steve
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140127/c20d8afd/attachment.html>
-------------- next part --------------
diff --git a/include/lldb/Target/DynamicLoader.h b/include/lldb/Target/DynamicLoader.h
index 272f64f..606690c 100644
--- a/include/lldb/Target/DynamicLoader.h
+++ b/include/lldb/Target/DynamicLoader.h
@@ -246,6 +246,59 @@ public:
 
 protected:
     //------------------------------------------------------------------
+    // Utility methods for derived classes
+    //------------------------------------------------------------------
+
+    /// Checks to see if the target module has changed, updates the target
+    /// accordingly and returns the target executable module.
+    lldb::ModuleSP
+    GetTargetExecutable();
+
+    /// Updates the load address of every allocatable section in @p module.
+    ///
+    /// @param module The module to traverse.
+    ///
+    /// @param link_map_addr The virtual address of the link map for the @p module.
+    ///
+    /// @param base_addr The virtual base address @p module is loaded at.
+    virtual void
+    UpdateLoadedSections(lldb::ModuleSP module,
+                         lldb::addr_t link_map_addr,
+                         lldb::addr_t base_addr);
+
+    // Utility method so base classes can share implementation of UpdateLoadedSections
+    void
+    UpdateLoadedSectionsCommon(lldb::ModuleSP module,
+                               lldb::addr_t link_map_addr,
+                               lldb::addr_t base_addr);
+
+    /// Removes the loaded sections from the target in @p module.
+    ///
+    /// @param module The module to traverse.
+    virtual void
+    UnloadSections(const lldb::ModuleSP module);
+
+    // Utility method so base classes can share implementation of UnloadSections
+    void
+    UnloadSectionsCommon(const lldb::ModuleSP module);
+
+    /// Locates or creates a module given by @p file and updates/loads the
+    /// resulting module at the virtual base address @p base_addr.
+    lldb::ModuleSP
+    LoadModuleAtAddress(const lldb_private::FileSpec &file, lldb::addr_t link_map_addr, lldb::addr_t base_addr);
+
+    const lldb_private::SectionList *
+    GetSectionListFromModule(const lldb::ModuleSP module) const;
+
+    // Read an int from the given process from memory at the given addr
+    static int
+    ReadInt(Process *process, lldb::addr_t addr);
+
+    // Read a pointer from the given process from memory at the given addr
+    static lldb::addr_t
+    ReadPointer(Process *process, lldb::addr_t addr);
+
+    //------------------------------------------------------------------
     // Member variables.
     //------------------------------------------------------------------
     Process* m_process; ///< The process that this dynamic loader plug-in is tracking.
diff --git a/source/Core/DynamicLoader.cpp b/source/Core/DynamicLoader.cpp
index 82f8404..b85c163 100644
--- a/source/Core/DynamicLoader.cpp
+++ b/source/Core/DynamicLoader.cpp
@@ -10,7 +10,11 @@
 #include "lldb/lldb-private.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -74,3 +78,152 @@ DynamicLoader::SetStopWhenImagesChange (bool stop)
     m_process->SetStopOnSharedLibraryEvents (stop);
 }
 
+ModuleSP
+DynamicLoader::GetTargetExecutable()
+{
+    Target &target = m_process->GetTarget();
+    ModuleSP executable = target.GetExecutableModule();
+
+    if (executable.get())
+    {
+        if (executable->GetFileSpec().Exists())
+        {
+            ModuleSpec module_spec (executable->GetFileSpec(), executable->GetArchitecture());
+            ModuleSP module_sp (new Module (module_spec));
+
+            // Check if the executable has changed and set it to the target executable if they differ.
+            if (module_sp.get() && module_sp->GetUUID().IsValid() && executable->GetUUID().IsValid())
+            {
+                if (module_sp->GetUUID() != executable->GetUUID())
+                    executable.reset();
+            }
+            else if (executable->FileHasChanged())
+            {
+                executable.reset();
+            }
+
+            if (!executable.get())
+            {
+                executable = target.GetSharedModule(module_spec);
+                if (executable.get() != target.GetExecutableModulePointer())
+                {
+                    // Don't load dependent images since we are in dyld where we will know
+                    // and find out about all images that are loaded
+                    const bool get_dependent_images = false;
+                    target.SetExecutableModule(executable, get_dependent_images);
+                }
+            }
+        }
+    }
+    return executable;
+}
+
+void
+DynamicLoader::UpdateLoadedSections(ModuleSP module, addr_t link_map_addr, addr_t base_addr)
+{
+    UpdateLoadedSectionsCommon(module, link_map_addr, base_addr);
+}
+
+void
+DynamicLoader::UpdateLoadedSectionsCommon(ModuleSP module, addr_t link_map_addr, addr_t base_addr)
+{
+    Target &target = m_process->GetTarget();
+    const SectionList *sections = GetSectionListFromModule(module);
+
+    assert(sections && "SectionList missing from loaded module.");
+
+    const size_t num_sections = sections->GetSize();
+
+    for (unsigned i = 0; i < num_sections; ++i)
+    {
+        SectionSP section_sp (sections->GetSectionAtIndex(i));
+        lldb::addr_t new_load_addr = section_sp->GetFileAddress() + base_addr;
+
+        // If the file address of the section is zero then this is not an
+        // allocatable/loadable section (property of ELF sh_addr).  Skip it.
+        if (new_load_addr == base_addr)
+            continue;
+
+        target.SetSectionLoadAddress(section_sp, new_load_addr);
+    }
+}
+
+void
+DynamicLoader::UnloadSections(const ModuleSP module)
+{
+    UnloadSectionsCommon(module);
+}
+
+void
+DynamicLoader::UnloadSectionsCommon(const ModuleSP module)
+{
+    Target &target = m_process->GetTarget();
+    const SectionList *sections = GetSectionListFromModule(module);
+
+    assert(sections && "SectionList missing from unloaded module.");
+
+    const size_t num_sections = sections->GetSize();
+    for (size_t i = 0; i < num_sections; ++i)
+    {
+        SectionSP section_sp (sections->GetSectionAtIndex(i));
+        target.SetSectionUnloaded(section_sp);
+    }
+}
+
+
+const SectionList *
+DynamicLoader::GetSectionListFromModule(const ModuleSP module) const
+{
+    SectionList *sections = nullptr;
+    if (module.get())
+    {
+        ObjectFile *obj_file = module->GetObjectFile();
+        if (obj_file)
+        {
+            sections = obj_file->GetSectionList();
+        }
+    }
+    return sections;
+}
+
+ModuleSP
+DynamicLoader::LoadModuleAtAddress(const FileSpec &file, addr_t link_map_addr, addr_t base_addr)
+{
+    Target &target = m_process->GetTarget();
+    ModuleList &modules = target.GetImages();
+    ModuleSP module_sp;
+
+    ModuleSpec module_spec (file, target.GetArchitecture());
+    if ((module_sp = modules.FindFirstModule (module_spec)))
+    {
+        UpdateLoadedSections(module_sp, link_map_addr, base_addr);
+    }
+    else if ((module_sp = target.GetSharedModule(module_spec)))
+    {
+        UpdateLoadedSections(module_sp, link_map_addr, base_addr);
+    }
+
+    return module_sp;
+}
+
+int
+DynamicLoader::ReadInt(Process *process, addr_t addr)
+{
+    Error error;
+    int value = (int)process->ReadUnsignedIntegerFromMemory(addr, sizeof(uint32_t), 0, error);
+    if (error.Fail())
+        return -1;
+    else
+        return value;
+}
+
+addr_t
+DynamicLoader::ReadPointer(Process *process, addr_t addr)
+{
+    Error error;
+    addr_t value = process->ReadPointerFromMemory(addr, error);
+    if (error.Fail())
+        return LLDB_INVALID_ADDRESS;
+    else
+        return value;
+}
diff --git a/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 7b29904..c1bdebb 100644
--- a/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -150,46 +150,6 @@ DynamicLoaderPOSIXDYLD::DidLaunch()
     }
 }
 
-ModuleSP
-DynamicLoaderPOSIXDYLD::GetTargetExecutable()
-{
-    Target &target = m_process->GetTarget();
-    ModuleSP executable = target.GetExecutableModule();
-
-    if (executable.get())
-    {
-        if (executable->GetFileSpec().Exists())
-        {
-            ModuleSpec module_spec (executable->GetFileSpec(), executable->GetArchitecture());
-            ModuleSP module_sp (new Module (module_spec));
-
-            // Check if the executable has changed and set it to the target executable if they differ.
-            if (module_sp.get() && module_sp->GetUUID().IsValid() && executable->GetUUID().IsValid())
-            {
-                if (module_sp->GetUUID() != executable->GetUUID())
-                    executable.reset();
-            }
-            else if (executable->FileHasChanged())
-            {
-                executable.reset();
-            }
-
-            if (!executable.get())
-            {
-                executable = target.GetSharedModule(module_spec);
-                if (executable.get() != target.GetExecutableModulePointer())
-                {
-                    // Don't load dependent images since we are in dyld where we will know
-                    // and find out about all images that are loaded
-                    const bool get_dependent_images = false;
-                    target.SetExecutableModule(executable, get_dependent_images);
-                }
-            }
-        }
-    }
-    return executable;
-}
-
 Error
 DynamicLoaderPOSIXDYLD::ExecutePluginCommand(Args &command, Stream *strm)
 {
@@ -211,45 +171,17 @@ DynamicLoaderPOSIXDYLD::CanLoadImage()
 void
 DynamicLoaderPOSIXDYLD::UpdateLoadedSections(ModuleSP module, addr_t link_map_addr, addr_t base_addr)
 {
-    Target &target = m_process->GetTarget();
-    const SectionList *sections = GetSectionListFromModule(module);
-
-    assert(sections && "SectionList missing from loaded module.");
-
     m_loaded_modules[module] = link_map_addr;
 
-    const size_t num_sections = sections->GetSize();
-
-    for (unsigned i = 0; i < num_sections; ++i)
-    {
-        SectionSP section_sp (sections->GetSectionAtIndex(i));
-        lldb::addr_t new_load_addr = section_sp->GetFileAddress() + base_addr;
-
-        // If the file address of the section is zero then this is not an
-        // allocatable/loadable section (property of ELF sh_addr).  Skip it.
-        if (new_load_addr == base_addr)
-            continue;
-
-        target.SetSectionLoadAddress(section_sp, new_load_addr);
-    }
+    UpdateLoadedSectionsCommon(module, link_map_addr, base_addr);
 }
 
 void
 DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP module)
 {
-    Target &target = m_process->GetTarget();
-    const SectionList *sections = GetSectionListFromModule(module);
-
-    assert(sections && "SectionList missing from unloaded module.");
-
     m_loaded_modules.erase(module);
 
-    const size_t num_sections = sections->GetSize();
-    for (size_t i = 0; i < num_sections; ++i)
-    {
-        SectionSP section_sp (sections->GetSectionAtIndex(i));
-        target.SetSectionUnloaded(section_sp);
-    }
+    UnloadSectionsCommon(module);
 }
 
 void
@@ -467,26 +399,6 @@ DynamicLoaderPOSIXDYLD::LoadAllCurrentModules()
     m_process->GetTarget().ModulesDidLoad(module_list);
 }
 
-ModuleSP
-DynamicLoaderPOSIXDYLD::LoadModuleAtAddress(const FileSpec &file, addr_t link_map_addr, addr_t base_addr)
-{
-    Target &target = m_process->GetTarget();
-    ModuleList &modules = target.GetImages();
-    ModuleSP module_sp;
-
-    ModuleSpec module_spec (file, target.GetArchitecture());
-    if ((module_sp = modules.FindFirstModule (module_spec))) 
-    {
-        UpdateLoadedSections(module_sp, link_map_addr, base_addr);
-    }
-    else if ((module_sp = target.GetSharedModule(module_spec))) 
-    {
-        UpdateLoadedSections(module_sp, link_map_addr, base_addr);
-    }
-
-    return module_sp;
-}
-
 addr_t
 DynamicLoaderPOSIXDYLD::ComputeLoadOffset()
 {
@@ -530,41 +442,6 @@ DynamicLoaderPOSIXDYLD::GetEntryPoint()
     return m_entry_point;
 }
 
-const SectionList *
-DynamicLoaderPOSIXDYLD::GetSectionListFromModule(const ModuleSP module) const
-{
-    SectionList *sections = nullptr;
-    if (module.get())
-    {
-        ObjectFile *obj_file = module->GetObjectFile();
-        if (obj_file)
-        {
-            sections = obj_file->GetSectionList();
-        }
-    }
-    return sections;
-}
-
-static int ReadInt(Process *process, addr_t addr)
-{
-    Error error;
-    int value = (int)process->ReadUnsignedIntegerFromMemory(addr, sizeof(uint32_t), 0, error);
-    if (error.Fail())
-        return -1;
-    else
-        return value;
-}
-
-static addr_t ReadPointer(Process *process, addr_t addr)
-{
-    Error error;
-    addr_t value = process->ReadPointerFromMemory(addr, error);
-    if (error.Fail())
-        return LLDB_INVALID_ADDRESS;
-    else
-        return value;
-}
-
 lldb::addr_t
 DynamicLoaderPOSIXDYLD::GetThreadLocalData (const lldb::ModuleSP module, const lldb::ThreadSP thread)
 {
diff --git a/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index 7997b34..9ced5da 100644
--- a/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -126,7 +126,7 @@ protected:
     /// @param link_map_addr The virtual address of the link map for the @p module.
     ///
     /// @param base_addr The virtual base address @p module is loaded at.
-    void
+    virtual void
     UpdateLoadedSections(lldb::ModuleSP module,
                          lldb::addr_t link_map_addr,
                          lldb::addr_t base_addr);
@@ -134,14 +134,9 @@ protected:
     /// Removes the loaded sections from the target in @p module.
     ///
     /// @param module The module to traverse.
-    void
+    virtual void
     UnloadSections(const lldb::ModuleSP module);
 
-    /// Locates or creates a module given by @p file and updates/loads the
-    /// resulting module at the virtual base address @p base_addr.
-    lldb::ModuleSP
-    LoadModuleAtAddress(const lldb_private::FileSpec &file, lldb::addr_t link_map_addr, lldb::addr_t base_addr);
-
     /// Resolves the entry point for the current inferior process and sets a
     /// breakpoint at that address.
     void
@@ -173,16 +168,8 @@ protected:
     lldb::addr_t
     GetEntryPoint();
 
-    /// Checks to see if the target module has changed, updates the target
-    /// accordingly and returns the target executable module.
-    lldb::ModuleSP
-    GetTargetExecutable();
-
 private:
     DISALLOW_COPY_AND_ASSIGN(DynamicLoaderPOSIXDYLD);
-
-    const lldb_private::SectionList *
-    GetSectionListFromModule(const lldb::ModuleSP module) const;
 };
 
 #endif  // liblldb_DynamicLoaderPOSIXDYLD_H_


More information about the lldb-dev mailing list