From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39636 invoked by alias); 22 Jun 2015 22:25:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 39618 invoked by uid 89); 22 Jun 2015 22:25:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f42.google.com Received: from mail-oi0-f42.google.com (HELO mail-oi0-f42.google.com) (209.85.218.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 22 Jun 2015 22:25:55 +0000 Received: by oigx81 with SMTP id x81so132357594oig.1 for ; Mon, 22 Jun 2015 15:25:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=2jXddj7pDvTNXLLCEy8kKirf/pORUAk5JsvvC1Sv6+U=; b=T7MWc4DLAnrvIyKRs040X4PjwmUJCXqAJh1SGUyblzr9mwZk0aJeYcVvpPRQog1CNQ +yO01iuIGHwnT9XBbFcuPVi2g+fBPGqRmaVyiyKF+HVFEqlIgta3etHx5KQXze9k4MyF bPS3XkgkYNeY0HfOAiuq2vGx2ZANf1HDafFxszxVRlpZ6vLcDHScRudaGVgBqdAFiGM5 DdfVK9U1khtnUN+BWHCOPriNPuzPf+pGKL9aSAUC/Iu18Nj2JQll7WQUDrP0y6eNaA/K tflNRSVPkpUwCIenT8jtSsFHEYhQTnUtVza77MQlyvn4icOIlBi2IU851vqNpIlMT1VQ M3Uw== X-Gm-Message-State: ALoCoQmok+G8QUvPzRPXSYUqxA6SJZi3dy6J25ThYTI2G2Ov2Nbomv2xXBF7Qy5zFgISOk4Ah4Wa MIME-Version: 1.0 X-Received: by 10.202.80.204 with SMTP id e195mr25607307oib.116.1435011953030; Mon, 22 Jun 2015 15:25:53 -0700 (PDT) Received: by 10.182.89.99 with HTTP; Mon, 22 Jun 2015 15:25:52 -0700 (PDT) In-Reply-To: <20150621101644.GA12733@host1.jankratochvil.net> References: <20150614192542.18346.87859.stgit@host1.jankratochvil.net> <20150614192655.18346.17075.stgit@host1.jankratochvil.net> <20150621101644.GA12733@host1.jankratochvil.net> Date: Mon, 22 Jun 2015 22:25:00 -0000 Message-ID: Subject: Re: [PATCH v8 09/10] Validate symbol file using build-id From: Doug Evans To: Jan Kratochvil Cc: gdb-patches , Aleksandar Ristovski Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00449.txt.bz2 On Sun, Jun 21, 2015 at 5:16 AM, Jan Kratochvil wrote: > Hi, > > updated for: > commit c74f7d1c6c5a968330208757f476c67a4bb66643 > Author: Jon Turney > 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 > 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 Jan Kratochvil > > 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 > > * 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 >