This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [rfc / remote protocol] Remote shared library events


Hi Daniel, all:

First, let me say that I really like this patch.  I find
the segment generalization elegant.  Makes a lot of sense.

On 5/9/07, Daniel Jacobowitz <drow@false.org> wrote:

+static void
+solib_target_remove_one_solib (char *soname, int num_bases,
+                              CORE_ADDR *segment_bases)
+{
+  struct so_list **slot, *removed;
+
+  /* We should already have queried the target for shared libraries
+     before this point.  If we haven't, we may have just connected;
+     we'll be querying shortly.  */
+  if (!solibs_fetched)
+    return;
+
+  for (slot = &solib_start; *slot != NULL; slot = &(*slot)->next)
+    {
+      int i;
+
+      if (soname != NULL && strcmp (soname, (*slot)->so_name) != 0)
+       continue;
+
+      if (num_bases != (*slot)->lm_info->num_bases)
+       continue;
+
+      for (i = 0; i < num_bases; i++)
+       if (segment_bases[i] != (*slot)->lm_info->segment_bases[i])
+         break;
+      if (i < num_bases)
+       continue;
+
+      /* Got a match.  */
+      break;
+    }
+
+  if (*slot == NULL)
+    return;

This doesn't match what was documented: "The name, the segment offsets, or both may be used to specify which DLL to unload"

Specifically, if just the dll name comes along (numbases == 0),
it will fail to find the so.

Also, I see that you've changed the unloading related to the
old version.  In that version only one segment was
enough on unload.  Is it ever possible to have two sos with
segments loaded at the same address?

While we're on to it, why not drop the "TextSeg", and
"DataSeg" segment names?  You are hardcoding text to segment
0 and data to segment 1 on the gdb side
(remote.c:parse_load_response), and as you've already noted,
if some more segments need to be reported, then we'd have
to add name pairs for them.  Did you consider having
packets like these instead?:

load:Name=<hex(name)>,Seg_0=<load_addr>,Seg_1=<load_addr>,Seg_2=<load_addr>;

unload:Seg_0=<load_addr>;

... and have parse_load_response handle Seg_nnn generically?

hummm, isn't there the possibility that
'TextSeg == seg[0] && DataSeg == seg[1]' isn't true in some
cases, like when data is loaded before text in memory?


@@ -103,7 +107,11 @@ struct target_so_ops
        Convenience function for remote debuggers finding host libs.  */
     int (*find_and_open_solib) (char *soname,
         unsigned o_flags, char **temp_pathname);
-
+
+    void (*add_one_solib) (char *soname, int num_bases,
+                          CORE_ADDR *segment_bases);
+    void (*remove_one_solib) (char *soname, int num_bases,
+                             CORE_ADDR *segment_bases);

I see this in some places in gdb, and I've been meaning to ask. I see you're following status quo in that file, but why aren't these SONAME params in target_so_ops 'const char *' ? Shouldn't we strive for const correctness? Not really important for this patch, just curious.

Cheers,
Pedro Alves





Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]