public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Remove unchecked casts to mi_interp
@ 2023-06-09 16:17 Tom Tromey
  2023-07-07 19:55 ` Tom Tromey
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Tromey @ 2023-06-09 16:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Tom Tromey

Simon noticed a crash that could be caused via new Python
gdb.execute_mi function.  Looking into this, I found a few unchecked
casts to mi_interp, like:

-  struct mi_interp *mi = (struct mi_interp *) command_interp ();

This patch replaces all such casts with safer variants.

For -gdb-exit and mi_load_progress, I chose to have the functions
simply not generate any output.  It didn't seem useful to do so.

Some casts I eliminated by adding a parameter to a function.  Then, in
mi_execute_command, I changed the code to use
gdb::checked_static_cast.  This is appropriate because this particular
overload can only be called by the MI interpreter.

There does not seem to be a very good way to test -gdb-exit.

Regression tested on x86-64 Fedora 36.
---
 gdb/mi/mi-interp.c |  9 ---------
 gdb/mi/mi-interp.h |  9 +++++++++
 gdb/mi/mi-main.c   | 48 +++++++++++++++++++++++++++++-----------------
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index a8de62ba630..0e51c884a65 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -72,15 +72,6 @@ display_mi_prompt (struct mi_interp *mi)
   ui->prompt_state = PROMPTED;
 }
 
-/* Returns the INTERP's data cast as mi_interp if INTERP is an MI, and
-   returns NULL otherwise.  */
-
-static struct mi_interp *
-as_mi_interp (struct interp *interp)
-{
-  return dynamic_cast<mi_interp *> (interp);
-}
-
 void
 mi_interp::on_command_error ()
 {
diff --git a/gdb/mi/mi-interp.h b/gdb/mi/mi-interp.h
index ce003242801..6326ec7d524 100644
--- a/gdb/mi/mi-interp.h
+++ b/gdb/mi/mi-interp.h
@@ -107,4 +107,13 @@ class mi_interp final : public interp
 
 void mi_output_solib_attribs (ui_out *uiout, struct so_list *solib);
 
+/* Returns the INTERP's data cast as mi_interp if INTERP is an MI, and
+   returns NULL otherwise.  */
+
+static inline struct mi_interp *
+as_mi_interp (struct interp *interp)
+{
+  return dynamic_cast<mi_interp *> (interp);
+}
+
 #endif /* MI_MI_INTERP_H */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 7503ffd2f18..6868852ca8a 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -144,14 +144,21 @@ static void print_diff (struct ui_file *file, struct mi_timestamp *start,
 void
 mi_cmd_gdb_exit (const char *command, const char *const *argv, int argc)
 {
-  struct mi_interp *mi = (struct mi_interp *) current_interpreter ();
-
-  /* We have to print everything right here because we never return.  */
-  if (current_token)
-    gdb_puts (current_token, mi->raw_stdout);
-  gdb_puts ("^exit\n", mi->raw_stdout);
-  mi_out_put (current_uiout, mi->raw_stdout);
-  gdb_flush (mi->raw_stdout);
+  struct mi_interp *mi = as_mi_interp (current_interpreter ());
+
+  /* If the current interpreter is not an MI interpreter, then just
+     don't bother printing anything.  This case can arise from using
+     the Python gdb.execute_mi function -- but here the result does
+     not matter, as gdb is about to exit anyway.  */
+  if (mi != nullptr)
+    {
+      /* We have to print everything right here because we never return.  */
+      if (current_token)
+	gdb_puts (current_token, mi->raw_stdout);
+      gdb_puts ("^exit\n", mi->raw_stdout);
+      mi_out_put (current_uiout, mi->raw_stdout);
+      gdb_flush (mi->raw_stdout);
+    }
   /* FIXME: The function called is not yet a formal libgdb function.  */
   quit_force (NULL, FROM_TTY);
 }
@@ -1802,10 +1809,9 @@ mi_cmd_remove_inferior (const char *command, const char *const *argv, int argc)
    prompt, display error).  */
 
 static void
-captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
+captured_mi_execute_command (struct mi_interp *mi, struct ui_out *uiout,
+			     struct mi_parse *context)
 {
-  struct mi_interp *mi = (struct mi_interp *) command_interp ();
-
   if (do_timings)
     current_command_ts = context->cmd_start;
 
@@ -1890,10 +1896,9 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
 /* Print a gdb exception to the MI output stream.  */
 
 static void
-mi_print_exception (const char *token, const struct gdb_exception &exception)
+mi_print_exception (struct mi_interp *mi, const char *token,
+		    const struct gdb_exception &exception)
 {
-  struct mi_interp *mi = (struct mi_interp *) current_interpreter ();
-
   gdb_puts (token, mi->raw_stdout);
   gdb_puts ("^error,msg=\"", mi->raw_stdout);
   if (exception.message == NULL)
@@ -1925,13 +1930,15 @@ mi_execute_command (const char *cmd, int from_tty)
 
   target_log_command (cmd);
 
+  struct mi_interp *mi
+    = gdb::checked_static_cast<mi_interp *> (command_interp ());
   try
     {
       command = mi_parse::make (cmd, &token);
     }
   catch (const gdb_exception &exception)
     {
-      mi_print_exception (token, exception);
+      mi_print_exception (mi, token, exception);
       xfree (token);
     }
 
@@ -1947,7 +1954,7 @@ mi_execute_command (const char *cmd, int from_tty)
 
       try
 	{
-	  captured_mi_execute_command (current_uiout, command.get ());
+	  captured_mi_execute_command (mi, current_uiout, command.get ());
 	}
       catch (const gdb_exception &result)
 	{
@@ -1960,7 +1967,7 @@ mi_execute_command (const char *cmd, int from_tty)
 
 	  /* The command execution failed and error() was called
 	     somewhere.  */
-	  mi_print_exception (command->token, result);
+	  mi_print_exception (mi, command->token, result);
 	  mi_out_rewind (current_uiout);
 
 	  /* Throw to a higher level catch for SIGTERM sent to GDB.  */
@@ -2189,7 +2196,12 @@ mi_load_progress (const char *section_name,
   static steady_clock::time_point last_update;
   static char *previous_sect_name = NULL;
   int new_section;
-  struct mi_interp *mi = (struct mi_interp *) current_interpreter ();
+  struct mi_interp *mi = as_mi_interp (current_interpreter ());
+
+  /* If the current interpreter is not an MI interpreter, then just
+     don't bother printing anything.  */
+  if (mi == nullptr)
+    return;
 
   /* This function is called through deprecated_show_load_progress
      which means uiout may not be correct.  Fix it for the duration
-- 
2.40.1


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

* Re: [PATCH] Remove unchecked casts to mi_interp
  2023-06-09 16:17 [PATCH] Remove unchecked casts to mi_interp Tom Tromey
@ 2023-07-07 19:55 ` Tom Tromey
  0 siblings, 0 replies; 2+ messages in thread
From: Tom Tromey @ 2023-07-07 19:55 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey, Simon Marchi

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Simon noticed a crash that could be caused via new Python
Tom> gdb.execute_mi function.  Looking into this, I found a few unchecked
Tom> casts to mi_interp, like:

Tom> -  struct mi_interp *mi = (struct mi_interp *) command_interp ();

Tom> This patch replaces all such casts with safer variants.

I'm checking this in.

Tom

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

end of thread, other threads:[~2023-07-07 19:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 16:17 [PATCH] Remove unchecked casts to mi_interp Tom Tromey
2023-07-07 19:55 ` Tom Tromey

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).