[PATCH v4 0/1] btrace: Resume recording after disconnect.

[PATCH v4 0/1] btrace: Resume recording after disconnect.

[Tim Wiederhake]

Hi,

This is v4 of
https://sourceware.org/ml/gdb-patches/2016-06/msg00165.html
which fixes
https://sourceware.org/ml/gdb-patches/2015-11/msg00420.html
and includes the advice (thanks!) in:
https://sourceware.org/ml/gdb-patches/2016-07/msg00263.html
https://sourceware.org/ml/gdb-patches/2016-07/msg00284.html

- Tim


*** BLURB HERE ***

Tim Wiederhake (1):
  btrace: Resume recording after disconnect.

 gdb/NEWS                               |  3 ++
 gdb/doc/gdb.texinfo                    |  4 +-
 gdb/record-btrace.c                    | 51 ++++++++++++++++------
 gdb/record-btrace.h                    | 28 ++++++++++++
 gdb/remote.c                           | 61 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.btrace/reconnect.c   | 25 +++++++++++
 gdb/testsuite/gdb.btrace/reconnect.exp | 79 ++++++++++++++++++++++++++++++++++
 7 files changed, 237 insertions(+), 14 deletions(-)
 create mode 100644 gdb/record-btrace.h
 create mode 100644 gdb/testsuite/gdb.btrace/reconnect.c
 create mode 100644 gdb/testsuite/gdb.btrace/reconnect.exp

-- 
2.7.4

[PATCH v4 1/1] btrace: Resume recording after disconnect.

[Tim Wiederhake]

This patch allows gdbserver to continue recording after disconnect.  On
reconnect, the recorded data is accessible to gdb as if no disconnect happened.

A possible application for this feature is remotely examine bugs that occur
at irregular intervals, where maintaining a gdb connection is inconvenient.

This also fixes the issue mentioned here:
https://sourceware.org/ml/gdb-patches/2015-11/msg00424.html

2016-07-22  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog:
	* NEWS: Resume btrace on reconnect.
	* record-btrace.c: Added record-btrace.h include.
	(record_btrace_open): Split into this and ...
	(record_btrace_push_target): ... this.
	(record_btrace_disconnect): New function.
	(init_record_btrace_ops): Use record_btrace_disconnect.
	* record-btrace.h: New file.
	* remote.c: Added record-btrace.h include.
	(remote_start_remote): Check recording status.
	(remote_btrace_maybe_reopen): New function.

gdb/doc/ChangeLog:
	* gdb.texinfo: Resume btrace on reconnect.

gdb/testsuite/ChangeLog:

	* gdb.btrace/reconnect.c: New file.
	* gdb.btrace/reconnect.exp: New file.


---
 gdb/NEWS                               |  3 ++
 gdb/doc/gdb.texinfo                    |  4 +-
 gdb/record-btrace.c                    | 51 ++++++++++++++++------
 gdb/record-btrace.h                    | 28 ++++++++++++
 gdb/remote.c                           | 61 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.btrace/reconnect.c   | 25 +++++++++++
 gdb/testsuite/gdb.btrace/reconnect.exp | 79 ++++++++++++++++++++++++++++++++++
 7 files changed, 237 insertions(+), 14 deletions(-)
 create mode 100644 gdb/record-btrace.h
 create mode 100644 gdb/testsuite/gdb.btrace/reconnect.c
 create mode 100644 gdb/testsuite/gdb.btrace/reconnect.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index c29e69a..721bed8 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 7.11
 
+* GDBserver now supports recording btrace without maintaining an active
+  GDB connection.
+
 * GDB now supports a negative repeat count in the 'x' command to examine
   memory backward from the given address.  For example:
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a068622..6e2cebf 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6631,7 +6631,9 @@ Hardware-supported instruction recording.  This method does not record
 data.  Further, the data is collected in a ring buffer so old data will
 be overwritten when the buffer is full.  It allows limited reverse
 execution.  Variables and registers are not available during reverse
