diff mbox

Fix mi-detach.exp on native-gdbserver

Message ID 1441229078-22073-1-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Sept. 2, 2015, 9:24 p.m. UTC 
  The =thread-exited event did not appear when detaching from a process
with "target remote".

When mourning an inferior, target remote takes a different path than
target extended_remote.  target remote calls unpush_target first which
calls remote_close in order to close the "remote" target.  remote_close
calls discard_all_inferiors, which exits all inferiors with
exit_inferior_silent.  Because it's the _silent version, we don't see
the MI event.

extended_remote_mourn calls generic_mourn_inferior, which calls
exit_inferior.  Since it's the non-silent version, we see the MI event.

When changing discard_all_inferiors to call exit_inferior instead of
exit_inferior_silent, the MI event appears.  Since remote_mourn is the
only place where discard_all_inferiors is used, it should be an otherwise
harmless change.

Regression-tested on Ubuntu 14.04, with native and native-gdbserver.

gdb/ChangeLog:

	* inferior.c (discard_all_inferiors): Call exit_inferior instead
	of exit_inferior_silent.
---
 gdb/inferior.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Pedro Alves Sept. 9, 2015, 9:39 a.m. UTC | #1 
On 09/02/2015 10:24 PM, Simon Marchi wrote:
> The =thread-exited event did not appear when detaching from a process
> with "target remote".
> 
> When mourning an inferior, target remote takes a different path than
> target extended_remote.  target remote calls unpush_target first which
> calls remote_close in order to close the "remote" target.  remote_close
> calls discard_all_inferiors, which exits all inferiors with
> exit_inferior_silent.  Because it's the _silent version, we don't see
> the MI event.
> 
> extended_remote_mourn calls generic_mourn_inferior, which calls
> exit_inferior.  Since it's the non-silent version, we see the MI event.

Please try enabling "set print inferior-events on".  With native
debugging, we currently get:

 (gdb) detach
 Detaching from program: /home/pedro/gdb/tests/main, process 15759
 [Inferior 15759 detached]
                 ^^^^^^^^

However, with target extended-remote, we get:

 (gdb) tar extended-remote :9999
 (gdb) detach
 Detaching from program: /home/pedro/gdb/tests/main, process 15789
 [Inferior 15789 exited]
                 ^^^^^^

And with target remote, we get nothing:

 (gdb) tar remote :9999
 0x0000003615a011f0 in _start () from target:/lib64/ld-linux-x86-64.so.2
 (gdb) detach
 Detaching from program: /home/pedro/gdb/tests/main, process 15767
 Ending remote debugging.
 (gdb)

With your patch, we'll print "exited" in the remote case too,
when it seems to me that in both extended-remote and remote
we should be saying "detached" like in the native case.

Also, if the user does "disconnect", after your patch gdb will
say the inferiors we were debugging "exited" while they were
left running on the target side.  Is that confusing?

Thanks,
Pedro Alves
Simon Marchi Sept. 11, 2015, 6:11 p.m. UTC | #2 
On 15-09-09 05:39 AM, Pedro Alves wrote:
> Please try enabling "set print inferior-events on".  With native
> debugging, we currently get:
> 
>  (gdb) detach
>  Detaching from program: /home/pedro/gdb/tests/main, process 15759
>  [Inferior 15759 detached]
>                  ^^^^^^^^
> 
> However, with target extended-remote, we get:
> 
>  (gdb) tar extended-remote :9999
>  (gdb) detach
>  Detaching from program: /home/pedro/gdb/tests/main, process 15789
>  [Inferior 15789 exited]
>                  ^^^^^^
> 
> And with target remote, we get nothing:
> 
>  (gdb) tar remote :9999
>  0x0000003615a011f0 in _start () from target:/lib64/ld-linux-x86-64.so.2
>  (gdb) detach
>  Detaching from program: /home/pedro/gdb/tests/main, process 15767
>  Ending remote debugging.
>  (gdb)
> 
> With your patch, we'll print "exited" in the remote case too,
> when it seems to me that in both extended-remote and remote
> we should be saying "detached" like in the native case.
> 
> Also, if the user does "disconnect", after your patch gdb will
> say the inferiors we were debugging "exited" while they were
> left running on the target side.  Is that confusing?
> 
> Thanks,
> Pedro Alves

The fact that different targets have different behaviors is certainly confusing
and I think it should be improved.

The problem seems to be that remote_mourn and extended_remote_mourn don't have
enough information as to why they are mourning an inferior, so they just end up
calling exit_inferior.  In the native case, the message is right because
inf_ptrace_detach calls directly detach_inferior (which is exactly like
exit_inferior, except the message).  By the way, it means that when detaching
natively, we never mourn the inferior, is it normal?.

According to these backtraces, it seems like it would be possible for the "mourn"
functions to take a "why are we mourning" argument, so that the message printed there
can be adjusted.

#0  remote_mourn (target=0x2136200 <remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/remote.c:8502
#1  0x00000000007fb350 in delegate_mourn_inferior (self=0x2136200 <remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:1285
#2  0x0000000000808b3e in target_mourn_inferior () at /home/emaisin/src/binutils-gdb/gdb/target.c:2317
#3  0x000000000064b2f2 in remote_detach_1 (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4723
#4  0x000000000064b353 in remote_detach (ops=0x2136200 <remote_ops>, args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4734
#5  0x00000000007f747d in delegate_detach (self=0x2136200 <remote_ops>, arg1=0x0, arg2=1) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:34
#6  0x0000000000808888 in target_detach (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/target.c:2216
#7  0x00000000007a4677 in detach_command (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/infcmd.c:2908

