public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Aaron Merey via Gdb-patches <gdb-patches@sourceware.org>,
	gdb-patches@sourceware.org
Cc: tom@tromey.com, Aaron Merey <amerey@redhat.com>
Subject: Re: [PATCH 4/7 v2] gdb: Buffer output during print_thread and print_frame_info
Date: Wed, 24 May 2023 10:41:46 +0100	[thread overview]
Message-ID: <87v8gi6p7p.fsf@redhat.com> (raw)
In-Reply-To: <20230417180556.1213862-1-amerey@redhat.com>

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 ()
>  
>  \f
>  
> +/* See ui-file.h.  */
> +
> +void
> +buffer_file::wrap_here (int indent)
> +{
> +  m_string_wraps.emplace (m_string_wraps.end (),
> +			  string_wrap_pair (m_string, indent));
> +  m_string.clear ();
> +}
> +
> +/* See ui-file.h.  */
> +
> +void
> +buffer_file::flush_to_stream ()
> +{
> +  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;
> +}
> +
> +\f
> +
>  stdio_file::stdio_file (FILE *file, bool close_p)
>  {
>    set_stream (file);
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index de24620e247..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


  parent reply	other threads:[~2023-05-24  9:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-17 18:05 Aaron Merey
2023-05-02 14:25 ` Aaron Merey
2023-05-09 13:47   ` [PING*2][PATCH " Aaron Merey
2023-05-16 14:49     ` [PING*3][PATCH " Aaron Merey
2023-05-23 20:56       ` [PING*4][PATCH " Aaron Merey
2023-05-24  9:41 ` Andrew Burgess [this message]
2023-05-24 19:05   ` [PATCH " Aaron Merey

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=87v8gi6p7p.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=amerey@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).