[4/7,v2] gdb: Buffer output during print_thread and print_frame_info

  v1 can be found here:
https://sourceware.org/pipermail/gdb-patches/2023-February/197457.html

v2 addresses Tom's feedback:
https://sourceware.org/pipermail/gdb-patches/2023-March/197716.html

To ensure that debuginfod progress messages appear correctly during the
printing of frame and thread info, v2 removes newline tracking and
instead uses buffered output streams described below.

---

Introduce new ui_file buffer_file to temporarily collect output
during print_thread and print_frame_info.  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 frame and thread
printing.  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 the frame/thread output in a buffer_file
and have progress messages skip the buffer and print to gdb_stdout
immediately.  This ensures progress messages print first, followed by
uninterrupted frame/thread 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            |  22 ++++-
 gdb/cli-out.h            |   1 +
 gdb/debuginfod-support.c |  15 ++--
 gdb/mi/mi-out.c          |  22 +++++
 gdb/mi/mi-out.h          |   1 +
 gdb/stack.c              |  38 ++++++---
 gdb/thread.c             | 174 +++++++++++++++++++++++----------------
 gdb/ui-file.c            |  77 +++++++++++++++++
 gdb/ui-file.h            |  42 +++++++++-
 gdb/ui-out.c             |  20 +++++
 gdb/ui-out.h             |  66 +++++++++++++++
 11 files changed, 387 insertions(+), 91 deletions(-)
  

Ping

Thanks,
Aaron

On Mon, Apr 17, 2023 at 2:06 PM Aaron Merey <amerey@redhat.com> wrote:
>
> v1 can be found here:
> https://sourceware.org/pipermail/gdb-patches/2023-February/197457.html
>
> v2 addresses Tom's feedback:
> https://sourceware.org/pipermail/gdb-patches/2023-March/197716.html
>
> To ensure that debuginfod progress messages appear correctly during the
> printing of frame and thread info, v2 removes newline tracking and
> instead uses buffered output streams described below.
>
> ---
>
> Introduce new ui_file buffer_file to temporarily collect output
> during print_thread and print_frame_info.  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 frame and thread
> printing.  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 the frame/thread output in a buffer_file
> and have progress messages skip the buffer and print to gdb_stdout
> immediately.  This ensures progress messages print first, followed by
> uninterrupted frame/thread 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            |  22 ++++-
>  gdb/cli-out.h            |   1 +
>  gdb/debuginfod-support.c |  15 ++--
>  gdb/mi/mi-out.c          |  22 +++++
>  gdb/mi/mi-out.h          |   1 +
>  gdb/stack.c              |  38 ++++++---
>  gdb/thread.c             | 174 +++++++++++++++++++++++----------------
>  gdb/ui-file.c            |  77 +++++++++++++++++
>  gdb/ui-file.h            |  42 +++++++++-
>  gdb/ui-out.c             |  20 +++++
>  gdb/ui-out.h             |  66 +++++++++++++++
>  11 files changed, 387 insertions(+), 91 deletions(-)
>
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index fdfd0f7f0cf..8162c1474ee 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -263,6 +263,19 @@ cli_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>
> +void
> +cli_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> +{
> +  if (buf_file != nullptr)
> +    {
> +      gdb_assert (m_streams.size () >= 1);
> +      buf_file->set_stream (m_streams.back ());
> +      do_redirect (buf_file);
> +    }
> +  else
> +    m_streams.pop_back ();
> +}
> +
>  /* Initialize a progress update to be displayed with
>     cli_ui_out::do_progress_notify.  */
>
> @@ -299,7 +312,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 +398,8 @@ cli_ui_out::do_progress_notify (const std::string &msg,
>  void
>  cli_ui_out::clear_current_line ()
>  {
> -  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 ();
>
>    if (!stream->isatty ()
> @@ -410,10 +425,11 @@ void
>  cli_ui_out::do_progress_end ()
>  {
>    struct ui_file *stream = m_streams.back ();
> -  m_progress_info.pop_back ();
>
>    if (stream->isatty ())
>      clear_current_line ();
> +
> +  m_progress_info.pop_back ();
>  }
>
>  /* local functions */
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index 0729834cbc6..2bccaa8e9e8 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 fb96dbaced1..22fa3ff2457 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -159,7 +159,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;
>      }
> @@ -275,10 +276,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/mi/mi-out.c b/gdb/mi/mi-out.c
> index 29a416a426d..8c4a920ac13 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -194,6 +194,28 @@ mi_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>
> +void
> +mi_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> +{
> +  if (buf_file != nullptr)
> +    {
> +      gdb_assert (m_streams.size () >= 1);
> +      ui_file *stream = m_streams.back ();
> +
> +      string_file *sstream = dynamic_cast<string_file *>(stream);
> +      if (sstream != nullptr)
> +       {
> +         buf_file->write (sstream->data (), sstream->size ());
> +         sstream->clear ();
> +       }
> +
> +      buf_file->set_stream (stream);
> +      m_streams.push_back (buf_file);
> +    }
> +  else
> +    m_streams.pop_back ();
> +}
> +
>  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/stack.c b/gdb/stack.c
> index 03e903d901b..cf82ef88ea5 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);
> @@ -1026,16 +1027,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)
> @@ -1111,7 +1111,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);
>
> @@ -1191,6 +1192,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
> @@ -1315,13 +1333,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 4d97ed3f2d1..f224e003690 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1012,6 +1012,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.  */
> @@ -1095,82 +1195,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..af02f7defc0 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -234,6 +234,83 @@ string_file::can_emit_style_escape ()
>
>
>
> +/* 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 ()
> +{
> +  if (m_stream == nullptr)
> +    return;
> +
> +  /* 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 || buf->m_stream == nullptr)
> +    return stream;
> +
> +  return get_unbuffered_stream (buf->m_stream);
> +}
> +
> +/* See ui-file.h.  */
> +
> +void
> +buffer_file::set_stream (ui_file *stream)
> +{
> +  if (m_stream == nullptr)
> +    m_stream = stream;
> +}
> +
> +/* See ui-file.h.  */
> +
> +bool
> +buffer_file::isatty ()
> +{
> +  if (m_stream != nullptr)
> +    return m_stream->isatty ();
> +
> +  return false;
> +}
> +
> +
> +
>  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..49967b3cfc6 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -220,13 +220,53 @@ 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)
> +    : string_file (term_out), m_stream (nullptr)
> +  {}
> +
> +  /* Associate STREAM with this buffer_file.  */
> +  void set_stream (ui_file *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..3a54ea401b7 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,17 @@ 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 ()),
> +  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..1e24a4db2b0 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 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.39.2
>
  

