public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Add logging for event loop events
@ 2020-09-25 15:48 Simon Marchi
  2020-09-25 15:48 ` [PATCH 1/4] gdb: give names to event loop file handlers Simon Marchi
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Simon Marchi @ 2020-09-25 15:48 UTC (permalink / raw)
  To: gdb-patches

I currently have both hands in event loop / infrun stuff, and I found
it useful to add debug prints to know when the event loop invoked an
event handler.  I would like to propose this for upstream, so that I
don't need to maintain my own patch on the side :).

Simon Marchi (4):
  gdb: give names to event loop file handlers
  gdb: give names to async event/signal handlers
  gdb: move debug_prefixed_vprintf to gdbsupport
  gdb: add debug prints in event loop

 gdb/async-event.c          | 49 +++++++++++++++++++++++-----
 gdb/async-event.h          | 15 ++++++---
 gdb/debug.c                | 13 +-------
 gdb/debug.h                | 32 ------------------
 gdb/event-top.c            | 67 +++++++++++++++++++++++++++++++++-----
 gdb/infrun.c               |  5 +--
 gdb/linux-nat.c            |  5 +--
 gdb/record-btrace.c        |  2 +-
 gdb/record-full.c          |  2 +-
 gdb/remote-notif.c         |  2 +-
 gdb/remote.c               |  3 +-
 gdb/run-on-main-thread.c   |  3 +-
 gdb/ser-base.c             |  6 ++--
 gdb/top.c                  |  4 +++
 gdb/top.h                  |  3 ++
 gdb/tui/tui-io.c           |  2 +-
 gdb/tui/tui-win.c          |  3 +-
 gdbserver/linux-low.cc     |  3 +-
 gdbserver/remote-utils.cc  | 10 +++---
 gdbserver/server.cc        | 15 +++++++++
 gdbsupport/common-debug.cc | 11 +++++++
 gdbsupport/common-debug.h  |  6 ++++
 gdbsupport/event-loop.cc   | 60 +++++++++++++++++++++++++---------
 gdbsupport/event-loop.h    | 54 ++++++++++++++++++++++++++++--
 24 files changed, 274 insertions(+), 101 deletions(-)
 delete mode 100644 gdb/debug.h

-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] gdb: give names to event loop file handlers
  2020-09-25 15:48 [PATCH 0/4] Add logging for event loop events Simon Marchi
@ 2020-09-25 15:48 ` Simon Marchi
  2020-09-25 15:48 ` [PATCH 2/4] gdb: give names to async event/signal handlers Simon Marchi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-09-25 15:48 UTC (permalink / raw)
  To: gdb-patches

Assign names to event loop file handlers.  They will be used in debug
messages when file handlers are invoked.  The name is copied into the
event loop data structures, because it sometimes needs to be computed.

In GDB, each UI used to get its own unique number, until commit
cbe256847e19 ("Remove ui::num").  Re-introduce this field, and use it to
make a unique name for the handler.

I'm not too sure what goes on in ser-base.c, all I know is that it's
what is used when debugging remotely.  I've just named the main handler
"serial".  It would be good to have unique names there too.  For instance
when debugging with two different remote connections, we'd ideally want
the handlers to have unique names.  I didn't do it in this patch though.

gdb/ChangeLog:

        * async-event.c (initialize_async_signal_handlers): Pass name to
	add_file_handler
        * event-top.c (ui_register_input_event_handler): Likewise.
        * linux-nat.c (linux_nat_target::async): Likewise.
        * run-on-main-thread.c (_initialize_run_on_main_thread):
	Likewise
        * ser-base.c (reschedule): Likewise.
        (ser_base_async): Likewise.
        * tui/tui-io.c: Likewise.
	* top.h (struct ui) <num>: New field.
	* top.c (highest_ui_num): New variable.
	(ui::ui): Initialize num.

gdbserver/ChangeLog:

        * linux-low.cc (linux_process_target::async): Pass name to
	add_file_handler.
        * remote-utils.cc (handle_accept_event): Likewise.
        (remote_open): Likewise.

gdbsupport/ChangeLog:

        * event-loop.h (add_file_handler): Add "name" parameter.
        * event-loop.cc (struct file_handler) <name>: New field.
        (create_file_handler): Add "name" parameter, assign it to file
	handler.
        (add_file_handler): Add "name" parameter.

Change-Id: I9f1545f73888ebb6778eb653a618ca44d105f92c
---
 gdb/async-event.c         |  2 +-
 gdb/event-top.c           |  3 ++-
 gdb/linux-nat.c           |  3 ++-
 gdb/run-on-main-thread.c  |  3 ++-
 gdb/ser-base.c            |  6 +++---
 gdb/top.c                 |  4 ++++
 gdb/top.h                 |  3 +++
 gdb/tui/tui-io.c          |  2 +-
 gdbserver/linux-low.cc    |  3 ++-
 gdbserver/remote-utils.cc | 10 ++++++----
 gdbsupport/event-loop.cc  | 32 +++++++++++++++++---------------
 gdbsupport/event-loop.h   | 14 ++++++++++++--
 12 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/gdb/async-event.c b/gdb/async-event.c
index ffc0edcde88c..e5cd63e309e4 100644
--- a/gdb/async-event.c
+++ b/gdb/async-event.c
@@ -114,7 +114,7 @@ initialize_async_signal_handlers (void)
   async_signal_handlers_serial_event = make_serial_event ();
 
   add_file_handler (serial_event_fd (async_signal_handlers_serial_event),
-		    async_signals_handler, NULL);
+		    async_signals_handler, NULL, "async-signals");
 }
 
 \f
diff --git a/gdb/event-top.c b/gdb/event-top.c
index ac0f3701016e..c96f10450dda 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -524,7 +524,8 @@ stdin_event_handler (int error, gdb_client_data client_data)
 void
 ui_register_input_event_handler (struct ui *ui)
 {
-  add_file_handler (ui->input_fd, stdin_event_handler, ui);
+  add_file_handler (ui->input_fd, stdin_event_handler, ui,
+		    string_printf ("ui-%d", ui->num));
 }
 
 /* See top.h.  */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 44b59485d7b9..e47a23454bf6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4121,7 +4121,8 @@ linux_nat_target::async (int enable)
       if (!linux_async_pipe (1))
 	{
 	  add_file_handler (linux_nat_event_pipe[0],
-			    handle_target_event, NULL);
+			    handle_target_event, NULL,
+			    "linux-nat");
 	  /* There may be pending events to handle.  Tell the event loop
 	     to poll them.  */
 	  async_file_mark ();
diff --git a/gdb/run-on-main-thread.c b/gdb/run-on-main-thread.c
index 2cc93e430d84..1e7bb5f5f8e3 100644
--- a/gdb/run-on-main-thread.c
+++ b/gdb/run-on-main-thread.c
@@ -94,5 +94,6 @@ void
 _initialize_run_on_main_thread ()
 {
   runnable_event = make_serial_event ();
-  add_file_handler (serial_event_fd (runnable_event), run_events, nullptr);
+  add_file_handler (serial_event_fd (runnable_event), run_events, nullptr,
+		    "run-on-main-thread");
 }
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index fb6f4e056ad5..a0c31b5f3807 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -82,7 +82,7 @@ reschedule (struct serial *scb)
 	case NOTHING_SCHEDULED:
 	  if (scb->bufcnt == 0)
 	    {
-	      add_file_handler (scb->fd, fd_event, scb);
+	      add_file_handler (scb->fd, fd_event, scb, "serial");
 	      next_state = FD_SCHEDULED;
 	    }
 	  else
@@ -94,7 +94,7 @@ reschedule (struct serial *scb)
 	  if (scb->bufcnt == 0)
 	    {
 	      delete_timer (scb->async_state);
-	      add_file_handler (scb->fd, fd_event, scb);
+	      add_file_handler (scb->fd, fd_event, scb, "serial");
 	      next_state = FD_SCHEDULED;
 	    }
 	  else
@@ -596,7 +596,7 @@ ser_base_async (struct serial *scb,
       reschedule (scb);
 
       if (scb->error_fd != -1)
-	add_file_handler (scb->error_fd, handle_error_fd, scb);
+	add_file_handler (scb->error_fd, handle_error_fd, scb, "serial-error");
     }
   else
     {
diff --git a/gdb/top.c b/gdb/top.c
index 8dbc7ced4dc3..acd31afb9a90 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -270,10 +270,14 @@ void (*deprecated_call_command_hook) (struct cmd_list_element * c,
 
 void (*deprecated_context_hook) (int id);
 
+/* The highest UI number ever assigned.  */
+static int highest_ui_num;
+
 /* See top.h.  */
 
 ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
   : next (nullptr),
+    num (++highest_ui_num),
     call_readline (nullptr),
     input_handler (nullptr),
     command_editing (0),
diff --git a/gdb/top.h b/gdb/top.h
index 92b096492f03..fd992977155f 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -65,6 +65,9 @@ struct ui
   /* Pointer to next in singly-linked list.  */
   struct ui *next;
 
+  /* Convenient handle (UI number).  Unique across all UIs.  */
+  int num;
+
   /* The UI's command line buffer.  This is to used to accumulate
      input until we have a whole command line.  */
   struct buffer line_buffer;
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 7698d7903f19..e76defc463e4 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -870,7 +870,7 @@ tui_initialize_io (void)
   (void) fcntl (tui_readline_pipe[0], F_SETFL, O_NDELAY);
 #endif
 #endif
-  add_file_handler (tui_readline_pipe[0], tui_readline_output, 0);
+  add_file_handler (tui_readline_pipe[0], tui_readline_output, 0, "tui");
 #else
   tui_rl_outstream = stdout;
 #endif
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 70d5521d4421..1ae72a21d6c6 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -6092,7 +6092,8 @@ linux_process_target::async (bool enable)
 
 	  /* Register the event loop handler.  */
 	  add_file_handler (linux_event_pipe[0],
-			    handle_target_event, NULL);
+			    handle_target_event, NULL,
+			    "linux-low");
 
 	  /* Always trigger a linux_wait.  */
 	  async_file_mark ();
diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
index c26668dc0f8a..5a6ceb1d9a10 100644
--- a/gdbserver/remote-utils.cc
+++ b/gdbserver/remote-utils.cc
@@ -194,7 +194,7 @@ handle_accept_event (int err, gdb_client_data client_data)
   enable_async_notification (remote_desc);
 
   /* Register the event loop handler.  */
-  add_file_handler (remote_desc, handle_serial_event, NULL);
+  add_file_handler (remote_desc, handle_serial_event, NULL, "remote-net");
 
   /* We have a new GDB connection now.  If we were disconnected
      tracing, there's a window where the target could report a stop
@@ -331,7 +331,7 @@ remote_open (const char *name)
       enable_async_notification (remote_desc);
 
       /* Register the event loop handler.  */
-      add_file_handler (remote_desc, handle_serial_event, NULL);
+      add_file_handler (remote_desc, handle_serial_event, NULL, "remote-stdio");
     }
 #ifndef USE_WIN32API
   else if (port_str == NULL)
@@ -372,7 +372,8 @@ remote_open (const char *name)
       enable_async_notification (remote_desc);
 
       /* Register the event loop handler.  */
-      add_file_handler (remote_desc, handle_serial_event, NULL);
+      add_file_handler (remote_desc, handle_serial_event, NULL,
+			"remote-device");
     }
 #endif /* USE_WIN32API */
   else
@@ -398,7 +399,8 @@ remote_open (const char *name)
       fflush (stderr);
 
       /* Register the event loop handler.  */
-      add_file_handler (listen_desc, handle_accept_event, NULL);
+      add_file_handler (listen_desc, handle_accept_event, NULL,
+			"remote-listen");
     }
 }
 
diff --git a/gdbsupport/event-loop.cc b/gdbsupport/event-loop.cc
index 59436d467ede..0d78122e0cc3 100644
--- a/gdbsupport/event-loop.cc
+++ b/gdbsupport/event-loop.cc
@@ -61,6 +61,9 @@ struct file_handler
   /* Argument to pass to proc.  */
   gdb_client_data client_data;
 
+  /* User-friendly name of this handler.  Heap-allocated, owned by this.*/
+  std::string *name;
+
   /* Was an error detected on this fd?  */
   int error;
 
@@ -160,7 +163,8 @@ static struct
 timer_list;
 
 static void create_file_handler (int fd, int mask, handler_func *proc,
-				 gdb_client_data client_data);
+				 gdb_client_data client_data,
+				 std::string &&name);
 static int gdb_wait_for_event (int);
 static int update_wait_timeout (void);
 static int poll_timers (void);
@@ -231,13 +235,11 @@ gdb_do_one_event (void)
   return 1;
 }
 
-\f
+/* See event-loop.h  */
 
-/* Wrapper function for create_file_handler, so that the caller
-   doesn't have to know implementation details about the use of poll
-   vs. select.  */
 void
-add_file_handler (int fd, handler_func * proc, gdb_client_data client_data)
+add_file_handler (int fd, handler_func *proc, gdb_client_data client_data,
+		  std::string &&name)
 {
 #ifdef HAVE_POLL
   struct pollfd fds;
@@ -263,21 +265,18 @@ add_file_handler (int fd, handler_func * proc, gdb_client_data client_data)
   if (use_poll)
     {
 #ifdef HAVE_POLL
-      create_file_handler (fd, POLLIN, proc, client_data);
+      create_file_handler (fd, POLLIN, proc, client_data, std::move (name));
 #else
       internal_error (__FILE__, __LINE__,
 		      _("use_poll without HAVE_POLL"));
 #endif
     }
   else
-    create_file_handler (fd, GDB_READABLE | GDB_EXCEPTION, 
-			 proc, client_data);
+    create_file_handler (fd, GDB_READABLE | GDB_EXCEPTION,
+			 proc, client_data, std::move (name));
 }
 
-/* Add a file handler/descriptor to the list of descriptors we are
-   interested in.
-
-   FD is the file descriptor for the file/stream to be listened to.
+/* Helper for add_file_handler.
 
    For the poll case, MASK is a combination (OR) of POLLIN,
    POLLRDNORM, POLLRDBAND, POLLPRI, POLLOUT, POLLWRNORM, POLLWRBAND:
@@ -289,8 +288,8 @@ add_file_handler (int fd, handler_func * proc, gdb_client_data client_data)
    occurs for FD.  CLIENT_DATA is the argument to pass to PROC.  */
 
 static void
-create_file_handler (int fd, int mask, handler_func * proc, 
-		     gdb_client_data client_data)
+create_file_handler (int fd, int mask, handler_func * proc,
+		     gdb_client_data client_data, std::string &&name)
 {
   file_handler *file_ptr;
 
@@ -358,6 +357,7 @@ create_file_handler (int fd, int mask, handler_func * proc,
   file_ptr->proc = proc;
   file_ptr->client_data = client_data;
   file_ptr->mask = mask;
+  file_ptr->name = new std::string (std::move (name));
 }
 
 /* Return the next file handler to handle, and advance to the next
@@ -489,6 +489,8 @@ delete_file_handler (int fd)
 	;
       prev_ptr->next_file = file_ptr->next_file;
     }
+
+  delete file_ptr->name;
   xfree (file_ptr);
 }
 
diff --git a/gdbsupport/event-loop.h b/gdbsupport/event-loop.h
index 2eaaa0c8b604..d7478b037a9c 100644
--- a/gdbsupport/event-loop.h
+++ b/gdbsupport/event-loop.h
@@ -78,8 +78,18 @@ typedef void (timer_handler_func) (gdb_client_data);
 
 extern int gdb_do_one_event (void);
 extern void delete_file_handler (int fd);
-extern void add_file_handler (int fd, handler_func *proc, 
-			      gdb_client_data client_data);
+
+/* Add a file handler/descriptor to the list of descriptors we are
+   interested in.
+
+   FD is the file descriptor for the file/stream to be listened to.
+
+   NAME is a user-friendly name for the handler.  */
+
+extern void add_file_handler (int fd, handler_func *proc,
+			      gdb_client_data client_data,
+			      std::string &&name);
+
 extern int create_timer (int milliseconds, 
 			 timer_handler_func *proc, 
 			 gdb_client_data client_data);
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/4] gdb: give names to async event/signal handlers
  2020-09-25 15:48 [PATCH 0/4] Add logging for event loop events Simon Marchi
  2020-09-25 15:48 ` [PATCH 1/4] gdb: give names to event loop file handlers Simon Marchi
