From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id lCkmHwoKqWUJiz8AWB0awg (envelope-from ) for ; Thu, 18 Jan 2024 06:22:50 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=G2O62fls; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 730C11E0C3; Thu, 18 Jan 2024 06:22:50 -0500 (EST) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 4E5571E092 for ; Thu, 18 Jan 2024 06:22:48 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E3A80385E007 for ; Thu, 18 Jan 2024 11:22:47 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C79FD3858286 for ; Thu, 18 Jan 2024 11:22:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C79FD3858286 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C79FD3858286 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705576943; cv=none; b=wCONyG6BqigfxF8sKRHM2kVitby2Ds+p9+3RPqTLX83R9wEmEUFU7AAw1rl8XH86seHGVYvlssJ9YH3omcd+sdOmW/QArjEccUJ+6h3aRDDnT8y9Dik3MG3lNJfRLXfXKApI9Gq8qXSzO14o5DFQlp9pRmnRNVNbzu/L5G5KOgc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705576943; c=relaxed/simple; bh=eWh6AcxanSRmNew7qtqekZDz0U82cBlvGo8sAz0FG2U=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=tgfVYYH8baALaPuJDfTGYYfLl/Z2kQ2b8rZkm2HVcjF6CEoK7TtQgQCkmtFzFgMTzMeQodzMLS9Haxwe0FmMkBfJiKaU/V78cUeFYr+esxRW3nISwcum9vTCf9rFXZmlEU4fh3J06juaaYN6cy1Mm+sQXNmVLXsyQ2UNg+O0ZRQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705576939; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=b2Io3SBVSQ9VbkstRGai2ea9m0KMENX9Ce7KsHfCc0o=; b=G2O62flsz296uzLK6u3GRW14h1C0FedUaxVqYgxG9l9LH3QxhKb/dBsHFeUSKRsY05S4vC NrRcKTqEOsUcp3K2x4EWFByV9ise9gBh3M0Mvwi7F+cjui+9RqzUOQadrO3dB/rGrZDVa+ bMvyZomBf/pTQ1eNiboc5G+pEO56c4Q= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-489-wKItg4OIN2GSJHKw3wrIpA-1; Thu, 18 Jan 2024 06:22:18 -0500 X-MC-Unique: wKItg4OIN2GSJHKw3wrIpA-1 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-337d7158aa7so19224f8f.2 for ; Thu, 18 Jan 2024 03:22:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705576937; x=1706181737; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=b2Io3SBVSQ9VbkstRGai2ea9m0KMENX9Ce7KsHfCc0o=; b=RE09d0azXc70eol8cX/lkBJTIEJZR/kK021klxcVkDdXNH3AuNwk1zmcu8ugFqgWrz GdftYd6yWhwB+rTLySU3AtP+mtUQMXElClUOveeNNY/Ks5f01+6rIQDadi8rprx1bZDP atBZyqnEt/guC46YlsIc5I0PwokfV5ohdKl4eij6AY85Eqk/knek/fK2xahnWdWf2YjD 7ZcQ7Gk2/3SSlOpH9umhmM1FFx67CcLW8vepWZEKQfvjSpFIohKp+3e2K6H4/VXZVfzn muEIHnsgPPcZBb6mRlTB+CkkkAT5hba/OvYvkwW2xRp21m80+VzhaJ7RA+PXDxO7TaOm Wkug== X-Gm-Message-State: AOJu0YwN/qqzKUcRdCS8m6Tra83X4tLdafM3b4qSGpCoHAeIvJzukZYH rWuwPJv4WOS0XJ5qKkimo2gbV+v0uk/sgzxbEJcIEsW3PhIy3HiT126Ex4tPlWsuzBliFZIrXwj rHbLOxIQJpN7JklimWmBxH5JIL2kf2uoVEg4qG7ujHOuQglykvY5xNw0R0fk= X-Received: by 2002:a5d:40c8:0:b0:337:9ca4:1cd4 with SMTP id b8-20020a5d40c8000000b003379ca41cd4mr264572wrq.220.1705576936622; Thu, 18 Jan 2024 03:22:16 -0800 (PST) X-Google-Smtp-Source: AGHT+IFUzTpjy0cYnLvOp6IsjFnuKlTaCVGYmqRAsbd7zZNL21Ep0dSIHv877Zk9KVr24XOks74KXQ== X-Received: by 2002:a5d:40c8:0:b0:337:9ca4:1cd4 with SMTP id b8-20020a5d40c8000000b003379ca41cd4mr264565wrq.220.1705576936032; Thu, 18 Jan 2024 03:22:16 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id d19-20020adf9c93000000b00337afd67f40sm3820843wre.1.2024.01.18.03.22.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jan 2024 03:22:15 -0800 (PST) From: Andrew Burgess To: Aaron Merey , gdb-patches@sourceware.org Cc: thiago.bauermann@linaro.org, Aaron Merey Subject: Re: [PATCH 1/4 v8] gdb: Buffer output streams during events that might download debuginfo In-Reply-To: <20240117154855.2057850-2-amerey@redhat.com> References: <20240117154855.2057850-1-amerey@redhat.com> <20240117154855.2057850-2-amerey@redhat.com> Date: Thu, 18 Jan 2024 11:22:15 +0000 Message-ID: <87sf2u6g48.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Aaron Merey writes: > v8 fixes a few style issues suggested by Thiago: > https://sourceware.org/pipermail/gdb-patches/2023-December/205504.html > > Introduce new ui_file buffering_file to temporarily collect output > written to gdb_std* output streams during print_thread, print_frame_info > and print_stop_event. > > This ensures that output during these functions is not interrupted > by debuginfod progress messages. > > With the addition of deferred debuginfo downloading it is possible > for download progress messages to print during these events. > Without any intervention we can end up with poorly formatted output: > > (gdb) backtrace > [...] > #8 0x00007fbe8af7d7cf in pygi_invoke_c_callable (Downloading separate debug info for /lib64/libpython3.11.so.1.0 > function_cache=0x561221b224d0, state=... > > To fix this we buffer writes to gdb_std* output streams while allowing > debuginfod progress messages to skip the buffers and print to the > underlying output streams immediately. Buffered output is then written > to the output streams. This ensures that progress messages print first, > followed by uninterrupted frame/thread/stop info: > > (gdb) backtrace > [...] > Downloading separate debug info for /lib64/libpython3.11.so.1.0 > #8 0x00007fbe8af7d7cf in pygi_invoke_c_callable (function_cache=0x561221b224d0, state=... > > Co-Authored-By: Andrew Burgess > --- > gdb/cli-out.c | 10 ++- > gdb/cli-out.h | 3 + > gdb/debuginfod-support.c | 15 ++-- > gdb/infrun.c | 16 +++- > gdb/mi/mi-out.h | 3 + > gdb/python/py-uiout.h | 3 + > gdb/stack.c | 35 +++++--- > gdb/thread.c | 180 ++++++++++++++++++++++---------------- > gdb/ui-file.h | 2 +- > gdb/ui-out.c | 141 ++++++++++++++++++++++++++++++ > gdb/ui-out.h | 184 +++++++++++++++++++++++++++++++++++++++ > 11 files changed, 493 insertions(+), 99 deletions(-) > > diff --git a/gdb/cli-out.c b/gdb/cli-out.c > index a1cde54feda..b4c7a657423 100644 > --- a/gdb/cli-out.c > +++ b/gdb/cli-out.c > @@ -299,7 +299,7 @@ cli_ui_out::do_progress_notify (const std::string &msg, > double howmuch, double total) > { > int chars_per_line = get_chars_per_line (); > - struct ui_file *stream = m_streams.back (); > + struct ui_file *stream = get_unbuffered (m_streams.back ()); > cli_progress_info &info (m_progress_info.back ()); > > if (chars_per_line > MAX_CHARS_PER_LINE) > @@ -384,7 +384,7 @@ cli_ui_out::do_progress_notify (const std::string &msg, > void > cli_ui_out::clear_progress_notify () > { > - struct ui_file *stream = m_streams.back (); > + struct ui_file *stream = get_unbuffered (m_streams.back ()); > int chars_per_line = get_chars_per_line (); > > scoped_restore save_pagination > @@ -413,10 +413,12 @@ void > cli_ui_out::do_progress_end () > { > struct ui_file *stream = m_streams.back (); > - m_progress_info.pop_back (); > + cli_progress_info &info (m_progress_info.back ()); > > - if (stream->isatty ()) > + if (stream->isatty () && info.state != progress_update::START) > clear_progress_notify (); > + > + m_progress_info.pop_back (); > } > > /* local functions */ > diff --git a/gdb/cli-out.h b/gdb/cli-out.h > index 34016182269..89b4aa40870 100644 > --- a/gdb/cli-out.h > +++ b/gdb/cli-out.h > @@ -35,6 +35,9 @@ class cli_ui_out : public ui_out > > bool can_emit_style_escape () const override; > > + ui_file *current_stream () const override > + { return m_streams.back (); } > + > protected: > > virtual void do_table_begin (int nbrofcols, int nr_rows, > diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c > index eb88c406ad6..35dcf56cb7e 100644 > --- a/gdb/debuginfod-support.c > +++ b/gdb/debuginfod-support.c > @@ -155,7 +155,8 @@ progressfn (debuginfod_client *c, long cur, long total) > > if (check_quit_flag ()) > { > - gdb_printf ("Cancelling download of %s %s...\n", > + ui_file *outstream = get_unbuffered (gdb_stdout); > + gdb_printf (outstream, _("Cancelling download of %s %s...\n"), > data->desc, styled_fname.c_str ()); > return 1; > } > @@ -295,10 +296,14 @@ static void > print_outcome (int fd, const char *desc, const char *fname) > { > if (fd < 0 && fd != -ENOENT) > - gdb_printf (_("Download failed: %s. Continuing without %s %ps.\n"), > - safe_strerror (-fd), > - desc, > - styled_string (file_name_style.style (), fname)); > + { > + ui_file *outstream = get_unbuffered (gdb_stdout); > + gdb_printf (outstream, > + _("Download failed: %s. Continuing without %s %ps.\n"), > + safe_strerror (-fd), > + desc, > + styled_string (file_name_style.style (), fname)); > + } > } > > /* See debuginfod-support.h */ > diff --git a/gdb/infrun.c b/gdb/infrun.c > index c769e972f49..b977ba815ff 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -9277,10 +9277,10 @@ print_stop_location (const target_waitstatus &ws) > print_stack_frame (get_selected_frame (nullptr), 0, source_flag, 1); > } > > -/* See infrun.h. */ > +/* See `print_stop_event` in infrun.h. */ > > -void > -print_stop_event (struct ui_out *uiout, bool displays) > +static void > +do_print_stop_event (struct ui_out *uiout, bool displays) > { > struct target_waitstatus last; > struct thread_info *tp; > @@ -9309,6 +9309,16 @@ print_stop_event (struct ui_out *uiout, bool displays) > } > } > > +/* See infrun.h. This function itself sets up buffered output for the > + duration of do_print_stop_event, which performs the actual event > + printing. */ > + > +void > +print_stop_event (struct ui_out *uiout, bool displays) > +{ > + do_with_buffered_output (do_print_stop_event, uiout, displays); > +} > + > /* See infrun.h. */ > > void > diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h > index 0dd7479a52f..68ff5faf632 100644 > --- a/gdb/mi/mi-out.h > +++ b/gdb/mi/mi-out.h > @@ -45,6 +45,9 @@ class mi_ui_out : public ui_out > return false; > } > > + ui_file *current_stream () const override > + { return m_streams.back (); } > + > protected: > > virtual void do_table_begin (int nbrofcols, int nr_rows, const char *tblid) > diff --git a/gdb/python/py-uiout.h b/gdb/python/py-uiout.h > index 9c9617ea831..5944d1b6e34 100644 > --- a/gdb/python/py-uiout.h > +++ b/gdb/python/py-uiout.h > @@ -56,6 +56,9 @@ class py_ui_out : public ui_out > return std::move (current ().obj); > } > > + ui_file *current_stream () const override > + { return nullptr; } > + > protected: > > void do_progress_end () override { } > diff --git a/gdb/stack.c b/gdb/stack.c > index fad4b62c6f7..42e919e1078 100644 > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -220,7 +220,8 @@ static void print_frame_local_vars (frame_info_ptr frame, > const char *regexp, const char *t_regexp, > int num_tabs, struct ui_file *stream); > > -static void print_frame (const frame_print_options &opts, > +static void print_frame (struct ui_out *uiout, > + const frame_print_options &opts, > frame_info_ptr frame, int print_level, > enum print_what print_what, int print_args, > struct symtab_and_line sal); > @@ -1020,16 +1021,15 @@ get_user_print_what_frame_info (std::optional *what) > Used in "where" output, and to emit breakpoint or step > messages. */ > > -void > -print_frame_info (const frame_print_options &fp_opts, > - frame_info_ptr frame, int print_level, > - enum print_what print_what, int print_args, > - int set_current_sal) > +static void > +do_print_frame_info (struct ui_out *uiout, const frame_print_options &fp_opts, > + frame_info_ptr frame, int print_level, > + enum print_what print_what, int print_args, > + int set_current_sal) > { > struct gdbarch *gdbarch = get_frame_arch (frame); > int source_print; > int location_print; > - struct ui_out *uiout = current_uiout; > > if (!current_uiout->is_mi_like_p () > && fp_opts.print_frame_info != print_frame_info_auto) > @@ -1105,7 +1105,8 @@ print_frame_info (const frame_print_options &fp_opts, > || print_what == LOC_AND_ADDRESS > || print_what == SHORT_LOCATION); > if (location_print || !sal.symtab) > - print_frame (fp_opts, frame, print_level, print_what, print_args, sal); > + print_frame (uiout, fp_opts, frame, print_level, > + print_what, print_args, sal); > > source_print = (print_what == SRC_LINE || print_what == SRC_AND_LOC); > > @@ -1185,6 +1186,20 @@ print_frame_info (const frame_print_options &fp_opts, > gdb_flush (gdb_stdout); > } > > +/* Redirect output to a temporary buffer for the duration > + of do_print_frame_info. */ > + > +void > +print_frame_info (const frame_print_options &fp_opts, > + frame_info_ptr frame, int print_level, > + enum print_what print_what, int print_args, > + int set_current_sal) > +{ > + do_with_buffered_output (do_print_frame_info, current_uiout, > + fp_opts, frame, print_level, print_what, > + print_args, set_current_sal); > +} > + > /* See stack.h. */ > > void > @@ -1309,13 +1324,13 @@ find_frame_funname (frame_info_ptr frame, enum language *funlang, > } > > static void > -print_frame (const frame_print_options &fp_opts, > +print_frame (struct ui_out *uiout, > + const frame_print_options &fp_opts, > frame_info_ptr frame, int print_level, > enum print_what print_what, int print_args, > struct symtab_and_line sal) > { > struct gdbarch *gdbarch = get_frame_arch (frame); > - struct ui_out *uiout = current_uiout; > enum language funlang = language_unknown; > struct value_print_options opts; > struct symbol *func; > diff --git a/gdb/thread.c b/gdb/thread.c > index f03a60c4d01..08c156d6687 100644 > --- a/gdb/thread.c > +++ b/gdb/thread.c > @@ -1089,6 +1089,107 @@ thread_target_id_str (thread_info *tp) > return target_id; > } > > +/* Print thread TP. GLOBAL_IDS indicates whether REQUESTED_THREADS > + is a list of global or per-inferior thread ids. */ > + > +static void > +do_print_thread (ui_out *uiout, const char *requested_threads, > + int global_ids, int pid, int show_global_ids, > + int default_inf_num, thread_info *tp, > + thread_info *current_thread) > +{ > + int core; > + > + /* In case REQUESTED_THREADS contains $_thread. */ > + if (current_thread != nullptr) > + switch_to_thread (current_thread); > + > + if (!should_print_thread (requested_threads, default_inf_num, > + global_ids, pid, tp)) > + return; > + > + ui_out_emit_tuple tuple_emitter (uiout, NULL); > + > + if (!uiout->is_mi_like_p ()) > + { > + if (tp == current_thread) > + uiout->field_string ("current", "*"); > + else > + uiout->field_skip ("current"); > + > + uiout->field_string ("id-in-tg", print_thread_id (tp)); > + } > + > + if (show_global_ids || uiout->is_mi_like_p ()) > + uiout->field_signed ("id", tp->global_num); > + > + /* Switch to the thread (and inferior / target). */ > + switch_to_thread (tp); > + > + /* For the CLI, we stuff everything into the target-id field. > + This is a gross hack to make the output come out looking > + correct. The underlying problem here is that ui-out has no > + way to specify that a field's space allocation should be > + shared by several fields. For MI, we do the right thing > + instead. */ > + > + if (uiout->is_mi_like_p ()) > + { > + uiout->field_string ("target-id", target_pid_to_str (tp->ptid)); > + > + const char *extra_info = target_extra_thread_info (tp); > + if (extra_info != nullptr) > + uiout->field_string ("details", extra_info); > + > + const char *name = thread_name (tp); > + if (name != NULL) > + uiout->field_string ("name", name); > + } > + else > + { > + uiout->field_string ("target-id", thread_target_id_str (tp)); > + } > + > + if (tp->state == THREAD_RUNNING) > + uiout->text ("(running)\n"); > + else > + { > + /* The switch above put us at the top of the stack (leaf > + frame). */ > + print_stack_frame (get_selected_frame (NULL), > + /* For MI output, print frame level. */ > + uiout->is_mi_like_p (), > + LOCATION, 0); > + } > + > + if (uiout->is_mi_like_p ()) > + { > + const char *state = "stopped"; > + > + if (tp->state == THREAD_RUNNING) > + state = "running"; > + uiout->field_string ("state", state); > + } > + > + core = target_core_of_thread (tp->ptid); > + if (uiout->is_mi_like_p () && core != -1) > + uiout->field_signed ("core", core); > +} > + > +/* Redirect output to a temporary buffer for the duration > + of do_print_thread. */ > + > +static void > +print_thread (ui_out *uiout, const char *requested_threads, > + int global_ids, int pid, int show_global_ids, > + int default_inf_num, thread_info *tp, thread_info *current_thread) > + > +{ > + do_with_buffered_output (do_print_thread, uiout, requested_threads, > + global_ids, pid, show_global_ids, > + default_inf_num, tp, current_thread); > +} > + > /* Like print_thread_info, but in addition, GLOBAL_IDS indicates > whether REQUESTED_THREADS is a list of global or per-inferior > thread ids. */ > @@ -1176,86 +1277,13 @@ print_thread_info_1 (struct ui_out *uiout, const char *requested_threads, > for (inferior *inf : all_inferiors ()) > for (thread_info *tp : inf->threads ()) > { > - int core; > - > any_thread = true; > + > if (tp == current_thread && tp->state == THREAD_EXITED) > current_exited = true; > > - /* In case REQUESTED_THREADS contains $_thread. */ > - if (current_thread != nullptr) > - switch_to_thread (current_thread); > - > - if (!should_print_thread (requested_threads, default_inf_num, > - global_ids, pid, tp)) > - continue; > - > - ui_out_emit_tuple tuple_emitter (uiout, NULL); > - > - if (!uiout->is_mi_like_p ()) > - { > - if (tp == current_thread) > - uiout->field_string ("current", "*"); > - else > - uiout->field_skip ("current"); > - > - uiout->field_string ("id-in-tg", print_thread_id (tp)); > - } > - > - if (show_global_ids || uiout->is_mi_like_p ()) > - uiout->field_signed ("id", tp->global_num); > - > - /* Switch to the thread (and inferior / target). */ > - switch_to_thread (tp); > - > - /* For the CLI, we stuff everything into the target-id field. > - This is a gross hack to make the output come out looking > - correct. The underlying problem here is that ui-out has no > - way to specify that a field's space allocation should be > - shared by several fields. For MI, we do the right thing > - instead. */ > - > - if (uiout->is_mi_like_p ()) > - { > - uiout->field_string ("target-id", target_pid_to_str (tp->ptid)); > - > - const char *extra_info = target_extra_thread_info (tp); > - if (extra_info != nullptr) > - uiout->field_string ("details", extra_info); > - > - const char *name = thread_name (tp); > - if (name != NULL) > - uiout->field_string ("name", name); > - } > - else > - { > - uiout->field_string ("target-id", thread_target_id_str (tp)); > - } > - > - if (tp->state == THREAD_RUNNING) > - uiout->text ("(running)\n"); > - else > - { > - /* The switch above put us at the top of the stack (leaf > - frame). */ > - print_stack_frame (get_selected_frame (NULL), > - /* For MI output, print frame level. */ > - uiout->is_mi_like_p (), > - LOCATION, 0); > - } > - > - if (uiout->is_mi_like_p ()) > - { > - const char *state = "stopped"; > - > - if (tp->state == THREAD_RUNNING) > - state = "running"; > - uiout->field_string ("state", state); > - } > - > - core = target_core_of_thread (tp->ptid); > - if (uiout->is_mi_like_p () && core != -1) > - uiout->field_signed ("core", core); > + print_thread (uiout, requested_threads, global_ids, pid, > + show_global_ids, default_inf_num, tp, current_thread); > } > > /* This end scope restores the current thread and the frame > diff --git a/gdb/ui-file.h b/gdb/ui-file.h > index 31f87ffd51d..8385033b441 100644 > --- a/gdb/ui-file.h > +++ b/gdb/ui-file.h > @@ -224,7 +224,7 @@ class string_file : public ui_file > bool empty () const { return m_string.empty (); } > void clear () { return m_string.clear (); } > > -private: > +protected: Is this needed? Everything seems to compile fine if this is left as private. > /* The internal buffer. */ > std::string m_string; > > diff --git a/gdb/ui-out.c b/gdb/ui-out.c > index e8653c6c127..75f9f1612cb 100644 > --- a/gdb/ui-out.c > +++ b/gdb/ui-out.c > @@ -871,3 +871,144 @@ ui_out::ui_out (ui_out_flags flags) > ui_out::~ui_out () > { > } > + > +/* See ui-out.h. */ > + > +void > +buffer_group::output_unit::flush () const > +{ > + if (!m_msg.empty ()) > + m_stream->puts (m_msg.c_str ()); > + > + if (m_wrap_hint >= 0) > + m_stream->wrap_here (m_wrap_hint); > + > + if (m_flush) > + m_stream->flush (); > +} > + > +/* See ui-out.h. */ > + > +void > +buffer_group::write (const char *buf, long length_buf, ui_file *stream) > +{ > + /* Record each line separately. */ > + for (size_t prev = 0, cur = 0; cur < length_buf; ++cur) > + if (buf[cur] == '\n' || cur == length_buf - 1) > + { > + std::string msg (buf + prev, cur - prev + 1); > + > + if (m_buffered_output.size () > 0 > + && m_buffered_output.back ().m_wrap_hint == -1 > + && m_buffered_output.back ().m_stream == stream > + && m_buffered_output.back ().m_msg.size () > 0 > + && m_buffered_output.back ().m_msg.back () != '\n') > + m_buffered_output.back ().m_msg.append (msg); > + else > + { > + m_buffered_output.emplace_back (msg); > + m_buffered_output.back ().m_stream = stream; > + } > + prev = cur + 1; > + } > +} > + > +/* See ui-out.h. */ > + > +void > +buffer_group::wrap_here (int indent, ui_file *stream) > +{ > + m_buffered_output.emplace_back ("", indent); > + m_buffered_output.back ().m_stream = stream; > +} > + > +/* See ui-out.h. */ > + > +void > +buffer_group::flush_here (ui_file *stream) > +{ > + m_buffered_output.emplace_back ("", -1, true); > + m_buffered_output.back ().m_stream = stream; > +} > + > +/* See ui-out.h. */ > + > +ui_file * > +get_unbuffered (ui_file * stream) Should be 'ui_file *stream'. > +{ > + buffering_file *buf = dynamic_cast (stream); > + > + if (buf == nullptr) > + return stream; > + > + return get_unbuffered (buf->stream ()); > +} > + > +buffered_streams::buffered_streams (buffer_group *group, ui_out *uiout) > + : m_buffered_stdout (group, gdb_stdout), > + m_buffered_stderr (group, gdb_stderr), > + m_buffered_stdlog (group, gdb_stdlog), > + m_buffered_stdtarg (group, gdb_stdtarg), > + m_buffered_stdtargerr (group, gdb_stdtargerr), > + m_uiout (uiout) > +{ > + gdb_stdout = &m_buffered_stdout; > + gdb_stderr = &m_buffered_stderr; > + gdb_stdlog = &m_buffered_stdlog; > + gdb_stdtarg = &m_buffered_stdtarg; > + gdb_stdtargerr = &m_buffered_stdtargerr; > + > + ui_file *stream = current_uiout->current_stream (); > + if (stream != nullptr) > + { > + m_buffered_current_uiout.emplace (group, stream); > + current_uiout->redirect (&(*m_buffered_current_uiout)); > + } > + > + stream = m_uiout->current_stream (); > + if (stream != nullptr && current_uiout != m_uiout) > + { > + m_buffered_uiout.emplace (group, stream); > + m_uiout->redirect (&(*m_buffered_uiout)); > + } > + > + m_buffers_in_place = true; > +} > + > +/* See ui-out.h. */ > + > +void > +buffered_streams::remove_buffers () > +{ > + if (!m_buffers_in_place) > + return; > + > + m_buffers_in_place = false; > + > + gdb_stdout = m_buffered_stdout.stream (); > + gdb_stderr = m_buffered_stderr.stream (); > + gdb_stdlog = m_buffered_stdlog.stream (); > + gdb_stdtarg = m_buffered_stdtarg.stream (); > + gdb_stdtargerr = m_buffered_stdtargerr.stream (); > + > + if (m_buffered_current_uiout.has_value ()) > + current_uiout->redirect (nullptr); > + > + if (m_buffered_uiout.has_value ()) > + m_uiout->redirect (nullptr); > +} > + > +buffer_group::buffer_group (ui_out *uiout) > + : m_buffered_streams (new buffered_streams (this, uiout)) > +{ /* Nothing. */ } > + > +/* See ui-out.h. */ > + > +void > +buffer_group::flush () const > +{ > + m_buffered_streams->remove_buffers (); > + > + for (const output_unit &ou : m_buffered_output) > + ou.flush (); > +} > diff --git a/gdb/ui-out.h b/gdb/ui-out.h > index 07567a1df35..2934d3a6b34 100644 > --- a/gdb/ui-out.h > +++ b/gdb/ui-out.h > @@ -278,6 +278,9 @@ class ui_out > escapes. */ > virtual bool can_emit_style_escape () const = 0; > > + /* Return the ui_file currently used for output. */ > + virtual ui_file *current_stream () const = 0; > + > /* An object that starts and finishes displaying progress updates. */ > class progress_update > { > @@ -470,4 +473,185 @@ class ui_out_redirect_pop > struct ui_out *m_uiout; > }; > > +struct buffered_streams; > + > +/* Organizes writes to a collection of buffered output streams > + so that when flushed, output is written to all streams in > + chronological order. */ > + > +struct buffer_group > +{ > + buffer_group (ui_out *uiout); > + > + /* Flush all buffered writes to the underlying output streams. */ > + void flush () const; > + > + /* Record contents of BUF and associate it with STREAM. */ > + void write (const char *buf, long length_buf, ui_file *stream); > + > + /* Record a wrap_here and associate it with STREAM. */ > + void wrap_here (int indent, ui_file *stream); > + > + /* Record a call to flush and associate it with STREAM. */ > + void flush_here (ui_file *stream); > + > +private: > + > + struct output_unit > + { > + output_unit (std::string msg, int wrap_hint = -1, bool flush = false) > + : m_msg (msg), m_wrap_hint (wrap_hint), m_flush (flush) > + {} > + > + /* Write contents of this output_unit to the underlying stream. */ > + void flush () const; > + > + /* Underlying stream for which this output unit will be written to. */ > + ui_file *m_stream; > + > + /* String to be written to underlying buffer. */ > + std::string m_msg; > + > + /* Argument to wrap_here. -1 indicates no wrap. Used to call wrap_here > + during buffer flush. */ > + int m_wrap_hint; > + > + /* Indicate that the underlying output stream's flush should be called. */ > + bool m_flush; > + }; > + > + /* Output_units to be written to buffered output streams. */ > + std::vector m_buffered_output; > + > + /* Buffered output streams. */ > + std::unique_ptr m_buffered_streams; > +}; > + > +/* If FILE is a buffering_file, return it's underlying stream. */ > + > +extern ui_file *get_unbuffered (ui_file *file); > + > +/* Buffer output to gdb_stdout and gdb_stderr for the duration of FUNC. */ > + > +template > +void > +do_with_buffered_output (F func, ui_out *uiout, Arg... args) > +{ > + buffer_group g (uiout); > + > + try > + { > + func (uiout, std::forward (args)...); > + } > + catch (gdb_exception &ex) This should be 'const gdb_exception &ex'. > + { > + /* Ideally flush would be called in the destructor of buffer_group, > + however flushing might cause an exception to be thrown. Catch it > + and ensure the first exception propagates. */ > + try > + { > + g.flush (); > + } > + catch (const gdb_exception &) > + { > + } > + > + throw_exception (std::move (ex)); This can be replaced with just 'throw;'. Object slicing has occurred when we caught the exception as a gdb_exception, but the current exception will still be the correct, full object, so we're OK to just rethrow that. > + } > + > + /* Try was successful. Let any further exceptions propagate. */ > + g.flush (); > +} > + > +/* Accumulate writes to an underlying ui_file. Output to the > + underlying file is deferred until required. */ > + > +struct buffering_file : public ui_file > +{ > + buffering_file (buffer_group *group, ui_file *stream) > + : m_group (group), > + m_stream (stream) > + { /* Nothing. */ } > + > + /* Return the underlying output stream. */ > + ui_file *stream () const > + { > + return m_stream; > + } > + > + /* Record the contents of BUF. */ > + void write (const char *buf, long length_buf) override > + { > + m_group->write (buf, length_buf, m_stream); > + } > + > + /* Record a wrap_here call with argument INDENT. */ > + void wrap_here (int indent) override > + { > + m_group->wrap_here (indent, m_stream); > + } > + > + /* Return true if the underlying stream is a tty. */ > + bool isatty () override > + { > + return m_stream->isatty (); > + } > + > + /* Return true if ANSI escapes can be used on the underlying stream. */ > + bool can_emit_style_escape () override > + { > + return m_stream->can_emit_style_escape (); > + } > + > + /* Flush the underlying output stream. */ > + void flush () override > + { > + return m_group->flush_here (m_stream); > + } > + > +private: > + > + /* Coordinates buffering across multiple buffering_files. */ > + buffer_group *m_group; > + > + /* The underlying output stream. */ > + ui_file *m_stream; > +}; > + > +/* Attaches and detaches buffers for each of the gdb_std* streams. */ > + > +struct buffered_streams > +{ > + buffered_streams (buffer_group *group, ui_out *uiout); > + > + ~buffered_streams () > + { > + this->remove_buffers (); > + } > + > + /* Remove buffering_files from all underlying streams. */ > + void remove_buffers (); > + > +private: > + > + /* True if buffers are still attached to each underlying output stream. */ > + bool m_buffers_in_place; > + > + /* Buffers for each gdb_std* output stream. */ > + buffering_file m_buffered_stdout; > + buffering_file m_buffered_stderr; > + buffering_file m_buffered_stdlog; > + buffering_file m_buffered_stdtarg; > + buffering_file m_buffered_stdtargerr; > + > + /* Buffer for current_uiout's output stream. */ > + std::optional m_buffered_current_uiout; > + > + /* Additional ui_out being buffered. */ > + ui_out *m_uiout; > + > + /* Buffer for m_uiout's output stream. */ > + std::optional m_buffered_uiout; > +}; I really dislike GDB's I/O sub-system. Every time I look at the code it always feels like it's far too complicated, and there must be a better way to do things. But I've not yet managed to come up with a plan that would actually make things simpler. This is adding yet more complexity on top, which isn't ideal. But I don't have any better suggestions, and I can see why this is needed. And I know the actual point of this series, the debuginfod work, is going to be super valuable. So, given nobody who knows the I/O stuff better has commented, I propose that, with the fixes I've pointed out above: Approved-By: Andrew Burgess Thanks, Andrew > + > #endif /* UI_OUT_H */ > -- > 2.43.0