[PATCH] record: automatically start recording

[PATCH] record: automatically start recording

[Markus Metzger]

When using process record and replay frequently, one sometimes forgets to start
recording after re-run.  This can be quite annoying.

I tried to automatically start recording for every new inferior with existing
GDB commands.  I came up with:

    (gdb) b _start
    Breakpoint 1 at 0x400560
    (gdb) command 1
    Type commands for breakpoint(s) 1, one per line.
    End with a line saying just "end".
    >record btrace
    >cont
    >end

This may not be completely obvious to everybody and you can't easily put it into
your gdbinit.  It also doesn't support the attach case (not sure that matters).

This patch adds a new option "auto-record" to automatically start recording for
every new inferior using GDB's inferior-created observer.

Is the added convenience worth a new option or do we want to point users to the
breakpoint-command solution?

2016-04-05  Markus Metzger  <markus.t.metzger@intel.com>
---
 gdb/NEWS                                 |  3 ++
 gdb/doc/gdb.texinfo                      | 12 ++++++
 gdb/record.c                             | 64 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.btrace/auto-enable.exp | 48 ++++++++++++++++++++++++
 4 files changed, 127 insertions(+)
 create mode 100644 gdb/testsuite/gdb.btrace/auto-enable.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 39d99b7..79f27b0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -37,6 +37,9 @@ skip -rfunction regular-expression
 maint info line-table REGEXP
   Display the contents of GDB's internal line table data struture.
 
+set|show auto-record
+  Automatically record newly created processes.
+
 * Support for tracepoints and fast tracepoints on s390-linux and s390x-linux
   was added in GDBserver, including JIT compiling fast tracepoint's
   conditional expression bytecode into native code.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7abd55e..7d46a95 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6681,6 +6681,18 @@ the asynchronous execution mode (@pxref{Background Execution}), not
 all recording methods are available.  The @code{full} recording method
 does not support these two modes.
 
+@kindex set auto-record
+@item set auto-record @var{method}
+Automatically record newly created processes using the recording
+method @var{method}.  If @var{method} is @code{off}, automatic
+recording is disabled.
+
+@kindex show auto-record
+@item show auto-record
+Show the recoding method used for automatically recording newly
+created processes.  If automatic recording is disabled, the recording
+method is @code{off}.
+
 @kindex record stop
 @kindex rec s
 @item record stop
diff --git a/gdb/record.c b/gdb/record.c
index 6190794..13a451a 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -53,10 +53,37 @@ struct cmd_list_element *set_record_cmdlist = NULL;
 struct cmd_list_element *show_record_cmdlist = NULL;
 struct cmd_list_element *info_record_cmdlist = NULL;
 
+/* Supported names for the "set|show auto-record" command.  */
+static const char auto_record_off[] = "off";
+static const char auto_record_full[] = "full";
+static const char auto_record_btrace[] = "btrace";
+static const char auto_record_bts[] = "bts";
+static const char auto_record_pt[] = "pt";
+static const char *const auto_record_names[] =
+{
+  auto_record_off,
+  auto_record_full,
+  auto_record_btrace,
+  auto_record_bts,
+  auto_record_pt,
+  NULL,
+};
+
+static const char *auto_record_string = auto_record_off;
+
 #define DEBUG(msg, args...)						\
   if (record_debug)							\
     fprintf_unfiltered (gdb_stdlog, "record: " msg "\n", ##args)
 
+/* The "show auto-record" command.  */
+
+static void
+show_auto_record_string (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Auto record is \"%s\".\n"),  value);
+}
+
 /* See record.h.  */
 
 struct target_ops *
@@ -733,6 +760,30 @@ set_record_call_history_size (char *args, int from_tty,
 			 &record_call_history_size);
 }
 
+/* The inferior-created observer implementing "set auto-record".  */
+
+static void
+record_inferior_created (struct target_ops *ops, int from_tty)
+{
+  if (auto_record_string == auto_record_off)
+    return;
+
+  TRY
+    {
+      char command[64];
+
+      snprintf (command, sizeof(command), "record %s", auto_record_string);
+
+      execute_command (command, from_tty);
+    }
+  CATCH (exception, RETURN_MASK_ALL)
+    {
+      warning (_("Could not enable record %s: %s"), auto_record_string,
+	       exception.message);
+    }
+  END_CATCH
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_record;
 
@@ -856,7 +907,20 @@ The number of functions to print can be defined with \"set record \
 function-call-history-size\"."),
            &record_cmdlist);
 