@ 2020-09-25 15:48 ` Simon Marchi
  2020-09-25 15:48 ` [PATCH 3/4] gdb: move debug_prefixed_vprintf to gdbsupport Simon Marchi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-09-25 15:48 UTC (permalink / raw)
  To: gdb-patches

Assign names to async event/signal handlers.  They will be used in debug
messages when file handlers are invoked.

Unlike in the previous patch, the names are not copied in the structure,
since we don't need to (all names are string literals for the moment).

gdb/ChangeLog:

	* async-event.h (create_async_signal_handler): Add name
	parameter.
	(create_async_event_handler): Likewise.
	* async-event.c (struct async_signal_handler) <name>: New field.
	(struct async_event_handler) <name>: New field.
	(create_async_signal_handler): Assign name.
	(create_async_event_handler): Assign name.
	* event-top.c (async_init_signals): Pass name when creating
	handler.
	* infrun.c (_initialize_infrun): Likewise.
	* record-btrace.c (record_btrace_push_target): Likewise.
	* record-full.c (record_full_open): Likewise.
	* remote-notif.c (remote_notif_state_allocate): Likewise.
	* remote.c (remote_target::open_1): Likewise.
	* tui/tui-win.c (tui_initialize_win): Likewise.

Change-Id: Icd9d9f775542ae5fc2cd148c12f481e7885936d5
---
 gdb/async-event.c   | 20 ++++++++++++++------
 gdb/async-event.h   | 15 +++++++++++----
 gdb/event-top.c     | 14 +++++++-------
 gdb/infrun.c        |  3 ++-
 gdb/record-btrace.c |  2 +-
 gdb/record-full.c   |  2 +-
 gdb/remote-notif.c  |  2 +-
 gdb/remote.c        |  3 ++-
 gdb/tui/tui-win.c   |  3 ++-
 9 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/gdb/async-event.c b/gdb/async-event.c
