[patch v10 21/21] btrace: fix crash when losing the remote connection on process exit

[Markus Metzger <markus dot t dot metzger at intel dot com>]

Fix a crash reported by Jan Kratochvil.

When a branch traced process exits in a remote debug scenario and we try to
disable branch tracing, we would realize that the target connection is gone and
thus pop all targets.
This causes another attempt to disable branch tracing when we discard all
threads, thus crashing GDB.

This patch adds another path to disable branch tracing that is used when freeing
the resources for a thread.  Branch tracing is disabled normally on native
targets - except that errors are silently ignored.  For remote targets, the
resources are freed when the threads are discarded.

We further split to_close into to_stop_recording and to_close so we avdoid
making any target access in to_close.  In fact, to_close is empty, now.

The patch will be split and merged into the btrace patch series.


2013-03-08  Markus Metzger <markus.t.metzger@intel.com>
---
 gdb/amd64-linux-nat.c |   10 ++++++++
 gdb/btrace.c          |   19 +++++++++++++++
 gdb/btrace.h          |    5 ++++
 gdb/i386-linux-nat.c  |   10 ++++++++
 gdb/record-btrace.c   |   22 ++++++++++-------
 gdb/record.c          |   61 ++++++++++++++++++++++++++++++++++--------------
 gdb/remote.c          |   10 ++++++++
 gdb/target.c          |   31 +++++++++++++++++++++++++
 gdb/target.h          |   15 ++++++++++++
 gdb/thread.c          |    2 +-
 10 files changed, 157 insertions(+), 28 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 6f13fca..8dfe7c5 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -1154,6 +1154,15 @@ amd64_linux_disable_btrace (struct btrace_target_info *tinfo)
     error (_("Could not disable branch tracing: %s."), safe_strerror (errcode));
 }
 
