From: Aaron Merey <amerey@redhat.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: Re: [PING*4][PATCH 4/7 v2] gdb: Buffer output during print_thread and print_frame_info
Date: Tue, 23 May 2023 16:56:32 -0400 [thread overview]
Message-ID: <CAJDtP-QZmUxteTqR3RBoyoKsseRrV1qQzZCuWx0qHPZBt3raqQ@mail.gmail.com> (raw)
In-Reply-To: <CAJDtP-QpQ=pUOGRt4EufDN-G2TmEzUbe8VmXkcORL56x3P+EzQ@mail.gmail.com>
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
> > > >
next prev parent reply other threads:[~2023-05-23 20:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-17 18:05 [PATCH " 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 ` Aaron Merey [this message]
2023-05-24 9:41 ` [PATCH " Andrew Burgess
2023-05-24 19:05 ` 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=CAJDtP-QZmUxteTqR3RBoyoKsseRrV1qQzZCuWx0qHPZBt3raqQ@mail.gmail.com \
--to=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).