index e5cd63e309e4..55be014484e5 100644
--- a/gdb/async-event.c
+++ b/gdb/async-event.c
@@ -46,6 +46,9 @@ struct async_signal_handler
 
   /* Argument to PROC.  */
   gdb_client_data client_data;
+
+  /* User-friendly name of this handler.  */
+  const char *name;
 };
 
 /* PROC is a function to be invoked when the READY flag is set.  This
@@ -68,6 +71,9 @@ struct async_event_handler
 
   /* Argument to PROC.  */
   gdb_client_data client_data;
+
+  /* User-friendly name of this handler.  */
+  const char *name;
 };
 
 /* All the async_signal_handlers gdb is interested in are kept onto
@@ -127,7 +133,8 @@ initialize_async_signal_handlers (void)
    whenever the handler is invoked.  */
 async_signal_handler *
 create_async_signal_handler (sig_handler_func * proc,
-			     gdb_client_data client_data)
+			     gdb_client_data client_data,
+			     const char *name)
 {
   async_signal_handler *async_handler_ptr;
 
@@ -136,6 +143,7 @@ create_async_signal_handler (sig_handler_func * proc,
   async_handler_ptr->next_handler = NULL;
   async_handler_ptr->proc = proc;
   async_handler_ptr->client_data = client_data;
+  async_handler_ptr->name = name;
   if (sighandler_list.first_handler == NULL)
     sighandler_list.first_handler = async_handler_ptr;
   else
@@ -236,13 +244,12 @@ delete_async_signal_handler (async_signal_handler ** async_handler_ptr)
   (*async_handler_ptr) = NULL;
 }
 
-/* Create an asynchronous event handler, allocating memory for it.
-   Return a pointer to the newly created handler.  PROC is the
-   function to call with CLIENT_DATA argument whenever the handler is
-   invoked.  */
+/* See async-event.h.  */
+
 async_event_handler *
 create_async_event_handler (async_event_handler_func *proc,
-			    gdb_client_data client_data)
+			    gdb_client_data client_data,
+			    const char *name)
 {
   async_event_handler *h;
 
@@ -251,6 +258,7 @@ create_async_event_handler (async_event_handler_func *proc,
   h->next_handler = NULL;
   h->proc = proc;
   h->client_data = client_data;
+  h->name = name;
   if (async_event_handler_list.first_handler == NULL)
     async_event_handler_list.first_handler = h;
   else
diff --git a/gdb/async-event.h b/gdb/async-event.h
index adf6dc8f8c4b..8f279d63d63c 100644
--- a/gdb/async-event.h
+++ b/gdb/async-event.h
@@ -27,8 +27,9 @@ typedef void (sig_handler_func) (gdb_client_data);
 typedef void (async_event_handler_func) (gdb_client_data);
 
 extern struct async_signal_handler *
-  create_async_signal_handler (sig_handler_func *proc, 
-			       gdb_client_data client_data);
+  create_async_signal_handler (sig_handler_func *proc,
+			       gdb_client_data client_data,
+			       const char *name);
 extern void delete_async_signal_handler (struct async_signal_handler **);
 
 /* Call the handler from HANDLER the next time through the event
@@ -48,10 +49,16 @@ extern void clear_async_signal_handler (struct async_signal_handler *handler);
    and set PROC as its callback.  CLIENT_DATA is passed as argument to
    PROC upon its invocation.  Returns a pointer to an opaque structure
    used to mark as ready and to later delete this event source from
-   the event loop.  */
+   the event loop.
+
+   NAME is a user-friendly name for the handler, used in debug statements.  The
+   name is not copied: its lifetime should be at least as long as that of the
+   handler.  */
+
 extern struct async_event_handler *
   create_async_event_handler (async_event_handler_func *proc,
-			      gdb_client_data client_data);
+			      gdb_client_data client_data,
+			      const char *name);
 
 /* Remove the event source pointed by HANDLER_PTR created by
    CREATE_ASYNC_EVENT_HANDLER from the event loop, and release it.  */
diff --git a/gdb/event-top.c b/gdb/event-top.c
index c96f10450dda..fdce5de6f429 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -916,10 +916,10 @@ async_init_signals (void)
 
   signal (SIGINT, handle_sigint);
   sigint_token =
-    create_async_signal_handler (async_request_quit, NULL);
+    create_async_signal_handler (async_request_quit, NULL, "sigint");
   signal (SIGTERM, handle_sigterm);
   async_sigterm_token
-    = create_async_signal_handler (async_sigterm_handler, NULL);
+    = create_async_signal_handler (async_sigterm_handler, NULL, "sigterm");
 
   /* If SIGTRAP was set to SIG_IGN, then the SIG_IGN will get passed
      to the inferior and breakpoints will be ignored.  */
@@ -938,23 +938,23 @@ async_init_signals (void)
      to SIG_DFL for us.  */
   signal (SIGQUIT, handle_sigquit);
   sigquit_token =
-    create_async_signal_handler (async_do_nothing, NULL);
+    create_async_signal_handler (async_do_nothing, NULL, "sigquit");
 #endif
 #ifdef SIGHUP
   if (signal (SIGHUP, handle_sighup) != SIG_IGN)
     sighup_token =
-      create_async_signal_handler (async_disconnect, NULL);
+      create_async_signal_handler (async_disconnect, NULL, "sighup");
   else
     sighup_token =
-      create_async_signal_handler (async_do_nothing, NULL);
+      create_async_signal_handler (async_do_nothing, NULL, "sighup");
 #endif
   signal (SIGFPE, handle_sigfpe);
   sigfpe_token =
-    create_async_signal_handler (async_float_handler, NULL);
+    create_async_signal_handler (async_float_handler, NULL, "sigfpe");
 
 #ifdef SIGTSTP
   sigtstp_token =
-    create_async_signal_handler (async_sigtstp_handler, NULL);
+    create_async_signal_handler (async_sigtstp_handler, NULL, "sigtstp");
 #endif
 
   install_handle_sigsegv ();
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6532b06ae52e..4e9492b35961 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -9278,7 +9278,8 @@ _initialize_infrun ()
 
   /* Register extra event sources in the event loop.  */
   infrun_async_inferior_event_token
-    = create_async_event_handler (infrun_async_inferior_event_handler, NULL);
+    = create_async_event_handler (infrun_async_inferior_event_handler, NULL,
+				  "infrun");
 
   add_info ("signals", info_signals_command, _("\
 What debugger does when program gets various signals.\n\
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index c1430e9b5335..20aedb6ed4f0 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -342,7 +342,7 @@ record_btrace_push_target (void)
 
   record_btrace_async_inferior_event_handler
     = create_async_event_handler (record_btrace_handle_async_inferior_event,
-				  NULL);
+				  NULL, "record-btrace");
   record_btrace_generating_corefile = 0;
 
   format = btrace_format_short_string (record_btrace_conf.format);
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 1c8b791ffb9e..b694d2a19312 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -986,7 +986,7 @@ record_full_open (const char *name, int from_tty)
   /* Register extra event sources in the event loop.  */
   record_full_async_inferior_event_token
     = create_async_event_handler (record_full_async_inferior_event_handler,
-				  NULL);
+				  NULL, "record-full");
 
   record_full_init_record_breakpoints ();
 
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 2e5f124284ba..f18bc8678e38 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -219,7 +219,7 @@ remote_notif_state_allocate (remote_target *remote)
 
   notif_state->get_pending_events_token
     = create_async_event_handler (remote_async_get_pending_events_handler,
-				  notif_state);
+				  notif_state, "remote-notif");
 
   return notif_state;
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index 5fc80ebc8f76..a6bc82f35a46 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5605,7 +5605,8 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
 
   /* Register extra event sources in the event loop.  */
   rs->remote_async_inferior_event_token
