[PATCH 1/2] Make probe_ops::get_probes fill an std::vector
Sergio Durigan Junior
sergiodj@redhat.com
Sun Sep 10 17:40:00 GMT 2017
On Sunday, September 10 2017, Simon Marchi wrote:
> This patch changes one usage of VEC to std::vector. It is a relatively
> straightforward 1:1 change. The implementations of
> sym_probe_fns::sym_get_probes return a borrowed reference to their probe
> vectors, meaning that the caller should not free it. In the new code, I
> made them return a const reference to the vector.
>
> This patch and the following one were tested by the buildbot. I didn't
> see any failures that looked related to this one.
Thanks for the patch, Simon. A few comments below.
> gdb/ChangeLog:
>
> * probe.h (struct probe_ops) <get_probes>: Change parameter from
> vec to std::vector.
> * probe.c (parse_probes_in_pspace): Update.
> (find_probes_in_objfile): Update.
> (find_probe_by_pc): Update.
> (collect_probes): Update.
> (probe_any_get_probes): Update.
> * symfile.h (struct sym_probe_fns) <sym_get_probes> Change
> return type to reference to std::vector.
> * dtrace-probe.c (dtrace_process_dof_probe): Change parameter to
> std::vector and update.
> (dtrace_process_dof): Likewise.
> (dtrace_get_probes): Likewise.
> * elfread.c (elf_get_probes): Change return type to std::vector,
> store an std::vector in bfd_data.
> (probe_key_free): Update to std::vector.
> * stap-probe.c (handle_stap_probe): Change parameter to
> std::vector and update.
> (stap_get_probes): Likewise.
> * symfile-debug.c (debug_sym_get_probes): Change return type to
> std::vector and update.
> ---
> gdb/dtrace-probe.c | 9 +++++----
> gdb/elfread.c | 27 ++++++++++-----------------
> gdb/probe.c | 38 ++++++++++++++------------------------
> gdb/probe.h | 2 +-
> gdb/stap-probe.c | 10 +++++-----
> gdb/symfile-debug.c | 8 ++++----
> gdb/symfile.h | 7 ++-----
> 7 files changed, 41 insertions(+), 60 deletions(-)
>
> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
> index c1a8cb0..f9209ec 100644
> --- a/gdb/dtrace-probe.c
> +++ b/gdb/dtrace-probe.c
> @@ -313,7 +313,8 @@ struct dtrace_dof_probe
>
> static void
> dtrace_process_dof_probe (struct objfile *objfile,
> - struct gdbarch *gdbarch, VEC (probe_p) **probesp,
> + struct gdbarch *gdbarch,
> + std::vector<probe *> *probesp,
Since you're doing this, what do you think about const-fying these
occurrences of "std::vector<probe *>"?
> struct dtrace_dof_hdr *dof,
> struct dtrace_dof_probe *probe,
> struct dtrace_dof_provider *provider,
> @@ -448,7 +449,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
> ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
>
> /* Successfully created probe. */
> - VEC_safe_push (probe_p, *probesp, (struct probe *) ret);
> + probesp->push_back ((struct probe *) ret);
> }
>
> do_cleanups (cleanup);
> @@ -461,7 +462,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
>
> static void
> dtrace_process_dof (asection *sect, struct objfile *objfile,
> - VEC (probe_p) **probesp, struct dtrace_dof_hdr *dof)
> + std::vector<probe *> *probesp, struct dtrace_dof_hdr *dof)
> {
> struct gdbarch *gdbarch = get_objfile_arch (objfile);
> struct dtrace_dof_sect *section;
> @@ -620,7 +621,7 @@ dtrace_get_arg (struct dtrace_probe *probe, unsigned n,
> /* Implementation of the get_probes method. */
>
> static void
> -dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> +dtrace_get_probes (std::vector<probe *> *probesp, struct objfile *objfile)
> {
> bfd *abfd = objfile->obfd;
> asection *sect = NULL;
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index f3d4641..8a64865 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1309,35 +1309,30 @@ elf_symfile_init (struct objfile *objfile)
>
> /* Implementation of `sym_get_probes', as documented in symfile.h. */
>
> -static VEC (probe_p) *
> +static const std::vector<probe *> &
> elf_get_probes (struct objfile *objfile)
> {
> - VEC (probe_p) *probes_per_bfd;
> + std::vector<probe *> *probes_per_bfd;
>
> /* Have we parsed this objfile's probes already? */
> - probes_per_bfd = (VEC (probe_p) *) bfd_data (objfile->obfd, probe_key);
> + probes_per_bfd = (std::vector<probe *> *) bfd_data (objfile->obfd, probe_key);
>
> - if (!probes_per_bfd)
> + if (probes_per_bfd == NULL)
> {
> int ix;
> const struct probe_ops *probe_ops;
> + probes_per_bfd = new std::vector<probe *>;
>
> /* Here we try to gather information about all types of probes from the
> objfile. */
> for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, probe_ops);
> ix++)
> - probe_ops->get_probes (&probes_per_bfd, objfile);
> -
> - if (probes_per_bfd == NULL)
> - {
> - VEC_reserve (probe_p, probes_per_bfd, 1);
> - gdb_assert (probes_per_bfd != NULL);
> - }
> + probe_ops->get_probes (probes_per_bfd, objfile);
>
> set_bfd_data (objfile->obfd, probe_key, probes_per_bfd);
> }
>
> - return probes_per_bfd;
> + return *probes_per_bfd;
> }
>
> /* Helper function used to free the space allocated for storing SystemTap
> @@ -1346,14 +1341,12 @@ elf_get_probes (struct objfile *objfile)
> static void
> probe_key_free (bfd *abfd, void *d)
> {
> - int ix;
> - VEC (probe_p) *probes = (VEC (probe_p) *) d;
> - struct probe *probe;
> + std::vector<probe *> *probes = (std::vector<probe *> *) d;
>
> - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++)
> + for (struct probe *probe : *probes)
> probe->pops->destroy (probe);
>
> - VEC_free (probe_p, probes);
> + delete probes;
> }
>
>
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 686e90e..2d68437 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -58,10 +58,6 @@ parse_probes_in_pspace (const struct probe_ops *probe_ops,
>
> ALL_PSPACE_OBJFILES (search_pspace, objfile)
> {
> - VEC (probe_p) *probes;
> - struct probe *probe;
> - int ix;
> -
> if (!objfile->sf || !objfile->sf->sym_probe_fns)
> continue;
>
> @@ -71,9 +67,10 @@ parse_probes_in_pspace (const struct probe_ops *probe_ops,
> objfile_namestr) != 0)
> continue;
>
> - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
> + const std::vector<probe *> &probes
> + = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
>
> - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++)
> + for (struct probe *probe : probes)
> {
> if (probe_ops != &probe_ops_any && probe->pops != probe_ops)
> continue;
> @@ -211,15 +208,14 @@ VEC (probe_p) *
> find_probes_in_objfile (struct objfile *objfile, const char *provider,
> const char *name)
Any reason for not converting this function's return as well?
> {
> - VEC (probe_p) *probes, *result = NULL;
> - int ix;
> - struct probe *probe;
> + VEC (probe_p) *result = NULL;
>
> if (!objfile->sf || !objfile->sf->sym_probe_fns)
> return NULL;
>
> - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
> - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++)
> + const std::vector<probe *> &probes
> + = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
> + for (struct probe *probe : probes)
> {
> if (strcmp (probe->provider, provider) != 0)
> continue;
> @@ -246,17 +242,14 @@ find_probe_by_pc (CORE_ADDR pc)
>
> ALL_OBJFILES (objfile)
> {
> - VEC (probe_p) *probes;
> - int ix;
> - struct probe *probe;
> -
> if (!objfile->sf || !objfile->sf->sym_probe_fns
> || objfile->sect_index_text == -1)
> continue;
>
> /* If this proves too inefficient, we can replace with a hash. */
> - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
> - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++)
> + const std::vector<probe *> &probes
> + = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
> + for (struct probe *probe : probes)
> if (get_probe_address (probe, objfile) == pc)
> {
> result.objfile = objfile;
> @@ -294,10 +287,6 @@ collect_probes (char *objname, char *provider, char *probe_name,
>
> ALL_OBJFILES (objfile)
> {
> - VEC (probe_p) *probes;
> - struct probe *probe;
> - int ix;
> -
> if (! objfile->sf || ! objfile->sf->sym_probe_fns)
> continue;
>
> @@ -307,9 +296,10 @@ collect_probes (char *objname, char *provider, char *probe_name,
> continue;
> }
>
> - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
> + const std::vector<probe *> &probes
> + = objfile->sf->sym_probe_fns->sym_get_probes (objfile);
>
> - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++)
> + for (struct probe *probe : probes)
> {
> struct bound_probe bound;
>
> @@ -907,7 +897,7 @@ probe_any_is_linespec (const char **linespecp)
> /* Dummy method used for `probe_ops_any'. */
>
> static void
> -probe_any_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> +probe_any_get_probes (std::vector<probe *> *probesp, struct objfile *objfile)
> {
> /* No probes can be provided by this dummy backend. */
> }
> diff --git a/gdb/probe.h b/gdb/probe.h
> index db7f1d1..61e3031 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -64,7 +64,7 @@ struct probe_ops
>
> /* Function that should fill PROBES with known probes from OBJFILE. */
>
> - void (*get_probes) (VEC (probe_p) **probes, struct objfile *objfile);
> + void (*get_probes) (std::vector<probe *> *probes, struct objfile *objfile);
>
> /* Compute the probe's relocated address. OBJFILE is the objfile
> in which the probe originated. */
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index c0cc662..032f796 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -1472,7 +1472,7 @@ stap_clear_semaphore (struct probe *probe_generic, struct objfile *objfile,
>
> static void
> handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
> - VEC (probe_p) **probesp, CORE_ADDR base)
> + std::vector<probe *> *probesp, CORE_ADDR base)
> {
> bfd *abfd = objfile->obfd;
> int size = bfd_get_arch_size (abfd) / 8;
> @@ -1543,7 +1543,7 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
> ret->args_u.text = probe_args;
>
> /* Successfully created probe. */
> - VEC_safe_push (probe_p, *probesp, (struct probe *) ret);
> + probesp->push_back ((struct probe *) ret);
> }
>
> /* Helper function which tries to find the base address of the SystemTap
> @@ -1588,7 +1588,7 @@ get_stap_base_address (bfd *obfd, bfd_vma *base)
> SystemTap probes from OBJFILE. */
>
> static void
> -stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> +stap_get_probes (std::vector<probe *> *probesp, struct objfile *objfile)
> {
> /* If we are here, then this is the first time we are parsing the
> SystemTap probe's information. We basically have to count how many
> @@ -1597,7 +1597,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> bfd *obfd = objfile->obfd;
> bfd_vma base;
> struct sdt_note *iter;
> - unsigned save_probesp_len = VEC_length (probe_p, *probesp);
> + unsigned save_probesp_len = probesp->size ();
>
> if (objfile->separate_debug_objfile_backlink != NULL)
> {
> @@ -1628,7 +1628,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> handle_stap_probe (objfile, iter, probesp, base);
> }
>
> - if (save_probesp_len == VEC_length (probe_p, *probesp))
> + if (save_probesp_len == probesp->size ())
> {
> /* If we are here, it means we have failed to parse every known
> probe. */
> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index e7890c9..32dafa8 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -384,20 +384,20 @@ static const struct quick_symbol_functions debug_sym_quick_functions =
>
> /* Debugging version of struct sym_probe_fns. */
>
> -static VEC (probe_p) *
> +static const std::vector<probe *> &
> debug_sym_get_probes (struct objfile *objfile)
> {
> const struct debug_sym_fns_data *debug_data
> = ((const struct debug_sym_fns_data *)
> objfile_data (objfile, symfile_debug_objfile_data_key));
> - VEC (probe_p) *retval;
>
> - retval = debug_data->real_sf->sym_probe_fns->sym_get_probes (objfile);
> + const std::vector<probe *> &retval
> + = debug_data->real_sf->sym_probe_fns->sym_get_probes (objfile);
>
> fprintf_filtered (gdb_stdlog,
> "probes->sym_get_probes (%s) = %s\n",
> objfile_debug_name (objfile),
> - host_address_to_string (retval));
> + host_address_to_string (retval.data ()));
>
> return retval;
> }
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index bb47fdf..1f4460c 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -314,11 +314,8 @@ struct quick_symbol_functions
>
> struct sym_probe_fns
> {
> - /* If non-NULL, return an array of probe objects.
> -
> - The returned value does not have to be freed and it has lifetime of the
> - OBJFILE. */
> - VEC (probe_p) *(*sym_get_probes) (struct objfile *);
> + /* If non-NULL, return a reference to vector of probe objects. */
> + const std::vector<probe *> &(*sym_get_probes) (struct objfile *);
> };
>
> /* Structure to keep track of symbol reading functions for various
> --
> 2.7.4
I know this patch is supposed to touch only probe_ops::get_probes, but
I'd love to see the whole VEC (probe_p) replaced (a few places on
breakpoint.c are also using it). Well, maybe on another patch :-).
Thanks!
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
More information about the Gdb-patches
mailing list