Re: [patch 6/6] gdbserver build-id attribute generator

[Gary Benson <gbenson@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

Aleksandar Ristovski wrote:
> On 13-04-09 07:29 AM, Gary Benson wrote:
> > Aleksandar Ristovski wrote:
> > > (linux_qxfer_libraries_svr4): Add optional build-id attribute
> > > to reply XML document.
> > 
> > For the record this is the only part of your patchset that clashes
> > with the gdbserver work I'm doing.  It's not an *awful* clash, but
> > it will require a manual merge.  I'm happy to do that if necessary.
> 
> Thank you for your update.  I'm sorry for the clash. If there is
> something obvious I can do in this patch to reduce the clash let me
> know.

There's nothing obvious, and you don't need to apologise.  We just
both happen to be working in the same area at the same time.

> BTW, I am not familiar with your patch, if you want you can give me
> an overview here or offline.

The gdbserver side of it is that GDB can pass arguments in the annex
of qXfer:libraries-svr4:read+ to instruct the remote to transfer the
list starting from a specific link map rather than sending the whole
list.

I've attached my work-in-progress patch for the gdbserver side, but
you can see the full thing in the gbenson/rtld-probes branch in archer
git if you like.  A lot of the code is actually the same, but somewhat
reordered and with indentation changes which makes the patch harder to
read.

Your work seems closer to being accepted than mine, so I imagine what
will happen is that you'll commit your patches and I'll rebase mine
on top of that.  I have no problem with that.

Cheers,
Gary

--
http://gbenson.net/

[-- Attachment #2: gdbserver-changes.patch --]
[-- Type: text/plain, Size: 8812 bytes --]

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 72c51e0..beb3b8f 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5677,6 +5677,12 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
     };
   const struct link_map_offsets *lmo;
   unsigned int machine;
+  int ptr_size;
+  CORE_ADDR lm_addr = 0, lm_prev = 0;
+  int allocated = 1024;
+  char *p;
+  CORE_ADDR l_name, l_addr, l_ld, l_next, l_prev;
+  int header_done = 0;
 
   if (writebuf != NULL)
     return -2;
@@ -5687,128 +5693,146 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
   xsnprintf (filename, sizeof filename, "/proc/%d/exe", pid);
   is_elf64 = elf_64_file_p (filename, &machine);
   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 (annex[0] == '\0')
+    {
+      int r_version = 0;
 
-  /* We failed to find DT_DEBUG.  Such situation will not change for this
-     inferior - do not retry it.  Report it to GDB as E01, see for the reasons
-     at the GDB solib-svr4.c side.  */
-  if (priv->r_debug == (CORE_ADDR) -1)
-    return -1;
+      if (priv->r_debug == 0)
+	priv->r_debug = get_r_debug (pid, is_elf64);
 
-  if (priv->r_debug == 0)
-    {
-      document = xstrdup ("<library-list-svr4 version=\"1.0\"/>\n");
+      /* We failed to find DT_DEBUG.  Such situation will not change
+	 for this inferior - do not retry it.  Report it to GDB as
+	 E01, see for the reasons at the GDB solib-svr4.c side.  */
+      if (priv->r_debug == (CORE_ADDR) -1)
+	return -1;
+
+      if (priv->r_debug != 0)
+	{
+	  if (linux_read_memory (priv->r_debug + lmo->r_version_offset,
+				 (unsigned char *) &r_version,
+				 sizeof (r_version)) != 0
+	      || r_version != 1)
+	    {
+	      warning ("unexpected r_debug version %d", r_version);
+	    }
+	  else if (read_one_ptr (priv->r_debug + lmo->r_map_offset,
+				 &lm_addr, ptr_size) != 0)
+	    {
+	      warning ("unable to read r_map from 0x%lx",
+		       (long) priv->r_debug + lmo->r_map_offset);
+	    }
+	}
     }
   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_ld, l_next, l_prev;
-      int r_version, header_done = 0;
-
-      document = xmalloc (allocated);
-      strcpy (document, "<library-list-svr4 version=\"1.0\"");
-      p = document + strlen (document);
-
-      r_version = 0;
-      if (linux_read_memory (priv->r_debug + lmo->r_version_offset,
-			     (unsigned char *) &r_version,
-			     sizeof (r_version)) != 0
-	  || r_version != 1)
+      while (annex[0] != '\0')
 	{
-	  warning ("unexpected r_debug version %d", r_version);
-	  goto done;
+	  const char *sep;
+	  CORE_ADDR *addrp;
+	  int len;
+
+	  sep = strchr (annex, '=');
+	  if (!sep)
+	    break;
+
+	  len = sep - annex;
+	  if (len == 5 && !strncmp (annex, "start", 5))
+	    addrp = &lm_addr;
+	  else if (len == 4 && !strncmp (annex, "prev", 4))
+	    addrp = &lm_prev;
+	  else
+	    {
+	      annex = strchr (sep, ';');
+	      if (!annex)
+		break;
+	      annex++;
+	      continue;
+	    }
+
+	  annex = decode_address_to_semicolon (addrp, sep + 1);
 	}
+    }
 
-      if (read_one_ptr (priv->r_debug + lmo->r_map_offset,
-			&lm_addr, ptr_size) != 0)
+  document = xmalloc (allocated);
+  strcpy (document, "<library-list-svr4 version=\"1.0\"");
+  p = document + strlen (document);
+
+  while (lm_addr
+	 && read_one_ptr (lm_addr + lmo->l_name_offset,
+			  &l_name, ptr_size) == 0
+	 && read_one_ptr (lm_addr + lmo->l_addr_offset,
+			  &l_addr, ptr_size) == 0
+	 && read_one_ptr (lm_addr + lmo->l_ld_offset,
+			  &l_ld, ptr_size) == 0
+	 && read_one_ptr (lm_addr + lmo->l_prev_offset,
+			  &l_prev, ptr_size) == 0
+	 && read_one_ptr (lm_addr + lmo->l_next_offset,
+			  &l_next, ptr_size) == 0)
+    {
+      unsigned char libname[PATH_MAX];
+
+      if (lm_prev != l_prev)
 	{
-	  warning ("unable to read r_map from 0x%lx",
-		   (long) priv->r_debug + lmo->r_map_offset);
-	  goto done;
+	  warning ("Corrupted shared library list: 0x%lx != 0x%lx",
+		   (long) lm_prev, (long) l_prev);
+	  break;
 	}
 
-      lm_prev = 0;
-      while (read_one_ptr (lm_addr + lmo->l_name_offset,
-			   &l_name, ptr_size) == 0
-	     && read_one_ptr (lm_addr + lmo->l_addr_offset,
-			      &l_addr, ptr_size) == 0
-	     && read_one_ptr (lm_addr + lmo->l_ld_offset,
-			      &l_ld, ptr_size) == 0
-	     && read_one_ptr (lm_addr + lmo->l_prev_offset,
-			      &l_prev, ptr_size) == 0
-	     && read_one_ptr (lm_addr + lmo->l_next_offset,
-			      &l_next, ptr_size) == 0)
+      /* Not checking for error because reading may stop before
+	 we've got PATH_MAX worth of characters.  */
+      libname[0] = '\0';
+      linux_read_memory (l_name, libname, sizeof (libname) - 1);
+      libname[sizeof (libname) - 1] = '\0';
+      if (libname[0] != '\0')
 	{
-	  unsigned char libname[PATH_MAX];
+	  /* 6x the size for xml_escape_text below.  */
+	  size_t len = 6 * strlen ((char *) libname);
+	  char *name;
 
-	  if (lm_prev != l_prev)
+	  if (!header_done)
 	    {
-	      warning ("Corrupted shared library list: 0x%lx != 0x%lx",
-		       (long) lm_prev, (long) l_prev);
-	      break;
+	      /* Terminate `<library-list-svr4'.  */
+	      *p++ = '>';
+	      header_done = 1;
 	    }
 
-	  /* Not checking for error because reading may stop before
-	     we've got PATH_MAX worth of characters.  */
-	  libname[0] = '\0';
-	  linux_read_memory (l_name, libname, sizeof (libname) - 1);
-	  libname[sizeof (libname) - 1] = '\0';
-	  if (libname[0] != '\0')
+	  while (allocated < p - document + len + 200)
 	    {
-	      /* 6x the size for xml_escape_text below.  */
-	      size_t len = 6 * strlen ((char *) libname);
-	      char *name;
-
-	      if (!header_done)
-		{
-		  /* Terminate `<library-list-svr4'.  */
-		  *p++ = '>';
-		  header_done = 1;
-		}
-
-	      while (allocated < p - document + len + 200)
-		{
-		  /* Expand to guarantee sufficient storage.  */
-		  uintptr_t document_len = p - document;
-
-		  document = xrealloc (document, 2 * allocated);
-		  allocated *= 2;
-		  p = document + document_len;
-		}
+	      /* Expand to guarantee sufficient storage.  */
+	      uintptr_t document_len = p - document;
 
-	      name = xml_escape_text ((char *) libname);
-	      p += sprintf (p, "<library name=\"%s\" lm=\"0x%lx\" "
-			       "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
-			    name, (unsigned long) lm_addr,
-			    (unsigned long) l_addr, (unsigned long) l_ld);
-	      free (name);
-	    }
-	  else if (lm_prev == 0)
-	    {
-	      sprintf (p, " main-lm=\"0x%lx\"", (unsigned long) lm_addr);
-	      p = p + strlen (p);
+	      document = xrealloc (document, 2 * allocated);
+	      allocated *= 2;
+	      p = document + document_len;
 	    }
 
-	  if (l_next == 0)
-	    break;
-
-	  lm_prev = lm_addr;
-	  lm_addr = l_next;
+	  name = xml_escape_text ((char *) libname);
+	  p += sprintf (p, "<library name=\"%s\" lm=\"0x%lx\" "
+			"l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
+			name, (unsigned long) lm_addr,
+			(unsigned long) l_addr, (unsigned long) l_ld);
+	  free (name);
 	}
-    done:
-      if (!header_done)
+      else if (lm_prev == 0)
 	{
-	  /* Empty list; terminate `<library-list-svr4'.  */
-	  strcpy (p, "/>");
+	  sprintf (p, " main-lm=\"0x%lx\"", (unsigned long) lm_addr);
+	  p = p + strlen (p);
 	}
-      else
-	strcpy (p, "</library-list-svr4>");
+
+      lm_prev = lm_addr;
+      lm_addr = l_next;
     }
 
+  if (!header_done)
+    {
+      /* Empty list; terminate `<library-list-svr4'.  */
+      strcpy (p, "/>");
+    }
+  else
+    strcpy (p, "</library-list-svr4>");
+
   document_len = strlen (document);
   if (offset < document_len)
     document_len -= offset;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 6bb36d8..0a8f68b 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1115,8 +1115,7 @@ handle_qxfer_libraries_svr4 (const char *annex,
   if (writebuf != NULL)
     return -2;
 
-  if (annex[0] != '\0' || !target_running ()
-      || the_target->qxfer_libraries_svr4 == NULL)
+  if (!target_running () || the_target->qxfer_libraries_svr4 == NULL)
     return -1;
 
   return the_target->qxfer_libraries_svr4 (annex, readbuf, writebuf, offset, len);
@@ -1743,7 +1742,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	       PBUFSIZ - 1);
 
       if (the_target->qxfer_libraries_svr4 != NULL)
-	strcat (own_buf, ";qXfer:libraries-svr4:read+");
+	strcat (own_buf, ";qXfer:libraries-svr4:read+"
+		";augmented-libraries-svr4-read+");
       else
 	{
 	  /* We do not have any hook to indicate whether the non-SVR4 target