From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: [PATCH 3/3] Fix control-c handling on Windows
Date: Mon, 5 Dec 2022 11:56:51 -0700 [thread overview]
Message-ID: <20221205185651.2704492-4-tromey@adacore.com> (raw)
In-Reply-To: <20221205185651.2704492-1-tromey@adacore.com>
As Hannes pointed out, the Windows target-async patches broke C-c
handling there. Looking into this, I found a few oddities, fixed
here.
First, windows_nat_target::interrupt calls GenerateConsoleCtrlEvent.
I think this event can be ignored by the inferior, so it's not a great
way to interrupt. Instead, using DebugBreakProcess (or a more
complicated thing for Wow64) seems better.
Second, windows_nat_target did not implement the pass_ctrlc method.
Implementing this lets us remove the special code to call
SetConsoleCtrlHandler and instead integrate into gdb's approach to C-c
handling. I believe that this should also fix the race that's
described in the comment that's being removed.
Initially, I thought a simpler version of this patch would work.
However, I think what happens is that some other library (I'm not sure
what) calls SetConsoleCtrlHandler while gdb is running, and this
intercepts and handles C-c -- so that the gdb SIGINT handler is not
called. C-break continues to work, presumably because whatever
handler is installed ignores it.
This patch works around this issue by ensuring that the gdb handler
always comes first.
---
gdb/event-top.c | 2 +-
gdb/extension.c | 5 +--
gdb/inferior.h | 10 ++++++
gdb/inflow.c | 8 ++---
gdb/mingw-hdep.c | 37 +++++++++++++++++++++
gdb/posix-hdep.c | 24 ++++++++++++++
gdb/windows-nat.c | 83 +++++++++++------------------------------------
7 files changed, 98 insertions(+), 71 deletions(-)
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 0e371194ee3..29dd151f0b5 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -1071,7 +1071,7 @@ gdb_init_signals (void)
sigint_token =
create_async_signal_handler (async_request_quit, NULL, "sigint");
- signal (SIGINT, handle_sigint);
+ install_sigint_handler (handle_sigint);
async_sigterm_token
= create_async_signal_handler (async_sigterm_handler, NULL, "sigterm");
diff --git a/gdb/extension.c b/gdb/extension.c
index 1d951d60041..ae8ef0d6e31 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -33,6 +33,7 @@
#include "python/python.h"
#include "guile/guile.h"
#include <array>
+#include "inferior.h"
static script_sourcer_func source_gdb_script;
static objfile_script_sourcer_func source_gdb_objfile_script;
@@ -661,7 +662,7 @@ install_ext_sigint_handler (const struct signal_handler *handler_state)
{
gdb_assert (handler_state->handler_saved);
- signal (SIGINT, handler_state->handler);
+ install_sigint_handler (handler_state->handler);
}
/* Install GDB's SIGINT handler, storing the previous version in *PREVIOUS.
@@ -675,7 +676,7 @@ install_gdb_sigint_handler (struct signal_handler *previous)
/* Save here to simplify comparison. */
sighandler_t handle_sigint_for_compare = handle_sigint;
- previous->handler = signal (SIGINT, handle_sigint);
+ previous->handler = install_sigint_handler (handle_sigint);
if (previous->handler != handle_sigint_for_compare)
previous->handler_saved = 1;
else
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 547e8751d08..6fc0a30b12c 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -178,6 +178,16 @@ extern tribool is_gdb_terminal (const char *tty);
extern tribool sharing_input_terminal (int pid);
+/* The type of the function that is called when SIGINT is handled. */
+
+typedef void c_c_handler_ftype (int);
+
+/* Install a new SIGINT handler in a host-dependent way. The previous
+ handler is returned. It is fine to pass SIG_IGN for FN, but not
+ SIG_DFL. */
+
+extern c_c_handler_ftype *install_sigint_handler (c_c_handler_ftype *fn);
+
extern void child_terminal_info (struct target_ops *self, const char *, int);
extern void child_terminal_ours (struct target_ops *self);
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 17b1e8ce282..344a4eaf509 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -332,7 +332,7 @@ child_terminal_inferior (struct target_ops *self)
if (!job_control)
{
- sigint_ours = signal (SIGINT, SIG_IGN);
+ sigint_ours = install_sigint_handler (SIG_IGN);
#ifdef SIGQUIT
sigquit_ours = signal (SIGQUIT, SIG_IGN);
#endif
@@ -480,7 +480,7 @@ child_terminal_ours_1 (target_terminal_state desired_state)
if (!job_control && desired_state == target_terminal_state::is_ours)
{
- signal (SIGINT, sigint_ours);
+ install_sigint_handler (sigint_ours);
#ifdef SIGQUIT
signal (SIGQUIT, sigquit_ours);
#endif
@@ -865,7 +865,7 @@ set_sigint_trap (void)
if (inf->attach_flag || !tinfo->run_terminal.empty ())
{
- osig = signal (SIGINT, pass_signal);
+ osig = install_sigint_handler (pass_signal);
osig_set = 1;
}
else
@@ -877,7 +877,7 @@ clear_sigint_trap (void)
{
if (osig_set)
{
- signal (SIGINT, osig);
+ install_sigint_handler (osig);
osig_set = 0;
}
}
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index 9d0337cf4ef..5a69282b99b 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -400,3 +400,40 @@ sharing_input_terminal (int pid)
return TRIBOOL_FALSE;
}
+
+/* Current C-c handler. */
+static c_c_handler_ftype *current_handler;
+
+/* The Windows callback that forwards requests to the C-c handler. */
+static BOOL WINAPI
+ctrl_c_handler (DWORD event_type)
+{
+ if (event_type == CTRL_BREAK_EVENT || event_type == CTRL_C_EVENT)
+ {
+ if (current_handler != SIG_IGN)
+ current_handler (SIGINT);
+ }
+ else
+ return FALSE;
+ return TRUE;
+}
+
+/* See inferior.h. */
+
+c_c_handler_ftype *
+install_sigint_handler (c_c_handler_ftype *fn)
+{
+ /* We want to make sure the gdb handler always comes first, so that
+ gdb gets to handle the C-c. This is why the handler is always
+ removed and reinstalled here. Note that trying to remove the
+ function without installing it first will cause a crash. */
+ static bool installed = false;
+ if (installed)
+ SetConsoleCtrlHandler (ctrl_c_handler, FALSE);
+ SetConsoleCtrlHandler (ctrl_c_handler, TRUE);
+ installed = true;
+
+ c_c_handler_ftype *result = current_handler;
+ current_handler = fn;
+ return result;
+}
diff --git a/gdb/posix-hdep.c b/gdb/posix-hdep.c
index 26211978d06..eddf73f38c9 100644
--- a/gdb/posix-hdep.c
+++ b/gdb/posix-hdep.c
@@ -21,6 +21,7 @@
#include "gdbsupport/event-loop.h"
#include "gdbsupport/gdb_select.h"
#include "inferior.h"
+#include <signal.h>
/* Wrapper for select. Nothing special needed on POSIX platforms. */
@@ -56,3 +57,26 @@ sharing_input_terminal (int pid)
return TRIBOOL_UNKNOWN;
#endif
}
+
+/* Current C-c handler. */
+static c_c_handler_ftype *current_handler;
+
+/* A wrapper that reinstalls the current signal handler. */
+static void
+handler_wrapper (int num)
+{
+ signal (num, handler_wrapper);
+ if (current_handler != SIG_IGN)
+ current_handler (num);
+}
+
+/* See inferior.h. */
+
+c_c_handler_ftype *
+install_sigint_handler (c_c_handler_ftype *fn)
+{
+ signal (SIGINT, handler_wrapper);
+ c_c_handler_ftype *result = current_handler;
+ current_handler = fn;
+ return result;
+}
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index b3329cd1a0d..dafda4781b9 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -298,6 +298,7 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
std::string pid_to_str (ptid_t) override;
void interrupt () override;
+ void pass_ctrlc () override;
const char *pid_to_exec_file (int pid) override;
@@ -1509,24 +1510,12 @@ windows_nat_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
windows_continue (continue_status, ptid.lwp (), 0);
}
-/* Ctrl-C handler used when the inferior is not run in the same console. The
- handler is in charge of interrupting the inferior using DebugBreakProcess.
- Note that this function is not available prior to Windows XP. In this case
- we emit a warning. */
-static BOOL WINAPI
-ctrl_c_handler (DWORD event_type)
-{
- const int attach_flag = current_inferior ()->attach_flag;
-
- /* Only handle Ctrl-C and Ctrl-Break events. Ignore others. */
- if (event_type != CTRL_C_EVENT && event_type != CTRL_BREAK_EVENT)
- return FALSE;
-
- /* If the inferior and the debugger share the same console, do nothing as
- the inferior has also received the Ctrl-C event. */
- if (!new_console && !attach_flag)
- return TRUE;
+/* Interrupt the inferior. */
+void
+windows_nat_target::interrupt ()
+{
+ DEBUG_EVENTS ("interrupt");
#ifdef __x86_64__
if (windows_process.wow64_process)
{
@@ -1548,19 +1537,24 @@ ctrl_c_handler (DWORD event_type)
windows_process.wow64_dbgbreak,
NULL, 0, NULL);
if (thread)
- CloseHandle (thread);
+ {
+ CloseHandle (thread);
+ return;
+ }
}
}
else
#endif
- {
- if (!DebugBreakProcess (windows_process.handle))
- warning (_("Could not interrupt program. "
- "Press Ctrl-c in the program console."));
- }
+ if (DebugBreakProcess (windows_process.handle))
+ return;
+ warning (_("Could not interrupt program. "
+ "Press Ctrl-c in the program console."));
+}
- /* Return true to tell that Ctrl-C has been handled. */
- return TRUE;
+void
+windows_nat_target::pass_ctrlc ()
+{
+ interrupt ();
}
/* Get the next event from the child. Returns the thread ptid. */
@@ -1840,35 +1834,7 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
while (1)
{
- /* If the user presses Ctrl-c while the debugger is waiting
- for an event, he expects the debugger to interrupt his program
- and to get the prompt back. There are two possible situations:
-
- - The debugger and the program do not share the console, in
- which case the Ctrl-c event only reached the debugger.
- In that case, the ctrl_c handler will take care of interrupting
- the inferior. Note that this case is working starting with
- Windows XP. For Windows 2000, Ctrl-C should be pressed in the
- inferior console.
-
- - The debugger and the program share the same console, in which
- case both debugger and inferior will receive the Ctrl-c event.
- In that case the ctrl_c handler will ignore the event, as the
- Ctrl-c event generated inside the inferior will trigger the
- expected debug event.
-
- FIXME: brobecker/2008-05-20: If the inferior receives the
- signal first and the delay until GDB receives that signal
- is sufficiently long, GDB can sometimes receive the SIGINT
- after we have unblocked the CTRL+C handler. This would
- lead to the debugger stopping prematurely while handling
- the new-thread event that comes with the handling of the SIGINT
- inside the inferior, and then stop again immediately when
- the user tries to resume the execution in the inferior.
- This is a classic race that we should try to fix one day. */
- SetConsoleCtrlHandler (&ctrl_c_handler, TRUE);
ptid_t result = get_windows_debug_event (pid, ourstatus, options);
- SetConsoleCtrlHandler (&ctrl_c_handler, FALSE);
if (result != null_ptid)
{
@@ -2868,17 +2834,6 @@ windows_nat_target::mourn_inferior ()
inf_child_target::mourn_inferior ();
}
-/* Send a SIGINT to the process group. This acts just like the user typed a
- ^C on the controlling terminal. */
-
-void
-windows_nat_target::interrupt ()
-{
- DEBUG_EVENTS ("GenerateConsoleCtrlEvent (CTRLC_EVENT, 0)");
- CHECK (GenerateConsoleCtrlEvent (CTRL_C_EVENT,
- windows_process.current_event.dwProcessId));
-}
-
/* Helper for windows_xfer_partial that handles memory transfers.
Arguments are like target_xfer_partial. */
--
2.34.3
next prev parent reply other threads:[~2022-12-05 18:57 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-05 18:56 [PATCH 0/3] Fix Windows C-c handling Tom Tromey
2022-12-05 18:56 ` [PATCH 1/3] Rename install_sigint_handler Tom Tromey
2022-12-05 18:56 ` [PATCH 2/3] Refactor code to check for terminal sharing Tom Tromey
2022-12-05 19:50 ` Eli Zaretskii
2022-12-12 17:27 ` Tom Tromey
2022-12-05 18:56 ` Tom Tromey [this message]
2022-12-07 17:13 ` [PATCH 3/3] Fix control-c handling on Windows Hannes Domani
2022-12-09 15:58 ` Tom Tromey
2022-12-09 16:19 ` Hannes Domani
2022-12-09 17:20 ` Tom Tromey
2022-12-09 18:13 ` Hannes Domani
2022-12-12 15:36 ` Tom Tromey
2022-12-12 17:05 ` Tom Tromey
2022-12-13 11:30 ` Hannes Domani
2022-12-13 19:51 ` Tom Tromey
2022-12-05 18:59 ` [PATCH 0/3] Fix Windows C-c handling Tom Tromey
2022-12-12 13:23 ` Jon Turney
2022-12-12 17:29 ` Tom Tromey
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=20221205185651.2704492-4-tromey@adacore.com \
--to=tromey@adacore.com \
--cc=gdb-patches@sourceware.org \
/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).