This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH RFA] solib.c relocation improvements
- To: Kevin Buettner <kevinb at cygnus dot com>
- Subject: Re: [PATCH RFA] solib.c relocation improvements
- From: Jim Blandy <jimb at zwingli dot cygnus dot com>
- Date: 30 Oct 2000 18:00:12 -0500
- Cc: gdb-patches at sourceware dot cygnus dot com
- References: <1001029002907.ZM8686@ocotillo.lan>
This change is approved.
Kevin Buettner <kevinb@cygnus.com> writes:
>
> The patch below is based on feedback from Peter Schauer regarding my
> solib.c reorg patch of last weekend. Specifically, Peter described
> some additional cleanup that would be necessary for making solib.c
> useful for platforms aside from the ones already using it. I used the
> following remarks from Peter as an outline for making the changes
> contained in the patch:
>
> > - This code from solib.c:solib_map_sections
> >
> > for (p = so->sections; p < so->sections_end; p++)
> > {
> > /* Relocate the section binding addresses as recorded in the shared
> > object's file by the base address to which the object was actually
> > mapped. */
> > p->addr += TARGET_SO_LM_ADDR (so);
> > p->endaddr += TARGET_SO_LM_ADDR (so);
> > so->lmend = max (p->endaddr, so->lmend);
> > if (STREQ (p->the_bfd_section->name, ".text"))
> > {
> > so->textsection = p;
> > }
> > }
> >
> > might be too specialized for future targets, as it assumes that all sections
> > are loaded with the same offset (perhaps I will find some time someday
> > to convert the horrible AIX shlib code to your scheme, and then we will
> > definitely need it).
> > It might be better to perform the relocation via a target_so_ops entry.
> >
> > - I am not sure if we shouldn't get rid of the TARGET_SO_LM_ADDR and
> > so->lmend crap altogether.
> >
> > solib_address could examine so->sections.
> >
> > info_sharedlibrary_command could put out the addresses of the textsection
> > (or of all sections, perhaps via a new `maint info sharedlibrary' command,
> > which I could have used quite often in the past).
> >
> > The lowest section handling in symbol_add_stub is questionable anyway
> > and needs some rethinking. I put in some debug output and ran the testsuite
> > on Solaris2 and Linux x86 targets, and it seems that all the business with
> > hacking sap->other[lowest_index].addr is not needed anyway, as
> > sap->other[lowest_index].addr == lowest_addr in all test cases.
> > I no longer have a SunOS4 platform handy, so perhaps it is needed there.
>
> I did my own examination of the lowest section handling code in
> symbol_add_stub() and concluded that it was completely unnecessary.
> Here's what it was doing:
>
> - It finds the "lowest" section. This is always taken to be the
> .text section if it exists. Otherwise it is the section with the
> lowest (unrelocated) address from BFD.
>
> In the case of the .text section, the lowest address was taken
> to be so->textsection->addr. Note that this is the relocated
> address of the .text section.
>
> In the case of one of the other sections, the lowest address is
> computed with the following expression:
>
> bfd_section_vma (so->abfd, lowest_sect) + TARGET_SO_LM_ADDR (so);
>
> I.e, it is the unrelocated address from the BFD section plus the
> address needed for relocation. Note that this value could have
> been obtained from the shared object's section table since
> solib_map_sections() already did the same work. (You have to
> look at build_section_table() and add_to_section_table() to
> see the bit regarding the BFD section vma though.)
>
> In either case, when we compute lowest_addr, it will have the same
> address as what's already in the section table. (This was also
> verified by Peter's empirical study. See above.)
>
> - A section_add_info struct is created from the shared object's
> section offsets. This is nothing more than putting the section
> offsets into a slightly different form so that we can pass them for
> purposes of doing symbol relocations to symbol_file_add().
>
> - Finally, the following line...
>
> sap->other[lowest_index].addr = lowest_addr;
>
> is attempting to assign the "lowest" address that we computed
> earlier to the corresponding address in the section_addr_info
> struct.
>
> But this code is WRONG. lowest_index is a BFD section index
> and if you look at the way that the section_addr_info struct
> is created, you'll see that it is possible for it to be created
> in such a way that the indices won't match those of the BFD
> section indices. (In particular, only those sections whose
> SEC_ALLOC or SEC_LOAD flags are included in these sections.)
>
> In order for this code to be correct, we'd need to have
> a loop which looks for the bfd section index.
>
> If it was correct, however, it would turn out that it isn't
> doing anything useful since the addr field is already set
> correctly.
>
> I've tested these changes on i686-pc-linux-gnu,
> sparc-sun-solaris2.5.1, and sparc-sun-sunos4.1.4. No regressions.
> Also, I've implemented a backend for AIX5/IA-64 which uses the
> TARGET_SO_RELOCATE_SECTION_ADDRESSES machinery for sections that are
> relocated by different amounts and it works quite well. I will add
> this file to the public repository this file as soon as IBM gives
> their approval.
>
> The one change that I haven't made from Peter's remarks above is
> to add a "maint info sharedlibrary" command. I agree that this
> would be useful, but Peter has sent me some other comments that
> have some bearing on this issue that should be addressed first.
>
> Okay to commit?
>
> Changes based on analysis from Peter Schauer:
> * solist.h (struct so_list): Remove field lmend.
> (struct target_so_ops): Remove field lm_addr. Add field
> relocate_section_addresses. Add comments for all fields
> in this structure
> (TARGET_SO_LM_ADDR): Remove.
> (TARGET_SO_RELOCATE_SECTION_ADDRESSES): New macro.
> * solib-svr4.c (svr4_relocate_section_addresses): New function.
> (_initialize_svr4_solib): Remove lm_addr initialization. Add
> initialization for relocate_section_addresses.
> * solib.c (solib_map_sections): Invoke
> TARGET_SO_RELOCATE_SECTION_ADDRESSES instead of using now
> defunct TARGET_SO_LM_ADDR to relocate the section addresses.
> Also, eliminate assignment to the lmend field since this
> field no longer exists.
> (symbol_add_stub): Remove machinery for determining the lowest
> section.
> (info_sharedlibrary_command): Print the text section starting
> and ending addresses.
> (solib_address): Don't use TARGET_SO_LM_ADDR, nor so->lmend to
> determine if an address is in a shared object. Instead, scan
> the section table and test against the starting and ending
> addresses for each section.
>
> Index: solib-svr4.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-svr4.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 solib-svr4.c
> --- solib-svr4.c 2000/10/24 20:05:35 1.1
> +++ solib-svr4.c 2000/10/28 22:51:30
> @@ -1567,12 +1567,20 @@ svr4_free_so (struct so_list *so)
> free (so->lm_info);
> }
>
> +static void
> +svr4_relocate_section_addresses (struct so_list *so,
> + struct section_table *sec)
> +{
> + sec->addr += LM_ADDR (so);
> + sec->endaddr += LM_ADDR (so);
> +}
> +
> static struct target_so_ops svr4_so_ops;
>
> void
> _initialize_svr4_solib (void)
> {
> - svr4_so_ops.lm_addr = LM_ADDR;
> + svr4_so_ops.relocate_section_addresses = svr4_relocate_section_addresses;
> svr4_so_ops.free_so = svr4_free_so;
> svr4_so_ops.clear_solib = svr4_clear_solib;
> svr4_so_ops.solib_create_inferior_hook = svr4_solib_create_inferior_hook;
> Index: solib.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 solib.c
> --- solib.c 2000/10/24 20:05:35 1.25
> +++ solib.c 2000/10/28 22:51:32
> @@ -180,9 +180,7 @@ solib_map_sections (PTR arg)
> /* Relocate the section binding addresses as recorded in the shared
> object's file by the base address to which the object was actually
> mapped. */
> - p->addr += TARGET_SO_LM_ADDR (so);
> - p->endaddr += TARGET_SO_LM_ADDR (so);
> - so->lmend = max (p->endaddr, so->lmend);
> + TARGET_SO_RELOCATE_SECTION_ADDRESSES (so, p);
> if (STREQ (p->the_bfd_section->name, ".text"))
> {
> so->textsection = p;
> @@ -248,9 +246,6 @@ symbol_add_stub (PTR arg)
> {
> register struct so_list *so = (struct so_list *) arg; /* catch_errs bogon */
> struct section_addr_info *sap;
> - CORE_ADDR lowest_addr = 0;
> - int lowest_index;
> - asection *lowest_sect = NULL;
>
> /* Have we already loaded this shared object? */
> ALL_OBJFILES (so->objfile)
> @@ -259,35 +254,9 @@ symbol_add_stub (PTR arg)
> return 1;
> }
>
> - /* Find the shared object's text segment. */
> - if (so->textsection)
> - {
> - lowest_addr = so->textsection->addr;
> - lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
> - lowest_index = lowest_sect->index;
> - }
> - else if (so->abfd != NULL)
> - {
> - /* If we didn't find a mapped non zero sized .text section, set
> - up lowest_addr so that the relocation in symbol_file_add does
> - no harm. */
> - lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
> - if (lowest_sect == NULL)
> - bfd_map_over_sections (so->abfd, find_lowest_section,
> - (PTR) &lowest_sect);
> - if (lowest_sect)
> - {
> - lowest_addr = bfd_section_vma (so->abfd, lowest_sect)
> - + TARGET_SO_LM_ADDR (so);
> - lowest_index = lowest_sect->index;
> - }
> - }
> -
> sap = build_section_addr_info_from_section_table (so->sections,
> so->sections_end);
>
> - sap->other[lowest_index].addr = lowest_addr;
> -
> so->objfile = symbol_file_add (so->so_name, so->from_tty,
> sap, 0, OBJF_SHARED);
> free_section_addr_info (sap);
> @@ -610,12 +579,17 @@ info_sharedlibrary_command (char *ignore
> }
>
> printf_unfiltered ("%-*s", addr_width,
> - local_hex_string_custom (
> - (unsigned long) TARGET_SO_LM_ADDR (so),
> - addr_fmt));
> + so->textsection != NULL
> + ? local_hex_string_custom (
> + (unsigned long) so->textsection->addr,
> + addr_fmt)
> + : "");
> printf_unfiltered ("%-*s", addr_width,
> - local_hex_string_custom ((unsigned long) so->lmend,
> - addr_fmt));
> + so->textsection != NULL
> + ? local_hex_string_custom (
> + (unsigned long) so->textsection->endaddr,
> + addr_fmt)
> + : "");
> printf_unfiltered ("%-12s", so->symbols_loaded ? "Yes" : "No");
> printf_unfiltered ("%s\n", so->so_name);
> }
> @@ -640,10 +614,7 @@ info_sharedlibrary_command (char *ignore
>
> Provides a hook for other gdb routines to discover whether or
> not a particular address is within the mapped address space of
> - a shared library. Any address between the base mapping address
> - and the first address beyond the end of the last mapping, is
> - considered to be within the shared library address space, for
> - our purposes.
> + a shared library.
>
> For example, this routine is called at one point to disable
> breakpoints which are in shared libraries that are not currently
> @@ -657,8 +628,13 @@ solib_address (CORE_ADDR address)
>
> for (so = so_list_head; so; so = so->next)
> {
> - if (TARGET_SO_LM_ADDR (so) <= address && address < so->lmend)
> - return (so->so_name);
> + struct section_table *p;
> +
> + for (p = so->sections; p < so->sections_end; p++)
> + {
> + if (p->addr <= address && address < p->endaddr)
> + return (so->so_name);
> + }
> }
>
> return (0);
> Index: solist.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/solist.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 solist.h
> --- solist.h 2000/10/24 20:05:35 1.1
> +++ solist.h 2000/10/28 22:51:32
> @@ -54,7 +54,6 @@ struct so_list
> are initialized when we actually add it to our symbol tables. */
>
> bfd *abfd;
> - CORE_ADDR lmend; /* upper addr bound of mapped object */
> char symbols_loaded; /* flag: symbols read in yet? */
> char from_tty; /* flag: print msgs? */
> struct objfile *objfile; /* objfile for loaded lib */
> @@ -65,12 +64,30 @@ struct so_list
>
> struct target_so_ops
> {
> - CORE_ADDR (*lm_addr) (struct so_list *so);
> + /* Adjust the section binding addresses by the base address at
> + which the object was actually mapped. */
> + void (*relocate_section_addresses) (struct so_list *so,
> + struct section_table *);
> +
> + /* Free the the link map info and any other private data
> + structures associated with a so_list entry. */
> void (*free_so) (struct so_list *so);
> +
> + /* Reset or free private data structures not associated with
> + so_list entries. */
> void (*clear_solib) (void);
> +
> + /* Target dependent code to run after child process fork. */
> void (*solib_create_inferior_hook) (void);
> +
> + /* Do additional symbol handling, lookup, etc. after symbols
> + for a shared object have been loaded. */
> void (*special_symbol_handling) (void);
> +
> + /* Construct a list of the currently loaded shared objects. */
> struct so_list *(*current_sos) (void);
> +
> + /* Find, open, and read the symbols for the main executable. */
> int (*open_symbol_file_object) (void *from_ttyp);
> };
>
> @@ -79,7 +96,8 @@ void free_so (struct so_list *so);
> /* FIXME: gdbarch needs to control this variable */
> extern struct target_so_ops *current_target_so_ops;
>
> -#define TARGET_SO_LM_ADDR (current_target_so_ops->lm_addr)
> +#define TARGET_SO_RELOCATE_SECTION_ADDRESSES \
> + (current_target_so_ops->relocate_section_addresses)
> #define TARGET_SO_FREE_SO (current_target_so_ops->free_so)
> #define TARGET_SO_CLEAR_SOLIB (current_target_so_ops->clear_solib)
> #define TARGET_SO_SOLIB_CREATE_INFERIOR_HOOK \
>
>