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: [patchv3 7/8] Validate symbol file using build-id


On Fri, 28 Feb 2014 21:34:00 +0100, Pedro Alves wrote:
> I see.  _This_ design rationale could be a comment somewhere, IMO.

This is only one example of more ELF transformations that can happen.
The comment was written general enough.


> I'd expect it near the definition of the build_id field.  And indeed
> I see something like that is already there:
> 
> +    /* Build id in raw format, contains verbatim contents of
> +       .note.gnu.build-id including note header.  This is actual
> +       BUILD_ID which comes either from the remote target via qXfer
> +       packet or via reading target memory.  Therefore, it may differ
> +       from the build-id of the associated bfd.  In a normal
> +       scenario, this so would soon lose its abfd due to failed
> +       validation.  */
> +    size_t build_idsz;
> +    gdb_byte *build_id;
>    };
> 
> But we could always extend it further, with a note on that prelink
> issue, if necessary.

I find it more appropriate place that solib-svr4.c as it is a place where
person implementing the feature will place the code - without possibly looking
at the definition of this field.  But it all does not matter too much.

Added for the 'build_idsz' field:
       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.  */


Thanks,
Jan
--- Begin Message ---
gdb/
2014-02-26  Aleksandar Ristovski  <aristovski@qnx.com
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Validate symbol file using build-id.
	* NEWS (Changes since GDB 7.7): Add 'set solib-build-id-force'
	and 'show solib-build-id-force'.  Add build-id attribute.
	* solib-darwin.c (_initialize_darwin_solib): Assign validate value.
	* solib-dsbt.c (_initialize_dsbt_solib): Ditto.
	* solib-frv.c (_initialize_frv_solib): Ditto.
	* solib-ia64-hpux.c (ia64_hpux_target_so_ops): Ditto.
	* solib-irix.c (_initialize_irix_solib): Ditto.
	* solib-osf.c (_initialize_osf_solib): Ditto.
	* solib-pa64.c (_initialize_pa64_solib): Ditto.
	* solib-som.c (_initialize_som_solib): Ditto.
	* solib-spu.c (set_spu_solib_ops): Ditto.
	* solib-svr4.c: Include rsp-low.h.
	(NOTE_GNU_BUILD_ID_NAME): New define.
	(svr4_validate): New function.
	(library_list_start_library): Parse 'build-id' attribute.
	(svr4_library_attributes): Add 'build-id' attribute.
	(_initialize_svr4_solib): Assign validate value.
	* solib-target.c (solib.h): Include.
	(_initialize_solib_target): Assign validate value.
	* solib.c (solib_build_id_force, show_solib_build_id_force): New.
	(solib_map_sections): Use ops->validate.
	(clear_so): Free build_id.
	(default_solib_validate): New function.
	(_initialize_solib): Add "solib-build-id-force".
	* solib.h (default_solib_validate): New declaration.
	* solist.h (struct so_list): New fields 'build_idsz' and 'build_id'.
	(target_so_ops): New field 'validate'.

gdb/doc/
2014-03-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Files): Add 'set solib-build-id-force'
	and 'show solib-build-id-force'.
---
 gdb/NEWS              | 10 +++++++++
 gdb/doc/gdb.texinfo   | 34 ++++++++++++++++++++++++++++++
 gdb/solib-darwin.c    |  1 +
 gdb/solib-dsbt.c      |  1 +
 gdb/solib-frv.c       |  1 +
 gdb/solib-ia64-hpux.c |  1 +
 gdb/solib-irix.c      |  1 +
 gdb/solib-osf.c       |  1 +
 gdb/solib-pa64.c      |  1 +
 gdb/solib-som.c       |  1 +
 gdb/solib-spu.c       |  1 +
 gdb/solib-svr4.c      | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/solib-target.c    |  2 ++
 gdb/solib.c           | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/solib.h           |  4 ++++
 gdb/solist.h          | 20 ++++++++++++++++++
 16 files changed, 188 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..629cc13 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
   the user manual for more details on descriptive types and the intended
   usage of this option.
 
+set solib-build-id-force (on|off)
+show solib-build-id-force
+  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 (on) or disables (off) loading of such symbol file.
+
 * New features in the GDB remote stub, GDBserver
 
   ** New option --debug-format=option1[,option2,...] allows one to add
@@ -51,6 +57,10 @@ maint ada show ignore-descriptive-types
      Timestamps can also be turned on with the
      "monitor set debug-format timestamps" command from GDB.
 