-    = create_async_event_handler (remote_async_inferior_event_handler, remote);
+    = create_async_event_handler (remote_async_inferior_event_handler, remote,
+				  "remote");
   rs->notif_state = remote_notif_state_allocate (remote);
 
   /* Reset the target state; these things will be queried either by
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index f906b0dc4fea..86c0fdba2bc1 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -576,7 +576,8 @@ tui_initialize_win (void)
 {
 #ifdef SIGWINCH
   tui_sigwinch_token
-    = create_async_signal_handler (tui_async_resize_screen, NULL);
+    = create_async_signal_handler (tui_async_resize_screen, NULL,
+				   "tui-sigwinch");
 
   {
 #ifdef HAVE_SIGACTION
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/4] gdb: move debug_prefixed_vprintf to gdbsupport
  2020-09-25 15:48 [PATCH 0/4] Add logging for event loop events Simon Marchi
  2020-09-25 15:48 ` [PATCH 1/4] gdb: give names to event loop file handlers Simon Marchi
  2020-09-25 15:48 ` [PATCH 2/4] gdb: give names to async event/signal handlers Simon Marchi
@ 2020-09-25 15:48 ` Simon Marchi
  2020-09-25 15:48 ` [PATCH 4/4] gdb: add debug prints in event loop Simon Marchi
  2020-10-02 18:49 ` [PATCH 0/4] Add logging for event loop events Simon Marchi
  4 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-09-25 15:48 UTC (permalink / raw)
  To: gdb-patches

The following patch needs to output debug prints in gdbsupport code.
Move debug_prefixed_vprintf so that it is possible to use it from there

gdb/ChangeLog:

	* debug.c (debug_prefixed_vprintf): Move to gdbsupport.
	* debug.h: Remove.
	* infrun.c: Include gdbsupport/common-debug.h.
	* linux-nat.c: Likewise.

gdbsupport/ChangeLog:

	* common-debug.cc (debug_prefixed_vprintf): Move here.
	* common-debug.h (debug_prefixed_vprintf): Move here.

Change-Id: I5170065fc10a7a49c0f1bba67c691decb2cf3bcb
---
 gdb/debug.c                | 13 +------------
 gdb/debug.h                | 32 --------------------------------
 gdb/infrun.c               |  2 +-
 gdb/linux-nat.c            |  2 +-
 gdbsupport/common-debug.cc | 11 +++++++++++
 gdbsupport/common-debug.h  |  6 ++++++
 6 files changed, 20 insertions(+), 46 deletions(-)
 delete mode 100644 gdb/debug.h

diff --git a/gdb/debug.c b/gdb/debug.c
index f845a7e3b1e7..fe571aaae393 100644
--- a/gdb/debug.c
+++ b/gdb/debug.c
@@ -19,7 +19,7 @@
 
 #include "defs.h"
 
-#include "debug.h"
+#include "gdbsupport/common-debug.h"
 
 /* See gdbsupport/common-debug.h.  */
 
