This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v6] Enable 'set print inferior-events' and improve detach/fork/kill/exit messages
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Date: Tue, 17 Apr 2018 16:07:49 -0400
- Subject: Re: [PATCH v6] Enable 'set print inferior-events' and improve detach/fork/kill/exit messages
- References: <20180124194714.26222-1-sergiodj@redhat.com> <20180416200444.30090-1-sergiodj@redhat.com> <ea2c0a62-a250-5120-6aeb-58bc1b5a646b@redhat.com>
On Tuesday, April 17 2018, Pedro Alves wrote:
> On 04/16/2018 09:04 PM, Sergio Durigan Junior wrote:
>> Changes from v5:
>>
>> - Save result from 'target_pid_to_str' using std::string instead of
>> 'const char *', avoiding a possible race condition.
>
> Sorry Sergio, we're getting there, but this will need more work.
I'm the one who apologizes.
>> Built and regtested on BuildBot, without regressions.
>>
>
>> * gdb.threads/process-dies-while-detaching.c
>> (parent_function): Use 'usleep' in order to avoid
>> race-conditions.
>
> This is stale. There's no 'usleep' any more.
Removed.
>> @@ -121,9 +120,11 @@ struct inferior *
>> add_inferior (int pid)
>> {
>> struct inferior *inf = add_inferior_silent (pid);
>> + std::string infpid = target_pid_to_str (pid_to_ptid (pid));
>>
>> if (print_inferior_events)
>> - printf_unfiltered (_("[New inferior %d]\n"), pid);
>> + printf_unfiltered (_("[New inferior %d (%s)]\n"),
>> + inf->num, infpid.c_str ());
>
> Let's call target_pid_to_str directly here without the
> deep copy.
>
> You needed to call target_pid_to_str early in kill_command
> because there first kill the target, and print the
> string _afterwards_. "target_kill" may pop the target from the
> target stack, so we can't use target_pid_to_str after the
> target is gone already.
>
> But here, we're not messing with the target stack.
>
> (Nor are we calling target_pid_to_str twice in a row in
> the same expression.)
I confess I knew this, but decided to do a deep copy anyway because I
was aiming for consistency. But no problem at all, I'll simplify this
part.
>
>> void
>> @@ -261,12 +259,13 @@ void
>> detach_inferior (inferior *inf)
>> {
>> /* Save the pid, since exit_inferior_1 will reset it. */
>> - int pid = inf->pid;
>> + std::string infpid = target_pid_to_str (pid_to_ptid (inf->pid));
>>
>> exit_inferior_1 (inf, 0);
>>
>> if (print_inferior_events)
>> - printf_unfiltered (_("[Inferior %d detached]\n"), pid);
>> + printf_unfiltered (_("[Inferior %d (%s) detached]\n"),
>> + inf->num, infpid.c_str ());
>
> Same here.
Fixed.
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index f54a38833b..be118c5d97 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -5137,7 +5137,12 @@ remote_detach_1 (int from_tty, inferior *inf)
>> /* If doing detach-on-fork, we don't mourn, because that will delete
>> breakpoints that should be available for the followed inferior. */
>> if (!is_fork_parent)
>> - target_mourn_inferior (inferior_ptid);
>> + {
>> + target_mourn_inferior (inferior_ptid);
>> + if (print_inferior_events)
>> + printf_unfiltered (_("[Inferior %d (process %d) detached]\n"),
>> + inf->num, pid);
>
> This is still using "(process %d)", which is incorrect
> for target remote.
You're right, sorry about this.
> Also, here, if debugging in "target remote" mode, and this is
> the last live process, that target_mourn_inferior call will
> unpush the remote target from the target stack. So this is another
> case where you need to call target_pid_to_str _before_ calling
> target_mourn_inferior.
Good point. This is fixed.
> As mentioned before, not all targets print "process $decimal" in
> their target_pid_to_str implementation. So your testsuite changes
> will need to adapt too.
>
> Please try this: hack away gdbserver/server.c's support for
> the RSP multi-process extensions (look for "multiprocess+"), and
> test with "target remote". That hack will emulate older gdbservers and
> other stubs that don't support telling gdb the process's pid.
> Manually confirm that you get "Remote target" instead of
> "process $pid" -- see remote.c's implementation of
> target_pid_to_str. Then run the testsuite against that gdbserver.
[ After a quick chat to clarify things... ]
I will make sure to adjust all regexps and take into account the many
outputs that can happen here.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/