This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix new inferior events output (Re: [PATCH v7] Enable 'set print inferior-events' and improve detach/fork/kill/exit messages)
On Wednesday, April 25 2018, Pedro Alves wrote:
> Hi,
>
> -- On 04/24/2018 08:49 PM, Sergio Durigan Junior wrote:
>>> On 04/19/2018 08:53 PM, Sergio Durigan Junior wrote:
>>>> Changes from v6:
>>>
>>>> - Readjusted a bunch of tests in order to be more flexible when
>>>> matching the "process $decimal" or "Thread $decimal.$decimal"
>>>> strings.
>>>
>>> I think you meant "process $decimal" or "Remote target", because
>>> "Thread $decimal.$decimal" won't be used with pid-only ptids.
>>
>> I actually had to adjust a few regexps (in previous versions of the
>> patch) to accept "Thread $decimal.$decimal". But that's a different
>> problem than the pid-only ptids, and I don't include the "Changes from
>> v..." in the commit log.
> Hmm, now that I try current master I think I see what you are
> saying. But that shows an actual problem with the patch, we really
> shouldn't be seeing those "Thread $decimal.$decimal". See patch below.
Thanks for catching this problem. I don't think I have anything to add,
other than "sorry for not catching this earlier". For whatever it is
worth, looks good to me.
> From 4b17a2be78349e6912f68b924a5e3761035ee831 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 25 Apr 2018 17:28:25 +0100
> Subject: [PATCH] Fix new inferior events output
>
> Since f67c0c917150 ("Enable 'set print inferior-events' and improve
> detach/fork/kill/exit messages"), when detaching a remote process, we
> get, for detach against a remote target:
>
> (gdb) detach
> Detaching from program: ...., process 5388
> Ending remote debugging.
> [Inferior 1 (Thread 5388.5388) detached]
> ^^^^^^^^^^^^^^^^
>
> That is incorrect, for it is printing a thread id as string while we
> should be printing the process id instead. I.e., either one of:
>
> [Inferior 1 (process 5388) detached]
> [Inferior 1 (Remote target) detached]
>
> depending on remote stub support for the multi-process extensions.
>
>
> Similarly, after killing a process, we're printing of thread ids while
> we should be printing process ids. E.g., on native GNU/Linux:
>
> (gdb) k
> Kill the program being debugged? (y or n) y
> [Inferior 1 (Thread 0x7ffff7faa8c0 (LWP 30721)) has been killed]
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> while it should have been:
>
> Kill the program being debugged? (y or n) y
> [Inferior 1 (process 30721) has been killed]
> ^^^^^^^^^^^^^
>
> There's a wording inconsistency between detach and kill:
>
> [Inferior 1 (process 30721) has been killed]
> [Inferior 1 (process 30721) detached]
>
> Given we were already saying "detached" instead of "has been
> detached", and we used to say just "exited", and given that the "has
> been" doesn't really add any information, this commit changes the the
> message to just "killed":
>
> [Inferior 1 (process 30721) killed]
>
> gdb/ChangeLog:
> yyyy-mm-dd Pedro Alves <palves@redhat.com>
>
> * infcmd.c (kill_command): Print the pid as string, not the whole
> thread's ptid. Add comment. s/has been killed/killed/ in output
> message.
> * remote.c (remote_detach_1): Print the pid as string, not the
> whole thread's ptid.
>
> gdb/testsuite/ChangeLog:
> yyyy-mm-dd Pedro Alves <palves@redhat.com>
>
> * gdb.base/hook-stop.exp: Expect "killed" instead of "has been
> killed".
> * gdb.base/kill-after-signal.exp: Likewise.
> * gdb.threads/kill.exp: Likewise.
> ---
> gdb/infcmd.c | 7 +++++--
> gdb/remote.c | 4 +++-
> gdb/testsuite/gdb.base/hook-stop.exp | 2 +-
> gdb/testsuite/gdb.base/kill-after-signal.exp | 2 +-
> gdb/testsuite/gdb.threads/kill.exp | 2 +-
> 5 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 6c9f885badd..21772b6cff0 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -2596,13 +2596,16 @@ kill_command (const char *arg, int from_tty)
> if (!query (_("Kill the program being debugged? ")))
> error (_("Not confirmed."));
>
> - std::string pid_str = target_pid_to_str (inferior_ptid);
> + int pid = current_inferior ()->pid;
> + /* Save the pid as a string before killing the inferior, since that
> + may unpush the current target, and we need the string after. */
> + std::string pid_str = target_pid_to_str (pid_to_ptid (pid));
> int infnum = current_inferior ()->num;
>
> target_kill ();
>
> if (print_inferior_events)
> - printf_unfiltered (_("[Inferior %d (%s) has been killed]\n"),
> + printf_unfiltered (_("[Inferior %d (%s) killed]\n"),
> infnum, pid_str.c_str ());
>
> /* If we still have other inferiors to debug, then don't mess with
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 61d1dcb5738..193037b6e7a 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -5138,7 +5138,9 @@ remote_detach_1 (int from_tty, inferior *inf)
> breakpoints that should be available for the followed inferior. */
> if (!is_fork_parent)
> {
> - std::string infpid = target_pid_to_str (inferior_ptid);
> + /* Save the pid as a string before mourning, since that will
> + unpush the remote target, and we need the string after. */
> + std::string infpid = target_pid_to_str (pid_to_ptid (pid));
>
> target_mourn_inferior (inferior_ptid);
> if (print_inferior_events)
> diff --git a/gdb/testsuite/gdb.base/hook-stop.exp b/gdb/testsuite/gdb.base/hook-stop.exp
> index 5717f94fc2a..b3f4084af1e 100644
> --- a/gdb/testsuite/gdb.base/hook-stop.exp
> +++ b/gdb/testsuite/gdb.base/hook-stop.exp
> @@ -86,7 +86,7 @@ proc hook_stop_kill {} {
>
> set test "run hook-stop"
> gdb_test_multiple "continue" "$test" {
> - -re "Continuing.\r\n\\\[Inferior $decimal \\(.*\\) has been killed\\\]\r\n${gdb_prompt} $" {
> + -re "Continuing.\r\n\\\[Inferior $decimal \\(.*\\) killed\\\]\r\n${gdb_prompt} $" {
> pass $test
> }
> }
> diff --git a/gdb/testsuite/gdb.base/kill-after-signal.exp b/gdb/testsuite/gdb.base/kill-after-signal.exp
> index 67e6f627504..8830d492383 100644
> --- a/gdb/testsuite/gdb.base/kill-after-signal.exp
> +++ b/gdb/testsuite/gdb.base/kill-after-signal.exp
> @@ -39,6 +39,6 @@ gdb_test "continue" "Program received signal SIGUSR1, .*"
> gdb_test "stepi" "\r\nhandler .*"
> gdb_test_multiple "kill" "kill" {
> -re "Kill the program being debugged\\? \\(y or n\\) $" {
> - gdb_test "y" "\\\[Inferior $decimal \\(.*\\) has been killed\\\]" "kill"
> + gdb_test "y" "\\\[Inferior $decimal \\(.*\\) killed\\\]" "kill"
> }
> }
> diff --git a/gdb/testsuite/gdb.threads/kill.exp b/gdb/testsuite/gdb.threads/kill.exp
> index aea4a986c64..82f221eef67 100644
> --- a/gdb/testsuite/gdb.threads/kill.exp
> +++ b/gdb/testsuite/gdb.threads/kill.exp
> @@ -70,7 +70,7 @@ proc test {threaded} {
>
> gdb_test_multiple "kill" "kill" {
> -re "Kill the program being debugged\\? \\(y or n\\) $" {
> - gdb_test "y" "\\\[Inferior $decimal \\(.*\\) has been killed\\\]" "kill"
> + gdb_test "y" "\\\[Inferior $decimal \\(.*\\) killed\\\]" "kill"
> }
> }
> }
> --
> 2.14.3
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/