[PING*2][PATCH v4] gdb/debuginfod: Ctrl-C asks to cancel further downloads
Aaron Merey
amerey@redhat.com
Fri May 17 14:11:46 GMT 2024
Ping re. non-documentation parts of the patch
Thanks,
Aaron
On Thu, Feb 22, 2024 at 5:26 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Ping
>
> Thanks,
> Aaron
>
> On Fri, Feb 2, 2024 at 9:15 PM Aaron Merey <amerey@redhat.com> wrote:
> >
> > v3: https://sourceware.org/pipermail/gdb-patches/2023-March/197679.html
> >
> > v4 includes changes needed to apply the patch to the current master branch.
> >
> > Currently gdb only allows for debuginfod downloads to be cancelled one at
> > a time via Ctrl-C. This can be a burden if one wishes to cancel a large
> > series of downloads. Additionally there can also be ambiguity between whether
> > Ctrl-C during a download was intended to cancel a download or interrupt the
> > inferior.
> >
> > This patch addresses these issues by adding a new debuginfod setting and
> > changing the behavior of Ctrl-C during a download.
> >
> > Add a new command "set debuginfod cancel one/all/ask", where:
> > - "one" means Ctrl-C cancels one download,
> > - "all" means Ctrl-C cancels all further downloads, and
> > - "ask" means Ctrl-C asks whether to cancel all further downloads. A "yes"
> > implies "set debuginfod cancel all", and a "no" implies "set debuginfod
> > cancel one", so the question is only asked once.
> >
> > Note that the behaviour as it was before this patch is equivalent to
> > "set debuginfod cancel one". Instead, the new default is "set debuginfod
> > cancel ask". Note that cancelling all further downloads implies "set
> > debuginfod enabled off".
> >
> > A single Ctrl-C during downloading now sets the quit_flag and proceeds with
> > all downloads. If the inferior has the terminal, then a second Ctrl-C during
> > downloading triggers a query asking whether to cancel the download or
> > interrupt the inferior. If the user wishes to cancel the download then
> > the setting of 'set debuginfod cancel' determines whether one or all
> > downloads are cancelled. In the case of "set debuginfod cancel ask",
> > there will be another query at this point asking whether to cancel one
> > or all downloads.
> >
> > If the inferior does not have the terminal, then a second Ctrl-C during
> > downloading simply cancels the download according to the setting of
> > "set debuginfod cancel". In this case there is no query asking whether
> > to interrupt the inferior or cancel a download.
> >
> > Example session where inferior has terminal:
> >
> > (gdb) run
> > [...]
> > Downloading separate debug info for /lib64/libxyz.so
> > [### ]^C^C
> > Cancel the current download?
> > If no, then Ctrl-C will be sent to the target process. ([y] or n) y
> > Cancelling download of separate debug info for /lib64/libxyz.so...
> > Cancel further downloading for this session? (y or [n]) n
> > Downloading separate debug info for /lib64/libabcd.so
> >
> > Example session where inferior does not have terminal:
> >
> > (gdb) run
> > [...]
> > Downloading separate debug info for /lib64/libxyz.so
> > [### ]^C^C
> > Cancelling download of separate debug info for /lib64/libxyz.so...
> > Cancel further downloading for this session? (y or [n]) y
> > Debuginfod has been disabled.
> > To re-enable use the 'set debuginfod enabled' command.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29582
> > Suggested-By: Martin Liška <mliska@suse.cz>
> > Co-Authored-By: Tom de Vries <tdevries@suse.de>
> > ---
> > gdb/debuginfod-support.c | 183 +++++++++++++++++++++++++++++++++++++--
> > gdb/doc/gdb.texinfo | 21 +++++
> > 2 files changed, 199 insertions(+), 5 deletions(-)
> >
> > diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> > index 7d8ada39e96..97d4b2f2bac 100644
> > --- a/gdb/debuginfod-support.c
> > +++ b/gdb/debuginfod-support.c
> > @@ -38,6 +38,8 @@ static cmd_list_element *maint_show_debuginfod_cmdlist;
> > static const char debuginfod_on[] = "on";
> > static const char debuginfod_off[] = "off";
> > static const char debuginfod_ask[] = "ask";
> > +static const char debuginfod_one[] = "one";
> > +static const char debuginfod_all[] = "all";
> >
> > static const char *debuginfod_enabled_enum[] =
> > {
> > @@ -47,6 +49,20 @@ static const char *debuginfod_enabled_enum[] =
> > nullptr
> > };
> >
> > +/* Valid values for set debuginfod cancel command. */
> > +
> > +static const char *debuginfod_cancel_enum[] =
> > +{
> > + debuginfod_one,
> > + debuginfod_all,
> > + debuginfod_ask,
> > + nullptr
> > +};
> > +
> > +/* Value of debuginfod cancellation mode. */
> > +
> > +static const char *debuginfod_cancel = debuginfod_ask;
> > +
> > static const char *debuginfod_enabled =
> > #if defined(HAVE_LIBDEBUGINFOD)
> > debuginfod_ask;
> > @@ -109,9 +125,11 @@ debuginfod_section_query (const unsigned char *build_id,
> > struct user_data
> > {
> > user_data (const char *desc, const char *fname)
> > - : desc (desc), fname (fname)
> > + : pass_quit_flag (false), inf_had_term (false), desc (desc), fname (fname)
> > { }
> >
> > + bool pass_quit_flag;
> > + bool inf_had_term;
> > const char * const desc;
> > const char * const fname;
> > ui_out::progress_update progress;
> > @@ -156,10 +174,83 @@ progressfn (debuginfod_client *c, long cur, long total)
> > if (check_quit_flag ())
> > {
> > ui_file *outstream = get_unbuffered (gdb_stdout);
> > - gdb_printf (outstream, _("Cancelling download of %s %s...\n"),
> > - data->desc, styled_fname.c_str ());
> > - return 1;
> > - }
> > +
> > + /* If a single Ctrl-C occurs during downloading, let it propagate to the
> > + target. If more than one Ctrl-C occurs, ask whether to cancel the
> > + current download or interrupt the target. If the download is
> > + cancelled, the setting of debuginfod_cancel will determine whether
> > + the current download is cancelled or debuginfod is disabled. */
> > + if (!data->pass_quit_flag)
> > + data->pass_quit_flag = true;
> > + else
> > + {
> > + int resp = 1;
> > + bool extra_nl = false;
> > +
> > + if (data->inf_had_term)
> > + {
> > + /* If Ctrl-C occurs during the following prompts, catch the
> > + exception to prevent unsafe early returns to gdb's main
> > + event loop. During these prompts, Ctrl-C is equivalent to
> > + answering 'y'. */
> > + try
> > + {
> > + resp = yquery (_("Cancel the current download?\nIf no, "
> > + "then Ctrl-C will be sent to the target "
> > + "process. "));
> > + }
> > + catch (const gdb_exception &)
> > + {
> > + /* If the query doesn't complete, then we need an additional
> > + newline to get "Cancelling download of..." printed on a
> > + separate line. */
> > + extra_nl = true;
> > + }
> > + }
> > + if (resp)
> > + {
> > + if (extra_nl)
> > + {
> > + gdb_printf (outstream, "\n");
> > + extra_nl = false;
> > + }
> > +
> > + gdb_printf (outstream, _("Cancelling download of %s %s...\n"),
> > + data->desc, styled_fname.c_str ());
> > + if (debuginfod_cancel == debuginfod_ask)
> > + {
> > + try
> > + {
> > + resp = nquery
> > + (_("Cancel further downloading for this session? "));
> > + }
> > + catch (const gdb_exception &)
> > + {
> > + resp = 1;
> > + extra_nl = true;
> > + }
> > +
> > + if (resp)
> > + debuginfod_cancel = debuginfod_all;
> > + else
> > + debuginfod_cancel = debuginfod_one;
> > + }
> > + if (debuginfod_cancel == debuginfod_all)
> > + {
> > + if (extra_nl)
> > + gdb_printf (outstream, "\n");
> > +
> > + gdb_printf (outstream,
> > + _("Debuginfod has been disabled.\nTo re-enable "
> > + "use the 'set debuginfod enabled' command.\n"));
> > + debuginfod_enabled = debuginfod_off;
> > + }
> > +
> > + data->pass_quit_flag = false;
> > + return 1;
> > + }
> > + }
> > + }
> >
> > if (debuginfod_verbose == 0)
> > return 0;
> > @@ -330,6 +421,10 @@ debuginfod_source_query (const unsigned char *build_id,
> > user_data data ("source file", srcpath);
> >
> > debuginfod_set_user_data (c, &data);
> > +
> > + if (!target_terminal::is_ours ())
> > + data.inf_had_term = true;
> > +
> > if (target_supports_terminal_ours ())
> > {
> > term_state.emplace ();
> > @@ -341,6 +436,11 @@ debuginfod_source_query (const unsigned char *build_id,
> > build_id_len,
> > srcpath,
> > &dname));
> > + if (data.pass_quit_flag)
> > + set_quit_flag ();
> > + if (data.inf_had_term && term_state.has_value ())
> > + target_terminal::inferior ();
> > +
> > debuginfod_set_user_data (c, nullptr);
> > }
> >
> > @@ -376,6 +476,10 @@ debuginfod_debuginfo_query (const unsigned char *build_id,
> > user_data data ("separate debug info for", filename);
> >
> > debuginfod_set_user_data (c, &data);
> > +
> > + if (!target_terminal::is_ours ())
> > + data.inf_had_term = true;
> > +
> > if (target_supports_terminal_ours ())
> > {
> > term_state.emplace ();
> > @@ -384,6 +488,12 @@ debuginfod_debuginfo_query (const unsigned char *build_id,
> >
> > fd = scoped_fd (debuginfod_find_debuginfo (c, build_id, build_id_len,
> > &dname));
> > +
> > + if (data.pass_quit_flag)
> > + set_quit_flag ();
> > + if (data.inf_had_term && term_state.has_value ())
> > + target_terminal::inferior ();
> > +
> > debuginfod_set_user_data (c, nullptr);
> > }
> >
> > @@ -419,6 +529,10 @@ debuginfod_exec_query (const unsigned char *build_id,
> > user_data data ("executable for", filename);
> >
> > debuginfod_set_user_data (c, &data);
> > +
> > + if (!target_terminal::is_ours ())
> > + data.inf_had_term = true;
> > +
> > if (target_supports_terminal_ours ())
> > {
> > term_state.emplace ();
> > @@ -427,6 +541,11 @@ debuginfod_exec_query (const unsigned char *build_id,
> >
> > fd = scoped_fd (debuginfod_find_executable (c, build_id, build_id_len,
> > &dname));
> > + if (data.pass_quit_flag)
> > + set_quit_flag ();
> > + if (data.inf_had_term && term_state.has_value ())
> > + target_terminal::inferior ();
> > +
> > debuginfod_set_user_data (c, nullptr);
> > }
> >
> > @@ -467,6 +586,10 @@ debuginfod_section_query (const unsigned char *build_id,
> > {
> > user_data data (desc.c_str (), filename);
> > debuginfod_set_user_data (c, &data);
> > +
> > + if (!target_terminal::is_ours ())
> > + data.inf_had_term = true;
> > +
> > if (target_supports_terminal_ours ())
> > {
> > term_state.emplace ();
> > @@ -475,6 +598,12 @@ debuginfod_section_query (const unsigned char *build_id,
> >
> > fd = scoped_fd (debuginfod_find_section (c, build_id, build_id_len,
> > section_name, &dname));
> > +
> > + if (data.pass_quit_flag)
> > + set_quit_flag ();
> > + if (data.inf_had_term && term_state.has_value ())
> > + target_terminal::inferior ();
> > +
> > debuginfod_set_user_data (c, nullptr);
> > }
> >
> > @@ -523,6 +652,33 @@ show_debuginfod_enabled (ui_file *file, int from_tty, cmd_list_element *cmd,
> > "\"%s\".\n"), debuginfod_enabled);
> > }
> >
> > +/* Set callback for "set debuginfod cancel". */
> > +
> > +static void
> > +set_debuginfod_cancel (const char *value)
> > +{
> > + debuginfod_cancel = value;
> > +}
> > +
> > +/* Get callback for "set debuginfod cancel". */
> > +
> > +static const char *
> > +get_debuginfod_cancel ()
> > +{
> > + return debuginfod_cancel;
> > +}
> > +
> > +/* Show callback for "set debuginfod cancel". */
> > +
> > +static void
> > +show_debuginfod_cancel (ui_file *file, int from_tty, cmd_list_element *cmd,
> > + const char *value)
> > +{
> > + gdb_printf (file,
> > + _("Debuginfod cancellation mode is currently set to "
> > + "\"%s\".\n"), debuginfod_cancel);
> > +}
> > +
> > /* Set callback for "set debuginfod urls". */
> >
> > static void
> > @@ -627,6 +783,23 @@ When set to \"ask\", prompt whether to enable or disable debuginfod." ),
> > &set_debuginfod_prefix_list,
> > &show_debuginfod_prefix_list);
> >
> > + add_setshow_enum_cmd ("cancel", class_run, debuginfod_cancel_enum,
> > + _("Set Ctrl-C behaviour for debuginfod."),
> > + _("Show Ctrl-C behaviour for debuginfod."),
> > + _("\
> > +When set to \'one\', pressing Ctrl-C twice cancels a single \
> > +download.\nWhen set to \'all\', pressing Ctrl-C twice cancels all further downloads.\n\
> > +When set to \'ask\', pressing Ctrl-C twice asks what to do.\nA single Ctrl-C during \
> > +downloading is passed to the target process being debugged.\nA second Ctrl-C \
> > +during downloading may raise a prompt asking whether to cancel the download or \
> > +send Ctrl-C to the target.\nIf the download is cancelled, then no Ctrl-C is \
> > +sent to the target."),
> > + set_debuginfod_cancel,
> > + get_debuginfod_cancel,
> > + show_debuginfod_cancel,
> > + &set_debuginfod_prefix_list,
> > + &show_debuginfod_prefix_list);
> > +
> > /* set/show debuginfod urls */
> > add_setshow_string_noescape_cmd ("urls", class_run, _("\
> > Set the list of debuginfod server URLs."), _("\
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index e98c15242bc..d6df28896b1 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -50259,6 +50259,27 @@ is set to @code{ask} for interactive sessions.
> > Display whether @code{debuginfod enabled} is set to @code{on}, @code{off} or
> > @code{ask}.
> >
> > +@kindex set debuginfod cancel
> > +@anchor{set debuginfod cancel}
> > +@item set debuginfod cancel
> > +@itemx set debuginfod cancel one
> > +@cindex debuginfod @kbd{Ctrl-C} behaviour
> > +Pressing @kbd{Ctrl-C} twice during downloading will cancel the current
> > +download.
> > +
> > +@item set debuginfod cancel all
> > +Pressing @kbd{Ctrl-C} twice during downloading will cancel this and all
> > +further downloads.
> > +
> > +@item set debuginfod cancel ask
> > +Pressing @kbd{Ctrl-C} twice during downloading will cancel the current
> > +download and prompt whether to cancel all further downloads. By default,
> > +@code{debuginfod cancel} is set to @code{ask} for interactive sessions.
> > +
> > +@kindex show debuginfod cancel
> > +@item show debuginfod cancel
> > +Display the current setting of @code{debuginfod cancel}.
> > +
> > @kindex set debuginfod urls
> > @cindex configure debuginfod URLs
> > @item set debuginfod urls
> > --
> > 2.43.0
> >
More information about the Gdb-patches
mailing list