public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 03/30] gdb: add interp::on_signal_received method
Date: Tue,  2 May 2023 16:49:43 -0400	[thread overview]
Message-ID: <20230502205011.132151-4-simon.marchi@efficios.com> (raw)
In-Reply-To: <20230502205011.132151-1-simon.marchi@efficios.com>

Instead of having the interpreter code registering observers for the
signal_received observable, add a "signal_received" virtual method to
struct interp.  Add a interps_notify_signal_received function that loops
over all UIs and calls the signal_received method on the interpreter.
Finally, add a notify_signal_received function that calls
interps_notify_signal_received and then notifies the observers.  Replace
all existing notifications to the signal_received observers with calls
to notify_signal_received.

Before this patch, the CLI and MI code both register a signal_received
observer.  These observer go over all UIs, and, for those that have a
interpreter of the right kind, print the stop notifiation.

After this patch, we have just one "loop over all UIs", inside
interps_notify_signal_received.  Since the interp::on_signal_received
method gets called once for each interpreter, the implementations only
need to deal with the current interpreter (the "this" pointer).

The motivation for this patch comes from a future patch, that makes the
amdgpu code register an observer to print a warning after the CLI's
signal stop message.  Since the amdgpu and the CLI code both use
observers, the order of the two messages is not stable, unless we define
the priority using the observer dependency system.  However, the
approach of using virtual methods on the interpreters seems like a good
change anyway, I think it's more straightforward and simple to
understand than the current solution that uses observers.  We are sure
that the amdgpu message gets printed after the CLI message, since
observers are notified after interpreters.

Keep the signal_received, even if nothing uses if, because we will be
using it in the upcoming amdgpu patch implementing the warning described
above.

Change-Id: I4d8614bb8f6e0717f4bfc2a59abded3702f23ac4
---
 gdb/cli/cli-interp.c | 17 +++--------------
 gdb/cli/cli-interp.h |  2 ++
 gdb/infrun.c         | 14 ++++++++++++--
 gdb/infrun.h         |  4 ++++
 gdb/interps.c        | 23 +++++++++++++++++++++++
 gdb/interps.h        |  8 ++++++++
 gdb/mi/mi-interp.c   | 20 ++++----------------
 gdb/mi/mi-interp.h   |  2 ++
 gdb/remote.c         |  2 +-
 9 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 84fe34a10148..51c78d96ee20 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -137,19 +137,10 @@ cli_base_on_normal_stop (struct bpstat *bs, int print_frame)
     }
 }
 
-/* Observer for the signal_received notification.  */
-
-static void
-cli_base_on_signal_received (enum gdb_signal siggnal)
+void
+cli_interp_base::on_signal_received (enum gdb_signal siggnal)
 {
-  SWITCH_THRU_ALL_UIS ()
-    {
-      cli_interp_base *cli = as_cli_interp_base (top_level_interpreter ());
-      if (cli == nullptr)
-	continue;
-
-      print_signal_received_reason (cli->interp_ui_out (), siggnal);
-    }
+  print_signal_received_reason (this->interp_ui_out (), siggnal);
 }
 
 /* Observer for the signalled notification.  */
@@ -408,8 +399,6 @@ _initialize_cli_interp ()
   /* Note these all work for both the CLI and TUI interpreters.  */
   gdb::observers::normal_stop.attach (cli_base_on_normal_stop,
 				      "cli-interp-base");
-  gdb::observers::signal_received.attach (cli_base_on_signal_received,
-					  "cli-interp-base");
   gdb::observers::signal_exited.attach (cli_base_on_signal_exited,
 					"cli-interp-base");
   gdb::observers::exited.attach (cli_base_on_exited, "cli-interp-base");
diff --git a/gdb/cli/cli-interp.h b/gdb/cli/cli-interp.h
index 5ed998f58bd5..f7bee4530b42 100644
--- a/gdb/cli/cli-interp.h
+++ b/gdb/cli/cli-interp.h
@@ -33,6 +33,8 @@ class cli_interp_base : public interp
   void pre_command_loop () override;
   bool supports_command_editing () override;
 
