From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 2D5873858C53 for ; Fri, 25 Aug 2023 10:19:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2D5873858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692958796; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gY09bbFM3GwupNIkXZ8OsYoi/DLMZ+US8Ok8iwXFgC4=; b=jM4efexfRtCIkHRRUiqMSDHniXHyY8iwhT9bDMo8pIyofyzabl4/ArRNsyv5y2ZQ+BYSP1 6ojJjTEt3tnFfF4M5tWtXbtr+p9okVgmtQO7pCpv8C6U7QfB5M1aTwnnORVcKE15ETgfOt QDGHLBZtE/fL+lyVBqhsLmaeRo9uRYc= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-651-mw84Lh8zNgC94H7UUe22gw-1; Fri, 25 Aug 2023 06:19:55 -0400 X-MC-Unique: mw84Lh8zNgC94H7UUe22gw-1 Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-9a2202c0a2bso57298766b.3 for ; Fri, 25 Aug 2023 03:19:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692958794; x=1693563594; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gY09bbFM3GwupNIkXZ8OsYoi/DLMZ+US8Ok8iwXFgC4=; b=ShaOdmCRwWGUloPgdmDBjRKqYvCvfhX7YINI4/Yjyjc9akxqGSuzbRoVM1GGTKJtfC w3wlBqdmFPGR2VfjI01876XrR707U2mw8nv26ZrPVhTc8mrmA18jSXg36N810m4LhSos UorILKf/Fnqqz53liusobykkE4UvMGyKCrssc7W8QH6JoXV2565VPErhsMDt7JJf9tud 1ATfBuV1MByFD+LS8+mnJdxFN/xBT0KAysvKoQWUFeCA2QIAauuXDEXY/X64S2NDYK3N p0HhbYTQD0MEd86/cz0zIa1CJKN76ToxcKjOYPNdzCTuOajOzWhjC6aOADvzztNuRhJv yFOA== X-Gm-Message-State: AOJu0YwHvLrfK59ZMb/7O1M1GqBDG+qP/Q08Da92psRnA1im4RogtmVI YA4buvenrPli9MrhzaZk9BrEKVx6PxzGy4+DApuDgvx1rEFyCHUQ2SsvaT1aYt54iIQmZ1zwFtA c9OtiekfOA+IHV1MMU9rcrg== X-Received: by 2002:a17:907:75c6:b0:99c:10e8:52d7 with SMTP id jl6-20020a17090775c600b0099c10e852d7mr12875183ejc.31.1692958794174; Fri, 25 Aug 2023 03:19:54 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEQJ93I2WI86KZHe1SWfssOhbt4M0y/mdFutDrtLYM3p8rQCFObuABlw2NRX6P1Tko/qfra3g== X-Received: by 2002:a17:907:75c6:b0:99c:10e8:52d7 with SMTP id jl6-20020a17090775c600b0099c10e852d7mr12875173ejc.31.1692958793801; Fri, 25 Aug 2023 03:19:53 -0700 (PDT) Received: from [192.168.0.129] (ip-94-112-225-44.bb.vodafone.cz. [94.112.225.44]) by smtp.gmail.com with ESMTPSA id y16-20020a1709064b1000b00992e94bcfabsm793362eju.167.2023.08.25.03.19.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 25 Aug 2023 03:19:53 -0700 (PDT) Message-ID: Date: Fri, 25 Aug 2023 12:19:52 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v3] gdb/debuginfod: Ctrl-C ask to cancel further downloads To: Aaron Merey , gdb-patches@sourceware.org Cc: =?UTF-8?Q?Martin_Li=c5=a1ka?= , Tom de Vries References: <20230307004134.127886-1-amerey@redhat.com> From: Guinevere Larsen In-Reply-To: <20230307004134.127886-1-amerey@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 07/03/2023 01:41, Aaron Merey via Gdb-patches wrote: > v1 can be found here: > https://sourceware.org/pipermail/gdb-patches/2022-September/192188.html > > v2 adds Ctrl-C pressed-twice functionality to debuginfod downloads based on > Pedro's feedback: > https://sourceware.org/pipermail/gdb-patches/2022-September/192225.html > > v3 improves Ctrl-C handling when pressed during the user prompts added > in this patch. This prevents unsafe early returns to the main event loop > that could leave some of gdb's internal structures in a broken state. > > v3 also includes doc improvements suggested by Eli: > https://sourceware.org/pipermail/gdb-patches/2022-December/194892.html > > 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 > Co-Authored-By: Tom de Vries Thanks for working on this! Sounds like a good change. Unfortunately, this patch no longer applies, and the change aren't trivial enough for me to test in current master. -- Cheers, Guinevere Larsen She/Her/Hers > --- > gdb/debuginfod-support.c | 166 ++++++++++++++++++++++++++++++++++++++- > gdb/doc/gdb.texinfo | 21 +++++ > 2 files changed, 183 insertions(+), 4 deletions(-) > > diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c > index 04d254a1601..41d14817ad0 100644 > --- a/gdb/debuginfod-support.c > +++ b/gdb/debuginfod-support.c > @@ -34,6 +34,8 @@ static cmd_list_element *show_debuginfod_prefix_list; > 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[] = > { > @@ -43,6 +45,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; > @@ -88,9 +104,11 @@ debuginfod_exec_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; > @@ -148,9 +166,81 @@ progressfn (debuginfod_client *c, long cur, long total) > > if (check_quit_flag ()) > { > - gdb_printf ("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 (gdb_exception &except) > + { > + /* 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 ("\n"); > + extra_nl = false; > + } > + > + gdb_printf (_("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 (gdb_exception &except) > + { > + 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 ("\n"); > + > + gdb_printf (_("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) > @@ -293,6 +383,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; > + > gdb::optional term_state; > if (target_supports_terminal_ours ()) > { > @@ -308,6 +402,10 @@ debuginfod_source_query (const unsigned char *build_id, > debuginfod_set_user_data (c, nullptr); > print_outcome (data, fd.get ()); > > + if (data.pass_quit_flag) > + set_quit_flag (); > + if (data.inf_had_term && term_state.has_value ()) > + target_terminal::inferior (); > if (fd.get () >= 0) > destname->reset (dname); > > @@ -334,6 +432,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; > + > gdb::optional term_state; > if (target_supports_terminal_ours ()) > { > @@ -346,6 +448,10 @@ debuginfod_debuginfo_query (const unsigned char *build_id, > debuginfod_set_user_data (c, nullptr); > print_outcome (data, fd.get ()); > > + if (data.pass_quit_flag) > + set_quit_flag (); > + if (data.inf_had_term && term_state.has_value ()) > + target_terminal::inferior (); > if (fd.get () >= 0) > destname->reset (dname); > > @@ -372,6 +478,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; > + > gdb::optional term_state; > if (target_supports_terminal_ours ()) > { > @@ -383,6 +493,10 @@ debuginfod_exec_query (const unsigned char *build_id, > debuginfod_set_user_data (c, nullptr); > print_outcome (data, fd.get ()); > > + if (data.pass_quit_flag) > + set_quit_flag (); > + if (data.inf_had_term && term_state.has_value ()) > + target_terminal::inferior (); > if (fd.get () >= 0) > destname->reset (dname); > > @@ -423,6 +537,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 > @@ -503,6 +644,23 @@ source files."), > &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 bfda7edc4f7..a79b263ee6d 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -48773,6 +48773,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