+  add_setshow_enum_cmd ("auto-record", class_support, auto_record_names,
+			&auto_record_string, _("\
+Set automatic recording mode."), _("\
+Show automatic recording mode."), _("\
+off     == no automatic recording.\n\
+full    == automatically start record full.\n\
+btrace  == automatically start record btrace.\n\
+bts     == automatically start record bts.\n\
+pt      == automatically start record pt."),
+			NULL, show_auto_record_string, &setlist, &showlist);
+
   /* Sync command control variables.  */
   record_insn_history_size_setshow_var = record_insn_history_size;
   record_call_history_size_setshow_var = record_call_history_size;
+
+  observer_attach_inferior_created (record_inferior_created);
 }
diff --git a/gdb/testsuite/gdb.btrace/auto-enable.exp b/gdb/testsuite/gdb.btrace/auto-enable.exp
new file mode 100644
index 0000000..1da0ca4
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/auto-enable.exp
@@ -0,0 +1,48 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2016 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/>.
+
+# check for btrace support
+if { [skip_btrace_tests] } { return -1 }
+
+# start inferior
+standard_testfile record_goto.c
+if [prepare_for_testing $testfile.exp $testfile $srcfile] {
+    return -1
+}
+
+gdb_test "show auto-record" "Auto record is \"off\"\." "show default"
+
+gdb_test_no_output "set auto-record btrace" "set btrace"
+gdb_test "show auto-record" "Auto record is \"btrace\"\." "show btrace"
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "info record" [multi_line \
+    "Active record target: record-btrace" \
+    "Recording format: .*" \
+    ] "recording"
+
+gdb_test_no_output "set auto-record off" "set off"
+gdb_test "show auto-record" "Auto record is \"off\"\." "show off"
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "info record" "No record target is currently active\." "not recording"
-- 
1.8.3.1

RE: [PATCH] record: automatically start recording

[Metzger, Markus T]