+  void on_signal_received (gdb_signal sig) override;
+
 private:
   struct saved_output_files
   {
diff --git a/gdb/infrun.c b/gdb/infrun.c
index efe2c00c489a..4716d73571c5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -76,6 +76,7 @@
 #include "gdbsupport/buildargv.h"
 #include "extension.h"
 #include "disasm.h"
+#include "interps.h"
 
 /* Prototypes for local functions */
 
@@ -6263,6 +6264,15 @@ finish_step_over (struct execution_control_state *ecs)
   return 0;
 }
 
+/* See infrun.h.  */
+
+void
+notify_signal_received (gdb_signal sig)
+{
+  interps_notify_signal_received (sig);
+  gdb::observers::signal_received.notify (sig);
+}
+
 /* Come here when the program has stopped with a signal.  */
 
 static void
@@ -6686,7 +6696,7 @@ handle_signal_stop (struct execution_control_state *ecs)
 	{
 	  /* The signal table tells us to print about this signal.  */
 	  target_terminal::ours_for_output ();
-	  gdb::observers::signal_received.notify (ecs->event_thread->stop_signal ());
+	  notify_signal_received (ecs->event_thread->stop_signal ());
 	  target_terminal::inferior ();
 	}
 
@@ -8828,7 +8838,7 @@ normal_stop ()
   update_thread_list ();
 
   if (last.kind () == TARGET_WAITKIND_STOPPED && stopped_by_random_signal)
-    gdb::observers::signal_received.notify (inferior_thread ()->stop_signal ());
+    notify_signal_received (inferior_thread ()->stop_signal ());
 
   /* As with the notification of thread events, we want to delay
      notifying the user that we've switched thread context until
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 9513bc570e46..f7b60a4d9fb7 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -210,6 +210,10 @@ extern void set_step_info (thread_info *tp,
 			   frame_info_ptr frame,
 			   struct symtab_and_line sal);
 
+/* Notify interpreters and observers that the current inferior has stopped with
+   signal SIG.  */
+extern void notify_signal_received (gdb_signal sig);
+
 /* Several print_*_reason helper functions to print why the inferior
    has stopped to the passed in UIOUT.  */
 
diff --git a/gdb/interps.c b/gdb/interps.c
index e3f6ee685123..5d061ad52aff 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -383,6 +383,29 @@ current_interpreter (void)
   return current_ui->current_interpreter;
 }
 
+/* Helper interps_notify_* functions.  Call METHOD on the top-level interpreter
+   of all UIs.  */
+
+template <typename ...Args>
+void
+interps_notify (void (interp::*method) (Args...), Args... args)
+{
+  SWITCH_THRU_ALL_UIS ()
+    {
+      interp *tli = top_level_interpreter ();
+      if (tli != nullptr)
+	(tli->*method) (args...);
+    }
+}
+
+/* See interps.h.  */
+
+void
+interps_notify_signal_received (gdb_signal sig)
+{
+  interps_notify (&interp::on_signal_received, sig);
+}
+
 /* This just adds the "interpreter-exec" command.  */
 void _initialize_interpreter ();
 void
diff --git a/gdb/interps.h b/gdb/interps.h
index da78a5d89fae..4762c4c93ff1 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -82,6 +82,10 @@ class interp : public intrusive_list_node<interp>
   const char *name () const
   { return m_name; }
 
+  /* Notify the interpreter that the current inferior has stopped with signal
+     SIG.  */
+  virtual void on_signal_received (gdb_signal sig) {}
+
 private:
   /* The memory for this is static, it comes from literal strings (e.g. "cli").  */
   const char *m_name;
@@ -170,6 +174,10 @@ extern void interpreter_completer (struct cmd_list_element *ignore,
 				   const char *text,
 				   const char *word);
 
