This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH V3 3/9] New commands `enable probe' and `disable probe'.
- 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: Fri, 31 Oct 2014 15:04:47 -0400
- Subject: Re: [PATCH V3 3/9] New commands `enable probe' and `disable probe'.
- Authentication-results: sourceware.org; auth=none
- References: <1414504218-31204-1-git-send-email-jose dot marchesi at oracle dot com> <1414504218-31204-4-git-send-email-jose dot marchesi at oracle dot com>
On Tuesday, October 28 2014, Jose E. Marchesi wrote:
> This patch adds the above-mentioned commands to the generic probe
> abstraction implemented in probe.[ch]. The effects associated to
> enabling or disabling a probe depend on the type of probe being
> handled, and is triggered by invoking two back-end hooks in
> `probe_ops'.
>
> In case some particular probe type does not support the notion of
> enabling and/or disabling, the corresponding fields on `probe_ops' can
> be initialized to NULL. This is the case of SystemTap probes.
Thanks for the patch.
> gdb/ChangeLog:
>
> 2014-10-28 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * stap-probe.c (stap_probe_ops): Add NULLs in the static
> stap_probe_ops for `enable_probe' and `disable_probe'.
> * probe.c (enable_probes_command): New function.
> (disable_probes_command): Likewise.
> (_initialize_probe): Define the cli commands `enable probe' and
> `disable probe'.
> (parse_probe_linespec): New function.
> (info_probes_for_ops): Use parse_probe_linespec.
> * probe.h (probe_ops): New hooks `enable_probe' and
> `disable_probe'.
>
> gdb/doc/ChangeLog:
>
> 2014-10-28 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * gdb.texinfo (Static Probe Points): Cover the `enable probe' and
> `disable probe' commands.
> ---
> gdb/ChangeLog | 13 +++++
> gdb/doc/ChangeLog | 5 ++
> gdb/doc/gdb.texinfo | 29 ++++++++++
> gdb/probe.c | 161 ++++++++++++++++++++++++++++++++++++++++++++-------
> gdb/probe.h | 12 ++++
> gdb/stap-probe.c | 2 +
> 6 files changed, 202 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 15c2908..cfdc506 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4976,6 +4976,35 @@ given, all object files are considered.
> List the available static probes, from all types.
> @end table
>
> +@cindex enabling and disabling probes
> +Some probe points can be enabled and/or disabled. The effect of
> +enabling or disabling a probe depends on the type of probe being
> +handled. @code{SystemTap} probes cannot be disabled.
> +
> +You can enable (or disable) one or more probes using the following
> +commands, with optional arguments:
> +
> +@table @code
> +@kindex enable probes
> +@item enable probes @r{[}@var{provider} @r{[}@var{name} @r{[}@var{objfile}@r{]}@r{]}@r{]}
> +If given, @var{provider} is a regular expression used to match against
> +provider names when selecting which probes to enable. If omitted,
> +all probes from all providers are enabled.
> +
> +If given, @var{name} is a regular expression to match against probe
> +names when selecting which probes to enable. If omitted, probe names
> +are not considered when deciding whether to enable them.
> +
> +If given, @var{objfile} is a regular expression used to select which
> +object files (executable or shared libraries) to examine. If not
> +given, all object files are considered.
> +
> +@kindex disable probes
> +@item disable probes @r{[}@var{provider} @r{[}@var{name} @r{[}@var{objfile}@r{]}@r{]}@r{]}
> +See the @code{enable probes} command above for a description of the
> +optional arguments accepted by this command.
> +@end table
> +
> @vindex $_probe_arg@r{, convenience variable}
> A probe may specify up to twelve arguments. These are available at the
> point at which the probe is defined---that is, when the current PC is
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 102c2e1..b96c8ac 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -423,18 +423,18 @@ print_ui_out_not_applicables (const struct probe_ops *pops)
> VEC (info_probe_column_s) *headings = NULL;
> info_probe_column_s *column;
> int ix;
> -
> +
> if (pops->gen_info_probes_table_header == NULL)
> return;
>
> c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings);
> pops->gen_info_probes_table_header (&headings);
> -
> +
> for (ix = 0;
> VEC_iterate (info_probe_column_s, headings, ix, column);
> ++ix)
> ui_out_field_string (current_uiout, column->field_name, _("n/a"));
> -
> +
> do_cleanups (c);
> }
>
This whole hunk should not be in this patch, because it is just removing
extraneous whitespaces. Please fix them in patch #1 instead :-).
> @@ -527,6 +527,24 @@ exists_probe_with_pops (VEC (bound_probe_s) *probes,
> return 0;
> }
>
> +/* Helper function that parses a probe linespec of the form [PROVIDER
> + [PROBE [OBJNAME]]] from the provided string STR. */
> +
> +static void
> +parse_probe_linespec (const char *str, char **provider,
> + char **probe_name, char **objname)
> +{
> + *probe_name = *objname = NULL;
> +
> + *provider = extract_arg_const (&str);
> + if (*provider != NULL)
> + {
> + *probe_name = extract_arg_const (&str);
> + if (*probe_name != NULL)
> + *objname = extract_arg_const (&str);
> + }
> +}
> +
> /* See comment in probe.h. */
>
> void
> @@ -546,26 +564,14 @@ info_probes_for_ops (const char *arg, int from_tty,
> struct bound_probe *probe;
> struct gdbarch *gdbarch = get_current_arch ();
>
> - /* Do we have a `provider:probe:objfile' style of linespec? */
> - provider = extract_arg_const (&arg);
> - if (provider)
> - {
> - make_cleanup (xfree, provider);
> -
> - probe_name = extract_arg_const (&arg);
> - if (probe_name)
> - {
> - make_cleanup (xfree, probe_name);
> -
> - objname = extract_arg_const (&arg);
> - if (objname)
> - make_cleanup (xfree, objname);
> - }
> - }
> + parse_probe_linespec (arg, &provider, &probe_name, &objname);
> + make_cleanup (xfree, provider);
> + make_cleanup (xfree, probe_name);
> + make_cleanup (xfree, objname);
>
> probes = collect_probes (objname, provider, probe_name, pops);
> make_cleanup (VEC_cleanup (probe_p), &probes);
> -
> +
This is fixing extraneous whitespaces added in patch #1. Fix them there
:-).
> if (pops == NULL)
> {
> const struct probe_ops *po;
> @@ -689,6 +695,98 @@ info_probes_command (char *arg, int from_tty)
> info_probes_for_ops (arg, from_tty, NULL);
> }
>
> +/* Implementation of the `enable probes' command. */
> +
> +static void
> +enable_probes_command (char *arg, int from_tty)
> +{
> + char *provider, *probe_name = NULL, *objname = NULL;
> + struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
> + VEC (bound_probe_s) *probes;
> + struct bound_probe *probe;
> + int i;
> +
> + parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname);
> + make_cleanup (xfree, provider);
> + make_cleanup (xfree, probe_name);
> + make_cleanup (xfree, objname);
> +
> + probes = collect_probes (objname, provider, probe_name, NULL);
> + if (VEC_empty (bound_probe_s, probes))
> + {
> + ui_out_message (current_uiout, 0, _("No probes matched.\n"));
> + do_cleanups (cleanup);
> + return;
> + }
> +
> + /* Enable the selected probes, provided their backends support the
> + notion of enabling a probe. */
> + for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
> + {
> + const struct probe_ops *pops = probe->probe->pops;
> +
> + if (pops->enable_probe != NULL)
> + {
> + pops->enable_probe (probe->probe);
> + ui_out_message (current_uiout, 0,
> + _("Probe %s:%s enabled.\n"),
> + probe->probe->provider, probe->probe->name);
> + }
> + else
> + ui_out_message (current_uiout, 0,
> + _("Probe %s:%s cannot be enabled.\n"),
> + probe->probe->provider, probe->probe->name);
> + }
> +
> + do_cleanups (cleanup);
> +}
> +
> +/* Implementation of the `disable probes' command. */
> +
> +static void
> +disable_probes_command (char *arg, int from_tty)
> +{
> + char *provider, *probe_name = NULL, *objname = NULL;
> + struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
> + VEC (bound_probe_s) *probes;
> + struct bound_probe *probe;
> + int i;
> +
> + parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname);
> + make_cleanup (xfree, provider);
> + make_cleanup (xfree, probe_name);
> + make_cleanup (xfree, objname);
> +
> + probes = collect_probes (objname, provider, probe_name, NULL /* pops */);
> + if (VEC_empty (bound_probe_s, probes))
> + {
> + ui_out_message (current_uiout, 0, _("No probes matched.\n"));
> + do_cleanups (cleanup);
> + return;
> + }
> +
> + /* Disable the selected probes, provided their backends support the
> + notion of enabling a probe. */
> + for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
> + {
> + const struct probe_ops *pops = probe->probe->pops;
> +
> + if (pops->disable_probe != NULL)
> + {
> + pops->disable_probe (probe->probe);
> + ui_out_message (current_uiout, 0,
> + _("Probe %s:%s disabled.\n"),
> + probe->probe->provider, probe->probe->name);
> + }
> + else
> + ui_out_message (current_uiout, 0,
> + _("Probe %s:%s cannot be disabled.\n"),
> + probe->probe->provider, probe->probe->name);
> + }
> +
> + do_cleanups (cleanup);
> +}
> +
> /* See comments in probe.h. */
>
> CORE_ADDR
> @@ -950,4 +1048,27 @@ _initialize_probe (void)
> _("\
> Show information about all type of probes."),
> info_probes_cmdlist_get ());
> +
> + add_cmd ("probes", class_breakpoint, enable_probes_command, _("\
> +Enable probes.\n\
> +Usage: enable probes [PROVIDER [NAME [OBJECT]]]\n\
> +Each argument is a regular expression, used to select probes.\n\
> +PROVIDER matches probe provider names.\n\
> +NAME matches the probe names.\n\
> +OBJECT matches the executable or shared library name.\n\
> +If you do not specify any argument then the command will enable\n\
> +all defined probes."),
> + &enablelist);
> +
> + add_cmd ("probes", class_breakpoint, disable_probes_command, _("\
> +Disable probes.\n\
> +Usage: disable probes [PROVIDER [NAME [OBJECT]]]\n\
> +Each argument is a regular expression, used to select probes.\n\
> +PROVIDER matches probe provider names.\n\
> +NAME matches the probe names.\n\
> +OBJECT matches the executable or shared library name.\n\
> +If you do not specify any argument then the command will disable\n\
> +all defined probes."),
> + &disablelist);
> +
> }
> diff --git a/gdb/probe.h b/gdb/probe.h
> index 66c8c53..c244a21 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -138,6 +138,18 @@ struct probe_ops
>
> void (*gen_info_probes_table_values) (struct probe *probe,
> VEC (const_char_ptr) **values);
> +
> + /* Enable a probe. The semantics of "enabling" a probe depend on
> + the specific backend and the field can be NULL in case enabling
> + probes is not supported. */
> +
> + void (*enable_probe) (struct probe *probe);
> +
> + /* Disable a probe. The semantics of "disabling" a probe depend
> + on the specific backend and the field can be NULL in case
> + disabling probes is not supported. */
> +
> + void (*disable_probe) (struct probe *probe);
> };
>
> /* Definition of a vector of probe_ops. */
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index ed4ce69..8366ce2 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -1684,6 +1684,8 @@ static const struct probe_ops stap_probe_ops =
> stap_type_name,
> stap_gen_info_probes_table_header,
> stap_gen_info_probes_table_values,
> + NULL, /* enable_probe */
> + NULL /* disable_probe */
> };
>
> /* Implementation of the `info probes stap' command. */
> --
> 1.7.10.4
Otherwise, the patch looks good.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/