ping.

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Tuesday, April 5, 2016 4:34 PM
> To: gdb-patches@sourceware.org
> Cc: palves@redhat.com; ak@linux.intel.com; jan.kratochvil@redhat.com
> Subject: [PATCH] record: automatically start recording
> 
> When using process record and replay frequently, one sometimes forgets to start
> recording after re-run.  This can be quite annoying.
> 
> I tried to automatically start recording for every new inferior with existing
> GDB commands.  I came up with:
> 
>     (gdb) b _start
>     Breakpoint 1 at 0x400560
>     (gdb) command 1
>     Type commands for breakpoint(s) 1, one per line.
>     End with a line saying just "end".
>     >record btrace
>     >cont
>     >end
> 
> This may not be completely obvious to everybody and you can't easily put it into
> your gdbinit.  It also doesn't support the attach case (not sure that matters).
> 
> This patch adds a new option "auto-record" to automatically start recording for
> every new inferior using GDB's inferior-created observer.
> 
> Is the added convenience worth a new option or do we want to point users to
> the
> breakpoint-command solution?
> 
> 2016-04-05  Markus Metzger  <markus.t.metzger@intel.com>
> ---
>  gdb/NEWS                                 |  3 ++
>  gdb/doc/gdb.texinfo                      | 12 ++++++
>  gdb/record.c                             | 64 ++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.btrace/auto-enable.exp | 48 ++++++++++++++++++++++++
>  4 files changed, 127 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.btrace/auto-enable.exp
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 39d99b7..79f27b0 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -37,6 +37,9 @@ skip -rfunction regular-expression
>  maint info line-table REGEXP
>    Display the contents of GDB's internal line table data struture.
> 
> +set|show auto-record
> +  Automatically record newly created processes.
> +
>  * Support for tracepoints and fast tracepoints on s390-linux and s390x-linux
>    was added in GDBserver, including JIT compiling fast tracepoint's
>    conditional expression bytecode into native code.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7abd55e..7d46a95 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6681,6 +6681,18 @@ the asynchronous execution mode
> (@pxref{Background Execution}), not
>  all recording methods are available.  The @code{full} recording method
>  does not support these two modes.
> 
> +@kindex set auto-record
> +@item set auto-record @var{method}
> +Automatically record newly created processes using the recording
> +method @var{method}.  If @var{method} is @code{off}, automatic
> +recording is disabled.
> +
> +@kindex show auto-record
> +@item show auto-record
> +Show the recoding method used for automatically recording newly
> +created processes.  If automatic recording is disabled, the recording
> +method is @code{off}.
> +
>  @kindex record stop
>  @kindex rec s
>  @item record stop
> diff --git a/gdb/record.c b/gdb/record.c
> index 6190794..13a451a 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -53,10 +53,37 @@ struct cmd_list_element *set_record_cmdlist = NULL;
>  struct cmd_list_element *show_record_cmdlist = NULL;
>  struct cmd_list_element *info_record_cmdlist = NULL;
> 
> +/* Supported names for the "set|show auto-record" command.  */
> +static const char auto_record_off[] = "off";
> +static const char auto_record_full[] = "full";
> +static const char auto_record_btrace[] = "btrace";
> +static const char auto_record_bts[] = "bts";
> +static const char auto_record_pt[] = "pt";
> +static const char *const auto_record_names[] =
> +{
> +  auto_record_off,
> +  auto_record_full,
> +  auto_record_btrace,
> +  auto_record_bts,
> +  auto_record_pt,
> +  NULL,
> +};
> +
> +static const char *auto_record_string = auto_record_off;
> +
>  #define DEBUG(msg, args...)						\
>    if (record_debug)							\
>      fprintf_unfiltered (gdb_stdlog, "record: " msg "\n", ##args)
> 
> +/* The "show auto-record" command.  */
> +
> +static void
> +show_auto_record_string (struct ui_file *file, int from_tty,
> +			 struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file, _("Auto record is \"%s\".\n"),  value);
> +}
> +
>  /* See record.h.  */
> 
>  struct target_ops *
> @@ -733,6 +760,30 @@ set_record_call_history_size (char *args, int from_tty,
>  			 &record_call_history_size);
>  }
> 
> +/* The inferior-created observer implementing "set auto-record".  */
> +
> +static void
> +record_inferior_created (struct target_ops *ops, int from_tty)
> +{
> +  if (auto_record_string == auto_record_off)
> +    return;
> +
> +  TRY
> +    {
> +      char command[64];
> +
> +      snprintf (command, sizeof(command), "record %s", auto_record_string);
> +
> +      execute_command (command, from_tty);
> +    }
> +  CATCH (exception, RETURN_MASK_ALL)
> +    {
> +      warning (_("Could not enable record %s: %s"), auto_record_string,
> +	       exception.message);
> +    }
> +  END_CATCH
> +}
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_record;
> 
> @@ -856,7 +907,20 @@ The number of functions to print can be defined with
> \"set record \
>  function-call-history-size\"."),
>             &record_cmdlist);
> 
> +  add_setshow_enum_cmd ("auto-record", class_support, auto_record_names,
> +			&auto_record_string, _("\
> +Set automatic recording mode."), _("\
> +Show automatic recording mode."), _("\
> +off     == no automatic recording.\n\
> +full    == automatically start record full.\n\
> +btrace  == automatically start record btrace.\n\
> +bts     == automatically start record bts.\n\
> +pt      == automatically start record pt."),
> +			NULL, show_auto_record_string, &setlist, &showlist);
> +
>    /* Sync command control variables.  */
>    record_insn_history_size_setshow_var = record_insn_history_size;
>    record_call_history_size_setshow_var = record_call_history_size;
> +
> +  observer_attach_inferior_created (record_inferior_created);
>  }
> diff --git a/gdb/testsuite/gdb.btrace/auto-enable.exp
> b/gdb/testsuite/gdb.btrace/auto-enable.exp
> new file mode 100644
> index 0000000..1da0ca4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.btrace/auto-enable.exp
> @@ -0,0 +1,48 @@
> +# This testcase is part of GDB, the GNU debugger.
> +#
> +# Copyright 2016 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/>.
> +
> +# check for btrace support
> +if { [skip_btrace_tests] } { return -1 }
> +
> +# start inferior
> +standard_testfile record_goto.c
> +if [prepare_for_testing $testfile.exp $testfile $srcfile] {
> +    return -1
> +}
> +
> +gdb_test "show auto-record" "Auto record is \"off\"\." "show default"
> +
> +gdb_test_no_output "set auto-record btrace" "set btrace"
> +gdb_test "show auto-record" "Auto record is \"btrace\"\." "show btrace"
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test "info record" [multi_line \
> +    "Active record target: record-btrace" \
> +    "Recording format: .*" \
> +    ] "recording"
> +
> +gdb_test_no_output "set auto-record off" "set off"
> +gdb_test "show auto-record" "Auto record is \"off\"\." "show off"
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test "info record" "No record target is currently active\." "not recording"
> --
> 1.8.3.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] record: automatically start recording

