[Lldb-commits] SBTarget::SetModuleLoadAddress() patch

Greg Clayton gclayton at apple.com
Tue Mar 27 14:10:32 PDT 2012


Dmitry, The classification of the for the thread local data is good, though we should just have a bool attribute on the section that specifies that the section is thread local. Any addresses that fall within these thread local sections will need to be resolved per thread. Eventually we will need to have each "lldb_private::Thread" subclass be able to resolve a section load address when we try to resolve a section that is thread specific. For now they just won't resolve, but they shouldn't get in the way.

I have checked in a different fix that should work for you:

Index: include/lldb/Target/Target.h
===================================================================
--- include/lldb/Target/Target.h	(revision 153535)
+++ include/lldb/Target/Target.h	(working copy)
@@ -844,33 +844,6 @@
         return m_section_load_list;
     }
 
-
-    //------------------------------------------------------------------
-    /// Load a module in this target by at the section file addresses
-    /// with an optional constant slide applied to each section.
-    ///
-    /// This function will load all top level sections at their file
-    /// addresses and apply an optional constant slide amount to each 
-    /// section. This can be used to easily load a module at the same 
-    /// addresses that are contained in the object file (trust that
-    /// the addresses in an object file are the correct load addresses).
-    ///
-    /// @param[in] module
-    ///     The module to load.
-    ///
-    /// @param[in] slide
-    ///     A constant slide to add to each file address as each section
-    ///     is being loaded.
-    ///
-    /// @return
-    ///     \b true if loading the module at the specified address 
-    ///     causes a section to be loaded when it previously wasn't, or
-    ///     if a section changes load address. Returns \b false if
-    ///     the sections were all already loaded at these addresses.
-    //------------------------------------------------------------------
-    bool
-    LoadModuleWithSlide (Module *module, lldb::addr_t slide);
-
     static Target *
     GetTargetFromContexts (const ExecutionContext *exe_ctx_ptr, 
                            const SymbolContext *sc_ptr);
Index: include/lldb/Core/Section.h
===================================================================
--- include/lldb/Core/Section.h	(revision 153535)
+++ include/lldb/Core/Section.h	(working copy)
@@ -261,7 +261,19 @@
     {
         return m_parent_wp.lock();
     }
+    
+    bool
+    IsThreadSpecific () const
+    {
+        return m_thread_specific;
+    }
 
+    void
+    SetIsThreadSpecific (bool b)
+    {
+        m_thread_specific = b;
+    }
+
 protected:
 
     lldb::SectionWP m_parent_wp;        // Weak pointer to parent section
@@ -277,7 +289,8 @@
                                         // children contains an address. This allows for gaps between the children
                                         // that are contained in the address range for this section, but do not produce
                                         // hits unless the children contain the address.
-                    m_encrypted:1;      // Set to true if the contents are encrypted
+                    m_encrypted:1,      // Set to true if the contents are encrypted
+                    m_thread_specific:1;// This section is thread specific
     lldb::SectionWP m_linked_section_wp;
     uint64_t        m_linked_offset;
 private:
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp	(revision 153535)
+++ source/Commands/CommandObjectTarget.cpp	(working copy)
@@ -2517,30 +2517,13 @@
                             SectionList *section_list = objfile->GetSectionList();
                             if (section_list)
                             {
+                                bool changed = false;
                                 if (argc == 0)
                                 {
                                     if (m_slide_option.GetOptionValue().OptionWasSet())
                                     {
-                                        Module *module = matching_modules.GetModulePointerAtIndex(0);
-                                        if (module)
-                                        {
-                                            ObjectFile *objfile = module->GetObjectFile();
-                                            if (objfile)
-                                            {
-                                                SectionList *section_list = objfile->GetSectionList();
-                                                if (section_list)
-                                                {
-                                                    const size_t num_sections = section_list->GetSize();
-                                                    const addr_t slide = m_slide_option.GetOptionValue().GetCurrentValue();
-                                                    for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx)
-                                                    {
-                                                        SectionSP section_sp (section_list->GetSectionAtIndex(sect_idx));
-                                                        if (section_sp)
-                                                            target->GetSectionLoadList().SetSectionLoadAddress (section_sp.get(), section_sp->GetFileAddress() + slide);
-                                                    }
-                                                }
-                                            }
-                                        }
+                                        const addr_t slide = m_slide_option.GetOptionValue().GetCurrentValue();
+                                        module->SetLoadAddress (*target, slide, changed);
                                     }
                                     else
                                     {
@@ -2572,8 +2555,18 @@
                                                 SectionSP section_sp (section_list->FindSectionByName(const_sect_name));
                                                 if (section_sp)
                                                 {
-                                                    target->GetSectionLoadList().SetSectionLoadAddress (section_sp.get(), load_addr);
-                                                    result.AppendMessageWithFormat("section '%s' loaded at 0x%llx\n", sect_name, load_addr);
+                                                    if (section_sp->IsThreadSpecific())
+                                                    {
+                                                        result.AppendErrorWithFormat ("thread specific sections are not yet supported (section '%s')\n", sect_name);
+                                                        result.SetStatus (eReturnStatusFailed);
+                                                        break;
+                                                    }
+                                                    else
+                                                    {
+                                                        if (target->GetSectionLoadList().SetSectionLoadAddress (section_sp.get(), load_addr))
+                                                            changed = true;
+                                                        result.AppendMessageWithFormat("section '%s' loaded at 0x%llx\n", sect_name, load_addr);
+                                                    }
                                                 }
                                                 else
                                                 {
@@ -2600,6 +2593,9 @@
                                         }
                                     }
                                 }