-execution.
+execution.  In remote debugging, recording continues on disconnect.
+Recorded data can be inspected after reconnecting.  The recording may
+be stopped using @code{record stop}.
 
 The recording format can be specified as parameter.  Without a parameter
 the command chooses the recording format.  The following recording
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 24594a9..4a51b8e 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -21,6 +21,7 @@
 
 #include "defs.h"
 #include "record.h"
+#include "record-btrace.h"
 #include "gdbthread.h"
 #include "target.h"
 #include "gdbcmd.h"
@@ -199,6 +200,26 @@ record_btrace_handle_async_inferior_event (gdb_client_data data)
   inferior_event_handler (INF_REG_EVENT, NULL);
 }
 
+/* See record-btrace.h.  */
+
+void
+record_btrace_push_target (void)
+{
+  const char *format;
+
+  record_btrace_auto_enable ();
+
+  push_target (&record_btrace_ops);
+
+  record_btrace_async_inferior_event_handler
+    = create_async_event_handler (record_btrace_handle_async_inferior_event,
+				  NULL);
+  record_btrace_generating_corefile = 0;
+
+  format = btrace_format_short_string (record_btrace_conf.format);
+  observer_notify_record_changed (current_inferior (), 1, "btrace", format);
+}
+
 /* The to_open method of target record-btrace.  */
 
 static void
@@ -206,7 +227,6 @@ record_btrace_open (const char *args, int from_tty)
 {
   struct cleanup *disable_chain;
   struct thread_info *tp;
-  const char *format;
 
   DEBUG ("open");
 
@@ -226,17 +246,7 @@ record_btrace_open (const char *args, int from_tty)
 	make_cleanup (record_btrace_disable_callback, tp);
       }
 
-  record_btrace_auto_enable ();
-
-  push_target (&record_btrace_ops);
-
-  record_btrace_async_inferior_event_handler
-    = create_async_event_handler (record_btrace_handle_async_inferior_event,
-				  NULL);
-  record_btrace_generating_corefile = 0;
-
-  format = btrace_format_short_string (record_btrace_conf.format);
-  observer_notify_record_changed (current_inferior (), 1, "btrace", format);
+  record_btrace_push_target ();
 
   discard_cleanups (disable_chain);
 }
@@ -257,6 +267,21 @@ record_btrace_stop_recording (struct target_ops *self)
       btrace_disable (tp);
 }
 
+/* The to_disconnect method of target record-btrace.  */
+
+static void
+record_btrace_disconnect (struct target_ops *self, const char *args,
+			  int from_tty)
+{
+  struct target_ops *beneath = self->beneath;
+
+  /* Do not stop recording, just clean up GDB side.  */
+  unpush_target (self);
+
+  /* Forward disconnect.  */
+  beneath->to_disconnect (beneath, args, from_tty);
+}
+
 /* The to_close method of target record-btrace.  */
 
 static void
@@ -2824,7 +2849,7 @@ init_record_btrace_ops (void)
   ops->to_close = record_btrace_close;
   ops->to_async = record_btrace_async;
   ops->to_detach = record_detach;
-  ops->to_disconnect = record_disconnect;
+  ops->to_disconnect = record_btrace_disconnect;
   ops->to_mourn_inferior = record_mourn_inferior;
   ops->to_kill = record_kill;
   ops->to_stop_recording = record_btrace_stop_recording;
diff --git a/gdb/record-btrace.h b/gdb/record-btrace.h
new file mode 100644
index 0000000..d2062b8
--- /dev/null
+++ b/gdb/record-btrace.h
@@ -0,0 +1,28 @@
+/* Branch trace support for GDB, the GNU debugger.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   Contributed by Intel Corp. <tim.wiederhake@intel.com>
+
+   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/>.  */
+
+#ifndef RECORD_BTRACE_H
+#define RECORD_BTRACE_H
+
+/* Push the record_btrace target.  */
+extern void record_btrace_push_target (void);
+
+#endif /* RECORD_BTRACE_H */
diff --git a/gdb/remote.c b/gdb/remote.c
index 5568346..7944983 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -70,6 +70,7 @@
 #include "ax-gdb.h"
 #include "agent.h"
 #include "btrace.h"
