[RFC,2/2] Change SIGINT handler for extension languages only when target terminal is ours
Commit Message
Pedro Alves <palves@redhat.com> writes:
> The reason this doesn't trigger with native debugging is that
> in that case, gdb is sharing the terminal with gdb, and with job
I think you meant "gdb is sharing the terminal with the inferior".
> control, then the ctrl-c always reaches the inferior instead.
>
> We can trigger the issue with native debugging as well, if
> we "attach" instead of "run", like in the patch at the bottom:
>
> $ (set -e; while true; do make check RUNTESTFLAGS="random-signal.exp"; done)
> ...
> FAIL: gdb.base/random-signal.exp: attach: stop with control-c (timeout)
> FAIL: gdb.base/random-signal.exp: attach: p wait_gdb = 0 (timeout)
> ...
>
> gdb.log:
>
> (gdb) PASS: gdb.base/random-signal.exp: attach: watch v
> continue
> Continuing.
> PASS: gdb.base/random-signal.exp: attach: continue
> ^CPython Exception <type 'exceptions.KeyboardInterrupt'> <type
> exceptions.KeyboardInterrupt'>:
> FAIL: gdb.base/random-signal.exp: attach: stop with control-c (timeout)
> p wait_gdb = 0
> FAIL: gdb.base/random-signal.exp: attach: p wait_gdb = 0 (timeout)
>
>
Yes, it is a good test case.
>> + if (!target_terminal_is_inferior ())
>
> I think this should check for terminal_is_ours instead. We also
> don't want this with terminal_ours_for_output. In that case, _input_
> is still sent to the inferior.
OK, target_terminal_is_ours is added and is used here. Patch is updated
as below,
>
> Here's the test tweak:
>
> From 2cf334c489d6278ea73e1f82905b9726f7d502fc Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 4 Jan 2016 14:25:31 +0000
> Subject: [PATCH] Test gdb.base/random-signal.exp with "attach"
>
> This exposes the issued fixed by:
> https://sourceware.org/ml/gdb-patches/2015-12/msg00423.html
> to native debugging as well.
>
> gdb/testsuite/ChangeLog:
> 2016-01-04 Pedro Alves <palves@redhat.com>
>
> * gdb.base/random-signal.c (wait): New global.
> (main): Use it.
> * gdb.base/random-signal.exp (do_test): New procedure, with body
> of testcase moved in.
> (top level) Call it twice, once with "run" and once with
> "attach".
Your patch looks good to me.
Comments
On 01/05/2016 04:52 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> The reason this doesn't trigger with native debugging is that
>> in that case, gdb is sharing the terminal with gdb, and with job
>
> I think you meant "gdb is sharing the terminal with the inferior".
Indeed.
>>> + if (!target_terminal_is_inferior ())
>>
>> I think this should check for terminal_is_ours instead. We also
>> don't want this with terminal_ours_for_output. In that case, _input_
>> is still sent to the inferior.
>
> OK, target_terminal_is_ours is added and is used here. Patch is updated
> as below,
This LGTM.
> Your patch looks good to me.
I'll push it after yours goes in.
Thanks,
Pedro Alves
Pedro Alves <palves@redhat.com> writes:
> This LGTM.
>
Patch is pushed in.
>> Your patch looks good to me.
>
> I'll push it after yours goes in.
Please do that. Thanks for new test.
On 01/08/2016 11:08 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>>> Your patch looks good to me.
>>
>> I'll push it after yours goes in.
>
> Please do that. Thanks for new test.
>
Done.
Thanks,
Pedro Alves
@@ -22,6 +22,7 @@
#include "defs.h"
#include <signal.h>
+#include "target.h"
#include "auto-load.h"
#include "breakpoint.h"
#include "event-top.h"
@@ -746,19 +747,24 @@ set_active_ext_lang (const struct extension_language_defn *now_active)
= XCNEW (struct active_ext_lang_state);
previous->ext_lang = active_ext_lang;
+ previous->sigint_handler.handler_saved = 0;
active_ext_lang = now_active;
- /* If the newly active extension language uses cooperative SIGINT handling
- then ensure GDB's SIGINT handler is installed. */
- if (now_active->language == EXT_LANG_GDB
- || now_active->ops->check_quit_flag != NULL)
- install_gdb_sigint_handler (&previous->sigint_handler);
-
- /* If there's a SIGINT recorded in the cooperative extension languages,
- move it to the new language, or save it in GDB's global flag if the newly
- active extension language doesn't use cooperative SIGINT handling. */
- if (check_quit_flag ())
- set_quit_flag ();
+ if (target_terminal_is_ours ())
+ {
+ /* If the newly active extension language uses cooperative SIGINT
+ handling then ensure GDB's SIGINT handler is installed. */
+ if (now_active->language == EXT_LANG_GDB
+ || now_active->ops->check_quit_flag != NULL)
+ install_gdb_sigint_handler (&previous->sigint_handler);
+
+ /* If there's a SIGINT recorded in the cooperative extension languages,
+ move it to the new language, or save it in GDB's global flag if the
+ newly active extension language doesn't use cooperative SIGINT
+ handling. */
+ if (check_quit_flag ())
+ set_quit_flag ();
+ }
return previous;
}
@@ -772,16 +778,19 @@ restore_active_ext_lang (struct active_ext_lang_state *previous)
active_ext_lang = previous->ext_lang;
- /* Restore the previous SIGINT handler if one was saved. */
- if (previous->sigint_handler.handler_saved)
- install_sigint_handler (&previous->sigint_handler);
-
- /* If there's a SIGINT recorded in the cooperative extension languages,
- move it to the new language, or save it in GDB's global flag if the newly
- active extension language doesn't use cooperative SIGINT handling. */
- if (check_quit_flag ())
- set_quit_flag ();
-
+ if (target_terminal_is_ours ())
+ {
+ /* Restore the previous SIGINT handler if one was saved. */
+ if (previous->sigint_handler.handler_saved)
+ install_sigint_handler (&previous->sigint_handler);
+
+ /* If there's a SIGINT recorded in the cooperative extension languages,
+ move it to the new language, or save it in GDB's global flag if the
+ newly active extension language doesn't use cooperative SIGINT
+ handling. */
+ if (check_quit_flag ())
+ set_quit_flag ();
+ }
xfree (previous);
}
@@ -466,6 +466,14 @@ target_terminal_is_inferior (void)
/* See target.h. */
+int
+target_terminal_is_ours (void)
+{
+ return (terminal_state == terminal_is_ours);
+}
+
+/* See target.h. */
+
void
target_terminal_inferior (void)
{
@@ -1503,6 +1503,10 @@ extern int target_remove_breakpoint (struct gdbarch *gdbarch,
extern int target_terminal_is_inferior (void);
+/* Returns true if our terminal settings are in effect. */
+
+extern int target_terminal_is_ours (void);
+
/* Initialize the terminal settings we record for the inferior,
before we actually run the inferior. */