@@ -28,14 +28,3 @@ debug_vprintf (const char *fmt, va_list ap)
 {
   vfprintf_unfiltered (gdb_stdlog, fmt, ap);
 }
-
-/* See debug.h.  */
-
-void
-debug_prefixed_vprintf (const char *module, const char *func, const char *format,
-			va_list args)
-{
-  debug_printf ("[%s] %s: ", module, func);
-  debug_vprintf (format, args);
-  debug_printf ("\n");
-}
diff --git a/gdb/debug.h b/gdb/debug.h
deleted file mode 100644
index 1d98fbebec6c..000000000000
--- a/gdb/debug.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/* Helpers to format and print debug statements
-
-   Copyright (C) 2020 Free Software Foundation, Inc.
-
-   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 DEBUG_H
-#define DEBUG_H
-
-/* Print a debug statement prefixed with the module and function name, and
-   with a newline at the end.  */
-
-void ATTRIBUTE_PRINTF (3, 0)
-debug_prefixed_vprintf (const char *module, const char *func, const char *format,
-			va_list args);
-
-#endif /* DEBUG_H */
-
-
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4e9492b35961..03ca333c38a2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -70,7 +70,7 @@
 #include "gdbsupport/selftest.h"
 #include "scoped-mock-context.h"
 #include "test-target.h"
-#include "debug.h"
+#include "gdbsupport/common-debug.h"
 
 /* Prototypes for local functions */
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index e47a23454bf6..5d616293196e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -68,7 +68,7 @@
 #include "gdbsupport/fileio.h"
 #include "gdbsupport/scope-exit.h"
 #include "gdbsupport/gdb-sigmask.h"
-#include "debug.h"
+#include "gdbsupport/common-debug.h"
 
 /* This comment documents high-level logic of this file.
 
diff --git a/gdbsupport/common-debug.cc b/gdbsupport/common-debug.cc
index d1131a0a8796..b8fd133159fb 100644
--- a/gdbsupport/common-debug.cc
+++ b/gdbsupport/common-debug.cc
@@ -35,3 +35,14 @@ debug_printf (const char *fmt, ...)
   debug_vprintf (fmt, ap);
   va_end (ap);
 }
+
+/* See gdbsupport/common-debug.h.  */
+
+void
+debug_prefixed_vprintf (const char *module, const char *func, const char *format,
+			va_list args)
+{
+  debug_printf ("[%s] %s: ", module, func);
+  debug_vprintf (format, args);
+  debug_printf ("\n");
+}
diff --git a/gdbsupport/common-debug.h b/gdbsupport/common-debug.h
index 9934ec543d28..afb427e18005 100644
--- a/gdbsupport/common-debug.h
+++ b/gdbsupport/common-debug.h
@@ -38,4 +38,10 @@ extern void debug_printf (const char *format, ...)
 extern void debug_vprintf (const char *format, va_list ap)
      ATTRIBUTE_PRINTF (1, 0);
 
+/* Print a debug statement prefixed with the module and function name, and
+   with a newline at the end.  */
+
+extern void ATTRIBUTE_PRINTF (3, 0) debug_prefixed_vprintf
+  (const char *module, const char *func, const char *format, va_list args);
+
 #endif /* COMMON_COMMON_DEBUG_H */
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 4/4] gdb: add debug prints in event loop
  2020-09-25 15:48 [PATCH 0/4] Add logging for event loop events Simon Marchi
                   ` (2 preceding siblings ...)
  2020-09-25 15:48 ` [PATCH 3/4] gdb: move debug_prefixed_vprintf to gdbsupport Simon Marchi
@ 2020-09-25 15:48 ` Simon Marchi
  2020-10-02 18:49 ` [PATCH 0/4] Add logging for event loop events Simon Marchi
  4 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-09-25 15:48 UTC (permalink / raw)
  To: gdb-patches

Add debug printouts about event loop-related events:

 - When a file descriptor handler gets invoked
 - When an async event/signal handler gets invoked
 - When an async event/signal handler gets marked or cleared

Since mark_async_signal_handler is meant to be used from a signal
handler, it needs to be async-signal-safe.  Therefore, we output the
debug statement "manually" using async-signal-safe methods.

Outputing messages about UI-related events is very distracting and gets
in the way of typing.  I therefore made "set debug event-loop" have
three possible values instead of just on/off: off, all-except-ui and
all.  Event sources (currently just file descriptor handlers) have a
flag indicating whether they are UI-related, so we can filter the
messages based on that.

