Re: [PATCH 05/11] Internal TLS support for aarch64, x86_64, riscv, ppc64, and s390x

[Luis Machado <luis.machado@arm.com>]

Thanks for the patch.

On 10/10/24 03:16, Kevin Buettner wrote:
> For each architecture, aarch64, x86_64, riscv, ppc64, and s390x,
> this commit defines a suitable 'get_tls_dtv_addr' method and,
> when necessary, a 'get_tls_dtp_offset' method.
> 
> It also registers linux_get_thread_local_address, defined in
> linux-tdep.c (in an earlier commit), as the get_thread_local_address
> gdbarch method.  It also registers its architecture specific code
> using linux_register_tls_methods().
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24548
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31563
> ---
>  gdb/aarch64-linux-tdep.c | 53 +++++++++++++++++++++++++++
>  gdb/amd64-linux-tdep.c   | 37 +++++++++++++++++++
>  gdb/ppc-linux-tdep.c     | 62 ++++++++++++++++++++++++++++++++
>  gdb/riscv-linux-tdep.c   | 78 ++++++++++++++++++++++++++++++++++++++++
>  gdb/s390-linux-tdep.c    | 43 ++++++++++++++++++++++
>  5 files changed, 273 insertions(+)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index c608a84bc71..b03c2162a1a 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -35,6 +35,7 @@
>  #include "target/target.h"
>  #include "expop.h"
>  #include "auxv.h"
> +#include "inferior.h"
>  
>  #include "regcache.h"
>  #include "regset.h"
> @@ -2693,6 +2694,55 @@ aarch64_use_target_description_from_corefile_notes (gdbarch *gdbarch,
>    return true;
>  }
>  
> +/* Fetch and return the TLS DTV (dynamic thread vector) address for PTID.
> +   Throw a suitable TLS error if something goes wrong.  */
> +
> +static CORE_ADDR
> +aarch64_linux_get_tls_dtv_addr (struct gdbarch *gdbarch, ptid_t ptid,
> +				linux_libc libc)
> +{
> +  /* On aarch64, the thread pointer is found in the tpidr register.  */
> +  regcache *regcache
> +    = get_thread_arch_regcache (current_inferior (), ptid, gdbarch);
> +  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +  target_fetch_registers (regcache, tdep->tls_regnum_base);
> +  ULONGEST thr_ptr;
> +  if (regcache->cooked_read (tdep->tls_regnum_base, &thr_ptr) != REG_VALID)

Since we're relying on TPIDR now, might be worth mentioning it is currently the
first register in the TLS feature. Just in case things change later on?

Otherwise the AArch64 bits look OK to me.

Reviewed-By: Luis Machado <luis.machado@arm.com>