+/* Notify all interpreters that the current inferior has stopped with signal
+   SIG.  */
+extern void interps_notify_signal_received (gdb_signal sig);
+
 /* well-known interpreters */
 #define INTERP_CONSOLE		"console"
 #define INTERP_MI2             "mi2"
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index b791d6b9a2d3..43e49a9b94af 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -60,7 +60,6 @@ static int mi_interp_query_hook (const char *ctlstr, va_list ap)
 static void mi_insert_notify_hooks (void);
 static void mi_remove_notify_hooks (void);
 
-static void mi_on_signal_received (enum gdb_signal siggnal);
 static void mi_on_signal_exited (enum gdb_signal siggnal);
 static void mi_on_exited (int exitstatus);
 static void mi_on_normal_stop (struct bpstat *bs, int print_frame);
@@ -524,21 +523,11 @@ find_mi_interp (void)
    inferior has stopped to both the MI event channel and to the MI
    console.  If the MI interpreter is not active, print nothing.  */
 
-/* Observer for the signal_received notification.  */
-
-static void
-mi_on_signal_received (enum gdb_signal siggnal)
+void
+mi_interp::on_signal_received (enum gdb_signal siggnal)
 {
-  SWITCH_THRU_ALL_UIS ()
-    {
-      struct mi_interp *mi = find_mi_interp ();
-
-      if (mi == NULL)
-	continue;
-
-      print_signal_received_reason (mi->mi_uiout, siggnal);
-      print_signal_received_reason (mi->cli_uiout, siggnal);
-    }
+  print_signal_received_reason (this->mi_uiout, siggnal);
+  print_signal_received_reason (this->cli_uiout, siggnal);
 }
 
 /* Observer for the signal_exited notification.  */
@@ -1303,7 +1292,6 @@ _initialize_mi_interp ()
   interp_factory_register (INTERP_MI4, mi_interp_factory);
   interp_factory_register (INTERP_MI, mi_interp_factory);
 
-  gdb::observers::signal_received.attach (mi_on_signal_received, "mi-interp");
   gdb::observers::signal_exited.attach (mi_on_signal_exited, "mi-interp");
   gdb::observers::exited.attach (mi_on_exited, "mi-interp");
   gdb::observers::no_history.attach (mi_on_no_history, "mi-interp");
diff --git a/gdb/mi/mi-interp.h b/gdb/mi/mi-interp.h
index 2d3a0013d4b0..75c17568d6fc 100644
--- a/gdb/mi/mi-interp.h
+++ b/gdb/mi/mi-interp.h
@@ -42,6 +42,8 @@ class mi_interp final : public interp
 		    bool debug_redirect) override;
   void pre_command_loop () override;
 
+  void on_signal_received (gdb_signal sig) override;
+
   /* MI's output channels */
   mi_console_file *out;
   mi_console_file *err;
diff --git a/gdb/remote.c b/gdb/remote.c
index 8eaa1b2c4d15..829cdbdef757 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4720,7 +4720,7 @@ remote_target::print_one_stopped_thread (thread_info *thread)
       enum gdb_signal sig = ws.sig ();
 
       if (signal_print_state (sig))
-	gdb::observers::signal_received.notify (sig);
+	notify_signal_received (sig);
     }
   gdb::observers::normal_stop.notify (NULL, 1);
 }