Ping,

Thanks,
Aaron

On Tue, May 2, 2023 at 10:25 AM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Mon, Apr 17, 2023 at 2:06 PM Aaron Merey <amerey@redhat.com> wrote:
> >
> > v1 can be found here:
> > https://sourceware.org/pipermail/gdb-patches/2023-February/197457.html
> >
> > v2 addresses Tom's feedback:
> > https://sourceware.org/pipermail/gdb-patches/2023-March/197716.html
> >
> > To ensure that debuginfod progress messages appear correctly during the
> > printing of frame and thread info, v2 removes newline tracking and
> > instead uses buffered output streams described below.
> >
> > ---
> >
> > Introduce new ui_file buffer_file to temporarily collect output
> > during print_thread and print_frame_info.  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 frame and thread
> > printing.  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 the frame/thread output in a buffer_file
> > and have progress messages skip the buffer and print to gdb_stdout
> > immediately.  This ensures progress messages print first, followed by
> > uninterrupted frame/thread 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            |  22 ++++-
> >  gdb/cli-out.h            |   1 +
> >  gdb/debuginfod-support.c |  15 ++--
> >  gdb/mi/mi-out.c          |  22 +++++
> >  gdb/mi/mi-out.h          |   1 +
> >  gdb/stack.c              |  38 ++++++---
> >  gdb/thread.c             | 174 +++++++++++++++++++++++----------------
> >  gdb/ui-file.c            |  77 +++++++++++++++++
> >  gdb/ui-file.h            |  42 +++++++++-
> >  gdb/ui-out.c             |  20 +++++
> >  gdb/ui-out.h             |  66 +++++++++++++++
> >  11 files changed, 387 insertions(+), 91 deletions(-)
> >
> > diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> > index fdfd0f7f0cf..8162c1474ee 100644
> > --- a/gdb/cli-out.c
> > +++ b/gdb/cli-out.c
> > @@ -263,6 +263,19 @@ cli_ui_out::do_redirect (ui_file *outstream)
> >      m_streams.pop_back ();
> >  }
> >
> > +void
> > +cli_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> > +{
> > +  if (buf_file != nullptr)
> > +    {
> > +      gdb_assert (m_streams.size () >= 1);
> > +      buf_file->set_stream (m_streams.back ());
> > +      do_redirect (buf_file);
> > +    }
> > +  else
> > +    m_streams.pop_back ();
> > +}
> > +
> >  /* Initialize a progress update to be displayed with
> >     cli_ui_out::do_progress_notify.  */
> >
> > @@ -299,7 +312,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 +398,8 @@ cli_ui_out::do_progress_notify (const std::string &msg,
> >  void
> >  cli_ui_out::clear_current_line ()
> >  {
> > -  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 ();
> >
> >    if (!stream->isatty ()
> > @@ -410,10 +425,11 @@ void
> >  cli_ui_out::do_progress_end ()
> >  {
> >    struct ui_file *stream = m_streams.back ();
> > -  m_progress_info.pop_back ();
> >
> >    if (stream->isatty ())
> >      clear_current_line ();
> > +
> > +  m_progress_info.pop_back ();
> >  }
> >
> >  /* local functions */
> > diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> > index 0729834cbc6..2bccaa8e9e8 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 fb96dbaced1..22fa3ff2457 100644
> > --- a/gdb/debuginfod-support.c
> > +++ b/gdb/debuginfod-support.c
> > @@ -159,7 +159,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;
> >      }
> > @@ -275,10 +276,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/mi/mi-out.c b/gdb/mi/mi-out.c
> > index 29a416a426d..8c4a920ac13 100644
> > --- a/gdb/mi/mi-out.c
> > +++ b/gdb/mi/mi-out.c
> > @@ -194,6 +194,28 @@ mi_ui_out::do_redirect (ui_file *outstream)
> >      m_streams.pop_back ();
> >  }
> >
> > +void
> > +mi_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> > +{
> > +  if (buf_file != nullptr)
> > +    {
> > +      gdb_assert (m_streams.size () >= 1);
> > +      ui_file *stream = m_streams.back ();
> > +
> > +      string_file *sstream = dynamic_cast<string_file *>(stream);
> > +      if (sstream != nullptr)
> > +       {
> > +         buf_file->write (sstream->data (), sstream->size ());
> > +         sstream->clear ();
> > +       }
> > +
> > +      buf_file->set_stream (stream);
> > +      m_streams.push_back (buf_file);
> > +    }
> > +  else
> > +    m_streams.pop_back ();
> > +}
> > +
> >  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/stack.c b/gdb/stack.c
> > index 03e903d901b..cf82ef88ea5 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);
> > @@ -1026,16 +1027,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)
> > @@ -1111,7 +1111,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);
> >
> > @@ -1191,6 +1192,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
> > @@ -1315,13 +1333,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 4d97ed3f2d1..f224e003690 100644
> > --- a/gdb/thread.c
> > +++ b/gdb/thread.c
> > @@ -1012,6 +1012,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.  */
> > @@ -1095,82 +1195,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..af02f7defc0 100644
> > --- a/gdb/ui-file.c
> > +++ b/gdb/ui-file.c
> > @@ -234,6 +234,83 @@ string_file::can_emit_style_escape ()
> >
> >
> >
> > +/* 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 ()
> > +{
> > +  if (m_stream == nullptr)
> > +    return;
> > +
> > +  /* 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 || buf->m_stream == nullptr)
> > +    return stream;
> > +
> > +  return get_unbuffered_stream (buf->m_stream);
> > +}
> > +
> > +/* See ui-file.h.  */
> > +
> > +void
> > +buffer_file::set_stream (ui_file *stream)
> > +{
> > +  if (m_stream == nullptr)
> > +    m_stream = stream;
> > +}
> > +
> > +/* See ui-file.h.  */
> > +
> > +bool
> > +buffer_file::isatty ()
> > +{
> > +  if (m_stream != nullptr)
> > +    return m_stream->isatty ();
> > +
> > +  return false;
> > +}
> > +
> > +
> > +
> >  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..49967b3cfc6 100644
> > --- a/gdb/ui-file.h
> > +++ b/gdb/ui-file.h
> > @@ -220,13 +220,53 @@ 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)
> > +    : string_file (term_out), m_stream (nullptr)
> > +  {}
> > +
> > +  /* Associate STREAM with this buffer_file.  */
> > +  void set_stream (ui_file *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..3a54ea401b7 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,17 @@ 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 ()),
> > +  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..1e24a4db2b0 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 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.39.2
> >
  

