[v12,06/32] Validate symbol file using build-id

Message ID 20150821212057.6673.70817.stgit@host1.jankratochvil.net
State New, archived
Headers

Commit Message

Jan Kratochvil Aug. 21, 2015, 9:20 p.m. UTC
  Consumer part of the "build-id" attribute.

gdb/ChangeLog
2015-07-15  Aleksandar Ristovski  <aristovski@qnx.com
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Validate symbol file using build-id.
	* NEWS (Changes since GDB 7.10): Add 'set validate-build-id'
	and 'show validate-build-id'.  Add build-id attribute.
	* build-id.c: Include gdbcmd.h.
	(validate_build_id, show_validate_build_id, _initialize_build_id): New.
	* build-id.h (validate_build_id): New declaration.
	* solib-svr4.c: Include rsp-low.h.
	(svr4_copy_library_list): Duplicate field build_id.
	(library_list_start_library): Parse 'build-id' attribute.
	(svr4_library_attributes): Add 'build-id' attribute.
	* solib.c (free_so): Free build_id.
	* solist.h (struct so_list): New fields 'build_idsz' and 'build_id'.
---
 0 files changed
  

Comments

Eli Zaretskii Aug. 22, 2015, 7:23 a.m. UTC | #1
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Date: Fri, 21 Aug 2015 23:20:57 +0200
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -35,6 +35,20 @@ show debug bfd-cache
>    The "/m" option is now considered deprecated: its "source-centric"
>    output hasn't proved useful in practice.
>  
> +* New options
> +
> +set validate-build-id (on|off)
> +show validate-build-id
> +  Inferior shared library and symbol file may contain unique build-id.
> +  If both build-ids are present but they do not match then this setting
> +  enables (off) or disables (on) loading of such symbol file.
> +
> +* New features in the GDB remote stub, GDBserver
> +
> +  ** library-list-svr4 contains also optional attribute 'build-id' for
> +     each library.  GDB does not load library with build-id that
> +     does not match such attribute.
> +
>  *** Changes in GDB 7.10

This is OK.

