* [PATCH 8/9] Documentation for DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-09-26 13:18 ` Eli Zaretskii
2014-09-26 9:43 ` [PATCH 5/9] New probe type: " Jose E. Marchesi
` (8 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
This patch modifies the `Static Probe Points' section on the GDB
manual in order to cover the support for DTrace USDT probes, in
addition to SystemTap SDT probes.
gdb/doc:
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
* gdb.texinfo (Static Probe Points): Add cindex `static probe
point, DTrace'.
(Static Probe Points): Modified to cover DTrace probes in addition
to SystemTap probes.
---
gdb/doc/ChangeLog | 8 +++++++
gdb/doc/gdb.texinfo | 58 +++++++++++++++++++++++++++++++++------------------
2 files changed, 46 insertions(+), 20 deletions(-)
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 5667bf1..6573d3e 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,13 @@
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+ * gdb.texinfo (Static Probe Points): Add cindex `static probe
+ point, DTrace'.
+ (Static Probe Points): Modified to cover DTrace probes in addition
+ to SystemTap probes. Also modified to cover the `enable probe'
+ and `disable probe' commands.
+
+2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+
* gdb.texinfo (Static Probe Points): Cover the `enable probe' and
`disable probe' commands.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4d6dfbc..ad1afdb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4932,34 +4932,49 @@ that can no longer be recreated.
@subsection Static Probe Points
@cindex static probe point, SystemTap
+@cindex static probe point, DTrace
@value{GDBN} supports @dfn{SDT} probes in the code. @acronym{SDT} stands
for Statically Defined Tracing, and the probes are designed to have a tiny
-runtime code and data footprint, and no dynamic relocations. They are
-usable from assembly, C and C@t{++} languages. See
-@uref{http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation}
-for a good reference on how the @acronym{SDT} probes are implemented.
+runtime code and data footprint, and no dynamic relocations.
+
+Currently, the following types of probes are supported on
+ELF-compatible systems:
-Currently, @code{SystemTap} (@uref{http://sourceware.org/systemtap/})
-@acronym{SDT} probes are supported on ELF-compatible systems. See
+@itemize @bullet
+@item @code{SystemTap} (@uref{http://sourceware.org/systemtap/})
+@acronym{SDT} probes@footnote{See
@uref{http://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps}
-for more information on how to add @code{SystemTap} @acronym{SDT} probes
-in your applications.
+for more information on how to add @code{SystemTap} @acronym{SDT}
+probes in your applications.}. @code{SystemTap} probes are usable
+from assembly, C and C@t{++} languages@footnote{See
+@uref{http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation}
+for a good reference on how the @acronym{SDT} probes are implemented.}.
+@item @code{DTrace} (@uref{http://oss.oracle.com/projects/DTrace})
+@acronym{USDT} probes. @code{DTrace} probes are usable from C and
+C@t{++} languages.
+@end itemize
@cindex semaphores on static probe points
-Some probes have an associated semaphore variable; for instance, this
-happens automatically if you defined your probe using a DTrace-style
-@file{.d} file. If your probe has a semaphore, @value{GDBN} will
-automatically enable it when you specify a breakpoint using the
-@samp{-probe-stap} notation. But, if you put a breakpoint at a probe's
-location by some other method (e.g., @code{break file:line}), then
-@value{GDBN} will not automatically set the semaphore.
+Some @code{SystemTap} probes have an associated semaphore variable;
+for instance, this happens automatically if you defined your probe
+using a DTrace-style @file{.d} file. If your probe has a semaphore,
+@value{GDBN} will automatically enable it when you specify a
+breakpoint using the @samp{-probe-stap} notation. But, if you put a
+breakpoint at a probe's location by some other method (e.g.,
+@code{break file:line}), then @value{GDBN} will not automatically set
+the semaphore. @code{DTrace} probes do not support the notion of
+semaphores.
You can examine the available static static probes using @code{info
probes}, with optional arguments:
@table @code
@kindex info probes
-@item info probes stap @r{[}@var{provider} @r{[}@var{name} @r{[}@var{objfile}@r{]}@r{]}@r{]}
+@item info probes @r{[}@var{type}@r{]} @r{[}@var{provider} @r{[}@var{name} @r{[}@var{objfile}@r{]}@r{]}@r{]}
+If given, @var{type} is either @code{stap} for listing
+@code{SystemTap} probes or @code{dtrace} for listing @code{DTrace}
+probes. If omitted all probes are listed regardless of their types.
+
If given, @var{provider} is a regular expression used to match against provider
names when selecting which probes to list. If omitted, probes by all
probes from all providers are listed.
@@ -4979,8 +4994,8 @@ List the available static probes, from all types.
@cindex enabling and disabling probes
Some probe points can be enabled and/or disabled. The effects
associated to enabling or disabling a probe depend on the type of
-probe being handled. @code{SystemTap} probes do not support these
-notions.
+probe being handled. Some @code{DTrace} probes can be enabled or
+disabled, but @code{SystemTap} probes do not support these notions.
You can enable (or disable) one or more probes using the following
commands, with optional arguments:
@@ -5011,8 +5026,11 @@ 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
at the probe's location. The arguments are available using the
convenience variables (@pxref{Convenience Vars})
-@code{$_probe_arg0}@dots{}@code{$_probe_arg11}. Each probe argument is
-an integer of the appropriate size; types are not preserved. The
+@code{$_probe_arg0}@dots{}@code{$_probe_arg11}. In @code{SystemTap}
+probes each probe argument is an integer of the appropriate size;
+types are not preserved. In @code{DTrace} probes types are preserved
+provided that they are recognized as such by @value{GDBN}; otherwise
+the value of the probe argument will be a long integer. The
convenience variable @code{$_probe_argc} holds the number of arguments
at the current probe point.
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 8/9] Documentation for DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 8/9] Documentation for DTrace USDT probes Jose E. Marchesi
@ 2014-09-26 13:18 ` Eli Zaretskii
2014-09-29 10:26 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2014-09-26 13:18 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
> From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
> Date: Fri, 26 Sep 2014 11:48:24 +0200
>
> gdb/doc:
>
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * gdb.texinfo (Static Probe Points): Add cindex `static probe
> point, DTrace'.
> (Static Probe Points): Modified to cover DTrace probes in addition
> to SystemTap probes.
No need to mention the name of the same node twice.
> +@itemize @bullet
> +@item @code{SystemTap} (@uref{http://sourceware.org/systemtap/})
> +@acronym{SDT} probes@footnote{See
> @uref{http://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps}
> -for more information on how to add @code{SystemTap} @acronym{SDT} probes
> -in your applications.
> +for more information on how to add @code{SystemTap} @acronym{SDT}
> +probes in your applications.}. @code{SystemTap} probes are usable
> +from assembly, C and C@t{++} languages@footnote{See
> +@uref{http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation}
> +for a good reference on how the @acronym{SDT} probes are implemented.}.
> +@item @code{DTrace} (@uref{http://oss.oracle.com/projects/DTrace})
> +@acronym{USDT} probes. @code{DTrace} probes are usable from C and
> +C@t{++} languages.
> +@end itemize
Please leave an empty line before each @item.
> +Some @code{SystemTap} probes have an associated semaphore variable;
> +for instance, this happens automatically if you defined your probe
> +using a DTrace-style @file{.d} file. If your probe has a semaphore,
> +@value{GDBN} will automatically enable it when you specify a
> +breakpoint using the @samp{-probe-stap} notation. But, if you put a
> +breakpoint at a probe's location by some other method (e.g.,
> +@code{break file:line}), then @value{GDBN} will not automatically set
> +the semaphore. @code{DTrace} probes do not support the notion of
> +semaphores.
The last sentence confused me: you first explain something that seems
to imply semaphores are part of DTrace probes, but then say that they
don't support semaphores. What am I missing?
> +probe being handled. Some @code{DTrace} probes can be enabled or
> +disabled, but @code{SystemTap} probes do not support these notions.
Which "notions"? If you want to say they cannot be disabled, please
say so explicitly.
Thanks.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 8/9] Documentation for DTrace USDT probes.
2014-09-26 13:18 ` Eli Zaretskii
@ 2014-09-29 10:26 ` Jose E. Marchesi
2014-09-29 13:35 ` Eli Zaretskii
0 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-29 10:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
> +Some @code{SystemTap} probes have an associated semaphore variable;
> +for instance, this happens automatically if you defined your probe
> +using a DTrace-style @file{.d} file. If your probe has a semaphore,
> +@value{GDBN} will automatically enable it when you specify a
> +breakpoint using the @samp{-probe-stap} notation. But, if you put a
> +breakpoint at a probe's location by some other method (e.g.,
> +@code{break file:line}), then @value{GDBN} will not automatically set
> +the semaphore. @code{DTrace} probes do not support the notion of
> +semaphores.
The last sentence confused me: you first explain something that seems
to imply semaphores are part of DTrace probes, but then say that they
don't support semaphores. What am I missing?
The paragraph starts explaining that SystemTap probes may have
associated semaphore variables, followed by a short description on how
gdb handles these variables. That was part of the original
documentation on SystemTap probes.
I just added the last sentence to note that DTrace probes do not support
semaphore variables at all.
I don't see where the confusion is?
> +probe being handled. Some @code{DTrace} probes can be enabled or
> +disabled, but @code{SystemTap} probes do not support these notions.
Which "notions"? If you want to say they cannot be disabled, please
say so explicitly.
No, I want to say that SystemTap probes do not even support the notion
of being enabled or disabled. That is not quite the same than saying
that SystemTap probes cannot be disabled: for example some DTrace probes
cannot be disabled because they are always enabled.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 8/9] Documentation for DTrace USDT probes.
2014-09-29 10:26 ` Jose E. Marchesi
@ 2014-09-29 13:35 ` Eli Zaretskii
2014-09-29 13:53 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2014-09-29 13:35 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
> From: jose.marchesi@oracle.com (Jose E. Marchesi)
> Cc: gdb-patches@sourceware.org
> Date: Mon, 29 Sep 2014 12:31:26 +0200
>
>
> > +Some @code{SystemTap} probes have an associated semaphore variable;
> > +for instance, this happens automatically if you defined your probe
> > +using a DTrace-style @file{.d} file. If your probe has a semaphore,
> > +@value{GDBN} will automatically enable it when you specify a
> > +breakpoint using the @samp{-probe-stap} notation. But, if you put a
> > +breakpoint at a probe's location by some other method (e.g.,
> > +@code{break file:line}), then @value{GDBN} will not automatically set
> > +the semaphore. @code{DTrace} probes do not support the notion of
> > +semaphores.
>
> The last sentence confused me: you first explain something that seems
> to imply semaphores are part of DTrace probes, but then say that they
> don't support semaphores. What am I missing?
>
> The paragraph starts explaining that SystemTap probes may have
> associated semaphore variables, followed by a short description on how
> gdb handles these variables. That was part of the original
> documentation on SystemTap probes.
>
> I just added the last sentence to note that DTrace probes do not support
> semaphore variables at all.
>
> I don't see where the confusion is?
The confusion starts with the fact that "DTrace" is mentioned twice,
the first time in the context of discussing semaphores.
How about the following rewrite:
Some @code{SystemTap} probes have an associated semaphore variable.
If your probe has a semaphore, @value{GDBN} will automatically
enable it when you specify a breakpoint using the @samp{-probe-stap}
notation. But, if you put a breakpoint at a probe's location by
some other method (e.g., @code{break file:line}), then @value{GDBN}
will not automatically set the semaphore. @code{DTrace} probes do
not support semaphore variables associated with them.
> > +probe being handled. Some @code{DTrace} probes can be enabled or
> > +disabled, but @code{SystemTap} probes do not support these notions.
>
> Which "notions"? If you want to say they cannot be disabled, please
> say so explicitly.
>
> No, I want to say that SystemTap probes do not even support the notion
> of being enabled or disabled. That is not quite the same than saying
> that SystemTap probes cannot be disabled: for example some DTrace probes
> cannot be disabled because they are always enabled.
Sorry, I don't see the difference between "cannot be disabled" and
"don't support the notion of being enabled or disabled". A trap that
cannot be disabled is by definition always enabled, right? So from
the user point of view, saying they cannot be disabled says it all,
right?
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 8/9] Documentation for DTrace USDT probes.
2014-09-29 13:35 ` Eli Zaretskii
@ 2014-09-29 13:53 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-29 13:53 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
> > +Some @code{SystemTap} probes have an associated semaphore variable;
> > +for instance, this happens automatically if you defined your probe
> > +using a DTrace-style @file{.d} file. If your probe has a semaphore,
> > +@value{GDBN} will automatically enable it when you specify a
> > +breakpoint using the @samp{-probe-stap} notation. But, if you put a
> > +breakpoint at a probe's location by some other method (e.g.,
> > +@code{break file:line}), then @value{GDBN} will not automatically set
> > +the semaphore. @code{DTrace} probes do not support the notion of
> > +semaphores.
>
> The last sentence confused me: you first explain something that seems
> to imply semaphores are part of DTrace probes, but then say that they
> don't support semaphores. What am I missing?
>
> The paragraph starts explaining that SystemTap probes may have
> associated semaphore variables, followed by a short description on how
> gdb handles these variables. That was part of the original
> documentation on SystemTap probes.
>
> I just added the last sentence to note that DTrace probes do not support
> semaphore variables at all.
>
> I don't see where the confusion is?
The confusion starts with the fact that "DTrace" is mentioned twice,
the first time in the context of discussing semaphores.
Aaah, that mention to the "DTrace-like" file. Yes, I agree, it can be
confusing now that DTrace probes are also supported.
How about the following rewrite:
Some @code{SystemTap} probes have an associated semaphore variable.
If your probe has a semaphore, @value{GDBN} will automatically
enable it when you specify a breakpoint using the @samp{-probe-stap}
notation. But, if you put a breakpoint at a probe's location by
some other method (e.g., @code{break file:line}), then @value{GDBN}
will not automatically set the semaphore. @code{DTrace} probes do
not support semaphore variables associated with them.
I think that is indeed better than the current wording. Thanks.
> > +probe being handled. Some @code{DTrace} probes can be enabled or
> > +disabled, but @code{SystemTap} probes do not support these notions.
>
> Which "notions"? If you want to say they cannot be disabled, please
> say so explicitly.
>
> No, I want to say that SystemTap probes do not even support the notion
> of being enabled or disabled. That is not quite the same than saying
> that SystemTap probes cannot be disabled: for example some DTrace probes
> cannot be disabled because they are always enabled.
Sorry, I don't see the difference between "cannot be disabled" and
"don't support the notion of being enabled or disabled". A trap that
cannot be disabled is by definition always enabled, right? So from
the user point of view, saying they cannot be disabled says it all,
right?
Well... right, from the user perspective SystemTap probes are always
enabled... Ok, I will follow your suggestion and just mention that
SystemTap probes cannot be disabled.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/9] New probe type: DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
2014-09-26 9:43 ` [PATCH 8/9] Documentation for DTrace USDT probes Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-09-26 13:19 ` Eli Zaretskii
2014-10-02 23:19 ` Sergio Durigan Junior
2014-09-26 9:43 ` [PATCH 3/9] New commands `enable probe' and `disable probe' Jose E. Marchesi
` (7 subsequent siblings)
9 siblings, 2 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
This patch adds a new type of probe to GDB: the DTrace USDT probes. The new
type is added by providing functions implementing all the entries of the
`probe_ops' structure defined in `probe.h'. The implementation is
self-contained and does not depend on DTrace source code in any way.
gdb:
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
* breakpoint.c (BREAK_ARGS_HELP): help string updated to mention
the -probe-dtrace new vpossible value for PROBE_MODIFIER.
* configure.ac (CONFIG_OBS): dtrace-probe.o added if BFD can
handle ELF files.
* Makefile.in (SFILES): dtrace-probe.c added.
* configure: Regenerate.
* dtrace-probe.c: New file.
(SHT_SUNW_dof): New constant.
(dtrace_probe_type): New enum.
(dtrace_probe_arg): New struct.
(dtrace_probe_arg_s): New typedef.
(struct dtrace_probe_enabler): New struct.
(dtrace_probe_enabler_s): New typedef.
(dtrace_probe): New struct.
(dtrace_probe_is_linespec): New function.
(dtrace_dof_sect_type): New enum.
(DTRACE_DOF_ID_MAG0-3): New constants.
(DTRACE_DOF_ID_ENCODING): Likewise.
(DTRACE_DOF_ENCODE_LSB): Likewise.
(DTRACE_DOF_ENCODE_MSB): Likewise.
(dtrace_dof_hdr): New struct.
(dtrace_dof_sect): Likewise.
(dtrace_dof_provider): Likewise.
(dtrace_dof_probe): Likewise.
(DOF_UINT): New macro.
(DTRACE_DOF_PTR): Likewise.
(DTRACE_DOF_SECT): Likewise.
(dtrace_process_dof_probe): New function.
(dtrace_process_dof): Likewise.
(dtrace_build_arg_exprs): Likewise.
(dtrace_get_arg): Likewise.
(dtrace_get_probes): Likewise.
(dtrace_get_probe_argument_count): Likewise.
(dtrace_can_evaluate_probe_arguments): Likewise.
(dtrace_evaluate_probe_argument): Likewise.
(dtrace_compile_to_ax): Likewise.
(dtrace_set_semaphore): Likewise.
(dtrace_clear_semaphore): Likewise.
(dtrace_probe_destroy): Likewise.
(dtrace_gen_info_probes_table_header): Likewise.
(dtrace_gen_info_probes_table_values): Likewise.
(dtrace_probe_is_enabled): Likewise.
(dtrace_probe_ops): New variable.
(info_probes_dtrace_command): New function.
(_initialize_dtrace_probe): Likewise.
---
gdb/ChangeLog | 50 ++++
gdb/Makefile.in | 3 +-
gdb/breakpoint.c | 3 +-
gdb/configure | 2 +-
gdb/configure.ac | 2 +-
gdb/dtrace-probe.c | 816 ++++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 872 insertions(+), 4 deletions(-)
create mode 100644 gdb/dtrace-probe.c
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7041cfb..eac03e7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,55 @@
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+ * breakpoint.c (BREAK_ARGS_HELP): help string updated to mention
+ the -probe-dtrace new vpossible value for PROBE_MODIFIER.
+
+ * configure.ac (CONFIG_OBS): dtrace-probe.o added if BFD can
+ handle ELF files.
+ * Makefile.in (SFILES): dtrace-probe.c added.
+ * configure: Regenerate.
+
+ * dtrace-probe.c: New file.
+ (SHT_SUNW_dof): New constant.
+ (dtrace_probe_type): New enum.
+ (dtrace_probe_arg): New struct.
+ (dtrace_probe_arg_s): New typedef.
+ (struct dtrace_probe_enabler): New struct.
+ (dtrace_probe_enabler_s): New typedef.
+ (dtrace_probe): New struct.
+ (dtrace_probe_is_linespec): New function.
+ (dtrace_dof_sect_type): New enum.
+ (DTRACE_DOF_ID_MAG0-3): New constants.
+ (DTRACE_DOF_ID_ENCODING): Likewise.
+ (DTRACE_DOF_ENCODE_LSB): Likewise.
+ (DTRACE_DOF_ENCODE_MSB): Likewise.
+ (dtrace_dof_hdr): New struct.
+ (dtrace_dof_sect): Likewise.
+ (dtrace_dof_provider): Likewise.
+ (dtrace_dof_probe): Likewise.
+ (DOF_UINT): New macro.
+ (DTRACE_DOF_PTR): Likewise.
+ (DTRACE_DOF_SECT): Likewise.
+ (dtrace_process_dof_probe): New function.
+ (dtrace_process_dof): Likewise.
+ (dtrace_build_arg_exprs): Likewise.
+ (dtrace_get_arg): Likewise.
+ (dtrace_get_probes): Likewise.
+ (dtrace_get_probe_argument_count): Likewise.
+ (dtrace_can_evaluate_probe_arguments): Likewise.
+ (dtrace_evaluate_probe_argument): Likewise.
+ (dtrace_compile_to_ax): Likewise.
+ (dtrace_set_semaphore): Likewise.
+ (dtrace_clear_semaphore): Likewise.
+ (dtrace_probe_destroy): Likewise.
+ (dtrace_gen_info_probes_table_header): Likewise.
+ (dtrace_gen_info_probes_table_values): Likewise.
+ (dtrace_probe_is_enabled): Likewise.
+ (dtrace_probe_ops): New variable.
+ (info_probes_dtrace_command): New function.
+ (_initialize_dtrace_probe): Likewise.
+
+2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+
* gdbarch.sh (dtrace_probe_argument): New.
(dtrace_probe_is_enabled): Likewise.
(dtrace_enable_probe): Likewise.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index cbec0d2..ad82215 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -799,7 +799,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
cp-abi.c cp-support.c cp-namespace.c cp-valprint.c \
d-exp.y d-lang.c d-support.c d-valprint.c \
cp-name-parser.y \
- dbxread.c demangle.c dictionary.c disasm.c doublest.c dummy-frame.c \
+ dbxread.c demangle.c dictionary.c disasm.c doublest.c \
+ dtrace-probe.c dummy-frame.c \
dwarf2expr.c dwarf2loc.c dwarf2read.c dwarf2-frame.c \
dwarf2-frame-tailcall.c \
elfread.c environ.c eval.c event-loop.c event-top.c \
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bec7f68..95323bf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -16197,7 +16197,8 @@ all_tracepoints (void)
command" [PROBE_MODIFIER] [LOCATION] [thread THREADNUM] [if CONDITION]\n\
PROBE_MODIFIER shall be present if the command is to be placed in a\n\
probe point. Accepted values are `-probe' (for a generic, automatically\n\
-guessed probe type) or `-probe-stap' (for a SystemTap probe).\n\
+guessed probe type), `-probe-stap' (for a SystemTap probe) or \n\
+`-probe-dtrace' (for a DTrace probe).\n\
LOCATION may be a line number, function name, or \"*\" and an address.\n\
If a line number is specified, break at start of code for that line.\n\
If a function is specified, break at start of code for that function.\n\
diff --git a/gdb/configure b/gdb/configure
index 6e7435f..ecd0cff 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -13281,7 +13281,7 @@ $as_echo "$gdb_cv_var_elf" >&6; }
LDFLAGS=$OLD_LDFLAGS
LIBS=$OLD_LIBS
if test $gdb_cv_var_elf = yes; then
- CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o"
+ CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o"
$as_echo "#define HAVE_ELF 1" >>confdefs.h
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 4f5fb7b..7b1133a 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2103,7 +2103,7 @@ AC_SUBST(WIN32LIBS)
GDB_AC_CHECK_BFD([for ELF support in BFD], gdb_cv_var_elf,
[bfd_get_elf_phdr_upper_bound (NULL)], elf-bfd.h)
if test $gdb_cv_var_elf = yes; then
- CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o"
+ CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o"
AC_DEFINE(HAVE_ELF, 1,
[Define if ELF support should be included.])
# -ldl is provided by bfd/Makfile.am (LIBDL) <PLUGINS>.
diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
new file mode 100644
index 0000000..f529ca5
--- /dev/null
+++ b/gdb/dtrace-probe.c
@@ -0,0 +1,816 @@
+/* DTrace probe support for GDB.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ Contributed by Oracle, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "probe.h"
+#include "vec.h"
+#include "elf-bfd.h"
+#include "gdbtypes.h"
+#include "obstack.h"
+#include "objfiles.h"
+#include "complaints.h"
+#include "value.h"
+#include "ax.h"
+#include "ax-gdb.h"
+#include "language.h"
+#include "parser-defs.h"
+#include "inferior.h"
+
+/* The type of the ELF sections where we will find the DOF programs
+ with information about probes. */
+
+#ifndef SHT_SUNW_dof
+# define SHT_SUNW_dof 0x6ffffff4
+#endif
+
+/* Forward declaration. */
+
+static const struct probe_ops dtrace_probe_ops;
+
+/* The following structure represents a single argument for the
+ probe. */
+
+struct dtrace_probe_arg
+{
+ /* The type of the probe argument. */
+ struct type *type;
+
+ /* A string describing the type. */
+ char *type_str;
+
+ /* The argument converted to an internal GDB expression. */
+ struct expression *expr;
+};
+
+typedef struct dtrace_probe_arg dtrace_probe_arg_s;
+DEF_VEC_O (dtrace_probe_arg_s);
+
+/* The following structure represents an enabler for a probe. */
+
+struct dtrace_probe_enabler
+{
+ /* Program counter where the is-enabled probe is installed. The
+ contents (nops, whatever...) stored at this address are
+ architecture dependent. */
+ CORE_ADDR address;
+};
+
+typedef struct dtrace_probe_enabler dtrace_probe_enabler_s;
+DEF_VEC_O (dtrace_probe_enabler_s);
+
+/* The following structure represents a dtrace probe. */
+
+struct dtrace_probe
+{
+ /* Generic information about the probe. This must be the first
+ element of this struct, in order to maintain binary compatibility
+ with the `struct probe' and be able to fully abstract it. */
+ struct probe p;
+
+ /* A probe can have zero or more arguments. */
+ int probe_argc;
+ VEC (dtrace_probe_arg_s) *args;
+
+ /* A probe can have zero or more "enablers" associated with it. */
+ VEC (dtrace_probe_enabler_s) *enablers;
+
+ /* Whether the expressions for the arguments have been built. */
+ int args_expr_built : 1;
+};
+
+/* Implementation of the probe_is_linespec method. */
+
+static int
+dtrace_probe_is_linespec (const char **linespecp)
+{
+ static const char *const keywords[] = { "-pdtrace", "-probe-dtrace", NULL };
+
+ return probe_is_linespec_by_keyword (linespecp, keywords);
+}
+
+/* DOF programs can contain an arbitrary number of sections of 26
+ different types. In order to support DTrace USDT probes we only
+ need to handle a subset of these section types, fortunately. These
+ section types are defined in the following enumeration.
+
+ See linux/dtrace/dof_defines.h for a complete list of section types
+ along with their values. */
+
+enum dtrace_dof_sect_type
+{
+ DTRACE_DOF_SECT_TYPE_NONE = 0,
+ DTRACE_DOF_SECT_TYPE_ECBDESC = 3,
+ DTRACE_DOF_SECT_TYPE_STRTAB = 8,
+ DTRACE_DOF_SECT_TYPE_PROVIDER = 15,
+ DTRACE_DOF_SECT_TYPE_PROBES = 16,
+ DTRACE_DOF_SECT_TYPE_PRARGS = 17,
+ DTRACE_DOF_SECT_TYPE_PROFFS = 18,
+ DTRACE_DOF_SECT_TYPE_PRENOFFS = 26
+};
+
+/* The following collection of data structures map the structure of
+ DOF entities. Again, we only cover the subset of DOF used to
+ implement USDT probes.
+
+ See linux/dtrace/dof.h header for a complete list of data
+ structures. */
+
+/* Indexes to use in `dofh_ident' below. */
+#define DTRACE_DOF_ID_MAG0 0
+#define DTRACE_DOF_ID_MAG1 1
+#define DTRACE_DOF_ID_MAG2 2
+#define DTRACE_DOF_ID_MAG3 3
+#define DTRACE_DOF_ID_ENCODING 5
+
+/* Possible values for dofh_ident[DOF_ID_ENCODING]. */
+#define DTRACE_DOF_ENCODE_LSB 1
+#define DTRACE_DOF_ENCODE_MSB 2
+
+struct dtrace_dof_hdr
+{
+ uint8_t dofh_ident[16];
+ uint32_t dofh_flags;
+ uint32_t dofh_hdrsize;
+ uint32_t dofh_secsize;
+ uint32_t dofh_secnum;
+ uint64_t dofh_secoff;
+ uint64_t dofh_loadsz;
+ uint64_t dofh_filesz;
+ uint64_t dofh_pad;
+};
+
+struct dtrace_dof_sect
+{
+ uint32_t dofs_type; /* See the enum dtrace_dof_sect above. */
+ uint32_t dofs_align;
+ uint32_t dofs_flags;
+ uint32_t dofs_entsize;
+ uint64_t dofs_offset; /* DOF + offset points to the section data. */
+ uint64_t dofs_size; /* Size of section data in bytes. */
+};
+
+struct dtrace_dof_provider
+{
+ uint32_t dofpv_strtab; /* Link to a DTRACE_DOF_SECT_TYPE_STRTAB section */
+ uint32_t dofpv_probes; /* Link to a DTRACE_DOF_SECT_TYPE_PROBES section */
+ uint32_t dofpv_prargs; /* Link to a DTRACE_DOF_SECT_TYPE_PRARGS section */
+ uint32_t dofpv_proffs; /* Link to a DTRACE_DOF_SECT_TYPE_PROFFS section */
+ uint32_t dofpv_name;
+ uint32_t dofpv_provattr;
+ uint32_t dofpv_modattr;
+ uint32_t dofpv_funcattr;
+ uint32_t dofpv_nameattr;
+ uint32_t dofpv_argsattr;
+ uint32_t dofpv_prenoffs; /* Link to a DTRACE_DOF_SECT_PRENOFFS section */
+};
+
+struct dtrace_dof_probe
+{
+ uint64_t dofpr_addr;
+ uint32_t dofpr_func;
+ uint32_t dofpr_name;
+ uint32_t dofpr_nargv;
+ uint32_t dofpr_xargv;
+ uint32_t dofpr_argidx;
+ uint32_t dofpr_offidx;
+ uint8_t dofpr_nargc;
+ uint8_t dofpr_xargc;
+ uint16_t dofpr_noffs;
+ uint32_t dofpr_enoffidx;
+ uint16_t dofpr_nenoffs;
+ uint16_t dofpr_pad1;
+ uint32_t dofpr_pad2;
+};
+
+/* DOF supports two different encodings: MSB (big-endian) and LSB
+ (little-endian). The encoding is itself encoded in the DOF header.
+ The following function returns an unsigned value in the host
+ endianness. */
+
+#define DOF_UINT(dof, field) \
+ extract_unsigned_integer ((gdb_byte *) &(field), \
+ sizeof ((field)), \
+ (((dof)->dofh_ident[DTRACE_DOF_ID_ENCODING] == DTRACE_DOF_ENCODE_MSB) \
+ ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE))
+
+/* The following macro applies a given byte offset to a DOF (a pointer
+ to a dtrace_dof_hdr structure) and returns the resulting
+ address. */
+
+#define DTRACE_DOF_PTR(dof, offset) (&((char *) (dof))[(offset)])
+
+/* The following macro returns a pointer to the beginning of a given
+ section in a DOF object. The section is referred to by its index
+ in the sections array. */
+
+#define DTRACE_DOF_SECT(dof, idx) \
+ ((struct dtrace_dof_sect *) \
+ DTRACE_DOF_PTR ((dof), \
+ DOF_UINT ((dof), (dof)->dofh_secoff) \
+ + ((idx) * DOF_UINT ((dof), (dof)->dofh_secsize))))
+
+/* Helper functions to extract the probe information from the DOF
+ object embedded in the .SUNW_dof section. */
+
+static void
+dtrace_process_dof_probe (struct objfile *objfile,
+ struct gdbarch *gdbarch, VEC (probe_p) **probesp,
+ struct dtrace_dof_hdr *dof, struct dtrace_dof_probe *probe,
+ struct dtrace_dof_provider *provider,
+ char *strtab, char *offtab, char *eofftab,
+ char *argtab, uint64_t strtab_size)
+{
+ struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+ int i, j, num_probes, num_enablers;
+ VEC (dtrace_probe_enabler_s) *enablers;
+ char *p;
+
+ /* Each probe section can define zero or more probes of two
+ different types:
+
+ - probe->dofpr_noffs regular probes whose program counters are
+ stored in 32bit words starting at probe->dofpr_addr +
+ offtab[probe->dofpr_offidx].
+
+ - probe->dofpr_nenoffs is-enabled probes whose program counters
+ are stored in 32bit words starting at probe->dofpr_addr +
+ eofftab[probe->dofpr_enoffidx].
+
+ However is-enabled probes are not probes per-se, but an
+ optimization hack that is implemented in the kernel in a very
+ similar way than normal probes. This is how we support
+ is-enabled probes on gdb:
+
+ - Our probes are always DTrace regular probes.
+
+ - Our probes can be associated with zero or more "enablers". The
+ list of enablers is built from the is-enabled probes defined in
+ the Probe section.
+
+ - Probes having a non-empty list of enablers can be enabled or
+ disabled using the `enable probe' and `disable probe' commands
+ respectively. The `Enabled' column in the output of `info
+ probes' will read `yes' if the enablers are activated, `no'
+ otherwise.
+
+ - Probes having an empty list of enablers are always enabled.
+ The `Enabled' column in the output of `info probes' will
+ read `always'.
+
+ It follows that if there are DTrace is-enabled probes defined for
+ some provider/name but no DTrace regular probes defined then the
+ gdb user wont be able to enable/disable these conditionals. */
+
+ num_probes = DOF_UINT (dof, probe->dofpr_noffs);
+ if (num_probes == 0)
+ return;
+
+ /* Build the list of enablers for the probes defined in this Probe
+ DOF section. */
+ enablers = NULL;
+ num_enablers = DOF_UINT (dof, probe->dofpr_nenoffs);
+ for (i = 0; i < num_enablers; i++)
+ {
+ struct dtrace_probe_enabler enabler;
+ uint32_t enabler_offset
+ = ((uint32_t *) eofftab)[DOF_UINT (dof, probe->dofpr_enoffidx) + i];
+
+ enabler.address = DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, enabler_offset);
+ VEC_safe_push (dtrace_probe_enabler_s, enablers, &enabler);
+ }
+
+ for (i = 0; i < num_probes; i++)
+ {
+ uint32_t probe_offset
+ = ((uint32_t *) offtab)[DOF_UINT (dof, probe->dofpr_offidx) + i];
+ struct dtrace_probe *ret
+ = obstack_alloc (&objfile->per_bfd->storage_obstack, sizeof (*ret));
+
+ ret->p.pops = &dtrace_probe_ops;
+ ret->p.arch = gdbarch;
+ ret->args_expr_built = 0;
+
+ /* Set the provider and the name of the probe. */
+ ret->p.provider = xstrdup (strtab + DOF_UINT (dof, provider->dofpv_name));
+ ret->p.name = xstrdup (strtab + DOF_UINT (dof, probe->dofpr_name));
+
+ /* The probe address. */
+ ret->p.address = DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, probe_offset);
+
+ /* Number of arguments in the probe. */
+ ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
+
+ /* Store argument type descriptions. A description of the type
+ of the argument is in the (J+1)th null-terminated string
+ starting at `strtab' + `probe->dofpr_nargv'. */
+ ret->args = NULL;
+ p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
+ for (j = 0; j < ret->probe_argc; j++)
+ {
+ struct dtrace_probe_arg arg;
+ struct expression *expr;
+
+ arg.type_str = xstrdup (p);
+ while (((p - strtab) < strtab_size) /* sentinel. */
+ && *p++);
+
+ /* Try to parse a type expression from the type string. If
+ this does not work then we set the type to `long
+ int'. */
+ arg.type = builtin_type (gdbarch)->builtin_long;
+ expr = parse_expression (arg.type_str);
+ if (expr->elts[0].opcode == OP_TYPE)
+ arg.type = expr->elts[1].type;
+
+ VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
+ }
+
+ /* Add the vector of enablers to this probe, if any. */
+ ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
+
+ /* Successfully created probe. */
+ VEC_safe_push (probe_p, *probesp, (struct probe *) ret);
+ }
+}
+
+static void
+dtrace_process_dof (asection *sect, struct objfile *objfile,
+ VEC (probe_p) **probesp, struct dtrace_dof_hdr *dof)
+{
+ bfd *abfd = objfile->obfd;
+ int size = bfd_get_arch_size (abfd) / 8;
+ struct gdbarch *gdbarch = get_objfile_arch (objfile);
+ struct dtrace_dof_sect *section;
+ int i;
+
+ /* The first step is to check for the DOF magic number. If no valid
+ DOF data is found in the section then a complaint is issued to
+ the user and the section skipped. */
+ if ((dof->dofh_ident[DTRACE_DOF_ID_MAG0] != 0x7F)
+ || (dof->dofh_ident[DTRACE_DOF_ID_MAG1] != 'D')
+ || (dof->dofh_ident[DTRACE_DOF_ID_MAG2] != 'O')
+ || (dof->dofh_ident[DTRACE_DOF_ID_MAG3] != 'F'))
+ goto invalid_dof_data;
+
+ /* Make sure this DOF is not an enabling DOF, i.e. there are no ECB
+ Description sections. */
+ section = (struct dtrace_dof_sect *) DTRACE_DOF_PTR (dof,
+ DOF_UINT (dof, dof->dofh_secoff));
+ for (i = 0; i < DOF_UINT (dof, dof->dofh_secnum); i++, section++)
+ if (section->dofs_type == DTRACE_DOF_SECT_TYPE_ECBDESC)
+ return;
+
+ /* Iterate over any section of type Provider and extract the probe
+ information from them. If there are no "provider" sections on
+ the DOF then we just return. */
+ section = (struct dtrace_dof_sect *) DTRACE_DOF_PTR (dof,
+ DOF_UINT (dof, dof->dofh_secoff));
+ for (i = 0; i < DOF_UINT (dof, dof->dofh_secnum); i++, section++)
+ if (DOF_UINT (dof, section->dofs_type) == DTRACE_DOF_SECT_TYPE_PROVIDER)
+ {
+ struct dtrace_dof_provider *provider = (struct dtrace_dof_provider *)
+ DTRACE_DOF_PTR (dof, DOF_UINT (dof, section->dofs_offset));
+
+ struct dtrace_dof_sect *strtab_s
+ = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_strtab));
+ struct dtrace_dof_sect *probes_s
+ = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_probes));
+ struct dtrace_dof_sect *args_s
+ = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_prargs));
+ struct dtrace_dof_sect *offsets_s
+ = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_proffs));
+ struct dtrace_dof_sect *eoffsets_s
+ = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_prenoffs));
+
+ char *strtab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, strtab_s->dofs_offset));
+ char *offtab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, offsets_s->dofs_offset));
+ char *eofftab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, eoffsets_s->dofs_offset));
+ char *argtab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, args_s->dofs_offset));
+
+ unsigned int entsize = DOF_UINT (dof, probes_s->dofs_entsize);
+ int num_probes;
+
+ /* Very, unlikely, but could crash gdb if not handled
+ properly. */
+ if (entsize == 0)
+ goto invalid_dof_data;
+
+ num_probes = DOF_UINT (dof, probes_s->dofs_size) / entsize;
+
+ for (i = 0; i < num_probes; i++)
+ {
+ struct dtrace_dof_probe *probe = (struct dtrace_dof_probe *)
+ DTRACE_DOF_PTR (dof, DOF_UINT (dof, probes_s->dofs_offset)
+ + (i * DOF_UINT (dof, probes_s->dofs_entsize)));
+ dtrace_process_dof_probe (objfile,
+ gdbarch, probesp,
+ dof, probe,
+ provider, strtab, offtab, eofftab, argtab,
+ DOF_UINT (dof, strtab_s->dofs_size));
+ }
+ }
+
+ return;
+
+ invalid_dof_data:
+ complaint (&symfile_complaints,
+ _("skipping section '%s' which does not contain valid DOF data."),
+ sect->name);
+ return;
+}
+
+/* Helper functions to make sure the expressions in the arguments
+ array are built as soon as their values are needed. */
+
+static void
+dtrace_build_arg_exprs (struct dtrace_probe *probe,
+ struct gdbarch *gdbarch)
+{
+ struct parser_state pstate;
+ struct cleanup *back_to;
+ struct dtrace_probe_arg *arg;
+ int i;
+
+ probe->args_expr_built = 1;
+
+ /* Iterate over the arguments in the probe and build the
+ corresponding GDB internal expression that will generate the
+ value of the argument when executed at the PC of the probe. */
+ for (i = 0; i < probe->probe_argc; i++)
+ {
+ arg = VEC_index (dtrace_probe_arg_s, probe->args, i);
+
+ /* Initialize the expression buffer in the parser state. The
+ language does not matter, since we are using our own
+ parser. */
+ initialize_expout (&pstate, 10, current_language, gdbarch);
+ back_to = make_cleanup (free_current_contents, &pstate.expout);
+
+ /* The argument value, which is ABI dependent and casted to
+ `long int'. */
+ gdbarch_dtrace_probe_argument (gdbarch, &pstate, i);
+
+ discard_cleanups (back_to);
+
+ /* Casting to the expected type, but only if the type was
+ recognized at probe load time. Otherwise the argument will
+ be evaluated as the long integer passed to the probe. */
+ if (arg->type)
+ {
+ write_exp_elt_opcode (&pstate, UNOP_CAST);
+ write_exp_elt_type (&pstate, arg->type);
+ write_exp_elt_opcode (&pstate, UNOP_CAST);
+ }
+
+ reallocate_expout (&pstate);
+ arg->expr = pstate.expout;
+ prefixify_expression (arg->expr);
+ }
+}
+
+static struct dtrace_probe_arg *
+dtrace_get_arg (struct dtrace_probe *probe, unsigned n,
+ struct gdbarch *gdbarch)
+{
+ if (!probe->args_expr_built)
+ dtrace_build_arg_exprs (probe, gdbarch);
+
+ return VEC_index (dtrace_probe_arg_s, probe->args, n);
+}
+
+/* Implementation of the get_probes method. */
+
+static void
+dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
+{
+ bfd *abfd = objfile->obfd;
+ asection *sect = NULL;
+
+ /* Do nothing in case this is a .debug file, instead of the objfile
+ itself. */
+ if (objfile->separate_debug_objfile_backlink != NULL)
+ return;
+
+ /* Iterate over the sections in OBJFILE looking for DTrace
+ information. */
+ for (sect = abfd->sections; sect != NULL; sect = sect->next)
+ {
+ if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
+ {
+ struct dtrace_dof_hdr *dof;
+
+ /* Read the contents of the DOF section and then process it to
+ extract the information of any probe defined into it. */
+ if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
+ {
+ complaint (&symfile_complaints,
+ _("could not obtain the contents of"
+ "section '%s' in objfile `%s'."),
+ sect->name, abfd->filename);
+ return;
+ }
+
+ dtrace_process_dof (sect, objfile, probesp, dof);
+ xfree (dof);
+ }
+ }
+}
+
+/* Helper function to determine whether a given probe is "enabled" or
+ "disabled". A disabled probe is a probe in which one or more
+ enablers are disabled. */
+
+static int
+dtrace_probe_is_enabled (struct dtrace_probe *probe)
+{
+ int i;
+ struct gdbarch *gdbarch = probe->p.arch;
+ struct dtrace_probe_enabler *enabler;
+
+ for (i = 0; VEC_iterate (dtrace_probe_enabler_s, probe->enablers, i, enabler); i++)
+ if (!gdbarch_dtrace_probe_is_enabled (gdbarch, enabler->address))
+ return 0;
+
+ return 1;
+}
+
+/* Implementation of the get_probe_address method. */
+
+static CORE_ADDR
+dtrace_get_probe_address (struct probe *probe, struct objfile *objfile)
+{
+ return probe->address + ANOFFSET (objfile->section_offsets,
+ SECT_OFF_DATA (objfile));
+}
+
+/* Implementation of the get_probe_argument_count method. */
+
+static unsigned
+dtrace_get_probe_argument_count (struct probe *probe_generic,
+ struct frame_info *frame)
+{
+ struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
+
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+
+ return dtrace_probe->probe_argc;
+}
+
+/* Implementation of the can_evaluate_probe_arguments method. */
+
+static int
+dtrace_can_evaluate_probe_arguments (struct probe *probe_generic)
+{
+ struct gdbarch *gdbarch = probe_generic->arch;
+
+ return gdbarch_dtrace_probe_argument_p (gdbarch);
+}
+
+/* Implementation of the evaluate_probe_argument method. */
+
+static struct value *
+dtrace_evaluate_probe_argument (struct probe *probe_generic, unsigned n,
+ struct frame_info *frame)
+{
+ struct gdbarch *gdbarch = probe_generic->arch;
+ struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
+ struct dtrace_probe_arg *arg;
+ int pos = 0;
+
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+
+ arg = dtrace_get_arg (dtrace_probe, n, gdbarch);
+ return evaluate_subexp_standard (arg->type, arg->expr, &pos, EVAL_NORMAL);
+}
+
+/* Implementation of the compile_to_ax method. */
+
+static void
+dtrace_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
+ struct axs_value *value, unsigned n)
+{
+ struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
+ struct dtrace_probe_arg *arg;
+ union exp_element *pc;
+
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+
+ arg = dtrace_get_arg (dtrace_probe, n, expr->gdbarch);
+
+ pc = arg->expr->elts;
+ gen_expr (arg->expr, &pc, expr, value);
+
+ require_rvalue (expr, value);
+ value->type = arg->type;
+}
+
+/* Implementation of the set_semaphore method. */
+
+static void
+dtrace_set_semaphore (struct probe *probe_generic, struct objfile *objfile,
+ struct gdbarch *gdbarch)
+{
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+}
+
+/* Implementation of the clear_semaphore method. */
+
+static void
+dtrace_clear_semaphore (struct probe *probe_generic, struct objfile *objfile,
+ struct gdbarch *gdbarch)
+{
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+}
+
+/* Implementation of the probe_destroy method. */
+
+static void
+dtrace_probe_destroy (struct probe *probe_generic)
+{
+ struct dtrace_probe *probe = (struct dtrace_probe *) probe_generic;
+ struct dtrace_probe_arg *arg;
+ int i;
+
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+
+ for (i = 0; VEC_iterate (dtrace_probe_arg_s, probe->args, i, arg); i++)
+ {
+ xfree (arg->type_str);
+ xfree (arg->expr);
+ }
+
+ VEC_free (dtrace_probe_enabler_s, probe->enablers);
+ VEC_free (dtrace_probe_arg_s, probe->args);
+}
+
+/* Implementation of the gen_info_probes_table_header method. */
+
+static void
+dtrace_gen_info_probes_table_header (VEC (info_probe_column_s) **heads)
+{
+ info_probe_column_s dtrace_probe_column;
+
+ dtrace_probe_column.field_name = "enabled";
+ dtrace_probe_column.print_name = _("Enabled");
+
+ VEC_safe_push (info_probe_column_s, *heads, &dtrace_probe_column);
+}
+
+/* Implementation of the gen_info_probes_table_values method. */
+
+static void
+dtrace_gen_info_probes_table_values (struct probe *probe_generic,
+ VEC (const_char_ptr) **ret)
+{
+ struct dtrace_probe *probe = (struct dtrace_probe *) probe_generic;
+ const char *val = NULL;
+
+ gdb_assert (probe_generic->pops == &dtrace_probe_ops);
+
+ if (VEC_empty (dtrace_probe_enabler_s, probe->enablers))
+ val = "always";
+ else if (!gdbarch_dtrace_probe_is_enabled_p (probe_generic->arch))
+ val = "unknown";
+ else if (dtrace_probe_is_enabled (probe))
+ val = "yes";
+ else
+ val = "no";
+
+ VEC_safe_push (const_char_ptr, *ret, val);
+}
+
+/* Implementation of the enable_probe method. */
+
+static void
+dtrace_enable_probe (struct probe *probe)
+{
+ struct gdbarch *gdbarch = probe->arch;
+ struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe;
+ struct dtrace_probe_enabler *enabler;
+ int i;
+
+ gdb_assert (probe->pops == &dtrace_probe_ops);
+
+ /* Enabling a dtrace probe implies patching the text section of the
+ running process, so make sure the inferior is indeed running. */
+ if (ptid_equal (inferior_ptid, null_ptid))
+ error (_("No inferior running"));
+
+ /* Fast path. */
+ if (dtrace_probe_is_enabled (dtrace_probe))
+ return;
+
+ /* Iterate over all defined enabler in the given probe and enable
+ them all using the corresponding gdbarch hook. */
+
+ for (i = 0;
+ VEC_iterate (dtrace_probe_enabler_s, dtrace_probe->enablers, i, enabler);
+ i++)
+ {
+ if (gdbarch_dtrace_enable_probe_p (gdbarch))
+ gdbarch_dtrace_enable_probe (gdbarch, enabler->address);
+ }
+}
+
+
+/* Implementation of the disable_probe method. */
+
+static void
+dtrace_disable_probe (struct probe *probe)
+{
+ struct gdbarch *gdbarch = probe->arch;
+ struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe;
+ struct dtrace_probe_enabler *enabler;
+ int i;
+
+ gdb_assert (probe->pops == &dtrace_probe_ops);
+
+ /* Disabling a dtrace probe implies patching the text section of the
+ running process, so make sure the inferior is indeed running. */
+ if (ptid_equal (inferior_ptid, null_ptid))
+ error (_("No inferior running"));
+
+ /* Fast path. */
+ if (!dtrace_probe_is_enabled (dtrace_probe))
+ return;
+
+ /* Are we trying to disable a probe that does not have any enabler
+ associated? */
+ if (VEC_empty (dtrace_probe_enabler_s, dtrace_probe->enablers))
+ error (_("Probe %s:%s cannot be disabled."), probe->provider, probe->name);
+
+ /* Iterate over all defined enabler in the given probe and disable
+ them all using the corresponding gdbarch hook. */
+
+ for (i = 0;
+ VEC_iterate (dtrace_probe_enabler_s, dtrace_probe->enablers, i, enabler);
+ i++)
+ {
+ if (gdbarch_dtrace_disable_probe_p (gdbarch))
+ gdbarch_dtrace_disable_probe (gdbarch, enabler->address);
+ }
+}
+
+/* DTrace probe_ops. */
+
+static const struct probe_ops dtrace_probe_ops =
+{
+ dtrace_probe_is_linespec,
+ dtrace_get_probes,
+ dtrace_get_probe_address,
+ dtrace_get_probe_argument_count,
+ dtrace_can_evaluate_probe_arguments,
+ dtrace_evaluate_probe_argument,
+ dtrace_compile_to_ax,
+ dtrace_set_semaphore,
+ dtrace_clear_semaphore,
+ dtrace_probe_destroy,
+ dtrace_gen_info_probes_table_header,
+ dtrace_gen_info_probes_table_values,
+ dtrace_enable_probe,
+ dtrace_disable_probe
+};
+
+/* Implementation of the `info probes dtrace' command. */
+
+static void
+info_probes_dtrace_command (char *arg, int from_tty)
+{
+ info_probes_for_ops (arg, from_tty, &dtrace_probe_ops);
+}
+
+void _initialize_dtrace_probe (void);
+
+void
+_initialize_dtrace_probe (void)
+{
+ VEC_safe_push (probe_ops_cp, all_probe_ops, &dtrace_probe_ops);
+
+ add_cmd ("dtrace", class_info, info_probes_dtrace_command,
+ _("\
+Show information about DTrace static probes.\n\
+Usage: info probes dtrace [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."),
+ info_probes_cmdlist_get ());
+}
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 5/9] New probe type: DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 5/9] New probe type: " Jose E. Marchesi
@ 2014-09-26 13:19 ` Eli Zaretskii
2014-10-02 23:19 ` Sergio Durigan Junior
1 sibling, 0 replies; 42+ messages in thread
From: Eli Zaretskii @ 2014-09-26 13:19 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
> From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
> Date: Fri, 26 Sep 2014 11:48:21 +0200
>
> + add_cmd ("dtrace", class_info, info_probes_dtrace_command,
> + _("\
> +Show information about DTrace static probes.\n\
> +Usage: info probes dtrace [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."),
> + info_probes_cmdlist_get ());
See my other mail with questions about the meaning of nesting of [..].
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 5/9] New probe type: DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 5/9] New probe type: " Jose E. Marchesi
2014-09-26 13:19 ` Eli Zaretskii
@ 2014-10-02 23:19 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
1 sibling, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-02 23:19 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 2014, Jose E. Marchesi wrote:
> This patch adds a new type of probe to GDB: the DTrace USDT probes. The new
> type is added by providing functions implementing all the entries of the
> `probe_ops' structure defined in `probe.h'. The implementation is
> self-contained and does not depend on DTrace source code in any way.
Thanks for the patch, Jose. It looks great. I have a few comments
(below).
> gdb:
>
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * breakpoint.c (BREAK_ARGS_HELP): help string updated to mention
> the -probe-dtrace new vpossible value for PROBE_MODIFIER.
>
> * configure.ac (CONFIG_OBS): dtrace-probe.o added if BFD can
> handle ELF files.
> * Makefile.in (SFILES): dtrace-probe.c added.
> * configure: Regenerate.
>
> * dtrace-probe.c: New file.
> (SHT_SUNW_dof): New constant.
> (dtrace_probe_type): New enum.
> (dtrace_probe_arg): New struct.
> (dtrace_probe_arg_s): New typedef.
> (struct dtrace_probe_enabler): New struct.
> (dtrace_probe_enabler_s): New typedef.
> (dtrace_probe): New struct.
> (dtrace_probe_is_linespec): New function.
> (dtrace_dof_sect_type): New enum.
> (DTRACE_DOF_ID_MAG0-3): New constants.
> (DTRACE_DOF_ID_ENCODING): Likewise.
> (DTRACE_DOF_ENCODE_LSB): Likewise.
> (DTRACE_DOF_ENCODE_MSB): Likewise.
> (dtrace_dof_hdr): New struct.
> (dtrace_dof_sect): Likewise.
> (dtrace_dof_provider): Likewise.
> (dtrace_dof_probe): Likewise.
> (DOF_UINT): New macro.
> (DTRACE_DOF_PTR): Likewise.
> (DTRACE_DOF_SECT): Likewise.
> (dtrace_process_dof_probe): New function.
> (dtrace_process_dof): Likewise.
> (dtrace_build_arg_exprs): Likewise.
> (dtrace_get_arg): Likewise.
> (dtrace_get_probes): Likewise.
> (dtrace_get_probe_argument_count): Likewise.
> (dtrace_can_evaluate_probe_arguments): Likewise.
> (dtrace_evaluate_probe_argument): Likewise.
> (dtrace_compile_to_ax): Likewise.
> (dtrace_set_semaphore): Likewise.
> (dtrace_clear_semaphore): Likewise.
> (dtrace_probe_destroy): Likewise.
> (dtrace_gen_info_probes_table_header): Likewise.
> (dtrace_gen_info_probes_table_values): Likewise.
> (dtrace_probe_is_enabled): Likewise.
> (dtrace_probe_ops): New variable.
> (info_probes_dtrace_command): New function.
> (_initialize_dtrace_probe): Likewise.
> ---
> gdb/ChangeLog | 50 ++++
> gdb/Makefile.in | 3 +-
> gdb/breakpoint.c | 3 +-
> gdb/configure | 2 +-
> gdb/configure.ac | 2 +-
> gdb/dtrace-probe.c | 816 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 872 insertions(+), 4 deletions(-)
> create mode 100644 gdb/dtrace-probe.c
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 7041cfb..eac03e7 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,55 @@
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> + * breakpoint.c (BREAK_ARGS_HELP): help string updated to mention
> + the -probe-dtrace new vpossible value for PROBE_MODIFIER.
> +
> + * configure.ac (CONFIG_OBS): dtrace-probe.o added if BFD can
> + handle ELF files.
> + * Makefile.in (SFILES): dtrace-probe.c added.
> + * configure: Regenerate.
> +
> + * dtrace-probe.c: New file.
> + (SHT_SUNW_dof): New constant.
> + (dtrace_probe_type): New enum.
> + (dtrace_probe_arg): New struct.
> + (dtrace_probe_arg_s): New typedef.
> + (struct dtrace_probe_enabler): New struct.
> + (dtrace_probe_enabler_s): New typedef.
> + (dtrace_probe): New struct.
> + (dtrace_probe_is_linespec): New function.
> + (dtrace_dof_sect_type): New enum.
> + (DTRACE_DOF_ID_MAG0-3): New constants.
> + (DTRACE_DOF_ID_ENCODING): Likewise.
> + (DTRACE_DOF_ENCODE_LSB): Likewise.
> + (DTRACE_DOF_ENCODE_MSB): Likewise.
> + (dtrace_dof_hdr): New struct.
> + (dtrace_dof_sect): Likewise.
> + (dtrace_dof_provider): Likewise.
> + (dtrace_dof_probe): Likewise.
> + (DOF_UINT): New macro.
> + (DTRACE_DOF_PTR): Likewise.
> + (DTRACE_DOF_SECT): Likewise.
> + (dtrace_process_dof_probe): New function.
> + (dtrace_process_dof): Likewise.
> + (dtrace_build_arg_exprs): Likewise.
> + (dtrace_get_arg): Likewise.
> + (dtrace_get_probes): Likewise.
> + (dtrace_get_probe_argument_count): Likewise.
> + (dtrace_can_evaluate_probe_arguments): Likewise.
> + (dtrace_evaluate_probe_argument): Likewise.
> + (dtrace_compile_to_ax): Likewise.
> + (dtrace_set_semaphore): Likewise.
> + (dtrace_clear_semaphore): Likewise.
> + (dtrace_probe_destroy): Likewise.
> + (dtrace_gen_info_probes_table_header): Likewise.
> + (dtrace_gen_info_probes_table_values): Likewise.
> + (dtrace_probe_is_enabled): Likewise.
> + (dtrace_probe_ops): New variable.
> + (info_probes_dtrace_command): New function.
> + (_initialize_dtrace_probe): Likewise.
> +
> +2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
> +
> * gdbarch.sh (dtrace_probe_argument): New.
> (dtrace_probe_is_enabled): Likewise.
> (dtrace_enable_probe): Likewise.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index cbec0d2..ad82215 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -799,7 +799,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
> cp-abi.c cp-support.c cp-namespace.c cp-valprint.c \
> d-exp.y d-lang.c d-support.c d-valprint.c \
> cp-name-parser.y \
> - dbxread.c demangle.c dictionary.c disasm.c doublest.c dummy-frame.c \
> + dbxread.c demangle.c dictionary.c disasm.c doublest.c \
> + dtrace-probe.c dummy-frame.c \
> dwarf2expr.c dwarf2loc.c dwarf2read.c dwarf2-frame.c \
> dwarf2-frame-tailcall.c \
> elfread.c environ.c eval.c event-loop.c event-top.c \
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index bec7f68..95323bf 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -16197,7 +16197,8 @@ all_tracepoints (void)
> command" [PROBE_MODIFIER] [LOCATION] [thread THREADNUM] [if CONDITION]\n\
> PROBE_MODIFIER shall be present if the command is to be placed in a\n\
> probe point. Accepted values are `-probe' (for a generic, automatically\n\
> -guessed probe type) or `-probe-stap' (for a SystemTap probe).\n\
> +guessed probe type), `-probe-stap' (for a SystemTap probe) or \n\
> +`-probe-dtrace' (for a DTrace probe).\n\
> LOCATION may be a line number, function name, or \"*\" and an address.\n\
> If a line number is specified, break at start of code for that line.\n\
> If a function is specified, break at start of code for that function.\n\
> diff --git a/gdb/configure b/gdb/configure
> index 6e7435f..ecd0cff 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -13281,7 +13281,7 @@ $as_echo "$gdb_cv_var_elf" >&6; }
> LDFLAGS=$OLD_LDFLAGS
> LIBS=$OLD_LIBS
> if test $gdb_cv_var_elf = yes; then
> - CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o"
> + CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o"
>
> $as_echo "#define HAVE_ELF 1" >>confdefs.h
>
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 4f5fb7b..7b1133a 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -2103,7 +2103,7 @@ AC_SUBST(WIN32LIBS)
> GDB_AC_CHECK_BFD([for ELF support in BFD], gdb_cv_var_elf,
> [bfd_get_elf_phdr_upper_bound (NULL)], elf-bfd.h)
> if test $gdb_cv_var_elf = yes; then
> - CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o"
> + CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o"
> AC_DEFINE(HAVE_ELF, 1,
> [Define if ELF support should be included.])
> # -ldl is provided by bfd/Makfile.am (LIBDL) <PLUGINS>.
> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
> new file mode 100644
> index 0000000..f529ca5
> --- /dev/null
> +++ b/gdb/dtrace-probe.c
> @@ -0,0 +1,816 @@
> +/* DTrace probe support for GDB.
> +
> + Copyright (C) 2014 Free Software Foundation, Inc.
> +
> + Contributed by Oracle, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "defs.h"
> +#include "probe.h"
> +#include "vec.h"
> +#include "elf-bfd.h"
> +#include "gdbtypes.h"
> +#include "obstack.h"
> +#include "objfiles.h"
> +#include "complaints.h"
> +#include "value.h"
> +#include "ax.h"
> +#include "ax-gdb.h"
> +#include "language.h"
> +#include "parser-defs.h"
> +#include "inferior.h"
> +
> +/* The type of the ELF sections where we will find the DOF programs
> + with information about probes. */
> +
> +#ifndef SHT_SUNW_dof
> +# define SHT_SUNW_dof 0x6ffffff4
> +#endif
Can this macro exist in another header file that you are including?
> +
> +/* Forward declaration. */
> +
> +static const struct probe_ops dtrace_probe_ops;
> +
> +/* The following structure represents a single argument for the
> + probe. */
> +
> +struct dtrace_probe_arg
> +{
> + /* The type of the probe argument. */
> + struct type *type;
> +
> + /* A string describing the type. */
> + char *type_str;
> +
> + /* The argument converted to an internal GDB expression. */
> + struct expression *expr;
> +};
> +
> +typedef struct dtrace_probe_arg dtrace_probe_arg_s;
> +DEF_VEC_O (dtrace_probe_arg_s);
> +
> +/* The following structure represents an enabler for a probe. */
> +
> +struct dtrace_probe_enabler
> +{
> + /* Program counter where the is-enabled probe is installed. The
> + contents (nops, whatever...) stored at this address are
> + architecture dependent. */
> + CORE_ADDR address;
> +};
> +
> +typedef struct dtrace_probe_enabler dtrace_probe_enabler_s;
> +DEF_VEC_O (dtrace_probe_enabler_s);
> +
> +/* The following structure represents a dtrace probe. */
> +
> +struct dtrace_probe
> +{
> + /* Generic information about the probe. This must be the first
> + element of this struct, in order to maintain binary compatibility
> + with the `struct probe' and be able to fully abstract it. */
> + struct probe p;
> +
> + /* A probe can have zero or more arguments. */
> + int probe_argc;
> + VEC (dtrace_probe_arg_s) *args;
> +
> + /* A probe can have zero or more "enablers" associated with it. */
> + VEC (dtrace_probe_enabler_s) *enablers;
> +
> + /* Whether the expressions for the arguments have been built. */
> + int args_expr_built : 1;
> +};
> +
> +/* Implementation of the probe_is_linespec method. */
> +
> +static int
> +dtrace_probe_is_linespec (const char **linespecp)
> +{
> + static const char *const keywords[] = { "-pdtrace", "-probe-dtrace", NULL };
> +
> + return probe_is_linespec_by_keyword (linespecp, keywords);
> +}
> +
> +/* DOF programs can contain an arbitrary number of sections of 26
> + different types. In order to support DTrace USDT probes we only
> + need to handle a subset of these section types, fortunately. These
> + section types are defined in the following enumeration.
> +
> + See linux/dtrace/dof_defines.h for a complete list of section types
> + along with their values. */
> +
> +enum dtrace_dof_sect_type
> +{
> + DTRACE_DOF_SECT_TYPE_NONE = 0,
> + DTRACE_DOF_SECT_TYPE_ECBDESC = 3,
> + DTRACE_DOF_SECT_TYPE_STRTAB = 8,
> + DTRACE_DOF_SECT_TYPE_PROVIDER = 15,
> + DTRACE_DOF_SECT_TYPE_PROBES = 16,
> + DTRACE_DOF_SECT_TYPE_PRARGS = 17,
> + DTRACE_DOF_SECT_TYPE_PROFFS = 18,
> + DTRACE_DOF_SECT_TYPE_PRENOFFS = 26
> +};
It's better to put a comment in each field, describing them.
> +
> +/* The following collection of data structures map the structure of
> + DOF entities. Again, we only cover the subset of DOF used to
> + implement USDT probes.
> +
> + See linux/dtrace/dof.h header for a complete list of data
> + structures. */
> +
> +/* Indexes to use in `dofh_ident' below. */
> +#define DTRACE_DOF_ID_MAG0 0
> +#define DTRACE_DOF_ID_MAG1 1
> +#define DTRACE_DOF_ID_MAG2 2
> +#define DTRACE_DOF_ID_MAG3 3
> +#define DTRACE_DOF_ID_ENCODING 5
> +
> +/* Possible values for dofh_ident[DOF_ID_ENCODING]. */
> +#define DTRACE_DOF_ENCODE_LSB 1
> +#define DTRACE_DOF_ENCODE_MSB 2
I know it is a matter of taste, but I like more when you use enum's for
these kind of definitions (like above) :-).
> +
> +struct dtrace_dof_hdr
> +{
> + uint8_t dofh_ident[16];
> + uint32_t dofh_flags;
> + uint32_t dofh_hdrsize;
> + uint32_t dofh_secsize;
> + uint32_t dofh_secnum;
> + uint64_t dofh_secoff;
> + uint64_t dofh_loadsz;
> + uint64_t dofh_filesz;
> + uint64_t dofh_pad;
> +};
> +
> +struct dtrace_dof_sect
> +{
> + uint32_t dofs_type; /* See the enum dtrace_dof_sect above. */
> + uint32_t dofs_align;
> + uint32_t dofs_flags;
> + uint32_t dofs_entsize;
> + uint64_t dofs_offset; /* DOF + offset points to the section data. */
> + uint64_t dofs_size; /* Size of section data in bytes. */
> +};
> +
> +struct dtrace_dof_provider
> +{
> + uint32_t dofpv_strtab; /* Link to a DTRACE_DOF_SECT_TYPE_STRTAB section */
> + uint32_t dofpv_probes; /* Link to a DTRACE_DOF_SECT_TYPE_PROBES section */
> + uint32_t dofpv_prargs; /* Link to a DTRACE_DOF_SECT_TYPE_PRARGS section */
> + uint32_t dofpv_proffs; /* Link to a DTRACE_DOF_SECT_TYPE_PROFFS section */
> + uint32_t dofpv_name;
> + uint32_t dofpv_provattr;
> + uint32_t dofpv_modattr;
> + uint32_t dofpv_funcattr;
> + uint32_t dofpv_nameattr;
> + uint32_t dofpv_argsattr;
> + uint32_t dofpv_prenoffs; /* Link to a DTRACE_DOF_SECT_PRENOFFS section */
> +};
> +
> +struct dtrace_dof_probe
> +{
> + uint64_t dofpr_addr;
> + uint32_t dofpr_func;
> + uint32_t dofpr_name;
> + uint32_t dofpr_nargv;
> + uint32_t dofpr_xargv;
> + uint32_t dofpr_argidx;
> + uint32_t dofpr_offidx;
> + uint8_t dofpr_nargc;
> + uint8_t dofpr_xargc;
> + uint16_t dofpr_noffs;
> + uint32_t dofpr_enoffidx;
> + uint16_t dofpr_nenoffs;
> + uint16_t dofpr_pad1;
> + uint32_t dofpr_pad2;
> +};
Each one of these structs need a comment on top describing it, and I
would also appreciate a comment on each field.
> +
> +/* DOF supports two different encodings: MSB (big-endian) and LSB
> + (little-endian). The encoding is itself encoded in the DOF header.
> + The following function returns an unsigned value in the host
> + endianness. */
> +
> +#define DOF_UINT(dof, field) \
> + extract_unsigned_integer ((gdb_byte *) &(field), \
> + sizeof ((field)), \
> + (((dof)->dofh_ident[DTRACE_DOF_ID_ENCODING] == DTRACE_DOF_ENCODE_MSB) \
This line is too long. You can break it before the "==".
> + ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE))
> +
> +/* The following macro applies a given byte offset to a DOF (a pointer
> + to a dtrace_dof_hdr structure) and returns the resulting
> + address. */
> +
> +#define DTRACE_DOF_PTR(dof, offset) (&((char *) (dof))[(offset)])
> +
> +/* The following macro returns a pointer to the beginning of a given
> + section in a DOF object. The section is referred to by its index
> + in the sections array. */
> +
> +#define DTRACE_DOF_SECT(dof, idx) \
> + ((struct dtrace_dof_sect *) \
> + DTRACE_DOF_PTR ((dof), \
> + DOF_UINT ((dof), (dof)->dofh_secoff) \
> + + ((idx) * DOF_UINT ((dof), (dof)->dofh_secsize))))
> +
> +/* Helper functions to extract the probe information from the DOF
> + object embedded in the .SUNW_dof section. */
Each function should have a comment. I understand this comment applies
to a set of functions, but you also need comments for the other
functions as well.
I would also appreciate if the comment gave a brief explanation of how
the function works in terms of the arguments it receives.
> +
> +static void
> +dtrace_process_dof_probe (struct objfile *objfile,
> + struct gdbarch *gdbarch, VEC (probe_p) **probesp,
> + struct dtrace_dof_hdr *dof, struct dtrace_dof_probe *probe,
Line too long.
> + struct dtrace_dof_provider *provider,
> + char *strtab, char *offtab, char *eofftab,
> + char *argtab, uint64_t strtab_size)
> +{
> + struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
You're not using this variable anywhere.
> + int i, j, num_probes, num_enablers;
> + VEC (dtrace_probe_enabler_s) *enablers;
> + char *p;
> +
> + /* Each probe section can define zero or more probes of two
> + different types:
> +
> + - probe->dofpr_noffs regular probes whose program counters are
> + stored in 32bit words starting at probe->dofpr_addr +
> + offtab[probe->dofpr_offidx].
> +
> + - probe->dofpr_nenoffs is-enabled probes whose program counters
> + are stored in 32bit words starting at probe->dofpr_addr +
> + eofftab[probe->dofpr_enoffidx].
> +
> + However is-enabled probes are not probes per-se, but an
> + optimization hack that is implemented in the kernel in a very
> + similar way than normal probes. This is how we support
> + is-enabled probes on gdb:
> +
> + - Our probes are always DTrace regular probes.
> +
> + - Our probes can be associated with zero or more "enablers". The
> + list of enablers is built from the is-enabled probes defined in
> + the Probe section.
> +
> + - Probes having a non-empty list of enablers can be enabled or
> + disabled using the `enable probe' and `disable probe' commands
> + respectively. The `Enabled' column in the output of `info
> + probes' will read `yes' if the enablers are activated, `no'
> + otherwise.
> +
> + - Probes having an empty list of enablers are always enabled.
> + The `Enabled' column in the output of `info probes' will
> + read `always'.
> +
> + It follows that if there are DTrace is-enabled probes defined for
> + some provider/name but no DTrace regular probes defined then the
> + gdb user wont be able to enable/disable these conditionals. */
Thanks for the explanation! Always good to have descriptive comments in
the code :-). While at it, could you please s/gdb/GDB/ in the comment?
> +
> + num_probes = DOF_UINT (dof, probe->dofpr_noffs);
> + if (num_probes == 0)
> + return;
> +
> + /* Build the list of enablers for the probes defined in this Probe
> + DOF section. */
> + enablers = NULL;
> + num_enablers = DOF_UINT (dof, probe->dofpr_nenoffs);
> + for (i = 0; i < num_enablers; i++)
> + {
> + struct dtrace_probe_enabler enabler;
> + uint32_t enabler_offset
> + = ((uint32_t *) eofftab)[DOF_UINT (dof, probe->dofpr_enoffidx) + i];
> +
> + enabler.address = DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, enabler_offset);
Line too long.
> + VEC_safe_push (dtrace_probe_enabler_s, enablers, &enabler);
> + }
> +
> + for (i = 0; i < num_probes; i++)
> + {
> + uint32_t probe_offset
> + = ((uint32_t *) offtab)[DOF_UINT (dof, probe->dofpr_offidx) + i];
> + struct dtrace_probe *ret
> + = obstack_alloc (&objfile->per_bfd->storage_obstack, sizeof (*ret));
> +
> + ret->p.pops = &dtrace_probe_ops;
> + ret->p.arch = gdbarch;
> + ret->args_expr_built = 0;
> +
> + /* Set the provider and the name of the probe. */
> + ret->p.provider = xstrdup (strtab + DOF_UINT (dof, provider->dofpv_name));
Line too long.
> + ret->p.name = xstrdup (strtab + DOF_UINT (dof, probe->dofpr_name));
I don't personally like this convention of aligning the function names;
it gets in the way when I'm grepping for some attribution. But again,
this is a matter of taste, so feel free to ignore the comment.
> +
> + /* The probe address. */
> + ret->p.address = DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, probe_offset);
Line too long.
> +
> + /* Number of arguments in the probe. */
> + ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
> +
> + /* Store argument type descriptions. A description of the type
> + of the argument is in the (J+1)th null-terminated string
> + starting at `strtab' + `probe->dofpr_nargv'. */
We're not using `' anymore; instead, we're using '' (GNU Coding Style
has been updated).
> + ret->args = NULL;
> + p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
> + for (j = 0; j < ret->probe_argc; j++)
> + {
> + struct dtrace_probe_arg arg;
> + struct expression *expr;
> +
> + arg.type_str = xstrdup (p);
> + while (((p - strtab) < strtab_size) /* sentinel. */
> + && *p++);
Again a matter of style, but for readability I prefer to write this loop
as:
/* Use strtab_size as a sentinel. */
while (*p != '\0' && p - strtab < strtab_size)
++p;
> +
> + /* Try to parse a type expression from the type string. If
> + this does not work then we set the type to `long
> + int'. */
> + arg.type = builtin_type (gdbarch)->builtin_long;
This line has a different indentation than the others.
> + expr = parse_expression (arg.type_str);
> + if (expr->elts[0].opcode == OP_TYPE)
> + arg.type = expr->elts[1].type;
> +
The line above contains extraneous whitespaces.
> + VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
> + }
> +
> + /* Add the vector of enablers to this probe, if any. */
> + ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
You should free the enablers VEC in the end of the function. You could
probably make a cleanup and call it later.
> +
Extraneous whitespaces
> + /* Successfully created probe. */
> + VEC_safe_push (probe_p, *probesp, (struct probe *) ret);
> + }
> +}
> +
> +static void
> +dtrace_process_dof (asection *sect, struct objfile *objfile,
> + VEC (probe_p) **probesp, struct dtrace_dof_hdr *dof)
> +{
Just a reminder, this function is missing a comment.
> + bfd *abfd = objfile->obfd;
> + int size = bfd_get_arch_size (abfd) / 8;
> + struct gdbarch *gdbarch = get_objfile_arch (objfile);
> + struct dtrace_dof_sect *section;
> + int i;
> +
> + /* The first step is to check for the DOF magic number. If no valid
> + DOF data is found in the section then a complaint is issued to
> + the user and the section skipped. */
> + if ((dof->dofh_ident[DTRACE_DOF_ID_MAG0] != 0x7F)
> + || (dof->dofh_ident[DTRACE_DOF_ID_MAG1] != 'D')
> + || (dof->dofh_ident[DTRACE_DOF_ID_MAG2] != 'O')
> + || (dof->dofh_ident[DTRACE_DOF_ID_MAG3] != 'F'))
Excessive use of parentheses.
> + goto invalid_dof_data;
> +
> + /* Make sure this DOF is not an enabling DOF, i.e. there are no ECB
> + Description sections. */
> + section = (struct dtrace_dof_sect *) DTRACE_DOF_PTR (dof,
> + DOF_UINT (dof, dof->dofh_secoff));
> + for (i = 0; i < DOF_UINT (dof, dof->dofh_secnum); i++, section++)
> + if (section->dofs_type == DTRACE_DOF_SECT_TYPE_ECBDESC)
> + return;
> +
> + /* Iterate over any section of type Provider and extract the probe
> + information from them. If there are no "provider" sections on
> + the DOF then we just return. */
> + section = (struct dtrace_dof_sect *) DTRACE_DOF_PTR (dof,
> + DOF_UINT (dof, dof->dofh_secoff));
> + for (i = 0; i < DOF_UINT (dof, dof->dofh_secnum); i++, section++)
> + if (DOF_UINT (dof, section->dofs_type) == DTRACE_DOF_SECT_TYPE_PROVIDER)
> + {
> + struct dtrace_dof_provider *provider = (struct dtrace_dof_provider *)
> + DTRACE_DOF_PTR (dof, DOF_UINT (dof, section->dofs_offset));
> +
> + struct dtrace_dof_sect *strtab_s
> + = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_strtab));
> + struct dtrace_dof_sect *probes_s
> + = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_probes));
> + struct dtrace_dof_sect *args_s
> + = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_prargs));
> + struct dtrace_dof_sect *offsets_s
> + = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_proffs));
> + struct dtrace_dof_sect *eoffsets_s
> + = DTRACE_DOF_SECT (dof, DOF_UINT (dof, provider->dofpv_prenoffs));
> +
> + char *strtab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, strtab_s->dofs_offset));
> + char *offtab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, offsets_s->dofs_offset));
> + char *eofftab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, eoffsets_s->dofs_offset));
> + char *argtab = DTRACE_DOF_PTR (dof, DOF_UINT (dof, args_s->dofs_offset));
> +
> + unsigned int entsize = DOF_UINT (dof, probes_s->dofs_entsize);
> + int num_probes;
No newlines between variables being declared.
> + /* Very, unlikely, but could crash gdb if not handled
> + properly. */
> + if (entsize == 0)
> + goto invalid_dof_data;
> +
> + num_probes = DOF_UINT (dof, probes_s->dofs_size) / entsize;
> +
> + for (i = 0; i < num_probes; i++)
> + {
> + struct dtrace_dof_probe *probe = (struct dtrace_dof_probe *)
> + DTRACE_DOF_PTR (dof, DOF_UINT (dof, probes_s->dofs_offset)
> + + (i * DOF_UINT (dof, probes_s->dofs_entsize)));
> + dtrace_process_dof_probe (objfile,
> + gdbarch, probesp,
> + dof, probe,
> + provider, strtab, offtab, eofftab, argtab,
> + DOF_UINT (dof, strtab_s->dofs_size));
And a newline here, between the variable declaration and the function
call :-).
> + }
> + }
> +
> + return;
> +
> + invalid_dof_data:
> + complaint (&symfile_complaints,
> + _("skipping section '%s' which does not contain valid DOF data."),
> + sect->name);
> + return;
You don't need this return.
> +}
> +
> +/* Helper functions to make sure the expressions in the arguments
> + array are built as soon as their values are needed. */
> +
> +static void
> +dtrace_build_arg_exprs (struct dtrace_probe *probe,
> + struct gdbarch *gdbarch)
> +{
> + struct parser_state pstate;
> + struct cleanup *back_to;
> + struct dtrace_probe_arg *arg;
> + int i;
> +
> + probe->args_expr_built = 1;
> +
Extraneous whitespaces.
> + /* Iterate over the arguments in the probe and build the
> + corresponding GDB internal expression that will generate the
> + value of the argument when executed at the PC of the probe. */
> + for (i = 0; i < probe->probe_argc; i++)
> + {
> + arg = VEC_index (dtrace_probe_arg_s, probe->args, i);
> +
> + /* Initialize the expression buffer in the parser state. The
> + language does not matter, since we are using our own
> + parser. */
> + initialize_expout (&pstate, 10, current_language, gdbarch);
> + back_to = make_cleanup (free_current_contents, &pstate.expout);
Can you declare back_to inside the for block, please?
> +
> + /* The argument value, which is ABI dependent and casted to
> + `long int'. */
> + gdbarch_dtrace_probe_argument (gdbarch, &pstate, i);
> +
> + discard_cleanups (back_to);
> +
> + /* Casting to the expected type, but only if the type was
> + recognized at probe load time. Otherwise the argument will
> + be evaluated as the long integer passed to the probe. */
> + if (arg->type)
Please, check explicitly against NULL here.
> + {
> + write_exp_elt_opcode (&pstate, UNOP_CAST);
> + write_exp_elt_type (&pstate, arg->type);
> + write_exp_elt_opcode (&pstate, UNOP_CAST);
> + }
> +
> + reallocate_expout (&pstate);
> + arg->expr = pstate.expout;
> + prefixify_expression (arg->expr);
> + }
> +}
> +
> +static struct dtrace_probe_arg *
> +dtrace_get_arg (struct dtrace_probe *probe, unsigned n,
> + struct gdbarch *gdbarch)
> +{
> + if (!probe->args_expr_built)
> + dtrace_build_arg_exprs (probe, gdbarch);
> +
> + return VEC_index (dtrace_probe_arg_s, probe->args, n);
> +}
> +
> +/* Implementation of the get_probes method. */
> +
> +static void
> +dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> +{
> + bfd *abfd = objfile->obfd;
> + asection *sect = NULL;
> +
> + /* Do nothing in case this is a .debug file, instead of the objfile
> + itself. */
> + if (objfile->separate_debug_objfile_backlink != NULL)
> + return;
> +
> + /* Iterate over the sections in OBJFILE looking for DTrace
> + information. */
> + for (sect = abfd->sections; sect != NULL; sect = sect->next)
> + {
> + if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
> + {
> + struct dtrace_dof_hdr *dof;
> +
> + /* Read the contents of the DOF section and then process it to
> + extract the information of any probe defined into it. */
> + if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
> + {
> + complaint (&symfile_complaints,
> + _("could not obtain the contents of"
> + "section '%s' in objfile `%s'."),
> + sect->name, abfd->filename);
> + return;
Why return here? Is there only one section whose type is SHT_SUNW_dof?
If no, then I guess the loop should keep rolling. Otherwise, then
besides calling return here you should call return after the "xfree"
below. Am I getting it right?
> + }
> +
> + dtrace_process_dof (sect, objfile, probesp, dof);
> + xfree (dof);
> + }
> + }
What about using bfd_map_over_sections instead of this for loop? I know
there is precedence of iterating over BFD sections by hand on GDB code,
but bfd_map_over_sections exists for this very purpose.
BTW, s/bfd_map_over_sections/bfd_sections_find_if/ if you should
terminate the loop when the first section is found.
> +}
> +
> +/* Helper function to determine whether a given probe is "enabled" or
> + "disabled". A disabled probe is a probe in which one or more
> + enablers are disabled. */
> +
> +static int
> +dtrace_probe_is_enabled (struct dtrace_probe *probe)
> +{
> + int i;
> + struct gdbarch *gdbarch = probe->p.arch;
> + struct dtrace_probe_enabler *enabler;
> +
> + for (i = 0; VEC_iterate (dtrace_probe_enabler_s, probe->enablers, i, enabler); i++)
Line too long.
> + if (!gdbarch_dtrace_probe_is_enabled (gdbarch, enabler->address))
> + return 0;
> +
> + return 1;
> +}
> +
> +/* Implementation of the get_probe_address method. */
> +
> +static CORE_ADDR
> +dtrace_get_probe_address (struct probe *probe, struct objfile *objfile)
> +{
> + return probe->address + ANOFFSET (objfile->section_offsets,
> + SECT_OFF_DATA (objfile));
> +}
I know this code is a copy-and-paste of the SDT's code, but it would be
nice to do the regular gdb_assert dance here. I will check in a patch
for stap-probe.c soon to fix that.
> +
> +/* Implementation of the get_probe_argument_count method. */
> +
> +static unsigned
> +dtrace_get_probe_argument_count (struct probe *probe_generic,
> + struct frame_info *frame)
> +{
> + struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
> +
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +
> + return dtrace_probe->probe_argc;
> +}
> +
> +/* Implementation of the can_evaluate_probe_arguments method. */
> +
> +static int
> +dtrace_can_evaluate_probe_arguments (struct probe *probe_generic)
> +{
> + struct gdbarch *gdbarch = probe_generic->arch;
> +
> + return gdbarch_dtrace_probe_argument_p (gdbarch);
> +}
Likewise (about the gdb_assert call).
> +
> +/* Implementation of the evaluate_probe_argument method. */
> +
> +static struct value *
> +dtrace_evaluate_probe_argument (struct probe *probe_generic, unsigned n,
> + struct frame_info *frame)
> +{
> + struct gdbarch *gdbarch = probe_generic->arch;
> + struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
> + struct dtrace_probe_arg *arg;
> + int pos = 0;
> +
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +
> + arg = dtrace_get_arg (dtrace_probe, n, gdbarch);
> + return evaluate_subexp_standard (arg->type, arg->expr, &pos, EVAL_NORMAL);
> +}
> +
> +/* Implementation of the compile_to_ax method. */
> +
> +static void
> +dtrace_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
> + struct axs_value *value, unsigned n)
> +{
> + struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
> + struct dtrace_probe_arg *arg;
> + union exp_element *pc;
> +
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +
> + arg = dtrace_get_arg (dtrace_probe, n, expr->gdbarch);
> +
> + pc = arg->expr->elts;
> + gen_expr (arg->expr, &pc, expr, value);
> +
> + require_rvalue (expr, value);
> + value->type = arg->type;
> +}
> +
> +/* Implementation of the set_semaphore method. */
> +
> +static void
> +dtrace_set_semaphore (struct probe *probe_generic, struct objfile *objfile,
> + struct gdbarch *gdbarch)
> +{
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +}
> +
> +/* Implementation of the clear_semaphore method. */
> +
> +static void
> +dtrace_clear_semaphore (struct probe *probe_generic, struct objfile *objfile,
> + struct gdbarch *gdbarch)
> +{
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +}
This shouldn't be needed, because USDT probes don't have the concept of
a semaphore, right? I will submit a patch soon to fix the fact that the
set/clear_semaphore functions are being called inconditionally.
> +
> +/* Implementation of the probe_destroy method. */
> +
> +static void
> +dtrace_probe_destroy (struct probe *probe_generic)
> +{
> + struct dtrace_probe *probe = (struct dtrace_probe *) probe_generic;
> + struct dtrace_probe_arg *arg;
> + int i;
> +
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +
> + for (i = 0; VEC_iterate (dtrace_probe_arg_s, probe->args, i, arg); i++)
> + {
> + xfree (arg->type_str);
> + xfree (arg->expr);
> + }
> +
> + VEC_free (dtrace_probe_enabler_s, probe->enablers);
> + VEC_free (dtrace_probe_arg_s, probe->args);
> +}
> +
> +/* Implementation of the gen_info_probes_table_header method. */
> +
> +static void
> +dtrace_gen_info_probes_table_header (VEC (info_probe_column_s) **heads)
> +{
> + info_probe_column_s dtrace_probe_column;
> +
> + dtrace_probe_column.field_name = "enabled";
> + dtrace_probe_column.print_name = _("Enabled");
> +
> + VEC_safe_push (info_probe_column_s, *heads, &dtrace_probe_column);
> +}
> +
> +/* Implementation of the gen_info_probes_table_values method. */
> +
> +static void
> +dtrace_gen_info_probes_table_values (struct probe *probe_generic,
> + VEC (const_char_ptr) **ret)
> +{
> + struct dtrace_probe *probe = (struct dtrace_probe *) probe_generic;
> + const char *val = NULL;
> +
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +
> + if (VEC_empty (dtrace_probe_enabler_s, probe->enablers))
> + val = "always";
> + else if (!gdbarch_dtrace_probe_is_enabled_p (probe_generic->arch))
> + val = "unknown";
> + else if (dtrace_probe_is_enabled (probe))
> + val = "yes";
> + else
> + val = "no";
> +
> + VEC_safe_push (const_char_ptr, *ret, val);
> +}
> +
> +/* Implementation of the enable_probe method. */
> +
> +static void
> +dtrace_enable_probe (struct probe *probe)
> +{
> + struct gdbarch *gdbarch = probe->arch;
> + struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe;
> + struct dtrace_probe_enabler *enabler;
> + int i;
> +
> + gdb_assert (probe->pops == &dtrace_probe_ops);
> +
> + /* Enabling a dtrace probe implies patching the text section of the
> + running process, so make sure the inferior is indeed running. */
> + if (ptid_equal (inferior_ptid, null_ptid))
> + error (_("No inferior running"));
> +
> + /* Fast path. */
> + if (dtrace_probe_is_enabled (dtrace_probe))
> + return;
> +
> + /* Iterate over all defined enabler in the given probe and enable
> + them all using the corresponding gdbarch hook. */
> +
> + for (i = 0;
> + VEC_iterate (dtrace_probe_enabler_s, dtrace_probe->enablers, i, enabler);
> + i++)
> + {
> + if (gdbarch_dtrace_enable_probe_p (gdbarch))
> + gdbarch_dtrace_enable_probe (gdbarch, enabler->address);
> + }
No need for the brackets here.
> +}
> +
> +
> +/* Implementation of the disable_probe method. */
> +
> +static void
> +dtrace_disable_probe (struct probe *probe)
> +{
> + struct gdbarch *gdbarch = probe->arch;
> + struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe;
> + struct dtrace_probe_enabler *enabler;
> + int i;
> +
> + gdb_assert (probe->pops == &dtrace_probe_ops);
> +
> + /* Disabling a dtrace probe implies patching the text section of the
> + running process, so make sure the inferior is indeed running. */
> + if (ptid_equal (inferior_ptid, null_ptid))
> + error (_("No inferior running"));
> +
> + /* Fast path. */
> + if (!dtrace_probe_is_enabled (dtrace_probe))
> + return;
> +
> + /* Are we trying to disable a probe that does not have any enabler
> + associated? */
> + if (VEC_empty (dtrace_probe_enabler_s, dtrace_probe->enablers))
> + error (_("Probe %s:%s cannot be disabled."), probe->provider, probe->name);
> +
> + /* Iterate over all defined enabler in the given probe and disable
> + them all using the corresponding gdbarch hook. */
> +
> + for (i = 0;
> + VEC_iterate (dtrace_probe_enabler_s, dtrace_probe->enablers, i, enabler);
> + i++)
> + {
> + if (gdbarch_dtrace_disable_probe_p (gdbarch))
> + gdbarch_dtrace_disable_probe (gdbarch, enabler->address);
> + }
> +}
> +
> +/* DTrace probe_ops. */
> +
> +static const struct probe_ops dtrace_probe_ops =
> +{
> + dtrace_probe_is_linespec,
> + dtrace_get_probes,
> + dtrace_get_probe_address,
> + dtrace_get_probe_argument_count,
> + dtrace_can_evaluate_probe_arguments,
> + dtrace_evaluate_probe_argument,
> + dtrace_compile_to_ax,
> + dtrace_set_semaphore,
> + dtrace_clear_semaphore,
> + dtrace_probe_destroy,
> + dtrace_gen_info_probes_table_header,
> + dtrace_gen_info_probes_table_values,
> + dtrace_enable_probe,
> + dtrace_disable_probe
> +};
> +
> +/* Implementation of the `info probes dtrace' command. */
> +
> +static void
> +info_probes_dtrace_command (char *arg, int from_tty)
> +{
> + info_probes_for_ops (arg, from_tty, &dtrace_probe_ops);
> +}
> +
> +void _initialize_dtrace_probe (void);
> +
> +void
> +_initialize_dtrace_probe (void)
> +{
> + VEC_safe_push (probe_ops_cp, all_probe_ops, &dtrace_probe_ops);
> +
Extraneous whitespaces.
> + add_cmd ("dtrace", class_info, info_probes_dtrace_command,
> + _("\
> +Show information about DTrace static probes.\n\
> +Usage: info probes dtrace [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."),
> + info_probes_cmdlist_get ());
> +}
> --
> 1.7.10.4
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 5/9] New probe type: DTrace USDT probes.
2014-10-02 23:19 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
2014-10-10 18:13 ` Sergio Durigan Junior
0 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
Hi Sergio.
> +/* The type of the ELF sections where we will find the DOF programs
> + with information about probes. */
> +
> +#ifndef SHT_SUNW_dof
> +# define SHT_SUNW_dof 0x6ffffff4
> +#endif
Can this macro exist in another header file that you are including?
That macro is defined in elf.h in Solaris, Minix, and probably other
systems too. I would not be surprised if it is eventually added to the
elf headers in GNU/Linux, and also in binutils. I strongly recommend to
keep that sentinel in place to avoid potential problems with indirect
includes in the future.
> +
> + /* Number of arguments in the probe. */
> + ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
> +
> + /* Store argument type descriptions. A description of the type
> + of the argument is in the (J+1)th null-terminated string
> + starting at `strtab' + `probe->dofpr_nargv'. */
We're not using `' anymore; instead, we're using '' (GNU Coding Style
has been updated).
A hard-to-die habit after so many years... :)
> + ret->args = NULL;
> + p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
> + for (j = 0; j < ret->probe_argc; j++)
> + {
> + struct dtrace_probe_arg arg;
> + struct expression *expr;
> +
> + arg.type_str = xstrdup (p);
> + while (((p - strtab) < strtab_size) /* sentinel. */
> + && *p++);
Again a matter of style, but for readability I prefer to write this loop
as:
/* Use strtab_size as a sentinel. */
while (*p != '\0' && p - strtab < strtab_size)
++p;
What you are suggesting is not exactly equivalent: it leaves `p' at the
blank character, while the idea is to leave `p' at the character next ot
the blank character. I changed the loop to:
/* Use strtab_size as a sentinel. */
while (*p++ != '\0' && p - strtab < strtab_size);
Which makes the comparison explicit and thus may be more palatable for
you :)
> + VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
> + }
> +
> + /* Add the vector of enablers to this probe, if any. */
> + ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
You should free the enablers VEC in the end of the function. You could
probably make a cleanup and call it later.
Hmm, I don't see the need of doing a deep copy of the vector, nor I
remember why I felt it was necessary to do it when I wrote the original
code.
I changed that to:
/* Add the vector of enablers to this probe, if any. */
ret->enablers = enablers;
But maybe(probably) I am missing something? :?
> +/* Implementation of the get_probes method. */
> +
> +static void
> +dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile)
> +{
> + bfd *abfd = objfile->obfd;
> + asection *sect = NULL;
> +
> + /* Do nothing in case this is a .debug file, instead of the objfile
> + itself. */
> + if (objfile->separate_debug_objfile_backlink != NULL)
> + return;
> +
> + /* Iterate over the sections in OBJFILE looking for DTrace
> + information. */
> + for (sect = abfd->sections; sect != NULL; sect = sect->next)
> + {
> + if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
> + {
> + struct dtrace_dof_hdr *dof;
> +
> + /* Read the contents of the DOF section and then process it to
> + extract the information of any probe defined into it. */
> + if (!bfd_malloc_and_get_section (abfd, sect, (bfd_byte **) &dof))
> + {
> + complaint (&symfile_complaints,
> + _("could not obtain the contents of"
> + "section '%s' in objfile `%s'."),
> + sect->name, abfd->filename);
> + return;
Why return here? Is there only one section whose type is SHT_SUNW_dof?
If no, then I guess the loop should keep rolling. Otherwise, then
besides calling return here you should call return after the "xfree"
below. Am I getting it right?
Yeah, in principle there can be more than one sections of type
SHT_SUNW_dof. I changed the code as suggested.
> + }
> +
> + dtrace_process_dof (sect, objfile, probesp, dof);
> + xfree (dof);
> + }
> + }
What about using bfd_map_over_sections instead of this for loop? I know
there is precedence of iterating over BFD sections by hand on GDB code,
but bfd_map_over_sections exists for this very purpose.
I considered that, but the need to define a new structure type for
passing `objfile' and `probesp' to the handler (not to mention the
handler itself) makes it a bit overkill to use bfd_map_over_sections in
this specific case IMO... especially considering that
dtrace_process_dof is only called by this function.
> +/* Implementation of the clear_semaphore method. */
> +
> +static void
> +dtrace_clear_semaphore (struct probe *probe_generic, struct objfile *objfile,
> + struct gdbarch *gdbarch)
> +{
> + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> +}
This shouldn't be needed, because USDT probes don't have the concept of
a semaphore, right? I will submit a patch soon to fix the fact that the
set/clear_semaphore functions are being called inconditionally.
Correct, that should not be needed and can go away as soon as you do
that change.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 5/9] New probe type: DTrace USDT probes.
2014-10-10 16:38 ` Jose E. Marchesi
@ 2014-10-10 18:13 ` Sergio Durigan Junior
2014-10-10 18:32 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-10 18:13 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, October 10 2014, Jose E. Marchesi wrote:
> > +/* The type of the ELF sections where we will find the DOF programs
> > + with information about probes. */
> > +
> > +#ifndef SHT_SUNW_dof
> > +# define SHT_SUNW_dof 0x6ffffff4
> > +#endif
>
> Can this macro exist in another header file that you are including?
>
> That macro is defined in elf.h in Solaris, Minix, and probably other
> systems too. I would not be surprised if it is eventually added to the
> elf headers in GNU/Linux, and also in binutils. I strongly recommend to
> keep that sentinel in place to avoid potential problems with indirect
> includes in the future.
Sure thing :-). I was mostly curious.
> > +
> > + /* Number of arguments in the probe. */
> > + ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
> > +
> > + /* Store argument type descriptions. A description of the type
> > + of the argument is in the (J+1)th null-terminated string
> > + starting at `strtab' + `probe->dofpr_nargv'. */
>
> We're not using `' anymore; instead, we're using '' (GNU Coding Style
> has been updated).
>
> A hard-to-die habit after so many years... :)
Haha, yeah, I can imagine :-).
> > + ret->args = NULL;
> > + p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
> > + for (j = 0; j < ret->probe_argc; j++)
> > + {
> > + struct dtrace_probe_arg arg;
> > + struct expression *expr;
> > +
> > + arg.type_str = xstrdup (p);
> > + while (((p - strtab) < strtab_size) /* sentinel. */
> > + && *p++);
>
> Again a matter of style, but for readability I prefer to write this loop
> as:
>
> /* Use strtab_size as a sentinel. */
> while (*p != '\0' && p - strtab < strtab_size)
> ++p;
>
> What you are suggesting is not exactly equivalent: it leaves `p' at the
> blank character, while the idea is to leave `p' at the character next ot
> the blank character. I changed the loop to:
>
> /* Use strtab_size as a sentinel. */
> while (*p++ != '\0' && p - strtab < strtab_size);
>
> Which makes the comparison explicit and thus may be more palatable for
> you :)
Ops, indeed, thanks for catching this :-). No wonder I proposed to make
the loop clearer :-P.
> > + VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
> > + }
> > +
> > + /* Add the vector of enablers to this probe, if any. */
> > + ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
>
> You should free the enablers VEC in the end of the function. You could
> probably make a cleanup and call it later.
>
> Hmm, I don't see the need of doing a deep copy of the vector, nor I
> remember why I felt it was necessary to do it when I wrote the original
> code.
>
> I changed that to:
>
> /* Add the vector of enablers to this probe, if any. */
> ret->enablers = enablers;
>
> But maybe(probably) I am missing something? :?
Hm, right. But if you do that, you will have to adjust
dtrace_probe_destroy, because it will be freeing the same 'enablers'
over and over...
> > + }
> > +
> > + dtrace_process_dof (sect, objfile, probesp, dof);
> > + xfree (dof);
> > + }
> > + }
>
> What about using bfd_map_over_sections instead of this for loop? I know
> there is precedence of iterating over BFD sections by hand on GDB code,
> but bfd_map_over_sections exists for this very purpose.
>
> I considered that, but the need to define a new structure type for
> passing `objfile' and `probesp' to the handler (not to mention the
> handler itself) makes it a bit overkill to use bfd_map_over_sections in
> this specific case IMO... especially considering that
> dtrace_process_dof is only called by this function.
OK, fair enough.
> > +/* Implementation of the clear_semaphore method. */
> > +
> > +static void
> > +dtrace_clear_semaphore (struct probe *probe_generic, struct objfile *objfile,
> > + struct gdbarch *gdbarch)
> > +{
> > + gdb_assert (probe_generic->pops == &dtrace_probe_ops);
> > +}
>
> This shouldn't be needed, because USDT probes don't have the concept of
> a semaphore, right? I will submit a patch soon to fix the fact that the
> set/clear_semaphore functions are being called inconditionally.
>
> Correct, that should not be needed and can go away as soon as you do
> that change.
I should be able to post something today. Will put you on the loop.
Cheers,
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 5/9] New probe type: DTrace USDT probes.
2014-10-10 18:13 ` Sergio Durigan Junior
@ 2014-10-10 18:32 ` Jose E. Marchesi
2014-10-10 18:44 ` Sergio Durigan Junior
0 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 18:32 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
> You should free the enablers VEC in the end of the function. You could
> probably make a cleanup and call it later.
>
> Hmm, I don't see the need of doing a deep copy of the vector, nor I
> remember why I felt it was necessary to do it when I wrote the original
> code.
>
> I changed that to:
>
> /* Add the vector of enablers to this probe, if any. */
> ret->enablers = enablers;
>
> But maybe(probably) I am missing something? :?
Hm, right. But if you do that, you will have to adjust
dtrace_probe_destroy, because it will be freeing the same 'enablers'
over and over...
Aaah, that was indeed the reason! A "DOF probe" translates into 0 or
more gdb probes, and they all share the same vector of enablers.
Maintaining a copy of the enablers per gdb probe makes it trivial to
manage its memory. Otherwise we would need to keep track of which
enabler vectors are shared by which gdb probes... argh!
I will roll back to the deep copy approach :)
> This shouldn't be needed, because USDT probes don't have the concept of
> a semaphore, right? I will submit a patch soon to fix the fact that the
> set/clear_semaphore functions are being called inconditionally.
>
> Correct, that should not be needed and can go away as soon as you do
> that change.
I should be able to post something today. Will put you on the loop.
Ah nice, I will update the patches accordingly.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 5/9] New probe type: DTrace USDT probes.
2014-10-10 18:32 ` Jose E. Marchesi
@ 2014-10-10 18:44 ` Sergio Durigan Junior
0 siblings, 0 replies; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-10 18:44 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, October 10 2014, Jose E. Marchesi wrote:
> > You should free the enablers VEC in the end of the function. You could
> > probably make a cleanup and call it later.
> >
> > Hmm, I don't see the need of doing a deep copy of the vector, nor I
> > remember why I felt it was necessary to do it when I wrote the original
> > code.
> >
> > I changed that to:
> >
> > /* Add the vector of enablers to this probe, if any. */
> > ret->enablers = enablers;
> >
> > But maybe(probably) I am missing something? :?
>
> Hm, right. But if you do that, you will have to adjust
> dtrace_probe_destroy, because it will be freeing the same 'enablers'
> over and over...
>
> Aaah, that was indeed the reason! A "DOF probe" translates into 0 or
> more gdb probes, and they all share the same vector of enablers.
>
> Maintaining a copy of the enablers per gdb probe makes it trivial to
> manage its memory. Otherwise we would need to keep track of which
> enabler vectors are shared by which gdb probes... argh!
>
> I will roll back to the deep copy approach :)
:-)
And then you need that cleanup I mentioned before :-P.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/9] New commands `enable probe' and `disable probe'.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
2014-09-26 9:43 ` [PATCH 8/9] Documentation for DTrace USDT probes Jose E. Marchesi
2014-09-26 9:43 ` [PATCH 5/9] New probe type: " Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-09-26 13:11 ` Eli Zaretskii
2014-09-30 23:13 ` Sergio Durigan Junior
2014-09-26 9:43 ` [PATCH 6/9] Support for DTrace USDT probes in x86_64 targets Jose E. Marchesi
` (6 subsequent siblings)
9 siblings, 2 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
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.
gdb:
2014-09-26 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'.
* probe.h (probe_ops): New hooks `enable_probe' and
`disable_probe'.
gdb/doc:
2014-09-26 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 | 30 +++++++++++++
gdb/probe.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/probe.h | 12 +++++
gdb/stap-probe.c | 2 +
6 files changed, 185 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5ec0d85..2f94e05 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
2014-09-26 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'.
+
+ * probe.h (probe_ops): New hooks `enable_probe' and
+ `disable_probe'.
+
+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.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 9a4bc09..5667bf1 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+
+ * gdb.texinfo (Static Probe Points): Cover the `enable probe' and
+ `disable probe' commands.
+
2014-09-22 Jan-Benedict Glaw <jbglaw@lug-owl.de>
* gdb.texinfo (Set Breaks): Add missing "@end table".
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 026706a..4d6dfbc 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4976,6 +4976,36 @@ 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 effects
+associated to enabling or disabling a probe depend on the type of
+probe being handled. @code{SystemTap} probes do not support these
+notions.
+
+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,
+probes by 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 d218f00..1e5c460 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -685,6 +685,106 @@ 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;
+
+ /* Do we have a `provider:probe:objfile' style of linespec? */
+ provider = extract_arg (&arg);
+ if (provider)
+ {
+ make_cleanup (xfree, provider);
+
+ probe_name = extract_arg (&arg);
+ if (probe_name)
+ {
+ make_cleanup (xfree, probe_name);
+
+ objname = extract_arg (&arg);
+ if (objname)
+ 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"));
+ 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)
+ {
+ pops->enable_probe (probe->probe);
+ ui_out_message (current_uiout, 0,
+ _("Probe %s:%s enabled.\n"),
+ probe->probe->provider, probe->probe->name);
+ }
+ }
+}
+
+/* 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;
+
+ /* Do we have a `provider:probe:objfile' style of linespec? */
+ provider = extract_arg (&arg);
+ if (provider)
+ {
+ make_cleanup (xfree, provider);
+
+ probe_name = extract_arg (&arg);
+ if (probe_name)
+ {
+ make_cleanup (xfree, probe_name);
+
+ objname = extract_arg (&arg);
+ if (objname)
+ 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"));
+ 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)
+ {
+ pops->disable_probe (probe->probe);
+ ui_out_message (current_uiout, 0,
+ _("Probe %s:%s disabled.\n"),
+ probe->probe->provider, probe->probe->name);
+ }
+ }
+}
+
/* See comments in probe.h. */
CORE_ADDR
@@ -946,4 +1046,27 @@ _initialize_probe (void)
_("\
Show information about all type of probes."),
info_probes_cmdlist_get ());
+
+ add_cmd ("probes", no_class, 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 disable\n\
+all defined probes."),
+ &enablelist);
+
+ add_cmd ("probes", no_class, 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 b4ff0a6..3f27d0d 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -132,6 +132,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 17a73fc..c5d0dbb 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1675,6 +1675,8 @@ static const struct probe_ops stap_probe_ops =
stap_probe_destroy,
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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/9] New commands `enable probe' and `disable probe'.
2014-09-26 9:43 ` [PATCH 3/9] New commands `enable probe' and `disable probe' Jose E. Marchesi
@ 2014-09-26 13:11 ` Eli Zaretskii
2014-09-29 10:26 ` Jose E. Marchesi
2014-09-30 23:13 ` Sergio Durigan Junior
1 sibling, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2014-09-26 13:11 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
> From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
> Date: Fri, 26 Sep 2014 11:48:19 +0200
>
> 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'.
Thanks.
> +@kindex enable probes
> +@item enable probes @r{[}@var{provider} @r{[}@var{name} @r{[}@var{objfile}@r{]}@r{]}@r{]}
Is it correct that NAME can be given only if PROVIDER is given, and
OBJFILE can be given only if both NAME and PROVIDER were given?
That's what the nesting of [..] suggests.
Otherwise, the part for the manual is OK.
> + add_cmd ("probes", no_class, enable_probes_command, _("\
> +Enable probes.\n\
> +Usage: enable probes [PROVIDER [NAME [OBJECT]]]\n\
Same comment as for the manual regarding the nesting of [..].
> +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."), ^^^^^^^^
A copy/paste typo.
Thanks.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/9] New commands `enable probe' and `disable probe'.
2014-09-26 13:11 ` Eli Zaretskii
@ 2014-09-29 10:26 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-29 10:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
Hi Eli, thanks for your review.
> +@kindex enable probes
> +@item enable probes @r{[}@var{provider} @r{[}@var{name} @r{[}@var{objfile}@r{]}@r{]}@r{]}
Is it correct that NAME can be given only if PROVIDER is given, and
OBJFILE can be given only if both NAME and PROVIDER were given?
That's what the nesting of [..] suggests.
Yes, that is correct. The same syntax is used in the `info probes'
command, so I just kept it.
> +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."), ^^^^^^^^
A copy/paste typo.
Fixed, thanks.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/9] New commands `enable probe' and `disable probe'.
2014-09-26 9:43 ` [PATCH 3/9] New commands `enable probe' and `disable probe' Jose E. Marchesi
2014-09-26 13:11 ` Eli Zaretskii
@ 2014-09-30 23:13 ` Sergio Durigan Junior
2014-09-30 23:20 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
1 sibling, 2 replies; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-09-30 23:13 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 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, Jose. Comments below, as usual.
> gdb:
>
> 2014-09-26 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'.
>
> * probe.h (probe_ops): New hooks `enable_probe' and
> `disable_probe'.
Same comment about blank lines in the ChangeLog entry applies here.
> gdb/doc:
>
> 2014-09-26 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 | 30 +++++++++++++
> gdb/probe.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++
> gdb/probe.h | 12 +++++
> gdb/stap-probe.c | 2 +
> 6 files changed, 185 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 5ec0d85..2f94e05 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,18 @@
> 2014-09-26 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'.
> +
> + * probe.h (probe_ops): New hooks `enable_probe' and
> + `disable_probe'.
> +
> +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.
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 9a4bc09..5667bf1 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,8 @@
> +2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
> +
> + * gdb.texinfo (Static Probe Points): Cover the `enable probe' and
> + `disable probe' commands.
> +
> 2014-09-22 Jan-Benedict Glaw <jbglaw@lug-owl.de>
>
> * gdb.texinfo (Set Breaks): Add missing "@end table".
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 026706a..4d6dfbc 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4976,6 +4976,36 @@ 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 effects
> +associated to enabling or disabling a probe depend on the type of
> +probe being handled. @code{SystemTap} probes do not support these
> +notions.
> +
> +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,
> +probes by 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 d218f00..1e5c460 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -685,6 +685,106 @@ 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;
> +
> + /* Do we have a `provider:probe:objfile' style of linespec? */
> + provider = extract_arg (&arg);
> + if (provider)
> + {
> + make_cleanup (xfree, provider);
> +
> + probe_name = extract_arg (&arg);
> + if (probe_name)
> + {
> + make_cleanup (xfree, probe_name);
> +
> + objname = extract_arg (&arg);
> + if (objname)
> + make_cleanup (xfree, objname);
> + }
> + }
This construct has become useful enough that it can be put in a
function, probably. Would you mind doing that?
> + probes = collect_probes (objname, provider, probe_name, NULL /* pops */);
When I code, I try not to put comments inside the parentheses. You can
put a comment above the function, or just remove it at all (there is
another call to collect_probes that doesn't have a comment).
> + if (VEC_empty (bound_probe_s, probes))
> + {
> + ui_out_message (current_uiout, 0, _("No probes matched.\n"));
> + return;
> + }
You should call do_cleanups before returning here (and in other places).
> + /* 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)
Newline between variable declaration and code. You are also comparing
things implicitly, and we do it explicitly:
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);
> + }
> + }
I would appreciate a warning here if pops->enable_probe == NULL, saying
that the probe cannot be enabled/disabled.
> +}
> +
> +/* 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;
> +
> + /* Do we have a `provider:probe:objfile' style of linespec? */
> + provider = extract_arg (&arg);
> + if (provider)
> + {
> + make_cleanup (xfree, provider);
> +
> + probe_name = extract_arg (&arg);
> + if (probe_name)
> + {
> + make_cleanup (xfree, probe_name);
> +
> + objname = extract_arg (&arg);
> + if (objname)
> + 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"));
> + 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)
> + {
> + pops->disable_probe (probe->probe);
> + ui_out_message (current_uiout, 0,
> + _("Probe %s:%s disabled.\n"),
> + probe->probe->provider, probe->probe->name);
> + }
> + }
> +}
Same comments for enable_probes_command apply here as well.
> +
> /* See comments in probe.h. */
>
> CORE_ADDR
> @@ -946,4 +1046,27 @@ _initialize_probe (void)
> _("\
> Show information about all type of probes."),
> info_probes_cmdlist_get ());
> +
> + add_cmd ("probes", no_class, 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 disable\n\
> +all defined probes."),
> + &enablelist);
> +
> + add_cmd ("probes", no_class, 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);
Those commands work on probes, which are special kinds of breakpoints,
so I would specify class_breakpoint instead of no_class.
> +
> }
> diff --git a/gdb/probe.h b/gdb/probe.h
> index b4ff0a6..3f27d0d 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -132,6 +132,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 17a73fc..c5d0dbb 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -1675,6 +1675,8 @@ static const struct probe_ops stap_probe_ops =
> stap_probe_destroy,
> 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
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/9] New commands `enable probe' and `disable probe'.
2014-09-30 23:13 ` Sergio Durigan Junior
@ 2014-09-30 23:20 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
1 sibling, 0 replies; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-09-30 23:20 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Tuesday, September 30 2014, I wrote:
>> +/* 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;
>> +
>> + /* Do we have a `provider:probe:objfile' style of linespec? */
>> + provider = extract_arg (&arg);
>> + if (provider)
>> + {
>> + make_cleanup (xfree, provider);
>> +
>> + probe_name = extract_arg (&arg);
>> + if (probe_name)
>> + {
>> + make_cleanup (xfree, probe_name);
>> +
>> + objname = extract_arg (&arg);
>> + if (objname)
>> + make_cleanup (xfree, objname);
>> + }
>> + }
>
> This construct has become useful enough that it can be put in a
> function, probably. Would you mind doing that?
>
>> + probes = collect_probes (objname, provider, probe_name, NULL /* pops */);
>
> When I code, I try not to put comments inside the parentheses. You can
> put a comment above the function, or just remove it at all (there is
> another call to collect_probes that doesn't have a comment).
>
>> + if (VEC_empty (bound_probe_s, probes))
>> + {
>> + ui_out_message (current_uiout, 0, _("No probes matched.\n"));
>> + return;
>> + }
>
> You should call do_cleanups before returning here (and in other places).
>
>> + /* 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)
>
> Newline between variable declaration and code. You are also comparing
> things implicitly, and we do it explicitly:
>
> 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);
>> + }
>> + }
>
> I would appreciate a warning here if pops->enable_probe == NULL, saying
> that the probe cannot be enabled/disabled.
BTW, I forgot to say, but you should call do_cleanups before the
function ends as well (the same applies for disable_probes_command).
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/9] New commands `enable probe' and `disable probe'.
2014-09-30 23:13 ` Sergio Durigan Junior
2014-09-30 23:20 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
1 sibling, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
Hi Sergio.
> 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, Jose. Comments below, as usual.
I agree/comply with all these comments :)
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 6/9] Support for DTrace USDT probes in x86_64 targets.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (2 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 3/9] New commands `enable probe' and `disable probe' Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-10-08 19:32 ` Sergio Durigan Junior
2014-09-26 9:43 ` [PATCH 9/9] Announce the DTrace USDT probes support in NEWS Jose E. Marchesi
` (5 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
This patch adds the target-specific code in order to support the
calculation of DTrace probes arguments in x86_64 targets, and also the
enabling and disabling of probes. This is done by implementing the
`dtrace_*' gdbarch handlers.
gdb:
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
* amd64-linux-tdep.h: Prototypes for
`amd64_dtrace_probe_argument', `amd64_dtrace_enable_probe',
`amd64_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
* amd64-linux-tdep.c (amd64_dtrace_probe_argument): New function.
(amd64_dtrace_probe_is_enabled): Likewise.
(amd64_dtrace_enable_probe): Likewise.
(amd64_dtrace_disable_probe): Likewise.
(amd64_linux_init_abi): Register the
`gdbarch_dtrace_probe_argument', `gdbarch_dtrace_enable_probe',
`gdbarch_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
---
gdb/ChangeLog | 14 +++++
gdb/amd64-linux-tdep.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
gdb/amd64-linux-tdep.h | 11 ++++
3 files changed, 175 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eac03e7..a32d303 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+ * amd64-linux-tdep.h: Prototypes for
+ `amd64_dtrace_probe_argument', `amd64_dtrace_enable_probe',
+ `amd64_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
+
+ * amd64-linux-tdep.c (amd64_dtrace_probe_argument): New function.
+ (amd64_dtrace_probe_is_enabled): Likewise.
+ (amd64_dtrace_enable_probe): Likewise.
+ (amd64_dtrace_disable_probe): Likewise.
+ (amd64_linux_init_abi): Register the
+ `gdbarch_dtrace_probe_argument', `gdbarch_dtrace_enable_probe',
+ `gdbarch_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
+
+2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+
* breakpoint.c (BREAK_ARGS_HELP): help string updated to mention
the -probe-dtrace new vpossible value for PROBE_MODIFIER.
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 850ca20..273f5c4 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -28,6 +28,8 @@
#include "gdbtypes.h"
#include "reggroups.h"
#include "regset.h"
+#include "parser-defs.h"
+#include "user-regs.h"
#include "amd64-linux-tdep.h"
#include "i386-linux-tdep.h"
#include "linux-tdep.h"
@@ -1609,6 +1611,148 @@ amd64_linux_core_read_description (struct gdbarch *gdbarch,
}
}
+/* Implementation of `gdbarch_dtrace_probe_is_enabled', as defined in
+ gdbarch.h. */
+
+int
+amd64_dtrace_probe_is_enabled (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+ /* The instruction sequence used in x86_64 machines for a disabled
+ is-enabled probe is:
+
+ xor %rax, %rax => 48 33 C0
+ ADDR: nop => 90
+ nop => 90
+
+ or
+
+ xor %rax, %rax => 48 33 C0
+ ADDR: ret => c3
+ nop => 90
+
+ This function returns 1 if the instructions at ADDR do _not_
+ follow any of these patterns.
+
+ Note that ADDR is offset 3 bytes from the beginning of these
+ sequences. */
+
+ gdb_byte buf[5];
+ read_memory (addr - 3, buf, 5);
+
+ return !((buf[0] == 0x48) && (buf[1] == 0x33) && (buf[2] == 0xc0) /* xor */
+ && ((buf[3] == 0x90) || (buf[3] == 0xc3)) /* nop | ret */
+ && (buf[4] == 0x90)); /* nop */
+}
+
+/* Implementation of `gdbarch_dtrace_enable_probe', as defined in
+ gdbarch.h. */
+
+void
+amd64_dtrace_enable_probe (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+ /* We use the following instruction sequence for enabling an
+ is-enabled probe:
+
+ mov $0x1, %eax => b8 01 00 00 00
+
+ Note also that ADDR is offset 3 bytes from the beginning of the
+ sequence. */
+
+ gdb_byte buf[5];
+
+ buf[0] = 0xb8; buf[1] = 0x01; buf[2] = 0x00; buf[3] = 0x00; buf[4] = 0x00;
+ write_memory (addr - 3, buf, 5);
+}
+
+/* Implementation of `gdbarch_dtrace_disable_probe', as defined in
+ gdbarch.h. */
+
+void
+amd64_dtrace_disable_probe (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+ /* We use the following instruction sequence for disabling an
+ is-enabled probe:
+
+ xor %rax, %rax; nop; nop => 48 33 C0 90 90
+
+ Note that ADDR is offset 3 bytes from the beginning of the
+ sequence. */
+
+ gdb_byte buf[5];
+
+ buf[0] = 0x48; buf[1] = 0x33; buf[2] = 0xc0; buf[3] = 0x90; buf[4] = 0x90;
+ write_memory (addr - 3, buf, 5);
+}
+
+/* Implementation of `gdbarch_dtrace_parse_special_token', as defined in
+ gdbarch.h. */
+
+void
+amd64_dtrace_probe_argument (struct gdbarch *gdbarch,
+ struct parser_state *pstate,
+ int narg)
+{
+ static int arg_reg_map[6] =
+ {
+ AMD64_RDI_REGNUM, /* Arg 1. */
+ AMD64_RSI_REGNUM, /* Arg 2. */
+ AMD64_RDX_REGNUM, /* Arg 3. */
+ AMD64_RCX_REGNUM, /* Arg 4. */
+ AMD64_R8_REGNUM, /* Arg 5. */
+ AMD64_R9_REGNUM /* Arg 6. */
+ };
+
+ enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+ struct frame_info *this_frame = get_selected_frame (NULL);
+ struct stoken str;
+
+ /* DTrace probe arguments can be found on the ABI-defined places for
+ regular arguments at the current PC. The probe abstraction
+ currently supports up to 12 arguments for probes. */
+
+ if (narg < 6)
+ {
+ int regno = arg_reg_map [narg];
+ const char *regname = user_reg_map_regnum_to_name (gdbarch, regno);
+
+ write_exp_elt_opcode (pstate, OP_REGISTER);
+ str.ptr = regname;
+ str.length = strlen (regname);
+ write_exp_string (pstate, str);
+ write_exp_elt_opcode (pstate, OP_REGISTER);
+ }
+ else
+ {
+ /* Additional arguments are passed on the stack. */
+
+ CORE_ADDR sp;
+ const char *regname = user_reg_map_regnum_to_name (gdbarch, AMD64_RSP_REGNUM);
+
+ /* Displacement. */
+ write_exp_elt_opcode (pstate, OP_LONG);
+ write_exp_elt_type (pstate, builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_longcst (pstate, narg - 6);
+ write_exp_elt_opcode (pstate, OP_LONG);
+
+ /* Register: SP. */
+ write_exp_elt_opcode (pstate, OP_REGISTER);
+ str.ptr = regname;
+ str.length = strlen (regname);
+ write_exp_string (pstate, str);
+ write_exp_elt_opcode (pstate, OP_REGISTER);
+
+ write_exp_elt_opcode (pstate, BINOP_ADD);
+
+ /* Cast to long. */
+ write_exp_elt_opcode (pstate, UNOP_CAST);
+ write_exp_elt_type (pstate,
+ lookup_pointer_type (builtin_type (gdbarch)->builtin_long));
+ write_exp_elt_opcode (pstate, UNOP_CAST);
+
+ write_exp_elt_opcode (pstate, UNOP_IND);
+ }
+}
+
static void
amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
{
@@ -1872,6 +2016,12 @@ amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
/* GNU/Linux uses SVR4-style shared libraries. */
set_solib_svr4_fetch_link_map_offsets
(gdbarch, svr4_lp64_fetch_link_map_offsets);
+
+ /* Register DTrace handlers. */
+ set_gdbarch_dtrace_probe_argument (gdbarch, amd64_dtrace_probe_argument);
+ set_gdbarch_dtrace_probe_is_enabled (gdbarch, amd64_dtrace_probe_is_enabled);
+ set_gdbarch_dtrace_enable_probe (gdbarch, amd64_dtrace_enable_probe);
+ set_gdbarch_dtrace_disable_probe (gdbarch, amd64_dtrace_disable_probe);
}
static void
diff --git a/gdb/amd64-linux-tdep.h b/gdb/amd64-linux-tdep.h
index 25563b8..b28dc50 100644
--- a/gdb/amd64-linux-tdep.h
+++ b/gdb/amd64-linux-tdep.h
@@ -594,4 +594,15 @@ enum amd64_x32_syscall {
amd64_x32_sys_getsockopt = (amd64_x32_syscall_bit + 542),
};
+/* DTrace related functions. */
+
+extern void amd64_dtrace_probe_argument (struct gdbarch *gdbarch,
+ struct parser_state *pstate,
+ int narg);
+
+extern int amd64_dtrace_probe_is_enabled (struct gdbarch *gdbarch, CORE_ADDR addr);
+
+extern void amd64_dtrace_enable_probe (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void amd64_dtrace_disable_probe (struct gdbarch *gdbarch, CORE_ADDR addr);
+
#endif /* amd64-linux-tdep.h */
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 6/9] Support for DTrace USDT probes in x86_64 targets.
2014-09-26 9:43 ` [PATCH 6/9] Support for DTrace USDT probes in x86_64 targets Jose E. Marchesi
@ 2014-10-08 19:32 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-08 19:32 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 2014, Jose E. Marchesi wrote:
> This patch adds the target-specific code in order to support the
> calculation of DTrace probes arguments in x86_64 targets, and also the
> enabling and disabling of probes. This is done by implementing the
> `dtrace_*' gdbarch handlers.
Thanks. Comments below.
> gdb:
>
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * amd64-linux-tdep.h: Prototypes for
> `amd64_dtrace_probe_argument', `amd64_dtrace_enable_probe',
> `amd64_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
>
> * amd64-linux-tdep.c (amd64_dtrace_probe_argument): New function.
> (amd64_dtrace_probe_is_enabled): Likewise.
> (amd64_dtrace_enable_probe): Likewise.
> (amd64_dtrace_disable_probe): Likewise.
> (amd64_linux_init_abi): Register the
> `gdbarch_dtrace_probe_argument', `gdbarch_dtrace_enable_probe',
> `gdbarch_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
> ---
> gdb/ChangeLog | 14 +++++
> gdb/amd64-linux-tdep.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
> gdb/amd64-linux-tdep.h | 11 ++++
> 3 files changed, 175 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index eac03e7..a32d303 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,19 @@
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> + * amd64-linux-tdep.h: Prototypes for
> + `amd64_dtrace_probe_argument', `amd64_dtrace_enable_probe',
> + `amd64_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
> +
> + * amd64-linux-tdep.c (amd64_dtrace_probe_argument): New function.
> + (amd64_dtrace_probe_is_enabled): Likewise.
> + (amd64_dtrace_enable_probe): Likewise.
> + (amd64_dtrace_disable_probe): Likewise.
> + (amd64_linux_init_abi): Register the
> + `gdbarch_dtrace_probe_argument', `gdbarch_dtrace_enable_probe',
> + `gdbarch_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
> +
> +2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
> +
> * breakpoint.c (BREAK_ARGS_HELP): help string updated to mention
> the -probe-dtrace new vpossible value for PROBE_MODIFIER.
>
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index 850ca20..273f5c4 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -28,6 +28,8 @@
> #include "gdbtypes.h"
> #include "reggroups.h"
> #include "regset.h"
> +#include "parser-defs.h"
> +#include "user-regs.h"
> #include "amd64-linux-tdep.h"
> #include "i386-linux-tdep.h"
> #include "linux-tdep.h"
> @@ -1609,6 +1611,148 @@ amd64_linux_core_read_description (struct gdbarch *gdbarch,
> }
> }
>
> +/* Implementation of `gdbarch_dtrace_probe_is_enabled', as defined in
> + gdbarch.h. */
> +
> +int
> +amd64_dtrace_probe_is_enabled (struct gdbarch *gdbarch, CORE_ADDR addr)
> +{
> + /* The instruction sequence used in x86_64 machines for a disabled
> + is-enabled probe is:
> +
> + xor %rax, %rax => 48 33 C0
> + ADDR: nop => 90
> + nop => 90
> +
> + or
> +
> + xor %rax, %rax => 48 33 C0
> + ADDR: ret => c3
> + nop => 90
> +
> + This function returns 1 if the instructions at ADDR do _not_
> + follow any of these patterns.
> +
> + Note that ADDR is offset 3 bytes from the beginning of these
> + sequences. */
This comment could be placed on the top of the function (after the
"Implementation of ..."), or after the declaration of the variables.
Sorry for the nitpick, but I find it strange to make the comment before
declaring the variables in the body of the function (and I don't
remember seeing this on GDB very frequently).
> + gdb_byte buf[5];
> + read_memory (addr - 3, buf, 5);
> +
> + return !((buf[0] == 0x48) && (buf[1] == 0x33) && (buf[2] == 0xc0) /* xor */
> + && ((buf[3] == 0x90) || (buf[3] == 0xc3)) /* nop | ret */
> + && (buf[4] == 0x90)); /* nop */
> +}
> +
> +/* Implementation of `gdbarch_dtrace_enable_probe', as defined in
> + gdbarch.h. */
> +
> +void
> +amd64_dtrace_enable_probe (struct gdbarch *gdbarch, CORE_ADDR addr)
> +{
> + /* We use the following instruction sequence for enabling an
> + is-enabled probe:
> +
> + mov $0x1, %eax => b8 01 00 00 00
> +
> + Note also that ADDR is offset 3 bytes from the beginning of the
> + sequence. */
> +
> + gdb_byte buf[5];
> +
> + buf[0] = 0xb8; buf[1] = 0x01; buf[2] = 0x00; buf[3] = 0x00; buf[4] = 0x00;
Each assignment should go on its own line.
> + write_memory (addr - 3, buf, 5);
> +}
> +
> +/* Implementation of `gdbarch_dtrace_disable_probe', as defined in
> + gdbarch.h. */
> +
> +void
> +amd64_dtrace_disable_probe (struct gdbarch *gdbarch, CORE_ADDR addr)
> +{
> + /* We use the following instruction sequence for disabling an
> + is-enabled probe:
> +
> + xor %rax, %rax; nop; nop => 48 33 C0 90 90
> +
> + Note that ADDR is offset 3 bytes from the beginning of the
> + sequence. */
> +
> + gdb_byte buf[5];
> +
> + buf[0] = 0x48; buf[1] = 0x33; buf[2] = 0xc0; buf[3] = 0x90; buf[4] = 0x90;
Likewise.
> + write_memory (addr - 3, buf, 5);
> +}
> +
> +/* Implementation of `gdbarch_dtrace_parse_special_token', as defined in
> + gdbarch.h. */
This comment needs an update, there is no
"gdbarch_dtrace_parse_special_token" :-).
> +
> +void
> +amd64_dtrace_probe_argument (struct gdbarch *gdbarch,
> + struct parser_state *pstate,
> + int narg)
> +{
> + static int arg_reg_map[6] =
> + {
> + AMD64_RDI_REGNUM, /* Arg 1. */
> + AMD64_RSI_REGNUM, /* Arg 2. */
> + AMD64_RDX_REGNUM, /* Arg 3. */
> + AMD64_RCX_REGNUM, /* Arg 4. */
> + AMD64_R8_REGNUM, /* Arg 5. */
> + AMD64_R9_REGNUM /* Arg 6. */
> + };
> +
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> + struct frame_info *this_frame = get_selected_frame (NULL);
> + struct stoken str;
No newline between variables being declared.
> +
> + /* DTrace probe arguments can be found on the ABI-defined places for
> + regular arguments at the current PC. The probe abstraction
> + currently supports up to 12 arguments for probes. */
> +
> + if (narg < 6)
> + {
> + int regno = arg_reg_map [narg];
No space between variable name and index operator "[".
> + const char *regname = user_reg_map_regnum_to_name (gdbarch, regno);
> +
> + write_exp_elt_opcode (pstate, OP_REGISTER);
> + str.ptr = regname;
> + str.length = strlen (regname);
> + write_exp_string (pstate, str);
> + write_exp_elt_opcode (pstate, OP_REGISTER);
> + }
> + else
> + {
> + /* Additional arguments are passed on the stack. */
> +
> + CORE_ADDR sp;
Spurious newline.
> + const char *regname = user_reg_map_regnum_to_name (gdbarch, AMD64_RSP_REGNUM);
> +
> + /* Displacement. */
> + write_exp_elt_opcode (pstate, OP_LONG);
> + write_exp_elt_type (pstate, builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_longcst (pstate, narg - 6);
> + write_exp_elt_opcode (pstate, OP_LONG);
As I said in another message, I'm not a fan of aligning the open paren
for function calls; it messes with a simple grep, for example.
> +
> + /* Register: SP. */
> + write_exp_elt_opcode (pstate, OP_REGISTER);
> + str.ptr = regname;
> + str.length = strlen (regname);
> + write_exp_string (pstate, str);
> + write_exp_elt_opcode (pstate, OP_REGISTER);
> +
> + write_exp_elt_opcode (pstate, BINOP_ADD);
> +
> + /* Cast to long. */
> + write_exp_elt_opcode (pstate, UNOP_CAST);
> + write_exp_elt_type (pstate,
> + lookup_pointer_type (builtin_type (gdbarch)->builtin_long));
> + write_exp_elt_opcode (pstate, UNOP_CAST);
> +
> + write_exp_elt_opcode (pstate, UNOP_IND);
> + }
> +}
> +
> static void
> amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch)
> {
> @@ -1872,6 +2016,12 @@ amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> /* GNU/Linux uses SVR4-style shared libraries. */
> set_solib_svr4_fetch_link_map_offsets
> (gdbarch, svr4_lp64_fetch_link_map_offsets);
> +
> + /* Register DTrace handlers. */
> + set_gdbarch_dtrace_probe_argument (gdbarch, amd64_dtrace_probe_argument);
> + set_gdbarch_dtrace_probe_is_enabled (gdbarch, amd64_dtrace_probe_is_enabled);
> + set_gdbarch_dtrace_enable_probe (gdbarch, amd64_dtrace_enable_probe);
> + set_gdbarch_dtrace_disable_probe (gdbarch, amd64_dtrace_disable_probe);
> }
>
> static void
> diff --git a/gdb/amd64-linux-tdep.h b/gdb/amd64-linux-tdep.h
> index 25563b8..b28dc50 100644
> --- a/gdb/amd64-linux-tdep.h
> +++ b/gdb/amd64-linux-tdep.h
> @@ -594,4 +594,15 @@ enum amd64_x32_syscall {
> amd64_x32_sys_getsockopt = (amd64_x32_syscall_bit + 542),
> };
>
> +/* DTrace related functions. */
> +
> +extern void amd64_dtrace_probe_argument (struct gdbarch *gdbarch,
> + struct parser_state *pstate,
> + int narg);
> +
> +extern int amd64_dtrace_probe_is_enabled (struct gdbarch *gdbarch, CORE_ADDR addr);
> +
> +extern void amd64_dtrace_enable_probe (struct gdbarch *gdbarch, CORE_ADDR addr);
> +extern void amd64_dtrace_disable_probe (struct gdbarch *gdbarch, CORE_ADDR addr);
> +
> #endif /* amd64-linux-tdep.h */
> --
> 1.7.10.4
Sorry for not being able to do an extensive review, but dtrace is
proprietary and I know very little about its architecture.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 6/9] Support for DTrace USDT probes in x86_64 targets.
2014-10-08 19:32 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
Hi Sergio.
> This patch adds the target-specific code in order to support the
> calculation of DTrace probes arguments in x86_64 targets, and also the
> enabling and disabling of probes. This is done by implementing the
> `dtrace_*' gdbarch handlers.
Thanks. Comments below.
I agree/comply with all these comments :)
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 9/9] Announce the DTrace USDT probes support in NEWS.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (3 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 6/9] Support for DTrace USDT probes in x86_64 targets Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-09-26 13:12 ` Eli Zaretskii
2014-09-26 9:43 ` [PATCH 7/9] Simple testsuite for DTrace USDT probes Jose E. Marchesi
` (4 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
This patch simply adds a small entry to `Changes since GDB 7.8' announcing the
support for dtrace probes.
gdb:
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
* NEWS: Announce the support for DTrace USDT probes.
Conflicts:
gdb/NEWS
---
gdb/ChangeLog | 4 ++++
gdb/NEWS | 3 +++
2 files changed, 7 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a32d303..9c47900 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+ * NEWS: Announce the support for DTrace SDT probes.
+
+2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+
* amd64-linux-tdep.h: Prototypes for
`amd64_dtrace_probe_argument', `amd64_dtrace_enable_probe',
`amd64_dtrace_disable_probe' and `gdbarch_dtrace_probe_is_enabled'.
diff --git a/gdb/NEWS b/gdb/NEWS
index 11326f1..bcc6be1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -40,6 +40,9 @@ queue-signal signal-name-or-number
even in non-stop mode. The "auto" mode has been removed, and "off"
is now the default mode.
+* GDB now has support for DTrace USDT (Userland Static Defined
+ Tracing) probes. The supported targets are x86_64-*-linux-gnu.
+
*** Changes in GDB 7.8
* New command line options
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 7/9] Simple testsuite for DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (4 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 9/9] Announce the DTrace USDT probes support in NEWS Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-10-08 19:30 ` Sergio Durigan Junior
2014-09-26 9:43 ` [PATCH 1/9] Adapt `info probes' to support printing probes of different types Jose E. Marchesi
` (3 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
This patch adds some simple tests testing the support for DTrace USDT
probes. The testsuite will be skipped as unsupported in case the user
does not have DTrace installed on her system. The tests included in
the test suite test breakpointing on DTrace probes, enabling and
disabling probes, printing of probe arguments of several types and
also breakpointing on several probes with the same name.
gdb/testsuite:
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
* gdb.base/dtrace-probe.exp: New file.
(dtrace_build_test_program): New function.
(dtrace_test): Likewise.
* gdb.base/dtrace-probe.d: New file.
(test): New DTrace provider with two probes: `progress-counter'
and `two-locations'.
* gdb.base/dtrace-probe.c: New file.
(main): New function.
---
gdb/testsuite/ChangeLog | 13 +++
gdb/testsuite/gdb.base/dtrace-probe.c | 38 ++++++++
gdb/testsuite/gdb.base/dtrace-probe.d | 21 +++++
gdb/testsuite/gdb.base/dtrace-probe.exp | 156 +++++++++++++++++++++++++++++++
4 files changed, 228 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/dtrace-probe.c
create mode 100644 gdb/testsuite/gdb.base/dtrace-probe.d
create mode 100644 gdb/testsuite/gdb.base/dtrace-probe.exp
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c93d7cf..5e3f472 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,18 @@
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+ * gdb.base/dtrace-probe.exp: New file.
+ (dtrace_build_test_program): New function.
+ (dtrace_test): Likewise.
+
+ * gdb.base/dtrace-probe.d: New file.
+ (test): New DTrace provider with two probes: `progress-counter'
+ and `two-locations'.
+
+ * gdb.base/dtrace-probe.c: New file.
+ (main): New function.
+
+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.
diff --git a/gdb/testsuite/gdb.base/dtrace-probe.c b/gdb/testsuite/gdb.base/dtrace-probe.c
new file mode 100644
index 0000000..45a77c5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dtrace-probe.c
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "dtrace-probe.h"
+
+int
+main ()
+{
+ char *name = "application";
+
+ TEST_TWO_LOCATIONS ();
+
+ int i = 0;
+ while (i < 10)
+ {
+ i++;
+ if (TEST_PROGRESS_COUNTER_ENABLED ())
+ TEST_PROGRESS_COUNTER (name, i);
+ }
+
+ TEST_TWO_LOCATIONS ();
+
+ return 0; /* last break here */
+}
diff --git a/gdb/testsuite/gdb.base/dtrace-probe.d b/gdb/testsuite/gdb.base/dtrace-probe.d
new file mode 100644
index 0000000..df8e6bb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dtrace-probe.d
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+provider test {
+ probe progress__counter (char *name, int);
+ probe two__locations ();
+};
diff --git a/gdb/testsuite/gdb.base/dtrace-probe.exp b/gdb/testsuite/gdb.base/dtrace-probe.exp
new file mode 100644
index 0000000..55af85d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dtrace-probe.exp
@@ -0,0 +1,156 @@
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+# Generate the test program with DTrace USDT probes.
+# This returns -1 on failure to build, 0 otherwise
+proc dtrace_build_test_program {} {
+ global testfile hex srcdir subdir srcfile binfile
+
+ # Make sure that dtrace is installed, it is the real one (not the
+ # script installed by SystemTap, for example) and of the right
+ # version (>= 0.4.0).
+
+ set dtrace "dtrace"
+
+ set result [catch "exec $dtrace -V" output]
+ if {$result != 0 || ![regexp {^dtrace: Sun D [0-9]\.[0-9]\.[0-9]$} $output]} {
+ untested dtrace-probe.exp
+ return -1
+ }
+
+ # Generate the demo program, which contains USDT probes. This
+ # involves running the `dtrace' program in order to generate some
+ # auxiliary files: a header file and an object file with the ELF
+ # sections containing the probes information.
+
+ set dscript_file "${srcdir}/${subdir}/${testfile}.d"
+ set out_header_file "${srcdir}/${subdir}/${testfile}.h"
+ set result \
+ [catch "exec $dtrace -h -s $dscript_file -o $out_header_file" output]
+ verbose -log $output
+ if {$result != 0} {
+ fail "invoke dtrace -h to generate the header file for USDT probes"
+ return -1
+ }
+
+ standard_testfile .c
+
+ if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {debug}] != ""} {
+ fail "compile ${binfile}.o"
+ return -1
+ }
+
+ set result \
+ [catch "exec $dtrace -G -s $dscript_file ${binfile}.o -o ${binfile}-p.o" output]
+ verbose -log $output
+ if {$result != 0} {
+ fail "invoke dtrace -G to generate the object file with probe information"
+ return -1
+ }
+
+ if {[gdb_compile "${binfile}.o ${binfile}-p.o" ${binfile} executable {debug}] != ""} {
+ fail "compile ${binfile}"
+ return -1
+ }
+}
+
+# Run the tests.
+# This returns -1 on failure to compile or start, 0 otherwise.
+proc dtrace_test {} {
+ global testfile hex srcfile binfile
+
+ if {[dtrace_build_test_program] == -1} {
+ return -1
+ }
+
+ clean_restart ${binfile}
+
+ if ![runto_main] {
+ return -1
+ }
+
+ gdb_test "print \$_probe_argc" "No probe at PC $hex" \
+ "check argument not at probe point"
+
+ # Test the 'info probes' command.
+ gdb_test "info probes dtrace" \
+ "test *progress-counter *$hex +no.*test *two-locations *$hex +always.*test *two-locations *$hex +always.*" \
+ "info probes dtrace"
+
+ # Disabling the probe test:two-locations shall have no effect,
+ # since no is-enabled probes are defined for it in the object
+ # file.
+
+ gdb_test "disable probe test two-locations" \
+ "Probe test:two-locations cannot be disabled.*" \
+ "disable probe test two-locations"
+
+ # On the other hand, the probe test:progress-counter can be
+ # enabled and then disabled again.
+
+ gdb_test "enable probe test progress-counter" \
+ "Probe test:progress-counter enabled.*" \
+ "enable probe test progress-counter"
+
+ gdb_test "disable probe test progress-counter" \
+ "Probe test:progress-counter disabled.*" \
+ "disable probe test progress-counter"
+
+ # Since test:progress-counter is disabled we can run to the second
+ # instance of the test:two-locations probe.
+
+ if {![runto "-probe-dtrace test:two-locations"]} {
+ fail "run to the first test:two-locations probe point"
+ }
+ gdb_test "continue" \
+ "Breakpoint \[0-9\]+, main \\(\\) at.*TEST_TWO_LOCATIONS.*" \
+ "run to the second test:two-locations probe point"
+
+ # Go back to the breakpoint on main() and enable the
+ # test:progress-counter probe. Set a breakpoint on it and see
+ # that it gets reached.
+
+ if ![runto_main] {
+ return -1
+ }
+
+ gdb_test "enable probe test progress-counter" \
+ "Probe test:progress-counter enabled.*" \
+ "enable probe test progress-counter"
+
+ gdb_test "break -probe-dtrace test:progress-counter" \
+ ".*Breakpoint \[0-9\]+ .*" "set breakpoint in test:progress-counter"
+ gdb_continue_to_breakpoint "test:progress-counter"
+
+ # Test probe arguments.
+ gdb_test "print \$_probe_argc" " = 2" \
+ "print \$_probe_argc for probe progress-counter"
+ gdb_test "print \$_probe_arg0" \
+ " = $hex \"application\"" \
+ "print \$_probe_arg0 for probe progress-counter"
+ gdb_test "print \$_probe_arg1" " = 1" \
+ "print \$_probe_arg1 for probe progress-counter"
+
+ # Set a breakpoint with multiple probe locations.
+ gdb_test "break -pdtrace test:two-locations" \
+ "Breakpoint \[0-9\]+ at $hex.*2 locations.*" \
+ "set multii-location probe breakpoint (probe two-locations)"
+
+ return 0
+}
+
+dtrace_test
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 7/9] Simple testsuite for DTrace USDT probes.
2014-09-26 9:43 ` [PATCH 7/9] Simple testsuite for DTrace USDT probes Jose E. Marchesi
@ 2014-10-08 19:30 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-08 19:30 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 2014, Jose E. Marchesi wrote:
> This patch adds some simple tests testing the support for DTrace USDT
> probes. The testsuite will be skipped as unsupported in case the user
> does not have DTrace installed on her system. The tests included in
> the test suite test breakpointing on DTrace probes, enabling and
> disabling probes, printing of probe arguments of several types and
> also breakpointing on several probes with the same name.
Thanks.
As you know, I had some issues trying to test this on my machine, since
I don't have access to dtrace from Oracle. Just one nitpick below.
> gdb/testsuite:
>
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * gdb.base/dtrace-probe.exp: New file.
> (dtrace_build_test_program): New function.
> (dtrace_test): Likewise.
>
> * gdb.base/dtrace-probe.d: New file.
> (test): New DTrace provider with two probes: `progress-counter'
> and `two-locations'.
>
> * gdb.base/dtrace-probe.c: New file.
> (main): New function.
> ---
> gdb/testsuite/ChangeLog | 13 +++
> gdb/testsuite/gdb.base/dtrace-probe.c | 38 ++++++++
> gdb/testsuite/gdb.base/dtrace-probe.d | 21 +++++
> gdb/testsuite/gdb.base/dtrace-probe.exp | 156 +++++++++++++++++++++++++++++++
> 4 files changed, 228 insertions(+)
> create mode 100644 gdb/testsuite/gdb.base/dtrace-probe.c
> create mode 100644 gdb/testsuite/gdb.base/dtrace-probe.d
> create mode 100644 gdb/testsuite/gdb.base/dtrace-probe.exp
>
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index c93d7cf..5e3f472 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,5 +1,18 @@
> 2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> + * gdb.base/dtrace-probe.exp: New file.
> + (dtrace_build_test_program): New function.
> + (dtrace_test): Likewise.
> +
> + * gdb.base/dtrace-probe.d: New file.
> + (test): New DTrace provider with two probes: `progress-counter'
> + and `two-locations'.
> +
> + * gdb.base/dtrace-probe.c: New file.
> + (main): New function.
> +
> +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.
> diff --git a/gdb/testsuite/gdb.base/dtrace-probe.c b/gdb/testsuite/gdb.base/dtrace-probe.c
> new file mode 100644
> index 0000000..45a77c5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dtrace-probe.c
> @@ -0,0 +1,38 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2014 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "dtrace-probe.h"
> +
> +int
> +main ()
> +{
> + char *name = "application";
> +
> + TEST_TWO_LOCATIONS ();
> +
> + int i = 0;
> + while (i < 10)
> + {
> + i++;
> + if (TEST_PROGRESS_COUNTER_ENABLED ())
> + TEST_PROGRESS_COUNTER (name, i);
> + }
> +
> + TEST_TWO_LOCATIONS ();
> +
> + return 0; /* last break here */
> +}
> diff --git a/gdb/testsuite/gdb.base/dtrace-probe.d b/gdb/testsuite/gdb.base/dtrace-probe.d
> new file mode 100644
> index 0000000..df8e6bb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dtrace-probe.d
> @@ -0,0 +1,21 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2014 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +provider test {
> + probe progress__counter (char *name, int);
> + probe two__locations ();
> +};
> diff --git a/gdb/testsuite/gdb.base/dtrace-probe.exp b/gdb/testsuite/gdb.base/dtrace-probe.exp
> new file mode 100644
> index 0000000..55af85d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dtrace-probe.exp
> @@ -0,0 +1,156 @@
> +# Copyright (C) 2014 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +standard_testfile
> +
> +# Generate the test program with DTrace USDT probes.
> +# This returns -1 on failure to build, 0 otherwise
> +proc dtrace_build_test_program {} {
> + global testfile hex srcdir subdir srcfile binfile
> +
> + # Make sure that dtrace is installed, it is the real one (not the
> + # script installed by SystemTap, for example) and of the right
> + # version (>= 0.4.0).
> +
> + set dtrace "dtrace"
> +
> + set result [catch "exec $dtrace -V" output]
> + if {$result != 0 || ![regexp {^dtrace: Sun D [0-9]\.[0-9]\.[0-9]$} $output]} {
> + untested dtrace-probe.exp
> + return -1
> + }
> +
> + # Generate the demo program, which contains USDT probes. This
> + # involves running the `dtrace' program in order to generate some
> + # auxiliary files: a header file and an object file with the ELF
> + # sections containing the probes information.
> +
> + set dscript_file "${srcdir}/${subdir}/${testfile}.d"
> + set out_header_file "${srcdir}/${subdir}/${testfile}.h"
> + set result \
> + [catch "exec $dtrace -h -s $dscript_file -o $out_header_file" output]
> + verbose -log $output
> + if {$result != 0} {
> + fail "invoke dtrace -h to generate the header file for USDT probes"
> + return -1
> + }
> +
> + standard_testfile .c
This is unecessary, I believe. You already called standard_testfile above.
> +
> + if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {debug}] != ""} {
> + fail "compile ${binfile}.o"
> + return -1
> + }
> +
> + set result \
> + [catch "exec $dtrace -G -s $dscript_file ${binfile}.o -o ${binfile}-p.o" output]
> + verbose -log $output
> + if {$result != 0} {
> + fail "invoke dtrace -G to generate the object file with probe information"
> + return -1
> + }
> +
> + if {[gdb_compile "${binfile}.o ${binfile}-p.o" ${binfile} executable {debug}] != ""} {
> + fail "compile ${binfile}"
> + return -1
> + }
> +}
> +
> +# Run the tests.
> +# This returns -1 on failure to compile or start, 0 otherwise.
> +proc dtrace_test {} {
> + global testfile hex srcfile binfile
> +
> + if {[dtrace_build_test_program] == -1} {
> + return -1
> + }
> +
> + clean_restart ${binfile}
> +
> + if ![runto_main] {
> + return -1
> + }
> +
> + gdb_test "print \$_probe_argc" "No probe at PC $hex" \
> + "check argument not at probe point"
> +
> + # Test the 'info probes' command.
> + gdb_test "info probes dtrace" \
> + "test *progress-counter *$hex +no.*test *two-locations *$hex +always.*test *two-locations *$hex +always.*" \
> + "info probes dtrace"
> +
> + # Disabling the probe test:two-locations shall have no effect,
> + # since no is-enabled probes are defined for it in the object
> + # file.
> +
> + gdb_test "disable probe test two-locations" \
> + "Probe test:two-locations cannot be disabled.*" \
> + "disable probe test two-locations"
> +
> + # On the other hand, the probe test:progress-counter can be
> + # enabled and then disabled again.
> +
> + gdb_test "enable probe test progress-counter" \
> + "Probe test:progress-counter enabled.*" \
> + "enable probe test progress-counter"
> +
> + gdb_test "disable probe test progress-counter" \
> + "Probe test:progress-counter disabled.*" \
> + "disable probe test progress-counter"
> +
> + # Since test:progress-counter is disabled we can run to the second
> + # instance of the test:two-locations probe.
> +
> + if {![runto "-probe-dtrace test:two-locations"]} {
> + fail "run to the first test:two-locations probe point"
> + }
> + gdb_test "continue" \
> + "Breakpoint \[0-9\]+, main \\(\\) at.*TEST_TWO_LOCATIONS.*" \
> + "run to the second test:two-locations probe point"
> +
> + # Go back to the breakpoint on main() and enable the
> + # test:progress-counter probe. Set a breakpoint on it and see
> + # that it gets reached.
> +
> + if ![runto_main] {
> + return -1
> + }
> +
> + gdb_test "enable probe test progress-counter" \
> + "Probe test:progress-counter enabled.*" \
> + "enable probe test progress-counter"
> +
> + gdb_test "break -probe-dtrace test:progress-counter" \
> + ".*Breakpoint \[0-9\]+ .*" "set breakpoint in test:progress-counter"
> + gdb_continue_to_breakpoint "test:progress-counter"
> +
> + # Test probe arguments.
> + gdb_test "print \$_probe_argc" " = 2" \
> + "print \$_probe_argc for probe progress-counter"
> + gdb_test "print \$_probe_arg0" \
> + " = $hex \"application\"" \
> + "print \$_probe_arg0 for probe progress-counter"
> + gdb_test "print \$_probe_arg1" " = 1" \
> + "print \$_probe_arg1 for probe progress-counter"
> +
> + # Set a breakpoint with multiple probe locations.
> + gdb_test "break -pdtrace test:two-locations" \
> + "Breakpoint \[0-9\]+ at $hex.*2 locations.*" \
> + "set multii-location probe breakpoint (probe two-locations)"
> +
> + return 0
> +}
> +
> +dtrace_test
> --
> 1.7.10.4
Other than that, the testcase looks good to me.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 7/9] Simple testsuite for DTrace USDT probes.
2014-10-08 19:30 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
> + # Generate the demo program, which contains USDT probes. This
> + # involves running the `dtrace' program in order to generate some
> + # auxiliary files: a header file and an object file with the ELF
> + # sections containing the probes information.
> +
> + set dscript_file "${srcdir}/${subdir}/${testfile}.d"
> + set out_header_file "${srcdir}/${subdir}/${testfile}.h"
> + set result \
> + [catch "exec $dtrace -h -s $dscript_file -o $out_header_file" output]
> + verbose -log $output
> + if {$result != 0} {
> + fail "invoke dtrace -h to generate the header file for USDT probes"
> + return -1
> + }
> +
> + standard_testfile .c
This is unecessary, I believe. You already called standard_testfile above.
Yep, I just removed it.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/9] Adapt `info probes' to support printing probes of different types.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (5 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 7/9] Simple testsuite for DTrace USDT probes Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-09-29 21:15 ` Sergio Durigan Junior
2014-09-26 9:43 ` [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c Jose E. Marchesi
` (2 subsequent siblings)
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
A "probe type" (backend for the probe abstraction implemented in probe.[ch])
can extend the information printed by `info probes' by defining additional
columns. This means that when `info probes' is used to print all the probes
regardless of their types, some of the columns will be "not applicable" to
some of the probes (like, say, the Semaphore column only makes sense for
SystemTap probes). This patch makes `info probes' fill these slots with "n/a"
marks (currently it breaks the table) and not include headers for which no
actual probe has been found in the list of defined probes.
gdb:
2014-09-25 Jose E. Marchesi <jose.marchesi@oracle.com>
* probe.c (print_ui_out_not_applicables): New function.
(get_num_probes_with_pops): Likewise.
(info_probes_for_ops): Do not include column headers for probe
types for which no probe has been actually found on any object.
Also invoke `print_ui_out_not_applicables' in order to match the
column rows with the header when probes of several types are
listed.
---
gdb/ChangeLog | 10 +++++++++
gdb/probe.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dbd222d..7cc4f00 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2014-09-25 Jose E. Marchesi <jose.marchesi@oracle.com>
+
+ * probe.c (print_ui_out_not_applicables): New function.
+ (get_num_probes_with_pops): Likewise.
+ (info_probes_for_ops): Do not include column headers for probe
+ types for which no probe has been actually found on any object.
+ Also invoke `print_ui_out_not_applicables' in order to match the
+ column rows with the header when probes of several types are
+ listed.
+
2014-09-25 Pedro Alves <palves@redhat.com>
* infrun.c (user_visible_resume_ptid): Don't check
diff --git a/gdb/probe.c b/gdb/probe.c
index 859e6e7..5458372 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -411,6 +411,33 @@ gen_ui_out_table_header_info (VEC (bound_probe_s) *probes,
do_cleanups (c);
}
+/* Helper function to print not-applicable strings for all the extra
+ columns defined in a probe_ops. */
+
+static void
+print_ui_out_not_applicables (const struct probe_ops *pops)
+{
+ struct cleanup *c;
+ 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);
+}
+
/* Helper function to print extra information about a probe and an objfile
represented by PROBE. */
@@ -483,6 +510,23 @@ get_number_extra_fields (const struct probe_ops *pops)
return n;
}
+/* Helper function that returns the number of probes in PROBES having
+ the given POPS. */
+
+static int
+get_num_probes_with_pops (VEC (bound_probe_s) *probes,
+ const struct probe_ops *pops)
+{
+ int res = 0;
+ struct bound_probe *probe;
+ int ix;
+
+ for (ix = 0; VEC_iterate (bound_probe_s, probes, ix, probe); ++ix)
+ res += (probe->probe->pops == pops);
+
+ return res;
+}
+
/* See comment in probe.h. */
void
@@ -518,6 +562,8 @@ info_probes_for_ops (const char *arg, int from_tty,
}
}
+ probes = collect_probes (objname, provider, probe_name, pops);
+
if (pops == NULL)
{
const struct probe_ops *po;
@@ -530,15 +576,16 @@ info_probes_for_ops (const char *arg, int from_tty,
To do that, we iterate over all probe_ops, querying each one about
its extra fields, and incrementing `ui_out_extra_fields' to reflect
- that number. */
+ that number. But note that we ignore the probe_ops for which no probes
+ are defined with the given search criteria. */
for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
- ui_out_extra_fields += get_number_extra_fields (po);
+ if (get_num_probes_with_pops (probes, po))
+ ui_out_extra_fields += get_number_extra_fields (po);
}
else
ui_out_extra_fields = get_number_extra_fields (pops);
- probes = collect_probes (objname, provider, probe_name, pops);
make_cleanup (VEC_cleanup (probe_p), &probes);
make_cleanup_ui_out_table_begin_end (current_uiout,
4 + ui_out_extra_fields,
@@ -572,10 +619,12 @@ info_probes_for_ops (const char *arg, int from_tty,
const struct probe_ops *po;
int ix;
- /* We have to generate the table header for each new probe type that we
- will print. */
+ /* We have to generate the table header for each new probe type
+ that we will print. Note that this excludes probe types not
+ having any defined probe with the search criteria. */
for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
- gen_ui_out_table_header_info (probes, po);
+ if (get_num_probes_with_pops (probes, po) > 0)
+ gen_ui_out_table_header_info (probes, po);
}
else
gen_ui_out_table_header_info (probes, pops);
@@ -605,6 +654,8 @@ info_probes_for_ops (const char *arg, int from_tty,
++ix)
if (probe->probe->pops == po)
print_ui_out_info (probe->probe);
+ else if (get_num_probes_with_pops (probes, po) > 0)
+ print_ui_out_not_applicables (po);
}
else
print_ui_out_info (probe->probe);
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 1/9] Adapt `info probes' to support printing probes of different types.
2014-09-26 9:43 ` [PATCH 1/9] Adapt `info probes' to support printing probes of different types Jose E. Marchesi
@ 2014-09-29 21:15 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-09-29 21:15 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 2014, Jose E. Marchesi wrote:
> A "probe type" (backend for the probe abstraction implemented in probe.[ch])
> can extend the information printed by `info probes' by defining additional
> columns. This means that when `info probes' is used to print all the probes
> regardless of their types, some of the columns will be "not applicable" to
> some of the probes (like, say, the Semaphore column only makes sense for
> SystemTap probes). This patch makes `info probes' fill these slots with "n/a"
> marks (currently it breaks the table) and not include headers for which no
> actual probe has been found in the list of defined probes.
Thanks for the patch, Jose. Comments below.
> gdb:
>
> 2014-09-25 Jose E. Marchesi <jose.marchesi@oracle.com>
>
> * probe.c (print_ui_out_not_applicables): New function.
> (get_num_probes_with_pops): Likewise.
> (info_probes_for_ops): Do not include column headers for probe
> types for which no probe has been actually found on any object.
> Also invoke `print_ui_out_not_applicables' in order to match the
> column rows with the header when probes of several types are
> listed.
> ---
> gdb/ChangeLog | 10 +++++++++
> gdb/probe.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index dbd222d..7cc4f00 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,13 @@
> +2014-09-25 Jose E. Marchesi <jose.marchesi@oracle.com>
> +
> + * probe.c (print_ui_out_not_applicables): New function.
> + (get_num_probes_with_pops): Likewise.
> + (info_probes_for_ops): Do not include column headers for probe
> + types for which no probe has been actually found on any object.
> + Also invoke `print_ui_out_not_applicables' in order to match the
> + column rows with the header when probes of several types are
> + listed.
> +
BTW, we usually don't include ChangeLog diff's in the message, because
they make it hard to apply the patch after some time (even hours!). I
am mentioning this because I had several conflicts when I applied yours
here ;-). (Of course, one can always delete the ChangeLog diff's from
the patches before applying them, but it is also annoying).
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 859e6e7..5458372 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -411,6 +411,33 @@ gen_ui_out_table_header_info (VEC (bound_probe_s) *probes,
> do_cleanups (c);
> }
>
> +/* Helper function to print not-applicable strings for all the extra
> + columns defined in a probe_ops. */
> +
> +static void
> +print_ui_out_not_applicables (const struct probe_ops *pops)
> +{
> + struct cleanup *c;
> + 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"));
> + }
You can remove the brackets, since it's a one-statement for.
> +
> + do_cleanups (c);
> +}
> +
> /* Helper function to print extra information about a probe and an objfile
> represented by PROBE. */
>
> @@ -483,6 +510,23 @@ get_number_extra_fields (const struct probe_ops *pops)
> return n;
> }
>
> +/* Helper function that returns the number of probes in PROBES having
> + the given POPS. */
> +
> +static int
> +get_num_probes_with_pops (VEC (bound_probe_s) *probes,
> + const struct probe_ops *pops)
> +{
> + int res = 0;
> + struct bound_probe *probe;
> + int ix;
> +
> + for (ix = 0; VEC_iterate (bound_probe_s, probes, ix, probe); ++ix)
> + res += (probe->probe->pops == pops);
Please, remove the parens, we don't use them in this case.
> +
> + return res;
> +}
Also, this function could be simplified. You are not using the count
anywhere, so why not just look for the first match of
"probe->probe->pops == pops", and return 1 if it is found (0 otherwise)?
The function name should be adjusted in this case.
> +
> /* See comment in probe.h. */
>
> void
> @@ -518,6 +562,8 @@ info_probes_for_ops (const char *arg, int from_tty,
> }
> }
>
> + probes = collect_probes (objname, provider, probe_name, pops);
> +
> if (pops == NULL)
> {
> const struct probe_ops *po;
> @@ -530,15 +576,16 @@ info_probes_for_ops (const char *arg, int from_tty,
>
> To do that, we iterate over all probe_ops, querying each one about
> its extra fields, and incrementing `ui_out_extra_fields' to reflect
> - that number. */
> + that number. But note that we ignore the probe_ops for which no probes
> + are defined with the given search criteria. */
>
> for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
> - ui_out_extra_fields += get_number_extra_fields (po);
> + if (get_num_probes_with_pops (probes, po))
> + ui_out_extra_fields += get_number_extra_fields (po);
Here is the only place you are doing an implicit comparison. I was
going to tell you to compare explicitly, i.e., "get_num_probes_with_pops
(probes, po) > 0", but when you implement the simplification I suggested
above, there is no need anymore.
> }
> else
> ui_out_extra_fields = get_number_extra_fields (pops);
>
> - probes = collect_probes (objname, provider, probe_name, pops);
> make_cleanup (VEC_cleanup (probe_p), &probes);
This call to "make_cleanup" should be moved up, too.
> make_cleanup_ui_out_table_begin_end (current_uiout,
> 4 + ui_out_extra_fields,
> @@ -572,10 +619,12 @@ info_probes_for_ops (const char *arg, int from_tty,
> const struct probe_ops *po;
> int ix;
>
> - /* We have to generate the table header for each new probe type that we
> - will print. */
> + /* We have to generate the table header for each new probe type
> + that we will print. Note that this excludes probe types not
> + having any defined probe with the search criteria. */
> for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po); ++ix)
> - gen_ui_out_table_header_info (probes, po);
> + if (get_num_probes_with_pops (probes, po) > 0)
> + gen_ui_out_table_header_info (probes, po);
> }
> else
> gen_ui_out_table_header_info (probes, pops);
> @@ -605,6 +654,8 @@ info_probes_for_ops (const char *arg, int from_tty,
> ++ix)
> if (probe->probe->pops == po)
> print_ui_out_info (probe->probe);
> + else if (get_num_probes_with_pops (probes, po) > 0)
> + print_ui_out_not_applicables (po);
Hm, this loop is kind of confusing. I have a few ideas to make it
clearer, but that's for another patch :-).
> }
Both places will need to be adjusted when you simplify the
get_num_probes_with_pops function.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 1/9] Adapt `info probes' to support printing probes of different types.
2014-09-29 21:15 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
Hi Sergio.
> A "probe type" (backend for the probe abstraction implemented in probe.[ch])
> can extend the information printed by `info probes' by defining additional
> columns. This means that when `info probes' is used to print all the probes
> regardless of their types, some of the columns will be "not applicable" to
> some of the probes (like, say, the Semaphore column only makes sense for
> SystemTap probes). This patch makes `info probes' fill these slots with "n/a"
> marks (currently it breaks the table) and not include headers for which no
> actual probe has been found in the list of defined probes.
Thanks for the patch, Jose. Comments below.
I agree/comply with all these comments :)
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (6 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 1/9] Adapt `info probes' to support printing probes of different types Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-09-30 0:02 ` Sergio Durigan Junior
2014-09-26 9:43 ` [PATCH 4/9] New gdbarch functions: dtrace_probe_argument, dtrace_probe_is_enabled, dtrace_enable_probe, dtrace_disable_probe Jose E. Marchesi
2014-10-08 19:40 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Sergio Durigan Junior
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
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".
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.
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;
}
+\f
+
+/* 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)
\f
-/* 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);
-}
-
-\f
-
/* 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
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c
2014-09-26 9:43 ` [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c Jose E. Marchesi
@ 2014-09-30 0:02 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-09-30 0:02 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
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;
> }
>
> +\f
> +
> +/* 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)
>
> \f
>
> -/* 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);
> -}
> -
> -\f
> -
> /* 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/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c
2014-09-30 0:02 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
> 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.
Fixed in the patch.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 4/9] New gdbarch functions: dtrace_probe_argument, dtrace_probe_is_enabled, dtrace_enable_probe, dtrace_disable_probe.
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (7 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 2/9] Move `compute_probe_arg' and `compile_probe_arg' to probe.c Jose E. Marchesi
@ 2014-09-26 9:43 ` Jose E. Marchesi
2014-10-02 21:34 ` Sergio Durigan Junior
2014-10-08 19:40 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Sergio Durigan Junior
9 siblings, 1 reply; 42+ messages in thread
From: Jose E. Marchesi @ 2014-09-26 9:43 UTC (permalink / raw)
To: gdb-patches
This patch adds several gdbarch functions (along with the
corresponding predicates): `dtrace_probe_argument',
`dtrace_probe_is_enabled', `dtrace_enable_probe' and
`dtrace_disable_probe'. These functions will be implemented by
target-specific code, and called from the DTrace probes implementation
in order to calculate the value of probe arguments, and manipulate
is-enabled probes.
gdb:
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
* gdbarch.sh (dtrace_probe_argument): New.
(dtrace_probe_is_enabled): Likewise.
(dtrace_enable_probe): Likewise.
(dtrace_disable_probe): Likewise.
* gdbarch.c: Regenerate.
* gdbarch.h: Regenerate.
---
gdb/ChangeLog | 9 ++++
gdb/gdbarch.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbarch.h | 36 ++++++++++++++++
gdb/gdbarch.sh | 16 +++++++
4 files changed, 189 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2f94e05..7041cfb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
2014-09-26 Jose E. Marchesi <jose.marchesi@oracle.com>
+ * gdbarch.sh (dtrace_probe_argument): New.
+ (dtrace_probe_is_enabled): Likewise.
+ (dtrace_enable_probe): Likewise.
+ (dtrace_disable_probe): Likewise.
+ * gdbarch.c: Regenerate.
+ * gdbarch.h: Regenerate.
+
+2014-09-26 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'.
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index b0ee79d..3d494a4 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -299,6 +299,10 @@ struct gdbarch
const char * stap_gdb_register_suffix;
gdbarch_stap_is_single_operand_ftype *stap_is_single_operand;
gdbarch_stap_parse_special_token_ftype *stap_parse_special_token;
+ gdbarch_dtrace_probe_argument_ftype *dtrace_probe_argument;
+ gdbarch_dtrace_probe_is_enabled_ftype *dtrace_probe_is_enabled;
+ gdbarch_dtrace_enable_probe_ftype *dtrace_enable_probe;
+ gdbarch_dtrace_disable_probe_ftype *dtrace_disable_probe;
int has_global_solist;
int has_global_breakpoints;
gdbarch_has_shared_address_space_ftype *has_shared_address_space;
@@ -611,6 +615,10 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of stap_gdb_register_suffix, invalid_p == 0 */
/* Skip verify of stap_is_single_operand, has predicate. */
/* Skip verify of stap_parse_special_token, has predicate. */
+ /* Skip verify of dtrace_probe_argument, has predicate. */
+ /* Skip verify of dtrace_probe_is_enabled, has predicate. */
+ /* Skip verify of dtrace_enable_probe, has predicate. */
+ /* Skip verify of dtrace_disable_probe, has predicate. */
/* Skip verify of has_global_solist, invalid_p == 0 */
/* Skip verify of has_global_breakpoints, invalid_p == 0 */
/* Skip verify of has_shared_address_space, invalid_p == 0 */
@@ -819,6 +827,30 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
"gdbarch_dump: double_format = %s\n",
pformat (gdbarch->double_format));
fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_dtrace_disable_probe_p() = %d\n",
+ gdbarch_dtrace_disable_probe_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: dtrace_disable_probe = <%s>\n",
+ host_address_to_string (gdbarch->dtrace_disable_probe));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_dtrace_enable_probe_p() = %d\n",
+ gdbarch_dtrace_enable_probe_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: dtrace_enable_probe = <%s>\n",
+ host_address_to_string (gdbarch->dtrace_enable_probe));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_dtrace_probe_argument_p() = %d\n",
+ gdbarch_dtrace_probe_argument_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: dtrace_probe_argument = <%s>\n",
+ host_address_to_string (gdbarch->dtrace_probe_argument));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_dtrace_probe_is_enabled_p() = %d\n",
+ gdbarch_dtrace_probe_is_enabled_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: dtrace_probe_is_enabled = <%s>\n",
+ host_address_to_string (gdbarch->dtrace_probe_is_enabled));
+ fprintf_unfiltered (file,
"gdbarch_dump: gdbarch_dummy_id_p() = %d\n",
gdbarch_dummy_id_p (gdbarch));
fprintf_unfiltered (file,
@@ -4095,6 +4127,102 @@ set_gdbarch_stap_parse_special_token (struct gdbarch *gdbarch,
}
int
+gdbarch_dtrace_probe_argument_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->dtrace_probe_argument != NULL;
+}
+
+void
+gdbarch_dtrace_probe_argument (struct gdbarch *gdbarch, struct parser_state *pstate, int narg)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->dtrace_probe_argument != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_dtrace_probe_argument called\n");
+ gdbarch->dtrace_probe_argument (gdbarch, pstate, narg);
+}
+
+void
+set_gdbarch_dtrace_probe_argument (struct gdbarch *gdbarch,
+ gdbarch_dtrace_probe_argument_ftype dtrace_probe_argument)
+{
+ gdbarch->dtrace_probe_argument = dtrace_probe_argument;
+}
+
+int
+gdbarch_dtrace_probe_is_enabled_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->dtrace_probe_is_enabled != NULL;
+}
+
+int
+gdbarch_dtrace_probe_is_enabled (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->dtrace_probe_is_enabled != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_dtrace_probe_is_enabled called\n");
+ return gdbarch->dtrace_probe_is_enabled (gdbarch, addr);
+}
+
+void
+set_gdbarch_dtrace_probe_is_enabled (struct gdbarch *gdbarch,
+ gdbarch_dtrace_probe_is_enabled_ftype dtrace_probe_is_enabled)
+{
+ gdbarch->dtrace_probe_is_enabled = dtrace_probe_is_enabled;
+}
+
+int
+gdbarch_dtrace_enable_probe_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->dtrace_enable_probe != NULL;
+}
+
+void
+gdbarch_dtrace_enable_probe (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->dtrace_enable_probe != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_dtrace_enable_probe called\n");
+ gdbarch->dtrace_enable_probe (gdbarch, addr);
+}
+
+void
+set_gdbarch_dtrace_enable_probe (struct gdbarch *gdbarch,
+ gdbarch_dtrace_enable_probe_ftype dtrace_enable_probe)
+{
+ gdbarch->dtrace_enable_probe = dtrace_enable_probe;
+}
+
+int
+gdbarch_dtrace_disable_probe_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->dtrace_disable_probe != NULL;
+}
+
+void
+gdbarch_dtrace_disable_probe (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->dtrace_disable_probe != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_dtrace_disable_probe called\n");
+ gdbarch->dtrace_disable_probe (gdbarch, addr);
+}
+
+void
+set_gdbarch_dtrace_disable_probe (struct gdbarch *gdbarch,
+ gdbarch_dtrace_disable_probe_ftype dtrace_disable_probe)
+{
+ gdbarch->dtrace_disable_probe = dtrace_disable_probe;
+}
+
+int
gdbarch_has_global_solist (struct gdbarch *gdbarch)
{
gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 0303b2e..89f3c83 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -57,6 +57,7 @@ struct syscall;
struct agent_expr;
struct axs_value;
struct stap_parse_info;
+struct parser_state;
struct ravenscar_arch_ops;
struct elf_internal_linux_prpsinfo;
@@ -1179,6 +1180,41 @@ typedef int (gdbarch_stap_parse_special_token_ftype) (struct gdbarch *gdbarch, s
extern int gdbarch_stap_parse_special_token (struct gdbarch *gdbarch, struct stap_parse_info *p);
extern void set_gdbarch_stap_parse_special_token (struct gdbarch *gdbarch, gdbarch_stap_parse_special_token_ftype *stap_parse_special_token);
+/* DTrace related functions.
+ The expression to compute the NARTGth+1 argument to a DTrace USDT probe.
+ NARG must be >= 0. */
+
+extern int gdbarch_dtrace_probe_argument_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_dtrace_probe_argument_ftype) (struct gdbarch *gdbarch, struct parser_state *pstate, int narg);
+extern void gdbarch_dtrace_probe_argument (struct gdbarch *gdbarch, struct parser_state *pstate, int narg);
+extern void set_gdbarch_dtrace_probe_argument (struct gdbarch *gdbarch, gdbarch_dtrace_probe_argument_ftype *dtrace_probe_argument);
+
+/* True if the given ADDR does not contain the instruction sequence
+ corresponding to a disabled DTrace is-enabled probe. */
+
+extern int gdbarch_dtrace_probe_is_enabled_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_dtrace_probe_is_enabled_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern int gdbarch_dtrace_probe_is_enabled (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void set_gdbarch_dtrace_probe_is_enabled (struct gdbarch *gdbarch, gdbarch_dtrace_probe_is_enabled_ftype *dtrace_probe_is_enabled);
+
+/* Enable a DTrace is-enabled probe at ADDR. */
+
+extern int gdbarch_dtrace_enable_probe_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_dtrace_enable_probe_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void gdbarch_dtrace_enable_probe (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void set_gdbarch_dtrace_enable_probe (struct gdbarch *gdbarch, gdbarch_dtrace_enable_probe_ftype *dtrace_enable_probe);
+
+/* Disable a DTrace is-enabled probe at ADDR. */
+
+extern int gdbarch_dtrace_disable_probe_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_dtrace_disable_probe_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void gdbarch_dtrace_disable_probe (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void set_gdbarch_dtrace_disable_probe (struct gdbarch *gdbarch, gdbarch_dtrace_disable_probe_ftype *dtrace_disable_probe);
+
/* True if the list of shared libraries is one and only for all
processes, as opposed to a list of shared libraries per inferior.
This usually means that all processes, although may or may not share
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 2a8bca8..0458134 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -944,6 +944,21 @@ M:int:stap_is_single_operand:const char *s:s
# parser), and should advance the buffer pointer (p->arg).
M:int:stap_parse_special_token:struct stap_parse_info *p:p
+# DTrace related functions.
+
+# The expression to compute the NARTGth+1 argument to a DTrace USDT probe.
+# NARG must be >= 0.
+M:void:dtrace_probe_argument:struct parser_state *pstate, int narg:pstate, narg
+
+# True if the given ADDR does not contain the instruction sequence
+# corresponding to a disabled DTrace is-enabled probe.
+M:int:dtrace_probe_is_enabled:CORE_ADDR addr:addr
+
+# Enable a DTrace is-enabled probe at ADDR.
+M:void:dtrace_enable_probe:CORE_ADDR addr:addr
+
+# Disable a DTrace is-enabled probe at ADDR.
+M:void:dtrace_disable_probe:CORE_ADDR addr:addr
# True if the list of shared libraries is one and only for all
# processes, as opposed to a list of shared libraries per inferior.
@@ -1146,6 +1161,7 @@ struct syscall;
struct agent_expr;
struct axs_value;
struct stap_parse_info;
+struct parser_state;
struct ravenscar_arch_ops;
struct elf_internal_linux_prpsinfo;
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/9] New gdbarch functions: dtrace_probe_argument, dtrace_probe_is_enabled, dtrace_enable_probe, dtrace_disable_probe.
2014-09-26 9:43 ` [PATCH 4/9] New gdbarch functions: dtrace_probe_argument, dtrace_probe_is_enabled, dtrace_enable_probe, dtrace_disable_probe Jose E. Marchesi
@ 2014-10-02 21:34 ` Sergio Durigan Junior
2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-02 21:34 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 2014, Jose E. Marchesi wrote:
> This patch adds several gdbarch functions (along with the
> corresponding predicates): `dtrace_probe_argument',
> `dtrace_probe_is_enabled', `dtrace_enable_probe' and
> `dtrace_disable_probe'. These functions will be implemented by
> target-specific code, and called from the DTrace probes implementation
> in order to calculate the value of probe arguments, and manipulate
> is-enabled probes.
Thanks for the patch. Comments below.
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 2a8bca8..0458134 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -944,6 +944,21 @@ M:int:stap_is_single_operand:const char *s:s
> # parser), and should advance the buffer pointer (p->arg).
> M:int:stap_parse_special_token:struct stap_parse_info *p:p
>
> +# DTrace related functions.
> +
> +# The expression to compute the NARTGth+1 argument to a DTrace USDT probe.
> +# NARG must be >= 0.
> +M:void:dtrace_probe_argument:struct parser_state *pstate, int narg:pstate, narg
This function is responsible for parsing the argument, right? I'd
prefer if you named it "dtrace_parse_probe_argument". WDYT?
> +# True if the given ADDR does not contain the instruction sequence
> +# corresponding to a disabled DTrace is-enabled probe.
> +M:int:dtrace_probe_is_enabled:CORE_ADDR addr:addr
> +
> +# Enable a DTrace is-enabled probe at ADDR.
> +M:void:dtrace_enable_probe:CORE_ADDR addr:addr
> +
> +# Disable a DTrace is-enabled probe at ADDR.
> +M:void:dtrace_disable_probe:CORE_ADDR addr:addr
>
> # True if the list of shared libraries is one and only for all
> # processes, as opposed to a list of shared libraries per inferior.
> @@ -1146,6 +1161,7 @@ struct syscall;
> struct agent_expr;
> struct axs_value;
> struct stap_parse_info;
> +struct parser_state;
> struct ravenscar_arch_ops;
> struct elf_internal_linux_prpsinfo;
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/9] New gdbarch functions: dtrace_probe_argument, dtrace_probe_is_enabled, dtrace_enable_probe, dtrace_disable_probe.
2014-10-02 21:34 ` Sergio Durigan Junior
@ 2014-10-10 16:38 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-10 16:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 2a8bca8..0458134 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -944,6 +944,21 @@ M:int:stap_is_single_operand:const char *s:s
> # parser), and should advance the buffer pointer (p->arg).
> M:int:stap_parse_special_token:struct stap_parse_info *p:p
>
> +# DTrace related functions.
> +
> +# The expression to compute the NARTGth+1 argument to a DTrace USDT probe.
> +# NARG must be >= 0.
> +M:void:dtrace_probe_argument:struct parser_state *pstate, int narg:pstate, narg
This function is responsible for parsing the argument, right? I'd
prefer if you named it "dtrace_parse_probe_argument". WDYT?
Yes, I agree, that name is better. Changed in the patch.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/9] Add support for DTrace USDT probes to gdb
2014-09-26 9:43 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Jose E. Marchesi
` (8 preceding siblings ...)
2014-09-26 9:43 ` [PATCH 4/9] New gdbarch functions: dtrace_probe_argument, dtrace_probe_is_enabled, dtrace_enable_probe, dtrace_disable_probe Jose E. Marchesi
@ 2014-10-08 19:40 ` Sergio Durigan Junior
2014-10-09 8:05 ` Jose E. Marchesi
9 siblings, 1 reply; 42+ messages in thread
From: Sergio Durigan Junior @ 2014-10-08 19:40 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: gdb-patches
On Friday, September 26 2014, Jose E. Marchesi wrote:
> This patch series introduces support in GDB for a new type of probe:
> DTrace USDT probes.
>
> The first three patches do some changes to the existing probe.[ch]
> code, fixing some minor problems associated to support several probe
> types, having several probes of different types defined in the same
> object and supporting the notion of enabling and disabling probes.
>
> The rest of the patches are the implementation of the new probe type,
> including target support for x86_64 targets, a testsuite and
> documentation.
OK, I think I reviewed all the patches (except the docs). First of all,
thanks for doing this!
I have mixed feelings about the inclusion of this feature. While I
obviously agree that support for a new probe type is good, I also don't
like the fact that we are talking about a feature that relies on
proprietary software (Oracle dtrace) to operate.
For example, when trying to test this here, I naively thought that using
something like "dtrace4linux" (<https://github.com/dtrace4linux>) would
be enough. It wasn't. So you kindly helped me with this, but I
know that other people who decide to test GDB might not be lucky to have
Jose helping them :-).
You mentioned that you were going to try to come up with some way to
test this feature using a tweaked asm source. I think this is a good
progress already, and I am looking forward to seeing this.
Either way, if we decide to include this feature on GDB, you (or someone
else from Oracle) will be responsible for it, because we outside Oracle
have no way to make sure that it doesn't break.
Having said that, I will wait until you post updated patches with our
comments addressed (and hopefully with a nice testcase :-P), and then we
can continue this discussion :-).
Cheers,
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 0/9] Add support for DTrace USDT probes to gdb
2014-10-08 19:40 ` [PATCH 0/9] Add support for DTrace USDT probes to gdb Sergio Durigan Junior
@ 2014-10-09 8:05 ` Jose E. Marchesi
0 siblings, 0 replies; 42+ messages in thread
From: Jose E. Marchesi @ 2014-10-09 8:05 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
Hi Sergio.
> This patch series introduces support in GDB for a new type of probe:
> DTrace USDT probes.
>
> The first three patches do some changes to the existing probe.[ch]
> code, fixing some minor problems associated to support several probe
> types, having several probes of different types defined in the same
> object and supporting the notion of enabling and disabling probes.
>
> The rest of the patches are the implementation of the new probe type,
> including target support for x86_64 targets, a testsuite and
> documentation.
OK, I think I reviewed all the patches (except the docs). First of all,
thanks for doing this!
Heh, thanks for the throughout review!
I have mixed feelings about the inclusion of this feature. While I
obviously agree that support for a new probe type is good, I also don't
like the fact that we are talking about a feature that relies on
proprietary software (Oracle dtrace) to operate.
As you know I am sympathetic with your concerns. Was I in your position
I would raise exactly the same point.
You mentioned that you were going to try to come up with some way to
test this feature using a tweaked asm source. I think this is a good
progress already, and I am looking forward to seeing this.
Yes, I am working in a .s file that, once compiled and linked, will
provide the DOF program corresponding to the testcase. That will not
replace dtrace -G for the general usage, and will have to be updated
should the testcase change, but at least will allow to test the probe
support without having to install proprietary software.
Either way, if we decide to include this feature on GDB, you (or someone
else from Oracle) will be responsible for it, because we outside Oracle
have no way to make sure that it doesn't break.
I can certainly take responsibility on it.
Having said that, I will wait until you post updated patches with our
comments addressed (and hopefully with a nice testcase :-P), and then we
can continue this discussion :-).
I will post a corrected version of the patch series soon.
Thanks again!
^ permalink raw reply [flat|nested] 42+ messages in thread