Re: [PATCH v2 6/7] Add thread_handle_to_thread_info support for remote targets

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

On 2017-04-09 02:07, Kevin Buettner wrote:
> This patch adds support to remote targets for converting a thread
> handle to a thread_info struct pointer.
> 
> A thread handle is fetched via a "handle" attribute which has been
> added to the qXfer:threads:read query packet.  An implementation is
> provided in gdbserver for targets using the Linux kernel.
> 
> gdb/gdbserver/ChangeLog:
> 
>     	* linux-low.h (struct lwp_info): Add new field, thread_handle.
>     	(thread_db_thread_handle): Declare.
>     	* linux-low.c (linux_target_ops): Initialize thread_handle.
>     	* server.c (handle_qxfer_threads_worker): Add support for
>     	"handle" attribute.
>     	* target.h (struct target_ops): Add new function pointer,
>     	thread_handle.
>     	(target_thread_handle): Define.
>     	* thread-db.c (find_one_thread, attach_thread): Set thread_handle
>     	field in lwp.
>     	(thread_db_thread_handle): New function.
> 
> gdb/ChangeLog:
> 
>     	* remote.c (vector): Include.
>     	(struct private_thread_info): Add field, thread_handle.
>     	(free_private_thread_info): Deallocate storage associated with
>     	thread handle.
>     	(get_private_info_thread): Initialize `thread_handle' field.
>     	(struct thread_item): Add field, thread_handle.
>     	(clear_threads_listing_context): Deallocate storage associated
>     	with thread handle.
>     	(start_thread): Add support for "handle" attribute.
>     	(thread_attributes): Add "handle".
>     	(remote_update_thread_list): Update thread_handle.
>     	(remote_thread_handle_to_thread_info): New function.
>     	(init_remote_ops): Initialize to_thread_handle_to_thread_info.

That looks good to me.  I was a bit thrown off by the std::vector 
pointers in the remote structures, but I think it's a good trade-off 
until we make the structures apt to have non trivial fields.

> diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> index 3cc2bc4..db8b8d7 100644
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -474,6 +474,11 @@ struct target_ops
> 
>    /* Return tdesc index for IPA.  */
>    int (*get_ipa_tdesc_idx) (void);
> +
> +  /* Thread ID to (numeric) thread handle: Return a non-zero status on
> +     success, 0 for failure.  Return pointer to thread handle via 
> HANDLE
> +     and the handle's length via HANDLE_LEN.  */
> +  int (*thread_handle) (ptid_t ptid, gdb_byte **handle, int 
> *handle_len);

Return a bool?

> @@ -13505,6 +13533,29 @@ remote_execution_direction (struct target_ops 
> *self)
>    return rs->last_resume_exec_dir;
>  }
> 
> +/* Return pointer to the thread_info struct which corresponds to
> +   THREAD_HANDLE (having length HANDLE_LEN).  */
> +static struct thread_info *
> +remote_thread_handle_to_thread_info (struct target_ops *ops,
> +				     const gdb_byte *thread_handle,
> +				     int handle_len)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());

byte_order is unused.

> +  struct thread_info *tp;
> +
> +  ALL_NON_EXITED_THREADS (tp)
> +    {
> +      struct private_thread_info *priv = get_private_info_thread (tp);
> +
> +      if (priv != NULL && handle_len == priv->thread_handle->size ()
> +          && memcmp (thread_handle, priv->thread_handle->data (),
> +	             handle_len) == 0)
> +        return tp;
> +    }
> +
> +  return NULL;
> +}
> +

Thanks,

Simon