For GDBserver, I followed the pattern of the existing "remote-debug"
flag.  Event loop debugging can be enabled using the --event-loop-debug
command line flag or the "set event-loop-debug" monitor command.  For
GDBserver, it's only a boolean switch, which toggles between off and all
messages.

gdb/ChangeLog:

	* async-event.c (mark_async_signal_handler): Add debug print.
	(clear_async_signal_handler): Likewise.
	(mark_async_event_handler): Likewise.
	(clear_async_signal_handler): Likewise.
	(invoke_async_signal_handlers): Likewise.
	(mark_async_event_handler): Likewise.
	(clear_async_event_handler): Likewise.
	(check_async_event_handlers): Likewise.
	* event-top.c (ui_register_input_event_handler): Pass true to
	add_file_handler's is_ui parameter.
	(debug_event_loop_off): New global.
	(debug_event_loop_all_except_ui): New global.
	(debug_event_loop_all): New global.
	(debug_event_loop_enum): New global.
	(debug_event_loop_value): New global.
	(set_debug_event_loop_command): New function.
	(show_debug_event_loop): New function.
	(_initialize_event_top): New function.

gdbserver/ChangeLog:

	* server.cc (monitor_show_help): Mention new command.
	(handle_monitor_command): Handle "set
	debug-event-loop".
	(gdbserver_usage): Mention new flag.
	(captured_main): Handle "--debug-event-loop".

gdbsupport/ChangeLog:

	* event-loop.h (add_file_handler): Add is_ui parameter.
	(enum class debug_event_loop_kind): New enum.
	(debug_event_loop): New variable declaration.
	(event_loop_debug_printf_1): New function declaration.
	(event_loop_debug_printf): New macro.
	* event-loop.cc (debug_event_loop): New variable.
	(struct file_handler) <is_ui>: New field.
	(create_file_handler): Add is_ui parameter.
	(add_file_handler): Likewise.
	(handle_file_event): Add debug print.
	(event_loop_debug_printf_1): New function.

Change-Id: If78ed3a69179881368e7895b42940ce13b6a1a05
---
 gdb/async-event.c        | 27 ++++++++++++++++++++-
 gdb/event-top.c          | 52 +++++++++++++++++++++++++++++++++++++++-
 gdbserver/server.cc      | 15 ++++++++++++
 gdbsupport/event-loop.cc | 38 ++++++++++++++++++++++++-----
 gdbsupport/event-loop.h  | 44 ++++++++++++++++++++++++++++++++--
 5 files changed, 166 insertions(+), 10 deletions(-)

diff --git a/gdb/async-event.c b/gdb/async-event.c
index 55be014484e5..4228dfb09e68 100644
--- a/gdb/async-event.c
+++ b/gdb/async-event.c
@@ -157,8 +157,23 @@ create_async_signal_handler (sig_handler_func * proc,
    for some event.  The caller of this function is the interrupt
    handler associated with a signal.  */
 void
-mark_async_signal_handler (async_signal_handler * async_handler_ptr)
+mark_async_signal_handler (async_signal_handler *async_handler_ptr)
 {
+  if (debug_event_loop != debug_event_loop_kind::OFF)
+    {
+      /* This is called by signal handlers, so we print it "by hand" using
+	 the async-signal-safe methods.  */
+      const char head[] = ("[event-loop] mark_async_signal_handler: marking"
+			   "async signal handler `");
+      gdb_stdlog->write_async_safe (head, strlen (head));
+
+      gdb_stdlog->write_async_safe (async_handler_ptr->name,
+				    strlen (async_handler_ptr->name));
+
+      const char tail[] = "`\n";
+      gdb_stdlog->write_async_safe (tail, strlen (tail));
+    }
+
   async_handler_ptr->ready = 1;
   serial_event_set (async_signal_handlers_serial_event);
 }
@@ -168,6 +183,8 @@ mark_async_signal_handler (async_signal_handler * async_handler_ptr)
 void
 clear_async_signal_handler (async_signal_handler *async_handler_ptr)
 {
+  event_loop_debug_printf ("clearing async signal handler `%s`",
+			   async_handler_ptr->name);
   async_handler_ptr->ready = 0;
 }
 
@@ -211,6 +228,8 @@ invoke_async_signal_handlers (void)
       /* Async signal handlers have no connection to whichever was the
 	 current UI, and thus always run on the main one.  */
       current_ui = main_ui;
+      event_loop_debug_printf ("invoking async signal handler `%s`",
+			       async_handler_ptr->name);
       (*async_handler_ptr->proc) (async_handler_ptr->client_data);
     }
 
@@ -274,6 +293,8 @@ create_async_event_handler (async_event_handler_func *proc,
 void
 mark_async_event_handler (async_event_handler *async_handler_ptr)
 {
+  event_loop_debug_printf ("marking async event handler `%s`",
+			   async_handler_ptr->name);
   async_handler_ptr->ready = 1;
 }
 
@@ -282,6 +303,8 @@ mark_async_event_handler (async_event_handler *async_handler_ptr)
 void
 clear_async_event_handler (async_event_handler *async_handler_ptr)
 {
+  event_loop_debug_printf ("clearing async event handler `%s`",
+			   async_handler_ptr->name);
   async_handler_ptr->ready = 0;
 }
 
@@ -300,6 +323,8 @@ check_async_event_handlers ()
       if (async_handler_ptr->ready)
 	{
 	  async_handler_ptr->ready = 0;
+	  event_loop_debug_printf ("invoking async event handler `%s`",
+				   async_handler_ptr->name);
 	  (*async_handler_ptr->proc) (async_handler_ptr->client_data);
 	  return 1;
 	}
diff --git a/gdb/event-top.c b/gdb/event-top.c
index fdce5de6f429..6bf7a106bc97 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -525,7 +525,7 @@ void
 ui_register_input_event_handler (struct ui *ui)
 {
   add_file_handler (ui->input_fd, stdin_event_handler, ui,
-		    string_printf ("ui-%d", ui->num));
+		    string_printf ("ui-%d", ui->num), true /* is_ui */);
 }
 
 /* See top.h.  */
@@ -1287,3 +1287,53 @@ gdb_disable_readline (void)
     gdb_rl_callback_handler_remove ();
   delete_file_handler (ui->input_fd);
 }