+                                
+                                if (changed)
+                                    target->ModulesDidLoad (matching_modules);
                             }
                             else
                             {
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(revision 153535)
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(working copy)
@@ -621,6 +621,8 @@
             static ConstString g_sect_name_text (".text");
             static ConstString g_sect_name_data (".data");
             static ConstString g_sect_name_bss (".bss");
+            static ConstString g_sect_name_tdata (".tdata");
+            static ConstString g_sect_name_tbss (".tbss");
             static ConstString g_sect_name_dwarf_debug_abbrev (".debug_abbrev");
             static ConstString g_sect_name_dwarf_debug_aranges (".debug_aranges");
             static ConstString g_sect_name_dwarf_debug_frame (".debug_frame");
@@ -636,9 +638,21 @@
 
             SectionType sect_type = eSectionTypeOther;
 
+            bool is_thread_specific = false;
+            
             if      (name == g_sect_name_text)                  sect_type = eSectionTypeCode;
             else if (name == g_sect_name_data)                  sect_type = eSectionTypeData;
             else if (name == g_sect_name_bss)                   sect_type = eSectionTypeZeroFill;
+            else if (name == g_sect_name_tdata)
+            {
+                sect_type = eSectionTypeData;
+                is_thread_specific = true;   
+            }
+            else if (name == g_sect_name_tbss)
+            {
+                sect_type = eSectionTypeZeroFill;   
+                is_thread_specific = true;   
+            }
             else if (name == g_sect_name_dwarf_debug_abbrev)    sect_type = eSectionTypeDWARFDebugAbbrev;
             else if (name == g_sect_name_dwarf_debug_aranges)   sect_type = eSectionTypeDWARFDebugAranges;
             else if (name == g_sect_name_dwarf_debug_frame)     sect_type = eSectionTypeDWARFDebugFrame;
@@ -653,7 +667,7 @@
             else if (name == g_sect_name_eh_frame)              sect_type = eSectionTypeEHFrame;
             
             
-            SectionSP section(new Section(
+            SectionSP section_sp(new Section(
                 GetModule(),        // Module to which this section belongs.
                 SectionIndex(I),    // Section ID.
                 name,               // Section name.
@@ -664,7 +678,9 @@
                 file_size,          // Size of the section as found in the file.
                 header.sh_flags));  // Flags for this section.
 
-            m_sections_ap->AddSection(section);
+            if (is_thread_specific)
+                section_sp->SetIsThreadSpecific (is_thread_specific);
+            m_sections_ap->AddSection(section_sp);
         }
     }
 
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp	(revision 153535)
+++ source/Target/Target.cpp	(working copy)
@@ -1957,46 +1957,7 @@
     result.GetImmediateErrorStream()->Flush();
 }
 
-bool 
-Target::LoadModuleWithSlide (Module *module, lldb::addr_t slide)
-{
-    bool changed = false;
-    if (module)
-    {
-        ObjectFile *object_file = module->GetObjectFile();
-        if (object_file)
-        {
-            SectionList *section_list = object_file->GetSectionList ();
-            if (section_list)
-            {
-                // All sections listed in the dyld image info structure will all
-                // either be fixed up already, or they will all be off by a single
-                // slide amount that is determined by finding the first segment
-                // that is at file offset zero which also has bytes (a file size
-                // that is greater than zero) in the object file.
-                
-                // Determine the slide amount (if any)
-                const size_t num_sections = section_list->GetSize();
-                size_t sect_idx = 0;
-                for (sect_idx = 0; sect_idx < num_sections; ++sect_idx)
-                {
-                    // Iterate through the object file sections to find the
-                    // first section that starts of file offset zero and that
-                    // has bytes in the file...
-                    Section *section = section_list->GetSectionAtIndex (sect_idx).get();
-                    if (section)
-                    {
-                        if (m_section_load_list.SetSectionLoadAddress (section, section->GetFileAddress() + slide))
-                            changed = true;
-                    }
-                }
-            }
-        }
-    }
-    return changed;
-}
 