Ping

Thanks,
Aaron

On Tue, May 9, 2023 at 9:47 AM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping,
>
> Thanks,
> Aaron
>
> On Tue, May 2, 2023 at 10:25 AM Aaron Merey <amerey@redhat.com> wrote:
> >
> > Ping
> >
> > Thanks,
> > Aaron
> >
> > On Mon, Apr 17, 2023 at 2:06 PM Aaron Merey <amerey@redhat.com> wrote:
> > >
> > > v1 can be found here:
> > > https://sourceware.org/pipermail/gdb-patches/2023-February/197457.html
> > >
> > > v2 addresses Tom's feedback:
> > > https://sourceware.org/pipermail/gdb-patches/2023-March/197716.html
> > >
> > > To ensure that debuginfod progress messages appear correctly during the
> > > printing of frame and thread info, v2 removes newline tracking and
> > > instead uses buffered output streams described below.
> > >
> > > ---
> > >
> > > Introduce new ui_file buffer_file to temporarily collect output
> > > during print_thread and print_frame_info.  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 frame and thread
> > > printing.  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 the frame/thread output in a buffer_file
> > > and have progress messages skip the buffer and print to gdb_stdout
> > > immediately.  This ensures progress messages print first, followed by
> > > uninterrupted frame/thread 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            |  22 ++++-
> > >  gdb/cli-out.h            |   1 +
> > >  gdb/debuginfod-support.c |  15 ++--
> > >  gdb/mi/mi-out.c          |  22 +++++
> > >  gdb/mi/mi-out.h          |   1 +
> > >  gdb/stack.c              |  38 ++++++---
> > >  gdb/thread.c             | 174 +++++++++++++++++++++++----------------
> > >  gdb/ui-file.c            |  77 +++++++++++++++++
> > >  gdb/ui-file.h            |  42 +++++++++-
> > >  gdb/ui-out.c             |  20 +++++
> > >  gdb/ui-out.h             |  66 +++++++++++++++
> > >  11 files changed, 387 insertions(+), 91 deletions(-)
> > >
> > > diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> > > index fdfd0f7f0cf..8162c1474ee 100644
> > > --- a/gdb/cli-out.c
> > > +++ b/gdb/cli-out.c
> > > @@ -263,6 +263,19 @@ cli_ui_out::do_redirect (ui_file *outstream)
> > >      m_streams.pop_back ();
> > >  }
> > >
> > > +void
> > > +cli_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> > > +{
> > > +  if (buf_file != nullptr)
> > > +    {
> > > +      gdb_assert (m_streams.size () >= 1);
> > > +      buf_file->set_stream (m_streams.back ());
> > > +      do_redirect (buf_file);
> > > +    }
> > > +  else
> > > +    m_streams.pop_back ();
> > > +}
> > > +
> > >  /* Initialize a progress update to be displayed with
> > >     cli_ui_out::do_progress_notify.  */
> > >
> > > @@ -299,7 +312,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 +398,8 @@ cli_ui_out::do_progress_notify (const std::string &msg,
> > >  void
> > >  cli_ui_out::clear_current_line ()
> > >  {
> > > -  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 ();
> > >
> > >    if (!stream->isatty ()
> > > @@ -410,10 +425,11 @@ void
> > >  cli_ui_out::do_progress_end ()
> > >  {
> > >    struct ui_file *stream = m_streams.back ();
> > > -  m_progress_info.pop_back ();
> > >
> > >    if (stream->isatty ())
> > >      clear_current_line ();
> > > +
> > > +  m_progress_info.pop_back ();
> > >  }
> > >
> > >  /* local functions */
> > > diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> > > index 0729834cbc6..2bccaa8e9e8 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 fb96dbaced1..22fa3ff2457 100644
> > > --- a/gdb/debuginfod-support.c
> > > +++ b/gdb/debuginfod-support.c
> > > @@ -159,7 +159,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;
> > >      }
> > > @@ -275,10 +276,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/mi/mi-out.c b/gdb/mi/mi-out.c
> > > index 29a416a426d..8c4a920ac13 100644
> > > --- a/gdb/mi/mi-out.c
> > > +++ b/gdb/mi/mi-out.c
> > > @@ -194,6 +194,28 @@ mi_ui_out::do_redirect (ui_file *outstream)
> > >      m_streams.pop_back ();
> > >  }
> > >
> > > +void
> > > +mi_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> > > +{
> > > +  if (buf_file != nullptr)
> > > +    {
> > > +      gdb_assert (m_streams.size () >= 1);
> > > +      ui_file *stream = m_streams.back ();
> > > +
> > > +      string_file *sstream = dynamic_cast<string_file *>(stream);
> > > +      if (sstream != nullptr)
> > > +       {
> > > +         buf_file->write (sstream->data (), sstream->size ());
> > > +         sstream->clear ();
> > > +       }
> > > +
> > > +      buf_file->set_stream (stream);
> > > +      m_streams.push_back (buf_file);
> > > +    }
> > > +  else
> > > +    m_streams.pop_back ();
> > > +}
> > > +
> > >  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/stack.c b/gdb/stack.c
> > > index 03e903d901b..cf82ef88ea5 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);
> > > @@ -1026,16 +1027,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)
> > > @@ -1111,7 +1111,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);
> > >
> > > @@ -1191,6 +1192,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
> > > @@ -1315,13 +1333,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 4d97ed3f2d1..f224e003690 100644
> > > --- a/gdb/thread.c
> > > +++ b/gdb/thread.c
> > > @@ -1012,6 +1012,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.  */
> > > @@ -1095,82 +1195,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..af02f7defc0 100644
> > > --- a/gdb/ui-file.c
> > > +++ b/gdb/ui-file.c
> > > @@ -234,6 +234,83 @@ string_file::can_emit_style_escape ()
> > >
> > >
> > >
> > > +/* 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 ()
> > > +{
> > > +  if (m_stream == nullptr)
> > > +    return;
> > > +
> > > +  /* 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 || buf->m_stream == nullptr)
> > > +    return stream;
> > > +
> > > +  return get_unbuffered_stream (buf->m_stream);
> > > +}
> > > +
> > > +/* See ui-file.h.  */
> > > +
> > > +void
> > > +buffer_file::set_stream (ui_file *stream)
> > > +{
> > > +  if (m_stream == nullptr)
> > > +    m_stream = stream;
> > > +}
> > > +
> > > +/* See ui-file.h.  */
> > > +
> > > +bool
> > > +buffer_file::isatty ()
> > > +{
> > > +  if (m_stream != nullptr)
> > > +    return m_stream->isatty ();
> > > +
> > > +  return false;
> > > +}
> > > +
> > > +
> > > +
> > >  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..49967b3cfc6 100644
> > > --- a/gdb/ui-file.h
> > > +++ b/gdb/ui-file.h
> > > @@ -220,13 +220,53 @@ 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)
> > > +    : string_file (term_out), m_stream (nullptr)
> > > +  {}
> > > +
> > > +  /* Associate STREAM with this buffer_file.  */
> > > +  void set_stream (ui_file *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..3a54ea401b7 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,17 @@ 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 ()),
> > > +  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..1e24a4db2b0 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 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.39.2
> > >
  

