[PATCH] btrace: continue recording after connection loss.

[Tim Wiederhake <tim dot wiederhake at intel dot com>]

If the connection between gdb and gdbserver drops unexpectedly, gdb
gets confused about whether a record target is active or not.

Current behavior (after connection loss and reconnect):
  (gdb) record btrace
  [record-btrace] open
  [record-btrace] open
  [btrace] enable thread 1 (Thread 23143)
  Sending packet: $Qbtrace-conf:bts:size=0x10000#e3...Packet received: OK
  Sending packet: $Qbtrace:bts#45...Packet received: E.Btrace already enabled.
  Could not enable branch tracing for Thread 23143: Btrace already enabled.
  (gdb) info record
  No record target is currently active.
  (gdb)

With this patch, gdb checks for running records on connect, pushes the record
target if necessary and allows to resume the recording.

2016-06-09  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/
	* record-btrace.c (record_btrace_open): Split into this and ...
	(record_btrace_push_target): ... this.
	* record-btrace.h: New file.
	* remote.c (remote_start_remote): Check recording status on connection
	and resync with running recording.
	(remote_btrace_read_config): Does not throw any more.
	(remote_btrace_maybe_reopen): New. Check for resumable recordings and
	resume if possible.
	(remote_enable_btrace): Account for remote_btrace_read_config not
	throwing anymore.

gdb/testsuite/

	* gdb.btrace/reconnect.c: New file.
	* gdb.btrace/reconnect.exp: New file.
---
 gdb/record-btrace.c                    |  29 ++++++---
 gdb/record-btrace.h                    |  28 +++++++++
 gdb/remote.c                           | 109 ++++++++++++++++++++++++---------
 gdb/testsuite/gdb.btrace/reconnect.c   |  25 ++++++++
 gdb/testsuite/gdb.btrace/reconnect.exp |  63 +++++++++++++++++++
 5 files changed, 214 insertions(+), 40 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/record-btrace.c b/gdb/record-btrace.c
index 77b5180..4331709 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,23 @@ 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)
+{
+  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;
+
+  observer_notify_record_changed (current_inferior (),  1);
+}
+
 /* The to_open method of target record-btrace.  */
 
 static void
@@ -225,16 +243,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;
-
-  observer_notify_record_changed (current_inferior (),  1);
+  record_btrace_push_target ();
 
   discard_cleanups (disable_chain);
 }
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 1f0d67c..0bb69c8 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
@@ -12667,6 +12674,77 @@ remote_btrace_reset (void)
   memset (&rs->btrace_config, 0, sizeof (rs->btrace_config));
 }
 
+/* Read the current thread's btrace configuration from the target and
+   store it into CONF.  Returns -1 on failure.  */
+
+static int
+remote_btrace_read_config (struct btrace_config *conf)
+{
+  int ret = 0;
+  char *xml = target_read_stralloc (&current_target,
+				    TARGET_OBJECT_BTRACE_CONF, "");
+
+  if (xml == NULL)
+    return -1;
+
+  TRY
+    {
+      parse_xml_btrace_conf (conf, xml);
+    }
+  CATCH (err, RETURN_MASK_ERROR)
+    {
+      if (err.message != NULL)
+	warning ("%s", err.message);
+      ret = -1;
+    }
+  END_CATCH
+
+  xfree (xml);
+  return ret;
+}
+
+/* Maybe reopen target btrace.  */
+
+static void
+remote_btrace_maybe_reopen (void)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct thread_info *tp;
+  int btrace_opened = 0;
+  int warned = 0;
+
+  ALL_NON_EXITED_THREADS (tp)
+    {
+      set_general_thread (tp->ptid);
+
+      if (remote_btrace_read_config (&rs->btrace_config) != -1)
+	{
+	  #if !defined (HAVE_LIBIPT)
+	    if (rs->btrace_config.format == BTRACE_FORMAT_PT)
+	      {
+		if (!warned)
+		  {
+		    warning (_("GDB does not support Intel Processor Trace."));
+		    warned = 1;
+		  }
+
+		continue;
+	      }
+
+	  #endif /* !defined (HAVE_LIBIPT) */
+
+	  btrace_opened = 1;
+
+	  tp->btrace.target = XCNEW (struct btrace_target_info);
+	  tp->btrace.target->ptid = tp->ptid;
+	  tp->btrace.target->conf = rs->btrace_config;
+	}
+    }
+
+  if (btrace_opened)
+    record_btrace_push_target ();
+}
+
 /* Check whether the target supports branch tracing.  */
 
 static int
@@ -12756,26 +12834,6 @@ btrace_sync_conf (const struct btrace_config *conf)
     }
 }
 
-/* Read the current thread's btrace configuration from the target and
-   store it into CONF.  */
-
-static void
-btrace_read_config (struct btrace_config *conf)
-{
-  char *xml;
-
-  xml = target_read_stralloc (&current_target,
-			      TARGET_OBJECT_BTRACE_CONF, "");
-  if (xml != NULL)
-    {
-      struct cleanup *cleanup;
-
-      cleanup = make_cleanup (xfree, xml);
-      parse_xml_btrace_conf (conf, xml);
-      do_cleanups (cleanup);
-    }
-}
-
 /* Enable branch tracing.  */
 
 static struct btrace_target_info *
@@ -12825,16 +12883,7 @@ remote_enable_btrace (struct target_ops *self, ptid_t ptid,
 
   /* If we fail to read the configuration, we lose some information, but the
      tracing itself is not impacted.  */
-  TRY
-    {
-      btrace_read_config (&tinfo->conf);
-    }
-  CATCH (err, RETURN_MASK_ERROR)
-    {
-      if (err.message != NULL)
-	warning ("%s", err.message);
-    }
-  END_CATCH
+  remote_btrace_read_config (&tinfo->conf);
 
   return tinfo;
 }
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..7e321ae
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/reconnect.exp
@@ -0,0 +1,63 @@
+# 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
+}
+
+# Start gdbserver.
+set gdbserver_reconnect_p 1
+set res [gdbserver_start "" $binfile]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+
+# Begin btrace.
+gdb_start
+gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+gdb_test_no_output "record btrace" "record btrace enable"
+
+set bp_location [gdb_get_line_number "return" $testfile.c]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "cont to $bp_location" ".*$testfile.c:$bp_location.*"
+
+# Simulate connection loss by killing gdb.
+set gdb_pid [exp_pid -i [board_info host fileid]]
+exec kill "-KILL" ${gdb_pid}
+
+# Cleanup and restart gdb.
+gdb_exit
+gdb_start
+gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+
+# Enabling btrace should not be possible here since gdbserver is
+# already recording.  Check for the correct error message.
+gdb_test "record btrace" "The process is already being recorded\\.  Use \"record stop\" to stop recording first\\." "record btrace re-enable"
+
+# Test if we can access the recorded data from before the connection
+# loss.
+gdb_test "info record" [multi_line \
+  "Active record target: .*" \
+  "Recorded $decimal instructions in .$decimal functions \\($decimal gaps\\) for thread 1 \\(Thread .*\\)."
+]
-- 
2.7.4