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: [patch] Implement qXfer:libraries for Linux/gdbserver


On Mon, 08 Aug 2011 20:55:40 +0200, Paul Pluzhnikov wrote:
> On Mon, Aug 8, 2011 at 11:36 AM, Jan Kratochvil > <jan.kratochvil@redhat.com> wrote:
[...]
> The trouble is that solib-target.c's notion of what info is in solib->lm_info
> is radically different from solib-svr4.c's notion, each having a private
> and distinct 'struct lm_info'. Un-tangling that seemed troublesome.

I agree the solib-svr4.c split looks a bit complex to me.


> > It would make libexpat mandatory even for the linux-nat.c usage so libexpat
> > could be bundler into the sourceware tree.
> 
> I think it's highly desirable to keep the current solib-srv4 code working
> (e.g. in case you have a gdbserver that answers with empty list; either
> because it is old, or because it's for a platform that hasn't been updated).
> 
> Given above, you can always fall back to it when libexpat is not detected.

I tried to copy how windows-nat.c works, which even in local mode produces and
consumes the XML document, exchanged internally as TARGET_OBJECT_LIBRARIES.
Therefore the core svr4_current_sos function would also produce
TARGET_OBJECT_LIBRARIES XML document, even in local run.

Sure other solutions exist but this unifies the remote and local codepath the
most.

I do not think it is any problem to require + bundle libexpat, there was even
some FAQ of people with broken GDB for ARM remote targets before some proper
error messages were added indicating libexpat is missing for GDB.


> The producers would have to stay separate anyway, as the details of getting
> the list differ greatly.

While it is a detail it saves 46 LoC (Lines of Code) and it IMO also makes the
code more readable.  Getting the list and producing the XML are two different
parts of code.


> Yes, the two isolated consumers are a bit bothersome.

Unfortunately the solib-svr4.c part is not so trivial as this one but IMO it
should be also done, I could get to it only later.


Thanks,
Jan


gdb/gdbserver/
2011-08-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* inferiors.c (clear_all_dlls): New function.
	(clear_inferiors): Move there the code from here, call it here.
	* linux-low.c (linux_qxfer_libraries): Rename to ...
	(linux_refresh_libraries): ... here.  Drop all the XML handling.  Call
	clear_all_dlls and loaded_dll instead.  Clear DLLS_CHANGED.
	(linux_target_ops): Rename linux_qxfer_libraries to
	linux_refresh_libraries.
	* server.c (handle_qxfer_libraries): Likewise.  Continue execution even
	if REFRESH_LIBRARIES exists.
	* server.h (clear_all_dlls): New prototype.
	* target.h (struct target_ops): Rename linux_qxfer_libraries to
	linux_refresh_libraries.  Drop its parameters and return type.

--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -312,14 +312,22 @@ unloaded_dll (const char *name, CORE_ADDR base_addr)
 #define clear_list(LIST) \
   do { (LIST)->head = (LIST)->tail = NULL; } while (0)
 
+/* Clear ALL_DLLS.  */
+
 void
-clear_inferiors (void)
+clear_all_dlls (void)
 {
-  for_each_inferior (&all_threads, free_one_thread);
   for_each_inferior (&all_dlls, free_one_dll);
+  clear_list (&all_dlls);
+}
 
+void
+clear_inferiors (void)
+{
+  for_each_inferior (&all_threads, free_one_thread);
   clear_list (&all_threads);
-  clear_list (&all_dlls);
+
+  clear_all_dlls ();
 
   current_inferior = NULL;
 }
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4939,16 +4939,13 @@ struct link_map_offsets
 
 /* Construct qXfer:libraries:read reply.  */
 
