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

Steve Pucci spucci at google.com
Tue Jan 28 07:19:19 PST 2014


OK, great, thanks Greg, I'll give it a go.

 - Steve


On Mon, Jan 27, 2014 at 4:31 PM, Greg Clayton <gclayton at apple.com> wrote:

> The first thing to do is just look at the section that has address of zero
> and see if it has any bits that the other don't or vice versa.
>
> I think the bit you are looking for is SHF_ALLOC.
>
> The "sh_flags" from the ELF section are indeed placed in the
> lldb_private::Section flags, so you should be able to do:
>
> for (section : sections)
> {
>     if (section->Test(SHF_ALLOC))
>     {
>         // Load this section
>     }
> }
>
>
>
>
>
> On Jan 27, 2014, at 4:23 PM, Steve Pucci <spucci at google.com> wrote:
>
> > OK, I understand, though I may need some help from someone with
> interpreting Section headers for Elf.  I'll let this group know if I have
> questions.
> >
> > Thanks again,
> >    Steve
> >
> >
> > On Mon, Jan 27, 2014 at 4:17 PM, Greg Clayton <gclayton at apple.com>
> wrote:
> >
> > On Jan 27, 2014, at 4:02 PM, Steve Pucci <spucci at google.com> wrote:
> >
> > > Thanks, Greg.
> > >
> > > I think it all makes sense, except for one bit:
> > >
> > > In ObjectFileELF::SetLoadAddress(), are you proposing I simply call
> Module::SetLoadAddress as it exists today?  That method walks through all
> sections and checks only section_sp->IsThreadSpecific() to decide whether
> to load the section, and there's no place to insert an ELF-specific check
> of the section to see if it's loadable.  Is that what you meant, or are you
> suggesting something else?
> >
> > Something else. When the ObjectFileELF parser parses the section
> headers, it places the flags (or it should if it isn't) into the flags
> field of the lldb_private::Section. So it should be able to use the flags
> from its sections to correctly in each lldb_private::Section, and correctly
> interpret them to know which sections need to be loaded and which don't. So
> let the ELF plugin that created the sections correctly interpret the flags
> it put into the sections.
> >
> > We then will need to change the Module::SetLoadAddress() to call this
> new ObjectFile function.
> >
> > >
> > > Instead of that I could have ObjectFileELF::SetLoadAddress iterate
> through the sections as UpdateLoadedSectionsCommon does below, OR I could
> somehow provide a callback to be called from Module::SetLoadAddress
> (perhaps by passing in the ObjectFile*).
> >
> > It should all be done in the ObjectFileELF::SetLoadAddress function.
> >
> > >
> > > Thanks,
> > >   Steve
> > >
> > >
> > >
> > > On Mon, Jan 27, 2014 at 3:14 PM, Greg Clayton <gclayton at apple.com>
> wrote:
> > >
> > > On Jan 27, 2014, at 3:05 PM, Greg Clayton <gclayton at apple.com> wrote:
> > >
> > > > Looks ok except for:
> > > >
> > > > This is ELF specific with the file address of zero, and it probably
> should more be done via flags and asking the section if it is loadable:
> > > >
> > > > +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);
> > > > +    }
> > > > +}
> > > >
> > > >
> > > > There is also a module function that does something similar to this,
> without the looking for the zero address:
> > > >
> > > > bool
> > > > Module::SetLoadAddress (Target &target, lldb::addr_t offset, bool
> &changed);
> > > >
> > > > So I would propose the following:
> > > >
> > > > Update DynamicLoader::UpdateLoadedSectionsCommon() to call into a
> new function that is a virtual function in ObjectFile:
> > > >
> > > > virtual bool SetLoadAddress (addr_t base_addr)
> > > > {
> > > >    return false;
> > > > }
> > > >
> > > > Then each object file (ObjectFileELF in your case) can choose to do
> the loading correctly given a single "base_addr":
> > > >
> > > > bool
> > > > ObjectFileELF::SetLoadAddress (addr_t base_addr)
> > > > {
> > > > }
> > > >
> > > > Then in ObjectFileELF::SetLoadAddress() you can use the section
> flags that were saved in the lldb_private::Section to properly determine
> which sections are loadable and which aren't. This function is for a rigid
> slide of all loadable sections.
> > > >
> > > > Does that make sense?
> > >
> > > I forgot the SetLoadAddress needs a target, and each object file
> already knows its module, so that doesn't need to be passed, it can be
> retrieved via the getter function:
> > >
> > > virtual bool SetLoadAddress (Target &target, addr_t base_addr)
> > > {
> > >    return false;
> > > }
> > >
> > > Then each object file (ObjectFileELF in your case) can choose to do
> the loading correctly given a single "base_addr":
> > >
> > > bool
> > > ObjectFileELF::SetLoadAddress (Target &target, addr_t base_addr)
> > > {
> > >      ModuleSP module_sp = GetModule();
> > >      if (module_sp)
> > >      {
> > >          ....
> > >      }
> > > }
> > >
> > >
> > >
> > > > Greg
> > > >
> > > >
> > > >
> > > > On Jan 27, 2014, at 2:32 PM, Steve Pucci <spucci at google.com> wrote:
> > > >
> > > >> 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
> > > >>
> > > >>
> > > >>
> <patch-FactorDynamicLibrary.txt>_______________________________________________
> > > >> lldb-dev mailing list
> > > >> lldb-dev at cs.uiuc.edu
> > > >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> > > >
> > > > _______________________________________________
> > > > lldb-dev mailing list
> > > > lldb-dev at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> > >
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140128/c19e286c/attachment.html>


More information about the lldb-dev mailing list