+
+static const char debug_event_loop_off[] = "off";
+static const char debug_event_loop_all_except_ui[] = "all-except-ui";
+static const char debug_event_loop_all[] = "all";
+
+static const char *debug_event_loop_enum[] = {
+  debug_event_loop_off,
+  debug_event_loop_all_except_ui,
+  debug_event_loop_all,
+  nullptr
+};
+
+static const char *debug_event_loop_value = debug_event_loop_off;
+
+static void
+set_debug_event_loop_command (const char *args, int from_tty,
+			      cmd_list_element *c)
+{
+  if (debug_event_loop_value == debug_event_loop_off)
+    debug_event_loop = debug_event_loop_kind::OFF;
+  else if (debug_event_loop_value == debug_event_loop_all_except_ui)
+    debug_event_loop = debug_event_loop_kind::ALL_EXCEPT_UI;
+  else if (debug_event_loop_value == debug_event_loop_all)
+    debug_event_loop = debug_event_loop_kind::ALL;
+  else
+    gdb_assert_not_reached ("Invalid debug event look kind value.");
+}
+
+static void
+show_debug_event_loop_command (struct ui_file *file, int from_tty,
+			       cmd_list_element *cmd, const char *value)
+{
+  fprintf_filtered (file, _("Event loop debugging is %s.\n"), value);
+}
+
+void _initialize_event_top ();
+void
+_initialize_event_top ()
+{
+  add_setshow_enum_cmd ("event-loop", class_maintenance,
+			debug_event_loop_enum,
+			&debug_event_loop_value,
+			_("Set event-loop debugging."),
+			_("Show event-loop debugging."),
+			_("\
+Control whether to show event loop-related debug messages."),
+			set_debug_event_loop_command,
+			show_debug_event_loop_command,
+			&setdebuglist, &showdebuglist);
+}
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index d45154d1f547..e6314e56506b 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -955,6 +955,8 @@ monitor_show_help (void)
   monitor_output ("    Enable h/w breakpoint/watchpoint debugging messages\n");
   monitor_output ("  set remote-debug <0|1>\n");
   monitor_output ("    Enable remote protocol debugging messages\n");
+  monitor_output ("  set event-loop-debug <0|1>\n");
+  monitor_output ("    Enable event loop debugging messages\n");
   monitor_output ("  set debug-format option1[,option2,...]\n");
   monitor_output ("    Add additional information to debugging messages\n");
   monitor_output ("    Options: all, none");
@@ -1389,6 +1391,16 @@ handle_monitor_command (char *mon, char *own_buf)
       remote_debug = 0;
       monitor_output ("Protocol debug output disabled.\n");
     }
+  else if (strcmp (mon, "set event-loop-debug 1") == 0)
+    {
+      debug_event_loop = debug_event_loop_kind::ALL;
+      monitor_output ("Event loop debug output enabled.\n");
+    }
+  else if (strcmp (mon, "set event-loop-debug 0") == 0)
+    {
+      debug_event_loop = debug_event_loop_kind::OFF;
+      monitor_output ("Event loop debug output disabled.\n");
+    }
   else if (startswith (mon, "set debug-format "))
     {
       std::string error_msg
@@ -3468,6 +3480,7 @@ gdbserver_usage (FILE *stream)
 	   "                            none\n"
 	   "                            timestamp\n"
 	   "  --remote-debug        Enable remote protocol debugging output.\n"
+	   "  --event-loop-debug    Enable event loop debugging output.\n"
 	   "  --disable-packet=OPT1[,OPT2,...]\n"
 	   "                        Disable support for RSP packets or features.\n"
 	   "                          Options:\n"
@@ -3683,6 +3696,8 @@ captured_main (int argc, char *argv[])
 	}
       else if (strcmp (*next_arg, "--remote-debug") == 0)
 	remote_debug = 1;
+      else if (strcmp (*next_arg, "--event-loop-debug") == 0)
+	debug_event_loop = debug_event_loop_kind::ALL;
       else if (startswith (*next_arg, "--debug-file="))
 	debug_set_output ((*next_arg) + sizeof ("--debug-file=") -1);
       else if (strcmp (*next_arg, "--disable-packet") == 0)
diff --git a/gdbsupport/event-loop.cc b/gdbsupport/event-loop.cc
index 0d78122e0cc3..94941580d6ab 100644
--- a/gdbsupport/event-loop.cc
+++ b/gdbsupport/event-loop.cc
@@ -34,6 +34,10 @@
 #include "gdbsupport/gdb_sys_time.h"
 #include "gdbsupport/gdb_select.h"
 
+/* See event-loop.h.  */
+
+debug_event_loop_kind debug_event_loop;
+
 /* Tell create_file_handler what events we are interested in.
    This is used by the select version of the event loop.  */
 
@@ -64,6 +68,9 @@ struct file_handler
   /* User-friendly name of this handler.  Heap-allocated, owned by this.*/
   std::string *name;
 
+  /* If set, this file descriptor is used for a user interface.  */
+  bool is_ui;
+
   /* Was an error detected on this fd?  */
   int error;
 
@@ -164,7 +171,7 @@ timer_list;
 
 static void create_file_handler (int fd, int mask, handler_func *proc,
 				 gdb_client_data client_data,
-				 std::string &&name);
+				 std::string &&name, bool is_ui);
 static int gdb_wait_for_event (int);
 static int update_wait_timeout (void);
 static int poll_timers (void);
@@ -239,7 +246,7 @@ gdb_do_one_event (void)
 
 void
 add_file_handler (int fd, handler_func *proc, gdb_client_data client_data,
-		  std::string &&name)
+		  std::string &&name, bool is_ui)
 {
 #ifdef HAVE_POLL
   struct pollfd fds;
@@ -265,7 +272,8 @@ add_file_handler (int fd, handler_func *proc, gdb_client_data client_data,
   if (use_poll)
     {
 #ifdef HAVE_POLL
-      create_file_handler (fd, POLLIN, proc, client_data, std::move (name));
+      create_file_handler (fd, POLLIN, proc, client_data, std::move (name),
+			   is_ui);
 #else
       internal_error (__FILE__, __LINE__,
 		      _("use_poll without HAVE_POLL"));
@@ -273,7 +281,7 @@ add_file_handler (int fd, handler_func *proc, gdb_client_data client_data,
     }
   else
     create_file_handler (fd, GDB_READABLE | GDB_EXCEPTION,
-			 proc, client_data, std::move (name));
+			 proc, client_data, std::move (name), is_ui);
 }
 
 /* Helper for add_file_handler.
@@ -289,7 +297,8 @@ add_file_handler (int fd, handler_func *proc, gdb_client_data client_data,
 
 static void
 create_file_handler (int fd, int mask, handler_func * proc,
-		     gdb_client_data client_data, std::string &&name)
+		     gdb_client_data client_data, std::string &&name,
+		     bool is_ui)
 {
   file_handler *file_ptr;
 
@@ -358,6 +367,7 @@ create_file_handler (int fd, int mask, handler_func * proc,
   file_ptr->client_data = client_data;
   file_ptr->mask = mask;
   file_ptr->name = new std::string (std::move (name));
+  file_ptr->is_ui = is_ui;
 }
 
 /* Return the next file handler to handle, and advance to the next
@@ -558,7 +568,12 @@ handle_file_event (file_handler *file_ptr, int ready_mask)
 
 	  /* If there was a match, then call the handler.  */
 	  if (mask != 0)
-	    (*file_ptr->proc) (file_ptr->error, file_ptr->client_data);
+	    {
+	      event_loop_ui_debug_printf (file_ptr->is_ui,
+					  "invoking fd file handler `%s`",
+					  file_ptr->name->c_str ());
+	      file_ptr->proc (file_ptr->error, file_ptr->client_data);
+	    }
 	}
     }
 }
