[PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Aaron Merey]

During debuginfod downloads, ctrl-c should result in the download
being cancelled and skipped.  However in some cases, ctrl-c fails to
get delivered to gdb during downloading.  This can result in downloads
being unskippable.

Fix this by ensuring that target_terminal::ours is in effect for the
duration of each download.

Co-authored-by: Tom de Vries <vries@gcc.gnu.org>
https://sourceware.org/bugzilla/show_bug.cgi?id=27026#c3
---
 gdb/debuginfod-support.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
index 2e1837da949..1f160e29714 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -23,6 +23,7 @@
 #include "gdbsupport/gdb_optional.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-style.h"
+#include "target.h"
 
 /* Set/show debuginfod commands.  */
 static cmd_list_element *set_debuginfod_prefix_list;
@@ -204,6 +205,13 @@ debuginfod_source_query (const unsigned char *build_id,
   user_data data ("source file", srcpath);
 
   debuginfod_set_user_data (c, &data);
+  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
+  if (target_supports_terminal_ours ())
+    {
+      term_state.emplace ();
+      target_terminal::ours ();
+    }
+
   scoped_fd fd (debuginfod_find_source (c,
 					build_id,
 					build_id_len,
@@ -242,6 +250,13 @@ debuginfod_debuginfo_query (const unsigned char *build_id,
   user_data data ("separate debug info for", filename);
 
   debuginfod_set_user_data (c, &data);
+  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
+  if (target_supports_terminal_ours ())
+    {
+      term_state.emplace ();
+      target_terminal::ours ();
+    }
+
   scoped_fd fd (debuginfod_find_debuginfo (c, build_id, build_id_len,
 					   &dname));
   debuginfod_set_user_data (c, nullptr);
-- 
2.33.1

Re: [PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Tom de Vries]

On 11/24/21 2:43 AM, Aaron Merey wrote:
> During debuginfod downloads, ctrl-c should result in the download
> being cancelled and skipped.  However in some cases, ctrl-c fails to
> get delivered to gdb during downloading.  This can result in downloads
> being unskippable.
> 
> Fix this by ensuring that target_terminal::ours is in effect for the
> duration of each download.
> 
> Co-authored-by: Tom de Vries <vries@gcc.gnu.org>

Hi Aaron,

thanks for picking this up.

Please use "Tom de Vries <tdevries@suse.de>" (as listed in gdb/MAINTAINERS).

Thanks,
- Tom

> https://sourceware.org/bugzilla/show_bug.cgi?id=27026#c3
> ---
>  gdb/debuginfod-support.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 2e1837da949..1f160e29714 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -23,6 +23,7 @@
>  #include "gdbsupport/gdb_optional.h"
>  #include "cli/cli-cmds.h"
>  #include "cli/cli-style.h"
> +#include "target.h"
>  
>  /* Set/show debuginfod commands.  */
>  static cmd_list_element *set_debuginfod_prefix_list;
> @@ -204,6 +205,13 @@ debuginfod_source_query (const unsigned char *build_id,
>    user_data data ("source file", srcpath);
>  
>    debuginfod_set_user_data (c, &data);
> +  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
> +  if (target_supports_terminal_ours ())
> +    {
> +      term_state.emplace ();
> +      target_terminal::ours ();
> +    }
> +
>    scoped_fd fd (debuginfod_find_source (c,
>  					build_id,
>  					build_id_len,
> @@ -242,6 +250,13 @@ debuginfod_debuginfo_query (const unsigned char *build_id,
>    user_data data ("separate debug info for", filename);
>  
>    debuginfod_set_user_data (c, &data);
> +  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
> +  if (target_supports_terminal_ours ())
> +    {
> +      term_state.emplace ();
> +      target_terminal::ours ();
> +    }
> +
>    scoped_fd fd (debuginfod_find_debuginfo (c, build_id, build_id_len,
>  					   &dname));
>    debuginfod_set_user_data (c, nullptr);
> 

Re: [PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Aaron Merey]

On Wed, Nov 24, 2021 at 2:34 AM Tom de Vries <tdevries@suse.de> wrote:
> On 11/24/21 2:43 AM, Aaron Merey wrote:
> > During debuginfod downloads, ctrl-c should result in the download
> > being cancelled and skipped.  However in some cases, ctrl-c fails to
> > get delivered to gdb during downloading.  This can result in downloads
> > being unskippable.
> >
> > Fix this by ensuring that target_terminal::ours is in effect for the
> > duration of each download.
> >
> > Co-authored-by: Tom de Vries <vries@gcc.gnu.org>
>
> Hi Aaron,
>
> thanks for picking this up.
>
> Please use "Tom de Vries <tdevries@suse.de>" (as listed in gdb/MAINTAINERS).

Thanks Tom. Fixed.

Aaron

Re: [PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Kevin Buettner]

On Tue, 23 Nov 2021 20:43:14 -0500
Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> wrote:

> During debuginfod downloads, ctrl-c should result in the download
> being cancelled and skipped.  However in some cases, ctrl-c fails to
> get delivered to gdb during downloading.  This can result in downloads
> being unskippable.
> 
> Fix this by ensuring that target_terminal::ours is in effect for the
> duration of each download.

Okay.

(Make sure that you include email address adjustment requested by Tom.)

Kevin

Re: [PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Aaron Merey]

On Mon, Nov 29, 2021 at 2:23 PM Kevin Buettner <kevinb@redhat.com> wrote:
>
> On Tue, 23 Nov 2021 20:43:14 -0500
> Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> wrote:
>
> > During debuginfod downloads, ctrl-c should result in the download
> > being cancelled and skipped.  However in some cases, ctrl-c fails to
> > get delivered to gdb during downloading.  This can result in downloads
> > being unskippable.
> >
> > Fix this by ensuring that target_terminal::ours is in effect for the
> > duration of each download.
>
> Okay.
>
> (Make sure that you include email address adjustment requested by Tom.)

Thanks Kevin. Pushed as commit b9db26b4c.

Aaron

Re: [PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Tom Tromey]

>>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

Aaron> +  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
Aaron> +  if (target_supports_terminal_ours ())
Aaron> +    {
Aaron> +      term_state.emplace ();
Aaron> +      target_terminal::ours ();
Aaron> +    }

I was curious to know if the gdb::optional and the
target_supports_terminal_ours checks are really needed here.
I think plenty of spots do this stuff unconditionally, so I
was wondering if this code has some special need.

Tom

Re: [PATCH] [PR gdb/27026] CTRL-C is ignored when debug info is downloaded

[Aaron Merey]

Hi Tom,

On Wed, Dec 1, 2021 at 11:45 AM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Aaron> +  gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
> Aaron> +  if (target_supports_terminal_ours ())
> Aaron> +    {
> Aaron> +      term_state.emplace ();
> Aaron> +      target_terminal::ours ();
> Aaron> +    }
>
> I was curious to know if the gdb::optional and the
> target_supports_terminal_ours checks are really needed here.
> I think plenty of spots do this stuff unconditionally, so I
> was wondering if this code has some special need.

These checks came from Tom's draft of this patch. AFAICT it's done
out of an abundance of caution in case this code runs before any
inferior is loaded. I'm not sure that is currently possible but I think
it's better to be safe than sorry here.

Aaron