This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: "Jose E. Marchesi" <jose dot marchesi at oracle dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 29 Sep 2014 20:02:44 -0400
- Subject: Re: [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c
- Authentication-results: sourceware.org; auth=none
- References: <1411724905-31234-1-git-send-email-jose dot marchesi at oracle dot com> <1411724905-31234-3-git-send-email-jose dot marchesi at oracle dot com>
On Friday, September 26 2014, Jose E. Marchesi wrote:
> This patch moves the `compute_probe_arg' and `compile_probe_arg' functions
> from stap-probe.c to probe.c. The rationale is that it is reasonable to
> assume that all backends will provide the `$_probe_argN' convenience
> variables, and that the user must be placed on the PC of the probe when
> requesting that information. The value and type of the argument can still be
> determined by the probe backend via the `pops->evaluate_probe_argument' and
> `pops->compile_to_ax' handlers.
>
> Note that a test in gdb.base/stap-probe.exp had to be adjusted because the "No
> SystemTap probe at PC" messages are now "No probe at PC".
Hi Jose,
Thanks for this patch.
Just a nit on the ChangeLog entry.
> gdb:
>
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * probe.c (compute_probe_arg): Function moved from the probe
> backends.
> (compile_probe_arg): Likewise.
> (probe_funcs): Likewise.
>
> * stap-probe.c (compute_probe_arg): Deleted.
> (compile_probe_arg): Likewise.
> (probe_funcs): Likewise.
First, ISTR that putting newlines between each file's entry is not used
anymore on GDB, so please write them all together.
I would also like that you mention things differently, to make it
explicit that we are just moving code here. I think something like:
* probe.c (compute_probe_arg): Moved from stap-probe.c.
...
* stap-probe.c (compute_probe_arg): Moved to probe.c.
...
Other than that, the patch is OK and can be applied.
Thanks,
> gdb/testsuite:
>
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * gdb.base/stap-probe.exp (stap_test): Remove "SystemTap" from
> expected message when trying to access $_probe_* convenience
> variables while not on a probe.
> ---
> gdb/ChangeLog | 11 ++++
> gdb/probe.c | 111 +++++++++++++++++++++++++++++++++
> gdb/stap-probe.c | 109 --------------------------------
> gdb/testsuite/ChangeLog | 6 ++
> gdb/testsuite/gdb.base/stap-probe.exp | 2 +-
> 5 files changed, 129 insertions(+), 110 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 7cc4f00..5ec0d85 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,14 @@
> +2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
> +
> + * probe.c (compute_probe_arg): Function moved from the probe
> + backends.
> + (compile_probe_arg): Likewise.
> + (probe_funcs): Likewise.
> +
> + * stap-probe.c (compute_probe_arg): Deleted.
> + (compile_probe_arg): Likewise.
> + (probe_funcs): Likewise.
> +
> 2014-09-25 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * probe.c (print_ui_out_not_applicables): New function.
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 5458372..d218f00 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -31,6 +31,9 @@
> #include "gdb_regex.h"
> #include "frame.h"
> #include "arch-utils.h"
> +#include "value.h"
> +#include "ax.h"
> +#include "ax-gdb.h"
> #include <ctype.h>
>
> typedef struct bound_probe bound_probe_s;
> @@ -822,6 +825,87 @@ will show information about all types of probes."),
> return &info_probes_cmdlist;
> }
>
> +
> +
> +/* This is called to compute the value of one of the $_probe_arg*
> + convenience variables. */
> +
> +static struct value *
> +compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> + void *data)
> +{
> + struct frame_info *frame = get_selected_frame (_("No frame selected"));
> + CORE_ADDR pc = get_frame_pc (frame);
> + int sel = (int) (uintptr_t) data;
> + struct bound_probe pc_probe;
> + const struct sym_probe_fns *pc_probe_fns;
> + unsigned n_args;
> +
> + /* SEL == -1 means "_probe_argc". */
> + gdb_assert (sel >= -1);
> +
> + pc_probe = find_probe_by_pc (pc);
> + if (pc_probe.probe == NULL)
> + error (_("No probe at PC %s"), core_addr_to_string (pc));
> +
> + n_args = get_probe_argument_count (pc_probe.probe, frame);
> + if (sel == -1)
> + return value_from_longest (builtin_type (arch)->builtin_int, n_args);
> +
> + if (sel >= n_args)
> + error (_("Invalid probe argument %d -- probe has %u arguments available"),
> + sel, n_args);
> +
> + return evaluate_probe_argument (pc_probe.probe, sel, frame);
> +}
> +
> +/* This is called to compile one of the $_probe_arg* convenience
> + variables into an agent expression. */
> +
> +static void
> +compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
> + struct axs_value *value, void *data)
> +{
> + CORE_ADDR pc = expr->scope;
> + int sel = (int) (uintptr_t) data;
> + struct bound_probe pc_probe;
> + const struct sym_probe_fns *pc_probe_fns;
> + int n_args;
> + struct frame_info *frame = get_selected_frame (NULL);
> +
> + /* SEL == -1 means "_probe_argc". */
> + gdb_assert (sel >= -1);
> +
> + pc_probe = find_probe_by_pc (pc);
> + if (pc_probe.probe == NULL)
> + error (_("No probe at PC %s"), core_addr_to_string (pc));
> +
> + n_args = get_probe_argument_count (pc_probe.probe, frame);
> +
> + if (sel == -1)
> + {
> + value->kind = axs_rvalue;
> + value->type = builtin_type (expr->gdbarch)->builtin_int;
> + ax_const_l (expr, n_args);
> + return;
> + }
> +
> + gdb_assert (sel >= 0);
> + if (sel >= n_args)
> + error (_("Invalid probe argument %d -- probe has %d arguments available"),
> + sel, n_args);
> +
> + pc_probe.probe->pops->compile_to_ax (pc_probe.probe, expr, value, sel);
> +}
> +
> +static const struct internalvar_funcs probe_funcs =
> +{
> + compute_probe_arg,
> + compile_probe_arg,
> + NULL
> +};
> +
> +
> VEC (probe_ops_cp) *all_probe_ops;
>
> void _initialize_probe (void);
> @@ -831,6 +915,33 @@ _initialize_probe (void)
> {
> VEC_safe_push (probe_ops_cp, all_probe_ops, &probe_ops_any);
>
> + create_internalvar_type_lazy ("_probe_argc", &probe_funcs,
> + (void *) (uintptr_t) -1);
> + create_internalvar_type_lazy ("_probe_arg0", &probe_funcs,
> + (void *) (uintptr_t) 0);
> + create_internalvar_type_lazy ("_probe_arg1", &probe_funcs,
> + (void *) (uintptr_t) 1);
> + create_internalvar_type_lazy ("_probe_arg2", &probe_funcs,
> + (void *) (uintptr_t) 2);
> + create_internalvar_type_lazy ("_probe_arg3", &probe_funcs,
> + (void *) (uintptr_t) 3);
> + create_internalvar_type_lazy ("_probe_arg4", &probe_funcs,
> + (void *) (uintptr_t) 4);
> + create_internalvar_type_lazy ("_probe_arg5", &probe_funcs,
> + (void *) (uintptr_t) 5);
> + create_internalvar_type_lazy ("_probe_arg6", &probe_funcs,
> + (void *) (uintptr_t) 6);
> + create_internalvar_type_lazy ("_probe_arg7", &probe_funcs,
> + (void *) (uintptr_t) 7);
> + create_internalvar_type_lazy ("_probe_arg8", &probe_funcs,
> + (void *) (uintptr_t) 8);
> + create_internalvar_type_lazy ("_probe_arg9", &probe_funcs,
> + (void *) (uintptr_t) 9);
> + create_internalvar_type_lazy ("_probe_arg10", &probe_funcs,
> + (void *) (uintptr_t) 10);
> + create_internalvar_type_lazy ("_probe_arg11", &probe_funcs,
> + (void *) (uintptr_t) 11);
> +
> add_cmd ("all", class_info, info_probes_command,
> _("\
> Show information about all type of probes."),
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index 23202d7..17a73fc 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -1366,79 +1366,6 @@ stap_probe_destroy (struct probe *probe_generic)
>
>
>
> -/* This is called to compute the value of one of the $_probe_arg*
> - convenience variables. */
> -
> -static struct value *
> -compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> - void *data)
> -{
> - struct frame_info *frame = get_selected_frame (_("No frame selected"));
> - CORE_ADDR pc = get_frame_pc (frame);
> - int sel = (int) (uintptr_t) data;
> - struct bound_probe pc_probe;
> - const struct sym_probe_fns *pc_probe_fns;
> - unsigned n_args;
> -
> - /* SEL == -1 means "_probe_argc". */
> - gdb_assert (sel >= -1);
> -
> - pc_probe = find_probe_by_pc (pc);
> - if (pc_probe.probe == NULL)
> - error (_("No SystemTap probe at PC %s"), core_addr_to_string (pc));
> -
> - n_args = get_probe_argument_count (pc_probe.probe, frame);
> - if (sel == -1)
> - return value_from_longest (builtin_type (arch)->builtin_int, n_args);
> -
> - if (sel >= n_args)
> - error (_("Invalid probe argument %d -- probe has %u arguments available"),
> - sel, n_args);
> -
> - return evaluate_probe_argument (pc_probe.probe, sel, frame);
> -}
> -
> -/* This is called to compile one of the $_probe_arg* convenience
> - variables into an agent expression. */
> -
> -static void
> -compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
> - struct axs_value *value, void *data)
> -{
> - CORE_ADDR pc = expr->scope;
> - int sel = (int) (uintptr_t) data;
> - struct bound_probe pc_probe;
> - const struct sym_probe_fns *pc_probe_fns;
> - int n_args;
> - struct frame_info *frame = get_selected_frame (NULL);
> -
> - /* SEL == -1 means "_probe_argc". */
> - gdb_assert (sel >= -1);
> -
> - pc_probe = find_probe_by_pc (pc);
> - if (pc_probe.probe == NULL)
> - error (_("No SystemTap probe at PC %s"), core_addr_to_string (pc));
> -
> - n_args = get_probe_argument_count (pc_probe.probe, frame);
> -
> - if (sel == -1)
> - {
> - value->kind = axs_rvalue;
> - value->type = builtin_type (expr->gdbarch)->builtin_int;
> - ax_const_l (expr, n_args);
> - return;
> - }
> -
> - gdb_assert (sel >= 0);
> - if (sel >= n_args)
> - error (_("Invalid probe argument %d -- probe has %d arguments available"),
> - sel, n_args);
> -
> - pc_probe.probe->pops->compile_to_ax (pc_probe.probe, expr, value, sel);
> -}
> -
> -
> -
> /* Set or clear a SystemTap semaphore. ADDRESS is the semaphore's
> address. SET is zero if the semaphore should be cleared, or one
> if it should be set. This is a helper function for `stap_semaphore_down'
> @@ -1515,15 +1442,6 @@ stap_clear_semaphore (struct probe *probe_generic, struct objfile *objfile,
> stap_modify_semaphore (addr, 0, gdbarch);
> }
>
> -/* Implementation of `$_probe_arg*' set of variables. */
> -
> -static const struct internalvar_funcs probe_funcs =
> -{
> - compute_probe_arg,
> - compile_probe_arg,
> - NULL
> -};
> -
> /* Helper function that parses the information contained in a
> SystemTap's probe. Basically, the information consists in:
>
> @@ -1784,33 +1702,6 @@ _initialize_stap_probe (void)
> show_stapexpressiondebug,
> &setdebuglist, &showdebuglist);
>
> - create_internalvar_type_lazy ("_probe_argc", &probe_funcs,
> - (void *) (uintptr_t) -1);
> - create_internalvar_type_lazy ("_probe_arg0", &probe_funcs,
> - (void *) (uintptr_t) 0);
> - create_internalvar_type_lazy ("_probe_arg1", &probe_funcs,
> - (void *) (uintptr_t) 1);
> - create_internalvar_type_lazy ("_probe_arg2", &probe_funcs,
> - (void *) (uintptr_t) 2);
> - create_internalvar_type_lazy ("_probe_arg3", &probe_funcs,
> - (void *) (uintptr_t) 3);
> - create_internalvar_type_lazy ("_probe_arg4", &probe_funcs,
> - (void *) (uintptr_t) 4);
> - create_internalvar_type_lazy ("_probe_arg5", &probe_funcs,
> - (void *) (uintptr_t) 5);
> - create_internalvar_type_lazy ("_probe_arg6", &probe_funcs,
> - (void *) (uintptr_t) 6);
> - create_internalvar_type_lazy ("_probe_arg7", &probe_funcs,
> - (void *) (uintptr_t) 7);
> - create_internalvar_type_lazy ("_probe_arg8", &probe_funcs,
> - (void *) (uintptr_t) 8);
> - create_internalvar_type_lazy ("_probe_arg9", &probe_funcs,
> - (void *) (uintptr_t) 9);
> - create_internalvar_type_lazy ("_probe_arg10", &probe_funcs,
> - (void *) (uintptr_t) 10);
> - create_internalvar_type_lazy ("_probe_arg11", &probe_funcs,
> - (void *) (uintptr_t) 11);
> -
> add_cmd ("stap", class_info, info_probes_stap_command,
> _("\
> Show information about SystemTap static probes.\n\
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 539a983..c93d7cf 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
> +
> + * gdb.base/stap-probe.exp (stap_test): Remove "SystemTap" from
> + expected message when trying to access $_probe_* convenience
> + variables while not on a probe.
> +
> 2014-09-22 Pedro Alves <palves@redhat.com>
>
> * gdb.threads/break-while-running.exp: New file.
> diff --git a/gdb/testsuite/gdb.base/stap-probe.exp b/gdb/testsuite/gdb.base/stap-probe.exp
> index 7710bc3..9f71d1d 100644
> --- a/gdb/testsuite/gdb.base/stap-probe.exp
> +++ b/gdb/testsuite/gdb.base/stap-probe.exp
> @@ -30,7 +30,7 @@ proc stap_test {exec_name {arg ""}} {
> return -1
> }
>
> - gdb_test "print \$_probe_argc" "No SystemTap probe at PC $hex" \
> + gdb_test "print \$_probe_argc" "No probe at PC $hex" \
> "check argument not at probe point"
>
> gdb_test "info probes stap" \
> --
> 1.7.10.4
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/