From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 407 invoked by alias); 5 Dec 2013 22:12:21 -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 393 invoked by uid 89); 5 Dec 2013 22:12:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Dec 2013 22:12:17 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rB5MC9HZ022585 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 5 Dec 2013 17:12:10 -0500 Received: from psique (ovpn-113-192.phx2.redhat.com [10.3.113.192]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rB5MC53D028015 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 5 Dec 2013 17:12:06 -0500 From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , Tom Tromey Subject: [PATCH] Sanitize gdbarch access on probe/SDT API (was: Re: [PATCH] Use "get_current_arch" instead of...) References: <1386225226-18549-1-git-send-email-sergiodj@redhat.com> <52A079DD.5050101@redhat.com> X-URL: http://www.redhat.com Date: Thu, 05 Dec 2013 22:12:00 -0000 In-Reply-To: <52A079DD.5050101@redhat.com> (Pedro Alves's message of "Thu, 05 Dec 2013 13:04:29 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2013-12/txt/msg00213.txt.bz2 On Thursday, December 05 2013, Pedro Alves wrote: > Adding get_current_arch calls outside initial command evaluation > raises eyebrows. > > /* Return "current" architecture. If the target is running, this is the > architecture of the selected frame. Otherwise, the "current" architecture > defaults to the target architecture. > > This function should normally be called solely by the command interpreter > routines to determine the architecture to execute a command in. */ > extern struct gdbarch *get_current_arch (void); Hm, thanks for the hint, I wasn't aware of such limitation/restriction. > struct value * > probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n) > { > ... > return evaluate_probe_argument (probe, n); > } > > FRAME here is the context the probe is being evaluated > in (get_current_frame(), the frame/thread that hit the event, > passed from infrun.) It'd be better to use the frame's arch > explicitly (that is, pass down the frame or arch to > evaluate_probe_argument). Right, could you please take a look at the following approach and tell me what you think? 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 * 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) : 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) : 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 @@ -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