+  ** 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.
+
 * The 'record instruction-history' command now starts counting instructions
   at one.  This also affects the instruction ranges reported by the
   'record function-call-history' command when given the /i modifier.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b1b29bd..c683ede 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17114,6 +17114,40 @@ libraries that were loaded by explicit user requests are not
 discarded.
 @end table
 
+@table @code
+@kindex set solib-build-id-force
+@item set solib-build-id-force @var{mode}
+Setting to override @value{GDBN} build-id check.
+
+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 (@var{mode} is @code{on}) or disables (@var{mode} is @code{off})
+loading of such symbol file.  On systems where build-id is not present
+in files this setting has no effect.  The default value is @code{off}.
+
+Loading non-matching symbol file may confuse debugging including breakage
+of backtrace output.
+
+By default @value{GDBN} will ignore symbol files with non-matching build-id
+while printing:
+
+@smallexample
+  Shared object "libfoo.so.1" could not be validated and will be ignored;
+  or use 'set solib-build-id-force'.
+@end smallexample
+
+Turning on this setting would load such symbol file while still printing:
+
+@smallexample
+  Shared object "libfoo.so.1" could not be validated but it is being loaded
+  due to 'set solib-build-id-force'.
+@end smallexample
+
+@kindex show solib-build-id-force
+@item show solib-build-id-force
+Display the current mode of build-id check override.
+@end table
+
 Sometimes you may wish that @value{GDBN} stops and gives you control
 when any of shared library events happen.  The best way to do this is
 to use @code{catch load} and @code{catch unload} (@pxref{Set
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index e8d4667..d65b2c8 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -650,4 +650,5 @@ _initialize_darwin_solib (void)
   darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code;
   darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
   darwin_so_ops.bfd_open = darwin_bfd_open;
+  darwin_so_ops.validate = default_solib_validate;
 }
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 0217a94..35223dd 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -1078,6 +1078,7 @@ _initialize_dsbt_solib (void)
   dsbt_so_ops.open_symbol_file_object = open_symbol_file_object;
   dsbt_so_ops.in_dynsym_resolve_code = dsbt_in_dynsym_resolve_code;
   dsbt_so_ops.bfd_open = solib_bfd_open;
+  dsbt_so_ops.validate = default_solib_validate;
 
   /* Debug this file's internals.  */
   add_setshow_zuinteger_cmd ("solib-dsbt", class_maintenance,
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 9724a3c..18a4aba 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -1181,6 +1181,7 @@ _initialize_frv_solib (void)
   frv_so_ops.open_symbol_file_object = open_symbol_file_object;
   frv_so_ops.in_dynsym_resolve_code = frv_in_dynsym_resolve_code;
   frv_so_ops.bfd_open = solib_bfd_open;
+  frv_so_ops.validate = default_solib_validate;
 
   /* Debug this file's internals.  */
   add_setshow_zuinteger_cmd ("solib-frv", class_maintenance,
diff --git a/gdb/solib-ia64-hpux.c b/gdb/solib-ia64-hpux.c
index b53caa8..a39bb03 100644
--- a/gdb/solib-ia64-hpux.c
+++ b/gdb/solib-ia64-hpux.c
@@ -688,6 +688,7 @@ ia64_hpux_target_so_ops (void)
   ops->open_symbol_file_object = ia64_hpux_open_symbol_file_object;
   ops->in_dynsym_resolve_code = ia64_hpux_in_dynsym_resolve_code;
   ops->bfd_open = solib_bfd_open;
+  ops->validate = default_solib_validate;
 
   return ops;
 }
diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 6266ee0..1f126a3 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -652,4 +652,5 @@ _initialize_irix_solib (void)
   irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
   irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
   irix_so_ops.bfd_open = solib_bfd_open;
+  irix_so_ops.validate = default_solib_validate;
 }
diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index 90a26e8..5490f83 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -633,6 +633,7 @@ _initialize_osf_solib (void)
   osf_so_ops.open_symbol_file_object = osf_open_symbol_file_object;
   osf_so_ops.in_dynsym_resolve_code = osf_in_dynsym_resolve_code;
   osf_so_ops.bfd_open = solib_bfd_open;
+  osf_so_ops.validate = default_solib_validate;
 
   /* FIXME: Don't do this here.  *_gdbarch_init() should set so_ops.  */
   current_target_so_ops = &osf_so_ops;
diff --git a/gdb/solib-pa64.c b/gdb/solib-pa64.c
index 099e1e7..da8a294 100644
--- a/gdb/solib-pa64.c
+++ b/gdb/solib-pa64.c
@@ -621,6 +621,7 @@ _initialize_pa64_solib (void)
   pa64_so_ops.open_symbol_file_object = pa64_open_symbol_file_object;
   pa64_so_ops.in_dynsym_resolve_code = pa64_in_dynsym_resolve_code;
   pa64_so_ops.bfd_open = solib_bfd_open;
+  pa64_so_ops.validate = default_solib_validate;
 
   memset (&dld_cache, 0, sizeof (dld_cache));
 }
