gdb: Buffer output streams during events that might download debuginfo

Introduce new ui_file buffering_file to temporarily collect output written to gdb_std* output streams during print_thread, print_frame_info and print_stop_event. This ensures that output during these functions is not interrupted by debuginfod progress messages. With the addition of deferred debuginfo downloading it is possible for download progress messages to print during these events. Without any intervention we can end up with poorly formatted output: (gdb) backtrace [...] #8 0x00007fbe8af7d7cf in pygi_invoke_c_callable (Downloading separate debug info for /lib64/libpython3.11.so.1.0 function_cache=0x561221b224d0, state=<optimized out>... To fix this we buffer writes to gdb_std* output streams while allowing debuginfod progress messages to skip the buffers and print to the underlying output streams immediately. Buffered output is then written to the output streams. This ensures that progress messages print first, followed by uninterrupted frame/thread/stop info: (gdb) backtrace [...] Downloading separate debug info for /lib64/libpython3.11.so.1.0 #8 0x00007fbe8af7d7cf in pygi_invoke_c_callable (function_cache=0x561221b224d0, state=<optimized out>... Co-Authored-By: Andrew Burgess <[email protected]> Approved-By: Andrew Burgess <[email protected]>
tromey · Jan 19, 2024 · 519d634 · 519d634
1 parent 64db3e4
commit 519d634
Show file tree

Hide file tree