@@ -897,3 +912,14 @@ poll_timers (void)
 
   return 0;
 }
+
+/* See event-loop.h.  */
+
+void
+event_loop_debug_printf_1 (const char *func_name, const char *fmt, ...)
+{
+  va_list args;
+  va_start (args, fmt);
+  debug_prefixed_vprintf ("event-loop", func_name, fmt, args);
+  va_end (args);
+}
diff --git a/gdbsupport/event-loop.h b/gdbsupport/event-loop.h
index d7478b037a9c..c29d6a861236 100644
--- a/gdbsupport/event-loop.h
+++ b/gdbsupport/event-loop.h
@@ -84,11 +84,13 @@ extern void delete_file_handler (int fd);
 
    FD is the file descriptor for the file/stream to be listened to.
 
-   NAME is a user-friendly name for the handler.  */
+   NAME is a user-friendly name for the handler.
+
+   If IS_UI is set, this file descriptor is used for a user interface.  */
 
 extern void add_file_handler (int fd, handler_func *proc,
 			      gdb_client_data client_data,
-			      std::string &&name);
+			      std::string &&name, bool is_ui = false);
 
 extern int create_timer (int milliseconds, 
 			 timer_handler_func *proc, 
@@ -109,4 +111,42 @@ extern int invoke_async_signal_handlers ();
 
 extern int check_async_event_handlers ();
 
+enum class debug_event_loop_kind
+{
+  OFF,
+
+  /* Print all event-loop related messages, except events from user-interface
+     event sources.  */
+  ALL_EXCEPT_UI,
+
+  /* Print all event-loop related messages.  */
+  ALL,
+};
+
+/* True if we are printing event loop debug statements.  */
+extern debug_event_loop_kind debug_event_loop;
+
+/* Print an "event loop" debug statement.  Should be used through
+   event_loop_debug_printf.  */
+void ATTRIBUTE_PRINTF (2, 3) event_loop_debug_printf_1
+  (const char *func_name, const char *fmt, ...);
+
+#define event_loop_debug_printf(fmt, ...) \
+  do \
+    { \
+      if (debug_event_loop != debug_event_loop_kind::OFF) \
+	event_loop_debug_printf_1 (__func__, fmt, ##__VA_ARGS__); \
+    } \
+  while (0)
+
+#define event_loop_ui_debug_printf(is_ui, fmt, ...) \
+  do \
+    { \
+      if (debug_event_loop == debug_event_loop_kind::ALL \
+	  || (debug_event_loop == debug_event_loop_kind::ALL_EXCEPT_UI \
+	      && !is_ui)) \
+	event_loop_debug_printf_1 (__func__, fmt, ##__VA_ARGS__); \
+    } \
+  while (0)
+
 #endif /* EVENT_LOOP_H */
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] Add logging for event loop events
  2020-09-25 15:48 [PATCH 0/4] Add logging for event loop events Simon Marchi
                   ` (3 preceding siblings ...)
  2020-09-25 15:48 ` [PATCH 4/4] gdb: add debug prints in event loop Simon Marchi
@ 2020-10-02 18:49 ` Simon Marchi
  4 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-10-02 18:49 UTC (permalink / raw)
  To: gdb-patches

On 2020-09-25 11:48 a.m., Simon Marchi wrote:
> I currently have both hands in event loop / infrun stuff, and I found
> it useful to add debug prints to know when the event loop invoked an
> event handler.  I would like to propose this for upstream, so that I
> don't need to maintain my own patch on the side :).
> 
> Simon Marchi (4):
>   gdb: give names to event loop file handlers
>   gdb: give names to async event/signal handlers
>   gdb: move debug_prefixed_vprintf to gdbsupport
>   gdb: add debug prints in event loop
> 
>  gdb/async-event.c          | 49 +++++++++++++++++++++++-----
>  gdb/async-event.h          | 15 ++++++---
>  gdb/debug.c                | 13 +-------
>  gdb/debug.h                | 32 ------------------
>  gdb/event-top.c            | 67 +++++++++++++++++++++++++++++++++-----
>  gdb/infrun.c               |  5 +--
>  gdb/linux-nat.c            |  5 +--
>  gdb/record-btrace.c        |  2 +-
>  gdb/record-full.c          |  2 +-
>  gdb/remote-notif.c         |  2 +-
>  gdb/remote.c               |  3 +-
>  gdb/run-on-main-thread.c   |  3 +-
>  gdb/ser-base.c             |  6 ++--
>  gdb/top.c                  |  4 +++
>  gdb/top.h                  |  3 ++
>  gdb/tui/tui-io.c           |  2 +-
>  gdb/tui/tui-win.c          |  3 +-
>  gdbserver/linux-low.cc     |  3 +-
>  gdbserver/remote-utils.cc  | 10 +++---
>  gdbserver/server.cc        | 15 +++++++++
>  gdbsupport/common-debug.cc | 11 +++++++
>  gdbsupport/common-debug.h  |  6 ++++
>  gdbsupport/event-loop.cc   | 60 +++++++++++++++++++++++++---------
>  gdbsupport/event-loop.h    | 54 ++++++++++++++++++++++++++++--
>  24 files changed, 274 insertions(+), 101 deletions(-)
>  delete mode 100644 gdb/debug.h
> 
> -- 
> 2.28.0
> 

I pushed this.  Note that I received an informal +1 from Luis on IRC :).

Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-02 18:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 15:48 [PATCH 0/4] Add logging for event loop events Simon Marchi
2020-09-25 15:48 ` [PATCH 1/4] gdb: give names to event loop file handlers Simon Marchi
2020-09-25 15:48 ` [PATCH 2/4] gdb: give names to async event/signal handlers Simon Marchi
2020-09-25 15:48 ` [PATCH 3/4] gdb: move debug_prefixed_vprintf to gdbsupport Simon Marchi
2020-09-25 15:48 ` [PATCH 4/4] gdb: add debug prints in event loop Simon Marchi
2020-10-02 18:49 ` [PATCH 0/4] Add logging for event loop events Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).