diff --git a/gdb/solib-som.c b/gdb/solib-som.c
index cba50d1..ed2cbfe 100644
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
@@ -816,6 +816,7 @@ _initialize_som_solib (void)
   som_so_ops.open_symbol_file_object = som_open_symbol_file_object;
   som_so_ops.in_dynsym_resolve_code = som_in_dynsym_resolve_code;
   som_so_ops.bfd_open = solib_bfd_open;
+  som_so_ops.validate = default_solib_validate;
 }
 
 void
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index 1c574b5..8385205 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -521,6 +521,7 @@ set_spu_solib_ops (struct gdbarch *gdbarch)
       spu_so_ops.current_sos = spu_current_sos;
       spu_so_ops.bfd_open = spu_bfd_open;
       spu_so_ops.lookup_lib_global_symbol = spu_lookup_lib_symbol;
+      spu_so_ops.validate = default_solib_validate;
     }
 
   set_solib_ops (gdbarch, &spu_so_ops);
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 4c94f9f..14c4c19 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -47,6 +47,9 @@
 #include "exceptions.h"
 #include "gdb_bfd.h"
 #include "probe.h"
+#include "rsp-low.h"
+
+#define NOTE_GNU_BUILD_ID_NAME  ".note.gnu.build-id"
 
 static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
 static int svr4_have_link_map_offsets (void);
@@ -959,6 +962,31 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
   return (name_lm >= vaddr && name_lm < vaddr + size);
 }
 
+/* Validate SO by comparing build-id from the associated bfd and
+   corresponding build-id from target memory.  */
+
+static int
+svr4_validate (const struct so_list *const so)
+{
+  gdb_assert (so != NULL);
+
+  /* Target doesn't support reporting the build ID.  */
+  if (so->build_id == NULL)
+    return 1;
+
+  if (so->abfd == NULL)
+    return 1;
+
+  if (!bfd_check_format (so->abfd, bfd_object)
+      || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour
+      || elf_tdata (so->abfd)->build_id == NULL)
+    return 1;
+
+  return (so->build_idsz == elf_tdata (so->abfd)->build_id->size
+	  && memcmp (so->build_id, elf_tdata (so->abfd)->build_id->data,
+		     so->build_idsz) == 0);
+}
+
 /* Implement the "open_symbol_file_object" target_so_ops method.
 
    If no open symbol file, attempt to locate and open the main symbol
@@ -1124,6 +1152,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);
@@ -1135,6 +1166,25 @@ 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 && (hex_build_id_len & 1U) == 0)
+	{
+	  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)
+	    {
+	      xfree (new_elem->build_id);
+	      new_elem->build_id = NULL;
+	      new_elem->build_idsz = 0;
+	    }
+	}
+    }
 
   *list->tailp = new_elem;
   list->tailp = &new_elem->next;
@@ -1169,6 +1219,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 }
 };
 
@@ -3159,4 +3210,5 @@ _initialize_svr4_solib (void)
   svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
   svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
   svr4_so_ops.handle_event = svr4_handle_solib_event;
+  svr4_so_ops.validate = svr4_validate;
 }
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index bb34e4b..23b2d06 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -25,6 +25,7 @@
 #include "target.h"
 #include "vec.h"
 #include "solib-target.h"
+#include "solib.h"
 
 #include <string.h>
 
@@ -502,6 +503,7 @@ _initialize_solib_target (void)
   solib_target_so_ops.in_dynsym_resolve_code
     = solib_target_in_dynsym_resolve_code;
   solib_target_so_ops.bfd_open = solib_bfd_open;
+  solib_target_so_ops.validate = default_solib_validate;
 
   /* Set current_target_so_ops to solib_target_so_ops if not already
      set.  */
