Re: [PING] Re: [PATCH v2 3/3] PR remote/19496, timeout in forking-threads-plus-bkpt

[Don Breazeal <donb at codesourcery dot com>]

Ping.
I checked, the patch still applies cleanly to mainline.
Thanks
--Don

On 2/25/2016 9:29 AM, Don Breazeal wrote:
> Ping
> Thanks,
> --Don
> 
> On 2/10/2016 4:28 PM, Don Breazeal wrote:
>> On 2/1/2016 12:09 PM, Pedro Alves wrote:
>>> On 02/01/2016 07:29 PM, Don Breazeal wrote:
>>>> On 2/1/2016 4:05 AM, Pedro Alves wrote:
>>>
>>>> Hi Pedro,
>> ---snip---
>>>> A fork event was reported to GDB before GDB knew about the parent thread,
>>>> followed immediately by a breakpoint event in a different thread.  The
>>>> parent thread was subsequently added via remote_notice_new_inferior in
>>>> process_stop_reply, but when the thread was added the thread_info.state
>>>> was set to THREAD_STOPPED.  The fork event was then handled correctly,
>>>> but when the fork parent was resumed via a call to keep_going, the state
>>>> was unchanged.
>>>
>>> Since this is non-stop, then it sounds to me like the bug is that the
>>> thread should have been added in THREAD_RUNNING state.
>>>
>>> Consider that infrun may be pulling target events out of the target_ops
>>> backend into its own event queue, but, not process them immediately.
>>>
>>> E.g., infrun may be stopping all threads temporarily for a step-over-breakpoint
>>> operation for thread A (stop_all_threads).  The waitstatus of all threads
>>> is thus left pending in the thread structure (save_status), including the
>>> fork event of thread B.  Right at this point, if the user
>>> does "info threads", that should show thread B (the fork parent) running,
>>> not stopped, even if internally, gdb is holding it paused for a little bit.
>>>
>>
>> Hi Pedro,
>> Here is a new patch that adds the threads with the state set to
>> THREAD_RUNNING for fork events.
>> Thanks!
>> --Don
>>
>> This patch addresses a failure in
>> gdb.threads/forking-threads-plus-breakpoint.exp:
>>
>> FAIL: gdb.threads/forking-threads-plus-breakpoint.exp: cond_bp_target=1:
>> detach_on_fork=on: inferior 1 exited (timeout)
>>
>> Cause:
>> A fork event was reported to GDB before GDB knew about the parent thread,
>> followed immediately by a breakpoint event in a different thread.  The
>> parent thread was subsequently added via remote_notice_new_inferior in
>> process_stop_reply, but when the thread was added the thread_info.state
>> was set to THREAD_STOPPED.  The fork event was then handled correctly,
>> but when the fork parent was resumed via a call to keep_going, the state
>> was unchanged.
>>
>> The breakpoint event was then handled, which caused all the non-breakpoint
>> threads to be stopped.  When the breakpoint thread was resumed, all the
>> non-breakpoint threads were resumed via infrun.c:restart_threads.  Our old
>> fork parent wasn't restarted, because it still had thread_info.state set to
>> THREAD_STOPPED.  Ultimately the program under debug hung waiting for a
>> pthread_join while the old fork parent was stopped forever by GDB.
>>
>> Fix:
>> Make sure to add the fork parent thread in the THREAD_RUNNING state by
>> calling remote_notice_new_inferior with RUNNING set to 1 when processing
>> a fork stop reply.
>>
>> Tested on x86_64 Linux and Nios II Linux target with x86 Linux host.
>>
>> gdb/ChangeLog:
>> 2016-02-10  Don Breazeal  <donb@codesourcery.com>
>>
>> 	* remote.c (process_stop_reply): Call remote_notice_new_inferior
>> 	with RUNNING set to 1 when handling fork events.
>>
>> ---
>>  gdb/remote.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index f09a06e..ab750a7 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -6818,7 +6818,14 @@ process_stop_reply (struct stop_reply *stop_reply,
>>  	  VEC_free (cached_reg_t, stop_reply->regcache);
>>  	}
>>  
>> -      remote_notice_new_inferior (ptid, 0);
>> +      /* If a fork event arrived before we knew about the parent thread,
>> +	 make sure to mark it as running when it is created.  */
>> +      if (status->kind == TARGET_WAITKIND_FORKED
>> +	  || status->kind == TARGET_WAITKIND_VFORKED)
>> +	remote_notice_new_inferior (ptid, 1);
>> +      else
>> +	remote_notice_new_inferior (ptid, 0);
>> +
>>        remote_thr = demand_private_info (ptid);
>>        remote_thr->core = stop_reply->core;
>>        remote_thr->stop_reason = stop_reply->stop_reason;
>>
>