This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Sanitize gdbarch access on probe/SDT API
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Tom Tromey <tromey at redhat dot com>
- Date: Thu, 05 Dec 2013 21:05:48 -0200
- Subject: Re: [PATCH] Sanitize gdbarch access on probe/SDT API
- Authentication-results: sourceware.org; auth=none
- References: <1386225226-18549-1-git-send-email-sergiodj at redhat dot com> <52A079DD dot 5050101 at redhat dot com> <m361r3hrrf dot fsf_-_ at redhat dot com>
On Thursday, December 05 2013, I wrote:
> Right, could you please take a look at the following approach and tell
> me what you think?
As it turns out, I saw Tom's approach to make probes program-space
independent, and my patch touches many points that are also touched by
Tom's. Therefore, I will wait until he commits his patches, and then
I'll rebase this one on top of them and resubmit.
Thanks,
> I tried to use get_selected_frame whenever I could, and when I couldn't,
> I used get_current_regcache.
>
> Tom, you asked me to try to use the expression's gdbarch for the
> compile_ax method, but as it turns out, the code would be too messy if I
> did that, so I have chosen to simplify it and use the agent_expr's
> gdbarch instead.
>
> I tested this patch on x86_64 Fedora 17 and ARM Fedora 19, and it works
> OK.
>
> Thanks,
>
> --
> Sergio
>
> 2013-12-06 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * break-catch-throw.c (fetch_probe_arguments): Use gdbarch from
> selected frame. Pass gdbarch to sym_get_probe_argument_count and
> sym_evaluate_probe_argument.
> * elfread.c (elf_get_probe_argument_count): Adjust declaration to
> accept gdbarch. Pass gdbarch to get_probe_argument_count.
> (elf_can_evaluate_probe_arguments): Likewise, but pass gdbarch to
> can_evaluate_probe_arguments.
> (elf_evaluate_probe_argument): Likewise, but pass gdbarch to
> evaluate_probe_argument.
> * probe.c (get_probe_argument_count): Use gdbarch from selected
> frame. Pass gdbarch to sym_get_probe_argument_count.
> (can_evaluate_probe_arguments): Likewise, but pass gdbarch to
> can_evaluate_probe_arguments.
> (evaluate_probe_argument): Likewise, but pass gdbarch to
> sym_evaluate_probe_argument.
> * probe.h (struct probe_ops) <get_probe_argument_count,
> can_evaluate_probe_arguments, evaluate_probe_argument>: Adjust
> declaration to accept gdbarch.
> * stap-probe.c: Include "regcache.h".
> (stap_parse_probe_arguments): Adjust declaration to accept
> gdbarch.
> (stap_get_probe_argument_count): Likewise. Pass gdbarch to
> can_evaluate_probe_arguments and stap_parse_probe_arguments.
> (stap_get_arg): Likewise, but pass gdbarch to
> stap_parse_probe_arguments.
> (stap_can_evaluate_probe_arguments): Adjust declaration to accept
> gdbarch.
> (stap_evaluate_probe_argument): Likewise. Pass gdbarch to
> stap_get_arg.
> (stap_compile_to_ax): Use agent_expr's gdbarch; pass it to
> stap_get_arg.
> (compute_probe_arg): Use gdbarch from selected frame. Pass
> gdbarch to sym_get_probe_argument_count and
> sym_evaluate_probe_argument.
> (compile_probe_arg): Use gdbarch from agent_expr. Pass it to
> sym_get_probe_argument_count.
> (handle_stap_probe): Adjust declaration to accept gdbarch.
> (stap_get_probes): Use gdbarch from curret regcache. Pass it to
> handle_stap_probe.
> (stap_gen_info_probes_table_values): Use gdbarch from selected
> frame.
> * symfile-debug.c (debug_sym_get_probe_argument_count): Adjust
> declaration to accept gdbarch. Pass it to
> sym_get_probe_argument_count.
> (debug_can_evaluate_probe_arguments): Likewise, but pass gdbarch
> to can_evaluate_probe_arguments.
> (debug_sym_evaluate_probe_argument): Likewise, but pass gdbarch to
> sym_evaluate_probe_argument.
> * symfile.h (struct sym_probe_fns) <sym_get_probe_argument_count,
> can_evaluate_probe_arguments, sym_evaluate_probe_argument>: Adjust
> declaration to accept gdbarch.
>
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index fb24725..769559b 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -105,6 +105,7 @@ static void
> fetch_probe_arguments (struct value **arg0, struct value **arg1)
> {
> struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> CORE_ADDR pc = get_frame_pc (frame);
> struct probe *pc_probe;
> const struct sym_probe_fns *pc_probe_fns;
> @@ -123,13 +124,13 @@ fetch_probe_arguments (struct value **arg0, struct value **arg1)
> gdb_assert (pc_probe->objfile->sf->sym_probe_fns != NULL);
>
> pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
> - n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
> + n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe, gdbarch);
> if (n_args < 2)
> error (_("C++ exception catchpoint has too few arguments"));
>
> if (arg0 != NULL)
> - *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0);
> - *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1);
> + *arg0 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 0, gdbarch);
> + *arg1 = pc_probe_fns->sym_evaluate_probe_argument (pc_probe, 1, gdbarch);
>
> if ((arg0 != NULL && *arg0 == NULL) || *arg1 == NULL)
> error (_("error computing probe argument at c++ exception catchpoint"));
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 42ab18f..b72cd96 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1509,27 +1509,30 @@ elf_get_probes (struct objfile *objfile)
> symfile.h. */
>
> static unsigned
> -elf_get_probe_argument_count (struct probe *probe)
> +elf_get_probe_argument_count (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> - return probe->pops->get_probe_argument_count (probe);
> + return probe->pops->get_probe_argument_count (probe, gdbarch);
> }
>
> /* Implementation of `sym_can_evaluate_probe_arguments', as documented in
> symfile.h. */
>
> static int
> -elf_can_evaluate_probe_arguments (struct probe *probe)
> +elf_can_evaluate_probe_arguments (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> - return probe->pops->can_evaluate_probe_arguments (probe);
> + return probe->pops->can_evaluate_probe_arguments (probe, gdbarch);
> }
>
> /* Implementation of `sym_evaluate_probe_argument', as documented in
> symfile.h. */
>
> static struct value *
> -elf_evaluate_probe_argument (struct probe *probe, unsigned n)
> +elf_evaluate_probe_argument (struct probe *probe, unsigned n,
> + struct gdbarch *gdbarch)
> {
> - return probe->pops->evaluate_probe_argument (probe, n);
> + return probe->pops->evaluate_probe_argument (probe, n, gdbarch);
> }
>
> /* Implementation of `sym_compile_to_ax', as documented in symfile.h. */
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 4046701..37314dd 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -617,6 +617,8 @@ unsigned
> get_probe_argument_count (struct probe *probe)
> {
> const struct sym_probe_fns *probe_fns;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> gdb_assert (probe->objfile != NULL);
> gdb_assert (probe->objfile->sf != NULL);
> @@ -625,7 +627,7 @@ get_probe_argument_count (struct probe *probe)
>
> gdb_assert (probe_fns != NULL);
>
> - return probe_fns->sym_get_probe_argument_count (probe);
> + return probe_fns->sym_get_probe_argument_count (probe, gdbarch);
> }
>
> /* See comments in probe.h. */
> @@ -634,6 +636,8 @@ int
> can_evaluate_probe_arguments (struct probe *probe)
> {
> const struct sym_probe_fns *probe_fns;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> gdb_assert (probe->objfile != NULL);
> gdb_assert (probe->objfile->sf != NULL);
> @@ -642,7 +646,7 @@ can_evaluate_probe_arguments (struct probe *probe)
>
> gdb_assert (probe_fns != NULL);
>
> - return probe_fns->can_evaluate_probe_arguments (probe);
> + return probe_fns->can_evaluate_probe_arguments (probe, gdbarch);
> }
>
> /* See comments in probe.h. */
> @@ -651,6 +655,8 @@ struct value *
> evaluate_probe_argument (struct probe *probe, unsigned n)
> {
> const struct sym_probe_fns *probe_fns;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> gdb_assert (probe->objfile != NULL);
> gdb_assert (probe->objfile->sf != NULL);
> @@ -659,7 +665,7 @@ evaluate_probe_argument (struct probe *probe, unsigned n)
>
> gdb_assert (probe_fns != NULL);
>
> - return probe_fns->sym_evaluate_probe_argument (probe, n);
> + return probe_fns->sym_evaluate_probe_argument (probe, n, gdbarch);
> }
>
> /* See comments in probe.h. */
> diff --git a/gdb/probe.h b/gdb/probe.h
> index dd5387b..6a08f3d 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -71,19 +71,22 @@ struct probe_ops
>
> /* Return the number of arguments of PROBE. */
>
> - unsigned (*get_probe_argument_count) (struct probe *probe);
> + unsigned (*get_probe_argument_count) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Return 1 if the probe interface can evaluate the arguments of probe
> PROBE, zero otherwise. See the comments on
> sym_probe_fns:can_evaluate_probe_arguments for more details. */
>
> - int (*can_evaluate_probe_arguments) (struct probe *probe);
> + int (*can_evaluate_probe_arguments) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Evaluate the Nth argument from the PROBE, returning a value
> corresponding to it. The argument number is represented N. */
>
> struct value *(*evaluate_probe_argument) (struct probe *probe,
> - unsigned n);
> + unsigned n,
> + struct gdbarch *gdbarch);
>
> /* Compile the Nth argument of the PROBE to an agent expression.
> The argument number is represented by N. */
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index a734793..e8b5805 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -38,6 +38,7 @@
> #include "parser-defs.h"
> #include "language.h"
> #include "elf-bfd.h"
> +#include "regcache.h"
>
> #include <ctype.h>
>
> @@ -914,10 +915,9 @@ stap_parse_argument (const char **arg, struct type *atype,
> this information. */
>
> static void
> -stap_parse_probe_arguments (struct stap_probe *probe)
> +stap_parse_probe_arguments (struct stap_probe *probe, struct gdbarch *gdbarch)
> {
> const char *cur;
> - struct gdbarch *gdbarch = get_objfile_arch (probe->p.objfile);
>
> gdb_assert (!probe->args_parsed);
> cur = probe->args_u.text;
> @@ -1002,7 +1002,8 @@ stap_parse_probe_arguments (struct stap_probe *probe)
> argument string. */
>
> static unsigned
> -stap_get_probe_argument_count (struct probe *probe_generic)
> +stap_get_probe_argument_count (struct probe *probe_generic,
> + struct gdbarch *gdbarch)
> {
> struct stap_probe *probe = (struct stap_probe *) probe_generic;
>
> @@ -1010,8 +1011,9 @@ stap_get_probe_argument_count (struct probe *probe_generic)
>
> if (!probe->args_parsed)
> {
> - if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic))
> - stap_parse_probe_arguments (probe);
> + if (probe_generic->pops->can_evaluate_probe_arguments (probe_generic,
> + gdbarch))
> + stap_parse_probe_arguments (probe, gdbarch);
> else
> {
> static int have_warned_stap_incomplete = 0;
> @@ -1072,10 +1074,10 @@ stap_is_operator (const char *op)
> }
>
> static struct stap_probe_arg *
> -stap_get_arg (struct stap_probe *probe, unsigned n)
> +stap_get_arg (struct stap_probe *probe, unsigned n, struct gdbarch *gdbarch)
> {
> if (!probe->args_parsed)
> - stap_parse_probe_arguments (probe);
> + stap_parse_probe_arguments (probe, gdbarch);
>
> return VEC_index (stap_probe_arg_s, probe->args_u.vec, n);
> }
> @@ -1083,10 +1085,10 @@ stap_get_arg (struct stap_probe *probe, unsigned n)
> /* Implement the `can_evaluate_probe_arguments' method of probe_ops. */
>
> static int
> -stap_can_evaluate_probe_arguments (struct probe *probe_generic)
> +stap_can_evaluate_probe_arguments (struct probe *probe_generic,
> + struct gdbarch *gdbarch)
> {
> struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
> - struct gdbarch *gdbarch = get_objfile_arch (stap_probe->p.objfile);
>
> /* For SystemTap probes, we have to guarantee that the method
> stap_is_single_operand is defined on gdbarch. If it is not, then it
> @@ -1098,7 +1100,8 @@ stap_can_evaluate_probe_arguments (struct probe *probe_generic)
> corresponding to it. Assertion is thrown if N does not exist. */
>
> static struct value *
> -stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n)
> +stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n,
> + struct gdbarch *gdbarch)
> {
> struct stap_probe *stap_probe = (struct stap_probe *) probe_generic;
> struct stap_probe_arg *arg;
> @@ -1106,7 +1109,7 @@ stap_evaluate_probe_argument (struct probe *probe_generic, unsigned n)
>
> gdb_assert (probe_generic->pops == &stap_probe_ops);
>
> - arg = stap_get_arg (stap_probe, n);
> + arg = stap_get_arg (stap_probe, n, gdbarch);
> return evaluate_subexp_standard (arg->atype, arg->aexpr, &pos, EVAL_NORMAL);
> }
>
> @@ -1123,7 +1126,7 @@ stap_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
>
> gdb_assert (probe_generic->pops == &stap_probe_ops);
>
> - arg = stap_get_arg (stap_probe, n);
> + arg = stap_get_arg (stap_probe, n, expr->gdbarch);
>
> pc = arg->aexpr->elts;
> gen_expr (arg->aexpr, &pc, expr, value);
> @@ -1165,6 +1168,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> {
> struct frame_info *frame = get_selected_frame (_("No frame selected"));
> CORE_ADDR pc = get_frame_pc (frame);
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> int sel = (int) (uintptr_t) data;
> struct probe *pc_probe;
> const struct sym_probe_fns *pc_probe_fns;
> @@ -1183,7 +1187,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
>
> pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
>
> - n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
> + n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe, gdbarch);
> if (sel == -1)
> return value_from_longest (builtin_type (arch)->builtin_int, n_args);
>
> @@ -1191,7 +1195,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> error (_("Invalid probe argument %d -- probe has %u arguments available"),
> sel, n_args);
>
> - return pc_probe_fns->sym_evaluate_probe_argument (pc_probe, sel);
> + return pc_probe_fns->sym_evaluate_probe_argument (pc_probe, sel, gdbarch);
> }
>
> /* This is called to compile one of the $_probe_arg* convenience
> @@ -1220,7 +1224,8 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
>
> pc_probe_fns = pc_probe->objfile->sf->sym_probe_fns;
>
> - n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe);
> + n_args = pc_probe_fns->sym_get_probe_argument_count (pc_probe,
> + expr->gdbarch);
>
> if (sel == -1)
> {
> @@ -1333,11 +1338,11 @@ static const struct internalvar_funcs probe_funcs =
>
> static void
> handle_stap_probe (struct objfile *objfile, struct sdt_note *el,
> - VEC (probe_p) **probesp, CORE_ADDR base)
> + VEC (probe_p) **probesp, CORE_ADDR base,
> + struct gdbarch *gdbarch)
> {
> bfd *abfd = objfile->obfd;
> int size = bfd_get_arch_size (abfd) / 8;
> - struct gdbarch *gdbarch = get_objfile_arch (objfile);
> struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> CORE_ADDR base_ref;
> const char *probe_args = NULL;
> @@ -1461,6 +1466,8 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> bfd_vma base;
> struct sdt_note *iter;
> unsigned save_probesp_len = VEC_length (probe_p, *probesp);
> + struct regcache *regcache = get_current_regcache ();
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
>
> if (objfile->separate_debug_objfile_backlink != NULL)
> {
> @@ -1486,7 +1493,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> {
> /* We first have to handle all the information about the
> probe which is present in the section. */
> - handle_stap_probe (objfile, iter, probesp, base);
> + handle_stap_probe (objfile, iter, probesp, base, gdbarch);
> }
>
> if (save_probesp_len == VEC_length (probe_p, *probesp))
> @@ -1535,13 +1542,12 @@ stap_gen_info_probes_table_values (struct probe *probe_generic,
> VEC (const_char_ptr) **ret)
> {
> struct stap_probe *probe = (struct stap_probe *) probe_generic;
> - struct gdbarch *gdbarch;
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> const char *val = NULL;
>
> gdb_assert (probe_generic->pops == &stap_probe_ops);
>
> - gdbarch = get_objfile_arch (probe->p.objfile);
> -
> if (probe->sem_addr)
> val = print_core_address (gdbarch, probe->sem_addr);
>
> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index b6e84d1..eb6f000 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -393,7 +393,8 @@ debug_sym_get_probes (struct objfile *objfile)
> }
>
> static unsigned
> -debug_sym_get_probe_argument_count (struct probe *probe)
> +debug_sym_get_probe_argument_count (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> struct objfile *objfile = probe->objfile;
> const struct debug_sym_fns_data *debug_data =
> @@ -401,7 +402,7 @@ debug_sym_get_probe_argument_count (struct probe *probe)
> unsigned retval;
>
> retval = debug_data->real_sf->sym_probe_fns->sym_get_probe_argument_count
> - (probe);
> + (probe, gdbarch);
>
> fprintf_filtered (gdb_stdlog,
> "probes->sym_get_probe_argument_count (%s) = %u\n",
> @@ -411,7 +412,8 @@ debug_sym_get_probe_argument_count (struct probe *probe)
> }
>
> static int
> -debug_can_evaluate_probe_arguments (struct probe *probe)
> +debug_can_evaluate_probe_arguments (struct probe *probe,
> + struct gdbarch *gdbarch)
> {
> struct objfile *objfile = probe->objfile;
> const struct debug_sym_fns_data *debug_data =
> @@ -419,7 +421,7 @@ debug_can_evaluate_probe_arguments (struct probe *probe)
> int retval;
>
> retval = debug_data->real_sf->sym_probe_fns->can_evaluate_probe_arguments
> - (probe);
> + (probe, gdbarch);
>
> fprintf_filtered (gdb_stdlog,
> "probes->can_evaluate_probe_arguments (%s) = %d\n",
> @@ -429,7 +431,8 @@ debug_can_evaluate_probe_arguments (struct probe *probe)
> }
>
> static struct value *
> -debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n)
> +debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n,
> + struct gdbarch *gdbarch)
> {
> struct objfile *objfile = probe->objfile;
> const struct debug_sym_fns_data *debug_data =
> @@ -441,7 +444,7 @@ debug_sym_evaluate_probe_argument (struct probe *probe, unsigned n)
> host_address_to_string (probe), n);
>
> retval = debug_data->real_sf->sym_probe_fns->sym_evaluate_probe_argument
> - (probe, n);
> + (probe, n, gdbarch);
>
> fprintf_filtered (gdb_stdlog,
> "probes->sym_evaluate_probe_argument (...) = %s\n",
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index 8e5909b..4c4a0dd 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -309,7 +309,8 @@ struct sym_probe_fns
> have come from a call to this objfile's sym_get_probes method.
> If you provide an implementation of sym_get_probes, you must
> implement this method as well. */
> - unsigned (*sym_get_probe_argument_count) (struct probe *probe);
> + unsigned (*sym_get_probe_argument_count) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Return 1 if the probe interface can evaluate the arguments of probe
> PROBE, zero otherwise. This function can be probe-specific, informing
> @@ -317,7 +318,8 @@ struct sym_probe_fns
> informing whether the probe interface is able to evaluate any kind of
> argument. If you provide an implementation of sym_get_probes, you must
> implement this method as well. */
> - int (*can_evaluate_probe_arguments) (struct probe *probe);
> + int (*can_evaluate_probe_arguments) (struct probe *probe,
> + struct gdbarch *gdbarch);
>
> /* Evaluate the Nth argument available to PROBE. PROBE will have
> come from a call to this objfile's sym_get_probes method. N will
> @@ -327,7 +329,8 @@ struct sym_probe_fns
> implementation of sym_get_probes, you must implement this method
> as well. */
> struct value *(*sym_evaluate_probe_argument) (struct probe *probe,
> - unsigned n);
> + unsigned n,
> + struct gdbarch *gdbarch);
>
> /* Compile the Nth probe argument to an agent expression. PROBE
> will have come from a call to this objfile's sym_get_probes
--
Sergio