+#include "record-btrace.h"
 
 /* Temp hacks for tracepoint encoding migration.  */
 static char *target_buf;
@@ -231,6 +232,8 @@ static int remote_can_run_breakpoint_commands (struct target_ops *self);
 
 static void remote_btrace_reset (void);
 
+static void remote_btrace_maybe_reopen (void);
+
 static int stop_reply_queue_length (void);
 
 static void readahead_cache_invalidate (void);
@@ -4298,6 +4301,10 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
       merge_uploaded_tracepoints (&uploaded_tps);
     }
 
+  /* Possibly the target has been engaged in a btrace record started
+     previously; find out where things are at.  */
+  remote_btrace_maybe_reopen ();
+
   /* The thread and inferior lists are now synchronized with the
      target, our symbols have been relocated, and we're merged the
      target's tracepoints with ours.  We're done with basic start
@@ -12774,6 +12781,60 @@ btrace_read_config (struct btrace_config *conf)
     }
 }
 
+/* Maybe reopen target btrace.  */
+
+static void
+remote_btrace_maybe_reopen (void)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct cleanup *cleanup;
+  struct thread_info *tp;
+  int btrace_target_pushed = 0;
+  int warned = 0;
+
+  cleanup = make_cleanup_restore_current_thread ();
+  ALL_NON_EXITED_THREADS (tp)
+    {
+      set_general_thread (tp->ptid);
+
+      memset (&rs->btrace_config, 0x00, sizeof (struct btrace_config));
+      btrace_read_config (&rs->btrace_config);
+
+      if (rs->btrace_config.format == BTRACE_FORMAT_NONE)
+	continue;
+
+#if !defined (HAVE_LIBIPT)
+      if (rs->btrace_config.format == BTRACE_FORMAT_PT)
+	{
+	  if (!warned)
+	    {
+	      warned = 1;
+	      warning (_("GDB does not support Intel Processor Trace. "
+			 "\"record\" will not work in this session."));
+	    }
+
+	  continue;
+	}
+#endif /* !defined (HAVE_LIBIPT) */
+
+      /* Push target, once, but before anything else happens.  This way our
+	 changes to the threads will be cleaned up by unpushing the target
+	 in case btrace_read_config () throws.  */
+      if (!btrace_target_pushed)
+	{
+	  btrace_target_pushed = 1;
+	  record_btrace_push_target ();
+	  printf_filtered (_("Target is recording using %s.\n"),
+			   btrace_format_string (rs->btrace_config.format));
+	}
+
+      tp->btrace.target = XCNEW (struct btrace_target_info);
+      tp->btrace.target->ptid = tp->ptid;
+      tp->btrace.target->conf = rs->btrace_config;
+    }
+  do_cleanups (cleanup);
+}
+
 /* Enable branch tracing.  */
 
 static struct btrace_target_info *