-static int
-linux_qxfer_libraries (const char *annex, unsigned char *readbuf,
-		       unsigned const char *writebuf,
-		       CORE_ADDR offset, int len)
+static void
+linux_refresh_libraries (void)
 {
-  char *document;
-  unsigned document_len;
   struct process_info_private *const priv = current_process ()->private;
   char filename[PATH_MAX];
-  int pid, is_elf64;
+  int pid, is_elf64, ptr_size, r_version;
+  CORE_ADDR lm_addr, lm_prev, l_name, l_addr, l_next, l_prev;
 
   static const struct link_map_offsets lmo_32bit_offsets =
     {
@@ -4971,34 +4968,17 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf,
     };
   const struct link_map_offsets *lmo;
 
-  if (writebuf != NULL)
-    return -2;
-  if (readbuf == NULL)
-    return -1;
-
   pid = lwpid_of (get_thread_lwp (current_inferior));
   xsnprintf (filename, sizeof filename, "/proc/%d/exe", pid);
   is_elf64 = elf_64_file_p (filename);
   lmo = is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets;
+  ptr_size = is_elf64 ? 8 : 4;
 
   if (priv->r_debug == 0)
     priv->r_debug = get_r_debug (pid, is_elf64);
 
   if (priv->r_debug == (CORE_ADDR) -1 || priv->r_debug == 0)
-    {
-      document = xstrdup ("<library-list/>\n");
-    }
-  else
-    {
-      int allocated = 1024;
-      char *p;
-      const int ptr_size = is_elf64 ? 8 : 4;
-      CORE_ADDR lm_addr, lm_prev, l_name, l_addr, l_next, l_prev;
-      int r_version;
-
-      document = xmalloc (allocated);
-      strcpy (document, "<library-list>");
-      p = document + strlen (document);
+    return;
 
       r_version = 0;
       if (linux_read_memory (priv->r_debug + lmo->r_version_offset,
@@ -5007,7 +4987,7 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf,
 	  || r_version != 1)
 	{
 	  warning ("unexpected r_debug version %d", r_version);
-	  goto done;
+	  return;
 	}
 
       if (read_one_ptr (priv->r_debug + lmo->r_map_offset,
@@ -5015,9 +4995,11 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf,
 	{
 	  warning ("unable to read r_map from 0x%lx",
 		   (long) priv->r_debug + lmo->r_map_offset);
-	  goto done;
+	  return;
 	}
 
+  clear_all_dlls ();
+
       lm_prev = 0;
       while (read_one_ptr (lm_addr + lmo->l_name_offset,
 			   &l_name, ptr_size) == 0
@@ -5042,33 +5024,7 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf,
 	  libname[0] = '\0';
 	  linux_read_memory (l_name, libname, sizeof (libname));
 	  if (libname[0] != '\0')
-	    {
-	      size_t len = strlen ((char *) libname);
-	      char *name;
-
-	      while (allocated < p - document + len + 100)
-		{
-		  /* Expand to guarantee sufficient storage.  */
-		  uintptr_t document_len = p - document;
-
-		  document = xrealloc (document, 2 * allocated);
-		  allocated *= 2;
-		  p = document + document_len;
-		}
-
-	      strcpy (p, "<library name=\"");
-	      p = p + strlen (p);
-	      name = xml_escape_text ((char *) libname);
-	      strcpy (p, name);
-	      free (name);
-	      p = p + strlen (p);
-	      strcpy (p, "\"><segment address=\"");
-	      p = p + strlen (p);
-	      sprintf (p, "0x%lx", (long) l_addr);
-	      p = p + strlen (p);
-	      strcpy (p, "\"/></library>");
-	      p = p + strlen (p);
-	    }
+	    loaded_dll ((const char *) libname, l_addr);
 
 	  if (l_next == 0)
 	    break;
@@ -5076,18 +5032,9 @@ linux_qxfer_libraries (const char *annex, unsigned char *readbuf,
 	  lm_prev = lm_addr;
 	  lm_addr = l_next;
 	}
-    done:
-      strcpy (p, "</library-list>");
-    }
 
-  document_len = strlen (document + offset);
-  if (len > document_len)
-    len = document_len;
-
-  memcpy (readbuf, document + offset, len);
-  xfree (document);
-
-  return len;
+  /* The library notification response is not expected for GNU/Linux.  */
+  dlls_changed = 0;
 }
 
 
@@ -5150,7 +5097,7 @@ static struct target_ops linux_target_ops = {
   linux_stabilize_threads,
   linux_install_fast_tracepoint_jump_pad,
   linux_emit_ops,
-  linux_qxfer_libraries
+  linux_refresh_libraries
 };
 
 static void
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -918,8 +918,8 @@ handle_qxfer_libraries (const char *annex,
   if (annex[0] != '\0' || !target_running ())
     return -1;
 
-  if (the_target->qxfer_libraries != NULL)
-    return the_target->qxfer_libraries (annex, readbuf, writebuf, offset, len);
+  if (the_target->refresh_libraries != NULL)
+    the_target->refresh_libraries ();
 
   /* Over-estimate the necessary memory.  Assume that every character
      in the library name must be escaped.  */
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -260,6 +260,7 @@ ptid_t thread_id_to_gdb_id (ptid_t);
 ptid_t thread_to_gdb_id (struct thread_info *);
 ptid_t gdb_id_to_thread_id (ptid_t);
 struct thread_info *gdb_id_to_thread (unsigned int);
+void clear_all_dlls (void);
 void clear_inferiors (void);
 struct inferior_list_entry *find_inferior
      (struct inferior_list *,
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -374,10 +374,8 @@ struct target_ops
      Returns NULL if bytecode compilation is not supported.  */
   struct emit_ops *(*emit_ops) (void);
 
-  /* Read solib info.  */
-  int (*qxfer_libraries) (const char *annex, unsigned char *readbuf,
-			  unsigned const char *writebuf,
-			  CORE_ADDR offset, int len);
+  /* Refresh ALL_DLLS.  */
+  void (*refresh_libraries) (void);
 };
 
 extern struct target_ops *the_target;


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