diff --git a/gdb/solib.c b/gdb/solib.c
index 3350bfd..5ce0d58 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -454,6 +454,20 @@ solib_bfd_open (char *pathname)
   return abfd;
 }
 
+/* Boolean for command 'set solib-build-id-force'.  */
+static int solib_build_id_force = 0;
+
+/* Implement 'show solib-build-id-force'.  */
+
+static void
+show_solib_build_id_force (struct ui_file *file, int from_tty,
+			   struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Loading of shared libraries "
+			    "with non-matching build-id is %s.\n"),
+		    value);
+}
+
 /* Given a pointer to one of the shared objects in our list of mapped
    objects, use the recorded name to open a bfd descriptor for the
    object, build a section table, relocate all the section addresses
@@ -486,6 +500,24 @@ solib_map_sections (struct so_list *so)
   /* Leave bfd open, core_xfer_memory and "info files" need it.  */
   so->abfd = abfd;
 
+  gdb_assert (ops->validate != NULL);
+
+  if (!ops->validate (so))
+    {
+      if (!solib_build_id_force)
+	{
+	  warning (_("Shared object \"%s\" could not be validated "
+		     "and will be ignored; or use 'set solib-build-id-force'."),
+		   so->so_name);
+	  gdb_bfd_unref (so->abfd);
+	  so->abfd = NULL;
+	  return 0;
+	}
+      warning (_("Shared object \"%s\" could not be validated "
+		 "but it is being loaded due to 'set solib-build-id-force'."),
+	       so->so_name);
+    }
+
   /* Copy the full path name into so_name, allowing symbol_file_add
      to find it later.  This also affects the =library-loaded GDB/MI
      event, and in particular the part of that notification providing
@@ -562,6 +594,9 @@ clear_so (struct so_list *so)
      of the symbol file.  */
   strcpy (so->so_name, so->so_original_name);
 
+  xfree (so->build_id);
+  so->build_id = NULL;
+
   /* Do the same for target-specific data.  */
   if (ops->clear_so != NULL)
     ops->clear_so (so);
@@ -1523,6 +1558,14 @@ remove_user_added_objfile (struct objfile *objfile)
     }
 }
 
+/* Default implementation does not perform any validation.  */
+
+int
+default_solib_validate (const struct so_list *const so)
+{
+  return 1; /* No validation.  */
+}
+
 extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
 
 void
@@ -1579,4 +1622,18 @@ PATH and LD_LIBRARY_PATH."),
 				     reload_shared_libraries,
 				     show_solib_search_path,
 				     &setlist, &showlist);
+
+  add_setshow_boolean_cmd ("solib-build-id-force", class_support,
+			   &solib_build_id_force, _("\
+Set loading of shared libraries with non-matching build-id."), _("\
+Show loading of shared libraries with non-matching build-id."), _("\
+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 (on) or disables (off) loading of such symbol file.\n\
+Loading non-matching symbol file may confuse debugging including breakage\n\
+of backtrace output."),
+			   NULL,
+			   show_solib_build_id_force,
+			   &setlist, &showlist);
+
 }
diff --git a/gdb/solib.h b/gdb/solib.h
index 29fe8f7..1f4d2b7 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -98,4 +98,8 @@ extern void update_solib_breakpoints (void);
 
 extern void handle_solib_event (void);
 
+/* Default validation always returns 1.  */
+
+extern int default_solib_validate (const struct so_list *so);
+
 #endif /* SOLIB_H */
diff --git a/gdb/solist.h b/gdb/solist.h
index ac1b1a7..b5fa91a 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -75,6 +75,22 @@ 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 in raw format, contains verbatim contents of
+       .note.gnu.build-id including note header.  This is actual
+       BUILD_ID which comes either from the remote target via qXfer
+       packet or via reading target memory.  Therefore, it may differ
+       from the build-id of the associated bfd.  In a normal
+       scenario, this so would soon lose its abfd due to failed
+       validation.
+       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
@@ -168,6 +184,10 @@ struct target_so_ops
        NULL, in which case no specific preprocessing is necessary
        for this target.  */
     void (*handle_event) (void);
+
+    /* Return 0 if SO does not match target SO it is supposed to
+       represent.  Return 1 otherwise.  */
+    int (*validate) (const struct so_list *so);
   };
 
 /* Free the memory associated with a (so_list *).  */
-- 
1.8.5.3

--- End Message ---

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