diff --git a/gdb/testsuite/gdb.btrace/reconnect.c b/gdb/testsuite/gdb.btrace/reconnect.c
new file mode 100644
index 0000000..f2a9d35
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/reconnect.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   Contributed by Intel Corp. <tim.wiederhake@intel.com>
+
+   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/>.  */
+
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.btrace/reconnect.exp b/gdb/testsuite/gdb.btrace/reconnect.exp
new file mode 100644
index 0000000..485a4df
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/reconnect.exp
@@ -0,0 +1,79 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2016 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <tim.wiederhake@intel.com>
+#
+# 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/>.
+
+load_lib gdbserver-support.exp
+
+if { [skip_btrace_tests] } { return -1 }
+if { [skip_gdbserver_tests] } { return -1 }
+
+standard_testfile
+if [prepare_for_testing $testfile.exp $testfile $srcfile] {
+    return -1
+}
+
+# Make sure we're disconnected and no recording is active, in case
+# we're testing with an extended-remote board, therefore already
+# connected.
+with_test_prefix "preparation" {
+  gdb_test "record stop" ".*"
+  gdb_test "disconnect" ".*"
+}
+
+# Start fresh gdbserver.
+set gdbserver_reconnect_p 1
+set res [gdbserver_start "" $binfile]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+
+# Create a record, check, reconnect
+with_test_prefix "first" {
+  gdb_test_no_output "record btrace" "record btrace enable"
+  gdb_test "stepi 19" "0x.* in .* from target.*"
+
+  gdb_test "info record" [multi_line \
+    "Active record target: .*" \
+    "Recorded 19 instructions in .+ functions \\(. gaps\\) for thread 1 \\(Thread .*\\)."
+  ]
+
+  gdb_test "disconnect" "Ending remote debugging."
+  gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+}
+
+# Test if we can access the recorded data from first connect.
+# Note: BTS loses the first function call entry with its associated
+# instructions for technical reasons.  This is why we test for
+# "a number between 10 and 19", so we catch at least the case where
+# there are 0 instructions in the record.
+with_test_prefix "second" {
+  gdb_test "info record" [multi_line \
+    "Active record target: .*" \
+    "Recorded 1. instructions in .+ functions \\(. gaps\\) for thread 1 \\(Thread .*\\)."
+  ]
+
+  gdb_test "record stop" "Process record is stopped and all execution logs are deleted."
+
+  gdb_test "disconnect" "Ending remote debugging."
+  gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+}
+
+# Test that recording is now off.
+with_test_prefix "third" {
+  gdb_test "info record" "No record target is currently active."
+}
-- 
2.7.4

Re: [PATCH v4 0/1] btrace: Resume recording after disconnect.

[Pedro Alves]

I have no further comments.

Thanks,
Pedro Alves

RE: [PATCH v4 0/1] btrace: Resume recording after disconnect.

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, July 22, 2016 1:42 PM
> To: Wiederhake, Tim <tim.wiederhake@intel.com>; gdb-
> patches@sourceware.org
> Cc: Metzger, Markus T <markus.t.metzger@intel.com>
> Subject: Re: [PATCH v4 0/1] btrace: Resume recording after disconnect.
> 
> I have no further comments.

The patch looks good to me, as well.  Thanks for reviewing the patch,
Pedro, and for point out the exceptions issue.

AFAIK Tim does not yet have write-access to the GDB repo.  Do you maybe
know where he can apply for it?

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 v4 0/1] btrace: Resume recording after disconnect.

[Pedro Alves]

On 07/22/2016 01:40 PM, Metzger, Markus T wrote:

> AFAIK Tim does not yet have write-access to the GDB repo.  Do you maybe
> know where he can apply for it?

https://sourceware.org/ -> "I need an account on sourceware for
git/svn/cvs write access to project XYZ?"

Feel free to list me as sponsor.

Thanks,
Pedro Alves

RE: [PATCH v4 0/1] btrace: Resume recording after disconnect.

[Wiederhake, Tim]

Thanks!

I just submitted the request.

- Tim

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, July 22, 2016 2:43 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; Wiederhake, Tim
> <tim.wiederhake@intel.com>; gdb-patches@sourceware.org
> Subject: Re: [PATCH v4 0/1] btrace: Resume recording after disconnect.
> 
> On 07/22/2016 01:40 PM, Metzger, Markus T wrote:
> 
> > AFAIK Tim does not yet have write-access to the GDB repo.  Do you maybe
> > know where he can apply for it?
> 
> https://sourceware.org/ -> "I need an account on sourceware for
> git/svn/cvs write access to project XYZ?"
> 
> Feel free to list me as sponsor.
> 
> Thanks,
> Pedro Alves

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