Ping

Thanks,
Aaron

On Tue, May 16, 2023 at 10:49 AM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Tue, May 9, 2023 at 9:47 AM Aaron Merey <amerey@redhat.com> wrote:
> >
> > Ping,
> >
> > Thanks,
> > Aaron
> >
> > On Tue, May 2, 2023 at 10:25 AM Aaron Merey <amerey@redhat.com> wrote:
> > >
> > > Ping
> > >
> > > Thanks,
> > > Aaron
> > >
> > > On Mon, Apr 17, 2023 at 2:06 PM Aaron Merey <amerey@redhat.com> wrote:
> > > >
> > > > v1 can be found here:
> > > > https://sourceware.org/pipermail/gdb-patches/2023-February/197457.html
> > > >
> > > > v2 addresses Tom's feedback:
> > > > https://sourceware.org/pipermail/gdb-patches/2023-March/197716.html
> > > >
> > > > To ensure that debuginfod progress messages appear correctly during the
> > > > printing of frame and thread info, v2 removes newline tracking and
> > > > instead uses buffered output streams described below.
> > > >
> > > > ---
> > > >
> > > > Introduce new ui_file buffer_file to temporarily collect output
> > > > during print_thread and print_frame_info.  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 frame and thread
> > > > printing.  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 the frame/thread output in a buffer_file
> > > > and have progress messages skip the buffer and print to gdb_stdout
> > > > immediately.  This ensures progress messages print first, followed by
> > > > uninterrupted frame/thread 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            |  22 ++++-
> > > >  gdb/cli-out.h            |   1 +
> > > >  gdb/debuginfod-support.c |  15 ++--
> > > >  gdb/mi/mi-out.c          |  22 +++++
> > > >  gdb/mi/mi-out.h          |   1 +
> > > >  gdb/stack.c              |  38 ++++++---
> > > >  gdb/thread.c             | 174 +++++++++++++++++++++++----------------
> > > >  gdb/ui-file.c            |  77 +++++++++++++++++
> > > >  gdb/ui-file.h            |  42 +++++++++-
> > > >  gdb/ui-out.c             |  20 +++++
> > > >  gdb/ui-out.h             |  66 +++++++++++++++
> > > >  11 files changed, 387 insertions(+), 91 deletions(-)
> > > >
> > > > diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> > > > index fdfd0f7f0cf..8162c1474ee 100644
> > > > --- a/gdb/cli-out.c
> > > > +++ b/gdb/cli-out.c
> > > > @@ -263,6 +263,19 @@ cli_ui_out::do_redirect (ui_file *outstream)
> > > >      m_streams.pop_back ();
> > > >  }
> > > >
> > > > +void
> > > > +cli_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> > > > +{
> > > > +  if (buf_file != nullptr)
> > > > +    {
> > > > +      gdb_assert (m_streams.size () >= 1);
> > > > +      buf_file->set_stream (m_streams.back ());
> > > > +      do_redirect (buf_file);
> > > > +    }
> > > > +  else
> > > > +    m_streams.pop_back ();
> > > > +}
> > > > +
> > > >  /* Initialize a progress update to be displayed with
> > > >     cli_ui_out::do_progress_notify.  */
> > > >
> > > > @@ -299,7 +312,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 +398,8 @@ cli_ui_out::do_progress_notify (const std::string &msg,
> > > >  void
> > > >  cli_ui_out::clear_current_line ()
> > > >  {
> > > > -  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 ();
> > > >
> > > >    if (!stream->isatty ()
> > > > @@ -410,10 +425,11 @@ void
> > > >  cli_ui_out::do_progress_end ()
> > > >  {
> > > >    struct ui_file *stream = m_streams.back ();
> > > > -  m_progress_info.pop_back ();
> > > >
> > > >    if (stream->isatty ())
> > > >      clear_current_line ();
> > > > +
> > > > +  m_progress_info.pop_back ();
> > > >  }
> > > >
> > > >  /* local functions */
> > > > diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> > > > index 0729834cbc6..2bccaa8e9e8 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 fb96dbaced1..22fa3ff2457 100644
> > > > --- a/gdb/debuginfod-support.c
> > > > +++ b/gdb/debuginfod-support.c
> > > > @@ -159,7 +159,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;
> > > >      }
> > > > @@ -275,10 +276,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/mi/mi-out.c b/gdb/mi/mi-out.c
> > > > index 29a416a426d..8c4a920ac13 100644
> > > > --- a/gdb/mi/mi-out.c
> > > > +++ b/gdb/mi/mi-out.c
> > > > @@ -194,6 +194,28 @@ mi_ui_out::do_redirect (ui_file *outstream)
> > > >      m_streams.pop_back ();
> > > >  }
> > > >
> > > > +void
> > > > +mi_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> > > > +{
> > > > +  if (buf_file != nullptr)
> > > > +    {
> > > > +      gdb_assert (m_streams.size () >= 1);
> > > > +      ui_file *stream = m_streams.back ();
> > > > +
> > > > +      string_file *sstream = dynamic_cast<string_file *>(stream);
> > > > +      if (sstream != nullptr)
> > > > +       {
> > > > +         buf_file->write (sstream->data (), sstream->size ());
> > > > +         sstream->clear ();
> > > > +       }
> > > > +
> > > > +      buf_file->set_stream (stream);
> > > > +      m_streams.push_back (buf_file);
> > > > +    }
> > > > +  else
> > > > +    m_streams.pop_back ();
> > > > +}
> > > > +
> > > >  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/stack.c b/gdb/stack.c
> > > > index 03e903d901b..cf82ef88ea5 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);
> > > > @@ -1026,16 +1027,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)
> > > > @@ -1111,7 +1111,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);
> > > >
> > > > @@ -1191,6 +1192,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
> > > > @@ -1315,13 +1333,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 4d97ed3f2d1..f224e003690 100644
> > > > --- a/gdb/thread.c
> > > > +++ b/gdb/thread.c
> > > > @@ -1012,6 +1012,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.  */
> > > > @@ -1095,82 +1195,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..af02f7defc0 100644
> > > > --- a/gdb/ui-file.c
> > > > +++ b/gdb/ui-file.c
> > > > @@ -234,6 +234,83 @@ string_file::can_emit_style_escape ()
> > > >
> > > >
> > > >
> > > > +/* 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 ()
> > > > +{
> > > > +  if (m_stream == nullptr)
> > > > +    return;
> > > > +
> > > > +  /* 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 || buf->m_stream == nullptr)
> > > > +    return stream;
> > > > +
> > > > +  return get_unbuffered_stream (buf->m_stream);
> > > > +}
> > > > +
> > > > +/* See ui-file.h.  */
> > > > +
> > > > +void
> > > > +buffer_file::set_stream (ui_file *stream)
> > > > +{
> > > > +  if (m_stream == nullptr)
> > > > +    m_stream = stream;
> > > > +}
> > > > +
> > > > +/* See ui-file.h.  */
> > > > +
> > > > +bool
> > > > +buffer_file::isatty ()
> > > > +{
> > > > +  if (m_stream != nullptr)
> > > > +    return m_stream->isatty ();
> > > > +
> > > > +  return false;
> > > > +}
> > > > +
> > > > +
> > > > +
> > > >  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..49967b3cfc6 100644
> > > > --- a/gdb/ui-file.h
> > > > +++ b/gdb/ui-file.h
> > > > @@ -220,13 +220,53 @@ 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)
> > > > +    : string_file (term_out), m_stream (nullptr)
> > > > +  {}
> > > > +
> > > > +  /* Associate STREAM with this buffer_file.  */
> > > > +  void set_stream (ui_file *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..3a54ea401b7 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,17 @@ 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 ()),
> > > > +  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..1e24a4db2b0 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 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.39.2
> > > >
  

Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

