Patchwork [1/3] btrace: Remove btrace disable cleanup

login
register
mail settings
Submitter Simon Marchi
Date March 4, 2018, 8:56 p.m.
Message ID <20180304205605.13037-1-simon.marchi@polymtl.ca>
Download mbox | patch
Permalink /patch/26190/
State New
Headers show

Comments

Simon Marchi - March 4, 2018, 8:56 p.m.
This patch removes a cleanup that disables btrace on threads in case of
failure, so we don't leave it enabled for some the threads and disabled
for the rest.

gdb/ChangeLog:

	* record-btrace.c (record_btrace_disable_callback): Remove.
	(struct scoped_btrace_disable): New.
	(record_btrace_open): Use scoped_btrace_disable.
---
 gdb/record-btrace.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)
Metzger, Markus T - March 5, 2018, 12:39 p.m.
Hello Simon,


> This patch removes a cleanup that disables btrace on threads in case of
> failure, so we don't leave it enabled for some the threads and disabled
> for the rest.
> 
> gdb/ChangeLog:
> 
> 	* record-btrace.c (record_btrace_disable_callback): Remove.
> 	(struct scoped_btrace_disable): New.
> 	(record_btrace_open): Use scoped_btrace_disable.

Looks good to me.

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

Patch

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index d9a1dc593f..b04a7e5049 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -159,16 +159,6 @@  record_btrace_enable_warn (struct thread_info *tp)
   END_CATCH
 }
 
-/* Callback function to disable branch tracing for one thread.  */
-
-static void
-record_btrace_disable_callback (void *arg)
-{
-  struct thread_info *tp = (struct thread_info *) arg;
-
-  btrace_disable (tp);
-}
-
 /* Enable automatic tracing of new threads.  */
 
 static void
@@ -223,12 +213,42 @@  record_btrace_push_target (void)
   observer_notify_record_changed (current_inferior (), 1, "btrace", format);
 }
 
+/* Disable btrace on a set of threads on scope exit.  */
+
+struct scoped_btrace_disable
+{
+  scoped_btrace_disable () = default;
+
+  DISABLE_COPY_AND_ASSIGN (scoped_btrace_disable);
+
+  ~scoped_btrace_disable ()
+  {
+    for (thread_info *tp : m_threads)
+      btrace_disable (tp);
+  }
+
+  void add_thread (thread_info *thread)
+  {
+    m_threads.push_front (thread);
+  }
+
+  void discard ()
+  {
+    m_threads.clear ();
+  }
+
+private:
+  std::forward_list<thread_info *> m_threads;
+};
+
 /* The to_open method of target record-btrace.  */
 
 static void
 record_btrace_open (const char *args, int from_tty)
 {
-  struct cleanup *disable_chain;
+  /* If we fail to enable btrace for one thread, disable it for the threads for
+     which it was successfully enabled.  */
+  scoped_btrace_disable btrace_disable;
   struct thread_info *tp;
 
   DEBUG ("open");
@@ -240,18 +260,17 @@  record_btrace_open (const char *args, int from_tty)
 
   gdb_assert (record_btrace_thread_observer == NULL);
 
-  disable_chain = make_cleanup (null_cleanup, NULL);
   ALL_NON_EXITED_THREADS (tp)
     if (args == NULL || *args == 0 || number_is_in_list (args, tp->global_num))
       {
 	btrace_enable (tp, &record_btrace_conf);
 
-	make_cleanup (record_btrace_disable_callback, tp);
+	btrace_disable.add_thread (tp);
       }
 
   record_btrace_push_target ();
 
-  discard_cleanups (disable_chain);
+  btrace_disable.discard ();
 }
 
 /* The to_stop_recording method of target record-btrace.  */