> +static void
> +show_validate_build_id (struct ui_file *file, int from_tty,
> +			struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file, _("Validation a build-id matches to load a shared "
> +			    "library is %s.\n"),

I think this message is not a valid English sentence.  I suggest this
instead:

  Validation of build-id match when loading a shared library is %s.\n

> +void
> +_initialize_build_id (void)
> +{
> +  add_setshow_boolean_cmd ("validate-build-id", class_support,
> +			   &validate_build_id, _("\
> +Set validation a build-id matches to load a shared library."), _("\

Likewise here, I suggest this alternative:

 Set whether to validate build-id match when loading a shared library.

> +SHow validation a build-id matches to load a shared library."), _("\
   ^^
Capitalization.  Also, if you accept my suggestion above, change this
text accordingly.

> +Inferior shared library and symbol file may contain unique build-id.\n\
> +If both build-ids are present but they do not match then this setting\n\
                                ^                     ^
Commas missing where indicated.

Thanks.
  
Jan Kratochvil Aug. 22, 2015, 7:19 p.m. UTC | #2
On Sat, 22 Aug 2015 09:23:06 +0200, Eli Zaretskii wrote:
> I think this message is not a valid English sentence.  I suggest this
> instead:
> 
>   Validation of build-id match when loading a shared library is %s.\n

I have additionally changed it a bit as the meaning has been changed:
	Validation of build-id match when loading a binary is %s.\n


> Likewise here, I suggest this alternative:
> 
>  Set whether to validate build-id match when loading a shared library.

Used:
	Set whether to validate build-id match when loading a binary.


> > +Inferior shared library and symbol file may contain unique build-id.\n\
> > +If both build-ids are present but they do not match then this setting\n\
>                                 ^                     ^
> Commas missing where indicated.

Done.  Plus again s/shared library/binary/.


Set whether to validate build-id match when loading a binary."), _("\
Show whether to validate build-id match when loading a binary."), _("\
Inferior binary and symbol file may contain unique build-id.\n\
If both build-ids are present, but they do not match, then this setting\n\
enables (off) or disables (on) loading of such symbol file.\n\
Loading non-matching symbol file may confuse debugging including breakage\n\
of backtrace output."),



All changed only in GIT.


Thanks,
Jan
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 2cedccd..fce33f3 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -35,6 +35,20 @@  show debug bfd-cache
   The "/m" option is now considered deprecated: its "source-centric"
   output hasn't proved useful in practice.
 
+* New options
+
+set validate-build-id (on|off)
+show validate-build-id
+  Inferior shared library and symbol file may contain unique build-id.
+  If both build-ids are present but they do not match then this setting
+  enables (off) or disables (on) loading of such symbol file.
+
+* New features in the GDB remote stub, GDBserver
+
+  ** library-list-svr4 contains also optional attribute 'build-id' for
+     each library.  GDB does not load library with build-id that
+     does not match such attribute.
+
 *** Changes in GDB 7.10
 
 * Support for process record-replay and reverse debugging on aarch64*-linux*
diff --git a/gdb/build-id.c b/gdb/build-id.c
index c89cd55..0b069fd 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -26,6 +26,21 @@ 
 #include "objfiles.h"
 #include "filenames.h"
 #include "gdbcore.h"
+#include "gdbcmd.h"
+
+/* Boolean for command 'set validate-build-id'.  */
+int validate_build_id = 1;
+
+/* Implement 'show validate-build-id'.  */
+
+static void
+show_validate_build_id (struct ui_file *file, int from_tty,
+			struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Validation a build-id matches to load a shared "
+			    "library is %s.\n"),
+		    value);
+}
 
 /* See build-id.h.  */
 
@@ -167,3 +182,22 @@  find_separate_debug_file_by_buildid (struct objfile *objfile)
     }
   return NULL;
 }
