Re: unloading shared objects

[Kevin Buettner <kevinb at cygnus dot com>]

On Mar 10,  4:15pm, Jim Blandy wrote:

> In case some enterprising soul wants to take it the rest of the way
> while I'm gone, I've enclosed my patches below.  I think the basic
> logic is okay; known issues are:
> 
> - free_objfile calls CLEAR_SOLIB, which isn't what we want, I think.
> - Selecting a core file and attaching to a process both add the shared
>   libraries' sections to the target_ops structure.  When we unload a
>   shared library, we close the BFD those sections refer to.  We
>   need to remove those sections from the target_ops structure.

I've been looking at solib.c recently also, though for different
reasons.  I was attempting to find the address of the .dynamic section
for various objfiles, but the start and end addresses were not
properly relocated (for shared objects).  I discovered that solib.c
did have the correct start and end addresses, but no means to get at
them.

I decided that I didn't really want to get at them; what I really
wanted was the section addresses in the objfile struct to be correct. 
There may be a better way to make them correct, but it seemed to me
that the easiest way was to simply propagate the values after creating
the so_list struct (and mapping the sections).  I've appended my patch
to the end of this message.

I've looked over your patches to make sure that we aren't working
at cross purposes and things look good to me.

Here's another issue...  It would be really nice if there were a way
to see the sections and their addresses for the various shared
libraries.  At present, if you say "info target" (or "info files" -
they do the same thing), you only get to see the sections associated
with the main executable.  It would have saved me a lot of time if
there'd been an easy way to see where each section of the shared
library was loaded.  (Hmm... maybe we could leave "info target"
as is and make "info files" give information about the current
target AND all of the shared objects that've been loaded.)

> And finally:
> 
> - Should solib.c be maintaining its own list of shared objects at all,
>   or should it always retrieve the full link map from the inferior,
>   and use the objfile list itself as our record of what we know about?
>   In other words, what does so_list_head offer that object_files doesn't
>   do better?

I have not studied the code as much as I would like to render such
opinions, but what the heck...  It looks to me that there is needless
duplication between these two lists.  Worse still, the API for getting
at a struct so_list is minimal (almost non-existent). 

If struct so_list needs to persist at all, it seems to me that it
could be pared down considerably since it contains a pointer to the
corresponding struct objfile.  And if you look at struct objfile, it
contains similar information.  (In an earlier (unsent) email I
congratulated you on removing so_name and abfd from the struct.  Then
I noticed that I was misreading the context diff and these fields
(plus some new ones) were still there.  Oh, well.)

> Anyway, I'll finish these when I get back, if nobody else does.  I'd
> love comments on the changes.

I like the fact that you've removed find_solib() and have
redistributed its functionality among a number of new functions.  I
didn't mind find_solib's way of iterating over the so_list entries;
what I did mind was the fact that it was also responsible for reading
the link map.  I put a breakpoint in it to watch what was going on and
was amazed to see that we were attempting to read the target's memory
from find_solib() while in clear_solib():

    Breakpoint 3, find_solib (so_list_ptr=0x0) at ../../src/gdb/solib.c:983
    983       struct so_list *so_list_next = NULL;
    (top-gdb) bt 9
    #0  find_solib (so_list_ptr=0x0) at ../../src/gdb/solib.c:983
    #1  0x80b3d19 in solib_address (address=134513782)
	at ../../src/gdb/solib.c:1353
    #2  0x80f4cd5 in disable_breakpoints_in_shlibs (silent=1)
	at ../../src/gdb/breakpoint.c:4027
    #3  0x80b3d87 in clear_solib () at ../../src/gdb/solib.c:1395
    #4  0x80c98b9 in free_objfile (objfile=0x8257008)
	at ../../src/gdb/objfiles.c:440
    #5  0x80ca1c0 in objfile_purge_solibs () at ../../src/gdb/objfiles.c:751
    #6  0x8098779 in run_command (args=0x0, from_tty=1)
	at ../../src/gdb/infcmd.c:273

(That just can't be right.)

Anyway, here's my patch.  If you (Jim B) can integrate it with your
patches, fine.  Otherwise, I'll submit an RFA for it to gdb-patches
after you finish your changes.

	* solib.c (solib_sync_sections): New function.
	(symbol_add_stub): Call solib_sync_sections().


Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.2
diff -u -p -r1.2 solib.c
--- solib.c	2000/03/06 18:04:56	1.2
+++ solib.c	2000/03/12 05:05:50
@@ -210,6 +210,8 @@ solib_add_common_symbols PARAMS ((struct
 
 #endif
 
+static void solib_sync_sections (struct so_list *);
+
 void _initialize_solib PARAMS ((void));
 
 /* If non-zero, this is a prefix that will be added to the front of the name
@@ -1113,6 +1115,7 @@ symbol_add_stub (arg)
   so->objfile =
     symbol_file_add (so->so_name, so->from_tty,
 		     &section_addrs, 0, OBJF_SHARED);
+  solib_sync_sections (so);
   return (1);
 }
 
@@ -1927,6 +1930,75 @@ sharedlibrary_command (args, from_tty)
 }
 
 #endif /* HAVE_LINK_H */
+
+
+/*
+
+   LOCAL FUNCTION
+
+   solib_sync_sections - propagate section addresses to corresponding objfile
+
+   SYNOPSIS
+
+   static void solib_sync_sections (struct so_list *so)
+
+   DESCRIPTION
+
+   The machinery contained within this file will compute accurate
+   starting and ending section addresses.  When the objfile
+   corresponding to a particular struct so_list * entry is created, it
+   will not necessarily get the section addresses correct.  This
+   function propagates the correct addresses from a so_list struct
+   to an objfile struct.
+
+ */
+
+static void
+solib_sync_sections (struct so_list *so)
+{
+  struct objfile *of = so->objfile;
+  struct obj_section *osect;
+  int num_ssects, num_osects;
+
+  num_ssects = so->sections_end - so->sections;
+
+  ALL_OBJFILE_OSECTIONS (of, osect)
+    {
+      struct section_table *ssect;
+      int idx;
+
+      /* Find the section of the same name in the so_list struct.
+         It will likely be at the same index, so try there first. */
+      idx = osect - of->sections;
+      ssect = NULL;
+      if (idx < num_ssects 
+          && strcmp (so->sections[idx].the_bfd_section->name,
+	             osect->the_bfd_section->name) == 0)
+	{
+	  ssect = so->sections + idx;
+	}
+
+      /* If it wasn't found at the same index, try again via a
+         linear search */
+      for (idx = 0; ssect == NULL && idx < num_ssects; idx++)
+	{
+	  if (strcmp (so->sections[idx].the_bfd_section->name,
+	              osect->the_bfd_section->name) == 0)
+	    {
+	      ssect = so->sections + idx;
+	    }
+	}
+
+      /* If we found a corresponding section, then propagate the
+         the section addresses from ssect to osect */
+      if (ssect != NULL)
+	{
+	  osect->addr = ssect->addr;
+	  osect->endaddr = ssect->endaddr;
+	}
+    }
+}
+
 
 void
 _initialize_solib ()