> v1 can be found here:
> https://sourceware.org/pipermail/gdb-patches/2023-February/197457.html
>
> v2 addresses Tom's feedback:
> https://sourceware.org/pipermail/gdb-patches/2023-March/197716.html
>
> To ensure that debuginfod progress messages appear correctly during the
> printing of frame and thread info, v2 removes newline tracking and
> instead uses buffered output streams described below.
>
> ---
>
> Introduce new ui_file buffer_file to temporarily collect output
> during print_thread and print_frame_info.  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 frame and thread
> printing.  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 the frame/thread output in a buffer_file
> and have progress messages skip the buffer and print to gdb_stdout
> immediately.  This ensures progress messages print first, followed by
> uninterrupted frame/thread 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            |  22 ++++-
>  gdb/cli-out.h            |   1 +
>  gdb/debuginfod-support.c |  15 ++--
>  gdb/mi/mi-out.c          |  22 +++++
>  gdb/mi/mi-out.h          |   1 +
>  gdb/stack.c              |  38 ++++++---
>  gdb/thread.c             | 174 +++++++++++++++++++++++----------------
>  gdb/ui-file.c            |  77 +++++++++++++++++
>  gdb/ui-file.h            |  42 +++++++++-
>  gdb/ui-out.c             |  20 +++++
>  gdb/ui-out.h             |  66 +++++++++++++++
>  11 files changed, 387 insertions(+), 91 deletions(-)

