[3/3] Fix control-c handling on Windows
Commit Message
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(-)
Comments
Am Montag, 5. Dezember 2022, 19:57:36 MEZ hat Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:
> 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.
I've now tested this a bit, it's a big improvement.
Now it even works to interrupt a GUI program that was started with
'new-console off', that didn't work before.
But I did notice a few problems:
1)
When I first started a program with 'new-console on', then the
sigint_ours variable would not be initialized, and I would later get
a crash on C-c.
I've fixed it like this:
diff --git a/gdb/inflow.c b/gdb/inflow.c
index f9926122099..50c93b6e15a 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -165,7 +165,7 @@ static struct terminal_info *get_inflow_inferior_data (struct inferior *);
save our handlers in these two variables and set SIGINT and SIGQUIT
to SIG_IGN. */
-static sighandler_t sigint_ours;
+static sighandler_t sigint_ours = SIG_IGN;
#ifdef SIGQUIT
static sighandler_t sigquit_ours;
#endif
@@ -805,7 +805,8 @@ child_terminal_ours_1 (target_terminal_state desired_state)
}
}
- if (!job_control && desired_state == target_terminal_state::is_ours)
+ if (!job_control && desired_state == target_terminal_state::is_ours
+ && sigint_ours != SIG_IGN)
{
install_sigint_handler (sigint_ours);
#ifdef SIGQUIT
Not sure if there is a better solution.
2)
And for some reason one of my builds (x86_64+TUI+python) needed the
#include <signal.h> in mingw-hdep.c, but my other (i686 basic) didn't.
3)
But my basic i686 build had another problem when starting a program with
'new-console on', because at program start it called install_sigint_handler(),
but rl_set_signals() would later override the SIGINT handler again, so C-c
didn't work work in this situation.
With 'new-console off' this didn't happen, since install_sigint_handler() was
again called later since it shared the console.
That's all I've seen so far, thanks for this.
Hannes
>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
Hannes> I've now tested this a bit, it's a big improvement.
Thanks for trying it. I appreciate that.
Hannes> When I first started a program with 'new-console on', then the
Hannes> sigint_ours variable would not be initialized, and I would later get
Hannes> a crash on C-c.
I looked at this and my feeling is that this is a latent bug.
I think what's going on is that sigint_ours is set under different
conditions than it is used.
That is, it is set like:
if (gdb_has_a_terminal ()
&& tinfo->ttystate != NULL
&& sharing_input_terminal (inf))
[...]
if (!job_control)
{
sigint_ours = install_sigint_handler (SIG_IGN);
[...]
gdb_tty_state = target_terminal_state::is_inferior;
However later it is used:
if (gdb_tty_state != desired_state)
[...]
if (!job_control && desired_state == target_terminal_state::is_ours)
{
install_sigint_handler (sigint_ours);
It's maybe hard to reason about but it seems to me that there's some
possibility for the value to be used even though it hasn't been set, and
I suspect that is what you are seeing.
It might be useful if you could confirm this. Just some simple logging
at these two points would be sufficient.
If that's the issue then I can write a patch to change sigint_ours to be
a gdb::optional and check it that way.
Hannes> And for some reason one of my builds (x86_64+TUI+python) needed the
Hannes> #include <signal.h> in mingw-hdep.c, but my other (i686 basic) didn't.
I didn't see this but I went ahead and added the include to my patch,
since it seems harmless.
Hannes> But my basic i686 build had another problem when starting a program with
Hannes> 'new-console on', because at program start it called install_sigint_handler(),
Hannes> but rl_set_signals() would later override the SIGINT handler again, so C-c
Hannes> didn't work work in this situation.
Hannes> With 'new-console off' this didn't happen, since install_sigint_handler() was
Hannes> again called later since it shared the console.
I really don't understand the interaction between signal and
SetConsoleCtrlHandler. I tried searching for some docs on this but
didn't find anything that was really enlightening.
Also it's somewhat surprising that the x86 and x86-64 builds would be
different in this regard.
Anyway ... I'm not sure what to do here yet. The interactions with
readline are pretty hard to understand. I guess the question is where
should install_sigint_handler be called where it is not called -- that
is, to reset the signals from readline. Alternatively, where is it
called on x86-64 but not on x86?
I did try 'new-console on' with my (x86-64) build, and that worked fine.
I can try an x86 build and see if that works any better.
Tom
Am Freitag, 9. Dezember 2022, 16:58:12 MEZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben:
> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
>
> Hannes> I've now tested this a bit, it's a big improvement.
>
> Thanks for trying it. I appreciate that.
>
> Hannes> When I first started a program with 'new-console on', then the
> Hannes> sigint_ours variable would not be initialized, and I would later get
> Hannes> a crash on C-c.
>
> I looked at this and my feeling is that this is a latent bug.
>
> I think what's going on is that sigint_ours is set under different
> conditions than it is used.
>
> That is, it is set like:
>
> if (gdb_has_a_terminal ()
> && tinfo->ttystate != NULL
> && sharing_input_terminal (inf))
> [...]
> if (!job_control)
> {
> sigint_ours = install_sigint_handler (SIG_IGN);
> [...]
> gdb_tty_state = target_terminal_state::is_inferior;
>
>
> However later it is used:
>
> if (gdb_tty_state != desired_state)
> [...]
> if (!job_control && desired_state == target_terminal_state::is_ours)
> {
> install_sigint_handler (sigint_ours);
>
> It's maybe hard to reason about but it seems to me that there's some
> possibility for the value to be used even though it hasn't been set, and
> I suspect that is what you are seeing.
>
> It might be useful if you could confirm this. Just some simple logging
> at these two points would be sufficient.
>
> If that's the issue then I can write a patch to change sigint_ours to be
> a gdb::optional and check it that way.
I should have been more clear about this.
I started with 'new-console on', so sharing_input_terminal() returned false,
and that's why sigint_ours was not set.
So yes, gdb::optional would probably fix this.
I just wonder if this is never an issue on Linux, e.g. if you attach,
of does signal() maybe ignore NULL-pointer functions?
> Hannes> And for some reason one of my builds (x86_64+TUI+python) needed the
> Hannes> #include <signal.h> in mingw-hdep.c, but my other (i686 basic) didn't.
>
> I didn't see this but I went ahead and added the include to my patch,
> since it seems harmless.
Great, thanks.
> Hannes> But my basic i686 build had another problem when starting a program with
> Hannes> 'new-console on', because at program start it called install_sigint_handler(),
> Hannes> but rl_set_signals() would later override the SIGINT handler again, so C-c
> Hannes> didn't work work in this situation.
> Hannes> With 'new-console off' this didn't happen, since install_sigint_handler() was
> Hannes> again called later since it shared the console.
>
> I really don't understand the interaction between signal and
> SetConsoleCtrlHandler. I tried searching for some docs on this but
> didn't find anything that was really enlightening.
As far as I could tell, signal() calls SetConsoleCtrlHandler(), probably
similar to how you handled this.
> Also it's somewhat surprising that the x86 and x86-64 builds would be
> different in this regard.
>
> Anyway ... I'm not sure what to do here yet. The interactions with
> readline are pretty hard to understand. I guess the question is where
> should install_sigint_handler be called where it is not called -- that
> is, to reset the signals from readline. Alternatively, where is it
> called on x86-64 but not on x86?
>
> I did try 'new-console on' with my (x86-64) build, and that worked fine.
> I can try an x86 build and see if that works any better.
Again, I wasn't clear enough here.
The difference is not because of i686 and x86_64, but that the x86_64 build
has TUI+python enabled, but my i686 build has not.
Some TUI startup code would later call install_sigint_handler() again,
overriding the SIGINT handler again, and everything is fine.
Sorry that I wasn't clear enough before.
Hannes
>> If that's the issue then I can write a patch to change sigint_ours to be
>> a gdb::optional and check it that way.
Hannes> I should have been more clear about this.
Hannes> I started with 'new-console on', so sharing_input_terminal() returned false,
Hannes> and that's why sigint_ours was not set.
Hannes> So yes, gdb::optional would probably fix this.
Hannes> I just wonder if this is never an issue on Linux, e.g. if you attach,
Hannes> of does signal() maybe ignore NULL-pointer functions?
Normally SIG_DFL is NULL, but also on Linux I couldn't get this to
trigger inappropriately. Maybe my theory about what's going on here is
incorrect.
Hannes> As far as I could tell, signal() calls SetConsoleCtrlHandler(), probably
Hannes> similar to how you handled this.
Yeah, that was my guess as well, but really we'd want more details.
Like, does calling signal reinstall the SetConsoleCtrlHandler? If so
then why didn't that work for gdb? But if not then why did we need to
call SetConsoleCtrlHandler again to tweak the ordering of callbacks?
Maybe I should go back and try to figure out what else is calling signal
and/or SetConsoleCtrlHandler. I somewhat suspect Python but I don't
really know.
>> I did try 'new-console on' with my (x86-64) build, and that worked fine.
>> I can try an x86 build and see if that works any better.
Hannes> Again, I wasn't clear enough here.
Hannes> The difference is not because of i686 and x86_64, but that the x86_64 build
Hannes> has TUI+python enabled, but my i686 build has not.
Aha, I see, thanks.
Hannes> Some TUI startup code would later call install_sigint_handler() again,
Hannes> overriding the SIGINT handler again, and everything is fine.
Do you know where this happens?
Anyway I am wondering if we can have gdb_rl_deprep_term_function call
rl_clear_signals and then reinstall the gdb signal handlers. This idea
makes me wonder if we even need SetConsoleCtrlHandler at all -- maybe gdb
could just use signal after all.
Tom
Am Freitag, 9. Dezember 2022, 18:21:27 MEZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben:
> >> If that's the issue then I can write a patch to change sigint_ours to be
> >> a gdb::optional and check it that way.
>
> Hannes> I should have been more clear about this.
> Hannes> I started with 'new-console on', so sharing_input_terminal() returned false,
> Hannes> and that's why sigint_ours was not set.
>
> Hannes> So yes, gdb::optional would probably fix this.
> Hannes> I just wonder if this is never an issue on Linux, e.g. if you attach,
> Hannes> of does signal() maybe ignore NULL-pointer functions?
>
> Normally SIG_DFL is NULL, but also on Linux I couldn't get this to
> trigger inappropriately. Maybe my theory about what's going on here is
> incorrect.
>
> Hannes> As far as I could tell, signal() calls SetConsoleCtrlHandler(), probably
> Hannes> similar to how you handled this.
>
> Yeah, that was my guess as well, but really we'd want more details.
> Like, does calling signal reinstall the SetConsoleCtrlHandler? If so
> then why didn't that work for gdb? But if not then why did we need to
> call SetConsoleCtrlHandler again to tweak the ordering of callbacks?
>
> Maybe I should go back and try to figure out what else is calling signal
> and/or SetConsoleCtrlHandler. I somewhat suspect Python but I don't
> really know.
I tried just now to debug signal a bit more, but now I can't reproduce
it calling SetConsoleCtrlHandler any more.
The more I look, the more confused I get again.
> >> I did try 'new-console on' with my (x86-64) build, and that worked fine.
> >> I can try an x86 build and see if that works any better.
>
> Hannes> Again, I wasn't clear enough here.
> Hannes> The difference is not because of i686 and x86_64, but that the x86_64 build
> Hannes> has TUI+python enabled, but my i686 build has not.
>
> Aha, I see, thanks.
>
> Hannes> Some TUI startup code would later call install_sigint_handler() again,
> Hannes> overriding the SIGINT handler again, and everything is fine.
>
> Do you know where this happens?
Yes, it's because of my extra python TUI windows, which call
gdbpy_register_tui_window:
#0 install_sigint_handler (fn=fn@entry=0x13fde3a50 <handle_sigint(int)>) at C:/src/repos/binutils-gdb.git/gdb/mingw-hdep.c:426
#1 0x000000013fde9cdf in install_gdb_sigint_handler (previous=0x27b88a8) at C:/src/repos/binutils-gdb.git/gdb/extension.c:679
#2 set_active_ext_lang (now_active=now_active@entry=0x1407aee00 <extension_language_python>) at C:/src/repos/binutils-gdb.git/gdb/extension.c:736
#3 0x000000013ff4503e in gdbpy_enter::gdbpy_enter (this=this@entry=0x24c6c0, gdbarch=gdbarch@entry=0x0, language=language@entry=0x0) at C:/src/repos/binutils-gdb.git/gdb/python/python.c:212
#4 0x000000013ff35ac0 in gdbpy_tui_window_maker::gdbpy_tui_window_maker (other=..., this=0x24c6b8) at C:/src/repos/binutils-gdb.git/gdb/python/py-tui.c:301
#5 gdbpy_register_tui_window (self=<optimized out>, args=<optimized out>, kw=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/python/py-tui.c:380
...
And also there is this call later on:
#0 install_sigint_handler (fn=fn@entry=0x13fde3a50 <handle_sigint(int)>) at C:/src/repos/binutils-gdb.git/gdb/mingw-hdep.c:426
#1 0x000000013fde9cdf in install_gdb_sigint_handler (previous=0x28ade48) at C:/src/repos/binutils-gdb.git/gdb/extension.c:679
#2 set_active_ext_lang (now_active=now_active@entry=0x1407aee00 <extension_language_python>) at C:/src/repos/binutils-gdb.git/gdb/extension.c:736
#3 0x000000013ff4503e in gdbpy_enter::gdbpy_enter (this=0x24f7b0, gdbarch=0x0, language=0x0) at C:/src/repos/binutils-gdb.git/gdb/python/python.c:212
#4 0x000000013ff4522a in gdbpy_before_prompt_hook (extlang=<optimized out>, current_gdb_prompt=0x140be96b0 <top_prompt+16> "(gdb) ") at C:/src/repos/binutils-gdb.git/gdb/python/python.c:1114
#5 0x000000013fde8fc3 in ext_lang_before_prompt (current_gdb_prompt=0x140be96b0 <top_prompt+16> "(gdb) ") at C:/src/repos/binutils-gdb.git/gdb/extension.c:963
#6 0x000000013fde4431 in std::function<void (char const*)>::operator()(char const*) const (__args#0=0x140be96b0 <top_prompt+16> "(gdb) ", this=0x4dee28) at c:/msys64/mingw64/x86_64-w64-mingw32/include/c++/11.2.0/bits/std_function.h:560
#7 gdb::observers::observable<char const*>::notify (args#0=0x140be96b0 <top_prompt+16> "(gdb) ", this=<optimized out>) at c:/src/repos/binutils-gdb.git/gdbsupport/observable.h:166
#8 top_level_prompt () at C:/src/repos/binutils-gdb.git/gdb/event-top.c:461
#9 display_gdb_prompt (new_prompt=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/event-top.c:428
#10 0x000000013fea4115 in captured_command_loop () at C:/src/repos/binutils-gdb.git/gdb/main.c:472
#11 0x000000013fea5de5 in captured_main (data=0x24fa70) at C:/src/repos/binutils-gdb.git/gdb/main.c:1341
#12 gdb_main (args=args@entry=0x24fad0) at C:/src/repos/binutils-gdb.git/gdb/main.c:1356
#13 0x0000000140692e27 in main (argc=2, argv=0x5c4cc0) at C:/src/repos/binutils-gdb.git/gdb/gdb.c:32
> Anyway I am wondering if we can have gdb_rl_deprep_term_function call
> rl_clear_signals and then reinstall the gdb signal handlers. This idea
> makes me wonder if we even need SetConsoleCtrlHandler at all -- maybe gdb
> could just use signal after all.
Good question, maybe it doesn't handle C-break as well?
Hannes
Hannes> Again, I wasn't clear enough here.
Hannes> The difference is not because of i686 and x86_64, but that the x86_64 build
Hannes> has TUI+python enabled, but my i686 build has not.
>> Aha, I see, thanks.
Today I built an x86-64 Windows gdb, but I disabled Python. I already
had the TUI disabled for Windows.
I used RDP and started powershell, then ran gdb there. I did 6 tests: a
C-c and a C-break test of "run", "run" with "set new-console 1", and
"attach" -- and in all these cases, it worked.
So now I'm wondering again what the difference could be between our
situations.
>> Anyway I am wondering if we can have gdb_rl_deprep_term_function call
>> rl_clear_signals and then reinstall the gdb signal handlers. This idea
>> makes me wonder if we even need SetConsoleCtrlHandler at all -- maybe gdb
>> could just use signal after all.
Hannes> Good question, maybe it doesn't handle C-break as well?
C-break can work with signal by installing a handler for SIGBREAK. This
worked fine when I tried it, but the issue was still that the SIGINT
handler somehow stopped working.
Tom
Tom> I used RDP and started powershell, then ran gdb there. I did 6 tests: a
Tom> C-c and a C-break test of "run", "run" with "set new-console 1", and
Tom> "attach" -- and in all these cases, it worked.
I found out how to reproduce the crash you saw. In my tests, I was
doing everything in a single gdb instance. However, if I start a new
gdb, then "set new-console 1", and then "run", I can make it crash.
Changing sigint_ours to a gdb::optional fixes this problem.
I couldn't reproduce your problem #3 ... my python-less build seems to
work fine in other situations.
I'll send v2 of this series shortly.
Tom
Am Montag, 12. Dezember 2022 um 18:06:23 MEZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben:
> Tom> I used RDP and started powershell, then ran gdb there. I did 6 tests: a
> Tom> C-c and a C-break test of "run", "run" with "set new-console 1", and
> Tom> "attach" -- and in all these cases, it worked.
>
> I found out how to reproduce the crash you saw. In my tests, I was
> doing everything in a single gdb instance. However, if I start a new
> gdb, then "set new-console 1", and then "run", I can make it crash.
>
> Changing sigint_ours to a gdb::optional fixes this problem.
>
> I couldn't reproduce your problem #3 ... my python-less build seems to
> work fine in other situations.
>
> I'll send v2 of this series shortly.
I also can't reproduce problem #3 any longer with the v2 series.
So I think that this was actually the same problem that you fixed with
gdb::optional, and it was just that the different signal() calls confused me.
So, I also have no problems any more with v2 applied.
Thanks, Hannes
Hannes> I also can't reproduce problem #3 any longer with the v2 series.
Hannes> So I think that this was actually the same problem that you fixed with
Hannes> gdb::optional, and it was just that the different signal() calls confused me.
Hannes> So, I also have no problems any more with v2 applied.
Thanks. I'm going to check it in now.
Tom
@@ -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");
@@ -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
@@ -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);
@@ -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;
}
}
@@ -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;
+}
@@ -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;
+}
@@ -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. */