From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1551) id 6A870385B506; Wed, 15 Feb 2023 21:01:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6A870385B506 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1676494908; bh=wG6ctt0zwNbrmw6aJ0vv4BaoTBgHRPu+It1iWWpE8YY=; h=From:To:Subject:Date:From; b=RODEmrrzNuGRcE1sCel9Y/EoIqMJniNCNs1TaFVvrG+zCC3Af/safW0SsYu1IJPdx EeA5T6EwhYgF6oOPJEY8E6S7oZwsSbwFXaJ+by9WY2ryKH66/uDKgYZ22wSrgvYLwt v0CoUneGSlqkNt6WtWDCNKY79vC2NG5DKRA3EfEs= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Pedro Alves To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Don't throw quit while handling inferior events X-Act-Checkin: binutils-gdb X-Git-Author: Pedro Alves X-Git-Refname: refs/heads/master X-Git-Oldrev: 91265a7d7cddc10314335ffcfbfae7159c7cecb1 X-Git-Newrev: 0ace6ace1bfc2982f62ec684cdb26de64aec3366 Message-Id: <20230215210148.6A870385B506@sourceware.org> Date: Wed, 15 Feb 2023 21:01:48 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D0ace6ace1bfc= 2982f62ec684cdb26de64aec3366 commit 0ace6ace1bfc2982f62ec684cdb26de64aec3366 Author: Pedro Alves Date: Thu Feb 9 13:46:28 2023 +0000 Don't throw quit while handling inferior events =20 This implements what I suggested here: =20 https://inbox.sourceware.org/gdb-patches/ab97c553-f406-b094-cdf3-ba031= fdea925@palves.net/ =20 Here is the current default quit_handler, a function that ends up called by the QUIT macro: =20 void default_quit_handler (void) { if (check_quit_flag ()) { if (target_terminal::is_ours ()) quit (); else target_pass_ctrlc (); } } =20 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(). =20 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. =20 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. =20 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. =20 The testcase added by this patch illustrates this. The test program just loops a number of times calling the "foo" function. =20 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. =20 It looks like this: =20 (gdb) p $counter =3D 0 $1 =3D 0 (gdb) b foo if $counter++ =3D=3D 10 || $_shell("kill -SIGINT `pidof g= db`") !=3D 0 Breakpoint 2 at 0x555555555131: file gdb.base/bg-exec-sigint-bp-cond.= c, line 21. (gdb) c& Continuing. (gdb) =20 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: =20 (gdb) <<< Ctrl-C (gdb) Quit <<< Ctrl-C (gdb) Quit <<< Ctrl-C (gdb) =20 However, here's what you see instead: =20 (gdb) c& Continuing. (gdb) Quit (gdb) =20 Just one Quit, and nothing else. If we look at the thread's state, we = see: =20 (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 =20 So the thread stopped, but we didn't report a stop... =20 Issuing another continue shows the same immediate-and-silent-stop: =20 (gdb) c& Continuing. (gdb) Quit (gdb) p $counter $2 =3D 2 =20 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. =20 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. =20 With the patch implementing that, and the same testcase, here's what you see instead: =20 (gdb) p $counter =3D 0 $1 =3D 0 (gdb) b foo if $counter++ =3D=3D 10 || $_shell("kill -SIGINT `pidof gd= b`") !=3D 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; =20 Approved-By: Tom Tromey Change-Id: I1f10d99496a7d67c94b258e45963e83e439e1778 Diff: --- gdb/infrun.c | 45 +++++++++++++ gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c | 33 ++++++++++ gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp | 77 +++++++++++++++++++= ++++ 3 files changed, 155 insertions(+) diff --git a/gdb/infrun.c b/gdb/infrun.c index e5d2b97f1dd..0d7e386dc86 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4105,6 +4105,44 @@ all_uis_on_sync_execution_starting (void) } } =20 +/* 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 + + 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 =3D make_scoped_restore (&pagination_enabled, false); =20 + /* 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 + =3D make_scoped_restore (&quit_handler, infrun_quit_handler); + /* End up with readline processing input, if necessary. */ { SCOPE_EXIT { reinstall_readline_callback_handler_cleanup (); }; diff --git a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c b/gdb/testsuit= e/gdb.base/bg-exec-sigint-bp-cond.c new file mode 100644 index 00000000000..b1cf35361f8 --- /dev/null +++ b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2023 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . = */ + +int +foo (void) +{ + return 0; +} + +int +main (void) +{ + int i; + + for (i =3D 0; i < 30; i++) + foo (); + + return 0; +} diff --git a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp b/gdb/testsu= ite/gdb.base/bg-exec-sigint-bp-cond.exp new file mode 100644 index 00000000000..257efb337f9 --- /dev/null +++ b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp @@ -0,0 +1,77 @@ +# Copyright 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Check that sending GDB a SIGINT while handling execution control +# does not interrupt the execution control. + +standard_testfile + +if {[build_executable "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +# Run the test. Sets a breakpoint with a condition that sends a +# SIGINT to GDB, and ensures that that doesn't make the breakpoint hit +# cause a premature stop. This emulates pressing Ctrl-C just while +# GDB is evaluating the breakpoint condition. +proc test {} { + clean_restart $::binfile + + if {![runto_main]} { + return + } + + delete_breakpoints + + set gdb_pid [exp_pid -i [board_info host fileid]] + + # Number of times the breakpoint should be hit before stopping. + set num_hits 10 + + # A counter used in the breakpoint's condition to ensure that it + # causes a stop after NUM_HITS hits. + gdb_test "p \$hit_count =3D 0" " =3D 0" "reset hit counter" + + # Set a breakpoint with a condition that sends a SIGINT to GDB. This + # emulates pressing Ctrl-C just while GDB is evaluating the breakpoint + # condition. + gdb_test \ + "break foo if \$hit_count\+\+ =3D=3D $num_hits || \$_shell(\"kill -SIGINT= $gdb_pid\") !=3D 0" \ + "Breakpoint .*" \ + "break foo if " + + # Number of times we've seen GDB print "Quit" followed by the + # prompt. We should see that exactly $NUM_HITS times. + set quit_count 0 + + gdb_test_multiple "c&" "SIGINT does not interrupt background execution= " { + -re "^c&\r\nContinuing\\.\r\n$::gdb_prompt " { + exp_continue + } + -re "^Quit\r\n$::gdb_prompt " { + incr quit_count + verbose -log "quit_count=3D$quit_count" + exp_continue + } + -re "^\r\nBreakpoint .*return 0;" { + gdb_assert {$quit_count =3D=3D $num_hits} $gdb_test_name + } + -re ".*Asynchronous execution not supported on this target\..*" { + unsupported "$gdb_test_name (asynchronous execution not supported)" + } + } +} + +test