-- 
2.40.1


  parent reply	other threads:[~2023-05-02 20:50 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-02 20:49 [PATCH 00/30] Switch interpreters to use virtual methods Simon Marchi
2023-05-02 20:49 ` [PATCH 01/30] gdb/mi: fix ^running record with multiple MI interpreters Simon Marchi
2023-05-03 14:42   ` Alexandra Petlanova Hajkova
2023-05-03 15:02     ` Simon Marchi
2023-05-29 14:53   ` Simon Marchi
2023-05-02 20:49 ` [PATCH 02/30] gdb/mi: make current_token a field of mi_interp Simon Marchi
2023-05-04 12:31   ` Alexandra Petlanova Hajkova
2023-05-02 20:49 ` Simon Marchi [this message]
2023-05-04 14:38   ` [PATCH 03/30] gdb: add interp::on_signal_received method Alexandra Petlanova Hajkova
2023-05-02 20:49 ` [PATCH 04/30] gdb: add interp::on_normal_stop method Simon Marchi
2023-05-02 20:49 ` [PATCH 05/30] gdb: add interp::on_signal_exited method Simon Marchi
2023-05-02 20:49 ` [PATCH 06/30] gdb: add interp::on_exited method Simon Marchi
2023-05-02 20:49 ` [PATCH 07/30] gdb: add interp::on_no_history method Simon Marchi
2023-05-02 20:49 ` [PATCH 08/30] gdb: add interp::on_sync_execution_done method Simon Marchi
2023-05-02 20:49 ` [PATCH 09/30] gdb: add interp::on_command_error method Simon Marchi
2023-05-02 20:49 ` [PATCH 10/30] gdb: add interp::on_user_selected_context_changed method Simon Marchi
2023-05-02 20:49 ` [PATCH 11/30] gdb: add interp::on_new_thread method Simon Marchi
2023-05-02 20:49 ` [PATCH 12/30] gdb: add interp::on_thread_exited method Simon Marchi
2023-05-02 20:49 ` [PATCH 13/30] gdb: add interp::on_inferior_added method Simon Marchi
2023-05-02 20:49 ` [PATCH 14/30] gdb: add interp::on_inferior_appeared method Simon Marchi
2023-05-02 20:49 ` [PATCH 15/30] gdb: add interp::on_inferior_disappeared method Simon Marchi
2023-05-02 20:49 ` [PATCH 16/30] gdb: add interp::on_inferior_removed method Simon Marchi
2023-05-02 20:49 ` [PATCH 17/30] gdb: add interp::on_record_changed method Simon Marchi
2023-05-02 20:49 ` [PATCH 18/30] gdb: add interp::on_target_resumed method Simon Marchi
2023-05-02 20:49 ` [PATCH 19/30] gdb: add interp::on_solib_loaded method Simon Marchi
2023-05-02 20:50 ` [PATCH 20/30] gdb: add interp::on_solib_unloaded method Simon Marchi
2023-05-02 20:50 ` [PATCH 21/30] gdb: add interp::on_about_to_proceed method Simon Marchi
2023-05-02 20:50 ` [PATCH 22/30] gdb: add interp::on_traceframe_changed method Simon Marchi
2023-05-02 20:50 ` [PATCH 23/30] gdb: add interp::on_tsv_created method Simon Marchi
2023-05-02 20:50 ` [PATCH 24/30] gdb: add interp::on_tsv_deleted method Simon Marchi
2023-05-02 20:50 ` [PATCH 25/30] gdb: add interp::on_tsv_modified method Simon Marchi
2023-05-02 20:50 ` [PATCH 26/30] gdb: add interp::on_breakpoint_created method Simon Marchi
2023-05-02 20:50 ` [PATCH 27/30] gdb: add interp::on_breakpoint_deleted method Simon Marchi
2023-05-02 20:50 ` [PATCH 28/30] gdb: add interp::on_breakpoint_modified method Simon Marchi
2023-05-02 20:50 ` [PATCH 29/30] gdb: add interp::on_param_changed method Simon Marchi
2023-05-02 20:50 ` [PATCH 30/30] gdb: add interp::on_memory_changed method Simon Marchi
2023-05-02 20:50 ` [PATCH 00/30] Make interpreters use virtual methods (instead of observers) Simon Marchi
2023-05-23 12:43 ` [PATCH 00/30] Switch interpreters to use virtual methods Simon Marchi
2023-05-30 19:09   ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230502205011.132151-4-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).