[Eli Zaretskii]

> From: "Metzger, Markus T" <markus.t.metzger@intel.com>
> CC: "palves@redhat.com" <palves@redhat.com>, "ak@linux.intel.com"	<ak@linux.intel.com>, "jan.kratochvil@redhat.com" <jan.kratochvil@redhat.com>
> Date: Mon, 9 May 2016 07:47:22 +0000
> 
> ping.

Sorry for missing the original submission.

The documentation parts are okay, with one comment: please state what
is the default value of this option.

Thanks.

RE: [PATCH] record: automatically start recording

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Eli Zaretskii
> Sent: Monday, May 9, 2016 6:19 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org; palves@redhat.com; ak@linux.intel.com;
> jan.kratochvil@redhat.com
> Subject: Re: [PATCH] record: automatically start recording


> The documentation parts are okay, with one comment: please state what
> is the default value of this option.

Thanks for the quick response.  Will do.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] record: automatically start recording

[Yao Qi]

Markus Metzger <markus.t.metzger@intel.com> writes:

Hi Markus,

> Is the added convenience worth a new option or do we want to point users to the
> breakpoint-command solution?
>

I don't think it can justify adding a option in GDB to do that, but I am
open to others' thought.  If people think it is useful to have such
option, see my comments below on the patch,

> +/* The inferior-created observer implementing "set auto-record".  */
> +
> +static void
> +record_inferior_created (struct target_ops *ops, int from_tty)
> +{
> +  if (auto_record_string == auto_record_off)
> +    return;
> +
> +  TRY
> +    {
> +      char command[64];
> +
> +      snprintf (command, sizeof(command), "record %s", auto_record_string);
> +

xsnprintf

> +      execute_command (command, from_tty);
> +    }
> +  CATCH (exception, RETURN_MASK_ALL)
> +    {
> +      warning (_("Could not enable record %s: %s"), auto_record_string,
> +	       exception.message);
> +    }
> +  END_CATCH
> +}
> +

> +
> +# check for btrace support
> +if { [skip_btrace_tests] } { return -1 }
> +

Can we test this option on the target without btrace?  I think we can.
The command/option itself is about restarting recording automatically,
but the record type (full/btrace/bts/pt/) doesn't matter.

-- 
Yao (齐尧)

RE: [PATCH] record: automatically start recording

[Metzger, Markus T]

> -----Original Message-----
> From: Yao Qi [mailto:qiyaoltc@gmail.com]
> Sent: Thursday, May 12, 2016 3:14 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org; palves@redhat.com; ak@linux.intel.com;
> jan.kratochvil@redhat.com
> Subject: Re: [PATCH] record: automatically start recording

Hi Yao,

Thanks for your review.  I changed the test to use "record full" and moved it
into testsuite/gdb.reverse.


> > Is the added convenience worth a new option or do we want to point users to
> the
> > breakpoint-command solution?
> >
> 
> I don't think it can justify adding a option in GDB to do that, but I am
> open to others' thought.  If people think it is useful to have such
> option, see my comments below on the patch,

I get such a request once in a while but I'm not sure about this myself.
Let's hear what others think.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928