-
 //--------------------------------------------------------------
 // class Target::StopHook
 //--------------------------------------------------------------
Index: source/Core/Module.cpp
===================================================================
--- source/Core/Module.cpp	(revision 153535)
+++ source/Core/Module.cpp	(working copy)
@@ -1104,11 +1104,11 @@
 bool 
 Module::SetLoadAddress (Target &target, lldb::addr_t offset, bool &changed)
 {
-    changed = false;
-    ObjectFile *image_object_file = GetObjectFile();
-    if (image_object_file)
+    size_t num_loaded_sections = 0;
+    ObjectFile *objfile = GetObjectFile();
+    if (objfile)
     {
-        SectionList *section_list = image_object_file->GetSectionList ();
+        SectionList *section_list = objfile->GetSectionList ();
         if (section_list)
         {
             const size_t num_sections = section_list->GetSize();
@@ -1119,16 +1119,17 @@
                 // first section that starts of file offset zero and that
                 // has bytes in the file...
                 Section *section = section_list->GetSectionAtIndex (sect_idx).get();
-                if (section)
+                // Only load non-thread specific sections when given a slide
+                if (section && !section->IsThreadSpecific())
                 {
                     if (target.GetSectionLoadList().SetSectionLoadAddress (section, section->GetFileAddress() + offset))
-                        changed = true;
+                        ++num_loaded_sections;
                 }
             }
-            return sect_idx > 0;
         }
     }
-    return false;
+    changed = num_loaded_sections > 0;
+    return num_loaded_sections > 0;
 }
 
 
Index: source/Core/Section.cpp
===================================================================
--- source/Core/Section.cpp	(revision 153535)
+++ source/Core/Section.cpp	(working copy)
@@ -36,6 +36,8 @@
     m_file_size     (file_size),
     m_children      (),
     m_fake          (false),
+    m_encrypted     (false),
+    m_thread_specific (false),
     m_linked_section_wp(),
     m_linked_offset (0)
 {
@@ -65,6 +67,8 @@
     m_file_size     (file_size),
     m_children      (),
     m_fake          (false),
+    m_encrypted     (false),
+    m_thread_specific (false),
     m_linked_section_wp(),
     m_linked_offset (0)
 {
Index: source/API/SBTarget.cpp
===================================================================
--- source/API/SBTarget.cpp	(revision 153535)
+++ source/API/SBTarget.cpp	(working copy)
@@ -2086,12 +2086,23 @@
         }
         else
         {
-            target_sp->GetSectionLoadList().SetSectionLoadAddress (section.GetSP().get(), section_base_addr);
+            SectionSP section_sp (section.GetSP());
+            if (section_sp)
+            {
+                if (section_sp->IsThreadSpecific())
+                {
+                    sb_error.SetErrorString ("thread specific sections are not yet supported");
+                }
+                else
+                {
+                    target_sp->GetSectionLoadList().SetSectionLoadAddress (section_sp.get(), section_base_addr);
+                }
+            }
         }
     }
     else
     {
-        sb_error.SetErrorStringWithFormat ("invalid target");
+        sb_error.SetErrorString ("invalid target");
     }
     return sb_error;
 }
@@ -2132,31 +2143,18 @@
         ModuleSP module_sp (module.GetSP());
         if (module_sp)
         {
-            ObjectFile *objfile = module_sp->GetObjectFile();
-            if (objfile)
+            bool changed = false;
+            if (module_sp->SetLoadAddress (*target_sp, slide_offset, changed))
             {
-                SectionList *section_list = objfile->GetSectionList();
-                if (section_list)
+                // The load was successful, make sure that at least some sections
+                // changed before we notify that our module was loaded.
+                if (changed)
                 {
-                    const size_t num_sections = section_list->GetSize();
-                    for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx)
-                    {
-                        SectionSP section_sp (section_list->GetSectionAtIndex(sect_idx));
-                        if (section_sp)
-                            target_sp->GetSectionLoadList().SetSectionLoadAddress (section_sp.get(), section_sp->GetFileAddress() + slide_offset);
-                    }
+                    ModuleList module_list;
+                    module_list.Append(module_sp);
+                    target_sp->ModulesDidLoad (module_list);
                 }
-                else
-                {
-                    module_sp->GetFileSpec().GetPath (path, sizeof(path));
-                    sb_error.SetErrorStringWithFormat ("no sections in object file '%s'", path);
-                }
             }