#0  extended_remote_mourn (target=0x2136780 <extended_remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/remote.c:8511
#1  0x00000000007fb2a8 in delegate_mourn_inferior (self=0x2136780 <extended_remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:1285
#2  0x0000000000808a7f in target_mourn_inferior () at /home/emaisin/src/binutils-gdb/gdb/target.c:2317
#3  0x000000000064b27b in remote_detach_1 (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4723
#4  0x000000000064b302 in extended_remote_detach (ops=0x2136780 <extended_remote_ops>, args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4740
#5  0x00000000007f73dd in delegate_detach (self=0x2136780 <extended_remote_ops>, arg1=0x0, arg2=1) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:34
#6  0x00000000008087d8 in target_detach (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/target.c:2216
#7  0x00000000007a45e1 in detach_command (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/infcmd.c:2908

Does it make sense?
Pedro Alves Sept. 22, 2015, 5:06 p.m. UTC | #3 
On 09/11/2015 07:11 PM, Simon Marchi wrote:
> On 15-09-09 05:39 AM, Pedro Alves wrote:
>> Please try enabling "set print inferior-events on".  With native
>> debugging, we currently get:
>>
>>  (gdb) detach
>>  Detaching from program: /home/pedro/gdb/tests/main, process 15759
>>  [Inferior 15759 detached]
>>                  ^^^^^^^^
>>
>> However, with target extended-remote, we get:
>>
>>  (gdb) tar extended-remote :9999
>>  (gdb) detach
>>  Detaching from program: /home/pedro/gdb/tests/main, process 15789
>>  [Inferior 15789 exited]
>>                  ^^^^^^
>>
>> And with target remote, we get nothing:
>>
>>  (gdb) tar remote :9999
>>  0x0000003615a011f0 in _start () from target:/lib64/ld-linux-x86-64.so.2
>>  (gdb) detach
>>  Detaching from program: /home/pedro/gdb/tests/main, process 15767
>>  Ending remote debugging.
>>  (gdb)
>>
>> With your patch, we'll print "exited" in the remote case too,
>> when it seems to me that in both extended-remote and remote
>> we should be saying "detached" like in the native case.
>>
>> Also, if the user does "disconnect", after your patch gdb will
>> say the inferiors we were debugging "exited" while they were
>> left running on the target side.  Is that confusing?
>>
>> Thanks,
>> Pedro Alves
> 
> The fact that different targets have different behaviors is certainly confusing
> and I think it should be improved.
> 
> The problem seems to be that remote_mourn and extended_remote_mourn don't have
> enough information as to why they are mourning an inferior, so they just end up
> calling exit_inferior.  In the native case, the message is right because
> inf_ptrace_detach calls directly detach_inferior (which is exactly like
> exit_inferior, except the message).  

remote_detach_1 also calls detach_inferior, though the condition
there looks a bit confusing.

> By the way, it means that when detaching
> natively, we never mourn the inferior, is it normal?.

I think most targets _don't_ mourn when detaching.  Maybe we could/should
make them all do so, not sure.  E.g., looking at generic_mourn_inferior, we
shouldn't need to mark breakpoints out when detaching, as we better have
really removed breakpoints from the inferior before we detached it.
Maybe we should instead have a generic_detach_inferior function.  That
also avoids the "why are we mourning the living?" question too.  :-)

Thanks,
Pedro Alves

> 
> According to these backtraces, it seems like it would be possible for the "mourn"
> functions to take a "why are we mourning" argument, so that the message printed there
> can be adjusted.
> 
> #0  remote_mourn (target=0x2136200 <remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/remote.c:8502
> #1  0x00000000007fb350 in delegate_mourn_inferior (self=0x2136200 <remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:1285
> #2  0x0000000000808b3e in target_mourn_inferior () at /home/emaisin/src/binutils-gdb/gdb/target.c:2317
> #3  0x000000000064b2f2 in remote_detach_1 (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4723
> #4  0x000000000064b353 in remote_detach (ops=0x2136200 <remote_ops>, args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4734
> #5  0x00000000007f747d in delegate_detach (self=0x2136200 <remote_ops>, arg1=0x0, arg2=1) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:34
> #6  0x0000000000808888 in target_detach (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/target.c:2216
> #7  0x00000000007a4677 in detach_command (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/infcmd.c:2908
> 
> #0  extended_remote_mourn (target=0x2136780 <extended_remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/remote.c:8511
> #1  0x00000000007fb2a8 in delegate_mourn_inferior (self=0x2136780 <extended_remote_ops>) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:1285
> #2  0x0000000000808a7f in target_mourn_inferior () at /home/emaisin/src/binutils-gdb/gdb/target.c:2317
> #3  0x000000000064b27b in remote_detach_1 (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4723
> #4  0x000000000064b302 in extended_remote_detach (ops=0x2136780 <extended_remote_ops>, args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/remote.c:4740
> #5  0x00000000007f73dd in delegate_detach (self=0x2136780 <extended_remote_ops>, arg1=0x0, arg2=1) at /home/emaisin/src/binutils-gdb/gdb/target-delegates.c:34
> #6  0x00000000008087d8 in target_detach (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/target.c:2216
> #7  0x00000000007a45e1 in detach_command (args=0x0, from_tty=1) at /home/emaisin/src/binutils-gdb/gdb/infcmd.c:2908
> 
> Does it make sense?
> 

Seems like one of those "have to try to tell" cases.
diff mbox

Patch

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 04e9a28..6ca18b8 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -315,7 +315,7 @@  discard_all_inferiors (void)
   for (inf = inferior_list; inf; inf = inf->next)
     {
       if (inf->pid != 0)
-	exit_inferior_silent (inf->pid);
+	exit_inferior (inf->pid);
     }
 }