[PATCH RFA] solib.c relocation improvements

[Kevin Buettner <kevinb at cygnus dot com>]

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 \