Showing 10 changed files with 492 additions and 98 deletions.
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
@@ -299,7 +299,7 @@ cli_ui_out::do_progress_notify (const std::string &msg,
 				double howmuch, double total)
 {
   int chars_per_line = get_chars_per_line ();
-  struct ui_file *stream = m_streams.back ();
+  struct ui_file *stream = get_unbuffered (m_streams.back ());
   cli_progress_info &info (m_progress_info.back ());
 
   if (chars_per_line > MAX_CHARS_PER_LINE)
@@ -384,7 +384,7 @@ cli_ui_out::do_progress_notify (const std::string &msg,
 void
 cli_ui_out::clear_progress_notify ()
 {
-  struct ui_file *stream = m_streams.back ();
+  struct ui_file *stream = get_unbuffered (m_streams.back ());
   int chars_per_line = get_chars_per_line ();
 
   scoped_restore save_pagination
@@ -413,10 +413,12 @@ void
 cli_ui_out::do_progress_end ()
 {
   struct ui_file *stream = m_streams.back ();
-  m_progress_info.pop_back ();
+  cli_progress_info &info (m_progress_info.back ());
 
-  if (stream->isatty ())
+  if (stream->isatty () && info.state != progress_update::START)
     clear_progress_notify ();
+
+  m_progress_info.pop_back ();
 }
 
 /* local functions */

diff --git a/gdb/cli-out.h b/gdb/cli-out.h
@@ -35,6 +35,9 @@ class cli_ui_out : public ui_out
 
   bool can_emit_style_escape () const override;
 
+  ui_file *current_stream () const override
+  { return m_streams.back (); }
+
 protected:
 
   virtual void do_table_begin (int nbrofcols, int nr_rows,

diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
@@ -155,7 +155,8 @@ progressfn (debuginfod_client *c, long cur, long total)
 
   if (check_quit_flag ())
     {
-      gdb_printf ("Cancelling download of %s %s...\n",
+      ui_file *outstream = get_unbuffered (gdb_stdout);
+      gdb_printf (outstream, _("Cancelling download of %s %s...\n"),
 		  data->desc, styled_fname.c_str ());
       return 1;
     }
@@ -295,10 +296,14 @@ static void
 print_outcome (int fd, const char *desc, const char *fname)
 {
   if (fd < 0 && fd != -ENOENT)
-    gdb_printf (_("Download failed: %s.  Continuing without %s %ps.\n"),
-		safe_strerror (-fd),
-		desc,
-		styled_string (file_name_style.style (), fname));
+    {
+      ui_file *outstream = get_unbuffered (gdb_stdout);
+      gdb_printf (outstream,
+		  _("Download failed: %s.  Continuing without %s %ps.\n"),
+		  safe_strerror (-fd),
+		  desc,
+		  styled_string (file_name_style.style (), fname));
+    }
 }
 
 /* See debuginfod-support.h  */

diff --git a/gdb/infrun.c b/gdb/infrun.c
@@ -9277,10 +9277,10 @@ print_stop_location (const target_waitstatus &ws)
     print_stack_frame (get_selected_frame (nullptr), 0, source_flag, 1);
 }
 
-/* See infrun.h.  */
+/* See `print_stop_event` in infrun.h.  */
 
-void
-print_stop_event (struct ui_out *uiout, bool displays)
+static void
+do_print_stop_event (struct ui_out *uiout, bool displays)
 {
   struct target_waitstatus last;
   struct thread_info *tp;
@@ -9309,6 +9309,16 @@ print_stop_event (struct ui_out *uiout, bool displays)
     }
 }
 
+/* See infrun.h.  This function itself sets up buffered output for the
+   duration of do_print_stop_event, which performs the actual event
+   printing.  */
+
+void
+print_stop_event (struct ui_out *uiout, bool displays)
+{
+  do_with_buffered_output (do_print_stop_event, uiout, displays);
+}
+
 /* See infrun.h.  */
 
 void

diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
@@ -45,6 +45,9 @@ class mi_ui_out : public ui_out
     return false;
   }
 
+  ui_file *current_stream () const override
+  { return m_streams.back (); }
+
 protected:
 
   virtual void do_table_begin (int nbrofcols, int nr_rows, const char *tblid)

diff --git a/gdb/python/py-uiout.h b/gdb/python/py-uiout.h
@@ -56,6 +56,9 @@ class py_ui_out : public ui_out
     return std::move (current ().obj);
   }
 
+  ui_file *current_stream () const override
+  { return nullptr; }
+
 protected:
 
   void do_progress_end () override { }

diff --git a/gdb/stack.c b/gdb/stack.c
@@ -220,7 +220,8 @@ static void print_frame_local_vars (frame_info_ptr frame,
 				    const char *regexp, const char *t_regexp,
 				    int num_tabs, struct ui_file *stream);
 
-static void print_frame (const frame_print_options &opts,
+static void print_frame (struct ui_out *uiout,
+			 const frame_print_options &opts,
 			 frame_info_ptr frame, int print_level,
 			 enum print_what print_what,  int print_args,
 			 struct symtab_and_line sal);
@@ -1020,16 +1021,15 @@ get_user_print_what_frame_info (std::optional<enum print_what> *what)
    Used in "where" output, and to emit breakpoint or step
    messages.  */
 
-void
-print_frame_info (const frame_print_options &fp_opts,
-		  frame_info_ptr frame, int print_level,
-		  enum print_what print_what, int print_args,
-		  int set_current_sal)
+static void
+do_print_frame_info (struct ui_out *uiout, const frame_print_options &fp_opts,
+		     frame_info_ptr frame, int print_level,
+		     enum print_what print_what, int print_args,
+		     int set_current_sal)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   int source_print;
   int location_print;
-  struct ui_out *uiout = current_uiout;
 
   if (!current_uiout->is_mi_like_p ()
       && fp_opts.print_frame_info != print_frame_info_auto)
@@ -1105,7 +1105,8 @@ print_frame_info (const frame_print_options &fp_opts,
 		    || print_what == LOC_AND_ADDRESS
 		    || print_what == SHORT_LOCATION);
   if (location_print || !sal.symtab)
-    print_frame (fp_opts, frame, print_level, print_what, print_args, sal);
+    print_frame (uiout, fp_opts, frame, print_level,
+		 print_what, print_args, sal);
 
   source_print = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
 
@@ -1185,6 +1186,20 @@ print_frame_info (const frame_print_options &fp_opts,
   gdb_flush (gdb_stdout);
 }
 
+/* Redirect output to a temporary buffer for the duration
+   of do_print_frame_info.  */
+
+void
+print_frame_info (const frame_print_options &fp_opts,
+		  frame_info_ptr frame, int print_level,
+		  enum print_what print_what, int print_args,
+		  int set_current_sal)
+{
+  do_with_buffered_output (do_print_frame_info, current_uiout,
+			   fp_opts, frame, print_level, print_what,
+			   print_args, set_current_sal);
+}
+
 /* See stack.h.  */
 
 void
@@ -1309,13 +1324,13 @@ find_frame_funname (frame_info_ptr frame, enum language *funlang,
 }
 
 static void
-print_frame (const frame_print_options &fp_opts,
+print_frame (struct ui_out *uiout,
+	     const frame_print_options &fp_opts,
 	     frame_info_ptr frame, int print_level,
 	     enum print_what print_what, int print_args,
 	     struct symtab_and_line sal)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct ui_out *uiout = current_uiout;
   enum language funlang = language_unknown;
   struct value_print_options opts;
   struct symbol *func;

diff --git a/gdb/thread.c b/gdb/thread.c
@@ -1089,6 +1089,107 @@ thread_target_id_str (thread_info *tp)
     return target_id;
 }
 
+/* Print thread TP.  GLOBAL_IDS indicates whether REQUESTED_THREADS
+   is a list of global or per-inferior thread ids.  */
+
+static void
+do_print_thread (ui_out *uiout, const char *requested_threads,
+		 int global_ids, int pid, int show_global_ids,
+		 int default_inf_num, thread_info *tp,
+		 thread_info *current_thread)
+{
+  int core;
+
+  /* In case REQUESTED_THREADS contains $_thread.  */
+  if (current_thread != nullptr)
+    switch_to_thread (current_thread);
+
+  if (!should_print_thread (requested_threads, default_inf_num,
+			    global_ids, pid, tp))
+    return;
+
+  ui_out_emit_tuple tuple_emitter (uiout, NULL);
+
+  if (!uiout->is_mi_like_p ())
+    {
+      if (tp == current_thread)
+	uiout->field_string ("current", "*");
+      else
+	uiout->field_skip ("current");
+
+      uiout->field_string ("id-in-tg", print_thread_id (tp));
+    }
+
+  if (show_global_ids || uiout->is_mi_like_p ())
+    uiout->field_signed ("id", tp->global_num);
+
+  /* Switch to the thread (and inferior / target).  */
+  switch_to_thread (tp);
+
+  /* For the CLI, we stuff everything into the target-id field.
+     This is a gross hack to make the output come out looking
+     correct.  The underlying problem here is that ui-out has no
+     way to specify that a field's space allocation should be
+     shared by several fields.  For MI, we do the right thing
+     instead.  */
+
+  if (uiout->is_mi_like_p ())
+    {
+      uiout->field_string ("target-id", target_pid_to_str (tp->ptid));
+
+      const char *extra_info = target_extra_thread_info (tp);
+      if (extra_info != nullptr)
+	uiout->field_string ("details", extra_info);
+
+      const char *name = thread_name (tp);
+      if (name != NULL)
+	uiout->field_string ("name", name);
+    }
+  else
+    {
+      uiout->field_string ("target-id", thread_target_id_str (tp));
+    }
+
+  if (tp->state == THREAD_RUNNING)
+    uiout->text ("(running)\n");
+  else
+    {
+      /* The switch above put us at the top of the stack (leaf
+	 frame).  */
+      print_stack_frame (get_selected_frame (NULL),
+			 /* For MI output, print frame level.  */
+			 uiout->is_mi_like_p (),
+			 LOCATION, 0);
+    }
+
+  if (uiout->is_mi_like_p ())
+    {
+      const char *state = "stopped";
+
+      if (tp->state == THREAD_RUNNING)
+	state = "running";
+      uiout->field_string ("state", state);
+    }
+
+  core = target_core_of_thread (tp->ptid);
+  if (uiout->is_mi_like_p () && core != -1)
+    uiout->field_signed ("core", core);
+}
+
+/* Redirect output to a temporary buffer for the duration
+   of do_print_thread.  */
+
+static void
+print_thread (ui_out *uiout, const char *requested_threads,
+	      int global_ids, int pid, int show_global_ids,
+	      int default_inf_num, thread_info *tp, thread_info *current_thread)
+
+{
+  do_with_buffered_output (do_print_thread, uiout, requested_threads,
+			   global_ids, pid, show_global_ids,
+			   default_inf_num, tp, current_thread);
+}
+
 /* Like print_thread_info, but in addition, GLOBAL_IDS indicates
    whether REQUESTED_THREADS is a list of global or per-inferior
    thread ids.  */
@@ -1176,86 +1277,13 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
     for (inferior *inf : all_inferiors ())
       for (thread_info *tp : inf->threads ())
 	{
-	  int core;
-
 	  any_thread = true;
+
 	  if (tp == current_thread && tp->state == THREAD_EXITED)
 	    current_exited = true;
 
-	  /* In case REQUESTED_THREADS contains $_thread.  */
-	  if (current_thread != nullptr)
-	    switch_to_thread (current_thread);
-
-	  if (!should_print_thread (requested_threads, default_inf_num,
-				    global_ids, pid, tp))
-	    continue;
-
-	  ui_out_emit_tuple tuple_emitter (uiout, NULL);
-
-	  if (!uiout->is_mi_like_p ())
-	    {
-	      if (tp == current_thread)
-		uiout->field_string ("current", "*");
-	      else
-		uiout->field_skip ("current");
-
-	      uiout->field_string ("id-in-tg", print_thread_id (tp));
-	    }
-
-	  if (show_global_ids || uiout->is_mi_like_p ())
-	    uiout->field_signed ("id", tp->global_num);
-
-	  /* Switch to the thread (and inferior / target).  */
-	  switch_to_thread (tp);
-
-	  /* For the CLI, we stuff everything into the target-id field.
-	     This is a gross hack to make the output come out looking
-	     correct.  The underlying problem here is that ui-out has no
-	     way to specify that a field's space allocation should be
-	     shared by several fields.  For MI, we do the right thing
-	     instead.  */
-
-	  if (uiout->is_mi_like_p ())
-	    {
-	      uiout->field_string ("target-id", target_pid_to_str (tp->ptid));
-
-	      const char *extra_info = target_extra_thread_info (tp);
-	      if (extra_info != nullptr)
-		uiout->field_string ("details", extra_info);
-
-	      const char *name = thread_name (tp);
-	      if (name != NULL)
-		uiout->field_string ("name", name);
-	    }
-	  else
-	    {
-	      uiout->field_string ("target-id", thread_target_id_str (tp));
-	    }
-
-	  if (tp->state == THREAD_RUNNING)
-	    uiout->text ("(running)\n");
-	  else
-	    {
-	      /* The switch above put us at the top of the stack (leaf
-		 frame).  */
-	      print_stack_frame (get_selected_frame (NULL),
-				 /* For MI output, print frame level.  */
-				 uiout->is_mi_like_p (),
-				 LOCATION, 0);
-	    }
-
-	  if (uiout->is_mi_like_p ())
-	    {
-	      const char *state = "stopped";
-
-	      if (tp->state == THREAD_RUNNING)
-		state = "running";
-	      uiout->field_string ("state", state);
-	    }
-
-	  core = target_core_of_thread (tp->ptid);
-	  if (uiout->is_mi_like_p () && core != -1)
-	    uiout->field_signed ("core", core);
+	  print_thread (uiout, requested_threads, global_ids, pid,
+			show_global_ids, default_inf_num, tp, current_thread);
 	}
 
     /* This end scope restores the current thread and the frame