Re: [PATCH v2] Restore terminal state in mi_thread_exit (PR gdb/17627)

[Pedro Alves <palves@redhat.com>]

On 12/05/2014 04:12 PM, Simon Marchi wrote:
>> That doesn't follow though.  We have a ton of places that call
>> target_terminal_our that don't need to put back the terminal in
>> whatever state is was before.  The reason is that usually we'll
>> be printing after the inferior stopped for some event.  If we end
>> up re-resuming it, we'll call target_terminal_inferior then.
> 
> I am not sure I understand that part. The thread exit is noticed because
> of a breakpoint being hit (a breakpoint in libthread-db).

Nope, it's not.  When that breakpoint is hit, the only thing we do
is mark the thread as "dying", in linux-thread-db.c:detach_thread.

The thread exit is noticed when the kernel reports an exit status
to the waitpid calls in linux-nat.c (WIFEXITED/WIFSIGNALED)
(and few other corner cases in that same file).  Look for exit_lwp.
E.g.:

(top-gdb) bt
#0  delete_thread (ptid=...) at /home/pedro/gdb/mygit/src/gdb/thread.c:371
#1  0x00000000004b76ee in exit_lwp (lp=0x1682000) at /home/pedro/gdb/mygit/src/gdb/linux-nat.c:895
#2  0x00000000004bbb20 in linux_nat_filter_event (lwpid=19001, status=0, new_pending_p=0x7fffffffcf50) at /home/pedro/gdb/mygit/src/gdb/linux-nat.c:2847
#3  0x00000000004bc6d8 in linux_nat_wait_1 (ops=0xddf5e0, ptid=..., ourstatus=0x7fffffffd300, target_options=1) at /home/pedro/gdb/mygit/src/gdb/linux-nat.c:3136
#4  0x00000000004bd583 in linux_nat_wait (ops=0xddf5e0, ptid=..., ourstatus=0x7fffffffd300, target_options=1) at /home/pedro/gdb/mygit/src/gdb/linux-nat.c:3505
#5  0x00000000004c597b in thread_db_wait (ops=0xd4f0c0 <thread_db_ops>, ptid=..., ourstatus=0x7fffffffd300, options=1)
    at /home/pedro/gdb/mygit/src/gdb/linux-thread-db.c:1532
#6  0x000000000065c037 in delegate_wait (self=0xd4f0c0 <thread_db_ops>, arg1=..., arg2=0x7fffffffd300, arg3=1) at /home/pedro/gdb/mygit/src/gdb/target-delegates.c:116
#7  0x000000000066c465 in target_wait (ptid=..., status=0x7fffffffd300, options=1) at /home/pedro/gdb/mygit/src/gdb/target.c:2175
#8  0x00000000006165ff in fetch_inferior_event (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/infrun.c:3241
#9  0x0000000000639d57 in inferior_event_handler (event_type=INF_REG_EVENT, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/inf-loop.c:57

> In all-stop, I
> would expect that when the thread-exit breakpoint is hit, we stop all
> threads, then process the event (display the message). After that, we
> would need to resume the remaining threads, so the application continues
> executing. If so, we would call target_terminal_inferior there.

Nope, thread exits are actually handled inside linux-nat.c, without
stopping all threads.

(I've got a patch to stop native gdb from using the create and death
event breakpoints, like gdbserver, btw.  Not necessary when we have
PTRACE_EVENT_CLONE.)

> 
> But, I also remember you mentioning that these thread-db breakpoints are
> not handled the same way as regular breakpoints. If I inspect the
> target-stack when debugging a threaded program:
> 
>     The current target stack is:
>       - multi-thread (multi-threaded child process.)
>       - native (Native process)
>       - exec (Local exec file)
>       - None (None)
> 
> I would guess that the layer responsible for the all-stop behavior (stop
> all the threads when one is stopped) is the native process one.

Right.

> The
> thread-db breakpoint handling, however, is done in the multi-thread layer,
> so the native target is not reached for handling these. So when the
> message is printed, the other threads are actually still running.
> 
> Is this last explanation correct?
> 
>> But in the thread exit case, there's nothing to be re-resumed,
>> so we need to care about it "manually".  If you take a backtrace
>> at the point the thread exit event is printed, we should be
>> still deep within the target backend code.
>>
>> Please update the commit log to clarify this.
> 
> I would write this:
> 
> We need to manually restore the terminal setting in this particular observer.
> In the case of the other observers that call target_terminal_ours, gdb will end

"other MI observers"?

> up resuming the inferior later in the execution and call
> target_terminal_inferior. In the case of the thread exit event, we still need
> to call target_terminal_ours to be able to print something, but there is nothing
> that gdb will need to resume after that. We therefore need to call
> target_terminal_inferior ourselves.
> 
> is this correct?

Yes.

Thanks,
Pedro Alves