+/* Teardown branch tracing.  */
+
+static void
+amd64_linux_teardown_btrace (struct btrace_target_info *tinfo)
+{
+  /* Ignore errors.  */
+  linux_disable_btrace (tinfo);
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_amd64_linux_nat (void);
 
@@ -1196,6 +1205,7 @@ _initialize_amd64_linux_nat (void)
   t->to_supports_btrace = linux_supports_btrace;
   t->to_enable_btrace = amd64_linux_enable_btrace;
   t->to_disable_btrace = amd64_linux_disable_btrace;
+  t->to_teardown_btrace = amd64_linux_teardown_btrace;
   t->to_read_btrace = linux_read_btrace;
 
   /* Register the target.  */
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 75ede3a..c39a32d 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -371,6 +371,25 @@ btrace_disable (struct thread_info *tp)
 /* See btrace.h.  */
 
 void
+btrace_teardown (struct thread_info *tp)
+{
+  struct btrace_thread_info *btp = &tp->btrace;
+  int errcode = 0;
+
+  if (btp->target == NULL)
+    return;
+
+  DEBUG ("teardown thread %d (%s)", tp->num, target_pid_to_str (tp->ptid));
+
+  target_teardown_btrace (btp->target);
+  btp->target = NULL;
+
+  btrace_clear (tp);
+}
+
+/* See btrace.h.  */
+
+void
 btrace_fetch (struct thread_info *tp)
 {
   struct btrace_thread_info *btinfo;
diff --git a/gdb/btrace.h b/gdb/btrace.h
index 35fb13a..bd8425d 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -122,6 +122,11 @@ extern void btrace_enable (struct thread_info *tp);
    This will also delete the current branch trace data.  */
 extern void btrace_disable (struct thread_info *);
 
+/* Disable branch tracing for a thread during teardown.
+   This is similar to btrace_disable, except that it will use
+   target_teardown_btrace instead of target_disable_btrace.  */
+extern void btrace_teardown (struct thread_info *);
+
 /* Fetch the branch trace for a single thread.  */
 extern void btrace_fetch (struct thread_info *);
 
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 715c6d4..be2b6c9 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -1081,6 +1081,15 @@ i386_linux_disable_btrace (struct btrace_target_info *tinfo)
     error (_("Could not disable branch tracing: %s."), safe_strerror (errcode));
 }
 
+/* Teardown branch tracing.  */
+
+static void
+i386_linux_teardown_btrace (struct btrace_target_info *tinfo)
+{
+  /* Ignore errors.  */
+  linux_disable_btrace (tinfo);
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_i386_linux_nat;
 
@@ -1118,6 +1127,7 @@ _initialize_i386_linux_nat (void)
   t->to_supports_btrace = linux_supports_btrace;
   t->to_enable_btrace = i386_linux_enable_btrace;
   t->to_disable_btrace = i386_linux_disable_btrace;
+  t->to_teardown_btrace = i386_linux_teardown_btrace;
   t->to_read_btrace = linux_read_btrace;
 
   /* Register the target.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 66b4a3d..bbb0bd5 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -99,16 +99,11 @@ record_btrace_enable_warn (struct thread_info *tp)
 static void
 record_btrace_disable_callback (void *arg)
 {
-  volatile struct gdb_exception error;
   struct thread_info *tp;
 
   tp = arg;
 
-  TRY_CATCH (error, RETURN_MASK_ERROR)
-    btrace_disable (tp);
-
-  if (error.message != NULL)
-    warning ("%s", error.message);
+  btrace_disable (tp);
 }
 
 /* Enable automatic tracing of new threads.  */
@@ -176,14 +171,14 @@ record_btrace_open (char *args, int from_tty)
   discard_cleanups (disable_chain);
 }
 
-/* The to_close method of target record-btrace.  */
+/* The to_stop_recording method of target record-btrace.  */
 
 static void
-record_btrace_close (int quitting)
+record_btrace_stop_recording (void)
 {
   struct thread_info *tp;
 
-  DEBUG ("close");
+  DEBUG ("stop recording");
 
   record_btrace_auto_disable ();
 
@@ -192,6 +187,14 @@ record_btrace_close (int quitting)
       btrace_disable (tp);
 }
 
+/* The to_close method of target record-btrace.  */
+
+static void
+record_btrace_close (int quitting)
+{
+  /* We already stopped recording.  */
+}
+
 /* The to_info_record method of target record-btrace.  */
 
 static void
@@ -649,6 +652,7 @@ init_record_btrace_ops (void)
   ops->to_mourn_inferior = record_mourn_inferior;
   ops->to_kill = record_kill;
   ops->to_create_inferior = find_default_create_inferior;
+  ops->to_stop_recording = record_btrace_stop_recording;
   ops->to_info_record = record_btrace_info;
   ops->to_insn_history = record_btrace_insn_history;
   ops->to_insn_history_from = record_btrace_insn_history_from;
diff --git a/gdb/record.c b/gdb/record.c
index 83f4e1b..2426885 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -92,17 +92,22 @@ record_read_memory (struct gdbarch *gdbarch,
   return ret;
 }
 
-/* Unpush the record target.  */
+/* Stop recording.  */
 
 static void
-record_unpush (void)
+record_stop (struct target_ops *t)
 {
-  struct target_ops *t;
+  DEBUG ("stop %s", t->to_shortname);
 
-  t = find_record_target ();
-  if (t == NULL)
-    internal_error (__FILE__, __LINE__, _("Couldn't find record target."));
+  if (t->to_stop_recording != NULL)
+    t->to_stop_recording ();
+}
 
+/* Unpush the record target.  */
+
+static void
+record_unpush (struct target_ops *t)
+{
   DEBUG ("unpush %s", t->to_shortname);
 
   unpush_target (t);
@@ -111,44 +116,62 @@ record_unpush (void)
 /* See record.h.  */
 
 void
-record_disconnect (struct target_ops *target, char *args, int from_tty)
+record_disconnect (struct target_ops *t, char *args, int from_tty)
 {
-  DEBUG ("disconnect");
+  gdb_assert (t->to_stratum == record_stratum);
+
+  DEBUG ("disconnect %s", t->to_shortname);
+
+  record_stop (t);
+  record_unpush (t);
 
-  record_unpush ();
   target_disconnect (args, from_tty);
 }
 
 /* See record.h.  */
 
 void
-record_detach (struct target_ops *ops, char *args, int from_tty)
+record_detach (struct target_ops *t, char *args, int from_tty)
 {
-  DEBUG ("detach");
+  gdb_assert (t->to_stratum == record_stratum);
+
+  DEBUG ("detach %s", t->to_shortname);
+
+  record_stop (t);
+  record_unpush (t);
 
-  record_unpush ();
   target_detach (args, from_tty);
 }
 
 /* See record.h.  */
 
 void
-record_mourn_inferior (struct target_ops *ops)
+record_mourn_inferior (struct target_ops *t)
 {
-  DEBUG ("mourn_inferior");
+  gdb_assert (t->to_stratum == record_stratum);
+
+  DEBUG ("mourn inferior %s", t->to_shortname);
+
+  /* It is safer to not stop recording.  Resources will be freed when
+     threads are discarded.  */
+  record_unpush (t);
 
-  record_unpush ();
   target_mourn_inferior ();
 }
 
 /* See record.h.  */
 
 void
-record_kill (struct target_ops *ops)
+record_kill (struct target_ops *t)
 {
-  DEBUG ("kill");
+  gdb_assert (t->to_stratum == record_stratum);
+
+  DEBUG ("kill %s", t->to_shortname);
+
+  /* It is safer to not stop recording.  Resources will be freed when
+     threads are discarded.  */
+  record_unpush (t);
 
-  record_unpush ();
   target_kill ();
 }
 
@@ -205,6 +228,8 @@ cmd_record_stop (char *args, int from_tty)
   struct target_ops *t;
 
   t = require_record_target ();
+
+  record_stop (t);
   unpush_target (t);
 
   printf_unfiltered (_("Process record is stopped and all execution "
diff --git a/gdb/remote.c b/gdb/remote.c
index 7ee0695..39cf5ab 100755
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11212,6 +11212,15 @@ remote_disable_btrace (struct btrace_target_info *tinfo)
   xfree (tinfo);
 }
 
+/* Teardown branch tracing.  */
+
+static void
+remote_teardown_btrace (struct btrace_target_info *tinfo)
+{
+  /* We must not talk to the target during teardown.  */
+  xfree (tinfo);
+}
+
 /* Read the branch trace.  */
 
 static VEC (btrace_block_s) *
@@ -11377,6 +11386,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_supports_btrace = remote_supports_btrace;
   remote_ops.to_enable_btrace = remote_enable_btrace;
   remote_ops.to_disable_btrace = remote_disable_btrace;
+  remote_ops.to_teardown_btrace = remote_teardown_btrace;
   remote_ops.to_read_btrace = remote_read_btrace;
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 5b5f784..2d8dc36 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4209,6 +4209,20 @@ target_disable_btrace (struct btrace_target_info *btinfo)
 
 /* See target.h.  */
 
+void
+target_teardown_btrace (struct btrace_target_info *btinfo)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    if (t->to_teardown_btrace != NULL)
+      return t->to_teardown_btrace (btinfo);
+
+  tcomplain ();
+}
+
+/* See target.h.  */
+
 VEC (btrace_block_s) *
 target_read_btrace (struct btrace_target_info *btinfo,
 		    enum btrace_read_type type)
@@ -4226,6 +4240,23 @@ target_read_btrace (struct btrace_target_info *btinfo,
 /* See target.h.  */
 
 void
+target_stop_recording (void)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    if (t->to_stop_recording != NULL)
+      {
+	t->to_stop_recording ();
+	return;
+      }
+
+  /* This is optional.  */
+}
+
+/* See target.h.  */
+
+void
 target_info_record (void)
 {
   struct target_ops *t;
diff --git a/gdb/target.h b/gdb/target.h
index 7403221..7a70e83 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -870,10 +870,19 @@ struct target_ops
     /* Disable branch tracing and deallocate @tinfo.  */
     void (*to_disable_btrace) (struct btrace_target_info *tinfo);
 
+    /* Disable branch tracing and deallocate @tinfo.  This function is similar
+       to to_disable_btrace, except that it is called during teardown and is
+       only allowed to perform actions that are safe.  A counter-example would
+       be attempting to talk to a remote target.  */
+    void (*to_teardown_btrace) (struct btrace_target_info *tinfo);
+
     /* Read branch trace data.  */
     VEC (btrace_block_s) *(*to_read_btrace) (struct btrace_target_info *,
 					     enum btrace_read_type);
 
+    /* Stop trace recording.  */
+    void (*to_stop_recording) (void);
+
     /* Print information about the recording.  */
     void (*to_info_record) (void);
 
@@ -1981,11 +1990,17 @@ extern struct btrace_target_info *target_enable_btrace (ptid_t ptid);
 /* Disable branch tracing. Deallocates @btinfo.  */
 extern void target_disable_btrace (struct btrace_target_info *btinfo);
 
+/* See to_teardown_btrace in struct target_ops.  */
+extern void target_teardown_btrace (struct btrace_target_info *btinfo);
+
 /* Read branch tracing data.
    Returns a vector of branch trace blocks with the latest entry at index 0.  */
 extern VEC (btrace_block_s) *target_read_btrace (struct btrace_target_info *,
 						 enum btrace_read_type);
 
+/* See to_stop_recording in struct target_ops.  */
+extern void target_stop_recording (void);
+
 /* See to_info_record in struct target_ops.  */
 extern void target_info_record (void);
 
diff --git a/gdb/thread.c b/gdb/thread.c
index 0d913dd..2a1d723 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -117,7 +117,7 @@ clear_thread_inferior_resources (struct thread_info *tp)
 
   bpstat_clear (&tp->control.stop_bpstat);
 
-  btrace_disable (tp);
+  btrace_teardown (tp);
 
   do_all_intermediate_continuations_thread (tp, 1);
   do_all_continuations_thread (tp, 1);
-- 
1.7.1