RE: [PATCH v2] gdbserver: select a thread, if necessary, to access memory (was: [PATCH] gdbserver: try selecting a thread first to access memory)

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

Kindly pinging. 

Regards
-Baris

On Tuesday, August 1, 2023 3:21 PM, Aktemur, Tankut Baris wrote:
> 
> Since commit
> 
>   commit 7f8acedeebe295fc8cc1d11ed971cbfc1942618c
>   Author: Pedro Alves <pedro@palves.net>
>   Date:   Tue Apr 5 13:57:11 2022 +0100
> 
>     gdbserver: track current process as well as current thread
> 
> gdbserver switches to a process, rather than a thread, before
> processing a memory access request coming from the GDB side.  This
> switch sets current_thread to null.  Some memory accesses on certain
> targets, however, may require having a thread context.  Therefore,
> switch to the selected thread, if the target would require a thread
> context.
> ---
>  gdbserver/server.cc | 16 ++++++++++++++--
>  gdbserver/target.cc |  6 ++++++
>  gdbserver/target.h  |  7 +++++++
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbserver/server.cc b/gdbserver/server.cc
> index c57270175b4..44642a64215 100644
> --- a/gdbserver/server.cc
> +++ b/gdbserver/server.cc
> @@ -1072,7 +1072,13 @@ gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int
> len)
>        /* (assume no half-trace half-real blocks for now) */
>      }
> 
> -  if (set_desired_process ())
> +  /* Some memory accesses may require having a thread context.
> +     Attempt switching to the selected thread, if necessary.
> +     Otherwise use the process as the context.  */
> +  bool need_thread = the_target->memaddr_is_thread_specific (memaddr);
> +
> +  if ((need_thread && set_desired_thread ())
> +      || (!need_thread && set_desired_process ()))
>      res = read_inferior_memory (memaddr, myaddr, len);
>    else
>      res = 1;
> @@ -1093,7 +1099,13 @@ gdb_write_memory (CORE_ADDR memaddr, const unsigned char
> *myaddr, int len)
>      {
>        int ret;
> 
> -      if (set_desired_process ())
> +      /* Some memory accesses may require having a thread context.
> +	 Attempt switching to the selected thread, if necessary.
> +	 Otherwise use the process as the context.  */
> +      bool need_thread = the_target->memaddr_is_thread_specific (memaddr);
> +
> +      if ((need_thread && set_desired_thread ())
> +	  || (!need_thread && set_desired_process ()))
>  	ret = target_write_memory (memaddr, myaddr, len);
>        else
>  	ret = EIO;
> diff --git a/gdbserver/target.cc b/gdbserver/target.cc
> index f8e592d20c3..85efe6d88c6 100644
> --- a/gdbserver/target.cc
> +++ b/gdbserver/target.cc
> @@ -832,3 +832,9 @@ process_stratum_target::get_ipa_tdesc_idx ()
>  {
>    return 0;
>  }
> +
> +bool
> +process_stratum_target::memaddr_is_thread_specific (CORE_ADDR memaddr)
> +{
> +  return false;
> +}
> diff --git a/gdbserver/target.h b/gdbserver/target.h
> index d993e361b76..bde2fb2eb4c 100644
> --- a/gdbserver/target.h
> +++ b/gdbserver/target.h
> @@ -508,6 +508,13 @@ class process_stratum_target
>       Returns true if successful and false otherwise.  */
>    virtual bool store_memtags (CORE_ADDR address, size_t len,
>  			      const gdb::byte_vector &tags, int type);
> +
> +  /* Some memory addresses may require having a thread context.  E.g.:
> +     an address whose upper bits encode a thread-specific address
> +     space.  Let the target tell if MEMADDR is such an address, so
> +     that the server can attempt switching the context before
> +     accessing the memory.  */
> +  virtual bool memaddr_is_thread_specific (CORE_ADDR memaddr);
>  };
> 
>  extern process_stratum_target *the_target;
> --
> 2.34.1


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928