+
+extern initialize_file_ftype _initialize_build_id; /* -Wmissing-prototypes */
+
+void
+_initialize_build_id (void)
+{
+  add_setshow_boolean_cmd ("validate-build-id", class_support,
+			   &validate_build_id, _("\
+Set validation a build-id matches to load a shared library."), _("\
+SHow validation a build-id matches to load a shared library."), _("\
+Inferior shared library and symbol file may contain unique build-id.\n\
+If both build-ids are present but they do not match then this setting\n\
+enables (off) or disables (on) loading of such symbol file.\n\
+Loading non-matching symbol file may confuse debugging including breakage\n\
+of backtrace output."),
+			   NULL,
+			   show_validate_build_id,
+			   &setlist, &showlist);
+}
diff --git a/gdb/build-id.h b/gdb/build-id.h
index bea761b..63b9d8d 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -45,4 +45,6 @@  extern bfd *build_id_to_debug_bfd (size_t build_id_len,
 
 extern char *find_separate_debug_file_by_buildid (struct objfile *objfile);
 
+extern int validate_build_id;
+
 #endif /* BUILD_ID_H */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 1fb07d5..070b95f 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -45,6 +45,7 @@ 
 #include "auxv.h"
 #include "gdb_bfd.h"
 #include "probe.h"
+#include "rsp-low.h"
 
 static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
 static int svr4_have_link_map_offsets (void);
@@ -1139,6 +1140,12 @@  svr4_copy_library_list (struct so_list *src)
       newobj->lm_info = xmalloc (sizeof (struct lm_info));
       memcpy (newobj->lm_info, src->lm_info, sizeof (struct lm_info));
 
+      if (newobj->build_id != NULL)
+	{
+	  newobj->build_id = xmalloc (src->build_idsz);
+	  memcpy (newobj->build_id, src->build_id, src->build_idsz);
+	}
+
       newobj->next = NULL;
       *link = newobj;
       link = &newobj->next;
@@ -1166,6 +1173,9 @@  library_list_start_library (struct gdb_xml_parser *parser,
   ULONGEST *lmp = xml_find_attribute (attributes, "lm")->value;
   ULONGEST *l_addrp = xml_find_attribute (attributes, "l_addr")->value;
   ULONGEST *l_ldp = xml_find_attribute (attributes, "l_ld")->value;
+  const struct gdb_xml_value *const att_build_id
+    = xml_find_attribute (attributes, "build-id");
+  const char *const hex_build_id = att_build_id ? att_build_id->value : NULL;
   struct so_list *new_elem;
 
   new_elem = XCNEW (struct so_list);
@@ -1177,6 +1187,37 @@  library_list_start_library (struct gdb_xml_parser *parser,
   strncpy (new_elem->so_name, name, sizeof (new_elem->so_name) - 1);
   new_elem->so_name[sizeof (new_elem->so_name) - 1] = 0;
   strcpy (new_elem->so_original_name, new_elem->so_name);
+  if (hex_build_id != NULL)
+    {
+      const size_t hex_build_id_len = strlen (hex_build_id);
+
+      if (hex_build_id_len == 0)
+	warning (_("Shared library \"%s\" received empty build-id "
+	           "from gdbserver"), new_elem->so_original_name);
+      else if ((hex_build_id_len & 1U) != 0)
+	warning (_("Shared library \"%s\" received odd number "
+		   "of build-id \"%s\" hex characters from gdbserver"),
+		 new_elem->so_original_name, hex_build_id);
+      else
+	{
+	  const size_t build_idsz = hex_build_id_len / 2;
+
+	  new_elem->build_id = xmalloc (build_idsz);
+	  new_elem->build_idsz = hex2bin (hex_build_id, new_elem->build_id,
+					  build_idsz);
+	  if (new_elem->build_idsz != build_idsz)
+	    {
+	      warning (_("Shared library \"%s\" received invalid "
+			 "build-id \"%s\" hex character at encoded byte "
+			 "position %s (first as 0) from gdbserver"),
+		       new_elem->so_original_name, hex_build_id,
+		       pulongest (new_elem->build_idsz));
+	      xfree (new_elem->build_id);
+	      new_elem->build_id = NULL;
+	      new_elem->build_idsz = 0;
+	    }
+	}
+    }
 
   *list->tailp = new_elem;
   list->tailp = &new_elem->next;
@@ -1211,6 +1252,7 @@  static const struct gdb_xml_attribute svr4_library_attributes[] =
   { "lm", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
   { "l_addr", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
   { "l_ld", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
+  { "build-id", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };
 
diff --git a/gdb/solib.c b/gdb/solib.c
index d2ea901..cf78f6a 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -650,6 +650,7 @@  free_so (struct so_list *so)
   clear_so (so);
   ops->free_so (so);
 
+  xfree (so->build_id);
   xfree (so);
 }
 
diff --git a/gdb/solist.h b/gdb/solist.h
index 844c6f9..af9acc2 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -75,6 +75,19 @@  struct so_list
        There may not be just one (e.g. if two segments are relocated
        differently); but this is only used for "info sharedlibrary".  */
     CORE_ADDR addr_low, addr_high;
+
+    /* Build id decoded from .note.gnu.build-id without note header.  This is
+       actual BUILD_ID which comes either from the remote target via qXfer
+       packet or via reading target memory.  Note that if there's a
+       mismatch with the associated bfd then so->abfd will be cleared.
+       Reading target memory should be done by following execution view
+       of the binary (following program headers in the case of ELF).
+       Computing address from the linking view (following ELF section
+       headers) may give incorrect build-id memory address despite the
+       symbols still match.
+       Such an example is a prelinked vs. unprelinked i386 ELF file.  */
+    size_t build_idsz;
+    gdb_byte *build_id;
   };
 
 struct target_so_ops