-            else
-            {
-                module_sp->GetFileSpec().GetPath (path, sizeof(path));
-                sb_error.SetErrorStringWithFormat ("no object file for module '%s'", path);
-            }
         }
         else
         {


On Mar 26, 2012, at 2:21 AM, Dmitry Vyukov wrote:

> ping
> 
> On Tue, Feb 28, 2012 at 1:57 PM, Dmitry Vyukov <dvyukov at google.com> wrote:
> Hi,
> 
> I would like to ask you to review and land the following patch.
> It improves SBTarget::SetModuleLoadAddress() (at least for Linux) by skipping sections that are not actually mapped. I had to hack around .tbss section, because it has weird elf representation on Linux (from everything you see in the elf itself it is mapped, but it is actually not).
> 
> 
> Index: include/lldb/lldb-enumerations.h
> ===================================================================
> --- include/lldb/lldb-enumerations.h	(revision 151625)
> +++ include/lldb/lldb-enumerations.h	(working copy)
> @@ -467,6 +467,7 @@
>          eSectionTypeCode,
>          eSectionTypeContainer,              // The section contains child sections
>          eSectionTypeData,
> +        eSectionTypeThreadData,             // Thread-local data
>          eSectionTypeDataCString,            // Inlined C string data
>          eSectionTypeDataCStringPointers,    // Pointers to C string data
>          eSectionTypeDataSymbolAddress,      // Address of a symbol in the symbol table
> @@ -476,6 +477,7 @@
>          eSectionTypeDataPointers,
>          eSectionTypeDebug,
>          eSectionTypeZeroFill,
> +        eSectionTypeThreadZeroFill,         // Thread-local zero data
>          eSectionTypeDataObjCMessageRefs,    // Pointer to function pointer + selector
>          eSectionTypeDataObjCCFStrings,      // Objective C const CFString/NSString objects
>          eSectionTypeDWARFDebugAbbrev,
> Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> ===================================================================
> --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(revision 151625)
> +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp	(working copy)
> @@ -620,6 +620,8 @@
>              static ConstString g_sect_name_text (".text");
>              static ConstString g_sect_name_data (".data");
>              static ConstString g_sect_name_bss (".bss");
> +            static ConstString g_sect_name_tdata (".tdata");
> +            static ConstString g_sect_name_tbss (".tbss");
>              static ConstString g_sect_name_dwarf_debug_abbrev (".debug_abbrev");
>              static ConstString g_sect_name_dwarf_debug_aranges (".debug_aranges");
>              static ConstString g_sect_name_dwarf_debug_frame (".debug_frame");
> @@ -638,6 +640,8 @@
>              if      (name == g_sect_name_text)                  sect_type = eSectionTypeCode;
>              else if (name == g_sect_name_data)                  sect_type = eSectionTypeData;
>              else if (name == g_sect_name_bss)                   sect_type = eSectionTypeZeroFill;
> +            else if (name == g_sect_name_tdata)                 sect_type = eSectionTypeThreadData;
> +            else if (name == g_sect_name_tbss)                  sect_type = eSectionTypeThreadZeroFill;
>              else if (name == g_sect_name_dwarf_debug_abbrev)    sect_type = eSectionTypeDWARFDebugAbbrev;
>              else if (name == g_sect_name_dwarf_debug_aranges)   sect_type = eSectionTypeDWARFDebugAranges;
>              else if (name == g_sect_name_dwarf_debug_frame)     sect_type = eSectionTypeDWARFDebugFrame;
> Index: source/API/SBTarget.cpp
> ===================================================================
> --- source/API/SBTarget.cpp	(revision 151625)
> +++ source/API/SBTarget.cpp	(working copy)
> @@ -2010,7 +2010,19 @@
>                      {
>                          SectionSP section_sp (section_list->GetSectionAtIndex(sect_idx));
>                          if (section_sp)
> +                        {
> +                            // These are not actually mapped.
> +                            if (section_sp->GetFileAddress() == 0)
> +                                continue;
> +                            // Skip .tbss
> +                            // On Linux I observe that .tbss has some real virtual address
> +                            // and ALLOC flag, but the next section starts at the same address.
> +                            // It suggests that .tbss is not actually mapped
> +                            // (even if it is mapped, it should be uninteresting for us).
> +                            if (section_sp->GetType() == eSectionTypeThreadZeroFill)
> +                                continue;
>                              target_sp->GetSectionLoadList().SetSectionLoadAddress (section_sp.get(), section_sp->GetFileAddress() + slide_offset);
> +                        }
>                      }
>                  }
>                  else
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20120327/d1c423b6/attachment.html>


More information about the lldb-commits mailing list