* [PATCH] [gdb/server] Emit warning for SIGINT failure @ 2022-11-09 20:32 Tom de Vries 2022-11-26 13:34 ` [PING] " Tom de Vries 2022-11-26 20:29 ` Simon Marchi 0 siblings, 2 replies; 4+ messages in thread From: Tom de Vries @ 2022-11-09 20:32 UTC (permalink / raw) To: gdb-patches Consider the executable from test-case gdb.base/interrupt-daemon.exp. When starting it using gdbserver: ... $ ./build/gdbserver/gdbserver localhost:2345 \ ./outputs/gdb.base/interrupt-daemon/interrupt-daemon ... and connecting to it using gdb: ... $ gdb -q -ex "target remote localhost:2345" \ -ex "set follow-fork-mode child" \ -ex "break daemon_main" -ex cont ... we are setup to do the same as in the test-case: interrupt a running inferior using ^C. So let's try: ... (gdb) continue Continuing. ^C ... After pressing ^C, nothing happens. This a known problem, filed as PR remote/18772. The problem is that in linux_process_target::request_interrupt, a kill is used to send a SIGINT, but it fails. And it fails silently. Make the failure verbose by adding a warning, such that the gdbserver output becomes more helpful: ... Process interrupt-daemon created; pid = 15068 Listening on port 2345 Remote debugging from host ::1, port 35148 Detaching from process 15068 Detaching from process 15085 gdbserver: Sending SIGINT to process group of pid 15068 failed: \ No such process ... Note that the failure can easily be reproduced using the test-case and target board native-gdbserver: ... (gdb) continue^M Continuing.^M PASS: gdb.base/interrupt-daemon.exp: fg: continue ^CFAIL: gdb.base/interrupt-daemon.exp: fg: ctrl-c stops process (timeout) ... as reported in PR server/23382. Tested on x86_64-linux. --- gdbserver/linux-low.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index 0cbfeb99086..f4df02d3ac9 100644 --- a/gdbserver/linux-low.cc +++ b/gdbserver/linux-low.cc @@ -5467,7 +5467,10 @@ linux_process_target::request_interrupt () { /* Send a SIGINT to the process group. This acts just like the user typed a ^C on the controlling terminal. */ - ::kill (-signal_pid, SIGINT); + int res = ::kill (-signal_pid, SIGINT); + if (res == -1) + warning (_("Sending SIGINT to process group of pid %ld failed: %s"), + signal_pid, safe_strerror (errno)); } bool base-commit: 3abad2f6a637a0c56af70311d94ec66eea1b4852 -- 2.35.3 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PING] [PATCH] [gdb/server] Emit warning for SIGINT failure 2022-11-09 20:32 [PATCH] [gdb/server] Emit warning for SIGINT failure Tom de Vries @ 2022-11-26 13:34 ` Tom de Vries 2022-11-26 20:29 ` Simon Marchi 1 sibling, 0 replies; 4+ messages in thread From: Tom de Vries @ 2022-11-26 13:34 UTC (permalink / raw) To: gdb-patches On 11/9/22 21:32, Tom de Vries via Gdb-patches wrote: > Consider the executable from test-case gdb.base/interrupt-daemon.exp. > > When starting it using gdbserver: > ... > $ ./build/gdbserver/gdbserver localhost:2345 \ > ./outputs/gdb.base/interrupt-daemon/interrupt-daemon > ... > and connecting to it using gdb: > ... > $ gdb -q -ex "target remote localhost:2345" \ > -ex "set follow-fork-mode child" \ > -ex "break daemon_main" -ex cont > ... > we are setup to do the same as in the test-case: interrupt a running inferior > using ^C. > > So let's try: > ... > (gdb) continue > Continuing. > ^C > ... > After pressing ^C, nothing happens. This a known problem, filed as > PR remote/18772. > > The problem is that in linux_process_target::request_interrupt, a kill is used > to send a SIGINT, but it fails. And it fails silently. > > Make the failure verbose by adding a warning, such that the gdbserver output > becomes more helpful: > ... > Process interrupt-daemon created; pid = 15068 > Listening on port 2345 > Remote debugging from host ::1, port 35148 > Detaching from process 15068 > Detaching from process 15085 > gdbserver: Sending SIGINT to process group of pid 15068 failed: \ > No such process > ... > > Note that the failure can easily be reproduced using the test-case and target > board native-gdbserver: > ... > (gdb) continue^M > Continuing.^M > PASS: gdb.base/interrupt-daemon.exp: fg: continue > ^CFAIL: gdb.base/interrupt-daemon.exp: fg: ctrl-c stops process (timeout) > ... > as reported in PR server/23382. > > Tested on x86_64-linux. Ping. Thanks, - Tom > --- > gdbserver/linux-low.cc | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index 0cbfeb99086..f4df02d3ac9 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -5467,7 +5467,10 @@ linux_process_target::request_interrupt () > { > /* Send a SIGINT to the process group. This acts just like the user > typed a ^C on the controlling terminal. */ > - ::kill (-signal_pid, SIGINT); > + int res = ::kill (-signal_pid, SIGINT); > + if (res == -1) > + warning (_("Sending SIGINT to process group of pid %ld failed: %s"), > + signal_pid, safe_strerror (errno)); > } > > bool > > base-commit: 3abad2f6a637a0c56af70311d94ec66eea1b4852 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [gdb/server] Emit warning for SIGINT failure 2022-11-09 20:32 [PATCH] [gdb/server] Emit warning for SIGINT failure Tom de Vries 2022-11-26 13:34 ` [PING] " Tom de Vries @ 2022-11-26 20:29 ` Simon Marchi 2022-11-27 9:27 ` Tom de Vries 1 sibling, 1 reply; 4+ messages in thread From: Simon Marchi @ 2022-11-26 20:29 UTC (permalink / raw) To: Tom de Vries, gdb-patches On 11/9/22 15:32, Tom de Vries via Gdb-patches wrote: > Consider the executable from test-case gdb.base/interrupt-daemon.exp. > > When starting it using gdbserver: > ... > $ ./build/gdbserver/gdbserver localhost:2345 \ > ./outputs/gdb.base/interrupt-daemon/interrupt-daemon > ... > and connecting to it using gdb: > ... > $ gdb -q -ex "target remote localhost:2345" \ > -ex "set follow-fork-mode child" \ > -ex "break daemon_main" -ex cont > ... > we are setup to do the same as in the test-case: interrupt a running inferior > using ^C. > > So let's try: > ... > (gdb) continue > Continuing. > ^C > ... > After pressing ^C, nothing happens. This a known problem, filed as > PR remote/18772. > > The problem is that in linux_process_target::request_interrupt, a kill is used > to send a SIGINT, but it fails. And it fails silently. > > Make the failure verbose by adding a warning, such that the gdbserver output > becomes more helpful: > ... > Process interrupt-daemon created; pid = 15068 > Listening on port 2345 > Remote debugging from host ::1, port 35148 > Detaching from process 15068 > Detaching from process 15085 > gdbserver: Sending SIGINT to process group of pid 15068 failed: \ > No such process > ... > > Note that the failure can easily be reproduced using the test-case and target > board native-gdbserver: > ... > (gdb) continue^M > Continuing.^M > PASS: gdb.base/interrupt-daemon.exp: fg: continue > ^CFAIL: gdb.base/interrupt-daemon.exp: fg: ctrl-c stops process (timeout) > ... > as reported in PR server/23382. > > Tested on x86_64-linux. > --- > gdbserver/linux-low.cc | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index 0cbfeb99086..f4df02d3ac9 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -5467,7 +5467,10 @@ linux_process_target::request_interrupt () > { > /* Send a SIGINT to the process group. This acts just like the user > typed a ^C on the controlling terminal. */ > - ::kill (-signal_pid, SIGINT); > + int res = ::kill (-signal_pid, SIGINT); > + if (res == -1) > + warning (_("Sending SIGINT to process group of pid %ld failed: %s"), > + signal_pid, safe_strerror (errno)); I don't fully understand the issue about the separate process group and all that (and I just have a few minutes so I won't dig into that right now), but I don't see any downside to your patch. Therefore: Approved-By: Simon Marchi <simon.marchi@efficios.com> On a tangential note, at first glance, this use of the signal_pid global seems bogus. If you run two inferiors, signal_pid will have the pid of the second one. Then, if you only resume the first one and press ctrl-c, I guess we'll send a SIGINT to the second one, that is not resumed, and then wait for it to report a stop, which won't happen? Simon ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [gdb/server] Emit warning for SIGINT failure 2022-11-26 20:29 ` Simon Marchi @ 2022-11-27 9:27 ` Tom de Vries 0 siblings, 0 replies; 4+ messages in thread From: Tom de Vries @ 2022-11-27 9:27 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 11/26/22 21:29, Simon Marchi wrote: > > > On 11/9/22 15:32, Tom de Vries via Gdb-patches wrote: >> Consider the executable from test-case gdb.base/interrupt-daemon.exp. >> >> When starting it using gdbserver: >> ... >> $ ./build/gdbserver/gdbserver localhost:2345 \ >> ./outputs/gdb.base/interrupt-daemon/interrupt-daemon >> ... >> and connecting to it using gdb: >> ... >> $ gdb -q -ex "target remote localhost:2345" \ >> -ex "set follow-fork-mode child" \ >> -ex "break daemon_main" -ex cont >> ... >> we are setup to do the same as in the test-case: interrupt a running inferior >> using ^C. >> >> So let's try: >> ... >> (gdb) continue >> Continuing. >> ^C >> ... >> After pressing ^C, nothing happens. This a known problem, filed as >> PR remote/18772. >> >> The problem is that in linux_process_target::request_interrupt, a kill is used >> to send a SIGINT, but it fails. And it fails silently. >> >> Make the failure verbose by adding a warning, such that the gdbserver output >> becomes more helpful: >> ... >> Process interrupt-daemon created; pid = 15068 >> Listening on port 2345 >> Remote debugging from host ::1, port 35148 >> Detaching from process 15068 >> Detaching from process 15085 >> gdbserver: Sending SIGINT to process group of pid 15068 failed: \ >> No such process >> ... >> >> Note that the failure can easily be reproduced using the test-case and target >> board native-gdbserver: >> ... >> (gdb) continue^M >> Continuing.^M >> PASS: gdb.base/interrupt-daemon.exp: fg: continue >> ^CFAIL: gdb.base/interrupt-daemon.exp: fg: ctrl-c stops process (timeout) >> ... >> as reported in PR server/23382. >> >> Tested on x86_64-linux. >> --- >> gdbserver/linux-low.cc | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc >> index 0cbfeb99086..f4df02d3ac9 100644 >> --- a/gdbserver/linux-low.cc >> +++ b/gdbserver/linux-low.cc >> @@ -5467,7 +5467,10 @@ linux_process_target::request_interrupt () >> { >> /* Send a SIGINT to the process group. This acts just like the user >> typed a ^C on the controlling terminal. */ >> - ::kill (-signal_pid, SIGINT); >> + int res = ::kill (-signal_pid, SIGINT); >> + if (res == -1) >> + warning (_("Sending SIGINT to process group of pid %ld failed: %s"), >> + signal_pid, safe_strerror (errno)); > > I don't fully understand the issue about the separate process group and > all that (and I just have a few minutes so I won't dig into that right > now), but I don't see any downside to your patch. Therefore: > > Approved-By: Simon Marchi <simon.marchi@efficios.com> > Thanks for the review, much appreciated. > On a tangential note, at first glance, this use of the signal_pid global > seems bogus. If you run two inferiors, signal_pid will have the pid of > the second one. Then, if you only resume the first one and press > ctrl-c, I guess we'll send a SIGINT to the second one, that is not > resumed, and then wait for it to report a stop, which won't happen? Agreed, and this warning won't trigger for that scenario, since the signal_pid will be valid. I don't understand at this point whether the correct pid is somewhere available in gdbserver or needs to be passed by gdb, but unfortunately I don't see an opportunity to look into this further before the upcoming gdb 13 release (and the fact that this has remained unfixed for 7 years suggests to me that it's not a trivial fix). Thanks, - Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-27 9:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-09 20:32 [PATCH] [gdb/server] Emit warning for SIGINT failure Tom de Vries 2022-11-26 13:34 ` [PING] " Tom de Vries 2022-11-26 20:29 ` Simon Marchi 2022-11-27 9:27 ` Tom de Vries
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).