diff options
author | Pedro Alves <pedro@palves.net> | 2023-02-09 13:46:28 +0000 |
---|---|---|
committer | Pedro Alves <pedro@palves.net> | 2023-02-15 20:58:10 +0000 |
commit | 0ace6ace1bfc2982f62ec684cdb26de64aec3366 (patch) | |
tree | 027a062d7a87ca8eb385d0ab559e931abaf1c6e7 /gdb/infrun.c | |
parent | 91265a7d7cddc10314335ffcfbfae7159c7cecb1 (diff) | |
download | gdb-0ace6ace1bfc2982f62ec684cdb26de64aec3366.zip gdb-0ace6ace1bfc2982f62ec684cdb26de64aec3366.tar.gz gdb-0ace6ace1bfc2982f62ec684cdb26de64aec3366.tar.bz2 |
Don't throw quit while handling inferior events
This implements what I suggested here:
https://inbox.sourceware.org/gdb-patches/ab97c553-f406-b094-cdf3-ba031fdea925@palves.net/
Here is the current default quit_handler, a function that ends up
called by the QUIT macro:
void
default_quit_handler (void)
{
if (check_quit_flag ())
{
if (target_terminal::is_ours ())
quit ();
else
target_pass_ctrlc ();
}
}
As we can see above, when the inferior is running in the foreground,
then a Ctrl-C is translated into a call to target_pass_ctrlc().
The target_terminal::is_ours() case above is there to handle the
scenario where GDB has the terminal, meaning it is handling some
command the user typed, like "list", or "p a + b" or some such.
However, when the inferior is running on the background, say with
"c&", GDB also has the terminal. Run control handling is now done in
the "background". The CLI is responsive to user commands. If users
type Ctrl-C, they're expecting it to interrupt whatever command they
next type in the CLI, which again, could be "list", "p a + b", etc.
It's as if background run control was handled by a separate thread,
and the Ctrl-C is meant to go to the main thread, handling the CLI.
However, when handling an event, inside fetch_inferior_event &
friends, a Ctrl-C _also_ results in a Quit exception, from the same
default_quit_handler function shown above. This quit aborts run
control handling, breakpoint condition evaluation, etc., and may even
leave run control in an inconsistent state.
The testcase added by this patch illustrates this. The test program
just loops a number of times calling the "foo" function.
The idea is to set a breakpoint in the "foo" function with a condition
that sends SIGINT to GDB, and then evaluates to false, which results
in the program being re-resumed in the background. The SIGINT-sending
emulates pressing Ctrl-C just while GDB was evaluating the breakpoint
condition, except, it's more deterministic.
It looks like this:
(gdb) p $counter = 0
$1 = 0
(gdb) b foo if $counter++ == 10 || $_shell("kill -SIGINT `pidof gdb`") != 0
Breakpoint 2 at 0x555555555131: file gdb.base/bg-exec-sigint-bp-cond.c, line 21.
(gdb) c&
Continuing.
(gdb)
After that background continue, the breakpoint should be hit 10 times,
and we should see 10 "Quit" being printed on the screen. As if the
user typed Ctrl-C on the prompt a number of times with no inferior
running:
(gdb) <<< Ctrl-C
(gdb) Quit <<< Ctrl-C
(gdb) Quit <<< Ctrl-C
(gdb)
However, here's what you see instead:
(gdb) c&
Continuing.
(gdb) Quit
(gdb)
Just one Quit, and nothing else. If we look at the thread's state, we see:
(gdb) info threads
Id Target Id Frame
* 1 Thread 0x7ffff7d6f740 (LWP 112192) "bg-exec-sigint-" foo () at gdb.base/bg-exec-sigint-bp-cond.c:21
So the thread stopped, but we didn't report a stop...
Issuing another continue shows the same immediate-and-silent-stop:
(gdb) c&
Continuing.
(gdb) Quit
(gdb) p $counter
$2 = 2
As mentioned, since the run control handling, and breakpoint and
watchpoint evaluation, etc. are running in the background from the
perspective of the CLI, when users type Ctrl-C in this situation,
they're thinking of aborting whatever other command they were typing
or running at the prompt, not the run control side, not the previous
"c&" command.
So I think that we should install a custom quit_handler while inside
fetch_inferior_event, where we already disable pagination and other
things for a similar reason. This custom quit handler does nothing if
GDB has the terminal, and forwards Ctrl-C to the inferior otherwise.
With the patch implementing that, and the same testcase, here's what
you see instead:
(gdb) p $counter = 0
$1 = 0
(gdb) b foo if $counter++ == 10 || $_shell("kill -SIGINT `pidof gdb`") != 0
Breakpoint 2 at 0x555555555131: file gdb.base/bg-exec-sigint-bp-cond.c, line 21.
(gdb) c&
Continuing.
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb) Quit
(gdb)
Breakpoint 2, foo () at gdb.base/bg-exec-sigint-bp-cond.c:21
21 return 0;
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I1f10d99496a7d67c94b258e45963e83e439e1778
Diffstat (limited to 'gdb/infrun.c')
-rw-r--r-- | gdb/infrun.c | 45 |
1 files changed, 45 insertions, 0 deletions
diff --git a/gdb/infrun.c b/gdb/infrun.c index e5d2b97..0d7e386 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4105,6 +4105,44 @@ all_uis_on_sync_execution_starting (void) } } +/* A quit_handler callback installed while we're handling inferior + events. */ + +static void +infrun_quit_handler () +{ + if (target_terminal::is_ours ()) + { + /* Do nothing. + + default_quit_handler would throw a quit in this case, but if + we're handling an event while we have the terminal, it means + the target is running a background execution command, and + thus when users press Ctrl-C, they're wanting to interrupt + whatever command they were executing in the command line. + E.g.: + + (gdb) c& + (gdb) foo bar whatever<ctrl-c> + + That Ctrl-C should clear the input line, not interrupt event + handling if it happens that the user types Ctrl-C at just the + "wrong" time! + + It's as-if background event handling was handled by a + separate background thread. + + To be clear, the Ctrl-C is not lost -- it will be processed + by the next QUIT call once we're out of fetch_inferior_event + again. */ + } + else + { + if (check_quit_flag ()) + target_pass_ctrlc (); + } +} + /* Asynchronous version of wait_for_inferior. It is called by the event loop whenever a change of state is detected on the file descriptor corresponding to the target. It can be called more than @@ -4133,6 +4171,13 @@ fetch_inferior_event () scoped_restore save_pagination = make_scoped_restore (&pagination_enabled, false); + /* Install a quit handler that does nothing if we have the terminal + (meaning the target is running a background execution command), + so that Ctrl-C never interrupts GDB before the event is fully + handled. */ + scoped_restore restore_quit_handler + = make_scoped_restore (&quit_handler, infrun_quit_handler); + /* End up with readline processing input, if necessary. */ { SCOPE_EXIT { reinstall_readline_callback_handler_cleanup (); }; |