I guess I would have expected some new tests in this commit.  Is there
any reason why we couldn't write a test here?

Thanks,
Andrew



>
> diff --git a/gdb/cli-out.c b/gdb/cli-out.c
> index fdfd0f7f0cf..8162c1474ee 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -263,6 +263,19 @@ cli_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>  
> +void
> +cli_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> +{
> +  if (buf_file != nullptr)
> +    {
> +      gdb_assert (m_streams.size () >= 1);
> +      buf_file->set_stream (m_streams.back ());
> +      do_redirect (buf_file);
> +    }
> +  else
> +    m_streams.pop_back ();
> +}
> +
>  /* Initialize a progress update to be displayed with
>     cli_ui_out::do_progress_notify.  */
>  
> @@ -299,7 +312,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 +398,8 @@ cli_ui_out::do_progress_notify (const std::string &msg,
>  void
>  cli_ui_out::clear_current_line ()
>  {
> -  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 ();
>  
>    if (!stream->isatty ()
> @@ -410,10 +425,11 @@ void
>  cli_ui_out::do_progress_end ()
>  {
>    struct ui_file *stream = m_streams.back ();
> -  m_progress_info.pop_back ();
>  
>    if (stream->isatty ())
>      clear_current_line ();
> +
> +  m_progress_info.pop_back ();
>  }
>  
>  /* local functions */
> diff --git a/gdb/cli-out.h b/gdb/cli-out.h
> index 0729834cbc6..2bccaa8e9e8 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 fb96dbaced1..22fa3ff2457 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -159,7 +159,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;
>      }
> @@ -275,10 +276,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/mi/mi-out.c b/gdb/mi/mi-out.c
> index 29a416a426d..8c4a920ac13 100644
> --- a/gdb/mi/mi-out.c
> +++ b/gdb/mi/mi-out.c
> @@ -194,6 +194,28 @@ mi_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>  
> +void
> +mi_ui_out::do_redirect_to_buffer (buffer_file *buf_file)
> +{
> +  if (buf_file != nullptr)
> +    {
> +      gdb_assert (m_streams.size () >= 1);
> +      ui_file *stream = m_streams.back ();
> +
> +      string_file *sstream = dynamic_cast<string_file *>(stream);
> +      if (sstream != nullptr)
> +	{
> +	  buf_file->write (sstream->data (), sstream->size ());
> +	  sstream->clear ();
> +	}
> +
> +      buf_file->set_stream (stream);
> +      m_streams.push_back (buf_file);
> +    }
> +  else
> +    m_streams.pop_back ();
> +}
> +
>  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/stack.c b/gdb/stack.c
> index 03e903d901b..cf82ef88ea5 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);
> @@ -1026,16 +1027,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)
> @@ -1111,7 +1111,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);
>  
> @@ -1191,6 +1192,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
> @@ -1315,13 +1333,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 4d97ed3f2d1..f224e003690 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1012,6 +1012,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.  */
> @@ -1095,82 +1195,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..af02f7defc0 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -234,6 +234,83 @@ string_file::can_emit_style_escape ()
>  
>  
>  
> +/* 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 ()
> +{
> +  if (m_stream == nullptr)
> +    return;
> +
> +  /* 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 || buf->m_stream == nullptr)
> +    return stream;
> +
> +  return get_unbuffered_stream (buf->m_stream);
> +}
> +
> +/* See ui-file.h.  */
> +
> +void
> +buffer_file::set_stream (ui_file *stream)
> +{
> +  if (m_stream == nullptr)
> +    m_stream = stream;
> +}
> +
> +/* See ui-file.h.  */
> +
> +bool
> +buffer_file::isatty ()
> +{
> +  if (m_stream != nullptr)
> +    return m_stream->isatty ();
> +
> +  return false;
> +}
> +
> +
> +
>  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..49967b3cfc6 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -220,13 +220,53 @@ 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)
> +    : string_file (term_out), m_stream (nullptr)
> +  {}
> +
> +  /* Associate STREAM with this buffer_file.  */
> +  void set_stream (ui_file *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..3a54ea401b7 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,17 @@ 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 ()),
> +  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..1e24a4db2b0 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 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.39.2
  

On Wed, May 24, 2023 at 5:41 AM Andrew Burgess <aburgess@redhat.com> wrote:
>
> I guess I would have expected some new tests in this commit.  Is there
> any reason why we couldn't write a test here?

My rationale was that existing tests which check the correctness of frame and
thread printing should suffice. However we are missing a test checking that
debuginfod progress output does not occur within frame/thread info (like
the example in the commit message describes).

I will include a test for this to the patch that adds section downloading since
progress messages during frame/thread printing can only occur with section
downloads.

Aaron