Re: [PATCH 3/6 v3] gdb: Buffer gdb_stdout during events that might download deferred debuginfo

[Andrew Burgess <aburgess@redhat.com>]

Aaron Merey <amerey@redhat.com> writes:

> v2: https://sourceware.org/pipermail/gdb-patches/2023-April/198944.html
>
> v3 simplifies do_redirect_to_buffer and adds output buffering during
> print_stop_event.
>
> This patch currently introduces regressions to three gdb.base/solib-display.exp
> 'after rerun' tests.  Because print_stop_event output to gdb_stdout is
> now buffered until the function exits, any writes to gdb_stderr during
> print_stop_event will always appear before any gdb_stdout output.
>
> In these testcases instead of getting the following expected output:
>
> ```
>
> Breakpoint 1, main () at /home/amerey/binutils-gdb/gdb/testsuite/gdb.base/solib-display-main.c:30
> 30        bar ();
> 1: (int) a_global = 41
> warning: Unable to display "(int) b_global": No symbol "b_global" in current context.
> 3: (int) c_global = 43
> ```
>
> gdb instead prints:
>
> ```
> warning: Unable to display "(int) b_global": No symbol "b_global" in current context.
>
> Breakpoint 1, main () at /home/amerey/binutils-gdb/gdb/testsuite/gdb.base/solib-display-main.c:30
> 30        bar ();
> 1: (int) a_global = 41
> 3: (int) c_global = 43
> ```
>
> I'm working on a fix that buffers both gdb_stdout and gdb_stderr and
> organizes the flush so that output to both streams appears in the
> same order that it was written.  However I didn't think this issue is
> severe enough to hold off on posting this patch as it is.  I can
> file a PR and mark these tests as KFAIL for now if desired.

If you're working on an updated patch then I think we have the time to
wait for that.  I don't see a need to get a partial fix in first, unless
I'm missing something.

Thanks,
Andrew

