* Re: [PATCH v9 09/10] Validate symbol file using build-id @ 2015-07-13 12:48 Aleksandar Ristovski 0 siblings, 0 replies; 10+ messages in thread From: Aleksandar Ristovski @ 2015-07-13 12:48 UTC (permalink / raw) To: Jan Kratochvil, Pedro Alves; +Cc: Eli Zaretskii, dje, gdb-patches Original Message From: Jan Kratochvil Sent: Monday, July 13, 2015 08:38 To: Pedro Alves Cc: Eli Zaretskii; dje@google.com; gdb-patches@sourceware.org; Aleksandar Ristovski Subject: Re: [PATCH v9 09/10] Validate symbol file using build-id ... [Jan] OK, so validate-build-id or solib-validate-build-id. ... [AR] I vote for validate-build-id It will apply to exe as well. Description may contain a note that currently validation does not work on exes. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 00/10] Validate binary before use
@ 2015-06-14 19:25 Jan Kratochvil
2015-06-14 19:27 ` [PATCH v7 09/10] Validate symbol file using build-id Jan Kratochvil
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kratochvil @ 2015-06-14 19:25 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
git://sourceware.org/git/archer.git
jankratochvil/gdbserverbuildid
an update. Patch below is an overall v6->v7 diff of the whole series.
Jan
v7
* move linux-maps.[ch] common/->nat/ and target-utils.[ch] common/->target/
* remove GDBSERVER #ifdefs
* rebase on top of the new 'struct inferior *inf' parameter
v6
* move also gdb_regex* to common/ as discussed above, also in config*.ac
* skip_to_space{,_const}() were moved to common/
* common/common-defs.h #include reordering
* new passing of enum filterflags from linux_qxfer_libraries_svr4()
* dropped refactoring of code moved to common/ that avoided GDB exceptions
* new svr4_copy_library_list() needs to handle new so_list->build_id
v5
* svr4_validate() considers missing local build-id as not-a-match
* target_so_ops->validate() now returns not-a-match reason as a string
* rename common/common-target.[ch] -> common/target-utils.[ch]
* testcase runs (but broken) even on different-filesystem remote target
* testcase simplified by using with_test_prefix()
v4
* NEWS, doc/gdb.texinfo additions.
* Used host-defs.h.
* New set/show solib-build-id-force.
* testsuite: Do not run on non-localhost remote targets.
v3
[patchv3 0/8] Validate binary before use
https://sourceware.org/ml/gdb-patches/2014-02/msg00842.html
Message-ID: <20140227213229.GA21121@host2.jankratochvil.net>
* Implemented the review comments I made.
* Removed fetching build-id in solib-svr4.c for NAT run.
v2
[PATCH 0/8] v2 - validate binary before use
https://sourceware.org/ml/gdb-patches/2013-04/msg00472.html
Message-ID: <1366127096-5744-1-git-send-email-ARistovski@qnx.com>
---
gdb/Makefile.in | 21 +
gdb/NEWS | 12 +
gdb/cli/cli-utils.c | 36 --
gdb/cli/cli-utils.h | 18 -
gdb/common/common-defs.h | 2
gdb/common/common-utils.c | 137 ++++++
gdb/common/common-utils.h | 20 +
gdb/common/common.m4 | 29 +
gdb/common/gdb_regex.c | 73 +++
gdb/common/gdb_regex.h | 36 ++
gdb/common/host-defs.h | 21 +
gdb/config/i386/linux.mh | 2
gdb/config/i386/linux64.mh | 2
gdb/configure | 102 ++--
gdb/configure.ac | 29 -
gdb/defs.h | 19 -
gdb/doc/gdb.texinfo | 55 ++
gdb/features/library-list-svr4.dtd | 13 -
gdb/gdb_regex.h | 36 --
gdb/gdbserver/Makefile.in | 12 -
gdb/gdbserver/config.in | 3
gdb/gdbserver/configure | 56 ++
gdb/gdbserver/configure.srv | 2
gdb/gdbserver/gdbreplay.c | 6
gdb/gdbserver/linux-low.c | 398 +++++++++++++++--
gdb/gdbserver/target.c | 36 ++
gdb/linux-tdep.c | 566 ++----------------------
gdb/monitor.c | 16 -
gdb/nat/linux-maps.c | 493 +++++++++++++++++++++
gdb/nat/linux-maps.h | 64 +++
gdb/solib-darwin.c | 1
gdb/solib-dsbt.c | 1
gdb/solib-frv.c | 1
gdb/solib-spu.c | 1
gdb/solib-svr4.c | 91 ++++
gdb/solib-target.c | 2
gdb/solib.c | 62 +++
gdb/solib.h | 4
gdb/solist.h | 21 +
gdb/target.c | 95 +---
gdb/target.h | 10
gdb/target/target-utils.c | 100 ++++
gdb/target/target-utils.h | 35 +
gdb/target/target.h | 11
gdb/testsuite/gdb.base/solib-mismatch-lib.c | 29 +
gdb/testsuite/gdb.base/solib-mismatch-libmod.c | 29 +
gdb/testsuite/gdb.base/solib-mismatch.c | 56 ++
gdb/testsuite/gdb.base/solib-mismatch.exp | 156 +++++++
gdb/utils.c | 154 -------
gdb/utils.h | 2
50 files changed, 2151 insertions(+), 1025 deletions(-)
create mode 100644 gdb/common/gdb_regex.c
create mode 100644 gdb/common/gdb_regex.h
delete mode 100644 gdb/gdb_regex.h
create mode 100644 gdb/nat/linux-maps.c
create mode 100644 gdb/nat/linux-maps.h
create mode 100644 gdb/target/target-utils.c
create mode 100644 gdb/target/target-utils.h
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch-lib.c
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch-libmod.c
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch.c
create mode 100644 gdb/testsuite/gdb.base/solib-mismatch.exp
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v7 09/10] Validate symbol file using build-id 2015-06-14 19:25 [PATCH v7 00/10] Validate binary before use Jan Kratochvil @ 2015-06-14 19:27 ` Jan Kratochvil 2015-06-21 10:16 ` [PATCH v8 " Jan Kratochvil 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-06-14 19:27 UTC (permalink / raw) To: gdb-patches; +Cc: Aleksandar Ristovski Hi, consumer part of the "build-id" attribute. Approved by: https://sourceware.org/ml/gdb-patches/2014-05/msg00424.html Jan gdb/ChangeLog 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.9): 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-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. (svr4_copy_library_list): Duplicate field build_id. (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/ChangeLog 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 | 12 +++++++ gdb/doc/gdb.texinfo | 38 +++++++++++++++++++++ gdb/solib-darwin.c | 1 + gdb/solib-dsbt.c | 1 + gdb/solib-frv.c | 1 + gdb/solib-spu.c | 1 + gdb/solib-svr4.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/solib-target.c | 2 + gdb/solib.c | 62 ++++++++++++++++++++++++++++++++++- gdb/solib.h | 4 ++ gdb/solist.h | 21 ++++++++++++ 11 files changed, 233 insertions(+), 1 deletion(-) diff --git a/gdb/NEWS b/gdb/NEWS index 85688c7..8f35dca 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -151,6 +151,12 @@ set debug linux-namespaces show debug linux-namespaces Control display of debugging info regarding Linux namespaces. +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. + * The command 'thread apply all' can now support new option '-ascending' to call its specified command for all threads in ascending order. @@ -225,6 +231,12 @@ fork-events and vfork-events features in qSupported * GDB now supports access to vector registers on S/390 GNU/Linux targets. +* 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. + * Removed command line options -xdb HP-UX XDB compatibility mode. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 10bc6e2..061abef 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -17900,6 +17900,44 @@ libraries that were loaded by explicit user requests are not discarded. @end table +@table @code +@kindex set solib-build-id-force +@cindex override @value{GDBN} build-id check +@item set solib-build-id-force @var{mode} +Setting to override @value{GDBN} build-id check. + +Inferior shared libraries and symbol files may contain unique build-id. +By default @value{GDBN} will ignore symbol files with non-matching build-id +while printing: + +@smallexample + warning: Shared object "libfoo.so.1" could not be validated (remote + build ID 2bc1745e does not match local build ID a08f8767) 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 + warning: Shared object "libfoo.so.1" could not be validated (remote + build ID 2bc1745e does not match local build ID a08f8767) but it is + being loaded due to 'set solib-build-id-force'. +@end smallexample + +If remote build-id is present but it does not match local build-id (or local +build-id is not present) 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 the remote system this setting has no effect. +The default value is @code{off}. + +Loading non-matching symbol file may confuse debugging including breakage +of backtrace output. + +@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 f96841f..9309c35 100644 --- a/gdb/solib-darwin.c +++ b/gdb/solib-darwin.c @@ -634,4 +634,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 7da5833..9fe6533 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -1080,6 +1080,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 f7ef38b..b768146 100644 --- a/gdb/solib-frv.c +++ b/gdb/solib-frv.c @@ -1183,6 +1183,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-spu.c b/gdb/solib-spu.c index 44fbf91..d162884 100644 --- a/gdb/solib-spu.c +++ b/gdb/solib-spu.c @@ -519,6 +519,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 909dfb7..881b8f3 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -45,6 +45,9 @@ #include "auxv.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); @@ -970,6 +973,64 @@ 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 char * +svr4_validate (const struct so_list *const so) +{ + bfd_byte *local_id; + size_t local_idsz; + + gdb_assert (so != NULL); + + /* Target doesn't support reporting the build ID or the remote shared library + does not have build ID. */ + if (so->build_id == NULL) + return NULL; + + /* Build ID may be present in the local file, just GDB is unable to retrieve + it. As it has been reported by gdbserver it is not FSF gdbserver. */ + if (so->abfd == NULL + || !bfd_check_format (so->abfd, bfd_object) + || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour) + return NULL; + + /* GDB has verified the local file really does not contain the build ID. */ + if (elf_tdata (so->abfd)->build_id == NULL) + { + char *remote_hex; + + remote_hex = alloca (so->build_idsz * 2 + 1); + bin2hex (so->build_id, remote_hex, so->build_idsz); + + return xstrprintf (_("remote build ID is %s " + "but local file does not have build ID"), + remote_hex); + } + + local_id = elf_tdata (so->abfd)->build_id->data; + local_idsz = elf_tdata (so->abfd)->build_id->size; + + if (so->build_idsz != local_idsz + || memcmp (so->build_id, local_id, so->build_idsz) != 0) + { + char *remote_hex, *local_hex; + + remote_hex = alloca (so->build_idsz * 2 + 1); + bin2hex (so->build_id, remote_hex, so->build_idsz); + local_hex = alloca (local_idsz * 2 + 1); + bin2hex (local_id, local_hex, local_idsz); + + return xstrprintf (_("remote build ID %s " + "does not match local build ID %s"), + remote_hex, local_hex); + } + + /* Both build IDs are present and they match. */ + return NULL; +} + /* Implement the "open_symbol_file_object" target_so_ops method. If no open symbol file, attempt to locate and open the main symbol @@ -1108,6 +1169,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; @@ -1135,6 +1202,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); @@ -1146,6 +1216,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; @@ -1180,6 +1269,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 } }; @@ -3258,4 +3348,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 f14363a..13cbd26 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" /* Private data for each loaded library. */ struct lm_info @@ -506,6 +507,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 0010c2f..4bd47d0 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -523,6 +523,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 @@ -539,7 +553,7 @@ static int solib_map_sections (struct so_list *so) { const struct target_so_ops *ops = solib_ops (target_gdbarch ()); - char *filename; + char *filename, *validate_error; struct target_section *p; struct cleanup *old_chain; bfd *abfd; @@ -555,6 +569,27 @@ 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); + + validate_error = ops->validate (so); + if (validate_error != NULL) + { + if (!solib_build_id_force) + { + warning (_("Shared object \"%s\" could not be validated (%s) and " + "will be ignored; or use 'set solib-build-id-force'."), + so->so_name, validate_error); + xfree (validate_error); + gdb_bfd_unref (so->abfd); + so->abfd = NULL; + return 0; + } + warning (_("Shared object \"%s\" could not be validated (%s) " + "but it is being loaded due to 'set solib-build-id-force'."), + so->so_name, validate_error); + xfree (validate_error); + } + /* 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 @@ -631,6 +666,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); @@ -1662,6 +1700,14 @@ remove_user_added_objfile (struct objfile *objfile) } } +/* Default implementation does not perform any validation. */ + +char * +default_solib_validate (const struct so_list *const so) +{ + return NULL; /* No validation. */ +} + extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */ void @@ -1719,4 +1765,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 336971d..c3bf529 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 char *default_solib_validate (const struct so_list *so); + #endif /* SOLIB_H */ diff --git a/gdb/solist.h b/gdb/solist.h index 7021f5c..f0d8653 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,11 @@ struct target_so_ops NULL, in which case no specific preprocessing is necessary for this target. */ void (*handle_event) (void); + + /* Return NULL if SO does match target SO it is supposed to + represent. Otherwise return string describing why it does not match. + Caller has to free the string. */ + char *(*validate) (const struct so_list *so); }; /* Free the memory associated with a (so_list *). */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 09/10] Validate symbol file using build-id 2015-06-14 19:27 ` [PATCH v7 09/10] Validate symbol file using build-id Jan Kratochvil @ 2015-06-21 10:16 ` Jan Kratochvil 2015-06-22 22:25 ` Doug Evans 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-06-21 10:16 UTC (permalink / raw) To: gdb-patches; +Cc: Aleksandar Ristovski [-- Attachment #1: Type: text/plain, Size: 240 bytes --] Hi, updated for: commit c74f7d1c6c5a968330208757f476c67a4bb66643 Author: Jon Turney <jon.turney@dronecode.org.uk> Date: Tue Apr 7 20:49:08 2015 +0100 Allow gdb to find debug symbols file by build-id for PE file format also Jan [-- Attachment #2: Type: message/rfc822, Size: 18272 bytes --] From: Jan Kratochvil <jan.kratochvil@redhat.com> Subject: [PATCH 1/2] Validate symbol file using build-id Date: Sun, 21 Jun 2015 11:51:52 +0200 Hi, consumer part of the "build-id" attribute. Approved by: https://sourceware.org/ml/gdb-patches/2014-05/msg00424.html Jan gdb/ChangeLog 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.9): 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-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. (svr4_copy_library_list): Duplicate field build_id. (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/ChangeLog 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 | 12 +++++++ gdb/doc/gdb.texinfo | 38 ++++++++++++++++++++++ gdb/solib-darwin.c | 1 + gdb/solib-dsbt.c | 1 + gdb/solib-frv.c | 1 + gdb/solib-spu.c | 1 + gdb/solib-svr4.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/solib-target.c | 2 ++ gdb/solib.c | 62 +++++++++++++++++++++++++++++++++++- gdb/solib.h | 4 +++ gdb/solist.h | 21 +++++++++++++ 11 files changed, 232 insertions(+), 1 deletion(-) diff --git a/gdb/NEWS b/gdb/NEWS index 3ec5851..8cbe1fc 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -159,6 +159,12 @@ set debug linux-namespaces show debug linux-namespaces Control display of debugging info regarding Linux namespaces. +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. + * The command 'thread apply all' can now support new option '-ascending' to call its specified command for all threads in ascending order. @@ -233,6 +239,12 @@ fork-events and vfork-events features in qSupported * GDB now supports access to vector registers on S/390 GNU/Linux targets. +* 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. + * Removed command line options -xdb HP-UX XDB compatibility mode. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 1460b7f..f7e4405 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -17912,6 +17912,44 @@ libraries that were loaded by explicit user requests are not discarded. @end table +@table @code +@kindex set solib-build-id-force +@cindex override @value{GDBN} build-id check +@item set solib-build-id-force @var{mode} +Setting to override @value{GDBN} build-id check. + +Inferior shared libraries and symbol files may contain unique build-id. +By default @value{GDBN} will ignore symbol files with non-matching build-id +while printing: + +@smallexample + warning: Shared object "libfoo.so.1" could not be validated (remote + build ID 2bc1745e does not match local build ID a08f8767) 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 + warning: Shared object "libfoo.so.1" could not be validated (remote + build ID 2bc1745e does not match local build ID a08f8767) but it is + being loaded due to 'set solib-build-id-force'. +@end smallexample + +If remote build-id is present but it does not match local build-id (or local +build-id is not present) 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 the remote system this setting has no effect. +The default value is @code{off}. + +Loading non-matching symbol file may confuse debugging including breakage +of backtrace output. + +@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 f96841f..9309c35 100644 --- a/gdb/solib-darwin.c +++ b/gdb/solib-darwin.c @@ -634,4 +634,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 7da5833..9fe6533 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -1080,6 +1080,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 f7ef38b..b768146 100644 --- a/gdb/solib-frv.c +++ b/gdb/solib-frv.c @@ -1183,6 +1183,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-spu.c b/gdb/solib-spu.c index 44fbf91..d162884 100644 --- a/gdb/solib-spu.c +++ b/gdb/solib-spu.c @@ -519,6 +519,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 909dfb7..b434c1f 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -45,6 +45,9 @@ #include "auxv.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); @@ -970,6 +973,63 @@ 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 char * +svr4_validate (const struct so_list *const so) +{ + const bfd_byte *local_id; + size_t local_idsz; + + gdb_assert (so != NULL); + + /* Target doesn't support reporting the build ID or the remote shared library + does not have build ID. */ + if (so->build_id == NULL) + return NULL; + + /* Build ID may be present in the local file, just GDB is unable to retrieve + it. As it has been reported by gdbserver it is not FSF gdbserver. */ + if (so->abfd == NULL + || !bfd_check_format (so->abfd, bfd_object)) + return NULL; + + /* GDB has verified the local file really does not contain the build ID. */ + if (so->abfd->build_id == NULL) + { + char *remote_hex; + + remote_hex = alloca (so->build_idsz * 2 + 1); + bin2hex (so->build_id, remote_hex, so->build_idsz); + + return xstrprintf (_("remote build ID is %s " + "but local file does not have build ID"), + remote_hex); + } + + local_id = so->abfd->build_id->data; + local_idsz = so->abfd->build_id->size; + + if (so->build_idsz != local_idsz + || memcmp (so->build_id, local_id, so->build_idsz) != 0) + { + char *remote_hex, *local_hex; + + remote_hex = alloca (so->build_idsz * 2 + 1); + bin2hex (so->build_id, remote_hex, so->build_idsz); + local_hex = alloca (local_idsz * 2 + 1); + bin2hex (local_id, local_hex, local_idsz); + + return xstrprintf (_("remote build ID %s " + "does not match local build ID %s"), + remote_hex, local_hex); + } + + /* Both build IDs are present and they match. */ + return NULL; +} + /* Implement the "open_symbol_file_object" target_so_ops method. If no open symbol file, attempt to locate and open the main symbol @@ -1108,6 +1168,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; @@ -1135,6 +1201,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); @@ -1146,6 +1215,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; @@ -1180,6 +1268,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 } }; @@ -3258,4 +3347,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 f14363a..13cbd26 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" /* Private data for each loaded library. */ struct lm_info @@ -506,6 +507,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 0010c2f..4bd47d0 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -523,6 +523,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 @@ -539,7 +553,7 @@ static int solib_map_sections (struct so_list *so) { const struct target_so_ops *ops = solib_ops (target_gdbarch ()); - char *filename; + char *filename, *validate_error; struct target_section *p; struct cleanup *old_chain; bfd *abfd; @@ -555,6 +569,27 @@ 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); + + validate_error = ops->validate (so); + if (validate_error != NULL) + { + if (!solib_build_id_force) + { + warning (_("Shared object \"%s\" could not be validated (%s) and " + "will be ignored; or use 'set solib-build-id-force'."), + so->so_name, validate_error); + xfree (validate_error); + gdb_bfd_unref (so->abfd); + so->abfd = NULL; + return 0; + } + warning (_("Shared object \"%s\" could not be validated (%s) " + "but it is being loaded due to 'set solib-build-id-force'."), + so->so_name, validate_error); + xfree (validate_error); + } + /* 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 @@ -631,6 +666,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); @@ -1662,6 +1700,14 @@ remove_user_added_objfile (struct objfile *objfile) } } +/* Default implementation does not perform any validation. */ + +char * +default_solib_validate (const struct so_list *const so) +{ + return NULL; /* No validation. */ +} + extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */ void @@ -1719,4 +1765,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 336971d..c3bf529 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 char *default_solib_validate (const struct so_list *so); + #endif /* SOLIB_H */ diff --git a/gdb/solist.h b/gdb/solist.h index 7021f5c..f0d8653 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,11 @@ struct target_so_ops NULL, in which case no specific preprocessing is necessary for this target. */ void (*handle_event) (void); + + /* Return NULL if SO does match target SO it is supposed to + represent. Otherwise return string describing why it does not match. + Caller has to free the string. */ + char *(*validate) (const struct so_list *so); }; /* Free the memory associated with a (so_list *). */ -- 2.1.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 09/10] Validate symbol file using build-id 2015-06-21 10:16 ` [PATCH v8 " Jan Kratochvil @ 2015-06-22 22:25 ` Doug Evans 2015-06-23 20:47 ` Jan Kratochvil 0 siblings, 1 reply; 10+ messages in thread From: Doug Evans @ 2015-06-22 22:25 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski On Sun, Jun 21, 2015 at 5:16 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote: > Hi, > > updated for: > commit c74f7d1c6c5a968330208757f476c67a4bb66643 > Author: Jon Turney <jon.turney@dronecode.org.uk> > Date: Tue Apr 7 20:49:08 2015 +0100 > Allow gdb to find debug symbols file by build-id for PE file format also > > > Jan > > > ---------- Forwarded message ---------- > From: Jan Kratochvil <jan.kratochvil@redhat.com> > To: > Cc: > Date: Sun, 21 Jun 2015 11:51:52 +0200 > Subject: [PATCH 1/2] Validate symbol file using build-id > Hi, > > consumer part of the "build-id" attribute. > > Approved by: > https://sourceware.org/ml/gdb-patches/2014-05/msg00424.html > > > Jan > > > gdb/ChangeLog > 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.9): 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-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. > (svr4_copy_library_list): Duplicate field build_id. > (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/ChangeLog > 2014-03-02 Jan Kratochvil <jan.kratochvil@redhat.com> > > * gdb.texinfo (Files): Add 'set solib-build-id-force' > and 'show solib-build-id-force'. IIUC this applies to symbol files (the main program) too, right? If so, having solib in the option name is confusing. set build-id-force or set require-build-id-match or some such would be clearer. > diff --git a/gdb/NEWS b/gdb/NEWS > index 3ec5851..8cbe1fc 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -159,6 +159,12 @@ set debug linux-namespaces > show debug linux-namespaces > Control display of debugging info regarding Linux namespaces. > > +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. > + > * The command 'thread apply all' can now support new option '-ascending' > to call its specified command for all threads in ascending order. > > @@ -233,6 +239,12 @@ fork-events and vfork-events features in qSupported > * GDB now supports access to vector registers on S/390 GNU/Linux > targets. > > +* 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. > + > * Removed command line options > > -xdb HP-UX XDB compatibility mode. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 1460b7f..f7e4405 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -17912,6 +17912,44 @@ libraries that were loaded by explicit user requests are not > discarded. > @end table > > +@table @code > +@kindex set solib-build-id-force > +@cindex override @value{GDBN} build-id check > +@item set solib-build-id-force @var{mode} > +Setting to override @value{GDBN} build-id check. > + > +Inferior shared libraries and symbol files may contain unique build-id. > +By default @value{GDBN} will ignore symbol files with non-matching build-id > +while printing: > + > +@smallexample > + warning: Shared object "libfoo.so.1" could not be validated (remote > + build ID 2bc1745e does not match local build ID a08f8767) 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 > + warning: Shared object "libfoo.so.1" could not be validated (remote > + build ID 2bc1745e does not match local build ID a08f8767) but it is > + being loaded due to 'set solib-build-id-force'. > +@end smallexample > + > +If remote build-id is present but it does not match local build-id (or local > +build-id is not present) 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 the remote system this setting has no effect. > +The default value is @code{off}. > + > +Loading non-matching symbol file may confuse debugging including breakage > +of backtrace output. > + > +@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 f96841f..9309c35 100644 > --- a/gdb/solib-darwin.c > +++ b/gdb/solib-darwin.c > @@ -634,4 +634,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 7da5833..9fe6533 100644 > --- a/gdb/solib-dsbt.c > +++ b/gdb/solib-dsbt.c > @@ -1080,6 +1080,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 f7ef38b..b768146 100644 > --- a/gdb/solib-frv.c > +++ b/gdb/solib-frv.c > @@ -1183,6 +1183,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-spu.c b/gdb/solib-spu.c > index 44fbf91..d162884 100644 > --- a/gdb/solib-spu.c > +++ b/gdb/solib-spu.c > @@ -519,6 +519,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 909dfb7..b434c1f 100644 > --- a/gdb/solib-svr4.c > +++ b/gdb/solib-svr4.c > @@ -45,6 +45,9 @@ > #include "auxv.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); > @@ -970,6 +973,63 @@ 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. */ Please document that the result is an error message or NULL for success (including missing build id), and that the caller must free it. I realize you say so in the docs for the "validate" "method", but the comment here doesn't mention it is the validate method (which would be a fine alternative to repeating all the docs of the method). > + > +static char * > +svr4_validate (const struct so_list *const so) > +{ > + const bfd_byte *local_id; > + size_t local_idsz; > + > + gdb_assert (so != NULL); > + > + /* Target doesn't support reporting the build ID or the remote shared library > + does not have build ID. */ > + if (so->build_id == NULL) > + return NULL; > + > + /* Build ID may be present in the local file, just GDB is unable to retrieve > + it. As it has been reported by gdbserver it is not FSF gdbserver. */ > + if (so->abfd == NULL > + || !bfd_check_format (so->abfd, bfd_object)) > + return NULL; > + > + /* GDB has verified the local file really does not contain the build ID. */ > + if (so->abfd->build_id == NULL) > + { > + char *remote_hex; > + > + remote_hex = alloca (so->build_idsz * 2 + 1); > + bin2hex (so->build_id, remote_hex, so->build_idsz); > + > + return xstrprintf (_("remote build ID is %s " > + "but local file does not have build ID"), > + remote_hex); > + } > + > + local_id = so->abfd->build_id->data; > + local_idsz = so->abfd->build_id->size; > + > + if (so->build_idsz != local_idsz > + || memcmp (so->build_id, local_id, so->build_idsz) != 0) > + { > + char *remote_hex, *local_hex; > + > + remote_hex = alloca (so->build_idsz * 2 + 1); > + bin2hex (so->build_id, remote_hex, so->build_idsz); > + local_hex = alloca (local_idsz * 2 + 1); > + bin2hex (local_id, local_hex, local_idsz); > + > + return xstrprintf (_("remote build ID %s " > + "does not match local build ID %s"), > + remote_hex, local_hex); > + } > + > + /* Both build IDs are present and they match. */ > + return NULL; > +} > + > /* Implement the "open_symbol_file_object" target_so_ops method. > > If no open symbol file, attempt to locate and open the main symbol > @@ -1108,6 +1168,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; > @@ -1135,6 +1201,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); > @@ -1146,6 +1215,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) > + { This happens for a malformed build id, right? A warning would be useful here. It'd also be nice to have a warning for an odd count. > + xfree (new_elem->build_id); > + new_elem->build_id = NULL; > + new_elem->build_idsz = 0; > + } > + } > + } > > *list->tailp = new_elem; > list->tailp = &new_elem->next; > @@ -1180,6 +1268,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 } > }; > > @@ -3258,4 +3347,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 f14363a..13cbd26 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" > > /* Private data for each loaded library. */ > struct lm_info > @@ -506,6 +507,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 0010c2f..4bd47d0 100644 > --- a/gdb/solib.c > +++ b/gdb/solib.c > @@ -523,6 +523,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 > @@ -539,7 +553,7 @@ static int > solib_map_sections (struct so_list *so) > { > const struct target_so_ops *ops = solib_ops (target_gdbarch ()); > - char *filename; > + char *filename, *validate_error; > struct target_section *p; > struct cleanup *old_chain; > bfd *abfd; > @@ -555,6 +569,27 @@ 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); > + > + validate_error = ops->validate (so); > + if (validate_error != NULL) > + { > + if (!solib_build_id_force) > + { > + warning (_("Shared object \"%s\" could not be validated (%s) and " > + "will be ignored; or use 'set solib-build-id-force'."), > + so->so_name, validate_error); > + xfree (validate_error); > + gdb_bfd_unref (so->abfd); > + so->abfd = NULL; > + return 0; > + } > + warning (_("Shared object \"%s\" could not be validated (%s) " > + "but it is being loaded due to 'set solib-build-id-force'."), > + so->so_name, validate_error); > + xfree (validate_error); > + } > + > /* 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 > @@ -631,6 +666,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); > @@ -1662,6 +1700,14 @@ remove_user_added_objfile (struct objfile *objfile) > } > } > > +/* Default implementation does not perform any validation. */ > + > +char * > +default_solib_validate (const struct so_list *const so) > +{ > + return NULL; /* No validation. */ > +} > + > extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */ > > void > @@ -1719,4 +1765,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 336971d..c3bf529 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 char *default_solib_validate (const struct so_list *so); > + > #endif /* SOLIB_H */ > diff --git a/gdb/solist.h b/gdb/solist.h > index 7021f5c..f0d8653 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. Including the note header will be confusing to readers. Is there a reason to include it? OTOH, given the above call to hex2bin, does this really include the 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. I can't read this last sentence. What are you trying to say here? > + 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,11 @@ struct target_so_ops > NULL, in which case no specific preprocessing is necessary > for this target. */ > void (*handle_event) (void); > + > + /* Return NULL if SO does match target SO it is supposed to > + represent. Otherwise return string describing why it does not match. > + Caller has to free the string. */ > + char *(*validate) (const struct so_list *so); > }; > > /* Free the memory associated with a (so_list *). */ > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 09/10] Validate symbol file using build-id 2015-06-22 22:25 ` Doug Evans @ 2015-06-23 20:47 ` Jan Kratochvil 2015-07-08 14:44 ` Pedro Alves 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-06-23 20:47 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches, Aleksandar Ristovski On Tue, 23 Jun 2015 00:25:52 +0200, Doug Evans wrote: > On Sun, Jun 21, 2015 at 5:16 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote: > > * gdb.texinfo (Files): Add 'set solib-build-id-force' > > and 'show solib-build-id-force'. > > IIUC this applies to symbol files (the main program) too, right? No. That is an extension I am working on as an add-on patchset, to be posted in a week or two. I expected this "solib" patchset will be already approved a long time ago so the add-on patchset will make sense. But given this patchset is still being reviewed and the new patchset will change some parts of this one I am curious whether I should not rather merge the second patchset into the first one and start the review process from scratch. > If so, having solib in the option name is confusing. > > set build-id-force > or > set require-build-id-match > or some such would be clearer. The new patchset is being cooked as the last commits without ChangeLogs at: https://sourceware.org/git/?p=archer.git;a=log;h=refs/heads/jankratochvil/gdbserverbuildid Particularly: https://sourceware.org/git/?p=archer.git;a=commitdiff;h=79c03cbb287878d3e5fcfb8104bdd21aa712f013 -set solib-build-id-force (on|off) +set build-id-force (on|off) > > +/* Validate SO by comparing build-id from the associated bfd and > > + corresponding build-id from target memory. */ > > Please document that the result is an error message or NULL for success > (including missing build id), and that the caller must free it. > I realize you say so in the docs for the "validate" "method", > but the comment here doesn't mention it is the validate method > (which would be a fine alternative to repeating all the docs > of the method). I agree; although it gets reworked in the add-on patchset anyway. https://sourceware.org/git/?p=archer.git;a=commitdiff;h=6d40ae1db39bdabb415a05aa909178d61cb519ed > > + > > +static char * > > +svr4_validate (const struct so_list *const so) > > @@ -1146,6 +1215,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) > > + { > > This happens for a malformed build id, right? Yes. > A warning would be useful here. > It'd also be nice to have a warning for an odd count. OK. > > --- 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. > > Including the note header will be confusing to readers. > Is there a reason to include it? It seems to simplify all the code. I will recheck how the code looks if it parses the note. Thanks, Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 09/10] Validate symbol file using build-id 2015-06-23 20:47 ` Jan Kratochvil @ 2015-07-08 14:44 ` Pedro Alves 2015-07-12 19:09 ` Jan Kratochvil 0 siblings, 1 reply; 10+ messages in thread From: Pedro Alves @ 2015-07-08 14:44 UTC (permalink / raw) To: Jan Kratochvil, Doug Evans; +Cc: gdb-patches, Aleksandar Ristovski Jan Kratochvil wrote: > +static char * > +svr4_validate (const struct so_list *const so) > +{ ... > + return xstrprintf (_("remote build ID is %s " > + "but local file does not have build ID"), > + remote_hex); Seems odd to say "remote" here. Can't these errors trigger with native debugging as well? Doug Evans wrote: > >> > If so, having solib in the option name is confusing. >> > >> > set build-id-force >> > or >> > set require-build-id-match >> > or some such would be clearer. "build-id-force" sound odd to me. The latter sounds OK, as would "set build-id-validation on/off/...". With that, and once the previous issues raised are addressed, I think this is good to go. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v8 09/10] Validate symbol file using build-id 2015-07-08 14:44 ` Pedro Alves @ 2015-07-12 19:09 ` Jan Kratochvil 2015-07-12 19:29 ` [PATCH v9 " Jan Kratochvil 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-07-12 19:09 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, gdb-patches, Aleksandar Ristovski On Wed, 08 Jul 2015 16:44:25 +0200, Pedro Alves wrote: > Jan Kratochvil wrote: > > > +static char * > > +svr4_validate (const struct so_list *const so) > > +{ > > ... > > > + return xstrprintf (_("remote build ID is %s " > > + "but local file does not have build ID"), > > + remote_hex); > > Seems odd to say "remote" here. Can't these errors trigger with native > debugging as well? For this patchset not yet. The message is changed in a later patchset being prepared where it applies also for local files. > Doug Evans wrote: > > > >> > If so, having solib in the option name is confusing. > >> > > >> > set build-id-force > >> > or > >> > set require-build-id-match > >> > or some such would be clearer. > > "build-id-force" sound odd to me. The latter sounds OK, > as would "set build-id-validation on/off/...". OK, I will change it to "set build-id-validation on/off/...". > With that, and once the previous issues raised are > addressed, I think this is good to go. There patches are left without reply: [PATCH v7 04/10] Create empty nat/linux-maps.[ch] and common/target-utils.[ch] [PATCH v7 06/10] Prepare linux_find_memory_regions_full & co. for move [PATCH v7 07/10] Move linux_find_memory_regions_full & co. There particularly the [PATCH v7 06/10] contains non-mechanical (a bit) changes. Thanks, Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v9 09/10] Validate symbol file using build-id 2015-07-12 19:09 ` Jan Kratochvil @ 2015-07-12 19:29 ` Jan Kratochvil 2015-07-12 19:54 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-07-12 19:29 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, gdb-patches, Aleksandar Ristovski, Eli Zaretskii [-- Attachment #1: Type: text/plain, Size: 615 bytes --] On Sun, 12 Jul 2015 21:09:21 +0200, Jan Kratochvil wrote: > On Wed, 08 Jul 2015 16:44:25 +0200, Pedro Alves wrote: > > Doug Evans wrote: > > > > > >> > If so, having solib in the option name is confusing. > > >> > > > >> > set build-id-force > > >> > or > > >> > set require-build-id-match > > >> > or some such would be clearer. > > > > "build-id-force" sound odd to me. The latter sounds OK, > > as would "set build-id-validation on/off/...". > > OK, I will change it to "set build-id-validation on/off/...". It required some language changes as it reversed its on<->off meaning, therefore Ccing Eli. Jan [-- Attachment #2: Type: message/rfc822, Size: 18973 bytes --] From: Jan Kratochvil <jan.kratochvil@redhat.com> Subject: [PATCH] Validate symbol file using build-id Date: Sun, 12 Jul 2015 20:59:02 +0200 Hi, consumer part of the "build-id" attribute. Approved by: https://sourceware.org/ml/gdb-patches/2014-05/msg00424.html Jan gdb/ChangeLog 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.9): Add 'set solib-build-id-validation' and 'show solib-build-id-validation'. 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-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. (svr4_copy_library_list): Duplicate field build_id. (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_validation, show_solib_build_id_validation): New. (solib_map_sections): Use ops->validate. (clear_so): Free build_id. (default_solib_validate): New function. (_initialize_solib): Add "solib-build-id-validation". * 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/ChangeLog 2014-03-02 Jan Kratochvil <jan.kratochvil@redhat.com> * gdb.texinfo (Files): Add 'set solib-build-id-validation' and 'show solib-build-id-validation'. --- gdb/NEWS | 12 ++++++ gdb/doc/gdb.texinfo | 38 +++++++++++++++++++ gdb/solib-darwin.c | 1 + gdb/solib-dsbt.c | 1 + gdb/solib-frv.c | 1 + gdb/solib-spu.c | 1 + gdb/solib-svr4.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/solib-target.c | 2 + gdb/solib.c | 64 +++++++++++++++++++++++++++++++- gdb/solib.h | 4 ++ gdb/solist.h | 18 +++++++++ 11 files changed, 244 insertions(+), 1 deletion(-) diff --git a/gdb/NEWS b/gdb/NEWS index 7ce9758..b07c973 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -194,6 +194,12 @@ maint set|show btrace pt skip-pad Set and show whether PAD packets are skipped when computing the packet history. +set solib-build-id-validation (on|off) +show solib-build-id-validation + 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. + * The command 'thread apply all' can now support new option '-ascending' to call its specified command for all threads in ascending order. @@ -277,6 +283,12 @@ fork-events and vfork-events features in qSupported * GDB now supports access to vector registers on S/390 GNU/Linux targets. +* 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. + * Removed command line options -xdb HP-UX XDB compatibility mode. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 932c38d..b017a6d 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -17950,6 +17950,44 @@ libraries that were loaded by explicit user requests are not discarded. @end table +@table @code +@kindex set solib-build-id-validation +@cindex override @value{GDBN} build-id check +@item set solib-build-id-validation @var{mode} +Setting to override @value{GDBN} build-id check. + +Inferior shared libraries and symbol files may contain unique build-id. +By default @value{GDBN} will ignore symbol files with non-matching build-id +while printing: + +@smallexample + warning: Shared object "libfoo.so.1" could not be validated (remote + build ID 2bc1745e does not match local build ID a08f8767) and will be + ignored; or use 'set solib-build-id-validation off'. +@end smallexample + +Turning off this setting would load such symbol file while still printing: + +@smallexample + warning: Shared object "libfoo.so.1" could not be validated (remote + build ID 2bc1745e does not match local build ID a08f8767) but it is + being loaded due to 'set solib-build-id-validation off'. +@end smallexample + +If remote build-id is present but it does not match local build-id (or local +build-id is not present) then this setting enables (@var{mode} is @code{off}) or +disables (@var{mode} is @code{on}) loading of such symbol file. On systems +where build-id is not present in the remote system this setting has no effect. +The default value is @code{on}. + +Loading non-matching symbol file may confuse debugging including breakage +of backtrace output. + +@kindex show solib-build-id-validation +@item show solib-build-id-validation +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 f96841f..9309c35 100644 --- a/gdb/solib-darwin.c +++ b/gdb/solib-darwin.c @@ -634,4 +634,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 7da5833..9fe6533 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -1080,6 +1080,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 f7ef38b..b768146 100644 --- a/gdb/solib-frv.c +++ b/gdb/solib-frv.c @@ -1183,6 +1183,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-spu.c b/gdb/solib-spu.c index 44fbf91..d162884 100644 --- a/gdb/solib-spu.c +++ b/gdb/solib-spu.c @@ -519,6 +519,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 909dfb7..71522c6 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -45,6 +45,9 @@ #include "auxv.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); @@ -970,6 +973,64 @@ 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. Return NULL for success + or a string for error. Caller must call xfree for the error string. */ + +static char * +svr4_validate (const struct so_list *const so) +{ + const bfd_byte *local_id; + size_t local_idsz; + + gdb_assert (so != NULL); + + /* Target doesn't support reporting the build ID or the remote shared library + does not have build ID. */ + if (so->build_id == NULL) + return NULL; + + /* Build ID may be present in the local file, just GDB is unable to retrieve + it. As it has been reported by gdbserver it is not FSF gdbserver. */ + if (so->abfd == NULL + || !bfd_check_format (so->abfd, bfd_object)) + return NULL; + + /* GDB has verified the local file really does not contain the build ID. */ + if (so->abfd->build_id == NULL) + { + char *remote_hex; + + remote_hex = alloca (so->build_idsz * 2 + 1); + bin2hex (so->build_id, remote_hex, so->build_idsz); + + return xstrprintf (_("remote build ID is %s " + "but local file does not have build ID"), + remote_hex); + } + + local_id = so->abfd->build_id->data; + local_idsz = so->abfd->build_id->size; + + if (so->build_idsz != local_idsz + || memcmp (so->build_id, local_id, so->build_idsz) != 0) + { + char *remote_hex, *local_hex; + + remote_hex = alloca (so->build_idsz * 2 + 1); + bin2hex (so->build_id, remote_hex, so->build_idsz); + local_hex = alloca (local_idsz * 2 + 1); + bin2hex (local_id, local_hex, local_idsz); + + return xstrprintf (_("remote build ID %s " + "does not match local build ID %s"), + remote_hex, local_hex); + } + + /* Both build IDs are present and they match. */ + return NULL; +} + /* Implement the "open_symbol_file_object" target_so_ops method. If no open symbol file, attempt to locate and open the main symbol @@ -1108,6 +1169,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; @@ -1135,6 +1202,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); @@ -1146,6 +1216,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; @@ -1180,6 +1281,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 } }; @@ -3258,4 +3360,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 f14363a..13cbd26 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" /* Private data for each loaded library. */ struct lm_info @@ -506,6 +507,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 eb933c0..2ab8b80 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -518,6 +518,20 @@ solib_bfd_open (char *pathname) return abfd; } +/* Boolean for command 'set solib-build-id-validation'. */ +static int solib_build_id_validation = 1; + +/* Implement 'show solib-build-id-validation'. */ + +static void +show_solib_build_id_validation (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); +} + /* 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 @@ -534,7 +548,7 @@ static int solib_map_sections (struct so_list *so) { const struct target_so_ops *ops = solib_ops (target_gdbarch ()); - char *filename; + char *filename, *validate_error; struct target_section *p; struct cleanup *old_chain; bfd *abfd; @@ -550,6 +564,29 @@ 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); + + validate_error = ops->validate (so); + if (validate_error != NULL) + { + if (solib_build_id_validation) + { + warning (_("Shared object \"%s\" could not be validated (%s) and " + "will be ignored; " + "or use 'set solib-build-id-validation off'."), + so->so_name, validate_error); + xfree (validate_error); + gdb_bfd_unref (so->abfd); + so->abfd = NULL; + return 0; + } + warning (_("Shared object \"%s\" could not be validated (%s) " + "but it is being loaded due to " + "'set solib-build-id-validation off'."), + so->so_name, validate_error); + xfree (validate_error); + } + /* 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 @@ -626,6 +663,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); @@ -1657,6 +1697,14 @@ remove_user_added_objfile (struct objfile *objfile) } } +/* Default implementation does not perform any validation. */ + +char * +default_solib_validate (const struct so_list *const so) +{ + return NULL; /* No validation. */ +} + extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */ void @@ -1714,4 +1762,18 @@ PATH and LD_LIBRARY_PATH."), reload_shared_libraries, show_solib_search_path, &setlist, &showlist); + + add_setshow_boolean_cmd ("solib-build-id-validation", class_support, + &solib_build_id_validation, _("\ +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_solib_build_id_validation, + &setlist, &showlist); + } diff --git a/gdb/solib.h b/gdb/solib.h index 336971d..c3bf529 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 char *default_solib_validate (const struct so_list *so); + #endif /* SOLIB_H */ diff --git a/gdb/solist.h b/gdb/solist.h index 7021f5c..e6e74af 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 @@ -168,6 +181,11 @@ struct target_so_ops NULL, in which case no specific preprocessing is necessary for this target. */ void (*handle_event) (void); + + /* Return NULL if SO does match target SO it is supposed to + represent. Otherwise return string describing why it does not match. + Caller has to free the string. */ + char *(*validate) (const struct so_list *so); }; /* Free the memory associated with a (so_list *). */ -- 2.1.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-12 19:29 ` [PATCH v9 " Jan Kratochvil @ 2015-07-12 19:54 ` Eli Zaretskii 2015-07-12 20:01 ` Jan Kratochvil 2015-07-13 10:34 ` Pedro Alves 0 siblings, 2 replies; 10+ messages in thread From: Eli Zaretskii @ 2015-07-12 19:54 UTC (permalink / raw) To: Jan Kratochvil; +Cc: palves, dje, gdb-patches, ARistovski > Date: Sun, 12 Jul 2015 21:29:11 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > Cc: Doug Evans <dje@google.com>, gdb-patches <gdb-patches@sourceware.org>, > Aleksandar Ristovski <ARistovski@qnx.com>, > Eli Zaretskii <eliz@gnu.org> > > > > "build-id-force" sound odd to me. The latter sounds OK, > > > as would "set build-id-validation on/off/...". > > > > OK, I will change it to "set build-id-validation on/off/...". > > It required some language changes as it reversed its on<->off meaning, > therefore Ccing Eli. It's OK, although I wonder whether solib-validate-build-id is perhaps a better name. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-12 19:54 ` Eli Zaretskii @ 2015-07-12 20:01 ` Jan Kratochvil 2015-07-13 2:30 ` Eli Zaretskii 2015-07-13 10:34 ` Pedro Alves 1 sibling, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-07-12 20:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: palves, dje, gdb-patches, ARistovski On Sun, 12 Jul 2015 21:54:17 +0200, Eli Zaretskii wrote: > > > OK, I will change it to "set build-id-validation on/off/...". > > > > It required some language changes as it reversed its on<->off meaning, > > therefore Ccing Eli. > > It's OK, although I wonder whether solib-validate-build-id is perhaps > a better name. I do not have an opinion on it so tell me if there is some settlement on a change request. Thanks, Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-12 20:01 ` Jan Kratochvil @ 2015-07-13 2:30 ` Eli Zaretskii 0 siblings, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2015-07-13 2:30 UTC (permalink / raw) To: Jan Kratochvil; +Cc: palves, dje, gdb-patches, ARistovski > Date: Sun, 12 Jul 2015 22:01:08 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > Cc: palves@redhat.com, dje@google.com, gdb-patches@sourceware.org, > ARistovski@qnx.com > > On Sun, 12 Jul 2015 21:54:17 +0200, Eli Zaretskii wrote: > > > > OK, I will change it to "set build-id-validation on/off/...". > > > > > > It required some language changes as it reversed its on<->off meaning, > > > therefore Ccing Eli. > > > > It's OK, although I wonder whether solib-validate-build-id is perhaps > > a better name. > > I do not have an opinion on it so tell me if there is some settlement on > a change request. It was just a thought, feel free to disregard if you are okay with the name you suggested. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-12 19:54 ` Eli Zaretskii 2015-07-12 20:01 ` Jan Kratochvil @ 2015-07-13 10:34 ` Pedro Alves 2015-07-13 12:38 ` Jan Kratochvil 1 sibling, 1 reply; 10+ messages in thread From: Pedro Alves @ 2015-07-13 10:34 UTC (permalink / raw) To: Eli Zaretskii, Jan Kratochvil; +Cc: dje, gdb-patches, ARistovski On 07/12/2015 08:54 PM, Eli Zaretskii wrote: >> Date: Sun, 12 Jul 2015 21:29:11 +0200 >> From: Jan Kratochvil <jan.kratochvil@redhat.com> >> Cc: Doug Evans <dje@google.com>, gdb-patches <gdb-patches@sourceware.org>, >> Aleksandar Ristovski <ARistovski@qnx.com>, >> Eli Zaretskii <eliz@gnu.org> >> >>>> "build-id-force" sound odd to me. The latter sounds OK, >>>> as would "set build-id-validation on/off/...". >>> >>> OK, I will change it to "set build-id-validation on/off/...". >> >> It required some language changes as it reversed its on<->off meaning, >> therefore Ccing Eli. > > It's OK, although I wonder whether solib-validate-build-id is perhaps > a better name. As the plan is for the feature to apply to the main executable too, it's better that "solib" isn't part of the setting name. I agree that "validate-build-id" sounds better than "build-id-validation". Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-13 10:34 ` Pedro Alves @ 2015-07-13 12:38 ` Jan Kratochvil 2015-07-14 16:14 ` Doug Evans 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-07-13 12:38 UTC (permalink / raw) To: Pedro Alves; +Cc: Eli Zaretskii, dje, gdb-patches, ARistovski On Mon, 13 Jul 2015 12:34:33 +0200, Pedro Alves wrote: > As the plan is for the feature to apply to the main executable too, > it's better that "solib" isn't part of the setting name. I agree > that "validate-build-id" sounds better than "build-id-validation". OK, so validate-build-id or solib-validate-build-id. Just this series being approved is still only for shlib and I do not yet have completely finished the second series for all binaries; although 7.10 is branched so till 7.11 there is now really enough time to have it all. So I do not know if I should call it in this shlib-only first series as validate-build-id or as solib-validate-build-id. Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-13 12:38 ` Jan Kratochvil @ 2015-07-14 16:14 ` Doug Evans 2015-07-15 8:21 ` Jan Kratochvil 0 siblings, 1 reply; 10+ messages in thread From: Doug Evans @ 2015-07-14 16:14 UTC (permalink / raw) To: Jan Kratochvil Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Aleksandar Ristovski On Mon, Jul 13, 2015 at 5:38 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote: > On Mon, 13 Jul 2015 12:34:33 +0200, Pedro Alves wrote: >> As the plan is for the feature to apply to the main executable too, >> it's better that "solib" isn't part of the setting name. I agree >> that "validate-build-id" sounds better than "build-id-validation". > > OK, so validate-build-id or solib-validate-build-id. > > Just this series being approved is still only for shlib and I do not yet have > completely finished the second series for all binaries; although 7.10 is > branched so till 7.11 there is now really enough time to have it all. > > So I do not know if I should call it in this shlib-only first series as > validate-build-id or as solib-validate-build-id. One of the important bits for me is to not add an option knowing we're going to rename it later. We've just branched 7.10 and I'm guessing all of this will be present for 7.11. So I'd be ok with just adding a simple note somewhere saying the option only applies to solibs at the moment and will eventually also apply to exes. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-14 16:14 ` Doug Evans @ 2015-07-15 8:21 ` Jan Kratochvil 2015-07-15 14:59 ` Doug Evans 0 siblings, 1 reply; 10+ messages in thread From: Jan Kratochvil @ 2015-07-15 8:21 UTC (permalink / raw) To: Doug Evans; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Aleksandar Ristovski On Tue, 14 Jul 2015 18:13:26 +0200, Doug Evans wrote: > So I'd be ok with just adding a simple note somewhere saying > the option only applies to solibs at the moment and will > eventually also apply to exes. So the option is now called 'validate-build-id'. Its description everywhere (NEWS+gdb.texinfo+'help set validate-build-id') talks about shared library now. Is it OK this way or should it be changed? Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v9 09/10] Validate symbol file using build-id 2015-07-15 8:21 ` Jan Kratochvil @ 2015-07-15 14:59 ` Doug Evans 0 siblings, 0 replies; 10+ messages in thread From: Doug Evans @ 2015-07-15 14:59 UTC (permalink / raw) To: Jan Kratochvil Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Aleksandar Ristovski On Wed, Jul 15, 2015 at 1:21 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote: > On Tue, 14 Jul 2015 18:13:26 +0200, Doug Evans wrote: >> So I'd be ok with just adding a simple note somewhere saying >> the option only applies to solibs at the moment and will >> eventually also apply to exes. > > So the option is now called 'validate-build-id'. > > Its description everywhere (NEWS+gdb.texinfo+'help set validate-build-id') > talks about shared library now. Is it OK this way or should it be changed? fine by me. thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-07-15 14:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-13 12:48 [PATCH v9 09/10] Validate symbol file using build-id Aleksandar Ristovski -- strict thread matches above, loose matches on Subject: below -- 2015-06-14 19:25 [PATCH v7 00/10] Validate binary before use Jan Kratochvil 2015-06-14 19:27 ` [PATCH v7 09/10] Validate symbol file using build-id Jan Kratochvil 2015-06-21 10:16 ` [PATCH v8 " Jan Kratochvil 2015-06-22 22:25 ` Doug Evans 2015-06-23 20:47 ` Jan Kratochvil 2015-07-08 14:44 ` Pedro Alves 2015-07-12 19:09 ` Jan Kratochvil 2015-07-12 19:29 ` [PATCH v9 " Jan Kratochvil 2015-07-12 19:54 ` Eli Zaretskii 2015-07-12 20:01 ` Jan Kratochvil 2015-07-13 2:30 ` Eli Zaretskii 2015-07-13 10:34 ` Pedro Alves 2015-07-13 12:38 ` Jan Kratochvil 2015-07-14 16:14 ` Doug Evans 2015-07-15 8:21 ` Jan Kratochvil 2015-07-15 14:59 ` Doug Evans
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox