Re: [PATCH v2 5/7] Add thread_db_notice_clone to gdbserver

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-04-09 02:07, Kevin Buettner wrote:
> While working on a patch for fetching a thread handle in gdbserver, I
> ran into a circumstance in which tests in gdb.mi/mi-nsmoribund.exp
> would occasionally fail.  Over a large enough number of runs, it would
> fail roughly 2% of the time.
> 
> That thread handle patch caused find_one_thread() to be called on
> every stop.  find_one_thread() calls td_ta_map_lwp2thr() which, in
> turn, can cause ps_get_thread_area() to be called.
> ps_get_thread_area() makes a call to ptrace() for getting the thread
> area address.  If this should happen when the thread is not stopped,
> the call to ptrace will return error which in turn propogates back to
> find_one_thread().  find_one_thread() calls error() in this instance
> which causes the program to die.
> 
> This patch causes find_one_thread() to be called upon reciept of a
> clone event.  Since the clone is stopped, the circumstances described
> above cannot occur.

This patch looks good to me, although I'm not very knowledgeable in the 
thread handling area.

Just one comment below.

> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index e27cbf8..6f06eb8 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -653,6 +653,8 @@ handle_extended_wait (struct lwp_info
> **orig_event_lwp, int wstat)
>  	  new_lwp->status_pending = status;
>  	}
> 
> +      thread_db_notice_clone (get_thread_process (event_thr), ptid);
> +
>        /* Don't report the event.  */
>        return 1;
>      }
> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index 7dcbfcf..87ce53a 100644
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -409,5 +409,6 @@ int thread_db_handle_monitor_command (char *);
>  int thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR 
> offset,
>  			       CORE_ADDR load_module, CORE_ADDR *address);
>  int thread_db_look_up_one_symbol (const char *name, CORE_ADDR *addrp);
> +void thread_db_notice_clone (struct process_info *proc, ptid_t lwp);

Could you add a comment to document this function?

Thanks,

Simon