>
> Commit message:
>
> Introduce new ui_file buffer_file to temporarily collect output
> during print_thread, print_frame_info and print_stop_event.
>
> This ensures that output from 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 accumulate writes to gdb_stdout in a buffer_file
> while progress messages skip the buffer and print to gdb_stdout
> immediately.  This ensures 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>...
> ---
>  gdb/cli-out.c            |  28 ++++++-
>  gdb/cli-out.h            |   1 +
>  gdb/debuginfod-support.c |  15 ++--
>  gdb/infrun.c             |  17 +++-
>  gdb/mi/mi-out.c          |  29 +++++++
>  gdb/mi/mi-out.h          |   1 +
>  gdb/python/py-mi.c       |   2 +
>  gdb/stack.c              |  38 ++++++---
>  gdb/thread.c             | 174 +++++++++++++++++++++++----------------
>  gdb/ui-file.c            |  62 ++++++++++++++
>  gdb/ui-file.h            |  45 +++++++++-
>  gdb/ui-out.c             |  21 +++++
>  gdb/ui-out.h             |  66 +++++++++++++++
>  13 files changed, 405 insertions(+), 94 deletions(-)
>
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index 20d3d93f1ad..a188457711c 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -263,6 +263,25 @@ cli_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>  
> +void
> +cli_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> +{
> +  auto it = std::find (m_streams.begin (), m_streams.end (), gdb_stdout);
> +
> +  if (it == m_streams.end ())
> +    return;
> +
> +  if (buf_file != nullptr)
> +    *it = buf_file;
> +  else
> +    {
> +      buffer_file *buf = dynamic_cast<buffer_file *>(*it);
> +
> +      if (buf != nullptr)
> +	*it = buf->get_stream ();
> +    }
> +}
> +
>  /* Initialize a progress update to be displayed with
>     cli_ui_out::do_progress_notify.  */
>  
> @@ -299,7 +318,8 @@ 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
> +    = buffer_file::get_unbuffered_stream (m_streams.back ());
>    cli_progress_info &info (m_progress_info.back ());
>  
>    if (chars_per_line > MAX_CHARS_PER_LINE)
> @@ -384,7 +404,8 @@ 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
> +    = buffer_file::get_unbuffered_stream (m_streams.back ());
>    int chars_per_line = get_chars_per_line ();
>  
>    scoped_restore save_pagination
> @@ -413,10 +434,11 @@ void
>  cli_ui_out::do_progress_end ()
>  {
>    struct ui_file *stream = m_streams.back ();
> -  m_progress_info.pop_back ();
>  
>    if (stream->isatty ())
>      clear_progress_notify ();
> +
> +  m_progress_info.pop_back ();
>  }
>  
>  /* local functions */
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index 34016182269..fa8ca1114e1 100644
> --- a/gdb/cli-out.h
> +++ b/gdb/cli-out.h
> @@ -71,6 +71,7 @@ class cli_ui_out : public ui_out
>    virtual void do_wrap_hint (int indent) override;
>    virtual void do_flush () override;
>    virtual void do_redirect (struct ui_file *outstream) override;
> +  virtual void do_redirect_to_buffer (buffer_file *buf_file) override;
>  
>    virtual void do_progress_start () override;
>    virtual void do_progress_notify (const std::string &, const char *,
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 6d0521b64e2..548e8c5f76b 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -168,7 +168,8 @@ progressfn (debuginfod_client *c, long cur, long total)
>  
>    if (check_quit_flag ())
>      {
> -      gdb_printf ("Cancelling download of %s %s...\n",
> +      ui_file *outstream = buffer_file::get_unbuffered_stream (gdb_stdout);
> +      gdb_printf (outstream, "Cancelling download of %s %s...\n",
>  		  data->desc, styled_fname.c_str ());
>        return 1;
>      }
> @@ -284,10 +285,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 = buffer_file::get_unbuffered_stream (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
> index 4d6df757d23..f719a4aad49 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8706,10 +8706,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;
> @@ -8738,6 +8738,17 @@ 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<void (ui_out *, bool)>
> +    (do_print_stop_event, uiout, displays);
> +}
> +
>  /* See infrun.h.  */
>  
>  void
> diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
> index 29a416a426d..b763d2a785c 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -194,6 +194,35 @@ mi_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>  
> +void
> +mi_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> +{
> +  auto it = std::find (m_streams.begin (), m_streams.end (), gdb_stdout);
> +
> +  if (it == m_streams.end ())
> +    return;
> +
> +  if (buf_file != nullptr)
> +    {
> +      string_file *sstream = dynamic_cast<string_file *>(*it);
> +
> +      if (sstream != nullptr)
> +	{
> +	  buf_file->write (sstream->data (), sstream->size ());
> +	  sstream->clear ();
> +	}
> +
> +      *it = buf_file;
> +    }
> +  else
> +    {
> +      buffer_file *buf = dynamic_cast<buffer_file *>(*it);
> +
> +      if (buf != nullptr)
> +	*it = buf->get_stream ();
> +    }
> +}
> +
>  void
>  mi_ui_out::field_separator ()
>  {
> diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
> index 10c9f8a4585..d2f2345daf5 100644
> --- a/gdb/mi/mi-out.h
> +++ b/gdb/mi/mi-out.h
> @@ -79,6 +79,7 @@ class mi_ui_out : public ui_out
>    virtual void do_wrap_hint (int indent) override;
>    virtual void do_flush () override;
>    virtual void do_redirect (struct ui_file *outstream) override;
> +  virtual void do_redirect_to_buffer (buffer_file *buf_file) override;
>  
>    virtual bool do_is_mi_like_p () const override
>    { return true; }
> diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
> index 0fcd57844e7..34540901888 100644
> --- a/gdb/python/py-mi.c
> +++ b/gdb/python/py-mi.c
> @@ -62,6 +62,8 @@ class py_ui_out : public ui_out
>    void do_progress_notify (const std::string &, const char *, double, double)
>      override
>    { }
> +  void do_redirect_to_buffer (buffer_file *) override
> +  { }
>  
>    void do_table_begin (int nbrofcols, int nr_rows, const char *tblid) override
>    {
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 002bf580634..608bb5fc1f0 100644
> --- 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 (gdb::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,23 @@ 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)
> +{
> +  using ftype = void (ui_out *, const frame_print_options &, frame_info_ptr,
> +		      int, enum print_what, int, int);
> +
> +  do_with_buffered_output<ftype> (do_print_frame_info, current_uiout,
> +				  fp_opts, frame, print_level, print_what,
> +				  print_args, set_current_sal);
> +}
> +
>  /* See stack.h.  */
>  
>  void
> @@ -1309,13 +1327,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
> index 7f7f035b5ab..03105fbd4ce 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1040,6 +1040,106 @@ 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;
> +
> +  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)
> +
> +{
> +  using ftype = void (ui_out *, const char *, int, int, int,
> +		      int, thread_info *, thread_info *);
> +
> +  do_with_buffered_output<ftype>
> +    (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.  */
> @@ -1123,82 +1223,12 @@ 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;
>  
> -	  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
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index e0814fe2b2d..73277cd853e 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -234,6 +234,68 @@ string_file::can_emit_style_escape ()
>  
>  \f
>  
> +/* See ui-file.h.  */
> +
> +void
> +buffer_file::wrap_here (int indent)
> +{
> +  m_string_wraps.emplace (m_string_wraps.end (),
> +			  string_wrap_pair (m_string, indent));
> +  m_string.clear ();
> +}
> +
> +/* See ui-file.h.  */
> +
> +void
> +buffer_file::flush_to_stream ()
> +{
> +  /* Add m_string to m_string_wraps with no corresponding wrap_here.  */
> +  wrap_here (-1);
> +
> +  for (string_wrap_pair pair : m_string_wraps)
> +    {
> +      std::string buf = std::move (pair.first);
> +      size_t size = buf.size ();
> +
> +      /* Write each line separately.  */
> +      for (size_t prev = 0, cur = 0; cur < size; ++cur)
> +	if (buf.at (cur) == '\n' || cur == size - 1)
> +	  {
> +	    std::string sub = buf.substr (prev, cur - prev + 1);
> +	    m_stream->puts (sub.c_str ());
> +	    prev = cur + 1;
> +	  }
> +
> +      if (pair.second >= 0)
> +	m_stream->wrap_here (pair.second);
> +    }
> +
> +  m_string_wraps.clear ();
> +}
> +
> +/* See ui-file.h.  */
> +
> +ui_file *
> +buffer_file::get_unbuffered_stream (ui_file *stream)
> +{
> +  buffer_file *buf = dynamic_cast<buffer_file *> (stream);
> +
> +  if (buf == nullptr)
> +    return stream;
> +
> +  return get_unbuffered_stream (buf->m_stream);
> +}
> +
> +/* See ui-file.h.  */
> +
> +bool
> +buffer_file::isatty ()
> +{
> +  return m_stream->isatty ();
> +}
> +
> +\f
> +
>  stdio_file::stdio_file (FILE *file, bool close_p)
>  {
>    set_stream (file);
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index de24620e247..a19eb7a9bfe 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -220,13 +220,56 @@ class string_file : public ui_file
>    bool empty () const { return m_string.empty (); }
>    void clear () { return m_string.clear (); }
>  
> -private:
> +protected:
>    /* The internal buffer.  */
>    std::string m_string;
>  
>    bool m_term_out;
>  };
>  
> +/* A string_file implementation that collects output on behalf of a
> +   given ui_file.  Provides access to the underlying ui_file so
> +   that buffering can be selectively bypassed.  */
> +
> +class buffer_file : public string_file
> +{
> +public:
> +  explicit buffer_file (bool term_out, ui_file *stream)
> +    : string_file (term_out), m_stream (stream)
> +  {}
> +
> +  /* Return the stream associated with this buffer_file.  */
> +  ui_file *get_stream ()
> +  {
> +    return m_stream;
> +  }
> +
> +  /* Record the wrap hint.  When flushing the buffer, the underlying
> +     ui_file's wrap_here will be called at the current point in the output.  */
> +  void wrap_here (int indent) override;
> +
> +  /* Flush collected output to the underlying ui_file.  */
> +  void flush_to_stream ();
> +
> +  /* Return true if the underlying stream is a tty.  */
> +  bool isatty () override;
> +
> +  /* Return a pointer to STREAM's underlying ui_file.  Recursively called until
> +     a non-buffer_file is found.  */
> +  static ui_file *get_unbuffered_stream (ui_file *stream);
> +
> +private:
> +
> +  /* The underlying output stream.  */
> +  ui_file *m_stream;
> +
> +  typedef std::pair<std::string, int> string_wrap_pair;
> +
> +  /* A collection of strings paired with an int representing the argument
> +     to a wrap_here call.  */
> +  std::vector<string_wrap_pair> m_string_wraps;
> +};
> +
>  /* A ui_file implementation that maps directly onto <stdio.h>'s FILE.
>     A stdio_file can either own its underlying file, or not.  If it
>     owns the file, then destroying the stdio_file closes the underlying
> diff --git a/gdb/ui-out.c b/gdb/ui-out.c
> index 9380630c320..3749317a25b 100644
> --- a/gdb/ui-out.c
> +++ b/gdb/ui-out.c
> @@ -799,6 +799,12 @@ ui_out::redirect (ui_file *outstream)
>    do_redirect (outstream);
>  }
>  
> +void
> +ui_out::redirect_to_buffer (buffer_file *buf_file)
> +{
> +  do_redirect_to_buffer (buf_file);
> +}
> +
>  /* Test the flags against the mask given.  */
>  ui_out_flags
>  ui_out::test_flags (ui_out_flags mask)
> @@ -871,3 +877,18 @@ ui_out::ui_out (ui_out_flags flags)
>  ui_out::~ui_out ()
>  {
>  }
> +
> +ui_out_buffer_pop::ui_out_buffer_pop (ui_out *uiout)
> +: m_uiout (uiout),
> +  m_buf_file (uiout->can_emit_style_escape (), gdb_stdout),
> +  m_prev_gdb_stdout (gdb_stdout)
> +{
> +  m_uiout->redirect_to_buffer (&m_buf_file);
> +  gdb_stdout = &m_buf_file;
> +}
> +
> +ui_out_buffer_pop::~ui_out_buffer_pop ()
> +{
> +  m_uiout->redirect_to_buffer (nullptr);
> +  gdb_stdout = m_prev_gdb_stdout;
> +}
> diff --git a/gdb/ui-out.h b/gdb/ui-out.h
> index ba5b1de68ab..ceee7be6938 100644
> --- a/gdb/ui-out.h
> +++ b/gdb/ui-out.h
> @@ -262,6 +262,9 @@ class ui_out
>    /* Redirect the output of a ui_out object temporarily.  */
>    void redirect (ui_file *outstream);
>  
> +  /* Redirect the output of a ui_out object to a buffer_file temporarily.  */
> +  void redirect_to_buffer (buffer_file *buf_file);
> +
>    ui_out_flags test_flags (ui_out_flags mask);
>  
>    /* HACK: Code in GDB is currently checking to see the type of ui_out
> @@ -360,6 +363,7 @@ class ui_out
>    virtual void do_wrap_hint (int indent) = 0;
>    virtual void do_flush () = 0;
>    virtual void do_redirect (struct ui_file *outstream) = 0;
> +  virtual void do_redirect_to_buffer (buffer_file *buf_file) = 0;
>  
>    virtual void do_progress_start () = 0;
>    virtual void do_progress_notify (const std::string &, const char *,
> @@ -470,4 +474,66 @@ class ui_out_redirect_pop
>    struct ui_out *m_uiout;
>  };
>  
> +/* On construction, redirect a uiout and gdb_stdout to a buffer_file.
> +   On destruction restore uiout and gdb_stdout.  */
> +
> +class ui_out_buffer_pop
> +{
> +public:
> +  ui_out_buffer_pop (ui_out *uiout);
> +
> +  ~ui_out_buffer_pop ();
> +
> +  /* Flush buffered output to the underlying output stream.  */
> +  void flush ()
> +  {
> +    m_buf_file.flush_to_stream ();
> +  }
> +
> +  ui_out_buffer_pop (const ui_out_buffer_pop &) = delete;
> +  ui_out_buffer_pop &operator= (const ui_out_buffer_pop &) = delete;
> +
> +private:
> +  /* ui_out being temporarily redirected.  */
> +  struct ui_out *m_uiout;
> +
> +  /* Buffer to which output is temporarily redirected to for the lifetime
> +     of this object.  */
> +  buffer_file m_buf_file;
> +
> +  /* Original gdb_stdout at the time of this object's construction.  */
> +  ui_file *m_prev_gdb_stdout;
> +};
> +
> +/* Redirect output to a buffer_file for the duration of FUNC.  */
> +
> +template<typename F, typename... Arg>
> +void
> +do_with_buffered_output (F func, ui_out *uiout, Arg... args)
> +{
> +  ui_out_buffer_pop buf (uiout);
> +
> +  try
> +    {
> +      func (uiout, std::forward<Arg> (args)...);
> +    }
> +  catch (gdb_exception &ex)
> +    {
> +      /* Ideally flush would be called in the destructor of buf,
> +	 however flushing might cause an exception to be thrown.
> +	 Catch it and ensure the first exception propagates.  */
> +      try
> +	{
> +	  buf.flush ();
> +	}
> +      catch (const gdb_exception &ignore)
> +	{
> +	}
> +
> +      throw_exception (std::move (ex));
> +    }
> +
> +  /* Try was successful.  Let any further exceptions propagate.  */
> +  buf.flush ();
> +}
>  #endif /* UI_OUT_H */
> -- 
> 2.40.1