Re: Crash of GDB with gdbserver btrace enabled [Re: [patch v9 00/23] branch tracing support for Atom]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi Markus,

On Wed, 06 Mar 2013 16:30:13 +0100, Metzger, Markus T wrote:
> I still disable threads in to_close which tries to talk to the target.

When one follows the Pedro's way which I forwarded to you in to_close one
should only clear resources, without doing any actions involving the rest of
GDB infrastructure.

Do you think the attached patch still has some leaks or other issues?

An unrelated small cleanup is that btrace_disconnect should be IMO called from
to_detach and not from detach_command.


Thanks,
Jan


diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 6f13fca..a1a2d42 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -1154,6 +1154,13 @@ amd64_linux_disable_btrace (struct btrace_target_info *tinfo)
     error (_("Could not disable branch tracing: %s."), safe_strerror (errcode));
 }
 
+static void
+amd64_linux_clear_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 +1203,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_clear_btrace = amd64_linux_clear_btrace;
   t->to_read_btrace = linux_read_btrace;
 
   /* Register the target.  */
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 2fe6bf5..bd81ba5 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -447,6 +447,12 @@ btrace_clear (struct thread_info *tp)
 
   btinfo = &tp->btrace;
 
+  if (btinfo->target != NULL)
+    {
+      target_clear_btrace (btinfo->target);
+      btinfo->target = NULL;
+    }
+
   VEC_free (btrace_block_s, btinfo->btrace);
   VEC_free (btrace_inst_s, btinfo->itrace);
   VEC_free (btrace_func_s, btinfo->ftrace);
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 715c6d4..f48af01 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -1081,6 +1081,13 @@ i386_linux_disable_btrace (struct btrace_target_info *tinfo)
     error (_("Could not disable branch tracing: %s."), safe_strerror (errcode));
 }
 
+static void
+i386_linux_clear_btrace (struct btrace_target_info *tinfo)
+{
+  /* Ignore errors.  */
+  linux_disable_btrace (tinfo);
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_i386_linux_nat;
 
@@ -1118,6 +1125,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_clear_btrace = i386_linux_clear_btrace;
   t->to_read_btrace = linux_read_btrace;
 
   /* Register the target.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index c1a7905..5af9f96 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -111,6 +111,14 @@ record_btrace_disable_callback (void *arg)
     warning ("%s", error.message);
 }
 
+static void
+record_btrace_clear_callback (void *arg)
+{
+  struct thread_info *tp = arg;
+
+  btrace_clear (tp);
+}
+
 /* Enable automatic tracing of new threads.  */
 
 static void
@@ -189,7 +197,7 @@ record_btrace_close (int quitting)
      turn errors into warnings since we cannot afford to throw an error.  */
   ALL_THREADS (tp)
     if (tp->btrace.target != NULL)
-      record_btrace_disable_callback (tp);
+      record_btrace_clear_callback (tp);
 
   record_btrace_auto_disable ();
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index f76846a..ec2b305 100755
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11204,6 +11204,12 @@ remote_disable_btrace (struct btrace_target_info *tinfo)
   xfree (tinfo);
 }
 
+static void
+remote_clear_btrace (struct btrace_target_info *tinfo)
+{
+  xfree (tinfo);
+}
+
 /* Read the branch trace.  */
 
 static VEC (btrace_block_s) *
@@ -11368,6 +11374,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_clear_btrace = remote_clear_btrace;
   remote_ops.to_read_btrace = remote_read_btrace;
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 778b6b9..20cf704 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4203,6 +4203,19 @@ target_disable_btrace (struct btrace_target_info *btinfo)
 }
 
 /* See target.h.  */
+void
+target_clear_btrace (struct btrace_target_info *btinfo)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    if (t->to_clear_btrace != NULL)
+      return t->to_clear_btrace (btinfo);
+
+  tcomplain ();
+}
+
+/* See target.h.  */
 VEC (btrace_block_s) *
 target_read_btrace (struct btrace_target_info *btinfo,
 		    enum btrace_read_type type)
diff --git a/gdb/target.h b/gdb/target.h
index 88c13cc..66e3a12 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -870,6 +870,8 @@ struct target_ops
     /* Disable branch tracing and deallocate @tinfo.  */
     void (*to_disable_btrace) (struct btrace_target_info *tinfo);
 
+    void (*to_clear_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);
@@ -1971,6 +1973,8 @@ 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);
 
+extern void target_clear_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 *,
diff --git a/gdb/thread.c b/gdb/thread.c
index cffaa42..433b91d 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_clear (tp);
 
   do_all_intermediate_continuations_thread (tp, 1);
   do_all_continuations_thread (tp, 1);